Re: [PATCH 06/18] fsinfo: Add a uniquifier ID to struct mount [ver #21]

From: David Howells
Date: Wed Aug 05 2020 - 15:23:28 EST

Miklos Szeredi <miklos@xxxxxxxxxx> wrote:

> > +#ifdef CONFIG_FSINFO
> > + u64 mnt_unique_id; /* ID unique over lifetime of kernel */
> > +#endif
> Not sure if it's worth making conditional.

You can't get at it without CONFIG_FSINFO=y as it stands, but making it
unconditional might be reasonable.

> > - n.auxiliary_mount = aux->mnt_id;
> > + n.auxiliary_mount = aux->mnt_unique_id;
> Hmm, so we now have two ID's:
> - one can be used to look up the mount
> - one is guaranteed to be unique
> With this change the mount cannot be looked up with FSINFO_FLAGS_QUERY_MOUNT,
> right?
> Should we be merging the two ID's into a single one which has both properties?

Ideally, yes... but... The 31-bit mnt_id is currently exposed to userspace in
various places, e.g. /proc, sys_name_to_handle_at(). So we have to keep that
as is and we can't expand it.

For fsinfo(), however, it might make sense to only use the 64-bit uniquifier
as the identifier to use for direct look up.

However, looking up that identifier requires some sort of structure for doing
this and it's kind of worst case for the IDR tree as the keys are gradually
going to spread out, causing it to eat more memory. It may be a tradeoff
worth making, and the memory consumption might not be that bad - or we could
use some other data structure such as an rbtree.

That's why I was going for the 31-bit identifier + uniquifier so that you can at
least tell if the identifier got recycled reasonably quickly.