On Mon, Jan 13, 2020 at 1:37 PM Miklos Szeredi <miklos@xxxxxxxxxx> wrote:
>
> On Sun, Dec 22, 2019 at 9:08 AM Amir Goldstein <amir73il@xxxxxxxxx> wrote:
> >
> > No code uses the sb returned from this helper, so make it retrun
> > a boolean and rename it to ovl_same_fs().
> >
> > The xino mode is irrelevant when all layers are on same fs, so
> > instead of describing samefs with mode OVL_XINO_OFF, use a new mode
> > OVL_XINO_SAME_FS, which is different than the case of non-samefs
> > with xino=off.
> >
> > Create a new helper ovl_same_dev(), to use instead of the common check
> > for (ovl_same_fs() || xinobits).
>
> What about OVL_XINO_AUTO: in this case xinobits would be zero but
> ovl_same_dev() would return true, no?
>
Yes, I missed it.
> More comments inline.
>
> > @@ -358,7 +352,8 @@ static int ovl_show_options(struct seq_file *m, struct dentry *dentry)
> > if (ofs->config.nfs_export != ovl_nfs_export_def)
> > seq_printf(m, ",nfs_export=%s", ofs->config.nfs_export ?
> > "on" : "off");
> > - if (ofs->config.xino != ovl_xino_def())
> > + if (ofs->config.xino != ovl_xino_def() &&
> > + ofs->config.xino != OVL_XINO_SAME_FS)
>
> I'm not sure I like this, although it doesn't contradict the policy of
> repeatability of mounts. Could we instead have a separate
> ofs->xino_state, that defaults to config.xino but can take the value
> of OVL_XINO_SAME_FS?
>
OK, I understand why you don't like it and I think is makes sense to
have an xino_state.
However, xino_state and xino_bits are quite redundant.
If we change:
unsigned int xino_bits;
to:
int xino_mode;
It can take the values:
-1 /* not same dev */
0 /* same fs */
1..32 /* xino_bits */
And:
static inline unsigned int ovl_xino_bits(struct super_block *sb)
{
return OVL_FS(sb)->xino_mode > 0 : OVL_FS(sb)->xino_mode : 0;
}
Thanks,
Amir.
