On Mon, Jul 12, 2021 at 12:01:52PM -0700, Linus Torvalds wrote: > On Mon, Jul 12, 2021 at 5:41 AM Dmitry Kadashev <dkadashev@xxxxxxxxx> wrote: > > > > Since this is on top of the stuff that is going to be in the Jens' tree > > only until the 5.15 merge window, I'm assuming this series should go > > there as well. > > Yeah. Unless Al wants to pick this whole series up. > > See my comments about the individual patches - some of them change > code flow, others do. And I think changing code flow as part of > cleanup is ok, but it at the very least needs to be mentioned (and it > might be good to do the "move code that is idempotent inside the > retry" as a separate patch from documentation purposes) TBH, my main problem with this is that ESTALE retry logics had never felt right. We ask e.g. filename_create() to get us the parent. We tell it whether we want it to be maximally suspicious or not. It still does the same RCU-normal-LOOKUP_REVAL sequence, only for "trust no one" variant it's RCU-LOOKUP_REVAL-LOOKUP_REVAL instead. We are *not* told how far in that sequence did it have to get. What's more, even if we had to get all way up to LOOKUP_REVAL, we ignore that when we do dcache lookup for the last component - only the argument of filename_create() is looked at. It really smells like the calling conventions are wrong. I agree that all of that is, by definition, a very slow path - it's just that the logics makes me go "WTF?" every time I see it... ;-/ Hell knows - perhaps the lookup_flags thing wants to be passed by reference (all the way into path_parentat()) and have the "we had to go for LOOKUP_REVAL" returned that way. Not sure... Al, still crawling out of the bloody ptrace/asm glue horrors at the moment...
