| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20250206-gasversorger-flugbereit-9eed46e951f9@brauner>
Date: Thu, 6 Feb 2025 14:22:34 +0100
From: Christian Brauner <brauner@...nel.org>
To: NeilBrown <neilb@...e.de>
Cc: Alexander Viro <viro@...iv.linux.org.uk>, Jan Kara <jack@...e.cz>,
Linus Torvalds <torvalds@...ux-foundation.org>, Jeff Layton <jlayton@...nel.org>,
Dave Chinner <david@...morbit.com>, linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 10/19] VFS: introduce inode flags to report locking needs
for directory ops
On Thu, Feb 06, 2025 at 04:42:47PM +1100, NeilBrown wrote:
> If a filesystem supports _async ops for some directory ops we can take a
> "shared" lock on i_rwsem otherwise we must take an "exclusive" lock. As
> the filesystem may support some async ops but not others we need to
> easily determine which.
>
> With this patch we group the ops into 4 groups that are likely be
> supported together:
>
> CREATE: create, link, mkdir, mknod
> REMOVE: rmdir, unlink
> RENAME: rename
> OPEN: atomic_open, create
>
> and set S_ASYNC_XXX for each when the inode in initialised.
>
> We also add a LOOKUP_REMOVE intent flag which will be used by locking
> interfaces to help know which group is being used.
>
> Signed-off-by: NeilBrown <neilb@...e.de>
> ---
> fs/dcache.c | 24 ++++++++++++++++++++++++
> include/linux/fs.h | 5 +++++
> include/linux/namei.h | 5 +++--
> 3 files changed, 32 insertions(+), 2 deletions(-)
>
> diff --git a/fs/dcache.c b/fs/dcache.c
> index e49607d00d2d..37c0f655166d 100644
> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -384,6 +384,27 @@ static inline void __d_set_inode_and_type(struct dentry *dentry,
> smp_store_release(&dentry->d_flags, flags);
> }
>
> +static void set_inode_flags(struct inode *inode)
> +{
> + const struct inode_operations *i_op = inode->i_op;
> +
> + lockdep_assert_held(&inode->i_lock);
> + if ((i_op->create_async || !i_op->create) &&
> + (i_op->link_async || !i_op->link) &&
> + (i_op->symlink_async || !i_op->symlink) &&
> + (i_op->mkdir_async || !i_op->mkdir) &&
> + (i_op->mknod_async || !i_op->mknod))
> + inode->i_flags |= S_ASYNC_CREATE;
> + if ((i_op->unlink_async || !i_op->unlink) &&
> + (i_op->mkdir_async || !i_op->mkdir))
> + inode->i_flags |= S_ASYNC_REMOVE;
> + if (i_op->rename_async)
> + inode->i_flags |= S_ASYNC_RENAME;
> + if (i_op->atomic_open_async ||
> + (!i_op->atomic_open && i_op->create_async))
> + inode->i_flags |= S_ASYNC_OPEN;
> +}
I think this is unpleasant. As I said we should fold _async into the
normal methods. Then we can add:
diff --git a/include/linux/fs.h b/include/linux/fs.h
index be3ad155ec9f..1d19f72448fc 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2186,6 +2186,7 @@ int wrap_directory_iterator(struct file *, struct dir_context *,
{ return wrap_directory_iterator(file, ctx, x); }
struct inode_operations {
+ iop_flags_t iop_flags;
struct dentry * (*lookup) (struct inode *,struct dentry *, unsigned int);
const char * (*get_link) (struct dentry *, struct inode *, struct delayed_call *);
int (*permission) (struct mnt_idmap *, struct inode *, int);
which is similar to what I did for
struct file_operations {
struct module *owner;
fop_flags_t fop_flags;
and introduce
IOP_ASYNC_CREATE
IOP_ASYNC_OPEN
etc and then filesystems can just do:
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index df9669d4ded7..90c7aeb49466 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -10859,6 +10859,7 @@ static void nfs4_disable_swap(struct inode *inode)
}
static const struct inode_operations nfs4_dir_inode_operations = {
+ .iop_flags = IOP_ASYNC_CREATE | IOP_ASYNC_OPEN,
.create = nfs_create,
.lookup = nfs_lookup,
.atomic_open = nfs_atomic_open,
and then you can raise S_ASYNC_OPEN and so on based on the flags, not
the individual methods.
> +
> static inline void __d_clear_type_and_inode(struct dentry *dentry)
> {
> unsigned flags = READ_ONCE(dentry->d_flags);
> @@ -1893,6 +1914,7 @@ static void __d_instantiate(struct dentry *dentry, struct inode *inode)
> raw_write_seqcount_begin(&dentry->d_seq);
> __d_set_inode_and_type(dentry, inode, add_flags);
> raw_write_seqcount_end(&dentry->d_seq);
> + set_inode_flags(inode);
> fsnotify_update_flags(dentry);
> spin_unlock(&dentry->d_lock);
> }
> @@ -1999,6 +2021,7 @@ static struct dentry *__d_obtain_alias(struct inode *inode, bool disconnected)
>
> spin_lock(&new->d_lock);
> __d_set_inode_and_type(new, inode, add_flags);
> + set_inode_flags(inode);
> hlist_add_head(&new->d_u.d_alias, &inode->i_dentry);
> if (!disconnected) {
> hlist_bl_lock(&sb->s_roots);
> @@ -2701,6 +2724,7 @@ static inline void __d_add(struct dentry *dentry, struct inode *inode)
> raw_write_seqcount_begin(&dentry->d_seq);
> __d_set_inode_and_type(dentry, inode, add_flags);
> raw_write_seqcount_end(&dentry->d_seq);
> + set_inode_flags(inode);
> fsnotify_update_flags(dentry);
> }
> __d_rehash(dentry);
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index e414400c2487..9a9282fef347 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2361,6 +2361,11 @@ struct super_operations {
> #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) */
>
> +#define S_ASYNC_CREATE BIT(18) /* create, link, symlink, mkdir, mknod all _async */
> +#define S_ASYNC_REMOVE BIT(19) /* unlink, mkdir both _async */
> +#define S_ASYNC_RENAME BIT(20) /* rename_async supported */
> +#define S_ASYNC_OPEN BIT(21) /* atomic_open_async or create_async supported */
> +
> /*
> * Note that nosuid etc flags are inode-specific: setting some file-system
> * flags just means all the inodes inherit those flags by default. It might be
> diff --git a/include/linux/namei.h b/include/linux/namei.h
> index 76c587a5ec3a..72e351640406 100644
> --- a/include/linux/namei.h
> +++ b/include/linux/namei.h
> @@ -40,10 +40,11 @@ enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT};
> #define LOOKUP_CREATE BIT(17) /* ... in object creation */
> #define LOOKUP_EXCL BIT(18) /* ... in target must not exist */
> #define LOOKUP_RENAME_TARGET BIT(19) /* ... in destination of rename() */
> +#define LOOKUP_REMOVE BIT(20) /* ... in target of object removal */
>
> #define LOOKUP_INTENT_FLAGS (LOOKUP_OPEN | LOOKUP_CREATE | LOOKUP_EXCL | \
> - LOOKUP_RENAME_TARGET)
> -/* 4 spare bits for intent */
> + LOOKUP_RENAME_TARGET | LOOKUP_REMOVE)
> +/* 3 spare bits for intent */
>
> /* Scoping flags for lookup. */
> #define LOOKUP_NO_SYMLINKS BIT(24) /* No symlink crossing. */
> --
> 2.47.1
>
Powered by blists - more mailing lists