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@xxxxxxxxxx> > > > > 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
