From: Mingming Cao On Tue, 2007-07-10 at 21:42 -0700, Andrew Morton wrote: On Tue, 10 Jul 2007 23:21:49 -0400 "Cédric Augonnet" wrote: > > > 2007/7/10, Andrew Morton : > > > > Hi all, > > > > > > + size = sizeof(struct transaction_stats_s); > > > > + s->stats = kmalloc(size, GFP_KERNEL); > > > > + if (s == NULL) { > > > > + kfree(s); > > > > + return -EIO; > > > > > > ENOMEM > > > > I'm sorry if i missed some point, but i just don't see the use of such > > a kfree here, not sure Andrew meant you should only return ENOMEM > > instead, but why issuing those kfree(NULL), instead of just a if (!s) > > return ENOMEM ? > > > > You found a bug. It was meant to be > > if (s->stats == NULL) > Signed-off-by: Mingming Cao --- fs/jbd2/journal.c | 2 +- include/linux/jbd2.h | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) Index: linux-2.6.23-rc6/fs/jbd2/journal.c =================================================================== --- linux-2.6.23-rc6.orig/fs/jbd2/journal.c 2007-09-20 17:26:01.000000000 -0700 +++ linux-2.6.23-rc6/fs/jbd2/journal.c 2007-09-20 17:26:02.000000000 -0700 @@ -751,7 +751,7 @@ static int jbd2_seq_history_open(struct return -EIO; size = sizeof(struct transaction_stats_s) * journal->j_history_max; s->stats = kmalloc(size, GFP_KERNEL); - if (s == NULL) { + if (s->stats == NULL) { kfree(s); return -EIO; } Index: linux-2.6.23-rc6/include/linux/jbd2.h =================================================================== --- linux-2.6.23-rc6.orig/include/linux/jbd2.h 2007-09-20 17:26:01.000000000 -0700 +++ linux-2.6.23-rc6/include/linux/jbd2.h 2007-09-20 17:26:02.000000000 -0700 @@ -941,6 +941,9 @@ struct journal_s struct transaction_stats_s *j_history; int j_history_max; int j_history_cur; + /* + * Protect the transactions statistics history + */ spinlock_t j_history_lock; struct proc_dir_entry *j_proc_entry; struct transaction_stats_s j_stats;