On Wed, Jul 02, 2025 at 10:03:54AM +0100, John Garry wrote:
> On 01/07/2025 19:08, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@xxxxxxxxxx>
> >
> > Try to align the AG size to the maximum hardware atomic write unit so
> > that we can give users maximum flexibility in choosing an RWF_ATOMIC
> > write size.
>
>
> Regardless of comments below, FWIW:
>
> Reviewed-by: John Garry <john.g.garry@xxxxxxxxxx>
>
>
> >
> > Signed-off-by: "Darrick J. Wong" <djwong@xxxxxxxxxx>
> > ---
> > libxfs/topology.h | 6 ++++--
> > libxfs/topology.c | 36 ++++++++++++++++++++++++++++++++++++
> > mkfs/xfs_mkfs.c | 48 +++++++++++++++++++++++++++++++++++++++++++-----
> > 3 files changed, 83 insertions(+), 7 deletions(-)
> >
> >
> > diff --git a/libxfs/topology.h b/libxfs/topology.h
> > index 207a8a7f150556..f0ca65f3576e92 100644
> > --- a/libxfs/topology.h
> > +++ b/libxfs/topology.h
> > @@ -13,8 +13,10 @@
> > struct device_topology {
> > int logical_sector_size; /* logical sector size */
> > int physical_sector_size; /* physical sector size */
> > - int sunit; /* stripe unit */
> > - int swidth; /* stripe width */
> > + int sunit; /* stripe unit */
> > + int swidth; /* stripe width */
> > + int awu_min; /* min atomic write unit in bbcounts */
>
> awu_min is not really used, but, like the kernel code does, I suppose useful
> to store it
>
> > + int awu_max; /* max atomic write unit in bbcounts */
> > };
> > struct fs_topology {
> > diff --git a/libxfs/topology.c b/libxfs/topology.c
> > index 96ee74b61b30f5..7764687beac000 100644
> > --- a/libxfs/topology.c
> > +++ b/libxfs/topology.c
> > @@ -4,11 +4,18 @@
> > * All Rights Reserved.
> > */
> > +#ifdef OVERRIDE_SYSTEM_STATX
> > +#define statx sys_statx
> > +#endif
> > +#include <fcntl.h>
> > +#include <sys/stat.h>
> > +
> > #include "libxfs_priv.h"
> > #include "libxcmd.h"
> > #include <blkid/blkid.h>
> > #include "xfs_multidisk.h"
> > #include "libfrog/platform.h"
> > +#include "libfrog/statx.h"
> > #define TERABYTES(count, blog) ((uint64_t)(count) << (40 - (blog)))
> > #define GIGABYTES(count, blog) ((uint64_t)(count) << (30 - (blog)))
> > @@ -278,6 +285,34 @@ blkid_get_topology(
> > device);
> > }
> > +static void
> > +get_hw_atomic_writes_topology(
> > + struct libxfs_dev *dev,
> > + struct device_topology *dt)
> > +{
> > + struct statx sx;
> > + int fd;
> > + int ret;
> > +
> > + fd = open(dev->name, O_RDONLY);
> > + if (fd < 0)
> > + return;
> > +
> > + ret = statx(fd, "", AT_EMPTY_PATH, STATX_WRITE_ATOMIC, &sx);
> > + if (ret)
> > + goto out_close;
> > +
> > + if (!(sx.stx_mask & STATX_WRITE_ATOMIC))
> > + goto out_close;
> > +
> > + dt->awu_min = sx.stx_atomic_write_unit_min >> 9;
> > + dt->awu_max = max(sx.stx_atomic_write_unit_max_opt,
> > + sx.stx_atomic_write_unit_max) >> 9;
>
> for a bdev, stx_atomic_write_unit_max_opt should be zero
>
> However, I suppose some bdev could have hybrid atomic write support, just
> like xfs, so what you are doing looks good
Yeah, it's unlikely ever to happen but if (say) you had a loop device
backed by an xfs file then maybe it'd useful to pass through both atomic
write maxima.
Thanks for the review.
--D
