diff mbox series

[net] net: sched: cbq: stop timer in cbq_destroy() when cbq_init() fails

Message ID 20221022104054.221968-1-shaozhengchao@huawei.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series [net] net: sched: cbq: stop timer in cbq_destroy() when cbq_init() fails | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net
netdev/fixes_present success Fixes tag present in non-next series
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 9 of 9 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 7 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

shaozhengchao Oct. 22, 2022, 10:40 a.m. UTC
When qdisc_create() fails to invoke the cbq_init() function for
initialization, the timer has been started. But cbq_destroy() doesn't
stop the timer. Fix it.

Fixes: 88a993540a65 ("[NET_SCHED]: sch_cbq: use hrtimer based watchdog")
Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com>
---
 net/sched/sch_cbq.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Cong Wang Oct. 23, 2022, 2:54 a.m. UTC | #1
On Sat, Oct 22, 2022 at 06:40:54PM +0800, Zhengchao Shao wrote:
> When qdisc_create() fails to invoke the cbq_init() function for
> initialization, the timer has been started. But cbq_destroy() doesn't
> stop the timer. Fix it.
> 

Hmm? qdisc_watchdog_init() only initializes it, not starts it, right?

Thanks.
shaozhengchao Oct. 24, 2022, 10:53 a.m. UTC | #2
On 2022/10/23 10:54, Cong Wang wrote:
> On Sat, Oct 22, 2022 at 06:40:54PM +0800, Zhengchao Shao wrote:
>> When qdisc_create() fails to invoke the cbq_init() function for
>> initialization, the timer has been started. But cbq_destroy() doesn't
>> stop the timer. Fix it.
>>
> 
> Hmm? qdisc_watchdog_init() only initializes it, not starts it, right?
> 
> Thanks.
Hi Wang:
	Thank you for your review. The description is incorrect,
qdisc_watchdog_init() only initializes timer, and cbq_destroy() missed
to cancle timer.

Zhengchao Shao
Kuniyuki Iwashima Oct. 25, 2022, 12:03 a.m. UTC | #3
From:   shaozhengchao <shaozhengchao@huawei.com>
Date:   Mon, 24 Oct 2022 18:53:30 +0800
> On 2022/10/23 10:54, Cong Wang wrote:
> > On Sat, Oct 22, 2022 at 06:40:54PM +0800, Zhengchao Shao wrote:
> >> When qdisc_create() fails to invoke the cbq_init() function for
> >> initialization, the timer has been started. But cbq_destroy() doesn't
> >> stop the timer. Fix it.
> >>
> > 
> > Hmm? qdisc_watchdog_init() only initializes it, not starts it, right?
> > 
> > Thanks.
> Hi Wang:
> 	Thank you for your review. The description is incorrect,
> qdisc_watchdog_init() only initializes timer, and cbq_destroy() missed
> to cancle timer.

In the ->init() failure path, we need not cancel timer.  Another path
where we call ->destroy() is qdisc_destroy(), but just before calling
->destroy(), we call qdisc_reset() and cbq_reset() cancels the timer.

So, I think we need not add qdisc_watchdog_cancel() in cbq_destroy().
diff mbox series

Patch

diff --git a/net/sched/sch_cbq.c b/net/sched/sch_cbq.c
index 6568e17c4c63..8fcdd74af4cc 100644
--- a/net/sched/sch_cbq.c
+++ b/net/sched/sch_cbq.c
@@ -1371,6 +1371,7 @@  static void cbq_destroy(struct Qdisc *sch)
 #ifdef CONFIG_NET_CLS_ACT
 	q->rx_class = NULL;
 #endif
+	qdisc_watchdog_cancel(&q->watchdog);
 	/*
 	 * Filters must be destroyed first because we don't destroy the
 	 * classes from root to leafs which means that filters can still