[PATCHv2,3/4] mm: vmalloc: add VM_DMA flag to indicate areas used by dma-mapping framework

Add new type of vm_area intented to be used for consisten mappings
created by dma-mapping framework.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Reviewed-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 include/linux/vmalloc.h |    1 +
 mm/vmalloc.c            |    3 +++
 2 files changed, 4 insertions(+)

On 05/17/2012 07:54 PM, Marek Szyprowski wrote:

> Add new type of vm_area intented to be used for consisten mappings
> created by dma-mapping framework.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Reviewed-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  include/linux/vmalloc.h |    1 +
>  mm/vmalloc.c            |    3 +++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
> index 6071e91..8a9555a 100644
> --- a/include/linux/vmalloc.h
> +++ b/include/linux/vmalloc.h
> @@ -14,6 +14,7 @@ struct vm_area_struct;		/* vma defining user mapping in mm_types.h */
>  #define VM_USERMAP	0x00000008	/* suitable for remap_vmalloc_range */
>  #define VM_VPAGES	0x00000010	/* buffer for pages was vmalloc'ed */
>  #define VM_UNLIST	0x00000020	/* vm_struct is not listed in vmlist */
> +#define VM_DMA		0x00000040	/* used by dma-mapping framework */
>  /* bits [20..32] reserved for arch specific ioremap internals */

>  

>  /*
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 8cb7f22..9c13bab 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -2582,6 +2582,9 @@ static int s_show(struct seq_file *m, void *p)
>  	if (v->flags & VM_IOREMAP)
>  		seq_printf(m, " ioremap");
>  
> +	if (v->flags & VM_DMA)
> +		seq_printf(m, " dma");
> +


Hmm, VM_DMA would become generic flag?
AFAIU, maybe VM_DMA would be used only on ARM arch.
Of course, it isn't performance sensitive part but there in no reason to check it, either
in other architecture except ARM.

I suggest following as

#ifdef CONFIG_ARM
#define VM_DMA	0x00000040
#else
#define VM_DMA	0x0
#end

Maybe it could remove check code at compile time.

>  	if (v->flags & VM_ALLOC)
>  		seq_printf(m, " vmalloc");
>

Hi Minchan,

On Tuesday, May 22, 2012 9:08 AM Minchan Kim wrote:

> On 05/17/2012 07:54 PM, Marek Szyprowski wrote:
> 
> > Add new type of vm_area intented to be used for consisten mappings
> > created by dma-mapping framework.
> >
> > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> > Reviewed-by: Kyungmin Park <kyungmin.park@samsung.com>
> > ---
> >  include/linux/vmalloc.h |    1 +
> >  mm/vmalloc.c            |    3 +++
> >  2 files changed, 4 insertions(+)
> >
> > diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
> > index 6071e91..8a9555a 100644
> > --- a/include/linux/vmalloc.h
> > +++ b/include/linux/vmalloc.h
> > @@ -14,6 +14,7 @@ struct vm_area_struct;		/* vma defining user mapping in
> mm_types.h */
> >  #define VM_USERMAP	0x00000008	/* suitable for remap_vmalloc_range */
> >  #define VM_VPAGES	0x00000010	/* buffer for pages was vmalloc'ed */
> >  #define VM_UNLIST	0x00000020	/* vm_struct is not listed in vmlist */
> > +#define VM_DMA		0x00000040	/* used by dma-mapping framework */
> >  /* bits [20..32] reserved for arch specific ioremap internals */
> 
> >
> 
> >  /*
> > diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> > index 8cb7f22..9c13bab 100644
> > --- a/mm/vmalloc.c
> > +++ b/mm/vmalloc.c
> > @@ -2582,6 +2582,9 @@ static int s_show(struct seq_file *m, void *p)
> >  	if (v->flags & VM_IOREMAP)
> >  		seq_printf(m, " ioremap");
> >
> > +	if (v->flags & VM_DMA)
> > +		seq_printf(m, " dma");
> > +
>
> Hmm, VM_DMA would become generic flag?
> AFAIU, maybe VM_DMA would be used only on ARM arch.

Right now yes, it will be used only on ARM architecture, but maybe other architecture will
start using it once it is available.

> Of course, it isn't performance sensitive part but there in no reason to check it, either
> in other architecture except ARM.
> 
> I suggest following as
> 
> #ifdef CONFIG_ARM
> #define VM_DMA	0x00000040
> #else
> #define VM_DMA	0x0
> #end
> 
> Maybe it could remove check code at compile time.

I've been told to avoid such #ifdef construction if there is no really good reason for it.
The only justification was significant impact on the performance, otherwise it would be 
just a good example of typical over-engineering.

> >  	if (v->flags & VM_ALLOC)
> >  		seq_printf(m, " vmalloc");

Best regards

On Thu, May 24, 2012 at 02:26:12PM +0200, Marek Szyprowski wrote:
> On Tuesday, May 22, 2012 9:08 AM Minchan Kim wrote:
> > Hmm, VM_DMA would become generic flag?
> > AFAIU, maybe VM_DMA would be used only on ARM arch.
> 
> Right now yes, it will be used only on ARM architecture, but maybe other architecture will
> start using it once it is available.
> 
There's very little about the code in question that is ARM-specific to
begin with. I plan to adopt similar changes on SH once the work has
settled one way or the other, so we'll probably use the VMA flag there,
too.

On Thu, May 24, 2012 at 8:28 AM, Paul Mundt <lethal@linux-sh.org> wrote:
> On Thu, May 24, 2012 at 02:26:12PM +0200, Marek Szyprowski wrote:
>> On Tuesday, May 22, 2012 9:08 AM Minchan Kim wrote:
>> > Hmm, VM_DMA would become generic flag?
>> > AFAIU, maybe VM_DMA would be used only on ARM arch.
>>
>> Right now yes, it will be used only on ARM architecture, but maybe other architecture will
>> start using it once it is available.
>>
> There's very little about the code in question that is ARM-specific to
> begin with. I plan to adopt similar changes on SH once the work has
> settled one way or the other, so we'll probably use the VMA flag there,
> too.

I don't think VM_DMA is good idea because x86_64 has two dma zones. x86 unaware
patches make no sense.

Hello,

On Sunday, May 27, 2012 2:35 PM KOSAKI Motohiro wrote:

> On Thu, May 24, 2012 at 8:28 AM, Paul Mundt <lethal@linux-sh.org> wrote:
> > On Thu, May 24, 2012 at 02:26:12PM +0200, Marek Szyprowski wrote:
> >> On Tuesday, May 22, 2012 9:08 AM Minchan Kim wrote:
> >> > Hmm, VM_DMA would become generic flag?
> >> > AFAIU, maybe VM_DMA would be used only on ARM arch.
> >>
> >> Right now yes, it will be used only on ARM architecture, but maybe other architecture will
> >> start using it once it is available.
> >>
> > There's very little about the code in question that is ARM-specific to
> > begin with. I plan to adopt similar changes on SH once the work has
> > settled one way or the other, so we'll probably use the VMA flag there,
> > too.
> 
> I don't think VM_DMA is good idea because x86_64 has two dma zones. x86 unaware
> patches make no sense.

I see no problems to add VM_DMA64 later if x86_64 starts using vmalloc areas for creating 
kernel mappings for the dma buffers (I assume that there are 2 dma zones: one 32bit and one
64bit). Right now x86 and x86_64 don't use vmalloc areas for dma buffers, so I hardly see
how this patch can be considered as 'x86 unaware'.

Best regards

On Mon, May 28, 2012 at 10:19:39AM +0200, Marek Szyprowski wrote:
> Hello,
> 
> On Sunday, May 27, 2012 2:35 PM KOSAKI Motohiro wrote:
> 
> > On Thu, May 24, 2012 at 8:28 AM, Paul Mundt <lethal@linux-sh.org> wrote:
> > > On Thu, May 24, 2012 at 02:26:12PM +0200, Marek Szyprowski wrote:
> > >> On Tuesday, May 22, 2012 9:08 AM Minchan Kim wrote:
> > >> > Hmm, VM_DMA would become generic flag?
> > >> > AFAIU, maybe VM_DMA would be used only on ARM arch.
> > >>
> > >> Right now yes, it will be used only on ARM architecture, but maybe other architecture will
> > >> start using it once it is available.
> > >>
> > > There's very little about the code in question that is ARM-specific to
> > > begin with. I plan to adopt similar changes on SH once the work has
> > > settled one way or the other, so we'll probably use the VMA flag there,
> > > too.
> > 
> > I don't think VM_DMA is good idea because x86_64 has two dma zones. x86 unaware
> > patches make no sense.
> 
> I see no problems to add VM_DMA64 later if x86_64 starts using vmalloc areas for creating 
> kernel mappings for the dma buffers (I assume that there are 2 dma zones: one 32bit and one
> 64bit). Right now x86 and x86_64 don't use vmalloc areas for dma buffers, so I hardly see
> how this patch can be considered as 'x86 unaware'.

Well they do - kind off. It is usually done by calling vmalloc_32 and then using
the DMA API on top of those pages (or sometimes the non-portable virt_to_phys macro).

Introducing this and replacing the vmalloc_32 with this seems like a nice step in making
those device drivers APIs more portable?