On 11/24/20 7:21 PM, Christoph Hellwig wrote:
This looks generally food:
Reviewed-by: Christoph Hellwig <hch@xxxxxx>
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.
Thanks. I will be more careful about this. I'm gonna set related
constraint in my vim, checkpatch etc. ;)
- 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;
Regards.
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.
Actually why I didn't choose 'BIO_SPLIT' at the very beginning is that,
currently only normal IO in mq routine gets marked when they gets split,
neither abnormal IO nor normal IO in dm/md routine, though this is
reasonable since currently only normal IO in mq routine is capable of
polling and gets stuck in this issue.
But BIO_SPLIT may be more clear? I could explain the above constraint in
the comment instead.
Thanks for reviewing.
--
Thanks,
Jeffle
