diff mbox

[v1,2/3] pnfs/blocklayout: Revalidate SCSI disks after registration

Message ID 7849a32ce8d429791eed114275496ec213dfaeb7.1512748321.git.bcodding@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Benjamin Coddington Dec. 8, 2017, 5:52 p.m. UTC
If a SCSI device already has an exclusive access registrants only
reservation, the device's size cannot be known by the client until after
registration.  We should therefore delay setting the block device's length
until after the registration and revalidation of the disk.

Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
---
 fs/nfs/blocklayout/dev.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Christoph Hellwig Dec. 12, 2017, 2:38 p.m. UTC | #1
On Fri, Dec 08, 2017 at 12:52:58PM -0500, Benjamin Coddington wrote:
> If a SCSI device already has an exclusive access registrants only
> reservation, the device's size cannot be known by the client until after
> registration.  We should therefore delay setting the block device's length
> until after the registration and revalidation of the disk.

Table 13 in sbc4r14.pdf clearly indicates READ CAPACITY is allowed even
for a not registered I_T nexus.  What kind of setup do you see this
problem with?
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Benjamin Coddington Dec. 13, 2017, 2:23 a.m. UTC | #2
On 12 Dec 2017, at 9:38, Christoph Hellwig wrote:

> On Fri, Dec 08, 2017 at 12:52:58PM -0500, Benjamin Coddington wrote:
>> If a SCSI device already has an exclusive access registrants only
>> reservation, the device's size cannot be known by the client until after
>> registration.  We should therefore delay setting the block device's length
>> until after the registration and revalidation of the disk.
>
> Table 13 in sbc4r14.pdf clearly indicates READ CAPACITY is allowed even
> for a not registered I_T nexus.  What kind of setup do you see this
> problem with?

It does indeed, I should have checked.  So it seems this patch should be
unnecessary, however I think if we do end up with d->len = 0, bad things
happen later.  My memory is fuzzy on this, I'll have to re-test it.

The SCSI devices are presented by the TCM driver on a 4.11 era kernel.  I
don't know that code at all, but with quick look it seems that
__target_execute_cmd() wants to check for reservations for all commands.

I think this patch should be dropped, and we can try to fix this on the TCM
side.

Ben
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/nfs/blocklayout/dev.c b/fs/nfs/blocklayout/dev.c
index a69ef4e9c24c..4d319098f83b 100644
--- a/fs/nfs/blocklayout/dev.c
+++ b/fs/nfs/blocklayout/dev.c
@@ -366,7 +366,6 @@  bl_parse_scsi(struct nfs_server *server, struct pnfs_block_dev *d,
 		return PTR_ERR(bdev);
 	d->bdev = bdev;
 
-	d->len = i_size_read(d->bdev->bd_inode);
 	d->map = bl_map_simple;
 	d->pr_key = v->scsi.pr_key;
 
@@ -388,6 +387,13 @@  bl_parse_scsi(struct nfs_server *server, struct pnfs_block_dev *d,
 		goto out_blkdev_put;
 	}
 
+	error = revalidate_disk(d->bdev->bd_disk);
+	if (error) {
+		pr_err("pNFS: failed to revalidate block device %s.",
+				d->bdev->bd_disk->disk_name);
+		goto out_blkdev_put;
+	}
+	d->len = i_size_read(d->bdev->bd_inode);
 	d->pr_registered = true;
 	return 0;