Re: [Xen-devel] [for-4.8 PATCH] xen: credit2: fix wrong assert in runq_tickle().

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 22.11.16 at 12:52, <dario.faggioli@xxxxxxxxxx> wrote:
> On Tue, 2016-11-22 at 04:21 -0700, Jan Beulich wrote:
>> > > > On 22.11.16 at 11:43, <dario.faggioli@xxxxxxxxxx> wrote:
>> And then, having looked back at the commit mentioned in the
>> description, that one resulted in two constructs like (taking the
>> code as it looks now)
>> 
>>             if ( cpumask_test_cpu(cpu, &rqd->idle) )
>>             {
>>                 __cpumask_clear_cpu(cpu, &rqd->idle);
>>                 ...
>> 
>> Is there a reason this can't or shouldn't be
>> 
>>             if ( __cpumask_test_and_clear_cpu(cpu, &rqd->idle) )
>>             {
>>                 ...
>> ?
>> 
> It can indeed. You can find it done as it is right now in other places,
> in scheduling code, though, I guess for cache betterness, as Juergen is
> saying. I remember not being sure which way to go, and eventually
> leaning toward this one.

The question really is whether this is in fact very frequently
executed (and hence bouncing cache lines). I can't easily
tell, but I'd guess the actual schedule functions shouldn't
typically run more than once every few milliseconds.

> I'm still not sure what's best, but it'd be a cleanup/optimization, and
> would IMO require, if done, more than just that (e.g., comments should
> be improved). So I'd be inclined to consider this 4.9 material...

Of course, it was just something I've noticed while looking at
that code.

> So, I'm resending this patch with the ASSERT moved below the if, and
> I'm keeping your Reviewed-by. Hope that's ok.

And again of course.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel