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

Shinichiro Kawasaki
2025-12-02T10:22:37Z

On Nov 25, 2025 / 09:42, Keith Busch wrote:
> 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.

Thanks for the confirmations. I have applied the patches. I took the liberty to
make follow-up commits for minor clean-ups, hoping they are fine for you.