Re: [PATCH v4] ovl: whiteout inode sharing

[Amir Goldstein <amir73il@gmail.com>]

On Fri, Apr 24, 2020 at 9:26 AM Chengguang Xu <cgxu519@mykernel.net> wrote:
>
>  ---- 在 星期五, 2020-04-24 14:02:00 Amir Goldstein <amir73il@gmail.com> 撰写 ----
>  > >  > > +               case OPT_WHITEOUT_LINK_MAX:
>  > >  > > +                       if (match_int(&args[0], &link_max))
>  > >  > > +                               return -EINVAL;
>  > >  > > +                       if (link_max < ovl_whiteout_link_max_def)
>  > >  > > +                               config->whiteout_link_max = link_max;
>  > >  >
>  > >  > Why not allow link_max > ovl_whiteout_link_max_def?
>  > >  > admin may want to disable ovl_whiteout_link_max_def by default
>  > >  > in module parameter, but allow it for specific overlay instances.
>  > >  >
>  > >
>  > > In this use case, seems we don't need module param any more, we just need to set  default value for option.
>  > >
>  > > I would like to treate module param as a total switch, so that it could disable the feathre for all instances at the same time.
>  > > I think sometimes it's helpful for lazy admin(like me).
>  > >
>  >
>  > I am not convinced.
>  >
>  > lazy admin could very well want to disable whiteout_link_max by default,
>  > but allow user to specify whiteout_link_max for a specific mount.
>  >
>  > In fact, in order to preserve existing behavior and not cause regression with
>  > some special filesystems, distros could decide that default disabled is
>  > a reasonable choice.
>  >
>  > I don't understand at all what the purpose of this limitation is.
>  >
>
> If user sets a ridiculous  link_max which is larger than valid range of upper fs, I think it is hard to verify in the stage of option parsing.
> So I hope to fix the upper limit using module parameter, we can set default mount option to  0/1 for the use case you mentioned above.
>

I didn't mean we need to check if link_max  is valid range for upper fs.
We anyway use minimum of user requested value and upper fs max.

Frankly, I think there are only two sane options for system wide configuration:
1. disable whiteout link
2. enable whiteout link with ofs->workdir->d_sb->s_max_links

So perhaps the module param should be a boolean ovl_whiteout_link_def?
Perhaps Kconfig should determine the build time default.

Setting whiteout_link smaller than d_sb->s_max_links should be
possible via mount option.

We may want to support the mount options:
whiteout_link_max=<N>
whiteout_link_max=auto

It should be simple to parse whiteout_link_max=auto, just
set config->whiteout_link_max to max uint and let later code
reduce it to upper fs max.

For ovl_show_options() is slightly more complicated to get right.

I am not hooked on any of the ideas above, but I find the current
configuration options in v4/v5 not good enough.

As an exercise you can try to document those options and
see how clear the text is.

Thanks,
Amir.