[RFC,0/2] errseq+overlayfs: accomodate the volatile upper layer use-case

Message

Jeff Layton Dec. 13, 2020, 1:27 p.m. UTC

What about this as an alternate approach to the problem that Sargun has
been working on? I have some minor concerns about the complexity of
managing a stateful object across two different words. That can be
done, but I think this may be simpler.

This set steals an extra flag bit from the errseq_t counter so that we
have two flags: one indicating whether to increment the counter at set
time, and another to indicate whether the error has been reported to
userland.

This should give you the semantics you want in the syncfs case, no?  If
this does look like it's a suitable approach, then I'll plan to clean up
the comments and docs.

I have a vague feeling that this might help us eventually kill the
AS_EIO and AS_ENOSPC bits too, but that would require a bit more work to
plumb in "since" samples at appropriate places.

Jeff Layton (2):
  errseq: split the SEEN flag into two new flags
  overlayfs: propagate errors from upper to overlay sb in sync_fs

 fs/overlayfs/ovl_entry.h |  1 +
 fs/overlayfs/super.c     | 14 +++++++--
 include/linux/errseq.h   |  2 ++
 lib/errseq.c             | 64 +++++++++++++++++++++++++++++++++-------
 4 files changed, 67 insertions(+), 14 deletions(-)

Comments

Sargun Dhillon Dec. 13, 2020, 8:31 p.m. UTC | #1

On Sun, Dec 13, 2020 at 08:27:11AM -0500, Jeff Layton wrote:
> What about this as an alternate approach to the problem that Sargun has
> been working on? I have some minor concerns about the complexity of
> managing a stateful object across two different words. That can be
> done, but I think this may be simpler.
> 
> This set steals an extra flag bit from the errseq_t counter so that we
> have two flags: one indicating whether to increment the counter at set
> time, and another to indicate whether the error has been reported to
> userland.
> 

This approach works, and I believe you suggested it early on, but I was unsure
whether it was okay to use another bit for state information.

> This should give you the semantics you want in the syncfs case, no?  If
> this does look like it's a suitable approach, then I'll plan to clean up
> the comments and docs.
> 
From a raw semantics perspective, this looks correct, and it looks like we could
stash it as well for later reference (there's no going backwards, and....well,
2**19 errors is unlikely.). We do ~10s of overlayfs mounts / sec at peak,
but even then we usually see a single disk error on a machine before it fails,
I'm not sure if in the field people get more churn out of the errseq than that.


> I have a vague feeling that this might help us eventually kill the
> AS_EIO and AS_ENOSPC bits too, but that would require a bit more work to
> plumb in "since" samples at appropriate places.
> 
> Jeff Layton (2):
>   errseq: split the SEEN flag into two new flags
>   overlayfs: propagate errors from upper to overlay sb in sync_fs
> 
>  fs/overlayfs/ovl_entry.h |  1 +
>  fs/overlayfs/super.c     | 14 +++++++--
>  include/linux/errseq.h   |  2 ++
>  lib/errseq.c             | 64 +++++++++++++++++++++++++++++++++-------
>  4 files changed, 67 insertions(+), 14 deletions(-)
> 
> -- 
> 2.29.2
>