Linux-Fsdevel Archive on lore.kernel.org
 help / color / Atom feed
From: Amir Goldstein <amir73il@gmail.com>
To: Chengguang Xu <cgxu519@mykernel.net>
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, 9 Nov 2020 09:07:18 +0200
Message-ID: <CAOQ4uxhVQC_PDPaYvO9KTSJ6Vrnds-yHmsyt631TSkBq6kqQ5g@mail.gmail.com> (raw)
In-Reply-To: <175ab1145ed.108462b5a912.9181293177019474923@mykernel.net>

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.

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 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.

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?

Thanks,
Amir.

  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     ` Amir Goldstein [this message]
2020-11-09  8:34       ` [RFC " Chengguang Xu
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=CAOQ4uxhVQC_PDPaYvO9KTSJ6Vrnds-yHmsyt631TSkBq6kqQ5g@mail.gmail.com \
    --to=amir73il@gmail.com \
    --cc=cgxu519@mykernel.net \
    --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