From: "Yurkov, Vyacheslav" <Vyacheslav.Yurkov@bruker.com>
To: Amir Goldstein <amir73il@gmail.com>
Cc: Miklos Szeredi <miklos@szeredi.hu>,
overlayfs <linux-unionfs@vger.kernel.org>
Subject: RE: [PATCH] ovl: do not set overlay.opaque for new directories
Date: Thu, 27 May 2021 12:18:55 +0000 [thread overview]
Message-ID: <AM8PR10MB41616A1FD00E71B2F0F2390086239@AM8PR10MB4161.EURPRD10.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <CAOQ4uximN3=VQ=CYryGrrkdXn0GpXe=skrrRma07MMRvz_gByw@mail.gmail.com>
Hi Amir,
Thanks for the follow-up.
Yes, this is existing problem I solved with a proposed patch on our devices. Of course having a proper solution would be better.
The only option I see enabled in my config is CONFIG_OVERLAY_FS_REDIRECT_ALWAYS_FOLLOW.
Took me some time to backport proposed changes to my kernel, but they worked fine. Will send a v2 shortly
The
- confidential -
> -----Original Message-----
> From: Amir Goldstein <amir73il@gmail.com>
> Sent: Tuesday, May 25, 2021 10:32
> To: Yurkov, Vyacheslav <Vyacheslav.Yurkov@bruker.com>
> Cc: Miklos Szeredi <miklos@szeredi.hu>; overlayfs <linux-
> unionfs@vger.kernel.org>
> Subject: Re: [PATCH] ovl: do not set overlay.opaque for new directories
>
> **EXTERNAL EMAIL**
>
> On Mon, May 3, 2021 at 9:43 PM Vyacheslav Yurkov
> <Vyacheslav.Yurkov@bruker.com> wrote:
> >
> > This optimization breaks existing use case when a lower layer directory
> > appears after directory was created on a merged layer. If overlay.opaque
> > is applied, new files on lower layer are not visible.
> >
> > Consider the following scenario:
> > - /lower and /upper are mounted to /merged
> > - directory /merged/new-dir is created with a file test1
> > - overlay is unmounted
> > - directory /lower/new-dir is created with a file test2
> > - overlay is mounted again
> >
> > If opaque is applied by default, file test2 is not going to be visible
> > without explicitly clearing the overlay.opaque attribute
> >
> > Signed-off-by: Vyacheslav Yurkov <Vyacheslav.Yurkov@bruker.com>
>
> Hi Vyacheslav,
>
> Sorry for the late reply.
> Is the described regression really happening in real deployments?
> I would like to avoid removing the optimization if possible.
>
> In any case, if we have to support existing deployments that use this
> practice,
> the optimization should be removed only for the case where the user did not
> opt-in to any of the new features, quoting overlayfs.rst:
> '
> Offline changes to the lower tree are only allowed if the
> "metadata only copy up", "inode index", "xino" and "redirect_dir" features
> have not been used. If the lower tree is modified and any of these
> features has been used, the behavior of the overlay is undefined,
> though it will not result in a crash or deadlock.
> '
>
> This means putting this check from ovl_lower_uuid_ok() into a helper:
>
> static inline bool ovl_allow_offline_changes(struct ovl_fs *ofs)
> {
> /*
> * To avoid regressions in existing setups with overlay lower offline
> * changes, we allow lower changes only if none of the new features
> * are used.
> */
> return (!ofs->config.index && !ofs->config.metacopy &&
> !ofs>config.redirect_dir && ofs->config.xino !=
> OVL_XINO_ON);
> }
>
> Note that ovl_lower_uuid_ok() does not currently check the redirect_dir
> feature, but it would be better to use the same helper in that case as well.
>
> Thanks,
> Amir.
>
> > ---
> > fs/overlayfs/dir.c | 5 -----
> > 1 file changed, 5 deletions(-)
> >
> > diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> > index 93efe7048a77..f66f96dd9f0c 100644
> > --- a/fs/overlayfs/dir.c
> > +++ b/fs/overlayfs/dir.c
> > @@ -338,11 +338,6 @@ static int ovl_create_upper(struct dentry *dentry,
> struct inode *inode,
> > if (IS_ERR(newdentry))
> > goto out_unlock;
> >
> > - if (ovl_type_merge(dentry->d_parent) && d_is_dir(newdentry)) {
> > - /* Setting opaque here is just an optimization, allow to fail */
> > - ovl_set_opaque(dentry, newdentry);
> > - }
> > -
> > err = ovl_instantiate(dentry, inode, newdentry, !!attr->hardlink);
> > if (err)
> > goto out_cleanup;
> > --
> > 2.25.1
> >
prev parent reply other threads:[~2021-05-27 12:26 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-03 18:42 [PATCH] ovl: do not set overlay.opaque for new directories Vyacheslav Yurkov
2021-05-25 8:31 ` Amir Goldstein
2021-05-27 12:18 ` Yurkov, Vyacheslav [this message]
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=AM8PR10MB41616A1FD00E71B2F0F2390086239@AM8PR10MB4161.EURPRD10.PROD.OUTLOOK.COM \
--to=vyacheslav.yurkov@bruker.com \
--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;
as well as URLs for NNTP newsgroup(s).