All of lore.kernel.org
 help / color / mirror / Atom feed
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: Ilya Yanok <yanok@emcraft.com>
Cc: linuxppc-dev@ozlabs.org, wd@denx.de, dzu@denx.de
Subject: Re: [PATCH] powerpc: rework dma-noncoherent to use generic vmap/vunmap functions
Date: Wed, 04 Feb 2009 15:43:45 +1100	[thread overview]
Message-ID: <1233722625.16867.249.camel@pasglop> (raw)
In-Reply-To: <1231505915-16082-1-git-send-email-yanok@emcraft.com>

On Fri, 2009-01-09 at 15:58 +0300, Ilya Yanok wrote:
> This patch rewrites consistent dma allocations support to use vmalloc
> layer to allocate virtual memory space from vmalloc pool and get rid
> of CONFIG_CONSISTENT_{START,SIZE}.

So as commented before, please drop the defconfig updates.

I'm happy with the idea but I have a few nits with the implementation:

> -/*
>   * Allocate DMA-coherent memory space and return both the kernel remapped
>   * virtual and bus address for that space.
>   */
> @@ -151,19 +41,17 @@ void *
>  __dma_alloc_coherent(size_t size, dma_addr_t *handle, gfp_t gfp)
>  {
>  	struct page *page;
> -	struct vm_region *c;
>  	unsigned long order;
> +	void *v;
> +	int i;
> +	struct page *pages[PAGE_ALIGN(size)>>PAGE_SHIFT];

I'm not -too- fan of that page list one the stack up there.

I understand why you don't wantto kmalloc something here etc... but
that's what __vmalloc_area() does and it's somewhat useful to keep track
of the page array that way, it might prove handy in the future.

Might even be worth adding a generic patch to add a VM_COHERENT_DMA flag
so they can be listed as such and make sure you set the "caller" field
yourself with your own caller.

(Hint: look at the output of /proc/vmallocinfo)

Also, the mucking around with PG_Reserved shouldn't be of any use
anymore.

Cheers,
Ben.

  parent reply	other threads:[~2009-02-04  4:44 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-01-09 12:58 [PATCH] powerpc: rework dma-noncoherent to use generic vmap/vunmap functions Ilya Yanok
2009-01-10 23:30 ` Benjamin Herrenschmidt
2009-01-11  0:31 ` Grant Likely
2009-01-12  0:47   ` Josh Boyer
2009-02-04  4:43 ` Benjamin Herrenschmidt [this message]
2009-02-12 17:40   ` Ilya Yanok
2009-02-12 20:41     ` Benjamin Herrenschmidt
2009-02-12 21:03       ` Ilya Yanok

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1233722625.16867.249.camel@pasglop \
    --to=benh@kernel.crashing.org \
    --cc=dzu@denx.de \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=wd@denx.de \
    --cc=yanok@emcraft.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.