On Fri 23-07-10 10:10:54, Ted Ts'o wrote:
> On Thu, Jul 22, 2010 at 04:24:23PM -0700, David Rientjes wrote:
> >
> > I didn't think about converting the existing GFP_NOFS | __GFP_NOFAIL
> > callers into the do-while loop above until you mentioned it, thanks. I'll
> > send patches to do that shortly.
...
> From 814be805d5e3d12343e590631ff9bc2d65c8f60a Mon Sep 17 00:00:00 2001
> From: Theodore Ts'o <tytso@xxxxxxx>
> Date: Fri, 23 Jul 2010 10:06:53 -0400
> Subject: [PATCH] jbd2: Remove __GFP_NOFAIL from jbd2 layer
>
> __GFP_NOFAIL is going away, so add our own retry loop. Also add
> jbd2__journal_start() and jbd2__journal_restart() which take a gfp
> mask, so that file systems can optionally (re)start transaction
> handles using GFP_KERNEL. If they do this, then they need to be
> prepared to handle receiving an PTR_ERR(-ENOMEM) error, and be ready
> to reflect that error up to userspace.
>
> Signed-off-by: "Theodore Ts'o" <tytso@xxxxxxx>
> ---
> fs/jbd2/journal.c | 14 +++++++++--
> fs/jbd2/transaction.c | 60 +++++++++++++++++++++++++++++++++---------------
> include/linux/jbd2.h | 4 ++-
> 3 files changed, 55 insertions(+), 23 deletions(-)
>
> diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
> index e214d68..43241c0 100644
> --- a/fs/jbd2/transaction.c
> +++ b/fs/jbd2/transaction.c
> @@ -83,30 +83,39 @@ jbd2_get_transaction(journal_t *journal, transaction_t *transaction)
> * transaction's buffer credits.
> */
>
> -static int start_this_handle(journal_t *journal, handle_t *handle)
> +static int start_this_handle(journal_t *journal, handle_t *handle,
> + int gfp_mask)
> {
> transaction_t *transaction;
> int needed;
> int nblocks = handle->h_buffer_credits;
> transaction_t *new_transaction = NULL;
> - int ret = 0;
> unsigned long ts = jiffies;
>
> if (nblocks > journal->j_max_transaction_buffers) {
> printk(KERN_ERR "JBD: %s wants too many credits (%d > %d)\n",
> current->comm, nblocks,
> journal->j_max_transaction_buffers);
> - ret = -ENOSPC;
> - goto out;
> + return -ENOSPC;
> }
>
> alloc_transaction:
> if (!journal->j_running_transaction) {
> - new_transaction = kzalloc(sizeof(*new_transaction),
> - GFP_NOFS|__GFP_NOFAIL);
> + retry_alloc:
> + new_transaction = kzalloc(sizeof(*new_transaction), gfp_mask);
> if (!new_transaction) {
> - ret = -ENOMEM;
> - goto out;
> + /*
> + * If __GFP_FS is not present, then we may be
> + * being called from inside the fs writeback
> + * layer, so we MUST NOT fail. Since
> + * __GFP_NOFAIL is going away, we will arrange
> + * to retry the allocation ourselves.
> + */
> + if ((gfp & __GFP_FS) == 0) {
> + congestion_wait(BLK_RW_ASYNC, HZ/50);
> + goto retry_alloc;
You could as well go to alloc_transaction label above...
> + }
> + return -ENOMEM;
> }
> }
>
...
> @@ -278,7 +285,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)
> +handle_t *jbd2__journal_start(journal_t *journal, int nblocks, int gfp_mask)
> {
> handle_t *handle = journal_current_handle();
> int err;
> @@ -298,7 +305,7 @@ handle_t *jbd2_journal_start(journal_t *journal, int nblocks)
>
> current->journal_info = handle;
>
> - err = start_this_handle(journal, handle);
> + err = start_this_handle(journal, handle, GFP_NOFS);
Here you want to use gfp_mask I guess.
> if (err < 0) {
> jbd2_free_handle(handle);
> current->journal_info = NULL;
> @@ -308,6 +315,15 @@ handle_t *jbd2_journal_start(journal_t *journal, int nblocks)
> out:
> 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);
> +
>
> /**
> * int jbd2_journal_extend() - extend buffer credits.
> @@ -394,8 +410,7 @@ out:
> * transaction capabable of guaranteeing the requested number of
> * credits.
> */
> -
> -int jbd2_journal_restart(handle_t *handle, int nblocks)
> +int jbd2__journal_restart(handle_t *handle, int nblocks, int gfp_mask)
> {
> transaction_t *transaction = handle->h_transaction;
> journal_t *journal = transaction->t_journal;
> @@ -428,10 +443,17 @@ int jbd2_journal_restart(handle_t *handle, int nblocks)
>
> lock_map_release(&handle->h_lockdep_map);
> handle->h_buffer_credits = nblocks;
> - ret = start_this_handle(journal, handle);
> + ret = start_this_handle(journal, handle, GFP_NOFS);
And here you want to use gfp_mask as well.
> 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);
>
> /**
> * void jbd2_journal_lock_updates () - establish a transaction barrier.
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
