[PATCH] Locking optimization for cache_reap

[PATCH] Locking optimization for cache_reap

[Dimitri Sivanich]

Here is another cache_reap optimization that reduces latency when
applied after the 'Move cache_reap out of timer context' patch I
submitted on 7/14 (for inclusion in -mm next week).

This applies to 2.6.8-rc2 + the above mentioned patch.

Signed-off-by: Dimitri Sivanich <sivanich@sgi.com>


Index: linux/mm/slab.c
===================================================================
--- linux.orig/mm/slab.c
+++ linux/mm/slab.c
@@ -2619,27 +2619,6 @@ static void enable_cpucache (kmem_cache_
 					cachep->name, -err);
 }
 
-static void drain_array(kmem_cache_t *cachep, struct array_cache *ac)
-{
-	int tofree;
-
-	check_irq_off();
-	if (ac->touched) {
-		ac->touched = 0;
-	} else if (ac->avail) {
-		tofree = (ac->limit+4)/5;
-		if (tofree > ac->avail) {
-			tofree = (ac->avail+1)/2;
-		}
-		spin_lock(&cachep->spinlock);
-		free_block(cachep, ac_entry(ac), tofree);
-		spin_unlock(&cachep->spinlock);
-		ac->avail -= tofree;
-		memmove(&ac_entry(ac)[0], &ac_entry(ac)[tofree],
-					sizeof(void*)*ac->avail);
-	}
-}
-
 static void drain_array_locked(kmem_cache_t *cachep,
 				struct array_cache *ac, int force)
 {
@@ -2697,16 +2676,14 @@ static void cache_reap (void *unused)
 			goto next;
 
 		check_irq_on();
-		local_irq_disable();
-		drain_array(searchp, ac_data(searchp));
 
-		if(time_after(searchp->lists.next_reap, jiffies))
-			goto next_irqon;
+		spin_lock_irq(&searchp->spinlock);
+
+		drain_array_locked(searchp, ac_data(searchp), 0);
 
-		spin_lock(&searchp->spinlock);
-		if(time_after(searchp->lists.next_reap, jiffies)) {
+		if(time_after(searchp->lists.next_reap, jiffies))
 			goto next_unlock;
-		}
+
 		searchp->lists.next_reap = jiffies + REAPTIMEOUT_LIST3;
 
 		if (searchp->lists.shared)
@@ -2739,9 +2716,7 @@ static void cache_reap (void *unused)
 			spin_lock_irq(&searchp->spinlock);
 		} while(--tofree > 0);
 next_unlock:
-		spin_unlock(&searchp->spinlock);
-next_irqon:
-		local_irq_enable();
+		spin_unlock_irq(&searchp->spinlock);
 next:
 		;
 	}
--
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:"aart@kvack.org"> aart@kvack.org </a>

Re: [PATCH] Locking optimization for cache_reap

[Andrew Morton]

Dimitri Sivanich <sivanich@sgi.com> wrote:
>
> Here is another cache_reap optimization that reduces latency when
> applied after the 'Move cache_reap out of timer context' patch I
> submitted on 7/14 (for inclusion in -mm next week).
> 
> This applies to 2.6.8-rc2 + the above mentioned patch.

How does it "reduce latency"?

It looks like a reasonable cleanup, but afaict it will result in the
per-cache spinlock actually being held for longer periods, thus increasing
latencies???

> 
> 
> Index: linux/mm/slab.c
> ===================================================================
> --- linux.orig/mm/slab.c
> +++ linux/mm/slab.c
> @@ -2619,27 +2619,6 @@ static void enable_cpucache (kmem_cache_
>  					cachep->name, -err);
>  }
>  
> -static void drain_array(kmem_cache_t *cachep, struct array_cache *ac)
> -{
> -	int tofree;
> -
> -	check_irq_off();
> -	if (ac->touched) {
> -		ac->touched = 0;
> -	} else if (ac->avail) {
> -		tofree = (ac->limit+4)/5;
> -		if (tofree > ac->avail) {
> -			tofree = (ac->avail+1)/2;
> -		}
> -		spin_lock(&cachep->spinlock);
> -		free_block(cachep, ac_entry(ac), tofree);
> -		spin_unlock(&cachep->spinlock);
> -		ac->avail -= tofree;
> -		memmove(&ac_entry(ac)[0], &ac_entry(ac)[tofree],
> -					sizeof(void*)*ac->avail);
> -	}
> -}
> -
>  static void drain_array_locked(kmem_cache_t *cachep,
>  				struct array_cache *ac, int force)
>  {
> @@ -2697,16 +2676,14 @@ static void cache_reap (void *unused)
>  			goto next;
>  
>  		check_irq_on();
> -		local_irq_disable();
> -		drain_array(searchp, ac_data(searchp));
>  
> -		if(time_after(searchp->lists.next_reap, jiffies))
> -			goto next_irqon;
> +		spin_lock_irq(&searchp->spinlock);
> +
> +		drain_array_locked(searchp, ac_data(searchp), 0);
>  
> -		spin_lock(&searchp->spinlock);
> -		if(time_after(searchp->lists.next_reap, jiffies)) {
> +		if(time_after(searchp->lists.next_reap, jiffies))
>  			goto next_unlock;
> -		}
> +
>  		searchp->lists.next_reap = jiffies + REAPTIMEOUT_LIST3;
>  
>  		if (searchp->lists.shared)
> @@ -2739,9 +2716,7 @@ static void cache_reap (void *unused)
>  			spin_lock_irq(&searchp->spinlock);
>  		} while(--tofree > 0);
>  next_unlock:
> -		spin_unlock(&searchp->spinlock);
> -next_irqon:
> -		local_irq_enable();
> +		spin_unlock_irq(&searchp->spinlock);
>  next:
>  		;
>  	}
--
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:"aart@kvack.org"> aart@kvack.org </a>

Re: [PATCH] Locking optimization for cache_reap

[Dimitri Sivanich]

On Mon, Jul 26, 2004 at 06:01:04PM -0700, Andrew Morton wrote:
> Dimitri Sivanich <sivanich@sgi.com> wrote:
> >
> > Here is another cache_reap optimization that reduces latency when
> > applied after the 'Move cache_reap out of timer context' patch I
> > submitted on 7/14 (for inclusion in -mm next week).
> > 
> > This applies to 2.6.8-rc2 + the above mentioned patch.
> 
> How does it "reduce latency"?
> 
> It looks like a reasonable cleanup, but afaict it will result in the
> per-cache spinlock actually being held for longer periods, thus increasing
> latencies???
> 

While you've got irq's disabled, drain_array() (the function my patch removes)
acquires the cache spin_lock, then releases it.  Cache_reap then acquires
it again (with irq's having been off the entire time).  My testing has found
that simply acquiring the lock once while irq's are off results in fewer
excessively long latencies.

Results probably vary somewhat depending on the circumstance.
--
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:"aart@kvack.org"> aart@kvack.org </a>

Re: [PATCH] Locking optimization for cache_reap

[Aroop MP]

Hi,

I have a simple doubt. Please answer it. 
Why the error " Inappropriate ioctl for device While reading flags 
......................" is ocuring?. YOur replies will be greatly 
appreciated.

Regards,
Aroop

 ---------------------------------------------------
 "NO MATTER WHERE YOU ARE IN THE WORLD,IF YOU HAVE DECIDED TO DO  SOMETHING 
DEEP FROM YOUR HEART YOU CAN DO IT. IT HAS ALWAYS BEEN THE   THOUGHT THAT 
MATTERS... "
 ---------------------------------------------------
--
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:"aart@kvack.org"> aart@kvack.org </a>

lsattr: Inappropriate ioctl for device While reading flags!!!

[Aroop MP]

Hi,

Thanks for your quick reply. When i check lsattr of the file 
/usr/local/cpanel/3rdparty/etc/php.ini  i got the following error.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Server[~]# lsattr /usr/local/cpanel/3rdparty/etc/php.ini
lsattr: Inappropriate ioctl for device While reading flags on 
/usr/local/cpanel/3rdparty/etc/php.ini
Server[~]#
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Please get back to me if you need any more  info. regarding this.

Thank You.

On Tuesday 27 Jul 2004 7:33 am, you wrote:
> On Tue, Jul 27, 2004 at 07:29:44AM +0530, Aroop MP wrote:
> > I have a simple doubt. Please answer it.
> > Why the error " Inappropriate ioctl for device While reading flags
> > ......................" is ocuring?. YOur replies will be greatly
> > appreciated.
>
> I have not seen this.  Can you elaborate?

-- 

 Regards, 
 Aroop M.P
 ---------------------------------------------------
 "NO MATTER WHERE YOU ARE IN THE WORLD,IF YOU HAVE DECIDED TO DO  SOMETHING 
DEEP FROM YOUR HEART YOU CAN DO IT. IT HAS ALWAYS BEEN THE   THOUGHT THAT 
MATTERS... "
 ---------------------------------------------------
--
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:"aart@kvack.org"> aart@kvack.org </a>

Re: lsattr: Inappropriate ioctl for device While reading flags!!!

[Philippe Troin]

Aroop MP <aroop@poornam.com> writes:

> Hi,
> 
> Thanks for your quick reply. When i check lsattr of the file 
> /usr/local/cpanel/3rdparty/etc/php.ini  i got the following error.
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> Server[~]# lsattr /usr/local/cpanel/3rdparty/etc/php.ini
> lsattr: Inappropriate ioctl for device While reading flags on 
> /usr/local/cpanel/3rdparty/etc/php.ini
> Server[~]#
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Please get back to me if you need any more  info. regarding this.

Because /usr/local/cpanel/3rdparty/etc/php.ini is not on an ext[23]
filesystem? Remote mounting via NFS, or your favorite network fs does
not count as ext[23].

Phil.
--
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:"aart@kvack.org"> aart@kvack.org </a>

Re: [PATCH] Locking optimization for cache_reap

[Dimitri Sivanich]

On Mon, Jul 26, 2004 at 08:47:57PM -0500, Dimitri Sivanich wrote:
> 
> While you've got irq's disabled, drain_array() (the function my patch removes)
> acquires the cache spin_lock, then releases it.  Cache_reap then acquires
> it again (with irq's having been off the entire time).  My testing has found
> that simply acquiring the lock once while irq's are off results in fewer
> excessively long latencies.
> 
> Results probably vary somewhat depending on the circumstance.

Of course, I should add that all of this is from the perspective of the
cpu doing the cache_reap.  If others feel that this may add too much
latency to other paths, other solutions may be in order.
--
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:"aart@kvack.org"> aart@kvack.org </a>