On 12/6/22 17:06, Paolo Valente wrote: > > >> Il giorno 21 nov 2022, alle ore 02:01, Damien Le Moal <damien.lemoal@xxxxxxxxxxxxxxxxxx> ha scritto: >> > > ... > >> >>> } >>> >>> static bool bfq_bio_merge(struct request_queue *q, struct bio *bio, >>> @@ -7144,6 +7159,8 @@ static int bfq_init_queue(struct request_queue *q, struct elevator_type *e) >>> { >>> struct bfq_data *bfqd; >>> struct elevator_queue *eq; >>> + unsigned int i; >>> + struct blk_independent_access_ranges *ia_ranges = q->disk->ia_ranges; >>> >>> eq = elevator_alloc(q, e); >>> if (!eq) >>> @@ -7187,10 +7204,31 @@ static int bfq_init_queue(struct request_queue *q, struct elevator_type *e) >>> bfqd->queue = q; >>> >>> /* >>> - * Multi-actuator support not complete yet, default to single >>> - * actuator for the moment. >>> + * If the disk supports multiple actuators, we copy the independent >>> + * access ranges from the request queue structure. >>> */ >>> - bfqd->num_actuators = 1; >>> + spin_lock_irq(&q->queue_lock); >>> + if (ia_ranges) { >>> + /* >>> + * Check if the disk ia_ranges size exceeds the current bfq >>> + * actuator limit. >>> + */ >>> + if (ia_ranges->nr_ia_ranges > BFQ_MAX_ACTUATORS) { >>> + pr_crit("nr_ia_ranges higher than act limit: iars=%d, max=%d.\n", >>> + ia_ranges->nr_ia_ranges, BFQ_MAX_ACTUATORS); >>> + pr_crit("Falling back to single actuator mode.\n"); >>> + bfqd->num_actuators = 0; >>> + } else { >>> + bfqd->num_actuators = ia_ranges->nr_ia_ranges; >>> + >>> + for (i = 0; i < bfqd->num_actuators; i++) >>> + bfqd->ia_ranges[i] = ia_ranges->ia_range[i]; >>> + } >>> + } else { >>> + bfqd->num_actuators = 0; >> >> That is very weird. The default should be 1 actuator. >> ia_ranges->nr_ia_ranges is 0 when the disk does not provide any range >> information, meaning it is a regular disk with a single actuator. > > Actually, IIUC this assignment to 0 seems to be done exactly when you > say that it should be done, i.e., when the disk does not provide any > range information (ia_ranges is NULL). Am I missing something else? No ranges reported means no extra actuators, so a single actuator an single LBA range for the entire device. In that case, bfq should process all IOs using bfqd->ia_ranges[0]. The get range function will always return that range. That makes the code clean and avoids different path for nr_ranges == 1 and nr_ranges > 1. No ? > > Once again, all other suggestions applied. I'm about to submit a V7. > > Thanks, > Paolo > -- Damien Le Moal Western Digital Research