From: Brian Foster <bfoster@redhat.com>
To: Al Viro <viro@zeniv.linux.org.uk>
Cc: Ian Kent <raven@themaw.net>, linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH] namei: clear nd->root.mnt before O_CREAT unlazy
Date: Fri, 7 Jan 2022 12:32:17 -0500 [thread overview]
Message-ID: <Ydh5If9ON/fRs7+N@bfoster> (raw)
In-Reply-To: <YdfngxyGWatLfa5h@zeniv-ca.linux.org.uk>
On Fri, Jan 07, 2022 at 07:10:59AM +0000, Al Viro wrote:
> On Fri, Jan 07, 2022 at 05:52:27AM +0000, Al Viro wrote:
>
> > > Looks good, assuming Al is ok with the re-factoring.
> > > Reviewed-by: Ian Kent <raven@themaw.net>
> >
> > Ummm.... Mind resending that? I'm still digging myself from under
> > the huge pile of mail, and this seems to have been lost in process...
>
> Non-obvious part is that current code only does this forgetting
> the root when we are certain that we won't look at it later in
> pathwalk. IOW, it's guaranteed to be the same through the entire
> thing. This patch changes that; the final component may very well
> be e.g. an absolute symlink. We won't know that until we unlazy,
> so we can't make forgetting conditional upon that.
>
> I _think_ it's not going to lead to any problems, but I'll need to
> take a good look at the entire thing after I get some sleep -
> I'm about to fall down right now.
>
Heh, Ok. I think I understand what you're getting at, but I'd have to
stare at the code much more to grok the details. Let me know if you
think the logic and/or commit log needs to change wrt to this and I'll
give it a shot.
> Other problems here (aside of whitespace damage - was that a
> cut'n'paste of some kind? Looks like 8859-1 NBSP for each
> leading space...) are
Hmm.. I don't see any whitespace damage, even if I pull the patch back
from the mailing list into my tree..?
> * misleading name of the new helper - it sounds like
> "non-RCU side of complete_walk()" and that's not what it does
The intent was the opposite, of course. :P I'm not sure how you infer
the above from _rcu(), but I'll name the helper whatever. Suggestions?
> * LOOKUP_CACHED needs to be mentioned in commit message
> (it's incompatible with O_CREAT, so it couldn't occur on that
> codepath - the transformation is an equivalent one, but that
> ought to be mentioned)
Ok. I can add a "for non-create path only" comment to that effect in the
helper as well if useful.
> * the change I mentioned above needs to be in commit
> message - this one is a lot more subtle.
>
Ack.
> Anyway, I'll look into that tomorrow - too sleepy right now
> to do any serious analysis.
>
Ack. Thanks for the first pass.
Brian
next prev parent reply other threads:[~2022-01-07 17:32 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-05 18:02 [PATCH] namei: clear nd->root.mnt before O_CREAT unlazy Brian Foster
2022-01-06 23:46 ` Ian Kent
2022-01-07 5:52 ` Al Viro
2022-01-07 7:04 ` Ian Kent
2022-01-07 7:22 ` Al Viro
2022-01-07 7:10 ` Al Viro
2022-01-07 17:32 ` Brian Foster [this message]
2022-01-07 17:51 ` Al Viro
2022-01-07 18:42 ` Brian Foster
2022-01-08 3:26 ` Ian Kent
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=Ydh5If9ON/fRs7+N@bfoster \
--to=bfoster@redhat.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=raven@themaw.net \
--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).