From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752305AbbCXXmL (ORCPT ); Tue, 24 Mar 2015 19:42:11 -0400 Received: from aserp1040.oracle.com ([141.146.126.69]:40504 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751842AbbCXXmI (ORCPT ); Tue, 24 Mar 2015 19:42:08 -0400 Message-ID: <5511F629.2070907@oracle.com> Date: Tue, 24 Mar 2015 18:41:29 -0500 From: Dave Kleikamp User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: David Rientjes , Andrew Morton CC: Dave Kleikamp , Christoph Hellwig , Sebastian Ott , Mikulas Patocka , Catalin Marinas , linux-kernel@vger.kernel.org, linux-mm@kvack.org, jfs-discussion@lists.sourceforge.net Subject: Re: [patch 1/4] fs, jfs: remove slab object constructor References: In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-Source-IP: userv0021.oracle.com [156.151.31.71] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 03/24/2015 06:08 PM, David Rientjes wrote: > Mempools based on slab caches with object constructors are risky because > element allocation can happen either from the slab cache itself, meaning > the constructor is properly called before returning, or from the mempool > reserve pool, meaning the constructor is not called before returning, > depending on the allocation context. > > For this reason, we should disallow creating mempools based on slab > caches that have object constructors. Callers of mempool_alloc() will > be responsible for properly initializing the returned element. > > Then, it doesn't matter if the element came from the slab cache or the > mempool reserved pool. > > The only occurrence of a mempool being based on a slab cache with an > object constructor in the tree is in fs/jfs/jfs_metapage.c. Remove it > and properly initialize the element in alloc_metapage(). > > At the same time, META_free is never used, so remove it as well. > > Signed-off-by: David Rientjes Acked-by: Dave Kleikamp > --- > fs/jfs/jfs_metapage.c | 31 ++++++++++++------------------- > fs/jfs/jfs_metapage.h | 1 - > 2 files changed, 12 insertions(+), 20 deletions(-) > > diff --git a/fs/jfs/jfs_metapage.c b/fs/jfs/jfs_metapage.c > --- a/fs/jfs/jfs_metapage.c > +++ b/fs/jfs/jfs_metapage.c > @@ -183,30 +183,23 @@ static inline void remove_metapage(struct page *page, struct metapage *mp) > > #endif > > -static void init_once(void *foo) > -{ > - struct metapage *mp = (struct metapage *)foo; > - > - mp->lid = 0; > - mp->lsn = 0; > - mp->flag = 0; > - mp->data = NULL; > - mp->clsn = 0; > - mp->log = NULL; > - set_bit(META_free, &mp->flag); > - init_waitqueue_head(&mp->wait); > -} > - > static inline struct metapage *alloc_metapage(gfp_t gfp_mask) > { > - return mempool_alloc(metapage_mempool, gfp_mask); > + struct metapage *mp = mempool_alloc(metapage_mempool, gfp_mask); > + > + if (mp) { > + mp->lid = 0; > + mp->lsn = 0; > + mp->data = NULL; > + mp->clsn = 0; > + mp->log = NULL; > + init_waitqueue_head(&mp->wait); > + } > + return mp; > } > > static inline void free_metapage(struct metapage *mp) > { > - mp->flag = 0; > - set_bit(META_free, &mp->flag); > - > mempool_free(mp, metapage_mempool); > } > > @@ -216,7 +209,7 @@ int __init metapage_init(void) > * Allocate the metapage structures > */ > metapage_cache = kmem_cache_create("jfs_mp", sizeof(struct metapage), > - 0, 0, init_once); > + 0, 0, NULL); > if (metapage_cache == NULL) > return -ENOMEM; > > diff --git a/fs/jfs/jfs_metapage.h b/fs/jfs/jfs_metapage.h > --- a/fs/jfs/jfs_metapage.h > +++ b/fs/jfs/jfs_metapage.h > @@ -48,7 +48,6 @@ struct metapage { > > /* metapage flag */ > #define META_locked 0 > -#define META_free 1 > #define META_dirty 2 > #define META_sync 3 > #define META_discard 4 >