On Fri, Apr 11, 2025 at 11:37:25PM +0300, Jarkko Sakkinen wrote:
This is going to enable and disable interrupts twice and that can be expensive, depending on the arch. I wonder if it would be better to do:
local_irq_save(flags); spin_lock(&key_graveyard_lock); list_add_tail(&key->graveyard_link, &key_graveyard); spin_unlock(&key_graveyard_lock); schedule_work(&key_gc_work); local_irq_restore(flags);I like this but shouldn't this also comprehend the quota update before (just asking for completeness sake)?
"This brings me on to another though: Should key_serial_lock be a seqlock? And should the gc use RCU + read_seqlock() and insertion write_seqlock()?" https://lore.kernel.org/keyrings/797521.1743602083@warthog.procyon.org.uk/
I think that should be done too (because it made whole a lot of sense) as a separate patch. I'd just prefer move slowly and in baby steps for better quality, and keep that as a separate follow-up patch.
It makes obviously sense given rare writes.
BR, Jarkko