On 10/30/22 4:07 AM, Christoph Hellwig wrote:
> We have less than a handful of elevators, and if someone adds a duplicate
> one it simply will never be found but other be harmless.
That isn't quite parseable...
> diff --git a/block/elevator.c b/block/elevator.c
> index d26aa787e29f0..ef9af17293ffb 100644
> --- a/block/elevator.c
> +++ b/block/elevator.c
> @@ -545,13 +545,7 @@ int elv_register(struct elevator_type *e)
> return -ENOMEM;
> }
>
> - /* register, don't allow duplicate names */
> spin_lock(&elv_list_lock);
> - if (elevator_find(e->elevator_name, 0)) {
> - spin_unlock(&elv_list_lock);
> - kmem_cache_destroy(e->icq_cache);
> - return -EBUSY;
> - }
> list_add_tail(&e->list, &elv_list);
> spin_unlock(&elv_list_lock);
What's the idea behind this? Yes it'll be harmless and list ordering
will dictate which one will be found, leaving the other(s) dead, but why
not catch this upfront? I agree likelihood of this ever happening to be
tiny, but seems like a good idea to catch and return BUSY for this case.
--
Jens Axboe
