On Wed, May 21, 2014 at 01:15:01PM +0100, Mel Gorman wrote: > Andrew had suggested dropping v4 of the patch entirely as the numbers were > marginal and the complexity was high. However, even on a relatively small > machine running simple workloads the overhead of page_waitqueue and wakeup > functions is around 5% of system CPU time. That's quite high for basic > operations so I felt it was worth another shot. The performance figures > are better with this version than they were for v4 and overall the patch > should be more comprehensible. Simpler patch and better performance, yay! > This patch introduces a new page flag for 64-bit capable machines, > PG_waiters, to signal there are processes waiting on PG_lock and uses it to > avoid memory barriers and waitqueue hash lookup in the unlock_page fastpath. The patch seems to also explicitly use it for PG_writeback, yet no mention of that here. > diff --git a/kernel/sched/wait.c b/kernel/sched/wait.c > index 0ffa20a..f829e73 100644 > --- a/kernel/sched/wait.c > +++ b/kernel/sched/wait.c > @@ -167,31 +167,39 @@ EXPORT_SYMBOL_GPL(__wake_up_sync); /* For internal use only */ > * stops them from bleeding out - it would still allow subsequent > * loads to move into the critical region). > */ > +static inline void Make that __always_inline, that way we're guaranteed to optimize the build time constant .page=NULL cases. > +__prepare_to_wait(wait_queue_head_t *q, wait_queue_t *wait, > + struct page *page, int state, bool exclusive) > { > unsigned long flags; > > + if (page && !PageWaiters(page)) > + SetPageWaiters(page); > + if (list_empty(&wait->task_list)) { > + if (exclusive) { > + wait->flags |= WQ_FLAG_EXCLUSIVE; > + __add_wait_queue_tail(q, wait); > + } else { I'm fairly sure we've just initialized the wait thing to 0, so clearing the bit would be superfluous. > + wait->flags &= ~WQ_FLAG_EXCLUSIVE; > + __add_wait_queue(q, wait); > + } > + } > set_current_state(state); > spin_unlock_irqrestore(&q->lock, flags); > } > + > +void > +prepare_to_wait(wait_queue_head_t *q, wait_queue_t *wait, int state) > +{ > + return __prepare_to_wait(q, wait, NULL, state, false); > +} > EXPORT_SYMBOL(prepare_to_wait); > > void > prepare_to_wait_exclusive(wait_queue_head_t *q, wait_queue_t *wait, int state) > { > + return __prepare_to_wait(q, wait, NULL, state, true); > } > EXPORT_SYMBOL(prepare_to_wait_exclusive); > > @@ -228,7 +236,8 @@ EXPORT_SYMBOL(prepare_to_wait_event); > * the wait descriptor from the given waitqueue if still > * queued. > */ > +static inline void __finish_wait(wait_queue_head_t *q, wait_queue_t *wait, > + struct page *page) > { Same thing, make that __always_inline. > unsigned long flags; > > @@ -249,9 +258,16 @@ void finish_wait(wait_queue_head_t *q, wait_queue_t *wait) > if (!list_empty_careful(&wait->task_list)) { > spin_lock_irqsave(&q->lock, flags); > list_del_init(&wait->task_list); > + if (page && !waitqueue_active(q)) > + ClearPageWaiters(page); > spin_unlock_irqrestore(&q->lock, flags); > } > } > + > +void finish_wait(wait_queue_head_t *q, wait_queue_t *wait) > +{ > + return __finish_wait(q, wait, NULL); > +} > EXPORT_SYMBOL(finish_wait); > > /** > @@ -374,6 +427,19 @@ int __sched out_of_line_wait_on_bit_lock(void *word, int bit, > } > EXPORT_SYMBOL(out_of_line_wait_on_bit_lock); > > +void __wake_up_page_bit(wait_queue_head_t *wqh, struct page *page, void *word, int bit) > +{ > + struct wait_bit_key key = __WAIT_BIT_KEY_INITIALIZER(word, bit); > + unsigned long flags; > + > + spin_lock_irqsave(&wqh->lock, flags); > + if (waitqueue_active(wqh)) > + __wake_up_common(wqh, TASK_NORMAL, 1, 0, &key); > + else > + ClearPageWaiters(page); > + spin_unlock_irqrestore(&wqh->lock, flags); > +} Seeing how word is always going to be &page->flags, might it make sense to remove that argument? Anyway, looks good in principle. Oleg? -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx"> email@xxxxxxxxx </a>