| 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: <200807250046.10392.nickpiggin@yahoo.com.au>
Date: Fri, 25 Jul 2008 00:46:09 +1000
From: Nick Piggin <nickpiggin@...oo.com.au>
To: Peter Zijlstra <a.p.zijlstra@...llo.nl>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>,
Andrew Morton <akpm@...ux-foundation.org>,
linux-kernel@...r.kernel.org, linux-mm@...ck.org,
netdev@...r.kernel.org, trond.myklebust@....uio.no,
Daniel Lezcano <dlezcano@...ibm.com>,
Pekka Enberg <penberg@...helsinki.fi>,
Neil Brown <neilb@...e.de>
Subject: Re: [PATCH 26/30] nfs: remove mempools
On Friday 25 July 2008 00:01, Peter Zijlstra wrote:
> With the introduction of the shared dirty page accounting in .19, NFS
> should not be able to surpise the VM with all dirty pages. Thus it should
> always be able to free some memory. Hence no more need for mempools.
It seems like a very backward step to me to go from a hard guarantee
to some heuristic that could break for someone in some particular
setup.
Filling with dirty pages isn't the only way to exaust free reclaimable
memory remember, it can also happen maybe due to mlock or kernel
allocations.
Is there pressing reason to remove them?
> Signed-off-by: Peter Zijlstra <a.p.zijlstra@...llo.nl>
> ---
> fs/nfs/read.c | 15 +++------------
> fs/nfs/write.c | 27 +++++----------------------
> 2 files changed, 8 insertions(+), 34 deletions(-)
>
> Index: linux-2.6/fs/nfs/read.c
> ===================================================================
> --- linux-2.6.orig/fs/nfs/read.c
> +++ linux-2.6/fs/nfs/read.c
> @@ -33,13 +33,10 @@ static const struct rpc_call_ops nfs_rea
> static const struct rpc_call_ops nfs_read_full_ops;
>
> static struct kmem_cache *nfs_rdata_cachep;
> -static mempool_t *nfs_rdata_mempool;
> -
> -#define MIN_POOL_READ (32)
>
> struct nfs_read_data *nfs_readdata_alloc(unsigned int pagecount)
> {
> - struct nfs_read_data *p = mempool_alloc(nfs_rdata_mempool, GFP_NOFS);
> + struct nfs_read_data *p = kmem_cache_alloc(nfs_rdata_cachep, GFP_NOFS);
>
> if (p) {
> memset(p, 0, sizeof(*p));
> @@ -50,7 +47,7 @@ struct nfs_read_data *nfs_readdata_alloc
> else {
> p->pagevec = kcalloc(pagecount, sizeof(struct page *), GFP_NOFS);
> if (!p->pagevec) {
> - mempool_free(p, nfs_rdata_mempool);
> + kmem_cache_free(nfs_rdata_cachep, p);
> p = NULL;
> }
> }
> @@ -62,7 +59,7 @@ static void nfs_readdata_free(struct nfs
> {
> if (p && (p->pagevec != &p->page_array[0]))
> kfree(p->pagevec);
> - mempool_free(p, nfs_rdata_mempool);
> + kmem_cache_free(nfs_rdata_cachep, p);
> }
>
> void nfs_readdata_release(void *data)
> @@ -614,16 +611,10 @@ int __init nfs_init_readpagecache(void)
> if (nfs_rdata_cachep == NULL)
> return -ENOMEM;
>
> - nfs_rdata_mempool = mempool_create_slab_pool(MIN_POOL_READ,
> - nfs_rdata_cachep);
> - if (nfs_rdata_mempool == NULL)
> - return -ENOMEM;
> -
> return 0;
> }
>
> void nfs_destroy_readpagecache(void)
> {
> - mempool_destroy(nfs_rdata_mempool);
> kmem_cache_destroy(nfs_rdata_cachep);
> }
> Index: linux-2.6/fs/nfs/write.c
> ===================================================================
> --- linux-2.6.orig/fs/nfs/write.c
> +++ linux-2.6/fs/nfs/write.c
> @@ -28,9 +28,6 @@
>
> #define NFSDBG_FACILITY NFSDBG_PAGECACHE
>
> -#define MIN_POOL_WRITE (32)
> -#define MIN_POOL_COMMIT (4)
> -
> /*
> * Local function declarations
> */
> @@ -45,12 +42,10 @@ static const struct rpc_call_ops nfs_wri
> static const struct rpc_call_ops nfs_commit_ops;
>
> static struct kmem_cache *nfs_wdata_cachep;
> -static mempool_t *nfs_wdata_mempool;
> -static mempool_t *nfs_commit_mempool;
>
> struct nfs_write_data *nfs_commitdata_alloc(void)
> {
> - struct nfs_write_data *p = mempool_alloc(nfs_commit_mempool, GFP_NOFS);
> + struct nfs_write_data *p = kmem_cache_alloc(nfs_wdata_cachep, GFP_NOFS);
>
> if (p) {
> memset(p, 0, sizeof(*p));
> @@ -63,12 +58,12 @@ void nfs_commit_free(struct nfs_write_da
> {
> if (p && (p->pagevec != &p->page_array[0]))
> kfree(p->pagevec);
> - mempool_free(p, nfs_commit_mempool);
> + kmem_cache_free(nfs_wdata_cachep, p);
> }
>
> struct nfs_write_data *nfs_writedata_alloc(unsigned int pagecount)
> {
> - struct nfs_write_data *p = mempool_alloc(nfs_wdata_mempool, GFP_NOFS);
> + struct nfs_write_data *p = kmem_cache_alloc(nfs_wdata_cachep, GFP_NOFS);
>
> if (p) {
> memset(p, 0, sizeof(*p));
> @@ -79,7 +74,7 @@ struct nfs_write_data *nfs_writedata_all
> else {
> p->pagevec = kcalloc(pagecount, sizeof(struct page *), GFP_NOFS);
> if (!p->pagevec) {
> - mempool_free(p, nfs_wdata_mempool);
> + kmem_cache_free(nfs_wdata_cachep, p);
> p = NULL;
> }
> }
> @@ -91,7 +86,7 @@ static void nfs_writedata_free(struct nf
> {
> if (p && (p->pagevec != &p->page_array[0]))
> kfree(p->pagevec);
> - mempool_free(p, nfs_wdata_mempool);
> + kmem_cache_free(nfs_wdata_cachep, p);
> }
>
> void nfs_writedata_release(void *data)
> @@ -1552,16 +1547,6 @@ int __init nfs_init_writepagecache(void)
> if (nfs_wdata_cachep == NULL)
> return -ENOMEM;
>
> - nfs_wdata_mempool = mempool_create_slab_pool(MIN_POOL_WRITE,
> - nfs_wdata_cachep);
> - if (nfs_wdata_mempool == NULL)
> - return -ENOMEM;
> -
> - nfs_commit_mempool = mempool_create_slab_pool(MIN_POOL_COMMIT,
> - nfs_wdata_cachep);
> - if (nfs_commit_mempool == NULL)
> - return -ENOMEM;
> -
> /*
> * NFS congestion size, scale with available memory.
> *
> @@ -1587,8 +1572,6 @@ int __init nfs_init_writepagecache(void)
>
> void nfs_destroy_writepagecache(void)
> {
> - mempool_destroy(nfs_commit_mempool);
> - mempool_destroy(nfs_wdata_mempool);
> kmem_cache_destroy(nfs_wdata_cachep);
> }
--
To unsubscribe from this list: send the line "unsubscribe netdev" 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