On Thu, Aug 20, 2020 at 4:19 PM Stephen Smalley <stephen.smalley.work@xxxxxxxxx> wrote: > In the previous change to convert the policy rwlock to RCU, > the update code was using rcu_dereference_check(..., 1) with > a comment to explain why it was safe without taking rcu_read_lock() > since the mutex used to provide exclusion was taken at a higher > level in selinuxfs. This change passes the mutex down to the > necessary functions and replaces rcu_dereference_check(..., 1) > with rcu_dereference_protected(..., lockdep_is_held(mutex)) so > that lockdep checking is correctly applied and the dependency > is made explicit in the code rather than relying on comments. > > Signed-off-by: Stephen Smalley <stephen.smalley.work@xxxxxxxxx> > --- > This is relative to the convert policy read-write lock to RCU patch, > https://patchwork.kernel.org/patch/11724997/. > > security/selinux/include/conditional.h | 3 +- > security/selinux/include/security.h | 6 ++-- > security/selinux/selinuxfs.c | 12 ++++--- > security/selinux/ss/services.c | 45 ++++++++------------------ > 4 files changed, 26 insertions(+), 40 deletions(-) Thanks for trying this out! I indeed like it more this way. The only thing I'd suggest is to perhaps name the mutex argument a little more descriptively, e.g. "check_mutex" or "rcu_mutex". I understand it'll make it harder to wrap some of the long lines, but I tend to think it's worth it in this case. Speaking about wrapping lines... I noticed only now that in this and earlier patches you align wrapped argument lists only by tabs (without extra spaces to align to the first argument). I'm not sure what is the preferred kernel style in this case, but I personally find the finely aligned argument lists much nicer to read (and I have always been aligning them like this in my patches). Obviously, I can't enforce my preferred style here, but I thought I'd raise this, since I had the impression we were trying to follow this style previously for new code (could be just confirmation bias on my part, though) and it might not have been your intention to change it (changed editor/settings?). -- Ondrej Mosnacek Software Engineer, Platform Security - SELinux kernel Red Hat, Inc.
