2011-06-11 09:36:53

by Manish Katiyar

[permalink] [raw]
Subject: [PATCH v2] jbd2:Make journal transaction allocations come from its own cache.

Add a cache for jbd2 journal transaction allocations. This also
helps to leverage fault-injection framework to test various memory
allocation failures in the jbd2 layer.

Signed-off-by: Manish Katiyar <[email protected]>
Acked-by: Jan Kara <[email protected]>

---
v1->v2 :
*) Drop SLAB_TEMPORARY flag for cache allocation.
*) Remove filling with pattern during frees.

fs/jbd2/checkpoint.c | 2 +-
fs/jbd2/commit.c | 2 +-
fs/jbd2/journal.c | 25 +++++++++++++++++++++++++
fs/jbd2/transaction.c | 7 ++++---
include/linux/jbd2.h | 17 +++++++++++++++++
5 files changed, 48 insertions(+), 5 deletions(-)

diff --git a/fs/jbd2/checkpoint.c b/fs/jbd2/checkpoint.c
index 6a79fd0..6f554ce 100644
--- a/fs/jbd2/checkpoint.c
+++ b/fs/jbd2/checkpoint.c
@@ -716,7 +716,7 @@ int __jbd2_journal_remove_checkpoint(struct journal_head *jh)
transaction->t_tid, stats);

__jbd2_journal_drop_transaction(journal, transaction);
- kfree(transaction);
+ jbd2_free_transaction(transaction);

/* Just in case anybody was waiting for more transactions to be
checkpointed... */
diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c
index 7f21cf3..8e33d84 100644
--- a/fs/jbd2/commit.c
+++ b/fs/jbd2/commit.c
@@ -1037,7 +1037,7 @@ restart_loop:
jbd_debug(1, "JBD: commit %d complete, head %d\n",
journal->j_commit_sequence, journal->j_tail_sequence);
if (to_free)
- kfree(commit_transaction);
+ jbd2_free_transaction(commit_transaction);

wake_up(&journal->j_wait_done_commit);
}
diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index 9a78269..61e0206 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -95,6 +95,7 @@ EXPORT_SYMBOL(jbd2_journal_init_jbd_inode);
EXPORT_SYMBOL(jbd2_journal_release_jbd_inode);
EXPORT_SYMBOL(jbd2_journal_begin_ordered_truncate);
EXPORT_SYMBOL(jbd2_inode_cache);
+EXPORT_SYMBOL(jbd2_transaction_cache);

static int journal_convert_superblock_v1(journal_t *, journal_superblock_t *);
static void __journal_abort_soft (journal_t *journal, int errno);
@@ -2371,6 +2372,27 @@ static void jbd2_journal_destroy_handle_cache(void)

}

+struct kmem_cache *jbd2_transaction_cache;
+
+static int journal_init_transaction_cache(void)
+{
+ J_ASSERT(jbd2_transaction_cache == NULL);
+ jbd2_transaction_cache = kmem_cache_create("jbd2_transaction",
+ sizeof(transaction_t),
+ 0, 0, NULL);
+ if (jbd2_transaction_cache == NULL) {
+ printk(KERN_EMERG "JBD2: failed to create transaction cache\n");
+ return -ENOMEM;
+ }
+ return 0;
+}
+
+static void jbd2_journal_destroy_transaction_cache(void)
+{
+ if (jbd2_transaction_cache)
+ kmem_cache_destroy(jbd2_transaction_cache);
+}
+
/*
* Module startup and shutdown
*/
@@ -2384,6 +2406,8 @@ static int __init journal_init_caches(void)
ret = journal_init_jbd2_journal_head_cache();
if (ret == 0)
ret = journal_init_handle_cache();
+ if (ret == 0)
+ ret = journal_init_transaction_cache();
return ret;
}

@@ -2392,6 +2416,7 @@ static void jbd2_journal_destroy_caches(void)
jbd2_journal_destroy_revoke_caches();
jbd2_journal_destroy_jbd2_journal_head_cache();
jbd2_journal_destroy_handle_cache();
+ jbd2_journal_destroy_transaction_cache();
jbd2_journal_destroy_slabs();
}

diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
index 3eec82d..58fbec7 100644
--- a/fs/jbd2/transaction.c
+++ b/fs/jbd2/transaction.c
@@ -131,7 +131,7 @@ static int start_this_handle(journal_t *journal, handle_t *handle,

alloc_transaction:
if (!journal->j_running_transaction) {
- new_transaction = kzalloc(sizeof(*new_transaction), gfp_mask);
+ new_transaction = jbd2_alloc_transaction(gfp_mask);
if (!new_transaction) {
/*
* If __GFP_FS is not present, then we may be
@@ -160,7 +160,7 @@ repeat:
if (is_journal_aborted(journal) ||
(journal->j_errno != 0 && !(journal->j_flags & JBD2_ACK_ERR))) {
read_unlock(&journal->j_state_lock);
- kfree(new_transaction);
+ jbd2_free_transaction(new_transaction);
return -EROFS;
}

@@ -282,7 +282,8 @@ repeat:
read_unlock(&journal->j_state_lock);

lock_map_acquire(&handle->h_lockdep_map);
- kfree(new_transaction);
+ jbd2_free_transaction(new_transaction);
+
return 0;
}

diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
index 4ecb7b1..803b5c4 100644
--- a/include/linux/jbd2.h
+++ b/include/linux/jbd2.h
@@ -1184,6 +1184,23 @@ static inline void jbd2_free_handle(handle_t *handle)
}

/*
+ * transaction management
+ */
+extern struct kmem_cache *jbd2_transaction_cache;
+
+static inline transaction_t *jbd2_alloc_transaction(gfp_t gfp_flags)
+{
+ return kmem_cache_zalloc(jbd2_transaction_cache, gfp_flags);
+}
+
+static inline void jbd2_free_transaction(transaction_t *transaction)
+{
+ if (transaction)
+ kmem_cache_free(jbd2_transaction_cache, transaction);
+}
+
+
+/*
* jbd2_inode management (optional, for those file systems that want to use
* dynamically allocated jbd2_inode structures)
*/
--
1.7.4.1



2011-07-22 18:22:03

by Manish Katiyar

[permalink] [raw]
Subject: Re: [PATCH v2] jbd2:Make journal transaction allocations come from its own cache.

On Sat, Jun 11, 2011 at 2:36 AM, Manish Katiyar <[email protected]> wrote:
> Add a cache for jbd2 journal transaction allocations. This also
> helps to leverage fault-injection framework to test various memory
> allocation failures in the jbd2 layer.
>
> Signed-off-by: Manish Katiyar <[email protected]>
> Acked-by: Jan Kara <[email protected]>
>
> ---
> v1->v2 :
> ?*) Drop SLAB_TEMPORARY flag for cache allocation.
> ?*) Remove filling with pattern during frees.
>
> ?fs/jbd2/checkpoint.c ?| ? ?2 +-
> ?fs/jbd2/commit.c ? ? ?| ? ?2 +-
> ?fs/jbd2/journal.c ? ? | ? 25 +++++++++++++++++++++++++
> ?fs/jbd2/transaction.c | ? ?7 ++++---
> ?include/linux/jbd2.h ?| ? 17 +++++++++++++++++
> ?5 files changed, 48 insertions(+), 5 deletions(-)
>
> diff --git a/fs/jbd2/checkpoint.c b/fs/jbd2/checkpoint.c
> index 6a79fd0..6f554ce 100644
> --- a/fs/jbd2/checkpoint.c
> +++ b/fs/jbd2/checkpoint.c
> @@ -716,7 +716,7 @@ int __jbd2_journal_remove_checkpoint(struct journal_head *jh)
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?transaction->t_tid, stats);
>
> ? ? ? ?__jbd2_journal_drop_transaction(journal, transaction);
> - ? ? ? kfree(transaction);
> + ? ? ? jbd2_free_transaction(transaction);
>
> ? ? ? ?/* Just in case anybody was waiting for more transactions to be
> ? ? ? ? ? ?checkpointed... */
> diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c
> index 7f21cf3..8e33d84 100644
> --- a/fs/jbd2/commit.c
> +++ b/fs/jbd2/commit.c
> @@ -1037,7 +1037,7 @@ restart_loop:
> ? ? ? ?jbd_debug(1, "JBD: commit %d complete, head %d\n",
> ? ? ? ? ? ? ? ? ?journal->j_commit_sequence, journal->j_tail_sequence);
> ? ? ? ?if (to_free)
> - ? ? ? ? ? ? ? kfree(commit_transaction);
> + ? ? ? ? ? ? ? jbd2_free_transaction(commit_transaction);
>
> ? ? ? ?wake_up(&journal->j_wait_done_commit);
> ?}
> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index 9a78269..61e0206 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -95,6 +95,7 @@ EXPORT_SYMBOL(jbd2_journal_init_jbd_inode);
> ?EXPORT_SYMBOL(jbd2_journal_release_jbd_inode);
> ?EXPORT_SYMBOL(jbd2_journal_begin_ordered_truncate);
> ?EXPORT_SYMBOL(jbd2_inode_cache);
> +EXPORT_SYMBOL(jbd2_transaction_cache);
>
> ?static int journal_convert_superblock_v1(journal_t *, journal_superblock_t *);
> ?static void __journal_abort_soft (journal_t *journal, int errno);
> @@ -2371,6 +2372,27 @@ static void jbd2_journal_destroy_handle_cache(void)
>
> ?}
>
> +struct kmem_cache *jbd2_transaction_cache;
> +
> +static int journal_init_transaction_cache(void)
> +{
> + ? ? ? J_ASSERT(jbd2_transaction_cache == NULL);
> + ? ? ? jbd2_transaction_cache = kmem_cache_create("jbd2_transaction",
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?sizeof(transaction_t),
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?0, 0, NULL);
> + ? ? ? if (jbd2_transaction_cache == NULL) {
> + ? ? ? ? ? ? ? printk(KERN_EMERG "JBD2: failed to create transaction cache\n");
> + ? ? ? ? ? ? ? return -ENOMEM;
> + ? ? ? }
> + ? ? ? return 0;
> +}
> +
> +static void jbd2_journal_destroy_transaction_cache(void)
> +{
> + ? ? ? if (jbd2_transaction_cache)
> + ? ? ? ? ? ? ? kmem_cache_destroy(jbd2_transaction_cache);
> +}
> +
> ?/*
> ?* Module startup and shutdown
> ?*/
> @@ -2384,6 +2406,8 @@ static int __init journal_init_caches(void)
> ? ? ? ? ? ? ? ?ret = journal_init_jbd2_journal_head_cache();
> ? ? ? ?if (ret == 0)
> ? ? ? ? ? ? ? ?ret = journal_init_handle_cache();
> + ? ? ? if (ret == 0)
> + ? ? ? ? ? ? ? ret = journal_init_transaction_cache();
> ? ? ? ?return ret;
> ?}
>
> @@ -2392,6 +2416,7 @@ static void jbd2_journal_destroy_caches(void)
> ? ? ? ?jbd2_journal_destroy_revoke_caches();
> ? ? ? ?jbd2_journal_destroy_jbd2_journal_head_cache();
> ? ? ? ?jbd2_journal_destroy_handle_cache();
> + ? ? ? jbd2_journal_destroy_transaction_cache();
> ? ? ? ?jbd2_journal_destroy_slabs();
> ?}
>
> diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
> index 3eec82d..58fbec7 100644
> --- a/fs/jbd2/transaction.c
> +++ b/fs/jbd2/transaction.c
> @@ -131,7 +131,7 @@ static int start_this_handle(journal_t *journal, handle_t *handle,
>
> ?alloc_transaction:
> ? ? ? ?if (!journal->j_running_transaction) {
> - ? ? ? ? ? ? ? new_transaction = kzalloc(sizeof(*new_transaction), gfp_mask);
> + ? ? ? ? ? ? ? new_transaction = jbd2_alloc_transaction(gfp_mask);
> ? ? ? ? ? ? ? ?if (!new_transaction) {
> ? ? ? ? ? ? ? ? ? ? ? ?/*
> ? ? ? ? ? ? ? ? ? ? ? ? * If __GFP_FS is not present, then we may be
> @@ -160,7 +160,7 @@ repeat:
> ? ? ? ?if (is_journal_aborted(journal) ||
> ? ? ? ? ? ?(journal->j_errno != 0 && !(journal->j_flags & JBD2_ACK_ERR))) {
> ? ? ? ? ? ? ? ?read_unlock(&journal->j_state_lock);
> - ? ? ? ? ? ? ? kfree(new_transaction);
> + ? ? ? ? ? ? ? jbd2_free_transaction(new_transaction);
> ? ? ? ? ? ? ? ?return -EROFS;
> ? ? ? ?}
>
> @@ -282,7 +282,8 @@ repeat:
> ? ? ? ?read_unlock(&journal->j_state_lock);
>
> ? ? ? ?lock_map_acquire(&handle->h_lockdep_map);
> - ? ? ? kfree(new_transaction);
> + ? ? ? jbd2_free_transaction(new_transaction);
> +
> ? ? ? ?return 0;
> ?}
>
> diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
> index 4ecb7b1..803b5c4 100644
> --- a/include/linux/jbd2.h
> +++ b/include/linux/jbd2.h
> @@ -1184,6 +1184,23 @@ static inline void jbd2_free_handle(handle_t *handle)
> ?}
>
> ?/*
> + * transaction management
> + */
> +extern struct kmem_cache *jbd2_transaction_cache;
> +
> +static inline transaction_t *jbd2_alloc_transaction(gfp_t gfp_flags)
> +{
> + ? ? ? return kmem_cache_zalloc(jbd2_transaction_cache, gfp_flags);
> +}
> +
> +static inline void jbd2_free_transaction(transaction_t *transaction)
> +{
> + ? ? ? if (transaction)
> + ? ? ? ? ? ? ? kmem_cache_free(jbd2_transaction_cache, transaction);
> +}
> +
> +
> +/*
> ?* jbd2_inode management (optional, for those file systems that want to use
> ?* dynamically allocated jbd2_inode structures)
> ?*/
> --
> 1.7.4.1
>
>

Hi Ted,

Any feedback on this ?


--
Thanks -
Manish