Re: [PATCH 1/5] jbd2: Pass extra bool parameter in journal routines to specify if its ok to fail the journal transaction allocation.

[Jan Kara <jack@suse.cz>]

On Tue 17-05-11 23:38:05, Manish Katiyar wrote:
> On Mon, May 16, 2011 at 8:51 AM, Jan Kara <jack@suse.cz> wrote:
> >  Hello,
> >
> > On Thu 12-05-11 23:37:05, Manish Katiyar wrote:
> >> On Wed, May 11, 2011 at 8:54 AM, Jan Kara <jack@suse.cz> 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.
  Yes, now the patch looks good! Thanks. Just one nit below:

> 
> Signed-off-by: Manish Katiyar <mkatiyar@gmail.com>
> ---
>  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/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);
>  }
> 
> +
  This empty line was added by accident.

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html