From: Hao Xu <hao.xu@linux.dev>
To: Christian Brauner <brauner@kernel.org>
Cc: io-uring@vger.kernel.org, Jens Axboe <axboe@kernel.dk>,
Dominique Martinet <asmadeus@codewreck.org>,
Pavel Begunkov <asml.silence@gmail.com>,
Alexander Viro <viro@zeniv.linux.org.uk>,
Stefan Roesch <shr@fb.com>, Clay Harris <bugs@claycon.org>,
Dave Chinner <david@fromorbit.com>,
linux-fsdevel@vger.kernel.org, Wanpeng Li <wanpengli@tencent.com>
Subject: Re: [PATCH RFC 5/5] disable fixed file for io_uring getdents for now
Date: Thu, 27 Jul 2023 20:09:48 +0800 [thread overview]
Message-ID: <e8567efc-41e8-34bc-3f15-8ccab18fe5e1@linux.dev> (raw)
In-Reply-To: <20230726-inhaftiert-hinunter-714e140c957c@brauner>
On 7/26/23 22:23, Christian Brauner wrote:
> On Tue, Jul 18, 2023 at 09:21:12PM +0800, Hao Xu wrote:
>> From: Hao Xu <howeyxu@tencent.com>
>>
>> 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.)
I see, forgot we already remove it from fdtable after close(fd) thus
cannot access it by fd any more. I'll fix it in next version. Thanks.
Regards,
Hao
next prev parent reply other threads:[~2023-07-27 12:10 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
2023-07-27 12:09 ` Hao Xu [this message]
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=e8567efc-41e8-34bc-3f15-8ccab18fe5e1@linux.dev \
--to=hao.xu@linux.dev \
--cc=asmadeus@codewreck.org \
--cc=asml.silence@gmail.com \
--cc=axboe@kernel.dk \
--cc=brauner@kernel.org \
--cc=bugs@claycon.org \
--cc=david@fromorbit.com \
--cc=io-uring@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=shr@fb.com \
--cc=viro@zeniv.linux.org.uk \
--cc=wanpengli@tencent.com \
/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;
as well as URLs for NNTP newsgroup(s).