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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20110518080930.GC25632@quack.suse.cz>
Date:	Wed, 18 May 2011 10:09:30 +0200
From:	Jan Kara <jack@...e.cz>
To:	Manish Katiyar <mkatiyar@...il.com>
Cc:	Jan Kara <jack@...e.cz>, ext4 <linux-ext4@...r.kernel.org>,
	Theodore Ts'o <tytso@....edu>
Subject: Re: [PATCH 1/5] jbd2: Pass extra bool parameter in journal
 routines to specify if its ok to fail the journal transaction allocation.

On Tue 17-05-11 23:38:05, Manish Katiyar wrote:
> On Mon, May 16, 2011 at 8:51 AM, Jan Kara <jack@...e.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@...e.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@...il.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@...e.cz>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html