| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20111114153557.GM5230@quack.suse.cz>
Date: Mon, 14 Nov 2011 16:35:57 +0100
From: Jan Kara <jack@...e.cz>
To: Manish Katiyar <mkatiyar@...il.com>
Cc: Jan Kara <jack@...e.cz>, Yongqiang Yang <xiaoqiangnk@...il.com>,
linux-ext4@...r.kernel.org
Subject: Re: [PATCH 1/2] jbd: allocate transacion from special cache
On Mon 14-11-11 07:22:03, Manish Katiyar wrote:
> On Mon, Nov 14, 2011 at 4:53 AM, Jan Kara <jack@...e.cz> 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@...il.com>
> > 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
Now I remember :) Anyway, "it makes debugging simpler" is really the
reason which would change Andrew's mind I guess. I actually used a patch
like this once or twice exactly to debug some stuff... That's why I think
the patch has a value for ext4/jbd2 and I'll follow their lead with ext3.
Honza
> > 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@...e.cz>
> > SUSE Labs, CR
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> > the body of a message to majordomo@...r.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
>
>
>
> --
> Thanks -
> Manish
--
Jan Kara <jack@...e.cz>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists