From owner-freebsd-arch@FreeBSD.ORG Fri May 31 16:16:57 2013 Return-Path: Delivered-To: arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id F03258CE for ; Fri, 31 May 2013 16:16:57 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from bigwig.baldwin.cx (bigwig.baldwin.cx [IPv6:2001:470:1f11:75::1]) by mx1.freebsd.org (Postfix) with ESMTP id D1CE6E25 for ; Fri, 31 May 2013 16:16:57 +0000 (UTC) Received: from jhbbsd.localnet (unknown [209.249.190.124]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 5548AB915 for ; Fri, 31 May 2013 12:16:57 -0400 (EDT) To: arch@freebsd.org Subject: [PATCH] Allow atomic sets of non-overlapping CPU sets for a global cpuset From: John Baldwin Date: Fri, 31 May 2013 12:16:56 -0400 MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201305311216.56558.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Fri, 31 May 2013 12:16:57 -0400 (EDT) X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 31 May 2013 16:16:58 -0000 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