[PATCH] blockdev: Remove the --setbsz (set blocksize) option which has never worked.

[PATCH] blockdev: Remove the --setbsz (set blocksize) option which has never worked.

[Richard W.M. Jones]

From: "Richard W.M. Jones" <rjones@redhat.com>

This option has never worked.  If you try setting the block size on a
block device, and then read it using --getbsz, you will see that the
block size never changes.

The reason for this is because the block size is specific to the
current file descriptor opening the block device, so the change of
block size only persists for as long as blockdev has the device open,
and is lost once blockdev exits.

Also the block size is not really used anywhere.  Filesystems, for
example, have their own idea of block size and ignore this setting
completely.

(Thanks Masayoshi Mizuma for diagnosing the problem)

Signed-off-by: Richard W.M. Jones <rjones@redhat.com>
---
 bash-completion/blockdev | 7 +------
 disk-utils/blockdev.8    | 2 --
 disk-utils/blockdev.c    | 7 -------
 3 files changed, 1 insertion(+), 15 deletions(-)

diff --git a/bash-completion/blockdev b/bash-completion/blockdev
index ce986cb..5889955 100644
--- a/bash-completion/blockdev
+++ b/bash-completion/blockdev
@@ -19,7 +19,6 @@ _blockdev_module()
 		--getalignoff
 		--getmaxsect
 		--getbsz
-		--setbsz
 		--getsize64
 		--setra
 		--getra
@@ -29,11 +28,7 @@ _blockdev_module()
 		--rereadpt
 		$DEVS"
 	case $prev in
-		'--setbsz')
-			COMPREPLY=( $(compgen -W "bytes" -- $cur) )
-			return 0
-			;;
-		'--setbsz'|'--setfra')
+		'--setfra')
 			COMPREPLY=( $(compgen -W "sectors" -- $cur) )
 			return 0
 			;;
diff --git a/disk-utils/blockdev.8 b/disk-utils/blockdev.8
index 2b3d64c..6ab4324 100644
--- a/disk-utils/blockdev.8
+++ b/disk-utils/blockdev.8
@@ -67,8 +67,6 @@ Print sectorsize in bytes - usually 512.
 Get size in 512-byte sectors.
 .IP "\fB\-\-rereadpt\fP"
 Reread partition table
-.IP "\fB\-\-setbsz\fP \fIbytes\fP"
-Set blocksize.
 .IP "\fB\-\-setfra\fP \fIsectors\fP"
 Set filesystem readahead (same like --setra on 2.6 kernels).
 .IP "\fB\-\-setra\fP \fIsectors\fP"
diff --git a/disk-utils/blockdev.c b/disk-utils/blockdev.c
index 4543818..dc02d48 100644
--- a/disk-utils/blockdev.c
+++ b/disk-utils/blockdev.c
@@ -122,13 +122,6 @@ static const struct bdc bdcms[] =
 		.argval = -1,
 		.help = N_("get blocksize")
 	},{
-		IOCTL_ENTRY(BLKBSZSET),
-		.name = "--setbsz",
-		.argname = "<bytes>",
-		.argtype = ARG_INT,
-		.flags = FL_NORESULT,
-	        .help = N_("set blocksize")
-	},{
 		IOCTL_ENTRY(BLKGETSIZE),
 		.name = "--getsize",
 		.argtype = ARG_ULONG,
-- 
1.8.3.1

Re: [PATCH] blockdev: Remove the --setbsz (set blocksize) option which has never worked.

[Karel Zak]

On Tue, Oct 08, 2013 at 09:54:10AM +0100, Richard W.M. Jones wrote:
>  bash-completion/blockdev | 7 +------
>  disk-utils/blockdev.8    | 2 --
>  disk-utils/blockdev.c    | 7 -------
>  3 files changed, 1 insertion(+), 15 deletions(-)

 Applied, thanks.

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH] blockdev: Remove the --setbsz (set blocksize) option which has never worked.

[Pádraig Brady]

On 10/08/2013 09:54 AM, Richard W.M. Jones wrote:
> From: "Richard W.M. Jones" <rjones@redhat.com>
> 
> This option has never worked.  If you try setting the block size on a
> block device, and then read it using --getbsz, you will see that the
> block size never changes.
> 
> The reason for this is because the block size is specific to the
> current file descriptor opening the block device, so the change of
> block size only persists for as long as blockdev has the device open,
> and is lost once blockdev exits.
> 
> Also the block size is not really used anywhere.  Filesystems, for
> example, have their own idea of block size and ignore this setting
> completely.
> 
> (Thanks Masayoshi Mizuma for diagnosing the problem)
> 
> Signed-off-by: Richard W.M. Jones <rjones@redhat.com>
> ---
>  bash-completion/blockdev | 7 +------
>  disk-utils/blockdev.8    | 2 --
>  disk-utils/blockdev.c    | 7 -------
>  3 files changed, 1 insertion(+), 15 deletions(-)
> 
> diff --git a/bash-completion/blockdev b/bash-completion/blockdev
> index ce986cb..5889955 100644
> --- a/bash-completion/blockdev
> +++ b/bash-completion/blockdev
> @@ -19,7 +19,6 @@ _blockdev_module()
>  		--getalignoff
>  		--getmaxsect
>  		--getbsz
> -		--setbsz
>  		--getsize64
>  		--setra
>  		--getra
> @@ -29,11 +28,7 @@ _blockdev_module()
>  		--rereadpt
>  		$DEVS"
>  	case $prev in
> -		'--setbsz')
> -			COMPREPLY=( $(compgen -W "bytes" -- $cur) )
> -			return 0
> -			;;
> -		'--setbsz'|'--setfra')
> +		'--setfra')
>  			COMPREPLY=( $(compgen -W "sectors" -- $cur) )
>  			return 0
>  			;;
> diff --git a/disk-utils/blockdev.8 b/disk-utils/blockdev.8
> index 2b3d64c..6ab4324 100644
> --- a/disk-utils/blockdev.8
> +++ b/disk-utils/blockdev.8
> @@ -67,8 +67,6 @@ Print sectorsize in bytes - usually 512.
>  Get size in 512-byte sectors.
>  .IP "\fB\-\-rereadpt\fP"
>  Reread partition table
> -.IP "\fB\-\-setbsz\fP \fIbytes\fP"
> -Set blocksize.
>  .IP "\fB\-\-setfra\fP \fIsectors\fP"
>  Set filesystem readahead (same like --setra on 2.6 kernels).
>  .IP "\fB\-\-setra\fP \fIsectors\fP"
> diff --git a/disk-utils/blockdev.c b/disk-utils/blockdev.c
> index 4543818..dc02d48 100644
> --- a/disk-utils/blockdev.c
> +++ b/disk-utils/blockdev.c
> @@ -122,13 +122,6 @@ static const struct bdc bdcms[] =
>  		.argval = -1,
>  		.help = N_("get blocksize")
>  	},{
> -		IOCTL_ENTRY(BLKBSZSET),
> -		.name = "--setbsz",
> -		.argname = "<bytes>",
> -		.argtype = ARG_INT,
> -		.flags = FL_NORESULT,
> -	        .help = N_("set blocksize")
> -	},{
>  		IOCTL_ENTRY(BLKGETSIZE),
>  		.name = "--getsize",
>  		.argtype = ARG_ULONG,
> 

Might one process the adjusted block device in a subshell?

$ (blockdev --setbsz 2048 /dev/stdin; blockdev --getbsz /dev/stdin) < /dev/loop0
2048

cheers,
Pádraig.

Re: [PATCH] blockdev: Remove the --setbsz (set blocksize) option which has never worked.

[Richard W.M. Jones]

On Tue, Oct 08, 2013 at 03:05:46PM +0100, Pádraig Brady wrote:
> On 10/08/2013 09:54 AM, Richard W.M. Jones wrote:
> > From: "Richard W.M. Jones" <rjones@redhat.com>
> > 
> > This option has never worked.  If you try setting the block size on a
> > block device, and then read it using --getbsz, you will see that the
> > block size never changes.
> > 
> > The reason for this is because the block size is specific to the
> > current file descriptor opening the block device, so the change of
> > block size only persists for as long as blockdev has the device open,
> > and is lost once blockdev exits.
> > 
> > Also the block size is not really used anywhere.  Filesystems, for
> > example, have their own idea of block size and ignore this setting
> > completely.
> > 
> > (Thanks Masayoshi Mizuma for diagnosing the problem)
> > 
> > Signed-off-by: Richard W.M. Jones <rjones@redhat.com>
> > ---
> >  bash-completion/blockdev | 7 +------
> >  disk-utils/blockdev.8    | 2 --
> >  disk-utils/blockdev.c    | 7 -------
> >  3 files changed, 1 insertion(+), 15 deletions(-)
> > 
> > diff --git a/bash-completion/blockdev b/bash-completion/blockdev
> > index ce986cb..5889955 100644
> > --- a/bash-completion/blockdev
> > +++ b/bash-completion/blockdev
> > @@ -19,7 +19,6 @@ _blockdev_module()
> >  		--getalignoff
> >  		--getmaxsect
> >  		--getbsz
> > -		--setbsz
> >  		--getsize64
> >  		--setra
> >  		--getra
> > @@ -29,11 +28,7 @@ _blockdev_module()
> >  		--rereadpt
> >  		$DEVS"
> >  	case $prev in
> > -		'--setbsz')
> > -			COMPREPLY=( $(compgen -W "bytes" -- $cur) )
> > -			return 0
> > -			;;
> > -		'--setbsz'|'--setfra')
> > +		'--setfra')
> >  			COMPREPLY=( $(compgen -W "sectors" -- $cur) )
> >  			return 0
> >  			;;
> > diff --git a/disk-utils/blockdev.8 b/disk-utils/blockdev.8
> > index 2b3d64c..6ab4324 100644
> > --- a/disk-utils/blockdev.8
> > +++ b/disk-utils/blockdev.8
> > @@ -67,8 +67,6 @@ Print sectorsize in bytes - usually 512.
> >  Get size in 512-byte sectors.
> >  .IP "\fB\-\-rereadpt\fP"
> >  Reread partition table
> > -.IP "\fB\-\-setbsz\fP \fIbytes\fP"
> > -Set blocksize.
> >  .IP "\fB\-\-setfra\fP \fIsectors\fP"
> >  Set filesystem readahead (same like --setra on 2.6 kernels).
> >  .IP "\fB\-\-setra\fP \fIsectors\fP"
> > diff --git a/disk-utils/blockdev.c b/disk-utils/blockdev.c
> > index 4543818..dc02d48 100644
> > --- a/disk-utils/blockdev.c
> > +++ b/disk-utils/blockdev.c
> > @@ -122,13 +122,6 @@ static const struct bdc bdcms[] =
> >  		.argval = -1,
> >  		.help = N_("get blocksize")
> >  	},{
> > -		IOCTL_ENTRY(BLKBSZSET),
> > -		.name = "--setbsz",
> > -		.argname = "<bytes>",
> > -		.argtype = ARG_INT,
> > -		.flags = FL_NORESULT,
> > -	        .help = N_("set blocksize")
> > -	},{
> >  		IOCTL_ENTRY(BLKGETSIZE),
> >  		.name = "--getsize",
> >  		.argtype = ARG_ULONG,
> > 
> 
> Might one process the adjusted block device in a subshell?
> 
> $ (blockdev --setbsz 2048 /dev/stdin; blockdev --getbsz /dev/stdin) < /dev/loop0
> 2048

Yes .. if that was documented anywhere, if it wasn't so obscure, and
if it was useful for anything except changing a handle property that
nothing uses.

There's an informative but unfortunately private exchange on the
subject in this bug:
https://bugzilla.redhat.com/show_bug.cgi?id=1002825

I have asked if we can make the contents of this bug public.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top

Re: [PATCH] blockdev: Remove the --setbsz (set blocksize) option which has never worked.

[Pádraig Brady]

On 10/08/2013 03:09 PM, Richard W.M. Jones wrote:
> On Tue, Oct 08, 2013 at 03:05:46PM +0100, Pádraig Brady wrote:
>> On 10/08/2013 09:54 AM, Richard W.M. Jones wrote:
>>> From: "Richard W.M. Jones" <rjones@redhat.com>
>>>
>>> This option has never worked.  If you try setting the block size on a
>>> block device, and then read it using --getbsz, you will see that the
>>> block size never changes.
>>>
>>> The reason for this is because the block size is specific to the
>>> current file descriptor opening the block device, so the change of
>>> block size only persists for as long as blockdev has the device open,
>>> and is lost once blockdev exits.
>>>
>>> Also the block size is not really used anywhere.  Filesystems, for
>>> example, have their own idea of block size and ignore this setting
>>> completely.
>>>
>>> (Thanks Masayoshi Mizuma for diagnosing the problem)
>>>
>>> Signed-off-by: Richard W.M. Jones <rjones@redhat.com>
>>> ---
>>>  bash-completion/blockdev | 7 +------
>>>  disk-utils/blockdev.8    | 2 --
>>>  disk-utils/blockdev.c    | 7 -------
>>>  3 files changed, 1 insertion(+), 15 deletions(-)
>>>
>>> diff --git a/bash-completion/blockdev b/bash-completion/blockdev
>>> index ce986cb..5889955 100644
>>> --- a/bash-completion/blockdev
>>> +++ b/bash-completion/blockdev
>>> @@ -19,7 +19,6 @@ _blockdev_module()
>>>  		--getalignoff
>>>  		--getmaxsect
>>>  		--getbsz
>>> -		--setbsz
>>>  		--getsize64
>>>  		--setra
>>>  		--getra
>>> @@ -29,11 +28,7 @@ _blockdev_module()
>>>  		--rereadpt
>>>  		$DEVS"
>>>  	case $prev in
>>> -		'--setbsz')
>>> -			COMPREPLY=( $(compgen -W "bytes" -- $cur) )
>>> -			return 0
>>> -			;;
>>> -		'--setbsz'|'--setfra')
>>> +		'--setfra')
>>>  			COMPREPLY=( $(compgen -W "sectors" -- $cur) )
>>>  			return 0
>>>  			;;
>>> diff --git a/disk-utils/blockdev.8 b/disk-utils/blockdev.8
>>> index 2b3d64c..6ab4324 100644
>>> --- a/disk-utils/blockdev.8
>>> +++ b/disk-utils/blockdev.8
>>> @@ -67,8 +67,6 @@ Print sectorsize in bytes - usually 512.
>>>  Get size in 512-byte sectors.
>>>  .IP "\fB\-\-rereadpt\fP"
>>>  Reread partition table
>>> -.IP "\fB\-\-setbsz\fP \fIbytes\fP"
>>> -Set blocksize.
>>>  .IP "\fB\-\-setfra\fP \fIsectors\fP"
>>>  Set filesystem readahead (same like --setra on 2.6 kernels).
>>>  .IP "\fB\-\-setra\fP \fIsectors\fP"
>>> diff --git a/disk-utils/blockdev.c b/disk-utils/blockdev.c
>>> index 4543818..dc02d48 100644
>>> --- a/disk-utils/blockdev.c
>>> +++ b/disk-utils/blockdev.c
>>> @@ -122,13 +122,6 @@ static const struct bdc bdcms[] =
>>>  		.argval = -1,
>>>  		.help = N_("get blocksize")
>>>  	},{
>>> -		IOCTL_ENTRY(BLKBSZSET),
>>> -		.name = "--setbsz",
>>> -		.argname = "<bytes>",
>>> -		.argtype = ARG_INT,
>>> -		.flags = FL_NORESULT,
>>> -	        .help = N_("set blocksize")
>>> -	},{
>>>  		IOCTL_ENTRY(BLKGETSIZE),
>>>  		.name = "--getsize",
>>>  		.argtype = ARG_ULONG,
>>>
>>
>> Might one process the adjusted block device in a subshell?
>>
>> $ (blockdev --setbsz 2048 /dev/stdin; blockdev --getbsz /dev/stdin) < /dev/loop0
>> 2048
> 
> Yes .. if that was documented anywhere, if it wasn't so obscure, and
> if it was useful for anything except changing a handle property that
> nothing uses.
> 
> There's an informative but unfortunately private exchange on the
> subject in this bug:
> https://bugzilla.redhat.com/show_bug.cgi?id=1002825
> 
> I have asked if we can make the contents of this bug public.
> 
> Rich.

For my own reference, the way I see it,
when accessing block devices Linux uses these block sizes:

physical	defined by the hardware device
logical		the smallest addressable unit used for the device
file system	ditto for the file system

--[gs]etbsz refers the _logical_ block size for the device.
Traditionally one had a physical of 512, but a larger logical size
of say 4096, with the size determining the standard I/O operations Vs
fragmentation tradeoff.

So why might one want to --setbsz?
I agree that would be fairly specialized.
I'm guessing that the logical size default is determined using
the device size, maxing out at the page size.
One might want to reduce that (temporarily), for odd sized devices,
where you might want to read/write slop at the end of the device?

But for low level access like this, one would probably use direct IO
to avoid the page cache (sized) reads/writes anyway.

I searched for uses of --setbz and noticed a couple:
Perf testing:
  https://lkml.org/lkml/2003/12/30/81
Functionality testing:
  http://www.linux-archive.org/device-mapper-development/699549-blockdev-fix-crash-when-block-size-changed-i-o-issued-simultaneously.html

So I'm not against removing it. If others wanted to leave it in,
then it would have to be documented that it was a transient setting.

thanks,
Pádraig.

Re: [PATCH] blockdev: Remove the --setbsz (set blocksize) option which has never worked.

[Karel Zak]

On Tue, Oct 08, 2013 at 04:56:23PM +0100, Pádraig Brady wrote:
> For my own reference, the way I see it,
> when accessing block devices Linux uses these block sizes:
> 
> physical	defined by the hardware device
> logical		the smallest addressable unit used for the device
> file system	ditto for the file system
> 
> --[gs]etbsz refers the _logical_ block size for the device.

 Well, let's be more pedantic ;-)

 The disks and raids provides I/O limits:

   physical block size  (BLKPBSZGET)
   logical block size   (BLKSSZGET)

 but kernel also uses 
 
   soft block size  BLKBSZGET
   
 this size is never smaller than logical size and (I guess) usually
 same as PAGE_SIZE. So it's really not *device* logical block size,
 it's more about the way how kernel uses the device.

> So why might one want to --setbsz?
> I agree that would be fairly specialized.

 Yes, it's practically useless in tool like blockdev(8).
 
> I searched for uses of --setbz and noticed a couple:
> Perf testing:
>   https://lkml.org/lkml/2003/12/30/81
> Functionality testing:
>   http://www.linux-archive.org/device-mapper-development/699549-blockdev-fix-crash-when-block-size-changed-i-o-issued-simultaneously.html

 Good point (it seems I was too rash to apply Rich's patch;-)

 Maybe we can keep --setbsz in the code for testing purpose and for
 backward compatibility, then all we need is describe the ioctl in the
 blockdev(8) man page.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH] blockdev: Remove the --setbsz (set blocksize) option which has never worked.

[Richard W.M. Jones]

On Tue, Oct 08, 2013 at 03:09:40PM +0100, Richard W.M. Jones wrote:
> There's an informative but unfortunately private exchange on the
> subject in this bug:
> https://bugzilla.redhat.com/show_bug.cgi?id=1002825
> 
> I have asked if we can make the contents of this bug public.

This bug should be public now.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

Re: [PATCH] blockdev: Remove the --setbsz (set blocksize) option which has never worked.

[Karel Zak]

On Tue, Oct 08, 2013 at 04:56:23PM +0100, Pádraig Brady wrote:
> So I'm not against removing it. If others wanted to leave it in,
> then it would have to be documented that it was a transient setting.

I have reverted Rich's patch and improved --setbsz docs.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com