[PATCH] mempool: add unlikely and likely hints

[PATCH] mempool: add unlikely and likely hints

[Mikulas Patocka]

This patch adds unlikely and likely hints to the function mempool_free. It
lays out the code in such a way that the common path is executed
straighforward and saves a cache line.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

---
 mm/mempool.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: linux-3.13.5/mm/mempool.c
===================================================================
--- linux-3.13.5.orig/mm/mempool.c	2014-02-23 22:50:11.481071417 +0100
+++ linux-3.13.5/mm/mempool.c	2014-03-06 23:02:24.264538587 +0100
@@ -306,9 +306,9 @@ void mempool_free(void *element, mempool
 	 * ensures that there will be frees which return elements to the
 	 * pool waking up the waiters.
 	 */
-	if (pool->curr_nr < pool->min_nr) {
+	if (unlikely(pool->curr_nr < pool->min_nr)) {
 		spin_lock_irqsave(&pool->lock, flags);
-		if (pool->curr_nr < pool->min_nr) {
+		if (likely(pool->curr_nr < pool->min_nr)) {
 			add_element(pool, element);
 			spin_unlock_irqrestore(&pool->lock, flags);
 			wake_up(&pool->wait);

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mempool: add unlikely and likely hints

[David Rientjes]

On Thu, 6 Mar 2014, Mikulas Patocka wrote:

> This patch adds unlikely and likely hints to the function mempool_free. It
> lays out the code in such a way that the common path is executed
> straighforward and saves a cache line.
> 

What observable performance benefit have you seen with this patch and 
with what architecture?  Could we include some data in the changelog?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mempool: add unlikely and likely hints

[Mikulas Patocka]

On Fri, 7 Mar 2014, David Rientjes wrote:

> On Thu, 6 Mar 2014, Mikulas Patocka wrote:
> 
> > This patch adds unlikely and likely hints to the function mempool_free. It
> > lays out the code in such a way that the common path is executed
> > straighforward and saves a cache line.
> > 
> 
> What observable performance benefit have you seen with this patch and 
> with what architecture?  Could we include some data in the changelog?

None - you usually don't get observable performance benefit from 
microoptimizations like this.

It may be that the cache line that the patch saves aliases some other 
important cache lines and then, the patch saves two cache line refills. 
Or, the saved cache line doesn't alias anything important and then the 
patch doesn't have any effect at all. It's not worth spending many days or 
weeks trying to recreate a situation when the code cache is used in such a 
way that the patch would help.

Mikulas

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mempool: add unlikely and likely hints

[David Rientjes]

On Fri, 7 Mar 2014, Mikulas Patocka wrote:

> > What observable performance benefit have you seen with this patch and 
> > with what architecture?  Could we include some data in the changelog?
> 
> None - you usually don't get observable performance benefit from 
> microoptimizations like this.
> 
> It may be that the cache line that the patch saves aliases some other 
> important cache lines and then, the patch saves two cache line refills. 
> Or, the saved cache line doesn't alias anything important and then the 
> patch doesn't have any effect at all. It's not worth spending many days or 
> weeks trying to recreate a situation when the code cache is used in such a 
> way that the patch would help.
> 

Not sure there's any benefit of merging the patch, then.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mempool: add unlikely and likely hints

[Mikulas Patocka]

On Fri, 7 Mar 2014, David Rientjes wrote:

> On Fri, 7 Mar 2014, Mikulas Patocka wrote:
> 
> > > What observable performance benefit have you seen with this patch and 
> > > with what architecture?  Could we include some data in the changelog?
> > 
> > None - you usually don't get observable performance benefit from 
> > microoptimizations like this.
> > 
> > It may be that the cache line that the patch saves aliases some other 
> > important cache lines and then, the patch saves two cache line refills. 
> > Or, the saved cache line doesn't alias anything important and then the 
> > patch doesn't have any effect at all. It's not worth spending many days or 
> > weeks trying to recreate a situation when the code cache is used in such a 
> > way that the patch would help.
> 
> Not sure there's any benefit of merging the patch, then.

That's right, no one can be sure. The patch maybe helps and maybe has no 
effect (it can't hurt) - so there is no reason not to merge it.

If you measured the effect of microoptimizations like this, you spend 
excessive amount of time doing it and in the end you either improve 
performance a little bit or not. If you apply the patch blindly without 
measuring, you either improve performance a little bit or not. So - trying 
to prove that it helps doesn't have any positive effect at all.

If the patch could hurt performance, it would be reasonable to do some 
measurement to prove that it doesn't. But this one can't hurt.

Mikulas

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>