| Message ID | 20210923130814.140814-8-cgxu519@mykernel.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | implement containerized syncfs for overlayfs | expand |
On Thu, 23 Sept 2021 at 15:08, Chengguang Xu <cgxu519@mykernel.net> wrote: > > Now drop overlayfs' inode will sync dirty data, > so we change to only drop clean inode. > > The purpose of doing this is to keep compatible > behavior with before because without this change > dropping overlayfs inode will not trigger syncing > of underlying dirty inode. > > Signed-off-by: Chengguang Xu <cgxu519@mykernel.net> > --- > fs/overlayfs/super.c | 16 +++++++++++++++- > 1 file changed, 15 insertions(+), 1 deletion(-) > > diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c > index cddae3ca2fa5..bf4000eb9be8 100644 > --- a/fs/overlayfs/super.c > +++ b/fs/overlayfs/super.c > @@ -441,11 +441,25 @@ static int ovl_write_inode(struct inode *inode, > return ret; > } > > +/* > + * In iput_final(), clean inode will drop directly and dirty inode will > + * keep in the cache until write back to sync dirty data then add to lru > + * list to wait reclaim. > + */ > +static int ovl_drop_inode(struct inode *inode) > +{ > + struct inode *upper = ovl_inode_upper(inode); > + > + if (!upper || !(inode->i_state & I_DIRTY_ALL)) Could we check upper dirtyness here? That would give a more precise result. Alternatively don't set .drop_inode (i.e. use generic_drop_inode()) and set I_DONTCACHE on overlay inodes. That would cause the upper inode to be always written back before eviction. The latter would result in simpler logic, and I think performance-wise it wouldn't matter. But I may be missing something. Thanks, Miklos
在 2021/10/7 19:09, Miklos Szeredi 写道: > On Thu, 23 Sept 2021 at 15:08, Chengguang Xu <cgxu519@mykernel.net> wrote: >> Now drop overlayfs' inode will sync dirty data, >> so we change to only drop clean inode. >> >> The purpose of doing this is to keep compatible >> behavior with before because without this change >> dropping overlayfs inode will not trigger syncing >> of underlying dirty inode. >> >> Signed-off-by: Chengguang Xu <cgxu519@mykernel.net> >> --- >> fs/overlayfs/super.c | 16 +++++++++++++++- >> 1 file changed, 15 insertions(+), 1 deletion(-) >> >> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c >> index cddae3ca2fa5..bf4000eb9be8 100644 >> --- a/fs/overlayfs/super.c >> +++ b/fs/overlayfs/super.c >> @@ -441,11 +441,25 @@ static int ovl_write_inode(struct inode *inode, >> return ret; >> } >> >> +/* >> + * In iput_final(), clean inode will drop directly and dirty inode will >> + * keep in the cache until write back to sync dirty data then add to lru >> + * list to wait reclaim. >> + */ >> +static int ovl_drop_inode(struct inode *inode) >> +{ >> + struct inode *upper = ovl_inode_upper(inode); >> + >> + if (!upper || !(inode->i_state & I_DIRTY_ALL)) > Could we check upper dirtyness here? That would give a more precise result. We keep tracking mmapped-file(shared mode) by explicitely marking overlay inode dirty, so if we drop overlay inode by checking upper dirtyness, we may lose control on those mmapped upper inodes. > > Alternatively don't set .drop_inode (i.e. use generic_drop_inode()) > and set I_DONTCACHE on overlay inodes. That would cause the upper > inode to be always written back before eviction. > > The latter would result in simpler logic, and I think performance-wise > it wouldn't matter. But I may be missing something. I think we may seperate mmapped-file(shared) inode and other inode by clear/set I_DONTCACHE flag on overlay inode if you prefer this approach. Thanks, Chengguang
On Thu, 7 Oct 2021 at 14:04, Chengguang Xu <cgxu519@139.com> wrote: > > 在 2021/10/7 19:09, Miklos Szeredi 写道: > > On Thu, 23 Sept 2021 at 15:08, Chengguang Xu <cgxu519@mykernel.net> wrote: > >> Now drop overlayfs' inode will sync dirty data, > >> so we change to only drop clean inode. > >> > >> The purpose of doing this is to keep compatible > >> behavior with before because without this change > >> dropping overlayfs inode will not trigger syncing > >> of underlying dirty inode. > >> > >> Signed-off-by: Chengguang Xu <cgxu519@mykernel.net> > >> --- > >> fs/overlayfs/super.c | 16 +++++++++++++++- > >> 1 file changed, 15 insertions(+), 1 deletion(-) > >> > >> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c > >> index cddae3ca2fa5..bf4000eb9be8 100644 > >> --- a/fs/overlayfs/super.c > >> +++ b/fs/overlayfs/super.c > >> @@ -441,11 +441,25 @@ static int ovl_write_inode(struct inode *inode, > >> return ret; > >> } > >> > >> +/* > >> + * In iput_final(), clean inode will drop directly and dirty inode will > >> + * keep in the cache until write back to sync dirty data then add to lru > >> + * list to wait reclaim. > >> + */ > >> +static int ovl_drop_inode(struct inode *inode) > >> +{ > >> + struct inode *upper = ovl_inode_upper(inode); > >> + > >> + if (!upper || !(inode->i_state & I_DIRTY_ALL)) > > Could we check upper dirtyness here? That would give a more precise result. > > We keep tracking mmapped-file(shared mode) by explicitely marking > overlay inode dirty, > > so if we drop overlay inode by checking upper dirtyness, we may lose > control on those mmapped upper inodes. That's fine, since there are no more mmaps at this point. > > > > Alternatively don't set .drop_inode (i.e. use generic_drop_inode()) > > and set I_DONTCACHE on overlay inodes. That would cause the upper > > inode to be always written back before eviction. > > > > The latter would result in simpler logic, and I think performance-wise > > it wouldn't matter. But I may be missing something. > > I think we may seperate mmapped-file(shared) inode and other inode by > > clear/set I_DONTCACHE flag on overlay inode if you prefer this approach. Same reasoning here: after upper inode is written out, the dirtyness in the overlay inode doesn't matter since there cannot be any active mmaps. Thanks, Miklos
diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index cddae3ca2fa5..bf4000eb9be8 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -441,11 +441,25 @@ static int ovl_write_inode(struct inode *inode, return ret; } +/* + * In iput_final(), clean inode will drop directly and dirty inode will + * keep in the cache until write back to sync dirty data then add to lru + * list to wait reclaim. + */ +static int ovl_drop_inode(struct inode *inode) +{ + struct inode *upper = ovl_inode_upper(inode); + + if (!upper || !(inode->i_state & I_DIRTY_ALL)) + return 1; + return generic_drop_inode(inode); +} + static const struct super_operations ovl_super_operations = { .alloc_inode = ovl_alloc_inode, .free_inode = ovl_free_inode, .destroy_inode = ovl_destroy_inode, - .drop_inode = generic_delete_inode, + .drop_inode = ovl_drop_inode, .evict_inode = ovl_evict_inode, .write_inode = ovl_write_inode, .put_super = ovl_put_super,
Now drop overlayfs' inode will sync dirty data, so we change to only drop clean inode. The purpose of doing this is to keep compatible behavior with before because without this change dropping overlayfs inode will not trigger syncing of underlying dirty inode. Signed-off-by: Chengguang Xu <cgxu519@mykernel.net> --- fs/overlayfs/super.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)