lkml.org 
[lkml]   [2015]   [Oct]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v3 1/3] uio: add ioctl support
From
Date



On 10/05/15 06:03, Greg KH wrote:
> On Sun, Oct 04, 2015 at 11:43:16PM +0300, Vlad Zolotarov wrote:
>> Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com>
>> ---
>> drivers/uio/uio.c | 15 +++++++++++++++
>> include/linux/uio_driver.h | 3 +++
>> 2 files changed, 18 insertions(+)
> You add an ioctl yet fail to justify _why_ you need/want that ioctl, and
> you don't document it at all? Come on, you know better than that, no
> one can take a patch that has no changelog comments at all like this :(

My bad. U are absolutely right here - it was late and I was tired that I
missed that to someone it may not be so "crystal clear" like it is to
me... :)
Again, my bad - let me clarify it here and if we agree I'll respin the
series with all relevant updates including the changelog.

>
> Also, I _REALLY_ don't want to add any ioctls to the UIO interface, so
> you had better have a really compelling argument as to why this is the
> _ONLY_ way you can solve this unknown problem by using such a horrid
> thing...

Pls., note that this doesn't _ADD_ any ioctls directly to UIO driver,
but only lets the underlying PCI drivers to have them. UIO in this case
is only a proxy.

The main idea of this series is, as mentioned in PATCH0, to add the MSI
and MSI-X support for uio_pci_generic driver.
While with MSI the things are quite simple and we may just ride the
existing infrastructure, with the MSI-X the things get a bit more
complicated since we may have more than one interrupt vector. Therefore
we have to decide which interface we want to give to the user.

One option could be to make all existing interrupts trigger the same
objects in UIO as the current single interrupt does, however this would
create an awkward, quite not-flexible semantics. For instance a regular
(kernel) driver has a separate state machine for each interrupt line,
which sometimes runs on a separate CPU, etc. This way we get to the
second option - allow indication for each separate interrupt vector. And
for obvious reasons (mentioned above) we (Stephen has sent a similar
series on a dpdk-dev list) chose a second approach.

In order not to invent the wheel we mimicked the VFIO approach, which
allows to bind the pre-allocated eventfd descriptor to the specific
interrupt vector using the ioctl().

The interface is simple. The UIO_PCI_GENERIC_IRQ_SET ioctl() data is:

struct uio_pci_generic_irq_set {
int vec; /* index of the IRQ to connect to starting from 0 */
int fd;
};


where "vec" is an index of the IRQ starting from 0 and "fd" is an
eventfd file descriptor a user wants to poll() for in order to get the
interrupt indications. If "fd" is less than 0, ioctl() will unbind the
interrupt from the previously bound eventfd descriptor.

This way a user may poll() for any IRQ it wants separately, or epoll()
for any subset of them, or do whatever he/she wants to do.

That's why we needed the ioctl(). I admit that it may not be the _ONLY_
way to achieve the goal described above but again we took VFIO approach
as a template for a solution and just followed it. If u think there is
more elegant/robust/better way to do so, pls., share. :)

thanks,
vlad


>
> thanks,
>
> greg k-h


On 10/05/15 06:03, Greg KH wrote:
> On Sun, Oct 04, 2015 at 11:43:16PM +0300, Vlad Zolotarov wrote:
>> Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com>
>> ---
>> drivers/uio/uio.c | 15 +++++++++++++++
>> include/linux/uio_driver.h | 3 +++
>> 2 files changed, 18 insertions(+)
> You add an ioctl yet fail to justify _why_ you need/want that ioctl, and
> you don't document it at all? Come on, you know better than that, no
> one can take a patch that has no changelog comments at all like this :(
>
> Also, I _REALLY_ don't want to add any ioctls to the UIO interface, so
> you had better have a really compelling argument as to why this is the
> _ONLY_ way you can solve this unknown problem by using such a horrid
> thing...
>
> thanks,
>
> greg k-h



\
 
 \ /
  Last update: 2015-10-05 10:01    [W:0.129 / U:1.008 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site