> > > In case of any unexpected errors, we could set a error limit(for example, 10),
> > > if link error count exceeds the limit then we disable the feature.
> > >
> >
> > As far as I am concerned, you may post the patch without auto disable,
> > but I object to using an arbitrary number of errors (10) as trigger for auto
> > disable.
> >
> > I do think that it is important to communicate to admin that the whiteout
> > sharing is not working as expected and it is not acceptable to flood
> > log with warning on each and every whiteout creation, not even with
> > pr_warn_ratelimited.
> >
> > There are several ways you can go about this, but here is a suggestion:
> >
> > + if (err) {
> > + ofs->whiteout_link_max >>= 1;
> > + pr_warn("failed to link whiteout (nlink=%u, err = %i)
> > - lowering whiteout_link_max to %u\n",
> > + ofs->whiteout->d_inode->i_nlink, err,
> > ofs->whiteout_link_max);
> > + should_link = false;
> > + ovl_cleanup(wdir, ofs->whiteout);
> > + }
> >
>
> Frankly speaking, I don't like heuristic method to automatically enable/disable feature,
> so I perfer to disable the feature immediately after hitting any error just like your previous suggestion,
Fine by me.
> and I also hope to add a mount option for this feature to mitigate global effect to different overlayfs instances.
>
I think that would make sense.
Off topic: from system perspective, the fact that whiteouts take up inodes
to begin with is a shame. If one had control over the underlying filesystem,
whiteout should have been implemented as a constant and immutable special
inode with no nlink count at all and getdents would return DT_WHITEOUT for
those entries.
Thanks,
Amir.
