All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Kara <jack@suse.cz>
To: Manish Katiyar <mkatiyar@gmail.com>
Cc: jack@suse.cz, tytso@mit.edu, linux-ext4@vger.kernel.org,
	mfasheh@suse.com, jlbec@evilplan.org
Subject: Re: [PATCH v2 2/3] jbd2: Add extra parameter in start_this_handle() to control allocation flags.
Date: Tue, 31 May 2011 13:22:53 +0200	[thread overview]
Message-ID: <20110531112253.GD5614@quack.suse.cz> (raw)
In-Reply-To: <1306563657-4334-1-git-send-email-mkatiyar@gmail.com>

On Fri 27-05-11 23:20:57, Manish Katiyar wrote:
> changes from v1 -> v2 :
> *) Update start_this_handle to take extra parameter to specify whether
> to retry the allocation or not.
> *) Added jbd allocation flags for callers to control the transaction allocation
> behavior. Callers can pass JBD2_TOPLEVEL if allocation needs to be done using GFP_KERNEL.
  The above changelog should be below where (*) is.

Also - this is mainly for Ted: I've looked at where we JBD2_TOPLEVEL could
actually be enabled and the results are: Pretty much nowhere.

The problem is that with ext4, we need i_mutex in io completion path to
end page writeback. So we cannot do GFP_KERNEL allocation whenever we hold
i_mutex because mm might wait in direct reclaim for IO to complete and that
cannot happen until we release i_mutex. And pretty much every write path in
ext4 holds i_mutex.

So JBD2_TOPLEVEL looks like a useless excercise to me and I'd just don't do
it.

> Pass extra flags in journal routines to specify if its ok to
> fail in the journal transaction allocation. Passing JBD2_FAIL_OK means caller is
> ok with journal start failures and can handle ENOMEM.
> 
> Update ocfs2 and ext4 routines to pass JBD2_NO_FAIL for the updated journal
> interface by default, to retain the existing behavior.
> 
> Signed-off-by: Manish Katiyar <mkatiyar@gmail.com>
> ---
>  fs/ext4/ext4_jbd2.h   |    2 +-
>  fs/ext4/super.c       |    2 +-
>  fs/jbd2/transaction.c |   44 ++++++++++++++++----------------------------
>  fs/ocfs2/journal.c    |    8 ++++----
>  include/linux/jbd2.h  |   13 +++++++++----
>  5 files changed, 31 insertions(+), 38 deletions(-)
  (*) HERE


> +/* JBD2 transaction allocation flags */
> +#define JBD2_NO_FAIL	0x00000001
> +#define JBD2_FAIL_OK	0x00000002
> +#define JBD2_TOPLEVEL	0x00000004
> +
  I guess there's no need for JBD2_FAIL_OK - if NOFAIL is not set, we can
fail. Otherwise the patch looks OK.

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

  reply	other threads:[~2011-05-31 11:22 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-28  6:16 [PATCH v2 1/3] ext4: Fix missing acl release in error path in acl.c Manish Katiyar
2011-05-28  6:20 ` [PATCH v2 2/3] jbd2: Add extra parameter in start_this_handle() to control allocation flags Manish Katiyar
2011-05-31 11:22   ` Jan Kara [this message]
2011-05-31 22:27     ` Ted Ts'o
2011-06-02  9:54       ` Jan Kara
2011-06-06  0:12         ` Manish Katiyar
2011-06-06  3:21         ` Ted Ts'o
2011-06-08 14:10           ` Ted Ts'o
2011-06-17  6:32             ` Manish Katiyar
2011-06-20 14:32               ` Jan Kara
2011-06-20 14:40                 ` Manish Katiyar
2011-06-20 17:57                   ` Ted Ts'o
2011-06-20 18:08                     ` Manish Katiyar
2011-05-28  6:21 ` [PATCH v2 3/3] ext4: Fix ext4 to use pass jbd allocation flags if they can handle ENOMEM Manish Katiyar
2011-05-31 12:10   ` Jan Kara

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110531112253.GD5614@quack.suse.cz \
    --to=jack@suse.cz \
    --cc=jlbec@evilplan.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=mfasheh@suse.com \
    --cc=mkatiyar@gmail.com \
    --cc=tytso@mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.