[PATCH v2 0/5] Misc overlay ino issues

[PATCH v2 0/5] Misc overlay ino issues

[Amir Goldstein]

Miklos,

This is v2 of the ino patches.
v1 is here [1]. I reabsed to overlayfs-next and addressed
your comments on the ino collision patch.

The branch passes overlay xfstests including the new tests 07[01]
that I wrote to test this series.

Note that i_ino uses the private atomic counter not only for xino
overflow case, but also for non-samefs with xino disabled, but it is
only used for directory inodes. I don't think that should cause any
performance regressions and the kernel gets rid of a potentially
massive abuser of the global get_next_ino() pool.

Thanks,
Amir.

Changes since v1:
- Cleanup patches merged
- Use private atomic counter for non-persistent ino
- Don't abuse i_generation
- Keep fsid an index to fs array (fewer magic shifts)
- Added xino_mode fix patch (for v5.6), which includes disabling xino
  on 32bit kernel

[1] https://lore.kernel.org/linux-unionfs/20200101175814.14144-1-amir73il@gmail.com/

Amir Goldstein (5):
  ovl: fix some xino configurations
  ovl: use a private non-persistent ino pool
  ovl: avoid possible inode number collisions with xino=on
  ovl: enable xino automatically in more cases
  ovl: document xino expected behavior

 Documentation/filesystems/overlayfs.rst | 38 +++++++++++++++-
 fs/overlayfs/Kconfig                    |  1 +
 fs/overlayfs/inode.c                    | 58 ++++++++++++++++++-------
 fs/overlayfs/overlayfs.h                | 16 +++++++
 fs/overlayfs/ovl_entry.h                |  2 +
 fs/overlayfs/readdir.c                  | 25 ++++++++---
 fs/overlayfs/super.c                    | 35 ++++++++-------
 7 files changed, 136 insertions(+), 39 deletions(-)

-- 
2.17.1

[PATCH v2 1/5] ovl: fix some xino configurations

[Amir Goldstein]

Fix up two bugs in the coversion to xino_mode:
1. xino=off does not alway end up in disabled mode
2. xino=auto on 32bit arch should end up in disabled mode

Take a proactive approach to disabling xino on 32bit kernel:
1. Disable XINO_AUTO config during build time
2. Disable xino with a warning on mount time

As a by product, xino=on on 32bit arch also ends up in disabled mode.
We never intended to enable xino on 32bit arch and this will make the
rest of the logic simpler.

Fixes: 0f831ec85eda ("ovl: simplify ovl_same_sb() helper")
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/overlayfs/Kconfig | 1 +
 fs/overlayfs/super.c | 9 ++++++++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/fs/overlayfs/Kconfig b/fs/overlayfs/Kconfig
index 444e2da4f60e..714c14c47ca5 100644
--- a/fs/overlayfs/Kconfig
+++ b/fs/overlayfs/Kconfig
@@ -93,6 +93,7 @@ config OVERLAY_FS_XINO_AUTO
 	bool "Overlayfs: auto enable inode number mapping"
 	default n
 	depends on OVERLAY_FS
+	depends on 64BIT
 	help
 	  If this config option is enabled then overlay filesystems will use
 	  unused high bits in undelying filesystem inode numbers to map all
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 6dc45bc7d664..f4c0ad69f9a6 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -1489,6 +1489,8 @@ static int ovl_get_layers(struct super_block *sb, struct ovl_fs *ofs,
 		if (ofs->config.xino == OVL_XINO_ON)
 			pr_info("\"xino=on\" is useless with all layers on same fs, ignore.\n");
 		ofs->xino_mode = 0;
+	} else if (ofs->config.xino == OVL_XINO_OFF) {
+		ofs->xino_mode = -1;
 	} else if (ofs->config.xino == OVL_XINO_ON && ofs->xino_mode < 0) {
 		/*
 		 * This is a roundup of number of bits needed for encoding
@@ -1735,8 +1737,13 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 	sb->s_stack_depth = 0;
 	sb->s_maxbytes = MAX_LFS_FILESIZE;
 	/* Assume underlaying fs uses 32bit inodes unless proven otherwise */
-	if (ofs->config.xino != OVL_XINO_OFF)
+	if (ofs->config.xino != OVL_XINO_OFF) {
 		ofs->xino_mode = BITS_PER_LONG - 32;
+		if (!ofs->config.xino) {
+			pr_warn("xino not supported on 32bit kernel, falling back to xino=off.\n");
+			ofs->config.xino = OVL_XINO_OFF;
+		}
+	}
 
 	/* alloc/destroy_inode needed for setting up traps in inode cache */
 	sb->s_op = &ovl_super_operations;
-- 
2.17.1

[PATCH v2 2/5] ovl: use a private non-persistent ino pool

[Amir Goldstein]

There is no reason to deplete the system's global get_next_ino() pool
for overlay non-persistent inode numbers and there is no reason at all
to allocate non-persistent inode numbers for non-directories.

For non-directories, it is much better to leave i_ino the same as
real i_ino, to be consistent with st_ino/d_ino.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/overlayfs/inode.c     | 15 ++++++++++++---
 fs/overlayfs/ovl_entry.h |  2 ++
 fs/overlayfs/super.c     |  1 +
 3 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 13219a5864c4..1d555cb1a5cd 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -561,6 +561,15 @@ static inline void ovl_lockdep_annotate_inode_mutex_key(struct inode *inode)
 #endif
 }
 
+static void ovl_next_ino(struct inode *inode)
+{
+	struct ovl_fs *ofs = inode->i_sb->s_fs_info;
+
+	inode->i_ino = atomic_long_inc_return(&ofs->last_ino);
+	if (unlikely(!inode->i_ino))
+		inode->i_ino = atomic_long_inc_return(&ofs->last_ino);
+}
+
 static void ovl_map_ino(struct inode *inode, unsigned long ino, int fsid)
 {
 	int xinobits = ovl_xino_bits(inode->i_sb);
@@ -572,12 +581,12 @@ static void ovl_map_ino(struct inode *inode, unsigned long ino, int fsid)
 	 * consistent with d_ino and st_ino values. An i_ino value inconsistent
 	 * with d_ino also causes nfsd readdirplus to fail.
 	 */
+	inode->i_ino = ino;
 	if (ovl_same_dev(inode->i_sb)) {
-		inode->i_ino = ino;
 		if (xinobits && fsid && !(ino >> (64 - xinobits)))
 			inode->i_ino |= (unsigned long)fsid << (64 - xinobits);
-	} else {
-		inode->i_ino = get_next_ino();
+	} else if (S_ISDIR(inode->i_mode)) {
+		ovl_next_ino(inode);
 	}
 }
 
diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
index 89015ea822e7..5762d802fe01 100644
--- a/fs/overlayfs/ovl_entry.h
+++ b/fs/overlayfs/ovl_entry.h
@@ -75,6 +75,8 @@ struct ovl_fs {
 	struct inode *indexdir_trap;
 	/* -1: disabled, 0: same fs, 1..32: number of unused ino bits */
 	int xino_mode;
+	/* For allocation of non-persistent inode numbers */
+	atomic_long_t last_ino;
 };
 
 static inline struct ovl_fs *OVL_FS(struct super_block *sb)
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index f4c0ad69f9a6..18b710344dd2 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -1736,6 +1736,7 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
 
 	sb->s_stack_depth = 0;
 	sb->s_maxbytes = MAX_LFS_FILESIZE;
+	atomic_long_set(&ofs->last_ino, 1);
 	/* Assume underlaying fs uses 32bit inodes unless proven otherwise */
 	if (ofs->config.xino != OVL_XINO_OFF) {
 		ofs->xino_mode = BITS_PER_LONG - 32;
-- 
2.17.1

[PATCH v2 3/5] ovl: avoid possible inode number collisions with xino=on

[Amir Goldstein]

When xino feature is enabled and a real directory inode number
overflows the lower xino bits, we cannot map this directory inode
number to a unique and persistent inode number and we fall back to
the real inode st_ino and overlay st_dev.

The real inode st_ino with high bits may collide with a lower inode
number on overlay st_dev that was mapped using xino.

To avoid possible collision with legitimate xino values, map a non
persistent inode number to a dedicated range in the xino address space.
The dedicated range is created by adding one more bit to the number of
reserved high xino bits.  We could have added just one more fsid, but
that would have had the undesired effect of changing persistent overlay
inode numbers on kernel or require more complex xino mapping code.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/overlayfs/inode.c   | 39 +++++++++++++++++++++++++++++----------
 fs/overlayfs/readdir.c | 10 ++++++++--
 fs/overlayfs/super.c   | 13 ++++++++-----
 3 files changed, 45 insertions(+), 17 deletions(-)

diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 1d555cb1a5cd..d19e4cba4f61 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -79,6 +79,7 @@ static int ovl_map_dev_ino(struct dentry *dentry, struct kstat *stat, int fsid)
 {
 	bool samefs = ovl_same_fs(dentry->d_sb);
 	unsigned int xinobits = ovl_xino_bits(dentry->d_sb);
+	unsigned int xinoshift = 64 - xinobits;
 
 	if (samefs) {
 		/*
@@ -89,20 +90,20 @@ static int ovl_map_dev_ino(struct dentry *dentry, struct kstat *stat, int fsid)
 		stat->dev = dentry->d_sb->s_dev;
 		return 0;
 	} else if (xinobits) {
-		unsigned int shift = 64 - xinobits;
 		/*
 		 * All inode numbers of underlying fs should not be using the
 		 * high xinobits, so we use high xinobits to partition the
 		 * overlay st_ino address space. The high bits holds the fsid
-		 * (upper fsid is 0). This way overlay inode numbers are unique
-		 * and all inodes use overlay st_dev. Inode numbers are also
-		 * persistent for a given layer configuration.
+		 * (upper fsid is 0). The lowest xinobit is reserved for mapping
+		 * the non-peresistent inode numbers range in case of overflow.
+		 * This way all overlay inode numbers are unique and use the
+		 * overlay st_dev.
 		 */
-		if (stat->ino >> shift) {
+		if (unlikely(stat->ino >> xinoshift)) {
 			pr_warn_ratelimited("inode number too big (%pd2, ino=%llu, xinobits=%d)\n",
 					    dentry, stat->ino, xinobits);
 		} else {
-			stat->ino |= ((u64)fsid) << shift;
+			stat->ino |= ((u64)fsid) << (xinoshift + 1);
 			stat->dev = dentry->d_sb->s_dev;
 			return 0;
 		}
@@ -573,6 +574,7 @@ static void ovl_next_ino(struct inode *inode)
 static void ovl_map_ino(struct inode *inode, unsigned long ino, int fsid)
 {
 	int xinobits = ovl_xino_bits(inode->i_sb);
+	unsigned int xinoshift = 64 - xinobits;
 
 	/*
 	 * When d_ino is consistent with st_ino (samefs or i_ino has enough
@@ -582,11 +584,28 @@ static void ovl_map_ino(struct inode *inode, unsigned long ino, int fsid)
 	 * with d_ino also causes nfsd readdirplus to fail.
 	 */
 	inode->i_ino = ino;
-	if (ovl_same_dev(inode->i_sb)) {
-		if (xinobits && fsid && !(ino >> (64 - xinobits)))
-			inode->i_ino |= (unsigned long)fsid << (64 - xinobits);
-	} else if (S_ISDIR(inode->i_mode)) {
+	if (ovl_same_fs(inode->i_sb)) {
+		return;
+	} else if (xinobits && likely(!(ino >> xinoshift))) {
+		inode->i_ino |= (unsigned long)fsid << (xinoshift + 1);
+		return;
+	}
+
+	/*
+	 * For directory inodes on non-samefs with xino disabled or xino
+	 * overflow, we allocate a non-persistent inode number, to be used for
+	 * resolving st_ino collisions in ovl_map_dev_ino().
+	 *
+	 * To avoid ino collision with legitimate xino values from upper
+	 * layer (fsid 0), use the lowest xinobit to map the non
+	 * persistent inode numbers to the unified st_ino address space.
+	 */
+	if (S_ISDIR(inode->i_mode)) {
 		ovl_next_ino(inode);
+		if (xinobits) {
+			inode->i_ino &= ~0UL >> xinobits;
+			inode->i_ino |= 1UL << xinoshift;
+		}
 	}
 }
 
diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
index 40ac9ce2465a..6325dcc4c48b 100644
--- a/fs/overlayfs/readdir.c
+++ b/fs/overlayfs/readdir.c
@@ -440,13 +440,19 @@ static struct ovl_dir_cache *ovl_cache_get(struct dentry *dentry)
 static u64 ovl_remap_lower_ino(u64 ino, int xinobits, int fsid,
 			       const char *name, int namelen)
 {
-	if (ino >> (64 - xinobits)) {
+	unsigned int xinoshift = 64 - xinobits;
+
+	if (unlikely(ino >> xinoshift)) {
 		pr_warn_ratelimited("d_ino too big (%.*s, ino=%llu, xinobits=%d)\n",
 				    namelen, name, ino, xinobits);
 		return ino;
 	}
 
-	return ino | ((u64)fsid) << (64 - xinobits);
+	/*
+	 * The lowest xinobit is reserved for mapping the non-peresistent inode
+	 * numbers range, but this range is only exposed via st_ino, not here.
+	 */
+	return ino | ((u64)fsid) << (xinoshift + 1);
 }
 
 /*
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 18b710344dd2..67cd9e59d467 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -1483,7 +1483,8 @@ static int ovl_get_layers(struct super_block *sb, struct ovl_fs *ofs,
 	 * free high bits in underlying fs to hold the unique fsid.
 	 * If overlayfs does encounter underlying inodes using the high xino
 	 * bits reserved for fsid, it emits a warning and uses the original
-	 * inode number.
+	 * inode number or a non persistent inode number allocated from a
+	 * dedicated range.
 	 */
 	if (ofs->numfs - !ofs->upper_mnt == 1) {
 		if (ofs->config.xino == OVL_XINO_ON)
@@ -1494,11 +1495,13 @@ static int ovl_get_layers(struct super_block *sb, struct ovl_fs *ofs,
 	} else if (ofs->config.xino == OVL_XINO_ON && ofs->xino_mode < 0) {
 		/*
 		 * This is a roundup of number of bits needed for encoding
-		 * fsid, where fsid 0 is reserved for upper fs even with
-		 * lower only overlay.
+		 * fsid, where fsid 0 is reserved for upper fs (even with
+		 * lower only overlay) +1 extra bit is reserved for the non
+		 * persistent inode number range that is used for resolving
+		 * xino lower bits overflow.
 		 */
-		BUILD_BUG_ON(ilog2(OVL_MAX_STACK) > 31);
-		ofs->xino_mode = ilog2(ofs->numfs - 1) + 1;
+		BUILD_BUG_ON(ilog2(OVL_MAX_STACK) > 30);
+		ofs->xino_mode = ilog2(ofs->numfs - 1) + 2;
 	}
 
 	if (ofs->xino_mode > 0) {
-- 
2.17.1

[PATCH v2 4/5] ovl: enable xino automatically in more cases

[Amir Goldstein]

So far, with xino=auto, we only enable xino if we know that all
underlying filesystem use 32bit inode numbers.

When users configure overlay with xino=auto, they already declare that
they are ready to handle 64bit inode number from overlay.

It is a very common case, that underlying filesystem uses 64bit ino,
but rarely or never uses the high inode number bits (e.g. tmpfs, xfs).
Leaving it for the users to declare high ino bits are unused with
xino=on is not a recipe for many users to enjoy the benefits of xino.

There appears to be very little reason not to enable xino when users
declare xino=auto even if we do not know how many bits underlying
filesystem uses for inode numbers.

In the worst case of xino bits overflow by real inode number, we
already fall back to the non-xino behavior - real inode number with
unique pseudo dev or to non persistent inode number and overlay st_dev
(for directories).

The only annoyance from auto enabling xino is that xino bits overflow
emits a warning to kmsg. Suppress those warnings unless users explicitly
asked for xino=on, suggesting that they expected high ino bits to be
unused by underlying filesystem.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/overlayfs/inode.c     |  8 ++++----
 fs/overlayfs/overlayfs.h | 16 ++++++++++++++++
 fs/overlayfs/readdir.c   | 15 ++++++++++-----
 fs/overlayfs/super.c     | 12 +++---------
 4 files changed, 33 insertions(+), 18 deletions(-)

diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index d19e4cba4f61..ff917d376bdd 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -99,13 +99,13 @@ static int ovl_map_dev_ino(struct dentry *dentry, struct kstat *stat, int fsid)
 		 * This way all overlay inode numbers are unique and use the
 		 * overlay st_dev.
 		 */
-		if (unlikely(stat->ino >> xinoshift)) {
-			pr_warn_ratelimited("inode number too big (%pd2, ino=%llu, xinobits=%d)\n",
-					    dentry, stat->ino, xinobits);
-		} else {
+		if (likely(!(stat->ino >> xinoshift))) {
 			stat->ino |= ((u64)fsid) << (xinoshift + 1);
 			stat->dev = dentry->d_sb->s_dev;
 			return 0;
+		} else if (ovl_xino_warn(dentry->d_sb)) {
+			pr_warn_ratelimited("inode number too big (%pd2, ino=%llu, xinobits=%d)\n",
+					    dentry, stat->ino, xinobits);
 		}
 	}
 
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index 68df20512dca..3ccf1725e3d2 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -48,6 +48,12 @@ enum ovl_entry_flag {
 	OVL_E_CONNECTED,
 };
 
+enum {
+	OVL_XINO_OFF,
+	OVL_XINO_AUTO,
+	OVL_XINO_ON,
+};
+
 /*
  * The tuple (fh,uuid) is a universal unique identifier for a copy up origin,
  * where:
@@ -301,6 +307,16 @@ static inline bool ovl_is_impuredir(struct dentry *dentry)
 	return ovl_check_dir_xattr(dentry, OVL_XATTR_IMPURE);
 }
 
+/*
+ * With xino=auto, we do best effort to keep all inodes on same st_dev and
+ * d_ino consistent with st_ino.
+ * With xino=on, we do the same effort but we warn if we failed.
+ */
+static inline bool ovl_xino_warn(struct super_block *sb)
+{
+	return OVL_FS(sb)->config.xino == OVL_XINO_ON;
+}
+
 /* All layers on same fs? */
 static inline bool ovl_same_fs(struct super_block *sb)
 {
diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
index 6325dcc4c48b..e452ff7d583d 100644
--- a/fs/overlayfs/readdir.c
+++ b/fs/overlayfs/readdir.c
@@ -438,13 +438,15 @@ static struct ovl_dir_cache *ovl_cache_get(struct dentry *dentry)
 
 /* Map inode number to lower fs unique range */
 static u64 ovl_remap_lower_ino(u64 ino, int xinobits, int fsid,
-			       const char *name, int namelen)
+			       const char *name, int namelen, bool warn)
 {
 	unsigned int xinoshift = 64 - xinobits;
 
 	if (unlikely(ino >> xinoshift)) {
-		pr_warn_ratelimited("d_ino too big (%.*s, ino=%llu, xinobits=%d)\n",
-				    namelen, name, ino, xinobits);
+		if (warn) {
+			pr_warn_ratelimited("d_ino too big (%.*s, ino=%llu, xinobits=%d)\n",
+					    namelen, name, ino, xinobits);
+		}
 		return ino;
 	}
 
@@ -521,7 +523,8 @@ static int ovl_cache_update_ino(struct path *path, struct ovl_cache_entry *p)
 	} else if (xinobits && !OVL_TYPE_UPPER(type)) {
 		ino = ovl_remap_lower_ino(ino, xinobits,
 					  ovl_layer_lower(this)->fsid,
-					  p->name, p->len);
+					  p->name, p->len,
+					  ovl_xino_warn(dir->d_sb));
 	}
 
 out:
@@ -651,6 +654,7 @@ struct ovl_readdir_translate {
 	u64 parent_ino;
 	int fsid;
 	int xinobits;
+	bool xinowarn;
 };
 
 static int ovl_fill_real(struct dir_context *ctx, const char *name,
@@ -671,7 +675,7 @@ static int ovl_fill_real(struct dir_context *ctx, const char *name,
 			ino = p->ino;
 	} else if (rdt->xinobits) {
 		ino = ovl_remap_lower_ino(ino, rdt->xinobits, rdt->fsid,
-					  name, namelen);
+					  name, namelen, rdt->xinowarn);
 	}
 
 	return orig_ctx->actor(orig_ctx, name, namelen, offset, ino, d_type);
@@ -702,6 +706,7 @@ static int ovl_iterate_real(struct file *file, struct dir_context *ctx)
 		.ctx.actor = ovl_fill_real,
 		.orig_ctx = ctx,
 		.xinobits = ovl_xino_bits(dir->d_sb),
+		.xinowarn = ovl_xino_warn(dir->d_sb),
 	};
 
 	if (rdt.xinobits && lower_layer)
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 67cd9e59d467..01938c93e5c8 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -317,12 +317,6 @@ static const char *ovl_redirect_mode_def(void)
 	return ovl_redirect_dir_def ? "on" : "off";
 }
 
-enum {
-	OVL_XINO_OFF,
-	OVL_XINO_AUTO,
-	OVL_XINO_ON,
-};
-
 static const char * const ovl_xino_str[] = {
 	"off",
 	"auto",
@@ -1479,8 +1473,8 @@ static int ovl_get_layers(struct super_block *sb, struct ovl_fs *ofs,
 
 	/*
 	 * When all layers on same fs, overlay can use real inode numbers.
-	 * With mount option "xino=on", mounter declares that there are enough
-	 * free high bits in underlying fs to hold the unique fsid.
+	 * With mount option "xino=<on|auto>", mounter declares that there are
+	 * enough free high bits in underlying fs to hold the unique fsid.
 	 * If overlayfs does encounter underlying inodes using the high xino
 	 * bits reserved for fsid, it emits a warning and uses the original
 	 * inode number or a non persistent inode number allocated from a
@@ -1492,7 +1486,7 @@ static int ovl_get_layers(struct super_block *sb, struct ovl_fs *ofs,
 		ofs->xino_mode = 0;
 	} else if (ofs->config.xino == OVL_XINO_OFF) {
 		ofs->xino_mode = -1;
-	} else if (ofs->config.xino == OVL_XINO_ON && ofs->xino_mode < 0) {
+	} else if (ofs->xino_mode < 0) {
 		/*
 		 * This is a roundup of number of bits needed for encoding
 		 * fsid, where fsid 0 is reserved for upper fs (even with
-- 
2.17.1

[PATCH v2 5/5] ovl: document xino expected behavior

[Amir Goldstein]

Summarize the inode properties of different configurations in a table.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 Documentation/filesystems/overlayfs.rst | 38 +++++++++++++++++++++++--
 1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/Documentation/filesystems/overlayfs.rst b/Documentation/filesystems/overlayfs.rst
index e398fdf7353e..c9d2bf96b02d 100644
--- a/Documentation/filesystems/overlayfs.rst
+++ b/Documentation/filesystems/overlayfs.rst
@@ -40,13 +40,46 @@ On 64bit systems, even if all overlay layers are not on the same
 underlying filesystem, the same compliant behavior could be achieved
 with the "xino" feature.  The "xino" feature composes a unique object
 identifier from the real object st_ino and an underlying fsid index.
+
 If all underlying filesystems support NFS file handles and export file
 handles with 32bit inode number encoding (e.g. ext4), overlay filesystem
 will use the high inode number bits for fsid.  Even when the underlying
 filesystem uses 64bit inode numbers, users can still enable the "xino"
 feature with the "-o xino=on" overlay mount option.  That is useful for the
 case of underlying filesystems like xfs and tmpfs, which use 64bit inode
-numbers, but are very unlikely to use the high inode number bit.
+numbers, but are very unlikely to use the high inode number bits.  In case
+the underlying inode number does overflow into the high xino bits, overlay
+filesystem will fall back to the non xino behavior for that inode.
+
+The following table summarizes what can be expected in different overlay
+configurations.
+
+Inode properties
+````````````````
+
++--------------+------------+------------+-----------------+----------------+
+|Configuration | Persistent | Uniform    | st_ino == d_ino | d_ino == i_ino |
+|              | st_ino     | st_dev     |                 | [*]            |
++==============+=====+======+=====+======+========+========+========+=======+
+|              | dir | !dir | dir | !dir |  dir   +  !dir  |  dir   | !dir  |
++--------------+-----+------+-----+------+--------+--------+--------+-------+
+| All layers   |  Y  |  Y   |  Y  |  Y   |  Y     |   Y    |  Y     |  Y    |
+| on same fs   |     |      |     |      |        |        |        |       |
++--------------+-----+------+-----+------+--------+--------+--------+-------+
+| Layers not   |  N  |  Y   |  Y  |  N   |  N     |   Y    |  N     |  Y    |
+| on same fs,  |     |      |     |      |        |        |        |       |
+| xino=off     |     |      |     |      |        |        |        |       |
++--------------+-----+------+-----+------+--------+--------+--------+-------+
+| xino=on/auto |  Y  |  Y   |  Y  |  Y   |  Y     |   Y    |  Y     |  Y    |
+|              |     |      |     |      |        |        |        |       |
++--------------+-----+------+-----+------+--------+--------+--------+-------+
+| xino=on/auto,|  N  |  Y   |  Y  |  N   |  N     |   Y    |  N     |  Y    |
+| ino overflow |     |      |     |      |        |        |        |       |
++--------------+-----+------+-----+------+--------+--------+--------+-------+
+
+[*] nfsd v3 readdirplus verifies d_ino == i_ino. i_ino is exposed via several
+/proc files, such as /proc/locks and /proc/self/fdinfo/<fd> of an inotify
+file descriptor.
 
 
 Upper and Lower
@@ -427,7 +460,8 @@ guarantee that the values of st_ino and st_dev returned by stat(2) and the
 value of d_ino returned by readdir(3) will act like on a normal filesystem.
 E.g. the value of st_dev may be different for two objects in the same
 overlay filesystem and the value of st_ino for directory objects may not be
-persistent and could change even while the overlay filesystem is mounted.
+persistent and could change even while the overlay filesystem is mounted, as
+summarized in the `Inode properties`_ table above.
 
 
 Changes to underlying filesystems
-- 
2.17.1

Re: [PATCH v2 1/5] ovl: fix some xino configurations

[Miklos Szeredi]

On Fri, Feb 21, 2020 at 3:34 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> Fix up two bugs in the coversion to xino_mode:
> 1. xino=off does not alway end up in disabled mode

s/alway/always/

> 2. xino=auto on 32bit arch should end up in disabled mode
>
> Take a proactive approach to disabling xino on 32bit kernel:
> 1. Disable XINO_AUTO config during build time
> 2. Disable xino with a warning on mount time
>
> As a by product, xino=on on 32bit arch also ends up in disabled mode.
> We never intended to enable xino on 32bit arch and this will make the
> rest of the logic simpler.
>
> Fixes: 0f831ec85eda ("ovl: simplify ovl_same_sb() helper")
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  fs/overlayfs/Kconfig | 1 +
>  fs/overlayfs/super.c | 9 ++++++++-
>  2 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/fs/overlayfs/Kconfig b/fs/overlayfs/Kconfig
> index 444e2da4f60e..714c14c47ca5 100644
> --- a/fs/overlayfs/Kconfig
> +++ b/fs/overlayfs/Kconfig
> @@ -93,6 +93,7 @@ config OVERLAY_FS_XINO_AUTO
>         bool "Overlayfs: auto enable inode number mapping"
>         default n
>         depends on OVERLAY_FS
> +       depends on 64BIT
>         help
>           If this config option is enabled then overlay filesystems will use
>           unused high bits in undelying filesystem inode numbers to map all
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index 6dc45bc7d664..f4c0ad69f9a6 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -1489,6 +1489,8 @@ static int ovl_get_layers(struct super_block *sb, struct ovl_fs *ofs,
>                 if (ofs->config.xino == OVL_XINO_ON)
>                         pr_info("\"xino=on\" is useless with all layers on same fs, ignore.\n");
>                 ofs->xino_mode = 0;
> +       } else if (ofs->config.xino == OVL_XINO_OFF) {
> +               ofs->xino_mode = -1;
>         } else if (ofs->config.xino == OVL_XINO_ON && ofs->xino_mode < 0) {
>                 /*
>                  * This is a roundup of number of bits needed for encoding
> @@ -1735,8 +1737,13 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
>         sb->s_stack_depth = 0;
>         sb->s_maxbytes = MAX_LFS_FILESIZE;
>         /* Assume underlaying fs uses 32bit inodes unless proven otherwise */
> -       if (ofs->config.xino != OVL_XINO_OFF)
> +       if (ofs->config.xino != OVL_XINO_OFF) {
>                 ofs->xino_mode = BITS_PER_LONG - 32;
> +               if (!ofs->config.xino) {

Did you mean (!ofs->xino_mode)?


Thanks,
Miklos

Re: [PATCH v2 1/5] ovl: fix some xino configurations

[Amir Goldstein]

On Mon, Feb 24, 2020 at 1:41 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Fri, Feb 21, 2020 at 3:34 PM Amir Goldstein <amir73il@gmail.com> wrote:
> >
> > Fix up two bugs in the coversion to xino_mode:
> > 1. xino=off does not alway end up in disabled mode
>
> s/alway/always/
>
> > 2. xino=auto on 32bit arch should end up in disabled mode
> >
> > Take a proactive approach to disabling xino on 32bit kernel:
> > 1. Disable XINO_AUTO config during build time
> > 2. Disable xino with a warning on mount time
> >
> > As a by product, xino=on on 32bit arch also ends up in disabled mode.
> > We never intended to enable xino on 32bit arch and this will make the
> > rest of the logic simpler.
> >
> > Fixes: 0f831ec85eda ("ovl: simplify ovl_same_sb() helper")
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > ---
> >  fs/overlayfs/Kconfig | 1 +
> >  fs/overlayfs/super.c | 9 ++++++++-
> >  2 files changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/overlayfs/Kconfig b/fs/overlayfs/Kconfig
> > index 444e2da4f60e..714c14c47ca5 100644
> > --- a/fs/overlayfs/Kconfig
> > +++ b/fs/overlayfs/Kconfig
> > @@ -93,6 +93,7 @@ config OVERLAY_FS_XINO_AUTO
> >         bool "Overlayfs: auto enable inode number mapping"
> >         default n
> >         depends on OVERLAY_FS
> > +       depends on 64BIT
> >         help
> >           If this config option is enabled then overlay filesystems will use
> >           unused high bits in undelying filesystem inode numbers to map all
> > diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> > index 6dc45bc7d664..f4c0ad69f9a6 100644
> > --- a/fs/overlayfs/super.c
> > +++ b/fs/overlayfs/super.c
> > @@ -1489,6 +1489,8 @@ static int ovl_get_layers(struct super_block *sb, struct ovl_fs *ofs,
> >                 if (ofs->config.xino == OVL_XINO_ON)
> >                         pr_info("\"xino=on\" is useless with all layers on same fs, ignore.\n");
> >                 ofs->xino_mode = 0;
> > +       } else if (ofs->config.xino == OVL_XINO_OFF) {
> > +               ofs->xino_mode = -1;
> >         } else if (ofs->config.xino == OVL_XINO_ON && ofs->xino_mode < 0) {
> >                 /*
> >                  * This is a roundup of number of bits needed for encoding
> > @@ -1735,8 +1737,13 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent)
> >         sb->s_stack_depth = 0;
> >         sb->s_maxbytes = MAX_LFS_FILESIZE;
> >         /* Assume underlaying fs uses 32bit inodes unless proven otherwise */
> > -       if (ofs->config.xino != OVL_XINO_OFF)
> > +       if (ofs->config.xino != OVL_XINO_OFF) {
> >                 ofs->xino_mode = BITS_PER_LONG - 32;
> > +               if (!ofs->config.xino) {
>
> Did you mean (!ofs->xino_mode)?

Yes, I certainly did :)
And no, I did not test on a 32bit kernel...

Thanks,
Amir.

Re: [PATCH v2 1/5] ovl: fix some xino configurations

[Amir Goldstein]

On Mon, Feb 24, 2020 at 3:27 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Mon, Feb 24, 2020 at 1:41 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
> >
> > On Fri, Feb 21, 2020 at 3:34 PM Amir Goldstein <amir73il@gmail.com> wrote:
> > >
> > > Fix up two bugs in the coversion to xino_mode:
> > > 1. xino=off does not alway end up in disabled mode
> >
> > s/alway/always/

Just noticed I did not fix this typo, so pushed now to ovl-fixes,
along with the llseek lock fix:

5e0801c4406b ovl: fix lock in ovl_llseek()
9a69ba7f8d14 ovl: fix some xino configurations

I suppose you are getting to that.

Thanks,
Amir.

Re: [PATCH v2 0/5] Misc overlay ino issues

[Miklos Szeredi]

On Fri, Feb 21, 2020 at 3:34 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> Miklos,
>
> This is v2 of the ino patches.
> v1 is here [1]. I reabsed to overlayfs-next and addressed
> your comments on the ino collision patch.
>
> The branch passes overlay xfstests including the new tests 07[01]
> that I wrote to test this series.
>
> Note that i_ino uses the private atomic counter not only for xino
> overflow case, but also for non-samefs with xino disabled, but it is
> only used for directory inodes. I don't think that should cause any
> performance regressions and the kernel gets rid of a potentially
> massive abuser of the global get_next_ino() pool.

Pushed these to #overlayfs-next

I'm running my tests, but the more the merrier.

Thanks,
Miklos

Re: [PATCH v2 0/5] Misc overlay ino issues

[Amir Goldstein]

On Fri, Mar 27, 2020 at 6:59 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Fri, Feb 21, 2020 at 3:34 PM Amir Goldstein <amir73il@gmail.com> wrote:
> >
> > Miklos,
> >
> > This is v2 of the ino patches.
> > v1 is here [1]. I reabsed to overlayfs-next and addressed
> > your comments on the ino collision patch.
> >
> > The branch passes overlay xfstests including the new tests 07[01]
> > that I wrote to test this series.
> >
> > Note that i_ino uses the private atomic counter not only for xino
> > overflow case, but also for non-samefs with xino disabled, but it is
> > only used for directory inodes. I don't think that should cause any
> > performance regressions and the kernel gets rid of a potentially
> > massive abuser of the global get_next_ino() pool.
>
> Pushed these to #overlayfs-next
>
> I'm running my tests, but the more the merrier.
>

Looks good on my end, including new overlay/072.

Will post it.

Thanks,
Amir.