xen/sched: remove pointless ASSERT() in credit2

[Xen-devel] [PATCH] xen/sched: remove pointless ASSERT() in credit2

Posted by Juergen Gross 4 years, 2 months ago

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(-)

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);
-- 
2.16.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] xen/sched: remove pointless ASSERT() in credit2

Posted by Dario Faggioli 4 years, 2 months ago

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
-- 
Dario Faggioli, Ph.D
http://about.me/dario.faggioli
Virtualization Software Engineer
SUSE Labs, SUSE https://www.suse.com/
-------------------------------------------------------------------
<<This happens because _I_ choose it to happen!>> (Raistlin Majere)

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] xen/sched: remove pointless ASSERT() in credit2

Posted by Jürgen Groß 4 years, 2 months ago

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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] xen/sched: remove pointless ASSERT() in credit2

Posted by Dario Faggioli 4 years, 2 months ago

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
-- 
Dario Faggioli, Ph.D
http://about.me/dario.faggioli
Virtualization Software Engineer
SUSE Labs, SUSE https://www.suse.com/
-------------------------------------------------------------------
<<This happens because _I_ choose it to happen!>> (Raistlin Majere)

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel