Skip to content

Commit 7979642

Browse files
committed
file: reinstate f_pos locking optimization for regular files
In commit 20ea1e7 ("file: always lock position for FMODE_ATOMIC_POS") we ended up always taking the file pos lock, because pidfd_getfd() could get a reference to the file even when it didn't have an elevated file count due to threading of other sharing cases. But Mateusz Guzik reports that the extra locking is actually measurable, so let's re-introduce the optimization, and only force the locking for directory traversal. Directories need the lock for correctness reasons, while regular files only need it for "POSIX semantics". Since pidfd_getfd() is about debuggers etc special things that are _way_ outside of POSIX, we can relax the rules for that case. Reported-by: Mateusz Guzik <mjguzik@gmail.com> Cc: Christian Brauner <brauner@kernel.org> Link: https://lore.kernel.org/linux-fsdevel/20230803095311.ijpvhx3fyrbkasul@f/ Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
1 parent c1a515d commit 7979642

File tree

1 file changed

+17
-1
lines changed

1 file changed

+17
-1
lines changed

fs/file.c

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1036,12 +1036,28 @@ unsigned long __fdget_raw(unsigned int fd)
10361036
return __fget_light(fd, 0);
10371037
}
10381038

1039+
/*
1040+
* Try to avoid f_pos locking. We only need it if the
1041+
* file is marked for FMODE_ATOMIC_POS, and it can be
1042+
* accessed multiple ways.
1043+
*
1044+
* Always do it for directories, because pidfd_getfd()
1045+
* can make a file accessible even if it otherwise would
1046+
* not be, and for directories this is a correctness
1047+
* issue, not a "POSIX requirement".
1048+
*/
1049+
static inline bool file_needs_f_pos_lock(struct file *file)
1050+
{
1051+
return (file->f_mode & FMODE_ATOMIC_POS) &&
1052+
(file_count(file) > 1 || S_ISDIR(file_inode(file)->i_mode));
1053+
}
1054+
10391055
unsigned long __fdget_pos(unsigned int fd)
10401056
{
10411057
unsigned long v = __fdget(fd);
10421058
struct file *file = (struct file *)(v & ~3);
10431059

1044-
if (file && (file->f_mode & FMODE_ATOMIC_POS)) {
1060+
if (file && file_needs_f_pos_lock(file)) {
10451061
v |= FDPUT_POS_UNLOCK;
10461062
mutex_lock(&file->f_pos_lock);
10471063
}

0 commit comments

Comments
 (0)