On 5/7/18 3:57 AM, Christoph Hellwig wrote:
>> -static inline struct rq_wait *get_rq_wait(struct rq_wb *rwb, bool is_kswapd)
>> +static inline struct rq_wait *get_rq_wait(struct rq_wb *rwb, bool is_trim,
>> + bool is_kswapd)
>> {
>> - return &rwb->rq_wait[is_kswapd];
>> + if (is_trim)
>> + return &rwb->rq_wait[WBT_REQ_DISCARD];
>> + else if (is_kswapd)
>> + return &rwb->rq_wait[WBT_REQ_KSWAPD];
>> + else
>> + return &rwb->rq_wait[WBT_REQ_BG];
>> }
>
> Wouldn't it be more useful to pass a enum wbt_flag here?
>
> Or just have a wbt_flag_to_wait_idx helper and do the array indexing
> in the callers?
It would be cleaner, but we don't have wbt_flag everywhere we need it.
Though I guess we could swap the masking in wbt_wait() and do it
before the __wbt_wait() call, and just use that. Since we only do
the indexing in that one spot, I don't think we should add a helper.
>
>> {
>> const int op = bio_op(bio);
>>
>> - /*
>> - * If not a WRITE, do nothing
>> - */
>> - if (op != REQ_OP_WRITE)
>> - return false;
>> + if (op == REQ_OP_WRITE) {
>> + /*
>> + * Don't throttle WRITE_ODIRECT
>> + */
>> + if ((bio->bi_opf & (REQ_SYNC | REQ_IDLE)) ==
>> + (REQ_SYNC | REQ_IDLE))
>> + return false;
>>
>> - /*
>> - * Don't throttle WRITE_ODIRECT
>> - */
>> - if ((bio->bi_opf & (REQ_SYNC | REQ_IDLE)) == (REQ_SYNC | REQ_IDLE))
>> - return false;
>> + return true;
>> + } else if (op == REQ_OP_DISCARD)
>> + return true;
>
> what about:
>
> switch (bio_op(bio)) {
> case REQ_OP_WRITE:
> /*
> * Don't throttle WRITE_ODIRECT
> */
> if ((bio->bi_opf & (REQ_SYNC | REQ_IDLE)) ==
> (REQ_SYNC | REQ_IDLE))
> return false;
> /*FALLTHROUGH*/
> case REQ_OP_DISCARD:
> return true;
> default:
> return false;
Sure, I can do that. I'll spin a v2.
--
Jens Axboe
