Jarkko Sakkinen <jar...@kernel.org> wrote:
> + spin_lock_irqsave(&key_graveyard_lock, flags);
> + list_splice_init(&key_graveyard, &graveyard);
> + spin_unlock_irqrestore(&key_graveyard_lock, flags);
I would wrap this bit in a check to see if key_graveyard is empty so that we
can avoid disabling irqs and taking the lock if the graveyard is empty.
> + if (!refcount_inc_not_zero(&key->usage)) {
Sorry, but eww. You're going to wangle the refcount twice on every key on the
system every time the gc does a pass. Further, in some cases inc_not_zero is
not the fastest op in the world.
> + spin_lock_irqsave(&key_graveyard_lock, flags);
> + list_add_tail(&key->graveyard_link, &key_graveyard);
> + spin_unlock_irqrestore(&key_graveyard_lock, flags);
> schedule_work(&key_gc_work);
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);
David
