Messages in this thread | | | Date | Sat, 22 Nov 2008 02:11:52 +0900 | From | Tejun Heo <> | Subject | Re: [PATCH RESEND] poll: allow f_op->poll to sleep, take #2 |
| |
Miklos Szeredi wrote: > On Thu, 20 Nov 2008, Tejun Heo wrote: > > [snip] > >> +int poll_schedule_timeout(struct poll_wqueues *pwq, int state, >> + ktime_t *expires, unsigned long slack) > > The 'state' parameter is unused, and is always called with the > TASK_INTERRUPTIBLE value. Shouldn't it be removed? > >> +{ >> + int rc = -EINTR; >> + >> + set_current_state(TASK_INTERRUPTIBLE);
Aieee... this should have been set_current_state(state). We can also remove @state but this being a schedule() function I think it's better to pass @state explicitly.
>> + if (!pwq->triggered) >> + rc = schedule_hrtimeout_range(expires, slack, HRTIMER_MODE_ABS); >> + __set_current_state(TASK_RUNNING); >> + >> + /* clear triggered for the next iteration */ >> + pwq->triggered = 0; >> + >> + return rc; >> +} >> + >> +EXPORT_SYMBOL(poll_schedule_timeout); > > Checkpatch warning: > > WARNING: EXPORT_SYMBOL(foo); should immediately follow its function/variable > #118: FILE: fs/select.c:216: > +EXPORT_SYMBOL(poll_schedule_timeout); > > [snip] > >> Index: work/include/linux/poll.h >> =================================================================== >> --- work.orig/include/linux/poll.h >> +++ work/include/linux/poll.h >> @@ -57,6 +57,8 @@ struct poll_table_entry { >> struct poll_wqueues { >> poll_table pt; >> struct poll_table_page * table; >> + struct task_struct * polling_task; >> + int triggered; > > Checkpatch error: > > ERROR: "foo * bar" should be "foo *bar" > #173: FILE: include/linux/poll.h:60: > + struct task_struct * polling_task;
For both, I was trying to stay consistent with the environment. I find mixed styles in close proximity much uglier than slightly different but consistent style. Eh... Is the consensus checkpatch or die?
Thanks.
-- tejun
| |