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

[Bart Van Assche <bvanassche@acm.org>]

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?

> +    /// 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".

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
-----  ------------------------------------------------------------

> +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.

Thanks,

Bart.