From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751567AbZEAFnT (ORCPT ); Fri, 1 May 2009 01:43:19 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1750778AbZEAFnG (ORCPT ); Fri, 1 May 2009 01:43:06 -0400 Received: from mx2.mail.elte.hu ([157.181.151.9]:52683 "EHLO mx2.mail.elte.hu" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750720AbZEAFnF (ORCPT ); Fri, 1 May 2009 01:43:05 -0400 Date: Fri, 1 May 2009 07:42:47 +0200 From: Ingo Molnar To: Frederic Weisbecker Cc: LKML , Jeff Mahoney , ReiserFS Development List , Chris Mason , Alexander Beregalov , Alessio Igor Bogani , Jonathan Corbet , Alexander Viro Subject: Re: [PATCH 6/6] kill-the-BKL/reiserfs: release the write lock on flush_commit_list() Message-ID: <20090501054247.GD5983@elte.hu> References: <1241145862-21700-1-git-send-email-fweisbec@gmail.com> <1241145862-21700-7-git-send-email-fweisbec@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1241145862-21700-7-git-send-email-fweisbec@gmail.com> User-Agent: Mutt/1.5.18 (2008-05-17) X-ELTE-VirusStatus: clean X-ELTE-SpamScore: -1.5 X-ELTE-SpamLevel: X-ELTE-SpamCheck: no X-ELTE-SpamVersion: ELTE 2.0 X-ELTE-SpamCheck-Details: score=-1.5 required=5.9 tests=BAYES_00 autolearn=no SpamAssassin version=3.2.3 -1.5 BAYES_00 BODY: Bayesian spam probability is 0 to 1% [score: 0.0000] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Frederic Weisbecker wrote: > flush_commit_list() uses ll_rw_block() to commit the pending log blocks. > ll_rw_block() might sleep, and the bkl was released at this point. Then > we can also relax the write lock at this point. > > [ Impact: release the reiserfs write lock when it is not needed ] > > Cc: Jeff Mahoney > Cc: Chris Mason > Cc: Alexander Beregalov > Signed-off-by: Frederic Weisbecker > --- > fs/reiserfs/journal.c | 7 +++++-- > 1 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/fs/reiserfs/journal.c b/fs/reiserfs/journal.c > index 373d080..b1ebd5a 100644 > --- a/fs/reiserfs/journal.c > +++ b/fs/reiserfs/journal.c > @@ -1120,8 +1120,11 @@ static int flush_commit_list(struct super_block *s, > SB_ONDISK_JOURNAL_SIZE(s); > tbh = journal_find_get_block(s, bn); > if (tbh) { > - if (buffer_dirty(tbh)) > - ll_rw_block(WRITE, 1, &tbh) ; > + if (buffer_dirty(tbh)) { > + reiserfs_write_unlock(s); > + ll_rw_block(WRITE, 1, &tbh); > + reiserfs_write_lock(s); > + } > put_bh(tbh) ; > } > } there's 7 other instances of ll_rw_block(): fs/reiserfs/journal.c- spin_unlock(lock); fs/reiserfs/journal.c: ll_rw_block(WRITE, 1, &bh); fs/reiserfs/journal.c- spin_lock(lock); -- fs/reiserfs/journal.c- reiserfs_write_unlock(s); fs/reiserfs/journal.c: ll_rw_block(WRITE, 1, &tbh); fs/reiserfs/journal.c- reiserfs_write_lock(s); -- fs/reiserfs/journal.c- /* read in the log blocks, memcpy to the corresponding real block */ fs/reiserfs/journal.c: ll_rw_block(READ, get_desc_trans_len(desc), log_blocks); fs/reiserfs/journal.c- for (i = 0; i < get_desc_trans_len(desc); i++) { -- fs/reiserfs/journal.c- set_buffer_dirty(real_blocks[i]); fs/reiserfs/journal.c: ll_rw_block(SWRITE, 1, real_blocks + i); fs/reiserfs/journal.c- } -- fs/reiserfs/journal.c- } fs/reiserfs/journal.c: ll_rw_block(READ, j, bhlist); fs/reiserfs/journal.c- for (i = 1; i < j; i++) -- fs/reiserfs/stree.c- if (!buffer_uptodate(bh[j])) fs/reiserfs/stree.c: ll_rw_block(READA, 1, bh + j); fs/reiserfs/stree.c- brelse(bh[j]); -- fs/reiserfs/stree.c- reada_blocks, reada_count); fs/reiserfs/stree.c: ll_rw_block(READ, 1, &bh); fs/reiserfs/stree.c- reiserfs_write_unlock(sb); -- fs/reiserfs/super.c-{ fs/reiserfs/super.c: ll_rw_block(READ, 1, &(SB_BUFFER_WITH_SB(s))); fs/reiserfs/super.c- reiserfs_write_unlock(s); in particular the second stree.c one and the super.c has a write-unlock straight before the lock-drop. I think the stree.c unlock could be moved to before the ll_rw_block() call straight away. The super.c one needs more care: first put &(SB_BUFFER_WITH_SB(s)) into a local variable, then unlock the wite-lock, then call ll_rw_block(). (This is important because &(SB_BUFFER_WITH_SB(s)) is global filesystem state that has to be read with the lock held.) ll_rw_block() generally always has a chance to block (especially on READ) - so the other places could be converted to drop the write-lock too. Most seem straightforward - some need similar local-variable treatment as super.c. Ingo