Re: [PATCH] file: always lock position

[Christian Brauner <brauner@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 701 bytes --]

> > thing to do is to just special-case S_ISDIR. Not lovely, but whatever.
> > 
> > So something like this instead? It's a smaller diff anyway, and it
> > gets the crazy afds/ceph cases right too.
> 
> If you really care about this we can do it. But if we can live with just

I see you went with the S_ISDIR thing for now. How do you feel about
adding something like the appended patch (untested) on top of this?

So instead of relying on the inode we could just check f_ops for
iterate/iterate_shared. That should amount to the same thing(*) but
looks cleaner to me. Alternatively we can do the flag thing you
mentioned ofc.

(*) I suffered from a proper cold so my brain is in a half-working state.

[-- Attachment #2: dir.diff --]
[-- Type: text/x-diff, Size: 650 bytes --]

diff --git a/fs/file.c b/fs/file.c
index dbca26ef7a01..dd49bc77d5cf 100644
--- a/fs/file.c
+++ b/fs/file.c
@@ -1046,10 +1046,11 @@ unsigned long __fdget_raw(unsigned int fd)
  * not be, and for directories this is a correctness
  * issue, not a "POSIX requirement".
  */
-static inline bool file_needs_f_pos_lock(struct file *file)
+static inline bool file_needs_f_pos_lock(const struct file *file)
 {
 	return (file->f_mode & FMODE_ATOMIC_POS) &&
-		(file_count(file) > 1 || S_ISDIR(file_inode(file)->i_mode));
+	       (file_count(file) > 1 ||
+		file->f_op->iterate_shared || file->f_op->iterate);
 }
 
 unsigned long __fdget_pos(unsigned int fd)