Re: [PATCH] Make cpu hotplug driver lock part of ppc_md

[Michael Ellerman <michael@ellerman.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2546 bytes --]

On Wed, 2009-12-23 at 08:48 -0600, Nathan Fontenot wrote:
> Michael Ellerman wrote:
> > On Tue, 2009-12-22 at 08:45 -0600, Nathan Fontenot wrote:
> >> The recently introduced cpu_hotplug_driver_lock used to serialize
> >> cpu hotplug operations, namely for the pseries platform, causes a build
> >> issue for other platforms.  The base cpu hotplug code attempts
> >> to take this lock, but it may not be needed for all platforms.  This patch
> >> moves the lock/unlock routines to be part of the ppc_md structure
> >> so that platforms needing the lock can take it.  This also makes the
> >> previous cpu_hotplug_driver_lock, defined in pseries code, pseries specific.
> >>
> >> The past failure without this patch was seen when building pmac and may
> >> be present in other platform builds.  The error is included below for reference.
> >>
> >> drivers/built-in.o: In function `.store_online':
> >> cpu.c:(.ref.text+0xf5c): undefined reference to `.cpu_hotplug_driver_lock'
> >> cpu.c:(.ref.text+0xfc8): undefined reference to `.cpu_hotplug_driver_unlock'
> >> make: *** [.tmp_vmlinux1] Error 1
> > 
> > Why does the pmac code /not/ need a lock? And would it be harmless if it
> > was locked too?
> 
> The intention of the cpu_hotplug_driver_locks to add additional serialization
> during cpu hotplug operations.  For pseries this is used during DLPAR of cpu
> operations so that cpu hotplug actions cannot be initiated whiloe a DLPAR
> operation is in flight.  For example, during DLPAR add we take the lock while
> acquiring the cpu from firmware and updating the device tree with the new
> cpu information, after which we hotplug add the cpu to the system.  

Right.

> There is nothing harmless about taking the lock on all platforms, I was just
> trying to avoid taking the lock if the additional serialization is not needed.

"nothing harmless" :)

But I think I know what you mean.

> > 
> > If so, you could just make the mutex available to all powerpc code, and
> > rename it, and then we wouldn't need all this jiggery pokery just to
> > take & release a lock.
> 
> I can make the lock available to all powerpc code and not go through the
> ppc_md struct, it makes no difference to me personally.  Of course this would
> make all that fun pokery jiggery go away :)

I think that would be a nicer result. Sure it adds an extra lock/unlock
on other platforms, but I think that's thoroughly insignificant compared
to a cpu hotplug.

Of course Ben might disagree with me ;)

cheers


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]