Message-ID: <ZOkw6KkdP1UWPNBW@dread.disaster.area>
Date:   Sat, 26 Aug 2023 08:53:28 +1000
From:   Dave Chinner <david@...morbit.com>
To:     Hao Xu <hao.xu@...ux.dev>
Cc:     io-uring@...r.kernel.org, Jens Axboe <axboe@...nel.dk>,
        Dominique Martinet <asmadeus@...ewreck.org>,
        Pavel Begunkov <asml.silence@...il.com>,
        Christian Brauner <brauner@...nel.org>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Stefan Roesch <shr@...com>, Clay Harris <bugs@...ycon.org>,
        "Darrick J . Wong" <djwong@...nel.org>,
        linux-fsdevel@...r.kernel.org, linux-xfs@...r.kernel.org,
        linux-ext4@...r.kernel.org, linux-cachefs@...hat.com,
        ecryptfs@...r.kernel.org, linux-nfs@...r.kernel.org,
        linux-unionfs@...r.kernel.org, bpf@...r.kernel.org,
        netdev@...r.kernel.org, linux-s390@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-block@...r.kernel.org,
        linux-btrfs@...r.kernel.org, codalist@...a.cs.cmu.edu,
        linux-f2fs-devel@...ts.sourceforge.net, cluster-devel@...hat.com,
        linux-mm@...ck.org, linux-nilfs@...r.kernel.org,
        devel@...ts.orangefs.org, linux-cifs@...r.kernel.org,
        samba-technical@...ts.samba.org, linux-mtd@...ts.infradead.org,
        Wanpeng Li <wanpengli@...cent.com>
Subject: Re: [PATCH RFC v5 00/29] io_uring getdents

On Fri, Aug 25, 2023 at 09:54:02PM +0800, Hao Xu wrote:
> From: Hao Xu <howeyxu@...cent.com>
> 
> This series introduce getdents64 to io_uring, the code logic is similar
> with the snychronized version's. It first try nowait issue, and offload
> it to io-wq threads if the first try fails.
> 
> Patch1 and Patch2 are some preparation
> Patch3 supports nowait for xfs getdents code
> Patch4-11 are vfs change, include adding helpers and trylock for locks
> Patch12-29 supports nowait for involved xfs journal stuff
> note, Patch24 and 27 are actually two questions, might be removed later.
> an xfs test may come later.

You need to drop all the XFS journal stuff. It's fundamentally
broken as it stands, and we cannot support non-blocking
transactional changes without first putting a massive investment in
transaction and intent chain rollback to allow correctly undoing
partially complete modifications.

Regardless, non-blocking transactions are completely unnecessary for
a non-blocking readdir implementation. readdir should only be
touching atime, and with relatime it should only occur once every 24
hours per inode. If that's a problem, then we have noatime mount
options. Hence I just don't see any point in worrying about having a
timestamp update block occasionally...

I also don't really don't see why you need to fiddle with xfs buffer
cache semantics - it already has the functionality "nowait" buffer
reads require (i.e.  XBF_INCORE|XBF_TRYLOCK).

However, the readahead IO that the xfs readdir code issues cannot
use your defined NOWAIT semantics - it must be able to allocate
memory and issue IO. Readahead already avoids blocking on memory
allocation and blocking on IO via the XBF_READ_AHEAD flag. This sets
__GFP_NORETRY for buffer allocation and REQ_RAHEAD for IO. Hence
readahead only needs the existing XBF_TRYLOCK flag to be set to be
compatible with the required NOWAIT semantics....

As for the NOIO memory allocation restrictions io_uring requires,
that should be enforced at the io_uring layer before calling into
the VFS using memalloc_noio_save/restore.  At that point no memory
allocation will trigger IO and none of the code running under NOWAIT
conditions even needs to be aware that io_uring has a GFP_NOIO
restriction on memory allocation....

Please go back to the simple "do non-blocking buffer IO"
implementation we started with and don't try to solve every little
blocking problem that might exist in the VFS and filesystems...

-Dave
-- 
Dave Chinner
david@...morbit.com

Powered by blists - more mailing lists