From: "Jan Beulich" <JBeulich@suse.com> To: Julien Grall <julien.grall@arm.com> Cc: Stefano Stabellini <sstabellini@kernel.org>, Wei Liu <wei.liu2@citrix.com>, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>, George Dunlap <George.Dunlap@eu.citrix.com>, Andrew Cooper <andrew.cooper3@citrix.com>, Ian Jackson <Ian.Jackson@eu.citrix.com>, Tim Deegan <tim@xen.org>, xen-devel <xen-devel@lists.xenproject.org> Subject: Re: [PATCH 1/2] core-parking: interact with runtime SMT-disabling Date: Thu, 25 Apr 2019 04:20:12 -0600 [thread overview] Message-ID: <5CC189DC0200007800228F84@prv1-mh.provo.novell.com> (raw) In-Reply-To: <3d520753-510d-69b2-b2f0-0965bc14cc6b@arm.com> >>> On 24.04.19 at 13:51, <julien.grall@arm.com> wrote: > On 11/04/2019 13:45, Jan Beulich wrote: >> --- a/xen/common/core_parking.c >> +++ b/xen/common/core_parking.c > > > [...] > >> +bool core_parking_remove(unsigned int cpu) > > Something looks wrong. This function is implemented in common code but... > >> +{ >> + unsigned int i; >> + bool found = false; >> + >> + spin_lock(&accounting_lock); >> + >> + for ( i = 0; i < cur_idle_nums; ++i ) >> + if ( core_parking_cpunum[i] == cpu ) >> + { >> + found = true; >> + --cur_idle_nums; >> + break; >> + } >> + >> + for ( ; i < cur_idle_nums; ++i ) >> + core_parking_cpunum[i] = core_parking_cpunum[i + 1]; >> + >> + spin_unlock(&accounting_lock); >> + >> + return found; >> +} >> + >> uint32_t get_cur_idle_nums(void) >> { >> return cur_idle_nums; >> --- a/xen/include/asm-x86/smp.h >> +++ b/xen/include/asm-x86/smp.h >> @@ -63,6 +63,7 @@ long cpu_up_helper(void *data); >> long cpu_down_helper(void *data); >> >> long core_parking_helper(void *data); >> +bool core_parking_remove(unsigned int cpu); > > The prototype is declared in asm-x86/smp.h. Actually, it seems all the function > exported in core_parking.c have their prototype declared in asm-x86/smp.h. I've noticed this too, but this is not an uncommon thing (hence the CORE_PARKING Kconfig setting). Various such instance have been eliminated since the introduction of Arm support, but others remain. It's not the purpose of this series to clean up this aspect, or ... > This raises the question of whether it makes sense to have core_parking.c in > common. If it makes sense, then the prototype should be declared in > include/xen. ... to decide whether the .c file or the declarations should move. Personally I think this is generic enough a concept that the .c should remain where it is, in which case we'd likely want to gain a xen/core-parking.h at some point. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
WARNING: multiple messages have this Message-ID (diff)
From: "Jan Beulich" <JBeulich@suse.com> To: "Julien Grall" <julien.grall@arm.com> Cc: Stefano Stabellini <sstabellini@kernel.org>, Wei Liu <wei.liu2@citrix.com>, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>, George Dunlap <George.Dunlap@eu.citrix.com>, Andrew Cooper <andrew.cooper3@citrix.com>, Ian Jackson <Ian.Jackson@eu.citrix.com>, Tim Deegan <tim@xen.org>, xen-devel <xen-devel@lists.xenproject.org> Subject: Re: [Xen-devel] [PATCH 1/2] core-parking: interact with runtime SMT-disabling Date: Thu, 25 Apr 2019 04:20:12 -0600 [thread overview] Message-ID: <5CC189DC0200007800228F84@prv1-mh.provo.novell.com> (raw) Message-ID: <20190425102012.iYT63yoE48mgwdSN3E-31aNNZozPpbmTdpAQmyZccEI@z> (raw) In-Reply-To: <3d520753-510d-69b2-b2f0-0965bc14cc6b@arm.com> >>> On 24.04.19 at 13:51, <julien.grall@arm.com> wrote: > On 11/04/2019 13:45, Jan Beulich wrote: >> --- a/xen/common/core_parking.c >> +++ b/xen/common/core_parking.c > > > [...] > >> +bool core_parking_remove(unsigned int cpu) > > Something looks wrong. This function is implemented in common code but... > >> +{ >> + unsigned int i; >> + bool found = false; >> + >> + spin_lock(&accounting_lock); >> + >> + for ( i = 0; i < cur_idle_nums; ++i ) >> + if ( core_parking_cpunum[i] == cpu ) >> + { >> + found = true; >> + --cur_idle_nums; >> + break; >> + } >> + >> + for ( ; i < cur_idle_nums; ++i ) >> + core_parking_cpunum[i] = core_parking_cpunum[i + 1]; >> + >> + spin_unlock(&accounting_lock); >> + >> + return found; >> +} >> + >> uint32_t get_cur_idle_nums(void) >> { >> return cur_idle_nums; >> --- a/xen/include/asm-x86/smp.h >> +++ b/xen/include/asm-x86/smp.h >> @@ -63,6 +63,7 @@ long cpu_up_helper(void *data); >> long cpu_down_helper(void *data); >> >> long core_parking_helper(void *data); >> +bool core_parking_remove(unsigned int cpu); > > The prototype is declared in asm-x86/smp.h. Actually, it seems all the function > exported in core_parking.c have their prototype declared in asm-x86/smp.h. I've noticed this too, but this is not an uncommon thing (hence the CORE_PARKING Kconfig setting). Various such instance have been eliminated since the introduction of Arm support, but others remain. It's not the purpose of this series to clean up this aspect, or ... > This raises the question of whether it makes sense to have core_parking.c in > common. If it makes sense, then the prototype should be declared in > include/xen. ... to decide whether the .c file or the declarations should move. Personally I think this is generic enough a concept that the .c should remain where it is, in which case we'd likely want to gain a xen/core-parking.h at some point. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
next prev parent reply other threads:[~2019-04-25 10:20 UTC|newest] Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-04-11 12:04 [PATCH 0/2] core-parking: SMT-disable and section adjustments Jan Beulich 2019-04-11 12:04 ` [Xen-devel] " Jan Beulich 2019-04-11 12:45 ` [PATCH 1/2] core-parking: interact with runtime SMT-disabling Jan Beulich 2019-04-11 12:45 ` [Xen-devel] " Jan Beulich 2019-04-11 19:06 ` Andrew Cooper 2019-04-11 19:06 ` [Xen-devel] " Andrew Cooper 2019-04-12 11:41 ` Jan Beulich 2019-04-12 11:41 ` [Xen-devel] " Jan Beulich 2019-05-27 9:36 ` Ping: " Jan Beulich 2019-05-27 9:36 ` [Xen-devel] " Jan Beulich 2019-04-24 11:51 ` Julien Grall 2019-04-24 11:51 ` [Xen-devel] " Julien Grall 2019-04-25 10:20 ` Jan Beulich [this message] 2019-04-25 10:20 ` Jan Beulich 2019-04-11 12:45 ` [PATCH 2/2] core-parking: adjust data/code placement Jan Beulich 2019-04-11 12:45 ` [Xen-devel] " Jan Beulich 2019-04-11 19:01 ` Andrew Cooper 2019-04-11 19:01 ` [Xen-devel] " Andrew Cooper
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=5CC189DC0200007800228F84@prv1-mh.provo.novell.com \ --to=jbeulich@suse.com \ --cc=George.Dunlap@eu.citrix.com \ --cc=Ian.Jackson@eu.citrix.com \ --cc=andrew.cooper3@citrix.com \ --cc=julien.grall@arm.com \ --cc=konrad.wilk@oracle.com \ --cc=sstabellini@kernel.org \ --cc=tim@xen.org \ --cc=wei.liu2@citrix.com \ --cc=xen-devel@lists.xenproject.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.