On Thu, 10 Jul 2025, Song Liu wrote: > > > > On Jul 9, 2025, at 3:24 PM, NeilBrown <neil@xxxxxxxxxx> wrote: > [...] > >> > >> How should the user handle -ECHILD without LOOKUP_RCU flag? Say the > >> following code in landlocked: > >> > >> /* Try RCU walk first */ > >> err = vfs_walk_ancestors(path, ll_cb, data, LOOKUP_RCU); > >> > >> if (err == -ECHILD) { > >> struct path walk_path = *path; > >> > >> /* reset any data changed by the walk */ > >> reset_data(data); > >> > >> /* now do ref-walk */ > >> err = vfs_walk_ancestors(&walk_path, ll_cb, data, 0); > >> } > >> > >> Or do you mean vfs_walk_ancestors will never return -ECHILD? > >> Then we need vfs_walk_ancestors to call reset_data logic, right? > > > > It isn't clear to me that vfs_walk_ancestors() needs to return anything. > > All the communication happens through walk_cb() > > > > walk_cb() is called with a path, the data, and a "may_sleep" flag. > > If it needs to sleep but may_sleep is not set, it returns "-ECHILD" > > which causes the walk to restart and use refcounts. > > If it wants to stop, it returns 0. > > If it wants to continue, it returns 1. > > If it wants a reference to the path then it can use (new) > > vfs_legitimize_path() which might fail. > > If it wants a reference to the path and may_sleep is true, it can use > > path_get() which won't fail. > > > > When returning -ECHILD (either because of a need to sleep or because > > vfs_legitimize_path() fails), walk_cb() would reset_data(). > > This might actually work. > > My only concern is with vfs_legitimize_path. It is probably safer if > we only allow taking references with may_sleep==true, so that path_get > won’t fail. In this case, we will not need walk_cb() to call > vfs_legitimize_path. If the user want a reference, the walk_cb will > first return -ECHILD, and call path_get when may_sleep is true. What is your concern with vfs_legitimize_path() ?? I've since realised that always restarting in response to -ECHILD isn't necessary and isn't how normal path-walk works. Restarting might be needed, but the first response to -ECHILD is to try legitimize_path(). If that succeeds, then it is safe to sleep. So returning -ECHILD might just result in vfs_walk_ancestors() calling legitimize_path() and then calling walk_cb() again. Why not have walk_cb() do the vfs_legitimize_path() call (which will almost always succeed in practice). NeilBrown > > Does this make sense? Did I miss any cases? > > Thanks, > Song > >