[RESENDING, BECAUSE BOUNCED OFF] > Il giorno 31 gen 2020, alle ore 11:20, Oleksandr Natalenko <oleksandr@xxxxxxxxxxxxxx> ha scritto: > > Hello. > > On 31.01.2020 10:24, Paolo Valente wrote: >> In bfq_bfqq_move(), the bfq_queue, say Q, to be moved to a new group >> may happen to be deactivated in the scheduling data structures of the >> source group (and then activated in the destination group). If Q is >> referred only by the data structures in the source group when the >> deactivation happens, then Q is freed upon the deactivation. >> This commit addresses this issue by getting an extra reference before >> the possible deactivation, and releasing this extra reference after Q >> has been moved. >> Tested-by: Chris Evich <cevich@xxxxxxxxxx> >> Signed-off-by: Paolo Valente <paolo.valente@xxxxxxxxxx> >> --- >> block/bfq-cgroup.c | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c >> index e1419edde2ec..8ab7f18ff8cb 100644 >> --- a/block/bfq-cgroup.c >> +++ b/block/bfq-cgroup.c >> @@ -651,6 +651,12 @@ void bfq_bfqq_move(struct bfq_data *bfqd, struct >> bfq_queue *bfqq, >> bfq_bfqq_expire(bfqd, bfqd->in_service_queue, >> false, BFQQE_PREEMPTED); >> + /* >> + * get extra reference to prevent bfqq from being freed in >> + * next possible deactivate >> + */ >> + bfqq->ref++; > > Shouldn't this be hidden under some macro (bfq_get_queue_ref(), for instance) and also converted from int into refcount_t? > Yeah, that's in my (long) todo list. Unfortunately, all BFQ code handles that ref increment in that rustic way (inherited from CFQ). It would take a little time to fix and check all occurrences. This fix is probably more critical, as this bug is crashing machines in some configurations. But I promise I won't forget your right suggestion. Thanks, Paolo >> + >> if (bfq_bfqq_busy(bfqq)) >> bfq_deactivate_bfqq(bfqd, bfqq, false, false); >> else if (entity->on_st) >> @@ -670,6 +676,8 @@ void bfq_bfqq_move(struct bfq_data *bfqd, struct >> bfq_queue *bfqq, >> if (!bfqd->in_service_queue && !bfqd->rq_in_driver) >> bfq_schedule_dispatch(bfqd); >> + /* release extra ref taken above */ >> + bfq_put_queue(bfqq); >> } >> /** > > -- > Oleksandr Natalenko (post-factum)
