| Message ID | 4f197c437a4d2f813ce518b4fd2d86fc225f500e.1268214633.git.quintela@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Wed, 10 Mar 2010, Juan Quintela wrote: > > Signed-off-by: Juan Quintela <quintela@redhat.com> > --- > vl.c | 35 ++++++++++++++--------------------- > 1 files changed, 14 insertions(+), 21 deletions(-) > > diff --git a/vl.c b/vl.c > index 10d8e34..83ff652 100644 > --- a/vl.c > +++ b/vl.c > @@ -2597,10 +2597,12 @@ typedef struct IOHandlerRecord { > void *opaque; > /* temporary data */ > struct pollfd *ufd; > - struct IOHandlerRecord *next; > + QTAILQ_ENTRY(IOHandlerRecord) next; > } IOHandlerRecord; > > -static IOHandlerRecord *first_io_handler; > +static QTAILQ_HEAD(, IOHandlerRecord) io_handlers = > + QTAILQ_HEAD_INITIALIZER(io_handlers); > + > > /* XXX: fd_read_poll should be suppressed, but an API change is > necessary in the character devices to suppress fd_can_read(). */ > @@ -2610,28 +2612,22 @@ int qemu_set_fd_handler2(int fd, > IOHandler *fd_write, > void *opaque) > { > - IOHandlerRecord **pioh, *ioh; > + IOHandlerRecord *ioh; > > if (!fd_read && !fd_write) { > - pioh = &first_io_handler; > - for(;;) { > - ioh = *pioh; > - if (ioh == NULL) > - break; > + QTAILQ_FOREACH(ioh, &io_handlers, next) { > if (ioh->fd == fd) { > ioh->deleted = 1; > break; > } > - pioh = &ioh->next; > } > } else { > - for(ioh = first_io_handler; ioh != NULL; ioh = ioh->next) { > + QTAILQ_FOREACH(ioh, &io_handlers, next) { > if (ioh->fd == fd) > goto found; > } > ioh = qemu_mallocz(sizeof(IOHandlerRecord)); > - ioh->next = first_io_handler; > - first_io_handler = ioh; > + QTAILQ_INSERT_TAIL(&io_handlers, ioh, next); The old code inserted at the head, didn't it? > found: > ioh->fd = fd; > ioh->fd_read_poll = fd_read_poll; > @@ -3822,7 +3818,7 @@ void main_loop_wait(int timeout) > FD_ZERO(&rfds); > FD_ZERO(&wfds); > FD_ZERO(&xfds); > - for(ioh = first_io_handler; ioh != NULL; ioh = ioh->next) { > + QTAILQ_FOREACH(ioh, &io_handlers, next) { > if (ioh->deleted) > continue; > if (ioh->fd_read && > @@ -3848,9 +3844,9 @@ void main_loop_wait(int timeout) > ret = select(nfds + 1, &rfds, &wfds, &xfds, &tv); > qemu_mutex_lock_iothread(); > if (ret > 0) { > - IOHandlerRecord **pioh; > + IOHandlerRecord *pioh; > > - for(ioh = first_io_handler; ioh != NULL; ioh = ioh->next) { > + QTAILQ_FOREACH(ioh, &io_handlers, next) { > if (!ioh->deleted && ioh->fd_read && FD_ISSET(ioh->fd, &rfds)) { > ioh->fd_read(ioh->opaque); > } > @@ -3860,14 +3856,11 @@ void main_loop_wait(int timeout) > } > > /* remove deleted IO handlers */ > - pioh = &first_io_handler; > - while (*pioh) { > - ioh = *pioh; > + QTAILQ_FOREACH_SAFE(ioh, &io_handlers, next, pioh) { > if (ioh->deleted) { > - *pioh = ioh->next; > + QTAILQ_REMOVE(&io_handlers, ioh, next); > qemu_free(ioh); > - } else > - pioh = &ioh->next; > + } > } > } > >
malc <av1474@comtv.ru> wrote: > On Wed, 10 Mar 2010, Juan Quintela wrote: >> - for(ioh = first_io_handler; ioh != NULL; ioh = ioh->next) { >> + QTAILQ_FOREACH(ioh, &io_handlers, next) { >> if (ioh->fd == fd) >> goto found; >> } >> ioh = qemu_mallocz(sizeof(IOHandlerRecord)); >> - ioh->next = first_io_handler; >> - first_io_handler = ioh; >> + QTAILQ_INSERT_TAIL(&io_handlers, ioh, next); > > The old code inserted at the head, didn't it? Sorry, you are right, it shouldn't matter too much, but it is a change. Later, Juan.
On Wed, 10 Mar 2010, Juan Quintela wrote: > malc <av1474@comtv.ru> wrote: > > On Wed, 10 Mar 2010, Juan Quintela wrote: > > >> - for(ioh = first_io_handler; ioh != NULL; ioh = ioh->next) { > >> + QTAILQ_FOREACH(ioh, &io_handlers, next) { > >> if (ioh->fd == fd) > >> goto found; > >> } > >> ioh = qemu_mallocz(sizeof(IOHandlerRecord)); > >> - ioh->next = first_io_handler; > >> - first_io_handler = ioh; > >> + QTAILQ_INSERT_TAIL(&io_handlers, ioh, next); > > > > The old code inserted at the head, didn't it? > > Sorry, you are right, it shouldn't matter too much, but it is a change. If it did, why queue instead of list? > > Later, Juan. >
malc <av1474@comtv.ru> wrote: > On Wed, 10 Mar 2010, Juan Quintela wrote: > >> malc <av1474@comtv.ru> wrote: >> > On Wed, 10 Mar 2010, Juan Quintela wrote: >> >> >> - for(ioh = first_io_handler; ioh != NULL; ioh = ioh->next) { >> >> + QTAILQ_FOREACH(ioh, &io_handlers, next) { >> >> if (ioh->fd == fd) >> >> goto found; >> >> } >> >> ioh = qemu_mallocz(sizeof(IOHandlerRecord)); >> >> - ioh->next = first_io_handler; >> >> - first_io_handler = ioh; >> >> + QTAILQ_INSERT_TAIL(&io_handlers, ioh, next); >> > >> > The old code inserted at the head, didn't it? >> >> Sorry, you are right, it shouldn't matter too much, but it is a change. > > If it did, why queue instead of list? Arbitrary. Example conversion nearer was QTAIL. Use is: - insert at the beggining - search for removal - loop all list insert/searchs should be less than loops. I am more interested/intrigued if setting more fd's in the select call could change behaviour. Changing to QLIST is search/replace, no big deal. >> >> Later, Juan. >>
On Wed, 10 Mar 2010, Juan Quintela wrote: > malc <av1474@comtv.ru> wrote: > > On Wed, 10 Mar 2010, Juan Quintela wrote: > > > >> malc <av1474@comtv.ru> wrote: > >> > On Wed, 10 Mar 2010, Juan Quintela wrote: > >> > >> >> - for(ioh = first_io_handler; ioh != NULL; ioh = ioh->next) { > >> >> + QTAILQ_FOREACH(ioh, &io_handlers, next) { > >> >> if (ioh->fd == fd) > >> >> goto found; > >> >> } > >> >> ioh = qemu_mallocz(sizeof(IOHandlerRecord)); > >> >> - ioh->next = first_io_handler; > >> >> - first_io_handler = ioh; > >> >> + QTAILQ_INSERT_TAIL(&io_handlers, ioh, next); > >> > > >> > The old code inserted at the head, didn't it? > >> > >> Sorry, you are right, it shouldn't matter too much, but it is a change. > > > > If it did, why queue instead of list? > > Arbitrary. Example conversion nearer was QTAIL. Please do `man 3 queue'. Specifically the comparison between the tail queues and lists. > > Use is: > - insert at the beggining > - search for removal > - loop all list > > insert/searchs should be less than loops. I am more > interested/intrigued if setting more fd's in the select call could > change behaviour. > > Changing to QLIST is search/replace, no big deal. > > >> > >> Later, Juan. > >> >
malc <av1474@comtv.ru> wrote: > On Wed, 10 Mar 2010, Juan Quintela wrote: > >> malc <av1474@comtv.ru> wrote: >> > On Wed, 10 Mar 2010, Juan Quintela wrote: >> > >> >> malc <av1474@comtv.ru> wrote: >> >> > On Wed, 10 Mar 2010, Juan Quintela wrote: >> >> >> >> >> - for(ioh = first_io_handler; ioh != NULL; ioh = ioh->next) { >> >> >> + QTAILQ_FOREACH(ioh, &io_handlers, next) { >> >> >> if (ioh->fd == fd) >> >> >> goto found; >> >> >> } >> >> >> ioh = qemu_mallocz(sizeof(IOHandlerRecord)); >> >> >> - ioh->next = first_io_handler; >> >> >> - first_io_handler = ioh; >> >> >> + QTAILQ_INSERT_TAIL(&io_handlers, ioh, next); >> >> > >> >> > The old code inserted at the head, didn't it? >> >> >> >> Sorry, you are right, it shouldn't matter too much, but it is a change. >> > >> > If it did, why queue instead of list? >> >> Arbitrary. Example conversion nearer was QTAIL. > > Please do `man 3 queue'. Specifically the comparison between the tail > queues and lists. Thanks very much for the info. Didn't knew that page. You win. Will change to QLISTS. Waiting for more comments. Later, Juan.
diff --git a/vl.c b/vl.c index 10d8e34..83ff652 100644 --- a/vl.c +++ b/vl.c @@ -2597,10 +2597,12 @@ typedef struct IOHandlerRecord { void *opaque; /* temporary data */ struct pollfd *ufd; - struct IOHandlerRecord *next; + QTAILQ_ENTRY(IOHandlerRecord) next; } IOHandlerRecord; -static IOHandlerRecord *first_io_handler; +static QTAILQ_HEAD(, IOHandlerRecord) io_handlers = + QTAILQ_HEAD_INITIALIZER(io_handlers); + /* XXX: fd_read_poll should be suppressed, but an API change is necessary in the character devices to suppress fd_can_read(). */ @@ -2610,28 +2612,22 @@ int qemu_set_fd_handler2(int fd, IOHandler *fd_write, void *opaque) { - IOHandlerRecord **pioh, *ioh; + IOHandlerRecord *ioh; if (!fd_read && !fd_write) { - pioh = &first_io_handler; - for(;;) { - ioh = *pioh; - if (ioh == NULL) - break; + QTAILQ_FOREACH(ioh, &io_handlers, next) { if (ioh->fd == fd) { ioh->deleted = 1; break; } - pioh = &ioh->next; } } else { - for(ioh = first_io_handler; ioh != NULL; ioh = ioh->next) { + QTAILQ_FOREACH(ioh, &io_handlers, next) { if (ioh->fd == fd) goto found; } ioh = qemu_mallocz(sizeof(IOHandlerRecord)); - ioh->next = first_io_handler; - first_io_handler = ioh; + QTAILQ_INSERT_TAIL(&io_handlers, ioh, next); found: ioh->fd = fd; ioh->fd_read_poll = fd_read_poll; @@ -3822,7 +3818,7 @@ void main_loop_wait(int timeout) FD_ZERO(&rfds); FD_ZERO(&wfds); FD_ZERO(&xfds); - for(ioh = first_io_handler; ioh != NULL; ioh = ioh->next) { + QTAILQ_FOREACH(ioh, &io_handlers, next) { if (ioh->deleted) continue; if (ioh->fd_read && @@ -3848,9 +3844,9 @@ void main_loop_wait(int timeout) ret = select(nfds + 1, &rfds, &wfds, &xfds, &tv); qemu_mutex_lock_iothread(); if (ret > 0) { - IOHandlerRecord **pioh; + IOHandlerRecord *pioh; - for(ioh = first_io_handler; ioh != NULL; ioh = ioh->next) { + QTAILQ_FOREACH(ioh, &io_handlers, next) { if (!ioh->deleted && ioh->fd_read && FD_ISSET(ioh->fd, &rfds)) { ioh->fd_read(ioh->opaque); } @@ -3860,14 +3856,11 @@ void main_loop_wait(int timeout) } /* remove deleted IO handlers */ - pioh = &first_io_handler; - while (*pioh) { - ioh = *pioh; + QTAILQ_FOREACH_SAFE(ioh, &io_handlers, next, pioh) { if (ioh->deleted) { - *pioh = ioh->next; + QTAILQ_REMOVE(&io_handlers, ioh, next); qemu_free(ioh); - } else - pioh = &ioh->next; + } } }
Signed-off-by: Juan Quintela <quintela@redhat.com> --- vl.c | 35 ++++++++++++++--------------------- 1 files changed, 14 insertions(+), 21 deletions(-)