slab: fix regression in touched logic

Hi,

This hasn't actually shown up in any real workloads, but if my following logic is correct then it should be a good fix. Comments?

Thanks,
Nick
--

When factoring common code into transfer_objects, the 'touched' logic got a bit broken. When refilling from the shared array (taking objects from the shared array), we are making use of the shared array so it should be marked as touched.

Subsequently pulling an element from the cpu array and allocating it should also touch the cpu array, but that is taken care of after the alloc_done label. (So yes, the cpu array was getting touched = 1 twice).

So revert this logic to how it worked in earlier kernels.

This also affects the behaviour in __drain_alien_cache, which would previously 'touch' the shared array and now does not. I think it is more logical not to touch there, because we are pushing objects into the shared array rather than pulling them off. So there is no good reason to postpone reaping them -- if the shared array is getting utilized, then it will get 'touched' in the alloc path (where this patch now restores the touch).

Signed-off-by Nick Piggin <[email protected]>

---
Index: linux-2.6/mm/slab.c
===================================================================

mm/slab.c +0 -1

@@ -935,7 +935,6 @@ static int transfer_objects(struct array
 
 	from->avail -= nr;
 	to->avail += nr;
-	to->touched = 1;
 	return nr;
 }
 

@@ -2963,8 +2962,10 @@ retry:
 	spin_lock(&l3->list_lock);

Quoted 29 lines · Nick Piggin

Hi,

This hasn't actually shown up in any real workloads, but if my following logic is correct then it should be a good fix. Comments?

Thanks,
Nick
--

When factoring common code into transfer_objects, the 'touched' logic got a bit broken. When refilling from the shared array (taking objects from the shared array), we are making use of the shared array so it should be marked as touched.

Subsequently pulling an element from the cpu array and allocating it should also touch the cpu array, but that is taken care of after the alloc_done label. (So yes, the cpu array was getting touched = 1 twice).

So revert this logic to how it worked in earlier kernels.

This also affects the behaviour in __drain_alien_cache, which would previously 'touch' the shared array and now does not. I think it is more logical not to touch there, because we are pushing objects into the shared array rather than pulling them off. So there is no good reason to postpone reaping them -- if the shared array is getting utilized, then it will get 'touched' in the alloc path (where this patch now restores the touch).

Quoted 1 line

Signed-off-by Nick Piggin <[email protected]>

Makes sense but the rework doesn't ring a bell for me and I didn't check the git logs yet. Christoph, comments?

Quoted 14 lines

---
Index: linux-2.6/mm/slab.c
===================================================================

mm/slab.c +0 -1

@@ -935,7 +935,6 @@ static int transfer_objects(struct array
 	from->avail -= nr;
 	to->avail += nr;
-	to->touched = 1;
 	return nr;
 }

@@ -2963,8 +2962,10 @@ retry:
 	spin_lock(&l3->list_lock);

mm/slab.c retry: · line 2971

Quoted 8 lines

mm/slab.c +3 -1

 	/* See if we can refill from the shared array */
-	if (l3->shared && transfer_objects(ac, l3->shared, batchcount))
+	if (l3->shared && transfer_objects(ac, l3->shared, batchcount)) {
+		l3->shared->touched = 1;
 		goto alloc_done;
+	}
 	while (batchcount > 0) {
 		struct list_head *entry;

Looks good to me.

Acked-by Christoph Lameter <[email protected]>