Hi, Jan
在 2023/03/07 18:20, Jan Kara 写道:
On Tue 07-03-23 17:36:19, Yu Kuai wrote:
Hi,
在 2023/03/07 17:13, Shinichiro Kawasaki 写道:
On Mar 07, 2023 / 16:57, Yu Kuai wrote:
[...]
Thanks for the report, can you help to provide the result of add2line of
following?
bfq_setup_cooperator+0x120b/0x1650
bfq_setup_cooperator+0xa41/0x1650
That will help to locate the problem.
Hi, Yu thanks for looking into this. Here are the faddr2line outputs:
$ ./scripts/faddr2line vmlinux bfq_setup_cooperator+0x120b/0x1650
bfq_setup_cooperator+0x120b/0x1650:
bfqq_process_refs at block/bfq-iosched.c:1200
(inlined by) bfq_setup_stable_merge at block/bfq-iosched.c:2855
(inlined by) bfq_setup_cooperator at block/bfq-iosched.c:2941
$ ./scripts/faddr2line vmlinux bfq_setup_cooperator+0xa41/0x1650
bfq_setup_cooperator+0xa41/0x1650:
bfq_setup_cooperator at block/bfq-iosched.c:2939
So, after a quick look at the code, the difference is that fd571df0ac5b
changes the logic when bfqq_proces_refs(stable_merge_bfqq) is 0.
I think perhaps following patch can work, at least 'stable_merge_bfqq'
won't be accessed after bfq_put_stable_ref() in this case:
diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 8a8d4441519c..881f74b3a556 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -2856,6 +2856,11 @@ bfq_setup_stable_merge(struct bfq_data *bfqd, struct
bfq_queue *bfqq,
bfqq_process_refs(stable_merge_bfqq));
struct bfq_queue *new_bfqq;
+ /* deschedule stable merge, because done or aborted here */
+ bfq_put_stable_ref(stable_merge_bfqq);
+
+ bfqq_data->stable_merge_bfqq = NULL;
+
I don't think this is going to help. stable_merge_bfqq is used just a few
lines below again in bfq_setup_merge(). The problem really is that the
reference from stable merge can be the last one keeping bfqq alive so in
bfq_setup_cooperator() we need to see if stable_merge_bfqq still has
process references (and cancel the merge if not) before dropping our ref.
I was thinking that bfq_setup_merge() will only be called if 'proc_ref'
is not 0, which means that stable_merge_bfqq won't be freed.
So rather doing something like:
bfqq_data->stable_merge_bfqq = NULL;
new_bfqq = bfq_setup_stable_merge(bfqd, bfqq,
stable_merge_bfqq, bfqq_data);
bfq_put_stable_ref(stable_merge_bfqq);
return new_bfqq;
should work in bfq_setup_cooperator().
Yes, this will work.
Thanks,
Kuai
Honza
