From patchwork Wed May 18 06:38:05 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Manish Katiyar X-Patchwork-Id: 96131 Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id D10E6B6F5D for ; Wed, 18 May 2011 16:38:27 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754020Ab1ERGi1 (ORCPT ); Wed, 18 May 2011 02:38:27 -0400 Received: from mail-qw0-f46.google.com ([209.85.216.46]:62241 "EHLO mail-qw0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753982Ab1ERGi0 convert rfc822-to-8bit (ORCPT ); Wed, 18 May 2011 02:38:26 -0400 Received: by qwk3 with SMTP id 3so632705qwk.19 for ; Tue, 17 May 2011 23:38:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-type:content-transfer-encoding; bh=nZCctTqkoyrEx8PQ5giTj4vTtOV7QF15EvY/rb1GYiM=; b=ju1osq1hNgcQAubG/WWpic2pmMGx0aVDXQiEM/445h/Z8p68WpxbUb4/42Z2+58LOI Bcz0VXRuj8EByWw5TQoeA4WRUC8JlSl+m3NMKWyzDnN09+6kU4aSlIwKgQ0Ub95Koc/U V/uZGkR2orno7d0sgq8G7xH5qvNwTVP5/cFMc= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc:content-type:content-transfer-encoding; b=DG9NXbVMFTUJLuIUOWGbOnlbO6C7W6OpSOLN5SZVqN6iqQsZIb7qZabwSrmH/Buj6O /dJbA24psX4uCti8qWZy7OC2fE27VSEvJ8Oae9h81O78gMb8XtzXG9KtqH0081fACRob jkYfFloN0/28q9ejnHxdrxz6CeFPjJpzCP2E0= Received: by 10.224.137.5 with SMTP id u5mr1092044qat.97.1305700705202; Tue, 17 May 2011 23:38:25 -0700 (PDT) MIME-Version: 1.0 Received: by 10.229.230.85 with HTTP; Tue, 17 May 2011 23:38:05 -0700 (PDT) In-Reply-To: <20110516155138.GI5344@quack.suse.cz> References: <20110511155447.GH5057@quack.suse.cz> <20110516155138.GI5344@quack.suse.cz> From: Manish Katiyar Date: Tue, 17 May 2011 23:38:05 -0700 Message-ID: Subject: Re: [PATCH 1/5] jbd2: Pass extra bool parameter in journal routines to specify if its ok to fail the journal transaction allocation. To: Jan Kara Cc: ext4 , "Theodore Ts'o" Sender: linux-ext4-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-ext4@vger.kernel.org On Mon, May 16, 2011 at 8:51 AM, Jan Kara wrote: >  Hello, > > On Thu 12-05-11 23:37:05, Manish Katiyar wrote: >> On Wed, May 11, 2011 at 8:54 AM, Jan Kara wrote: >> >  Hi, >> > >> >  sorry I got to your patches with a delay. One general note - please do >> > not attach patches. It is enough to have them in the email... >> > >> > On Sun 24-04-11 17:10:41, Manish Katiyar wrote: >> >> Pass extra bool parameter in journal routines to specify if its ok to >> >> fail the journal transaction allocation. If 'true' is passed >> >> transaction allocation is done through GFP_KERNEL  and ENOMEM is >> >> returned else GFP_NOFS is used. >> >  Please, do not mix error handling with gfp masks. Instead just rename >> > jbd2__journal_start() to jbd2_journal_start() and change gfp_mask parameter >> > to "bool errok". >> >> ok. >> >> > Use GFP_NOFS gfp mask for start_this_handle(). >> I think I didn't completely understand this line. You meant passing >> GFP_KERNEL or GFP_NOFS based on errok right ? >  No, I meant passing GFP_NOFS always. Currently, GFP_NOFS is used in all > the cases (noone uses GFP_KERNEL variant) and GFP_KERNEL can really be used > only when we do not hold other filesystem locks (as GFP_KERNEL allocation > can recurse back into filesystem to reclaim memory). So using GFP_KERNEL > would need more auditting and is a separate issue anyway. Hi Jan, How about this ? As suggested I have removed special handlers for ocfs2, always pass false as default for allocation and have removed jbd2__journal_start and collapsed it in jbd2_journal_start. Pass extra bool parameter in journal routines to specify if its ok to fail the journal transaction allocation. Update ocfs2 and ext4 routines to pass false for the updated journal interface. Signed-off-by: Manish Katiyar --- fs/ext4/ext4_jbd2.h | 2 +- fs/ext4/super.c | 3 ++- fs/jbd2/transaction.c | 24 +++++------------------- fs/ocfs2/journal.c | 6 +++--- include/linux/jbd2.h | 6 ++---- 5 files changed, 13 insertions(+), 28 deletions(-) diff --git a/fs/ext4/ext4_jbd2.h b/fs/ext4/ext4_jbd2.h index d0f5353..0abee1b 100644 --- a/fs/ext4/ext4_jbd2.h +++ b/fs/ext4/ext4_jbd2.h @@ -225,7 +225,7 @@ static inline int ext4_journal_extend(handle_t *handle, int nblocks) static inline int ext4_journal_restart(handle_t *handle, int nblocks) { if (ext4_handle_valid(handle)) - return jbd2_journal_restart(handle, nblocks); + return jbd2_journal_restart(handle, nblocks, false); return 0; } diff --git a/fs/ext4/super.c b/fs/ext4/super.c index 8553dfb..c165ffe 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -279,9 +279,10 @@ handle_t *ext4_journal_start_sb(struct super_block *sb, int nblocks) ext4_abort(sb, "Detected aborted journal"); return ERR_PTR(-EROFS); } - return jbd2_journal_start(journal, nblocks); + return jbd2_journal_start(journal, nblocks, false); } + /* * The only special thing we need to do here is to make sure that all * jbd2_journal_stop calls result in the superblock being marked dirty, so diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c index 05fa77a..b5c2550 100644 --- a/fs/jbd2/transaction.c +++ b/fs/jbd2/transaction.c @@ -318,7 +318,7 @@ static handle_t *new_handle(int nblocks) * * Return a pointer to a newly allocated handle, or NULL on failure */ -handle_t *jbd2__journal_start(journal_t *journal, int nblocks, int gfp_mask) +handle_t *jbd2_journal_start(journal_t *journal, int nblocks, bool errok) { handle_t *handle = journal_current_handle(); int err; @@ -338,7 +338,7 @@ handle_t *jbd2__journal_start(journal_t *journal, int nblocks, int gfp_mask) current->journal_info = handle; - err = start_this_handle(journal, handle, gfp_mask); + err = start_this_handle(journal, handle, GFP_NOFS); if (err < 0) { jbd2_free_handle(handle); current->journal_info = NULL; @@ -346,13 +346,6 @@ handle_t *jbd2__journal_start(journal_t *journal, int nblocks, int gfp_mask) } return handle; } -EXPORT_SYMBOL(jbd2__journal_start); - - -handle_t *jbd2_journal_start(journal_t *journal, int nblocks) -{ - return jbd2__journal_start(journal, nblocks, GFP_NOFS); -} EXPORT_SYMBOL(jbd2_journal_start); @@ -441,7 +434,7 @@ out: * transaction capabable of guaranteeing the requested number of * credits. */ -int jbd2__journal_restart(handle_t *handle, int nblocks, int gfp_mask) +int jbd2_journal_restart(handle_t *handle, int nblocks, bool errok) { transaction_t *transaction = handle->h_transaction; journal_t *journal = transaction->t_journal; @@ -477,16 +470,9 @@ int jbd2__journal_restart(handle_t *handle, int nblocks, int gfp_mask) lock_map_release(&handle->h_lockdep_map); handle->h_buffer_credits = nblocks; - ret = start_this_handle(journal, handle, gfp_mask); + ret = start_this_handle(journal, handle, GFP_NOFS); return ret; } -EXPORT_SYMBOL(jbd2__journal_restart); - - -int jbd2_journal_restart(handle_t *handle, int nblocks) -{ - return jbd2__journal_restart(handle, nblocks, GFP_NOFS); -} EXPORT_SYMBOL(jbd2_journal_restart); /** @@ -1436,7 +1422,7 @@ int jbd2_journal_force_commit(journal_t *journal) handle_t *handle; int ret; - handle = jbd2_journal_start(journal, 1); + handle = jbd2_journal_start(journal, 1, false); if (IS_ERR(handle)) { ret = PTR_ERR(handle); } else { diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c index b141a44..cca0f76 100644 --- a/fs/ocfs2/journal.c +++ b/fs/ocfs2/journal.c @@ -353,11 +353,11 @@ handle_t *ocfs2_start_trans(struct ocfs2_super *osb, int max_buffs) /* Nested transaction? Just return the handle... */ if (journal_current_handle()) - return jbd2_journal_start(journal, max_buffs); + return jbd2_journal_start(journal, max_buffs, false); down_read(&osb->journal->j_trans_barrier); - handle = jbd2_journal_start(journal, max_buffs); + handle = jbd2_journal_start(journal, max_buffs, false); if (IS_ERR(handle)) { up_read(&osb->journal->j_trans_barrier); @@ -438,7 +438,7 @@ int ocfs2_extend_trans(handle_t *handle, int nblocks) if (status > 0) { trace_ocfs2_extend_trans_restart(old_nblocks + nblocks); status = jbd2_journal_restart(handle, - old_nblocks + nblocks); + old_nblocks + nblocks, false); if (status < 0) { mlog_errno(status); goto bail; diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h index a32dcae..67f0f4f 100644 --- a/include/linux/jbd2.h +++ b/include/linux/jbd2.h @@ -1103,10 +1103,8 @@ static inline handle_t *journal_current_handle(void) * Register buffer modifications against the current transaction. */ -extern handle_t *jbd2_journal_start(journal_t *, int nblocks); -extern handle_t *jbd2__journal_start(journal_t *, int nblocks, int gfp_mask); -extern int jbd2_journal_restart(handle_t *, int nblocks); -extern int jbd2__journal_restart(handle_t *, int nblocks, int gfp_mask); +extern handle_t *jbd2_journal_start(journal_t *, int nblocks, bool errok); +extern int jbd2_journal_restart(handle_t *, int nblocks, bool errok); extern int jbd2_journal_extend (handle_t *, int nblocks); extern int jbd2_journal_get_write_access(handle_t *, struct buffer_head *); extern int jbd2_journal_get_create_access (handle_t *, struct buffer_head *);