Re: [PATCH v3 04/13] ltp/fsx.c: Add atomic writes support to fsx

Darrick J. Wong
2025-07-23T14:57:36Z

On Tue, Jul 22, 2025 at 03:29:02PM +0530, Ojaswin Mujoo wrote:
> On Thu, Jul 17, 2025 at 09:17:47AM -0700, Darrick J. Wong wrote:
>
> <snip>
>
> > > +
> > > +/*
> > > + * Round down n to nearest power of 2.
> > > + * If n is already a power of 2, return n;
> > > + */
> > > +static int rounddown_pow_of_2(int n) {
> > > + int i = 0;
> > > +
> > > + if (is_power_of_2(n))
> > > + return n;
> > > +
> > > + for (; (1 << i) < n; i++);
> > > +
> > > + return 1 << (i - 1);
> > > +}
> > > +
> > > void
> > > dowrite(unsigned offset, unsigned size, int flags)
> > > {
> > > @@ -1081,6 +1113,27 @@ dowrite(unsigned offset, unsigned size, int flags)
> > > offset -= offset % writebdy;
> > > if (o_direct)
> > > size -= size % writebdy;
> > > + if (flags & RWF_ATOMIC) {
> > > + /* atomic write len must be inbetween awu_min and awu_max */
> > > + if (size < awu_min)
> > > + size = awu_min;
> > > + if (size > awu_max)
> > > + size = awu_max;
> > > +
> > > + /* atomic writes need power-of-2 sizes */
> > > + size = rounddown_pow_of_2(size);
> > > +
> > > + /* atomic writes need naturally aligned offsets */
> > > + offset -= offset % size;
> >
> > I don't think you should be modifying offset/size here. Normally for
> > fsx we do all the rounding of the file range in the switch statement
> > after the "calculate appropriate op to run" comment statement.
> >
> > --D
>
> Yes, I noticed that but then I saw we make size/offset adjustments in
> do write for writebdy and I wanted atomic writes adjustments to be done
> after that.

<nod> ok then, I forgot that we already tweak the file range for
write...
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> Regads,
> ojaswin
>
> >
> > > +
> > > + /* Skip the write if we are crossing max filesize */
> > > + if ((offset + size) > maxfilelen) {
> > > + if (!quiet && testcalls > simulatedopcount)
> > > + prt("skipping atomic write past maxfilelen\n");
> > > + log4(OP_WRITE_ATOMIC, offset, size, FL_SKIPPED);
> > > + return;
> > > + }
> > > + }
> > > if (size == 0) {
> > > if (!quiet && testcalls > simulatedopcount && !o_direct)
> > > prt("skipping zero size write\n");
> > > @@ -1088,7 +1141,10 @@ dowrite(unsigned offset, unsigned size, int flags)
> > > return;
> > > }
> > >
> > > - log4(OP_WRITE, offset, size, FL_NONE);
> > > + if (flags & RWF_ATOMIC)
> > > + log4(OP_WRITE_ATOMIC, offset, size, FL_NONE);
> > > + else
> > > + log4(OP_WRITE, offset, size, FL_NONE);
> > >
> > > gendata(original_buf, good_buf, offset, size);
> > > if (offset + size > file_size) {
> > > @@ -1108,8 +1164,9 @@ dowrite(unsigned offset, unsigned size, int flags)
> > > (monitorstart == -1 ||
> > > (offset + size > monitorstart &&
> > > (monitorend == -1 || offset <= monitorend))))))
> > > - prt("%lld write\t0x%x thru\t0x%x\t(0x%x bytes)\tdontcache=%d\n", testcalls,
> > > - offset, offset + size - 1, size, (flags & RWF_DONTCACHE) != 0);
> > > + prt("%lld write\t0x%x thru\t0x%x\t(0x%x bytes)\tdontcache=%d atomic_wr=%d\n", testcalls,
> > > + offset, offset + size - 1, size, (flags & RWF_DONTCACHE) != 0,
> > > + (flags & RWF_ATOMIC) != 0);
> > > iret = fsxwrite(fd, good_buf + offset, size, offset, flags);
> > > if (iret != size) {
> > > if (iret == -1)
> > > @@ -1785,6 +1842,30 @@ do_dedupe_range(unsigned offset, unsigned length, unsigned dest)
> > > }
> > > #endif
> > >
> > > +int test_atomic_writes(void) {
> > > + int ret;
> > > + struct statx stx;
> > > +
>