On 2023-02-16 16:08:10 [-0800], John Johansen wrote:I did look at local_locks and there was a reason I didn't use them. I
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -49,12 +49,19 @@ union aa_buffer {
char buffer[1];
};
+struct aa_local_cache {
+ unsigned int contention;
+ unsigned int hold;
+ struct list_head head;
+};
if you stick a local_lock_t into that struct, then you could replace
cache = get_cpu_ptr(&aa_local_buffers);
with
local_lock(&aa_local_buffers.lock);
cache = this_cpu_ptr(&aa_local_buffers);
You would get the preempt_disable() based locking for the per-CPU
variable (as with get_cpu_ptr()) and additionally some lockdep
validation which would warn if it is used outside of task context (IRQ).
I didn't parse completely the hold/contention logic but it seems to worksadly I messed up the reordering of this and the debug patch. This will be
;)
You check "cache->count >= 2" twice but I don't see an inc/ dec of it
nor is it part of aa_local_cache.
I can't parse how many items can end up on the local list if the globalSo this iteration, forces pushing back to global list if there are already
list is locked. My guess would be more than 2 due the ->hold parameter.
Do you have any numbers on the machine and performance it improved? It
sure will be a good selling point.