Message ID | 200904211716.45245.ossrosch@linux.vnet.ibm.com (mailing list archive) |
---|---|
State | Not Applicable, archived |
Headers | show |
> + queue->queue_pages = kmalloc(nr_of_pages * sizeof(void *), GFP_KERNEL);
How big might this buffer be? Any chance of allocation failure due to
memory fragmentation?
- R.
Hi Roland,
thanks for the quick review. I was hoping you could apply these changes
for 2.6.30 because this will be the codebase for the next OFED release.
The patch is well tested in HPC environment and we haven't seen any
issues.
Regarding Antons patch you are right. If a user allocates an
unrealistically large queue pair it could happen that kmalloc() is not
able to allocate the memory. In this case we will return ENOMEM to the
user so the kernel will not be affected at all. We plan to add vmalloc()
call in case kmalloc() fails for the next kernel release.
Mit freundlichen Grüßen / Kind regards
Stefan Roscher
eHCA/eHEA Linux Driver Development
IBM Systems &Technology Group, Systems Software Development / FW I/O
Firmware Entwicklung 2
-------------------------------------------------------------------------------------------------------------------------------------------
IBM Deutschland
Schoenaicher Str. 220
71032 Boeblingen
Phone: +49-7031-16-2015
E-Mail: stefan.roscher@de.ibm.com
-------------------------------------------------------------------------------------------------------------------------------------------
IBM Deutschland Research & Development GmbH / Vorsitzender des
Aufsichtsrats: Martin Jetter
Geschäftsführung: Herbert Kircher
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart,
HRB 243294
From:
Roland Dreier <rdreier@cisco.com>
To:
Stefan Roscher <ossrosch@linux.vnet.ibm.com>
Cc:
"LinuxPPC-Dev" <linuxppc-dev@ozlabs.org>, LKML
<linux-kernel@vger.kernel.org>, "OF-EWG" <ewg@lists.openfabrics.org>,
Roland Dreier <rolandd@cisco.com>, Joachim Fenkes/Germany/IBM@IBMDE,
Christoph Raisch/Germany/IBM@IBMDE, Alexander Schmidt1/Germany/IBM@IBMDE,
Stefan Roscher/Germany/IBM@IBMDE, Hoang-Nam Nguyen/Germany/IBM@IBMDE
Date:
21.04.2009 19:34
Subject:
Re: [PATCH 1/3] IB/ehca: Replace vmalloc with kmalloc
> + queue->queue_pages = kmalloc(nr_of_pages * sizeof(void
*), GFP_KERNEL);
How big might this buffer be? Any chance of allocation failure due to
memory fragmentation?
- R.
On Tue, 2009-04-21 at 17:16 +0200, Stefan Roscher wrote: > From: Anton Blanchard <antonb at au1.ibm.com> > > To improve performance of driver ressource allocation, > replace the vmalloc() call with kmalloc(). Just curious, but how big are these allocations? Why was vmalloc() even ever used if we know they'll be small? -- Dave
On Tuesday 28 April 2009 05:12:51 pm Dave Hansen wrote: > On Tue, 2009-04-21 at 17:16 +0200, Stefan Roscher wrote: > > From: Anton Blanchard <antonb at au1.ibm.com> > > > > To improve performance of driver ressource allocation, > > replace the vmalloc() call with kmalloc(). > > Just curious, but how big are these allocations? Why was vmalloc() even > ever used if we know they'll be small? > > -- Dave > > The theoretical maximum size can be 512k, but for common queue pairs less than 128k is used.Because of the theoretical maximum we implemented vmalloc() first, but recognized a huge performance impact. -- Stefan
thanks, applied. > From: Anton Blanchard <antonb at au1.ibm.com> > Signed-off-by: Stefan Roscher <stefan.roscher at de.ibm.com> please use '@' signs so these are real email addresses. - R.
diff --git a/drivers/infiniband/hw/ehca/ipz_pt_fn.c b/drivers/infiniband/hw/ehca/ipz_pt_fn.c index c3a3284..a260559 100644 --- a/drivers/infiniband/hw/ehca/ipz_pt_fn.c +++ b/drivers/infiniband/hw/ehca/ipz_pt_fn.c @@ -220,7 +220,7 @@ int ipz_queue_ctor(struct ehca_pd *pd, struct ipz_queue *queue, queue->small_page = NULL; /* allocate queue page pointers */ - queue->queue_pages = vmalloc(nr_of_pages * sizeof(void *)); + queue->queue_pages = kmalloc(nr_of_pages * sizeof(void *), GFP_KERNEL); if (!queue->queue_pages) { ehca_gen_err("Couldn't allocate queue page list"); return 0; @@ -240,7 +240,7 @@ int ipz_queue_ctor(struct ehca_pd *pd, struct ipz_queue *queue, ipz_queue_ctor_exit0: ehca_gen_err("Couldn't alloc pages queue=%p " "nr_of_pages=%x", queue, nr_of_pages); - vfree(queue->queue_pages); + kfree(queue->queue_pages); return 0; } @@ -262,7 +262,7 @@ int ipz_queue_dtor(struct ehca_pd *pd, struct ipz_queue *queue) free_page((unsigned long)queue->queue_pages[i]); } - vfree(queue->queue_pages); + kfree(queue->queue_pages); return 1; }