linux-kernel - Re: [RFC/PATCH 0/4] CPUSET driven CPU isolation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20080228113728.7b8e5811.pj@sgi.com>
Date:	Thu, 28 Feb 2008 11:37:28 -0600
From:	Paul Jackson <pj@....com>
To:	David Rientjes <rientjes@...gle.com>
Cc:	mingo@...e.hu, a.p.zijlstra@...llo.nl, tglx@...utronix.de,
	oleg@...sign.ru, rostedt@...dmis.org, maxk@...lcomm.com,
	linux-kernel@...r.kernel.org
Subject: Re: [RFC/PATCH 0/4] CPUSET driven CPU isolation

I don't have strong opinions either way on this patch; it adds an error
check that makes sense.  I haven't seen much problem not having this check,
nor do I know of any code that depends on doing what this check prohibits.

Except for three details:

 1) +	if (unlikely((p->flags & PF_CPU_BOUND) && p != current &&
    +	    	     !cpus_equal(p->cpus_allowed, new_mask))) {
    +		ret = -EINVAL;

    The check for equal cpus allowed seems too strong.  Shouldn't you be
    checking that all of task p's cpus_allowed would still be allowed in
    the new mask:

    +	if (unlikely((p->flags & PF_CPU_BOUND) && p != current &&
    +	    	     !cpus_subset(p->cpus_allowed, new_mask))) {
    +		ret = -EINVAL;

 2) Doesn't this leave out a check for the flip side -- shrinking
    the cpus allowed by a cpuset so that it no longer contains those
    required by any PF_CPU_BOUND tasks in that cpuset?  I'm not sure
    if this second check is a good idea or not.

 3) Could we call this PF_CPU_PINNED instead?  I tend to use "cpu
    bound" to refer to tasks that consume alot of CPU cycles (which
    these pinned tasks rarely do), and "pinned" to refer to what is
    done to confine a task to a particular subset of all possible CPUs.
    It looks to me like some code in kernel/sched.c already uses the
    word pinned in this same way, so PF_CPU_PINNED would be more
    consistent terminology.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@....com> 1.940.382.4214
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/