On 11/23/21 9:46 AM, Jens Axboe wrote: > 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. Actually may not be that trivial - if a thread is cloned with CLONE_IO, then it shares the io_context, and hence also the io priority. That will auto-propagate if one linked task changes it, and putting it in the task_struct would break that. So I don't think we can do that - which is a shame, as it would be a nice cleanup. -- Jens Axboe
