Linux-Fsdevel Archive on lore.kernel.org
 help / color / Atom feed
From: Chengguang Xu <cgxu519@mykernel.net>
To: "Amir Goldstein" <amir73il@gmail.com>
Cc: "miklos" <miklos@szeredi.hu>, "jack" <jack@suse.cz>,
	"linux-unionfs" <linux-unionfs@vger.kernel.org>,
	"linux-fsdevel" <linux-fsdevel@vger.kernel.org>
Subject: Re: [RFC PATCH v3 09/10] ovl: introduce helper of syncfs writeback inode waiting
Date: Mon, 09 Nov 2020 16:34:10 +0800
Message-ID: <175ac242078.1287a39451704.7442694321257329129@mykernel.net> (raw)
In-Reply-To: <CAOQ4uxhVQC_PDPaYvO9KTSJ6Vrnds-yHmsyt631TSkBq6kqQ5g@mail.gmail.com>

 ---- 在 星期一, 2020-11-09 15:07:18 Amir Goldstein <amir73il@gmail.com> 撰写 ----
 > On Mon, Nov 9, 2020 at 5:34 AM Chengguang Xu <cgxu519@mykernel.net> wrote:
 > >
 > >  ---- 在 星期日, 2020-11-08 22:03:06 Chengguang Xu <cgxu519@mykernel.net> 撰写 ----
 > >  > Introduce a helper ovl_wait_wb_inodes() to wait until all
 > >  > target upper inodes finish writeback.
 > >  >
 > >  > Signed-off-by: Chengguang Xu <cgxu519@mykernel.net>
 > >  > ---
 > >  >  fs/overlayfs/super.c | 30 ++++++++++++++++++++++++++++++
 > >  >  1 file changed, 30 insertions(+)
 > >  >
 > >  > diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
 > >  > index e5607a908d82..9a535fc11221 100644
 > >  > --- a/fs/overlayfs/super.c
 > >  > +++ b/fs/overlayfs/super.c
 > >  > @@ -255,6 +255,36 @@ static void ovl_put_super(struct super_block *sb)
 > >  >      ovl_free_fs(ofs);
 > >  >  }
 > >  >
 > >  > +void ovl_wait_wb_inodes(struct ovl_fs *ofs)
 > >  > +{
 > >  > +    LIST_HEAD(tmp_list);
 > >  > +    struct ovl_inode *oi;
 > >  > +    struct inode *upper;
 > >  > +
 > >  > +    spin_lock(&ofs->syncfs_wait_list_lock);
 > >  > +    list_splice_init(&ofs->syncfs_wait_list, &tmp_list);
 > >  > +
 > >  > +    while (!list_empty(&tmp_list)) {
 > >  > +        oi = list_first_entry(&tmp_list, struct ovl_inode, wait_list);
 > >  > +        list_del_init(&oi->wait_list);
 > >  > +        ihold(&oi->vfs_inode);
 > >
 > > Maybe I overlooked race condition with inode eviction, so still need to introduce
 > > OVL_EVICT_PENDING flag just like we did in old syncfs efficiency patch series.
 > >
 > 
 > I am not sure why you added the ovl wait list.
 > 
 > I think you misunderstood Jan's suggestion.
 > I think what Jan meant is that ovl_sync_fs() should call
 > wait_sb_inodes(upper_sb)
 > to wait for writeback of ALL upper inodes after sync_filesystem()
 > started writeback
 > only on this ovl instance upper inodes.
 > 


Maybe you are right, the wait list is just for accuracy that can completely
avoid interferes between ovl instances, otherwise we may need to face
waiting interferes  in high density environment. 


 > I am not sure if this is acceptable or not - it is certainly an improvement over
 > current situation, but I have a feeling that on a large scale (many
 > containers) it
 > won't be enough.
 > 

The same as your thought. 


 > The idea was to keep it simple without over optimizing, since anyway
 > you are going for the "correct" solution long term (ovl inode aops),
 > so I wouldn't
 > add the wait list.
 > 

Maybe, I think it depends on how to implement ovl page-cache, so at current
stage I have no idea for the wait list.


 > As long as the upper inode is still dirty, we can keep the ovl inode in cache,
 > so the worst outcome is that drop_caches needs to get called twice before the
 > ovl inode can be evicted, no?
 > 
 
IIUC, since currently ovl does not have it's own page-cache, so there is no affect to page-cache reclaim, 
also  there is no ovl shrinker to reclaim slab because we drop ovl inode directly after final iput. 
So should we add a shrinker in this series?


Thanks,
Chengguang




  reply index

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-08 14:02 [RFC PATCH v3 00/10] implement containerized syncfs for overlayfs Chengguang Xu
2020-11-08 14:02 ` [RFC PATCH v3 01/10] ovl: setup overlayfs' private bdi Chengguang Xu
2020-11-08 14:02 ` [RFC PATCH v3 02/10] ovl: introduce waiting list for syncfs Chengguang Xu
2020-11-08 14:03 ` [RFC PATCH v3 03/10] ovl: implement ->writepages operation Chengguang Xu
2020-11-08 14:03 ` [RFC PATCH v3 04/10] ovl: implement overlayfs' ->evict_inode operation Chengguang Xu
2020-11-08 14:03 ` [RFC PATCH v3 05/10] ovl: mark overlayfs' inode dirty on modification Chengguang Xu
2020-11-08 14:03 ` [RFC PATCH v3 06/10] ovl: mark overlayfs' inode dirty on shared mmap Chengguang Xu
2020-11-11 13:05   ` 回复:[RFC " Chengguang Xu
2020-11-11 15:20     ` [RFC " Amir Goldstein
2020-11-11 16:09       ` Chengguang Xu
2020-11-08 14:03 ` [RFC PATCH v3 07/10] ovl: implement overlayfs' ->write_inode operation Chengguang Xu
2020-11-10 13:45   ` Jan Kara
2020-11-10 15:12     ` Chengguang Xu
2020-11-11 10:54       ` Jan Kara
2020-11-10 16:18     ` Amir Goldstein
2020-11-08 14:03 ` [RFC PATCH v3 08/10] ovl: cache dirty overlayfs' inode Chengguang Xu
2020-11-08 14:03 ` [RFC PATCH v3 09/10] ovl: introduce helper of syncfs writeback inode waiting Chengguang Xu
2020-11-09  3:33   ` 回复:[RFC " Chengguang Xu
2020-11-09  7:07     ` [RFC " Amir Goldstein
2020-11-09  8:34       ` Chengguang Xu [this message]
2020-11-09 10:07         ` Amir Goldstein
2020-11-09 12:06           ` Chengguang Xu
2020-11-08 14:03 ` [RFC PATCH v3 10/10] ovl: implement containerized syncfs for overlayfs Chengguang Xu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=175ac242078.1287a39451704.7442694321257329129@mykernel.net \
    --to=cgxu519@mykernel.net \
    --cc=amir73il@gmail.com \
    --cc=jack@suse.cz \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-unionfs@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Fsdevel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-fsdevel/0 linux-fsdevel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-fsdevel linux-fsdevel/ https://lore.kernel.org/linux-fsdevel \
		linux-fsdevel@vger.kernel.org
	public-inbox-index linux-fsdevel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-fsdevel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git