From: Kevin Locke <kevin@kevinlocke.name>
To: Amir Goldstein <amir73il@gmail.com>
Cc: Miklos Szeredi <miklos@szeredi.hu>,
overlayfs <linux-unionfs@vger.kernel.org>
Subject: Re: [PATCH] ovl: warn about orphan metacopy
Date: Sun, 23 Aug 2020 08:30:43 -0600 [thread overview]
Message-ID: <20200823143043.GA14919@kevinlocke.name> (raw)
In-Reply-To: <CAOQ4uxj0HevW0iwLq61LwohsH2=-JhxYG1i_MUfmqgB3V4bQCQ@mail.gmail.com>
On Sun, 2020-08-23 at 17:12 +0300, Amir Goldstein wrote:
> On Sun, Aug 23, 2020 at 5:14 AM Kevin Locke <kevin@kevinlocke.name> 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
next prev parent reply other threads:[~2020-08-23 14:30 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-23 2:14 [PATCH] ovl: warn about orphan metacopy Kevin Locke
2020-08-23 14:12 ` Amir Goldstein
2020-08-23 14:30 ` Kevin Locke [this message]
2020-08-23 14:38 ` [PATCH v2] " Kevin Locke
2020-10-30 12:30 ` Miklos Szeredi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200823143043.GA14919@kevinlocke.name \
--to=kevin@kevinlocke.name \
--cc=amir73il@gmail.com \
--cc=linux-unionfs@vger.kernel.org \
--cc=miklos@szeredi.hu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox