Re: [PATCH V4 2/9] nvme: add sysfs attribute to change admin timeout per nvme controller

["Maurizio Lombardi" <mlombard@arkamax.eu>]

On Mon May 11, 2026 at 10:07 AM CEST, Christoph Hellwig wrote:
> On Fri, May 08, 2026 at 03:33:28PM +0200, Maurizio Lombardi wrote:
>> Currently, there is no method to adjust the timeout values
>> on a per controller basis with nvme admin queues.
>> Add an admin_timeout attribute to nvme so that different
>> nvme controllers which may have different timeout
>> requirements can have custom admin timeouts set.
>
> Please use up all 73 characters for commit messages.
>
>>  
>> +static ssize_t nvme_admin_timeout_show(struct device *dev,
>> +			struct device_attribute *attr, char *buf)
>
> The indent for prototype continuations should use two, not three tabs.
>
>> +	 * Wait until the controller reaches the LIVE state
>> +	 * to be sure that admin_q and fabrics_q are
>> +	 * properly initialized.
>
> Please use up all 80 characters for comments.

Ok.

>
>> +	 */
>> +	if (!test_bit(NVME_CTRL_STARTED_ONCE, &ctrl->flags))
>> +		return -EBUSY;
>> +
>> +	err = kstrtou32(buf, 10, &timeout);
>> +	if (err || !timeout)
>> +		return -EINVAL;
>> +
>> +	ctrl->admin_timeout = msecs_to_jiffies(timeout);
>> +
>> +	blk_queue_rq_timeout(ctrl->admin_q, ctrl->admin_timeout);
>> +	if (ctrl->fabrics_q)
>> +		blk_queue_rq_timeout(ctrl->fabrics_q, ctrl->admin_timeout);
>
> Do we really want to apply the the admin timeout for the fabrics queue?
> If so can you document here why we do that?

Both admin_q and fabrics_q are initialized to share the same
NVME_ADMIN_TIMEOUT value, therefore keeping them in sync maintains the
consistency.

If we didn't apply the timeout to fabrics_q, it would end up
operating under a different timeout than the standard admin_q, is there
any reason why we would want that?

Maurizio