On 04/20/2017 02:33 PM, Stephen Bates wrote:
>
>> Nah, let's just leave it as-is then, even though I don't think it's the
>> prettiest thing I've ever seen.
>
> I did look at making the stats buckets in the request_queue struct
> based on dir and size. Something like:
>
> - struct blk_rq_stat poll_stat[2];
> + struct blk_rq_stat poll_stat[2][BLK_MQ_POLL_STATS_BKTS/2];
>
> This actually did clean up some in some places but because the
> callback still uses a linear array of buckets we do get this:
>
> - if (cb->stat[READ].nr_samples)
> - q->poll_stat[READ] = cb->stat[READ];
> - if (cb->stat[WRITE].nr_samples)
> - q->poll_stat[WRITE] = cb->stat[WRITE];
> + for (bucket = 0; bucket < BLK_MQ_POLL_STATS_BKTS; bucket++) {
> + if (cb->stat[bucket].nr_samples)
> + q->poll_stat[bucket%2][bucket/2] = cb->stat[bucket];
>
> I tend to agree with Omar that keeping the buckets in a linear array
> is overall cleaner and more generalized.
I agree, it's fine as-is. We should queue it up for 4.12.
> However right now I am stuck as I am seeing the kernel oops I reported
> before in testing of my latest patchset [1]. I will try and find some
> time to bisect that but it looks like it was introduced when the
> support for mq schedulers was added (on or around bd166ef18).
Just replied to that one, let me know if the suggestion works.
--
Jens Axboe
