Re: [PATCH 1/1] pNFS: Serialize SCSI PR registration to avoid reservation conflicts

[Christoph Hellwig <hch@infradead.org>]

On Tue, Mar 03, 2026 at 10:21:07AM -0800, Dai Ngo wrote:
> > >   			struct pnfs_block_dev_map *map);
> > > +	struct mutex			pbd_mutex;
> > Can you keep this up with the non-function pointer fields?
> 
> Can you please clarify this, you meant move this mutex up above
> the (*map)() declaration?

Yes.

> > > -	if (test_and_set_bit(PNFS_BDEV_REGISTERED, &dev->flags))
> > > +	mutex_lock(&dev->pbd_mutex);
> > > +	if (test_and_set_bit(PNFS_BDEV_REGISTERED, &dev->flags)) {
> > > +		mutex_unlock(&dev->pbd_mutex);
> > >   		return true;
> > > +	}
> > This seems to only lock the registration side, and not the
> > unregistration side, which is a bit odd.
> 
> The reason I did not use the mutex on unregistration is because
> unregistration happens when the export is unmounted and I don't
> see any race condition can happen at that time. Besides, even if
> there is race condition on the unregistration the consequence is
> a duplicate SCSI PR unregistration which is harmless.
> 
> However, if you think we should also protect the unregistration
> then I can add it in. At the very least, it makes the code look
> symmetric.

And it clearly defines what the mutex protects, so please yes.

> 
> >    If you fully protect
> > register/unregister we also don't need atomic bitops for
> > PNFS_BDEV_REGISTERED and have a more consistent locking scheme.
> 
> Even we fully protect register/unregister don't we still need the
> PNFS_BDEV_REGISTERED bit so the others thread can check and skip
> the register/unregister op?

Yes, but it doesn't need to use the atomic bit ops.