[Spice-devel] [PATCH spice] chardev: remove write polling
Marc-André Lureau
mlureau at redhat.com
Mon Oct 27 06:11:24 PDT 2014
----- Mail original -----
> Hi Marc-Andre,
>
> On 10/24/2014 01:17 PM, Marc-André Lureau wrote:
> > In an effort to reduce the wakeups per second, get rid of the
> > "write_to_dev" timer when the implementation supports
> > SPICE_CHAR_DEVICE_NOTIFY_WRITABLE.
> >
> > When this flag is set, the frontend instance is responsible for calling
> > spice_char_device_wakeup() when the device is ready to perform IO.
> >
> > Related to:
> > https://bugzilla.redhat.com/show_bug.cgi?id=912763
> > ---
> >
> > in v2:
> > - Check char device version before accessing the flags field.
> >
> > server/char_device.c | 28 +++++++++++++++++++++-------
> > server/spice.h | 9 +++++++--
> > 2 files changed, 28 insertions(+), 9 deletions(-)
> >
> > diff --git a/server/char_device.c b/server/char_device.c
> > index 6d2339e..487a5c5 100644
> > --- a/server/char_device.c
> > +++ b/server/char_device.c
> > @@ -438,7 +438,10 @@ static int
> > spice_char_device_write_to_device(SpiceCharDeviceState *dev)
> > }
> >
> > spice_char_device_state_ref(dev);
> > - core->timer_cancel(dev->write_to_dev_timer);
> > +
> > + if (dev->write_to_dev_timer) {
> > + core->timer_cancel(dev->write_to_dev_timer);
> > + }
> >
> > sif = SPICE_CONTAINEROF(dev->sin->base.sif, SpiceCharDeviceInterface,
> > base);
> > while (dev->running) {
> > @@ -473,8 +476,10 @@ static int
> > spice_char_device_write_to_device(SpiceCharDeviceState *dev)
> > /* retry writing as long as the write queue is not empty */
> > if (dev->running) {
> > if (dev->cur_write_buf) {
> > - core->timer_start(dev->write_to_dev_timer,
> > - CHAR_DEVICE_WRITE_TO_TIMEOUT);
> > + if (dev->write_to_dev_timer) {
> > + core->timer_start(dev->write_to_dev_timer,
> > + CHAR_DEVICE_WRITE_TO_TIMEOUT);
> > + }
> > } else {
> > spice_assert(ring_is_empty(&dev->write_queue));
> > }
> > @@ -635,6 +640,7 @@ SpiceCharDeviceState
> > *spice_char_device_state_create(SpiceCharDeviceInstance *si
> > void *opaque)
> > {
> > SpiceCharDeviceState *char_dev;
> > + SpiceCharDeviceInterface *sif;
> >
> > spice_assert(sin);
> > spice_assert(cbs->read_one_msg_from_device && cbs->ref_msg_to_client
> > &&
> > @@ -652,10 +658,15 @@ SpiceCharDeviceState
> > *spice_char_device_state_create(SpiceCharDeviceInstance *si
> > ring_init(&char_dev->write_bufs_pool);
> > ring_init(&char_dev->clients);
> >
> > - char_dev->write_to_dev_timer =
> > core->timer_add(spice_char_dev_write_retry, char_dev);
> > - if (!char_dev->write_to_dev_timer) {
> > - spice_error("failed creating char dev write timer");
> > + sif = SPICE_CONTAINEROF(char_dev->sin->base.sif,
> > SpiceCharDeviceInterface, base);
> > + if (sif->base.minor_version >= 1 && sif->base.minor_version >= 3
> > + && !(sif->flags & SPICE_CHAR_DEVICE_NOTIFY_WRITABLE)) {
>
> The condition here seems wrong:
> a) base.MAJOR_version >=1
> b) if e.g. sif.base.minor_version == 2, you want to use the timer, right ?
oops, you are correct, fixing that
>
> Uri
>
> > + char_dev->write_to_dev_timer =
> > core->timer_add(spice_char_dev_write_retry, char_dev);
> > + if (!char_dev->write_to_dev_timer) {
> > + spice_error("failed creating char dev write timer");
> > + }
> > }
> > +
> > char_dev->refs = 1;
> > sin->st = char_dev;
> > spice_debug("sin %p dev_state %p", sin, char_dev);
> > @@ -697,7 +708,9 @@ static void
> > spice_char_device_state_unref(SpiceCharDeviceState *char_dev)
> > void spice_char_device_state_destroy(SpiceCharDeviceState *char_dev)
> > {
> > reds_on_char_device_state_destroy(char_dev);
> > - core->timer_remove(char_dev->write_to_dev_timer);
> > + if (char_dev->write_to_dev_timer) {
> > + core->timer_remove(char_dev->write_to_dev_timer);
> > + }
> > write_buffers_queue_free(&char_dev->write_queue);
> > write_buffers_queue_free(&char_dev->write_bufs_pool);
> > if (char_dev->cur_write_buf) {
> > @@ -842,6 +855,7 @@ void spice_char_device_reset(SpiceCharDeviceState *dev)
> >
> > void spice_char_device_wakeup(SpiceCharDeviceState *dev)
> > {
> > + spice_char_device_write_to_device(dev);
> > spice_char_device_read_from_device(dev);
> > }
> >
> > diff --git a/server/spice.h b/server/spice.h
> > index 58700d1..bd5bba8 100644
> > --- a/server/spice.h
> > +++ b/server/spice.h
> > @@ -24,7 +24,7 @@
> > #include <spice/vd_agent.h>
> > #include <spice/macros.h>
> >
> > -#define SPICE_SERVER_VERSION 0x000c05 /* release 0.12.5 */
> > +#define SPICE_SERVER_VERSION 0x000c06 /* release 0.12.6 */
> >
> > #ifdef SPICE_SERVER_INTERNAL
> > #undef SPICE_GNUC_DEPRECATED
> > @@ -402,11 +402,15 @@ void
> > spice_server_set_record_rate(SpiceRecordInstance *sin, uint32_t
> > frequen
> >
> > #define SPICE_INTERFACE_CHAR_DEVICE "char_device"
> > #define SPICE_INTERFACE_CHAR_DEVICE_MAJOR 1
> > -#define SPICE_INTERFACE_CHAR_DEVICE_MINOR 2
> > +#define SPICE_INTERFACE_CHAR_DEVICE_MINOR 3
> > typedef struct SpiceCharDeviceInterface SpiceCharDeviceInterface;
> > typedef struct SpiceCharDeviceInstance SpiceCharDeviceInstance;
> > typedef struct SpiceCharDeviceState SpiceCharDeviceState;
> >
> > +typedef enum {
> > + SPICE_CHAR_DEVICE_NOTIFY_WRITABLE = 1 << 0,
> > +} spice_char_device_flags;
> > +
> > struct SpiceCharDeviceInterface {
> > SpiceBaseInterface base;
> >
> > @@ -414,6 +418,7 @@ struct SpiceCharDeviceInterface {
> > int (*write)(SpiceCharDeviceInstance *sin, const uint8_t *buf, int
> > len);
> > int (*read)(SpiceCharDeviceInstance *sin, uint8_t *buf, int len);
> > void (*event)(SpiceCharDeviceInstance *sin, uint8_t event);
> > + spice_char_device_flags flags;
> > };
> >
> > struct SpiceCharDeviceInstance {
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>
More information about the Spice-devel
mailing list