On Mon, Nov 14, 2011 at 4:53 AM, Jan Kara <jack@xxxxxxx> wrote: > On Sun 13-11-11 19:29:31, Yongqiang Yang wrote: >> transaction_t is 100 bytes under 32-bit systems. Transactions >> allocated from general cache comsume 128 bytes. This patch >> lets jbd allocates transacion from special cache. >> >> Signed-off-by: Yongqiang Yang <xiaoqiangnk@xxxxxxxxx> > Space saving isn't probably a good argument here since the number of > transactions on the system is going to be rather low (a couple per ext3 > filesystem). The reason why I wanted this change for JBD2 is that it makes > debugging of memory corruption of a transaction structure much easier (and > I've been tracking down issues like this several times already). > > That being said, I don't find strong need to have this in ext3 and thus > I'll take the change in jbd/ext3 I had submitted a similar patch for ext3, but it was rejected by Andrew Morton. http://lists.openwall.net/linux-ext4/2011/06/13/15 > if Ted takes the similar change to > jbd2/ext4 to keep compatibility. Manish Katiyar was working on it > (http://comments.gmane.org/gmane.comp.file-systems.ext4/25884) but it seems > it wasn't merged yet. > > Honza > >> --- >> fs/jbd/checkpoint.c | 2 +- >> fs/jbd/journal.c | 3 +++ >> fs/jbd/transaction.c | 31 +++++++++++++++++++++++++++++-- >> include/linux/jbd.h | 5 +++++ >> 4 files changed, 38 insertions(+), 3 deletions(-) >> >> diff --git a/fs/jbd/checkpoint.c b/fs/jbd/checkpoint.c >> index f94fc48..4d98046 100644 >> --- a/fs/jbd/checkpoint.c >> +++ b/fs/jbd/checkpoint.c >> @@ -764,5 +764,5 @@ void __journal_drop_transaction(journal_t *journal, transaction_t *transaction) >> >> trace_jbd_drop_transaction(journal, transaction); >> jbd_debug(1, "Dropping transaction %d, all done\n", transaction->t_tid); >> - kfree(transaction); >> + journal_free_transaction(transaction); >> } >> diff --git a/fs/jbd/journal.c b/fs/jbd/journal.c >> index 9fe061f..45ca982 100644 >> --- a/fs/jbd/journal.c >> +++ b/fs/jbd/journal.c >> @@ -2004,6 +2004,8 @@ static int __init journal_init_caches(void) >> ret = journal_init_journal_head_cache(); >> if (ret == 0) >> ret = journal_init_handle_cache(); >> + if (ret == 0) >> + ret = journal_init_transaction_cache(); >> return ret; >> } >> >> @@ -2012,6 +2014,7 @@ static void journal_destroy_caches(void) >> journal_destroy_revoke_caches(); >> journal_destroy_journal_head_cache(); >> journal_destroy_handle_cache(); >> + journal_destroy_transaction_cache(); >> } >> >> static int __init journal_init(void) >> diff --git a/fs/jbd/transaction.c b/fs/jbd/transaction.c >> index 7e59c6e..20d76f1 100644 >> --- a/fs/jbd/transaction.c >> +++ b/fs/jbd/transaction.c >> @@ -30,6 +30,32 @@ >> >> static void __journal_temp_unlink_buffer(struct journal_head *jh); >> >> +static struct kmem_cache *transaction_cache; >> +int __init journal_init_transaction_cache(void) >> +{ >> + J_ASSERT(!transaction_cache); >> + transaction_cache = KMEM_CACHE(transaction_s, >> + SLAB_HWCACHE_ALIGN|SLAB_TEMPORARY); >> + if (transaction_cache) >> + return 0; >> + return -ENOMEM; >> +} >> + >> +void __init journal_destroy_transaction_cache(void) >> +{ >> + if (transaction_cache) { >> + kmem_cache_destroy(transaction_cache); >> + transaction_cache = NULL; >> + } >> +} >> + >> +void journal_free_transaction(transaction_t *transaction) >> +{ >> + if (unlikely(ZERO_OR_NULL_PTR(transaction))) >> + return; >> + kmem_cache_free(transaction_cache, transaction); >> +} >> + >> /* >> * get_transaction: obtain a new transaction_t object. >> * >> @@ -100,11 +126,12 @@ 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_NOFS); >> + new_transaction = kmem_cache_alloc(transaction_cache, GFP_NOFS); >> if (!new_transaction) { >> congestion_wait(BLK_RW_ASYNC, HZ/50); >> goto alloc_transaction; >> } >> + memset(new_transaction, 0, sizeof(*new_transaction)); >> } >> >> jbd_debug(3, "New handle %p going live.\n", handle); >> @@ -233,7 +260,7 @@ repeat_locked: >> lock_map_acquire(&handle->h_lockdep_map); >> out: >> if (unlikely(new_transaction)) /* It's usually NULL */ >> - kfree(new_transaction); >> + journal_free_transaction(new_transaction); >> return ret; >> } >> >> diff --git a/include/linux/jbd.h b/include/linux/jbd.h >> index c7acdde..0f9f0b6 100644 >> --- a/include/linux/jbd.h >> +++ b/include/linux/jbd.h >> @@ -807,6 +807,11 @@ journal_write_metadata_buffer(transaction_t *transaction, >> /* Transaction locking */ >> extern void __wait_on_journal (journal_t *); >> >> +/* Transaction cache support */ >> +extern void journal_destroy_transaction_cache(void); >> +extern int journal_init_transaction_cache(void); >> +extern void journal_free_transaction(transaction_t *); >> + >> /* >> * Journal locking. >> * >> -- >> 1.7.5.1 >> > -- > 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 > -- Thanks - Manish -- 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
