Re: [PATCH 08/15] block: pass a queue_limits argument to blk_alloc_queue

[Christoph Hellwig <hch@lst.de>]

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.