linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jens Axboe <axboe@kernel.dk>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	io-uring <io-uring@vger.kernel.org>,
	Al Viro <viro@zeniv.linux.org.uk>
Subject: Re: [PATCHSET RFC] support RESOLVE_CACHED for statx
Date: Mon, 25 Jan 2021 18:06:37 -0700	[thread overview]
Message-ID: <4bd713e8-58e7-e961-243e-dbbdc2a1f60c@kernel.dk> (raw)
In-Reply-To: <CAHk-=whh=+nkoZFqb1zztY9kUo-Ua75+zY16HeU_3j1RV4JR0Q@mail.gmail.com>

On 1/25/21 4:39 PM, Linus Torvalds wrote:
> On Mon, Jan 25, 2021 at 1:36 PM Jens Axboe <axboe@kernel.dk> wrote:
>>
>>     Patch 2 is the
>> mostly ugly part, but not sure how we can do this any better - we need
>> to ensure that any sort of revalidation or sync in ->getattr() honors
>> it too.
> 
> Yeah, that's not pretty, but I agree - it looks like this just
> requires the filesystem to check whether it needs to revalidate or
> not.
> 
> But I think that patch could do better than what your patch does. Some
> of them are "filesystems could decide to be more finegrained") -  your
> cifs patch comes to mind - but some of your "return -EAGAIN if cached"
> seem to be just plain pointless.

Which ones in particular? Outside of the afs one you looked a below,
the rest should all be of the "need to do IO of some sort" and hence
-EAGAIN is reasonable.

cifs could be cleaner, but that'd require more checking in there. I
just tried to keep it simple, and leave the harder work for the
file system developers if they care. If not, it'll still work just
like it does today, we're no worse off there than before (at least
from an io_uring POV).

But I can go ahead and makes eg cifs more accurate in that regard,
if that's what you're objecting to.

> In afs, for example, you return -EAGAIN instead of just doing the
> read-seqlock thing. That's a really cheap CPU-only operation. We're
> talking "cheaper than a spinlock" sequence.

Yep agree on that one, that looks silly and should just go away. I've
killed it.

-- 
Jens Axboe


  reply	other threads:[~2021-01-26  1:45 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-25 21:36 [PATCHSET RFC] support RESOLVE_CACHED for statx Jens Axboe
2021-01-25 21:36 ` [PATCH 1/3] fs: add support for AT_STATX_CACHED Jens Axboe
2021-01-25 21:36 ` [PATCH 2/3] fs: ensure that ->getattr() honors AT_STATX_CACHED Jens Axboe
2021-01-25 21:36 ` [PATCH 3/3] io_uring: use AT_STATX_CACHED for IORING_OP_STATX fast path Jens Axboe
2021-01-25 23:39 ` [PATCHSET RFC] support RESOLVE_CACHED for statx Linus Torvalds
2021-01-26  1:06   ` Jens Axboe [this message]
2021-01-26  1:28     ` Linus Torvalds
2021-01-26  1:31       ` Jens Axboe

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=4bd713e8-58e7-e961-243e-dbbdc2a1f60c@kernel.dk \
    --to=axboe@kernel.dk \
    --cc=io-uring@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    /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).