]> git.ipfire.org Git - thirdparty/xfsprogs-dev.git/commitdiff
libxfs: refactor the fs_topology structure
authorChristoph Hellwig <hch@lst.de>
Mon, 15 Apr 2024 23:07:46 +0000 (16:07 -0700)
committerDarrick J. Wong <djwong@kernel.org>
Wed, 17 Apr 2024 21:06:27 +0000 (14:06 -0700)
fs_topology is a mess that mixes up data and RT device reporting,
and to make things worse reuses lsectorsize for the logical sector
size while other parts of xfsprogs use it for the log sector size.

Split out a device_topology structure that reports the topology for
one device and embedded two of them into the fs_topology struture,
and pass them directly to blkid_get_topology.

Rename the sector size members to be more explicit, and move some
of the sanity checking from mkfs into the topology helpers.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
libxfs/topology.c
libxfs/topology.h
mkfs/xfs_mkfs.c
repair/sb.c

index 4515d238d5e01ee7da7e7fa667d483d932bc804f..63f0b96a56d0a0d8f155ea421d4644ce1b54d349 100644 (file)
@@ -170,18 +170,14 @@ out:
        return ret;
 }
 
-static void blkid_get_topology(
-       const char      *device,
-       int             *sunit,
-       int             *swidth,
-       int             *lsectorsize,
-       int             *psectorsize,
-       int             force_overwrite)
+static void
+blkid_get_topology(
+       const char              *device,
+       struct device_topology  *dt,
+       int                     force_overwrite)
 {
-
        blkid_topology tp;
        blkid_probe pr;
-       unsigned long val;
        struct stat statbuf;
 
        /* can't get topology info from a file */
@@ -200,31 +196,28 @@ static void blkid_get_topology(
        if (!tp)
                goto out_free_probe;
 
-       val = blkid_topology_get_logical_sector_size(tp);
-       *lsectorsize = val;
-       val = blkid_topology_get_physical_sector_size(tp);
-       *psectorsize = val;
-       val = blkid_topology_get_minimum_io_size(tp);
-       *sunit = val;
-       val = blkid_topology_get_optimal_io_size(tp);
-       *swidth = val;
+       dt->logical_sector_size = blkid_topology_get_logical_sector_size(tp);
+       dt->physical_sector_size = blkid_topology_get_physical_sector_size(tp);
+       dt->sunit = blkid_topology_get_minimum_io_size(tp);
+       dt->swidth = blkid_topology_get_optimal_io_size(tp);
 
        /*
         * If the reported values are the same as the physical sector size
         * do not bother to report anything.  It will only cause warnings
         * if people specify larger stripe units or widths manually.
         */
-       if (*sunit == *psectorsize || *swidth == *psectorsize) {
-               *sunit = 0;
-               *swidth = 0;
+       if (dt->sunit == dt->physical_sector_size ||
+           dt->swidth == dt->physical_sector_size) {
+               dt->sunit = 0;
+               dt->swidth = 0;
        }
 
        /*
         * Blkid reports the information in terms of bytes, but we want it in
         * terms of 512 bytes blocks (only to convert it to bytes later..)
         */
-       *sunit = *sunit >> 9;
-       *swidth = *swidth >> 9;
+       dt->sunit >>= 9;
+       dt->swidth >>= 9;
 
        if (blkid_topology_get_alignment_offset(tp) != 0) {
                fprintf(stderr,
@@ -238,7 +231,7 @@ static void blkid_get_topology(
                        exit(EXIT_FAILURE);
                }
                /* Do not use physical sector size if the device is misaligned */
-               *psectorsize = *lsectorsize;
+               dt->physical_sector_size = dt->logical_sector_size;
        }
 
        blkid_free_probe(pr);
@@ -251,47 +244,61 @@ out_free_probe:
                device);
 }
 
-void
-get_topology(
-       struct libxfs_init      *xi,
-       struct fs_topology      *ft,
+static void
+get_device_topology(
+       struct libxfs_dev       *dev,
+       struct device_topology  *dt,
        int                     force_overwrite)
 {
-       struct stat statbuf;
+       struct stat             st;
+
+       /*
+        * Nothing to do if this particular subvolume doesn't exist.
+        */
+       if (!dev->name)
+               return;
 
        /*
         * If our target is a regular file, use platform_findsizes
         * to try to obtain the underlying filesystem's requirements
         * for direct IO; we'll set our sector size to that if possible.
         */
-       if (xi->data.isfile ||
-           (!stat(xi->data.name, &statbuf) && S_ISREG(statbuf.st_mode))) {
-               int fd;
+       if (dev->isfile || (!stat(dev->name, &st) && S_ISREG(st.st_mode))) {
                int flags = O_RDONLY;
                long long dummy;
+               int fd;
 
                /* with xi->disfile we may not have the file yet! */
-               if (xi->data.isfile)
+               if (dev->isfile)
                        flags |= O_CREAT;
 
-               fd = open(xi->data.name, flags, 0666);
+               fd = open(dev->name, flags, 0666);
                if (fd >= 0) {
-                       platform_findsizes(xi->data.name, fd, &dummy,
-                                       &ft->lsectorsize);
+                       platform_findsizes(dev->name, fd, &dummy,
+                                       &dt->logical_sector_size);
                        close(fd);
-                       ft->psectorsize = ft->lsectorsize;
-               } else
-                       ft->psectorsize = ft->lsectorsize = BBSIZE;
+               } else {
+                       dt->logical_sector_size = BBSIZE;
+               }
        } else {
-               blkid_get_topology(xi->data.name, &ft->dsunit, &ft->dswidth,
-                                  &ft->lsectorsize, &ft->psectorsize,
-                                  force_overwrite);
+               blkid_get_topology(dev->name, dt, force_overwrite);
        }
 
-       if (xi->rt.name && !xi->rt.isfile) {
-               int sunit, lsectorsize, psectorsize;
+       ASSERT(dt->logical_sector_size);
 
-               blkid_get_topology(xi->rt.name, &sunit, &ft->rtswidth,
-                                  &lsectorsize, &psectorsize, force_overwrite);
-       }
+       /*
+        * Older kernels may not have physical/logical distinction.
+        */
+       if (!dt->physical_sector_size)
+               dt->physical_sector_size = dt->logical_sector_size;
+}
+
+void
+get_topology(
+       struct libxfs_init      *xi,
+       struct fs_topology      *ft,
+       int                     force_overwrite)
+{
+       get_device_topology(&xi->data, &ft->data, force_overwrite);
+       get_device_topology(&xi->rt, &ft->rt, force_overwrite);
 }
index 3a309a4dae05cae3e23d79171e748c1d446667fb..ba0c8f6696a74d0872dc17d56151acbda1da0717 100644 (file)
 /*
  * Device topology information.
  */
+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  */
+};
+
 struct fs_topology {
-       int     dsunit;         /* stripe unit - data subvolume */
-       int     dswidth;        /* stripe width - data subvolume */
-       int     rtswidth;       /* stripe width - rt subvolume */
-       int     lsectorsize;    /* logical sector size &*/
-       int     psectorsize;    /* physical sector size */
+       struct device_topology  data;
+       struct device_topology  rt;
 };
 
 void
index 1a0a71dbec781524d12c252624fea93311c69213..79d6eaa37a1a824fc5abb2f31f0ef5e9d578256e 100644 (file)
@@ -1986,31 +1986,24 @@ validate_sectorsize(
                 * than that, then we can use logical, but warn about the
                 * inefficiency.
                 *
-                * Set the topology sectors if they were not probed to the
-                * minimum supported sector size.
-                */
-               if (!ft->lsectorsize)
-                       ft->lsectorsize = dft->sectorsize;
-
-               /*
-                * Older kernels may not have physical/logical distinction.
-                *
                 * Some architectures have a page size > XFS_MAX_SECTORSIZE.
                 * In that case, a ramdisk or persistent memory device may
                 * advertise a physical sector size that is too big to use.
                 */
-               if (!ft->psectorsize || ft->psectorsize > XFS_MAX_SECTORSIZE)
-                       ft->psectorsize = ft->lsectorsize;
+               if (ft->data.physical_sector_size > XFS_MAX_SECTORSIZE) {
+                       ft->data.physical_sector_size =
+                               ft->data.logical_sector_size;
+               }
 
-               cfg->sectorsize = ft->psectorsize;
+               cfg->sectorsize = ft->data.physical_sector_size;
                if (cfg->blocksize < cfg->sectorsize &&
-                   cfg->blocksize >= ft->lsectorsize) {
+                   cfg->blocksize >= ft->data.logical_sector_size) {
                        fprintf(stderr,
 _("specified blocksize %d is less than device physical sector size %d\n"
   "switching to logical sector size %d\n"),
-                               cfg->blocksize, ft->psectorsize,
-                               ft->lsectorsize);
-                       cfg->sectorsize = ft->lsectorsize;
+                               cfg->blocksize, ft->data.physical_sector_size,
+                               ft->data.logical_sector_size);
+                       cfg->sectorsize = ft->data.logical_sector_size;
                }
        } else
                cfg->sectorsize = cli->sectorsize;
@@ -2031,9 +2024,9 @@ _("block size %d cannot be smaller than sector size %d\n"),
                usage();
        }
 
-       if (cfg->sectorsize < ft->lsectorsize) {
+       if (cfg->sectorsize < ft->data.logical_sector_size) {
                fprintf(stderr, _("illegal sector size %d; hw sector is %d\n"),
-                       cfg->sectorsize, ft->lsectorsize);
+                       cfg->sectorsize, ft->data.logical_sector_size);
                usage();
        }
 }
@@ -2455,7 +2448,7 @@ validate_rtextsize(
 
                if (!cfg->sb_feat.nortalign && !cli->xi->rt.isfile &&
                    !(!cli->rtsize && cli->xi->data.isfile))
-                       rswidth = ft->rtswidth;
+                       rswidth = ft->rt.swidth;
                else
                        rswidth = 0;
 
@@ -2700,13 +2693,14 @@ _("data stripe width (%lld) is too large of a multiple of the data stripe unit (
        /* if no stripe config set, use the device default */
        if (!dsunit) {
                /* Ignore nonsense from device report. */
-               if (!libxfs_validate_stripe_geometry(NULL, BBTOB(ft->dsunit),
-                               BBTOB(ft->dswidth), 0, true)) {
+               if (!libxfs_validate_stripe_geometry(NULL, BBTOB(ft->data.sunit),
+                               BBTOB(ft->data.swidth), 0, true)) {
                        fprintf(stderr,
 _("%s: Volume reports invalid stripe unit (%d) and stripe width (%d), ignoring.\n"),
-                               progname, BBTOB(ft->dsunit), BBTOB(ft->dswidth));
-                       ft->dsunit = 0;
-                       ft->dswidth = 0;
+                               progname,
+                               BBTOB(ft->data.sunit), BBTOB(ft->data.swidth));
+                       ft->data.sunit = 0;
+                       ft->data.swidth = 0;
                } else if (cfg->dblocks < GIGABYTES(1, cfg->blocklog)) {
                        /*
                         * Don't use automatic stripe detection if the device
@@ -2714,29 +2708,29 @@ _("%s: Volume reports invalid stripe unit (%d) and stripe width (%d), ignoring.\
                         * on such a small system are not worth the risk that
                         * we'll end up with an undersized log.
                         */
-                       if (ft->dsunit || ft->dswidth)
+                       if (ft->data.sunit || ft->data.swidth)
                                fprintf(stderr,
 _("%s: small data volume, ignoring data volume stripe unit %d and stripe width %d\n"),
-                                               progname, ft->dsunit,
-                                               ft->dswidth);
-                       ft->dsunit = 0;
-                       ft->dswidth = 0;
+                                               progname, ft->data.sunit,
+                                               ft->data.swidth);
+                       ft->data.sunit = 0;
+                       ft->data.swidth = 0;
                } else {
-                       dsunit = ft->dsunit;
-                       dswidth = ft->dswidth;
+                       dsunit = ft->data.sunit;
+                       dswidth = ft->data.swidth;
                        use_dev = true;
                }
        } else {
                /* check and warn if user-specified alignment is sub-optimal */
-               if (ft->dsunit && ft->dsunit != dsunit) {
+               if (ft->data.sunit && ft->data.sunit != dsunit) {
                        fprintf(stderr,
 _("%s: Specified data stripe unit %d is not the same as the volume stripe unit %d\n"),
-                               progname, dsunit, ft->dsunit);
+                               progname, dsunit, ft->data.sunit);
                }
-               if (ft->dswidth && ft->dswidth != dswidth) {
+               if (ft->data.swidth && ft->data.swidth != dswidth) {
                        fprintf(stderr,
 _("%s: Specified data stripe width %d is not the same as the volume stripe width %d\n"),
-                               progname, dswidth, ft->dswidth);
+                               progname, dswidth, ft->data.swidth);
                }
        }
 
index faf79d9d08350fbf57c463b4e6c042c98b0c7790..4b49c1b33c6c83d3e520ff3f6a84d13b06ed4587 100644 (file)
@@ -189,7 +189,7 @@ guess_default_geometry(
         * Use default block size (2^12)
         */
        blocklog = 12;
-       multidisk = ft.dswidth | ft.dsunit;
+       multidisk = ft.data.swidth | ft.data.sunit;
        dblocks = x->data.size >> (blocklog - BBSHIFT);
        calc_default_ag_geometry(blocklog, dblocks, multidisk,
                                 agsize, agcount);