Re: [Xen-devel] [PATCH] credit2: avoid NULL deref in csched2_res_pick() when tracing

[Jürgen Groß <jgross@xxxxxxxx>]

On 02.03.20 19:16, Dario Faggioli wrote:
> On Mon, 2020-03-02 at 17:59 +0100, Jürgen Groß wrote:
> > On 02.03.20 17:49, Dario Faggioli wrote:
> > > On Mon, 2020-03-02 at 09:58 +0100, Jan Beulich wrote:
> > > >      @@ -2360,6 +2360,8 @@
> > > >                           unit->cpu_soft_affinity);
> > > >               cpumask_and(cpumask_scratch_cpu(cpu),
> > > > cpumask_scratch_cpu(cpu),
> > > >                           &min_s_rqd->active);
> > > >      +
> > > >      +        BUG_ON(!min_rqd);
> > > >           }
> > > >           else if ( min_rqd )
> > > >           {
> > > >
> > > > possibly accompanied by a comment. Thoughts?
> > > Yes, I think this is a good idea.
> > >
> > > Personally, I'd put the BUG_ON() outside of the "if {} else if {}
> > > else
> > > {}" block (i.e., just above the cpumask_cycle().
> > I don't think so.
> >
> > Otherwise the "else if ( min_rqd )" wouldn't make sense.
> Why wouldn't it?
>
> I mean, what I was saying is that I think it would be nice to have,
> just before this:
>
>   new_cpu = cpumask_cycle(min_rqd->pick_bias, cpumask_scratch_cpu(cpu));
>   min_rqd->pick_bias = new_cpu;
Sorry, my brain was in a low power state.

I managed to understand you completely wrong, thinking you wanted to
add the BUG_ON() before the if ... block.

Sorry for the noise,


Juergen

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