[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 10/10] qdev: fix create in place obj's life cycl
From: |
Avi Kivity |
Subject: |
Re: [Qemu-devel] [PATCH 10/10] qdev: fix create in place obj's life cycle problem |
Date: |
Sat, 01 Sep 2012 01:46:43 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:14.0) Gecko/20120717 Thunderbird/14.0 |
On 08/30/2012 12:08 AM, Jan Kiszka wrote:
> >>>
> >>> We are dispatching according to the memory region (parameters, op
> >>> handlers, opaques). If we end up in device object is not decided at this
> >>> level. A memory region describes a dispatchable area - not to confuse
> >>> with a device that may only partially be able to receive such requests.
> >>
> > But I think the meaning of the memory region is for dispatching. If no
> > dispatching associated with mr, why need it exist in the system?
>
> Where did I say that memory regions should no longer be used for
> dispatching? The point is to keep the clean layer separation between
> memory regions and device objects instead of merging them together.
Believe me, that's exactly how I feel. I guess no author of a module
wants to see it swallowed by a larger framework and see its design
completely changed. Modules should be decoupled as much as possible.
But I don't see a better way yet.
>
> Given
>
> Object
> ^ ^
> | |
> Region 1 Region 2
>
> you protect the object during dispatch, implicitly (and that is bad)
> requiring that no region must change in that period.
We only protect the object against removal. After object_ref() has run,
the region may still be disabled.
> I say what rather
> needs protection are the regions so that Region 2 can pass away and
> maybe reappear independent of Region 1.
Region 2 can go away until the device-supplied dispatch code takes the
device lock. If the device chooses to use finer-grained locking, it can
allow region 2 (or even region 1) to change during dispatch. The only
requirement is that region 1 is not destroyed.
> And: I won't need to know the
> type of that object the regions are referring to in this model. That's
> the difference.
Sorry, I lost the reference. What model? Are you referring to my
broken MemoryRegionImpl split?
> > And
> > could you elaborate that who will be the ref holder of mr?
>
> The memory subsystem while running a memory region handler. If that will
> be a reference counter or a per-region lock like Avi suggested, we still
> need to find out.
>
If we make the refcount/lock internal to the region, we must remove the
opaque, since the region won't protect it.
Replacing the opaque with container_of(mr) doesn't help, since we can't
refcount mr, only mr->impl.
We could externalize the refcounting and push it into device code. This
means:
memory_region_init_io(&s->mem, dev)
...
object_ref(dev)
memory_region_add_subregion(..., &dev->mr)
...
memory_region_del_subregion(..., &dev->mr) // implied flush
object_unref(dev)
er, this must be wrong, since it's so simple
--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.
- Re: [Qemu-devel] [PATCH 10/10] qdev: fix create in place obj's life cycle problem, Avi Kivity, 2012/09/01
- Re: [Qemu-devel] [PATCH 10/10] qdev: fix create in place obj's life cycle problem,
Avi Kivity <=
- Re: [Qemu-devel] [PATCH 10/10] qdev: fix create in place obj's life cycle problem, liu ping fan, 2012/09/03
- Re: [Qemu-devel] [PATCH 10/10] qdev: fix create in place obj's life cycle problem, liu ping fan, 2012/09/03
- Re: [Qemu-devel] [PATCH 10/10] qdev: fix create in place obj's life cycle problem, liu ping fan, 2012/09/05
- Re: [Qemu-devel] [PATCH 10/10] qdev: fix create in place obj's life cycle problem, Avi Kivity, 2012/09/05
- Re: [Qemu-devel] [PATCH 10/10] qdev: fix create in place obj's life cycle problem, Jan Kiszka, 2012/09/05
- Re: [Qemu-devel] [PATCH 10/10] qdev: fix create in place obj's life cycle problem, Avi Kivity, 2012/09/05