* Re: [PATCH 1/1] Userspace I/O (UIO): Add support for userspace DMA
[not found] <43FC624C55D8C746A914570B66D642610367F29B@cos-us-mb03.cos.agilent.com>
@ 2008-12-04 8:39 ` Peter Zijlstra
2008-12-04 10:27 ` Hugh Dickins
` (2 more replies)
0 siblings, 3 replies; 20+ messages in thread
From: Peter Zijlstra @ 2008-12-04 8:39 UTC (permalink / raw)
To: edward_estabrook
Cc: linux-kernel, hjk, gregkh, edward.estabrook, hugh, linux-mm,
Thomas Gleixner
On Wed, 2008-12-03 at 14:39 -0700, edward_estabrook@agilent.com wrote:
> From: Edward Estabrook <Edward_Estabrook@agilent.com>
>
> Here is a patch that adds the ability to dynamically allocate (and
> use) coherent DMA from userspace by extending the userspace IO driver.
> This patch applies against 2.6.28-rc6.
>
> The gist of this implementation is to overload uio's mmap
> functionality to allocate and map a new DMA region on demand. The
> bus-specific DMA address as returned by dma_alloc_coherent is made
> available to userspace in the 1st long word of the newly created
> region (as well as through the conventional 'addr' file in sysfs).
>
> To allocate a DMA region you use the following:
> /* Pass this magic number to mmap as offset to dynamically allocate a
> chunk of memory */ #define DMA_MEM_ALLOCATE_MMAP_OFFSET 0xFFFFF000UL
>
> void* memory = mmap (NULL, size, PROT_READ | PROT_WRITE , MAP_SHARED,
> fd, DMA_MEM_ALLOCATE_MMAP_OFFSET); u_int64_t *addr = *(u_int64_t *)
> memory;
>
> where 'size' is the size in bytes of the region you want and fd is the
> opened /dev/uioN file.
>
> Allocation occurs in page sized pieces by design to ensure that
> buffers are page-aligned.
>
> Memory is released when uio_unregister_device() is called.
>
> I have used this extensively on a 2.6.21-based kernel and ported it to
> 2.6.28-rc6 for review / submission here.
>
> Comments appreciated!
Yuck!
Why not create another special device that will give you DMA memory when
you mmap it? That would also allow you to obtain the physical address
without this utter horrid hack of writing it in the mmap'ed memory.
/dev/uioN-dma would seem like a fine name for that.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/1] Userspace I/O (UIO): Add support for userspace DMA
2008-12-04 8:39 ` [PATCH 1/1] Userspace I/O (UIO): Add support for userspace DMA Peter Zijlstra
@ 2008-12-04 10:27 ` Hugh Dickins
2008-12-23 21:32 ` Max Krasnyansky
2008-12-04 18:08 ` Hans J. Koch
2009-12-12 0:02 ` Earl Chew
2 siblings, 1 reply; 20+ messages in thread
From: Hugh Dickins @ 2008-12-04 10:27 UTC (permalink / raw)
To: Peter Zijlstra
Cc: edward_estabrook, linux-kernel, hjk, gregkh, edward.estabrook,
linux-mm, Thomas Gleixner
On Thu, 4 Dec 2008, Peter Zijlstra wrote:
> On Wed, 2008-12-03 at 14:39 -0700, edward_estabrook@agilent.com wrote:
> >
> > The gist of this implementation is to overload uio's mmap
> > functionality to allocate and map a new DMA region on demand. The
> > bus-specific DMA address as returned by dma_alloc_coherent is made
> > available to userspace in the 1st long word of the newly created
> > region (as well as through the conventional 'addr' file in sysfs).
> >
> > To allocate a DMA region you use the following:
> > /* Pass this magic number to mmap as offset to dynamically allocate a
> > chunk of memory */ #define DMA_MEM_ALLOCATE_MMAP_OFFSET 0xFFFFF000UL
> > ...
> > Comments appreciated!
>
> Yuck!
>
> Why not create another special device that will give you DMA memory when
> you mmap it? That would also allow you to obtain the physical address
> without this utter horrid hack of writing it in the mmap'ed memory.
>
> /dev/uioN-dma would seem like a fine name for that.
I couldn't agree more. It sounds fine as a local hack for Edward to
try out some functionality he needed in a hurry; but as something
that should enter the mainline kernel in that form - no.
Hugh
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/1] Userspace I/O (UIO): Add support for userspace DMA
2008-12-04 8:39 ` [PATCH 1/1] Userspace I/O (UIO): Add support for userspace DMA Peter Zijlstra
2008-12-04 10:27 ` Hugh Dickins
@ 2008-12-04 18:08 ` Hans J. Koch
2008-12-05 7:10 ` Peter Zijlstra
2009-12-12 0:02 ` Earl Chew
2 siblings, 1 reply; 20+ messages in thread
From: Hans J. Koch @ 2008-12-04 18:08 UTC (permalink / raw)
To: Peter Zijlstra
Cc: edward_estabrook, linux-kernel, hjk, gregkh, edward.estabrook,
hugh, linux-mm, Thomas Gleixner
On Thu, Dec 04, 2008 at 09:39:02AM +0100, Peter Zijlstra wrote:
> On Wed, 2008-12-03 at 14:39 -0700, edward_estabrook@agilent.com wrote:
> > From: Edward Estabrook <Edward_Estabrook@agilent.com>
> >
> > Here is a patch that adds the ability to dynamically allocate (and
> > use) coherent DMA from userspace by extending the userspace IO driver.
> > This patch applies against 2.6.28-rc6.
> >
> > The gist of this implementation is to overload uio's mmap
> > functionality to allocate and map a new DMA region on demand. The
> > bus-specific DMA address as returned by dma_alloc_coherent is made
> > available to userspace in the 1st long word of the newly created
> > region (as well as through the conventional 'addr' file in sysfs).
> >
> > To allocate a DMA region you use the following:
> > /* Pass this magic number to mmap as offset to dynamically allocate a
> > chunk of memory */ #define DMA_MEM_ALLOCATE_MMAP_OFFSET 0xFFFFF000UL
> >
> > void* memory = mmap (NULL, size, PROT_READ | PROT_WRITE , MAP_SHARED,
> > fd, DMA_MEM_ALLOCATE_MMAP_OFFSET); u_int64_t *addr = *(u_int64_t *)
> > memory;
> >
> > where 'size' is the size in bytes of the region you want and fd is the
> > opened /dev/uioN file.
> >
> > Allocation occurs in page sized pieces by design to ensure that
> > buffers are page-aligned.
> >
> > Memory is released when uio_unregister_device() is called.
> >
> > I have used this extensively on a 2.6.21-based kernel and ported it to
> > 2.6.28-rc6 for review / submission here.
> >
> > Comments appreciated!
>
> Yuck!
>
> Why not create another special device that will give you DMA memory when
> you mmap it? That would also allow you to obtain the physical address
> without this utter horrid hack of writing it in the mmap'ed memory.
>
> /dev/uioN-dma would seem like a fine name for that.
I don't like to have a separate device for DMA memory. It would completely
break the current concept of userspace drivers if you had to get normal
memory from one device and DMA memory from another. Note that one driver
can have both.
But I agree that it's confusing if the physical address is stored somewhere
in the mapped memory. That should simply be omitted, we have that information
in sysfs anyway - like for any other memory mappings. But I guess we need
some kind of "type" or "flags" attribute for the mappings so that userspace
can find out if a mapping is DMA capable or not.
Thanks,
Hans
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/1] Userspace I/O (UIO): Add support for userspace DMA
2008-12-04 18:08 ` Hans J. Koch
@ 2008-12-05 7:10 ` Peter Zijlstra
2008-12-05 9:44 ` Hans J. Koch
0 siblings, 1 reply; 20+ messages in thread
From: Peter Zijlstra @ 2008-12-05 7:10 UTC (permalink / raw)
To: Hans J. Koch
Cc: edward_estabrook, linux-kernel, gregkh, edward.estabrook, hugh,
linux-mm, Thomas Gleixner
On Thu, 2008-12-04 at 19:08 +0100, Hans J. Koch wrote:
> On Thu, Dec 04, 2008 at 09:39:02AM +0100, Peter Zijlstra wrote:
> > On Wed, 2008-12-03 at 14:39 -0700, edward_estabrook@agilent.com wrote:
> > > From: Edward Estabrook <Edward_Estabrook@agilent.com>
> > >
> > > Here is a patch that adds the ability to dynamically allocate (and
> > > use) coherent DMA from userspace by extending the userspace IO driver.
> > > This patch applies against 2.6.28-rc6.
> > >
> > > The gist of this implementation is to overload uio's mmap
> > > functionality to allocate and map a new DMA region on demand. The
> > > bus-specific DMA address as returned by dma_alloc_coherent is made
> > > available to userspace in the 1st long word of the newly created
> > > region (as well as through the conventional 'addr' file in sysfs).
> > >
> > > To allocate a DMA region you use the following:
> > > /* Pass this magic number to mmap as offset to dynamically allocate a
> > > chunk of memory */ #define DMA_MEM_ALLOCATE_MMAP_OFFSET 0xFFFFF000UL
> > >
> > > void* memory = mmap (NULL, size, PROT_READ | PROT_WRITE , MAP_SHARED,
> > > fd, DMA_MEM_ALLOCATE_MMAP_OFFSET); u_int64_t *addr = *(u_int64_t *)
> > > memory;
> > >
> > > where 'size' is the size in bytes of the region you want and fd is the
> > > opened /dev/uioN file.
> > >
> > > Allocation occurs in page sized pieces by design to ensure that
> > > buffers are page-aligned.
> > >
> > > Memory is released when uio_unregister_device() is called.
> > >
> > > I have used this extensively on a 2.6.21-based kernel and ported it to
> > > 2.6.28-rc6 for review / submission here.
> > >
> > > Comments appreciated!
> >
> > Yuck!
> >
> > Why not create another special device that will give you DMA memory when
> > you mmap it? That would also allow you to obtain the physical address
> > without this utter horrid hack of writing it in the mmap'ed memory.
> >
> > /dev/uioN-dma would seem like a fine name for that.
>
> I don't like to have a separate device for DMA memory. It would completely
> break the current concept of userspace drivers if you had to get normal
> memory from one device and DMA memory from another. Note that one driver
> can have both.
How would that break anything, the one driver can simply open both
files.
> But I agree that it's confusing if the physical address is stored somewhere
> in the mapped memory. That should simply be omitted, we have that information
> in sysfs anyway - like for any other memory mappings. But I guess we need
> some kind of "type" or "flags" attribute for the mappings so that userspace
> can find out if a mapping is DMA capable or not.
We have that, different file.
I'll NAK any attempt that rapes the mmap interface like proposed - that
is just not an option.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/1] Userspace I/O (UIO): Add support for userspace DMA
2008-12-05 7:10 ` Peter Zijlstra
@ 2008-12-05 9:44 ` Hans J. Koch
2008-12-06 0:32 ` Edward Estabrook
0 siblings, 1 reply; 20+ messages in thread
From: Hans J. Koch @ 2008-12-05 9:44 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Hans J. Koch, edward_estabrook, linux-kernel, gregkh,
edward.estabrook, hugh, linux-mm, Thomas Gleixner
On Fri, Dec 05, 2008 at 08:10:59AM +0100, Peter Zijlstra wrote:
> > I don't like to have a separate device for DMA memory. It would completely
> > break the current concept of userspace drivers if you had to get normal
> > memory from one device and DMA memory from another. Note that one driver
> > can have both.
>
> How would that break anything, the one driver can simply open both
> files.
I agree there's not much breakage, but it's a difference in handling things.
People who wrote libraries that generically find mappable memory of a UIO
device need to change their handling.
>
> > But I agree that it's confusing if the physical address is stored somewhere
> > in the mapped memory. That should simply be omitted, we have that information
> > in sysfs anyway - like for any other memory mappings. But I guess we need
> > some kind of "type" or "flags" attribute for the mappings so that userspace
> > can find out if a mapping is DMA capable or not.
>
> We have that, different file.
>
> I'll NAK any attempt that rapes the mmap interface like proposed - that
> is just not an option.
Well, UIO already rapes the mmap interface by using the "offset" parameter to
pass in the number of the mapping.
But I'll NAK the current concept, too. It's a UIO kernel driver's task to tell
userspace which memory a device has to offer. The UIO core prevents userspace
as much as possible from mapping anything different. And it should stay that
way.
Thanks,
Hans
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/1] Userspace I/O (UIO): Add support for userspace DMA
2008-12-05 9:44 ` Hans J. Koch
@ 2008-12-06 0:32 ` Edward Estabrook
2008-12-12 17:25 ` Peter Zijlstra
0 siblings, 1 reply; 20+ messages in thread
From: Edward Estabrook @ 2008-12-06 0:32 UTC (permalink / raw)
To: Hans J. Koch
Cc: Peter Zijlstra, edward_estabrook, linux-kernel, gregkh,
edward.estabrook, hugh, linux-mm, Thomas Gleixner
> Well, UIO already rapes the mmap interface by using the "offset" parameter to
> pass in the number of the mapping.
Exactly.
> But I'll NAK the current concept, too. It's a UIO kernel driver's task to tell
> userspace which memory a device has to offer. The UIO core prevents userspace
> as much as possible from mapping anything different. And it should stay that
> way.
The ultimate purpose (I thought) of the UIO driver is to simplify
driver development
by pushing device control into userspace. There is a very real need
for efficient
dynamic control over the DMA allocation of a device. Why not 'allow' this to
happen in userspace if it can be done safely and without breaking anything else?
Remember that for devices employing ring buffers it is not a question of
'how much memory a device has to offer' but rather 'how much system
memory would the
driver like to configure that device to use'.
I don't want to stop my DMA engine and reload the driver to create
more buffers (and I don't
want to pre-allocate more than I need as contingency).
Cheers,
Ed
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/1] Userspace I/O (UIO): Add support for userspace DMA
2008-12-06 0:32 ` Edward Estabrook
@ 2008-12-12 17:25 ` Peter Zijlstra
2008-12-13 0:29 ` Hans J. Koch
0 siblings, 1 reply; 20+ messages in thread
From: Peter Zijlstra @ 2008-12-12 17:25 UTC (permalink / raw)
To: Edward Estabrook
Cc: Hans J. Koch, edward_estabrook, linux-kernel, gregkh,
edward.estabrook, hugh, linux-mm, Thomas Gleixner
On Fri, 2008-12-05 at 16:32 -0800, Edward Estabrook wrote:
> > Well, UIO already rapes the mmap interface by using the "offset" parameter to
> > pass in the number of the mapping.
>
> Exactly.
Had I known about it then, I'd NAK'd it, but I guess now that its
already merged changing it will be hard :/
Also, having done something bad in the past doesn't mean you can
continue doing the wrong thing.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/1] Userspace I/O (UIO): Add support for userspace DMA
2008-12-12 17:25 ` Peter Zijlstra
@ 2008-12-13 0:29 ` Hans J. Koch
0 siblings, 0 replies; 20+ messages in thread
From: Hans J. Koch @ 2008-12-13 0:29 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Edward Estabrook, Hans J. Koch, edward_estabrook, linux-kernel,
gregkh, edward.estabrook, hugh, linux-mm, Thomas Gleixner
On Fri, Dec 12, 2008 at 06:25:12PM +0100, Peter Zijlstra wrote:
> On Fri, 2008-12-05 at 16:32 -0800, Edward Estabrook wrote:
> > > Well, UIO already rapes the mmap interface by using the "offset" parameter to
> > > pass in the number of the mapping.
> >
> > Exactly.
>
> Had I known about it then, I'd NAK'd it, but I guess now that its
> already merged changing it will be hard :/
It was in -mm for half a year before it went to mainline in 2.6.23, the
documentation being present all the time. It was discussed intensively
on lkml, and several core kernel developers reviewed it. The special use
of the mmap() offset parameter was never even mentioned by anybody. I
remember that so well because I actually expected critizism, but everybody
was fine with it. And to be honest, even though it's unusual, I still find
it a good solution.
Thanks,
Hans
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/1] Userspace I/O (UIO): Add support for userspace DMA
2008-12-04 10:27 ` Hugh Dickins
@ 2008-12-23 21:32 ` Max Krasnyansky
0 siblings, 0 replies; 20+ messages in thread
From: Max Krasnyansky @ 2008-12-23 21:32 UTC (permalink / raw)
To: Hugh Dickins
Cc: Peter Zijlstra, edward_estabrook, linux-kernel, hjk, gregkh,
edward.estabrook, linux-mm, Thomas Gleixner
Hugh Dickins wrote:
> On Thu, 4 Dec 2008, Peter Zijlstra wrote:
>> On Wed, 2008-12-03 at 14:39 -0700, edward_estabrook@agilent.com wrote:
>>> The gist of this implementation is to overload uio's mmap
>>> functionality to allocate and map a new DMA region on demand. The
>>> bus-specific DMA address as returned by dma_alloc_coherent is made
>>> available to userspace in the 1st long word of the newly created
>>> region (as well as through the conventional 'addr' file in sysfs).
>>>
>>> To allocate a DMA region you use the following:
>>> /* Pass this magic number to mmap as offset to dynamically allocate a
>>> chunk of memory */ #define DMA_MEM_ALLOCATE_MMAP_OFFSET 0xFFFFF000UL
>>> ...
>>> Comments appreciated!
>> Yuck!
>>
>> Why not create another special device that will give you DMA memory when
>> you mmap it? That would also allow you to obtain the physical address
>> without this utter horrid hack of writing it in the mmap'ed memory.
>>
>> /dev/uioN-dma would seem like a fine name for that.
>
> I couldn't agree more. It sounds fine as a local hack for Edward to
> try out some functionality he needed in a hurry; but as something
> that should enter the mainline kernel in that form - no.
Agree with Peter and Hugh here. Also I have a use case where I need to share
DMA buffers between two or more devices. So I think we need a generic DMA
device that does operations like alloc, mmap, etc. Mmapped regions can then be
used with UIO devices.
I'll put together a prototype of that some time early next year.
Max
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/1] Userspace I/O (UIO): Add support for userspace DMA
2008-12-04 8:39 ` [PATCH 1/1] Userspace I/O (UIO): Add support for userspace DMA Peter Zijlstra
2008-12-04 10:27 ` Hugh Dickins
2008-12-04 18:08 ` Hans J. Koch
@ 2009-12-12 0:02 ` Earl Chew
2009-12-14 19:23 ` Hans J. Koch
2 siblings, 1 reply; 20+ messages in thread
From: Earl Chew @ 2009-12-12 0:02 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: linux-kernel, hjk, gregkh, hugh, linux-mm, Thomas Gleixner
I'm taking another look at the changes that were submitted in
http://lkml.org/lkml/2008/12/3/453
to see if they can be made more palatable.
In http://lkml.org/lkml/2008/12/4/64 you wrote:
> Why not create another special device that will give you DMA memory when
> you mmap it? That would also allow you to obtain the physical address
> without this utter horrid hack of writing it in the mmap'ed memory.
>
> /dev/uioN-dma would seem like a fine name for that.
I understand the main objection was the hack to return the physical
address of the allocated DMA buffer within the buffer itself amongst
some other things.
Your suggestion was to create /dev/uioN-dma for the purpose of
allocating DMA memory.
I'm having trouble figuring out how this would help to return the
physical (bus) address of the DMA memory in a more elegant manner.
What idea did you have for the userspace program to obtain
the physical (bus) of the allocated DMA memory buffer?
Earl
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/1] Userspace I/O (UIO): Add support for userspace DMA
2009-12-12 0:02 ` Earl Chew
@ 2009-12-14 19:23 ` Hans J. Koch
2009-12-15 13:34 ` Earl Chew
0 siblings, 1 reply; 20+ messages in thread
From: Hans J. Koch @ 2009-12-14 19:23 UTC (permalink / raw)
To: Earl Chew
Cc: Peter Zijlstra, linux-kernel, hjk, gregkh, hugh, linux-mm,
Thomas Gleixner
On Fri, Dec 11, 2009 at 04:02:17PM -0800, Earl Chew wrote:
> I'm taking another look at the changes that were submitted in
>
> http://lkml.org/lkml/2008/12/3/453
>
> to see if they can be made more palatable.
>
>
> In http://lkml.org/lkml/2008/12/4/64 you wrote:
>
> > Why not create another special device that will give you DMA memory when
> > you mmap it? That would also allow you to obtain the physical address
> > without this utter horrid hack of writing it in the mmap'ed memory.
> >
> > /dev/uioN-dma would seem like a fine name for that.
>
>
> I understand the main objection was the hack to return the physical
> address of the allocated DMA buffer within the buffer itself amongst
> some other things.
The general thing is this: The UIO core supports only static mappings.
The possible number of mappings is usually set at compile time or module
load time and is currently limited to MAX_UIO_MAPS (== 5). This number
is usually sufficient for devices like PCI cards, which have a limited
number of mappings, too. All drivers currently in the kernel only need
one or two.
The current implementation of the UIO core is simply not made for
dynamic allocation of an unlimited amount of new mappings at runtime. As
we have seen in this patch, it needs raping of a documented kernel
interface to userspace. This is not acceptable.
So the easiest correct solution is to create a new device (e.g.
/dev/uioN-dma, as Peter suggested). It should only be created for a UIO
driver if it has a certain flag set, something like UIO_NEEDS_DYN_DMA_ALLOC.
>
> Your suggestion was to create /dev/uioN-dma for the purpose of
> allocating DMA memory.
>
> I'm having trouble figuring out how this would help to return the
> physical (bus) address of the DMA memory in a more elegant manner.
If you create this new device, you can invent any (reasonable) interface you
like. It should probably be something in sysfs, where you can write to a
file to allocate a new buffer, and read the address from some other.
It should also be possible to free a buffer again.
Thanks,
Hans
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/1] Userspace I/O (UIO): Add support for userspace DMA
2009-12-14 19:23 ` Hans J. Koch
@ 2009-12-15 13:34 ` Earl Chew
2009-12-15 17:47 ` Earl Chew
2009-12-15 21:00 ` Hans J. Koch
0 siblings, 2 replies; 20+ messages in thread
From: Earl Chew @ 2009-12-15 13:34 UTC (permalink / raw)
To: Hans J. Koch
Cc: Peter Zijlstra, linux-kernel, gregkh, hugh, linux-mm, Thomas Gleixner
Hans,
Thanks for the considered reply.
Hans J. Koch wrote:
> The general thing is this: The UIO core supports only static mappings.
> The possible number of mappings is usually set at compile time or module
> load time and is currently limited to MAX_UIO_MAPS (== 5). This number
> is usually sufficient for devices like PCI cards, which have a limited
> number of mappings, too. All drivers currently in the kernel only need
> one or two.
I'd like to proceed by changing struct uio_mem [MAX_UIO_MAPS] to a
linked list.
The driver code in uio_find_mem_index(), uio_dev_add_attributes(), etc,
iterate through the (small) array anyway, and the list space and
performance overhead is not significant for the cases mentioned.
Such a change would make it easier to track dynamically allocated
regions as well as pre-allocated mapping regions in the same data
structure.
It also plays more nicely into the next part ...
> The current implementation of the UIO core is simply not made for
> dynamic allocation of an unlimited amount of new mappings at runtime. As
> we have seen in this patch, it needs raping of a documented kernel
> interface to userspace. This is not acceptable.
>
> So the easiest correct solution is to create a new device (e.g.
> /dev/uioN-dma, as Peter suggested). It should only be created for a UIO
> driver if it has a certain flag set, something like UIO_NEEDS_DYN_DMA_ALLOC.
An approach which would play better with our existing codebase would
be to introduce a two-step ioctl-mmap.
a. Use an ioctl() to allocate the DMA buffer. The ioctl returns two
things:
1. A mapping (page) number
2. A physical (bus) address
b. Use the existing mmap() interface to gain access to the
DMA buffer allocated in (a). Clients would invoke mmap()
and use the mapping (page) number returned in (a) to
obtain userspace access to the DMA buffer.
I think that the second step (b) would play nicely with the existing
mmap() interface exposed by the UIO driver.
Using an ioctl() provides a cleaner way to return the physical
(bus) address of the DMA buffer.
Existing client code that is not interested in DMA buffers do
not incur a penalty because it will not invoke the new ioctl().
Earl
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/1] Userspace I/O (UIO): Add support for userspace DMA
2009-12-15 13:34 ` Earl Chew
@ 2009-12-15 17:47 ` Earl Chew
2009-12-15 21:33 ` Hans J. Koch
2009-12-15 21:00 ` Hans J. Koch
1 sibling, 1 reply; 20+ messages in thread
From: Earl Chew @ 2009-12-15 17:47 UTC (permalink / raw)
To: Hans J. Koch
Cc: Peter Zijlstra, linux-kernel, gregkh, hugh, linux-mm, Thomas Gleixner
Earl Chew wrote:
> I'd like to proceed by changing struct uio_mem [MAX_UIO_MAPS] to a
> linked list.
Here's my first go at changing to a linked list. I mistakenly
said that I would proceed by modifying struct uio_mem [MAX_UIO_MAPS].
In fact, I think that mem[MAX_UIO_MAPS] in struct uio_info can remain
intact -- thus preserving the client facing API.
The linked list change occurs in struct uio_device which is private
to uio.c ... thus hiding the change from client facing code.
Tying the regions into a linked list held by struct uio_device
simplifies the search-modify code in the rest of uio.c.
Earl
diff -uNpw uio_driver.h.old uio_driver.h
--- c:/tmp/makereview.AGgMh 2009-12-15 09:41:23.146608800 -0800
+++ c:/tmp/makereview.oXYWW 2009-12-15 09:41:23.255983800 -0800
@@ -20,6 +20,8 @@
/**
* struct uio_mem - description of a UIO memory region
+ * @list_node: list node for struct uio_device list
+ * @mapid: mapping number for this region
* @kobj: kobject for this mapping
* @addr: address of the device's memory
* @size: size of IO
@@ -27,6 +29,8 @@
* @internal_addr: ioremap-ped version of addr, for driver internal use
*/
struct uio_mem {
+ struct list_head list_node;
+ unsigned map_id;
struct kobject kobj;
unsigned long addr;
unsigned long size;
diff -uNpw uio.c.old uio.c
--- c:/tmp/makereview.O27Xv 2009-12-15 09:41:26.740358800 -0800
+++ c:/tmp/makereview.DAhCY 2009-12-15 09:41:26.834108800 -0800
@@ -35,6 +35,7 @@ struct uio_device {
int vma_count;
struct uio_info *info;
struct kset map_attr_kset;
+ struct list_head mem_list;
};
static int uio_major;
@@ -153,7 +154,7 @@ static int uio_dev_add_attributes(struct
if (ret)
goto err_group;
- for (mi = 0; mi < MAX_UIO_MAPS; mi++) {
+ for (mi = 0; mi < ARRAY_SIZE(idev->info->mem); mi++) {
mem = &idev->info->mem[mi];
if (mem->size == 0)
break;
@@ -166,6 +167,12 @@ static int uio_dev_add_attributes(struct
if (ret)
goto err_remove_group;
}
+
+ mem->map_id = mi;
+
+ INIT_LIST_HEAD(&mem->list_node);
+ list_add(&mem->list_node, &idev->mem_list);
+
kobject_init(&mem->kobj);
kobject_set_name(&mem->kobj,"map%d",mi);
mem->kobj.parent = &idev->map_attr_kset.kobj;
@@ -180,6 +187,7 @@ static int uio_dev_add_attributes(struct
err_remove_maps:
for (mi--; mi>=0; mi--) {
mem = &idev->info->mem[mi];
+ list_del(&mem->list_node);
kobject_unregister(&mem->kobj);
}
kset_unregister(&idev->map_attr_kset); /* Needed ? */
@@ -194,10 +202,11 @@ static void uio_dev_del_attributes(struc
{
int mi;
struct uio_mem *mem;
- for (mi = 0; mi < MAX_UIO_MAPS; mi++) {
+ for (mi = 0; mi < ARRAY_SIZE(idev->info->mem); mi++) {
mem = &idev->info->mem[mi];
if (mem->size == 0)
break;
+ list_del(&mem->list_node);
kobject_unregister(&mem->kobj);
}
kset_unregister(&idev->map_attr_kset);
@@ -386,18 +395,20 @@ static ssize_t uio_read(struct file *fil
return retval;
}
-static int uio_find_mem_index(struct vm_area_struct *vma)
+static struct uio_mem *uio_find_mem_index(struct vm_area_struct *vma)
{
- int mi;
struct uio_device *idev = vma->vm_private_data;
+ struct list_head *mem_list;
+
+ list_for_each(mem_list, &idev->mem_list) {
- for (mi = 0; mi < MAX_UIO_MAPS; mi++) {
- if (idev->info->mem[mi].size == 0)
- return -1;
- if (vma->vm_pgoff == mi)
- return mi;
+ struct uio_mem *mem =
+ list_entry(mem_list, struct uio_mem, list_node);
+
+ if (vma->vm_pgoff == mem->map_id)
+ return mem;
}
- return -1;
+ return NULL;
}
static void uio_vma_open(struct vm_area_struct *vma)
@@ -415,17 +426,16 @@ static void uio_vma_close(struct vm_area
static struct page *uio_vma_nopage(struct vm_area_struct *vma,
unsigned long address, int *type)
{
- struct uio_device *idev = vma->vm_private_data;
struct page* page = NOPAGE_SIGBUS;
- int mi = uio_find_mem_index(vma);
- if (mi < 0)
+ struct uio_mem *mem = uio_find_mem_index(vma);
+ if (mem == NULL)
return page;
- if (idev->info->mem[mi].memtype == UIO_MEM_LOGICAL)
- page = virt_to_page(idev->info->mem[mi].addr);
+ if (mem->memtype == UIO_MEM_LOGICAL)
+ page = virt_to_page(mem->addr);
else
- page = vmalloc_to_page((void*)idev->info->mem[mi].addr);
+ page = vmalloc_to_page((void*)mem->addr);
get_page(page);
if (type)
*type = VM_FAULT_MINOR;
@@ -440,16 +450,15 @@ static struct vm_operations_struct uio_v
static int uio_mmap_physical(struct vm_area_struct *vma)
{
- struct uio_device *idev = vma->vm_private_data;
- int mi = uio_find_mem_index(vma);
- if (mi < 0)
+ struct uio_mem *mem = uio_find_mem_index(vma);
+ if (mem == NULL)
return -EINVAL;
vma->vm_flags |= VM_IO | VM_RESERVED;
return remap_pfn_range(vma,
vma->vm_start,
- idev->info->mem[mi].addr >> PAGE_SHIFT,
+ mem->addr >> PAGE_SHIFT,
vma->vm_end - vma->vm_start,
vma->vm_page_prot);
}
@@ -466,7 +475,7 @@ static int uio_mmap(struct file *filep,
{
struct uio_listener *listener = filep->private_data;
struct uio_device *idev = listener->dev;
- int mi;
+ struct uio_mem *mem;
unsigned long requested_pages, actual_pages;
int ret = 0;
@@ -475,12 +484,12 @@ static int uio_mmap(struct file *filep,
vma->vm_private_data = idev;
- mi = uio_find_mem_index(vma);
- if (mi < 0)
+ mem = uio_find_mem_index(vma);
+ if (mem == NULL)
return -EINVAL;
requested_pages = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
- actual_pages = (idev->info->mem[mi].size + PAGE_SIZE -1) >> PAGE_SHIFT;
+ actual_pages = (mem->size + PAGE_SIZE -1) >> PAGE_SHIFT;
if (requested_pages > actual_pages)
return -EINVAL;
@@ -492,7 +501,7 @@ static int uio_mmap(struct file *filep,
return ret;
}
- switch (idev->info->mem[mi].memtype) {
+ switch (mem->memtype) {
case UIO_MEM_PHYS:
return uio_mmap_physical(vma);
case UIO_MEM_LOGICAL:
@@ -613,6 +622,7 @@ int __uio_register_device(struct module
idev->info = info;
init_waitqueue_head(&idev->wait);
atomic_set(&idev->event, 0);
+ INIT_LIST_HEAD(&idev->mem_list);
ret = uio_get_minor(idev);
if (ret)
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/1] Userspace I/O (UIO): Add support for userspace DMA
2009-12-15 13:34 ` Earl Chew
2009-12-15 17:47 ` Earl Chew
@ 2009-12-15 21:00 ` Hans J. Koch
2009-12-15 21:47 ` Earl Chew
1 sibling, 1 reply; 20+ messages in thread
From: Hans J. Koch @ 2009-12-15 21:00 UTC (permalink / raw)
To: Earl Chew
Cc: Hans J. Koch, Peter Zijlstra, linux-kernel, gregkh, hugh,
linux-mm, Thomas Gleixner
On Tue, Dec 15, 2009 at 05:34:19AM -0800, Earl Chew wrote:
> Hans,
Hi Earl,
>
> Thanks for the considered reply.
>
>
> Hans J. Koch wrote:
> > The general thing is this: The UIO core supports only static mappings.
> > The possible number of mappings is usually set at compile time or module
> > load time and is currently limited to MAX_UIO_MAPS (== 5). This number
> > is usually sufficient for devices like PCI cards, which have a limited
> > number of mappings, too. All drivers currently in the kernel only need
> > one or two.
>
>
> I'd like to proceed by changing struct uio_mem [MAX_UIO_MAPS] to a
> linked list.
>
> The driver code in uio_find_mem_index(), uio_dev_add_attributes(), etc,
> iterate through the (small) array anyway, and the list space and
> performance overhead is not significant for the cases mentioned.
>
> Such a change would make it easier to track dynamically allocated
> regions as well as pre-allocated mapping regions in the same data
> structure.
Sorry, I think I wasn't clear enough: The current interface for static
mappings shouldn't be changed. Dynamically added mappings need a new
interface.
>
> It also plays more nicely into the next part ...
>
> > The current implementation of the UIO core is simply not made for
> > dynamic allocation of an unlimited amount of new mappings at runtime. As
> > we have seen in this patch, it needs raping of a documented kernel
> > interface to userspace. This is not acceptable.
> >
> > So the easiest correct solution is to create a new device (e.g.
> > /dev/uioN-dma, as Peter suggested). It should only be created for a UIO
> > driver if it has a certain flag set, something like UIO_NEEDS_DYN_DMA_ALLOC.
>
> An approach which would play better with our existing codebase would
> be to introduce a two-step ioctl-mmap.
>
> a. Use an ioctl() to allocate the DMA buffer. The ioctl returns two
> things:
No. We don't want any new ioctls in the kernel.
>
> 1. A mapping (page) number
> 2. A physical (bus) address
>
>
> b. Use the existing mmap() interface to gain access to the
> DMA buffer allocated in (a). Clients would invoke mmap()
> and use the mapping (page) number returned in (a) to
> obtain userspace access to the DMA buffer.
>
>
> I think that the second step (b) would play nicely with the existing
> mmap() interface exposed by the UIO driver.
The existing interface is for static mappings only.
>
>
> Using an ioctl() provides a cleaner way to return the physical
> (bus) address of the DMA buffer.
ioctl() is out of fashion today. We have sysfs. Note that ioctls are neither
typesafe nor much faster than sysfs.
>
>
> Existing client code that is not interested in DMA buffers do
> not incur a penalty because it will not invoke the new ioctl().
What about userspace tools that rely on the fact that the number of mappings
for a UIO device cannot change? This is a documented property of UIO.
Dynamically allocated mappings really call for a new device as Peter suggested.
In fact, that would make life much easier for you. Since your the one who
implements that stuff, your free to define a new interface. Surely that new
interface will be discussed and rejected two or three times, but in the end
we'll have a nice interface that allows UIO to use DMA, even with dyynamically
allocated buffers.
Use that freedom and create a new device with a new interface. There's no
point in trying to change existing and well documented interfaces to userspace.
Thanks,
Hans
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/1] Userspace I/O (UIO): Add support for userspace DMA
2009-12-15 17:47 ` Earl Chew
@ 2009-12-15 21:33 ` Hans J. Koch
0 siblings, 0 replies; 20+ messages in thread
From: Hans J. Koch @ 2009-12-15 21:33 UTC (permalink / raw)
To: Earl Chew
Cc: Hans J. Koch, Peter Zijlstra, linux-kernel, gregkh, hugh,
linux-mm, Thomas Gleixner
(Deleted hugh <hugh@veritas.com> from Cc:, this address has permanent errors.)
On Tue, Dec 15, 2009 at 09:47:34AM -0800, Earl Chew wrote:
> Earl Chew wrote:
> > I'd like to proceed by changing struct uio_mem [MAX_UIO_MAPS] to a
> > linked list.
>
> Here's my first go at changing to a linked list. I mistakenly
> said that I would proceed by modifying struct uio_mem [MAX_UIO_MAPS].
NAK, for the reasons given in my other mail.
Thanks,
Hans
>
> In fact, I think that mem[MAX_UIO_MAPS] in struct uio_info can remain
> intact -- thus preserving the client facing API.
>
> The linked list change occurs in struct uio_device which is private
> to uio.c ... thus hiding the change from client facing code.
>
>
> Tying the regions into a linked list held by struct uio_device
> simplifies the search-modify code in the rest of uio.c.
>
>
> Earl
>
>
> diff -uNpw uio_driver.h.old uio_driver.h
> --- c:/tmp/makereview.AGgMh 2009-12-15 09:41:23.146608800 -0800
> +++ c:/tmp/makereview.oXYWW 2009-12-15 09:41:23.255983800 -0800
> @@ -20,6 +20,8 @@
>
> /**
> * struct uio_mem - description of a UIO memory region
> + * @list_node: list node for struct uio_device list
> + * @mapid: mapping number for this region
> * @kobj: kobject for this mapping
> * @addr: address of the device's memory
> * @size: size of IO
> @@ -27,6 +29,8 @@
> * @internal_addr: ioremap-ped version of addr, for driver internal use
> */
> struct uio_mem {
> + struct list_head list_node;
> + unsigned map_id;
> struct kobject kobj;
> unsigned long addr;
> unsigned long size;
>
> diff -uNpw uio.c.old uio.c
> --- c:/tmp/makereview.O27Xv 2009-12-15 09:41:26.740358800 -0800
> +++ c:/tmp/makereview.DAhCY 2009-12-15 09:41:26.834108800 -0800
> @@ -35,6 +35,7 @@ struct uio_device {
> int vma_count;
> struct uio_info *info;
> struct kset map_attr_kset;
> + struct list_head mem_list;
> };
>
> static int uio_major;
> @@ -153,7 +154,7 @@ static int uio_dev_add_attributes(struct
> if (ret)
> goto err_group;
>
> - for (mi = 0; mi < MAX_UIO_MAPS; mi++) {
> + for (mi = 0; mi < ARRAY_SIZE(idev->info->mem); mi++) {
> mem = &idev->info->mem[mi];
> if (mem->size == 0)
> break;
> @@ -166,6 +167,12 @@ static int uio_dev_add_attributes(struct
> if (ret)
> goto err_remove_group;
> }
> +
> + mem->map_id = mi;
> +
> + INIT_LIST_HEAD(&mem->list_node);
> + list_add(&mem->list_node, &idev->mem_list);
> +
> kobject_init(&mem->kobj);
> kobject_set_name(&mem->kobj,"map%d",mi);
> mem->kobj.parent = &idev->map_attr_kset.kobj;
> @@ -180,6 +187,7 @@ static int uio_dev_add_attributes(struct
> err_remove_maps:
> for (mi--; mi>=0; mi--) {
> mem = &idev->info->mem[mi];
> + list_del(&mem->list_node);
> kobject_unregister(&mem->kobj);
> }
> kset_unregister(&idev->map_attr_kset); /* Needed ? */
> @@ -194,10 +202,11 @@ static void uio_dev_del_attributes(struc
> {
> int mi;
> struct uio_mem *mem;
> - for (mi = 0; mi < MAX_UIO_MAPS; mi++) {
> + for (mi = 0; mi < ARRAY_SIZE(idev->info->mem); mi++) {
> mem = &idev->info->mem[mi];
> if (mem->size == 0)
> break;
> + list_del(&mem->list_node);
> kobject_unregister(&mem->kobj);
> }
> kset_unregister(&idev->map_attr_kset);
> @@ -386,18 +395,20 @@ static ssize_t uio_read(struct file *fil
> return retval;
> }
>
> -static int uio_find_mem_index(struct vm_area_struct *vma)
> +static struct uio_mem *uio_find_mem_index(struct vm_area_struct *vma)
> {
> - int mi;
> struct uio_device *idev = vma->vm_private_data;
> + struct list_head *mem_list;
> +
> + list_for_each(mem_list, &idev->mem_list) {
>
> - for (mi = 0; mi < MAX_UIO_MAPS; mi++) {
> - if (idev->info->mem[mi].size == 0)
> - return -1;
> - if (vma->vm_pgoff == mi)
> - return mi;
> + struct uio_mem *mem =
> + list_entry(mem_list, struct uio_mem, list_node);
> +
> + if (vma->vm_pgoff == mem->map_id)
> + return mem;
> }
> - return -1;
> + return NULL;
> }
>
> static void uio_vma_open(struct vm_area_struct *vma)
> @@ -415,17 +426,16 @@ static void uio_vma_close(struct vm_area
> static struct page *uio_vma_nopage(struct vm_area_struct *vma,
> unsigned long address, int *type)
> {
> - struct uio_device *idev = vma->vm_private_data;
> struct page* page = NOPAGE_SIGBUS;
>
> - int mi = uio_find_mem_index(vma);
> - if (mi < 0)
> + struct uio_mem *mem = uio_find_mem_index(vma);
> + if (mem == NULL)
> return page;
>
> - if (idev->info->mem[mi].memtype == UIO_MEM_LOGICAL)
> - page = virt_to_page(idev->info->mem[mi].addr);
> + if (mem->memtype == UIO_MEM_LOGICAL)
> + page = virt_to_page(mem->addr);
> else
> - page = vmalloc_to_page((void*)idev->info->mem[mi].addr);
> + page = vmalloc_to_page((void*)mem->addr);
> get_page(page);
> if (type)
> *type = VM_FAULT_MINOR;
> @@ -440,16 +450,15 @@ static struct vm_operations_struct uio_v
>
> static int uio_mmap_physical(struct vm_area_struct *vma)
> {
> - struct uio_device *idev = vma->vm_private_data;
> - int mi = uio_find_mem_index(vma);
> - if (mi < 0)
> + struct uio_mem *mem = uio_find_mem_index(vma);
> + if (mem == NULL)
> return -EINVAL;
>
> vma->vm_flags |= VM_IO | VM_RESERVED;
>
> return remap_pfn_range(vma,
> vma->vm_start,
> - idev->info->mem[mi].addr >> PAGE_SHIFT,
> + mem->addr >> PAGE_SHIFT,
> vma->vm_end - vma->vm_start,
> vma->vm_page_prot);
> }
> @@ -466,7 +475,7 @@ static int uio_mmap(struct file *filep,
> {
> struct uio_listener *listener = filep->private_data;
> struct uio_device *idev = listener->dev;
> - int mi;
> + struct uio_mem *mem;
> unsigned long requested_pages, actual_pages;
> int ret = 0;
>
> @@ -475,12 +484,12 @@ static int uio_mmap(struct file *filep,
>
> vma->vm_private_data = idev;
>
> - mi = uio_find_mem_index(vma);
> - if (mi < 0)
> + mem = uio_find_mem_index(vma);
> + if (mem == NULL)
> return -EINVAL;
>
> requested_pages = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
> - actual_pages = (idev->info->mem[mi].size + PAGE_SIZE -1) >> PAGE_SHIFT;
> + actual_pages = (mem->size + PAGE_SIZE -1) >> PAGE_SHIFT;
> if (requested_pages > actual_pages)
> return -EINVAL;
>
> @@ -492,7 +501,7 @@ static int uio_mmap(struct file *filep,
> return ret;
> }
>
> - switch (idev->info->mem[mi].memtype) {
> + switch (mem->memtype) {
> case UIO_MEM_PHYS:
> return uio_mmap_physical(vma);
> case UIO_MEM_LOGICAL:
> @@ -613,6 +622,7 @@ int __uio_register_device(struct module
> idev->info = info;
> init_waitqueue_head(&idev->wait);
> atomic_set(&idev->event, 0);
> + INIT_LIST_HEAD(&idev->mem_list);
>
> ret = uio_get_minor(idev);
> if (ret)
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/1] Userspace I/O (UIO): Add support for userspace DMA
2009-12-15 21:00 ` Hans J. Koch
@ 2009-12-15 21:47 ` Earl Chew
2009-12-15 22:28 ` Hans J. Koch
0 siblings, 1 reply; 20+ messages in thread
From: Earl Chew @ 2009-12-15 21:47 UTC (permalink / raw)
To: Hans J. Koch
Cc: Peter Zijlstra, linux-kernel, gregkh, hugh, linux-mm, Thomas Gleixner
Hans J. Koch wrote:
> Sorry, I think I wasn't clear enough: The current interface for static
> mappings shouldn't be changed. Dynamically added mappings need a new
> interface.
Thanks for the quick reply.
Are you ok with changes to the (internal) struct uio_device ?
This content of this structure is only known to uio.c and only its
name is exposed through the client visible uio_driver.h.
Earl
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/1] Userspace I/O (UIO): Add support for userspace DMA
2009-12-15 21:47 ` Earl Chew
@ 2009-12-15 22:28 ` Hans J. Koch
2009-12-16 0:20 ` Earl Chew
0 siblings, 1 reply; 20+ messages in thread
From: Hans J. Koch @ 2009-12-15 22:28 UTC (permalink / raw)
To: Earl Chew
Cc: Hans J. Koch, Peter Zijlstra, linux-kernel, gregkh, linux-mm,
Thomas Gleixner
On Tue, Dec 15, 2009 at 01:47:04PM -0800, Earl Chew wrote:
> Hans J. Koch wrote:
> > Sorry, I think I wasn't clear enough: The current interface for static
> > mappings shouldn't be changed. Dynamically added mappings need a new
> > interface.
>
> Thanks for the quick reply.
>
> Are you ok with changes to the (internal) struct uio_device ?
Hey, we live in a free world :)
Anything can be changed as long as it's a technically sensible solution and
doesn't break existing interfaces to userspace.
The DMA-for-UIO thing is something that shouldn't be taken lightly. If we
define an interface for that, it should cover all possible applications.
There are not only devices that need to allocate new DMA buffers at runtime,
but also devices which could very well live with one or two statically
allocated DMA buffers. We need to cover all these cases.
One example: An A/D converter has an on-chip 32k buffer. It causes an
interrupt as soon as the buffer is filled up to a certain high-water mark.
Such cases would easily fit into the current UIO system. The UIO core could
simply DMA the data to one of the mappings. A new flag for that mapping and
a few other changes are all it takes. After the DMA transfer is complete, the
interrupt is passed on to userspace, which would find the buffer already
filled with the desired data. Just a thought, unfortunately I haven't got
such hardware to try it.
When it comes to dynamically allocated DMA buffers, it might well be possible
to add a new directory in sysfs besides the "mem" directory, e.g. something
like /sys/class/uio/uioN/dma-mem/. This would save us the trouble of creating
a new device. Maybe the example above would better fit in here, too. Who knows.
These are only some thoughts, I haven't got any DMA capable hardware to deal
with ATM.
You certainly notice that there are important design decisions to make.
Remember that once a kernel interface to userspace exists, it is etched in
stone forever.
Thanks,
Hans
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/1] Userspace I/O (UIO): Add support for userspace DMA
2009-12-15 22:28 ` Hans J. Koch
@ 2009-12-16 0:20 ` Earl Chew
2009-12-16 1:23 ` Hans J. Koch
0 siblings, 1 reply; 20+ messages in thread
From: Earl Chew @ 2009-12-16 0:20 UTC (permalink / raw)
To: Hans J. Koch
Cc: Peter Zijlstra, linux-kernel, gregkh, linux-mm, Thomas Gleixner
Hans J. Koch wrote:
> One example: An A/D converter has an on-chip 32k buffer. It causes an
> interrupt as soon as the buffer is filled up to a certain high-water mark.
> Such cases would easily fit into the current UIO system. The UIO core could
> simply DMA the data to one of the mappings. A new flag for that mapping and
> a few other changes are all it takes. After the DMA transfer is complete, the
> interrupt is passed on to userspace, which would find the buffer already
> filled with the desired data. Just a thought, unfortunately I haven't got
> such hardware to try it.
Hans,
Is this case already covered by the pre-existing UIO_MEM_LOGICAL
option ?
I'm thinking that since the memory is statically defined, it can be
described using one of the existing struct uio_mem mem[] slots in
struct uio_info and marked as UIO_MEM_LOGICAL.
The userspace program can map that into its process space using the
existing mmap() interface.
What am I missing?
> When it comes to dynamically allocated DMA buffers, it might well be possible
> to add a new directory in sysfs besides the "mem" directory, e.g. something
> like /sys/class/uio/uioN/dma-mem/. This would save us the trouble of creating
> a new device. Maybe the example above would better fit in here, too. Who knows.
I looked at the 2.6.32 source at
http://lxr.linux.no/#linux+v2.6.32/drivers/uio/uio.c
and didn't see any reference to /sys/class/uio/uioN/mem . Perhaps
you're referring to something new.
In any case, I think you're describing adding
/sys/class/uio/uioN/dma-mem
as a means to control /dev/uioN . Presumably writing to
/sys/class/uio/uioN/dma-mem would create additional dynamic
DMA buffers.
I can't yet see a way to make this request-response. When requesting
a dynamic buffer I need to indicate the size that I want, and in
return I need to obtain a handle to the buffer (either its mapping
number, address, etc). Once I have that, I can query other
interesting information (eg its bus address).
Earl
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/1] Userspace I/O (UIO): Add support for userspace DMA
2009-12-16 0:20 ` Earl Chew
@ 2009-12-16 1:23 ` Hans J. Koch
2009-12-16 1:45 ` Earl Chew
0 siblings, 1 reply; 20+ messages in thread
From: Hans J. Koch @ 2009-12-16 1:23 UTC (permalink / raw)
To: Earl Chew
Cc: Hans J. Koch, Peter Zijlstra, linux-kernel, gregkh, linux-mm,
Thomas Gleixner
On Tue, Dec 15, 2009 at 04:20:56PM -0800, Earl Chew wrote:
> Hans J. Koch wrote:
> > One example: An A/D converter has an on-chip 32k buffer. It causes an
> > interrupt as soon as the buffer is filled up to a certain high-water mark.
> > Such cases would easily fit into the current UIO system. The UIO core could
> > simply DMA the data to one of the mappings. A new flag for that mapping and
> > a few other changes are all it takes. After the DMA transfer is complete, the
> > interrupt is passed on to userspace, which would find the buffer already
> > filled with the desired data. Just a thought, unfortunately I haven't got
> > such hardware to try it.
>
> Hans,
>
> Is this case already covered by the pre-existing UIO_MEM_LOGICAL
> option ?
>
> I'm thinking that since the memory is statically defined, it can be
> described using one of the existing struct uio_mem mem[] slots in
> struct uio_info and marked as UIO_MEM_LOGICAL.
>
> The userspace program can map that into its process space using the
> existing mmap() interface.
>
> What am I missing?
Nothing. The UIO core can map all kinds of memory. I thought about a generic
DMA routine that does the transfer if a flag like UIO_DMA_FROM_DEVICE_PRE_USER
is set.
>
> > When it comes to dynamically allocated DMA buffers, it might well be possible
> > to add a new directory in sysfs besides the "mem" directory, e.g. something
> > like /sys/class/uio/uioN/dma-mem/. This would save us the trouble of creating
> > a new device. Maybe the example above would better fit in here, too. Who knows.
>
> I looked at the 2.6.32 source at
>
> http://lxr.linux.no/#linux+v2.6.32/drivers/uio/uio.c
>
> and didn't see any reference to /sys/class/uio/uioN/mem . Perhaps
> you're referring to something new.
Just look at the sysfs files you get when creating a UIO device, then look at
uio.c to see how it is done.
>
> In any case, I think you're describing adding
>
> /sys/class/uio/uioN/dma-mem
>
> as a means to control /dev/uioN . Presumably writing to
> /sys/class/uio/uioN/dma-mem would create additional dynamic
> DMA buffers.
No, dma-mem would be a directory containing some more attributes. Maybe one
called "create" that allocates a new buffer.
>
> I can't yet see a way to make this request-response. When requesting
> a dynamic buffer I need to indicate the size that I want, and in
> return I need to obtain a handle to the buffer (either its mapping
> number, address, etc). Once I have that, I can query other
> interesting information (eg its bus address).
Writing the size to that supposed "create" attribute could allocate the
buffer and and create more attributes that contain the information you need.
Thanks,
Hans
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/1] Userspace I/O (UIO): Add support for userspace DMA
2009-12-16 1:23 ` Hans J. Koch
@ 2009-12-16 1:45 ` Earl Chew
0 siblings, 0 replies; 20+ messages in thread
From: Earl Chew @ 2009-12-16 1:45 UTC (permalink / raw)
To: Hans J. Koch
Cc: Peter Zijlstra, linux-kernel, gregkh, linux-mm, Thomas Gleixner
Hans J. Koch wrote:
> No, dma-mem would be a directory containing some more attributes. Maybe one
> called "create" that allocates a new buffer.
>
[ .. snip ..]
> Writing the size to that supposed "create" attribute could allocate the
> buffer and and create more attributes that contain the information you need.
Hmm ... I can't see how to make this into a transaction.
Suppose two threads write to /sys/.../create simultaneously (or
very close together) and further suppose that each call succeeds.
It's not clear to me how each can figure out where to find the
outcome of its operation because write() doesn't return anything
other than the number of octets written.
Writing "id, size" might work, but sorting out a good enough id
might be a little clunky. A process id wouldn't be good enough (with
different threads), and a thread id might get recycled.
Any other ideas ?
Earl
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2009-12-16 1:46 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <43FC624C55D8C746A914570B66D642610367F29B@cos-us-mb03.cos.agilent.com>
2008-12-04 8:39 ` [PATCH 1/1] Userspace I/O (UIO): Add support for userspace DMA Peter Zijlstra
2008-12-04 10:27 ` Hugh Dickins
2008-12-23 21:32 ` Max Krasnyansky
2008-12-04 18:08 ` Hans J. Koch
2008-12-05 7:10 ` Peter Zijlstra
2008-12-05 9:44 ` Hans J. Koch
2008-12-06 0:32 ` Edward Estabrook
2008-12-12 17:25 ` Peter Zijlstra
2008-12-13 0:29 ` Hans J. Koch
2009-12-12 0:02 ` Earl Chew
2009-12-14 19:23 ` Hans J. Koch
2009-12-15 13:34 ` Earl Chew
2009-12-15 17:47 ` Earl Chew
2009-12-15 21:33 ` Hans J. Koch
2009-12-15 21:00 ` Hans J. Koch
2009-12-15 21:47 ` Earl Chew
2009-12-15 22:28 ` Hans J. Koch
2009-12-16 0:20 ` Earl Chew
2009-12-16 1:23 ` Hans J. Koch
2009-12-16 1:45 ` Earl Chew
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox