DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Burakov, Anatoly" <anatoly.burakov@intel.com>
To: Alejandro Lucero <alejandro.lucero@netronome.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH] vfio: add hotplug support
Date: Fri, 24 Mar 2017 16:56:41 +0000	[thread overview]
Message-ID: <C6ECDF3AB251BE4894318F4E45123697821FA331@IRSMSX109.ger.corp.intel.com> (raw)
In-Reply-To: <CAD+H993_LKZvD7saTA0-uTWdQbCQOLq1+azv4fgBaVuDsaAVUA@mail.gmail.com>

Hi Alejandro,

>Those lines are not error messages. Are you suggesting not splitting lines in the commit comment?

Apologies, that (and other instances) was supposed to mean "snipped" :)
 
> What do you refer to? The TODO line?

No, I was referring to "map BARS" comment in an unmapping function. Should probably say "unmap"?

> When mapping, a BAR with the msix table needs to be handled specifically, doing two mmap calls leaving out the msix table addresses. But although there are two mmap calls, there is just one mmap region (from the process point of view), because the address to use for the second call is just the end of the first mmap. Note this is about the virtual addresses to be used by the app and not the physical addresses really used at the end. So inside the maps array, just one mmap region is saved. Doing the unmap is just a matter of using that maps array and use the maps[x].size accordingly.

Yep, you're right.

> Uhmm, I think a secondary process can invoke rte_eth_dev_detach, so not sure what event are you referring to.
> I'm afraid I have not done any test regarding secondary processes calling that function, but being honest, documentations is clear about how unsafe is to use hotplug API from multiple threads, and I would add same uncertainty when used from secondary processes.

I think hotplug never worked for secondary processes. So really, I'm inclined to leave this out entirely. That would at least make it consistent in its brokenness :) Plus, this doesn't work the other way around (i.e. if a primary process unplugs a NIC, secondary process isn't notified), so we might as well just not bother supporting secondary processes at all. Opinions?
 
> Not sure I understand your concern, but I will take your comment as a thumb up ;-)

Yes, that's a thumbs up :)

Thanks,
Anatoly


      reply	other threads:[~2017-03-24 16:56 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-16 10:51 Alejandro Lucero
2017-03-17 11:07 ` Eelco Chaudron
2017-03-22 13:49 ` Burakov, Anatoly
2017-03-23 12:08   ` Burakov, Anatoly
2017-03-24 16:39     ` Alejandro Lucero
2017-03-24 16:38   ` Alejandro Lucero
2017-03-24 16:56     ` Burakov, Anatoly [this message]

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=C6ECDF3AB251BE4894318F4E45123697821FA331@IRSMSX109.ger.corp.intel.com \
    --to=anatoly.burakov@intel.com \
    --cc=alejandro.lucero@netronome.com \
    --cc=dev@dpdk.org \
    /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).