public inbox for [email protected]
 help / color / mirror / Atom feed
From: Christian Brauner <[email protected]>
To: Hao Xu <[email protected]>
Cc: [email protected], Jens Axboe <[email protected]>,
	Dominique Martinet <[email protected]>,
	Pavel Begunkov <[email protected]>,
	Alexander Viro <[email protected]>,
	Stefan Roesch <[email protected]>, Clay Harris <[email protected]>,
	Dave Chinner <[email protected]>,
	[email protected], Wanpeng Li <[email protected]>
Subject: Re: [PATCH RFC 5/5] disable fixed file for io_uring getdents for now
Date: Wed, 26 Jul 2023 16:23:28 +0200	[thread overview]
Message-ID: <20230726-inhaftiert-hinunter-714e140c957c@brauner> (raw)
In-Reply-To: <[email protected]>

On Tue, Jul 18, 2023 at 09:21:12PM +0800, Hao Xu wrote:
> From: Hao Xu <[email protected]>
> 
> Fixed file for io_uring getdents can trigger race problem. Users can
> register a file to be fixed file in io_uring and then remove other
> reference so that there are only fixed file reference of that file.
> And then they can issue concurrent async getdents requests or both
> async and sync getdents requests without holding the f_pos_lock
> since there is a f_count == 1 optimization.

Afaict, that limitation isn't necessary. This version ow works fine with
fixed files.

Based on the commit message there seems to be a misunderstanding.
Your previous version of the patchset copied the f_count optimization
into io_uring's locking which would've caused the race I described
in the other thread.

There regular system call interface was always safe because as long as
the original fd is kept the file count will be greater than 1 and both
the fixed file and regular system call interface will acquire the lock.

So fixed file's not being usable was entirely causes by copying the
f_count optimization into io_uring. Since this patchset now doesn't use
that optimization and unconditionally locks things are fine. (And even
if, the point is now moot anyway since we dropped that optimization from
the regular system call path anyway because of another issue.)

  reply	other threads:[~2023-07-26 14:23 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-18 13:21 [PATCH v4 0/5] io_uring getdents Hao Xu
2023-07-18 13:21 ` [PATCH 1/5] fs: split off vfs_getdents function of getdents64 syscall Hao Xu
2023-07-18 13:21 ` [PATCH 2/5] vfs_getdents/struct dir_context: add flags field Hao Xu
2023-07-18 13:21 ` [PATCH 3/5] io_uring: add support for getdents Hao Xu
2023-07-19  8:56   ` Hao Xu
2023-07-26 15:00   ` Christian Brauner
2023-07-27 11:51     ` Hao Xu
2023-07-27 14:27       ` Christian Brauner
2023-07-27 15:12         ` Pavel Begunkov
2023-07-27 15:52           ` Christian Brauner
2023-07-27 16:17             ` Pavel Begunkov
2023-07-27 16:28               ` Christian Brauner
2023-07-31  1:58                 ` Dave Chinner
2023-07-31  7:34                   ` Hao Xu
2023-07-31  7:50                     ` Christian Brauner
2023-07-31  7:40                   ` Christian Brauner
2023-07-30 18:02         ` Hao Xu
2023-07-31  8:18           ` Christian Brauner
2023-07-31  9:31             ` Hao Xu
2023-07-31  1:33         ` Dave Chinner
2023-07-31  8:13           ` Christian Brauner
2023-07-31 15:26             ` Darrick J. Wong
2023-07-31 22:18               ` Dave Chinner
2023-08-01  0:28               ` Jens Axboe
2023-08-01  0:47                 ` Matthew Wilcox
2023-08-01  0:49                   ` Jens Axboe
2023-08-01  1:01                     ` Matthew Wilcox
2023-08-01  7:00                       ` Christian Brauner
2023-08-01  6:59                     ` Christian Brauner
2023-08-01  7:17                 ` Christian Brauner
2023-08-08  4:34                 ` Hao Xu
2023-08-08  5:18                   ` Hao Xu
2023-08-08  9:33                 ` Hao Xu
2023-08-08 22:55                   ` Jens Axboe
2023-08-01 18:39             ` Hao Xu
2023-07-18 13:21 ` [PATCH 4/5] xfs: add NOWAIT semantics for readdir Hao Xu
2023-07-19  2:35   ` kernel test robot
2023-07-18 13:21 ` [PATCH RFC 5/5] disable fixed file for io_uring getdents for now Hao Xu
2023-07-26 14:23   ` Christian Brauner [this message]
2023-07-27 12:09     ` Hao Xu
2023-07-19  6:04 ` [PATCH v4 0/5] io_uring getdents Christian Brauner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230726-inhaftiert-hinunter-714e140c957c@brauner \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox