Message ID | 20200210164533.29549-1-jgross@suse.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | xen/sched: remove pointless ASSERT() in credit2 | expand |
On Mon, 2020-02-10 at 17:45 +0100, Juergen Gross wrote: > The ASSERT() at the top of csched2_context_saved() is completely > pointless, as the BUG_ON() just in front of it catches the same > problem > already. > Yep, I went double checking and this is my fault. :-( Apparently, in ccf2ead7f52 ("xen: credit2: make the code less experimental"), for this specific case, I added the assert but forgot to remove the BUG_ON(). Thanks for noticing and acting on this. :-) TBH, though, considering the nature of the check, I'd rather keep the ASSERT() and kill the BUG_ON(). I can do the patch myself if you don't want to respin it that way. Thanks again and Regards
On 11.02.20 10:29, Dario Faggioli wrote: > On Mon, 2020-02-10 at 17:45 +0100, Juergen Gross wrote: >> The ASSERT() at the top of csched2_context_saved() is completely >> pointless, as the BUG_ON() just in front of it catches the same >> problem >> already. >> > Yep, I went double checking and this is my fault. :-( > > Apparently, in ccf2ead7f52 ("xen: credit2: make the code less > experimental"), for this specific case, I added the assert but forgot > to remove the BUG_ON(). > > Thanks for noticing and acting on this. :-) > > TBH, though, considering the nature of the check, I'd rather keep the > ASSERT() and kill the BUG_ON(). > > I can do the patch myself if you don't want to respin it that way. I'll respin. Juergen
On Tue, 2020-02-11 at 10:36 +0100, Jürgen Groß wrote: > On 11.02.20 10:29, Dario Faggioli wrote: > > > > TBH, though, considering the nature of the check, I'd rather keep > > the > > ASSERT() and kill the BUG_ON(). > > > > I can do the patch myself if you don't want to respin it that way. > > I'll respin. > Thanks! On an not so related note... I have that other patch of yours, the one about Credit2 runqueues on my list. Sorry it's taking a while to review it properly. I'll try to get to it ASAP. Regards
diff --git a/xen/common/sched/credit2.c b/xen/common/sched/credit2.c index b965cd1c7b..78467b772c 100644 --- a/xen/common/sched/credit2.c +++ b/xen/common/sched/credit2.c @@ -2167,10 +2167,8 @@ csched2_context_saved(const struct scheduler *ops, struct sched_unit *unit) s_time_t now = NOW(); LIST_HEAD(were_parked); - BUG_ON( !is_idle_unit(unit) && - svc->rqd != c2rqd(ops, sched_unit_master(unit))); - ASSERT(is_idle_unit(unit) || - svc->rqd == c2rqd(ops, sched_unit_master(unit))); + BUG_ON(!is_idle_unit(unit) && + svc->rqd != c2rqd(ops, sched_unit_master(unit))); /* This unit is now eligible to be put on the runqueue again */ __clear_bit(__CSFLAG_scheduled, &svc->flags);
The ASSERT() at the top of csched2_context_saved() is completely pointless, as the BUG_ON() just in front of it catches the same problem already. While at it remove a bogus space in the BUG_ON(). Signed-off-by: Juergen Gross <jgross@suse.com> --- xen/common/sched/credit2.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)