Re: [PATCHv2 1/2] blktests: test direct io offsets

[Keith Busch <kbusch@kernel.org>]

On Tue, Nov 25, 2025 at 11:26:31AM +0000, Shinichiro Kawasaki wrote:
> On Nov 19, 2025 / 11:54, Keith Busch wrote:
> > From: Keith Busch <kbusch@kernel.org>
> > 
> > Tests various direct IO memory and length alignments against the
> > device's queue limits reported from sysfs.
> > 
> > Signed-off-by: Keith Busch <kbusch@kernel.org>
> > ---
> [...]
> > diff --git a/src/dio-offsets.c b/src/dio-offsets.c
> > new file mode 100644
> > index 0000000..8e46091
> > --- /dev/null
> > +++ b/src/dio-offsets.c
> [...]
> > +static void init_buffers()
> > +{
> > +	unsigned long lb_mask = logical_block_size - 1;
> > +	int fd, ret;
> > +
> > +	buf_size = max_bytes * max_segments / 2;
> > +	if (buf_size < logical_block_size * max_segments)
> > +		err(EINVAL, "%s: logical block size is too big", __func__);
> > +
> > +	if (buf_size < logical_block_size * 1024 * 4)
> > +		buf_size = logical_block_size * 1024 * 4;
> > +
> > +	if (buf_size & lb_mask)
> > +		buf_size = (buf_size + lb_mask) & ~(lb_mask);
> 
> I investigated why this new test case fails with my QEMU SATA device and noticed
> that the device has 128MiB size. The calculation above sets buf_size = 192MiB.
> Then pwrite() in test_full_size_aligned() is called with 192MiB size and it was
> truncated to 128MiB. Then the following compare() check failed.
> 
> Is it okay to cap the buf_size with the device size? If so, I suggest the change
> below. With this change, the test case passes with the device. If you are okay
> with the change, I can fold it in when I apply the patch.

Yeah, totally fine. Much of the sizes were just made up as I went along,
but it should probably be programatically calculated based on the device
under test.

> > +#!/bin/bash
> 
> Is it okay to add any GPL SPDX license and your copyright here? It is not ideal
> that I add your copyright and license, but if you specify them in this thread, I
> will add the Link to this thread in the commit message and fold-in the specifed
> SPDX license and your copyright.

Yes, sorry, it should have been there. I added those to some files, but
missed this one.