On Fri, Jun 13, 2025 at 12:11 PM Luis Henriques <luis@xxxxxxxxxx> wrote:
>
> The assert in function file_seek_cur_needs_f_lock() can be triggered very
> easily because there are many users of vfs_llseek() (such as overlayfs)
> that do their custom locking around llseek instead of relying on
> fdget_pos(). Just drop the overzealous assertion.
>
> Fixes: da06e3c51794 ("fs: don't needlessly acquire f_lock")
> Suggested-by: Jan Kara <jack@xxxxxxx>
> Suggested-by: Mateusz Guzik <mjguzik@xxxxxxxxx>
> Signed-off-by: Luis Henriques <luis@xxxxxxxxxx>
> ---
> Hi!
>
> As suggested by Mateusz, I'm adding a comment (also suggested by him!) to
> replace the assertion. I'm also adding the 'Suggested-by:' tags, although
> I'm not sure it's the correct tag to use -- the authorship of this patch
> isn't really clear at this point :-)
>
> fs/file.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/fs/file.c b/fs/file.c
> index 3a3146664cf3..b6db031545e6 100644
> --- a/fs/file.c
> +++ b/fs/file.c
> @@ -1198,8 +1198,12 @@ bool file_seek_cur_needs_f_lock(struct file *file)
> if (!(file->f_mode & FMODE_ATOMIC_POS) && !file->f_op->iterate_shared)
> return false;
>
> - VFS_WARN_ON_ONCE((file_count(file) > 1) &&
> - !mutex_is_locked(&file->f_pos_lock));
> + /*
> + * Note that we are not guaranteed to be called after fdget_pos() on
> + * this file obj, in which case the caller is expected to provide the
> + * appropriate locking.
> + */
> +
> return true;
> }
>
well i think this is fine, obviously ;-)
I was hoping a native speaker would do some touch ups on the comment though.
--
Mateusz Guzik <mjguzik gmail.com>
