From: Brian Foster <bfoster@redhat.com>
To: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 3/3] xfs: refactor the geometry structure filling function
Date: Fri, 5 Jan 2018 08:08:35 -0500 [thread overview]
Message-ID: <20180105130835.GC27973@bfoster.bfoster> (raw)
In-Reply-To: <151512359134.14363.8696043274247703808.stgit@magnolia>
On Thu, Jan 04, 2018 at 07:39:51PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
>
> Refactor the geometry structure filling function to use the superblock
> to fill the fields. While we're at it, make the function less indenty
> and use some whitespace to make the function easier to read.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
Nice cleanup...
> fs/xfs/libxfs/xfs_sb.c | 167 ++++++++++++++++++++++++++++--------------------
> fs/xfs/libxfs/xfs_sb.h | 5 +
> fs/xfs/xfs_ioctl.c | 4 +
> fs/xfs/xfs_ioctl32.c | 2 -
> 4 files changed, 103 insertions(+), 75 deletions(-)
>
>
> diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
> index 139517a..70e5126 100644
> --- a/fs/xfs/libxfs/xfs_sb.c
> +++ b/fs/xfs/libxfs/xfs_sb.c
> @@ -879,77 +879,104 @@ xfs_sync_sb(
>
> int
> xfs_fs_geometry(
> - xfs_mount_t *mp,
> - xfs_fsop_geom_t *geo,
> - int new_version)
> + struct xfs_sb *sbp,
> + struct xfs_fsop_geom *geo,
> + int struct_version)
> {
...
> + geo->version = XFS_FSOP_GEOM_VERSION;
> + geo->flags = XFS_FSOP_GEOM_FLAGS_NLINK |
> + XFS_FSOP_GEOM_FLAGS_DIRV2;
> +
> +
One whitespace line too many here..?
> + if (xfs_sb_version_hasattr(sbp))
> + geo->flags |= XFS_FSOP_GEOM_FLAGS_ATTR;
> +
Obviously just a nit... but IMO the extra whitespace between each of
these feature checks is a bit much. It actually reads cleaner to me to
kill off the extra lines between them. Just my .02 though..
...
> +
> + geo->rtsectsize = sbp->sb_blocksize;
> + geo->dirblocksize = 1 << (sbp->sb_blocklog + sbp->sb_dirblklog);
> +
Slightly unfortunate that we have the geo abstraction and can't use it
for a simple reporting case. Is there a larger (userspace?) reason mp
was dropped as a parameter or was that just a cleanup? If the latter,
perhaps we could continue to pass mp and create a local sbp pointer. If
the former, then oh well, not a big deal. :P
Brian
> + if (struct_version < 3)
> + return 0;
> +
> + if (xfs_sb_version_haslogv2(sbp))
> + geo->flags |= XFS_FSOP_GEOM_FLAGS_LOGV2;
> +
> + geo->logsunit = sbp->sb_logsunit;
>
> - memset(geo, 0, sizeof(*geo));
> -
> - geo->blocksize = mp->m_sb.sb_blocksize;
> - geo->rtextsize = mp->m_sb.sb_rextsize;
> - geo->agblocks = mp->m_sb.sb_agblocks;
> - geo->agcount = mp->m_sb.sb_agcount;
> - geo->logblocks = mp->m_sb.sb_logblocks;
> - geo->sectsize = mp->m_sb.sb_sectsize;
> - geo->inodesize = mp->m_sb.sb_inodesize;
> - geo->imaxpct = mp->m_sb.sb_imax_pct;
> - geo->datablocks = mp->m_sb.sb_dblocks;
> - geo->rtblocks = mp->m_sb.sb_rblocks;
> - geo->rtextents = mp->m_sb.sb_rextents;
> - geo->logstart = mp->m_sb.sb_logstart;
> - ASSERT(sizeof(geo->uuid) == sizeof(mp->m_sb.sb_uuid));
> - memcpy(geo->uuid, &mp->m_sb.sb_uuid, sizeof(mp->m_sb.sb_uuid));
> - if (new_version >= 2) {
> - geo->sunit = mp->m_sb.sb_unit;
> - geo->swidth = mp->m_sb.sb_width;
> - }
> - if (new_version >= 3) {
> - geo->version = XFS_FSOP_GEOM_VERSION;
> - geo->flags = XFS_FSOP_GEOM_FLAGS_NLINK |
> - XFS_FSOP_GEOM_FLAGS_DIRV2 |
> - (xfs_sb_version_hasattr(&mp->m_sb) ?
> - XFS_FSOP_GEOM_FLAGS_ATTR : 0) |
> - (xfs_sb_version_hasquota(&mp->m_sb) ?
> - XFS_FSOP_GEOM_FLAGS_QUOTA : 0) |
> - (xfs_sb_version_hasalign(&mp->m_sb) ?
> - XFS_FSOP_GEOM_FLAGS_IALIGN : 0) |
> - (xfs_sb_version_hasdalign(&mp->m_sb) ?
> - XFS_FSOP_GEOM_FLAGS_DALIGN : 0) |
> - (xfs_sb_version_hasextflgbit(&mp->m_sb) ?
> - XFS_FSOP_GEOM_FLAGS_EXTFLG : 0) |
> - (xfs_sb_version_hassector(&mp->m_sb) ?
> - XFS_FSOP_GEOM_FLAGS_SECTOR : 0) |
> - (xfs_sb_version_hasasciici(&mp->m_sb) ?
> - XFS_FSOP_GEOM_FLAGS_DIRV2CI : 0) |
> - (xfs_sb_version_haslazysbcount(&mp->m_sb) ?
> - XFS_FSOP_GEOM_FLAGS_LAZYSB : 0) |
> - (xfs_sb_version_hasattr2(&mp->m_sb) ?
> - XFS_FSOP_GEOM_FLAGS_ATTR2 : 0) |
> - (xfs_sb_version_hasprojid32bit(&mp->m_sb) ?
> - XFS_FSOP_GEOM_FLAGS_PROJID32 : 0) |
> - (xfs_sb_version_hascrc(&mp->m_sb) ?
> - XFS_FSOP_GEOM_FLAGS_V5SB : 0) |
> - (xfs_sb_version_hasftype(&mp->m_sb) ?
> - XFS_FSOP_GEOM_FLAGS_FTYPE : 0) |
> - (xfs_sb_version_hasfinobt(&mp->m_sb) ?
> - XFS_FSOP_GEOM_FLAGS_FINOBT : 0) |
> - (xfs_sb_version_hassparseinodes(&mp->m_sb) ?
> - XFS_FSOP_GEOM_FLAGS_SPINODES : 0) |
> - (xfs_sb_version_hasrmapbt(&mp->m_sb) ?
> - XFS_FSOP_GEOM_FLAGS_RMAPBT : 0) |
> - (xfs_sb_version_hasreflink(&mp->m_sb) ?
> - XFS_FSOP_GEOM_FLAGS_REFLINK : 0);
> - geo->logsectsize = xfs_sb_version_hassector(&mp->m_sb) ?
> - mp->m_sb.sb_logsectsize : BBSIZE;
> - geo->rtsectsize = mp->m_sb.sb_blocksize;
> - geo->dirblocksize = mp->m_dir_geo->blksize;
> - }
> - if (new_version >= 4) {
> - geo->flags |=
> - (xfs_sb_version_haslogv2(&mp->m_sb) ?
> - XFS_FSOP_GEOM_FLAGS_LOGV2 : 0);
> - geo->logsunit = mp->m_sb.sb_logsunit;
> - }
> return 0;
> }
> diff --git a/fs/xfs/libxfs/xfs_sb.h b/fs/xfs/libxfs/xfs_sb.h
> index a16632c..63dcd2a 100644
> --- a/fs/xfs/libxfs/xfs_sb.h
> +++ b/fs/xfs/libxfs/xfs_sb.h
> @@ -34,7 +34,8 @@ extern void xfs_sb_from_disk(struct xfs_sb *to, struct xfs_dsb *from);
> extern void xfs_sb_to_disk(struct xfs_dsb *to, struct xfs_sb *from);
> extern void xfs_sb_quota_from_disk(struct xfs_sb *sbp);
>
> -extern int xfs_fs_geometry(struct xfs_mount *mp, struct xfs_fsop_geom *geo,
> - int nversion);
> +#define XFS_FS_GEOM_MAX_STRUCT_VER (4)
> +extern int xfs_fs_geometry(struct xfs_sb *sbp, struct xfs_fsop_geom *geo,
> + int struct_version);
>
> #endif /* __XFS_SB_H__ */
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index 3015e17..89fb1eb 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -810,7 +810,7 @@ xfs_ioc_fsgeometry_v1(
> xfs_fsop_geom_t fsgeo;
> int error;
>
> - error = xfs_fs_geometry(mp, &fsgeo, 3);
> + error = xfs_fs_geometry(&mp->m_sb, &fsgeo, 3);
> if (error)
> return error;
>
> @@ -832,7 +832,7 @@ xfs_ioc_fsgeometry(
> xfs_fsop_geom_t fsgeo;
> int error;
>
> - error = xfs_fs_geometry(mp, &fsgeo, 4);
> + error = xfs_fs_geometry(&mp->m_sb, &fsgeo, 4);
> if (error)
> return error;
>
> diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
> index 66cc3cd..10fbde3 100644
> --- a/fs/xfs/xfs_ioctl32.c
> +++ b/fs/xfs/xfs_ioctl32.c
> @@ -67,7 +67,7 @@ xfs_compat_ioc_fsgeometry_v1(
> xfs_fsop_geom_t fsgeo;
> int error;
>
> - error = xfs_fs_geometry(mp, &fsgeo, 3);
> + error = xfs_fs_geometry(&mp->m_sb, &fsgeo, 3);
> if (error)
> return error;
> /* The 32-bit variant simply has some padding at the end */
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2018-01-05 13:08 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-01-05 3:39 [PATCH 0/3] xfs: refactors for xfsprogs Darrick J. Wong
2018-01-05 3:39 ` [PATCH 1/3] xfs: trace log reservations at mount time Darrick J. Wong
2018-01-05 13:08 ` Brian Foster
2018-01-05 3:39 ` [PATCH 2/3] xfs: hoist xfs_fs_geometry to libxfs Darrick J. Wong
2018-01-05 13:08 ` Brian Foster
2018-01-05 3:39 ` [PATCH 3/3] xfs: refactor the geometry structure filling function Darrick J. Wong
2018-01-05 13:08 ` Brian Foster [this message]
2018-01-05 17:25 ` Darrick J. Wong
2018-01-05 17:34 ` [PATCH v2 " Darrick J. Wong
2018-01-05 19:02 ` Brian Foster
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180105130835.GC27973@bfoster.bfoster \
--to=bfoster@redhat.com \
--cc=darrick.wong@oracle.com \
--cc=linux-xfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.