Re: [PATCH v2 2/3] loop: Select I/O scheduler 'none' from inside add_disk()

[Ming Lei <ming.lei@redhat.com>]

On Wed, Aug 04, 2021 at 10:43:05AM -0700, Bart Van Assche wrote:
> On 8/3/21 7:39 PM, Ming Lei wrote:
> > On Tue, Aug 03, 2021 at 11:23:03AM -0700, Bart Van Assche wrote:
> > > We noticed that the user interface of Android devices becomes very slow
> > > under memory pressure. This is because Android uses the zram driver on top
> > > of the loop driver for swapping, because under memory pressure the swap
> > > code alternates reads and writes quickly, because mq-deadline is the
> > > default scheduler for loop devices and because mq-deadline delays writes by
> > 
> > Maybe we can bypass io scheduler always for request with REQ_SWAP, such as:
> > 
> > diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c
> > index 0f006cabfd91..d86709ac9d1f 100644
> > --- a/block/blk-mq-sched.c
> > +++ b/block/blk-mq-sched.c
> > @@ -420,7 +420,8 @@ static bool blk_mq_sched_bypass_insert(struct blk_mq_hw_ctx *hctx,
> >   	 * passthrough request is added to scheduler queue, there isn't any
> >   	 * chance to dispatch it given we prioritize requests in hctx->dispatch.
> >   	 */
> > -	if ((rq->rq_flags & RQF_FLUSH_SEQ) || blk_rq_is_passthrough(rq))
> > +	if ((rq->rq_flags & RQF_FLUSH_SEQ) || blk_rq_is_passthrough(rq) ||
> > +			blk_rq_is_swap(rq))
> >   		return true;
> >   	return false;
> > diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> > index d3afea47ade6..71aaa99614ab 100644
> > --- a/include/linux/blkdev.h
> > +++ b/include/linux/blkdev.h
> > @@ -251,6 +251,11 @@ static inline bool blk_rq_is_passthrough(struct request *rq)
> >   	return blk_op_is_passthrough(req_op(rq));
> >   }
> > +static inline bool blk_rq_is_swap(struct request *rq)
> > +{
> > +	return rq->cmd_flags & REQ_SWAP;
> > +}
> > +
> >   static inline unsigned short req_get_ioprio(struct request *req)
> >   {
> >   	return req->ioprio;
> 
> Hi Ming,
> 
> Thanks for having suggested an alternative. However, isn't the above a
> policy? Shouldn't a kernel provide mechanisms instead of policies?

REQ_SWAP means it is one IO written to swap device/file, so the system
is suffering memory pressure, then isn't it reasonable to bypass io
scheduler for all SWAP IOs?

The issue you described in comment log is neither loop specific, nor
mq-deadline, I think it isn't good to just workaround the issue for loop.

> 
> Additionally, the above patch does not address all Android loop driver use
> cases. Reading APEX files is regular I/O and hence REQ_SWAP is not set while
> reading from APEX files.

Is APEX file one swap file? If not, can you explain it a bit why you want to
switch to none when reading APEX file?


Thanks,
Ming