From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kara Subject: Re: [PATCH 1/1] writeback fix bdi thread race in mark_inode_dirty Date: Mon, 22 Jul 2013 15:32:56 +0200 Message-ID: <20130722133256.GF23658@quack.suse.cz> References: <1374257140-26729-1-git-send-email-srinivas.eeda@oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: linux-fsdevel@vger.kernel.org, jack@suse.cz, axboe@kernel.dk To: Srinivas Eeda Return-path: Received: from cantor2.suse.de ([195.135.220.15]:51735 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752301Ab3GVNc7 (ORCPT ); Mon, 22 Jul 2013 09:32:59 -0400 Content-Disposition: inline In-Reply-To: <1374257140-26729-1-git-send-email-srinivas.eeda@oracle.com> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Fri 19-07-13 11:05:40, Srinivas Eeda wrote: > In __mark_inode_dirty, a process checks !wb_has_dirty_io outside of list_lock > spinlock. This could cause a race, where process sees that list has dirty io > and decides not wake up bdi thread and waits for spinlock to add to dirty list. > Right at this time bdi_writeback_workfn finished write-back on last inode. > It sees the list is empty and ends. Process could now get the spinlock and > add inode to dirty list and doesn't wakeup bdi thread. Future calls to > __mark_inode_dirty also do not wake up the thread because list is not empty > any more. > > Fix is to get wb.list_lock spinlock before checking the dirty list > > Signed-off-by: Srinivas Eeda Good catch! The patch looks good. You can add: Reviewed-by: Jan Kara Honza > --- > fs/fs-writeback.c | 5 +++-- > 1 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c > index 68851ff..a5abc6c 100644 > --- a/fs/fs-writeback.c > +++ b/fs/fs-writeback.c > @@ -1173,6 +1173,9 @@ void __mark_inode_dirty(struct inode *inode, int flags) > bool wakeup_bdi = false; > bdi = inode_to_bdi(inode); > > + spin_unlock(&inode->i_lock); > + spin_lock(&bdi->wb.list_lock); > + > if (bdi_cap_writeback_dirty(bdi)) { > WARN(!test_bit(BDI_registered, &bdi->state), > "bdi-%s not registered\n", bdi->name); > @@ -1187,8 +1190,6 @@ void __mark_inode_dirty(struct inode *inode, int flags) > wakeup_bdi = true; > } > > - spin_unlock(&inode->i_lock); > - spin_lock(&bdi->wb.list_lock); > inode->dirtied_when = jiffies; > list_move(&inode->i_wb_list, &bdi->wb.b_dirty); > spin_unlock(&bdi->wb.list_lock); > -- > 1.5.6.5 > -- Jan Kara SUSE Labs, CR