On Sat, Oct 19, 2024 at 06:03:22AM +0100, Al Viro wrote: > On Fri, Oct 18, 2024 at 08:38:22PM +0100, Al Viro wrote: > > On Fri, Oct 18, 2024 at 05:51:58PM +0100, Al Viro wrote: > > > > > Extra cycles where? If anything, I'd expect a too-small-to-measure > > > speedup due to dereference shifted from path_init() to __set_nameidata(). > > > Below is literally all it takes to make filename_lookup() treat NULL > > > as empty-string name. > > > > > > NOTE: I'm not talking about forcing the pure by-descriptor case through > > > the dfd+pathname codepath; not without serious profiling. But treating > > > AT_FDCWD + NULL by the delta below and passing NULL struct filename to > > > filename_lookup()? Where do you expect to have the lost cycles on that? > > > > [snip] > > > > BTW, could you give me a reference to the mail with those objections? > > I don't see anything in my mailbox, but... > > > > Or was that in one of those AT_EMPTY_PATH_NOCHECK (IIRC?) threads? > > > > Anyway, what I'm suggesting is > > > > 1) teach filename_lookup() to handle NULL struct filename * argument, treating > > it as "". Trivial and does not impose any overhead on the normal cases. > > > > 2) have statx try to recognize AT_EMPTY_PATH, "" and AT_EMPTY_PATH, NULL. > > If we have that and dfd is *NOT* AT_FDCWD, we have a nice descriptor-based > > case and can deal with it. > > If the name is not empty, we have to go for dfd+filename path. Also obvious. > > Where we get trouble is AT_FDCWD, NULL case. But with (1) we can simply > > route that to the same dfd+filename path, just passing it NULL for filename. > > > > That handles the currently broken case, with very little disruption to > > anything else. > > See #getname.fixup; on top of #base.getname and IMO worth folding into it. Yes, please fold so I can rebase my series on top of it. > I don't believe that it's going to give any measurable slowdown compared to > mainline. Again, if the concerns about wasted cycles had been about routing > the dfd,"" and dfd,NULL cases through the filename_lookup(), this does *NOT* > happen with that patch. Christian, Linus? This looks good! Reviewed-by: Christian Brauner <brauner@xxxxxxxxxx>
