Received-SPF: pass (google.com: domain of linux-kernel+bounces-177574-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
From: Andreas Hindborg <nmi@metaspace.dk>
To: Bart Van Assche <bvanassche@acm.org>
Cc: Jens Axboe <axboe@kernel.dk>,  Christoph Hellwig <hch@lst.de>,  Keith
 Busch <kbusch@kernel.org>,  Damien Le Moal <Damien.LeMoal@wdc.com>,
  Hannes Reinecke <hare@suse.de>,  Ming Lei <ming.lei@redhat.com>,
  "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,  Andreas
 Hindborg <a.hindborg@samsung.com>,  Wedson Almeida Filho
 <wedsonaf@gmail.com>,  Greg KH <gregkh@linuxfoundation.org>,  Matthew
 Wilcox <willy@infradead.org>,  Miguel Ojeda <ojeda@kernel.org>,  Alex
 Gaynor <alex.gaynor@gmail.com>,  Boqun Feng <boqun.feng@gmail.com>,  Gary
 Guo <gary@garyguo.net>,  =?utf-8?Q?Bj=C3=B6rn?= Roy Baron
 <bjorn3_gh@protonmail.com>,
  Benno Lossin <benno.lossin@proton.me>,  Alice Ryhl
 <aliceryhl@google.com>,  Chaitanya Kulkarni <chaitanyak@nvidia.com>,  Luis
 Chamberlain <mcgrof@kernel.org>,  Yexuan Yang <1182282462@bupt.edu.cn>,
  Sergio =?utf-8?Q?Gonz=C3=A1lez?= Collado <sergio.collado@gmail.com>,  Joel
 Granados
 <j.granados@samsung.com>,  "Pankaj Raghav (Samsung)"
 <kernel@pankajraghav.com>,  Daniel Gomez <da.gomez@samsung.com>,  Niklas
 Cassel <Niklas.Cassel@wdc.com>,  Philipp Stanner <pstanner@redhat.com>,
  Conor Dooley <conor@kernel.org>,  Johannes Thumshirn
 <Johannes.Thumshirn@wdc.com>,  Matias =?utf-8?Q?Bj=C3=B8rling?=
 <m@bjorling.me>,  open list
 <linux-kernel@vger.kernel.org>,  "rust-for-linux@vger.kernel.org"
 <rust-for-linux@vger.kernel.org>,  "lsf-pc@lists.linux-foundation.org"
 <lsf-pc@lists.linux-foundation.org>,  "gost.dev@samsung.com"
 <gost.dev@samsung.com>
Subject: Re: [PATCH 1/3] rust: block: introduce `kernel::block::mq` module
In-Reply-To: <1b618942-a0fe-45d9-90de-eede429e7284@acm.org> (Bart Van Assche's
	message of "Mon, 13 May 2024 06:22:31 -0600")
References: <20240512183950.1982353-1-nmi@metaspace.dk>
	<20240512183950.1982353-2-nmi@metaspace.dk>
	<1b618942-a0fe-45d9-90de-eede429e7284@acm.org>
User-Agent: mu4e 1.12.4; emacs 29.3
Date: Mon, 13 May 2024 06:48:18 -0600
Message-ID: <87r0e5j2st.fsf@metaspace.dk>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable


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 bu=
t 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 =E2=80=94 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 sect=
ors (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