All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] nbd: set the logical and physical blocksize properly
@ 2017-02-10 18:06 Josef Bacik
  2017-02-10 20:07 ` [Nbd] " Alex Bligh
  2017-02-13 15:39 ` [PATCH V2] " Josef Bacik
  0 siblings, 2 replies; 6+ messages in thread
From: Josef Bacik @ 2017-02-10 18:06 UTC (permalink / raw)
  To: axboe, linux-block, kernel-team, nbd-general

We noticed when trying to do O_DIRECT to an export on the server side
that we were getting requests smaller than the 4k sectorsize of the
device.  This is because the client isn't setting the logical and
physical blocksizes properly for the underlying device.  Fix this up by
setting the queue blocksizes and then calling bd_set_size.

Signed-off-by: Josef Bacik <jbacik@fb.com>
---
 drivers/block/nbd.c | 39 ++++++++++++++++-----------------------
 1 file changed, 16 insertions(+), 23 deletions(-)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 34a280a..e0d770c 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -183,7 +183,7 @@ static int nbd_clear_reconnect(struct nbd_device *nbd)
 
 static int nbd_size_clear(struct nbd_device *nbd, struct block_device *bdev)
 {
-	bdev->bd_inode->i_size = 0;
+	bd_set_size(bdev, 0);
 	set_capacity(nbd->disk, 0);
 	kobject_uevent(&nbd_to_dev(nbd)->kobj, KOBJ_CHANGE);
 
@@ -192,29 +192,20 @@ static int nbd_size_clear(struct nbd_device *nbd, struct block_device *bdev)
 
 static void nbd_size_update(struct nbd_device *nbd, struct block_device *bdev)
 {
-	if (!nbd_is_connected(nbd))
-		return;
-
-	bdev->bd_inode->i_size = nbd->bytesize;
+	blk_queue_logical_block_size(nbd->disk->queue, nbd->blksize);
+	blk_queue_physical_block_size(nbd->disk->queue, nbd->blksize);
+	bd_set_size(bdev, nbd->bytesize);
 	set_capacity(nbd->disk, nbd->bytesize >> 9);
 	kobject_uevent(&nbd_to_dev(nbd)->kobj, KOBJ_CHANGE);
 }
 
-static int nbd_size_set(struct nbd_device *nbd, struct block_device *bdev,
+static void nbd_size_set(struct nbd_device *nbd, struct block_device *bdev,
 			loff_t blocksize, loff_t nr_blocks)
 {
-	int ret;
-
-	ret = set_blocksize(bdev, blocksize);
-	if (ret)
-		return ret;
-
 	nbd->blksize = blocksize;
 	nbd->bytesize = blocksize * nr_blocks;
-
-	nbd_size_update(nbd, bdev);
-
-	return 0;
+	if (nbd_is_connected(nbd))
+		nbd_size_update(nbd, bdev);
 }
 
 static void nbd_end_request(struct nbd_cmd *cmd)
@@ -875,6 +866,7 @@ static void send_disconnects(struct nbd_device *nbd)
 
 static int nbd_disconnect(struct nbd_device *nbd, struct block_device *bdev)
 {
+	printk(KERN_ERR "%u is the blocksize at disconnect\n", bdev_logical_block_size(bdev));
 	dev_info(disk_to_dev(nbd->disk), "NBD_DISCONNECT\n");
 	if (!nbd_socks_get_unless_zero(nbd))
 		return -EINVAL;
@@ -997,15 +989,16 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
 	case NBD_SET_SOCK:
 		return nbd_add_socket(nbd, bdev, arg);
 	case NBD_SET_BLKSIZE:
-		return nbd_size_set(nbd, bdev, arg,
-				    div_s64(nbd->bytesize, arg));
+		nbd_size_set(nbd, bdev, arg,
+			     div_s64(nbd->bytesize, arg));
+		return 0;
 	case NBD_SET_SIZE:
-		return nbd_size_set(nbd, bdev, nbd->blksize,
-					div_s64(arg, nbd->blksize));
-
+		nbd_size_set(nbd, bdev, nbd->blksize,
+			     div_s64(arg, nbd->blksize));
+		return 0;
 	case NBD_SET_SIZE_BLOCKS:
-		return nbd_size_set(nbd, bdev, nbd->blksize, arg);
-
+		nbd_size_set(nbd, bdev, nbd->blksize, arg);
+		return 0;
 	case NBD_SET_TIMEOUT:
 		nbd->tag_set.timeout = arg * HZ;
 		return 0;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [Nbd] [PATCH] nbd: set the logical and physical blocksize properly
  2017-02-10 18:06 [PATCH] nbd: set the logical and physical blocksize properly Josef Bacik
@ 2017-02-10 20:07 ` Alex Bligh
  2017-02-10 21:47   ` Josef Bacik
  2017-02-13 15:39 ` [PATCH V2] " Josef Bacik
  1 sibling, 1 reply; 6+ messages in thread
From: Alex Bligh @ 2017-02-10 20:07 UTC (permalink / raw)
  To: Josef Bacik
  Cc: Alex Bligh, axboe, linux-block, kernel-team,
	nbd-general@lists.sourceforge.net


> On 10 Feb 2017, at 19:06, Josef Bacik <jbacik@fb.com> wrote:
> 
> We noticed when trying to do O_DIRECT to an export on the server side
> that we were getting requests smaller than the 4k sectorsize of the
> device.  This is because the client isn't setting the logical and
> physical blocksizes properly for the underlying device.  Fix this up by
> setting the queue blocksizes and then calling bd_set_size.

Interesting. Some input into the info extension (re blocksizes) would
definitely be appreciated.

-- 
Alex Bligh

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Nbd] [PATCH] nbd: set the logical and physical blocksize properly
  2017-02-10 20:07 ` [Nbd] " Alex Bligh
@ 2017-02-10 21:47   ` Josef Bacik
  2017-02-11 11:43     ` Wouter Verhelst
  0 siblings, 1 reply; 6+ messages in thread
From: Josef Bacik @ 2017-02-10 21:47 UTC (permalink / raw)
  To: Alex Bligh
  Cc: axboe, linux-block, kernel-team,
	nbd-general@lists.sourceforge.net

On Fri, 2017-02-10 at 21:07 +0100, Alex Bligh wrote:
> > 
> > On 10 Feb 2017, at 19:06, Josef Bacik <jbacik@fb.com> wrote:
> > 
> > We noticed when trying to do O_DIRECT to an export on the server
> > side
> > that we were getting requests smaller than the 4k sectorsize of the
> > device.  This is because the client isn't setting the logical and
> > physical blocksizes properly for the underlying device.  Fix this
> > up by
> > setting the queue blocksizes and then calling bd_set_size.
> Interesting. Some input into the info extension (re blocksizes) would
> definitely be appreciated.
> 

What do you mean?  Right now the client is just calling NBD_SET_BLKSIZE
with 4k blocksize since all of our devices are 4k drives.  Thanks,

Josef

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Nbd] [PATCH] nbd: set the logical and physical blocksize properly
  2017-02-10 21:47   ` Josef Bacik
@ 2017-02-11 11:43     ` Wouter Verhelst
  2017-02-11 16:13       ` Alex Bligh
  0 siblings, 1 reply; 6+ messages in thread
From: Wouter Verhelst @ 2017-02-11 11:43 UTC (permalink / raw)
  To: Josef Bacik
  Cc: Alex Bligh, axboe, linux-block, kernel-team,
	nbd-general@lists.sourceforge.net

On Fri, Feb 10, 2017 at 04:47:42PM -0500, Josef Bacik wrote:
> On Fri, 2017-02-10 at 21:07 +0100, Alex Bligh wrote:
> > > 
> > > On 10 Feb 2017, at 19:06, Josef Bacik <jbacik@fb.com> wrote:
> > > 
> > > We noticed when trying to do O_DIRECT to an export on the server
> > > side
> > > that we were getting requests smaller than the 4k sectorsize of the
> > > device.��This is because the client isn't setting the logical and
> > > physical blocksizes properly for the underlying device.��Fix this
> > > up by
> > > setting the queue blocksizes and then calling bd_set_size.
> > Interesting. Some input into the info extension (re blocksizes) would
> > definitely be appreciated.
> > 
> 
> What do you mean? �Right now the client is just calling�NBD_SET_BLKSIZE
> with 4k blocksize since all of our devices are 4k drives. �Thanks,

He's talking about
<https://github.com/NetworkBlockDevice/nbd/blob/extension-info/doc/proto.md#block-size-constraints>,
which is a protocol extension that hasn't been implemented yet but would
be relevant to this patch.

-- 
< ron> I mean, the main *practical* problem with C++, is there's like a dozen
       people in the world who think they really understand all of its rules,
       and pretty much all of them are just lying to themselves too.
 -- #debian-devel, OFTC, 2016-02-12

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Nbd] [PATCH] nbd: set the logical and physical blocksize properly
  2017-02-11 11:43     ` Wouter Verhelst
@ 2017-02-11 16:13       ` Alex Bligh
  0 siblings, 0 replies; 6+ messages in thread
From: Alex Bligh @ 2017-02-11 16:13 UTC (permalink / raw)
  To: Wouter Verhelst
  Cc: Alex Bligh, Josef Bacik, axboe, linux-block, Kernel Team,
	nbd-general@lists.sourceforge.net


> On 11 Feb 2017, at 12:43, Wouter Verhelst <w@uter.be> wrote:
>=20
>>> Interesting. Some input into the info extension (re blocksizes) =
would
>>> definitely be appreciated.
>>>=20
>>=20
>> What do you mean?  Right now the client is just calling =
NBD_SET_BLKSIZE
>> with 4k blocksize since all of our devices are 4k drives.  Thanks,
>=20
> He's talking about
> =
<https://github.com/NetworkBlockDevice/nbd/blob/extension-info/doc/proto.m=
d#block-size-constraints>,
> which is a protocol extension that hasn't been implemented yet but =
would
> be relevant to this patch.

Indeed. Specifically, review of the patch from a kernel perspective
would be useful.

--=20
Alex Bligh

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH V2] nbd: set the logical and physical blocksize properly
  2017-02-10 18:06 [PATCH] nbd: set the logical and physical blocksize properly Josef Bacik
  2017-02-10 20:07 ` [Nbd] " Alex Bligh
@ 2017-02-13 15:39 ` Josef Bacik
  1 sibling, 0 replies; 6+ messages in thread
From: Josef Bacik @ 2017-02-13 15:39 UTC (permalink / raw)
  To: axboe, linux-block, kernel-team, nbd-general

We noticed when trying to do O_DIRECT to an export on the server side
that we were getting requests smaller than the 4k sectorsize of the
device.  This is because the client isn't setting the logical and
physical blocksizes properly for the underlying device.  Fix this up by
setting the queue blocksizes and then calling bd_set_size.

Signed-off-by: Josef Bacik <jbacik@fb.com>
---
V1->V2:
-remove an debug printk that I forgot to delete.

 drivers/block/nbd.c | 38 +++++++++++++++-----------------------
 1 file changed, 15 insertions(+), 23 deletions(-)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index afb1353..25891a1 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -152,7 +152,7 @@ static void nbd_socks_put(struct nbd_device *nbd)
 
 static int nbd_size_clear(struct nbd_device *nbd, struct block_device *bdev)
 {
-	bdev->bd_inode->i_size = 0;
+	bd_set_size(bdev, 0);
 	set_capacity(nbd->disk, 0);
 	kobject_uevent(&nbd_to_dev(nbd)->kobj, KOBJ_CHANGE);
 
@@ -161,29 +161,20 @@ static int nbd_size_clear(struct nbd_device *nbd, struct block_device *bdev)
 
 static void nbd_size_update(struct nbd_device *nbd, struct block_device *bdev)
 {
-	if (!nbd_is_connected(nbd))
-		return;
-
-	bdev->bd_inode->i_size = nbd->bytesize;
+	blk_queue_logical_block_size(nbd->disk->queue, nbd->blksize);
+	blk_queue_physical_block_size(nbd->disk->queue, nbd->blksize);
+	bd_set_size(bdev, nbd->bytesize);
 	set_capacity(nbd->disk, nbd->bytesize >> 9);
 	kobject_uevent(&nbd_to_dev(nbd)->kobj, KOBJ_CHANGE);
 }
 
-static int nbd_size_set(struct nbd_device *nbd, struct block_device *bdev,
+static void nbd_size_set(struct nbd_device *nbd, struct block_device *bdev,
 			loff_t blocksize, loff_t nr_blocks)
 {
-	int ret;
-
-	ret = set_blocksize(bdev, blocksize);
-	if (ret)
-		return ret;
-
 	nbd->blksize = blocksize;
 	nbd->bytesize = blocksize * nr_blocks;
-
-	nbd_size_update(nbd, bdev);
-
-	return 0;
+	if (nbd_is_connected(nbd))
+		nbd_size_update(nbd, bdev);
 }
 
 static void nbd_end_request(struct nbd_cmd *cmd)
@@ -926,15 +917,16 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
 	case NBD_SET_SOCK:
 		return nbd_add_socket(nbd, bdev, arg);
 	case NBD_SET_BLKSIZE:
-		return nbd_size_set(nbd, bdev, arg,
-				    div_s64(nbd->bytesize, arg));
+		nbd_size_set(nbd, bdev, arg,
+			     div_s64(nbd->bytesize, arg));
+		return 0;
 	case NBD_SET_SIZE:
-		return nbd_size_set(nbd, bdev, nbd->blksize,
-					div_s64(arg, nbd->blksize));
-
+		nbd_size_set(nbd, bdev, nbd->blksize,
+			     div_s64(arg, nbd->blksize));
+		return 0;
 	case NBD_SET_SIZE_BLOCKS:
-		return nbd_size_set(nbd, bdev, nbd->blksize, arg);
-
+		nbd_size_set(nbd, bdev, nbd->blksize, arg);
+		return 0;
 	case NBD_SET_TIMEOUT:
 		nbd->tag_set.timeout = arg * HZ;
 		return 0;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-02-13 15:39 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-02-10 18:06 [PATCH] nbd: set the logical and physical blocksize properly Josef Bacik
2017-02-10 20:07 ` [Nbd] " Alex Bligh
2017-02-10 21:47   ` Josef Bacik
2017-02-11 11:43     ` Wouter Verhelst
2017-02-11 16:13       ` Alex Bligh
2017-02-13 15:39 ` [PATCH V2] " Josef Bacik

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.