linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Avi Kivity <avi@scylladb.com>
To: Greg KH <gregkh@linuxfoundation.org>,
	Vlad Zolotarov <vladz@cloudius-systems.com>
Cc: linux-kernel@vger.kernel.org, mst@redhat.com, hjk@hansjkoch.de,
	corbet@lwn.net, bruce.richardson@intel.com,
	avi@cloudius-systems.com, gleb@cloudius-systems.com,
	stephen@networkplumber.org, alexander.duyck@gmail.com
Subject: Re: [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support
Date: Mon, 5 Oct 2015 14:09:32 +0300	[thread overview]
Message-ID: <56125A6C.7030107@scylladb.com> (raw)
In-Reply-To: <20151005105715.GA23459@kroah.com>

On 10/05/2015 01:57 PM, Greg KH wrote:
> On Mon, Oct 05, 2015 at 01:48:39PM +0300, Vlad Zolotarov wrote:
>>
>> On 10/05/15 10:56, Greg KH wrote:
>>> On Mon, Oct 05, 2015 at 10:41:39AM +0300, Vlad Zolotarov wrote:
>>>>>> +struct msix_info {
>>>>>> +	int num_irqs;
>>>>>> +	struct msix_entry *table;
>>>>>> +	struct uio_msix_irq_ctx {
>>>>>> +		struct eventfd_ctx *trigger;	/* MSI-x vector to eventfd */
>>>>> Why are you using eventfd for msi vectors?  What's the reason for
>>>>> needing this?
>>>> A small correction - for MSI-X vectors. There may be only one MSI vector per
>>>> PCI function and if it's used it would use the same interface as a legacy
>>>> INT#x interrupt uses at the moment.
>>>> So, for MSI-X case the reason is that there may be (in most cases there will
>>>> be) more than one interrupt vector. Thus, as I've explained in a PATCH1
>>>> thread we need a way to indicated each of them separately. eventfd seems
>>>> like a good way of doing so. If u have better ideas, pls., share.
>>> You need to document what you are doing here, I don't see any
>>> explaination for using eventfd at all.
>>>
>>> And no, I don't know of any other solution as I don't know what you are
>>> trying to do here (hint, the changelog didn't document it...)
>>>
>>>>> You haven't documented how this api works at all, you are going to have
>>>>> to a lot more work to justify this, as this greatly increases the
>>>>> complexity of the user/kernel api in unknown ways.
>>>> I actually do documented it a bit. Pls., check PATCH3 out.
>>> That provided no information at all about how to use the api.
>>>
>>> If it did, you would see that your api is broken for 32/64bit kernels
>>> and will fall over into nasty pieces the first time you try to use it
>>> there, which means it hasn't been tested at all :(
>> It has been tested of course ;)
>> I tested it only in 64 bit environment however where both kernel and user
>> space applications were compiled on the same machine with the same compiler
>> and it could be that "int" had the same number of bytes both in kernel and
>> in user space application. Therefore it worked perfectly - I patched DPDK to
>> use the new uio_pci_generic MSI-X API to test this and I have verified that
>> all 3 interrupt modes work: MSI-X with SR-IOV VF device in Amazon EC2 guest
>> and INT#x and MSI with a PF device on bare metal server.
>>
>> However I agree using uint32_t for "vec" and "fd" would be much more
>> correct.
> I don't think file descriptors are __u32 on a 64bit arch, are they?
>
> And NEVER use the _t types in kernel code, the namespaces is all wrong
> and it is not applicable for us, sorry.

Wasn't the real reason that they aren't defined (or reserved) by C89, 
and therefore could clash with a user identifier, rather than some 
inherent wrongness?


  reply	other threads:[~2015-10-05 11:09 UTC|newest]

Thread overview: 96+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-04 20:43 [PATCH v3 0/3] uio: add MSI/MSI-X support to uio_pci_generic driver Vlad Zolotarov
2015-10-04 20:43 ` [PATCH v3 1/3] uio: add ioctl support Vlad Zolotarov
2015-10-05  3:03   ` Greg KH
2015-10-05  7:33     ` Vlad Zolotarov
2015-10-05  8:01       ` Greg KH
2015-10-05 10:36         ` Vlad Zolotarov
2015-10-05 20:02           ` Michael S. Tsirkin
     [not found]             ` <CAOYyTHZ2=UCYxuJKvd5S6qxp=84DBq5bMadg5wL0rFLZBh2-8Q@mail.gmail.com>
2015-10-05 22:29               ` Michael S. Tsirkin
2015-10-06  8:33                 ` Vlad Zolotarov
2015-10-06 14:19                   ` Michael S. Tsirkin
2015-10-06 14:30                     ` Gleb Natapov
2015-10-06 15:19                       ` Michael S. Tsirkin
2015-10-06 15:31                         ` Vlad Zolotarov
2015-10-06 15:57                         ` Gleb Natapov
2015-10-04 20:43 ` [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support Vlad Zolotarov
2015-10-05  3:11   ` Greg KH
2015-10-05  7:41     ` Vlad Zolotarov
2015-10-05  7:56       ` Greg KH
2015-10-05 10:48         ` Vlad Zolotarov
2015-10-05 10:57           ` Greg KH
2015-10-05 11:09             ` Avi Kivity [this message]
2015-10-05 13:08               ` Greg KH
2015-10-05 11:41             ` Vlad Zolotarov
2015-10-05 11:47               ` Avi Kivity
2015-10-05 11:53                 ` Vlad Zolotarov
2015-10-05  8:28     ` Avi Kivity
2015-10-05  9:49       ` Greg KH
2015-10-05 10:20         ` Avi Kivity
2015-10-06 14:38           ` Michael S. Tsirkin
2015-10-06 14:43             ` Vlad Zolotarov
2015-10-06 14:56               ` Michael S. Tsirkin
2015-10-06 15:23                 ` Avi Kivity
2015-10-06 18:51                   ` Alex Williamson
2015-10-06 21:32                     ` Stephen Hemminger
2015-10-06 21:41                       ` Alex Williamson
     [not found]                         ` <CAOaVG152OrQz-Bbnpr0VeE+vLH7nMGsG6A3sD7eTQHormNGVUg@mail.gmail.com>
2015-10-07  7:57                           ` Vlad Zolotarov
     [not found]                           ` <5614C160.6000203@scylladb.com>
2015-10-07  8:00                             ` Vlad Zolotarov
2015-10-07  8:01                               ` Vlad Zolotarov
2015-10-07  6:52                     ` Avi Kivity
2015-10-07 16:31                       ` Alex Williamson
2015-10-07 16:39                         ` Avi Kivity
2015-10-07 21:05                           ` Michael S. Tsirkin
2015-10-08  4:19                             ` Gleb Natapov
2015-10-08  7:41                               ` Michael S. Tsirkin
2015-10-08  7:59                                 ` Gleb Natapov
2015-10-08  9:38                                   ` Michael S. Tsirkin
2015-10-08  9:45                                     ` Gleb Natapov
2015-10-08 12:15                                       ` Michael S. Tsirkin
2015-10-08  5:33                             ` Avi Kivity
2015-10-08  7:32                               ` Michael S. Tsirkin
2015-10-08  8:46                                 ` Avi Kivity
2015-10-08  9:16                                   ` Michael S. Tsirkin
2015-10-08  9:44                                     ` Avi Kivity
2015-10-08 12:06                                       ` Michael S. Tsirkin
2015-10-08 12:27                                         ` Gleb Natapov
2015-10-08 13:20                                           ` Michael S. Tsirkin
2015-10-08 13:28                                             ` Gleb Natapov
2015-10-08 16:43                                               ` Michael S. Tsirkin
2015-10-08 17:01                                                 ` Gleb Natapov
2015-10-08 17:39                                                   ` Michael S. Tsirkin
2015-10-08 17:53                                                     ` Gleb Natapov
2015-10-08 18:38                                                     ` Greg KH
2015-10-08  8:32                               ` Michael S. Tsirkin
2015-10-08  8:52                                 ` Gleb Natapov
2015-10-08  9:19                                 ` Avi Kivity
2015-10-08 10:26                                   ` Michael S. Tsirkin
2015-10-08 13:20                                     ` Avi Kivity
2015-10-08 14:17                                       ` Michael S. Tsirkin
2015-10-08 15:31                                       ` Alex Williamson
2015-10-07 20:05                         ` Michael S. Tsirkin
2015-10-07  7:55                     ` Vlad Zolotarov
2015-10-08  8:48                       ` Michael S. Tsirkin
2015-10-06 15:28                 ` Vlad Zolotarov
2015-10-06 14:46       ` Michael S. Tsirkin
2015-10-06 15:27         ` Avi Kivity
2015-10-05  8:41   ` Stephen Hemminger
2015-10-05  9:08     ` Vlad Zolotarov
2015-10-05 10:06       ` Vlad Zolotarov
2015-10-05 20:09         ` Michael S. Tsirkin
2015-10-05  9:11     ` Vlad Zolotarov
2015-10-05 19:16   ` Michael S. Tsirkin
2015-10-04 20:43 ` [PATCH v3 3/3] Documentation: update uio-howto Vlad Zolotarov
2015-10-04 20:45 ` [PATCH v3 0/3] uio: add MSI/MSI-X support to uio_pci_generic driver Vlad Zolotarov
2015-10-05 19:50 ` Michael S. Tsirkin
2015-10-06  8:37   ` Vlad Zolotarov
2015-10-06 14:30     ` Michael S. Tsirkin
2015-10-06 14:40       ` Vlad Zolotarov
2015-10-06 15:13         ` Michael S. Tsirkin
2015-10-06 16:35           ` Vlad Zolotarov
2015-10-06 15:11       ` Avi Kivity
2015-10-06 15:15         ` Michael S. Tsirkin
2015-10-06 16:00           ` Gleb Natapov
2015-10-06 16:09           ` Avi Kivity
2015-10-07 10:25             ` Michael S. Tsirkin
2015-10-07 10:28               ` Avi Kivity
  -- strict thread matches above, loose matches on Subject: below --
2015-10-04 20:39 Vlad Zolotarov
2015-10-04 20:39 ` [PATCH v3 2/3] uio_pci_generic: add MSI/MSI-X support Vlad Zolotarov

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=56125A6C.7030107@scylladb.com \
    --to=avi@scylladb.com \
    --cc=alexander.duyck@gmail.com \
    --cc=avi@cloudius-systems.com \
    --cc=bruce.richardson@intel.com \
    --cc=corbet@lwn.net \
    --cc=gleb@cloudius-systems.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hjk@hansjkoch.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mst@redhat.com \
    --cc=stephen@networkplumber.org \
    --cc=vladz@cloudius-systems.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).