Re: [PATCH net] net: devmem: use READ_ONCE/WRITE_ONCE on binding->dev

[Bobby Eshleman]

On Wed, Feb 25, 2026 at 09:31:48AM -0800, Mina Almasry wrote:
> On Wed, Feb 25, 2026 at 7:14 AM Bobby Eshleman <bobbyeshleman@xxxxxxxxx> wrote:
> >
> > On Tue, Feb 24, 2026 at 05:49:42PM -0800, Stanislav Fomichev wrote:
> > > On 02/23, Bobby Eshleman wrote:
> > > > From: Bobby Eshleman <bobbyeshleman@xxxxxxxx>
> > > >
> > > > binding->dev is protected on the write-side in
> > > > mp_dmabuf_devmem_uninstall() against concurrent writes, but due to the
> > > > concurrent bare read in net_devmem_get_binding() it should be wrapped in
> > > > a READ_ONCE/WRITE_ONCE pair to make sure no compiler optimizations play
> > > > with the underlying register in unforeseen ways.
> > > >
> > > > Fixes: bd61848900bf ("net: devmem: Implement TX path")
> > > > Signed-off-by: Bobby Eshleman <bobbyeshleman@xxxxxxxx>
> 
> Looks correct to me, and AFAIU Stan is right, we might as well
> annotate all the reads of ->dev as technically there could be a dmabuf
> uninstall happing concurrently on another CPU. I also think it's
> probably good to annotate potential races.
> 
> The ->dev write in dmabuf binding doesn't need WRITE_ONCE annotation I
> guess because it's initialization, it can't race with any reads.
> 
> This makes me wonder what other fields in dmabuf need annotations. I
> hope I didn't miss many more.
> 
> I would add this is really not a critical bug because
> net_devmem_get_binding() is in TX path, and it is more than fine here
> if we fail this check if there is an unbind happening in paraller with
> sendmsg(), but it's probably good to annotate potential races anyway.
> 

Sounds good. I'll take a look at some of the other fields while my mind
is in this space.

I agree about it being non-critical, good point about TX passing through
fine on failed check.

Best,
Bobby