Right now we check for errors on super block in syncfs().
ret2 = errseq_check_and_advance(&sb->s_wb_err, &f.file->f_sb_err);
overlayfs does not update sb->s_wb_err and it is tracked on upper filesystem.
So provide a superblock operation to check errors so that filesystem
can provide override generic method and provide its own method to
check for writeback errors.
Signed-off-by: Vivek Goyal <[email protected]>
---
fs/sync.c | 5 ++++-
include/linux/fs.h | 1 +
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/fs/sync.c b/fs/sync.c
index b5fb83a734cd..57e43a16dfca 100644
--- a/fs/sync.c
+++ b/fs/sync.c
@@ -176,7 +176,10 @@ SYSCALL_DEFINE1(syncfs, int, fd)
ret = sync_filesystem(sb);
up_read(&sb->s_umount);
- ret2 = errseq_check_and_advance(&sb->s_wb_err, &f.file->f_sb_err);
+ if (sb->s_op->errseq_check_advance)
+ ret2 = sb->s_op->errseq_check_advance(sb, f.file);
+ else
+ ret2 = errseq_check_and_advance(&sb->s_wb_err, &f.file->f_sb_err);
fdput(f);
return ret ? ret : ret2;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 8667d0cdc71e..4297b6127adf 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1965,6 +1965,7 @@ struct super_operations {
struct shrink_control *);
long (*free_cached_objects)(struct super_block *,
struct shrink_control *);
+ int (*errseq_check_advance)(struct super_block *, struct file *);
};
/*
--
2.25.4
On Mon, Dec 21, 2020 at 02:50:54PM -0500, Vivek Goyal wrote:
> - ret2 = errseq_check_and_advance(&sb->s_wb_err, &f.file->f_sb_err);
> + if (sb->s_op->errseq_check_advance)
> + ret2 = sb->s_op->errseq_check_advance(sb, f.file);
What a terrible name for an fs operation. You don't seem to be able
to distinguish between semantics and implementation. How about
check_error()?
On Tue, Dec 22, 2020 at 04:19:00PM +0000, Matthew Wilcox wrote:
> On Mon, Dec 21, 2020 at 02:50:54PM -0500, Vivek Goyal wrote:
> > - ret2 = errseq_check_and_advance(&sb->s_wb_err, &f.file->f_sb_err);
> > + if (sb->s_op->errseq_check_advance)
> > + ret2 = sb->s_op->errseq_check_advance(sb, f.file);
>
> What a terrible name for an fs operation. You don't seem to be able
> to distinguish between semantics and implementation. How about
> check_error()?
check_error() sounds better. I was not very happy with the name either.
Thought of starting with something.
Vivek
On Tue, 2020-12-22 at 11:25 -0500, Vivek Goyal wrote:
> On Tue, Dec 22, 2020 at 04:19:00PM +0000, Matthew Wilcox wrote:
> > On Mon, Dec 21, 2020 at 02:50:54PM -0500, Vivek Goyal wrote:
> > > - ret2 = errseq_check_and_advance(&sb->s_wb_err, &f.file->f_sb_err);
> > > + if (sb->s_op->errseq_check_advance)
> > > + ret2 = sb->s_op->errseq_check_advance(sb, f.file);
> >
> > What a terrible name for an fs operation. You don't seem to be able
> > to distinguish between semantics and implementation. How about
> > check_error()?
>
> check_error() sounds better. I was not very happy with the name either.
> Thought of starting with something.
>
Maybe report_error() ?
The same error won't be reported on the next call on the same fd. I
think the important point to make here is that this error must be
reported to syncfs() or something like it once you call this.
(In hindsight, I sort of wish I had done s/serrseq_set/errseq_record/
and s/errseq_check_and_advance/errseq_report/ when I initially did this,
if only to make the API a little less dependent on the implementation.)
--
Jeff Layton <[email protected]>
On Mon, 2020-12-21 at 14:50 -0500, Vivek Goyal wrote:
> Right now we check for errors on super block in syncfs().
>
> ret2 = errseq_check_and_advance(&sb->s_wb_err, &f.file->f_sb_err);
>
> overlayfs does not update sb->s_wb_err and it is tracked on upper filesystem.
> So provide a superblock operation to check errors so that filesystem
> can provide override generic method and provide its own method to
> check for writeback errors.
>
> Signed-off-by: Vivek Goyal <[email protected]>
> ---
> ?fs/sync.c | 5 ++++-
> ?include/linux/fs.h | 1 +
> ?2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/fs/sync.c b/fs/sync.c
> index b5fb83a734cd..57e43a16dfca 100644
> --- a/fs/sync.c
> +++ b/fs/sync.c
> @@ -176,7 +176,10 @@ SYSCALL_DEFINE1(syncfs, int, fd)
> ? ret = sync_filesystem(sb);
> ? up_read(&sb->s_umount);
> ?
>
> - ret2 = errseq_check_and_advance(&sb->s_wb_err, &f.file->f_sb_err);
> + if (sb->s_op->errseq_check_advance)
> + ret2 = sb->s_op->errseq_check_advance(sb, f.file);
> + else
> + ret2 = errseq_check_and_advance(&sb->s_wb_err, &f.file->f_sb_err);
> ?
>
> ? fdput(f);
> ? return ret ? ret : ret2;
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 8667d0cdc71e..4297b6127adf 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1965,6 +1965,7 @@ struct super_operations {
> ? struct shrink_control *);
> ? long (*free_cached_objects)(struct super_block *,
> ? struct shrink_control *);
> + int (*errseq_check_advance)(struct super_block *, struct file *);
> ?};
> ?
>
> ?/*
Also, the other super_operations generally don't take a superblock
pointer when you pass in a different fs object pointer. This should
probably just take a struct file * and then the operation can chase
pointers to the superblock from there.
--
Jeff Layton <[email protected]>
On Wed, Dec 23, 2020 at 07:48:52AM -0500, Jeff Layton wrote:
> On Mon, 2020-12-21 at 14:50 -0500, Vivek Goyal wrote:
> > Right now we check for errors on super block in syncfs().
> >
> > ret2 = errseq_check_and_advance(&sb->s_wb_err, &f.file->f_sb_err);
> >
> > overlayfs does not update sb->s_wb_err and it is tracked on upper filesystem.
> > So provide a superblock operation to check errors so that filesystem
> > can provide override generic method and provide its own method to
> > check for writeback errors.
> >
> > Signed-off-by: Vivek Goyal <[email protected]>
> > ---
> > ?fs/sync.c | 5 ++++-
> > ?include/linux/fs.h | 1 +
> > ?2 files changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/sync.c b/fs/sync.c
> > index b5fb83a734cd..57e43a16dfca 100644
> > --- a/fs/sync.c
> > +++ b/fs/sync.c
> > @@ -176,7 +176,10 @@ SYSCALL_DEFINE1(syncfs, int, fd)
> > ? ret = sync_filesystem(sb);
> > ? up_read(&sb->s_umount);
> > ?
> >
> > - ret2 = errseq_check_and_advance(&sb->s_wb_err, &f.file->f_sb_err);
> > + if (sb->s_op->errseq_check_advance)
> > + ret2 = sb->s_op->errseq_check_advance(sb, f.file);
> > + else
> > + ret2 = errseq_check_and_advance(&sb->s_wb_err, &f.file->f_sb_err);
> > ?
> >
> > ? fdput(f);
> > ? return ret ? ret : ret2;
> > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > index 8667d0cdc71e..4297b6127adf 100644
> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -1965,6 +1965,7 @@ struct super_operations {
> > ? struct shrink_control *);
> > ? long (*free_cached_objects)(struct super_block *,
> > ? struct shrink_control *);
> > + int (*errseq_check_advance)(struct super_block *, struct file *);
> > ?};
> > ?
> >
> > ?/*
>
> Also, the other super_operations generally don't take a superblock
> pointer when you pass in a different fs object pointer. This should
> probably just take a struct file * and then the operation can chase
> pointers to the superblock from there.
Ok, I will drop super_block * argument and just pass in "struct file *".
Vivek
>
> --
> Jeff Layton <[email protected]>
>