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