| Message ID | 1337931796-2660-2-git-send-email-ronniesahlberg@gmail.com |
|---|---|
| State | New |
| Headers | show |
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 <ronniesahlberg@gmail.com> > --- > 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
On Fri, May 25, 2012 at 7:59 PM, Paolo Bonzini <pbonzini@redhat.com> 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 <ronniesahlberg@gmail.com> >> --- >> 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?
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; + } + + 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(); + } + } } static void @@ -790,6 +824,8 @@ static void iscsi_close(BlockDriverState *bs) IscsiLun *iscsilun = bs->opaque; struct iscsi_context *iscsi = iscsilun->iscsi; + iscsilun->read_handler = NULL; + iscsilun->write_handler = NULL; qemu_aio_set_fd_handler(iscsi_get_fd(iscsi), NULL, NULL, NULL, NULL); iscsi_destroy_context(iscsi); memset(iscsilun, 0, sizeof(IscsiLun));
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 <ronniesahlberg@gmail.com> --- block/iscsi.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++---------- 1 files changed, 46 insertions(+), 10 deletions(-)