On 11/23/21 9:39 AM, Christoph Hellwig wrote: >> + /* create task io_context, if we don't have one already */ >> + if (unlikely(!current->io_context)) >> + create_task_io_context(current, GFP_ATOMIC, rq->q->node); > > Wouldn't it be nicer to have an interface that hides the check > and the somewhat pointless current argument? And name that with a > blk_ prefix? Yeah, we can do that. >> + >> + blk_mq_sched_assign_ioc(rq); > > But thinking about this a little more: > > struct io_context has two uses, one is the unsigned short ioprio, > and the other is the whole bfq mess. > > Can't we just split the ioprio out (we'll find a hole somewhere > in task_struct) and then just allocate the io_context in > blk_mq_sched_assign_ioc, which would also avoid the pointless > ioc_lookup_icq on a newly allocated io_context. I'd also really > expect blk_mq_sched_assign_ioc to be implemented in blk-ioc.c. It would be nice to decouple ioprio from the io_context, and just leave the io_context for BFQ essentially. I'll give it a shot. > While we're at it: bfq_bic_lookup is a really weird helper which > gets passed an unused bfqd argument. Unsurprising. -- Jens Axboe