On Sun, 2020-08-23 at 17:12 +0300, Amir Goldstein wrote:
> On Sun, Aug 23, 2020 at 5:14 AM Kevin Locke <kevin@xxxxxxxxxxxxxxx> wrote:
>> diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
>> index f7d4358db637..30e1c10800ab 100644
>> --- a/fs/overlayfs/namei.c
>> +++ b/fs/overlayfs/namei.c
>> @@ -1000,6 +1000,7 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
>> * Just make sure a corresponding data dentry has been found.
>> */
>> if (d.metacopy || (uppermetacopy && !ctr)) {
>> + pr_warn_ratelimited("orphan metacopy (%pd2)\n", dentry);
>
> Funny. You started this thread because of a pain point - you did not know
> what caused EIO in your setup.
>
> Try to go back to where you stood when you got EIO.
> Would that message in the log would have helped you understand?
> Would it have helped someone who is less skilled than you are in reading
> kernel code? I doubt it.
After I was unable to reproduce EIO by accessing the file on upper
directly and had no error message, I started grepping for EIO in
overlayfs, which led to multiple results and no clear next step (kernel
tracing helped, but was still difficult to isolate the source of EIO).
Having any error message would get me to the point in the code where the
error was encountered. Mentioning metacopy gets me to a causal feature,
which would have been helpful for understanding.
> You better be more explicit about what has gone wrong, e.g.:
> "metacopy upper with no lower data found - abort lookup..."
>
> It is nice that you followed a precedent of "orphan index", but if you
> look closely you will see that those cases do not end up with a user
> error - they end up with auto cleaning those "orphan index", so the
> kernel messages are just FYI - it doesn't matter if users understand them
> because they do not require users to take any action.
Sure. That wording sounds better to me. I'll send an updated patch
shortly.
Thanks,
Kevin
