On 3/9/22 18:15, Damien Le Moal wrote: > On 3/10/22 11:02, Chaitanya Kulkarni wrote: >> On 3/9/22 17:57, Chaitanya Kulkarni wrote: >>> On 3/9/22 15:38, Damien Le Moal wrote: >>>> On 3/10/22 07:02, Chaitanya Kulkarni wrote: >>> >>> [..] >>> >>>>> @@ -2044,7 +2044,7 @@ static int null_add_dev(struct nullb_device *dev) >>>>> blk_queue_flag_clear(QUEUE_FLAG_ADD_RANDOM, nullb->q); >>>>> mutex_lock(&lock); >>>>> - nullb->index = ida_simple_get(&nullb_indexes, 0, 0, GFP_KERNEL); >>>>> + nullb->index = ida_alloc(&nullb_indexes, GFP_KERNEL); >>>> >>>> Do we need error check here ? Not entirely sure if ida_free() tolerates >>>> being passed a failed ida_alloc() nullb_indexes... A quick look at >>>> ida_free() does not show anything obvious, so it may be worth checking >>>> in detail. >>>> >>> >>> Good point, but original code doesn't have error checking, this patch >>> eventually ends up calling same function what original code was doing. >>> >>> Since this is just a replacement patch should we add a 2nd patch on the >>> top of this for error handling ? or you prefer to have it in the same >>> one ? >>> >>> -ck >>> >> >> Also nullb->index is defined as unsigned int [1] so in order to add >> error handling we need to change the type of variable, so I think it >> makes to make it a separate patch than removing deprecated API, lmk. > > One patch to add the missing error check and change the index type, with > cc stable for backport, and a second patch to switch to the new api on > top of the fix, without cc stable. No ? > yes, will send out V2 soon. -ck
