LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] powerpc/eeh: Update MAINTAINERS
From: Gavin Shan @ 2013-06-28  2:18 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: Linas Vepstas, linuxppc-dev, Gavin Shan
In-Reply-To: <1372385489.18612.52.camel@pasglop>

On Fri, Jun 28, 2013 at 12:11:29PM +1000, Benjamin Herrenschmidt wrote:
>On Fri, 2013-06-28 at 09:59 +0800, Gavin Shan wrote:
>> Update MAINTAINERS to reflect recent changes.
>> 
>> Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
>> ---
>>  MAINTAINERS |    4 ++++
>>  1 files changed, 4 insertions(+), 0 deletions(-)
>> 
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 5be702c..b447392 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -6146,10 +6146,14 @@ F:	drivers/firmware/pcdp.*
>>  
>>  PCI ERROR RECOVERY
>>  M:	 
>> +M:	Gavin Shan <shangw@linux.vnet.ibm.com>
>
>Remove Linas, he isn't involved anymore as far as I can tell
>(are you ?)
>
>>  L:	linux-pci@vger.kernel.org
>> +L:	linuxppc-dev@lists.ozlabs.org
>>  S:	Supported
>>  F:	Documentation/PCI/pci-error-recovery.txt
>>  F:	Documentation/powerpc/eeh-pci-error-recovery.txt
>> +F:	arch/powerpc/kernel/eeh*.c
>> +F:	drivers/pci/pcie/aer/
>
>Not sure about the AER code. You are not maintaining *that* at least :-)
>Maybe we should split EEH from the rest ?
>

Ok. Split EEH from it and keep everything for "PCI ERROR RECOVERY".

Thanks,
Gavin

^ permalink raw reply

* Re: [PATCH] powerpc/eeh: Update MAINTAINERS
From: Benjamin Herrenschmidt @ 2013-06-28  2:11 UTC (permalink / raw)
  To: Gavin Shan; +Cc: Linas Vepstas, linuxppc-dev
In-Reply-To: <1372384784-20788-1-git-send-email-shangw@linux.vnet.ibm.com>

On Fri, 2013-06-28 at 09:59 +0800, Gavin Shan wrote:
> Update MAINTAINERS to reflect recent changes.
> 
> Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
> ---
>  MAINTAINERS |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5be702c..b447392 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6146,10 +6146,14 @@ F:	drivers/firmware/pcdp.*
>  
>  PCI ERROR RECOVERY
>  M:	 
> +M:	Gavin Shan <shangw@linux.vnet.ibm.com>

Remove Linas, he isn't involved anymore as far as I can tell
(are you ?)

>  L:	linux-pci@vger.kernel.org
> +L:	linuxppc-dev@lists.ozlabs.org
>  S:	Supported
>  F:	Documentation/PCI/pci-error-recovery.txt
>  F:	Documentation/powerpc/eeh-pci-error-recovery.txt
> +F:	arch/powerpc/kernel/eeh*.c
> +F:	drivers/pci/pcie/aer/

Not sure about the AER code. You are not maintaining *that* at least :-)
Maybe we should split EEH from the rest ?

Cheers,
Ben.

>  PCI SUBSYSTEM
>  M:	Bjorn Helgaas <bhelgaas@google.com>

^ permalink raw reply

* [PATCH] powerpc/eeh: Update MAINTAINERS
From: Gavin Shan @ 2013-06-28  1:59 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Gavin Shan

Update MAINTAINERS to reflect recent changes.

Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
---
 MAINTAINERS |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 5be702c..b447392 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6146,10 +6146,14 @@ F:	drivers/firmware/pcdp.*
 
 PCI ERROR RECOVERY
 M:	Linas Vepstas <linasvepstas@gmail.com>
+M:	Gavin Shan <shangw@linux.vnet.ibm.com>
 L:	linux-pci@vger.kernel.org
+L:	linuxppc-dev@lists.ozlabs.org
 S:	Supported
 F:	Documentation/PCI/pci-error-recovery.txt
 F:	Documentation/powerpc/eeh-pci-error-recovery.txt
+F:	arch/powerpc/kernel/eeh*.c
+F:	drivers/pci/pcie/aer/
 
 PCI SUBSYSTEM
 M:	Bjorn Helgaas <bhelgaas@google.com>
-- 
1.7.5.4

^ permalink raw reply related

* Re: [PATCH 1/2] powerpc: enable the relocatable support for the fsl booke 32bit kernel
From: Scott Wood @ 2013-06-28  1:52 UTC (permalink / raw)
  To: Kevin Hao; +Cc: linuxppc
In-Reply-To: <20130628013637.GA3173@pek-khao-d1.corp.ad.wrs.com>

On 06/27/2013 08:36:37 PM, Kevin Hao wrote:
> On Thu, Jun 27, 2013 at 02:58:34PM -0500, Scott Wood wrote:
> > On 06/26/2013 09:00:33 PM, Kevin Hao wrote:
> > >This is based on the codes in the head_44x.S. Since we always =20
> align to
> > >256M before mapping the PAGE_OFFSET for a relocatable kernel, we =20
> also
> > >change the init tlb map to 256M size.
> >
> > Why 256M?
>=20
> For two reasons:
>   1. This is the size which both e500v1 and e500v2 support.
>   2. Since we always use the PAGE_OFFSET as 0xc0000000, the 256M is
>      max alignment value we can use for this virtual address.

0xc0000000 is 1G-aligned, so I don't see why 256M is the maximum (after =20
verifying that enough memory is present with the right alignment, of =20
course).  The TLB1 savings would probably not be enough to justify =20
figuring that out, though.

-Scott=

^ permalink raw reply

* Re: [PATCH 1/2] powerpc: enable the relocatable support for the fsl booke 32bit kernel
From: Scott Wood @ 2013-06-28  1:47 UTC (permalink / raw)
  To: Kevin Hao; +Cc: linuxppc
In-Reply-To: <20130628013637.GA3173@pek-khao-d1.corp.ad.wrs.com>

On 06/27/2013 08:36:37 PM, Kevin Hao wrote:
> On Thu, Jun 27, 2013 at 02:58:34PM -0500, Scott Wood wrote:
> > On 06/26/2013 09:00:33 PM, Kevin Hao wrote:
> > >This is based on the codes in the head_44x.S. Since we always =20
> align to
> > >256M before mapping the PAGE_OFFSET for a relocatable kernel, we =20
> also
> > >change the init tlb map to 256M size.
> >
> > Why 256M?
>=20
> For two reasons:
>   1. This is the size which both e500v1 and e500v2 support.
>   2. Since we always use the PAGE_OFFSET as 0xc0000000, the 256M is
>      max alignment value we can use for this virtual address.

Is there any reason why 64M won't continue to work here?

> > This tightens the alignment requirement for dynamic memstart.
>=20
> Yes. But since RELOCATABLE is a superset of DYNAMIC_MEMSTART, we can =20
> always
> use RELOCATABLE instead of DYNAMIC_MEMSTART for fsl booke board in =20
> any cases.

The extra flexibility of RELOCATABLE may help some use cases, but you'd =20
still require the entire 256M naturally aligned region containing the =20
kernel to be present and owned by this instance of Linux.

> So DYNAMIC_MEMSTART will seem not so useful after we enable this =20
> feature.

Then why doesn't this patch remove it?

> >  And
> > what about boards with less than 256 MiB of RAM?
>=20
> It should be fine. We just create the map in the tlb. The MM still use
> the real size of memory.

No, you must not map anything that is not present with a mapping that =20
is executable and/or not guarded, or you could get speculative accesses =20
to who-knows-what.  Even if RAM is present there but owned by some =20
other entity, you could be creating illegal aliases if that other =20
entity mapped it cache-inhibited or similar.

-Scott=

^ permalink raw reply

* Re: [PATCH v2] vfio: add external user support
From: Alexey Kardashevskiy @ 2013-06-28  1:38 UTC (permalink / raw)
  To: Alex Williamson
  Cc: kvm, linux-kernel, Paul Mackerras, linuxppc-dev, David Gibson
In-Reply-To: <1372380089.30572.689.camel@ul30vt.home>

On 06/28/2013 10:41 AM, Alex Williamson wrote:
> On Fri, 2013-06-28 at 08:57 +1000, Alexey Kardashevskiy wrote:
>> On 06/28/2013 01:44 AM, Alex Williamson wrote:
>>> On Thu, 2013-06-27 at 17:14 +1000, Alexey Kardashevskiy wrote:
>>>> VFIO is designed to be used via ioctls on file descriptors
>>>> returned by VFIO.
>>>>
>>>> However in some situations support for an external user is required.
>>>> The first user is KVM on PPC64 (SPAPR TCE protocol) which is going to
>>>> use the existing VFIO groups for exclusive access in real/virtual mode
>>>> in the host kernel to avoid passing map/unmap requests to the user
>>>> space which would made things pretty slow.
>>>>
>>>> The proposed protocol includes:
>>>>
>>>> 1. do normal VFIO init stuff such as opening a new container, attaching
>>>> group(s) to it, setting an IOMMU driver for a container. When IOMMU is
>>>> set for a container, all groups in it are considered ready to use by
>>>> an external user.
>>>>
>>>> 2. pass a fd of the group we want to accelerate to KVM. KVM calls
>>>> vfio_group_iommu_id_from_file() to verify if the group is initialized
>>>> and IOMMU is set for it. The current TCE IOMMU driver marks the whole
>>>> IOMMU table as busy when IOMMU is set for a container what this prevents
>>>> other DMA users from allocating from it so it is safe to pass the group
>>>> to the user space.
>>>>
>>>> 3. KVM increases the container users counter via
>>>> vfio_group_add_external_user(). This prevents the VFIO group from
>>>> being disposed prior to exiting KVM.
>>>>
>>>> 4. When KVM is finished and doing cleanup, it releases the group file
>>>> and decrements the container users counter. Everything gets released.
>>>>
>>>> 5. KVM also keeps the group file as otherwise its fd might have been
>>>> closed at the moment of KVM finish so vfio_group_del_external_user()
>>>> call will not be possible.
>>>
>>> This is the wrong order in my mind.  An external user has no business
>>> checking or maintaining any state of a group until it calls
>>> add_external_user().  Only after that call is successful can the user
>>> assume the filep to group relationship is static and get the iommu_id.
>>> Any use of the "external user" API should start with "add" and end with
>>> "del".
>>
>> Yes, this is what I actually do, just wrong commit message, will fix.
>>
>>>
>>>> The "vfio: Limit group opens" patch is also required for the consistency.
>>>>
>>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>>> ---
>>>>
>>>> v1->v2: added definitions to vfio.h :)
>>>> Should not compile but compiled. Hm.
>>>>
>>>> ---
>>>>  drivers/vfio/vfio.c  |   54 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>  include/linux/vfio.h |    7 +++++++
>>>>  2 files changed, 61 insertions(+)
>>>>
>>>> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
>>>> index c488da5..40875d2 100644
>>>> --- a/drivers/vfio/vfio.c
>>>> +++ b/drivers/vfio/vfio.c
>>>> @@ -1370,6 +1370,60 @@ static const struct file_operations vfio_device_fops = {
>>>>  };
>>>>  
>>>>  /**
>>>> + * External user API, exported by symbols to be linked dynamically.
>>>> + */
>>>> +
>>>> +/* Allows an external user (for example, KVM) to lock an IOMMU group */
>>>> +int vfio_group_add_external_user(struct file *filep)
>>>> +{
>>>> +	struct vfio_group *group = filep->private_data;
>>>> +
>>>> +	if (filep->f_op != &vfio_group_fops)
>>>> +		return -EINVAL;
>>>> +
>>>> +	if (!atomic_inc_not_zero(&group->container_users))
>>>> +		return -EINVAL;
>>>
>>> This is the place where I was suggesting we need tests to match
>>> get_device_fd.  It's not clear what the external user is holding if the
>>> group has no iommu or is not viable here.
>>
>>
>> In my mind this test must include test for iommu id so I would merge it
>> with vfio_group_iommu_id_from_file().
> 
> I'm not sure what that means.

Sorry. Still a mess in my head :( I'll to explain.

vfio_group_add_external_user() should tell if the group is viable and has
iommu (does not the latter include check for viable?).

vfio_group_iommu_id_from_file() tells the group id which has to be compared
by KVM with what KVM got from the userspace and KVM should reject if the
group id is wrong.

So there are 3 checks. KVM can continue if all three passed.

>> Till I check iommu id, I still cannot
>> use this group so where to put check for iommu/viable does not really
>> matter (for me).
> 
> The difference is that getting the group id may just be the first of
> several external user API interfaces.  The idea of external user
> interface is that from add->del the group is maintained in the same
> state as if a device was opened.

Good point.

> If we disassemble that so that add
> sets up some stuff and getting the group id does a little more, what
> happens if we start adding more external user API callbacks?  A user of
> the interface shouldn't need to know the internals to know which
> interface allows what aspect of use.  Besides, I don't want to have to
> worry about managing another state slightly different from that used by
> the device fd.



>>>
>>>
>>> if (!group->container->iommu_driver || !vfio_group_viable(group)) {
>>> 	vfio_group_try_dissolve_container(group);
>>> 	return -EINVAL;
>>> }
>>>
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(vfio_group_add_external_user);
>>>> +
>>>> +/* Allows an external user (for example, KVM) to unlock an IOMMU group */
>>>> +void vfio_group_del_external_user(struct file *filep)
>>>> +{
>>>> +	struct vfio_group *group = filep->private_data;
>>>> +
>>>> +	if (WARN_ON(filep->f_op != &vfio_group_fops))
>>>> +		return;
>>>
>>> How about we make this return int so we can return 0/-EINVAL and the
>>> caller can decide the severity of the response?
>>
>> And what can the caller possibly do on !0?
> 
> What if the caller is just passing a filep from userspace, should they
> be allowed to fill the logs by hitting this WARN_ON?  I don't know where
> it comes from here and whether the caller can return an error to
> userspace.  If this is the same filep that the caller used on add, they
> they can legitimately WARN_ON, but we can't tell if that's the case
> here.  Thanks,

Well, we say that holding file* is a part of API. Why would anyone call
vfio_group_del_external_user() on something but the file* it got when
opened a group fd?


-- 
Alexey

^ permalink raw reply

* Re: [PATCH 1/2] powerpc: enable the relocatable support for the fsl booke 32bit kernel
From: Kevin Hao @ 2013-06-28  1:36 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc
In-Reply-To: <1372363114.8183.54@snotra>

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

On Thu, Jun 27, 2013 at 02:58:34PM -0500, Scott Wood wrote:
> On 06/26/2013 09:00:33 PM, Kevin Hao wrote:
> >This is based on the codes in the head_44x.S. Since we always align to
> >256M before mapping the PAGE_OFFSET for a relocatable kernel, we also
> >change the init tlb map to 256M size.
> 
> Why 256M?

For two reasons:
  1. This is the size which both e500v1 and e500v2 support.
  2. Since we always use the PAGE_OFFSET as 0xc0000000, the 256M is
     max alignment value we can use for this virtual address.

> 
> This tightens the alignment requirement for dynamic memstart.

Yes. But since RELOCATABLE is a superset of DYNAMIC_MEMSTART, we can always
use RELOCATABLE instead of DYNAMIC_MEMSTART for fsl booke board in any cases.
So DYNAMIC_MEMSTART will seem not so useful after we enable this feature.

>  And
> what about boards with less than 256 MiB of RAM?

It should be fine. We just create the map in the tlb. The MM still use
the real size of memory.

> 
> >@@ -176,6 +176,8 @@ skpinv:	addi	r6,r6,1				/* Increment */
> > /* 7. Jump to KERNELBASE mapping */
> > 	lis	r6,(KERNELBASE & ~0xfff)@h
> > 	ori	r6,r6,(KERNELBASE & ~0xfff)@l
> >+	rlwinm	r7,r25,0,4,31
> >+	add	r6,r7,r6
> 
> Please consider using the more readable form of rlwinm/rlwimi:
> 
> 	rlwinm	r7,r25,0,0x0fffffff

Sure.

> 
> > #elif defined(ENTRY_MAPPING_KEXEC_SETUP)
> > /*
> >diff --git a/arch/powerpc/kernel/head_fsl_booke.S
> >b/arch/powerpc/kernel/head_fsl_booke.S
> >index d10a7ca..c3b4c8e53 100644
> >--- a/arch/powerpc/kernel/head_fsl_booke.S
> >+++ b/arch/powerpc/kernel/head_fsl_booke.S
> >@@ -83,10 +83,43 @@ _ENTRY(_start);
> > 	andc	r31,r20,r18		/* r31 = page base */
> > 	or	r31,r31,r19		/* r31 = devtree phys addr */
> > 	mfspr	r30,SPRN_MAS7
> >-
> >-	li	r25,0			/* phys kernel start (low) */
> > 	li	r24,0			/* CPU number */
> >-	li	r23,0			/* phys kernel start (high) */
> >+
> >+#ifdef CONFIG_RELOCATABLE
> >+	bl	0f				/* Get our runtime address */
> >+0:	mflr	r3				/* Make it accessible */
> >+	addis	r3,r3,(_stext - 0b)@ha
> >+	addi	r3,r3,(_stext - 0b)@l 	/* Get our current runtime base */
> >+
> >+	/* Translate _stext address to physical, save in r23/r25 */
> >+	tlbsx	0,r3			/* must succeed */
> >+
> >+	mfspr	r16,SPRN_MAS1
> >+	mfspr	r20,SPRN_MAS3
> >+	rlwinm	r17,r16,25,0x1f		/* r17 = log2(page size) */
> >+	li	r18,1024
> >+	slw	r18,r18,r17		/* r18 = page size */
> >+	addi	r18,r18,-1
> >+	and	r19,r3,r18		/* r19 = page offset */
> >+	andc	r25,r20,r18		/* r25 = page base */
> >+	or	r25,r25,r19		/* r25 = _stext phys addr */
> >+	mfspr	r23,SPRN_MAS7
> 
> This duplicates the code for finding the device tree physical
> address... maybe factor it out into a function?

Sure.

> 
> >@@ -197,7 +230,58 @@ _ENTRY(__early_start)
> >
> > 	bl	early_init
> >
> >-#ifdef CONFIG_DYNAMIC_MEMSTART
> >+#ifdef CONFIG_RELOCATABLE
> >+	/*
> >+	 * Relocatable kernel support based on processing of dynamic
> >+	 * relocation entries.
> >+	 *
> >+	 * r25/r23 will contain RPN/ERPN for the start address of memory
> 
> The start of memory or the start of the kernel?

Should be the start of the kernel. Will fix the comment.

> 
> >+	 */
> >+	lis	r3,kernstart_addr@ha
> >+	la	r3,kernstart_addr@l(r3)
> >+
> >+#ifdef CONFIG_PHYS_64BIT
> >+	stw	r23,0(r3)
> >+	stw	r25,4(r3)
> >+#else
> >+	stw	r25,0(r3)
> >+#endif
> 
> This part looks the same for relocatable and dynamic memstart -- can
> you avoid duplicating?

OK.

> 
> >+	/*
> >+	 * Compute the virt_phys_offset :
> >+	 * virt_phys_offset = stext.run - kernstart_addr
> >+	 *
> >+	 * stext.run = (KERNELBASE & ~0xfffffff) + (kernstart_addr &
> >0xfffffff)
> >+	 * When we relocate, we have :
> >+	 *
> >+	 *	(kernstart_addr & 0xfffffff) = (stext.run & 0xfffffff)
> >+	 *
> >+	 * hence:
> >+	 *  virt_phys_offset = (KERNELBASE & ~0xfffffff) -
> >+	 *                              (kernstart_addr & ~0xfffffff)
> >+	 *
> >+	 */
> >+
> >+	/* KERNELBASE&~0xfffffff => (r4,r5) */
> 
> >+	li	r4, 0		/* higer 32bit */
> >+	lis	r5,KERNELBASE@h
> 
> Please be consistent with whitespace.

Fixed.

Thanks,
Kevin

> 
> -Scott

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply

* Re: [PATCH v2] vfio: add external user support
From: Alex Williamson @ 2013-06-28  0:41 UTC (permalink / raw)
  To: Alexey Kardashevskiy
  Cc: kvm, linux-kernel, Paul Mackerras, linuxppc-dev, David Gibson
In-Reply-To: <51CCC353.7030905@ozlabs.ru>

On Fri, 2013-06-28 at 08:57 +1000, Alexey Kardashevskiy wrote:
> On 06/28/2013 01:44 AM, Alex Williamson wrote:
> > On Thu, 2013-06-27 at 17:14 +1000, Alexey Kardashevskiy wrote:
> >> VFIO is designed to be used via ioctls on file descriptors
> >> returned by VFIO.
> >>
> >> However in some situations support for an external user is required.
> >> The first user is KVM on PPC64 (SPAPR TCE protocol) which is going to
> >> use the existing VFIO groups for exclusive access in real/virtual mode
> >> in the host kernel to avoid passing map/unmap requests to the user
> >> space which would made things pretty slow.
> >>
> >> The proposed protocol includes:
> >>
> >> 1. do normal VFIO init stuff such as opening a new container, attaching
> >> group(s) to it, setting an IOMMU driver for a container. When IOMMU is
> >> set for a container, all groups in it are considered ready to use by
> >> an external user.
> >>
> >> 2. pass a fd of the group we want to accelerate to KVM. KVM calls
> >> vfio_group_iommu_id_from_file() to verify if the group is initialized
> >> and IOMMU is set for it. The current TCE IOMMU driver marks the whole
> >> IOMMU table as busy when IOMMU is set for a container what this prevents
> >> other DMA users from allocating from it so it is safe to pass the group
> >> to the user space.
> >>
> >> 3. KVM increases the container users counter via
> >> vfio_group_add_external_user(). This prevents the VFIO group from
> >> being disposed prior to exiting KVM.
> >>
> >> 4. When KVM is finished and doing cleanup, it releases the group file
> >> and decrements the container users counter. Everything gets released.
> >>
> >> 5. KVM also keeps the group file as otherwise its fd might have been
> >> closed at the moment of KVM finish so vfio_group_del_external_user()
> >> call will not be possible.
> > 
> > This is the wrong order in my mind.  An external user has no business
> > checking or maintaining any state of a group until it calls
> > add_external_user().  Only after that call is successful can the user
> > assume the filep to group relationship is static and get the iommu_id.
> > Any use of the "external user" API should start with "add" and end with
> > "del".
> 
> Yes, this is what I actually do, just wrong commit message, will fix.
> 
> > 
> >> The "vfio: Limit group opens" patch is also required for the consistency.
> >>
> >> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> >> ---
> >>
> >> v1->v2: added definitions to vfio.h :)
> >> Should not compile but compiled. Hm.
> >>
> >> ---
> >>  drivers/vfio/vfio.c  |   54 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >>  include/linux/vfio.h |    7 +++++++
> >>  2 files changed, 61 insertions(+)
> >>
> >> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> >> index c488da5..40875d2 100644
> >> --- a/drivers/vfio/vfio.c
> >> +++ b/drivers/vfio/vfio.c
> >> @@ -1370,6 +1370,60 @@ static const struct file_operations vfio_device_fops = {
> >>  };
> >>  
> >>  /**
> >> + * External user API, exported by symbols to be linked dynamically.
> >> + */
> >> +
> >> +/* Allows an external user (for example, KVM) to lock an IOMMU group */
> >> +int vfio_group_add_external_user(struct file *filep)
> >> +{
> >> +	struct vfio_group *group = filep->private_data;
> >> +
> >> +	if (filep->f_op != &vfio_group_fops)
> >> +		return -EINVAL;
> >> +
> >> +	if (!atomic_inc_not_zero(&group->container_users))
> >> +		return -EINVAL;
> > 
> > This is the place where I was suggesting we need tests to match
> > get_device_fd.  It's not clear what the external user is holding if the
> > group has no iommu or is not viable here.
> 
> 
> In my mind this test must include test for iommu id so I would merge it
> with vfio_group_iommu_id_from_file().

I'm not sure what that means.

> Till I check iommu id, I still cannot
> use this group so where to put check for iommu/viable does not really
> matter (for me).

The difference is that getting the group id may just be the first of
several external user API interfaces.  The idea of external user
interface is that from add->del the group is maintained in the same
state as if a device was opened.  If we disassemble that so that add
sets up some stuff and getting the group id does a little more, what
happens if we start adding more external user API callbacks?  A user of
the interface shouldn't need to know the internals to know which
interface allows what aspect of use.  Besides, I don't want to have to
worry about managing another state slightly different from that used by
the device fd.

> > 
> > 
> > if (!group->container->iommu_driver || !vfio_group_viable(group)) {
> > 	vfio_group_try_dissolve_container(group);
> > 	return -EINVAL;
> > }
> > 
> >> +
> >> +	return 0;
> >> +}
> >> +EXPORT_SYMBOL_GPL(vfio_group_add_external_user);
> >> +
> >> +/* Allows an external user (for example, KVM) to unlock an IOMMU group */
> >> +void vfio_group_del_external_user(struct file *filep)
> >> +{
> >> +	struct vfio_group *group = filep->private_data;
> >> +
> >> +	if (WARN_ON(filep->f_op != &vfio_group_fops))
> >> +		return;
> > 
> > How about we make this return int so we can return 0/-EINVAL and the
> > caller can decide the severity of the response?
> 
> And what can the caller possibly do on !0?

What if the caller is just passing a filep from userspace, should they
be allowed to fill the logs by hitting this WARN_ON?  I don't know where
it comes from here and whether the caller can return an error to
userspace.  If this is the same filep that the caller used on add, they
they can legitimately WARN_ON, but we can't tell if that's the case
here.  Thanks,

Alex

^ permalink raw reply

* Re: [1/1] MPC831x: fix PCI express probing
From: Scott Wood @ 2013-06-27 23:16 UTC (permalink / raw)
  To: Sergey Gerasimov; +Cc: linuxppc-dev
In-Reply-To: <1363687105-24835-1-git-send-email-Sergey.Gerasimov@astrosoft-development.com>

On Tue, Mar 19, 2013 at 10:58:25AM +0100, Sergey Gerasimov wrote:
> For MPC831x the bus probing function also needs the fixup to assign
> addresses to the PCI devices as it was for MPC85xx and MPC86xx.
> The fixup of the bridge vendor and device ID should be done early in
> PCI probing. Else the bridge is not detected as FIXUP_HEADER is called
> too late.
> 
> Signed-off-by: Sergey Gerasimov <Sergey.Gerasimov@astrosoft-development.com>
> 
> ---
> arch/powerpc/sysdev/fsl_pci.c | 67 +++++++++++++++++++------------------------
>  1 file changed, 30 insertions(+), 37 deletions(-)
> 
> diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
> index 682084d..b4f0873 100644
> --- a/arch/powerpc/sysdev/fsl_pci.c
> +++ b/arch/powerpc/sysdev/fsl_pci.c
> @@ -64,6 +64,34 @@ static int __init fsl_pcie_check_link(struct pci_controller *hose)
>  	return 0;
>  }
>  
> +void fsl_pcibios_fixup_bus(struct pci_bus *bus)
> +{
> +	struct pci_controller *hose = (struct pci_controller *) bus->sysdata;
> +	int i;
> +
> +
> +	if ((bus->parent == hose->bus)
> +		&& ((fsl_pcie_bus_fixup
> +			&& pci_bus_find_capability(bus, 0, PCI_CAP_ID_EXP))
> +				|| (hose->indirect_type
> +					& PPC_INDIRECT_TYPE_NO_PCIE_LINK))) {
> +		for (i = 0; i < 4; ++i) {
[snip]
> -void fsl_pcibios_fixup_bus(struct pci_bus *bus)
> -{
> -	struct pci_controller *hose = pci_bus_to_host(bus);
> -	int i, is_pcie = 0, no_link;
> -
> -	/* The root complex bridge comes up with bogus resources,
> -	 * we copy the PHB ones in.
> -	 *
> -	 * With the current generic PCI code, the PHB bus no longer
> -	 * has bus->resource[0..4] set, so things are a bit more
> -	 * tricky.
> -	 */
> -
> -	if (fsl_pcie_bus_fixup)
> -		is_pcie = early_find_capability(hose, 0, 0, PCI_CAP_ID_EXP);
> -	no_link = !!(hose->indirect_type & PPC_INDIRECT_TYPE_NO_PCIE_LINK);
> -
> -	if (bus->parent == hose->bus && (is_pcie || no_link)) {
> -		for (i = 0; i < PCI_BRIDGE_RESOURCE_NUM; ++i) {

It looks like you're reverting commit
13635dfdc6aa8d2890e02dc441decfcb4ae63e14 ("powerpc/fsl/pci: Fix PCIe
fixup regression"), presumably due to a bad merge conflict resolution.

> @@ -515,7 +507,8 @@ no_bridge:
>  }
>  #endif /* CONFIG_FSL_SOC_BOOKE || CONFIG_PPC_86xx */
>  
> -DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID, quirk_fsl_pcie_header);
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID,
> +	quirk_fsl_pcie_header);
>  
>  #if defined(CONFIG_PPC_83xx) || defined(CONFIG_PPC_MPC512x)
>  struct mpc83xx_pcie_priv {

Ben/Kumar, any comments on this, or on needing to call
fsl_pcibios_fixup_bus on 83xx?

-Scott

^ permalink raw reply

* Re: [PATCH v2] vfio: add external user support
From: Alexey Kardashevskiy @ 2013-06-27 22:57 UTC (permalink / raw)
  To: Alex Williamson
  Cc: kvm, linux-kernel, Paul Mackerras, linuxppc-dev, David Gibson
In-Reply-To: <1372347850.30572.659.camel@ul30vt.home>

On 06/28/2013 01:44 AM, Alex Williamson wrote:
> On Thu, 2013-06-27 at 17:14 +1000, Alexey Kardashevskiy wrote:
>> VFIO is designed to be used via ioctls on file descriptors
>> returned by VFIO.
>>
>> However in some situations support for an external user is required.
>> The first user is KVM on PPC64 (SPAPR TCE protocol) which is going to
>> use the existing VFIO groups for exclusive access in real/virtual mode
>> in the host kernel to avoid passing map/unmap requests to the user
>> space which would made things pretty slow.
>>
>> The proposed protocol includes:
>>
>> 1. do normal VFIO init stuff such as opening a new container, attaching
>> group(s) to it, setting an IOMMU driver for a container. When IOMMU is
>> set for a container, all groups in it are considered ready to use by
>> an external user.
>>
>> 2. pass a fd of the group we want to accelerate to KVM. KVM calls
>> vfio_group_iommu_id_from_file() to verify if the group is initialized
>> and IOMMU is set for it. The current TCE IOMMU driver marks the whole
>> IOMMU table as busy when IOMMU is set for a container what this prevents
>> other DMA users from allocating from it so it is safe to pass the group
>> to the user space.
>>
>> 3. KVM increases the container users counter via
>> vfio_group_add_external_user(). This prevents the VFIO group from
>> being disposed prior to exiting KVM.
>>
>> 4. When KVM is finished and doing cleanup, it releases the group file
>> and decrements the container users counter. Everything gets released.
>>
>> 5. KVM also keeps the group file as otherwise its fd might have been
>> closed at the moment of KVM finish so vfio_group_del_external_user()
>> call will not be possible.
> 
> This is the wrong order in my mind.  An external user has no business
> checking or maintaining any state of a group until it calls
> add_external_user().  Only after that call is successful can the user
> assume the filep to group relationship is static and get the iommu_id.
> Any use of the "external user" API should start with "add" and end with
> "del".

Yes, this is what I actually do, just wrong commit message, will fix.

> 
>> The "vfio: Limit group opens" patch is also required for the consistency.
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>>
>> v1->v2: added definitions to vfio.h :)
>> Should not compile but compiled. Hm.
>>
>> ---
>>  drivers/vfio/vfio.c  |   54 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  include/linux/vfio.h |    7 +++++++
>>  2 files changed, 61 insertions(+)
>>
>> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
>> index c488da5..40875d2 100644
>> --- a/drivers/vfio/vfio.c
>> +++ b/drivers/vfio/vfio.c
>> @@ -1370,6 +1370,60 @@ static const struct file_operations vfio_device_fops = {
>>  };
>>  
>>  /**
>> + * External user API, exported by symbols to be linked dynamically.
>> + */
>> +
>> +/* Allows an external user (for example, KVM) to lock an IOMMU group */
>> +int vfio_group_add_external_user(struct file *filep)
>> +{
>> +	struct vfio_group *group = filep->private_data;
>> +
>> +	if (filep->f_op != &vfio_group_fops)
>> +		return -EINVAL;
>> +
>> +	if (!atomic_inc_not_zero(&group->container_users))
>> +		return -EINVAL;
> 
> This is the place where I was suggesting we need tests to match
> get_device_fd.  It's not clear what the external user is holding if the
> group has no iommu or is not viable here.


In my mind this test must include test for iommu id so I would merge it
with vfio_group_iommu_id_from_file(). Till I check iommu id, I still cannot
use this group so where to put check for iommu/viable does not really
matter (for me).

> 
> 
> if (!group->container->iommu_driver || !vfio_group_viable(group)) {
> 	vfio_group_try_dissolve_container(group);
> 	return -EINVAL;
> }
> 
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(vfio_group_add_external_user);
>> +
>> +/* Allows an external user (for example, KVM) to unlock an IOMMU group */
>> +void vfio_group_del_external_user(struct file *filep)
>> +{
>> +	struct vfio_group *group = filep->private_data;
>> +
>> +	if (WARN_ON(filep->f_op != &vfio_group_fops))
>> +		return;
> 
> How about we make this return int so we can return 0/-EINVAL and the
> caller can decide the severity of the response?

And what can the caller possibly do on !0?


-- 
Alexey

^ permalink raw reply

* Re: sgy-cts-1000: New DTS file for Servergy CTS-1000 systems
From: Scott Wood @ 2013-06-27 21:52 UTC (permalink / raw)
  To: Benjamin Collins; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <E1UJ1fR-0004i4-H8@swissweb.swissdisk.com>

On Tue, Mar 19, 2013 at 01:14:22AM -0400, Benjamin Collins wrote:
> This isn't specifically needed in order to build the kernel. It's
> stored in flash with firmware. However, keep it in the kernel for
> reference (and to have an example for fsl_dpa device tree usage).
> 
> Signed-off-by: Ben Collins <ben.c@servergy.com>
> Cc: linuxppc-dev@lists.ozlabs.org
> 
> ---
> arch/powerpc/boot/dts/sgy-cts-1000.dts | 1570 ++++++++++++++++++++++++++++++++
>  1 file changed, 1570 insertions(+)
>  create mode 100644 arch/powerpc/boot/dts/sgy-cts-1000.dts

Applying: sgy-cts-1000: New DTS file for Servergy CTS-1000 systems
/home/scott/fsl/git/linux/upstream/.git/rebase-apply/patch:201: space
/before tab in indent.
  				      84 2 0 0
/home/scott/fsl/git/linux/upstream/.git/rebase-apply/patch:873: trailing
whitespace.
				/* 
warning: 2 lines add whitespace errors.

> diff --git a/arch/powerpc/boot/dts/sgy-cts-1000.dts b/arch/powerpc/boot/dts/sgy-cts-1000.dts
> new file mode 100644
> index 0000000..1efa01a
> --- /dev/null
> +++ b/arch/powerpc/boot/dts/sgy-cts-1000.dts
> @@ -0,0 +1,1570 @@
> +/*
> + * Servergy CTS-1000 Device Tree Source
> + *
> + * Copyright 2009-2011 Freescale Semiconductor Inc.
> + * Copyright 2011-2013 Servergy, Inc.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions are met:
> + *     * Redistributions of source code must retain the above copyright
> + *       notice, this list of conditions and the following disclaimer.
> + *     * Redistributions in binary form must reproduce the above copyright
> + *       notice, this list of conditions and the following disclaimer in the
> + *       documentation and/or other materials provided with the distribution.
> + *     * Neither the name of Freescale Semiconductor nor the
> + *       names of its contributors may be used to endorse or promote products
> + *       derived from this software without specific prior written permission.
> + *
> + *
> + * ALTERNATIVELY, this software may be distributed under the terms of the
> + * GNU General Public License ("GPL") as published by the Free Software
> + * Foundation, either version 2 of that License or (at your option) any
> + * later version.
> + *
> + * THIS SOFTWARE IS PROVIDED BY Freescale Semiconductor ``AS IS'' AND ANY
> + * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
> + * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
> + * DISCLAIMED. IN NO EVENT SHALL Freescale Semiconductor BE LIABLE FOR ANY
> + * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
> + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
> + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
> + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
> + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +/dts-v1/;
> +
> +/ {
> +	model = "sgy,cts-1000";
> +	compatible = "fsl,P4080DS";

This isn't a P4080DS.  Your board needs its own toplevel compatible.

> +		fman0 = &fman0;
> +		fman0_oh0 = &fman0_oh0;
> +		fman0_oh1 = &fman0_oh1;
> +		fman0_oh2 = &fman0_oh2;
> +		fman0_oh3 = &fman0_oh3;
> +		fman0_oh4 = &fman0_oh4;
> +		fman0_oh5 = &fman0_oh5;
> +		fman0_oh6 = &fman0_oh6;
> +		fman0_rx0 = &fman0_rx0;
> +		fman0_rx1 = &fman0_rx1;
> +		fman0_rx2 = &fman0_rx2;
> +		fman0_rx3 = &fman0_rx3;
> +		fman0_rx4 = &fman0_rx4;
> +
> +		fman1 = &fman1;
> +		fman1_oh0 = &fman1_oh0;
> +		fman1_oh1 = &fman1_oh1;
> +		fman1_oh2 = &fman1_oh2;
> +		fman1_oh3 = &fman1_oh3;
> +		fman1_oh4 = &fman1_oh4;
> +		fman1_oh5 = &fman1_oh5;
> +		fman1_oh6 = &fman1_oh6;
> +		fman1_rx0 = &fman1_rx0;
> +		fman1_rx1 = &fman1_rx1;
> +		fman1_rx2 = &fman1_rx2;
> +		fman1_rx3 = &fman1_rx3;
> +		fman1_rx4 = &fman1_rx4;
> +	};

We need a proper device tree binding for this stuff first.

> +	dcsr: dcsr@f00000000 {
> +		ranges = <0x00000000 0xf 0x00000000 0x01008000>;
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		compatible = "fsl,dcsr", "simple-bus";

Please use fsl/p4080si-post.dtsi rather than duplicating all of this.

You appear to have based this on an old SDK kernel, rather than the
current mainline kernel.

> +			hwmon@4c {
> +				compatible = "adt,adt7461";
> +				reg = <0x4c>;
> +				/* 
> +				 * Enabling this causes a flood of interrupts
> +				 * on the ds3232 device (and since it never
> +				 * gets ACK'd, it consumes 100% of a CPU).
> +				 * -- BenC
> +				interrupts = <1 10 0 0
> +					      1 11 0 0>;
> +				 */

"1 10 0 0" and "1 11 0 0" don't look like valid MPIC interrupt specifiers
to me, though.  Maybe that's the problem?  Is it supposed to be:

	<10 1 0 0 11 1 0 0>

?

> +	localbus@ffe124000 {
> +		compatible = "fsl,p4080-elbc", "fsl,elbc", "simple-bus";
> +		reg = <0xf 0xfe124000 0 0x1000>;
> +		interrupts = <25 2 0 0>;
> +		#address-cells = <2>;
> +		#size-cells = <1>;
> +
> +		ranges = <0 0 0xf 0xe8000000 0x08000000>;
> +
> +		/* 256M Flash device */
> +		flash@0,0 {
> +			compatible = "cfi-flash";
> +			reg = <0 0 0x08000000>;
> +			bank-width = <2>;
> +			device-width = <2>;
> +			#address-cells = <0x1>;
> +			#size-cells = <0x1>;
> +
> +			/* 16M for kernel */
> +			partition@0 {
> +				label = "sgyboot-kernel";
> +				reg = <0x00000000 0x01000000>;
> +			};
> +			/* 64M for root fs */
> +			partition@1 {
> +				label = "sgyboot-initrd";
> +				reg = <0x01000000 0x04000000>;
> +			};
> +			/* 256K for FMan microcode */
> +			partition@2 {
> +				label = "fsl-fman-ucode";
> +				reg = <0x05000000 0x00040000>;
> +			};
> +			/* 256K for Device-tree */
> +			partition@3 {
> +				label = "device-tree";
> +				reg = <0x05040000 0x00040000>;
> +			};
> +			/* 128K for u-Boot environment */
> +			partition@4 {
> +				label = "u-boot-env";
> +				reg = <0x05080000 0x00020000>;
> +			};
> +			/* 128K Unused */
> +
> +			/* 1Meg for splash screen in u-boot's UDL driver */
> +			partition@6 {
> +				label = "udl-splash";
> +				reg = <0x050c0000 0x00100000>;
> +			};
> +
> +			/* ~44Megs Unused */
> +
> +			/* 2.5M for u-Boot */
> +			partition@5 {
> +				label = "u-boot";
> +				reg = <0x07D80000 0x00280000>;
> +			};
> +
> +			/* The whole flash area */
> +			partition@20 {
> +				label = "whole";
> +				reg = <0x00000000 0x08000000>;
> +			};

Are overlapping partitions allowed?

Plus, unit addresses are supposed to match reg.

> +	pci0: pcie@ffe200000 {
> +		compatible = "fsl,p4080-pcie";
> +		device_type = "pci";
> +		#size-cells = <2>;
> +		#address-cells = <3>;
> +		reg = <0xf 0xfe200000 0 0x1000>;
> +		bus-range = <0x0 0xff>;
> +		ranges = <0x02000000 0 0xe0000000 0xc 0x00000000 0x0 0x20000000
> +			  0x01000000 0 0x00000000 0xf 0xf8000000 0x0 0x00010000>;
> +		clock-frequency = <0x1fca055>;
> +		fsl,msi = <&msi0>;
> +		interrupts = <16 2 1 15>;

Where does "fsl,msi" come from?

-Scott

^ permalink raw reply

* Re: [v2] edac/85xx: Add PCIe error interrupt edac support
From: Scott Wood @ 2013-06-27 21:25 UTC (permalink / raw)
  To: Chunhe Lan; +Cc: linuxppc-dev
In-Reply-To: <1363250473-30652-1-git-send-email-Chunhe.Lan@freescale.com>

On Thu, Mar 14, 2013 at 04:41:13PM +0800, Chunhe Lan wrote:
> Adding pcie error interrupt edac support for mpc85xx, p3041, p4080,
> and p5020. The mpc85xx uses the legacy interrupt report mechanism -
> the error interrupts are reported directly to mpic. While, the p3041/
> p4080/p5020 attaches the most of error interrupts to interrupt zero.
> And report error interrupts to mpic via interrupt 0.
> 
> This patch can handle both of them.
> 
> Signed-off-by: Chunhe Lan <Chunhe.Lan@freescale.com>
> 
> ---
> drivers/edac/mpc85xx_edac.c |   94 ++++++++++++++++++++++++++++++++++++------
>  drivers/edac/mpc85xx_edac.h |    5 ++
>  2 files changed, 85 insertions(+), 14 deletions(-)

This should go to Doug Thompson and the linux-edac mailing list (see
MAINTAINERS).

I'm not sure what the relevance is of the shared error mechanism of newer
chips -- isn't this driver just going to request whatever interrupt it
finds in the device tree?

-Scott

^ permalink raw reply

* [PATCH] eeh: add eeh_dev to the cache during boot
From: Thadeu Lima de Souza Cascardo @ 2013-06-27 21:00 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: paulus, shangw, Thadeu Lima de Souza Cascardo

commit f8f7d63fd96ead101415a1302035137a866f8998 ("powerpc/eeh: Trace eeh
device from I/O cache") broke EEH on pseries for devices that were
present during boot and have not been hotplugged/DLPARed.

eeh_check_failure will get the eeh_dev from the cache, and will get
NULL. eeh_addr_cache_build adds the addresses to the cache, but eeh_dev
for the giving pci_device is not set yet. Just reordering the call to
eeh_addr_cache_insert_dev works fine. The ordering is similar to the one
in eeh_add_device_late.

Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
---

Note that this is broken since v3.7-rc1 and this patch applies on top of
v3.10-rc7. Just changing the patches will apply it on top of
next-20130626, where it has moved to arch/powerpc/kernel/.

I also realized when writing the log that maybe calling
eeh_add_device_late instead could be a better option. Please, share your
comments.

---
 arch/powerpc/platforms/pseries/eeh_cache.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/eeh_cache.c b/arch/powerpc/platforms/pseries/eeh_cache.c
index 5a4c879..5ce3ba7 100644
--- a/arch/powerpc/platforms/pseries/eeh_cache.c
+++ b/arch/powerpc/platforms/pseries/eeh_cache.c
@@ -294,8 +294,6 @@ void __init eeh_addr_cache_build(void)
 	spin_lock_init(&pci_io_addr_cache_root.piar_lock);
 
 	for_each_pci_dev(dev) {
-		eeh_addr_cache_insert_dev(dev);
-
 		dn = pci_device_to_OF_node(dev);
 		if (!dn)
 			continue;
@@ -308,6 +306,8 @@ void __init eeh_addr_cache_build(void)
 		dev->dev.archdata.edev = edev;
 		edev->pdev = dev;
 
+		eeh_addr_cache_insert_dev(dev);
+
 		eeh_sysfs_add_device(dev);
 	}
 
-- 
1.7.1

^ permalink raw reply related

* Re: [PATCH] of: Specify initrd location using 64-bit
From: Rob Herring @ 2013-06-27 20:54 UTC (permalink / raw)
  To: Santosh Shilimkar
  Cc: Nicolas Pitre, linux-mips, Catalin Marinas, Aurelien Jacquiot,
	Sebastian Andrzej Siewior, Will Deacon, Max Filippov,
	Paul Mackerras, Jonas Bonn, Russell King, linux-c6x-dev, x86, arm,
	Mark Salter, Grant Likely, linux-xtensa, James Hogan,
	devicetree-discuss, Rob Herring, linux-arm-kernel, Chris Zankel,
	Michal Simek, Vineet Gupta, linux-kernel, Ralf Baechle,
	linuxppc-dev
In-Reply-To: <51C48B5A.2040404@ti.com>

On 06/21/2013 12:20 PM, Santosh Shilimkar wrote:
> On Friday 21 June 2013 05:04 AM, Sebastian Andrzej Siewior wrote:
>> On 06/21/2013 02:52 AM, Santosh Shilimkar wrote:
>>> diff --git a/arch/microblaze/kernel/prom.c b/arch/microblaze/kernel/prom.c
>>> index 0a2c68f..62e2e8f 100644
>>> --- a/arch/microblaze/kernel/prom.c
>>> +++ b/arch/microblaze/kernel/prom.c
>>> @@ -136,8 +136,7 @@ void __init early_init_devtree(void *params)
>>>  }
>>>  
>>>  #ifdef CONFIG_BLK_DEV_INITRD
>>> -void __init early_init_dt_setup_initrd_arch(unsigned long start,
>>> -		unsigned long end)
>>> +void __init early_init_dt_setup_initrd_arch(u64 start, u64 end)
>>>  {
>>>  	initrd_start = (unsigned long)__va(start);
>>>  	initrd_end = (unsigned long)__va(end);
>>
>> I think it would better to go here for phys_addr_t instead of u64. This
>> would force you in of_flat_dt_match() to check if the value passed from
>> DT specifies a memory address outside of 32bit address space and the
>> kernel can't deal with this because its phys_addr_t is 32bit only due
>> to a Kconfig switch.
>>
>> For x86, the initrd has to remain in the 32bit address space so passing
>> the initrd in the upper range would violate the ABI. Not sure if this
>> is true for other archs as well (ARM obviously not).
>>
> That pretty much means phys_addr_t. It will work for me as well but
> in last thread from consistency with memory and reserved node, Rob
> insisted to keep it as u64. So before I re-spin another version,
> would like to here what Rob has to say considering the x86 requirement.
> 
> Rob,
> Are you ok with phys_addr_t since your concern was about rest
> of the memory specific bits of the device-tree code use u64 ?

No. I still think it should be u64 for same reasons I said originally.

Rob

^ permalink raw reply

* Re: [PATCH v3 16/45] rcu: Use cpu_is_offline_nocheck() to avoid false-positive warnings
From: Paul E. McKenney @ 2013-06-27 20:12 UTC (permalink / raw)
  To: Srivatsa S. Bhat
  Cc: peterz, fweisbec, linux-kernel, walken, mingo, linux-arch,
	vincent.guittot, xiaoguangrong, wangyun, nikunj, linux-pm, rusty,
	rostedt, namhyung, tglx, laijs, zhong, netdev, oleg, sbw,
	David.Laight, tj, akpm, linuxppc-dev
In-Reply-To: <20130627195517.29830.64108.stgit@srivatsabhat.in.ibm.com>

On Fri, Jun 28, 2013 at 01:25:17AM +0530, Srivatsa S. Bhat wrote:
> In RCU code, rcu_implicit_dynticks_qs() checks if a CPU is offline,
> while being protected by a spinlock. At first, it appears as if we need to
> use the get/put_online_cpus_atomic() APIs to properly synchronize with CPU
> hotplug, once we get rid of stop_machine(). However, RCU has adequate
> synchronization with CPU hotplug, making that unnecessary. But since the
> locking details are non-trivial, it is hard to teach this to the rudimentary
> hotplug locking validator.
> 
> So use the _nocheck() variants of the cpu accessor functions to prevent false-
> positive warnings from the CPU hotplug debug code. Also, add a comment
> explaining the hotplug synchronization design used in RCU, so that its easy
> to see why it is justified to use the _nocheck() variants.
> 
> Cc: Dipankar Sarma <dipankar@in.ibm.com>
> Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>

Reviewed-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

> ---
> 
>  kernel/rcutree.c |   12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/rcutree.c b/kernel/rcutree.c
> index cf3adc6..ced28a45 100644
> --- a/kernel/rcutree.c
> +++ b/kernel/rcutree.c
> @@ -794,7 +794,17 @@ static int rcu_implicit_dynticks_qs(struct rcu_data *rdp)
>  	if (ULONG_CMP_GE(rdp->rsp->gp_start + 2, jiffies))
>  		return 0;  /* Grace period is not old enough. */
>  	barrier();
> -	if (cpu_is_offline(rdp->cpu)) {
> +
> +	/*
> +	 * It is safe to use the _nocheck() version of cpu_is_offline() here
> +	 * (to avoid false-positive warnings from CPU hotplug debug code),
> +	 * because:
> +	 * 1. rcu_gp_init() holds off CPU hotplug operations during grace
> +	 *    period initialization.
> +	 * 2. The current grace period has not ended yet.
> +	 * So it is safe to sample the offline state without synchronization.
> +	 */
> +	if (cpu_is_offline_nocheck(rdp->cpu)) {
>  		trace_rcu_fqs(rdp->rsp->name, rdp->gpnum, rdp->cpu, "ofl");
>  		rdp->offline_fqs++;
>  		return 1;
> 

^ permalink raw reply

* [PATCH v3 45/45] tile: Use get/put_online_cpus_atomic() to prevent CPU offline
From: Srivatsa S. Bhat @ 2013-06-27 20:00 UTC (permalink / raw)
  To: tglx, peterz, tj, oleg, paulmck, rusty, mingo, akpm, namhyung,
	walken, vincent.guittot, laijs, David.Laight
  Cc: linux-arch, nikunj, zhong, linux-pm, Chris Metcalf, fweisbec,
	linux-kernel, rostedt, xiaoguangrong, sbw, wangyun,
	Srivatsa S. Bhat, netdev, linuxppc-dev
In-Reply-To: <20130627195136.29830.10445.stgit@srivatsabhat.in.ibm.com>

Once stop_machine() is gone from the CPU offline path, we won't be able
to depend on disabling preemption to prevent CPUs from going offline
from under us.

Use the get/put_online_cpus_atomic() APIs to prevent CPUs from going
offline, while invoking from atomic context.

Cc: Chris Metcalf <cmetcalf@tilera.com>
Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---

 arch/tile/kernel/module.c |    3 +++
 arch/tile/kernel/tlb.c    |   15 +++++++++++++++
 arch/tile/mm/homecache.c  |    3 +++
 3 files changed, 21 insertions(+)

diff --git a/arch/tile/kernel/module.c b/arch/tile/kernel/module.c
index 4918d91..db7d858 100644
--- a/arch/tile/kernel/module.c
+++ b/arch/tile/kernel/module.c
@@ -20,6 +20,7 @@
 #include <linux/fs.h>
 #include <linux/string.h>
 #include <linux/kernel.h>
+#include <linux/cpu.h>
 #include <asm/pgtable.h>
 #include <asm/homecache.h>
 #include <arch/opcode.h>
@@ -79,8 +80,10 @@ void module_free(struct module *mod, void *module_region)
 	vfree(module_region);
 
 	/* Globally flush the L1 icache. */
+	get_online_cpus_atomic();
 	flush_remote(0, HV_FLUSH_EVICT_L1I, cpu_online_mask,
 		     0, 0, 0, NULL, NULL, 0);
+	put_online_cpus_atomic();
 
 	/*
 	 * FIXME: If module_region == mod->module_init, trim exception
diff --git a/arch/tile/kernel/tlb.c b/arch/tile/kernel/tlb.c
index 3fd54d5..a32b9dd 100644
--- a/arch/tile/kernel/tlb.c
+++ b/arch/tile/kernel/tlb.c
@@ -14,6 +14,7 @@
  */
 
 #include <linux/cpumask.h>
+#include <linux/cpu.h>
 #include <linux/module.h>
 #include <linux/hugetlb.h>
 #include <asm/tlbflush.h>
@@ -35,6 +36,8 @@ void flush_tlb_mm(struct mm_struct *mm)
 {
 	HV_Remote_ASID asids[NR_CPUS];
 	int i = 0, cpu;
+
+	get_online_cpus_atomic();
 	for_each_cpu(cpu, mm_cpumask(mm)) {
 		HV_Remote_ASID *asid = &asids[i++];
 		asid->y = cpu / smp_topology.width;
@@ -43,6 +46,7 @@ void flush_tlb_mm(struct mm_struct *mm)
 	}
 	flush_remote(0, HV_FLUSH_EVICT_L1I, mm_cpumask(mm),
 		     0, 0, 0, NULL, asids, i);
+	put_online_cpus_atomic();
 }
 
 void flush_tlb_current_task(void)
@@ -55,8 +59,11 @@ void flush_tlb_page_mm(struct vm_area_struct *vma, struct mm_struct *mm,
 {
 	unsigned long size = vma_kernel_pagesize(vma);
 	int cache = (vma->vm_flags & VM_EXEC) ? HV_FLUSH_EVICT_L1I : 0;
+
+	get_online_cpus_atomic();
 	flush_remote(0, cache, mm_cpumask(mm),
 		     va, size, size, mm_cpumask(mm), NULL, 0);
+	put_online_cpus_atomic();
 }
 
 void flush_tlb_page(struct vm_area_struct *vma, unsigned long va)
@@ -71,13 +78,18 @@ void flush_tlb_range(struct vm_area_struct *vma,
 	unsigned long size = vma_kernel_pagesize(vma);
 	struct mm_struct *mm = vma->vm_mm;
 	int cache = (vma->vm_flags & VM_EXEC) ? HV_FLUSH_EVICT_L1I : 0;
+
+	get_online_cpus_atomic();
 	flush_remote(0, cache, mm_cpumask(mm), start, end - start, size,
 		     mm_cpumask(mm), NULL, 0);
+	put_online_cpus_atomic();
 }
 
 void flush_tlb_all(void)
 {
 	int i;
+
+	get_online_cpus_atomic();
 	for (i = 0; ; ++i) {
 		HV_VirtAddrRange r = hv_inquire_virtual(i);
 		if (r.size == 0)
@@ -89,10 +101,13 @@ void flush_tlb_all(void)
 			     r.start, r.size, HPAGE_SIZE, cpu_online_mask,
 			     NULL, 0);
 	}
+	put_online_cpus_atomic();
 }
 
 void flush_tlb_kernel_range(unsigned long start, unsigned long end)
 {
+	get_online_cpus_atomic();
 	flush_remote(0, HV_FLUSH_EVICT_L1I, cpu_online_mask,
 		     start, end - start, PAGE_SIZE, cpu_online_mask, NULL, 0);
+	put_online_cpus_atomic();
 }
diff --git a/arch/tile/mm/homecache.c b/arch/tile/mm/homecache.c
index 1ae9119..7ff5bf0 100644
--- a/arch/tile/mm/homecache.c
+++ b/arch/tile/mm/homecache.c
@@ -397,9 +397,12 @@ void homecache_change_page_home(struct page *page, int order, int home)
 	BUG_ON(page_count(page) > 1);
 	BUG_ON(page_mapcount(page) != 0);
 	kva = (unsigned long) page_address(page);
+
+	get_online_cpus_atomic();
 	flush_remote(0, HV_FLUSH_EVICT_L2, &cpu_cacheable_map,
 		     kva, pages * PAGE_SIZE, PAGE_SIZE, cpu_online_mask,
 		     NULL, 0);
+	put_online_cpus_atomic();
 
 	for (i = 0; i < pages; ++i, kva += PAGE_SIZE) {
 		pte_t *ptep = virt_to_pte(NULL, kva);

^ permalink raw reply related

* [PATCH v3 44/45] sparc: Use get/put_online_cpus_atomic() to prevent CPU offline
From: Srivatsa S. Bhat @ 2013-06-27 20:00 UTC (permalink / raw)
  To: tglx, peterz, tj, oleg, paulmck, rusty, mingo, akpm, namhyung,
	walken, vincent.guittot, laijs, David.Laight
  Cc: linux-arch, nikunj, zhong, linux-pm, fweisbec, linux-kernel,
	rostedt, xiaoguangrong, Sam Ravnborg, sbw, Dave Kleikamp, wangyun,
	Srivatsa S. Bhat, netdev, sparclinux, Thomas Gleixner,
	linuxppc-dev, David S. Miller
In-Reply-To: <20130627195136.29830.10445.stgit@srivatsabhat.in.ibm.com>

Once stop_machine() is gone from the CPU offline path, we won't be able
to depend on disabling preemption to prevent CPUs from going offline
from under us.

Use the get/put_online_cpus_atomic() APIs to prevent CPUs from going
offline, while invoking from atomic context.

Cc: "David S. Miller" <davem@davemloft.net>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Dave Kleikamp <dave.kleikamp@oracle.com>
Cc: sparclinux@vger.kernel.org
Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---

 arch/sparc/kernel/smp_64.c |   12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/arch/sparc/kernel/smp_64.c b/arch/sparc/kernel/smp_64.c
index 77539ed..4f71a95 100644
--- a/arch/sparc/kernel/smp_64.c
+++ b/arch/sparc/kernel/smp_64.c
@@ -792,7 +792,9 @@ static void smp_cross_call_masked(unsigned long *func, u32 ctx, u64 data1, u64 d
 /* Send cross call to all processors except self. */
 static void smp_cross_call(unsigned long *func, u32 ctx, u64 data1, u64 data2)
 {
+	get_online_cpus_atomic();
 	smp_cross_call_masked(func, ctx, data1, data2, cpu_online_mask);
+	put_online_cpus_atomic();
 }
 
 extern unsigned long xcall_sync_tick;
@@ -896,7 +898,7 @@ void smp_flush_dcache_page_impl(struct page *page, int cpu)
 	atomic_inc(&dcpage_flushes);
 #endif
 
-	this_cpu = get_cpu();
+	this_cpu = get_online_cpus_atomic();
 
 	if (cpu == this_cpu) {
 		__local_flush_dcache_page(page);
@@ -922,7 +924,7 @@ void smp_flush_dcache_page_impl(struct page *page, int cpu)
 		}
 	}
 
-	put_cpu();
+	put_online_cpus_atomic();
 }
 
 void flush_dcache_page_all(struct mm_struct *mm, struct page *page)
@@ -933,7 +935,7 @@ void flush_dcache_page_all(struct mm_struct *mm, struct page *page)
 	if (tlb_type == hypervisor)
 		return;
 
-	preempt_disable();
+	get_online_cpus_atomic();
 
 #ifdef CONFIG_DEBUG_DCFLUSH
 	atomic_inc(&dcpage_flushes);
@@ -958,7 +960,7 @@ void flush_dcache_page_all(struct mm_struct *mm, struct page *page)
 	}
 	__local_flush_dcache_page(page);
 
-	preempt_enable();
+	put_online_cpus_atomic();
 }
 
 void __irq_entry smp_new_mmu_context_version_client(int irq, struct pt_regs *regs)
@@ -1150,6 +1152,7 @@ void smp_capture(void)
 {
 	int result = atomic_add_ret(1, &smp_capture_depth);
 
+	get_online_cpus_atomic();
 	if (result == 1) {
 		int ncpus = num_online_cpus();
 
@@ -1166,6 +1169,7 @@ void smp_capture(void)
 		printk("done\n");
 #endif
 	}
+	put_online_cpus_atomic();
 }
 
 void smp_release(void)

^ permalink raw reply related

* [PATCH v3 43/45] sh: Use get/put_online_cpus_atomic() to prevent CPU offline
From: Srivatsa S. Bhat @ 2013-06-27 20:00 UTC (permalink / raw)
  To: tglx, peterz, tj, oleg, paulmck, rusty, mingo, akpm, namhyung,
	walken, vincent.guittot, laijs, David.Laight
  Cc: linux-arch, nikunj, zhong, linux-pm, fweisbec, linux-sh,
	linux-kernel, rostedt, xiaoguangrong, sbw, Paul Mundt, wangyun,
	Srivatsa S. Bhat, netdev, Thomas Gleixner, linuxppc-dev
In-Reply-To: <20130627195136.29830.10445.stgit@srivatsabhat.in.ibm.com>

Once stop_machine() is gone from the CPU offline path, we won't be able
to depend on disabling preemption to prevent CPUs from going offline
from under us.

Use the get/put_online_cpus_atomic() APIs to prevent CPUs from going
offline, while invoking from atomic context.

Cc: Paul Mundt <lethal@linux-sh.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-sh@vger.kernel.org
Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---

 arch/sh/kernel/smp.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/sh/kernel/smp.c b/arch/sh/kernel/smp.c
index 4569645..42ec182 100644
--- a/arch/sh/kernel/smp.c
+++ b/arch/sh/kernel/smp.c
@@ -357,7 +357,7 @@ static void flush_tlb_mm_ipi(void *mm)
  */
 void flush_tlb_mm(struct mm_struct *mm)
 {
-	preempt_disable();
+	get_online_cpus_atomic();
 
 	if ((atomic_read(&mm->mm_users) != 1) || (current->mm != mm)) {
 		smp_call_function(flush_tlb_mm_ipi, (void *)mm, 1);
@@ -369,7 +369,7 @@ void flush_tlb_mm(struct mm_struct *mm)
 	}
 	local_flush_tlb_mm(mm);
 
-	preempt_enable();
+	put_online_cpus_atomic();
 }
 
 struct flush_tlb_data {
@@ -390,7 +390,7 @@ void flush_tlb_range(struct vm_area_struct *vma,
 {
 	struct mm_struct *mm = vma->vm_mm;
 
-	preempt_disable();
+	get_online_cpus_atomic();
 	if ((atomic_read(&mm->mm_users) != 1) || (current->mm != mm)) {
 		struct flush_tlb_data fd;
 
@@ -405,7 +405,7 @@ void flush_tlb_range(struct vm_area_struct *vma,
 				cpu_context(i, mm) = 0;
 	}
 	local_flush_tlb_range(vma, start, end);
-	preempt_enable();
+	put_online_cpus_atomic();
 }
 
 static void flush_tlb_kernel_range_ipi(void *info)
@@ -433,7 +433,7 @@ static void flush_tlb_page_ipi(void *info)
 
 void flush_tlb_page(struct vm_area_struct *vma, unsigned long page)
 {
-	preempt_disable();
+	get_online_cpus_atomic();
 	if ((atomic_read(&vma->vm_mm->mm_users) != 1) ||
 	    (current->mm != vma->vm_mm)) {
 		struct flush_tlb_data fd;
@@ -448,7 +448,7 @@ void flush_tlb_page(struct vm_area_struct *vma, unsigned long page)
 				cpu_context(i, vma->vm_mm) = 0;
 	}
 	local_flush_tlb_page(vma, page);
-	preempt_enable();
+	put_online_cpus_atomic();
 }
 
 static void flush_tlb_one_ipi(void *info)

^ permalink raw reply related

* [PATCH v3 42/45] powerpc: Use get/put_online_cpus_atomic() to avoid false-positive warning
From: Srivatsa S. Bhat @ 2013-06-27 20:00 UTC (permalink / raw)
  To: tglx, peterz, tj, oleg, paulmck, rusty, mingo, akpm, namhyung,
	walken, vincent.guittot, laijs, David.Laight
  Cc: linux-arch, nikunj, zhong, linux-pm, fweisbec, linux-kernel,
	rostedt, xiaoguangrong, sbw, Paul Mackerras, wangyun,
	Srivatsa S. Bhat, netdev, Thomas Gleixner, linuxppc-dev,
	Zhao Chenhui
In-Reply-To: <20130627195136.29830.10445.stgit@srivatsabhat.in.ibm.com>

Bringing a secondary CPU online is a special case in which, accessing
the cpu_online_mask is safe, even though that task (which running on the
CPU coming online) is not the hotplug writer.

It is a little hard to teach this to the debugging checks under
CONFIG_DEBUG_HOTPLUG_CPU. But luckily powerpc is one of the few places
where the CPU coming online traverses the cpu_online_mask before fully
coming online. So wrap that part under get/put_online_cpus_atomic(), to
avoid false-positive warnings from the CPU hotplug debug code.

Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Kumar Gala <galak@kernel.crashing.org>
Cc: Zhao Chenhui <chenhui.zhao@freescale.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---

 arch/powerpc/kernel/smp.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 2123bec..59c9a09 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -657,6 +657,7 @@ __cpuinit void start_secondary(void *unused)
 		cpumask_set_cpu(base + i, cpu_core_mask(cpu));
 	}
 	l2_cache = cpu_to_l2cache(cpu);
+	get_online_cpus_atomic();
 	for_each_online_cpu(i) {
 		struct device_node *np = cpu_to_l2cache(i);
 		if (!np)
@@ -667,6 +668,7 @@ __cpuinit void start_secondary(void *unused)
 		}
 		of_node_put(np);
 	}
+	put_online_cpus_atomic();
 	of_node_put(l2_cache);
 
 	local_irq_enable();

^ permalink raw reply related

* [PATCH v3 41/45] powerpc: Use get/put_online_cpus_atomic() to prevent CPU offline
From: Srivatsa S. Bhat @ 2013-06-27 20:00 UTC (permalink / raw)
  To: tglx, peterz, tj, oleg, paulmck, rusty, mingo, akpm, namhyung,
	walken, vincent.guittot, laijs, David.Laight
  Cc: Gleb Natapov, linux-arch, Zhao Chenhui, Alexander Graf,
	xiaoguangrong, oprofile-list, wangyun, fweisbec, Rob Herring,
	cbe-oss-dev, nikunj, linux-pm, rostedt, kvm-ppc, kvm, zhong,
	netdev, linux-kernel, sbw, Srivatsa S. Bhat, linuxppc-dev
In-Reply-To: <20130627195136.29830.10445.stgit@srivatsabhat.in.ibm.com>

Once stop_machine() is gone from the CPU offline path, we won't be able
to depend on disabling preemption to prevent CPUs from going offline
from under us.

Use the get/put_online_cpus_atomic() APIs to prevent CPUs from going
offline, while invoking from atomic context.

Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Gleb Natapov <gleb@redhat.com>
Cc: Alexander Graf <agraf@suse.de>
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Kumar Gala <galak@kernel.crashing.org>
Cc: Zhao Chenhui <chenhui.zhao@freescale.com>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: kvm@vger.kernel.org
Cc: kvm-ppc@vger.kernel.org
Cc: oprofile-list@lists.sf.net
Cc: cbe-oss-dev@lists.ozlabs.org
Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---

 arch/powerpc/kernel/irq.c                  |    7 ++++++-
 arch/powerpc/kernel/machine_kexec_64.c     |    4 ++--
 arch/powerpc/kernel/smp.c                  |    2 ++
 arch/powerpc/kvm/book3s_hv.c               |    5 +++--
 arch/powerpc/mm/mmu_context_nohash.c       |    3 +++
 arch/powerpc/oprofile/cell/spu_profiler.c  |    3 +++
 arch/powerpc/oprofile/cell/spu_task_sync.c |    4 ++++
 arch/powerpc/oprofile/op_model_cell.c      |    6 ++++++
 8 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
index ca39bac..41e9961 100644
--- a/arch/powerpc/kernel/irq.c
+++ b/arch/powerpc/kernel/irq.c
@@ -45,6 +45,7 @@
 #include <linux/irq.h>
 #include <linux/seq_file.h>
 #include <linux/cpumask.h>
+#include <linux/cpu.h>
 #include <linux/profile.h>
 #include <linux/bitops.h>
 #include <linux/list.h>
@@ -410,7 +411,10 @@ void migrate_irqs(void)
 	unsigned int irq;
 	static int warned;
 	cpumask_var_t mask;
-	const struct cpumask *map = cpu_online_mask;
+	const struct cpumask *map;
+
+	get_online_cpus_atomic();
+	map = cpu_online_mask;
 
 	alloc_cpumask_var(&mask, GFP_ATOMIC);
 
@@ -436,6 +440,7 @@ void migrate_irqs(void)
 	}
 
 	free_cpumask_var(mask);
+	put_online_cpus_atomic();
 
 	local_irq_enable();
 	mdelay(1);
diff --git a/arch/powerpc/kernel/machine_kexec_64.c b/arch/powerpc/kernel/machine_kexec_64.c
index 611acdf..38f6d75 100644
--- a/arch/powerpc/kernel/machine_kexec_64.c
+++ b/arch/powerpc/kernel/machine_kexec_64.c
@@ -187,7 +187,7 @@ static void kexec_prepare_cpus_wait(int wait_state)
 	int my_cpu, i, notified=-1;
 
 	hw_breakpoint_disable();
-	my_cpu = get_cpu();
+	my_cpu = get_online_cpus_atomic();
 	/* Make sure each CPU has at least made it to the state we need.
 	 *
 	 * FIXME: There is a (slim) chance of a problem if not all of the CPUs
@@ -266,7 +266,7 @@ static void kexec_prepare_cpus(void)
 	 */
 	kexec_prepare_cpus_wait(KEXEC_STATE_REAL_MODE);
 
-	put_cpu();
+	put_online_cpus_atomic();
 }
 
 #else /* ! SMP */
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index ee7ac5e..2123bec 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -277,9 +277,11 @@ void smp_send_debugger_break(void)
 	if (unlikely(!smp_ops))
 		return;
 
+	get_online_cpus_atomic();
 	for_each_online_cpu(cpu)
 		if (cpu != me)
 			do_message_pass(cpu, PPC_MSG_DEBUGGER_BREAK);
+	put_online_cpus_atomic();
 }
 #endif
 
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 2efa9dd..9d8a973 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -28,6 +28,7 @@
 #include <linux/fs.h>
 #include <linux/anon_inodes.h>
 #include <linux/cpumask.h>
+#include <linux/cpu.h>
 #include <linux/spinlock.h>
 #include <linux/page-flags.h>
 #include <linux/srcu.h>
@@ -78,7 +79,7 @@ void kvmppc_fast_vcpu_kick(struct kvm_vcpu *vcpu)
 		++vcpu->stat.halt_wakeup;
 	}
 
-	me = get_cpu();
+	me = get_online_cpus_atomic();
 
 	/* CPU points to the first thread of the core */
 	if (cpu != me && cpu >= 0 && cpu < nr_cpu_ids) {
@@ -88,7 +89,7 @@ void kvmppc_fast_vcpu_kick(struct kvm_vcpu *vcpu)
 		else if (cpu_online(cpu))
 			smp_send_reschedule(cpu);
 	}
-	put_cpu();
+	put_online_cpus_atomic();
 }
 
 /*
diff --git a/arch/powerpc/mm/mmu_context_nohash.c b/arch/powerpc/mm/mmu_context_nohash.c
index e779642..c7bdcb4 100644
--- a/arch/powerpc/mm/mmu_context_nohash.c
+++ b/arch/powerpc/mm/mmu_context_nohash.c
@@ -194,6 +194,8 @@ void switch_mmu_context(struct mm_struct *prev, struct mm_struct *next)
 	unsigned int i, id, cpu = smp_processor_id();
 	unsigned long *map;
 
+	get_online_cpus_atomic();
+
 	/* No lockless fast path .. yet */
 	raw_spin_lock(&context_lock);
 
@@ -280,6 +282,7 @@ void switch_mmu_context(struct mm_struct *prev, struct mm_struct *next)
 	pr_hardcont(" -> %d\n", id);
 	set_context(id, next->pgd);
 	raw_spin_unlock(&context_lock);
+	put_online_cpus_atomic();
 }
 
 /*
diff --git a/arch/powerpc/oprofile/cell/spu_profiler.c b/arch/powerpc/oprofile/cell/spu_profiler.c
index b129d00..ab6e6c1 100644
--- a/arch/powerpc/oprofile/cell/spu_profiler.c
+++ b/arch/powerpc/oprofile/cell/spu_profiler.c
@@ -14,6 +14,7 @@
 
 #include <linux/hrtimer.h>
 #include <linux/smp.h>
+#include <linux/cpu.h>
 #include <linux/slab.h>
 #include <asm/cell-pmu.h>
 #include <asm/time.h>
@@ -142,6 +143,7 @@ static enum hrtimer_restart profile_spus(struct hrtimer *timer)
 	if (!spu_prof_running)
 		goto stop;
 
+	get_online_cpus_atomic();
 	for_each_online_cpu(cpu) {
 		if (cbe_get_hw_thread_id(cpu))
 			continue;
@@ -177,6 +179,7 @@ static enum hrtimer_restart profile_spus(struct hrtimer *timer)
 				       oprof_spu_smpl_arry_lck_flags);
 
 	}
+	put_online_cpus_atomic();
 	smp_wmb();	/* insure spu event buffer updates are written */
 			/* don't want events intermingled... */
 
diff --git a/arch/powerpc/oprofile/cell/spu_task_sync.c b/arch/powerpc/oprofile/cell/spu_task_sync.c
index 28f1af2..8464ef6 100644
--- a/arch/powerpc/oprofile/cell/spu_task_sync.c
+++ b/arch/powerpc/oprofile/cell/spu_task_sync.c
@@ -28,6 +28,7 @@
 #include <linux/oprofile.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
+#include <linux/cpu.h>
 #include "pr_util.h"
 
 #define RELEASE_ALL 9999
@@ -448,11 +449,14 @@ static int number_of_online_nodes(void)
 {
         u32 cpu; u32 tmp;
         int nodes = 0;
+
+	get_online_cpus_atomic();
         for_each_online_cpu(cpu) {
                 tmp = cbe_cpu_to_node(cpu) + 1;
                 if (tmp > nodes)
                         nodes++;
         }
+	put_online_cpus_atomic();
         return nodes;
 }
 
diff --git a/arch/powerpc/oprofile/op_model_cell.c b/arch/powerpc/oprofile/op_model_cell.c
index b9589c1..c9bb028 100644
--- a/arch/powerpc/oprofile/op_model_cell.c
+++ b/arch/powerpc/oprofile/op_model_cell.c
@@ -22,6 +22,7 @@
 #include <linux/oprofile.h>
 #include <linux/percpu.h>
 #include <linux/smp.h>
+#include <linux/cpu.h>
 #include <linux/spinlock.h>
 #include <linux/timer.h>
 #include <asm/cell-pmu.h>
@@ -463,6 +464,7 @@ static void cell_virtual_cntr(unsigned long data)
 	 * not both playing with the counters on the same node.
 	 */
 
+	get_online_cpus_atomic();
 	spin_lock_irqsave(&cntr_lock, flags);
 
 	prev_hdw_thread = hdw_thread;
@@ -550,6 +552,7 @@ static void cell_virtual_cntr(unsigned long data)
 	}
 
 	spin_unlock_irqrestore(&cntr_lock, flags);
+	put_online_cpus_atomic();
 
 	mod_timer(&timer_virt_cntr, jiffies + HZ / 10);
 }
@@ -608,6 +611,8 @@ static void spu_evnt_swap(unsigned long data)
 	/* Make sure spu event interrupt handler and spu event swap
 	 * don't access the counters simultaneously.
 	 */
+
+	get_online_cpus_atomic();
 	spin_lock_irqsave(&cntr_lock, flags);
 
 	cur_spu_evnt_phys_spu_indx = spu_evnt_phys_spu_indx;
@@ -673,6 +678,7 @@ static void spu_evnt_swap(unsigned long data)
 	}
 
 	spin_unlock_irqrestore(&cntr_lock, flags);
+	put_online_cpus_atomic();
 
 	/* swap approximately every 0.1 seconds */
 	mod_timer(&timer_spu_event_swap, jiffies + HZ / 25);

^ permalink raw reply related

* [PATCH v3 40/45] powerpc, irq: Use GFP_ATOMIC allocations in atomic context
From: Srivatsa S. Bhat @ 2013-06-27 19:59 UTC (permalink / raw)
  To: tglx, peterz, tj, oleg, paulmck, rusty, mingo, akpm, namhyung,
	walken, vincent.guittot, laijs, David.Laight
  Cc: linux-arch, nikunj, Li Zhong, linux-pm, fweisbec, linux-kernel,
	Steven Rostedt, xiaoguangrong, sbw, Paul Mackerras, wangyun,
	Srivatsa S. Bhat, netdev, linuxppc-dev, Ian Munsie
In-Reply-To: <20130627195136.29830.10445.stgit@srivatsabhat.in.ibm.com>

The function migrate_irqs() is called with interrupts disabled
and hence its not safe to do GFP_KERNEL allocations inside it,
because they can sleep. So change the gfp mask to GFP_ATOMIC.

Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Michael Ellerman <michael@ellerman.id.au>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Ian Munsie <imunsie@au1.ibm.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Michael Ellerman <michael@ellerman.id.au>
Cc: Li Zhong <zhong@linux.vnet.ibm.com>
Cc: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---

 arch/powerpc/kernel/irq.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
index ea185e0..ca39bac 100644
--- a/arch/powerpc/kernel/irq.c
+++ b/arch/powerpc/kernel/irq.c
@@ -412,7 +412,7 @@ void migrate_irqs(void)
 	cpumask_var_t mask;
 	const struct cpumask *map = cpu_online_mask;
 
-	alloc_cpumask_var(&mask, GFP_KERNEL);
+	alloc_cpumask_var(&mask, GFP_ATOMIC);
 
 	for_each_irq_desc(irq, desc) {
 		struct irq_data *data;

^ permalink raw reply related

* [PATCH v3 39/45] mn10300: Use get/put_online_cpus_atomic() to prevent CPU offline
From: Srivatsa S. Bhat @ 2013-06-27 19:59 UTC (permalink / raw)
  To: tglx, peterz, tj, oleg, paulmck, rusty, mingo, akpm, namhyung,
	walken, vincent.guittot, laijs, David.Laight
  Cc: linux-arch, linux-am33-list, nikunj, zhong, linux-pm, fweisbec,
	linux-kernel, rostedt, xiaoguangrong, David Howells, sbw, wangyun,
	Srivatsa S. Bhat, netdev, Koichi Yasutake, linuxppc-dev
In-Reply-To: <20130627195136.29830.10445.stgit@srivatsabhat.in.ibm.com>

Once stop_machine() is gone from the CPU offline path, we won't be able
to depend on disabling preemption to prevent CPUs from going offline
from under us.

Use the get/put_online_cpus_atomic() APIs to prevent CPUs from going
offline, while invoking from atomic context.

Cc: David Howells <dhowells@redhat.com>
Cc: Koichi Yasutake <yasutake.koichi@jp.panasonic.com>
Cc: linux-am33-list@redhat.com
Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---

 arch/mn10300/mm/cache-smp.c |    3 +++
 arch/mn10300/mm/tlb-smp.c   |   17 +++++++++--------
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/arch/mn10300/mm/cache-smp.c b/arch/mn10300/mm/cache-smp.c
index 2d23b9e..406357d 100644
--- a/arch/mn10300/mm/cache-smp.c
+++ b/arch/mn10300/mm/cache-smp.c
@@ -13,6 +13,7 @@
 #include <linux/mman.h>
 #include <linux/threads.h>
 #include <linux/interrupt.h>
+#include <linux/cpu.h>
 #include <asm/page.h>
 #include <asm/pgtable.h>
 #include <asm/processor.h>
@@ -91,6 +92,7 @@ void smp_cache_interrupt(void)
 void smp_cache_call(unsigned long opr_mask,
 		    unsigned long start, unsigned long end)
 {
+	get_online_cpus_atomic();
 	smp_cache_mask = opr_mask;
 	smp_cache_start = start;
 	smp_cache_end = end;
@@ -102,4 +104,5 @@ void smp_cache_call(unsigned long opr_mask,
 	while (!cpumask_empty(&smp_cache_ipi_map))
 		/* nothing. lockup detection does not belong here */
 		mb();
+	put_online_cpus_atomic();
 }
diff --git a/arch/mn10300/mm/tlb-smp.c b/arch/mn10300/mm/tlb-smp.c
index 3e57faf..8856fd3 100644
--- a/arch/mn10300/mm/tlb-smp.c
+++ b/arch/mn10300/mm/tlb-smp.c
@@ -23,6 +23,7 @@
 #include <linux/sched.h>
 #include <linux/profile.h>
 #include <linux/smp.h>
+#include <linux/cpu.h>
 #include <asm/tlbflush.h>
 #include <asm/bitops.h>
 #include <asm/processor.h>
@@ -61,7 +62,7 @@ void smp_flush_tlb(void *unused)
 {
 	unsigned long cpu_id;
 
-	cpu_id = get_cpu();
+	cpu_id = get_online_cpus_atomic();
 
 	if (!cpumask_test_cpu(cpu_id, &flush_cpumask))
 		/* This was a BUG() but until someone can quote me the line
@@ -82,7 +83,7 @@ void smp_flush_tlb(void *unused)
 	cpumask_clear_cpu(cpu_id, &flush_cpumask);
 	smp_mb__after_clear_bit();
 out:
-	put_cpu();
+	put_online_cpus_atomic();
 }
 
 /**
@@ -144,7 +145,7 @@ void flush_tlb_mm(struct mm_struct *mm)
 {
 	cpumask_t cpu_mask;
 
-	preempt_disable();
+	get_online_cpus_atomic();
 	cpumask_copy(&cpu_mask, mm_cpumask(mm));
 	cpumask_clear_cpu(smp_processor_id(), &cpu_mask);
 
@@ -152,7 +153,7 @@ void flush_tlb_mm(struct mm_struct *mm)
 	if (!cpumask_empty(&cpu_mask))
 		flush_tlb_others(cpu_mask, mm, FLUSH_ALL);
 
-	preempt_enable();
+	put_online_cpus_atomic();
 }
 
 /**
@@ -163,7 +164,7 @@ void flush_tlb_current_task(void)
 	struct mm_struct *mm = current->mm;
 	cpumask_t cpu_mask;
 
-	preempt_disable();
+	get_online_cpus_atomic();
 	cpumask_copy(&cpu_mask, mm_cpumask(mm));
 	cpumask_clear_cpu(smp_processor_id(), &cpu_mask);
 
@@ -171,7 +172,7 @@ void flush_tlb_current_task(void)
 	if (!cpumask_empty(&cpu_mask))
 		flush_tlb_others(cpu_mask, mm, FLUSH_ALL);
 
-	preempt_enable();
+	put_online_cpus_atomic();
 }
 
 /**
@@ -184,7 +185,7 @@ void flush_tlb_page(struct vm_area_struct *vma, unsigned long va)
 	struct mm_struct *mm = vma->vm_mm;
 	cpumask_t cpu_mask;
 
-	preempt_disable();
+	get_online_cpus_atomic();
 	cpumask_copy(&cpu_mask, mm_cpumask(mm));
 	cpumask_clear_cpu(smp_processor_id(), &cpu_mask);
 
@@ -192,7 +193,7 @@ void flush_tlb_page(struct vm_area_struct *vma, unsigned long va)
 	if (!cpumask_empty(&cpu_mask))
 		flush_tlb_others(cpu_mask, mm, va);
 
-	preempt_enable();
+	put_online_cpus_atomic();
 }
 
 /**

^ permalink raw reply related

* [PATCH v3 38/45] MIPS: Use get/put_online_cpus_atomic() to prevent CPU offline
From: Srivatsa S. Bhat @ 2013-06-27 19:59 UTC (permalink / raw)
  To: tglx, peterz, tj, oleg, paulmck, rusty, mingo, akpm, namhyung,
	walken, vincent.guittot, laijs, David.Laight
  Cc: linux-mips, David Daney, fweisbec, linux-arch, xiaoguangrong,
	Steven J. Hill, wangyun, nikunj, linux-pm, rostedt, Sanjay Lal,
	Thomas Gleixner, Florian Fainelli, John Crispin, zhong,
	Yong Zhang, netdev, linux-kernel, Ralf Baechle, sbw,
	Srivatsa S. Bhat, linuxppc-dev
In-Reply-To: <20130627195136.29830.10445.stgit@srivatsabhat.in.ibm.com>

Once stop_machine() is gone from the CPU offline path, we won't be able
to depend on disabling preemption to prevent CPUs from going offline
from under us.

Use the get/put_online_cpus_atomic() APIs to prevent CPUs from going
offline, while invoking from atomic context.

Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: David Daney <david.daney@cavium.com>
Cc: Yong Zhang <yong.zhang0@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Sanjay Lal <sanjayl@kymasys.com>
Cc: "Steven J. Hill" <sjhill@mips.com>
Cc: John Crispin <blogic@openwrt.org>
Cc: Florian Fainelli <florian@openwrt.org>
Cc: linux-mips@linux-mips.org
Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---

 arch/mips/kernel/cevt-smtc.c |    7 +++++++
 arch/mips/kernel/smp.c       |   16 ++++++++--------
 arch/mips/kernel/smtc.c      |   12 ++++++++++++
 arch/mips/mm/c-octeon.c      |    4 ++--
 arch/mips/mm/c-r4k.c         |    5 +++--
 5 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/arch/mips/kernel/cevt-smtc.c b/arch/mips/kernel/cevt-smtc.c
index 9de5ed7..2e6c0cd 100644
--- a/arch/mips/kernel/cevt-smtc.c
+++ b/arch/mips/kernel/cevt-smtc.c
@@ -11,6 +11,7 @@
 #include <linux/interrupt.h>
 #include <linux/percpu.h>
 #include <linux/smp.h>
+#include <linux/cpu.h>
 #include <linux/irq.h>
 
 #include <asm/smtc_ipi.h>
@@ -84,6 +85,8 @@ static int mips_next_event(unsigned long delta,
 	unsigned long nextcomp = 0L;
 	int vpe = current_cpu_data.vpe_id;
 	int cpu = smp_processor_id();
+
+	get_online_cpus_atomic();
 	local_irq_save(flags);
 	mtflags = dmt();
 
@@ -164,6 +167,7 @@ static int mips_next_event(unsigned long delta,
 	}
 	emt(mtflags);
 	local_irq_restore(flags);
+	put_online_cpus_atomic();
 	return 0;
 }
 
@@ -177,6 +181,7 @@ void smtc_distribute_timer(int vpe)
 	unsigned long nextstamp;
 	unsigned long reference;
 
+	get_online_cpus_atomic();
 
 repeat:
 	nextstamp = 0L;
@@ -229,6 +234,8 @@ repeat:
 			> (unsigned long)LONG_MAX)
 				goto repeat;
 	}
+
+	put_online_cpus_atomic();
 }
 
 
diff --git a/arch/mips/kernel/smp.c b/arch/mips/kernel/smp.c
index 6e7862a..be152b6 100644
--- a/arch/mips/kernel/smp.c
+++ b/arch/mips/kernel/smp.c
@@ -250,12 +250,12 @@ static inline void smp_on_other_tlbs(void (*func) (void *info), void *info)
 
 static inline void smp_on_each_tlb(void (*func) (void *info), void *info)
 {
-	preempt_disable();
+	get_online_cpus_atomic();
 
 	smp_on_other_tlbs(func, info);
 	func(info);
 
-	preempt_enable();
+	put_online_cpus_atomic();
 }
 
 /*
@@ -273,7 +273,7 @@ static inline void smp_on_each_tlb(void (*func) (void *info), void *info)
 
 void flush_tlb_mm(struct mm_struct *mm)
 {
-	preempt_disable();
+	get_online_cpus_atomic();
 
 	if ((atomic_read(&mm->mm_users) != 1) || (current->mm != mm)) {
 		smp_on_other_tlbs(flush_tlb_mm_ipi, mm);
@@ -287,7 +287,7 @@ void flush_tlb_mm(struct mm_struct *mm)
 	}
 	local_flush_tlb_mm(mm);
 
-	preempt_enable();
+	put_online_cpus_atomic();
 }
 
 struct flush_tlb_data {
@@ -307,7 +307,7 @@ void flush_tlb_range(struct vm_area_struct *vma, unsigned long start, unsigned l
 {
 	struct mm_struct *mm = vma->vm_mm;
 
-	preempt_disable();
+	get_online_cpus_atomic();
 	if ((atomic_read(&mm->mm_users) != 1) || (current->mm != mm)) {
 		struct flush_tlb_data fd = {
 			.vma = vma,
@@ -325,7 +325,7 @@ void flush_tlb_range(struct vm_area_struct *vma, unsigned long start, unsigned l
 		}
 	}
 	local_flush_tlb_range(vma, start, end);
-	preempt_enable();
+	put_online_cpus_atomic();
 }
 
 static void flush_tlb_kernel_range_ipi(void *info)
@@ -354,7 +354,7 @@ static void flush_tlb_page_ipi(void *info)
 
 void flush_tlb_page(struct vm_area_struct *vma, unsigned long page)
 {
-	preempt_disable();
+	get_online_cpus_atomic();
 	if ((atomic_read(&vma->vm_mm->mm_users) != 1) || (current->mm != vma->vm_mm)) {
 		struct flush_tlb_data fd = {
 			.vma = vma,
@@ -371,7 +371,7 @@ void flush_tlb_page(struct vm_area_struct *vma, unsigned long page)
 		}
 	}
 	local_flush_tlb_page(vma, page);
-	preempt_enable();
+	put_online_cpus_atomic();
 }
 
 static void flush_tlb_one_ipi(void *info)
diff --git a/arch/mips/kernel/smtc.c b/arch/mips/kernel/smtc.c
index 75a4fd7..3cda8eb 100644
--- a/arch/mips/kernel/smtc.c
+++ b/arch/mips/kernel/smtc.c
@@ -21,6 +21,7 @@
 #include <linux/kernel.h>
 #include <linux/sched.h>
 #include <linux/smp.h>
+#include <linux/cpu.h>
 #include <linux/cpumask.h>
 #include <linux/interrupt.h>
 #include <linux/kernel_stat.h>
@@ -1143,6 +1144,8 @@ static irqreturn_t ipi_interrupt(int irq, void *dev_idm)
 	 * for the current TC, so we ought not to have to do it explicitly here.
 	 */
 
+	get_online_cpus_atomic();
+
 	for_each_online_cpu(cpu) {
 		if (cpu_data[cpu].vpe_id != my_vpe)
 			continue;
@@ -1180,6 +1183,8 @@ static irqreturn_t ipi_interrupt(int irq, void *dev_idm)
 		}
 	}
 
+	put_online_cpus_atomic();
+
 	return IRQ_HANDLED;
 }
 
@@ -1383,6 +1388,7 @@ void smtc_get_new_mmu_context(struct mm_struct *mm, unsigned long cpu)
 	 * them, let's be really careful...
 	 */
 
+	get_online_cpus_atomic();
 	local_irq_save(flags);
 	if (smtc_status & SMTC_TLB_SHARED) {
 		mtflags = dvpe();
@@ -1438,6 +1444,7 @@ void smtc_get_new_mmu_context(struct mm_struct *mm, unsigned long cpu)
 	else
 		emt(mtflags);
 	local_irq_restore(flags);
+	put_online_cpus_atomic();
 }
 
 /*
@@ -1496,6 +1503,7 @@ void smtc_cflush_lockdown(void)
 {
 	int cpu;
 
+	get_online_cpus_atomic();
 	for_each_online_cpu(cpu) {
 		if (cpu != smp_processor_id()) {
 			settc(cpu_data[cpu].tc_id);
@@ -1504,6 +1512,7 @@ void smtc_cflush_lockdown(void)
 		}
 	}
 	mips_ihb();
+	put_online_cpus_atomic();
 }
 
 /* It would be cheating to change the cpu_online states during a flush! */
@@ -1512,6 +1521,8 @@ void smtc_cflush_release(void)
 {
 	int cpu;
 
+	get_online_cpus_atomic();
+
 	/*
 	 * Start with a hazard barrier to ensure
 	 * that all CACHE ops have played through.
@@ -1525,4 +1536,5 @@ void smtc_cflush_release(void)
 		}
 	}
 	mips_ihb();
+	put_online_cpus_atomic();
 }
diff --git a/arch/mips/mm/c-octeon.c b/arch/mips/mm/c-octeon.c
index 8557fb5..8e1bcf6 100644
--- a/arch/mips/mm/c-octeon.c
+++ b/arch/mips/mm/c-octeon.c
@@ -73,7 +73,7 @@ static void octeon_flush_icache_all_cores(struct vm_area_struct *vma)
 	mb();
 	octeon_local_flush_icache();
 #ifdef CONFIG_SMP
-	preempt_disable();
+	get_online_cpus_atomic();
 	cpu = smp_processor_id();
 
 	/*
@@ -88,7 +88,7 @@ static void octeon_flush_icache_all_cores(struct vm_area_struct *vma)
 	for_each_cpu(cpu, &mask)
 		octeon_send_ipi_single(cpu, SMP_ICACHE_FLUSH);
 
-	preempt_enable();
+	put_online_cpus_atomic();
 #endif
 }
 
diff --git a/arch/mips/mm/c-r4k.c b/arch/mips/mm/c-r4k.c
index 21813be..86c8b3e 100644
--- a/arch/mips/mm/c-r4k.c
+++ b/arch/mips/mm/c-r4k.c
@@ -14,6 +14,7 @@
 #include <linux/linkage.h>
 #include <linux/sched.h>
 #include <linux/smp.h>
+#include <linux/cpu.h>
 #include <linux/mm.h>
 #include <linux/module.h>
 #include <linux/bitops.h>
@@ -46,13 +47,13 @@
  */
 static inline void r4k_on_each_cpu(void (*func) (void *info), void *info)
 {
-	preempt_disable();
+	get_online_cpus_atomic();
 
 #if !defined(CONFIG_MIPS_MT_SMP) && !defined(CONFIG_MIPS_MT_SMTC)
 	smp_call_function(func, info, 1);
 #endif
 	func(info);
-	preempt_enable();
+	put_online_cpus_atomic();
 }
 
 #if defined(CONFIG_MIPS_CMP)

^ permalink raw reply related

* [PATCH v3 37/45] m32r: Use get/put_online_cpus_atomic() to prevent CPU offline
From: Srivatsa S. Bhat @ 2013-06-27 19:59 UTC (permalink / raw)
  To: tglx, peterz, tj, oleg, paulmck, rusty, mingo, akpm, namhyung,
	walken, vincent.guittot, laijs, David.Laight
  Cc: linux-arch, linux-m32r-ja, linux-m32r, nikunj, zhong, linux-pm,
	fweisbec, Hirokazu Takata, linux-kernel, rostedt, xiaoguangrong,
	sbw, wangyun, Srivatsa S. Bhat, netdev, linuxppc-dev
In-Reply-To: <20130627195136.29830.10445.stgit@srivatsabhat.in.ibm.com>

Once stop_machine() is gone from the CPU offline path, we won't be able
to depend on disabling preemption to prevent CPUs from going offline
from under us.

Use the get/put_online_cpus_atomic() APIs to prevent CPUs from going
offline, while invoking from atomic context.

Cc: Hirokazu Takata <takata@linux-m32r.org>
Cc: linux-m32r@ml.linux-m32r.org
Cc: linux-m32r-ja@ml.linux-m32r.org
Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---

 arch/m32r/kernel/smp.c |   16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/m32r/kernel/smp.c b/arch/m32r/kernel/smp.c
index ce7aea3..ffafdba 100644
--- a/arch/m32r/kernel/smp.c
+++ b/arch/m32r/kernel/smp.c
@@ -151,7 +151,7 @@ void smp_flush_cache_all(void)
 	cpumask_t cpumask;
 	unsigned long *mask;
 
-	preempt_disable();
+	get_online_cpus_atomic();
 	cpumask_copy(&cpumask, cpu_online_mask);
 	cpumask_clear_cpu(smp_processor_id(), &cpumask);
 	spin_lock(&flushcache_lock);
@@ -162,7 +162,7 @@ void smp_flush_cache_all(void)
 	while (flushcache_cpumask)
 		mb();
 	spin_unlock(&flushcache_lock);
-	preempt_enable();
+	put_online_cpus_atomic();
 }
 
 void smp_flush_cache_all_interrupt(void)
@@ -197,12 +197,12 @@ void smp_flush_tlb_all(void)
 {
 	unsigned long flags;
 
-	preempt_disable();
+	get_online_cpus_atomic();
 	local_irq_save(flags);
 	__flush_tlb_all();
 	local_irq_restore(flags);
 	smp_call_function(flush_tlb_all_ipi, NULL, 1);
-	preempt_enable();
+	put_online_cpus_atomic();
 }
 
 /*==========================================================================*
@@ -250,7 +250,7 @@ void smp_flush_tlb_mm(struct mm_struct *mm)
 	unsigned long *mmc;
 	unsigned long flags;
 
-	preempt_disable();
+	get_online_cpus_atomic();
 	cpu_id = smp_processor_id();
 	mmc = &mm->context[cpu_id];
 	cpumask_copy(&cpu_mask, mm_cpumask(mm));
@@ -268,7 +268,7 @@ void smp_flush_tlb_mm(struct mm_struct *mm)
 	if (!cpumask_empty(&cpu_mask))
 		flush_tlb_others(cpu_mask, mm, NULL, FLUSH_ALL);
 
-	preempt_enable();
+	put_online_cpus_atomic();
 }
 
 /*==========================================================================*
@@ -320,7 +320,7 @@ void smp_flush_tlb_page(struct vm_area_struct *vma, unsigned long va)
 	unsigned long *mmc;
 	unsigned long flags;
 
-	preempt_disable();
+	get_online_cpus_atomic();
 	cpu_id = smp_processor_id();
 	mmc = &mm->context[cpu_id];
 	cpumask_copy(&cpu_mask, mm_cpumask(mm));
@@ -341,7 +341,7 @@ void smp_flush_tlb_page(struct vm_area_struct *vma, unsigned long va)
 	if (!cpumask_empty(&cpu_mask))
 		flush_tlb_others(cpu_mask, mm, vma, va);
 
-	preempt_enable();
+	put_online_cpus_atomic();
 }
 
 /*==========================================================================*

^ permalink raw reply related

* [PATCH v3 36/45] ia64: smp, tlb: Use get/put_online_cpus_atomic() to prevent CPU offline
From: Srivatsa S. Bhat @ 2013-06-27 19:59 UTC (permalink / raw)
  To: tglx, peterz, tj, oleg, paulmck, rusty, mingo, akpm, namhyung,
	walken, vincent.guittot, laijs, David.Laight
  Cc: linux-arch, Fenghua Yu, Tony Luck, linux-ia64, nikunj, zhong,
	linux-pm, fweisbec, linux-kernel, rostedt, xiaoguangrong, sbw,
	wangyun, Srivatsa S. Bhat, netdev, linuxppc-dev
In-Reply-To: <20130627195136.29830.10445.stgit@srivatsabhat.in.ibm.com>

Once stop_machine() is gone from the CPU offline path, we won't be able
to depend on disabling preemption to prevent CPUs from going offline
from under us.

Use the get/put_online_cpus_atomic() APIs to prevent CPUs from going
offline, while invoking from atomic context.

Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: linux-ia64@vger.kernel.org
Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---

 arch/ia64/kernel/smp.c |   12 ++++++------
 arch/ia64/mm/tlb.c     |    4 ++--
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/ia64/kernel/smp.c b/arch/ia64/kernel/smp.c
index 9fcd4e6..25991ba 100644
--- a/arch/ia64/kernel/smp.c
+++ b/arch/ia64/kernel/smp.c
@@ -24,6 +24,7 @@
 #include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/smp.h>
+#include <linux/cpu.h>
 #include <linux/kernel_stat.h>
 #include <linux/mm.h>
 #include <linux/cache.h>
@@ -259,8 +260,7 @@ smp_flush_tlb_cpumask(cpumask_t xcpumask)
 	cpumask_t cpumask = xcpumask;
 	int mycpu, cpu, flush_mycpu = 0;
 
-	preempt_disable();
-	mycpu = smp_processor_id();
+	mycpu = get_online_cpus_atomic();
 
 	for_each_cpu_mask(cpu, cpumask)
 		counts[cpu] = local_tlb_flush_counts[cpu].count & 0xffff;
@@ -280,7 +280,7 @@ smp_flush_tlb_cpumask(cpumask_t xcpumask)
 		while(counts[cpu] == (local_tlb_flush_counts[cpu].count & 0xffff))
 			udelay(FLUSH_DELAY);
 
-	preempt_enable();
+	put_online_cpus_atomic();
 }
 
 void
@@ -293,12 +293,12 @@ void
 smp_flush_tlb_mm (struct mm_struct *mm)
 {
 	cpumask_var_t cpus;
-	preempt_disable();
+	get_online_cpus_atomic();
 	/* this happens for the common case of a single-threaded fork():  */
 	if (likely(mm == current->active_mm && atomic_read(&mm->mm_users) == 1))
 	{
 		local_finish_flush_tlb_mm(mm);
-		preempt_enable();
+		put_online_cpus_atomic();
 		return;
 	}
 	if (!alloc_cpumask_var(&cpus, GFP_ATOMIC)) {
@@ -313,7 +313,7 @@ smp_flush_tlb_mm (struct mm_struct *mm)
 	local_irq_disable();
 	local_finish_flush_tlb_mm(mm);
 	local_irq_enable();
-	preempt_enable();
+	put_online_cpus_atomic();
 }
 
 void arch_send_call_function_single_ipi(int cpu)
diff --git a/arch/ia64/mm/tlb.c b/arch/ia64/mm/tlb.c
index ed61297..8c55ef5 100644
--- a/arch/ia64/mm/tlb.c
+++ b/arch/ia64/mm/tlb.c
@@ -87,11 +87,11 @@ wrap_mmu_context (struct mm_struct *mm)
 	 * can't call flush_tlb_all() here because of race condition
 	 * with O(1) scheduler [EF]
 	 */
-	cpu = get_cpu(); /* prevent preemption/migration */
+	cpu = get_online_cpus_atomic(); /* prevent preemption/migration */
 	for_each_online_cpu(i)
 		if (i != cpu)
 			per_cpu(ia64_need_tlb_flush, i) = 1;
-	put_cpu();
+	put_online_cpus_atomic();
 	local_flush_tlb_all();
 }
 

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox