Messages in this thread | | | Subject | Re: [RFC PATCH v2 2/2] blk-mq: Lockout tagset iter when freeing rqs | From | John Garry <> | Date | Fri, 18 Dec 2020 09:30:55 +0000 |
| |
On 18/12/2020 01:55, Bart Van Assche wrote: > On 12/17/20 3:07 AM, John Garry wrote: >> diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c >> index a6df2d5df88a..853ed5b889aa 100644 >> --- a/block/blk-mq-tag.c >> +++ b/block/blk-mq-tag.c >> @@ -358,10 +358,19 @@ void blk_mq_tagset_busy_iter(struct blk_mq_tag_set *tagset, >> { >> int i; >> >> for (i = 0; i < tagset->nr_hw_queues; i++) { >> - if (tagset->tags && tagset->tags[i]) >> - __blk_mq_all_tag_iter(tagset->tags[i], fn, priv, >> + if (tagset->tags && tagset->tags[i]) { >> + struct blk_mq_tags *tags = tagset->tags[i]; >> + >> + if (!atomic_inc_not_zero(&tags->iter_usage_counter)) >> + continue; >> + >> + __blk_mq_all_tag_iter(tags, fn, priv, >> BT_TAG_ITER_STARTED); >> + >> + atomic_dec(&tags->iter_usage_counter); >> + } >> } >> } > > Atomic operations are (a) more expensive than rcu_read_lock() / > rcu_read_lock() and (b) do not provide the same guarantees. > rcu_read_lock() has acquire semantics and rcu_read_unlock() has > release semantics. Regular atomic operations do not have these > semantics which means that the CPU is allowed to reorder certain > regular loads and stores against atomic operations. Additionally, > atomic operations are more expensive than the corresponding RCU > primitives. In other words, I would be much happier if this patch > series would use RCU instead of atomics. >
Hi Bart,
In terms of solving the problem with RCU, can you provide more details on how it would actually work?
I saw that you mentioned kfree_rcu() at the following, so guess it's related: https://lore.kernel.org/linux-block/5c3ac5af-ed81-11e4-fee3-f92175f14daf@acm.org/T/#m830071bca03af31516800c14f8cccbe63661c5db
In terms of expense of atomic operations, we're just adding these operations around the iter function, so I can't see much impact really on fastpath.
Thanks, John
| |