On 6/8/21 10:03 PM, Damien Le Moal wrote:
> On 2021/06/09 8:07, Bart Van Assche wrote:
>> struct deadline_data {
>> /*
>> * run time data
>> */
>>
>> /*
>> - * requests (deadline_rq s) are present on both sort_list and fifo_list
>> + * Requests are present on both sort_list[] and fifo_list[][]. The
>> + * first index of fifo_list[][] is the I/O priority class (DD_*_PRIO).
>> + * The second index is the data direction (rq_data_dir(rq)).
>> */
>> struct rb_root sort_list[DD_DIR_COUNT];
>> - struct list_head fifo_list[DD_DIR_COUNT];
>> + struct list_head fifo_list[DD_PRIO_COUNT][DD_DIR_COUNT];
>
> Would it make sense to pack these 2 into a sub structure ? e.g.:
>
> struct deadline_lists {
> struct rb_root sort_list;
> struct list_head fifo_list[DD_PRIO_COUNT];
> };
>
> struct deadline_data {
> ...
> /*
> * Requests are present on both sort_list[] and fifo_list[][]. The
> * first index of fifo_list[][] is the I/O priority class (DD_*_PRIO).
> * The second index is the data direction (rq_data_dir(rq)).
> */
> struct deadline_lists lists[DD_DIR_COUNT];
The deadline_fifo_request() function and several other functions
examine both directions so I think that grouping per direction would
complicate these functions. Grouping per I/O priority might help to make
these functions easier to read. Do you want me to look further into this?
>> + struct deadline_data *dd = q->elevator->elevator_data;
>> + const u8 ioprio_class = dd_rq_ioclass(next);
>> + const enum dd_prio prio = ioprio_class_to_prio[ioprio_class];
>> +
>> + if (next->elv.priv[0]) {
>> + dd_count(dd, merged, prio);
>> + } else {
>> + WARN_ON_ONCE(true);
>> + }
>
> No need for the curly brackets I think.
I can leave these out but you may want to know that leaving the curly
brackets out from this patch will make it necessary to introduce these
in the next patch in this series.
>> +/* Number of requests queued for a given priority level. */
>> +static u32 dd_queued(struct deadline_data *dd, enum dd_prio prio)
>> +{
>> + return dd_sum(dd, inserted, prio) - dd_sum(dd, completed, prio);
>
> This also includes requests that are being executed on the device. Is that OK ?
Yes, and that's on purpose. Please note that this function is only used
to export statistics to user space.
Thanks,
Bart.
