On Mon, Jan 31, 2022 at 5:13 PM David Howells <dhowells@xxxxxxxxxx> 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@xxxxxxxxxx>
> cc: Amir Goldstein <amir73il@xxxxxxxxx>
> cc: Miklos Szeredi <miklos@xxxxxxxxxx>
> cc: linux-unionfs@xxxxxxxxxxxxxxx
> cc: linux-cachefs@xxxxxxxxxx
> ---
>
> 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.
