From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH 05/10] KVM: Adds ability to signal userspace using a file-descriptor Date: Mon, 14 May 2007 15:22:32 +0300 Message-ID: <46485488.2010608@qumranet.com> References: <20070510123831.10200.4769.stgit@novell1.haskins.net> <20070510124706.10200.68571.stgit@novell1.haskins.net> <46470BF7.5080108@qumranet.com> <46481A61.BA47.005A.0@novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org, Davide Libenzi To: Gregory Haskins Return-path: In-Reply-To: <46481A61.BA47.005A.0-Et1tbQHTxzrQT0dZR+AlfA@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org Errors-To: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org List-Id: kvm.vger.kernel.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/