Re: udf: allow implicit blocksize specification during mount

Re: udf: allow implicit blocksize specification during mount

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 1107 bytes --]

Hi! I found that following UDF patch was included into linus tree:
https://patchwork.kernel.org/patch/9524557/

It is really a good improvement to recognize UDF file system which have 
block size different from disk sector size and also different from 2048.

But should not detection on 4K native disks (4096/4096) try to also use 
block size of 512 bytes? Because current loop is from logical sector 
size to 4096.

Most of current hard drivers are 512/4096 (logical/physical), but there 
are available USB based SATA controllers which via USB mass storage mode 
reports logical block size value from physical block size of SATA disk. 
So via such SATA controller computer see 4096/4096 disk and is unable to 
detect UDF file system if was formatted with 512 block size.

Anyway, macro UDF_DEFAULT_BLOCKSIZE is not used in udf source code 
anymore, so it can be deleted. I think its name was incorrect as default 
block size for UDF should be logical block size of disk, not hardcoded 
value 2048 which is logical block size for optical media.

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: udf: allow implicit blocksize specification during mount

[Jan Kara]

Hi,

On Mon 12-06-17 22:40:14, Pali Rohár wrote:
> Hi! I found that following UDF patch was included into linus tree:
> https://patchwork.kernel.org/patch/9524557/
> 
> It is really a good improvement to recognize UDF file system which have 
> block size different from disk sector size and also different from 2048.
> 
> But should not detection on 4K native disks (4096/4096) try to also use 
> block size of 512 bytes? Because current loop is from logical sector 
> size to 4096.

By definition, bdev_logical_block_size() is the smallest block size a
device can support. So if it is larger than 512, the device driver had
explicitely declared that it cannot handle smaller blocks... Look at the
comment before "blk_queue_logical_block_size":

/**
 * blk_queue_logical_block_size - set logical block size for the queue
 * @q:  the request queue for the device
 * @size:  the logical block size, in bytes
 *
 * Description:
 *   This should be set to the lowest possible block size that the
 *   storage device can address.  The default of 512 covers most
 *   hardware.
 **/


> Anyway, macro UDF_DEFAULT_BLOCKSIZE is not used in udf source code 
> anymore, so it can be deleted. I think its name was incorrect as default 
> block size for UDF should be logical block size of disk, not hardcoded 
> value 2048 which is logical block size for optical media.

Thanks. Removed.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: udf: allow implicit blocksize specification during mount

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 2004 bytes --]

On Tuesday 13 June 2017 14:59:55 Jan Kara wrote:
> Hi,
> 
> On Mon 12-06-17 22:40:14, Pali Rohár wrote:
> > Hi! I found that following UDF patch was included into linus tree:
> > https://patchwork.kernel.org/patch/9524557/
> > 
> > It is really a good improvement to recognize UDF file system which
> > have block size different from disk sector size and also different
> > from 2048.
> > 
> > But should not detection on 4K native disks (4096/4096) try to also
> > use block size of 512 bytes? Because current loop is from logical
> > sector size to 4096.
> 
> By definition, bdev_logical_block_size() is the smallest block size a
> device can support. So if it is larger than 512, the device driver
> had explicitely declared that it cannot handle smaller blocks...

Ok, but it is a really problem when trying to read data from filesystem 
which has smaller blocks as the smallest block size of a device?

In the worst case filesystem driver needs to read 512 bytes, but device 
can send only block of 4096 bytes (as it does not support smaller 
block). Driver receives 4096 bytes, then it process just first 512 bytes 
and do not care about remaining data...

> Look at the comment before "blk_queue_logical_block_size":
> 
> /**
>  * blk_queue_logical_block_size - set logical block size for the
> queue * @q:  the request queue for the device
>  * @size:  the logical block size, in bytes
>  *
>  * Description:
>  *   This should be set to the lowest possible block size that the
>  *   storage device can address.  The default of 512 covers most
>  *   hardware.
>  **/
> 
> > Anyway, macro UDF_DEFAULT_BLOCKSIZE is not used in udf source code
> > anymore, so it can be deleted. I think its name was incorrect as
> > default block size for UDF should be logical block size of disk,
> > not hardcoded value 2048 which is logical block size for optical
> > media.
> 
> Thanks. Removed.
> 
> 								Honza


-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: udf: allow implicit blocksize specification during mount

[Jan Kara]

On Wed 14-06-17 21:36:45, Pali Rohár wrote:
> On Tuesday 13 June 2017 14:59:55 Jan Kara wrote:
> > Hi,
> > 
> > On Mon 12-06-17 22:40:14, Pali Rohár wrote:
> > > Hi! I found that following UDF patch was included into linus tree:
> > > https://patchwork.kernel.org/patch/9524557/
> > > 
> > > It is really a good improvement to recognize UDF file system which
> > > have block size different from disk sector size and also different
> > > from 2048.
> > > 
> > > But should not detection on 4K native disks (4096/4096) try to also
> > > use block size of 512 bytes? Because current loop is from logical
> > > sector size to 4096.
> > 
> > By definition, bdev_logical_block_size() is the smallest block size a
> > device can support. So if it is larger than 512, the device driver
> > had explicitely declared that it cannot handle smaller blocks...
> 
> Ok, but it is a really problem when trying to read data from filesystem 
> which has smaller blocks as the smallest block size of a device?
> 
> In the worst case filesystem driver needs to read 512 bytes, but device 
> can send only block of 4096 bytes (as it does not support smaller 
> block). Driver receives 4096 bytes, then it process just first 512 bytes 
> and do not care about remaining data...

Well, as much as I agree this is possible in principle, the block layer,
block device page cache etc. don't handle this so it would be a non-trivial
effort to support this.

								Honza

-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: udf: allow implicit blocksize specification during mount

[Pali Rohár]

On Thursday 15 June 2017 10:34:27 Jan Kara wrote:
> On Wed 14-06-17 21:36:45, Pali Rohár wrote:
> > On Tuesday 13 June 2017 14:59:55 Jan Kara wrote:
> > > Hi,
> > > 
> > > On Mon 12-06-17 22:40:14, Pali Rohár wrote:
> > > > Hi! I found that following UDF patch was included into linus tree:
> > > > https://patchwork.kernel.org/patch/9524557/
> > > > 
> > > > It is really a good improvement to recognize UDF file system which
> > > > have block size different from disk sector size and also different
> > > > from 2048.
> > > > 
> > > > But should not detection on 4K native disks (4096/4096) try to also
> > > > use block size of 512 bytes? Because current loop is from logical
> > > > sector size to 4096.
> > > 
> > > By definition, bdev_logical_block_size() is the smallest block size a
> > > device can support. So if it is larger than 512, the device driver
> > > had explicitely declared that it cannot handle smaller blocks...
> > 
> > Ok, but it is a really problem when trying to read data from filesystem 
> > which has smaller blocks as the smallest block size of a device?
> > 
> > In the worst case filesystem driver needs to read 512 bytes, but device 
> > can send only block of 4096 bytes (as it does not support smaller 
> > block). Driver receives 4096 bytes, then it process just first 512 bytes 
> > and do not care about remaining data...
> 
> Well, as much as I agree this is possible in principle, the block layer,
> block device page cache etc. don't handle this so it would be a non-trivial
> effort to support this.

Ok, so it is not a problem in UDF driver nor hardware, just it is
limitation of kernel block layer which do not support it.

-- 
Pali Rohár
pali.rohar@gmail.com