Date: Fri, 31 May 2013 12:16:56 -0400 From: John Baldwin <jhb@freebsd.org> To: arch@freebsd.org Subject: [PATCH] Allow atomic sets of non-overlapping CPU sets for a global cpuset Message-ID: <201305311216.56558.jhb@freebsd.org>
next in thread | raw e-mail | index | archive | help
So there's an oddity with cpuset I've run into recently at work. Suppose I have created a new cpuset and want to change the set of CPUs for that set (say from a mask of just CPU 1 to a mask of just CPU 2). I can't do that atomically. I have to first set the mask to contain both the old set (CPU 1) and the new set (CPU 2) and then change it a second time to only contain the new set (CPU 2). The reason is that cpuset_modify() runs cpuset_testupdate() on the set it is about to modify, so when I try to change it in a single operation the new mask doesn't overlap with the old mask and it fails with EDEADLK. % cpuset -c -l 1 /bin/sh $ cpuset -gi pid -1 cpuset id: 2 $ cpuset -g pid -1 mask: 1 $ cpuset -l 2 -s 2 cpuset: setaffinity: Resource deadlock avoided I think that the correct logic here is that we should only check descendants of the set we are changing, but not the set we are about to change. The patch does this and allows my test case above to work: Index: kern_cpuset.c =================================================================== --- kern_cpuset.c (revision 251132) +++ kern_cpuset.c (working copy) @@ -303,7 +303,7 @@ cpuset_create(struct cpuset **setp, struct cpuset * empty as well as RDONLY flags. */ static int -cpuset_testupdate(struct cpuset *set, cpuset_t *mask) +cpuset_testupdate(struct cpuset *set, cpuset_t *mask, int check_mask) { struct cpuset *nset; cpuset_t newmask; @@ -312,13 +312,16 @@ static int mtx_assert(&cpuset_lock, MA_OWNED); if (set->cs_flags & CPU_SET_RDONLY) return (EPERM); - if (!CPU_OVERLAP(&set->cs_mask, mask)) - return (EDEADLK); - CPU_COPY(&set->cs_mask, &newmask); - CPU_AND(&newmask, mask); + if (check_mask) { + if (!CPU_OVERLAP(&set->cs_mask, mask)) + return (EDEADLK); + CPU_COPY(&set->cs_mask, &newmask); + CPU_AND(&newmask, mask); + } else + CPU_COPY(mask, &newmask); error = 0; LIST_FOREACH(nset, &set->cs_children, cs_siblings) - if ((error = cpuset_testupdate(nset, &newmask)) != 0) + if ((error = cpuset_testupdate(nset, &newmask, 1)) != 0) break; return (error); } @@ -370,11 +373,11 @@ cpuset_modify(struct cpuset *set, cpuset_t *mask) if (root && !CPU_SUBSET(&root->cs_mask, mask)) return (EINVAL); mtx_lock_spin(&cpuset_lock); - error = cpuset_testupdate(set, mask); + error = cpuset_testupdate(set, mask, 0); if (error) goto out; + CPU_COPY(mask, &set->cs_mask); cpuset_update(set, mask); - CPU_COPY(mask, &set->cs_mask); out: mtx_unlock_spin(&cpuset_lock); -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201305311216.56558.jhb>