diff mbox series

[RFC,17/20] ubifs: store cookie in private data

Message ID 20240830-vfs-file-f_version-v1-17-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/ubifs/dir.c | 65 ++++++++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 47 insertions(+), 18 deletions(-)

Comments

Jan Kara Sept. 3, 2024, 1:39 p.m. UTC | #1 
On Fri 30-08-24 15:04:58, 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 to me. Feel free to add:

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

								Honza

> ---
>  fs/ubifs/dir.c | 65 ++++++++++++++++++++++++++++++++++++++++++----------------
>  1 file changed, 47 insertions(+), 18 deletions(-)
> 
> diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
> index c77ea57fe696..76bcafa92200 100644
> --- a/fs/ubifs/dir.c
> +++ b/fs/ubifs/dir.c
> @@ -555,6 +555,11 @@ static unsigned int vfs_dent_type(uint8_t type)
>  	return 0;
>  }
>  
> +struct ubifs_dir_data {
> +	struct ubifs_dent_node *dent;
> +	u64 cookie;
> +};
> +
>  /*
>   * The classical Unix view for directory is that it is a linear array of
>   * (name, inode number) entries. Linux/VFS assumes this model as well.
> @@ -582,6 +587,7 @@ static int ubifs_readdir(struct file *file, struct dir_context *ctx)
>  	struct inode *dir = file_inode(file);
>  	struct ubifs_info *c = dir->i_sb->s_fs_info;
>  	bool encrypted = IS_ENCRYPTED(dir);
> +	struct ubifs_dir_data *data = file->private_data;
>  
>  	dbg_gen("dir ino %lu, f_pos %#llx", dir->i_ino, ctx->pos);
>  
> @@ -604,27 +610,27 @@ static int ubifs_readdir(struct file *file, struct dir_context *ctx)
>  		fstr_real_len = fstr.len;
>  	}
>  
> -	if (file->f_version == 0) {
> +	if (data->cookie == 0) {
>  		/*
> -		 * The file was seek'ed, which means that @file->private_data
> +		 * The file was seek'ed, which means that @data->dent
>  		 * is now invalid. This may also be just the first
>  		 * 'ubifs_readdir()' invocation, in which case
> -		 * @file->private_data is NULL, and the below code is
> +		 * @data->dent is NULL, and the below code is
>  		 * basically a no-op.
>  		 */
> -		kfree(file->private_data);
> -		file->private_data = NULL;
> +		kfree(data->dent);
> +		data->dent = NULL;
>  	}
>  
>  	/*
> -	 * 'generic_file_llseek()' unconditionally sets @file->f_version to
> -	 * zero, and we use this for detecting whether the file was seek'ed.
> +	 * 'ubifs_dir_llseek()' sets @data->cookie to zero, and we use this
> +	 * for detecting whether the file was seek'ed.
>  	 */
> -	file->f_version = 1;
> +	data->cookie = 1;
>  
>  	/* File positions 0 and 1 correspond to "." and ".." */
>  	if (ctx->pos < 2) {
> -		ubifs_assert(c, !file->private_data);
> +		ubifs_assert(c, !data->dent);
>  		if (!dir_emit_dots(file, ctx)) {
>  			if (encrypted)
>  				fscrypt_fname_free_buffer(&fstr);
> @@ -641,10 +647,10 @@ static int ubifs_readdir(struct file *file, struct dir_context *ctx)
>  		}
>  
>  		ctx->pos = key_hash_flash(c, &dent->key);
> -		file->private_data = dent;
> +		data->dent = dent;
>  	}
>  
> -	dent = file->private_data;
> +	dent = data->dent;
>  	if (!dent) {
>  		/*
>  		 * The directory was seek'ed to and is now readdir'ed.
> @@ -658,7 +664,7 @@ static int ubifs_readdir(struct file *file, struct dir_context *ctx)
>  			goto out;
>  		}
>  		ctx->pos = key_hash_flash(c, &dent->key);
> -		file->private_data = dent;
> +		data->dent = dent;
>  	}
>  
>  	while (1) {
> @@ -701,15 +707,15 @@ static int ubifs_readdir(struct file *file, struct dir_context *ctx)
>  			goto out;
>  		}
>  
> -		kfree(file->private_data);
> +		kfree(data->dent);
>  		ctx->pos = key_hash_flash(c, &dent->key);
> -		file->private_data = dent;
> +		data->dent = dent;
>  		cond_resched();
>  	}
>  
>  out:
> -	kfree(file->private_data);
> -	file->private_data = NULL;
> +	kfree(data->dent);
> +	data->dent = NULL;
>  
>  	if (encrypted)
>  		fscrypt_fname_free_buffer(&fstr);
> @@ -733,7 +739,10 @@ static int ubifs_readdir(struct file *file, struct dir_context *ctx)
>  /* Free saved readdir() state when the directory is closed */
>  static int ubifs_dir_release(struct inode *dir, struct file *file)
>  {
> -	kfree(file->private_data);
> +	struct ubifs_dir_data *data = file->private_data;
> +
> +	kfree(data->dent);
> +	kfree(data);
>  	file->private_data = NULL;
>  	return 0;
>  }
> @@ -1712,6 +1721,24 @@ int ubifs_getattr(struct mnt_idmap *idmap, const struct path *path,
>  	return 0;
>  }
>  
> +static int ubifs_dir_open(struct inode *inode, struct file *file)
> +{
> +	struct ubifs_dir_data	*data;
> +
> +	data = kzalloc(sizeof(struct ubifs_dir_data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +	file->private_data = data;
> +	return 0;
> +}
> +
> +static loff_t ubifs_dir_llseek(struct file *file, loff_t offset, int whence)
> +{
> +	struct ubifs_dir_data *data = file->private_data;
> +
> +	return generic_llseek_cookie(file, offset, whence, &data->cookie);
> +}
> +
>  const struct inode_operations ubifs_dir_inode_operations = {
>  	.lookup      = ubifs_lookup,
>  	.create      = ubifs_create,
> @@ -1732,7 +1759,9 @@ const struct inode_operations ubifs_dir_inode_operations = {
>  };
>  
>  const struct file_operations ubifs_dir_operations = {
> -	.llseek         = generic_file_llseek,
> +	.open		= ubifs_dir_open,
> +	.release	= ubifs_dir_release,
> +	.llseek         = ubifs_dir_llseek,
>  	.release        = ubifs_dir_release,
>  	.read           = generic_read_dir,
>  	.iterate_shared = ubifs_readdir,
> 
> -- 
> 2.45.2
>
diff mbox series

Patch

diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index c77ea57fe696..76bcafa92200 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -555,6 +555,11 @@  static unsigned int vfs_dent_type(uint8_t type)
 	return 0;
 }
 
+struct ubifs_dir_data {
+	struct ubifs_dent_node *dent;
+	u64 cookie;
+};
+
 /*
  * The classical Unix view for directory is that it is a linear array of
  * (name, inode number) entries. Linux/VFS assumes this model as well.
@@ -582,6 +587,7 @@  static int ubifs_readdir(struct file *file, struct dir_context *ctx)
 	struct inode *dir = file_inode(file);
 	struct ubifs_info *c = dir->i_sb->s_fs_info;
 	bool encrypted = IS_ENCRYPTED(dir);
+	struct ubifs_dir_data *data = file->private_data;
 
 	dbg_gen("dir ino %lu, f_pos %#llx", dir->i_ino, ctx->pos);
 
@@ -604,27 +610,27 @@  static int ubifs_readdir(struct file *file, struct dir_context *ctx)
 		fstr_real_len = fstr.len;
 	}
 
-	if (file->f_version == 0) {
+	if (data->cookie == 0) {
 		/*
-		 * The file was seek'ed, which means that @file->private_data
+		 * The file was seek'ed, which means that @data->dent
 		 * is now invalid. This may also be just the first
 		 * 'ubifs_readdir()' invocation, in which case
-		 * @file->private_data is NULL, and the below code is
+		 * @data->dent is NULL, and the below code is
 		 * basically a no-op.
 		 */
-		kfree(file->private_data);
-		file->private_data = NULL;
+		kfree(data->dent);
+		data->dent = NULL;
 	}
 
 	/*
-	 * 'generic_file_llseek()' unconditionally sets @file->f_version to
-	 * zero, and we use this for detecting whether the file was seek'ed.
+	 * 'ubifs_dir_llseek()' sets @data->cookie to zero, and we use this
+	 * for detecting whether the file was seek'ed.
 	 */
-	file->f_version = 1;
+	data->cookie = 1;
 
 	/* File positions 0 and 1 correspond to "." and ".." */
 	if (ctx->pos < 2) {
-		ubifs_assert(c, !file->private_data);
+		ubifs_assert(c, !data->dent);
 		if (!dir_emit_dots(file, ctx)) {
 			if (encrypted)
 				fscrypt_fname_free_buffer(&fstr);
@@ -641,10 +647,10 @@  static int ubifs_readdir(struct file *file, struct dir_context *ctx)
 		}
 
 		ctx->pos = key_hash_flash(c, &dent->key);
-		file->private_data = dent;
+		data->dent = dent;
 	}
 
-	dent = file->private_data;
+	dent = data->dent;
 	if (!dent) {
 		/*
 		 * The directory was seek'ed to and is now readdir'ed.
@@ -658,7 +664,7 @@  static int ubifs_readdir(struct file *file, struct dir_context *ctx)
 			goto out;
 		}
 		ctx->pos = key_hash_flash(c, &dent->key);
-		file->private_data = dent;
+		data->dent = dent;
 	}
 
 	while (1) {
@@ -701,15 +707,15 @@  static int ubifs_readdir(struct file *file, struct dir_context *ctx)
 			goto out;
 		}
 
-		kfree(file->private_data);
+		kfree(data->dent);
 		ctx->pos = key_hash_flash(c, &dent->key);
-		file->private_data = dent;
+		data->dent = dent;
 		cond_resched();
 	}
 
 out:
-	kfree(file->private_data);
-	file->private_data = NULL;
+	kfree(data->dent);
+	data->dent = NULL;
 
 	if (encrypted)
 		fscrypt_fname_free_buffer(&fstr);
@@ -733,7 +739,10 @@  static int ubifs_readdir(struct file *file, struct dir_context *ctx)
 /* Free saved readdir() state when the directory is closed */
 static int ubifs_dir_release(struct inode *dir, struct file *file)
 {
-	kfree(file->private_data);
+	struct ubifs_dir_data *data = file->private_data;
+
+	kfree(data->dent);
+	kfree(data);
 	file->private_data = NULL;
 	return 0;
 }
@@ -1712,6 +1721,24 @@  int ubifs_getattr(struct mnt_idmap *idmap, const struct path *path,
 	return 0;
 }
 
+static int ubifs_dir_open(struct inode *inode, struct file *file)
+{
+	struct ubifs_dir_data	*data;
+
+	data = kzalloc(sizeof(struct ubifs_dir_data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+	file->private_data = data;
+	return 0;
+}
+
+static loff_t ubifs_dir_llseek(struct file *file, loff_t offset, int whence)
+{
+	struct ubifs_dir_data *data = file->private_data;
+
+	return generic_llseek_cookie(file, offset, whence, &data->cookie);
+}
+
 const struct inode_operations ubifs_dir_inode_operations = {
 	.lookup      = ubifs_lookup,
 	.create      = ubifs_create,
@@ -1732,7 +1759,9 @@  const struct inode_operations ubifs_dir_inode_operations = {
 };
 
 const struct file_operations ubifs_dir_operations = {
-	.llseek         = generic_file_llseek,
+	.open		= ubifs_dir_open,
+	.release	= ubifs_dir_release,
+	.llseek         = ubifs_dir_llseek,
 	.release        = ubifs_dir_release,
 	.read           = generic_read_dir,
 	.iterate_shared = ubifs_readdir,