Re: [PATCH 05/10] KVM: Adds ability to signal userspace using a file-descriptor

[Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org>]

Gregory Haskins wrote:
>>
>> Is having a read() (or a write()) actually necessary?
>>     
>
> Based on what I know: yes.  It could be a case of ignorance, however ;)
>
> Heres why I think its necessary:  You need poll to simply tell you when something is pending.  You can't clear the pending status in poll because you cannot predict the internal access pattern (e.g. I assume it could be polled multiple times by the kernel without returning immediately to userspace).  Therefore, you need a second method to actually clear the pending "signal", which I use the read() method for.  I can be convinced otherwise, but that was my original thinking.
>   

I think you are right, but am cc'ing an expert. Davide, we're using an 
fd to signal something to userspace, but have nothing to actually read() 
or write(). Is a read() or write() avoidable?

>   
>>> +
>>> +	if (indirect_sig && waitqueue_active(&vcpu- >irq.wq))
>>> +		wake_up(&vcpu- >irq.wq);
>>>  }
>>>  
>>>   
>>>       
>> Did you check that we can actually deliver signals with this?  I think a 
>> fasync_struct or something like that is necessary, but not sure.
>>     
>
> Actually, my signals *didn't* seem to be working, but they werent working with "send_sig()" either so I just assumed I had a userspace coding problem.  Based on what I read, it seemed like what I did should work if you do a fcntl(F_SETSIG), etc.  But again, it could be ignorance.  I am not familiar with fasync_struct.  If you have any pointers, please forward.
>
>   

fs/pipe.c. hairy stuff.

>> Another implementation option (which I've only thought of now, sorry) is 
>> to have an ioctl which returns a real eventfd, reducing some code 
>> duplication.
>>     
>
> So based on this, I assume eventfd must be in the kernel already?  Cool.

It is in 2.6.22-rc1. As is the anonymous inodes source which can be used 
to retire kvmfs (which will probably break the record for shortest-lived 
filesystem ever).

>   Even if its not, I like this idea much better than what I did.  There was still an unresolved problem regarding how I was going to expose the signaling mechanism to QEMU without giving away the vcpu_fd from the kvmctl library that this solves nicely.
>
> With this methodology, I can simply provide a function like "kvm_vcpu_get_eventfd()" in the library, and return the eventfd directly to the QEMU process.  Then we dont have to worry about layering violations.  Nice!
>   

I hadn't though of it. Looks like a win from all directions.

It means we need to package eventfd for the external module, but that's 
easily done.

-- 
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/