Messages in this thread | | | Date | Fri, 14 Apr 2006 12:31:18 -0700 | From | Andrew Morton <> | Subject | Re: [PATCH] Poll microoptimizations. |
| |
Vadim Lobanov <vlobanov@speakeasy.net> wrote: > > Patch to provide some microoptimizations for the poll() system call > implementation. The loop that traverses over the "struct pollfd" entries > was moved from do_pollfd() to its single caller do_poll(), so that > do_pollfd() no longer mucks around with the "count" and the "pt" > variables that should belong to do_poll() alone. This saves unnecessary > levels of indirection. Modifications were run tested. > > > diff -Npru linux-2.6.17-rc1/fs/select.c linux-new/fs/select.c > --- linux-2.6.17-rc1/fs/select.c 2006-04-12 20:31:54.000000000 -0700 > +++ linux-new/fs/select.c 2006-04-13 18:54:14.000000000 -0700 > @@ -544,37 +544,30 @@ struct poll_list { > > #define POLLFD_PER_PAGE ((PAGE_SIZE-sizeof(struct poll_list)) / sizeof(struct pollfd)) > > -static void do_pollfd(unsigned int num, struct pollfd * fdpage, > - poll_table ** pwait, int *count) > +static int do_pollfd(struct pollfd * pollfd, poll_table * pwait)
Please omit the space after the asterisk:
static int do_pollfd(struct pollfd *pollfd, poll_table *pwait)
because it doesn't impart any information, it is sightly misleading, it wastes screen real-estate and we should be consistent.
> { > - int i; > + unsigned int mask; > + int fd; > > - for (i = 0; i < num; i++) { > - int fd; > - unsigned int mask; > - struct pollfd *fdp; > - > - mask = 0; > - fdp = fdpage+i; > - fd = fdp->fd; > - if (fd >= 0) { > - int fput_needed; > - struct file * file = fget_light(fd, &fput_needed); > - mask = POLLNVAL; > - if (file != NULL) { > - mask = DEFAULT_POLLMASK; > - if (file->f_op && file->f_op->poll) > - mask = file->f_op->poll(file, *pwait); > - mask &= fdp->events | POLLERR | POLLHUP; > - fput_light(file, fput_needed); > - } > - if (mask) { > - *pwait = NULL; > - (*count)++; > - } > + mask = 0; > + fd = pollfd->fd; > + if (fd >= 0) { > + int fput_needed; > + struct file * file; > + > + file = fget_light(fd, &fput_needed); > + mask = POLLNVAL; > + if (file != NULL) { > + mask = DEFAULT_POLLMASK; > + if (file->f_op && file->f_op->poll) > + mask = file->f_op->poll(file, pwait); > + mask &= pollfd->events | POLLERR | POLLHUP; > + fput_light(file, fput_needed); > } > - fdp->revents = mask; > } > + pollfd->revents = mask; > + > + return (mask != 0); > }
So do_poll_fd() returns either 0 or 1.
> static int do_poll(unsigned int nfds, struct poll_list *list, > @@ -592,10 +585,19 @@ static int do_poll(unsigned int nfds, s > long __timeout; > > set_current_state(TASK_INTERRUPTIBLE); > - walk = list; > - while(walk != NULL) { > - do_pollfd( walk->len, walk->entries, &pt, &count); > - walk = walk->next; > + for (walk = list; walk != NULL; walk = walk->next) { > + struct pollfd * pfd, * pfd_end; > + > + pfd = walk->entries; > + pfd_end = pfd + walk->len; > + for (; pfd != pfd_end; pfd++) { > + int ev; > + > + ev = do_pollfd(pfd, pt);
`ev' is either 0 or 1.
> + count += ev; > + ev--;
`ev' is either -1 or 0.
> + pt = (poll_table*)((unsigned long)pt & ev);
So as long as the sign-extension works as we hope (which I think it will), `pt' is either unaltered or is NULL.
Yuk. Sorry, no.
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |