| Message ID | 20201113065555.147276-10-cgxu519@mykernel.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | implement containerized syncfs for overlayfs | expand |
On Fri, Nov 13, 2020 at 7:57 AM Chengguang Xu <cgxu519@mykernel.net> wrote: > > Now overlayfs can only sync dirty inode during syncfs, > so remove unnecessary sync_filesystem() on upper file > system. > > Signed-off-by: Chengguang Xu <cgxu519@mykernel.net> > --- > fs/overlayfs/super.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c > index 982b3954b47c..58507f1cd583 100644 > --- a/fs/overlayfs/super.c > +++ b/fs/overlayfs/super.c > @@ -15,6 +15,8 @@ > #include <linux/seq_file.h> > #include <linux/posix_acl_xattr.h> > #include <linux/exportfs.h> > +#include <linux/blkdev.h> > +#include <linux/writeback.h> > #include "overlayfs.h" > > MODULE_AUTHOR("Miklos Szeredi <miklos@szeredi.hu>"); > @@ -270,8 +272,7 @@ static int ovl_sync_fs(struct super_block *sb, int wait) > * Not called for sync(2) call or an emergency sync (SB_I_SKIP_SYNC). > * All the super blocks will be iterated, including upper_sb. > * > - * If this is a syncfs(2) call, then we do need to call > - * sync_filesystem() on upper_sb, but enough if we do it when being > + * if this is a syncfs(2) call, it will be enough we do it when being > * called with wait == 1. > */ > if (!wait) > @@ -280,7 +281,11 @@ static int ovl_sync_fs(struct super_block *sb, int wait) > upper_sb = ovl_upper_mnt(ofs)->mnt_sb; > > down_read(&upper_sb->s_umount); > - ret = sync_filesystem(upper_sb); > + wait_sb_inodes(upper_sb); > + if (upper_sb->s_op->sync_fs) > + ret = upper_sb->s_op->sync_fs(upper_sb, wait); > + if (!ret) > + ret = sync_blockdev(upper_sb->s_bdev); Should this instead be __sync_blockdev(..., wait)? Thanks, Miklos
---- 在 星期五, 2021-04-09 21:51:26 Miklos Szeredi <miklos@szeredi.hu> 撰写 ---- > On Fri, Nov 13, 2020 at 7:57 AM Chengguang Xu <cgxu519@mykernel.net> wrote: > > > > Now overlayfs can only sync dirty inode during syncfs, > > so remove unnecessary sync_filesystem() on upper file > > system. > > > > Signed-off-by: Chengguang Xu <cgxu519@mykernel.net> > > --- > > fs/overlayfs/super.c | 11 ++++++++--- > > 1 file changed, 8 insertions(+), 3 deletions(-) > > > > diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c > > index 982b3954b47c..58507f1cd583 100644 > > --- a/fs/overlayfs/super.c > > +++ b/fs/overlayfs/super.c > > @@ -15,6 +15,8 @@ > > #include <linux/seq_file.h> > > #include <linux/posix_acl_xattr.h> > > #include <linux/exportfs.h> > > +#include <linux/blkdev.h> > > +#include <linux/writeback.h> > > #include "overlayfs.h" > > > > MODULE_AUTHOR("Miklos Szeredi <miklos@szeredi.hu>"); > > @@ -270,8 +272,7 @@ static int ovl_sync_fs(struct super_block *sb, int wait) > > * Not called for sync(2) call or an emergency sync (SB_I_SKIP_SYNC). > > * All the super blocks will be iterated, including upper_sb. > > * > > - * If this is a syncfs(2) call, then we do need to call > > - * sync_filesystem() on upper_sb, but enough if we do it when being > > + * if this is a syncfs(2) call, it will be enough we do it when being > > * called with wait == 1. > > */ > > if (!wait) > > @@ -280,7 +281,11 @@ static int ovl_sync_fs(struct super_block *sb, int wait) > > upper_sb = ovl_upper_mnt(ofs)->mnt_sb; > > > > down_read(&upper_sb->s_umount); > > - ret = sync_filesystem(upper_sb); > > + wait_sb_inodes(upper_sb); > > + if (upper_sb->s_op->sync_fs) > > + ret = upper_sb->s_op->sync_fs(upper_sb, wait); > > + if (!ret) > > + ret = sync_blockdev(upper_sb->s_bdev); > > Should this instead be __sync_blockdev(..., wait)? I don't remember why we skipped the case of (wait == 0) here, just guess it's not worth to export internal function __sync_blockdev() to modules, do you prefer to call __sync_blockdev() and handle both nowait and wait cases? Thanks, Chengguang
On Mon, Apr 12, 2021 at 2:24 PM Chengguang Xu <cgxu519@mykernel.net> wrote: > > ---- 在 星期五, 2021-04-09 21:51:26 Miklos Szeredi <miklos@szeredi.hu> 撰写 ---- > > On Fri, Nov 13, 2020 at 7:57 AM Chengguang Xu <cgxu519@mykernel.net> wrote: > > > > > > Now overlayfs can only sync dirty inode during syncfs, > > > so remove unnecessary sync_filesystem() on upper file > > > system. > > > > > > Signed-off-by: Chengguang Xu <cgxu519@mykernel.net> > > > --- > > > fs/overlayfs/super.c | 11 ++++++++--- > > > 1 file changed, 8 insertions(+), 3 deletions(-) > > > > > > diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c > > > index 982b3954b47c..58507f1cd583 100644 > > > --- a/fs/overlayfs/super.c > > > +++ b/fs/overlayfs/super.c > > > @@ -15,6 +15,8 @@ > > > #include <linux/seq_file.h> > > > #include <linux/posix_acl_xattr.h> > > > #include <linux/exportfs.h> > > > +#include <linux/blkdev.h> > > > +#include <linux/writeback.h> > > > #include "overlayfs.h" > > > > > > MODULE_AUTHOR("Miklos Szeredi <miklos@szeredi.hu>"); > > > @@ -270,8 +272,7 @@ static int ovl_sync_fs(struct super_block *sb, int wait) > > > * Not called for sync(2) call or an emergency sync (SB_I_SKIP_SYNC). > > > * All the super blocks will be iterated, including upper_sb. > > > * > > > - * If this is a syncfs(2) call, then we do need to call > > > - * sync_filesystem() on upper_sb, but enough if we do it when being > > > + * if this is a syncfs(2) call, it will be enough we do it when being > > > * called with wait == 1. > > > */ > > > if (!wait) > > > @@ -280,7 +281,11 @@ static int ovl_sync_fs(struct super_block *sb, int wait) > > > upper_sb = ovl_upper_mnt(ofs)->mnt_sb; > > > > > > down_read(&upper_sb->s_umount); > > > - ret = sync_filesystem(upper_sb); > > > + wait_sb_inodes(upper_sb); > > > + if (upper_sb->s_op->sync_fs) > > > + ret = upper_sb->s_op->sync_fs(upper_sb, wait); > > > + if (!ret) > > > + ret = sync_blockdev(upper_sb->s_bdev); > > > > Should this instead be __sync_blockdev(..., wait)? > > I don't remember why we skipped the case of (wait == 0) here, just guess it's not worth > to export internal function __sync_blockdev() to modules, do you prefer to call __sync_blockdev() > and handle both nowait and wait cases? Possibly it would make most sense to export the "->sync_fs() + __sync_blockdev()" part of __sync_filesystem() as a separate helper. Thanks, Miklos
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index 982b3954b47c..58507f1cd583 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -15,6 +15,8 @@ #include <linux/seq_file.h> #include <linux/posix_acl_xattr.h> #include <linux/exportfs.h> +#include <linux/blkdev.h> +#include <linux/writeback.h> #include "overlayfs.h" MODULE_AUTHOR("Miklos Szeredi <miklos@szeredi.hu>"); @@ -270,8 +272,7 @@ static int ovl_sync_fs(struct super_block *sb, int wait) * Not called for sync(2) call or an emergency sync (SB_I_SKIP_SYNC). * All the super blocks will be iterated, including upper_sb. * - * If this is a syncfs(2) call, then we do need to call - * sync_filesystem() on upper_sb, but enough if we do it when being + * if this is a syncfs(2) call, it will be enough we do it when being * called with wait == 1. */ if (!wait) @@ -280,7 +281,11 @@ static int ovl_sync_fs(struct super_block *sb, int wait) upper_sb = ovl_upper_mnt(ofs)->mnt_sb; down_read(&upper_sb->s_umount); - ret = sync_filesystem(upper_sb); + wait_sb_inodes(upper_sb); + if (upper_sb->s_op->sync_fs) + ret = upper_sb->s_op->sync_fs(upper_sb, wait); + if (!ret) + ret = sync_blockdev(upper_sb->s_bdev); up_read(&upper_sb->s_umount); return ret;
Now overlayfs can only sync dirty inode during syncfs, so remove unnecessary sync_filesystem() on upper file system. Signed-off-by: Chengguang Xu <cgxu519@mykernel.net> --- fs/overlayfs/super.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)