On Mon, Jan 31, 2022 at 5:13 PM David Howells <dhowells@xxxxxxxxxx> 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@xxxxxxxxx> > 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 > Link: https://lore.kernel.org/r/CAOQ4uxhRS3MGEnCUDcsB1RL0d1Oy0g0Rzm75hVFAJw2dJ7uKSA@xxxxxxxxxxxxxx/ [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.
