Re: [Qemu-devel] [PATCH v2 00/38] Delay destruction of memory regions to instance_finalize

[Paolo Bonzini]

Il 17/09/2013 19:26, Michael S. Tsirkin ha scritto:
> On Tue, Sep 17, 2013 at 07:16:15PM +0200, Paolo Bonzini wrote:
>> Il 17/09/2013 19:07, Michael S. Tsirkin ha scritto:
>>> After memory_region_del_subregion returns,
>>> it's a promise that there will not be accesses
>>> to the region.
>>
>> It's racy anyway.  You can have memory_region_del_subregion happen one
>> clock cycle after the other (physical) CPU has done checked that there
>> will not be accesses to the region.
> 
> Fix it then. Stick synchronize_rcu() in memory_region_del_subregion.

Easier said than done since you shouldn't do synchronize_rcu() with the
BQL taken.

> Are you trying to convince me there's no way to have
> synchronous APIs in presence of RCU?

No, I'm not.  But in the presence of coarse locks it's harder to use
synchronous APIs.  Sticking to past QEMU experiences, the AIO
synchronous cancellation API is a nightmare because the monitor cannot
run while I/O is cancelled.

>> A real bus has a "big PCI lock" (there can be only one transaction at a
>> time), which is exactly what we want to get rid of.
> 
> Not really, in a bridged setup transactions on the
> secondary bus are handled in parallel with transactions
> on the primary bus.

That's not what you're suggesting though.  A device removal on the
secondary bus, triggered by configuration space access on the primary
bus, wouldn't complete the configuration space access on the primary bus
until the removal on the secondary bus is complete.  So you still have a
big lock around transactions on each bus.  Would a PCI bridge behave any
differently?

It's still too coarse for emulation purposes.

> Also, there are split transactions completed
> asynchronously: guests that care about ordering
> do flushes e.g. using memory reads.

Can you do split burst transactions?  If not, the granularity is still
measured in bytes, not pages or more.

Paolo

> The logic is in the device/memory region really.
> 
>>> So I'm not even sure we really need to move destroy to finalize anymore ...
>>
>> We definitely need to move it.  Even if we added a flag that we set in
>> memory_region_del_subregion, we need to check it later when AIO completes.
>>
>> Paolo
> 
> Flag where? region can become subregion of another
> region entirely.
> 
> The solution for RCU is to flush on changes, not add
> more flags and locks.
>