mbox series

[SRU,F:linux-bluefield,v1,0/1] UBUNTU: SAUCE: tmfifo: Fix a memory barrier issue

Message ID cover.1620233587.git.limings@nvidia.com
Headers show
Series UBUNTU: SAUCE: tmfifo: Fix a memory barrier issue | expand

Message

Liming Sun May 5, 2021, 4:58 p.m. UTC 
BugLink: https://bugs.launchpad.net/bugs/1927262

SRU Justification:

[Impact]

* The virtio framework uses wmb() when updating avail->idx. It guarantees
  the write order, but not necessarily loading order for the code accessing
  the memory. So potentially it could cause traffic stuck which has been
  observed in the field.

[Fix]
* This commit adds a load barrier after reading the avail->idx to make sure
  all the data in the descriptor is visible. It also adds a barrier when
  returning the packet to virtio framework to make sure read/writes are
  visible to the virtio code.

[Test Case]
* Just normal test. This change doesn't affect any functionality.

[Regression Potential]

* This version of the driver was tested by QA/verification for a while so no
  known regression at the moment.

Comments

Kelsey Skunberg May 6, 2021, 4:56 a.m. UTC | #1 
Hi Liming,

Please also send v2 as its own thread. Same response as I left on your
other patch set "UBUNTU: SAUCE: platform/mellanox: Add ctrl message and 
MAC configuration".

Thank you!

-Kelsey

On 2021-05-05 12:58:27 , Liming Sun wrote:
> BugLink: https://bugs.launchpad.net/bugs/1927262
> 
> SRU Justification:
> 
> [Impact]
> 
> * The virtio framework uses wmb() when updating avail->idx. It guarantees
>   the write order, but not necessarily loading order for the code accessing
>   the memory. So potentially it could cause traffic stuck which has been
>   observed in the field.
> 
> [Fix]
> * This commit adds a load barrier after reading the avail->idx to make sure
>   all the data in the descriptor is visible. It also adds a barrier when
>   returning the packet to virtio framework to make sure read/writes are
>   visible to the virtio code.
> 
> [Test Case]
> * Just normal test. This change doesn't affect any functionality.
> 
> [Regression Potential]
> 
> * This version of the driver was tested by QA/verification for a while so no
>   known regression at the moment.
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
Liming Sun May 6, 2021, 12:31 p.m. UTC | #2 
Thanks for the comments! I resent patch v3 in its own thread.

- Liming

> -----Original Message-----
> From: Kelsey Skunberg <kelsey.skunberg@canonical.com>
> Sent: Thursday, May 6, 2021 12:57 AM
> To: Liming Sun <limings@nvidia.com>
> Cc: kernel-team@lists.ubuntu.com
> Subject: NACK/CMT: [SRU][F:linux-bluefield][PATCH v1 0/1] UBUNTU:
> SAUCE: tmfifo: Fix a memory barrier issue
> 
> Hi Liming,
> 
> Please also send v2 as its own thread. Same response as I left on your
> other patch set "UBUNTU: SAUCE: platform/mellanox: Add ctrl message and
> MAC configuration".
> 
> Thank you!
> 
> -Kelsey
> 
> On 2021-05-05 12:58:27 , Liming Sun wrote:
> > BugLink:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs
> .launchpad.net%2Fbugs%2F1927262&amp;data=04%7C01%7Climings%40nvid
> ia.com%7C9301bd3977774c0402b108d9104b62fd%7C43083d15727340c1b7db3
> 9efd9ccc17a%7C0%7C0%7C637558738235771349%7CUnknown%7CTWFpbGZs
> b3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn
> 0%3D%7C1000&amp;sdata=Zny7l5%2F88zWYPrPWJgKT0wGqMc8c%2BfmV%
> 2Br6GhrGX0vk%3D&amp;reserved=0
> >
> > SRU Justification:
> >
> > [Impact]
> >
> > * The virtio framework uses wmb() when updating avail->idx. It guarantees
> >   the write order, but not necessarily loading order for the code accessing
> >   the memory. So potentially it could cause traffic stuck which has been
> >   observed in the field.
> >
> > [Fix]
> > * This commit adds a load barrier after reading the avail->idx to make sure
> >   all the data in the descriptor is visible. It also adds a barrier when
> >   returning the packet to virtio framework to make sure read/writes are
> >   visible to the virtio code.
> >
> > [Test Case]
> > * Just normal test. This change doesn't affect any functionality.
> >
> > [Regression Potential]
> >
> > * This version of the driver was tested by QA/verification for a while so no
> >   known regression at the moment.
> >
> > --
> > kernel-team mailing list
> > kernel-team@lists.ubuntu.com
> >
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.
> ubuntu.com%2Fmailman%2Flistinfo%2Fkernel-
> team&amp;data=04%7C01%7Climings%40nvidia.com%7C9301bd3977774c040
> 2b108d9104b62fd%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637
> 558738235771349%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAi
> LCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=N
> 85ehvQuTFbIAuWQRm7HhUgFtMXMxLtrRFRyN6emqco%3D&amp;reserved
> =0