On 12/15/20 9:08 AM, Jens Axboe wrote: > On 12/15/20 8:37 AM, Jens Axboe wrote: >> On 12/15/20 8:33 AM, Matthew Wilcox wrote: >>> On Tue, Dec 15, 2020 at 08:29:40AM -0700, Jens Axboe wrote: >>>> On 12/15/20 5:24 AM, Matthew Wilcox wrote: >>>>> On Mon, Dec 14, 2020 at 12:13:22PM -0700, Jens Axboe wrote: >>>>>> +++ b/fs/namei.c >>>>>> @@ -686,6 +686,8 @@ static bool try_to_unlazy(struct nameidata *nd) >>>>>> BUG_ON(!(nd->flags & LOOKUP_RCU)); >>>>>> >>>>>> nd->flags &= ~LOOKUP_RCU; >>>>>> + if (nd->flags & LOOKUP_NONBLOCK) >>>>>> + goto out1; >>>>> >>>>> If we try a walk in a non-blocking context, it fails, then we punt to >>>>> a thread, do we want to prohibit that thread trying an RCU walk first? >>>>> I can see arguments both ways -- this may only be a temporary RCU walk >>>>> failure, or we may never be able to RCU walk this path. >>>> >>>> In my opinion, it's not worth it trying to over complicate matters by >>>> handling the retry side differently. Better to just keep them the >>>> same. We'd need a lookup anyway to avoid aliasing. >>> >>> but by clearing LOOKUP_RCU here, aren't you making the retry handle >>> things differently? maybe i got lost. >> >> That's already how it works, I'm just clearing LOOKUP_NONBLOCK (which >> relies on LOOKUP_RCU) when we're clearing LOOKUP_RCU. I can try and >> benchmark skipping LOOKUP_RCU when we do the blocking retry, but my gut >> tells me it'll be noise. > > OK, ran some numbers. The test app benchmarks opening X files, I just > used /usr on my test box. That's 182677 files. To mimic real worldy > kind of setups, 33% of the files can be looked up hot, so LOOKUP_NONBLOCK > will succeed. > > Patchset as posted: > > Method Time (usec) > --------------------------- > openat 2,268,930 > openat 2,274,256 > openat 2,274,256 > io_uring 917,813 > io_uring 921,448 > io_uring 915,233 > > And with a LOOKUP_NO_RCU flag, which io_uring sets when it has to do > retry, and which will make namei skip the first LOOKUP_RCU for path > resolution: > > Method Time (usec) > --------------------------- > io_uring 902,410 > io_uring 902,725 > io_uring 896,289 > > Definitely not faster - whether that's just reboot noise, or if it's > significant, I'd need to look deeper to figure out. If you're puzzled by the conclusion based on the numbers, there's a good reason. The first table is io_uring + LOOKUP_NO_RCU for retry, second table is io_uring as posted. I mistakenly swapped the numbers around... So conclusion still stands, I just pasted in the wrong set for the table. -- Jens Axboe
