On Thu, 2009-06-04 at 22:40 +0200, Geert Uytterhoeven wrote:
On Thu, Jun 4, 2009 at 20:07, Greg KH [off-list ref] wrote:
quoted
On Thu, Jun 04, 2009 at 07:01:32PM +0530, Subrata Modak wrote:
quoted
CC [M] drivers/staging/comedi/drivers.o
drivers/staging/comedi/drivers.c: In function ���comedi_buf_alloc���:
drivers/staging/comedi/drivers.c:496: error: ���PAGE_KERNEL_NOCACHE��� undeclared (first use in this function)
drivers/staging/comedi/drivers.c:496: error: (Each undeclared identifier is reported only once
drivers/staging/comedi/drivers.c:496: error: for each function it appears in.)
make[3]: *** [drivers/staging/comedi/drivers.o] Error 1
make[2]: *** [drivers/staging/comedi] Error 2
make[1]: *** [drivers/staging] Error 2
make: *** [drivers] Error 2
Is it still normal to expect this failure ?
Yes, and I took some patches that were supposed to fix this. I can't
duplicate this here, care to send a patch that would fix it?
I tried this. But, with some catch. ���PAGE_KERNEL_NOCACHE��� seems to be the
choice for majority of architectures like frv, m32r, sh, x86, etc, as Geert
mentions below. However, i believe POWERPC defines it as ���PAGE_KERNEL_NC���
found at arch/powerpc/include/asm/pte-common.h.
Paul/Banjamin,
Can you please confirm this ?
The below patch fixes the build error.
Signed-off-by: Subrata Modak <redacted>
---
@@ -492,8 +496,13 @@ int comedi_buf_alloc(struct comedi_devic}if(i==n_pages){async->prealloc_buf=+#ifdef __powerpc64__+vmap(pages,n_pages,VM_MAP,+PAGE_KERNEL_NC);+#elsevmap(pages,n_pages,VM_MAP,PAGE_KERNEL_NOCACHE);+#endif}if(pages){vfree(pages);
---
Regards--
Subrata
PAGE_KERNEL_NOCACHE is defined on a few architectures only, namely
frv, m32r, mn10300, sh, and x86[*]. On all others, it will fail.
[*] Interestingly, sparc refers to it in one of its include files,
without providing a
definition, Probably that code is unused.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
On Fri, Jun 05, 2009 at 01:26:25PM -0500, Subrata Modak wrote:
quoted
On Thu, 2009-06-04 at 22:40 +0200, Geert Uytterhoeven wrote:
On Thu, Jun 4, 2009 at 20:07, Greg KH [off-list ref] wrote:
quoted
On Thu, Jun 04, 2009 at 07:01:32PM +0530, Subrata Modak wrote:
quoted
CC [M] drivers/staging/comedi/drivers.o
drivers/staging/comedi/drivers.c: In function ‘comedi_buf_alloc’:
drivers/staging/comedi/drivers.c:496: error: ‘PAGE_KERNEL_NOCACHE’ undeclared (first use in this function)
drivers/staging/comedi/drivers.c:496: error: (Each undeclared identifier is reported only once
drivers/staging/comedi/drivers.c:496: error: for each function it appears in.)
make[3]: *** [drivers/staging/comedi/drivers.o] Error 1
make[2]: *** [drivers/staging/comedi] Error 2
make[1]: *** [drivers/staging] Error 2
make: *** [drivers] Error 2
Is it still normal to expect this failure ?
Yes, and I took some patches that were supposed to fix this. I can't
duplicate this here, care to send a patch that would fix it?
I tried this. But, with some catch. ‘PAGE_KERNEL_NOCACHE’ seems to be the
choice for majority of architectures like frv, m32r, sh, x86, etc, as Geert
mentions below. However, i believe POWERPC defines it as ‘PAGE_KERNEL_NC‘
found at arch/powerpc/include/asm/pte-common.h.
Paul/Banjamin,
Can you please confirm this ?
The below patch fixes the build error.
Should the comedi layer just not be using PAGE_KERNEL_NOCACHE here? I
can't believe that we need to do something like this in a driver. What
should the proper fix for this be?
thanks,
greg k-h
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2009-06-05 23:33:35
I tried this. But, with some catch. ‘PAGE_KERNEL_NOCACHE’ seems to be the
choice for majority of architectures like frv, m32r, sh, x86, etc, as Geert
mentions below. However, i believe POWERPC defines it as ‘PAGE_KERNEL_NC‘
found at arch/powerpc/include/asm/pte-common.h.
Paul/Banjamin,
Can you please confirm this ?
Read my reply to Greg. Why the heck are you trying to map memory
non-cacheable in the first place ?
Cheers
Ben.
quoted hunk
The below patch fixes the build error.
Signed-off-by: Subrata Modak <redacted>
---
@@ -492,8 +496,13 @@ int comedi_buf_alloc(struct comedi_devic}if(i==n_pages){async->prealloc_buf=+#ifdef __powerpc64__+vmap(pages,n_pages,VM_MAP,+PAGE_KERNEL_NC);+#elsevmap(pages,n_pages,VM_MAP,PAGE_KERNEL_NOCACHE);+#endif}if(pages){vfree(pages);
---
Regards--
Subrata
quoted
PAGE_KERNEL_NOCACHE is defined on a few architectures only, namely
frv, m32r, mn10300, sh, and x86[*]. On all others, it will fail.
[*] Interestingly, sparc refers to it in one of its include files,
without providing a
definition, Probably that code is unused.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2009-06-05 23:33:46
On Fri, 2009-06-05 at 11:26 -0700, Greg KH wrote:
Should the comedi layer just not be using PAGE_KERNEL_NOCACHE here? I
can't believe that we need to do something like this in a driver. What
should the proper fix for this be?
Actualy, I think powerpc -had- PAGE_KERNEL_NOCACHE and I removed it ...
Oh well... My understanding was that the exposed interface isn't that
but instead pgprot_noncached().
Nowadays we provide these on ppc:
#define pgprot_noncached(prot) (__pgprot((pgprot_val(prot) & ~_PAGE_CACHE_CTL) | \
_PAGE_NO_CACHE | _PAGE_GUARDED))
#define pgprot_noncached_wc(prot) (__pgprot((pgprot_val(prot) & ~_PAGE_CACHE_CTL) | \
_PAGE_NO_CACHE))
#define pgprot_cached(prot) (__pgprot((pgprot_val(prot) & ~_PAGE_CACHE_CTL) | \
_PAGE_COHERENT))
#define pgprot_cached_wthru(prot) (__pgprot((pgprot_val(prot) & ~_PAGE_CACHE_CTL) | \
_PAGE_COHERENT | _PAGE_WRITETHRU))
Now, of course, expect interesting problems if you use it in vmap, since
you are mapping struct pages, you are effectively mapping memory.
On some platforms, it's absolutely illegal to map memory non-cacheable while
this memory is mapped cacheable elsewhere and can be fatal. But the pages you
are mapping here, I suppose, are also part of the linear mapping which is ...
cacheable.
Why would you need that non-cacheable mapping in the first place ? If it's
for DMA, it's the wrong interface I believe....
Cheers,
Ben.
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2009-06-06 04:17:29
On Fri, 2009-06-05 at 17:56 -0700, David Miller wrote:
quoted
Read my reply to Greg. Why the heck are you trying to map memory
non-cacheable in the first place ?
I agree, this is extremely fishy.
I guess the issue is that the driver wants consistent DMA memory
but wants to allocate a huge area vmap() style.
That's my guess too, and I suppose we should be able to provide an
appropriate interface for that... There are two aspects that
are completely separate here:
- One is the allocation of the pages themselves which much match
the various criteria for DMA'bility to the target device (fit the
DMA mask, etc...)
- One is the creation of the virtual mapping in kernel space for which
appropriate pgprot for DMA must be provided.
For the first one, I don't know how legit it would be to allocate the
pages using dma_alloc_coherent one page at a time and try to figure out
the struct page * out of it. Sounds fishy and possibly non-portable. So
appart from using normal GFP and crossing fingers I'm not sure what
would be the right way to obtain the pages in the first place. Maybe we
should provide something.
The second could be as simple as having a pgprot_dma_coherent() like we
have a pgprot_uncached() for example, which would be either uncached or
cached depending on the consistency of DMA on the platform. But we need
to run that through things like MIPS which may have additional virtual
address space requirements.
Cheers,
Ben.
On Sat, Jun 06, 2009 at 02:16:46PM +1000, Benjamin Herrenschmidt wrote:
On Fri, 2009-06-05 at 17:56 -0700, David Miller wrote:
quoted
quoted
Read my reply to Greg. Why the heck are you trying to map memory
non-cacheable in the first place ?
I agree, this is extremely fishy.
I guess the issue is that the driver wants consistent DMA memory
but wants to allocate a huge area vmap() style.
That's my guess too, and I suppose we should be able to provide an
appropriate interface for that... There are two aspects that
are completely separate here:
- One is the allocation of the pages themselves which much match
the various criteria for DMA'bility to the target device (fit the
DMA mask, etc...)
- One is the creation of the virtual mapping in kernel space for which
appropriate pgprot for DMA must be provided.
For the first one, I don't know how legit it would be to allocate the
pages using dma_alloc_coherent one page at a time and try to figure out
the struct page * out of it. Sounds fishy and possibly non-portable. So
appart from using normal GFP and crossing fingers I'm not sure what
would be the right way to obtain the pages in the first place. Maybe we
should provide something.
The second could be as simple as having a pgprot_dma_coherent() like we
have a pgprot_uncached() for example, which would be either uncached or
cached depending on the consistency of DMA on the platform. But we need
to run that through things like MIPS which may have additional virtual
address space requirements.
All good questions. So, let's ask the original authors :)
Frank and Ian, any thoughts about the vmap call in the
comedi_buf_alloc() call? Why is it using PAGE_KERNEL_NOCACHE, and what
is the prealloc_buf buffer used for?
The problem is that PAGE_KERNEL_NOCACHE isn't a "standard" interface,
and not all architectures support it.
thanks,
greg k-h
From: Frank Mori Hess <hidden> Date: 2009-06-06 13:36:30
On Saturday 06 June 2009, Greg KH wrote:
Frank and Ian, any thoughts about the vmap call in the
comedi_buf_alloc() call? Why is it using PAGE_KERNEL_NOCACHE, and what
is the prealloc_buf buffer used for?
It is a circular buffer used to hold data streaming either to or from a
board (for example when producing an analog output waveform). Reads and
writes to the device files read/write to the circular buffer, plus a few
drivers do dma directly to/from it. I personally don't have a problem
with requiring drivers to have their own dma buffers and making them copy
data between their private dma buffers and the main circular buffer. I
guess the original design wanted to support zero-copy dma.
On Sat, 2009-06-06 at 09:36 -0400, Frank Mori Hess wrote:
On Saturday 06 June 2009, Greg KH wrote:
quoted
Frank and Ian, any thoughts about the vmap call in the
comedi_buf_alloc() call? Why is it using PAGE_KERNEL_NOCACHE, and what
is the prealloc_buf buffer used for?
It is a circular buffer used to hold data streaming either to or from a
board (for example when producing an analog output waveform). Reads and
writes to the device files read/write to the circular buffer, plus a few
drivers do dma directly to/from it. I personally don't have a problem
with requiring drivers to have their own dma buffers and making them copy
data between their private dma buffers and the main circular buffer. I
guess the original design wanted to support zero-copy dma.
Great to hear that. How about a patch that solves my build problem on
PPC64(the problem seems to be existing for long) ?
Regards--
Subrata
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2009-06-09 03:51:17
On Sun, 2009-06-07 at 20:06 +0530, Subrata Modak wrote:
On Sat, 2009-06-06 at 09:36 -0400, Frank Mori Hess wrote:
quoted
On Saturday 06 June 2009, Greg KH wrote:
quoted
Frank and Ian, any thoughts about the vmap call in the
comedi_buf_alloc() call? Why is it using PAGE_KERNEL_NOCACHE, and what
is the prealloc_buf buffer used for?
It is a circular buffer used to hold data streaming either to or from a
board (for example when producing an analog output waveform). Reads and
writes to the device files read/write to the circular buffer, plus a few
drivers do dma directly to/from it. I personally don't have a problem
with requiring drivers to have their own dma buffers and making them copy
data between their private dma buffers and the main circular buffer. I
guess the original design wanted to support zero-copy dma.
Great to hear that. How about a patch that solves my build problem on
PPC64(the problem seems to be existing for long) ?
In any case, doing PAGE_KERNEL_NOCACHE for DMA memory is incorrect on
many architectures. So at this stage, there's no much option but ifdef I
suspect for now until this is fixed properly.
It does make sense to want to have some memory like that shared between
user space and DMA, though I don't know what the right approach that
works on all archs is at this stage. Worth asking the Alsa guys, I think
they have similar issues :-)
But doing double buffering might do the trick fine for now.
Cheers,
Ben.
On Tue, 2009-06-09 at 13:50 +1000, Benjamin Herrenschmidt wrote:
On Sun, 2009-06-07 at 20:06 +0530, Subrata Modak wrote:
quoted
On Sat, 2009-06-06 at 09:36 -0400, Frank Mori Hess wrote:
quoted
On Saturday 06 June 2009, Greg KH wrote:
quoted
Frank and Ian, any thoughts about the vmap call in the
comedi_buf_alloc() call? Why is it using PAGE_KERNEL_NOCACHE, and what
is the prealloc_buf buffer used for?
It is a circular buffer used to hold data streaming either to or from a
board (for example when producing an analog output waveform). Reads and
writes to the device files read/write to the circular buffer, plus a few
drivers do dma directly to/from it. I personally don't have a problem
with requiring drivers to have their own dma buffers and making them copy
data between their private dma buffers and the main circular buffer. I
guess the original design wanted to support zero-copy dma.
Great to hear that. How about a patch that solves my build problem on
PPC64(the problem seems to be existing for long) ?
In any case, doing PAGE_KERNEL_NOCACHE for DMA memory is incorrect on
many architectures. So at this stage, there's no much option but ifdef I
suspect for now until this is fixed properly.
Ok. But, i am not sure whether Greg will agree to this. If, Ok, is the
following patch i sent earlier Ok ?
http://lkml.org/lkml/2009/6/5/462,
Regards--
Subrata
It does make sense to want to have some memory like that shared between
user space and DMA, though I don't know what the right approach that
works on all archs is at this stage. Worth asking the Alsa guys, I think
they have similar issues :-)
But doing double buffering might do the trick fine for now.
Cheers,
Ben.
On Tue, Jun 9, 2009 at 20:34, Subrata Modak[off-list ref] wro=
te:
On Tue, 2009-06-09 at 13:50 +1000, Benjamin Herrenschmidt wrote:
quoted
On Sun, 2009-06-07 at 20:06 +0530, Subrata Modak wrote:
quoted
On Sat, 2009-06-06 at 09:36 -0400, Frank Mori Hess wrote:
quoted
On Saturday 06 June 2009, Greg KH wrote:
quoted
Frank and Ian, any thoughts about the vmap call in the
comedi_buf_alloc() call? =C2=A0Why is it using PAGE_KERNEL_NOCACHE=
, and what
quoted
quoted
quoted
quoted
is the prealloc_buf buffer used for?
It is a circular buffer used to hold data streaming either to or fro=
m a
quoted
quoted
quoted
board (for example when producing an analog output waveform). =C2=A0=
Reads and
quoted
quoted
quoted
writes to the device files read/write to the circular buffer, plus a=
few
quoted
quoted
quoted
drivers do dma directly to/from it. =C2=A0I personally don't have a =
problem
quoted
quoted
quoted
with requiring drivers to have their own dma buffers and making them=
copy
quoted
quoted
quoted
data between their private dma buffers and the main circular buffer.=
=C2=A0I
quoted
quoted
quoted
guess the original design wanted to support zero-copy dma.
Great to hear that. How about a patch that solves my build problem on
PPC64(the problem seems to be existing for long) ?
In any case, doing PAGE_KERNEL_NOCACHE for DMA memory is incorrect on
many architectures. So at this stage, there's no much option but ifdef I
suspect for now until this is fixed properly.
Ok. But, i am not sure whether Greg will agree to this. If, Ok, is the
following patch i sent earlier Ok ?
http://lkml.org/lkml/2009/6/5/462,
Your patch helps powerpc only. Compilation is still broken on most
other architectures.
quoted
It does make sense to want to have some memory like that shared between
user space and DMA, though I don't know what the right approach that
works on all archs is at this stage. Worth asking the Alsa guys, I think
they have similar issues :-)
But doing double buffering might do the trick fine for now.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k=
.org
In personal conversations with technical people, I call myself a hacker. Bu=
t
when I'm talking to journalists I just say "programmer" or something like t=
hat.
-- Linus Torvalds
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2009-06-09 22:36:51
quoted
In any case, doing PAGE_KERNEL_NOCACHE for DMA memory is incorrect on
many architectures. So at this stage, there's no much option but ifdef I
suspect for now until this is fixed properly.
Ok. But, i am not sure whether Greg will agree to this. If, Ok, is the
following patch i sent earlier Ok ?
http://lkml.org/lkml/2009/6/5/462,
Not really.
You probably want to use a constant (call it MY_DMA_MAP_PGPROT), and
in a header, you have a bunch of ifdef's that set it to PAGE_KERNEL,
PAGE_KERNEL_NOCACHE or PAGE_KERNEL_NC depending on what's needed.
Today, you can pretty much assume that
- x86*, sparc*, ia64*, alpha, ... needs PAGE_KERNEL
- powerpc needs PAGE_KERNEL if !CONFIG_NOT_COHERENT_CACHE
- powerpc needs PAGE_KERNEL_NC if CONFIG_NOT_COHERENT_CACHE
- ARM and MIPS, I think, needs PAGE_KERNEL_NOCACHE
- ... others I don't know.
Cheers,
Ben.
Regards--
Subrata
quoted
It does make sense to want to have some memory like that shared between
user space and DMA, though I don't know what the right approach that
works on all archs is at this stage. Worth asking the Alsa guys, I think
they have similar issues :-)
But doing double buffering might do the trick fine for now.
Cheers,
Ben.