lkml.org 
[lkml]   [2013]   [Aug]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH] cpuset: mm: Reduce large amounts of memory barrier related damage v3
On Fri, Aug 23, 2013 at 03:03:32PM +0200, Peter Zijlstra wrote:
> So I think this patch is broken (still). Suppose we have an
> INTERLEAVE mempol like 0x3 and change it to 0xc.
>
> Original: 0x3
> Rebind Step 1: 0xf /* set bits */
> Rebind Step 2: 0xc /* clear bits */
>
> Now look at what can happen with offset_il_node() when its ran
> concurrently with step 2:
>
> nnodes = nodes_weight(pol->v.nodes); /* observes 0xf and returns 4 */
>
> /* now we clear the actual bits */
>
> target = (unsigned int)off % nnodes; /* assume target >= 2 */
> c = 0;
> do {
> nid = next_node(nid, pol->v.nodes);
> c++;
> } while (c <= target);
>
> /* here nid := MAX_NUMNODES */
>
>
> This nid is then blindly inserted into node_zonelist() which does an
> NODE_DATA() array access out of bounds and off we go.
>
> This would suggest we put the whole seqcount thing inside
> offset_il_node().

Oh bloody grrr. Its not directly related at all, the patch in question
fixes a cpuset task_struct::mems_allowed problem while the above is a
mempolicy issue and of course the cpuset and mempolicy code are
completely bloody different :/

So I guess the quick and ugly solution is something like the below.

--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -1762,19 +1762,21 @@ unsigned slab_node(void)
static unsigned offset_il_node(struct mempolicy *pol,
struct vm_area_struct *vma, unsigned long off)
{
- unsigned nnodes = nodes_weight(pol->v.nodes);
- unsigned target;
- int c;
- int nid = -1;
+ unsigned nnodes, target;
+ int c, nid;

+again:
+ nnodes = nodes_weight(pol->v.nodes);
if (!nnodes)
return numa_node_id();
+
target = (unsigned int)off % nnodes;
- c = 0;
- do {
+ for (c = 0, nid = -1; c <= target; c++)
nid = next_node(nid, pol->v.nodes);
- c++;
- } while (c <= target);
+
+ if (unlikely((unsigned)nid >= MAX_NUMNODES))
+ goto again;
+
return nid;
}


\
 
 \ /
  Last update: 2013-08-23 20:41    [W:0.307 / U:1.340 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site