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 <rient...@google.com>
Acked-by: Dave Kleikamp <dave.kleik...@oracle.com>
> ---
> 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
>
------------------------------------------------------------------------------
Dive into the World of Parallel Programming The Go Parallel Website, sponsored
by Intel and developed in partnership with Slashdot Media, is your hub for all
things parallel software development, from weekly thought leadership blogs to
news, videos, case studies, tutorials and more. Take a look and join the
conversation now. http://goparallel.sourceforge.net/
_______________________________________________
Jfs-discussion mailing list
Jfs-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/jfs-discussion
