Re: [PATCH] blk-mq: implement queue quiesce via percpu_ref for BLK_MQ_F_BLOCKING

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index bb5636cc17b9..5fa8bc1bb7a8 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -572,6 +572,10 @@ struct request_queue {
   	struct list_head	tag_set_list;
   	struct bio_set		bio_split;
+	/* only used for BLK_MQ_F_BLOCKING */
+	struct percpu_ref	dispatch_counter;

Can this be moved to be next to the q_usage_counter? they
will be taken together for blocking drivers...

I don't think it is a good idea, at least hctx->srcu is put at the end
of hctx, do you want to move it at beginning of hctx?

I'd think it'd be an improvement, yes.

.q_usage_counter should have been put in the 1st cacheline of
request queue. If it is moved to the 1st cacheline of request queue,
we shouldn't put 'dispatch_counter' there, because it may hurt other
non-blocking drivers.

q_usage_counter currently there, and the two will always be taken
together, and there are several other stuff that we can remove from
that cacheline without hurting performance for anything.

And when q_usage_counter is moved to the first cacheline, then I'd
expect that the dispatch_counter also moves to the front (maybe not
the first if it is on the expense of other hot members, but definitely
it should be treated as a hot member).

Anyways, I think that for now we should place them together.

Also maybe a better name is needed here since it's just
for blocking hctxs.

+	wait_queue_head_t	mq_quiesce_wq;
+
   	struct dentry		*debugfs_dir;
   #ifdef CONFIG_BLK_DEBUG_FS


What I think is needed here is at a minimum test quiesce/unquiesce loops
during I/O. code auditing is not enough, there may be driver assumptions
broken with this change (although I hope there shouldn't be).

We have elevator switch / updating nr_request stress test, and both relies
on quiesce/unquiesce, and I did run such test with this patch.

You have a blktest for this? If not, I strongly suggest that one is
added to validate the change also moving forward.



[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux