On Fri, Apr 26, 2019 at 01:30:53PM -0400, Jeff Layton wrote: > > I _probably_ would take allocation out of the loop (e.g. make it > > __getname(), called unconditionally) and turned it into the > > d_path.c-style read_seqbegin_or_lock()/need_seqretry()/done_seqretry() > > loop, so that the first pass would go under rcu_read_lock(), while > > the second (if needed) would just hold rename_lock exclusive (without > > bumping the refcount). But that's a matter of (theoretical) livelock > > avoidance, not the locking correctness for ->d_name accesses. > > > > Yeah, that does sound better. I want to think about this code a bit FWIW, is there any reason to insist that the pathname is put into the beginning of the buffer? I mean, instead of path + pathlen we might return path + offset, with the pathname going from path + offset to path + PATH_MAX - 1 inclusive, with path being the thing eventually freed. It's easier to build the string backwards, seeing that we are walking from leaf to root...