Davide Libenzi wrote:
> On Thu, 17 May 2007, Avi Kivity wrote:
>
>
>> Davide Libenzi wrote:
>>
>>> On Wed, 16 May 2007, Avi Kivity wrote:
>>>
>>>
>>>
>>>> IMO doing eventfd_fget() asap is best. I much prefer refcounted pointers
>>>> to
>>>> handles in the kernel: it's easier to see what things point to, and there
>>>> is
>>>> to context needed for dereferencing.
>>>>
>>>>
>>> There are concerns (from Al and Christoph) about file lifetime tracking of
>>> an fd passed down to the kernel.
>>>
>>>
>> What concerns are these?
>>
>> I have my own concerns about bare fds, which can change their backing file,
>> and which are dependent on the context. The kernel appears to be moving away
>> from handles to pointers, as seen by the pid -> task_struck/struct pid
>> conversion.
>>
>
> The concern is that by keeping a reference to the file* you may end up
> with a file that has no more userspace links. Although the file_count()==1
> will tell you that you've the only reference left, this relies on you
> doing the check. I really don't know how this will be used in KVM, so I
> can't really say how well these concerns applies.
>
>
That's perfectly fine for kvm (and as far as I can see, any other
application: the user losing their eventfd reference is equivalent to
the user ignoring any events, which should be survivable by the kernel).
>
>
>
>>> Avi, how about you go the other way around? I expose you something like:
>>>
>>> long eventfd_create(unsigned int count, void (*release)(void *),
>>> void *priv);
>>>
>>> This returns you an eventfd (or error), and lets you install a callback to
>>> be used when the file_operations->release is called (basically, userspace
>>> closed the last file instance).
>>> Can KVM pass back an fd to userspace instead of the other way around?
>>>
>>>
>> That was my original thought, but when I saw your proposal I preferred that
>> as
>> more flexible.
>>
>> If we go this way, I prefer to have a struct file * and do away with the
>> callback, but that brings us back to the file lifetime concerns.
>>
>
> Doing the above requires some care too. Access to your copy of the file*
> must be protected by a lock (you can sleep from inside the callback, so a
> mutex is fine too). I'll sketch you some code:
>
>
> /* Defined in linux/eventfd.h */
> struct eventfd_relcb {
> struct list_head lnk;
> void (*proc)(struct eventfd_relcb *);
> };
>
> /* Your context data */
> struct your_ctx {
> whatever_lock your_lock;
> ...
> struct eventfd_relcb rcb;
> struct file *evfile;
> };
>
> /* Your eventfd release callback */
> void rcb_callback(struct eventfd_relcb *rcb) {
> struct your_ctx *c = container_of(rcb, struct your_ctx, rcb);
>
> whatever_lock_lock(&c->your_lock);
> ...
> c->evfile = NULL;
> whatever_lock_unlock(&c->your_lock);
> }
>
> /* Your notify userspace function */
> void notify_userspace(struct your_ctx *c) {
>
> whatever_lock_lock(&c->your_lock);
> if (c->evfile != NULL)
> eventfd_signal(c->evfile, 1);
> whatever_lock_unlock(&c->your_lock);
> }
>
> /* Your eventfd create/setup function (modulo error checks) */
> void setup_eventfd(struct your_ctx *c) {
> int fd;
>
> c->rcb.proc = rcb_callback;
> fd = eventfd_create(0, &c->rcb);
> c->evfile = eventfd_fget(fd);
> /* Then return fd to userspace in some way */
> ...
> }
>
>
>
> If, by any chance, you need to detach yourself from the eventfd:
>
> void detach_eventfd(struct your_ctx *c) {
>
> whatever_lock_lock(&c->your_lock);
> if (c->evfile != NULL) {
> eventfd_del_release_cb(c->evfile, &c->rcb);
> /* And, optionally ... */
> eventfd_set_shutdown(c->evfile);
> }
> whatever_lock_unlock(&c->your_lock);
> }
>
>
> The eventfd_set_shutdown() function will make a POLLIN signaled to
> userspace, and a read(2) on the eventfd will return 0 (like peer
> disconnected sockets). Then userspace will know that no more eventfs will
> show up in there.
> Tentative patch below (builds, not tested yet).
>
>
>
Looks like a lot of code for what was supposed to be a code reduction...
I think I'll look at Gregory's notification-free suggestion instead.
--
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/
_______________________________________________
kvm-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/kvm-devel
