linux-unionfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC][PATCH 0/5] vfs, overlayfs, cachefiles: Combine I_OVL_INUSE and S_KERNEL_FILE and split out no-remove
@ 2022-01-31 15:12 David Howells
  2022-01-31 15:12 ` [PATCH 1/5] vfs, overlayfs, cachefiles: Turn I_OVL_INUSE into something generic David Howells
                   ` (6 more replies)
  0 siblings, 7 replies; 12+ messages in thread
From: David Howells @ 2022-01-31 15:12 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: linux-unionfs, linux-cachefs, Miklos Szeredi, dhowells,
	Christoph Hellwig, Miklos Szeredi, Jeff Layton, Alexander Viro,
	torvalds, linux-unionfs, linux-cachefs, linux-fsdevel,
	linux-kernel


Hi Amir,

How about this as a set of patches to do what you suggest[1] and hoist the
handler functions for I_OVL_INUSE into common code and rename the flag to
I_EXCL_INUSE.  This can then be shared with cachefiles - allowing me to get
rid of S_KERNEL_FILE.

I did split out the functionality for preventing file/dir removal to a
separate flag, I_NO_REMOVE, so that it's not tied to I_EXCL_INUSE in case
overlayfs doesn't want to use it.  The downside to that, though is that it
requires a separate locking of i_lock to set/clear it.

I also added four general tracepoints to log successful lock/unlock,
failure to lock and a bad unlock.  The lock tracepoints log which driver
asked for the lock and all tracepoints allow the driver to log an arbitrary
reference number (in cachefiles's case this is the object debug ID).

Questions:

 (1) Should it be using a flag in i_state or a flag in i_flags?  I'm not
     sure what the difference is really.

 (2) Do we really need to take i_lock when testing I_EXCL_INUSE?  Would
     READ_ONCE() suffice?


The patches are on a branch here:

	https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=fscache-fixes

David

Link: https://lore.kernel.org/r/CAOQ4uxhRS3MGEnCUDcsB1RL0d1Oy0g0Rzm75hVFAJw2dJ7uKSA@mail.gmail.com/ [1]

---
David Howells (5):
      vfs, overlayfs, cachefiles: Turn I_OVL_INUSE into something generic
      vfs: Add tracepoints for inode_excl_inuse_trylock/unlock
      cachefiles: Split removal-prevention from S_KERNEL_FILE and extend effects
      cachefiles: Use I_EXCL_INUSE instead of S_KERNEL_FILE
      cachefiles: Remove the now-unused mark-inode-in-use tracepoints


 fs/cachefiles/namei.c             | 54 +++++++++++++-------------
 fs/inode.c                        | 56 +++++++++++++++++++++++++++
 fs/namei.c                        |  8 ++--
 fs/overlayfs/overlayfs.h          |  3 --
 fs/overlayfs/super.c              | 14 ++++---
 fs/overlayfs/util.c               | 43 ---------------------
 include/linux/fs.h                | 33 ++++++++++++++--
 include/trace/events/cachefiles.h | 63 -------------------------------
 8 files changed, 126 insertions(+), 148 deletions(-)



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

* [PATCH 1/5] vfs, overlayfs, cachefiles: Turn I_OVL_INUSE into something generic
  2022-01-31 15:12 [RFC][PATCH 0/5] vfs, overlayfs, cachefiles: Combine I_OVL_INUSE and S_KERNEL_FILE and split out no-remove David Howells
@ 2022-01-31 15:12 ` David Howells
  2022-01-31 15:29   ` Amir Goldstein
  2022-01-31 15:32   ` David Howells
  2022-01-31 15:13 ` [PATCH 2/5] vfs: Add tracepoints for inode_excl_inuse_trylock/unlock David Howells
                   ` (5 subsequent siblings)
  6 siblings, 2 replies; 12+ messages in thread
From: David Howells @ 2022-01-31 15:12 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: Miklos Szeredi, linux-unionfs, linux-cachefs, dhowells,
	Christoph Hellwig, Miklos Szeredi, Jeff Layton, Alexander Viro,
	torvalds, linux-unionfs, linux-cachefs, linux-fsdevel,
	linux-kernel

Turn overlayfs's I_OVL_INUSE into something generic that cachefiles can
also use for excluding access to its own cache files by renaming it to
I_EXCL_INUSE as suggested by Amir[1] and hoisting the helpers to generic
code.

Suggested-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Amir Goldstein <amir73il@gmail.com>
cc: Miklos Szeredi <miklos@szeredi.hu>
cc: linux-unionfs@vger.kernel.org
cc: linux-cachefs@redhat.com
Link: https://lore.kernel.org/r/CAOQ4uxhRS3MGEnCUDcsB1RL0d1Oy0g0Rzm75hVFAJw2dJ7uKSA@mail.gmail.com/ [1]
---

 fs/inode.c               |   43 +++++++++++++++++++++++++++++++++++++++++++
 fs/overlayfs/overlayfs.h |    3 ---
 fs/overlayfs/super.c     |   12 ++++++------
 fs/overlayfs/util.c      |   43 -------------------------------------------
 include/linux/fs.h       |   22 +++++++++++++++++++---
 5 files changed, 68 insertions(+), 55 deletions(-)

diff --git a/fs/inode.c b/fs/inode.c
index 63324df6fa27..954719f66113 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -2405,3 +2405,46 @@ struct timespec64 current_time(struct inode *inode)
 	return timestamp_truncate(now, inode);
 }
 EXPORT_SYMBOL(current_time);
+
+/**
+ * inode_excl_inuse_trylock - Try to exclusively lock an inode for kernel access
+ * @dentry: Reference to the inode to be locked
+ *
+ * Try to gain exclusive access to an inode for a kernel service, returning
+ * true if successful.
+ */
+bool inode_excl_inuse_trylock(struct dentry *dentry)
+{
+	struct inode *inode = d_inode(dentry);
+	bool locked = false;
+
+	spin_lock(&inode->i_lock);
+	if (!(inode->i_state & I_EXCL_INUSE)) {
+		inode->i_state |= I_EXCL_INUSE;
+		locked = true;
+	}
+	spin_unlock(&inode->i_lock);
+
+	return locked;
+}
+EXPORT_SYMBOL(inode_excl_inuse_trylock);
+
+/**
+ * inode_excl_inuse_unlock - Unlock exclusive kernel access to an inode
+ * @dentry: Reference to the inode to be unlocked
+ *
+ * Drop exclusive access to an inode for a kernel service.  A warning is given
+ * if the inode was not marked for exclusive access.
+ */
+void inode_excl_inuse_unlock(struct dentry *dentry)
+{
+	if (dentry) {
+		struct inode *inode = d_inode(dentry);
+
+		spin_lock(&inode->i_lock);
+		WARN_ON(!(inode->i_state & I_EXCL_INUSE));
+		inode->i_state &= ~I_EXCL_INUSE;
+		spin_unlock(&inode->i_lock);
+	}
+}
+EXPORT_SYMBOL(inode_excl_inuse_unlock);
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index 2cd5741c873b..8415c0c6d40c 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -337,9 +337,6 @@ int ovl_check_setxattr(struct ovl_fs *ofs, struct dentry *upperdentry,
 		       enum ovl_xattr ox, const void *value, size_t size,
 		       int xerr);
 int ovl_set_impure(struct dentry *dentry, struct dentry *upperdentry);
-bool ovl_inuse_trylock(struct dentry *dentry);
-void ovl_inuse_unlock(struct dentry *dentry);
-bool ovl_is_inuse(struct dentry *dentry);
 bool ovl_need_index(struct dentry *dentry);
 int ovl_nlink_start(struct dentry *dentry);
 void ovl_nlink_end(struct dentry *dentry);
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 7bb0a47cb615..5c3361a2dc7c 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -224,10 +224,10 @@ static void ovl_free_fs(struct ovl_fs *ofs)
 	dput(ofs->indexdir);
 	dput(ofs->workdir);
 	if (ofs->workdir_locked)
-		ovl_inuse_unlock(ofs->workbasedir);
+		inode_excl_inuse_unlock(ofs->workbasedir);
 	dput(ofs->workbasedir);
 	if (ofs->upperdir_locked)
-		ovl_inuse_unlock(ovl_upper_mnt(ofs)->mnt_root);
+		inode_excl_inuse_unlock(ovl_upper_mnt(ofs)->mnt_root);
 
 	/* Hack!  Reuse ofs->layers as a vfsmount array before freeing it */
 	mounts = (struct vfsmount **) ofs->layers;
@@ -1239,7 +1239,7 @@ static int ovl_get_upper(struct super_block *sb, struct ovl_fs *ofs,
 	if (upper_mnt->mnt_sb->s_flags & SB_NOSEC)
 		sb->s_flags |= SB_NOSEC;
 
-	if (ovl_inuse_trylock(ovl_upper_mnt(ofs)->mnt_root)) {
+	if (inode_excl_inuse_trylock(ovl_upper_mnt(ofs)->mnt_root)) {
 		ofs->upperdir_locked = true;
 	} else {
 		err = ovl_report_in_use(ofs, "upperdir");
@@ -1499,7 +1499,7 @@ static int ovl_get_workdir(struct super_block *sb, struct ovl_fs *ofs,
 
 	ofs->workbasedir = dget(workpath.dentry);
 
-	if (ovl_inuse_trylock(ofs->workbasedir)) {
+	if (inode_excl_inuse_trylock(ofs->workbasedir)) {
 		ofs->workdir_locked = true;
 	} else {
 		err = ovl_report_in_use(ofs, "workdir");
@@ -1722,7 +1722,7 @@ static int ovl_get_layers(struct super_block *sb, struct ovl_fs *ofs,
 		if (err)
 			goto out;
 
-		if (ovl_is_inuse(stack[i].dentry)) {
+		if (inode_is_excl_inuse(stack[i].dentry)) {
 			err = ovl_report_in_use(ofs, "lowerdir");
 			if (err) {
 				iput(trap);
@@ -1872,7 +1872,7 @@ static int ovl_check_layer(struct super_block *sb, struct ovl_fs *ofs,
 		if (is_lower && ovl_lookup_trap_inode(sb, parent)) {
 			err = -ELOOP;
 			pr_err("overlapping %s path\n", name);
-		} else if (ovl_is_inuse(parent)) {
+		} else if (inode_is_excl_inuse(parent)) {
 			err = ovl_report_in_use(ofs, name);
 		}
 		next = parent;
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index f48284a2a896..748c4b22deb3 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -724,49 +724,6 @@ int ovl_set_protattr(struct inode *inode, struct dentry *upper,
 	return 0;
 }
 
-/**
- * Caller must hold a reference to inode to prevent it from being freed while
- * it is marked inuse.
- */
-bool ovl_inuse_trylock(struct dentry *dentry)
-{
-	struct inode *inode = d_inode(dentry);
-	bool locked = false;
-
-	spin_lock(&inode->i_lock);
-	if (!(inode->i_state & I_OVL_INUSE)) {
-		inode->i_state |= I_OVL_INUSE;
-		locked = true;
-	}
-	spin_unlock(&inode->i_lock);
-
-	return locked;
-}
-
-void ovl_inuse_unlock(struct dentry *dentry)
-{
-	if (dentry) {
-		struct inode *inode = d_inode(dentry);
-
-		spin_lock(&inode->i_lock);
-		WARN_ON(!(inode->i_state & I_OVL_INUSE));
-		inode->i_state &= ~I_OVL_INUSE;
-		spin_unlock(&inode->i_lock);
-	}
-}
-
-bool ovl_is_inuse(struct dentry *dentry)
-{
-	struct inode *inode = d_inode(dentry);
-	bool inuse;
-
-	spin_lock(&inode->i_lock);
-	inuse = (inode->i_state & I_OVL_INUSE);
-	spin_unlock(&inode->i_lock);
-
-	return inuse;
-}
-
 /*
  * Does this overlay dentry need to be indexed on copy up?
  */
diff --git a/include/linux/fs.h b/include/linux/fs.h
index f3daaea16554..4c15e270f1ac 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2320,8 +2320,10 @@ static inline void kiocb_clone(struct kiocb *kiocb, struct kiocb *kiocb_src,
  *			wb stat updates to grab the i_pages lock.  See
  *			inode_switch_wbs_work_fn() for details.
  *
- * I_OVL_INUSE		Used by overlayfs to get exclusive ownership on upper
- *			and work dirs among overlayfs mounts.
+ * I_EXCL_INUSE		Marked for exclusive use by a kernel service.  Used by
+ *			overlayfs to get exclusive ownership on upper and work
+ *			dirs among overlayfs mounts and by cachefiles to prevent
+ *			multiple access to a cache file.
  *
  * I_CREATING		New object's inode in the middle of setting up.
  *
@@ -2351,7 +2353,7 @@ static inline void kiocb_clone(struct kiocb *kiocb, struct kiocb *kiocb_src,
 #define I_LINKABLE		(1 << 10)
 #define I_DIRTY_TIME		(1 << 11)
 #define I_WB_SWITCH		(1 << 13)
-#define I_OVL_INUSE		(1 << 14)
+#define I_EXCL_INUSE		(1 << 14)
 #define I_CREATING		(1 << 15)
 #define I_DONTCACHE		(1 << 16)
 #define I_SYNC_QUEUED		(1 << 17)
@@ -2387,6 +2389,20 @@ static inline bool inode_is_dirtytime_only(struct inode *inode)
 				  I_FREEING | I_WILL_FREE)) == I_DIRTY_TIME;
 }
 
+bool inode_excl_inuse_trylock(struct dentry *dentry);
+void inode_excl_inuse_unlock(struct dentry *dentry);
+
+static inline bool inode_is_excl_inuse(struct dentry *dentry)
+{
+	struct inode *inode = d_inode(dentry);
+	bool inuse;
+
+	spin_lock(&inode->i_lock);
+	inuse = (inode->i_state & I_EXCL_INUSE);
+	spin_unlock(&inode->i_lock);
+	return inuse;
+}
+
 extern void inc_nlink(struct inode *inode);
 extern void drop_nlink(struct inode *inode);
 extern void clear_nlink(struct inode *inode);



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

* [PATCH 2/5] vfs: Add tracepoints for inode_excl_inuse_trylock/unlock
  2022-01-31 15:12 [RFC][PATCH 0/5] vfs, overlayfs, cachefiles: Combine I_OVL_INUSE and S_KERNEL_FILE and split out no-remove David Howells
  2022-01-31 15:12 ` [PATCH 1/5] vfs, overlayfs, cachefiles: Turn I_OVL_INUSE into something generic David Howells
@ 2022-01-31 15:13 ` David Howells
  2022-01-31 15:32   ` Amir Goldstein
  2022-01-31 15:13 ` [PATCH 3/5] cachefiles: Split removal-prevention from S_KERNEL_FILE and extend effects David Howells
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 12+ messages in thread
From: David Howells @ 2022-01-31 15:13 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: Miklos Szeredi, linux-unionfs, linux-cachefs, dhowells,
	Christoph Hellwig, Miklos Szeredi, Jeff Layton, Alexander Viro,
	torvalds, linux-unionfs, linux-cachefs, linux-fsdevel,
	linux-kernel

Add tracepoints for inode_excl_inuse_trylock/unlock() to record successful
and lock, failed lock, successful unlock and unlock when it wasn't locked.

Signed-off-by: David Howells <dhowells@redhat.com>
cc: Amir Goldstein <amir73il@gmail.com>
cc: Miklos Szeredi <miklos@szeredi.hu>
cc: linux-unionfs@vger.kernel.org
cc: linux-cachefs@redhat.com
---

 fs/inode.c           |   21 +++++++++++++++++----
 fs/overlayfs/super.c |   10 ++++++----
 include/linux/fs.h   |    9 +++++++--
 3 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/fs/inode.c b/fs/inode.c
index 954719f66113..61b93a89853f 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -22,6 +22,8 @@
 #include <linux/iversion.h>
 #include <trace/events/writeback.h>
 #include "internal.h"
+#define CREATE_TRACE_POINTS
+#include <trace/events/vfs.h>
 
 /*
  * Inode locking rules:
@@ -2409,11 +2411,14 @@ EXPORT_SYMBOL(current_time);
 /**
  * inode_excl_inuse_trylock - Try to exclusively lock an inode for kernel access
  * @dentry: Reference to the inode to be locked
+ * @o: Private reference for the kernel service
+ * @who: Which kernel service is trying to gain the lock
  *
  * Try to gain exclusive access to an inode for a kernel service, returning
  * true if successful.
  */
-bool inode_excl_inuse_trylock(struct dentry *dentry)
+bool inode_excl_inuse_trylock(struct dentry *dentry, unsigned int o,
+			      enum inode_excl_inuse_by who)
 {
 	struct inode *inode = d_inode(dentry);
 	bool locked = false;
@@ -2421,7 +2426,10 @@ bool inode_excl_inuse_trylock(struct dentry *dentry)
 	spin_lock(&inode->i_lock);
 	if (!(inode->i_state & I_EXCL_INUSE)) {
 		inode->i_state |= I_EXCL_INUSE;
+		trace_inode_excl_inuse_lock(inode, o, who);
 		locked = true;
+	} else {
+		trace_inode_excl_inuse_lock_failed(inode, o, who);
 	}
 	spin_unlock(&inode->i_lock);
 
@@ -2432,18 +2440,23 @@ EXPORT_SYMBOL(inode_excl_inuse_trylock);
 /**
  * inode_excl_inuse_unlock - Unlock exclusive kernel access to an inode
  * @dentry: Reference to the inode to be unlocked
+ * @o: Private reference for the kernel service
  *
  * Drop exclusive access to an inode for a kernel service.  A warning is given
  * if the inode was not marked for exclusive access.
  */
-void inode_excl_inuse_unlock(struct dentry *dentry)
+void inode_excl_inuse_unlock(struct dentry *dentry, unsigned int o)
 {
 	if (dentry) {
 		struct inode *inode = d_inode(dentry);
 
 		spin_lock(&inode->i_lock);
-		WARN_ON(!(inode->i_state & I_EXCL_INUSE));
-		inode->i_state &= ~I_EXCL_INUSE;
+		if (WARN_ON(!(inode->i_state & I_EXCL_INUSE))) {
+			trace_inode_excl_inuse_unlock_bad(inode, o);
+		} else {
+			inode->i_state &= ~I_EXCL_INUSE;
+			trace_inode_excl_inuse_unlock(inode, o);
+		}
 		spin_unlock(&inode->i_lock);
 	}
 }
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 5c3361a2dc7c..6434ae11496d 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -224,10 +224,10 @@ static void ovl_free_fs(struct ovl_fs *ofs)
 	dput(ofs->indexdir);
 	dput(ofs->workdir);
 	if (ofs->workdir_locked)
-		inode_excl_inuse_unlock(ofs->workbasedir);
+		inode_excl_inuse_unlock(ofs->workbasedir, 0);
 	dput(ofs->workbasedir);
 	if (ofs->upperdir_locked)
-		inode_excl_inuse_unlock(ovl_upper_mnt(ofs)->mnt_root);
+		inode_excl_inuse_unlock(ovl_upper_mnt(ofs)->mnt_root, 0);
 
 	/* Hack!  Reuse ofs->layers as a vfsmount array before freeing it */
 	mounts = (struct vfsmount **) ofs->layers;
@@ -1239,7 +1239,8 @@ static int ovl_get_upper(struct super_block *sb, struct ovl_fs *ofs,
 	if (upper_mnt->mnt_sb->s_flags & SB_NOSEC)
 		sb->s_flags |= SB_NOSEC;
 
-	if (inode_excl_inuse_trylock(ovl_upper_mnt(ofs)->mnt_root)) {
+	if (inode_excl_inuse_trylock(ovl_upper_mnt(ofs)->mnt_root, 0,
+				     inode_excl_inuse_by_overlayfs)) {
 		ofs->upperdir_locked = true;
 	} else {
 		err = ovl_report_in_use(ofs, "upperdir");
@@ -1499,7 +1500,8 @@ static int ovl_get_workdir(struct super_block *sb, struct ovl_fs *ofs,
 
 	ofs->workbasedir = dget(workpath.dentry);
 
-	if (inode_excl_inuse_trylock(ofs->workbasedir)) {
+	if (inode_excl_inuse_trylock(ofs->workbasedir, 0,
+				     inode_excl_inuse_by_overlayfs)) {
 		ofs->workdir_locked = true;
 	} else {
 		err = ovl_report_in_use(ofs, "workdir");
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 4c15e270f1ac..f461883d66a8 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2389,8 +2389,13 @@ static inline bool inode_is_dirtytime_only(struct inode *inode)
 				  I_FREEING | I_WILL_FREE)) == I_DIRTY_TIME;
 }
 
-bool inode_excl_inuse_trylock(struct dentry *dentry);
-void inode_excl_inuse_unlock(struct dentry *dentry);
+enum inode_excl_inuse_by {
+	inode_excl_inuse_by_overlayfs,
+};
+
+bool inode_excl_inuse_trylock(struct dentry *dentry, unsigned int o,
+			      enum inode_excl_inuse_by who);
+void inode_excl_inuse_unlock(struct dentry *dentry, unsigned int o);
 
 static inline bool inode_is_excl_inuse(struct dentry *dentry)
 {



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

* [PATCH 3/5] cachefiles: Split removal-prevention from S_KERNEL_FILE and extend effects
  2022-01-31 15:12 [RFC][PATCH 0/5] vfs, overlayfs, cachefiles: Combine I_OVL_INUSE and S_KERNEL_FILE and split out no-remove David Howells
  2022-01-31 15:12 ` [PATCH 1/5] vfs, overlayfs, cachefiles: Turn I_OVL_INUSE into something generic David Howells
  2022-01-31 15:13 ` [PATCH 2/5] vfs: Add tracepoints for inode_excl_inuse_trylock/unlock David Howells
@ 2022-01-31 15:13 ` David Howells
  2022-01-31 15:14 ` [PATCH 4/5] cachefiles: Use I_EXCL_INUSE instead of S_KERNEL_FILE David Howells
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: David Howells @ 2022-01-31 15:13 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: Miklos Szeredi, linux-unionfs, linux-cachefs, dhowells,
	Christoph Hellwig, Miklos Szeredi, Jeff Layton, Alexander Viro,
	torvalds, linux-unionfs, linux-cachefs, linux-fsdevel,
	linux-kernel

Split removal-prevention from the S_KERNEL_FILE flag as it's really a
separate job and it should affect unlnk and rename too, not just rmdir[1].
This new flag is called I_NO_REMOVE and moved to inode->i_state.

If this is set, the file or directory may not be removed, renamed or
unlinked.  This can then be used by cachefiles to prevent userspace
removing or renaming files and directories that the are being used.  It
could also be used by overlayfs to prevent fiddling in its work
directories.

The directory layout in its cache is very important to cachefiles as the
names are how it indexes the contents.  Removing objects can cause
cachefilesd to malfunction as it may find it can't reach bits of the
structure that it previously created and still has dentry pointers to.

This also closes a race between cachefilesd trying to cull an empty
directory and cachefiles trying to create something in it.

Amir Goldstein suggested that the check in vfs_rmdir() should be moved to
may_delete()[1], but it really needs to be done whilst the inode lock is
held.

I_NO_REMOVE should only be test/set/cleared under the inode lock without
the lock being dropped between that and the VFS operations that might be
affected by it lest races occur.

Note also that I_NO_REMOVE will prevent vfs_unlink() vfs_rmdir() and
vfs_rename() from operating on a file.

Signed-off-by: David Howells <dhowells@redhat.com>
cc: Amir Goldstein <amir73il@gmail.com>
cc: Miklos Szeredi <miklos@szeredi.hu>
cc: linux-unionfs@vger.kernel.org
cc: linux-cachefs@redhat.com
Link: https://lore.kernel.org/r/CAOQ4uxjEcvffv=rNXS-r+NLz+=6yk4abRuX_AMq9v-M4nf_PtA@mail.gmail.com [1]
---

 fs/cachefiles/namei.c |   12 ++++++++++--
 fs/namei.c            |    8 +++++---
 include/linux/fs.h    |    4 ++++
 3 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/fs/cachefiles/namei.c b/fs/cachefiles/namei.c
index f256c8aff7bb..8930c767d93a 100644
--- a/fs/cachefiles/namei.c
+++ b/fs/cachefiles/namei.c
@@ -20,8 +20,10 @@ static bool __cachefiles_mark_inode_in_use(struct cachefiles_object *object,
 	struct inode *inode = d_backing_inode(dentry);
 	bool can_use = false;
 
+	spin_lock(&inode->i_lock);
 	if (!(inode->i_flags & S_KERNEL_FILE)) {
 		inode->i_flags |= S_KERNEL_FILE;
+		inode->i_state |= I_NO_REMOVE;
 		trace_cachefiles_mark_active(object, inode);
 		can_use = true;
 	} else {
@@ -29,6 +31,7 @@ static bool __cachefiles_mark_inode_in_use(struct cachefiles_object *object,
 		pr_notice("cachefiles: Inode already in use: %pd (B=%lx)\n",
 			  dentry, inode->i_ino);
 	}
+	spin_unlock(&inode->i_lock);
 
 	return can_use;
 }
@@ -53,7 +56,10 @@ static void __cachefiles_unmark_inode_in_use(struct cachefiles_object *object,
 {
 	struct inode *inode = d_backing_inode(dentry);
 
+	spin_lock(&inode->i_lock);
 	inode->i_flags &= ~S_KERNEL_FILE;
+	inode->i_state &= ~I_NO_REMOVE;
+	spin_unlock(&inode->i_lock);
 	trace_cachefiles_mark_inactive(object, inode);
 }
 
@@ -392,8 +398,10 @@ int cachefiles_bury_object(struct cachefiles_cache *cache,
 		};
 		trace_cachefiles_rename(object, d_inode(rep)->i_ino, why);
 		ret = cachefiles_inject_read_error();
-		if (ret == 0)
+		if (ret == 0) {
+			__cachefiles_unmark_inode_in_use(object, rep);
 			ret = vfs_rename(&rd);
+		}
 		if (ret != 0)
 			trace_cachefiles_vfs_error(object, d_inode(dir), ret,
 						   cachefiles_trace_rename_error);
@@ -402,7 +410,6 @@ int cachefiles_bury_object(struct cachefiles_cache *cache,
 					    "Rename failed with error %d", ret);
 	}
 
-	__cachefiles_unmark_inode_in_use(object, rep);
 	unlock_rename(cache->graveyard, dir);
 	dput(grave);
 	_leave(" = 0");
@@ -426,6 +433,7 @@ int cachefiles_delete_object(struct cachefiles_object *object,
 	dget(dentry);
 
 	inode_lock_nested(d_backing_inode(fan), I_MUTEX_PARENT);
+	cachefiles_unmark_inode_in_use(object, object->file);
 	ret = cachefiles_unlink(volume->cache, object, fan, dentry, why);
 	inode_unlock(d_backing_inode(fan));
 	dput(dentry);
diff --git a/fs/namei.c b/fs/namei.c
index 3f1829b3ab5b..ea17377794d3 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -4009,7 +4009,7 @@ int vfs_rmdir(struct user_namespace *mnt_userns, struct inode *dir,
 
 	error = -EBUSY;
 	if (is_local_mountpoint(dentry) ||
-	    (dentry->d_inode->i_flags & S_KERNEL_FILE))
+	    (dentry->d_inode->i_flags & I_NO_REMOVE))
 		goto out;
 
 	error = security_inode_rmdir(dir, dentry);
@@ -4139,7 +4139,8 @@ int vfs_unlink(struct user_namespace *mnt_userns, struct inode *dir,
 	inode_lock(target);
 	if (IS_SWAPFILE(target))
 		error = -EPERM;
-	else if (is_local_mountpoint(dentry))
+	else if (is_local_mountpoint(dentry) ||
+		 (dentry->d_inode->i_flags & I_NO_REMOVE))
 		error = -EBUSY;
 	else {
 		error = security_inode_unlink(dir, dentry);
@@ -4653,7 +4654,8 @@ int vfs_rename(struct renamedata *rd)
 		goto out;
 
 	error = -EBUSY;
-	if (is_local_mountpoint(old_dentry) || is_local_mountpoint(new_dentry))
+	if (is_local_mountpoint(old_dentry) || is_local_mountpoint(new_dentry) ||
+	    (old_dentry->d_inode->i_flags & I_NO_REMOVE))
 		goto out;
 
 	if (max_links && new_dir != old_dir) {
diff --git a/include/linux/fs.h b/include/linux/fs.h
index f461883d66a8..a273d5cde731 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2332,6 +2332,9 @@ static inline void kiocb_clone(struct kiocb *kiocb, struct kiocb *kiocb_src,
  * I_SYNC_QUEUED	Inode is queued in b_io or b_more_io writeback lists.
  *			Used to detect that mark_inode_dirty() should not move
  * 			inode between dirty lists.
+ * I_NO_REMOVE		Unlink, rmdir and rename are not allowed to remove the
+ *			object or any of its hard links.
+ *
  *
  * I_PINNING_FSCACHE_WB	Inode is pinning an fscache object for writeback.
  *
@@ -2358,6 +2361,7 @@ static inline void kiocb_clone(struct kiocb *kiocb, struct kiocb *kiocb_src,
 #define I_DONTCACHE		(1 << 16)
 #define I_SYNC_QUEUED		(1 << 17)
 #define I_PINNING_FSCACHE_WB	(1 << 18)
+#define I_NO_REMOVE		(1 << 19)
 
 #define I_DIRTY_INODE (I_DIRTY_SYNC | I_DIRTY_DATASYNC)
 #define I_DIRTY (I_DIRTY_INODE | I_DIRTY_PAGES)



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

* [PATCH 4/5] cachefiles: Use I_EXCL_INUSE instead of S_KERNEL_FILE
  2022-01-31 15:12 [RFC][PATCH 0/5] vfs, overlayfs, cachefiles: Combine I_OVL_INUSE and S_KERNEL_FILE and split out no-remove David Howells
                   ` (2 preceding siblings ...)
  2022-01-31 15:13 ` [PATCH 3/5] cachefiles: Split removal-prevention from S_KERNEL_FILE and extend effects David Howells
@ 2022-01-31 15:14 ` David Howells
  2022-01-31 15:14 ` [PATCH 5/5] cachefiles: Remove the now-unused mark-inode-in-use tracepoints David Howells
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: David Howells @ 2022-01-31 15:14 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: Miklos Szeredi, linux-cachefs, linux-unionfs, dhowells,
	Christoph Hellwig, Miklos Szeredi, Jeff Layton, Alexander Viro,
	torvalds, linux-unionfs, linux-cachefs, linux-fsdevel,
	linux-kernel

Get rid of S_KERNEL_FILE and use I_EXCL_INUSE instead, thereby sharing that
flag with overlayfs.  This is used by cachefiles for two purposes: firstly,
to prevent simultaneous access to a backing file, which could cause data
corruption, and secondly, to allow cachefilesd to find out if it's allowed
to cull a backing file without having to have duplicate lookup
infrastructure (the VFS already has all the infrastructure that is
necessary to do the lookup; cachefiles just needs a single bit flag).

Signed-off-by: David Howells <dhowells@redhat.com>
cc: Amir Goldstein <amir73il@gmail.com>
cc: Miklos Szeredi <miklos@szeredi.hu>
cc: linux-cachefs@redhat.com
cc: linux-unionfs@vger.kernel.org
Link: https://lore.kernel.org/r/CAOQ4uxhRS3MGEnCUDcsB1RL0d1Oy0g0Rzm75hVFAJw2dJ7uKSA@mail.gmail.com/ [1]
---

 fs/cachefiles/namei.c |   46 ++++++++++++++++++++--------------------------
 include/linux/fs.h    |    2 +-
 2 files changed, 21 insertions(+), 27 deletions(-)

diff --git a/fs/cachefiles/namei.c b/fs/cachefiles/namei.c
index 8930c767d93a..0c88c82c188f 100644
--- a/fs/cachefiles/namei.c
+++ b/fs/cachefiles/namei.c
@@ -18,22 +18,19 @@ static bool __cachefiles_mark_inode_in_use(struct cachefiles_object *object,
 					   struct dentry *dentry)
 {
 	struct inode *inode = d_backing_inode(dentry);
-	bool can_use = false;
+	bool locked;
 
-	spin_lock(&inode->i_lock);
-	if (!(inode->i_flags & S_KERNEL_FILE)) {
-		inode->i_flags |= S_KERNEL_FILE;
+	locked = inode_excl_inuse_trylock(dentry, object ? object->debug_id : 0,
+					  inode_excl_inuse_by_cachefiles);
+	if (locked) {
+		spin_lock(&inode->i_lock);
 		inode->i_state |= I_NO_REMOVE;
-		trace_cachefiles_mark_active(object, inode);
-		can_use = true;
+		spin_unlock(&inode->i_lock);
 	} else {
-		trace_cachefiles_mark_failed(object, inode);
 		pr_notice("cachefiles: Inode already in use: %pd (B=%lx)\n",
 			  dentry, inode->i_ino);
 	}
-	spin_unlock(&inode->i_lock);
-
-	return can_use;
+	return locked;
 }
 
 static bool cachefiles_mark_inode_in_use(struct cachefiles_object *object,
@@ -57,10 +54,9 @@ static void __cachefiles_unmark_inode_in_use(struct cachefiles_object *object,
 	struct inode *inode = d_backing_inode(dentry);
 
 	spin_lock(&inode->i_lock);
-	inode->i_flags &= ~S_KERNEL_FILE;
-	inode->i_state &= ~I_NO_REMOVE;
+	inode->i_state |= I_NO_REMOVE;
 	spin_unlock(&inode->i_lock);
-	trace_cachefiles_mark_inactive(object, inode);
+	inode_excl_inuse_unlock(dentry, object ? object->debug_id : 0);
 }
 
 /*
@@ -754,7 +750,7 @@ static struct dentry *cachefiles_lookup_for_cull(struct cachefiles_cache *cache,
 		goto lookup_error;
 	if (d_is_negative(victim))
 		goto lookup_put;
-	if (d_inode(victim)->i_flags & S_KERNEL_FILE)
+	if (inode_is_excl_inuse(victim))
 		goto lookup_busy;
 	return victim;
 
@@ -790,6 +786,7 @@ int cachefiles_cull(struct cachefiles_cache *cache, struct dentry *dir,
 {
 	struct dentry *victim;
 	struct inode *inode;
+	bool locked;
 	int ret;
 
 	_enter(",%pd/,%s", dir, filename);
@@ -798,19 +795,16 @@ int cachefiles_cull(struct cachefiles_cache *cache, struct dentry *dir,
 	if (IS_ERR(victim))
 		return PTR_ERR(victim);
 
-	/* check to see if someone is using this object */
+	/* Check to see if someone is using this object and, if not, stop the
+	 * cache from picking it back up.
+	 */
 	inode = d_inode(victim);
 	inode_lock(inode);
-	if (inode->i_flags & S_KERNEL_FILE) {
-		ret = -EBUSY;
-	} else {
-		/* Stop the cache from picking it back up */
-		inode->i_flags |= S_KERNEL_FILE;
-		ret = 0;
-	}
+	locked = inode_excl_inuse_trylock(victim, 0,
+					  inode_excl_inuse_by_cachefiles);
 	inode_unlock(inode);
-	if (ret < 0)
-		goto error_unlock;
+	if (!locked)
+		goto busy;
 
 	ret = cachefiles_bury_object(cache, NULL, dir, victim,
 				     FSCACHE_OBJECT_WAS_CULLED);
@@ -822,8 +816,8 @@ int cachefiles_cull(struct cachefiles_cache *cache, struct dentry *dir,
 	_leave(" = 0");
 	return 0;
 
-error_unlock:
-	inode_unlock(d_inode(dir));
+busy:
+	ret = -EBUSY;
 error:
 	dput(victim);
 	if (ret == -ENOENT)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index a273d5cde731..009ca9f783bd 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2161,7 +2161,6 @@ struct super_operations {
 #define S_ENCRYPTED	(1 << 14) /* Encrypted file (using fs/crypto/) */
 #define S_CASEFOLD	(1 << 15) /* Casefolded file */
 #define S_VERITY	(1 << 16) /* Verity file (using fs/verity/) */
-#define S_KERNEL_FILE	(1 << 17) /* File is in use by the kernel (eg. fs/cachefiles) */
 
 /*
  * Note that nosuid etc flags are inode-specific: setting some file-system
@@ -2394,6 +2393,7 @@ static inline bool inode_is_dirtytime_only(struct inode *inode)
 }
 
 enum inode_excl_inuse_by {
+	inode_excl_inuse_by_cachefiles,
 	inode_excl_inuse_by_overlayfs,
 };
 



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

* [PATCH 5/5] cachefiles: Remove the now-unused mark-inode-in-use tracepoints
  2022-01-31 15:12 [RFC][PATCH 0/5] vfs, overlayfs, cachefiles: Combine I_OVL_INUSE and S_KERNEL_FILE and split out no-remove David Howells
                   ` (3 preceding siblings ...)
  2022-01-31 15:14 ` [PATCH 4/5] cachefiles: Use I_EXCL_INUSE instead of S_KERNEL_FILE David Howells
@ 2022-01-31 15:14 ` David Howells
  2022-01-31 16:28 ` [RFC][PATCH 0/5] vfs, overlayfs, cachefiles: Combine I_OVL_INUSE and S_KERNEL_FILE and split out no-remove Amir Goldstein
  2022-02-01 10:44 ` [PATCH 2/5] vfs: Add tracepoints for inode_excl_inuse_trylock/unlock David Howells
  6 siblings, 0 replies; 12+ messages in thread
From: David Howells @ 2022-01-31 15:14 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: linux-cachefs, dhowells, Christoph Hellwig, Miklos Szeredi,
	Jeff Layton, Alexander Viro, torvalds, linux-unionfs,
	linux-cachefs, linux-fsdevel, linux-kernel

The cachefiles tracepoints that relate to marking an inode in-use with the
S_KERNEL_FILE inode flag are now unused, superseded by general tracepoints.

Signed-off-by: David Howells <dhowells@redhat.com>
cc: linux-cachefs@redhat.com
---

 include/trace/events/cachefiles.h |   63 -------------------------------------
 1 file changed, 63 deletions(-)

diff --git a/include/trace/events/cachefiles.h b/include/trace/events/cachefiles.h
index c6f5aa74db89..1c56f9889f69 100644
--- a/include/trace/events/cachefiles.h
+++ b/include/trace/events/cachefiles.h
@@ -552,69 +552,6 @@ TRACE_EVENT(cachefiles_trunc,
 		      __entry->to)
 	    );
 
-TRACE_EVENT(cachefiles_mark_active,
-	    TP_PROTO(struct cachefiles_object *obj,
-		     struct inode *inode),
-
-	    TP_ARGS(obj, inode),
-
-	    /* Note that obj may be NULL */
-	    TP_STRUCT__entry(
-		    __field(unsigned int,		obj		)
-		    __field(ino_t,			inode		)
-			     ),
-
-	    TP_fast_assign(
-		    __entry->obj	= obj ? obj->debug_id : 0;
-		    __entry->inode	= inode->i_ino;
-			   ),
-
-	    TP_printk("o=%08x B=%lx",
-		      __entry->obj, __entry->inode)
-	    );
-
-TRACE_EVENT(cachefiles_mark_failed,
-	    TP_PROTO(struct cachefiles_object *obj,
-		     struct inode *inode),
-
-	    TP_ARGS(obj, inode),
-
-	    /* Note that obj may be NULL */
-	    TP_STRUCT__entry(
-		    __field(unsigned int,		obj		)
-		    __field(ino_t,			inode		)
-			     ),
-
-	    TP_fast_assign(
-		    __entry->obj	= obj ? obj->debug_id : 0;
-		    __entry->inode	= inode->i_ino;
-			   ),
-
-	    TP_printk("o=%08x B=%lx",
-		      __entry->obj, __entry->inode)
-	    );
-
-TRACE_EVENT(cachefiles_mark_inactive,
-	    TP_PROTO(struct cachefiles_object *obj,
-		     struct inode *inode),
-
-	    TP_ARGS(obj, inode),
-
-	    /* Note that obj may be NULL */
-	    TP_STRUCT__entry(
-		    __field(unsigned int,		obj		)
-		    __field(ino_t,			inode		)
-			     ),
-
-	    TP_fast_assign(
-		    __entry->obj	= obj ? obj->debug_id : 0;
-		    __entry->inode	= inode->i_ino;
-			   ),
-
-	    TP_printk("o=%08x B=%lx",
-		      __entry->obj, __entry->inode)
-	    );
-
 TRACE_EVENT(cachefiles_vfs_error,
 	    TP_PROTO(struct cachefiles_object *obj, struct inode *backer,
 		     int error, enum cachefiles_error_trace where),



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

* Re: [PATCH 1/5] vfs, overlayfs, cachefiles: Turn I_OVL_INUSE into something generic
  2022-01-31 15:12 ` [PATCH 1/5] vfs, overlayfs, cachefiles: Turn I_OVL_INUSE into something generic David Howells
@ 2022-01-31 15:29   ` Amir Goldstein
  2022-01-31 15:32   ` David Howells
  1 sibling, 0 replies; 12+ messages in thread
From: Amir Goldstein @ 2022-01-31 15:29 UTC (permalink / raw)
  To: David Howells
  Cc: Miklos Szeredi, overlayfs, linux-cachefs, Christoph Hellwig,
	Jeff Layton, Alexander Viro, Linus Torvalds, linux-fsdevel,
	linux-kernel

On Mon, Jan 31, 2022 at 5:13 PM David Howells <dhowells@redhat.com> wrote:
>
> Turn overlayfs's I_OVL_INUSE into something generic that cachefiles can
> also use for excluding access to its own cache files by renaming it to
> I_EXCL_INUSE as suggested by Amir[1] and hoisting the helpers to generic
> code.
>
> Suggested-by: Amir Goldstein <amir73il@gmail.com>
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Amir Goldstein <amir73il@gmail.com>
> cc: Miklos Szeredi <miklos@szeredi.hu>
> cc: linux-unionfs@vger.kernel.org
> cc: linux-cachefs@redhat.com
> Link: https://lore.kernel.org/r/CAOQ4uxhRS3MGEnCUDcsB1RL0d1Oy0g0Rzm75hVFAJw2dJ7uKSA@mail.gmail.com/ [1]
> ---
>
>  fs/inode.c               |   43 +++++++++++++++++++++++++++++++++++++++++++
>  fs/overlayfs/overlayfs.h |    3 ---
>  fs/overlayfs/super.c     |   12 ++++++------
>  fs/overlayfs/util.c      |   43 -------------------------------------------
>  include/linux/fs.h       |   22 +++++++++++++++++++---
>  5 files changed, 68 insertions(+), 55 deletions(-)
>
> diff --git a/fs/inode.c b/fs/inode.c
> index 63324df6fa27..954719f66113 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -2405,3 +2405,46 @@ struct timespec64 current_time(struct inode *inode)
>         return timestamp_truncate(now, inode);
>  }
>  EXPORT_SYMBOL(current_time);
> +
> +/**
> + * inode_excl_inuse_trylock - Try to exclusively lock an inode for kernel access
> + * @dentry: Reference to the inode to be locked
> + *
> + * Try to gain exclusive access to an inode for a kernel service, returning
> + * true if successful.
> + */
> +bool inode_excl_inuse_trylock(struct dentry *dentry)
> +{
> +       struct inode *inode = d_inode(dentry);
> +       bool locked = false;
> +
> +       spin_lock(&inode->i_lock);
> +       if (!(inode->i_state & I_EXCL_INUSE)) {
> +               inode->i_state |= I_EXCL_INUSE;
> +               locked = true;
> +       }
> +       spin_unlock(&inode->i_lock);
> +
> +       return locked;
> +}
> +EXPORT_SYMBOL(inode_excl_inuse_trylock);
> +
> +/**
> + * inode_excl_inuse_unlock - Unlock exclusive kernel access to an inode
> + * @dentry: Reference to the inode to be unlocked
> + *
> + * Drop exclusive access to an inode for a kernel service.  A warning is given
> + * if the inode was not marked for exclusive access.
> + */
> +void inode_excl_inuse_unlock(struct dentry *dentry)
> +{
> +       if (dentry) {
> +               struct inode *inode = d_inode(dentry);
> +
> +               spin_lock(&inode->i_lock);
> +               WARN_ON(!(inode->i_state & I_EXCL_INUSE));
> +               inode->i_state &= ~I_EXCL_INUSE;
> +               spin_unlock(&inode->i_lock);
> +       }
> +}
> +EXPORT_SYMBOL(inode_excl_inuse_unlock);
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index 2cd5741c873b..8415c0c6d40c 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -337,9 +337,6 @@ int ovl_check_setxattr(struct ovl_fs *ofs, struct dentry *upperdentry,
>                        enum ovl_xattr ox, const void *value, size_t size,
>                        int xerr);
>  int ovl_set_impure(struct dentry *dentry, struct dentry *upperdentry);
> -bool ovl_inuse_trylock(struct dentry *dentry);
> -void ovl_inuse_unlock(struct dentry *dentry);
> -bool ovl_is_inuse(struct dentry *dentry);
>  bool ovl_need_index(struct dentry *dentry);
>  int ovl_nlink_start(struct dentry *dentry);
>  void ovl_nlink_end(struct dentry *dentry);
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index 7bb0a47cb615..5c3361a2dc7c 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -224,10 +224,10 @@ static void ovl_free_fs(struct ovl_fs *ofs)
>         dput(ofs->indexdir);
>         dput(ofs->workdir);
>         if (ofs->workdir_locked)
> -               ovl_inuse_unlock(ofs->workbasedir);
> +               inode_excl_inuse_unlock(ofs->workbasedir);
>         dput(ofs->workbasedir);
>         if (ofs->upperdir_locked)
> -               ovl_inuse_unlock(ovl_upper_mnt(ofs)->mnt_root);
> +               inode_excl_inuse_unlock(ovl_upper_mnt(ofs)->mnt_root);
>
>         /* Hack!  Reuse ofs->layers as a vfsmount array before freeing it */
>         mounts = (struct vfsmount **) ofs->layers;
> @@ -1239,7 +1239,7 @@ static int ovl_get_upper(struct super_block *sb, struct ovl_fs *ofs,
>         if (upper_mnt->mnt_sb->s_flags & SB_NOSEC)
>                 sb->s_flags |= SB_NOSEC;
>
> -       if (ovl_inuse_trylock(ovl_upper_mnt(ofs)->mnt_root)) {
> +       if (inode_excl_inuse_trylock(ovl_upper_mnt(ofs)->mnt_root)) {
>                 ofs->upperdir_locked = true;
>         } else {
>                 err = ovl_report_in_use(ofs, "upperdir");
> @@ -1499,7 +1499,7 @@ static int ovl_get_workdir(struct super_block *sb, struct ovl_fs *ofs,
>
>         ofs->workbasedir = dget(workpath.dentry);
>
> -       if (ovl_inuse_trylock(ofs->workbasedir)) {
> +       if (inode_excl_inuse_trylock(ofs->workbasedir)) {
>                 ofs->workdir_locked = true;
>         } else {
>                 err = ovl_report_in_use(ofs, "workdir");
> @@ -1722,7 +1722,7 @@ static int ovl_get_layers(struct super_block *sb, struct ovl_fs *ofs,
>                 if (err)
>                         goto out;
>
> -               if (ovl_is_inuse(stack[i].dentry)) {
> +               if (inode_is_excl_inuse(stack[i].dentry)) {
>                         err = ovl_report_in_use(ofs, "lowerdir");
>                         if (err) {
>                                 iput(trap);
> @@ -1872,7 +1872,7 @@ static int ovl_check_layer(struct super_block *sb, struct ovl_fs *ofs,
>                 if (is_lower && ovl_lookup_trap_inode(sb, parent)) {
>                         err = -ELOOP;
>                         pr_err("overlapping %s path\n", name);
> -               } else if (ovl_is_inuse(parent)) {
> +               } else if (inode_is_excl_inuse(parent)) {
>                         err = ovl_report_in_use(ofs, name);
>                 }
>                 next = parent;
> diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
> index f48284a2a896..748c4b22deb3 100644
> --- a/fs/overlayfs/util.c
> +++ b/fs/overlayfs/util.c
> @@ -724,49 +724,6 @@ int ovl_set_protattr(struct inode *inode, struct dentry *upper,
>         return 0;
>  }
>
> -/**
> - * Caller must hold a reference to inode to prevent it from being freed while
> - * it is marked inuse.
> - */
> -bool ovl_inuse_trylock(struct dentry *dentry)
> -{
> -       struct inode *inode = d_inode(dentry);
> -       bool locked = false;
> -
> -       spin_lock(&inode->i_lock);
> -       if (!(inode->i_state & I_OVL_INUSE)) {
> -               inode->i_state |= I_OVL_INUSE;
> -               locked = true;
> -       }
> -       spin_unlock(&inode->i_lock);
> -
> -       return locked;
> -}
> -
> -void ovl_inuse_unlock(struct dentry *dentry)
> -{
> -       if (dentry) {
> -               struct inode *inode = d_inode(dentry);
> -
> -               spin_lock(&inode->i_lock);
> -               WARN_ON(!(inode->i_state & I_OVL_INUSE));
> -               inode->i_state &= ~I_OVL_INUSE;
> -               spin_unlock(&inode->i_lock);
> -       }
> -}
> -
> -bool ovl_is_inuse(struct dentry *dentry)
> -{
> -       struct inode *inode = d_inode(dentry);
> -       bool inuse;
> -
> -       spin_lock(&inode->i_lock);
> -       inuse = (inode->i_state & I_OVL_INUSE);
> -       spin_unlock(&inode->i_lock);
> -
> -       return inuse;
> -}

Please leave ovl_* as wrappers instead of changing super.c.

Thanks,
Amir.

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

* Re: [PATCH 1/5] vfs, overlayfs, cachefiles: Turn I_OVL_INUSE into something generic
  2022-01-31 15:12 ` [PATCH 1/5] vfs, overlayfs, cachefiles: Turn I_OVL_INUSE into something generic David Howells
  2022-01-31 15:29   ` Amir Goldstein
@ 2022-01-31 15:32   ` David Howells
  2022-01-31 16:20     ` Amir Goldstein
  1 sibling, 1 reply; 12+ messages in thread
From: David Howells @ 2022-01-31 15:32 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: dhowells, Miklos Szeredi, overlayfs, linux-cachefs,
	Christoph Hellwig, Jeff Layton, Alexander Viro, Linus Torvalds,
	linux-fsdevel, linux-kernel

Amir Goldstein <amir73il@gmail.com> wrote:

> Please leave ovl_* as wrappers instead of changing super.c.

Do you want them turning into inline functions?

David


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

* Re: [PATCH 2/5] vfs: Add tracepoints for inode_excl_inuse_trylock/unlock
  2022-01-31 15:13 ` [PATCH 2/5] vfs: Add tracepoints for inode_excl_inuse_trylock/unlock David Howells
@ 2022-01-31 15:32   ` Amir Goldstein
  0 siblings, 0 replies; 12+ messages in thread
From: Amir Goldstein @ 2022-01-31 15:32 UTC (permalink / raw)
  To: David Howells
  Cc: Miklos Szeredi, overlayfs, linux-cachefs, Christoph Hellwig,
	Jeff Layton, Alexander Viro, Linus Torvalds, linux-fsdevel,
	linux-kernel

On Mon, Jan 31, 2022 at 5:13 PM David Howells <dhowells@redhat.com> wrote:
>
> Add tracepoints for inode_excl_inuse_trylock/unlock() to record successful
> and lock, failed lock, successful unlock and unlock when it wasn't locked.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Amir Goldstein <amir73il@gmail.com>
> cc: Miklos Szeredi <miklos@szeredi.hu>
> cc: linux-unionfs@vger.kernel.org
> cc: linux-cachefs@redhat.com
> ---
>
>  fs/inode.c           |   21 +++++++++++++++++----
>  fs/overlayfs/super.c |   10 ++++++----
>  include/linux/fs.h   |    9 +++++++--
>  3 files changed, 30 insertions(+), 10 deletions(-)
>
> diff --git a/fs/inode.c b/fs/inode.c
> index 954719f66113..61b93a89853f 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -22,6 +22,8 @@
>  #include <linux/iversion.h>
>  #include <trace/events/writeback.h>
>  #include "internal.h"
> +#define CREATE_TRACE_POINTS
> +#include <trace/events/vfs.h>
>
>  /*
>   * Inode locking rules:
> @@ -2409,11 +2411,14 @@ EXPORT_SYMBOL(current_time);
>  /**
>   * inode_excl_inuse_trylock - Try to exclusively lock an inode for kernel access
>   * @dentry: Reference to the inode to be locked
> + * @o: Private reference for the kernel service
> + * @who: Which kernel service is trying to gain the lock
>   *
>   * Try to gain exclusive access to an inode for a kernel service, returning
>   * true if successful.
>   */
> -bool inode_excl_inuse_trylock(struct dentry *dentry)
> +bool inode_excl_inuse_trylock(struct dentry *dentry, unsigned int o,
> +                             enum inode_excl_inuse_by who)
>  {
>         struct inode *inode = d_inode(dentry);
>         bool locked = false;
> @@ -2421,7 +2426,10 @@ bool inode_excl_inuse_trylock(struct dentry *dentry)
>         spin_lock(&inode->i_lock);
>         if (!(inode->i_state & I_EXCL_INUSE)) {
>                 inode->i_state |= I_EXCL_INUSE;
> +               trace_inode_excl_inuse_lock(inode, o, who);
>                 locked = true;
> +       } else {
> +               trace_inode_excl_inuse_lock_failed(inode, o, who);
>         }
>         spin_unlock(&inode->i_lock);
>
> @@ -2432,18 +2440,23 @@ EXPORT_SYMBOL(inode_excl_inuse_trylock);
>  /**
>   * inode_excl_inuse_unlock - Unlock exclusive kernel access to an inode
>   * @dentry: Reference to the inode to be unlocked
> + * @o: Private reference for the kernel service
>   *
>   * Drop exclusive access to an inode for a kernel service.  A warning is given
>   * if the inode was not marked for exclusive access.
>   */
> -void inode_excl_inuse_unlock(struct dentry *dentry)
> +void inode_excl_inuse_unlock(struct dentry *dentry, unsigned int o)
>  {
>         if (dentry) {
>                 struct inode *inode = d_inode(dentry);
>
>                 spin_lock(&inode->i_lock);
> -               WARN_ON(!(inode->i_state & I_EXCL_INUSE));
> -               inode->i_state &= ~I_EXCL_INUSE;
> +               if (WARN_ON(!(inode->i_state & I_EXCL_INUSE))) {
> +                       trace_inode_excl_inuse_unlock_bad(inode, o);
> +               } else {
> +                       inode->i_state &= ~I_EXCL_INUSE;
> +                       trace_inode_excl_inuse_unlock(inode, o);
> +               }
>                 spin_unlock(&inode->i_lock);
>         }
>  }
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index 5c3361a2dc7c..6434ae11496d 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -224,10 +224,10 @@ static void ovl_free_fs(struct ovl_fs *ofs)
>         dput(ofs->indexdir);
>         dput(ofs->workdir);
>         if (ofs->workdir_locked)
> -               inode_excl_inuse_unlock(ofs->workbasedir);
> +               inode_excl_inuse_unlock(ofs->workbasedir, 0);
>         dput(ofs->workbasedir);
>         if (ofs->upperdir_locked)
> -               inode_excl_inuse_unlock(ovl_upper_mnt(ofs)->mnt_root);
> +               inode_excl_inuse_unlock(ovl_upper_mnt(ofs)->mnt_root, 0);
>
>         /* Hack!  Reuse ofs->layers as a vfsmount array before freeing it */
>         mounts = (struct vfsmount **) ofs->layers;
> @@ -1239,7 +1239,8 @@ static int ovl_get_upper(struct super_block *sb, struct ovl_fs *ofs,
>         if (upper_mnt->mnt_sb->s_flags & SB_NOSEC)
>                 sb->s_flags |= SB_NOSEC;
>
> -       if (inode_excl_inuse_trylock(ovl_upper_mnt(ofs)->mnt_root)) {
> +       if (inode_excl_inuse_trylock(ovl_upper_mnt(ofs)->mnt_root, 0,
> +                                    inode_excl_inuse_by_overlayfs)) {
>                 ofs->upperdir_locked = true;
>         } else {
>                 err = ovl_report_in_use(ofs, "upperdir");
> @@ -1499,7 +1500,8 @@ static int ovl_get_workdir(struct super_block *sb, struct ovl_fs *ofs,
>
>         ofs->workbasedir = dget(workpath.dentry);
>
> -       if (inode_excl_inuse_trylock(ofs->workbasedir)) {
> +       if (inode_excl_inuse_trylock(ofs->workbasedir, 0,
> +                                    inode_excl_inuse_by_overlayfs)) {

More incentive to keep the ovl_* wrappers.

Thanks,
Amir.

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

* Re: [PATCH 1/5] vfs, overlayfs, cachefiles: Turn I_OVL_INUSE into something generic
  2022-01-31 15:32   ` David Howells
@ 2022-01-31 16:20     ` Amir Goldstein
  0 siblings, 0 replies; 12+ messages in thread
From: Amir Goldstein @ 2022-01-31 16:20 UTC (permalink / raw)
  To: David Howells
  Cc: Miklos Szeredi, overlayfs, linux-cachefs, Christoph Hellwig,
	Jeff Layton, Alexander Viro, Linus Torvalds, linux-fsdevel,
	linux-kernel

On Mon, Jan 31, 2022 at 5:32 PM David Howells <dhowells@redhat.com> wrote:
>
> Amir Goldstein <amir73il@gmail.com> wrote:
>
> > Please leave ovl_* as wrappers instead of changing super.c.
>
> Do you want them turning into inline functions?
>

inline would be fine.

But I just noticed something that may wreck this party.

The assumption, when I proposed this merger, was that an inode for
upper/work and is exclusively owned by ovl driver, so there should be no
conflicts with other drivers setting inuse flag.

However, in ovl_check_layer(), we walk back to root to verify that an ovl
layer of a new instance is not a descendant of another ovl upper/work inuse.
So the meaning of I_OVL_INUSE is somewhat stronger than an exclusive inode lock.
It implies ownership on the entire subtree.

I guess there is no conflict with cachefiles since ovl upper/work should not be
intersecting with any cachefiles storage, but that complicates the
semantics when
it comes to a generic flag.

OTOH, I am not sure if this check on ovl mount is so smart to begin with.
This check was added (after the exclusive ownership meaning) to silence syzbot
that kept mutating to weird overlapping ovl setups.
I think that a better approach would be to fail a lookup in the upper layer
that results with a d_mountpoint() - those are not expected inside the overlay
upper layer AFAICT.

Anyway, I can make those changes if Miklos agrees with them, but I don't
want to complicate your work on this, so maybe for now, create the I_EXCL_INUSE
generic flag without changing overlayfs and I can make the cleanup to get rid of
I_OVL_INUSE later.

Thanks,
Amir.

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

* Re: [RFC][PATCH 0/5] vfs, overlayfs, cachefiles: Combine I_OVL_INUSE and S_KERNEL_FILE and split out no-remove
  2022-01-31 15:12 [RFC][PATCH 0/5] vfs, overlayfs, cachefiles: Combine I_OVL_INUSE and S_KERNEL_FILE and split out no-remove David Howells
                   ` (4 preceding siblings ...)
  2022-01-31 15:14 ` [PATCH 5/5] cachefiles: Remove the now-unused mark-inode-in-use tracepoints David Howells
@ 2022-01-31 16:28 ` Amir Goldstein
  2022-02-01 10:44 ` [PATCH 2/5] vfs: Add tracepoints for inode_excl_inuse_trylock/unlock David Howells
  6 siblings, 0 replies; 12+ messages in thread
From: Amir Goldstein @ 2022-01-31 16:28 UTC (permalink / raw)
  To: David Howells
  Cc: overlayfs, linux-cachefs, Miklos Szeredi, Christoph Hellwig,
	Jeff Layton, Alexander Viro, Linus Torvalds, linux-fsdevel,
	linux-kernel

On Mon, Jan 31, 2022 at 5:12 PM David Howells <dhowells@redhat.com> wrote:
>
>
> Hi Amir,
>
> How about this as a set of patches to do what you suggest[1] and hoist the
> handler functions for I_OVL_INUSE into common code and rename the flag to
> I_EXCL_INUSE.  This can then be shared with cachefiles - allowing me to get
> rid of S_KERNEL_FILE.
>

They look like what I had in mind.
Unfortunately, I had forgotten about another use that ovl makes of the flag
(see comment on patch 1/5). I'd made a suggestion on how to get rid of that use
case, but I hope this won't complicate things too much for you.

> I did split out the functionality for preventing file/dir removal to a
> separate flag, I_NO_REMOVE, so that it's not tied to I_EXCL_INUSE in case
> overlayfs doesn't want to use it.  The downside to that, though is that it
> requires a separate locking of i_lock to set/clear it.
>
> I also added four general tracepoints to log successful lock/unlock,
> failure to lock and a bad unlock.  The lock tracepoints log which driver
> asked for the lock and all tracepoints allow the driver to log an arbitrary
> reference number (in cachefiles's case this is the object debug ID).
>
> Questions:
>
>  (1) Should it be using a flag in i_state or a flag in i_flags?  I'm not
>      sure what the difference is really.

Me neither.

>
>  (2) Do we really need to take i_lock when testing I_EXCL_INUSE?  Would
>      READ_ONCE() suffice?
>

For ovl_is_inuse() I think READ_ONCE() should suffice.

Thanks,
Amir.

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

* Re: [PATCH 2/5] vfs: Add tracepoints for inode_excl_inuse_trylock/unlock
  2022-01-31 15:12 [RFC][PATCH 0/5] vfs, overlayfs, cachefiles: Combine I_OVL_INUSE and S_KERNEL_FILE and split out no-remove David Howells
                   ` (5 preceding siblings ...)
  2022-01-31 16:28 ` [RFC][PATCH 0/5] vfs, overlayfs, cachefiles: Combine I_OVL_INUSE and S_KERNEL_FILE and split out no-remove Amir Goldstein
@ 2022-02-01 10:44 ` David Howells
  6 siblings, 0 replies; 12+ messages in thread
From: David Howells @ 2022-02-01 10:44 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: dhowells, Miklos Szeredi, linux-unionfs, linux-cachefs,
	Christoph Hellwig, Jeff Layton, Alexander Viro, torvalds,
	linux-fsdevel, linux-kernel

I forgot to add the tracepoint header to the commit.

David
---
commit c8cefa2ac359254ecebfb20dcd0676bf9a167277
Author: David Howells <dhowells@redhat.com>
Date:   Mon Jan 31 11:52:44 2022 +0000

    vfs: Add tracepoints for inode_excl_inuse_trylock/unlock
    
    Add tracepoints for inode_excl_inuse_trylock/unlock() to record successful
    and lock, failed lock, successful unlock and unlock when it wasn't locked.
    
    Signed-off-by: David Howells <dhowells@redhat.com>
    cc: Amir Goldstein <amir73il@gmail.com>
    cc: Miklos Szeredi <miklos@szeredi.hu>
    cc: linux-unionfs@vger.kernel.org
    cc: linux-cachefs@redhat.com

diff --git a/fs/inode.c b/fs/inode.c
index 954719f66113..61b93a89853f 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -22,6 +22,8 @@
 #include <linux/iversion.h>
 #include <trace/events/writeback.h>
 #include "internal.h"
+#define CREATE_TRACE_POINTS
+#include <trace/events/vfs.h>
 
 /*
  * Inode locking rules:
@@ -2409,11 +2411,14 @@ EXPORT_SYMBOL(current_time);
 /**
  * inode_excl_inuse_trylock - Try to exclusively lock an inode for kernel access
  * @dentry: Reference to the inode to be locked
+ * @o: Private reference for the kernel service
+ * @who: Which kernel service is trying to gain the lock
  *
  * Try to gain exclusive access to an inode for a kernel service, returning
  * true if successful.
  */
-bool inode_excl_inuse_trylock(struct dentry *dentry)
+bool inode_excl_inuse_trylock(struct dentry *dentry, unsigned int o,
+			      enum inode_excl_inuse_by who)
 {
 	struct inode *inode = d_inode(dentry);
 	bool locked = false;
@@ -2421,7 +2426,10 @@ bool inode_excl_inuse_trylock(struct dentry *dentry)
 	spin_lock(&inode->i_lock);
 	if (!(inode->i_state & I_EXCL_INUSE)) {
 		inode->i_state |= I_EXCL_INUSE;
+		trace_inode_excl_inuse_lock(inode, o, who);
 		locked = true;
+	} else {
+		trace_inode_excl_inuse_lock_failed(inode, o, who);
 	}
 	spin_unlock(&inode->i_lock);
 
@@ -2432,18 +2440,23 @@ EXPORT_SYMBOL(inode_excl_inuse_trylock);
 /**
  * inode_excl_inuse_unlock - Unlock exclusive kernel access to an inode
  * @dentry: Reference to the inode to be unlocked
+ * @o: Private reference for the kernel service
  *
  * Drop exclusive access to an inode for a kernel service.  A warning is given
  * if the inode was not marked for exclusive access.
  */
-void inode_excl_inuse_unlock(struct dentry *dentry)
+void inode_excl_inuse_unlock(struct dentry *dentry, unsigned int o)
 {
 	if (dentry) {
 		struct inode *inode = d_inode(dentry);
 
 		spin_lock(&inode->i_lock);
-		WARN_ON(!(inode->i_state & I_EXCL_INUSE));
-		inode->i_state &= ~I_EXCL_INUSE;
+		if (WARN_ON(!(inode->i_state & I_EXCL_INUSE))) {
+			trace_inode_excl_inuse_unlock_bad(inode, o);
+		} else {
+			inode->i_state &= ~I_EXCL_INUSE;
+			trace_inode_excl_inuse_unlock(inode, o);
+		}
 		spin_unlock(&inode->i_lock);
 	}
 }
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index 5c3361a2dc7c..6434ae11496d 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -224,10 +224,10 @@ static void ovl_free_fs(struct ovl_fs *ofs)
 	dput(ofs->indexdir);
 	dput(ofs->workdir);
 	if (ofs->workdir_locked)
-		inode_excl_inuse_unlock(ofs->workbasedir);
+		inode_excl_inuse_unlock(ofs->workbasedir, 0);
 	dput(ofs->workbasedir);
 	if (ofs->upperdir_locked)
-		inode_excl_inuse_unlock(ovl_upper_mnt(ofs)->mnt_root);
+		inode_excl_inuse_unlock(ovl_upper_mnt(ofs)->mnt_root, 0);
 
 	/* Hack!  Reuse ofs->layers as a vfsmount array before freeing it */
 	mounts = (struct vfsmount **) ofs->layers;
@@ -1239,7 +1239,8 @@ static int ovl_get_upper(struct super_block *sb, struct ovl_fs *ofs,
 	if (upper_mnt->mnt_sb->s_flags & SB_NOSEC)
 		sb->s_flags |= SB_NOSEC;
 
-	if (inode_excl_inuse_trylock(ovl_upper_mnt(ofs)->mnt_root)) {
+	if (inode_excl_inuse_trylock(ovl_upper_mnt(ofs)->mnt_root, 0,
+				     inode_excl_inuse_by_overlayfs)) {
 		ofs->upperdir_locked = true;
 	} else {
 		err = ovl_report_in_use(ofs, "upperdir");
@@ -1499,7 +1500,8 @@ static int ovl_get_workdir(struct super_block *sb, struct ovl_fs *ofs,
 
 	ofs->workbasedir = dget(workpath.dentry);
 
-	if (inode_excl_inuse_trylock(ofs->workbasedir)) {
+	if (inode_excl_inuse_trylock(ofs->workbasedir, 0,
+				     inode_excl_inuse_by_overlayfs)) {
 		ofs->workdir_locked = true;
 	} else {
 		err = ovl_report_in_use(ofs, "workdir");
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 4c15e270f1ac..f461883d66a8 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2389,8 +2389,13 @@ static inline bool inode_is_dirtytime_only(struct inode *inode)
 				  I_FREEING | I_WILL_FREE)) == I_DIRTY_TIME;
 }
 
-bool inode_excl_inuse_trylock(struct dentry *dentry);
-void inode_excl_inuse_unlock(struct dentry *dentry);
+enum inode_excl_inuse_by {
+	inode_excl_inuse_by_overlayfs,
+};
+
+bool inode_excl_inuse_trylock(struct dentry *dentry, unsigned int o,
+			      enum inode_excl_inuse_by who);
+void inode_excl_inuse_unlock(struct dentry *dentry, unsigned int o);
 
 static inline bool inode_is_excl_inuse(struct dentry *dentry)
 {
diff --git a/include/trace/events/vfs.h b/include/trace/events/vfs.h
new file mode 100644
index 000000000000..f053752109dd
--- /dev/null
+++ b/include/trace/events/vfs.h
@@ -0,0 +1,134 @@
+/* VFS tracepoints
+ *
+ * Copyright (C) 2022 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells (dhowells@redhat.com)
+ */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM vfs
+
+#if !defined(_TRACE_VFS_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_VFS_H
+
+#include <linux/tracepoint.h>
+#include <linux/fs.h>
+
+/*
+ * Define enum -> string mappings for display.
+ */
+#define inode_excl_inuse_by_traces				\
+	EM(inode_excl_inuse_by_cachefiles,	"cachefiles")	\
+	E_(inode_excl_inuse_by_overlayfs,	"overlayfs")
+
+
+/*
+ * Export enum symbols via userspace.
+ */
+#undef EM
+#undef E_
+#define EM(a, b) TRACE_DEFINE_ENUM(a);
+#define E_(a, b) TRACE_DEFINE_ENUM(a);
+
+inode_excl_inuse_by_traces;
+
+/*
+ * Now redefine the EM() and E_() macros to map the enums to the strings that
+ * will be printed in the output.
+ */
+#undef EM
+#undef E_
+#define EM(a, b)	{ a, b },
+#define E_(a, b)	{ a, b }
+
+
+TRACE_EVENT(inode_excl_inuse_lock,
+	    TP_PROTO(struct inode *inode, unsigned int o,
+		     enum inode_excl_inuse_by who),
+
+	    TP_ARGS(inode, o, who),
+
+	    TP_STRUCT__entry(
+		    __field(ino_t,			inode		)
+		    __field(unsigned int,		o		)
+		    __field(enum inode_excl_inuse_by,	who		)
+			     ),
+
+	    TP_fast_assign(
+		    __entry->inode	= inode->i_ino;
+		    __entry->o		= o;
+		    __entry->who	= who;
+			   ),
+
+	    TP_printk("B=%lx %s o=%08x",
+		      __entry->inode,
+		      __print_symbolic(__entry->who, inode_excl_inuse_by_traces),
+		      __entry->o)
+	    );
+
+TRACE_EVENT(inode_excl_inuse_lock_failed,
+	    TP_PROTO(struct inode *inode, unsigned int o,
+		     enum inode_excl_inuse_by who),
+
+	    TP_ARGS(inode, o, who),
+
+	    TP_STRUCT__entry(
+		    __field(ino_t,			inode		)
+		    __field(unsigned int,		o		)
+		    __field(enum inode_excl_inuse_by,	who		)
+			     ),
+
+	    TP_fast_assign(
+		    __entry->inode	= inode->i_ino;
+		    __entry->o		= o;
+		    __entry->who	= who;
+			   ),
+
+	    TP_printk("B=%lx %s o=%08x",
+		      __entry->inode,
+		      __print_symbolic(__entry->who, inode_excl_inuse_by_traces),
+		      __entry->o)
+	    );
+
+TRACE_EVENT(inode_excl_inuse_unlock,
+	    TP_PROTO(struct inode *inode, unsigned int o),
+
+	    TP_ARGS(inode, o),
+
+	    TP_STRUCT__entry(
+		    __field(ino_t,			inode		)
+		    __field(unsigned int,		o		)
+			     ),
+
+	    TP_fast_assign(
+		    __entry->inode	= inode->i_ino;
+		    __entry->o		= o;
+			   ),
+
+	    TP_printk("B=%lx o=%08x",
+		      __entry->inode,
+		      __entry->o)
+	    );
+
+TRACE_EVENT(inode_excl_inuse_unlock_bad,
+	    TP_PROTO(struct inode *inode, unsigned int o),
+
+	    TP_ARGS(inode, o),
+
+	    TP_STRUCT__entry(
+		    __field(ino_t,			inode		)
+		    __field(unsigned int,		o		)
+			     ),
+
+	    TP_fast_assign(
+		    __entry->inode	= inode->i_ino;
+		    __entry->o		= o;
+			   ),
+
+	    TP_printk("B=%lx o=%08x",
+		      __entry->inode,
+		      __entry->o)
+	    );
+
+#endif /* _TRACE_VFS_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>


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

end of thread, other threads:[~2022-02-01 10:44 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-31 15:12 [RFC][PATCH 0/5] vfs, overlayfs, cachefiles: Combine I_OVL_INUSE and S_KERNEL_FILE and split out no-remove David Howells
2022-01-31 15:12 ` [PATCH 1/5] vfs, overlayfs, cachefiles: Turn I_OVL_INUSE into something generic David Howells
2022-01-31 15:29   ` Amir Goldstein
2022-01-31 15:32   ` David Howells
2022-01-31 16:20     ` Amir Goldstein
2022-01-31 15:13 ` [PATCH 2/5] vfs: Add tracepoints for inode_excl_inuse_trylock/unlock David Howells
2022-01-31 15:32   ` Amir Goldstein
2022-01-31 15:13 ` [PATCH 3/5] cachefiles: Split removal-prevention from S_KERNEL_FILE and extend effects David Howells
2022-01-31 15:14 ` [PATCH 4/5] cachefiles: Use I_EXCL_INUSE instead of S_KERNEL_FILE David Howells
2022-01-31 15:14 ` [PATCH 5/5] cachefiles: Remove the now-unused mark-inode-in-use tracepoints David Howells
2022-01-31 16:28 ` [RFC][PATCH 0/5] vfs, overlayfs, cachefiles: Combine I_OVL_INUSE and S_KERNEL_FILE and split out no-remove Amir Goldstein
2022-02-01 10:44 ` [PATCH 2/5] vfs: Add tracepoints for inode_excl_inuse_trylock/unlock David Howells

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).