Unverified Commit 24a988f7 authored by Christian Brauner's avatar Christian Brauner
Browse files

Merge patch series "file: remove f_version" 

Christian Brauner <brauner@kernel.org> says:

The f_version member in struct file isn't particularly well-defined. It
is mainly used as a cookie to detect concurrent seeks when iterating
directories. But it is also abused by some subsystems for completely
unrelated things.

It is mostly a directory specific thing that doesn't really need to live
in struct file and with its wonky semantics it really lacks a specific
function.

For pipes, f_version is (ab)used to defer poll notifications until a
write has happened. And struct pipe_inode_info is used by multiple
struct files in their ->private_data so there's no chance of pushing
that down into file->private_data without introducing another pointer
indirection.

But this should be a solvable problem. Only regular files with
FMODE_ATOMIC_POS and directories require f_pos_lock. Pipes and other
files don't. So this adds a union into struct file encompassing
f_pos_lock and a pipe specific f_pipe member that pipes can use. This
union of course can be extended to other file types and is similar to
what we do in struct inode already.

* patches from https://lore.kernel.org/r/20240830-vfs-file-f_version-v1-0-6d3e4816aa7b@kernel.org:
  fs: remove f_version
  pipe: use f_pipe
  fs: add f_pipe
  ubifs: store cookie in private data
  ufs: store cookie in private data
  udf: store cookie in private data
  proc: store cookie in private data
  ocfs2: store cookie in private data
  input: remove f_version abuse
  ext4: store cookie in private data
  ext2: store cookie in private data
  affs: store cookie in private data
  fs: add generic_llseek_cookie()
  fs: use must_set_pos()
  fs: add must_set_pos()
  fs: add vfs_setpos_cookie()
  s390: remove unused f_version
  ceph: remove unused f_version
  adi: remove unused f_version
  file: remove pointless comment

Link: https://lore.kernel.org/r/20240830-vfs-file-f_version-v1-0-6d3e4816aa7b@kernel.org


Signed-off-by: default avatarChristian Brauner <brauner@kernel.org>
parents 0f389adb 11068e0b

drivers/char/adi.c

+0 −1
Original line number Diff line number Diff line
@@ -196,7 +196,6 @@ static loff_t adi_llseek(struct file *file, loff_t offset, int whence) 


	if (offset != file->f_pos) {

		file->f_pos = offset;

		file->f_version = 0;

		ret = offset;

	}

drivers/input/input.c

+22 −25
Original line number Diff line number Diff line
@@ -1079,33 +1079,31 @@ static inline void input_wakeup_procfs_readers(void) 
	wake_up(&input_devices_poll_wait);

}



struct input_seq_state {

	unsigned short pos;

	bool mutex_acquired;

	int input_devices_state;

};



static __poll_t input_proc_devices_poll(struct file *file, poll_table *wait)

{

	struct seq_file *seq = file->private_data;

	struct input_seq_state *state = seq->private;



	poll_wait(file, &input_devices_poll_wait, wait);

	if (file->f_version != input_devices_state) {

		file->f_version = input_devices_state;

	if (state->input_devices_state != input_devices_state) {

		state->input_devices_state = input_devices_state;

		return EPOLLIN | EPOLLRDNORM;

	}



	return 0;

}



union input_seq_state {

	struct {

		unsigned short pos;

		bool mutex_acquired;

	};

	void *p;

};



static void *input_devices_seq_start(struct seq_file *seq, loff_t *pos)

{

	union input_seq_state *state = (union input_seq_state *)&seq->private;

	struct input_seq_state *state = seq->private;

	int error;



	/* We need to fit into seq->private pointer */

	BUILD_BUG_ON(sizeof(union input_seq_state) != sizeof(seq->private));



	error = mutex_lock_interruptible(&input_mutex);

	if (error) {

		state->mutex_acquired = false;
@@ -1124,7 +1122,7 @@ static void *input_devices_seq_next(struct seq_file *seq, void *v, loff_t *pos) 


static void input_seq_stop(struct seq_file *seq, void *v)

{

	union input_seq_state *state = (union input_seq_state *)&seq->private;

	struct input_seq_state *state = seq->private;



	if (state->mutex_acquired)

		mutex_unlock(&input_mutex);
@@ -1210,7 +1208,8 @@ static const struct seq_operations input_devices_seq_ops = { 


static int input_proc_devices_open(struct inode *inode, struct file *file)

{

	return seq_open(file, &input_devices_seq_ops);

	return seq_open_private(file, &input_devices_seq_ops,

				sizeof(struct input_seq_state));

}



static const struct proc_ops input_devices_proc_ops = {
@@ -1218,17 +1217,14 @@ static const struct proc_ops input_devices_proc_ops = { 
	.proc_poll	= input_proc_devices_poll,

	.proc_read	= seq_read,

	.proc_lseek	= seq_lseek,

	.proc_release	= seq_release,

	.proc_release	= seq_release_private,

};



static void *input_handlers_seq_start(struct seq_file *seq, loff_t *pos)

{

	union input_seq_state *state = (union input_seq_state *)&seq->private;

	struct input_seq_state *state = seq->private;

	int error;



	/* We need to fit into seq->private pointer */

	BUILD_BUG_ON(sizeof(union input_seq_state) != sizeof(seq->private));



	error = mutex_lock_interruptible(&input_mutex);

	if (error) {

		state->mutex_acquired = false;
@@ -1243,7 +1239,7 @@ static void *input_handlers_seq_start(struct seq_file *seq, loff_t *pos) 


static void *input_handlers_seq_next(struct seq_file *seq, void *v, loff_t *pos)

{

	union input_seq_state *state = (union input_seq_state *)&seq->private;

	struct input_seq_state *state = seq->private;



	state->pos = *pos + 1;

	return seq_list_next(v, &input_handler_list, pos);
@@ -1252,7 +1248,7 @@ static void *input_handlers_seq_next(struct seq_file *seq, void *v, loff_t *pos) 
static int input_handlers_seq_show(struct seq_file *seq, void *v)

{

	struct input_handler *handler = container_of(v, struct input_handler, node);

	union input_seq_state *state = (union input_seq_state *)&seq->private;

	struct input_seq_state *state = seq->private;



	seq_printf(seq, "N: Number=%u Name=%s", state->pos, handler->name);

	if (handler->filter)
@@ -1273,14 +1269,15 @@ static const struct seq_operations input_handlers_seq_ops = { 


static int input_proc_handlers_open(struct inode *inode, struct file *file)

{

	return seq_open(file, &input_handlers_seq_ops);

	return seq_open_private(file, &input_handlers_seq_ops,

				sizeof(struct input_seq_state));

}



static const struct proc_ops input_handlers_proc_ops = {

	.proc_open	= input_proc_handlers_open,

	.proc_read	= seq_read,

	.proc_lseek	= seq_lseek,

	.proc_release	= seq_release,

	.proc_release	= seq_release_private,

};



static int __init input_proc_init(void)

drivers/s390/char/hmcdrv_dev.c

+0 −3
Original line number Diff line number Diff line
@@ -186,9 +186,6 @@ static loff_t hmcdrv_dev_seek(struct file *fp, loff_t pos, int whence) 
	if (pos < 0)

		return -EINVAL;



	if (fp->f_pos != pos)

		++fp->f_version;



	fp->f_pos = pos;

	return pos;

}

fs/affs/dir.c

+38 −6
Original line number Diff line number Diff line
@@ -17,13 +17,44 @@ 
#include <linux/iversion.h>

#include "affs.h"



struct affs_dir_data {

	unsigned long ino;

	u64 cookie;

};



static int affs_readdir(struct file *, struct dir_context *);



static loff_t affs_dir_llseek(struct file *file, loff_t offset, int whence)

{

	struct affs_dir_data *data = file->private_data;



	return generic_llseek_cookie(file, offset, whence, &data->cookie);

}



static int affs_dir_open(struct inode *inode, struct file *file)

{

	struct affs_dir_data	*data;



	data = kzalloc(sizeof(struct affs_dir_data), GFP_KERNEL);

	if (!data)

		return -ENOMEM;

	file->private_data = data;

	return 0;

}



static int affs_dir_release(struct inode *inode, struct file *file)

{

	kfree(file->private_data);

	return 0;

}



const struct file_operations affs_dir_operations = {

	.open		= affs_dir_open,

	.read		= generic_read_dir,

	.llseek		= generic_file_llseek,

	.llseek		= affs_dir_llseek,

	.iterate_shared	= affs_readdir,

	.fsync		= affs_file_fsync,

	.release	= affs_dir_release,

};



/*
@@ -45,6 +76,7 @@ static int 
affs_readdir(struct file *file, struct dir_context *ctx)

{

	struct inode		*inode = file_inode(file);

	struct affs_dir_data	*data = file->private_data;

	struct super_block	*sb = inode->i_sb;

	struct buffer_head	*dir_bh = NULL;

	struct buffer_head	*fh_bh = NULL;
@@ -59,7 +91,7 @@ affs_readdir(struct file *file, struct dir_context *ctx) 
	pr_debug("%s(ino=%lu,f_pos=%llx)\n", __func__, inode->i_ino, ctx->pos);



	if (ctx->pos < 2) {

		file->private_data = (void *)0;

		data->ino = 0;

		if (!dir_emit_dots(file, ctx))

			return 0;

	}
@@ -80,8 +112,8 @@ affs_readdir(struct file *file, struct dir_context *ctx) 
	/* If the directory hasn't changed since the last call to readdir(),

	 * we can jump directly to where we left off.

	 */

	ino = (u32)(long)file->private_data;

	if (ino && inode_eq_iversion(inode, file->f_version)) {

	ino = data->ino;

	if (ino && inode_eq_iversion(inode, data->cookie)) {

		pr_debug("readdir() left off=%d\n", ino);

		goto inside;

	}
@@ -131,8 +163,8 @@ affs_readdir(struct file *file, struct dir_context *ctx) 
		} while (ino);

	}

done:

	file->f_version = inode_query_iversion(inode);

	file->private_data = (void *)(long)ino;

	data->cookie = inode_query_iversion(inode);

	data->ino = ino;

	affs_brelse(fh_bh);



out_brelse_dir:

fs/ceph/dir.c

+0 −1
Original line number Diff line number Diff line
@@ -707,7 +707,6 @@ static loff_t ceph_dir_llseek(struct file *file, loff_t offset, int whence) 


		if (offset != file->f_pos) {

			file->f_pos = offset;

			file->f_version = 0;

			dfi->file_info.flags &= ~CEPH_F_ATEND;

		}

		retval = offset;