On Sun, Jun 8, 2025 at 10:34 AM Tingmao Wang <m@xxxxxxxxxx> wrote:
[...]
> Hi Song, Christian, Al and others,
>
> Previously I proposed in [1] to add ability to do a reference-less parent
> walk for Landlock. However, as Christian pointed out and I do agree in
> hindsight, it is not a good idea to do things like this in non-VFS code.
>
> However, I still think this is valuable to consider given the performance
> improvement, and after some discussion with Mickaël, I would like to
> propose extending Song's helper to support such usage. While I recognize
> that this patch series is already in its v3, and I do not want to delay it
> by too much, putting this proposal out now is still better than after this
> has merged, so that we may consider signature changes.
>
> I've created a proof-of-concept and did some brief testing. The
> performance improvement attained here is the same as in [1] (with a "git
> status" workload, median landlock overhead 35% -> 28%, median time in
> landlock decreases by 26.6%).
>
> If this idea is accepted, I'm happy to work on it further, split out this
> patch, update the comments and do more testing etc, potentially in
> collaboration with Song.
>
> An alternative to this is perhaps to add a new helper
> path_walk_parent_rcu, also living in namei.c, that will be used directly
> by Landlock. I'm happy to do it either way, but with some experimentation
> I personally think that the code in this patch is still clean enough, and
> can avoid some duplication.
>
> Patch title: path_walk_parent: support reference-less walk
>
> A later commit will update the BPF path iterator to use this.
>
> Signed-off-by: Tingmao Wang <m@xxxxxxxxxx>
[...]
>
> -bool path_walk_parent(struct path *path, const struct path *root);
> +struct parent_iterator {
> + struct path path;
> + struct path root;
> + bool rcu;
> + /* expected seq of path->dentry */
> + unsigned next_seq;
> + unsigned m_seq, r_seq;
Most of parent_iterator is not really used by reference walk.
So it is probably just separate the two APIs?
Also, is it ok to make m_seq and r_seq available out of fs/?
> +};
> +
> +#define PATH_WALK_PARENT_UPDATED 0
> +#define PATH_WALK_PARENT_ALREADY_ROOT -1
> +#define PATH_WALK_PARENT_RETRY -2
> +
> +void path_walk_parent_start(struct parent_iterator *pit,
> + const struct path *path, const struct path *root,
> + bool ref_less);
> +int path_walk_parent(struct parent_iterator *pit, struct path *next_parent);
> +int path_walk_parent_end(struct parent_iterator *pit);
I think it is better to make this rcu walk a separate set of APIs.
IOW, we will have:
int path_walk_parent(struct path *path, struct path *root);
and
void path_walk_parent_rcu_start(struct parent_iterator *pit,
const struct path *path, const struct path *root);
int path_walk_parent_rcu_next(struct parent_iterator *pit, struct path
*next_parent);
int path_walk_parent_rcu_end(struct parent_iterator *pit);
Thanks,
Song
[...]
