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

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

[Dai Ngo]

With SCSI layouts, the NFS client must not submit I/O to the data server
until the Persistent Reservation (PR) registration has completed.

Currently, bl_register_scsi() sets PNFS_BDEV_REGISTERED before performing
the PR operation. If multiple threads concurrently start I/O to the same
SCSI device, the first thread sets the flag and begins registration,
while other threads observe the flag, skip registration, and proceed to
issue I/O. Those I/Os can hit RESERVATION CONFLICT, forcing fall back to
the MDS.

Protect the registration/unregistration operation path with a mutex so only
one thread performs the op at a time. Other threads wait for the operation
to finish and only then and only then re-check PNFS_BDEV_REGISTERED flag.
 
Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
---
 fs/nfs/blocklayout/blocklayout.h |  3 +++
 fs/nfs/blocklayout/dev.c         | 14 ++++++++++++--
 2 files changed, 15 insertions(+), 2 deletions(-)

v2:
    . remove fio test from commit message.
    . rename pbd_mutex to pbd_registration_mutex and add a description
      of its usage.
    . move declaration of pbd_registration_mutex before the (*map)().
    . protect unregistration op with pbd_registration_mutex.

diff --git a/fs/nfs/blocklayout/blocklayout.h b/fs/nfs/blocklayout/blocklayout.h
index 6da40ca19570..934a5b75ed1e 100644
--- a/fs/nfs/blocklayout/blocklayout.h
+++ b/fs/nfs/blocklayout/blocklayout.h
@@ -114,9 +114,12 @@ struct pnfs_block_dev {
 	unsigned long			flags;
 
 	u64				pr_key;
+	/* Mutex to serialize SCSI PR register/unregister operations. */
+	struct mutex			pbd_registration_mutex;
 
 	bool (*map)(struct pnfs_block_dev *dev, u64 offset,
 			struct pnfs_block_dev_map *map);
+
 };
 
 /* pnfs_block_dev flag bits */
diff --git a/fs/nfs/blocklayout/dev.c b/fs/nfs/blocklayout/dev.c
index cc6327d97a91..8d57368c7cf4 100644
--- a/fs/nfs/blocklayout/dev.c
+++ b/fs/nfs/blocklayout/dev.c
@@ -33,10 +33,15 @@ static bool bl_register_scsi(struct pnfs_block_dev *dev)
 	const struct pr_ops *ops = bdev->bd_disk->fops->pr_ops;
 	int status;
 
-	if (test_and_set_bit(PNFS_BDEV_REGISTERED, &dev->flags))
+	mutex_lock(&dev->pbd_registration_mutex);
+	if (dev->flags & BIT(PNFS_BDEV_REGISTERED)) {
+		mutex_unlock(&dev->pbd_registration_mutex);
 		return true;
+	}
+	dev->flags |= BIT(PNFS_BDEV_REGISTERED);
 
 	status = ops->pr_register(bdev, 0, dev->pr_key, true);
+	mutex_unlock(&dev->pbd_registration_mutex);
 	if (status) {
 		trace_bl_pr_key_reg_err(bdev, dev->pr_key, status);
 		return false;
@@ -55,9 +60,13 @@ static void bl_unregister_dev(struct pnfs_block_dev *dev)
 		return;
 	}
 
+	mutex_lock(&dev->pbd_registration_mutex);
 	if (dev->type == PNFS_BLOCK_VOLUME_SCSI &&
-		test_and_clear_bit(PNFS_BDEV_REGISTERED, &dev->flags))
+			dev->flags & BIT(PNFS_BDEV_REGISTERED)) {
+		dev->flags &= ~BIT(PNFS_BDEV_REGISTERED);
 		bl_unregister_scsi(dev);
+	}
+	mutex_unlock(&dev->pbd_registration_mutex);
 }
 
 bool bl_register_dev(struct pnfs_block_dev *dev)
@@ -572,6 +581,7 @@ bl_alloc_deviceid_node(struct nfs_server *server, struct pnfs_device *pdev,
 	top = kzalloc_obj(*top, gfp_mask);
 	if (!top)
 		goto out_free_volumes;
+	mutex_init(&top->pbd_registration_mutex);
 
 	ret = bl_parse_deviceid(server, top, volumes, nr_volumes - 1, gfp_mask);
 
-- 
2.47.3

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

[Christoph Hellwig]

> -	if (test_and_set_bit(PNFS_BDEV_REGISTERED, &dev->flags))
> +	mutex_lock(&dev->pbd_registration_mutex);
> +	if (dev->flags & BIT(PNFS_BDEV_REGISTERED)) {

dev->flags can now become a simple unsigned int, and you can
define PNFS_BDEV_REGISTERED as an actual value instead of using
BIT everywhere.

Otherwise this looks good.