On Thu 09-06-11 23:21:17, Manish Katiyar wrote:
> On Wed, Jun 8, 2011 at 9:49 AM, Jan Kara <jack@xxxxxxx> wrote:
> > On Sun 05-06-11 01:28:30, Manish Katiyar 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 <mkatiyar@xxxxxxxxx>
> >> ---
> >> fs/jbd2/checkpoint.c | 2 +-
> >> fs/jbd2/commit.c | 2 +-
> >> fs/jbd2/journal.c | 25 +++++++++++++++++++++++++
> >> fs/jbd2/transaction.c | 7 ++++---
> >> include/linux/jbd2.h | 21 +++++++++++++++++++++
> >> 5 files changed, 52 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..c0ec463 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, SLAB_TEMPORARY, NULL);
> > Transactions are not really short-lived in the memory-management sense I
> > think. They usually live for seconds while I'd understand 'short-lived' to
> > mean a milisecond or less. So just drop this flag (it doesn't do anything
> > these days anyway).
> >
> >> + 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)
> > How can this happen?
>
> Hi Jan,
>
> re-reading the code, I think its possible and we should be checking
> for it. Since in journal_init_caches() if any of the initialization
> fails we would call jbd2_journal_destroy_caches() on all the caches.
> So we should be destroying it only if its initialized.
Ah, good point. Thanks for checking this.
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
