[Qemu-devel] qxl: pthread_yield on QXL_IO_NOTIFY_OOM

[Qemu-devel] qxl: pthread_yield on QXL_IO_NOTIFY_OOM

[Jan Kiszka]

Hi Gerd,

can you (or anyone familiar with those bits) comment on pthread_yield()
in ioport_write() of hw/qxl.c? Which threads are supposed to run this
way? Can't this relation be expressed explicitly? If not, can we use a
sleep here (how long?)?

The background is that yielding easily breaks into pieces when leaving
the SCHED_OTHER space (what we are working on).

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] qxl: pthread_yield on QXL_IO_NOTIFY_OOM

[Gerd Hoffmann]

On 09/20/11 16:27, Jan Kiszka wrote:
> Hi Gerd,
>
> can you (or anyone familiar with those bits) comment on pthread_yield()
> in ioport_write() of hw/qxl.c?  Which threads are supposed to run this
> way?

spice server thread.

> Can't this relation be expressed explicitly?

The thread is created by libspice-server, so we don't have a handle for it.

> If not, can we use a
> sleep here (how long?)?

Good question.  I'm tempted to just rip it out and run qxl_spice_oom() 
unconditionally.  The yield thing is best effort only anyway, there is 
no guarantee that the spice server thread actually gets scheduled and 
puts some stuff into the release ring.

cheers,
   Gerd

Re: [Qemu-devel] qxl: pthread_yield on QXL_IO_NOTIFY_OOM

[Jan Kiszka]

On 2011-09-20 16:48, Gerd Hoffmann wrote:
> On 09/20/11 16:27, Jan Kiszka wrote:
>> Hi Gerd,
>>
>> can you (or anyone familiar with those bits) comment on pthread_yield()
>> in ioport_write() of hw/qxl.c?  Which threads are supposed to run this
>> way?
> 
> spice server thread.
> 
>> Can't this relation be expressed explicitly?
> 
> The thread is created by libspice-server, so we don't have a handle for it.

And also no communication channel to kick?

> 
>> If not, can we use a
>> sleep here (how long?)?
> 
> Good question.  I'm tempted to just rip it out and run qxl_spice_oom() 
> unconditionally.  The yield thing is best effort only anyway, there is 
> no guarantee that the spice server thread actually gets scheduled and 
> puts some stuff into the release ring.

And the issue with sleeping is that we stall the vcpu. So just reporting
oom is likely better.

Thanks,
Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] qxl: pthread_yield on QXL_IO_NOTIFY_OOM

[Gerd Hoffmann]

On 09/20/11 17:00, Jan Kiszka wrote:
> On 2011-09-20 16:48, Gerd Hoffmann wrote:
>> On 09/20/11 16:27, Jan Kiszka wrote:
>>> Hi Gerd,
>>>
>>> can you (or anyone familiar with those bits) comment on pthread_yield()
>>> in ioport_write() of hw/qxl.c?  Which threads are supposed to run this
>>> way?
>>
>> spice server thread.
>>
>>> Can't this relation be expressed explicitly?
>>
>> The thread is created by libspice-server, so we don't have a handle for it.
>
> And also no communication channel to kick?

There is a pipe, and qxl_spice_oom() will kick that way.

I think the yield tries to avoid that kick in case the spice server 
thread is already busy processing stuff.  I'm not fully sure though, the 
code predates /me working on spice.  Each time I look at that piece of 
code I'm asking myself whenever it actually has a positive effect and I 
somehow doubt it.  I never found the time for deeper investigation though.

>> Good question.  I'm tempted to just rip it out and run qxl_spice_oom()
>> unconditionally.  The yield thing is best effort only anyway, there is
>> no guarantee that the spice server thread actually gets scheduled and
>> puts some stuff into the release ring.
>
> And the issue with sleeping is that we stall the vcpu. So just reporting
> oom is likely better.

Agree on that.

cheers,
   Gerd