Re: [Nbd] Setting the physical block size

[Alex Bligh <alex@...872...>]

Goswin,

--On 2 March 2012 22:54:14 +0100 Goswin von Brederlow <goswin-v-b@...186...> 
wrote:

> Alex Bligh <alex@...872...> writes:
>
> > --On 1 March 2012 17:17:41 +0100 Goswin von Brederlow
> > <goswin-v-b@...186...> wrote:
> >
> > > There is an ioctl NBD_SET_BLKSIZE, which sets the lo->blksize. The only
> > ...
> > > Or did you mean that there should be a new ioctl NBD_SET_PHYS_BLKSIZE?
> >
> > No, I meant was there a non-nbd-specific block device ioctl to do this.
> > Getting changes into nbd-client userspace code is (quite correctly)
> > easier than getting them into the kernel.
>
> Ahh, no idea. I wouldn't expect those values to be settable from
> userspace for normal devices. They are something the driver sets to
> reflet the hardware capabilities. But can't hurt to ask around.

One problem with block sizes is that there are a zillion different
block sizes, with different and overlapping naming conventions.

So, there is the BLKBSZSET ioctl, which is commented to 'set the
logical block size'. What it actually does is call set_blocksize().

Does that do what you need?

What your patch does is calls blk_queue_physical_block_size. But that's
the 'physical' block size from the perspective of the queue, which is
one layer up. IE it's setting q->limits.physical_block_size, as
opposed to q->limits.logical_block_size.

I can't help thinking that must always be at least
a multiple of the 'real' block size one layer down.

set_blocksize() seems to control the block size that an fs would use
and in turn it's looking at bdev_logical_block_size(bdev)

> On another note setting the physical block size does not prevent the
> kernel to send requests aligned and sized to multiples of the logical
> block size. Setting the logical block size to > 512 means that O_DIRECT
> access needs to also use blocks >512 bytes. So for example "dd
> if=/dev/zero of=/dev/nbd0 oflag=direct" fails but works if you add
> "bs=4k".
>
> Can anyone think of other reasons why setting the logical block size
> might be bad?

More that I'm not sure we are necessarily setting the right thing.

If we are trying to set a block size specified by the /client/, I
think we should be using the existing ioctl, and set_blocksize(),
rather than getting down lower. After all, other block sizes will
work.

If we are trying to set a block size specified by the /server/ through
negotiation (i.e. 'this is the smallest blocksize that will actually
work') I think that corresponds to the bdev_logical_block_size
(or possibly the physical equivalent).

I think you are doing the former, in which case I think we can just
use the ioctl. If you want reason why:
1. No kernel changes, and you will thus get your patch in far far
  quicker.
2. Because otherwise it's not possible to change it back to a lower
  one with the ioctl, which I'd expect to work

For full disclosure, the plethora of different blocksize settings confuse
the hell out of me, so I might be wrong.

--
Alex Bligh