public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 00/17] xfs: sparse inode chunks
@ 2014-11-03 16:12 Brian Foster
  2014-11-03 16:12 ` [PATCH v2 01/17] xfs: add sparse inode chunk alignment superblock field Brian Foster
                   ` (16 more replies)
  0 siblings, 17 replies; 24+ messages in thread
From: Brian Foster @ 2014-11-03 16:12 UTC (permalink / raw)
  To: xfs

Hi all,

Here's another pass of sparse inode chunk support. There are still some
outstanding things to resolve, but I think this hits on some of the
larger comments of v1 and serves as a decent checkpoint.

My biggest open question at this point is how to handle backwards
compatibility in relation to the behavior of the new ir_count field
introduced in v2. The current behavior is to use ir_count whenever
sparse inode chunk support is enabled (from a feature bit perspective,
see xfs_inobt_insert()). The side effect of this is that backwards
compatibility is no longer solely tied to the existence of sparse inode
chunks. In other words, we cannot simply toggle off a feature bit for
fs' without sparse inode allocations, as the existing allocations have
an ir_count == 64 value in the upper bytes of the old ir_free field.

We can follow a couple alternate directions that I can think of:

- Only use ir_count for actual sparse chunks. E.g., ir_count == 0 for
  chunks with XFS_INODES_PER_CHUNK inodes. 
- Always use ir_count when feature bit enabled, consider inverting
  ir_holemask to ir_allocmask or some such to simplify.

The further I progress on this work, the more I think the backwards
compatibility has limited utility, poor cost-benefit and I lean towards
the second approach.

That said, I don't necessarily see a problem with retaining the
backwards compatible nature of the on-disk structures with the
understanding that the ability for such conversions probably will not be
provided in the near term. E.g., backwards compat. simply becomes a
design characteristic to leave the window open for future work. But I'm
also not a huge fan of letting some unknown conversion mechanism from
the future put too many constraints on simplification of an existing
design. E.g., what's the point if the end result ends up being some kind
of userspace conversion via xfs_repair (for example) that can track
sparse chunks and fix up the records appropriately?

Thoughts?

Brian

v2:
- Use a manually set feature bit instead of dynamic based on the
  existence of sparse inode chunks.
- Add sb/mp fields for sparse alloc. granularity (use instead of cluster
  size).
- Undo xfs_inobt_insert() loop removal to avoid breakage of larger page
  size arches.
- Rename sparse record overlap helper and do XFS_LOOKUP_LE search.
- Use byte of pad space in inobt record for inode count field.
- Convert bitmap mgmt to use generic bitmap code.
- Rename XFS_INODES_PER_SPCHUNK to XFS_INODES_PER_HOLEMASK_BIT.
- Add fs geometry bit for sparse inodes.
- Rebase to latest for-next (bulkstat refactor).
v1: http://oss.sgi.com/archives/xfs/2014-07/msg00355.html

Brian Foster (17):
  xfs: add sparse inode chunk alignment superblock field
  xfs: use sparse chunk alignment for min. inode allocation requirement
  xfs: define sparse inode chunks v5 sb feature bit and helper function
  xfs: introduce inode record hole mask for sparse inode chunks
  xfs: create macros/helpers for dealing with sparse inode chunks
  xfs: pass inode count through ordered icreate log item
  xfs: handle sparse inode chunks in icreate log recovery
  xfs: create helper to manage record overlap for sparse inode chunks
  xfs: allocate sparse inode chunks on full chunk allocation failure
  xfs: helpers to convert holemask to/from generic bitmap
  xfs: filter out sparse regions from individual inode allocation
  xfs: update free inode record logic to support sparse inode records
  xfs: only free allocated regions of inode chunks
  xfs: skip unallocated regions of inode chunks in xfs_ifree_cluster()
  xfs: use actual inode count for sparse records in bulkstat/inumbers
  xfs: add fs geometry bit for sparse inode chunks
  xfs: enable sparse inode chunks for v5 superblocks

 fs/xfs/libxfs/xfs_format.h       |  17 +-
 fs/xfs/libxfs/xfs_ialloc.c       | 456 ++++++++++++++++++++++++++++++++++++---
 fs/xfs/libxfs/xfs_ialloc.h       |  17 +-
 fs/xfs/libxfs/xfs_ialloc_btree.c |   4 +-
 fs/xfs/libxfs/xfs_sb.c           |   9 +-
 fs/xfs/libxfs/xfs_sb.h           |  16 +-
 fs/xfs/xfs_fs.h                  |   1 +
 fs/xfs/xfs_fsops.c               |   4 +-
 fs/xfs/xfs_inode.c               |  28 ++-
 fs/xfs/xfs_itable.c              |  16 +-
 fs/xfs/xfs_log_recover.c         |  23 +-
 fs/xfs/xfs_mount.h               |   2 +
 12 files changed, 525 insertions(+), 68 deletions(-)

-- 
1.8.3.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 24+ messages in thread
* [PATCH v2 01/17] xfs: add sparse inode chunk alignment superblock field
  2014-11-03 16:12 [PATCH v2 00/17] xfs: sparse inode chunks Brian Foster
@ 2014-11-03 16:12 ` Brian Foster
  2014-11-03 16:12 ` [PATCH v2 02/17] xfs: use sparse chunk alignment for min. inode allocation requirement Brian Foster
                   ` (15 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Brian Foster @ 2014-11-03 16:12 UTC (permalink / raw)
  To: xfs

Add sb_spinoalignmt to the superblock to specify sparse inode chunk
alignment. This also currently represents the minimum allowable sparse
chunk allocation size.

Signed-off-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/libxfs/xfs_sb.c | 4 ++--
 fs/xfs/libxfs/xfs_sb.h | 6 +++---
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
index 5f902fa..6677180 100644
--- a/fs/xfs/libxfs/xfs_sb.c
+++ b/fs/xfs/libxfs/xfs_sb.c
@@ -99,7 +99,7 @@ static const struct {
 	{ offsetof(xfs_sb_t, sb_features_incompat),	0 },
 	{ offsetof(xfs_sb_t, sb_features_log_incompat),	0 },
 	{ offsetof(xfs_sb_t, sb_crc),		0 },
-	{ offsetof(xfs_sb_t, sb_pad),		0 },
+	{ offsetof(xfs_sb_t, sb_spinoalignmt),	0 },
 	{ offsetof(xfs_sb_t, sb_pquotino),	0 },
 	{ offsetof(xfs_sb_t, sb_lsn),		0 },
 	{ sizeof(xfs_sb_t),			0 }
@@ -447,7 +447,7 @@ __xfs_sb_from_disk(
 				be32_to_cpu(from->sb_features_log_incompat);
 	/* crc is only used on disk, not in memory; just init to 0 here. */
 	to->sb_crc = 0;
-	to->sb_pad = 0;
+	to->sb_spinoalignmt = be32_to_cpu(from->sb_spinoalignmt);
 	to->sb_pquotino = be64_to_cpu(from->sb_pquotino);
 	to->sb_lsn = be64_to_cpu(from->sb_lsn);
 	/* Convert on-disk flags to in-memory flags? */
diff --git a/fs/xfs/libxfs/xfs_sb.h b/fs/xfs/libxfs/xfs_sb.h
index 2e73970..4e5aaa8 100644
--- a/fs/xfs/libxfs/xfs_sb.h
+++ b/fs/xfs/libxfs/xfs_sb.h
@@ -157,7 +157,7 @@ typedef struct xfs_sb {
 	__uint32_t	sb_features_log_incompat;
 
 	__uint32_t	sb_crc;		/* superblock crc */
-	__uint32_t	sb_pad;
+	xfs_extlen_t	sb_spinoalignmt;/* sparse inode chunk alignment */
 
 	xfs_ino_t	sb_pquotino;	/* project quota inode */
 	xfs_lsn_t	sb_lsn;		/* last write sequence */
@@ -243,7 +243,7 @@ typedef struct xfs_dsb {
 	__be32		sb_features_log_incompat;
 
 	__le32		sb_crc;		/* superblock crc */
-	__be32		sb_pad;
+	__be32		sb_spinoalignmt;/* sparse inode chunk alignment */
 
 	__be64		sb_pquotino;	/* project quota inode */
 	__be64		sb_lsn;		/* last write sequence */
@@ -269,7 +269,7 @@ typedef enum {
 	XFS_SBS_LOGSECTLOG, XFS_SBS_LOGSECTSIZE, XFS_SBS_LOGSUNIT,
 	XFS_SBS_FEATURES2, XFS_SBS_BAD_FEATURES2, XFS_SBS_FEATURES_COMPAT,
 	XFS_SBS_FEATURES_RO_COMPAT, XFS_SBS_FEATURES_INCOMPAT,
-	XFS_SBS_FEATURES_LOG_INCOMPAT, XFS_SBS_CRC, XFS_SBS_PAD,
+	XFS_SBS_FEATURES_LOG_INCOMPAT, XFS_SBS_CRC, XFS_SBS_SPINOALIGNMT,
 	XFS_SBS_PQUOTINO, XFS_SBS_LSN,
 	XFS_SBS_FIELDCOUNT
 } xfs_sb_field_t;
-- 
1.8.3.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v2 02/17] xfs: use sparse chunk alignment for min. inode allocation requirement
  2014-11-03 16:12 [PATCH v2 00/17] xfs: sparse inode chunks Brian Foster
  2014-11-03 16:12 ` [PATCH v2 01/17] xfs: add sparse inode chunk alignment superblock field Brian Foster
@ 2014-11-03 16:12 ` Brian Foster
  2014-11-03 16:12 ` [PATCH v2 03/17] xfs: define sparse inode chunks v5 sb feature bit and helper function Brian Foster
                   ` (14 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Brian Foster @ 2014-11-03 16:12 UTC (permalink / raw)
  To: xfs

xfs_ialloc_ag_select() iterates through the allocation groups looking
for free inodes or free space to determine whether to allow an inode
allocation to proceed. If no free inodes are available, it assumes that
an AG must have an extent longer than mp->m_ialloc_blks.

Sparse inode chunk support currently allows for allocations smaller than
the traditional inode chunk size specified in m_ialloc_blks. The current
minimum sparse allocation is set in the superblock sb_spinoalignmt field
at mkfs time. Create a new m_ialloc_min_blks field in xfs_mount and use
this to represent the minimum supported allocation size for inode
chunks. Initialize m_ialloc_min_blks at mount time based on whether
sparse inodes are supported.

Signed-off-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/libxfs/xfs_ialloc.c | 2 +-
 fs/xfs/libxfs/xfs_sb.c     | 5 +++++
 fs/xfs/xfs_mount.h         | 2 ++
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
index 23dcb72..89949a3 100644
--- a/fs/xfs/libxfs/xfs_ialloc.c
+++ b/fs/xfs/libxfs/xfs_ialloc.c
@@ -635,7 +635,7 @@ xfs_ialloc_ag_select(
 		 * Is there enough free space for the file plus a block of
 		 * inodes? (if we need to allocate some)?
 		 */
-		ineed = mp->m_ialloc_blks;
+		ineed = mp->m_ialloc_min_blks;
 		longest = pag->pagf_longest;
 		if (!longest)
 			longest = pag->pagf_flcount > 0;
diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
index 6677180..0416ae9 100644
--- a/fs/xfs/libxfs/xfs_sb.c
+++ b/fs/xfs/libxfs/xfs_sb.c
@@ -756,6 +756,11 @@ xfs_sb_mount_common(
 	mp->m_ialloc_inos = (int)MAX((__uint16_t)XFS_INODES_PER_CHUNK,
 					sbp->sb_inopblock);
 	mp->m_ialloc_blks = mp->m_ialloc_inos >> sbp->sb_inopblog;
+
+	if (sbp->sb_spinoalignmt)
+		mp->m_ialloc_min_blks = sbp->sb_spinoalignmt;
+	else
+		mp->m_ialloc_min_blks = mp->m_ialloc_blks;
 }
 
 /*
diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index b0447c8..42023a6 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -136,6 +136,8 @@ typedef struct xfs_mount {
 	__uint64_t		m_flags;	/* global mount flags */
 	int			m_ialloc_inos;	/* inodes in inode allocation */
 	int			m_ialloc_blks;	/* blocks in inode allocation */
+	int			m_ialloc_min_blks;/* min blocks in sparse inode
+						   * allocation */
 	int			m_inoalign_mask;/* mask sb_inoalignmt if used */
 	uint			m_qflags;	/* quota status flags */
 	struct xfs_trans_resv	m_resv;		/* precomputed res values */
-- 
1.8.3.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v2 03/17] xfs: define sparse inode chunks v5 sb feature bit and helper function
  2014-11-03 16:12 [PATCH v2 00/17] xfs: sparse inode chunks Brian Foster
  2014-11-03 16:12 ` [PATCH v2 01/17] xfs: add sparse inode chunk alignment superblock field Brian Foster
  2014-11-03 16:12 ` [PATCH v2 02/17] xfs: use sparse chunk alignment for min. inode allocation requirement Brian Foster
@ 2014-11-03 16:12 ` Brian Foster
  2014-11-03 16:12 ` [PATCH v2 04/17] xfs: introduce inode record hole mask for sparse inode chunks Brian Foster
                   ` (13 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Brian Foster @ 2014-11-03 16:12 UTC (permalink / raw)
  To: xfs

The sparse inode chunks feature uses the helper function to enable the
allocation of sparse inode chunks. The incompatible feature bit is set
on disk at mkfs time.

Signed-off-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/libxfs/xfs_sb.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/fs/xfs/libxfs/xfs_sb.h b/fs/xfs/libxfs/xfs_sb.h
index 4e5aaa8..eeb4058 100644
--- a/fs/xfs/libxfs/xfs_sb.h
+++ b/fs/xfs/libxfs/xfs_sb.h
@@ -509,6 +509,7 @@ xfs_sb_has_ro_compat_feature(
 }
 
 #define XFS_SB_FEAT_INCOMPAT_FTYPE	(1 << 0)	/* filetype in dirent */
+#define XFS_SB_FEAT_INCOMPAT_SPINODES	(1 << 1)	/* sparse inode chunks */
 #define XFS_SB_FEAT_INCOMPAT_ALL \
 		(XFS_SB_FEAT_INCOMPAT_FTYPE)
 
@@ -558,6 +559,12 @@ static inline int xfs_sb_version_hasfinobt(xfs_sb_t *sbp)
 		(sbp->sb_features_ro_compat & XFS_SB_FEAT_RO_COMPAT_FINOBT);
 }
 
+static inline bool xfs_sb_version_hassparseinodes(struct xfs_sb *sbp)
+{
+	return XFS_SB_VERSION_NUM(sbp) == XFS_SB_VERSION_5 &&
+		xfs_sb_has_incompat_feature(sbp, XFS_SB_FEAT_INCOMPAT_SPINODES);
+}
+
 /*
  * end of superblock version macros
  */
-- 
1.8.3.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v2 04/17] xfs: introduce inode record hole mask for sparse inode chunks
  2014-11-03 16:12 [PATCH v2 00/17] xfs: sparse inode chunks Brian Foster
                   ` (2 preceding siblings ...)
  2014-11-03 16:12 ` [PATCH v2 03/17] xfs: define sparse inode chunks v5 sb feature bit and helper function Brian Foster
@ 2014-11-03 16:12 ` Brian Foster
  2014-11-03 16:12 ` [PATCH v2 05/17] xfs: create macros/helpers for dealing with " Brian Foster
                   ` (12 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Brian Foster @ 2014-11-03 16:12 UTC (permalink / raw)
  To: xfs

The inode btrees track 64 inodes per record, regardless of inode size.
Thus, inode chunks on disk vary in size depending on the size of the
inodes. This creates a contiguous allocation requirement for new inode
chunks that can be difficult to satisfy on an aged and fragmented (free
space) filesystem.

The inode record freecount currently uses 4 bytes on disk to track the
free inode count. With a maximum freecount value of 64, only one byte is
required. Convert the freecount field to a single byte and reserve two
of the remaining 3 higher order bytes left to the hole mask field.
The left over byte to the total count field.

The hole mask field tracks holes in the chunks of physical space that
the inode record refers to. This facilitates the sparse allocation of
inode chunks when contiguous chunks are not available and allows the
inode btrees to identify what portions of the chunk contain valid
inodes. The total count field contains the total number of valid inodes
referred to by the record. This can also be deduced from the hole mask.
The count field provides clarity and redundancy for internal record
verification.

Note that both fields are initialized to zero to maintain backwards
compatibility with existing filesystems (e.g., the higher order bytes of
freecount are always 0). Tracking holes means that the hole mask is
initialized to zero and thus remains "valid" in accordance with a
non-sparse inode fs when no sparse chunks are physically allocated.
Update the inode record management functions to handle the new fields
and initialize to zero.

[XXX: The count field breaks this notion of easy backwards compatibility
with !sparseinode fs. Should we reconsider the addition of total count
or the idea of converting back and forth between sparse inode fs with a
simple feature bit? I lean towards the latter at the moment as we could
implement conversion in xfs_repair if there is ever user demand.]

Signed-off-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/libxfs/xfs_format.h       |  8 ++++++--
 fs/xfs/libxfs/xfs_ialloc.c       | 12 ++++++++++--
 fs/xfs/libxfs/xfs_ialloc_btree.c |  4 +++-
 3 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
index 7e42bba..873c4d1 100644
--- a/fs/xfs/libxfs/xfs_format.h
+++ b/fs/xfs/libxfs/xfs_format.h
@@ -217,13 +217,17 @@ static inline xfs_inofree_t xfs_inobt_maskn(int i, int n)
  */
 typedef struct xfs_inobt_rec {
 	__be32		ir_startino;	/* starting inode number */
-	__be32		ir_freecount;	/* count of free inodes (set bits) */
+	__be16		ir_holemask;	/* hole mask for sparse chunks */
+	__u8		ir_count;	/* total inode count */
+	__u8		ir_freecount;	/* count of free inodes (set bits) */
 	__be64		ir_free;	/* free inode mask */
 } xfs_inobt_rec_t;
 
 typedef struct xfs_inobt_rec_incore {
 	xfs_agino_t	ir_startino;	/* starting inode number */
-	__int32_t	ir_freecount;	/* count of free inodes (set bits) */
+	__uint16_t	ir_holemask;	/* hole mask for sparse chunks */
+	__uint8_t	ir_count;	/* total inode count */
+	__uint8_t	ir_freecount;	/* count of free inodes (set bits) */
 	xfs_inofree_t	ir_free;	/* free inode mask */
 } xfs_inobt_rec_incore_t;
 
diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
index 89949a3..fcc8baa 100644
--- a/fs/xfs/libxfs/xfs_ialloc.c
+++ b/fs/xfs/libxfs/xfs_ialloc.c
@@ -68,6 +68,8 @@ xfs_inobt_lookup(
 	int			*stat)	/* success/failure */
 {
 	cur->bc_rec.i.ir_startino = ino;
+	cur->bc_rec.i.ir_holemask = 0;
+	cur->bc_rec.i.ir_count = 0;
 	cur->bc_rec.i.ir_freecount = 0;
 	cur->bc_rec.i.ir_free = 0;
 	return xfs_btree_lookup(cur, dir, stat);
@@ -85,7 +87,9 @@ xfs_inobt_update(
 	union xfs_btree_rec	rec;
 
 	rec.inobt.ir_startino = cpu_to_be32(irec->ir_startino);
-	rec.inobt.ir_freecount = cpu_to_be32(irec->ir_freecount);
+	rec.inobt.ir_holemask = cpu_to_be16(irec->ir_holemask);
+	rec.inobt.ir_count = irec->ir_count;
+	rec.inobt.ir_freecount = irec->ir_freecount;
 	rec.inobt.ir_free = cpu_to_be64(irec->ir_free);
 	return xfs_btree_update(cur, &rec);
 }
@@ -105,7 +109,9 @@ xfs_inobt_get_rec(
 	error = xfs_btree_get_rec(cur, &rec, stat);
 	if (!error && *stat == 1) {
 		irec->ir_startino = be32_to_cpu(rec->inobt.ir_startino);
-		irec->ir_freecount = be32_to_cpu(rec->inobt.ir_freecount);
+		irec->ir_holemask = be16_to_cpu(rec->inobt.ir_holemask);
+		irec->ir_count = rec->inobt.ir_count;
+		irec->ir_freecount = rec->inobt.ir_freecount;
 		irec->ir_free = be64_to_cpu(rec->inobt.ir_free);
 	}
 	return error;
@@ -121,6 +127,8 @@ xfs_inobt_insert_rec(
 	xfs_inofree_t		free,
 	int			*stat)
 {
+	cur->bc_rec.i.ir_holemask = 0;
+	cur->bc_rec.i.ir_count = 0; /* zero for backwards compatibility */
 	cur->bc_rec.i.ir_freecount = freecount;
 	cur->bc_rec.i.ir_free = free;
 	return xfs_btree_insert(cur, stat);
diff --git a/fs/xfs/libxfs/xfs_ialloc_btree.c b/fs/xfs/libxfs/xfs_ialloc_btree.c
index c9b06f3..b2a9117 100644
--- a/fs/xfs/libxfs/xfs_ialloc_btree.c
+++ b/fs/xfs/libxfs/xfs_ialloc_btree.c
@@ -169,7 +169,9 @@ xfs_inobt_init_rec_from_cur(
 	union xfs_btree_rec	*rec)
 {
 	rec->inobt.ir_startino = cpu_to_be32(cur->bc_rec.i.ir_startino);
-	rec->inobt.ir_freecount = cpu_to_be32(cur->bc_rec.i.ir_freecount);
+	rec->inobt.ir_holemask = cpu_to_be16(cur->bc_rec.i.ir_holemask);
+	rec->inobt.ir_count = cur->bc_rec.i.ir_count;
+	rec->inobt.ir_freecount = cur->bc_rec.i.ir_freecount;
 	rec->inobt.ir_free = cpu_to_be64(cur->bc_rec.i.ir_free);
 }
 
-- 
1.8.3.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v2 05/17] xfs: create macros/helpers for dealing with sparse inode chunks
  2014-11-03 16:12 [PATCH v2 00/17] xfs: sparse inode chunks Brian Foster
                   ` (3 preceding siblings ...)
  2014-11-03 16:12 ` [PATCH v2 04/17] xfs: introduce inode record hole mask for sparse inode chunks Brian Foster
@ 2014-11-03 16:12 ` Brian Foster
  2014-11-03 22:12   ` Fanael Linithien
  2014-11-03 16:12 ` [PATCH v2 06/17] xfs: pass inode count through ordered icreate log item Brian Foster
                   ` (11 subsequent siblings)
  16 siblings, 1 reply; 24+ messages in thread
From: Brian Foster @ 2014-11-03 16:12 UTC (permalink / raw)
  To: xfs

Sparse inode chunks allow the traditional inode btree record format to
describe an inode chunk that is not fully allocated and/or contiguous.
The granularity of a sparse chunk is defined by the the 16-bit holemask
field in the inode record. Assuming 64 inodes per full chunk, a single
holemask bit accounts for 4 inodes.

Define a constant for the number of inodes per holemask bit and a helper
function to easily detect sparse inode chunks.

Signed-off-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/libxfs/xfs_format.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
index 873c4d1..c81c1a7 100644
--- a/fs/xfs/libxfs/xfs_format.h
+++ b/fs/xfs/libxfs/xfs_format.h
@@ -207,6 +207,10 @@ typedef	__uint64_t	xfs_inofree_t;
 #define	XFS_INOBT_ALL_FREE		((xfs_inofree_t)-1)
 #define	XFS_INOBT_MASK(i)		((xfs_inofree_t)1 << (i))
 
+#define XFS_INOBT_HOLEMASK_BITS		(NBBY * sizeof(__uint16_t))
+#define XFS_INODES_PER_HOLEMASK_BIT	\
+	(XFS_INODES_PER_CHUNK / (NBBY * sizeof(__uint16_t)))
+
 static inline xfs_inofree_t xfs_inobt_maskn(int i, int n)
 {
 	return ((n >= XFS_INODES_PER_CHUNK ? 0 : XFS_INOBT_MASK(n)) - 1) << i;
@@ -231,6 +235,10 @@ typedef struct xfs_inobt_rec_incore {
 	xfs_inofree_t	ir_free;	/* free inode mask */
 } xfs_inobt_rec_incore_t;
 
+static inline bool xfs_inobt_issparse(uint16_t holemask)
+{
+	return holemask == 0 ? false : true;
+}
 
 /*
  * Key structure
-- 
1.8.3.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v2 06/17] xfs: pass inode count through ordered icreate log item
  2014-11-03 16:12 [PATCH v2 00/17] xfs: sparse inode chunks Brian Foster
                   ` (4 preceding siblings ...)
  2014-11-03 16:12 ` [PATCH v2 05/17] xfs: create macros/helpers for dealing with " Brian Foster
@ 2014-11-03 16:12 ` Brian Foster
  2014-11-03 16:12 ` [PATCH v2 07/17] xfs: handle sparse inode chunks in icreate log recovery Brian Foster
                   ` (10 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Brian Foster @ 2014-11-03 16:12 UTC (permalink / raw)
  To: xfs

v5 superblocks use an ordered log item for logging the initialization of
inode chunks. The icreate log item is currently hardcoded to an inode
count of 64 inodes.

The agbno and extent length are used to initialize the inode chunk from
log recovery. While an incorrect inode count does not lead to bad inode
chunk initialization, we should pass the correct inode count such that log
recovery has enough data to perform meaningful validity checks on the
chunk.

Signed-off-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/libxfs/xfs_ialloc.c | 7 ++++---
 fs/xfs/libxfs/xfs_ialloc.h | 2 +-
 fs/xfs/xfs_log_recover.c   | 4 ++--
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
index fcc8baa..b53fb5d 100644
--- a/fs/xfs/libxfs/xfs_ialloc.c
+++ b/fs/xfs/libxfs/xfs_ialloc.c
@@ -231,6 +231,7 @@ xfs_ialloc_inode_init(
 	struct xfs_mount	*mp,
 	struct xfs_trans	*tp,
 	struct list_head	*buffer_list,
+	int			icount,
 	xfs_agnumber_t		agno,
 	xfs_agblock_t		agbno,
 	xfs_agblock_t		length,
@@ -286,7 +287,7 @@ xfs_ialloc_inode_init(
 		 * they track in the AIL as if they were physically logged.
 		 */
 		if (tp)
-			xfs_icreate_log(tp, agno, agbno, mp->m_ialloc_inos,
+			xfs_icreate_log(tp, agno, agbno, icount,
 					mp->m_sb.sb_inodesize, length, gen);
 	} else
 		version = 2;
@@ -505,8 +506,8 @@ xfs_ialloc_ag_alloc(
 	 * rather than a linear progression to prevent the next generation
 	 * number from being easily guessable.
 	 */
-	error = xfs_ialloc_inode_init(args.mp, tp, NULL, agno, args.agbno,
-			args.len, prandom_u32());
+	error = xfs_ialloc_inode_init(args.mp, tp, NULL, newlen, agno,
+			args.agbno, args.len, prandom_u32());
 
 	if (error)
 		return error;
diff --git a/fs/xfs/libxfs/xfs_ialloc.h b/fs/xfs/libxfs/xfs_ialloc.h
index 95ad1c0..62c1381 100644
--- a/fs/xfs/libxfs/xfs_ialloc.h
+++ b/fs/xfs/libxfs/xfs_ialloc.h
@@ -156,7 +156,7 @@ int xfs_inobt_get_rec(struct xfs_btree_cur *cur,
  * Inode chunk initialisation routine
  */
 int xfs_ialloc_inode_init(struct xfs_mount *mp, struct xfs_trans *tp,
-			  struct list_head *buffer_list,
+			  struct list_head *buffer_list, int icount,
 			  xfs_agnumber_t agno, xfs_agblock_t agbno,
 			  xfs_agblock_t length, unsigned int gen);
 
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index 00cd7f3..e47fdf9 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -3093,8 +3093,8 @@ xlog_recover_do_icreate_pass2(
 			XFS_AGB_TO_DADDR(mp, agno, agbno), length, 0))
 		return 0;
 
-	xfs_ialloc_inode_init(mp, NULL, buffer_list, agno, agbno, length,
-					be32_to_cpu(icl->icl_gen));
+	xfs_ialloc_inode_init(mp, NULL, buffer_list, count, agno, agbno, length,
+			      be32_to_cpu(icl->icl_gen));
 	return 0;
 }
 
-- 
1.8.3.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v2 07/17] xfs: handle sparse inode chunks in icreate log recovery
  2014-11-03 16:12 [PATCH v2 00/17] xfs: sparse inode chunks Brian Foster
                   ` (5 preceding siblings ...)
  2014-11-03 16:12 ` [PATCH v2 06/17] xfs: pass inode count through ordered icreate log item Brian Foster
@ 2014-11-03 16:12 ` Brian Foster
  2014-11-03 16:12 ` [PATCH v2 08/17] xfs: create helper to manage record overlap for sparse inode chunks Brian Foster
                   ` (9 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Brian Foster @ 2014-11-03 16:12 UTC (permalink / raw)
  To: xfs

Recovery of icreate transactions assumes hardcoded values for the inode
count and chunk length.

Sparse inode chunks are allocated in units of m_ialloc_min_blks. Update
the icreate validity checks to allow for appropriately sized inode
chunks and verify the inode count matches what is expected based on the
extent length rather than assuming a hardcoded count.

Signed-off-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/xfs_log_recover.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index e47fdf9..d665b44 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -3070,12 +3070,19 @@ xlog_recover_do_icreate_pass2(
 		return -EINVAL;
 	}
 
-	/* existing allocation is fixed value */
-	ASSERT(count == mp->m_ialloc_inos);
-	ASSERT(length == mp->m_ialloc_blks);
-	if (count != mp->m_ialloc_inos ||
-	     length != mp->m_ialloc_blks) {
-		xfs_warn(log->l_mp, "xlog_recover_do_icreate_trans: bad count 2");
+	/* inode chunk is either full or sparse */
+	if (length != mp->m_ialloc_blks &&
+	    length != mp->m_ialloc_min_blks) {
+		xfs_warn(log->l_mp,
+			 "%s: unsupported chunk length", __FUNCTION__);
+		return -EINVAL;
+	}
+
+	/* verify inode count is consistent with extent length */
+	if ((count >> mp->m_sb.sb_inopblog) != length) {
+		xfs_warn(log->l_mp,
+			 "%s: inconsistent inode count and chunk length",
+			 __FUNCTION__);
 		return -EINVAL;
 	}
 
-- 
1.8.3.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v2 08/17] xfs: create helper to manage record overlap for sparse inode chunks
  2014-11-03 16:12 [PATCH v2 00/17] xfs: sparse inode chunks Brian Foster
                   ` (6 preceding siblings ...)
  2014-11-03 16:12 ` [PATCH v2 07/17] xfs: handle sparse inode chunks in icreate log recovery Brian Foster
@ 2014-11-03 16:12 ` Brian Foster
  2014-11-03 16:12 ` [PATCH v2 09/17] xfs: allocate sparse inode chunks on full chunk allocation failure Brian Foster
                   ` (8 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Brian Foster @ 2014-11-03 16:12 UTC (permalink / raw)
  To: xfs

Create xfs_inobt_rec_exists() to receive the parameters of a new sparse
inode chunk allocation and identify whether a record exists that is
capable of tracking this sparse chunk.

Signed-off-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/libxfs/xfs_ialloc.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
index b53fb5d..6879213 100644
--- a/fs/xfs/libxfs/xfs_ialloc.c
+++ b/fs/xfs/libxfs/xfs_ialloc.c
@@ -359,6 +359,67 @@ xfs_ialloc_inode_init(
 }
 
 /*
+ * Determine whether part of a sparse inode chunk that has just been allocated
+ * is covered by an existing inobt record.
+ */
+STATIC int
+xfs_inobt_rec_exists(
+	struct xfs_mount		*mp,
+	struct xfs_trans		*tp,
+	struct xfs_buf			*agbp,
+	xfs_agino_t			newino,
+	xfs_agino_t			count,
+	xfs_btnum_t			btnum,
+	struct xfs_inobt_rec_incore	*orec)
+{
+	struct xfs_btree_cur		*cur;
+	struct xfs_agi			*agi = XFS_BUF_TO_AGI(agbp);
+	xfs_agnumber_t			agno = be32_to_cpu(agi->agi_seqno);
+	int				error;
+	int				i;
+	struct xfs_inobt_rec_incore	rec;
+
+	orec->ir_startino = NULLAGINO;
+
+	cur = xfs_inobt_init_cursor(mp, tp, agbp, agno, btnum);
+
+	/*
+	 * Find the first record prior to the inode that has just been
+	 * allocated.
+	 */
+	error = xfs_inobt_lookup(cur, newino, XFS_LOOKUP_LE, &i);
+	if (error)
+		goto error;
+	if (i == 0)
+		goto out;
+
+	error = xfs_inobt_get_rec(cur, &rec, &i);
+	if (error)
+		goto error;
+	XFS_WANT_CORRUPTED_GOTO(i == 1, error);
+
+	/*
+	 * Check whether the record covers a range that includes the new inodes.
+	 * We can't allocate a sparse chunk that spans multiple records, so we
+	 * only need to check the ino range.
+	 */
+	if (rec.ir_startino + XFS_INODES_PER_CHUNK <= newino)
+		goto out;
+
+	ASSERT(rec.ir_startino <= newino &&
+	       rec.ir_startino + XFS_INODES_PER_CHUNK > newino);
+
+	*orec = rec;	/* struct copy */
+
+out:
+	xfs_btree_del_cursor(cur, XFS_BTREE_NOERROR);
+	return 0;
+error:
+	xfs_btree_del_cursor(cur, XFS_BTREE_ERROR);
+	return error;
+}
+
+/*
  * Allocate new inodes in the allocation group specified by agbp.
  * Return 0 for success, else error code.
  */
-- 
1.8.3.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v2 09/17] xfs: allocate sparse inode chunks on full chunk allocation failure
  2014-11-03 16:12 [PATCH v2 00/17] xfs: sparse inode chunks Brian Foster
                   ` (7 preceding siblings ...)
  2014-11-03 16:12 ` [PATCH v2 08/17] xfs: create helper to manage record overlap for sparse inode chunks Brian Foster
@ 2014-11-03 16:12 ` Brian Foster
  2014-11-03 16:12 ` [PATCH v2 10/17] xfs: helpers to convert holemask to/from generic bitmap Brian Foster
                   ` (7 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Brian Foster @ 2014-11-03 16:12 UTC (permalink / raw)
  To: xfs

xfs_ialloc_ag_alloc() makes several attempts to allocate a full inode
chunk. If all else fails, reduce the allocation to the minimum sparse
granularity and attempt to allocate a sparse inode chunk.

If sparse chunk allocation succeeds, check whether an inobt record
already exists that can track the chunk. If so, inherit and update the
existing record. Otherwise, insert a new record for the sparse chunk.

Update xfs_inobt_insert_rec() to take the holemask as a parameter and
set the associated field on disk. Create the xfs_inobt_update_insert()
helper to handle the sparse chunk allocation case - insert or update an
existing record depending on whether it already exists.

Signed-off-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/libxfs/xfs_ialloc.c | 149 +++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 136 insertions(+), 13 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
index 6879213..d22dd8a 100644
--- a/fs/xfs/libxfs/xfs_ialloc.c
+++ b/fs/xfs/libxfs/xfs_ialloc.c
@@ -123,12 +123,16 @@ xfs_inobt_get_rec(
 STATIC int
 xfs_inobt_insert_rec(
 	struct xfs_btree_cur	*cur,
+	__uint16_t		holemask,
+	__uint8_t		count,
 	__int32_t		freecount,
 	xfs_inofree_t		free,
 	int			*stat)
 {
-	cur->bc_rec.i.ir_holemask = 0;
-	cur->bc_rec.i.ir_count = 0; /* zero for backwards compatibility */
+	ASSERT(count == 0 || xfs_sb_version_hassparseinodes(&cur->bc_mp->m_sb));
+
+	cur->bc_rec.i.ir_holemask = holemask;
+	cur->bc_rec.i.ir_count = count;
 	cur->bc_rec.i.ir_freecount = freecount;
 	cur->bc_rec.i.ir_free = free;
 	return xfs_btree_insert(cur, stat);
@@ -152,6 +156,19 @@ xfs_inobt_insert(
 	xfs_agino_t		thisino;
 	int			i;
 	int			error;
+	uint8_t			count;
+
+	/*
+	 * Only set ir_count in the inobt record if the sparse inodes feature is
+	 * enabled. If disabled, we must maintain backwards compatibility with
+	 * the older inobt record format where the current count and holemask
+	 * fields map to the higher order bytes of freecount and thus must be
+	 * zeroed.
+	 */
+	if (xfs_sb_version_hassparseinodes(&mp->m_sb))
+		count = XFS_INODES_PER_CHUNK;
+	else
+		count = 0;
 
 	cur = xfs_inobt_init_cursor(mp, tp, agbp, agno, btnum);
 
@@ -165,7 +182,7 @@ xfs_inobt_insert(
 		}
 		ASSERT(i == 0);
 
-		error = xfs_inobt_insert_rec(cur, XFS_INODES_PER_CHUNK,
+		error = xfs_inobt_insert_rec(cur, 0, count, XFS_INODES_PER_CHUNK,
 					     XFS_INOBT_ALL_FREE, &i);
 		if (error) {
 			xfs_btree_del_cursor(cur, XFS_BTREE_ERROR);
@@ -175,8 +192,45 @@ xfs_inobt_insert(
 	}
 
 	xfs_btree_del_cursor(cur, XFS_BTREE_NOERROR);
+	return 0;
+}
+
+STATIC int
+xfs_inobt_update_insert(
+	struct xfs_mount		*mp,
+	struct xfs_trans		*tp,
+	struct xfs_buf			*agbp,
+	struct xfs_inobt_rec_incore	*rec,   /* record to update/insert */
+	xfs_btnum_t			btnum)
+{
+	struct xfs_btree_cur		*cur;
+	struct xfs_agi			*agi = XFS_BUF_TO_AGI(agbp);
+	xfs_agnumber_t			agno = be32_to_cpu(agi->agi_seqno);
+	int				i;
+	int				error;
+
+	cur = xfs_inobt_init_cursor(mp, tp, agbp, agno, btnum);
+
+	error = xfs_inobt_lookup(cur, rec->ir_startino, XFS_LOOKUP_EQ, &i);
+	if (i == 1) {
+		error = xfs_inobt_update(cur, rec);
+		if (error)
+			goto error;
+	} else {
+		error = xfs_inobt_insert_rec(cur, rec->ir_holemask,
+				rec->ir_count, rec->ir_freecount, rec->ir_free,
+				&i);
+		if (error)
+			goto error;
+		ASSERT(i == 1);
+	}
 
+	xfs_btree_del_cursor(cur, XFS_BTREE_NOERROR);
 	return 0;
+
+error:
+	xfs_btree_del_cursor(cur, XFS_BTREE_ERROR);
+	return error;
 }
 
 /*
@@ -437,6 +491,10 @@ xfs_ialloc_ag_alloc(
 	xfs_agino_t	newlen;		/* new number of inodes */
 	int		isaligned = 0;	/* inode allocation at stripe unit */
 					/* boundary */
+	uint16_t	allocmask = (uint16_t) -1; /* init. to full chunk */
+	struct xfs_inobt_rec_incore rec;
+	int		offset;
+
 	struct xfs_perag *pag;
 
 	memset(&args, 0, sizeof(args));
@@ -552,6 +610,27 @@ xfs_ialloc_ag_alloc(
 			return error;
 	}
 
+	/*
+	 * Finally, try a sparse allocation if the filesystem supports it.
+	 */
+	if (xfs_sb_version_hassparseinodes(&args.mp->m_sb) &&
+	    args.fsbno == NULLFSBLOCK) {
+		args.type = XFS_ALLOCTYPE_NEAR_BNO;
+		args.agbno = be32_to_cpu(agi->agi_root);
+		args.fsbno = XFS_AGB_TO_FSB(args.mp, agno, args.agbno);
+		args.alignment = args.mp->m_sb.sb_inoalignmt;
+
+		args.minlen = args.mp->m_ialloc_min_blks;
+		args.maxlen = args.minlen;
+
+		error = xfs_alloc_vextent(&args);
+		if (error)
+			return error;
+
+		newlen = args.len << args.mp->m_sb.sb_inopblog;
+		allocmask = (1 << (newlen / XFS_INODES_PER_HOLEMASK_BIT)) - 1;
+	}
+
 	if (args.fsbno == NULLFSBLOCK) {
 		*alloc = 0;
 		return 0;
@@ -583,20 +662,62 @@ xfs_ialloc_ag_alloc(
 	xfs_perag_put(pag);
 	agi->agi_newino = cpu_to_be32(newino);
 
-	/*
-	 * Insert records describing the new inode chunk into the btrees.
-	 */
-	error = xfs_inobt_insert(args.mp, tp, agbp, newino, newlen,
-				 XFS_BTNUM_INO);
-	if (error)
-		return error;
+	if (xfs_inobt_issparse(~allocmask)) {
+		/*
+		 * We've allocated a sparse chunk...
+		 */
+		error = xfs_inobt_rec_exists(args.mp, tp, agbp, newino,
+					     newlen, XFS_BTNUM_INO, &rec);
+		if (error)
+			return error;
+		if (rec.ir_startino == NULLAGINO) {
+			/* no existing record, set all fields */
+			rec.ir_startino = newino;
+			rec.ir_holemask = ~allocmask;
+			rec.ir_count = newlen;
+			rec.ir_freecount = newlen;
+			rec.ir_free = XFS_INOBT_ALL_FREE;
+		} else {
+			/* we already have a record, update it */
+			offset = newino - rec.ir_startino;
+			allocmask <<= offset / XFS_INODES_PER_HOLEMASK_BIT;
+
+			ASSERT(offset % XFS_INODES_PER_HOLEMASK_BIT == 0);
+			ASSERT(rec.ir_count + newlen <= XFS_INODES_PER_CHUNK);
+			ASSERT(rec.ir_freecount + newlen <=
+				XFS_INODES_PER_CHUNK);
+
+			rec.ir_count += newlen;
+			rec.ir_freecount += newlen;
+			rec.ir_holemask &= ~allocmask;
+		}
 
-	if (xfs_sb_version_hasfinobt(&args.mp->m_sb)) {
+		error = xfs_inobt_update_insert(args.mp, tp, agbp, &rec,
+						XFS_BTNUM_INO);
+		if (error)
+			return error;
+
+		if (xfs_sb_version_hasfinobt(&args.mp->m_sb)) {
+			error = xfs_inobt_update_insert(args.mp, tp, agbp, &rec,
+							XFS_BTNUM_FINO);
+			if (error)
+				return error;
+		}
+	} else {
+		/* full chunk - insert new records to both btrees */
 		error = xfs_inobt_insert(args.mp, tp, agbp, newino, newlen,
-					 XFS_BTNUM_FINO);
+					 XFS_BTNUM_INO);
 		if (error)
 			return error;
+
+		if (xfs_sb_version_hasfinobt(&args.mp->m_sb)) {
+			error = xfs_inobt_insert(args.mp, tp, agbp, newino,
+						 newlen, XFS_BTNUM_FINO);
+			if (error)
+				return error;
+		}
 	}
+
 	/*
 	 * Log allocation group header fields
 	 */
@@ -1657,7 +1778,9 @@ xfs_difree_finobt(
 		 */
 		XFS_WANT_CORRUPTED_GOTO(ibtrec->ir_freecount == 1, error);
 
-		error = xfs_inobt_insert_rec(cur, ibtrec->ir_freecount,
+		error = xfs_inobt_insert_rec(cur, ibtrec->ir_holemask,
+					     ibtrec->ir_count,
+					     ibtrec->ir_freecount,
 					     ibtrec->ir_free, &i);
 		if (error)
 			goto error;
-- 
1.8.3.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v2 10/17] xfs: helpers to convert holemask to/from generic bitmap
  2014-11-03 16:12 [PATCH v2 00/17] xfs: sparse inode chunks Brian Foster
                   ` (8 preceding siblings ...)
  2014-11-03 16:12 ` [PATCH v2 09/17] xfs: allocate sparse inode chunks on full chunk allocation failure Brian Foster
@ 2014-11-03 16:12 ` Brian Foster
  2014-11-03 16:12 ` [PATCH v2 11/17] xfs: filter out sparse regions from individual inode allocation Brian Foster
                   ` (6 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Brian Foster @ 2014-11-03 16:12 UTC (permalink / raw)
  To: xfs

The inobt record holemask field is a condensed data type designed to fit
into the existing on-disk record and is zero based (allocated regions
are set to 0, sparse regions are set to 1) to provide backwards
compatibility. This makes the type somewhat complex for use in higher
level inode manipulations such as individual inode allocation, etc.

Rather than foist the complexity of dealing with this field to every bit
of logic that requires inode granular information, create the
xfs_inobt_ialloc_bitmap() helper to convert the holemask to an inode
allocation bitmap. The inode allocation bitmap is inode granularity
similar to the inobt record free mask and indicates which inodes of the
chunk are physically allocated on disk, irrespective of whether the
inode is considered allocated or free by the filesystem. The generic
bitmap type requires the use of generic kernel bitmap APIs.

The bitmap_to_u64() helper is provided to convert the bitmap back to a
native 64-bit type (for native bitwise operations). This is required
because the generic bitmap code represents a bitmap as an array of
unsigned long in a little endian style (though each array value is cpu
order). To ensure compatibility with various word sizes and endianness',
bitmap_to_u64() exports the bitmap to little endian and swaps back to
cpu byte order.

Signed-off-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/libxfs/xfs_format.h |  1 +
 fs/xfs/libxfs/xfs_ialloc.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 68 insertions(+)

diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
index c81c1a7..f2ff6d5 100644
--- a/fs/xfs/libxfs/xfs_format.h
+++ b/fs/xfs/libxfs/xfs_format.h
@@ -202,6 +202,7 @@ typedef __be32 xfs_alloc_ptr_t;
 #define	XFS_FIBT_CRC_MAGIC	0x46494233	/* 'FIB3' */
 
 typedef	__uint64_t	xfs_inofree_t;
+typedef	__uint64_t	xfs_inoalloc_t;
 #define	XFS_INODES_PER_CHUNK		(NBBY * sizeof(xfs_inofree_t))
 #define	XFS_INODES_PER_CHUNK_LOG	(XFS_NBBYLOG + 3)
 #define	XFS_INOBT_ALL_FREE		((xfs_inofree_t)-1)
diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
index d22dd8a..88ca72f 100644
--- a/fs/xfs/libxfs/xfs_ialloc.c
+++ b/fs/xfs/libxfs/xfs_ialloc.c
@@ -910,6 +910,73 @@ xfs_ialloc_get_rec(
 	return 0;
 }
 
+static inline uint64_t
+bitmap_to_u64(
+	const unsigned long	*bitmap,
+	int			nbits)
+{
+	__le64			lebitmap;
+
+	ASSERT(nbits <= 64);
+
+	/*
+	 * The bitmap format depends on the native word size. E.g., bit 0 could
+	 * land in the middle of the 64 bits on a big endian 32-bit arch (see
+	 * arch/powerpc/include/asm/bitops.h). To handle this, export the bitmap
+	 * as 64-bit little endian and convert back to native byte order.
+	 */
+	bitmap_copy_le(&lebitmap, bitmap, nbits);
+	return le64_to_cpu(lebitmap);
+}
+
+/*
+ * Convert the inode record holemask to an inode allocation bitmap. The inode
+ * allocation bitmap is inode granularity and specifies whether an inode is
+ * physically allocated on disk (not whether the inode is considered allocated
+ * or free by the fs).
+ *
+ * We have to be careful with regard to byte order and word size since the
+ * generic bitmap code uses primitive types.
+ */
+STATIC void
+xfs_inobt_ialloc_bitmap(
+	unsigned long			*allocbmap,
+	struct xfs_inobt_rec_incore	*rec)
+{
+	int				nextbit;
+	DECLARE_BITMAP(holemask, 16);
+
+	bitmap_zero(allocbmap, 64);
+
+	/*
+	 * bitmaps are represented as an array of unsigned long (each in cpu
+	 * byte order). ir_holemask is only 16 bits, so direct assignment is
+	 * safe.
+	 */
+	ASSERT(sizeof(rec->ir_holemask) <= sizeof(holemask[0]));
+	holemask[0] = rec->ir_holemask;
+
+	/*
+	 * holemask is a 16-bit bitmap and inode records span 64 inodes. Thus
+	 * each holemask bit represents multiple (XFS_INODES_PER_HOLEMASK_BIT)
+	 * inodes. Since holemask bits represent holes, each set bit represents
+	 * a region of the physical chunk that is not tracked by the record.
+	 *
+	 * We want an inode granularity allocation bitmap. We have to invert the
+	 * holemask and set XFS_INODES_PER_HOLEMASK_BIT bits for each set bit.
+	 * We invert and expand simultaneously by walking the unset (zeroed)
+	 * holemask bits. For each zero bit in holemask, set the corresponding
+	 * XFS_INODES_PER_HOLEMASK_BIT bits in the allocation bitmap.
+	 */
+	nextbit = find_first_zero_bit(holemask, 16);
+	while (nextbit < 16) {
+		bitmap_set(allocbmap, nextbit * XFS_INODES_PER_HOLEMASK_BIT,
+			   XFS_INODES_PER_HOLEMASK_BIT);
+
+		nextbit = find_next_zero_bit(holemask, 16, nextbit + 1);
+	}
+}
+
 /*
  * Allocate an inode using the inobt-only algorithm.
  */
-- 
1.8.3.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v2 11/17] xfs: filter out sparse regions from individual inode allocation
  2014-11-03 16:12 [PATCH v2 00/17] xfs: sparse inode chunks Brian Foster
                   ` (9 preceding siblings ...)
  2014-11-03 16:12 ` [PATCH v2 10/17] xfs: helpers to convert holemask to/from generic bitmap Brian Foster
@ 2014-11-03 16:12 ` Brian Foster
  2014-11-03 16:12 ` [PATCH v2 12/17] xfs: update free inode record logic to support sparse inode records Brian Foster
                   ` (5 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Brian Foster @ 2014-11-03 16:12 UTC (permalink / raw)
  To: xfs

Inode allocation from an existing record with free inodes traditionally
selects the first inode available according to the ir_free mask. With
sparse inode chunks, the ir_free mask could refer to an unallocated
region. We must mask the unallocated regions out of ir_free before using
it to select a free inode in the chunk.

Create the xfs_inobt_first_free_inode() helper to find the first free
inode available of the allocated regions of the inode chunk.

Signed-off-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/libxfs/xfs_ialloc.c | 28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
index 88ca72f..2bc998d 100644
--- a/fs/xfs/libxfs/xfs_ialloc.c
+++ b/fs/xfs/libxfs/xfs_ialloc.c
@@ -978,6 +978,30 @@ xfs_inobt_ialloc_bitmap(
 }
 
 /*
+ * Return the offset of the first free inode in the record. If the inode chunk
+ * is sparsely allocated, we convert the record holemask to inode granularity
+ * and mask off the unallocated regions from the inode free mask.
+ */
+STATIC int
+xfs_inobt_first_free_inode(
+	struct xfs_inobt_rec_incore	*rec)
+{
+	xfs_inofree_t	realfree;
+	DECLARE_BITMAP(alloc, 64);
+
+	/* if there are no holes, return the first available offset */
+	if (!xfs_inobt_issparse(rec->ir_holemask))
+		return xfs_lowbit64(rec->ir_free);
+
+	xfs_inobt_ialloc_bitmap(alloc, rec);
+	realfree = bitmap_to_u64(alloc, 64);
+
+	realfree &= rec->ir_free;
+
+	return xfs_lowbit64(realfree);
+}
+
+/*
  * Allocate an inode using the inobt-only algorithm.
  */
 STATIC int
@@ -1207,7 +1231,7 @@ newino:
 	}
 
 alloc_inode:
-	offset = xfs_lowbit64(rec.ir_free);
+	offset = xfs_inobt_first_free_inode(&rec);
 	ASSERT(offset >= 0);
 	ASSERT(offset < XFS_INODES_PER_CHUNK);
 	ASSERT((XFS_AGINO_TO_OFFSET(mp, rec.ir_startino) %
@@ -1460,7 +1484,7 @@ xfs_dialloc_ag(
 	if (error)
 		goto error_cur;
 
-	offset = xfs_lowbit64(rec.ir_free);
+	offset = xfs_inobt_first_free_inode(&rec);
 	ASSERT(offset >= 0);
 	ASSERT(offset < XFS_INODES_PER_CHUNK);
 	ASSERT((XFS_AGINO_TO_OFFSET(mp, rec.ir_startino) %
-- 
1.8.3.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v2 12/17] xfs: update free inode record logic to support sparse inode records
  2014-11-03 16:12 [PATCH v2 00/17] xfs: sparse inode chunks Brian Foster
                   ` (10 preceding siblings ...)
  2014-11-03 16:12 ` [PATCH v2 11/17] xfs: filter out sparse regions from individual inode allocation Brian Foster
@ 2014-11-03 16:12 ` Brian Foster
  2014-11-03 16:12 ` [PATCH v2 13/17] xfs: only free allocated regions of inode chunks Brian Foster
                   ` (4 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Brian Foster @ 2014-11-03 16:12 UTC (permalink / raw)
  To: xfs

xfs_difree_inobt() uses logic in a couple places that assume inobt
records refer to fully allocated chunks. Specifically, the use of
mp->m_ialloc_inos can cause problems for inode chunks that are sparsely
allocated. Sparse inode chunks can, by definition, define a smaller
number of inodes than a full inode chunk.

Fix the logic that determines whether an inode record should be removed
from the inobt to use the ir_free mask rather than ir_freecount.

Fix the agi counters modification to use ir_freecount to add the actual
number of inodes freed rather than assuming a full inode chunk.

Signed-off-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/libxfs/xfs_ialloc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
index 2bc998d..c9c7289 100644
--- a/fs/xfs/libxfs/xfs_ialloc.c
+++ b/fs/xfs/libxfs/xfs_ialloc.c
@@ -1774,7 +1774,7 @@ xfs_difree_inobt(
 	 * When an inode cluster is free, it becomes eligible for removal
 	 */
 	if (!(mp->m_flags & XFS_MOUNT_IKEEP) &&
-	    (rec.ir_freecount == mp->m_ialloc_inos)) {
+	    (rec.ir_free == XFS_INOBT_ALL_FREE)) {
 
 		*deleted = 1;
 		*first_ino = XFS_AGINO_TO_INO(mp, agno, rec.ir_startino);
@@ -1784,7 +1784,7 @@ xfs_difree_inobt(
 		 * AGI and Superblock inode counts, and mark the disk space
 		 * to be freed when the transaction is committed.
 		 */
-		ilen = mp->m_ialloc_inos;
+		ilen = rec.ir_freecount;
 		be32_add_cpu(&agi->agi_count, -ilen);
 		be32_add_cpu(&agi->agi_freecount, -(ilen - 1));
 		xfs_ialloc_log_agi(tp, agbp, XFS_AGI_COUNT | XFS_AGI_FREECOUNT);
@@ -1907,7 +1907,7 @@ xfs_difree_finobt(
 	 * keeping inode chunks permanently on disk, remove the record.
 	 * Otherwise, update the record with the new information.
 	 */
-	if (rec.ir_freecount == mp->m_ialloc_inos &&
+	if (rec.ir_free == XFS_INOBT_ALL_FREE &&
 	    !(mp->m_flags & XFS_MOUNT_IKEEP)) {
 		error = xfs_btree_delete(cur, &i);
 		if (error)
-- 
1.8.3.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v2 13/17] xfs: only free allocated regions of inode chunks
  2014-11-03 16:12 [PATCH v2 00/17] xfs: sparse inode chunks Brian Foster
                   ` (11 preceding siblings ...)
  2014-11-03 16:12 ` [PATCH v2 12/17] xfs: update free inode record logic to support sparse inode records Brian Foster
@ 2014-11-03 16:12 ` Brian Foster
  2014-11-03 16:12 ` [PATCH v2 14/17] xfs: skip unallocated regions of inode chunks in xfs_ifree_cluster() Brian Foster
                   ` (3 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Brian Foster @ 2014-11-03 16:12 UTC (permalink / raw)
  To: xfs

An inode chunk is currently added to the transaction free list based on
a simple fsb conversion and hardcoded chunk length. The nature of sparse
chunks is such that the physical chunk of inodes on disk may consist of
one or more discontiguous parts. Blocks that reside in the holes of the
inode chunk are not inodes and could be allocated to any other use or
not allocated at all.

Refactor the existing xfs_bmap_add_free() call into the
xfs_difree_inode_chunk() helper. The new helper uses the existing
calculation if a chunk is not sparse. Otherwise, use the inobt record
holemask to free the contiguous regions of the chunk.

Signed-off-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/libxfs/xfs_ialloc.c | 81 ++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 78 insertions(+), 3 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
index c9c7289..af16d5e 100644
--- a/fs/xfs/libxfs/xfs_ialloc.c
+++ b/fs/xfs/libxfs/xfs_ialloc.c
@@ -1709,6 +1709,83 @@ out_error:
 	return error;
 }
 
+/*
+ * Free the blocks of an inode chunk. We must consider that the inode chunk
+ * might be sparse and only free the regions that are allocated as part of the
+ * chunk.
+ */
+STATIC void
+xfs_difree_inode_chunk(
+	struct xfs_mount		*mp,
+	xfs_agnumber_t			agno,
+	struct xfs_inobt_rec_incore	*rec,
+	struct xfs_bmap_free		*flist)
+{
+	xfs_agblock_t	sagbno = XFS_AGINO_TO_AGBNO(mp, rec->ir_startino);
+	int		startidx, endidx;
+	int		nextbit;
+	xfs_agblock_t	agbno;
+	int		contigblk;
+	DECLARE_BITMAP(holemask, XFS_INOBT_HOLEMASK_BITS);
+
+	if (!xfs_inobt_issparse(rec->ir_holemask)) {
+		/* not sparse, calculate extent info directly */
+		xfs_bmap_add_free(XFS_AGB_TO_FSB(mp, agno,
+				  XFS_AGINO_TO_AGBNO(mp, rec->ir_startino)),
+				  mp->m_ialloc_blks, flist, mp);
+		return;
+	}
+
+	/* holemask is only 16-bits (fits in an unsigned long) */
+	ASSERT(sizeof(rec->ir_holemask) <= sizeof(holemask[0]));
+	holemask[0] = rec->ir_holemask;
+
+	/*
+	 * Find contiguous ranges of zeroes (i.e., allocated regions) in the
+	 * holemask and convert the start/end index of each range to an extent.
+	 * We start with the start and end index both pointing at the first 0 in
+	 * the mask.
+	 */
+	startidx = endidx = find_first_zero_bit(holemask,
+						XFS_INOBT_HOLEMASK_BITS);
+	nextbit = startidx + 1;
+	while (startidx < XFS_INOBT_HOLEMASK_BITS) {
+		nextbit = find_next_zero_bit(holemask, XFS_INOBT_HOLEMASK_BITS,
+					     nextbit);
+		/*
+		 * If the next zero bit is contiguous, update the end index of
+		 * the current range and continue.
+		 */
+		if (nextbit != XFS_INOBT_HOLEMASK_BITS &&
+		    nextbit == endidx + 1) {
+			endidx = nextbit;
+			goto next;
+		}
+
+		/*
+		 * nextbit is not contiguous with the current end index. Convert
+		 * the current start/end to an extent and add it to the free
+		 * list.
+		 */
+		agbno = sagbno + (startidx * XFS_INODES_PER_HOLEMASK_BIT) /
+				  mp->m_sb.sb_inopblock;
+		contigblk = ((endidx - startidx + 1) *
+			     XFS_INODES_PER_HOLEMASK_BIT) /
+			    mp->m_sb.sb_inopblock;
+
+		ASSERT(agbno % mp->m_sb.sb_spinoalignmt == 0);
+		ASSERT(contigblk % mp->m_sb.sb_spinoalignmt == 0);
+		xfs_bmap_add_free(XFS_AGB_TO_FSB(mp, agno, agbno), contigblk,
+				  flist, mp);
+
+		/* reset range to current bit and carry on... */
+		startidx = endidx = nextbit;
+
+next:
+		nextbit++;
+	}
+}
+
 STATIC int
 xfs_difree_inobt(
 	struct xfs_mount		*mp,
@@ -1800,9 +1877,7 @@ xfs_difree_inobt(
 			goto error0;
 		}
 
-		xfs_bmap_add_free(XFS_AGB_TO_FSB(mp, agno,
-				  XFS_AGINO_TO_AGBNO(mp, rec.ir_startino)),
-				  mp->m_ialloc_blks, flist, mp);
+		xfs_difree_inode_chunk(mp, agno, &rec, flist);
 	} else {
 		*deleted = 0;
 
-- 
1.8.3.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v2 14/17] xfs: skip unallocated regions of inode chunks in xfs_ifree_cluster()
  2014-11-03 16:12 [PATCH v2 00/17] xfs: sparse inode chunks Brian Foster
                   ` (12 preceding siblings ...)
  2014-11-03 16:12 ` [PATCH v2 13/17] xfs: only free allocated regions of inode chunks Brian Foster
@ 2014-11-03 16:12 ` Brian Foster
  2014-11-03 16:12 ` [PATCH v2 15/17] xfs: use actual inode count for sparse records in bulkstat/inumbers Brian Foster
                   ` (2 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Brian Foster @ 2014-11-03 16:12 UTC (permalink / raw)
  To: xfs

xfs_ifree_cluster() is called to mark all in-memory inodes and inode
buffers as stale. This occurs after we've removed the inobt records and
dropped any references of inobt data. xfs_ifree_cluster() uses the
starting inode number to walk the namespace of inodes expected for a
single chunk a cluster buffer at a time. The cluster buffer disk
addresses are calculated by decoding the sequential inode numbers
expected from the chunk.

The problem with this approach is that if the inode chunk being removed
is a sparse chunk, not all of the buffer addresses that are calculated
as part of this sequence may be inode clusters. Attempting to acquire
the buffer based on expected inode characterstics (i.e., cluster length)
can lead to errors and is generally incorrect.

We already use a couple variables to carry requisite state from
xfs_difree() to xfs_ifree_cluster(). Rather than add a third, define a
new internal structure to carry the existing parameters through these
functions. Add an alloc field that represents the physical allocation
bitmap of inodes in the chunk being removed. Modify xfs_ifree_cluster()
to check each inode against the bitmap and skip the clusters that were
never allocated as real inodes on disk.

Signed-off-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/libxfs/xfs_ialloc.c | 18 +++++++++---------
 fs/xfs/libxfs/xfs_ialloc.h | 10 ++++++++--
 fs/xfs/xfs_inode.c         | 28 ++++++++++++++++++++--------
 3 files changed, 37 insertions(+), 19 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
index af16d5e..1fbd0a8 100644
--- a/fs/xfs/libxfs/xfs_ialloc.c
+++ b/fs/xfs/libxfs/xfs_ialloc.c
@@ -1793,8 +1793,7 @@ xfs_difree_inobt(
 	struct xfs_buf			*agbp,
 	xfs_agino_t			agino,
 	struct xfs_bmap_free		*flist,
-	int				*deleted,
-	xfs_ino_t			*first_ino,
+	struct xfs_icluster		*xic,
 	struct xfs_inobt_rec_incore	*orec)
 {
 	struct xfs_agi			*agi = XFS_BUF_TO_AGI(agbp);
@@ -1852,9 +1851,12 @@ xfs_difree_inobt(
 	 */
 	if (!(mp->m_flags & XFS_MOUNT_IKEEP) &&
 	    (rec.ir_free == XFS_INOBT_ALL_FREE)) {
+		DECLARE_BITMAP(alloc, 64);
 
-		*deleted = 1;
-		*first_ino = XFS_AGINO_TO_INO(mp, agno, rec.ir_startino);
+		xic->deleted = 1;
+		xic->first_ino = XFS_AGINO_TO_INO(mp, agno, rec.ir_startino);
+		xfs_inobt_ialloc_bitmap(alloc, &rec);
+		xic->alloc = bitmap_to_u64(alloc, 64);
 
 		/*
 		 * Remove the inode cluster from the AGI B+Tree, adjust the
@@ -1879,7 +1881,7 @@ xfs_difree_inobt(
 
 		xfs_difree_inode_chunk(mp, agno, &rec, flist);
 	} else {
-		*deleted = 0;
+		xic->deleted = 0;
 
 		error = xfs_inobt_update(cur, &rec);
 		if (error) {
@@ -2018,8 +2020,7 @@ xfs_difree(
 	struct xfs_trans	*tp,		/* transaction pointer */
 	xfs_ino_t		inode,		/* inode to be freed */
 	struct xfs_bmap_free	*flist,		/* extents to free */
-	int			*deleted,/* set if inode cluster was deleted */
-	xfs_ino_t		*first_ino)/* first inode in deleted cluster */
+	struct xfs_icluster	*xic)	/* cluster info if deleted */
 {
 	/* REFERENCED */
 	xfs_agblock_t		agbno;	/* block number containing inode */
@@ -2070,8 +2071,7 @@ xfs_difree(
 	/*
 	 * Fix up the inode allocation btree.
 	 */
-	error = xfs_difree_inobt(mp, tp, agbp, agino, flist, deleted, first_ino,
-				 &rec);
+	error = xfs_difree_inobt(mp, tp, agbp, agino, flist, xic, &rec);
 	if (error)
 		goto error0;
 
diff --git a/fs/xfs/libxfs/xfs_ialloc.h b/fs/xfs/libxfs/xfs_ialloc.h
index 62c1381..5aa8d6f 100644
--- a/fs/xfs/libxfs/xfs_ialloc.h
+++ b/fs/xfs/libxfs/xfs_ialloc.h
@@ -28,6 +28,13 @@ struct xfs_btree_cur;
 /* Move inodes in clusters of this size */
 #define	XFS_INODE_BIG_CLUSTER_SIZE	8192
 
+struct xfs_icluster {
+	bool		deleted;	/* record is deleted */
+	xfs_ino_t	first_ino;	/* first inode number */
+	xfs_inoalloc_t	alloc;		/* inode phys. allocation bitmap for
+					 * sparse chunks */
+};
+
 /* Calculate and return the number of filesystem blocks per inode cluster */
 static inline int
 xfs_icluster_size_fsb(
@@ -90,8 +97,7 @@ xfs_difree(
 	struct xfs_trans *tp,		/* transaction pointer */
 	xfs_ino_t	inode,		/* inode to be freed */
 	struct xfs_bmap_free *flist,	/* extents to free */
-	int		*deleted,	/* set if inode cluster was deleted */
-	xfs_ino_t	*first_ino);	/* first inode in deleted cluster */
+	struct xfs_icluster *ifree);	/* cluster info if deleted */
 
 /*
  * Return the location of the inode in imap, for mapping it into a buffer.
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 8ed049d..91a1739 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -2185,9 +2185,9 @@ xfs_iunlink_remove(
  */
 STATIC int
 xfs_ifree_cluster(
-	xfs_inode_t	*free_ip,
-	xfs_trans_t	*tp,
-	xfs_ino_t	inum)
+	xfs_inode_t		*free_ip,
+	xfs_trans_t		*tp,
+	struct xfs_icluster	*xic)
 {
 	xfs_mount_t		*mp = free_ip->i_mount;
 	int			blks_per_cluster;
@@ -2200,13 +2200,26 @@ xfs_ifree_cluster(
 	xfs_inode_log_item_t	*iip;
 	xfs_log_item_t		*lip;
 	struct xfs_perag	*pag;
+	xfs_ino_t		inum;
 
+	inum = xic->first_ino;
 	pag = xfs_perag_get(mp, XFS_INO_TO_AGNO(mp, inum));
 	blks_per_cluster = xfs_icluster_size_fsb(mp);
 	inodes_per_cluster = blks_per_cluster << mp->m_sb.sb_inopblog;
 	nbufs = mp->m_ialloc_blks / blks_per_cluster;
 
 	for (j = 0; j < nbufs; j++, inum += inodes_per_cluster) {
+		/*
+		 * The allocation bitmap tells us which inodes of the chunk were
+		 * physically allocated. Skip the cluster if an inode falls into
+		 * a sparse region.
+		 */
+		if ((xic->alloc & XFS_INOBT_MASK(inum - xic->first_ino)) == 0) {
+			ASSERT(((inum - xic->first_ino) %
+				inodes_per_cluster) == 0);
+			continue;
+		}
+
 		blkno = XFS_AGB_TO_DADDR(mp, XFS_INO_TO_AGNO(mp, inum),
 					 XFS_INO_TO_AGBNO(mp, inum));
 
@@ -2364,8 +2377,7 @@ xfs_ifree(
 	xfs_bmap_free_t	*flist)
 {
 	int			error;
-	int			delete;
-	xfs_ino_t		first_ino;
+	struct xfs_icluster	xic = { 0 };
 
 	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
 	ASSERT(ip->i_d.di_nlink == 0);
@@ -2381,7 +2393,7 @@ xfs_ifree(
 	if (error)
 		return error;
 
-	error = xfs_difree(tp, ip->i_ino, flist, &delete, &first_ino);
+	error = xfs_difree(tp, ip->i_ino, flist, &xic);
 	if (error)
 		return error;
 
@@ -2398,8 +2410,8 @@ xfs_ifree(
 	ip->i_d.di_gen++;
 	xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
 
-	if (delete)
-		error = xfs_ifree_cluster(ip, tp, first_ino);
+	if (xic.deleted)
+		error = xfs_ifree_cluster(ip, tp, &xic);
 
 	return error;
 }
-- 
1.8.3.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v2 15/17] xfs: use actual inode count for sparse records in bulkstat/inumbers
  2014-11-03 16:12 [PATCH v2 00/17] xfs: sparse inode chunks Brian Foster
                   ` (13 preceding siblings ...)
  2014-11-03 16:12 ` [PATCH v2 14/17] xfs: skip unallocated regions of inode chunks in xfs_ifree_cluster() Brian Foster
@ 2014-11-03 16:12 ` Brian Foster
  2014-11-03 16:12 ` [PATCH v2 16/17] xfs: add fs geometry bit for sparse inode chunks Brian Foster
  2014-11-03 16:12 ` [PATCH v2 17/17] xfs: enable sparse inode chunks for v5 superblocks Brian Foster
  16 siblings, 0 replies; 24+ messages in thread
From: Brian Foster @ 2014-11-03 16:12 UTC (permalink / raw)
  To: xfs

The bulkstat and inumbers mechanisms make the assumption that inode
records consist of a full 64 inode chunk in several places. E.g., this
is used to track how many inodes have been processed overall as well as
to determine when all in-use inodes of a record have been processed. The
record processing, in particular, increments the record freecount for
each in-use inode until it hits the expected max of 64.

This is invalid for sparse inode records. While all inodes might be
marked free in the free mask regardless of whether they are allocated on
disk, ir_freecount is based on the total number of physically allocated
inodes and thus may be less than 64 inodes on a completely free inode
chunk.

Create the xfs_inobt_count() helper to calculate the total number of
physically allocated inodes based on the holemask. Use the helper in
xfs_bulkstat() and xfs_inumbers() instead of the fixed
XFS_INODE_PER_CHUNK value to ensure correct accounting in the event of
sparse inode records.

Signed-off-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/libxfs/xfs_ialloc.c | 29 +++++++++++++++++++++++++++++
 fs/xfs/libxfs/xfs_ialloc.h |  5 +++++
 fs/xfs/xfs_itable.c        | 16 +++++++++++-----
 3 files changed, 45 insertions(+), 5 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
index 1fbd0a8..5e92422 100644
--- a/fs/xfs/libxfs/xfs_ialloc.c
+++ b/fs/xfs/libxfs/xfs_ialloc.c
@@ -1002,6 +1002,35 @@ xfs_inobt_first_free_inode(
 }
 
 /*
+ * Calculate the real count of inodes in a chunk.
+ */
+int
+xfs_inobt_count(
+	struct xfs_mount		*mp,
+	struct xfs_inobt_rec_incore	*rec)
+{
+	int	inocount;
+	DECLARE_BITMAP(allocbmap, XFS_INODES_PER_CHUNK);
+
+	if (xfs_sb_version_hassparseinodes(&mp->m_sb)) {
+		/*
+		 * allocated inode bit count should match ir_count
+		 *
+		 * TODO: it might be preferable to tie this validation to an
+		 * fs operation rather than a simple utility function call
+		 */
+		xfs_inobt_ialloc_bitmap(allocbmap, rec);
+		inocount = bitmap_weight(allocbmap, XFS_INODES_PER_CHUNK);
+		WARN_ON(inocount != rec->ir_count);
+	}
+
+	if (!xfs_inobt_issparse(rec->ir_holemask))
+		return XFS_INODES_PER_CHUNK;
+
+	return rec->ir_count;
+}
+
+/*
  * Allocate an inode using the inobt-only algorithm.
  */
 STATIC int
diff --git a/fs/xfs/libxfs/xfs_ialloc.h b/fs/xfs/libxfs/xfs_ialloc.h
index 5aa8d6f..527d889 100644
--- a/fs/xfs/libxfs/xfs_ialloc.h
+++ b/fs/xfs/libxfs/xfs_ialloc.h
@@ -166,4 +166,9 @@ int xfs_ialloc_inode_init(struct xfs_mount *mp, struct xfs_trans *tp,
 			  xfs_agnumber_t agno, xfs_agblock_t agbno,
 			  xfs_agblock_t length, unsigned int gen);
 
+/*
+ * Calculate the real count of inodes in a chunk.
+ */
+int xfs_inobt_count(struct xfs_mount *mp, struct xfs_inobt_rec_incore *rec);
+
 #endif	/* __XFS_IALLOC_H__ */
diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
index 7765ff7..b44a9f0 100644
--- a/fs/xfs/xfs_itable.c
+++ b/fs/xfs/xfs_itable.c
@@ -254,7 +254,8 @@ xfs_bulkstat_grab_ichunk(
 		}
 
 		irec->ir_free |= xfs_inobt_maskn(0, idx);
-		*icount = XFS_INODES_PER_CHUNK - irec->ir_freecount;
+		*icount = xfs_inobt_count(cur->bc_mp, irec) -
+			  irec->ir_freecount;
 	}
 
 	return 0;
@@ -285,7 +286,7 @@ xfs_bulkstat_ag_ichunk(
 
 	for (agino = irbp->ir_startino, chunkidx = clustidx = 0;
 	     XFS_BULKSTAT_UBLEFT(ubleft) &&
-	     irbp->ir_freecount < XFS_INODES_PER_CHUNK;
+	     irbp->ir_freecount < xfs_inobt_count(mp, irbp);
 	     chunkidx++, clustidx++, agino++) {
 		int		fmterror;	/* bulkstat formatter result */
 		int		ubused;
@@ -430,6 +431,8 @@ xfs_bulkstat(
 				goto del_cursor;
 			if (icount) {
 				irbp->ir_startino = r.ir_startino;
+				irbp->ir_holemask = r.ir_holemask;
+				irbp->ir_count = r.ir_count;
 				irbp->ir_freecount = r.ir_freecount;
 				irbp->ir_free = r.ir_free;
 				irbp++;
@@ -461,13 +464,16 @@ xfs_bulkstat(
 			 * If this chunk has any allocated inodes, save it.
 			 * Also start read-ahead now for this chunk.
 			 */
-			if (r.ir_freecount < XFS_INODES_PER_CHUNK) {
+			if (r.ir_freecount < xfs_inobt_count(mp, &r)) {
 				xfs_bulkstat_ichunk_ra(mp, agno, &r);
 				irbp->ir_startino = r.ir_startino;
+				irbp->ir_holemask = r.ir_holemask;
+				irbp->ir_count = r.ir_count;
 				irbp->ir_freecount = r.ir_freecount;
 				irbp->ir_free = r.ir_free;
 				irbp++;
-				icount += XFS_INODES_PER_CHUNK - r.ir_freecount;
+				icount += xfs_inobt_count(mp, &r) -
+					  r.ir_freecount;
 			}
 			/*
 			 * Set agino to after this chunk and bump the cursor.
@@ -620,7 +626,7 @@ xfs_inumbers(
 		buffer[bufidx].xi_startino =
 			XFS_AGINO_TO_INO(mp, agno, r.ir_startino);
 		buffer[bufidx].xi_alloccount =
-			XFS_INODES_PER_CHUNK - r.ir_freecount;
+			xfs_inobt_count(mp, &r) - r.ir_freecount;
 		buffer[bufidx].xi_allocmask = ~r.ir_free;
 		if (++bufidx == bcount) {
 			long	written;
-- 
1.8.3.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v2 16/17] xfs: add fs geometry bit for sparse inode chunks
  2014-11-03 16:12 [PATCH v2 00/17] xfs: sparse inode chunks Brian Foster
                   ` (14 preceding siblings ...)
  2014-11-03 16:12 ` [PATCH v2 15/17] xfs: use actual inode count for sparse records in bulkstat/inumbers Brian Foster
@ 2014-11-03 16:12 ` Brian Foster
  2014-11-03 16:12 ` [PATCH v2 17/17] xfs: enable sparse inode chunks for v5 superblocks Brian Foster
  16 siblings, 0 replies; 24+ messages in thread
From: Brian Foster @ 2014-11-03 16:12 UTC (permalink / raw)
  To: xfs

Define an fs geometry bit for sparse inode chunks such that the
characteristic of the fs can be identified by userspace.

Signed-off-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/xfs_fs.h    | 1 +
 fs/xfs/xfs_fsops.c | 4 +++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_fs.h b/fs/xfs/xfs_fs.h
index 18dc721..89689c6 100644
--- a/fs/xfs/xfs_fs.h
+++ b/fs/xfs/xfs_fs.h
@@ -239,6 +239,7 @@ typedef struct xfs_fsop_resblks {
 #define XFS_FSOP_GEOM_FLAGS_V5SB	0x8000	/* version 5 superblock */
 #define XFS_FSOP_GEOM_FLAGS_FTYPE	0x10000	/* inode directory types */
 #define XFS_FSOP_GEOM_FLAGS_FINOBT	0x20000	/* free inode btree */
+#define XFS_FSOP_GEOM_FLAGS_SPINODES	0x40000	/* sparse inode chunks	*/
 
 /*
  * Minimum and maximum sizes need for growth checks.
diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
index c05ac8b..8d925e8 100644
--- a/fs/xfs/xfs_fsops.c
+++ b/fs/xfs/xfs_fsops.c
@@ -103,7 +103,9 @@ xfs_fs_geometry(
 			(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_FSOP_GEOM_FLAGS_FINOBT : 0) |
+			(xfs_sb_version_hassparseinodes(&mp->m_sb) ?
+				XFS_FSOP_GEOM_FLAGS_SPINODES : 0);
 		geo->logsectsize = xfs_sb_version_hassector(&mp->m_sb) ?
 				mp->m_sb.sb_logsectsize : BBSIZE;
 		geo->rtsectsize = mp->m_sb.sb_blocksize;
-- 
1.8.3.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 24+ messages in thread
* [PATCH v2 17/17] xfs: enable sparse inode chunks for v5 superblocks
  2014-11-03 16:12 [PATCH v2 00/17] xfs: sparse inode chunks Brian Foster
                   ` (15 preceding siblings ...)
  2014-11-03 16:12 ` [PATCH v2 16/17] xfs: add fs geometry bit for sparse inode chunks Brian Foster
@ 2014-11-03 16:12 ` Brian Foster
  16 siblings, 0 replies; 24+ messages in thread
From: Brian Foster @ 2014-11-03 16:12 UTC (permalink / raw)
  To: xfs

Enable mounting of filesystems with sparse inode support enabled. Add
the incompat. feature bit to the *_ALL mask.

Signed-off-by: Brian Foster <bfoster@redhat.com>
---
 fs/xfs/libxfs/xfs_sb.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/libxfs/xfs_sb.h b/fs/xfs/libxfs/xfs_sb.h
index eeb4058..b4c0d38 100644
--- a/fs/xfs/libxfs/xfs_sb.h
+++ b/fs/xfs/libxfs/xfs_sb.h
@@ -511,7 +511,8 @@ xfs_sb_has_ro_compat_feature(
 #define XFS_SB_FEAT_INCOMPAT_FTYPE	(1 << 0)	/* filetype in dirent */
 #define XFS_SB_FEAT_INCOMPAT_SPINODES	(1 << 1)	/* sparse inode chunks */
 #define XFS_SB_FEAT_INCOMPAT_ALL \
-		(XFS_SB_FEAT_INCOMPAT_FTYPE)
+		(XFS_SB_FEAT_INCOMPAT_FTYPE|	\
+		 XFS_SB_FEAT_INCOMPAT_SPINODES)
 
 #define XFS_SB_FEAT_INCOMPAT_UNKNOWN	~XFS_SB_FEAT_INCOMPAT_ALL
 static inline bool
-- 
1.8.3.1

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 24+ messages in thread
* Re: [PATCH v2 05/17] xfs: create macros/helpers for dealing with sparse inode chunks
  2014-11-03 16:12 ` [PATCH v2 05/17] xfs: create macros/helpers for dealing with " Brian Foster
@ 2014-11-03 22:12   ` Fanael Linithien
  2014-11-03 22:34     ` Brian Foster
  0 siblings, 1 reply; 24+ messages in thread
From: Fanael Linithien @ 2014-11-03 22:12 UTC (permalink / raw)
  Cc: xfs

2014-11-03 17:12 GMT+01:00 Brian Foster <bfoster@redhat.com>:
>
> +static inline bool xfs_inobt_issparse(uint16_t holemask)
> +{
> +       return holemask == 0 ? false : true;
> +}

Surely that should be "return holemask != 0;"?

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v2 05/17] xfs: create macros/helpers for dealing with sparse inode chunks
  2014-11-03 22:12   ` Fanael Linithien
@ 2014-11-03 22:34     ` Brian Foster
  2014-11-03 23:56       ` Fanael Linithien
  0 siblings, 1 reply; 24+ messages in thread
From: Brian Foster @ 2014-11-03 22:34 UTC (permalink / raw)
  To: Fanael Linithien; +Cc: xfs

On Mon, Nov 03, 2014 at 11:12:12PM +0100, Fanael Linithien wrote:
> 2014-11-03 17:12 GMT+01:00 Brian Foster <bfoster@redhat.com>:
> >
> > +static inline bool xfs_inobt_issparse(uint16_t holemask)
> > +{
> > +       return holemask == 0 ? false : true;
> > +}
> 
> Surely that should be "return holemask != 0;"?
> 

ir_holemask bits are set for holes in the inode chunk and unset for
allocated regions. This means that ir_holemask == 0 for a normal,
fully-allocated chunk and != 0 otherwise (some bits are set to indicate
the chunk has a hole). Check out the commit log for patch 4 for
reasoning.

Brian

> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v2 05/17] xfs: create macros/helpers for dealing with sparse inode chunks
  2014-11-03 22:34     ` Brian Foster
@ 2014-11-03 23:56       ` Fanael Linithien
  2014-11-04  1:33         ` Dave Chinner
  2014-11-04 12:14         ` Brian Foster
  0 siblings, 2 replies; 24+ messages in thread
From: Fanael Linithien @ 2014-11-03 23:56 UTC (permalink / raw)
  To: Brian Foster; +Cc: xfs

2014-11-03 23:34 GMT+01:00 Brian Foster <bfoster@redhat.com>:
> On Mon, Nov 03, 2014 at 11:12:12PM +0100, Fanael Linithien wrote:
>> 2014-11-03 17:12 GMT+01:00 Brian Foster <bfoster@redhat.com>:
>> >
>> > +static inline bool xfs_inobt_issparse(uint16_t holemask)
>> > +{
>> > +       return holemask == 0 ? false : true;
>> > +}
>>
>> Surely that should be "return holemask != 0;"?
>>
>
> ir_holemask bits are set for holes in the inode chunk and unset for
> allocated regions. This means that ir_holemask == 0 for a normal,
> fully-allocated chunk and != 0 otherwise (some bits are set to indicate
> the chunk has a hole). Check out the commit log for patch 4 for
> reasoning.

Oh, I don't comment on the logic, as I don't really know much about
XFS code. It's purely a stylistic suggestion: "holemask == 0 ? false :
true" looks entirely equivalent to "holemask != 0".

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v2 05/17] xfs: create macros/helpers for dealing with sparse inode chunks
  2014-11-03 23:56       ` Fanael Linithien
@ 2014-11-04  1:33         ` Dave Chinner
  2014-11-04 12:15           ` Brian Foster
  2014-11-04 12:14         ` Brian Foster
  1 sibling, 1 reply; 24+ messages in thread
From: Dave Chinner @ 2014-11-04  1:33 UTC (permalink / raw)
  To: Fanael Linithien; +Cc: Brian Foster, xfs

On Tue, Nov 04, 2014 at 12:56:17AM +0100, Fanael Linithien wrote:
> 2014-11-03 23:34 GMT+01:00 Brian Foster <bfoster@redhat.com>:
> > On Mon, Nov 03, 2014 at 11:12:12PM +0100, Fanael Linithien wrote:
> >> 2014-11-03 17:12 GMT+01:00 Brian Foster <bfoster@redhat.com>:
> >> >
> >> > +static inline bool xfs_inobt_issparse(uint16_t holemask)
> >> > +{
> >> > +       return holemask == 0 ? false : true;
> >> > +}
> >>
> >> Surely that should be "return holemask != 0;"?
> >>
> >
> > ir_holemask bits are set for holes in the inode chunk and unset for
> > allocated regions. This means that ir_holemask == 0 for a normal,
> > fully-allocated chunk and != 0 otherwise (some bits are set to indicate
> > the chunk has a hole). Check out the commit log for patch 4 for
> > reasoning.
> 
> Oh, I don't comment on the logic, as I don't really know much about
> XFS code. It's purely a stylistic suggestion: "holemask == 0 ? false :
> true" looks entirely equivalent to "holemask != 0".

Even that is unnecessary. booleans are handled by the
compiler quite nicely - just casting a bool
type will give the same result. i.e. (bool)holemask results in a
value of "false" if holemask is zero, "true" if holemask has any
value other than zero.

static inline bool xfs_inobt_issparse(uint16_t holemask)
{
       return holemask;
}

Will give the desired result as there is an implicit typecast to
bool in that return statement.....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v2 05/17] xfs: create macros/helpers for dealing with sparse inode chunks
  2014-11-03 23:56       ` Fanael Linithien
  2014-11-04  1:33         ` Dave Chinner
@ 2014-11-04 12:14         ` Brian Foster
  1 sibling, 0 replies; 24+ messages in thread
From: Brian Foster @ 2014-11-04 12:14 UTC (permalink / raw)
  To: Fanael Linithien; +Cc: xfs

On Tue, Nov 04, 2014 at 12:56:17AM +0100, Fanael Linithien wrote:
> 2014-11-03 23:34 GMT+01:00 Brian Foster <bfoster@redhat.com>:
> > On Mon, Nov 03, 2014 at 11:12:12PM +0100, Fanael Linithien wrote:
> >> 2014-11-03 17:12 GMT+01:00 Brian Foster <bfoster@redhat.com>:
> >> >
> >> > +static inline bool xfs_inobt_issparse(uint16_t holemask)
> >> > +{
> >> > +       return holemask == 0 ? false : true;
> >> > +}
> >>
> >> Surely that should be "return holemask != 0;"?
> >>
> >
> > ir_holemask bits are set for holes in the inode chunk and unset for
> > allocated regions. This means that ir_holemask == 0 for a normal,
> > fully-allocated chunk and != 0 otherwise (some bits are set to indicate
> > the chunk has a hole). Check out the commit log for patch 4 for
> > reasoning.
> 
> Oh, I don't comment on the logic, as I don't really know much about
> XFS code. It's purely a stylistic suggestion: "holemask == 0 ? false :
> true" looks entirely equivalent to "holemask != 0".

Ah right, sorry. Yes, that makes sense. I'll fix this up in the next
version. Thanks!

Brian

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: [PATCH v2 05/17] xfs: create macros/helpers for dealing with sparse inode chunks
  2014-11-04  1:33         ` Dave Chinner
@ 2014-11-04 12:15           ` Brian Foster
  0 siblings, 0 replies; 24+ messages in thread
From: Brian Foster @ 2014-11-04 12:15 UTC (permalink / raw)
  To: Dave Chinner; +Cc: Fanael Linithien, xfs

On Tue, Nov 04, 2014 at 12:33:08PM +1100, Dave Chinner wrote:
> On Tue, Nov 04, 2014 at 12:56:17AM +0100, Fanael Linithien wrote:
> > 2014-11-03 23:34 GMT+01:00 Brian Foster <bfoster@redhat.com>:
> > > On Mon, Nov 03, 2014 at 11:12:12PM +0100, Fanael Linithien wrote:
> > >> 2014-11-03 17:12 GMT+01:00 Brian Foster <bfoster@redhat.com>:
> > >> >
> > >> > +static inline bool xfs_inobt_issparse(uint16_t holemask)
> > >> > +{
> > >> > +       return holemask == 0 ? false : true;
> > >> > +}
> > >>
> > >> Surely that should be "return holemask != 0;"?
> > >>
> > >
> > > ir_holemask bits are set for holes in the inode chunk and unset for
> > > allocated regions. This means that ir_holemask == 0 for a normal,
> > > fully-allocated chunk and != 0 otherwise (some bits are set to indicate
> > > the chunk has a hole). Check out the commit log for patch 4 for
> > > reasoning.
> > 
> > Oh, I don't comment on the logic, as I don't really know much about
> > XFS code. It's purely a stylistic suggestion: "holemask == 0 ? false :
> > true" looks entirely equivalent to "holemask != 0".
> 
> Even that is unnecessary. booleans are handled by the
> compiler quite nicely - just casting a bool
> type will give the same result. i.e. (bool)holemask results in a
> value of "false" if holemask is zero, "true" if holemask has any
> value other than zero.
> 
> static inline bool xfs_inobt_issparse(uint16_t holemask)
> {
>        return holemask;
> }
> 
> Will give the desired result as there is an implicit typecast to
> bool in that return statement.....
> 

Indeed, thanks.

Brian

> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 24+ messages in thread
end of thread, other threads:[~2014-11-04 12:15 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-03 16:12 [PATCH v2 00/17] xfs: sparse inode chunks Brian Foster
2014-11-03 16:12 ` [PATCH v2 01/17] xfs: add sparse inode chunk alignment superblock field Brian Foster
2014-11-03 16:12 ` [PATCH v2 02/17] xfs: use sparse chunk alignment for min. inode allocation requirement Brian Foster
2014-11-03 16:12 ` [PATCH v2 03/17] xfs: define sparse inode chunks v5 sb feature bit and helper function Brian Foster
2014-11-03 16:12 ` [PATCH v2 04/17] xfs: introduce inode record hole mask for sparse inode chunks Brian Foster
2014-11-03 16:12 ` [PATCH v2 05/17] xfs: create macros/helpers for dealing with " Brian Foster
2014-11-03 22:12   ` Fanael Linithien
2014-11-03 22:34     ` Brian Foster
2014-11-03 23:56       ` Fanael Linithien
2014-11-04  1:33         ` Dave Chinner
2014-11-04 12:15           ` Brian Foster
2014-11-04 12:14         ` Brian Foster
2014-11-03 16:12 ` [PATCH v2 06/17] xfs: pass inode count through ordered icreate log item Brian Foster
2014-11-03 16:12 ` [PATCH v2 07/17] xfs: handle sparse inode chunks in icreate log recovery Brian Foster
2014-11-03 16:12 ` [PATCH v2 08/17] xfs: create helper to manage record overlap for sparse inode chunks Brian Foster
2014-11-03 16:12 ` [PATCH v2 09/17] xfs: allocate sparse inode chunks on full chunk allocation failure Brian Foster
2014-11-03 16:12 ` [PATCH v2 10/17] xfs: helpers to convert holemask to/from generic bitmap Brian Foster
2014-11-03 16:12 ` [PATCH v2 11/17] xfs: filter out sparse regions from individual inode allocation Brian Foster
2014-11-03 16:12 ` [PATCH v2 12/17] xfs: update free inode record logic to support sparse inode records Brian Foster
2014-11-03 16:12 ` [PATCH v2 13/17] xfs: only free allocated regions of inode chunks Brian Foster
2014-11-03 16:12 ` [PATCH v2 14/17] xfs: skip unallocated regions of inode chunks in xfs_ifree_cluster() Brian Foster
2014-11-03 16:12 ` [PATCH v2 15/17] xfs: use actual inode count for sparse records in bulkstat/inumbers Brian Foster
2014-11-03 16:12 ` [PATCH v2 16/17] xfs: add fs geometry bit for sparse inode chunks Brian Foster
2014-11-03 16:12 ` [PATCH v2 17/17] xfs: enable sparse inode chunks for v5 superblocks Brian Foster

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox