public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Cyrill Gorcunov <gorcunov@gmail.com>
To: Tejun Heo <tj@kernel.org>
Cc: linux-kernel@vger.kernel.org, mingo@redhat.com,
	tglx@linutronix.de, hpa@zytor.com, x86@kernel.org,
	eric.dumazet@gmail.com, yinghai@kernel.org, brgerst@gmail.com,
	penberg@kernel.org
Subject: Re: [PATCH UPDATED#2 04/16] x86: Use local variable to cache smp_processor_id() in setup_local_APIC()
Date: Tue, 30 Nov 2010 23:52:57 +0300	[thread overview]
Message-ID: <20101130205257.GD20818@lenovo> (raw)
In-Reply-To: <4CF553A3.8030000@kernel.org>

On Tue, Nov 30, 2010 at 08:42:27PM +0100, Tejun Heo wrote:
...
> Not really.  smp_processor_id() outside of preemption/irq disabled
> area is suspicious exactly the same and having @cpu initialized at the
> head of the function has the advantage of making it clear that the
> function as a whole is special w.r.t. which cpu it runs on.  In fact,
> I think it's actually worse to put it inside preemption disabled area
> as it gives the wrong impression that the function isn't special and

well, i take the opposite, if function is not ever supposed to be called
outside of cpu it runs on there should not be references even in local
variables outside of preempt scope (even if we know that it is safe
to cache 'cpu' early), but I think this is a question of habbit :)

> might be able to handle being called from another cpu.  In fact,
> caching smp_processor_id() in @cpu is better than calling it every
> time as it makes it clear that the function is bound to that cpu.
> 
> That said, I really don't think this matters one way or the other.
> This function is called during smp bring up to initialize the CPU.
> Things don't get much more arch/cpu dependent than this and the code
> is thoroughly tested on each and every boot and power management
> operations.  How @cpu is cached and used just doesn't matter, which is
> why I didn't really care about it and changed as suggested.  So, let's
> just let it go.

*sure*, as one Ingo merge it we would be able to clean it a bit on top

[ in particular I believe we should preempt disable at the very beginning
  on the function and use cached cpu then until the preemption gets on
  at the end, and we would cover cmci_recheck with cached cpu as well.
  this would eliminate all the questions I believe ]

> 
> Thanks.
> 
> -- 
> tejun
> 
  Cyrill

  reply	other threads:[~2010-11-30 20:53 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-11-27 15:21 [PATCHSET] x86: unify x86_32 and 64 NUMA init paths, take#2 Tejun Heo
2010-11-27 15:21 ` [PATCH 01/16] x86: Kill unused static boot_cpu_logical_apicid in smpboot.c Tejun Heo
2010-11-27 15:21 ` [PATCH 02/16] x86: Rename x86_32 MAX_APICID to MAX_LOCAL_APIC Tejun Heo
2010-11-27 15:21 ` [PATCH 03/16] x86: Make default_send_IPI_mask_sequence/allbutself_logical() 32bit only Tejun Heo
2010-11-27 15:21 ` [PATCH 04/16] x86: Use local variable to cache smp_processor_id() in setup_local_APIC() Tejun Heo
2010-11-27 21:32   ` Cyrill Gorcunov
2010-11-30 16:17   ` [PATCH UPDATED " Tejun Heo
2010-11-30 18:11   ` [PATCH UPDATED#2 " Tejun Heo
2010-11-30 18:22     ` Cyrill Gorcunov
2010-11-30 19:42       ` Tejun Heo
2010-11-30 20:52         ` Cyrill Gorcunov [this message]
2010-12-09  9:37     ` Pekka Enberg
2010-12-09 10:47   ` [PATCH UPDATED#3 04/16] x86: setup_local_APIC() must always be called with preemption disabled Tejun Heo
2010-12-09 16:17     ` Cyrill Gorcunov
2010-12-09 16:18     ` Pekka Enberg
2010-12-09 21:56     ` Thomas Gleixner
2010-12-10 11:09       ` Tejun Heo
2010-12-10 11:10         ` Thomas Gleixner
2010-12-10 12:48     ` [tip:x86/apic-cleanups] x86: apic: Cleanup and simplify setup_local_APIC() tip-bot for Tejun Heo
2010-11-27 15:21 ` [PATCH 05/16] x86: Replace cpu_2_logical_apicid[] with early percpu variable Tejun Heo
2010-11-27 15:21 ` [PATCH 06/16] x86: Always use x86_cpu_to_logical_apicid for cpu -> logical apic id Tejun Heo
2010-11-27 15:21 ` [PATCH 07/16] x86: Remove custom apic->cpu_to_logical_apicid() implementations Tejun Heo
2010-12-09 21:28   ` Thomas Gleixner
2010-12-10 11:12     ` Tejun Heo
2010-12-10 11:18       ` Thomas Gleixner
2010-11-27 15:21 ` [PATCH 08/16] x86: Use apic->cpu_to_logical_apicid() to find out logical apic id mapping early during boot Tejun Heo
2010-11-27 15:21 ` [PATCH 09/16] x86: Implement cpu_to_logical_apicid() for bigsmp_32 Tejun Heo
2010-11-27 15:21 ` [PATCH 10/16] x86: Implement cpu_to_logical_apicid() for summit_32 Tejun Heo
2010-11-27 15:22 ` [PATCH 11/16] x86: Implement cpu_to_logical_apicid() for numaq_32 Tejun Heo
2010-11-27 15:22 ` [PATCH 12/16] x86: Replace apic->apicid_to_node() with ->numa_cpu_node() Tejun Heo
2010-11-27 15:22 ` [PATCH 13/16] x86: Unify cpu/apicid <-> NUMA node mapping between 32 and 64bit Tejun Heo
2010-12-09 21:43   ` Thomas Gleixner
2010-12-10 20:45     ` Tejun Heo
2010-12-10 20:54       ` Tejun Heo
2010-12-10 21:17         ` Yinghai Lu
2010-11-27 15:22 ` [PATCH 14/16] x86: Unify CPU -> " Tejun Heo
2010-12-09 21:49   ` Thomas Gleixner
2010-12-10 11:13     ` Tejun Heo
2010-11-27 15:22 ` [PATCH 15/16] x86: Unify node_to_cpumask_map handling " Tejun Heo
2010-11-27 15:22 ` [PATCH 16/16] x86: Unify NUMA initialization " Tejun Heo
2010-12-07 16:22 ` [PATCHSET] x86: unify x86_32 and 64 NUMA init paths, take#2 Tejun Heo

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=20101130205257.GD20818@lenovo \
    --to=gorcunov@gmail.com \
    --cc=brgerst@gmail.com \
    --cc=eric.dumazet@gmail.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=penberg@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=tj@kernel.org \
    --cc=x86@kernel.org \
    --cc=yinghai@kernel.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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox