diff mbox series

[RFC,11/20] ext4: store cookie in private data

Message ID 20240830-vfs-file-f_version-v1-11-6d3e4816aa7b@kernel.org (mailing list archive)
State New
Headers show
Series file: remove f_version | expand

Commit Message

Christian Brauner Aug. 30, 2024, 1:04 p.m. UTC 
Store the cookie to detect concurrent seeks on directories in
file->private_data.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 fs/ext4/dir.c    | 50 ++++++++++++++++++++++++++++----------------------
 fs/ext4/ext4.h   |  2 ++
 fs/ext4/inline.c |  7 ++++---
 3 files changed, 34 insertions(+), 25 deletions(-)

Comments

Theodore Ts'o Sept. 1, 2024, 7:36 p.m. UTC | #1 
On Fri, Aug 30, 2024 at 03:04:52PM +0200, Christian Brauner wrote:
> Store the cookie to detect concurrent seeks on directories in
> file->private_data.
> 
> Signed-off-by: Christian Brauner <brauner@kernel.org>

Acked-by: Theodore Ts'o <tytso@mit.edu>
Jan Kara Sept. 3, 2024, 11:37 a.m. UTC | #2 
On Fri 30-08-24 15:04:52, Christian Brauner wrote:
> Store the cookie to detect concurrent seeks on directories in
> file->private_data.
> 
> Signed-off-by: Christian Brauner <brauner@kernel.org>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/ext4/dir.c    | 50 ++++++++++++++++++++++++++++----------------------
>  fs/ext4/ext4.h   |  2 ++
>  fs/ext4/inline.c |  7 ++++---
>  3 files changed, 34 insertions(+), 25 deletions(-)
> 
> diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c
> index ff4514e4626b..13196afe55ce 100644
> --- a/fs/ext4/dir.c
> +++ b/fs/ext4/dir.c
> @@ -133,6 +133,7 @@ static int ext4_readdir(struct file *file, struct dir_context *ctx)
>  	struct super_block *sb = inode->i_sb;
>  	struct buffer_head *bh = NULL;
>  	struct fscrypt_str fstr = FSTR_INIT(NULL, 0);
> +	struct dir_private_info *info = file->private_data;
>  
>  	err = fscrypt_prepare_readdir(inode);
>  	if (err)
> @@ -229,7 +230,7 @@ static int ext4_readdir(struct file *file, struct dir_context *ctx)
>  		 * readdir(2), then we might be pointing to an invalid
>  		 * dirent right now.  Scan from the start of the block
>  		 * to make sure. */
> -		if (!inode_eq_iversion(inode, file->f_version)) {
> +		if (!inode_eq_iversion(inode, info->cookie)) {
>  			for (i = 0; i < sb->s_blocksize && i < offset; ) {
>  				de = (struct ext4_dir_entry_2 *)
>  					(bh->b_data + i);
> @@ -249,7 +250,7 @@ static int ext4_readdir(struct file *file, struct dir_context *ctx)
>  			offset = i;
>  			ctx->pos = (ctx->pos & ~(sb->s_blocksize - 1))
>  				| offset;
> -			file->f_version = inode_query_iversion(inode);
> +			info->cookie = inode_query_iversion(inode);
>  		}
>  
>  		while (ctx->pos < inode->i_size
> @@ -384,6 +385,7 @@ static inline loff_t ext4_get_htree_eof(struct file *filp)
>  static loff_t ext4_dir_llseek(struct file *file, loff_t offset, int whence)
>  {
>  	struct inode *inode = file->f_mapping->host;
> +	struct dir_private_info *info = file->private_data;
>  	int dx_dir = is_dx_dir(inode);
>  	loff_t ret, htree_max = ext4_get_htree_eof(file);
>  
> @@ -392,7 +394,7 @@ static loff_t ext4_dir_llseek(struct file *file, loff_t offset, int whence)
>  						    htree_max, htree_max);
>  	else
>  		ret = ext4_llseek(file, offset, whence);
> -	file->f_version = inode_peek_iversion(inode) - 1;
> +	info->cookie = inode_peek_iversion(inode) - 1;
>  	return ret;
>  }
>  
> @@ -429,18 +431,15 @@ static void free_rb_tree_fname(struct rb_root *root)
>  	*root = RB_ROOT;
>  }
>  
> -
> -static struct dir_private_info *ext4_htree_create_dir_info(struct file *filp,
> -							   loff_t pos)
> +static void ext4_htree_init_dir_info(struct file *filp, loff_t pos)
>  {
> -	struct dir_private_info *p;
> -
> -	p = kzalloc(sizeof(*p), GFP_KERNEL);
> -	if (!p)
> -		return NULL;
> -	p->curr_hash = pos2maj_hash(filp, pos);
> -	p->curr_minor_hash = pos2min_hash(filp, pos);
> -	return p;
> +	struct dir_private_info *p = filp->private_data;
> +
> +	if (is_dx_dir(file_inode(filp)) && !p->initialized) {
> +		p->curr_hash = pos2maj_hash(filp, pos);
> +		p->curr_minor_hash = pos2min_hash(filp, pos);
> +		p->initialized = true;
> +	}
>  }
>  
>  void ext4_htree_free_dir_info(struct dir_private_info *p)
> @@ -552,12 +551,7 @@ static int ext4_dx_readdir(struct file *file, struct dir_context *ctx)
>  	struct fname *fname;
>  	int ret = 0;
>  
> -	if (!info) {
> -		info = ext4_htree_create_dir_info(file, ctx->pos);
> -		if (!info)
> -			return -ENOMEM;
> -		file->private_data = info;
> -	}
> +	ext4_htree_init_dir_info(file, ctx->pos);
>  
>  	if (ctx->pos == ext4_get_htree_eof(file))
>  		return 0;	/* EOF */
> @@ -590,10 +584,10 @@ static int ext4_dx_readdir(struct file *file, struct dir_context *ctx)
>  		 * cached entries.
>  		 */
>  		if ((!info->curr_node) ||
> -		    !inode_eq_iversion(inode, file->f_version)) {
> +		    !inode_eq_iversion(inode, info->cookie)) {
>  			info->curr_node = NULL;
>  			free_rb_tree_fname(&info->root);
> -			file->f_version = inode_query_iversion(inode);
> +			info->cookie = inode_query_iversion(inode);
>  			ret = ext4_htree_fill_tree(file, info->curr_hash,
>  						   info->curr_minor_hash,
>  						   &info->next_hash);
> @@ -664,7 +658,19 @@ int ext4_check_all_de(struct inode *dir, struct buffer_head *bh, void *buf,
>  	return 0;
>  }
>  
> +static int ext4_dir_open(struct inode *inode, struct file *file)
> +{
> +	struct dir_private_info *info;
> +
> +	info = kzalloc(sizeof(*info), GFP_KERNEL);
> +	if (!info)
> +		return -ENOMEM;
> +	file->private_data = info;
> +	return 0;
> +}
> +
>  const struct file_operations ext4_dir_operations = {
> +	.open		= ext4_dir_open,
>  	.llseek		= ext4_dir_llseek,
>  	.read		= generic_read_dir,
>  	.iterate_shared	= ext4_readdir,
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 08acd152261e..d62a4b9b26ce 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -2553,6 +2553,8 @@ struct dir_private_info {
>  	__u32		curr_hash;
>  	__u32		curr_minor_hash;
>  	__u32		next_hash;
> +	u64		cookie;
> +	bool		initialized;
>  };
>  
>  /* calculate the first block number of the group */
> diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
> index e7a09a99837b..4282e12dc405 100644
> --- a/fs/ext4/inline.c
> +++ b/fs/ext4/inline.c
> @@ -1460,6 +1460,7 @@ int ext4_read_inline_dir(struct file *file,
>  	struct ext4_iloc iloc;
>  	void *dir_buf = NULL;
>  	int dotdot_offset, dotdot_size, extra_offset, extra_size;
> +	struct dir_private_info *info = file->private_data;
>  
>  	ret = ext4_get_inode_loc(inode, &iloc);
>  	if (ret)
> @@ -1503,12 +1504,12 @@ int ext4_read_inline_dir(struct file *file,
>  	extra_size = extra_offset + inline_size;
>  
>  	/*
> -	 * If the version has changed since the last call to
> +	 * If the cookie has changed since the last call to
>  	 * readdir(2), then we might be pointing to an invalid
>  	 * dirent right now.  Scan from the start of the inline
>  	 * dir to make sure.
>  	 */
> -	if (!inode_eq_iversion(inode, file->f_version)) {
> +	if (!inode_eq_iversion(inode, info->cookie)) {
>  		for (i = 0; i < extra_size && i < offset;) {
>  			/*
>  			 * "." is with offset 0 and
> @@ -1540,7 +1541,7 @@ int ext4_read_inline_dir(struct file *file,
>  		}
>  		offset = i;
>  		ctx->pos = offset;
> -		file->f_version = inode_query_iversion(inode);
> +		info->cookie = inode_query_iversion(inode);
>  	}
>  
>  	while (ctx->pos < extra_size) {
> 
> -- 
> 2.45.2
>
diff mbox series

Patch

diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c
index ff4514e4626b..13196afe55ce 100644
--- a/fs/ext4/dir.c
+++ b/fs/ext4/dir.c
@@ -133,6 +133,7 @@  static int ext4_readdir(struct file *file, struct dir_context *ctx)
 	struct super_block *sb = inode->i_sb;
 	struct buffer_head *bh = NULL;
 	struct fscrypt_str fstr = FSTR_INIT(NULL, 0);
+	struct dir_private_info *info = file->private_data;
 
 	err = fscrypt_prepare_readdir(inode);
 	if (err)
@@ -229,7 +230,7 @@  static int ext4_readdir(struct file *file, struct dir_context *ctx)
 		 * readdir(2), then we might be pointing to an invalid
 		 * dirent right now.  Scan from the start of the block
 		 * to make sure. */
-		if (!inode_eq_iversion(inode, file->f_version)) {
+		if (!inode_eq_iversion(inode, info->cookie)) {
 			for (i = 0; i < sb->s_blocksize && i < offset; ) {
 				de = (struct ext4_dir_entry_2 *)
 					(bh->b_data + i);
@@ -249,7 +250,7 @@  static int ext4_readdir(struct file *file, struct dir_context *ctx)
 			offset = i;
 			ctx->pos = (ctx->pos & ~(sb->s_blocksize - 1))
 				| offset;
-			file->f_version = inode_query_iversion(inode);
+			info->cookie = inode_query_iversion(inode);
 		}
 
 		while (ctx->pos < inode->i_size
@@ -384,6 +385,7 @@  static inline loff_t ext4_get_htree_eof(struct file *filp)
 static loff_t ext4_dir_llseek(struct file *file, loff_t offset, int whence)
 {
 	struct inode *inode = file->f_mapping->host;
+	struct dir_private_info *info = file->private_data;
 	int dx_dir = is_dx_dir(inode);
 	loff_t ret, htree_max = ext4_get_htree_eof(file);
 
@@ -392,7 +394,7 @@  static loff_t ext4_dir_llseek(struct file *file, loff_t offset, int whence)
 						    htree_max, htree_max);
 	else
 		ret = ext4_llseek(file, offset, whence);
-	file->f_version = inode_peek_iversion(inode) - 1;
+	info->cookie = inode_peek_iversion(inode) - 1;
 	return ret;
 }
 
@@ -429,18 +431,15 @@  static void free_rb_tree_fname(struct rb_root *root)
 	*root = RB_ROOT;
 }
 
-
-static struct dir_private_info *ext4_htree_create_dir_info(struct file *filp,
-							   loff_t pos)
+static void ext4_htree_init_dir_info(struct file *filp, loff_t pos)
 {
-	struct dir_private_info *p;
-
-	p = kzalloc(sizeof(*p), GFP_KERNEL);
-	if (!p)
-		return NULL;
-	p->curr_hash = pos2maj_hash(filp, pos);
-	p->curr_minor_hash = pos2min_hash(filp, pos);
-	return p;
+	struct dir_private_info *p = filp->private_data;
+
+	if (is_dx_dir(file_inode(filp)) && !p->initialized) {
+		p->curr_hash = pos2maj_hash(filp, pos);
+		p->curr_minor_hash = pos2min_hash(filp, pos);
+		p->initialized = true;
+	}
 }
 
 void ext4_htree_free_dir_info(struct dir_private_info *p)
@@ -552,12 +551,7 @@  static int ext4_dx_readdir(struct file *file, struct dir_context *ctx)
 	struct fname *fname;
 	int ret = 0;
 
-	if (!info) {
-		info = ext4_htree_create_dir_info(file, ctx->pos);
-		if (!info)
-			return -ENOMEM;
-		file->private_data = info;
-	}
+	ext4_htree_init_dir_info(file, ctx->pos);
 
 	if (ctx->pos == ext4_get_htree_eof(file))
 		return 0;	/* EOF */
@@ -590,10 +584,10 @@  static int ext4_dx_readdir(struct file *file, struct dir_context *ctx)
 		 * cached entries.
 		 */
 		if ((!info->curr_node) ||
-		    !inode_eq_iversion(inode, file->f_version)) {
+		    !inode_eq_iversion(inode, info->cookie)) {
 			info->curr_node = NULL;
 			free_rb_tree_fname(&info->root);
-			file->f_version = inode_query_iversion(inode);
+			info->cookie = inode_query_iversion(inode);
 			ret = ext4_htree_fill_tree(file, info->curr_hash,
 						   info->curr_minor_hash,
 						   &info->next_hash);
@@ -664,7 +658,19 @@  int ext4_check_all_de(struct inode *dir, struct buffer_head *bh, void *buf,
 	return 0;
 }
 
+static int ext4_dir_open(struct inode *inode, struct file *file)
+{
+	struct dir_private_info *info;
+
+	info = kzalloc(sizeof(*info), GFP_KERNEL);
+	if (!info)
+		return -ENOMEM;
+	file->private_data = info;
+	return 0;
+}
+
 const struct file_operations ext4_dir_operations = {
+	.open		= ext4_dir_open,
 	.llseek		= ext4_dir_llseek,
 	.read		= generic_read_dir,
 	.iterate_shared	= ext4_readdir,
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 08acd152261e..d62a4b9b26ce 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2553,6 +2553,8 @@  struct dir_private_info {
 	__u32		curr_hash;
 	__u32		curr_minor_hash;
 	__u32		next_hash;
+	u64		cookie;
+	bool		initialized;
 };
 
 /* calculate the first block number of the group */
diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
index e7a09a99837b..4282e12dc405 100644
--- a/fs/ext4/inline.c
+++ b/fs/ext4/inline.c
@@ -1460,6 +1460,7 @@  int ext4_read_inline_dir(struct file *file,
 	struct ext4_iloc iloc;
 	void *dir_buf = NULL;
 	int dotdot_offset, dotdot_size, extra_offset, extra_size;
+	struct dir_private_info *info = file->private_data;
 
 	ret = ext4_get_inode_loc(inode, &iloc);
 	if (ret)
@@ -1503,12 +1504,12 @@  int ext4_read_inline_dir(struct file *file,
 	extra_size = extra_offset + inline_size;
 
 	/*
-	 * If the version has changed since the last call to
+	 * If the cookie has changed since the last call to
 	 * readdir(2), then we might be pointing to an invalid
 	 * dirent right now.  Scan from the start of the inline
 	 * dir to make sure.
 	 */
-	if (!inode_eq_iversion(inode, file->f_version)) {
+	if (!inode_eq_iversion(inode, info->cookie)) {
 		for (i = 0; i < extra_size && i < offset;) {
 			/*
 			 * "." is with offset 0 and
@@ -1540,7 +1541,7 @@  int ext4_read_inline_dir(struct file *file,
 		}
 		offset = i;
 		ctx->pos = offset;
-		file->f_version = inode_query_iversion(inode);
+		info->cookie = inode_query_iversion(inode);
 	}
 
 	while (ctx->pos < extra_size) {