All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] xfs: refactors for xfsprogs
@ 2018-01-05  3:39 Darrick J. Wong
  2018-01-05  3:39 ` [PATCH 1/3] xfs: trace log reservations at mount time Darrick J. Wong
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Darrick J. Wong @ 2018-01-05  3:39 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

Hi all,

This is a short patch series of libxfs refactors mostly done for the
benefit of debugging purposes and fixing things in xfsprogs:

The first patch reports transaction reservation data via tracepoints;
eventually xfs_db will grow a similar command so that one can compare
transaction reservations to debug "log minimum size checks fail on
freshly mkfs'd fs" problems.

Patches 2 & 3 move the xfs_fsop_geometry generator function into
libsxfs so that xfsprogs can have common functions to generate the
geometry from a superblock and print the geometry.  Eventually I'll
teach xfs_db, mkfs, and xfs_spaceman to query fs geometry (either
directly or via ioctl) and then pretty print the same information.

--D

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/3] xfs: trace log reservations at mount time
  2018-01-05  3:39 [PATCH 0/3] xfs: refactors for xfsprogs Darrick J. Wong
@ 2018-01-05  3:39 ` 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  3:39 ` [PATCH 3/3] xfs: refactor the geometry structure filling function Darrick J. Wong
  2 siblings, 1 reply; 10+ messages in thread
From: Darrick J. Wong @ 2018-01-05  3:39 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

At each mount, emit the transaction reservation type information via
tracepoints.  This makes it easier to compare the log reservation info
calculated by the kernel and xfsprogs so that we can more easily diagnose
minimum log size failures on freshly formatted filesystems.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_log_rlimit.c |    2 +-
 fs/xfs/libxfs/xfs_shared.h     |    3 +++
 fs/xfs/xfs_trace.h             |   26 ++++++++++++++++++++++++++
 fs/xfs/xfs_trans.c             |   22 ++++++++++++++++++++++
 4 files changed, 52 insertions(+), 1 deletion(-)


diff --git a/fs/xfs/libxfs/xfs_log_rlimit.c b/fs/xfs/libxfs/xfs_log_rlimit.c
index c105979..cc4cbe2 100644
--- a/fs/xfs/libxfs/xfs_log_rlimit.c
+++ b/fs/xfs/libxfs/xfs_log_rlimit.c
@@ -55,7 +55,7 @@ xfs_log_calc_max_attrsetm_res(
  * the maximum one in terms of the pre-calculated values which were done
  * at mount time.
  */
-STATIC void
+void
 xfs_log_get_max_trans_res(
 	struct xfs_mount	*mp,
 	struct xfs_trans_res	*max_resp)
diff --git a/fs/xfs/libxfs/xfs_shared.h b/fs/xfs/libxfs/xfs_shared.h
index 67ccb1a..d0b84da 100644
--- a/fs/xfs/libxfs/xfs_shared.h
+++ b/fs/xfs/libxfs/xfs_shared.h
@@ -76,6 +76,9 @@ struct xfs_log_item_desc {
 int	xfs_log_calc_unit_res(struct xfs_mount *mp, int unit_bytes);
 int	xfs_log_calc_minimum_size(struct xfs_mount *);
 
+struct xfs_trans_res;
+void	xfs_log_get_max_trans_res(struct xfs_mount *mp,
+				  struct xfs_trans_res *max_resp);
 
 /*
  * Values for t_flags.
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index 9235b2c..b6251f8 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -3313,6 +3313,32 @@ DEFINE_GETFSMAP_EVENT(xfs_getfsmap_low_key);
 DEFINE_GETFSMAP_EVENT(xfs_getfsmap_high_key);
 DEFINE_GETFSMAP_EVENT(xfs_getfsmap_mapping);
 
+TRACE_EVENT(xfs_trans_resv_calc,
+	TP_PROTO(struct xfs_mount *mp, unsigned int type,
+		 struct xfs_trans_res *res),
+	TP_ARGS(mp, type, res),
+	TP_STRUCT__entry(
+		__field(dev_t, dev)
+		__field(int, type)
+		__field(uint, logres)
+		__field(int, logcount)
+		__field(int, logflags)
+	),
+	TP_fast_assign(
+		__entry->dev = mp->m_super->s_dev;
+		__entry->type = type;
+		__entry->logres = res->tr_logres;
+		__entry->logcount = res->tr_logcount;
+		__entry->logflags = res->tr_logflags;
+	),
+	TP_printk("dev %d:%d type %d logres %u logcount %d flags 0x%x",
+		  MAJOR(__entry->dev), MINOR(__entry->dev),
+		  __entry->type,
+		  __entry->logres,
+		  __entry->logcount,
+		  __entry->logflags)
+);
+
 #endif /* _TRACE_XFS_H */
 
 #undef TRACE_INCLUDE_PATH
diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
index a87f657..86f92df 100644
--- a/fs/xfs/xfs_trans.c
+++ b/fs/xfs/xfs_trans.c
@@ -35,6 +35,27 @@
 kmem_zone_t	*xfs_trans_zone;
 kmem_zone_t	*xfs_log_item_desc_zone;
 
+#if defined(CONFIG_TRACEPOINTS)
+static void
+xfs_trans_trace_reservations(
+	struct xfs_mount	*mp)
+{
+	struct xfs_trans_res	resv;
+	struct xfs_trans_res	*res;
+	struct xfs_trans_res	*end_res;
+	int			i;
+
+	res = (struct xfs_trans_res *)M_RES(mp);
+	end_res = (struct xfs_trans_res *)(M_RES(mp) + 1);
+	for (i = 0; res < end_res; i++, res++)
+		trace_xfs_trans_resv_calc(mp, i, res);
+	xfs_log_get_max_trans_res(mp, &resv);
+	trace_xfs_trans_resv_calc(mp, -1, &resv);
+}
+#else
+# define xfs_trans_trace_reservations(mp)
+#endif
+
 /*
  * Initialize the precomputed transaction reservation values
  * in the mount structure.
@@ -44,6 +65,7 @@ xfs_trans_init(
 	struct xfs_mount	*mp)
 {
 	xfs_trans_resv_calc(mp, M_RES(mp));
+	xfs_trans_trace_reservations(mp);
 }
 
 /*


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/3] xfs: hoist xfs_fs_geometry to libxfs
  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  3:39 ` 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
  2 siblings, 1 reply; 10+ messages in thread
From: Darrick J. Wong @ 2018-01-05  3:39 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Move xfs_fs_geometry to libxfs so that we can clean up the fs geometry
reporting in xfsprogs.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_sb.c |   79 ++++++++++++++++++++++++++++++++++++++++++++++++
 fs/xfs/libxfs/xfs_sb.h |    3 ++
 fs/xfs/xfs_fsops.c     |   77 -----------------------------------------------
 fs/xfs/xfs_fsops.h     |    1 -
 fs/xfs/xfs_ioctl.c     |    1 +
 fs/xfs/xfs_ioctl32.c   |    1 +
 6 files changed, 84 insertions(+), 78 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
index 63e0331..139517a 100644
--- a/fs/xfs/libxfs/xfs_sb.c
+++ b/fs/xfs/libxfs/xfs_sb.c
@@ -40,6 +40,8 @@
 #include "xfs_rmap_btree.h"
 #include "xfs_bmap.h"
 #include "xfs_refcount_btree.h"
+#include "xfs_da_format.h"
+#include "xfs_da_btree.h"
 
 /*
  * Physical superblock buffer manipulations. Shared with libxfs in userspace.
@@ -874,3 +876,80 @@ xfs_sync_sb(
 		xfs_trans_set_sync(tp);
 	return xfs_trans_commit(tp);
 }
+
+int
+xfs_fs_geometry(
+	xfs_mount_t		*mp,
+	xfs_fsop_geom_t		*geo,
+	int			new_version)
+{
+
+	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 961e647..a16632c 100644
--- a/fs/xfs/libxfs/xfs_sb.h
+++ b/fs/xfs/libxfs/xfs_sb.h
@@ -34,4 +34,7 @@ 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);
+
 #endif	/* __XFS_SB_H__ */
diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
index 60a2e12..84d7383 100644
--- a/fs/xfs/xfs_fsops.c
+++ b/fs/xfs/xfs_fsops.c
@@ -49,83 +49,6 @@
  * File system operations
  */
 
-int
-xfs_fs_geometry(
-	xfs_mount_t		*mp,
-	xfs_fsop_geom_t		*geo,
-	int			new_version)
-{
-
-	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;
-}
-
 static struct xfs_buf *
 xfs_growfs_get_hdr_buf(
 	struct xfs_mount	*mp,
diff --git a/fs/xfs/xfs_fsops.h b/fs/xfs/xfs_fsops.h
index 2954c13..20484ed 100644
--- a/fs/xfs/xfs_fsops.h
+++ b/fs/xfs/xfs_fsops.h
@@ -18,7 +18,6 @@
 #ifndef __XFS_FSOPS_H__
 #define	__XFS_FSOPS_H__
 
-extern int xfs_fs_geometry(xfs_mount_t *mp, xfs_fsop_geom_t *geo, int nversion);
 extern int xfs_growfs_data(xfs_mount_t *mp, xfs_growfs_data_t *in);
 extern int xfs_growfs_log(xfs_mount_t *mp, xfs_growfs_log_t *in);
 extern int xfs_fs_counts(xfs_mount_t *mp, xfs_fsop_counts_t *cnt);
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 20dc65f..3015e17 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -45,6 +45,7 @@
 #include <linux/fsmap.h>
 #include "xfs_fsmap.h"
 #include "scrub/xfs_scrub.h"
+#include "xfs_sb.h"
 
 #include <linux/capability.h>
 #include <linux/cred.h>
diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
index 35c79e2..66cc3cd 100644
--- a/fs/xfs/xfs_ioctl32.c
+++ b/fs/xfs/xfs_ioctl32.c
@@ -37,6 +37,7 @@
 #include "xfs_ioctl.h"
 #include "xfs_ioctl32.h"
 #include "xfs_trace.h"
+#include "xfs_sb.h"
 
 #define  _NATIVE_IOC(cmd, type) \
 	  _IOC(_IOC_DIR(cmd), _IOC_TYPE(cmd), _IOC_NR(cmd), sizeof(type))


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/3] xfs: refactor the geometry structure filling function
  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  3:39 ` [PATCH 2/3] xfs: hoist xfs_fs_geometry to libxfs Darrick J. Wong
@ 2018-01-05  3:39 ` Darrick J. Wong
  2018-01-05 13:08   ` Brian Foster
  2018-01-05 17:34   ` [PATCH v2 " Darrick J. Wong
  2 siblings, 2 replies; 10+ messages in thread
From: Darrick J. Wong @ 2018-01-05  3:39 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

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>
---
 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)
 {
+	memset(geo, 0, sizeof(struct xfs_fsop_geom));
+
+	geo->blocksize = sbp->sb_blocksize;
+	geo->rtextsize = sbp->sb_rextsize;
+	geo->agblocks = sbp->sb_agblocks;
+	geo->agcount = sbp->sb_agcount;
+	geo->logblocks = sbp->sb_logblocks;
+	geo->sectsize = sbp->sb_sectsize;
+	geo->inodesize = sbp->sb_inodesize;
+	geo->imaxpct = sbp->sb_imax_pct;
+	geo->datablocks = sbp->sb_dblocks;
+	geo->rtblocks = sbp->sb_rblocks;
+	geo->rtextents = sbp->sb_rextents;
+	geo->logstart = sbp->sb_logstart;
+	BUILD_BUG_ON(sizeof(geo->uuid) != sizeof(sbp->sb_uuid));
+	memcpy(geo->uuid, &sbp->sb_uuid, sizeof(sbp->sb_uuid));
+
+	if (struct_version < 2)
+		return 0;
+
+	geo->sunit = sbp->sb_unit;
+	geo->swidth = sbp->sb_width;
+
+	if (struct_version < 3)
+		return 0;
+
+	geo->version = XFS_FSOP_GEOM_VERSION;
+	geo->flags = XFS_FSOP_GEOM_FLAGS_NLINK |
+		     XFS_FSOP_GEOM_FLAGS_DIRV2;
+
+
+	if (xfs_sb_version_hasattr(sbp))
+		geo->flags |= XFS_FSOP_GEOM_FLAGS_ATTR;
+
+	if (xfs_sb_version_hasquota(sbp))
+		geo->flags |= XFS_FSOP_GEOM_FLAGS_QUOTA;
+
+	if (xfs_sb_version_hasalign(sbp))
+		geo->flags |= XFS_FSOP_GEOM_FLAGS_IALIGN;
+
+	if (xfs_sb_version_hasdalign(sbp))
+		geo->flags |= XFS_FSOP_GEOM_FLAGS_DALIGN;
+
+	if (xfs_sb_version_hasextflgbit(sbp))
+		geo->flags |= XFS_FSOP_GEOM_FLAGS_EXTFLG;
+
+	if (xfs_sb_version_hassector(sbp))
+		geo->flags |= XFS_FSOP_GEOM_FLAGS_SECTOR;
+
+	if (xfs_sb_version_hasasciici(sbp))
+		geo->flags |= XFS_FSOP_GEOM_FLAGS_DIRV2CI;
+
+	if (xfs_sb_version_haslazysbcount(sbp))
+		geo->flags |= XFS_FSOP_GEOM_FLAGS_LAZYSB;
+
+	if (xfs_sb_version_hasattr2(sbp))
+		geo->flags |= XFS_FSOP_GEOM_FLAGS_ATTR2;
+
+	if (xfs_sb_version_hasprojid32bit(sbp))
+		geo->flags |= XFS_FSOP_GEOM_FLAGS_PROJID32;
+
+	if (xfs_sb_version_hascrc(sbp))
+		geo->flags |= XFS_FSOP_GEOM_FLAGS_V5SB;
+
+	if (xfs_sb_version_hasftype(sbp))
+		geo->flags |= XFS_FSOP_GEOM_FLAGS_FTYPE;
+
+	if (xfs_sb_version_hasfinobt(sbp))
+		geo->flags |= XFS_FSOP_GEOM_FLAGS_FINOBT;
+
+	if (xfs_sb_version_hassparseinodes(sbp))
+		geo->flags |= XFS_FSOP_GEOM_FLAGS_SPINODES;
+
+	if (xfs_sb_version_hasrmapbt(sbp))
+		geo->flags |= XFS_FSOP_GEOM_FLAGS_RMAPBT;
+
+	if (xfs_sb_version_hasreflink(sbp))
+		geo->flags |= XFS_FSOP_GEOM_FLAGS_REFLINK;
+
+	if (xfs_sb_version_hassector(sbp))
+		geo->logsectsize = sbp->sb_logsectsize;
+	else
+		geo->logsectsize = BBSIZE;
+
+	geo->rtsectsize = sbp->sb_blocksize;
+	geo->dirblocksize = 1 << (sbp->sb_blocklog + sbp->sb_dirblklog);
+
+	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 */


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] xfs: trace log reservations at mount time
  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
  0 siblings, 0 replies; 10+ messages in thread
From: Brian Foster @ 2018-01-05 13:08 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs

On Thu, Jan 04, 2018 at 07:39:27PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> At each mount, emit the transaction reservation type information via
> tracepoints.  This makes it easier to compare the log reservation info
> calculated by the kernel and xfsprogs so that we can more easily diagnose
> minimum log size failures on freshly formatted filesystems.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Seems fine:

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/libxfs/xfs_log_rlimit.c |    2 +-
>  fs/xfs/libxfs/xfs_shared.h     |    3 +++
>  fs/xfs/xfs_trace.h             |   26 ++++++++++++++++++++++++++
>  fs/xfs/xfs_trans.c             |   22 ++++++++++++++++++++++
>  4 files changed, 52 insertions(+), 1 deletion(-)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_log_rlimit.c b/fs/xfs/libxfs/xfs_log_rlimit.c
> index c105979..cc4cbe2 100644
> --- a/fs/xfs/libxfs/xfs_log_rlimit.c
> +++ b/fs/xfs/libxfs/xfs_log_rlimit.c
> @@ -55,7 +55,7 @@ xfs_log_calc_max_attrsetm_res(
>   * the maximum one in terms of the pre-calculated values which were done
>   * at mount time.
>   */
> -STATIC void
> +void
>  xfs_log_get_max_trans_res(
>  	struct xfs_mount	*mp,
>  	struct xfs_trans_res	*max_resp)
> diff --git a/fs/xfs/libxfs/xfs_shared.h b/fs/xfs/libxfs/xfs_shared.h
> index 67ccb1a..d0b84da 100644
> --- a/fs/xfs/libxfs/xfs_shared.h
> +++ b/fs/xfs/libxfs/xfs_shared.h
> @@ -76,6 +76,9 @@ struct xfs_log_item_desc {
>  int	xfs_log_calc_unit_res(struct xfs_mount *mp, int unit_bytes);
>  int	xfs_log_calc_minimum_size(struct xfs_mount *);
>  
> +struct xfs_trans_res;
> +void	xfs_log_get_max_trans_res(struct xfs_mount *mp,
> +				  struct xfs_trans_res *max_resp);
>  
>  /*
>   * Values for t_flags.
> diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> index 9235b2c..b6251f8 100644
> --- a/fs/xfs/xfs_trace.h
> +++ b/fs/xfs/xfs_trace.h
> @@ -3313,6 +3313,32 @@ DEFINE_GETFSMAP_EVENT(xfs_getfsmap_low_key);
>  DEFINE_GETFSMAP_EVENT(xfs_getfsmap_high_key);
>  DEFINE_GETFSMAP_EVENT(xfs_getfsmap_mapping);
>  
> +TRACE_EVENT(xfs_trans_resv_calc,
> +	TP_PROTO(struct xfs_mount *mp, unsigned int type,
> +		 struct xfs_trans_res *res),
> +	TP_ARGS(mp, type, res),
> +	TP_STRUCT__entry(
> +		__field(dev_t, dev)
> +		__field(int, type)
> +		__field(uint, logres)
> +		__field(int, logcount)
> +		__field(int, logflags)
> +	),
> +	TP_fast_assign(
> +		__entry->dev = mp->m_super->s_dev;
> +		__entry->type = type;
> +		__entry->logres = res->tr_logres;
> +		__entry->logcount = res->tr_logcount;
> +		__entry->logflags = res->tr_logflags;
> +	),
> +	TP_printk("dev %d:%d type %d logres %u logcount %d flags 0x%x",
> +		  MAJOR(__entry->dev), MINOR(__entry->dev),
> +		  __entry->type,
> +		  __entry->logres,
> +		  __entry->logcount,
> +		  __entry->logflags)
> +);
> +
>  #endif /* _TRACE_XFS_H */
>  
>  #undef TRACE_INCLUDE_PATH
> diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
> index a87f657..86f92df 100644
> --- a/fs/xfs/xfs_trans.c
> +++ b/fs/xfs/xfs_trans.c
> @@ -35,6 +35,27 @@
>  kmem_zone_t	*xfs_trans_zone;
>  kmem_zone_t	*xfs_log_item_desc_zone;
>  
> +#if defined(CONFIG_TRACEPOINTS)
> +static void
> +xfs_trans_trace_reservations(
> +	struct xfs_mount	*mp)
> +{
> +	struct xfs_trans_res	resv;
> +	struct xfs_trans_res	*res;
> +	struct xfs_trans_res	*end_res;
> +	int			i;
> +
> +	res = (struct xfs_trans_res *)M_RES(mp);
> +	end_res = (struct xfs_trans_res *)(M_RES(mp) + 1);
> +	for (i = 0; res < end_res; i++, res++)
> +		trace_xfs_trans_resv_calc(mp, i, res);
> +	xfs_log_get_max_trans_res(mp, &resv);
> +	trace_xfs_trans_resv_calc(mp, -1, &resv);
> +}
> +#else
> +# define xfs_trans_trace_reservations(mp)
> +#endif
> +
>  /*
>   * Initialize the precomputed transaction reservation values
>   * in the mount structure.
> @@ -44,6 +65,7 @@ xfs_trans_init(
>  	struct xfs_mount	*mp)
>  {
>  	xfs_trans_resv_calc(mp, M_RES(mp));
> +	xfs_trans_trace_reservations(mp);
>  }
>  
>  /*
> 
> --
> 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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/3] xfs: hoist xfs_fs_geometry to libxfs
  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
  0 siblings, 0 replies; 10+ messages in thread
From: Brian Foster @ 2018-01-05 13:08 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs

On Thu, Jan 04, 2018 at 07:39:45PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Move xfs_fs_geometry to libxfs so that we can clean up the fs geometry
> reporting in xfsprogs.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Looks like a straightforward move:

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/libxfs/xfs_sb.c |   79 ++++++++++++++++++++++++++++++++++++++++++++++++
>  fs/xfs/libxfs/xfs_sb.h |    3 ++
>  fs/xfs/xfs_fsops.c     |   77 -----------------------------------------------
>  fs/xfs/xfs_fsops.h     |    1 -
>  fs/xfs/xfs_ioctl.c     |    1 +
>  fs/xfs/xfs_ioctl32.c   |    1 +
>  6 files changed, 84 insertions(+), 78 deletions(-)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
> index 63e0331..139517a 100644
> --- a/fs/xfs/libxfs/xfs_sb.c
> +++ b/fs/xfs/libxfs/xfs_sb.c
> @@ -40,6 +40,8 @@
>  #include "xfs_rmap_btree.h"
>  #include "xfs_bmap.h"
>  #include "xfs_refcount_btree.h"
> +#include "xfs_da_format.h"
> +#include "xfs_da_btree.h"
>  
>  /*
>   * Physical superblock buffer manipulations. Shared with libxfs in userspace.
> @@ -874,3 +876,80 @@ xfs_sync_sb(
>  		xfs_trans_set_sync(tp);
>  	return xfs_trans_commit(tp);
>  }
> +
> +int
> +xfs_fs_geometry(
> +	xfs_mount_t		*mp,
> +	xfs_fsop_geom_t		*geo,
> +	int			new_version)
> +{
> +
> +	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 961e647..a16632c 100644
> --- a/fs/xfs/libxfs/xfs_sb.h
> +++ b/fs/xfs/libxfs/xfs_sb.h
> @@ -34,4 +34,7 @@ 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);
> +
>  #endif	/* __XFS_SB_H__ */
> diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
> index 60a2e12..84d7383 100644
> --- a/fs/xfs/xfs_fsops.c
> +++ b/fs/xfs/xfs_fsops.c
> @@ -49,83 +49,6 @@
>   * File system operations
>   */
>  
> -int
> -xfs_fs_geometry(
> -	xfs_mount_t		*mp,
> -	xfs_fsop_geom_t		*geo,
> -	int			new_version)
> -{
> -
> -	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;
> -}
> -
>  static struct xfs_buf *
>  xfs_growfs_get_hdr_buf(
>  	struct xfs_mount	*mp,
> diff --git a/fs/xfs/xfs_fsops.h b/fs/xfs/xfs_fsops.h
> index 2954c13..20484ed 100644
> --- a/fs/xfs/xfs_fsops.h
> +++ b/fs/xfs/xfs_fsops.h
> @@ -18,7 +18,6 @@
>  #ifndef __XFS_FSOPS_H__
>  #define	__XFS_FSOPS_H__
>  
> -extern int xfs_fs_geometry(xfs_mount_t *mp, xfs_fsop_geom_t *geo, int nversion);
>  extern int xfs_growfs_data(xfs_mount_t *mp, xfs_growfs_data_t *in);
>  extern int xfs_growfs_log(xfs_mount_t *mp, xfs_growfs_log_t *in);
>  extern int xfs_fs_counts(xfs_mount_t *mp, xfs_fsop_counts_t *cnt);
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index 20dc65f..3015e17 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -45,6 +45,7 @@
>  #include <linux/fsmap.h>
>  #include "xfs_fsmap.h"
>  #include "scrub/xfs_scrub.h"
> +#include "xfs_sb.h"
>  
>  #include <linux/capability.h>
>  #include <linux/cred.h>
> diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
> index 35c79e2..66cc3cd 100644
> --- a/fs/xfs/xfs_ioctl32.c
> +++ b/fs/xfs/xfs_ioctl32.c
> @@ -37,6 +37,7 @@
>  #include "xfs_ioctl.h"
>  #include "xfs_ioctl32.h"
>  #include "xfs_trace.h"
> +#include "xfs_sb.h"
>  
>  #define  _NATIVE_IOC(cmd, type) \
>  	  _IOC(_IOC_DIR(cmd), _IOC_TYPE(cmd), _IOC_NR(cmd), sizeof(type))
> 
> --
> 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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/3] xfs: refactor the geometry structure filling function
  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
  2018-01-05 17:25     ` Darrick J. Wong
  2018-01-05 17:34   ` [PATCH v2 " Darrick J. Wong
  1 sibling, 1 reply; 10+ messages in thread
From: Brian Foster @ 2018-01-05 13:08 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/3] xfs: refactor the geometry structure filling function
  2018-01-05 13:08   ` Brian Foster
@ 2018-01-05 17:25     ` Darrick J. Wong
  0 siblings, 0 replies; 10+ messages in thread
From: Darrick J. Wong @ 2018-01-05 17:25 UTC (permalink / raw)
  To: Brian Foster; +Cc: linux-xfs

On Fri, Jan 05, 2018 at 08:08:35AM -0500, Brian Foster wrote:
> 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..?

Yep, fixed.

> > +	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..

Yeah, I suppose since we proceed linearly through that section without
branching outside the block that it needn't the additional whitespace.
Fixed.

> ...
> > +
> > +	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

At the point that mkfs wants to call this function, it has a fully built
xfs_sb ready to go, but it hasn't called libxfs_init.  Therefore,
mp->m_dir_geo hasn't yet been built, and I wanted to preserve the
behavior that mkfs can spit out the geometry even if libxfs
initialization fails for some other reason.

That said, there isn't any reason why I couldn't just make this a helper
in xfs_da_format.h and refactor all the opencoded instances:

/* Number of bytes in a directory block. */
static inline unsigned int xfs_dir2_dirblock_bytes(struct xfs_sb *sbp)
{
	return 1 << (sbp->sb_blocklog + sbp->sb_dirblklog);
}

--D

> 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
> --
> 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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 3/3] xfs: refactor the geometry structure filling function
  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
@ 2018-01-05 17:34   ` Darrick J. Wong
  2018-01-05 19:02     ` Brian Foster
  1 sibling, 1 reply; 10+ messages in thread
From: Darrick J. Wong @ 2018-01-05 17:34 UTC (permalink / raw)
  To: linux-xfs; +Cc: Brian Foster

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>
---
v2: fix whitespacing, refactor the directory block size calculation
---
 fs/xfs/libxfs/xfs_da_format.h |    6 ++
 fs/xfs/libxfs/xfs_dir2.c      |    5 +
 fs/xfs/libxfs/xfs_sb.c        |  148 ++++++++++++++++++++++-------------------
 fs/xfs/libxfs/xfs_sb.h        |    5 +
 fs/xfs/xfs_ioctl.c            |    4 +
 fs/xfs/xfs_ioctl32.c          |    2 -
 6 files changed, 92 insertions(+), 78 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_da_format.h b/fs/xfs/libxfs/xfs_da_format.h
index 3771edc..7e77299 100644
--- a/fs/xfs/libxfs/xfs_da_format.h
+++ b/fs/xfs/libxfs/xfs_da_format.h
@@ -875,4 +875,10 @@ struct xfs_attr3_rmt_hdr {
 	((bufsize) - (xfs_sb_version_hascrc(&(mp)->m_sb) ? \
 			sizeof(struct xfs_attr3_rmt_hdr) : 0))
 
+/* Number of bytes in a directory block. */
+static inline unsigned int xfs_dir2_dirblock_bytes(struct xfs_sb *sbp)
+{
+	return 1 << (sbp->sb_blocklog + sbp->sb_dirblklog);
+}
+
 #endif /* __XFS_DA_FORMAT_H__ */
diff --git a/fs/xfs/libxfs/xfs_dir2.c b/fs/xfs/libxfs/xfs_dir2.c
index e10778c..92f94e1 100644
--- a/fs/xfs/libxfs/xfs_dir2.c
+++ b/fs/xfs/libxfs/xfs_dir2.c
@@ -119,8 +119,7 @@ xfs_da_mount(
 
 
 	ASSERT(mp->m_sb.sb_versionnum & XFS_SB_VERSION_DIRV2BIT);
-	ASSERT((1 << (mp->m_sb.sb_blocklog + mp->m_sb.sb_dirblklog)) <=
-	       XFS_MAX_BLOCKSIZE);
+	ASSERT(xfs_dir2_dirblock_bytes(&mp->m_sb) <= XFS_MAX_BLOCKSIZE);
 
 	mp->m_dir_inode_ops = xfs_dir_get_ops(mp, NULL);
 	mp->m_nondir_inode_ops = xfs_nondir_get_ops(mp, NULL);
@@ -140,7 +139,7 @@ xfs_da_mount(
 	dageo = mp->m_dir_geo;
 	dageo->blklog = mp->m_sb.sb_blocklog + mp->m_sb.sb_dirblklog;
 	dageo->fsblog = mp->m_sb.sb_blocklog;
-	dageo->blksize = 1 << dageo->blklog;
+	dageo->blksize = xfs_dir2_dirblock_bytes(&mp->m_sb);
 	dageo->fsbcount = 1 << mp->m_sb.sb_dirblklog;
 
 	/*
diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
index 139517a..35b005d 100644
--- a/fs/xfs/libxfs/xfs_sb.c
+++ b/fs/xfs/libxfs/xfs_sb.c
@@ -879,77 +879,85 @@ 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)
 {
+	memset(geo, 0, sizeof(struct xfs_fsop_geom));
+
+	geo->blocksize = sbp->sb_blocksize;
+	geo->rtextsize = sbp->sb_rextsize;
+	geo->agblocks = sbp->sb_agblocks;
+	geo->agcount = sbp->sb_agcount;
+	geo->logblocks = sbp->sb_logblocks;
+	geo->sectsize = sbp->sb_sectsize;
+	geo->inodesize = sbp->sb_inodesize;
+	geo->imaxpct = sbp->sb_imax_pct;
+	geo->datablocks = sbp->sb_dblocks;
+	geo->rtblocks = sbp->sb_rblocks;
+	geo->rtextents = sbp->sb_rextents;
+	geo->logstart = sbp->sb_logstart;
+	BUILD_BUG_ON(sizeof(geo->uuid) != sizeof(sbp->sb_uuid));
+	memcpy(geo->uuid, &sbp->sb_uuid, sizeof(sbp->sb_uuid));
+
+	if (struct_version < 2)
+		return 0;
+
+	geo->sunit = sbp->sb_unit;
+	geo->swidth = sbp->sb_width;
+
+	if (struct_version < 3)
+		return 0;
+
+	geo->version = XFS_FSOP_GEOM_VERSION;
+	geo->flags = XFS_FSOP_GEOM_FLAGS_NLINK |
+		     XFS_FSOP_GEOM_FLAGS_DIRV2;
+	if (xfs_sb_version_hasattr(sbp))
+		geo->flags |= XFS_FSOP_GEOM_FLAGS_ATTR;
+	if (xfs_sb_version_hasquota(sbp))
+		geo->flags |= XFS_FSOP_GEOM_FLAGS_QUOTA;
+	if (xfs_sb_version_hasalign(sbp))
+		geo->flags |= XFS_FSOP_GEOM_FLAGS_IALIGN;
+	if (xfs_sb_version_hasdalign(sbp))
+		geo->flags |= XFS_FSOP_GEOM_FLAGS_DALIGN;
+	if (xfs_sb_version_hasextflgbit(sbp))
+		geo->flags |= XFS_FSOP_GEOM_FLAGS_EXTFLG;
+	if (xfs_sb_version_hassector(sbp))
+		geo->flags |= XFS_FSOP_GEOM_FLAGS_SECTOR;
+	if (xfs_sb_version_hasasciici(sbp))
+		geo->flags |= XFS_FSOP_GEOM_FLAGS_DIRV2CI;
+	if (xfs_sb_version_haslazysbcount(sbp))
+		geo->flags |= XFS_FSOP_GEOM_FLAGS_LAZYSB;
+	if (xfs_sb_version_hasattr2(sbp))
+		geo->flags |= XFS_FSOP_GEOM_FLAGS_ATTR2;
+	if (xfs_sb_version_hasprojid32bit(sbp))
+		geo->flags |= XFS_FSOP_GEOM_FLAGS_PROJID32;
+	if (xfs_sb_version_hascrc(sbp))
+		geo->flags |= XFS_FSOP_GEOM_FLAGS_V5SB;
+	if (xfs_sb_version_hasftype(sbp))
+		geo->flags |= XFS_FSOP_GEOM_FLAGS_FTYPE;
+	if (xfs_sb_version_hasfinobt(sbp))
+		geo->flags |= XFS_FSOP_GEOM_FLAGS_FINOBT;
+	if (xfs_sb_version_hassparseinodes(sbp))
+		geo->flags |= XFS_FSOP_GEOM_FLAGS_SPINODES;
+	if (xfs_sb_version_hasrmapbt(sbp))
+		geo->flags |= XFS_FSOP_GEOM_FLAGS_RMAPBT;
+	if (xfs_sb_version_hasreflink(sbp))
+		geo->flags |= XFS_FSOP_GEOM_FLAGS_REFLINK;
+	if (xfs_sb_version_hassector(sbp))
+		geo->logsectsize = sbp->sb_logsectsize;
+	else
+		geo->logsectsize = BBSIZE;
+	geo->rtsectsize = sbp->sb_blocksize;
+	geo->dirblocksize = xfs_dir2_dirblock_bytes(sbp);
+
+	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 */

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 3/3] xfs: refactor the geometry structure filling function
  2018-01-05 17:34   ` [PATCH v2 " Darrick J. Wong
@ 2018-01-05 19:02     ` Brian Foster
  0 siblings, 0 replies; 10+ messages in thread
From: Brian Foster @ 2018-01-05 19:02 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs

On Fri, Jan 05, 2018 at 09:34:00AM -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>
> ---
> v2: fix whitespacing, refactor the directory block size calculation
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/libxfs/xfs_da_format.h |    6 ++
>  fs/xfs/libxfs/xfs_dir2.c      |    5 +
>  fs/xfs/libxfs/xfs_sb.c        |  148 ++++++++++++++++++++++-------------------
>  fs/xfs/libxfs/xfs_sb.h        |    5 +
>  fs/xfs/xfs_ioctl.c            |    4 +
>  fs/xfs/xfs_ioctl32.c          |    2 -
>  6 files changed, 92 insertions(+), 78 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_da_format.h b/fs/xfs/libxfs/xfs_da_format.h
> index 3771edc..7e77299 100644
> --- a/fs/xfs/libxfs/xfs_da_format.h
> +++ b/fs/xfs/libxfs/xfs_da_format.h
> @@ -875,4 +875,10 @@ struct xfs_attr3_rmt_hdr {
>  	((bufsize) - (xfs_sb_version_hascrc(&(mp)->m_sb) ? \
>  			sizeof(struct xfs_attr3_rmt_hdr) : 0))
>  
> +/* Number of bytes in a directory block. */
> +static inline unsigned int xfs_dir2_dirblock_bytes(struct xfs_sb *sbp)
> +{
> +	return 1 << (sbp->sb_blocklog + sbp->sb_dirblklog);
> +}
> +
>  #endif /* __XFS_DA_FORMAT_H__ */
> diff --git a/fs/xfs/libxfs/xfs_dir2.c b/fs/xfs/libxfs/xfs_dir2.c
> index e10778c..92f94e1 100644
> --- a/fs/xfs/libxfs/xfs_dir2.c
> +++ b/fs/xfs/libxfs/xfs_dir2.c
> @@ -119,8 +119,7 @@ xfs_da_mount(
>  
>  
>  	ASSERT(mp->m_sb.sb_versionnum & XFS_SB_VERSION_DIRV2BIT);
> -	ASSERT((1 << (mp->m_sb.sb_blocklog + mp->m_sb.sb_dirblklog)) <=
> -	       XFS_MAX_BLOCKSIZE);
> +	ASSERT(xfs_dir2_dirblock_bytes(&mp->m_sb) <= XFS_MAX_BLOCKSIZE);
>  
>  	mp->m_dir_inode_ops = xfs_dir_get_ops(mp, NULL);
>  	mp->m_nondir_inode_ops = xfs_nondir_get_ops(mp, NULL);
> @@ -140,7 +139,7 @@ xfs_da_mount(
>  	dageo = mp->m_dir_geo;
>  	dageo->blklog = mp->m_sb.sb_blocklog + mp->m_sb.sb_dirblklog;
>  	dageo->fsblog = mp->m_sb.sb_blocklog;
> -	dageo->blksize = 1 << dageo->blklog;
> +	dageo->blksize = xfs_dir2_dirblock_bytes(&mp->m_sb);
>  	dageo->fsbcount = 1 << mp->m_sb.sb_dirblklog;
>  
>  	/*
> diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
> index 139517a..35b005d 100644
> --- a/fs/xfs/libxfs/xfs_sb.c
> +++ b/fs/xfs/libxfs/xfs_sb.c
> @@ -879,77 +879,85 @@ 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)
>  {
> +	memset(geo, 0, sizeof(struct xfs_fsop_geom));
> +
> +	geo->blocksize = sbp->sb_blocksize;
> +	geo->rtextsize = sbp->sb_rextsize;
> +	geo->agblocks = sbp->sb_agblocks;
> +	geo->agcount = sbp->sb_agcount;
> +	geo->logblocks = sbp->sb_logblocks;
> +	geo->sectsize = sbp->sb_sectsize;
> +	geo->inodesize = sbp->sb_inodesize;
> +	geo->imaxpct = sbp->sb_imax_pct;
> +	geo->datablocks = sbp->sb_dblocks;
> +	geo->rtblocks = sbp->sb_rblocks;
> +	geo->rtextents = sbp->sb_rextents;
> +	geo->logstart = sbp->sb_logstart;
> +	BUILD_BUG_ON(sizeof(geo->uuid) != sizeof(sbp->sb_uuid));
> +	memcpy(geo->uuid, &sbp->sb_uuid, sizeof(sbp->sb_uuid));
> +
> +	if (struct_version < 2)
> +		return 0;
> +
> +	geo->sunit = sbp->sb_unit;
> +	geo->swidth = sbp->sb_width;
> +
> +	if (struct_version < 3)
> +		return 0;
> +
> +	geo->version = XFS_FSOP_GEOM_VERSION;
> +	geo->flags = XFS_FSOP_GEOM_FLAGS_NLINK |
> +		     XFS_FSOP_GEOM_FLAGS_DIRV2;
> +	if (xfs_sb_version_hasattr(sbp))
> +		geo->flags |= XFS_FSOP_GEOM_FLAGS_ATTR;
> +	if (xfs_sb_version_hasquota(sbp))
> +		geo->flags |= XFS_FSOP_GEOM_FLAGS_QUOTA;
> +	if (xfs_sb_version_hasalign(sbp))
> +		geo->flags |= XFS_FSOP_GEOM_FLAGS_IALIGN;
> +	if (xfs_sb_version_hasdalign(sbp))
> +		geo->flags |= XFS_FSOP_GEOM_FLAGS_DALIGN;
> +	if (xfs_sb_version_hasextflgbit(sbp))
> +		geo->flags |= XFS_FSOP_GEOM_FLAGS_EXTFLG;
> +	if (xfs_sb_version_hassector(sbp))
> +		geo->flags |= XFS_FSOP_GEOM_FLAGS_SECTOR;
> +	if (xfs_sb_version_hasasciici(sbp))
> +		geo->flags |= XFS_FSOP_GEOM_FLAGS_DIRV2CI;
> +	if (xfs_sb_version_haslazysbcount(sbp))
> +		geo->flags |= XFS_FSOP_GEOM_FLAGS_LAZYSB;
> +	if (xfs_sb_version_hasattr2(sbp))
> +		geo->flags |= XFS_FSOP_GEOM_FLAGS_ATTR2;
> +	if (xfs_sb_version_hasprojid32bit(sbp))
> +		geo->flags |= XFS_FSOP_GEOM_FLAGS_PROJID32;
> +	if (xfs_sb_version_hascrc(sbp))
> +		geo->flags |= XFS_FSOP_GEOM_FLAGS_V5SB;
> +	if (xfs_sb_version_hasftype(sbp))
> +		geo->flags |= XFS_FSOP_GEOM_FLAGS_FTYPE;
> +	if (xfs_sb_version_hasfinobt(sbp))
> +		geo->flags |= XFS_FSOP_GEOM_FLAGS_FINOBT;
> +	if (xfs_sb_version_hassparseinodes(sbp))
> +		geo->flags |= XFS_FSOP_GEOM_FLAGS_SPINODES;
> +	if (xfs_sb_version_hasrmapbt(sbp))
> +		geo->flags |= XFS_FSOP_GEOM_FLAGS_RMAPBT;
> +	if (xfs_sb_version_hasreflink(sbp))
> +		geo->flags |= XFS_FSOP_GEOM_FLAGS_REFLINK;
> +	if (xfs_sb_version_hassector(sbp))
> +		geo->logsectsize = sbp->sb_logsectsize;
> +	else
> +		geo->logsectsize = BBSIZE;
> +	geo->rtsectsize = sbp->sb_blocksize;
> +	geo->dirblocksize = xfs_dir2_dirblock_bytes(sbp);
> +
> +	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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2018-01-05 19:02 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
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

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.