On Fri, 2018-08-10 at 09:39 +0800, jianchao.wang wrote:
> On 08/10/2018 03:41 AM, Bart Van Assche wrote:
> > +/*
> > + * Whether or not blk_queue_enter() should proceed. RQF_PM requests are
> > always
> > + * allowed. RQF_DV requests are allowed if the PM_ONLY queue flag has not
> > been
> > + * set. Other requests are only allowed if neither PM_ONLY nor DV_ONLY has
> > been
> > + * set.
> > + */
> > +static inline bool blk_enter_allowed(struct request_queue *q,
> > + blk_mq_req_flags_t flags)
> > +{
> > + return flags & BLK_MQ_REQ_PM ||
> > + (!blk_queue_pm_only(q) &&
> > + (flags & BLK_MQ_REQ_DV || !blk_queue_dv_only(q)));
> > +}
>
> If a new state is indeed necessary, I think this kind of checking in hot path
> is inefficient.
> How about introduce a new state into request_queue, such as
> request_queue->gate_state.
> Set the PM_ONLY and DV_ONLY into this state, then we could just check
> request_queue->gate_state > 0
> before do further checking.
Hello Jianchao,
I agree with you that the hot path should be as efficient as possible. But I
don't think that the above code adds significantly more overhead to the hot
path. The requests for which we care about performance are the requests that
read and write data. BLK_MQ_REQ_PM is not set for any of these requests.
For the high performance case we care about the pm-only flag won't be set.
If that flag is not set the rest of the boolean expression does not have to
be evaluated (flags & BLK_MQ_REQ_DV || !blk_queue_dv_only(q)). So I think the
above change only adds one additional boolean test to the hot path. That
shouldn't have a measurable performance impact.
Bart.
