From: Tejun Heo <tj@kernel.org> To: axboe@kernel.dk Cc: linux-kernel@vger.kernel.org, jack@suse.cz, hch@infradead.org, hannes@cmpxchg.org, linux-fsdevel@vger.kernel.org, vgoyal@redhat.com, lizefan@huawei.com, cgroups@vger.kernel.org, linux-mm@kvack.org, mhocko@suse.cz, clm@fb.com, fengguang.wu@intel.com, david@fromorbit.com, gthelen@google.com, khlebnikov@yandex-team.ru, Tejun Heo <tj@kernel.org> Subject: [PATCH 4/9] writeback: implement [locked_]inode_to_wb_and_lock_list() Date: Thu, 28 May 2015 14:50:52 -0400 [thread overview] Message-ID: <1432839057-17609-5-git-send-email-tj@kernel.org> (raw) In-Reply-To: <1432839057-17609-1-git-send-email-tj@kernel.org> cgroup writeback currently assumes that inode to wb association doesn't change; however, with the planned foreign inode wb switching mechanism, the association will change dynamically. When an inode needs to be put on one of the IO lists of its wb, the current code simply calls inode_to_wb() and locks the returned wb; however, with the planned wb switching, the association may change before locking the wb and may even get released. This patch implements [locked_]inode_to_wb_and_lock_list() which pins the associated wb while holding i_lock, releases it, acquires wb->list_lock and verifies that the association hasn't changed inbetween. As the association will be protected by both locks among other things, this guarantees that the wb is the inode's associated wb until the list_lock is released. Signed-off-by: Tejun Heo <tj@kernel.org> Cc: Jens Axboe <axboe@kernel.dk> Cc: Jan Kara <jack@suse.cz> Cc: Wu Fengguang <fengguang.wu@intel.com> Cc: Greg Thelen <gthelen@google.com> --- fs/fs-writeback.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 75 insertions(+), 5 deletions(-) diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c index f98d403..f67b956 100644 --- a/fs/fs-writeback.c +++ b/fs/fs-writeback.c @@ -259,6 +259,56 @@ void __inode_attach_wb(struct inode *inode, struct page *page) } /** + * locked_inode_to_wb_and_lock_list - determine a locked inode's wb and lock it + * @inode: inode of interest with i_lock held + * + * Returns @inode's wb with its list_lock held. @inode->i_lock must be + * held on entry and is released on return. The returned wb is guaranteed + * to stay @inode's associated wb until its list_lock is released. + */ +static struct bdi_writeback * +locked_inode_to_wb_and_lock_list(struct inode *inode) + __releases(&inode->i_lock) + __acquires(&wb->list_lock) +{ + while (true) { + struct bdi_writeback *wb = inode_to_wb(inode); + + /* + * inode_to_wb() association is protected by both + * @inode->i_lock and @wb->list_lock but list_lock nests + * outside i_lock. Drop i_lock and verify that the + * association hasn't changed after acquiring list_lock. + */ + wb_get(wb); + spin_unlock(&inode->i_lock); + spin_lock(&wb->list_lock); + wb_put(wb); /* not gonna deref it anymore */ + + if (likely(wb == inode_to_wb(inode))) + return wb; /* @inode already has ref */ + + spin_unlock(&wb->list_lock); + cpu_relax(); + spin_lock(&inode->i_lock); + } +} + +/** + * inode_to_wb_and_lock_list - determine an inode's wb and lock it + * @inode: inode of interest + * + * Same as locked_inode_to_wb_and_lock_list() but @inode->i_lock isn't held + * on entry. + */ +static struct bdi_writeback *inode_to_wb_and_lock_list(struct inode *inode) + __acquires(&wb->list_lock) +{ + spin_lock(&inode->i_lock); + return locked_inode_to_wb_and_lock_list(inode); +} + +/** * wbc_attach_and_unlock_inode - associate wbc with target inode and unlock it * @wbc: writeback_control of interest * @inode: target inode @@ -603,6 +653,27 @@ static void bdi_split_work_to_wbs(struct backing_dev_info *bdi, #else /* CONFIG_CGROUP_WRITEBACK */ +static struct bdi_writeback * +locked_inode_to_wb_and_lock_list(struct inode *inode) + __releases(&inode->i_lock) + __acquires(&wb->list_lock) +{ + struct bdi_writeback *wb = inode_to_wb(inode); + + spin_unlock(&inode->i_lock); + spin_lock(&wb->list_lock); + return wb; +} + +static struct bdi_writeback *inode_to_wb_and_lock_list(struct inode *inode) + __acquires(&wb->list_lock) +{ + struct bdi_writeback *wb = inode_to_wb(inode); + + spin_lock(&wb->list_lock); + return wb; +} + static long wb_split_bdi_pages(struct bdi_writeback *wb, long nr_pages) { return nr_pages; @@ -678,9 +749,9 @@ void wb_start_background_writeback(struct bdi_writeback *wb) */ void inode_wb_list_del(struct inode *inode) { - struct bdi_writeback *wb = inode_to_wb(inode); + struct bdi_writeback *wb; - spin_lock(&wb->list_lock); + wb = inode_to_wb_and_lock_list(inode); inode_wb_list_del_locked(inode, wb); spin_unlock(&wb->list_lock); } @@ -1784,12 +1855,11 @@ void __mark_inode_dirty(struct inode *inode, int flags) * reposition it (that would break b_dirty time-ordering). */ if (!was_dirty) { - struct bdi_writeback *wb = inode_to_wb(inode); + struct bdi_writeback *wb; struct list_head *dirty_list; bool wakeup_bdi = false; - spin_unlock(&inode->i_lock); - spin_lock(&wb->list_lock); + wb = locked_inode_to_wb_and_lock_list(inode); WARN(bdi_cap_writeback_dirty(wb->bdi) && !test_bit(WB_registered, &wb->state), -- 2.4.0
WARNING: multiple messages have this Message-ID (diff)
From: Tejun Heo <tj@kernel.org> To: axboe@kernel.dk Cc: linux-kernel@vger.kernel.org, jack@suse.cz, hch@infradead.org, hannes@cmpxchg.org, linux-fsdevel@vger.kernel.org, vgoyal@redhat.com, lizefan@huawei.com, cgroups@vger.kernel.org, linux-mm@kvack.org, mhocko@suse.cz, clm@fb.com, fengguang.wu@intel.com, david@fromorbit.com, gthelen@google.com, khlebnikov@yandex-team.ru, Tejun Heo <tj@kernel.org> Subject: [PATCH 4/9] writeback: implement [locked_]inode_to_wb_and_lock_list() Date: Thu, 28 May 2015 14:50:52 -0400 [thread overview] Message-ID: <1432839057-17609-5-git-send-email-tj@kernel.org> (raw) In-Reply-To: <1432839057-17609-1-git-send-email-tj@kernel.org> cgroup writeback currently assumes that inode to wb association doesn't change; however, with the planned foreign inode wb switching mechanism, the association will change dynamically. When an inode needs to be put on one of the IO lists of its wb, the current code simply calls inode_to_wb() and locks the returned wb; however, with the planned wb switching, the association may change before locking the wb and may even get released. This patch implements [locked_]inode_to_wb_and_lock_list() which pins the associated wb while holding i_lock, releases it, acquires wb->list_lock and verifies that the association hasn't changed inbetween. As the association will be protected by both locks among other things, this guarantees that the wb is the inode's associated wb until the list_lock is released. Signed-off-by: Tejun Heo <tj@kernel.org> Cc: Jens Axboe <axboe@kernel.dk> Cc: Jan Kara <jack@suse.cz> Cc: Wu Fengguang <fengguang.wu@intel.com> Cc: Greg Thelen <gthelen@google.com> --- fs/fs-writeback.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 75 insertions(+), 5 deletions(-) diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c index f98d403..f67b956 100644 --- a/fs/fs-writeback.c +++ b/fs/fs-writeback.c @@ -259,6 +259,56 @@ void __inode_attach_wb(struct inode *inode, struct page *page) } /** + * locked_inode_to_wb_and_lock_list - determine a locked inode's wb and lock it + * @inode: inode of interest with i_lock held + * + * Returns @inode's wb with its list_lock held. @inode->i_lock must be + * held on entry and is released on return. The returned wb is guaranteed + * to stay @inode's associated wb until its list_lock is released. + */ +static struct bdi_writeback * +locked_inode_to_wb_and_lock_list(struct inode *inode) + __releases(&inode->i_lock) + __acquires(&wb->list_lock) +{ + while (true) { + struct bdi_writeback *wb = inode_to_wb(inode); + + /* + * inode_to_wb() association is protected by both + * @inode->i_lock and @wb->list_lock but list_lock nests + * outside i_lock. Drop i_lock and verify that the + * association hasn't changed after acquiring list_lock. + */ + wb_get(wb); + spin_unlock(&inode->i_lock); + spin_lock(&wb->list_lock); + wb_put(wb); /* not gonna deref it anymore */ + + if (likely(wb == inode_to_wb(inode))) + return wb; /* @inode already has ref */ + + spin_unlock(&wb->list_lock); + cpu_relax(); + spin_lock(&inode->i_lock); + } +} + +/** + * inode_to_wb_and_lock_list - determine an inode's wb and lock it + * @inode: inode of interest + * + * Same as locked_inode_to_wb_and_lock_list() but @inode->i_lock isn't held + * on entry. + */ +static struct bdi_writeback *inode_to_wb_and_lock_list(struct inode *inode) + __acquires(&wb->list_lock) +{ + spin_lock(&inode->i_lock); + return locked_inode_to_wb_and_lock_list(inode); +} + +/** * wbc_attach_and_unlock_inode - associate wbc with target inode and unlock it * @wbc: writeback_control of interest * @inode: target inode @@ -603,6 +653,27 @@ static void bdi_split_work_to_wbs(struct backing_dev_info *bdi, #else /* CONFIG_CGROUP_WRITEBACK */ +static struct bdi_writeback * +locked_inode_to_wb_and_lock_list(struct inode *inode) + __releases(&inode->i_lock) + __acquires(&wb->list_lock) +{ + struct bdi_writeback *wb = inode_to_wb(inode); + + spin_unlock(&inode->i_lock); + spin_lock(&wb->list_lock); + return wb; +} + +static struct bdi_writeback *inode_to_wb_and_lock_list(struct inode *inode) + __acquires(&wb->list_lock) +{ + struct bdi_writeback *wb = inode_to_wb(inode); + + spin_lock(&wb->list_lock); + return wb; +} + static long wb_split_bdi_pages(struct bdi_writeback *wb, long nr_pages) { return nr_pages; @@ -678,9 +749,9 @@ void wb_start_background_writeback(struct bdi_writeback *wb) */ void inode_wb_list_del(struct inode *inode) { - struct bdi_writeback *wb = inode_to_wb(inode); + struct bdi_writeback *wb; - spin_lock(&wb->list_lock); + wb = inode_to_wb_and_lock_list(inode); inode_wb_list_del_locked(inode, wb); spin_unlock(&wb->list_lock); } @@ -1784,12 +1855,11 @@ void __mark_inode_dirty(struct inode *inode, int flags) * reposition it (that would break b_dirty time-ordering). */ if (!was_dirty) { - struct bdi_writeback *wb = inode_to_wb(inode); + struct bdi_writeback *wb; struct list_head *dirty_list; bool wakeup_bdi = false; - spin_unlock(&inode->i_lock); - spin_lock(&wb->list_lock); + wb = locked_inode_to_wb_and_lock_list(inode); WARN(bdi_cap_writeback_dirty(wb->bdi) && !test_bit(WB_registered, &wb->state), -- 2.4.0 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2015-05-28 18:53 UTC|newest] Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top 2015-05-28 18:50 [PATCHSET 3/3 v4 block/for-4.2/core] writeback: implement foreign cgroup inode bdi_writeback switching Tejun Heo 2015-05-28 18:50 ` Tejun Heo 2015-05-28 18:50 ` Tejun Heo 2015-05-28 18:50 ` [PATCH 1/9] writeback: relocate wb[_try]_get(), wb_put(), inode_{attach|detach}_wb() Tejun Heo 2015-05-28 18:50 ` Tejun Heo 2015-05-28 18:50 ` Tejun Heo 2015-05-28 18:50 ` [PATCH 2/9] writeback: make writeback_control track the inode being written back Tejun Heo 2015-05-28 18:50 ` Tejun Heo 2015-05-28 18:50 ` [PATCH 3/9] writeback: implement foreign cgroup inode detection Tejun Heo 2015-05-28 18:50 ` Tejun Heo 2015-05-28 18:50 ` Tejun Heo [this message] 2015-05-28 18:50 ` [PATCH 4/9] writeback: implement [locked_]inode_to_wb_and_lock_list() Tejun Heo 2015-05-28 18:50 ` [PATCH 5/9] writeback: implement unlocked_inode_to_wb transaction and use it for stat updates Tejun Heo 2015-05-28 18:50 ` Tejun Heo 2015-05-28 18:50 ` [PATCH 6/9] writeback: use unlocked_inode_to_wb transaction in inode_congested() Tejun Heo 2015-05-28 18:50 ` Tejun Heo 2015-05-28 18:50 ` [PATCH 7/9] writeback: add lockdep annotation to inode_to_wb() Tejun Heo 2015-05-28 18:50 ` Tejun Heo 2015-05-28 18:50 ` Tejun Heo 2015-05-28 18:50 ` [PATCH 8/9] writeback: implement foreign cgroup inode bdi_writeback switching Tejun Heo 2015-05-28 18:50 ` Tejun Heo 2015-05-28 18:50 ` [PATCH 9/9] writeback: disassociate inodes from dying bdi_writebacks Tejun Heo 2015-05-28 18:50 ` Tejun Heo -- strict thread matches above, loose matches on Subject: below -- 2015-05-22 22:36 [PATCHSET 3/3 v3 block/for-4.2/core] writeback: implement foreign cgroup inode bdi_writeback switching Tejun Heo 2015-05-22 22:36 ` [PATCH 4/9] writeback: implement [locked_]inode_to_wb_and_lock_list() Tejun Heo 2015-05-22 22:36 ` Tejun Heo
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=1432839057-17609-5-git-send-email-tj@kernel.org \ --to=tj@kernel.org \ --cc=axboe@kernel.dk \ --cc=cgroups@vger.kernel.org \ --cc=clm@fb.com \ --cc=david@fromorbit.com \ --cc=fengguang.wu@intel.com \ --cc=gthelen@google.com \ --cc=hannes@cmpxchg.org \ --cc=hch@infradead.org \ --cc=jack@suse.cz \ --cc=khlebnikov@yandex-team.ru \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=lizefan@huawei.com \ --cc=mhocko@suse.cz \ --cc=vgoyal@redhat.com \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.