NACK: [SRU][UNSTABLE/FOCAL/BIONIC][PATCH v2] UBUNTU: SAUCE: shiftfs: fix dentry revalidation

[Christian Brauner]

On Tue, May 19, 2020 at 08:02:25AM +0200, Christian Brauner wrote:
> On Tue, May 19, 2020 at 07:43:21AM +0200, Stefan Bader wrote:
> > On 18.05.20 20:18, Christian Brauner wrote:
> > > BugLink: https://bugs.launchpad.net/bugs/1879196
> > > 
> > > While fixing [1] we introduced a regression and triggered [2] whereby
> > > the underlay and overlay go out of sync because they disagree about the
> > > dcache. Such situation easily arise when the underlay is modified
> > > directly. This means the overlay needs to revalidate the dentry in the
> > > dcache to catchup with the underlay.
> > > 
> > > Note, in general it's not advisable to directly modify the underlay
> > > while a shiftfs mount is on top. In some way this means we need to keep
> > > two caches in sync and it's hard enough to keep a single cache happy.
> > > But shiftfs' use-case is inherently prone to be used for exactly that.
> > > So this is something we have to navigate carefully and honestly we have
> > > no full model upstream that does the same. Overlayfs has the copy-up
> > > behavior which let's it get around most of the issues but we don't have
> > > it and ecryptfs is broken in such scenarios which we verified quite a
> > > while back.
> > > In any case, I built a kernel with this patch and re-ran all regressions
> > > that are related to this that we have so far (cf.  [1], [2], and [3]).
> > > None of them were reproducible with this patch here. So we still fix the
> > > ESTALE issue but also keep underlay and overlay in sync.
> > 
> > This test kernel should be provided to the bug reporter, so it can be confirmed
> > to fix the issue there as well.
> 
> See
> https://drive.google.com/open?id=19iTwaFSYNS95_I-gD_rvFoV9cMAfy6io
> Elmo is also Cced here. I don't have any other place I can upload this,
> I think.

Just so I don't get railed for this later in case there's an issue.
Here's an asciinema recording of all passing tests:

https://asciinema.org/a/331649

> 
> > But more importantly, you submitted this for Bionic and there is no shiftfs in
> > that release. Only Eoan. I am not convinced you tried at least to apply the
> 
> Yeah, sorry that's always my thing mixing up that shiftfs is in Eoan and
> not in Bionic.
> Similar to all the other patches, Eoan doesn't need a separate patch
> I've verified this on top of Eoan master-next that has all the other
> shiftfs patches:
> 
> ubuntu at amdgpu:~/linux/ubuntu/ubuntu-eoan$ git am ./0001-UBUNTU-SAUCE-shiftfs-fix-dentry-revalidation.patch
> Applying: UBUNTU: SAUCE: shiftfs: fix dentry revalidation
> 
> on top of
> 
> ubuntu at amdgpu:~/linux/ubuntu/ubuntu-eoan$ git am ./0001-UBUNTU-SAUCE-shiftfs-fix-dentry-revalidation.patch
> Applying: UBUNTU: SAUCE: shiftfs: fix dentry revalidation
> commit d53d4b6679bb89ae78787dca0fea951752e27d48 (upstream/master-next)
> Author: Kleber Sacilotto de Souza <kleber.souza at canonical.com>
> Date:   Fri May 15 13:03:42 2020 +0200
> 
>     UBUNTU: Ubuntu-5.3.0-54.48
> 
>     Signed-off-by: Kleber Sacilotto de Souza <kleber.souza at canonical.com>
> 
> I'm not letting shiftfs across kernel versions go out of sync precisely
> (at least until we absolutely have to) so that patches can be applied
> trivially across releases so it's easier for all of you.

Do you still want me to resend with s/BIONIC/EOAN/ in there?

Thanks!
Christian