Re: [PATCH 1/3] rust: block: introduce `kernel::block::mq` module

[Andreas Hindborg <nmi@metaspace.dk>]

Hi Bart,

Bart Van Assche <bvanassche@acm.org> writes:

> On 5/12/24 11:39, Andreas Hindborg wrote:
>> +    /// Set the logical block size of the device.
>> +    ///
>> +    /// This is the smallest unit the storage device can address. It is
>> +    /// typically 512 bytes.
>
> Hmm ... all block devices that I have encountered recently have a
> logical block size of 4096 bytes. Isn't this the preferred logical
> block size for SSDs and for SMR hard disks?

Yes, that is probably true. This text was lifted from the entry on the
sysfs attribute in `Documentation/ABI/stable/sysfs-block`, but maybe
that needs to be updated as well.

>
>> +    /// Set the physical block size of the device.
>> +    ///
>> +    /// This is the smallest unit a physical storage device can write
>> +    /// atomically. It is usually the same as the logical block size but may be
>> +    /// bigger. One example is SATA drives with 4KB sectors that expose a
>> +    /// 512-byte logical block size to the operating system.
>
> Please be consistent and change "4 KB sectors" into "4 KB physical block
> size".

OK, I will. I can CC the changes to
`Documentation/ABI/stable/sysfs-block` then'

>
> I think that the physical block size can also be smaller than the
> logical block size. From the SCSI SBC standard:
>
> Table 91 — LOGICAL BLOCKS PER PHYSICAL BLOCK EXPONENT field
> -----  ------------------------------------------------------------
> Code   Description
> -----  ------------------------------------------------------------
> 0      One or more physical blocks per logical block (the number of
>        physical blocks per logical block is not reported).
> n > 0  2**n logical blocks per physical block
> -----  ------------------------------------------------------------

How does that work? Would the drive do a read/modify/write internally?
Would that not make the physical block size as seen from the OS equal to
the smaller logical block size?

>
>> +impl<T: Operations, S: GenDiskState> GenDisk<T, S> {
>> +    /// Call to tell the block layer the capacity of the device in sectors (512B).
>
> Why to use any other unit than bytes in Rust block::mq APIs? sector_t
> was introduced before 64-bit CPUs became available to reduce the number
> of bytes required to represent offsets. I don't think that this is still
> a concern today. Hence my proposal to be consistent in the Rust block::mq API
> and to use bytes as the unit in all APIs.

I think that is very good idea. How do others feel about this?

BR Andreas