On Thu, Jan 25, 2024 at 09:45:20AM +0000, John Garry wrote:
>> +struct request_queue *blk_alloc_queue(struct queue_limits *lim, int node_id)
>> {
>> struct request_queue *q;
>> + int error;
>> q = kmem_cache_alloc_node(blk_requestq_cachep, GFP_KERNEL |
>> __GFP_ZERO,
>> node_id);
>> @@ -404,13 +405,26 @@ struct request_queue *blk_alloc_queue(int node_id)
>
> Is there actually an issue in that blk_alloc_queue() can return NULL, and
> we should be checking IS_ERR_OR_NULL() in the callers?
>
> I don't think that IS_ERR() picks up on NULL pointers, right?
>
> Or make this change:
Yes, that's the right thing to do, I'll add it.
> nit: This is only ever going to return -EINVAL or 0 by its very nature,
> right? I suppose that it could return a bool and we do the conversion to
> EINVAL here. It's a personal taste thing, I suppose.
I actually had that during most of the development, but then the callers
had to convert it. Either way works, but this seemed a bit cleaner.
>> + if (error)
>> + goto fail_q;
>> + q->limits = *lim;
>
> nit: It might be neater to do this in blk_validate_limits()
The limits assigment? I'd really like to keep blk_validate_limits limited
to only look at the passed in queue_limits and never look at a live
object.
