Re: [PATCH] mm: filemap: Avoid unnecessary barries and waitqueue lookups in unlock_page fastpath v5

[Peter Zijlstra]

On Thu, May 22, 2014 at 01:07:15AM +0100, Mel Gorman wrote:

> +PAGEFLAG(Waiters, waiters) __CLEARPAGEFLAG(Waiters, waiters)
> +	TESTCLEARFLAG(Waiters, waiters)
> +#define __PG_WAITERS		(1 << PG_waiters)
> +#else
> +/* Always fallback to slow path on 32-bit */
> +static inline bool PageWaiters(struct page *page)
> +{
> +	return true;
> +}
> +static inline void __ClearPageWaiters(struct page *page) {}
> +static inline void ClearPageWaiters(struct page *page) {}
> +static inline void SetPageWaiters(struct page *page) {}
> +#define __PG_WAITERS		0


> +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;
> +
> +	/*
> +	 * Unlike __wake_up_bit it is necessary to check waitqueue_active to be
> +	 * checked under the wqh->lock to avoid races with parallel additions
> +	 * to the waitqueue. Otherwise races could result in lost wakeups
> +	 */

Well, you could do something like:

	if (!__PG_WAITERS && !waitqueue_active(wqh))
		return;

Which at least for 32bit restores some of the performance loss of this
patch (did you have 32bit numbers in that massive changelog?, I totally
tl;dr it).

> +	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);
> +}

Attachment: pgpeur0OPbrsZ.pgp
Description: PGP signature