NUMA memchr_inv() in mm/vmstat.c:need_update()?

NUMA memchr_inv() in mm/vmstat.c:need_update()?

[Janne Huttunen]

Hi,

The commit 1d90ca897A changed the type of the vm_numa_stat_diff
from s8 into u16. It also changed the associated BUILD_BUG_ON()
in need_update(), but didn't touch the memchr_inv() call after
it. Is the memchr_inv() call still supposed to cover the whole
array or am I just misreading the code?

Re: NUMA memchr_inv() in mm/vmstat.c:need_update()?

[Michal Hocko]

On Fri 02-11-18 14:44:11, Janne Huttunen wrote:
> Hi,
> 
> The commit 1d90ca897 changed the type of the vm_numa_stat_diff
> from s8 into u16. It also changed the associated BUILD_BUG_ON()
> in need_update(), but didn't touch the memchr_inv() call after
> it. Is the memchr_inv() call still supposed to cover the whole
> array or am I just misreading the code?
 
I guess you are not missing anything. We need something like
diff --git a/mm/vmstat.c b/mm/vmstat.c
index 6038ce593ce3..c42f01fbe964 100644
--- a/mm/vmstat.c
+++ b/mm/vmstat.c
@@ -1829,10 +1829,10 @@ static bool need_update(int cpu)
 		 * The fast way of checking if there are any vmstat diffs.
 		 * This works because the diffs are byte sized items.
 		 */
-		if (memchr_inv(p->vm_stat_diff, 0, NR_VM_ZONE_STAT_ITEMS))
+		if (memchr_inv(p->vm_stat_diff, 0, sizeof(*p->vm_stat_diff) * NR_VM_ZONE_STAT_ITEMS))
 			return true;
 #ifdef CONFIG_NUMA
-		if (memchr_inv(p->vm_numa_stat_diff, 0, NR_VM_NUMA_STAT_ITEMS))
+		if (memchr_inv(p->vm_numa_stat_diff, 0, sizeof(p->vm_numa_stat_diff) * NR_VM_NUMA_STAT_ITEMS))
 			return true;
 #endif
 	}

sizeof(p->vm_numa_stat_diff) would be shorter but more fragile if we
ever decide to change the type to be a pointer rather than a fixed
arrasy.

Care to send a full patch?
-- 
Michal Hocko
SUSE Labs