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

Bart Van Assche
2024-05-13T12:23:04Z

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.