lkml.org 
[lkml]   [2022]   [Dec]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH V6 4/8] block, bfq: turn bfqq_data into an array in bfq_io_cq
From
Date


> Il giorno 21 nov 2022, alle ore 01:39, Damien Le Moal <damien.lemoal@opensource.wdc.com> ha scritto:
...
>>
>> bfqq = bfq_split_bfqq(bic, bfqq);
>> split = true;
>> diff --git a/block/bfq-iosched.h b/block/bfq-iosched.h
>> index f2e8ab91951c..e27897d66a0f 100644
>> --- a/block/bfq-iosched.h
>> +++ b/block/bfq-iosched.h
>> @@ -416,7 +416,7 @@ struct bfq_queue {
>> struct bfq_iocq_bfqq_data {
>> /*
>> * Snapshot of the has_short_time flag before merging; taken
>> - * to remember its value while the queue is merged, so as to
>> + * to remember its values while the queue is merged, so as to
>> * be able to restore it in case of split.
>> */
>> bool saved_has_short_ttime;
>> @@ -430,7 +430,7 @@ struct bfq_iocq_bfqq_data {
>> u64 saved_tot_idle_time;
>>
>> /*
>> - * Same purpose as the previous fields for the value of the
>> + * Same purpose as the previous fields for the values of the
>> * field keeping the queue's belonging to a large burst
>> */
>> bool saved_in_large_burst;
>> @@ -493,8 +493,12 @@ struct bfq_io_cq {
>> uint64_t blkcg_serial_nr; /* the current blkcg serial */
>> #endif
>>
>> - /* persistent data for associated synchronous process queue */
>> - struct bfq_iocq_bfqq_data bfqq_data;
>> + /*
>> + * Persistent data for associated synchronous process queues
>> + * (one queue per actuator, see field bfqq above). In
>> + * particular, each of these queues may undergo a merge.
>> + */
>> + struct bfq_iocq_bfqq_data bfqq_data[BFQ_MAX_ACTUATORS];
>
> I wonder if packing this together with struct bfq_queue would be cleaner.
> That would avoid the 2 arrays you have in this struct. Something like this:
>
> struct bfq_queue_data {
> struct bfq_queue *bfqq[2];
> struct bfq_iocq_bfqq_data iocq_data;
> }
>
> struct bfq_io_cq {
> ...
> struct bfq_queue_data bfqqd[BFQ_MAX_ACTUATORS];
> ...
> }
>
> Thinking aloud here. That may actually make the code more complicated.

I see your point, but, yes, this change would entail one more
indirection when accessing queues from io contexts.

Apart from this, I have applied all of your other suggestions here.

Thanks,
Paolo

>
>>
>> unsigned int requests; /* Number of requests this process has in flight */
>> };
>
> --
> Damien Le Moal
> Western Digital Research

\
 
 \ /
  Last update: 2022-12-06 09:01    [W:0.069 / U:1.232 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site