Re: [PATCH 09/24] xfs: don't allow log IO to be throttled

[Dave Chinner <david@fromorbit.com>]

On Thu, Aug 01, 2019 at 01:39:34PM +0000, Chris Mason wrote:
> On 31 Jul 2019, at 22:17, Dave Chinner wrote:
> 
> > From: Dave Chinner <dchinner@redhat.com>
> >
> > Running metadata intensive workloads, I've been seeing the AIL
> > pushing getting stuck on pinned buffers and triggering log forces.
> > The log force is taking a long time to run because the log IO is
> > getting throttled by wbt_wait() - the block layer writeback
> > throttle. It's being throttled because there is a huge amount of
> > metadata writeback going on which is filling the request queue.
> >
> > IOWs, we have a priority inversion problem here.
> >
> > Mark the log IO bios with REQ_IDLE so they don't get throttled
> > by the block layer writeback throttle. When we are forcing the CIL,
> > we are likely to need to to tens of log IOs, and they are issued as
> > fast as they can be build and IO completed. Hence REQ_IDLE is
> > appropriate - it's an indication that more IO will follow shortly.
> >
> > And because we also set REQ_SYNC, the writeback throttle will no
> > treat log IO the same way it treats direct IO writes - it will not
> > throttle them at all. Hence we solve the priority inversion problem
> > caused by the writeback throttle being unable to distinguish between
> > high priority log IO and background metadata writeback.
> >
>   [ cc Jens ]
> 
> We spent a lot of time getting rid of these inversions in io.latency 
> (and the new io.cost), where REQ_META just blows through the throttling 
> and goes into back charging instead.

Which simply reinforces the fact that that request type based
throttling is a fundamentally broken architecture.

> It feels awkward to have one set of prio inversion workarounds for io.* 
> and another for wbt.  Jens, should we make an explicit one that doesn't 
> rely on magic side effects, or just decide that metadata is meta enough 
> to break all the rules?

The problem isn't REQ_META blows throw the throttling, the problem
is that different REQ_META IOs have different priority.

IOWs, the problem here is that we are trying to infer priority from
the request type rather than an actual priority assigned by the
submitter. There is no way direct IO has higher priority in a
filesystem than log IO tagged with REQ_META as direct IO can require
log IO to make progress. Priority is a policy determined by the
submitter, not the mechanism doing the throttling.

Can we please move this all over to priorites based on
bio->b_ioprio? And then document how the range of priorities are
managed, such as:

(99 = highest prio to 0 = lowest)

swap out
swap in				>90
User hard RT max		89
User hard RT min		80
filesystem max			79
ionice max			60
background data writeback	40
ionice min			20
filesystem min			10
idle				0

So that we can appropriately prioritise different types of kernel
internal IO w.r.t user controlled IO priorities? This way we can
still tag the bios with the type of data they contain, but we
no longer use that to determine whether to throttle that IO or not -
throttling/scheduling should be done entirely on a priority basis.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com