Date: Thu, 18 Jul 2013 22:12:47 -0700 From: John-Mark Gurney <jmg@funkthat.com> To: Hans Petter Selasky <hps@bitfrost.no> Cc: freebsd-multimedia@freebsd.org, Bernhard =?iso-8859-1?Q?Fr=F6hlich?= <decke@bluelife.at>, Juergen Lock <nox@jelal.kn-bremen.de> Subject: Re: kqueue(2) vs. cuse4bsd? Message-ID: <20130719051247.GF26412@funkthat.com> In-Reply-To: <51E85CFC.3060305@bitfrost.no> References: <201307181834.r6IIY35e055755@triton8.kn-bremen.de> <51E844C9.4060106@bitfrost.no> <CAE-m3X2_QdQPf6_0mF8cn3fdmMvmagBdXhq6SCqWON3NF8NANQ@mail.gmail.com> <51E85CFC.3060305@bitfrost.no>
next in thread | previous in thread | raw e-mail | index | archive | help
Hans Petter Selasky wrote this message on Thu, Jul 18, 2013 at 23:24 +0200:
> Hi,
>
> >>
> >>kevent() support is not implemented for cuse4bsd. The devfs callback is
> >NULL. Possibly could add that, but have to read up on kqueue first.
> >
> >Oh what a bummer. It would definitely be good to add that callback but is
> >this everything that is needed or should we expect more problems?
> >
>
> Hi,
>
> Can you try the attached patch for cuse4bsd. Note sure if it works.
> Check the CPU usage if it is spinning 100% or not. I had to fake some
> kqueue events. Also need to know if this code compiles with 8-stable.
>
> --HPS
> Index: cuse4bsd_kmod.c
> ===================================================================
> --- cuse4bsd_kmod.c (revision 2614)
> +++ cuse4bsd_kmod.c (working copy)
> @@ -1,5 +1,5 @@
> /*-
> - * Copyright (c) 2010-2012 Hans Petter Selasky. All rights reserved.
> + * Copyright (c) 2010-2013 Hans Petter Selasky. All rights reserved.
> *
> * Redistribution and use in source and binary forms, with or without
> * modification, are permitted provided that the following conditions
> @@ -134,6 +134,15 @@
> static int cuse_alloc_unit_id[CUSE_DEVICES_MAX];
> static struct cuse_memory cuse_mem[CUSE_ALLOC_UNIT_MAX];
>
> +static void cuse_client_kqfilter_detach(struct knote *kn);
> +static int cuse_client_kqfilter_event(struct knote *kn, long hint);
> +
> +static struct filterops cuse_client_kqfilter_ops = {
> + .f_isfd = 1,
> + .f_detach = cuse_client_kqfilter_detach,
> + .f_event = cuse_client_kqfilter_event,
> +};
> +
> static d_open_t cuse_client_open;
> static d_close_t cuse_client_close;
> static d_ioctl_t cuse_client_ioctl;
> @@ -141,6 +150,7 @@
> static d_write_t cuse_client_write;
> static d_poll_t cuse_client_poll;
> static d_mmap_t cuse_client_mmap;
> +static d_kqfilter_t cuse_client_kqfilter;
>
> static struct cdevsw cuse_client_devsw = {
> .d_version = D_VERSION,
> @@ -153,6 +163,7 @@
> .d_write = cuse_client_write,
> .d_poll = cuse_client_poll,
> .d_mmap = cuse_client_mmap,
> + .d_kqfilter = cuse_client_kqfilter,
> };
>
> static d_open_t cuse_server_open;
> @@ -635,6 +646,10 @@
>
> cuse_server_free_memory(pcs);
>
> + seldrain(&pcs->selinfo);
> +
> + knlist_destroy(&pcs->selinfo.si_note);
> +
> cuse_unlock();
>
> cv_destroy(&pcs->cv);
> @@ -662,6 +677,8 @@
>
> cv_init(&pcs->cv, "cuse-server-cv");
>
> + knlist_init_mtx(&pcs->selinfo.si_note, &cuse_mtx);
> +
> cuse_lock();
> pcs->refs++;
> TAILQ_INSERT_TAIL(&cuse_server_head, pcs, entry);
> @@ -1698,3 +1715,38 @@
>
> return (0);
> }
> +
> +static void
> +cuse_client_kqfilter_detach(struct knote *kn)
> +{
> + struct cuse_server *pcs;
> +
> + cuse_lock();
> + pcs = kn->kn_hook;
> + knlist_remove(&pcs->selinfo.si_note, kn, 0);
> + cuse_unlock();
If cuse_lock locks the cuse_mtx passed in above, the _lock/_unlock is not
needed, since your 0 arg is saying that you aren't locked..
> +}
> +
> +static int
> +cuse_client_kqfilter_event(struct knote *kn, long hint)
> +{
> + return (1);
> +}
This should always check to make sure that an event is available for
reading/writing... This will likely either cause the application to
spin, or possibly hang, since it will try to read, and then the fd
could block...
I'm not sure how cuse works... usually it's only for read/write style
operations, though if it doesn't support those types of operations and
only ioctls, than I'm not sure why it needs kqueue support...
> +static int
> +cuse_client_kqfilter(struct cdev *dev, struct knote *kn)
> +{
> + struct cuse_client *pcc;
> + int error;
> +
> + error = cuse_client_get(&pcc);
> + if (error != 0)
> + return (error);
> +
> + cuse_lock();
> + kn->kn_hook = pcc->server;
> + kn->kn_fop = &cuse_client_kqfilter_ops;
> + knlist_add(&pcc->server->selinfo.si_note, kn, 0);
> + cuse_unlock();
> + return (0);
> +}
Again, the 0 arg to knlist_add says that the cuse_mtx lock has already
been locked.. either change the arg to 1, or drop the _lock/_unlock()...
--
John-Mark Gurney Voice: +1 415 225 5579
"All that I will do, has been done, All that I have, has not."
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130719051247.GF26412>