LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Stephen Smalley <sds@tycho.nsa.gov>
To: Peter Enderborg <peter.enderborg@sony.com>,
	Paul Moore <paul@paul-moore.com>,
	Eric Paris <eparis@parisplace.org>,
	James Morris <james.l.morris@oracle.com>,
	Daniel Jurgens <danielj@mellanox.com>,
	Doug Ledford <dledford@redhat.com>,
	selinux@tycho.nsa.gov, linux-security-module@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	"Serge E . Hallyn" <serge@hallyn.com>,
	"Paul E . McKenney" <paulmck@linux.vnet.ibm.com>
Subject: Re: [PATCH V3 0/5] selinux:Significant reduce of preempt_disable holds
Date: Wed, 30 May 2018 16:34:49 -0400	[thread overview]
Message-ID: <8bbb095e-31c3-0062-d17c-662e4832cc17@tycho.nsa.gov> (raw)
In-Reply-To: <20180530141104.28569-1-peter.enderborg@sony.com>

On 05/30/2018 10:10 AM, Peter Enderborg wrote:
> Holding the preempt_disable is very bad for low latency tasks
> such as audio and therefore we need to break out the rule-set dependent
> part from this disable. By using a RCU instead of rwlock we
> have an efficient locking and less preemption interference.
> 
> Selinux uses a lot of read_locks. This patch replaces the rwlock
> with RCU that does not hold preempt_disable.
> 
> Intel Xeon W3520 2.67 Ghz running FC27 with 4.15.0-rc9git (+measurement)
> I get preempt_disable of about 1.2ms in security_compute_av().
> With the patch I get 960us as the longest security_compute_av()
> without preempt disabeld. There are very much noise in the measurement
> but it is not likely a degrade.
> 
> And the preempt_disable times is also very dependent on the selinux
> rule-set.
> 
> In security_get_user_sids() we have two nested for-loops and the
> inner part calls sittab_context_to_sid() that calls
> sidtab_search_context() that has a for loop() over a while() where
> the loops is dependent on the rules.
> 
> On the test system the average lookup time is 60us and does
> not change with the introduced RCU usage.
> 
> The boolean change becomes a lot more heavy with this patch,
> but it is a very rare usage in compare with read only operations.
> The lock held during a policydb_copy is about 1ms on a XEON.

This has a very substantial performance impact on setsebool, e.g. time setsebool httpd_can_sendmail=1.
That's because you are doing a full vmalloc();policydb_write();policydb_read();vfree() sequence on it.
In comparison, KaiGai's old attempt to replace the policy rwlock with RCU only duplicated the conditional policydb state (via a cond_policydb_dup) that he introduced.  Is there a reason you couldn't use that approach?

> 
> To use RCU the structure of policydb has to be accesses through a pointer.
> We need 5 patches to get there.
>  
> [PATCH V3 1/5 selinux-next] selinux: Make allocation atomic in policydb objects functions.
> This patch change the allocation for policydb objects. They are in its own patch
> to make the complicated part easier to read.
> 
> [PATCH V3 2/5 selinux-next] selinux: Introduce selinux_ruleset struct
> This makes the access for the rule evaluation going though a single pointer.
> 
> [PATCH V3 3/5 selinux-next] selinux: sidtab_clone switch to use rwlock.
> We need to make sidtabs copys so this patch change the locks to a rwlock
> and create a copy function.
> 
> [PATCH V3 4/5 selinux-next] selinux: seqno separation
> This patch adds separation of the read and write and uses
> the pointer to switch rule set. It uses seqno for error handling
> since there are a possibility to have multiple access.
> 
> [PATCH V3 5/5 selinux-next] selinux: Switch to rcu read locks for avc_compute
> All the preparation is done so this patch do the change of locks to rcu.
> 
> History:
> V1 rwsem
> V2 did not handle all policydb objects, solved with the policydb_copy
>    did not handle sidtab for booleans, I think this one does however
>    shutdown is not used but not removed. 
> 
> 


  parent reply	other threads:[~2018-05-30 20:33 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-30 14:10 Peter Enderborg
2018-05-30 14:11 ` [PATCH V3 1/5 selinux-next] selinux: Make allocation atomic in policydb objects functions Peter Enderborg
2018-05-30 14:11 ` [PATCH V3 2/5 selinux-next] selinux: Introduce selinux_ruleset struct Peter Enderborg
2018-05-30 21:15   ` J Freyensee
2018-06-01 13:48   ` kbuild test robot
2018-06-01 13:56   ` kbuild test robot
2018-05-30 14:11 ` [PATCH V3 3/5 selinux-next] selinux: sidtab_clone switch to use rwlock Peter Enderborg
2018-05-30 21:22   ` J Freyensee
2018-05-31  5:35     ` peter enderborg
2018-05-30 14:11 ` [PATCH V3 4/5 selinux-next] selinux: seqno separation Peter Enderborg
2018-05-30 14:11 ` [PATCH V3 5/5 selinux-next] selinux: Switch to rcu read locks for avc_compute Peter Enderborg
2018-05-30 20:34 ` Stephen Smalley [this message]
2018-05-31  9:04   ` [PATCH V3 0/5] selinux:Significant reduce of preempt_disable holds peter enderborg
2018-05-31 12:42     ` Stephen Smalley
2018-05-31 14:12       ` peter enderborg
2018-05-31 14:21         ` Stephen Smalley
2018-05-31 16:40           ` Stephen Smalley
2018-06-01 11:18       ` peter enderborg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8bbb095e-31c3-0062-d17c-662e4832cc17@tycho.nsa.gov \
    --to=sds@tycho.nsa.gov \
    --cc=danielj@mellanox.com \
    --cc=dledford@redhat.com \
    --cc=eparis@parisplace.org \
    --cc=james.l.morris@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=paul@paul-moore.com \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=peter.enderborg@sony.com \
    --cc=selinux@tycho.nsa.gov \
    --cc=serge@hallyn.com \
    --subject='Re: [PATCH V3 0/5] selinux:Significant reduce of preempt_disable holds' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).