----- 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@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/spice-devel > _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel