Re: [PATCH] blk-mq: remove hybrid polling

[Jens Axboe <axboe@kernel.dk>]

On 3/20/23 11:52?AM, Keith Busch wrote:
> On Mon, Mar 20, 2023 at 11:16:40AM -0600, Jens Axboe wrote:
>>> diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
>>> index f1fce1c7fa44b..c6c231f3d0f10 100644
>>> --- a/block/blk-sysfs.c
>>> +++ b/block/blk-sysfs.c
>>> @@ -408,36 +408,7 @@ queue_rq_affinity_store(struct request_queue *q, const char *page, size_t count)
>>>  
>>>  static ssize_t queue_poll_delay_show(struct request_queue *q, char *page)
>>>  {
>>> -	int val;
>>> -
>>> -	if (q->poll_nsec == BLK_MQ_POLL_CLASSIC)
>>> -		val = BLK_MQ_POLL_CLASSIC;
>>> -	else
>>> -		val = q->poll_nsec / 1000;
>>> -
>>> -	return sprintf(page, "%d\n", val);
>>> -}
>>> -
>>> -static ssize_t queue_poll_delay_store(struct request_queue *q, const char *page,
>>> -				size_t count)
>>> -{
>>> -	int err, val;
>>> -
>>> -	if (!q->mq_ops || !q->mq_ops->poll)
>>> -		return -EINVAL;
>>> -
>>> -	err = kstrtoint(page, 10, &val);
>>> -	if (err < 0)
>>> -		return err;
>>> -
>>> -	if (val == BLK_MQ_POLL_CLASSIC)
>>> -		q->poll_nsec = BLK_MQ_POLL_CLASSIC;
>>> -	else if (val >= 0)
>>> -		q->poll_nsec = val * 1000;
>>> -	else
>>> -		return -EINVAL;
>>> -
>>> -	return count;
>>> +	return sprintf(page, "%d\n", -1);
>>>  }
>>
>> Do we want to retain the _store setting here to avoid breaking anything?
> 
> I was thinking users would want to know the kernel isn't going to
> honor the requested value. Errors can already happen if you're using a
> stacked device, so I assmued removing '_store' wouldn't break anyone
> using this interface.
> 
> But I can see it both ways though, so whichever you prefer. At the
> very least, though, I need to update Documentation's sysfs-block, so
> I'll do that in the v2.

Users knowing == things breaking. Because it isn't a person looking at
that thing, it's some script or application. So I do think it's better
to just pretend we did something, and just not do anything. Because it
won't change anything in terms of the application working. If you did
you use hybrid polling, it'll still work fine with classic.

Arguably not a high risk thing, but I'd prefer decoupling the two
changes and then we can yank the store method at some later point in
time.

-- 
Jens Axboe