Re: [PATCH v5] block: disable iopoll for split bio
From: Christoph Hellwig <hch@infradead.org>
Date: 2020-11-24 11:21:06
This looks generally food: Reviewed-by: Christoph Hellwig <hch@lst.de> Some cosmetic nitpicks below:
+ /* + * Bio splitting may cause subtle trouble such as hang + * when doing sync iopoll in direct IO routine. Given + * performance gain of iopoll for big IO can be trival, + * disable iopoll when split needed. + */
You can use up all 80 characters for comments, and doing so generally helps readability.
- return cookie; + return bio_flagged(bio, BIO_NONE_COOKIE) ? BLK_QC_T_NONE : cookie;
I'd write this a little more easily flowing as: if (bio_flagged(bio, BIO_NONE_COOKIE)) return BLK_QC_T_NONE; return cookie;
quoted hunk ↗ jump to hunk
queue_exit: blk_queue_exit(q); return BLK_QC_T_NONE;diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h index d9b69bbde5cc..938fd25d2c68 100644 --- a/include/linux/blk_types.h +++ b/include/linux/blk_types.h@@ -284,6 +284,7 @@ enum { * of this bio. */ BIO_CGROUP_ACCT, /* has been accounted to a cgroup */ BIO_TRACKED, /* set if bio goes through the rq_qos path */ + BIO_NONE_COOKIE, /* disable iopoll for split bio */
I'd rename this to BIO_SPLIT and update the comment. The rationale is BIO_SPLIT is what happened to the bio. The fact that blk_mq_submit_bio doesn't return the cookie is just one good use of such information.