[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] ISCSI: redo how we set up the events to onl
From: |
ronnie sahlberg |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] ISCSI: redo how we set up the events to only call qemu_aio_set_fd_handler() and qemu_notify_event() if something has changed. |
Date: |
Fri, 25 May 2012 20:06:29 +1000 |
On Fri, May 25, 2012 at 7:59 PM, Paolo Bonzini <address@hidden> wrote:
> Il 25/05/2012 09:43, Ronnie Sahlberg ha scritto:
>> Also first call out to the socket write functions director, and only set up
>> the write event if the socket is full.
>>
>> This means that we will only need to invoke these two functions very rarely
>> which will improve performance.
>>
>> Signed-off-by: Ronnie Sahlberg <address@hidden>
>> ---
>> block/iscsi.c | 56
>> ++++++++++++++++++++++++++++++++++++++++++++++----------
>> 1 files changed, 46 insertions(+), 10 deletions(-)
>>
>> diff --git a/block/iscsi.c b/block/iscsi.c
>> index ed1ad7b..9642ee6 100644
>> --- a/block/iscsi.c
>> +++ b/block/iscsi.c
>> @@ -40,6 +40,8 @@ typedef struct IscsiLun {
>> int lun;
>> int block_size;
>> unsigned long num_blocks;
>> + IOHandler *read_handler;
>> + IOHandler *write_handler;
>> } IscsiLun;
>>
>> typedef struct IscsiAIOCB {
>> @@ -105,18 +107,50 @@ static void
>> iscsi_set_events(IscsiLun *iscsilun)
>> {
>> struct iscsi_context *iscsi = iscsilun->iscsi;
>> + int need_set_fd = 0;
>> + int need_notify_event = 0;
>>
>> - qemu_aio_set_fd_handler(iscsi_get_fd(iscsi), iscsi_process_read,
>> - (iscsi_which_events(iscsi) & POLLOUT)
>> - ? iscsi_process_write : NULL,
>> - iscsi_process_flush, iscsilun);
>> -
>> - /* If we just added the event for writeable we must call
>> - and the socket is already writeable the callback might
>> - not be invoked until after a short delay unless we call
>> - qemu_notify_event().
>> + /* Try to write as much as we can to the socket
>> + * without setting up an event
>> */
>> - qemu_notify_event();
>> + if (iscsi_which_events(iscsi) & POLLOUT) {
>> + iscsi_process_write(iscsilun);
>> + }
>> +
>> + if (iscsilun->read_handler == NULL) {
>> + iscsilun->read_handler = iscsi_process_read;
>> + need_set_fd = 1;
>> + need_notify_event = 1;
>> + }
>
> This is not needed, just pass iscsi_process_read unconditionally to
> qemu_aio_set_fd_handler.
>
>> + if (iscsi_which_events(iscsi) & POLLOUT) {
>> + if (iscsilun->write_handler == NULL) {
>> + iscsilun->write_handler = iscsi_process_write;
>> + need_set_fd = 1;
>> + need_notify_event = 1;
>> + }
>> + } else {
>> + if (iscsilun->write_handler != NULL) {
>> + iscsilun->write_handler = NULL;
>> + need_set_fd = 1;
>> + }
>> + }
>> +
>> + if (need_set_fd) {
>> + qemu_aio_set_fd_handler(iscsi_get_fd(iscsi),
>> + iscsilun->read_handler,
>> + iscsilun->write_handler,
>> + iscsi_process_flush,
>> + iscsilun);
>> +
>> + /* If we just added an event that may trigger almost immediately
>> + the callback might not be invoked until after a short
>> + delay unless we call qemu_notify_event().
>> + */
>> + if (need_notify_event) {
>> + qemu_notify_event();
>> + }
>> + }
>
> What about this:
>
> /* We always register a read handler. */
> ev = POLLIN;
> ev |= iscsi_which_events(iscsi);
> if (ev != iscsilun->events) {
> qemu_aio_set_fd_handler(iscsi_get_fd(iscsi),
> iscsi_process_read,
> (ev & POLLOUT) ? iscsi_process_write : NULL,
> iscsi_process_flush,
> iscsilun);
>
> }
>
> /* If we just added an event, the callback might be delayed
> * unless we call qemu_notify_event().
> */
> if (ev & ~iscsilun->events) {
> qemu_notify_event();
> }
> iscsilun->events = ev;
>
> ?
>
> Paolo
Yes. Even better.
Will you check that in or should I send a new patch with your suggestion?