mount api: Q on behavior of mount_single vs. get_tree_single

mount api: Q on behavior of mount_single vs. get_tree_single

[Eric Sandeen]

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

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

[Eric Sandeen]

(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
> 
> 

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

[Christian Brauner]

On Wed, Jan 15, 2025 at 10:50:31AM -0600, 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

Yes, some legacy filesystems behave that way unforunately.

> 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?

I would think we should make this the filesystems problem or add a
get_tree_single_reconfigure() helper that the few filesystems that need
this behavior can use. Thoughts?

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

[Eric Sandeen]

On 1/16/25 4:28 AM, Christian Brauner wrote:
> On Wed, Jan 15, 2025 at 10:50:31AM -0600, 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
> 
> Yes, some legacy filesystems behave that way unforunately.

Is it not the case that every current (or past) user of mount_single
behave[sd] this way due to the internals of mount_single()?

gadgetfs, configfs, debugfs, tracefs, efivars, openpromfs and more all
currently call get_tree_single (and used to call mount_single, which did
the reconfigure for them).

Or am I missing something?

...

> I would think we should make this the filesystems problem or add a
> get_tree_single_reconfigure() helper that the few filesystems that need
> this behavior can use. Thoughts?

I wasn't seeing this as individual filesystems needing it, it looked to
me like every filesystem that used to call mount_single() automatically
got the behavior. So was looking at this from a consistency/regression
POV.

While it seems odd to me that mounting any "_single" sb filesystem would
reconfigure all of its current mounts on a new mount, that's the way it
used to work for everyone, as far as I can tell, so I was assuming that
we'd need to keep the behavior. But if it's a question of just fixing it
up per-filesystem, I don't mind going that route either.

Thanks,
-Eric

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

[Christian Brauner]

On Thu, Jan 16, 2025 at 09:38:07AM -0600, Eric Sandeen wrote:
> On 1/16/25 4:28 AM, Christian Brauner wrote:
> > On Wed, Jan 15, 2025 at 10:50:31AM -0600, 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
> > 
> > Yes, some legacy filesystems behave that way unforunately.
> 
> Is it not the case that every current (or past) user of mount_single
> behave[sd] this way due to the internals of mount_single()?
> 
> gadgetfs, configfs, debugfs, tracefs, efivars, openpromfs and more all
> currently call get_tree_single (and used to call mount_single, which did
> the reconfigure for them).
> 
> Or am I missing something?
> 
> ...
> 
> > I would think we should make this the filesystems problem or add a
> > get_tree_single_reconfigure() helper that the few filesystems that need
> > this behavior can use. Thoughts?
> 
> I wasn't seeing this as individual filesystems needing it, it looked to
> me like every filesystem that used to call mount_single() automatically
> got the behavior. So was looking at this from a consistency/regression
> POV.
> 
> While it seems odd to me that mounting any "_single" sb filesystem would
> reconfigure all of its current mounts on a new mount, that's the way it
> used to work for everyone, as far as I can tell, so I was assuming that
> we'd need to keep the behavior. But if it's a question of just fixing it
> up per-filesystem, I don't mind going that route either.

So far it hasn't lead to any regressions and we had the new behavior for
quite a while.

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

[Eric Sandeen]

On 1/20/25 5:23 AM, Christian Brauner wrote:

...

>> While it seems odd to me that mounting any "_single" sb filesystem would
>> reconfigure all of its current mounts on a new mount, that's the way it
>> used to work for everyone, as far as I can tell, so I was assuming that
>> we'd need to keep the behavior. But if it's a question of just fixing it
>> up per-filesystem, I don't mind going that route either.
> 
> So far it hasn't lead to any regressions and we had the new behavior for
> quite a while.

Well, it regressed tracefs & devtmpfs so far. *shrug* Working on pstore now,
which also ends up with diff. behavior by default. But I'll just work around
it for that patch, I suppose.

Thanks,
-Eric