* [PATCH] xen: credit2: clear bit instead of skip step in runq_tickle()
@ 2017-01-18 0:30 Dario Faggioli
2017-01-18 10:21 ` George Dunlap
0 siblings, 1 reply; 6+ messages in thread
From: Dario Faggioli @ 2017-01-18 0:30 UTC (permalink / raw)
To: xen-devel; +Cc: George Dunlap
Since we are doing cpumask manipulation already, clear a bit
in the mask at once. Doing that will save us an if, later in
the code.
No functional change intended.
Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
---
Cc: George Dunlap <george.dunlap@eu.citrix.com>
---
xen/common/sched_credit2.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index ef8e0d8..d086264 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -985,7 +985,7 @@ runq_tickle(const struct scheduler *ops, struct csched2_vcpu *new, s_time_t now)
cpumask_andnot(&mask, &rqd->active, &rqd->idle);
cpumask_andnot(&mask, &mask, &rqd->tickled);
cpumask_and(&mask, &mask, new->vcpu->cpu_hard_affinity);
- if ( cpumask_test_cpu(cpu, &mask) )
+ if ( __cpumask_test_and_clear_cpu(cpu, &mask) )
{
cur = CSCHED2_VCPU(curr_on_cpu(cpu));
burn_credits(rqd, cur, now);
@@ -1001,8 +1001,7 @@ runq_tickle(const struct scheduler *ops, struct csched2_vcpu *new, s_time_t now)
for_each_cpu(i, &mask)
{
/* Already looked at this one above */
- if ( i == cpu )
- continue;
+ ASSERT(i != cpu);
cur = CSCHED2_VCPU(curr_on_cpu(i));
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] xen: credit2: clear bit instead of skip step in runq_tickle()
2017-01-18 0:30 [PATCH] xen: credit2: clear bit instead of skip step in runq_tickle() Dario Faggioli
@ 2017-01-18 10:21 ` George Dunlap
2017-01-18 10:30 ` Jan Beulich
0 siblings, 1 reply; 6+ messages in thread
From: George Dunlap @ 2017-01-18 10:21 UTC (permalink / raw)
To: Dario Faggioli, xen-devel; +Cc: George Dunlap, Jan Beulich
On 18/01/17 00:30, Dario Faggioli wrote:
> Since we are doing cpumask manipulation already, clear a bit
> in the mask at once. Doing that will save us an if, later in
> the code.
>
> No functional change intended.
>
> Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
> ---
> Cc: George Dunlap <george.dunlap@eu.citrix.com>
> ---
> xen/common/sched_credit2.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
> index ef8e0d8..d086264 100644
> --- a/xen/common/sched_credit2.c
> +++ b/xen/common/sched_credit2.c
> @@ -985,7 +985,7 @@ runq_tickle(const struct scheduler *ops, struct csched2_vcpu *new, s_time_t now)
> cpumask_andnot(&mask, &rqd->active, &rqd->idle);
> cpumask_andnot(&mask, &mask, &rqd->tickled);
> cpumask_and(&mask, &mask, new->vcpu->cpu_hard_affinity);
> - if ( cpumask_test_cpu(cpu, &mask) )
> + if ( __cpumask_test_and_clear_cpu(cpu, &mask) )
Since we're micro-optimizing -- isn't test-and-clear a locked operation?
Would that be more expensive than the if() statement below?
-George
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xen: credit2: clear bit instead of skip step in runq_tickle()
2017-01-18 10:21 ` George Dunlap
@ 2017-01-18 10:30 ` Jan Beulich
2017-01-18 11:05 ` Dario Faggioli
2017-01-26 1:00 ` Dario Faggioli
0 siblings, 2 replies; 6+ messages in thread
From: Jan Beulich @ 2017-01-18 10:30 UTC (permalink / raw)
To: George Dunlap; +Cc: George Dunlap, xen-devel, Dario Faggioli
>>> On 18.01.17 at 11:21, <george.dunlap@citrix.com> wrote:
> On 18/01/17 00:30, Dario Faggioli wrote:
>> index ef8e0d8..d086264 100644
>> --- a/xen/common/sched_credit2.c
>> +++ b/xen/common/sched_credit2.c
>> @@ -985,7 +985,7 @@ runq_tickle(const struct scheduler *ops, struct csched2_vcpu *new, s_time_t now)
>> cpumask_andnot(&mask, &rqd->active, &rqd->idle);
>> cpumask_andnot(&mask, &mask, &rqd->tickled);
>> cpumask_and(&mask, &mask, new->vcpu->cpu_hard_affinity);
>> - if ( cpumask_test_cpu(cpu, &mask) )
>> + if ( __cpumask_test_and_clear_cpu(cpu, &mask) )
>
> Since we're micro-optimizing -- isn't test-and-clear a locked operation?
> Would that be more expensive than the if() statement below?
cpumask_test_and_clear_cpu() is, but __cpumask_test_and_clear_cpu()
isn't.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xen: credit2: clear bit instead of skip step in runq_tickle()
2017-01-18 10:30 ` Jan Beulich
@ 2017-01-18 11:05 ` Dario Faggioli
2017-01-26 1:00 ` Dario Faggioli
1 sibling, 0 replies; 6+ messages in thread
From: Dario Faggioli @ 2017-01-18 11:05 UTC (permalink / raw)
To: Jan Beulich, George Dunlap; +Cc: George Dunlap, xen-devel
[-- Attachment #1.1: Type: text/plain, Size: 1463 bytes --]
On Wed, 2017-01-18 at 03:30 -0700, Jan Beulich wrote:
> > > > On 18.01.17 at 11:21, <george.dunlap@citrix.com> wrote:
> > On 18/01/17 00:30, Dario Faggioli wrote:
> > >
> > > --- a/xen/common/sched_credit2.c
> > > +++ b/xen/common/sched_credit2.c
> > > @@ -985,7 +985,7 @@ runq_tickle(const struct scheduler *ops,
> > > struct csched2_vcpu *new, s_time_t now)
> > > cpumask_andnot(&mask, &rqd->active, &rqd->idle);
> > > cpumask_andnot(&mask, &mask, &rqd->tickled);
> > > cpumask_and(&mask, &mask, new->vcpu->cpu_hard_affinity);
> > > - if ( cpumask_test_cpu(cpu, &mask) )
> > > + if ( __cpumask_test_and_clear_cpu(cpu, &mask) )
> >
> > Since we're micro-optimizing -- isn't test-and-clear a locked
> > operation?
> > Would that be more expensive than the if() statement below?
>
> cpumask_test_and_clear_cpu() is, but __cpumask_test_and_clear_cpu()
> isn't.
>
As Jan said.
And, FWIW, I personally like how the code looks after this patch
better, even leaving aside performance.
I find it cleaner (probably because dislike 'continue'), and more in
line with what we do in the rest of the file.
Thanks and Regadrs,
Dario
--
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
[-- Attachment #2: Type: text/plain, Size: 127 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xen: credit2: clear bit instead of skip step in runq_tickle()
2017-01-18 10:30 ` Jan Beulich
2017-01-18 11:05 ` Dario Faggioli
@ 2017-01-26 1:00 ` Dario Faggioli
2017-02-01 15:00 ` George Dunlap
1 sibling, 1 reply; 6+ messages in thread
From: Dario Faggioli @ 2017-01-26 1:00 UTC (permalink / raw)
To: George Dunlap; +Cc: George Dunlap, xen-devel, Jan Beulich
[-- Attachment #1.1: Type: text/plain, Size: 1264 bytes --]
On Wed, 2017-01-18 at 03:30 -0700, Jan Beulich wrote:
> > > > On 18.01.17 at 11:21, <george.dunlap@citrix.com> wrote:
> > On 18/01/17 00:30, Dario Faggioli wrote:
> > > index ef8e0d8..d086264 100644
> > > --- a/xen/common/sched_credit2.c
> > > +++ b/xen/common/sched_credit2.c
> > > @@ -985,7 +985,7 @@ runq_tickle(const struct scheduler *ops,
> > > struct csched2_vcpu *new, s_time_t now)
> > > cpumask_andnot(&mask, &rqd->active, &rqd->idle);
> > > cpumask_andnot(&mask, &mask, &rqd->tickled);
> > > cpumask_and(&mask, &mask, new->vcpu->cpu_hard_affinity);
> > > - if ( cpumask_test_cpu(cpu, &mask) )
> > > + if ( __cpumask_test_and_clear_cpu(cpu, &mask) )
> >
> > Since we're micro-optimizing -- isn't test-and-clear a locked
> > operation?
> > Would that be more expensive than the if() statement below?
>
> cpumask_test_and_clear_cpu() is, but __cpumask_test_and_clear_cpu()
> isn't.
>
George, ping?
Thanks and Regards,
Dario
--
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
[-- Attachment #2: Type: text/plain, Size: 127 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xen: credit2: clear bit instead of skip step in runq_tickle()
2017-01-26 1:00 ` Dario Faggioli
@ 2017-02-01 15:00 ` George Dunlap
0 siblings, 0 replies; 6+ messages in thread
From: George Dunlap @ 2017-02-01 15:00 UTC (permalink / raw)
To: Dario Faggioli; +Cc: George Dunlap, xen-devel, Jan Beulich
On 26/01/17 01:00, Dario Faggioli wrote:
> On Wed, 2017-01-18 at 03:30 -0700, Jan Beulich wrote:
>>>>> On 18.01.17 at 11:21, <george.dunlap@citrix.com> wrote:
>>> On 18/01/17 00:30, Dario Faggioli wrote:
>>>> index ef8e0d8..d086264 100644
>>>> --- a/xen/common/sched_credit2.c
>>>> +++ b/xen/common/sched_credit2.c
>>>> @@ -985,7 +985,7 @@ runq_tickle(const struct scheduler *ops,
>>>> struct csched2_vcpu *new, s_time_t now)
>>>> cpumask_andnot(&mask, &rqd->active, &rqd->idle);
>>>> cpumask_andnot(&mask, &mask, &rqd->tickled);
>>>> cpumask_and(&mask, &mask, new->vcpu->cpu_hard_affinity);
>>>> - if ( cpumask_test_cpu(cpu, &mask) )
>>>> + if ( __cpumask_test_and_clear_cpu(cpu, &mask) )
>>>
>>> Since we're micro-optimizing -- isn't test-and-clear a locked
>>> operation?
>>> Would that be more expensive than the if() statement below?
>>
>> cpumask_test_and_clear_cpu() is, but __cpumask_test_and_clear_cpu()
>> isn't.
>>
> George, ping?
Yes, this looks fine then. But it didn't apply cleanly when I tried to
apply it -- please re-send it with the other patches you have outstanding.
Thanks.
-George
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-02-01 15:01 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-18 0:30 [PATCH] xen: credit2: clear bit instead of skip step in runq_tickle() Dario Faggioli
2017-01-18 10:21 ` George Dunlap
2017-01-18 10:30 ` Jan Beulich
2017-01-18 11:05 ` Dario Faggioli
2017-01-26 1:00 ` Dario Faggioli
2017-02-01 15:00 ` George Dunlap
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).