[PATCH 0/2] slightly change shrinker behaviour for very small object sets

[PATCH 0/2] slightly change shrinker behaviour for very small object sets

[Glauber Costa]

Hi,

I've recently noticed some glitches in the object shrinker mechanism when a
very small number of objects is used. Those situations are theoretically
possible, albeit unlikely. But although it may feel like it is purely
theoretical, they can become common in environments with many small containers
(cgroups) in a box.

Those patches came from some experimentation I am doing with targetted-shrinking
for kmem-limited memory cgroups (Dave Shrinnker is already aware of such work).
In such scenarios, one can set the available memory to very low limits, and it
becomes easy to see this.

Glauber Costa (2):
  super: fix calculation of shrinkable objects for small numbers
  vmscan: take at least one pass with shrinkers

 fs/super.c  | 2 +-
 mm/vmscan.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

-- 
1.7.11.7

[PATCH 1/2] super: fix calculation of shrinkable objects for small numbers

[Glauber Costa]

The sysctl knob sysctl_vfs_cache_pressure is used to determine which
percentage of the shrinkable objects in our cache we should actively try
to shrink.

It works great in situations in which we have many objects (at least
more than 100), because the aproximation errors will be negligible. But
if this is not the case, specially when total_objects < 100, we may end
up concluding that we have no objects at all (total / 100 = 0,  if total
< 100).

This is certainly not the biggest killer in the world, but may matter in
very low kernel memory situations.

Signed-off-by: Glauber Costa <glommer@parallels.com>
CC: Dave Chinner <david@fromorbit.com>
CC: "Theodore Ts'o" <tytso@mit.edu>
CC: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/super.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/super.c b/fs/super.c
index 12f1237..660552c 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -104,7 +104,7 @@ static int prune_super(struct shrinker *shrink, struct shrink_control *sc)
 				sb->s_nr_inodes_unused + fs_objects;
 	}
 
-	total_objects = (total_objects / 100) * sysctl_vfs_cache_pressure;
+	total_objects = mult_frac(total_objects, sysctl_vfs_cache_pressure, 100);
 	drop_super(sb);
 	return total_objects;
 }
-- 
1.7.11.7

--
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>

[PATCH 2/2] vmscan: take at least one pass with shrinkers

[Glauber Costa]

In very low free kernel memory situations, it may be the case that we
have less objects to free than our initial batch size. If this is the
case, it is better to shrink those, and open space for the new workload
then to keep them and fail the new allocations.

More specifically, this happens because we encode this in a loop with
the condition: "while (total_scan >= batch_size)". So if we are in such
a case, we'll not even enter the loop.

This patch modifies turns it into a do () while {} loop, that will
guarantee that we scan it at least once, while keeping the behaviour
exactly the same for the cases in which total_scan > batch_size.

Signed-off-by: Glauber Costa <glommer@parallels.com>
CC: Dave Chinner <david@fromorbit.com>
CC: "Theodore Ts'o" <tytso@mit.edu>
CC: Al Viro <viro@zeniv.linux.org.uk>
---
 mm/vmscan.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 7f30961..fcd1aa0 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -280,7 +280,7 @@ unsigned long shrink_slab(struct shrink_control *shrink,
 					nr_pages_scanned, lru_pages,
 					max_pass, delta, total_scan);
 
-		while (total_scan >= batch_size) {
+		do {
 			int nr_before;
 
 			nr_before = do_shrinker_shrink(shrinker, shrink, 0);
@@ -294,7 +294,7 @@ unsigned long shrink_slab(struct shrink_control *shrink,
 			total_scan -= batch_size;
 
 			cond_resched();
-		}
+		} while (total_scan >= batch_size);
 
 		/*
 		 * move the unused scan count back into the shrinker in a
-- 
1.7.11.7

--
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 1/2] super: fix calculation of shrinkable objects for small numbers

[Dave Chinner]

On Wed, Dec 19, 2012 at 12:40:17PM +0400, Glauber Costa wrote:
> The sysctl knob sysctl_vfs_cache_pressure is used to determine which
> percentage of the shrinkable objects in our cache we should actively try
> to shrink.
> 
> It works great in situations in which we have many objects (at least
> more than 100), because the aproximation errors will be negligible. But
> if this is not the case, specially when total_objects < 100, we may end
> up concluding that we have no objects at all (total / 100 = 0,  if total
> < 100).
> 
> This is certainly not the biggest killer in the world, but may matter in
> very low kernel memory situations.
> 
> Signed-off-by: Glauber Costa <glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
> CC: Dave Chinner <david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org>
> CC: "Theodore Ts'o" <tytso-3s7WtUTddSA@public.gmane.org>
> CC: Al Viro <viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
> ---
>  fs/super.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/super.c b/fs/super.c
> index 12f1237..660552c 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -104,7 +104,7 @@ static int prune_super(struct shrinker *shrink, struct shrink_control *sc)
>  				sb->s_nr_inodes_unused + fs_objects;
>  	}
>  
> -	total_objects = (total_objects / 100) * sysctl_vfs_cache_pressure;
> +	total_objects = mult_frac(total_objects, sysctl_vfs_cache_pressure, 100);
>  	drop_super(sb);
>  	return total_objects;

Hi Glauber,

sysctl_vfs_cache_pressure all over the place with exactly the same
calculation. Can you fix all of them in one pass?

Cheers,

Dave.
-- 
Dave Chinner
david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org

Re: [PATCH 2/2] vmscan: take at least one pass with shrinkers

[Dave Chinner]

On Wed, Dec 19, 2012 at 12:40:18PM +0400, Glauber Costa wrote:
> In very low free kernel memory situations, it may be the case that we
> have less objects to free than our initial batch size. If this is the
> case, it is better to shrink those, and open space for the new workload
> then to keep them and fail the new allocations.
> 
> More specifically, this happens because we encode this in a loop with
> the condition: "while (total_scan >= batch_size)". So if we are in such
> a case, we'll not even enter the loop.
> 
> This patch modifies turns it into a do () while {} loop, that will
> guarantee that we scan it at least once, while keeping the behaviour
> exactly the same for the cases in which total_scan > batch_size.

Looks good to me, seems to work just fine.

Reviewed-by: Dave Chinner <dchinner@redhat.com>

-- 
Dave Chinner
david@fromorbit.com

--
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 1/2] super: fix calculation of shrinkable objects for small numbers

[Glauber Costa]

On 12/20/2012 11:06 AM, Dave Chinner wrote:
> On Wed, Dec 19, 2012 at 12:40:17PM +0400, Glauber Costa wrote:
>> The sysctl knob sysctl_vfs_cache_pressure is used to determine which
>> percentage of the shrinkable objects in our cache we should actively try
>> to shrink.
>>
>> It works great in situations in which we have many objects (at least
>> more than 100), because the aproximation errors will be negligible. But
>> if this is not the case, specially when total_objects < 100, we may end
>> up concluding that we have no objects at all (total / 100 = 0,  if total
>> < 100).
>>
>> This is certainly not the biggest killer in the world, but may matter in
>> very low kernel memory situations.
>>
>> Signed-off-by: Glauber Costa <glommer@parallels.com>
>> CC: Dave Chinner <david@fromorbit.com>
>> CC: "Theodore Ts'o" <tytso@mit.edu>
>> CC: Al Viro <viro@zeniv.linux.org.uk>
>> ---
>>  fs/super.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/fs/super.c b/fs/super.c
>> index 12f1237..660552c 100644
>> --- a/fs/super.c
>> +++ b/fs/super.c
>> @@ -104,7 +104,7 @@ static int prune_super(struct shrinker *shrink, struct shrink_control *sc)
>>  				sb->s_nr_inodes_unused + fs_objects;
>>  	}
>>  
>> -	total_objects = (total_objects / 100) * sysctl_vfs_cache_pressure;
>> +	total_objects = mult_frac(total_objects, sysctl_vfs_cache_pressure, 100);
>>  	drop_super(sb);
>>  	return total_objects;
> 
> Hi Glauber,
> 
> sysctl_vfs_cache_pressure all over the place with exactly the same
> calculation. Can you fix all of them in one pass?
> 

affirmative, sir!