Re: [dpdk-dev] [PATCH] vfio: add hotplug support

["Burakov, Anatoly" <anatoly.burakov@intel.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