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. 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(). Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
