On Tue, Dec 04, 2018 at 09:58:35AM +0100, Peter Zijlstra wrote: > On Mon, Dec 03, 2018 at 11:39:54PM -0800, Alison Schofield wrote: > > > +void vma_put_encrypt_ref(struct vm_area_struct *vma) > > +{ > > + if (vma_keyid(vma)) > > + if (refcount_dec_and_test(&encrypt_count[vma_keyid(vma)])) { > > + mktme_map_lock(); > > + mktme_map_free_keyid(vma_keyid(vma)); > > + mktme_map_unlock(); > > + } > > This violates CodingStyle Got it! Will fix this and the other instances where you noticed poorly nested if statements. > > + if (refcount_dec_and_test(&encrypt_count[keyid])) { > > + mktme_map_lock(); > > That smells like it wants to use refcount_dec_and_lock() instead. > > Also, if you write that like: > > if (!refcount_dec_and_lock(&encrypt_count[keyid], &lock)) > return; > > you loose an indent level. Looks good! I need to make sure it's OK to switch to a spinlock to use the *_lock functions.