Re: mount api: Q on behavior of mount_single vs. get_tree_single

[Eric Sandeen <sandeen@redhat.com>]

(ugh, curse auto complete on addresses, cc: fixed sorry.)

On 1/15/25 10:50 AM, Eric Sandeen wrote:
> I was finally getting back to some more mount API conversions,
> and was looking at pstore, which I thought would be straightforward.
> 
> I noticed that mount options weren't being accepted, and I'm fairly
> sure that this is because there's some internal or hidden mount of
> pstore, and mount is actually a remount due to it using a single
> superblock. (looks like it was some sort of clone mount done under
> the covers by various processes, still not sure.)
> 
> In any case, that led me to wonder:
> 
> Should get_tree_single() be doing an explicit reconfigure like
> mount_single does?
> 
> mount_single() {
> ...
>         if (!s->s_root) {
>                 error = fill_super(s, data, flags & SB_SILENT ? 1 : 0);
>                 if (!error)
>                         s->s_flags |= SB_ACTIVE;
>         } else {
>                 error = reconfigure_single(s, flags, data);
>         }
> ...
> 
> My pstore problem abovec reminded me of the recent issue with tracefs
> after the mount api conversion, fixed with:
> 
> e4d32142d1de tracing: Fix tracefs mount options
> 
> and discussed at:
> 
> https://lore.kernel.org/lkml/20241030171928.4168869-2-kaleshsingh@google.com/
> 
> which in turn reminded me of:
> 
> a6097180d884 devtmpfs regression fix: reconfigure on each mount
> 
> so we've seen this difference in behavior with get_tree_single twice already,
> and then I ran into it again on pstore.
> 
> Should get_tree_single() callers be fixing this up themselves ala devtmpfs
> and tracefs, or should get_tree_single() be handling this internally?
> 
> Right now in my pstore patch I'm doing:
> 
> static int pstore_get_tree(struct fs_context *fc)
> {
>        if (fc->root)
>                return pstore_reconfigure(fc);
> 
>        return get_tree_single(fc, pstore_fill_super);
> }
> 
> but it really feels like this should be handled by core code instead.
> 
> Thoughts?
> 
> Thanks,
> -Eric
> 
>