* Re: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
From: Scott Wood @ 2013-06-26 21:42 UTC (permalink / raw)
To: Joakim Tjernlund
Cc: Wood Scott-B07421, Zang Roy-R61911, Bob Cochran,
linuxppc-dev@ozlabs.org, Dan Malek, Support
In-Reply-To: <OF239E7E70.94A77B69-ONC1257B95.0020B40D-C1257B95.0020FEEE@transmode.se>
On 06/25/2013 01:00:23 AM, Joakim Tjernlund wrote:
> Scott Wood <scottwood@freescale.com> wrote on 2013/06/25 02:51:00:
> >
> > On Fri, Jul 20, 2012 at 10:37:17AM +0200, Joakim Tjernlund wrote:
> > > Zang Roy-R61911 <r61911@freescale.com> wrote on 2012/07/20 =20
> 10:27:52:
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From:
> linuxppc-dev-bounces+tie-fei.zang=3Dfreescale.com@lists.ozlabs.org
> > > > > [
> mailto:linuxppc-dev-bounces+tie-fei.zang=3Dfreescale.com@lists.ozlabs.org=
]
> > > > > On Behalf Of Joakim Tjernlund
> > > > > Sent: Friday, June 01, 2012 6:36 AM
> > > > > To: Wood Scott-B07421
> > > > > Cc: linuxppc-dev@ozlabs.org; Dan Malek; Bob Cochran; Support
> > > > > Subject: Re: [RFC] [PATCH] powerpc: Add MSR_DE to MSR_KERNEL
> > > > >
> > > > > It just occurred to me that you guys have this already in your
> Linux SDK so
> > > > > it can't be that bad.
> > > > No. MSR_DE is ONLY added when using CW debug in SDK.
> > > > Roy
> > > >
> > >
> > > Yes, and I later found that user space debugging is busted if you =20
> turn
> on MSR_DE in
> > > kernel.
> >
> > So, how should we handle the CONFIG_BDI_SWITCH patch? It seems =20
> like it
> > should at least have a warning in the kconfig help text that it =20
> breaks
> > userspace debugging (to the point of causing a kernel oops if it's
> > tried). Or maybe it can deselect CONFIG_PPC_ADV_DEBUG_REGS?
> >
> > It'd also be nice to keep things like this, that are a consequence =20
> of
> how
> > external debug works on e500, separate from the Abatron-specific =20
> stuff.
> >
>=20
> I was hoping the kernel would grow per context handling of MSR_DE. =20
> Then
> one could have
> MSR_DE on in MSR_KERNEL but off in user space(unless gdb request it =20
> on a
> per process basis).
What if the external debugger wants debugging to work when in userspace =20
as well? Plus, you wouldn't be able to debug from the very beginning =20
of an exception handler if the exception came from userspace.
-Scott=
^ permalink raw reply
* Re: [PATCH v2 15/45] rcu: Use get/put_online_cpus_atomic() to prevent CPU offline
From: Tejun Heo @ 2013-06-26 21:34 UTC (permalink / raw)
To: Srivatsa S. Bhat
Cc: peterz, fweisbec, linux-kernel, walken, mingo, linux-arch,
vincent.guittot, xiaoguangrong, wangyun, paulmck, nikunj,
linux-pm, rusty, Steven Rostedt, namhyung, tglx, laijs, zhong,
netdev, oleg, sbw, David Laight, akpm, linuxppc-dev
In-Reply-To: <51CB32E0.3040106@linux.vnet.ibm.com>
Hey,
On Wed, Jun 26, 2013 at 11:58:48PM +0530, Srivatsa S. Bhat wrote:
> Yes, we were discussing hot-unplug latency for use-cases such as
> suspend/resume. We didn't want to make those operations slower in the
> process of removing stop_machine() from hotplug.
Can you please explain why tho? How much would it help in terms of
power-saving? Or are there other issues in taking longer to shut down
cpus?
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH 3/3 v16] iommu/fsl: Freescale PAMU driver and iommu implementation.
From: Alex Williamson @ 2013-06-26 19:14 UTC (permalink / raw)
To: Sethi Varun-B16395
Cc: Wood Scott-B07421, Timur Tabi, joro@8bytes.org,
linux-kernel@vger.kernel.org, Yoder Stuart-B08248,
iommu@lists.linux-foundation.org, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <C5ECD7A89D1DC44195F34B25E172658D5DD70C@039-SN2MPN1-011.039d.mgd.msft.net>
On Wed, 2013-06-26 at 06:24 +0000, Sethi Varun-B16395 wrote:
>
> > -----Original Message-----
> > From: Alex Williamson [mailto:alex.williamson@redhat.com]
> > Sent: Tuesday, June 25, 2013 10:27 AM
> > To: Sethi Varun-B16395
> > Cc: joro@8bytes.org; iommu@lists.linux-foundation.org; linuxppc-
> > dev@lists.ozlabs.org; linux-kernel@vger.kernel.org;
> > benh@kernel.crashing.org; galak@kernel.crashing.org; Yoder Stuart-B08248;
> > Wood Scott-B07421; Timur Tabi
> > Subject: Re: [PATCH 3/3 v16] iommu/fsl: Freescale PAMU driver and iommu
> > implementation.
> >
> > On Thu, 2013-06-20 at 21:31 +0530, Varun Sethi wrote:
> >
> > > +#define REQ_ACS_FLAGS (PCI_ACS_SV | PCI_ACS_RR | PCI_ACS_CR |
> > PCI_ACS_UF)
> > > +
> > > +static struct iommu_group *get_device_iommu_group(struct device *dev)
> > > +{
> > > + struct iommu_group *group;
> > > +
> > > + group = iommu_group_get(dev);
> > > + if (!group)
> > > + group = iommu_group_alloc();
> > > +
> > > + return group;
> > > +}
> > > +
> > [snip]
> > > +
> >
> > This really gets parent or peer, right?
> >
> > > +static struct iommu_group *get_peer_pci_device_group(struct pci_dev
> > > +*pdev) {
> > > + struct iommu_group *group = NULL;
> > > +
> > > + /* check if this is the first device on the bus*/
> > > + if (pdev->bus_list.next == pdev->bus_list.prev) {
> >
> > It's a list_head, use list functions. The list implementation should be
> > treated as opaque.
> >
> > if (list_is_singular(&pdev->bus_list))
> >
> > > + struct pci_bus *bus = pdev->bus->parent;
> > > + /* Traverese the parent bus list to get
> > > + * pdev & dev for the sibling device.
> > > + */
> > > + while (bus) {
> > > + if (!list_empty(&bus->devices)) {
> > > + pdev = container_of(bus->devices.next,
> > > + struct pci_dev, bus_list);
> >
> > pdev = list_first_entry(&bus->devices, struct pci_dev, bus_list);
> >
> > > + group = iommu_group_get(&pdev->dev);
> > > + break;
> > > + } else
> > > + bus = bus->parent;
> >
> > Is this ever reached? Don't you always have bus->self?
> >
> [Sethi Varun-B16395] Not sure I understand. Trying to get the group
> information from the parent bus, if there are no sibling devices on
> the current bus.
I assume there's always a bridge on a bus, but maybe that bridge
(parent->self) is not in the list of parent->devices? Is that the case?
If not, then there's always a device on the bus, the bridge that created
it.
> > > + }
> > > + } else {
> > > + /*
> > > + * Get the pdev & dev for the sibling device
> > > + */
> > > + pdev = container_of(pdev->bus_list.prev,
> > > + struct pci_dev, bus_list);
> >
> > How do you know if you're at the head or tail of the list?
> >
> > struct pci_dev *tmp;
> > list_for_each_entry(tmp, &pdev->bus_list, bus_list) {
> > if (tmp == pdev)
> > continue;
> >
> > group = iommu_group_get(&tmp->dev);
> > break;
> > }
> >
> > > + group = iommu_group_get(&pdev->dev);
> > > + }
> > > +
> > > + return group;
> > > +}
> > > +
> > > +static struct iommu_group *get_pci_device_group(struct pci_dev *pdev)
> > > +{
> > > + struct iommu_group *group = NULL;
> > > + struct pci_dev *bridge, *dma_pdev = NULL;
> > > + struct pci_controller *pci_ctl;
> > > + bool pci_endpt_partioning;
> > > +
> > > + pci_ctl = pci_bus_to_host(pdev->bus);
> > > + pci_endpt_partioning = check_pci_ctl_endpt_part(pci_ctl);
> > > + /* We can partition PCIe devices so assign device group to the
> > device */
> > > + if (pci_endpt_partioning) {
> > > + bridge = pci_find_upstream_pcie_bridge(pdev);
> > > + if (bridge) {
> > > + if (pci_is_pcie(bridge))
> > > + dma_pdev = pci_get_domain_bus_and_slot(
> > > + pci_domain_nr(pdev->bus),
> > > + bridge->subordinate->number, 0);
> > > + if (!dma_pdev)
> > > + dma_pdev = pci_dev_get(bridge);
> > > + } else
> > > + dma_pdev = pci_dev_get(pdev);
> > > +
> > > + /* Account for quirked devices */
> > > + swap_pci_ref(&dma_pdev, pci_get_dma_source(dma_pdev));
> > > +
> > > + /*
> > > + * If it's a multifunction device that does not support our
> > > + * required ACS flags, add to the same group as function 0.
> > > + */
> >
> > See c14d2690 in Joerg's next tree, using function 0 was a poor
> > assumption.
> [Sethi Varun-B16395] ok.
>
> >
> > > + if (dma_pdev->multifunction &&
> > > + !pci_acs_enabled(dma_pdev, REQ_ACS_FLAGS))
> > > + swap_pci_ref(&dma_pdev,
> > > + pci_get_slot(dma_pdev->bus,
> > > + PCI_DEVFN(PCI_SLOT(dma_pdev-
> > >devfn),
> > > + 0)));
> > > +
> > > + group = get_device_iommu_group(&pdev->dev);
> > > + pci_dev_put(pdev);
> >
> > What was the point of all the above if we use pdev here instead of
> > dma_pdev? Wrong device and broken reference counting.
> [Sethi Varun-B16395] Will fix this
>
> This also isn't
> > testing ACS all the way up to the root complex or controller.
> [Sethi Varun-B16395] In our case the IOMMU can differentiate
> transactions based on the LIODN. The PCIe controller can generate a
> unique LIODN based on the bus,device,function number. I believe this
> would even be true for devices connected to a PCIe bridge (and not on
> the root bus). So, do we still need to check for ACS up to the root
> node?
ACS is the PCI capability that tells us whether a device allows
peer-to-peer or requires all DMA to be forwarded upstream. At any step
along the way from an endpoint to the IOMMU the transaction can be
re-routed. We therefore need to test the ACS property not only of the
endpoint, but every device between the endpoint and the IOMMU to form
the IOMMU groups. An IOMMU cannot isolate a device if a DMA access is
re-routed by a link in the topology that prevents the IOMMU from even
being part of the transaction. Thanks,
Alex
^ permalink raw reply
* Re: powerpc/85xx: Move ePAPR paravirt initialization earlier
From: Scott Wood @ 2013-06-26 18:56 UTC (permalink / raw)
To: Tudor Laurentiu; +Cc: linuxppc-dev
In-Reply-To: <51CAE047.8030605@freescale.com>
On 06/26/2013 07:36:23 AM, Tudor Laurentiu wrote:
> On 06/26/2013 02:14 AM, Scott Wood wrote:
>> This would require converting
>> the code to use the early device tree functions.
>>=20
>=20
> I see. Had a look at that api and it seems pretty limited.
> I couldn't find a simple way of reading a property other than =20
> scanning the entire tree with of_scan_flat_dt().
What's wrong with scanning the entire tree? The data format doesn't =20
allow any more efficient type of lookup (it was designed to be simple =20
and small, not fast).
-Scott=
^ permalink raw reply
* Re: [PATCH v2 15/45] rcu: Use get/put_online_cpus_atomic() to prevent CPU offline
From: Srivatsa S. Bhat @ 2013-06-26 18:28 UTC (permalink / raw)
To: Tejun Heo
Cc: peterz, fweisbec, linux-kernel, walken, mingo, linux-arch,
vincent.guittot, xiaoguangrong, wangyun, paulmck, nikunj,
linux-pm, rusty, Steven Rostedt, namhyung, tglx, laijs, zhong,
netdev, oleg, sbw, David Laight, akpm, linuxppc-dev
In-Reply-To: <20130626172948.GD4405@mtj.dyndns.org>
On 06/26/2013 10:59 PM, Tejun Heo wrote:
> Hello,
>
> On Wed, Jun 26, 2013 at 11:33:43AM -0400, Steven Rostedt wrote:
>> I thought the whole deal with this patchset was to remove stop_machine
>> from CPU hotplug. Why halt all CPUs just to remove one? stomp_machine()
>> is extremely intrusive for the entire system, where as one CPU making
>> sure all CPUs schedule isn't very intrusive at all.
>>
>> I didn't think the idea of this patch set was to make CPU hotplug
>> faster, just less intrusive to the system.
>
> Yeap, removal of stop_machine is a great improvement in itself.
Absolutely. To make hotplug less intrusive on the system.
> ISTR
> mentions of hot-unplug latency but I could be mistaken. Srivatsa, can
> you please chime in on that?
>
Yes, we were discussing hot-unplug latency for use-cases such as
suspend/resume. We didn't want to make those operations slower in the
process of removing stop_machine() from hotplug.
Regards,
Srivatsa S. Bhat
^ permalink raw reply
* Re: [PATCH v2 15/45] rcu: Use get/put_online_cpus_atomic() to prevent CPU offline
From: Srivatsa S. Bhat @ 2013-06-26 18:22 UTC (permalink / raw)
To: Tejun Heo
Cc: peterz, fweisbec, oleg, walken, mingo, linux-arch,
vincent.guittot, xiaoguangrong, wangyun, paulmck, nikunj,
linux-pm, rusty, Steven Rostedt, namhyung, tglx, laijs, zhong,
netdev, linux-kernel, sbw, David Laight, akpm, linuxppc-dev
In-Reply-To: <20130626152148.GA4405@mtj.dyndns.org>
On 06/26/2013 08:51 PM, Tejun Heo wrote:
> On Wed, Jun 26, 2013 at 10:51:11AM -0400, Steven Rostedt wrote:
>>> It would also increase the latency of CPU-hotunplug operations.
>>
>> Is that a big deal?
>
> I thought that was the whole deal with this patchset - making cpu
> hotunplugs lighter and faster mostly for powersaving. That said, just
> removing stop_machine call would be a pretty good deal and I don't
> know how meaningful reducing CPU hotunplug latency is. Srivatsa?
>
Keeping the hotunplug latency is important for suspend/resume, where
we take all non-boot CPUs in a loop. That's an interesting use-case
where intrusiveness doesn't matter much, but latency does. So yes,
making CPU hotplug faster is also one of the goals of this patchset.
Regards,
Srivatsa S. Bhat
^ permalink raw reply
* Re: [PATCH v2 15/45] rcu: Use get/put_online_cpus_atomic() to prevent CPU offline
From: Srivatsa S. Bhat @ 2013-06-26 18:18 UTC (permalink / raw)
To: David Laight
Cc: peterz, fweisbec, oleg, walken, mingo, linux-arch,
vincent.guittot, xiaoguangrong, wangyun, paulmck, nikunj,
linux-pm, rusty, rostedt, namhyung, tglx, laijs, zhong, netdev,
linux-kernel, sbw, tj, akpm, linuxppc-dev
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B72AA@saturn3.aculab.com>
On 06/26/2013 07:59 PM, David Laight wrote:
>> 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.
>
> Could you use an rcu-like sequence so that disabling pre-emption
> would be enough?
>
> Something like rebuilding the cpu list, then forcing yourself
> to run on all the cpu.
>
Certainly, and we had debated schemes similar to that (including
schemes invoking synchronize_sched() itself) in earlier discussions.
(But IIRC even those schemes required converting call-sites from
preempt_disable() to get/put_online_cpus_atomic(), to properly
synchronize).
> That would be far less intrusive.
>
But that would increase the latency of hotplug operations like Paul
pointed out, and that in turn is not good for use-cases such as
suspend/resume, where we take all non-boot CPUs offline in a loop.
(That would mean executing the above logic num_online_cpus() times!).
So we started hunting for ideas that can make the hotplug writer
side not only less intrusive, but also remain fast. That's how
the later designs evolved.
Regards,
Srivatsa S. Bhat
^ permalink raw reply
* Re: [PATCH v2 15/45] rcu: Use get/put_online_cpus_atomic() to prevent CPU offline
From: Tejun Heo @ 2013-06-26 17:29 UTC (permalink / raw)
To: Steven Rostedt
Cc: peterz, fweisbec, oleg, walken, mingo, linux-arch,
vincent.guittot, xiaoguangrong, wangyun, paulmck, nikunj,
linux-pm, rusty, namhyung, tglx, laijs, zhong, netdev,
linux-kernel, sbw, David Laight, Srivatsa S. Bhat, akpm,
linuxppc-dev
In-Reply-To: <1372260823.18733.263.camel@gandalf.local.home>
Hello,
On Wed, Jun 26, 2013 at 11:33:43AM -0400, Steven Rostedt wrote:
> I thought the whole deal with this patchset was to remove stop_machine
> from CPU hotplug. Why halt all CPUs just to remove one? stomp_machine()
> is extremely intrusive for the entire system, where as one CPU making
> sure all CPUs schedule isn't very intrusive at all.
>
> I didn't think the idea of this patch set was to make CPU hotplug
> faster, just less intrusive to the system.
Yeap, removal of stop_machine is a great improvement in itself. ISTR
mentions of hot-unplug latency but I could be mistaken. Srivatsa, can
you please chime in on that?
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH v2 15/45] rcu: Use get/put_online_cpus_atomic() to prevent CPU offline
From: Steven Rostedt @ 2013-06-26 15:33 UTC (permalink / raw)
To: Tejun Heo
Cc: peterz, fweisbec, oleg, walken, mingo, linux-arch,
vincent.guittot, xiaoguangrong, wangyun, paulmck, nikunj,
linux-pm, rusty, namhyung, tglx, laijs, zhong, netdev,
linux-kernel, sbw, David Laight, Srivatsa S. Bhat, akpm,
linuxppc-dev
In-Reply-To: <20130626152148.GA4405@mtj.dyndns.org>
On Wed, 2013-06-26 at 08:21 -0700, Tejun Heo wrote:
> On Wed, Jun 26, 2013 at 10:51:11AM -0400, Steven Rostedt wrote:
> > > It would also increase the latency of CPU-hotunplug operations.
> >
> > Is that a big deal?
>
> I thought that was the whole deal with this patchset - making cpu
> hotunplugs lighter and faster mostly for powersaving. That said, just
> removing stop_machine call would be a pretty good deal and I don't
> know how meaningful reducing CPU hotunplug latency is. Srivatsa?
I thought the whole deal with this patchset was to remove stop_machine
from CPU hotplug. Why halt all CPUs just to remove one? stomp_machine()
is extremely intrusive for the entire system, where as one CPU making
sure all CPUs schedule isn't very intrusive at all.
I didn't think the idea of this patch set was to make CPU hotplug
faster, just less intrusive to the system.
-- Steve
^ permalink raw reply
* Re: [PATCH v2 15/45] rcu: Use get/put_online_cpus_atomic() to prevent CPU offline
From: Tejun Heo @ 2013-06-26 15:21 UTC (permalink / raw)
To: Steven Rostedt
Cc: peterz, fweisbec, oleg, walken, mingo, linux-arch,
vincent.guittot, xiaoguangrong, wangyun, paulmck, nikunj,
linux-pm, rusty, namhyung, tglx, laijs, zhong, netdev,
linux-kernel, sbw, David Laight, Srivatsa S. Bhat, akpm,
linuxppc-dev
In-Reply-To: <1372258271.18733.256.camel@gandalf.local.home>
On Wed, Jun 26, 2013 at 10:51:11AM -0400, Steven Rostedt wrote:
> > It would also increase the latency of CPU-hotunplug operations.
>
> Is that a big deal?
I thought that was the whole deal with this patchset - making cpu
hotunplugs lighter and faster mostly for powersaving. That said, just
removing stop_machine call would be a pretty good deal and I don't
know how meaningful reducing CPU hotunplug latency is. Srivatsa?
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH v2 15/45] rcu: Use get/put_online_cpus_atomic() to prevent CPU offline
From: Steven Rostedt @ 2013-06-26 14:51 UTC (permalink / raw)
To: paulmck
Cc: peterz, fweisbec, oleg, walken, mingo, linux-arch,
vincent.guittot, xiaoguangrong, wangyun, nikunj, linux-pm, rusty,
namhyung, tglx, laijs, zhong, netdev, linux-kernel, sbw,
David Laight, Srivatsa S. Bhat, tj, akpm, linuxppc-dev
In-Reply-To: <20130626143424.GN3828@linux.vnet.ibm.com>
On Wed, 2013-06-26 at 07:34 -0700, Paul E. McKenney wrote:
> On Wed, Jun 26, 2013 at 03:29:40PM +0100, David Laight wrote:
> > > 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.
> >
> > Could you use an rcu-like sequence so that disabling pre-emption
> > would be enough?
> >
> > Something like rebuilding the cpu list, then forcing yourself
> > to run on all the cpu.
> >
> > That would be far less intrusive.
>
> It would also increase the latency of CPU-hotunplug operations.
>
Is that a big deal?
-- Steve
^ permalink raw reply
* Re: [PATCH v2 15/45] rcu: Use get/put_online_cpus_atomic() to prevent CPU offline
From: Paul E. McKenney @ 2013-06-26 14:45 UTC (permalink / raw)
To: David Laight
Cc: peterz, fweisbec, oleg, walken, mingo, linux-arch,
vincent.guittot, xiaoguangrong, wangyun, nikunj, linux-pm, rusty,
rostedt, namhyung, tglx, laijs, zhong, netdev, linux-kernel, sbw,
Srivatsa S. Bhat, tj, akpm, linuxppc-dev
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B72AA@saturn3.aculab.com>
On Wed, Jun 26, 2013 at 03:29:40PM +0100, David Laight wrote:
> > 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.
>
> Could you use an rcu-like sequence so that disabling pre-emption
> would be enough?
>
> Something like rebuilding the cpu list, then forcing yourself
> to run on all the cpu.
>
> That would be far less intrusive.
Maybe... I must defer to Srivatsa on this question.
Thanx, Paul
^ permalink raw reply
* Re: [PATCH v2 15/45] rcu: Use get/put_online_cpus_atomic() to prevent CPU offline
From: Paul E. McKenney @ 2013-06-26 14:33 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, tj,
akpm, linuxppc-dev
In-Reply-To: <51CAF624.6060004@linux.vnet.ibm.com>
On Wed, Jun 26, 2013 at 07:39:40PM +0530, Srivatsa S. Bhat wrote:
> On 06/26/2013 03:30 AM, Paul E. McKenney wrote:
> > On Wed, Jun 26, 2013 at 01:57:55AM +0530, Srivatsa S. Bhat wrote:
> >> 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.
> >>
> >> In RCU code, rcu_implicit_dynticks_qs() checks if a CPU is offline,
> >> while being protected by a spinlock. Use the get/put_online_cpus_atomic()
> >> APIs to prevent CPUs from going offline, while invoking from atomic context.
> >
> > I am not completely sure that this is needed. Here is a (quite possibly
> > flawed) argument for its not being needed:
> >
> > o rcu_gp_init() holds off CPU-hotplug operations during
> > grace-period initialization. Therefore, RCU will avoid
> > looking for quiescent states from CPUs that were offline
> > (and thus in an extended quiescent state) at the beginning
> > of the grace period.
> >
> > o If force_qs_rnp() is looking for a quiescent state from
> > a given CPU, and if it senses that CPU as being offline,
> > then even without synchronization we know that the CPU
> > was offline some time during the current grace period.
> >
> > After all, it was online at the beginning of the grace
> > period (otherwise, we would not be looking at it at all),
> > and our later sampling of its state must have therefore
> > happened after the start of the grace period. Given that
> > the grace period has not yet ended, it also has to happened
> > before the end of the grace period.
> >
> > o Therefore, we should be able to sample the offline state
> > without synchronization.
> >
>
> Thanks a lot for explaining the synchronization design in detail, Paul!
> I agree that get/put_online_cpus_atomic() is not necessary here.
>
> Regarding the debug checks under CONFIG_DEBUG_HOTPLUG_CPU, to avoid
> false-positives, I'm thinking of introducing a few _nocheck() variants,
> on a case-by-case basis, like cpu_is_offline_nocheck() (useful here in RCU)
> and for_each_online_cpu_nocheck() (useful in percpu-counter code, as
> pointed out by Tejun Heo). These fine synchronization details are kinda
> hard to encapsulate in that debug logic, so we can use the _nocheck()
> variants here to avoid getting splats when running with DEBUG_HOTPLUG_CPU
> enabled.
Good point, and seems like a reasonable approach to me.
Thanx, Paul
^ permalink raw reply
* Re: [PATCH v2 15/45] rcu: Use get/put_online_cpus_atomic() to prevent CPU offline
From: Paul E. McKenney @ 2013-06-26 14:34 UTC (permalink / raw)
To: David Laight
Cc: peterz, fweisbec, oleg, walken, mingo, linux-arch,
vincent.guittot, xiaoguangrong, wangyun, nikunj, linux-pm, rusty,
rostedt, namhyung, tglx, laijs, zhong, netdev, linux-kernel, sbw,
Srivatsa S. Bhat, tj, akpm, linuxppc-dev
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B72AA@saturn3.aculab.com>
On Wed, Jun 26, 2013 at 03:29:40PM +0100, David Laight wrote:
> > 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.
>
> Could you use an rcu-like sequence so that disabling pre-emption
> would be enough?
>
> Something like rebuilding the cpu list, then forcing yourself
> to run on all the cpu.
>
> That would be far less intrusive.
It would also increase the latency of CPU-hotunplug operations.
Thanx, Paul
^ permalink raw reply
* RE: [PATCH v2 15/45] rcu: Use get/put_online_cpus_atomic() to prevent CPU offline
From: David Laight @ 2013-06-26 14:29 UTC (permalink / raw)
To: Srivatsa S. Bhat, paulmck
Cc: peterz, fweisbec, oleg, walken, mingo, linux-arch,
vincent.guittot, xiaoguangrong, wangyun, nikunj, linux-pm, rusty,
rostedt, namhyung, tglx, laijs, zhong, netdev, linux-kernel, sbw,
tj, akpm, linuxppc-dev
In-Reply-To: <51CAF624.6060004@linux.vnet.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.
Could you use an rcu-like sequence so that disabling pre-emption
would be enough?
Something like rebuilding the cpu list, then forcing yourself
to run on all the cpu.
That would be far less intrusive.
David
^ permalink raw reply
* Re: linux-next: manual merge of the akpm tree with the powerpc tree
From: Oleg Nesterov @ 2013-06-26 14:19 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Stephen Rothwell, Michael Neuling, linux-kernel, linux-next,
Andrew Morton, linuxppc-dev
In-Reply-To: <1372234231.18612.6.camel@pasglop>
On 06/26, Benjamin Herrenschmidt wrote:
>
> On Wed, 2013-06-26 at 16:56 +1000, Stephen Rothwell wrote:
> > Today's linux-next merge of the akpm tree got a conflict in
> > arch/powerpc/kernel/ptrace.c between commit b0b0aa9c7faf
> > ("powerpc/hw_brk: Fix setting of length for exact mode breakpoints") from
> > the powerpc tree and commit "ptrace/powerpc: revert "hw_breakpoints: Fix
> > racy access to ptrace breakpoints"" from the akpm tree.
> >
> > I fixed it up (see below) and can carry the fix as necessary (no action
> > is required).
>
> Where does the latter come from ?
ptrace-powerpc-revert-hw_breakpoints-fix-racy-access-to-ptrace-breakpoints.patch
attached below.
You were cc'ed every time ;)
> Why didn't it go through the powerpc tree ?
Because this series needs to update any user of ptrace_get/put_breakpoints
in arch/ (simply remove these calls), then change the core kernel code, then
fix arch/86.
------------------------------------------------------
From: Oleg Nesterov <oleg@redhat.com>
Subject: ptrace/powerpc: revert "hw_breakpoints: Fix racy access to ptrace breakpoints"
This reverts commit 07fa7a0a8a586 ("hw_breakpoints: Fix racy access to
ptrace breakpoints") and removes ptrace_get/put_breakpoints() added by
other commits.
The patch was fine but we can no longer race with SIGKILL after 9899d11f
("ptrace: ensure arch_ptrace/ptrace_request can never race with SIGKILL"),
the __TASK_TRACED tracee can't be woken up and ->ptrace_bps[] can't go
away.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Michael Neuling <mikey@neuling.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jan Kratochvil <jan.kratochvil@redhat.com>
Cc: Paul Mundt <lethal@linux-sh.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Prasad <prasad@linux.vnet.ibm.com>
Cc: Russell King <linux@arm.linux.org.uk>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
arch/powerpc/kernel/ptrace.c | 20 --------------------
1 file changed, 20 deletions(-)
diff -puN arch/powerpc/kernel/ptrace.c~ptrace-powerpc-revert-hw_breakpoints-fix-racy-access-to-ptrace-breakpoints arch/powerpc/kernel/ptrace.c
--- a/arch/powerpc/kernel/ptrace.c~ptrace-powerpc-revert-hw_breakpoints-fix-racy-access-to-ptrace-breakpoints
+++ a/arch/powerpc/kernel/ptrace.c
@@ -974,16 +974,12 @@ int ptrace_set_debugreg(struct task_stru
hw_brk.type = (data & HW_BRK_TYPE_DABR) | HW_BRK_TYPE_PRIV_ALL;
hw_brk.len = 8;
#ifdef CONFIG_HAVE_HW_BREAKPOINT
- if (ptrace_get_breakpoints(task) < 0)
- return -ESRCH;
-
bp = thread->ptrace_bps[0];
if ((!data) || !(hw_brk.type & HW_BRK_TYPE_RDWR)) {
if (bp) {
unregister_hw_breakpoint(bp);
thread->ptrace_bps[0] = NULL;
}
- ptrace_put_breakpoints(task);
return 0;
}
if (bp) {
@@ -996,11 +992,9 @@ int ptrace_set_debugreg(struct task_stru
ret = modify_user_hw_breakpoint(bp, &attr);
if (ret) {
- ptrace_put_breakpoints(task);
return ret;
}
thread->ptrace_bps[0] = bp;
- ptrace_put_breakpoints(task);
thread->hw_brk = hw_brk;
return 0;
}
@@ -1015,12 +1009,9 @@ int ptrace_set_debugreg(struct task_stru
ptrace_triggered, NULL, task);
if (IS_ERR(bp)) {
thread->ptrace_bps[0] = NULL;
- ptrace_put_breakpoints(task);
return PTR_ERR(bp);
}
- ptrace_put_breakpoints(task);
-
#endif /* CONFIG_HAVE_HW_BREAKPOINT */
task->thread.hw_brk = hw_brk;
#else /* CONFIG_PPC_ADV_DEBUG_REGS */
@@ -1439,9 +1430,6 @@ static long ppc_set_hwdebug(struct task_
if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_WRITE)
brk.type |= HW_BRK_TYPE_WRITE;
#ifdef CONFIG_HAVE_HW_BREAKPOINT
- if (ptrace_get_breakpoints(child) < 0)
- return -ESRCH;
-
/*
* Check if the request is for 'range' breakpoints. We can
* support it if range < 8 bytes.
@@ -1449,12 +1437,10 @@ static long ppc_set_hwdebug(struct task_
if (bp_info->addr_mode == PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE) {
len = bp_info->addr2 - bp_info->addr;
} else if (bp_info->addr_mode != PPC_BREAKPOINT_MODE_EXACT) {
- ptrace_put_breakpoints(child);
return -EINVAL;
}
bp = thread->ptrace_bps[0];
if (bp) {
- ptrace_put_breakpoints(child);
return -ENOSPC;
}
@@ -1468,11 +1454,9 @@ static long ppc_set_hwdebug(struct task_
ptrace_triggered, NULL, child);
if (IS_ERR(bp)) {
thread->ptrace_bps[0] = NULL;
- ptrace_put_breakpoints(child);
return PTR_ERR(bp);
}
- ptrace_put_breakpoints(child);
return 1;
#endif /* CONFIG_HAVE_HW_BREAKPOINT */
@@ -1516,16 +1500,12 @@ static long ppc_del_hwdebug(struct task_
return -EINVAL;
#ifdef CONFIG_HAVE_HW_BREAKPOINT
- if (ptrace_get_breakpoints(child) < 0)
- return -ESRCH;
-
bp = thread->ptrace_bps[0];
if (bp) {
unregister_hw_breakpoint(bp);
thread->ptrace_bps[0] = NULL;
} else
ret = -ENOENT;
- ptrace_put_breakpoints(child);
return ret;
#else /* CONFIG_HAVE_HW_BREAKPOINT */
if (child->thread.hw_brk.address == 0)
_
Patches currently in -mm which might be from oleg@redhat.com are
posix_cpu_timer-consolidate-expiry-time-type.patch
posix_cpu_timers-consolidate-timer-list-cleanups.patch
posix_cpu_timers-consolidate-expired-timers-check.patch
posix-timers-correctly-get-dying-task-time-sample-in-posix_cpu_timer_schedule.patch
posix_timers-fix-racy-timer-delta-caching-on-task-exit.patch
lockdep-introduce-lock_acquire_exclusive-shared-helper-macros.patch
lglock-update-lockdep-annotations-to-report-recursive-local-locks.patch
autofs4-allow-autofs-to-work-outside-the-initial-pid-namespace.patch
autofs4-translate-pids-to-the-right-namespace-for-the-daemon.patch
ptrace-x86-revert-hw_breakpoints-fix-racy-access-to-ptrace-breakpoints.patch
ptrace-powerpc-revert-hw_breakpoints-fix-racy-access-to-ptrace-breakpoints.patch
ptrace-arm-revert-hw_breakpoints-fix-racy-access-to-ptrace-breakpoints.patch
ptrace-sh-revert-hw_breakpoints-fix-racy-access-to-ptrace-breakpoints.patch
ptrace-revert-prepare-to-fix-racy-accesses-on-task-breakpoints.patch
ptrace-x86-simplify-the-disable-logic-in-ptrace_write_dr7.patch
ptrace-x86-dont-delay-disable-till-second-pass-in-ptrace_write_dr7.patch
ptrace-x86-introduce-ptrace_register_breakpoint.patch
ptrace-x86-ptrace_write_dr7-should-create-bp-if-disabled.patch
ptrace-x86-cleanup-ptrace_set_debugreg.patch
ptrace-ptrace_detach-should-do-flush_ptrace_hw_breakpointchild.patch
ptrace-x86-flush_ptrace_hw_breakpoint-shoule-clear-the-virtual-debug-registers.patch
x86-kill-tif_debug.patch
^ permalink raw reply
* Re: Regression in RCU subsystem in latest mainline kernel
From: Paul E. McKenney @ 2013-06-26 14:16 UTC (permalink / raw)
To: Michael Ellerman
Cc: Rojhalat Ibrahim, linuxppc-dev, linux-kernel, Steven Rostedt
In-Reply-To: <20130626081057.GB10796@concordia>
On Wed, Jun 26, 2013 at 06:10:58PM +1000, Michael Ellerman wrote:
> On Tue, Jun 25, 2013 at 09:03:32AM -0700, Paul E. McKenney wrote:
> > On Tue, Jun 25, 2013 at 05:44:23PM +1000, Michael Ellerman wrote:
> > > On Tue, Jun 25, 2013 at 05:19:14PM +1000, Michael Ellerman wrote:
> > > >
> > > > Here's another trace from 3.10-rc7 plus a few local patches.
> > >
> > > And here's another with CONFIG_RCU_CPU_STALL_INFO=y in case that's useful:
> > >
> > > PASS running test_pmc5_6_overuse()
> > > INFO: rcu_sched self-detected stall on CPU
> > > 8: (1 GPs behind) idle=8eb/140000000000002/0 softirq=215/220
> >
> > So this CPU has been out of action since before the beginning of the
> > current grace period ("1 GPs behind"). It is not idle, having taken
> > a pair of nested interrupts from process context (matching the stack
> > below). This CPU has take five softirqs since the last grace period
> > that it noticed, which makes it likely that the loop is within the
> > softirq handler.
> >
> > > (t=2100 jiffies g=18446744073709551583 c=18446744073709551582 q=13)
> >
> > Assuming HZ=100, this stall has been going on for 21 seconds. There
> > is a grace period in progress according to RCU's global state (which
> > this CPU is not yet aware of). There are a total of 13 RCU callbacks
> > queued across the entire system.
> >
> > If the system is at all responsive, I suggest using ftrace (either from
> > the boot command line or at runtime) to trace __do_softirq() and
> > hrtimer_interrupt().
>
> Thanks for decoding it Paul.
>
> I've narrowed down the test case and I think this is probably just a
> case of too many perf interrupts. If I reduce the sampling period by
> half the test runs fine.
>
> There is logic in perf to detect an interrupt storm, but for some reason
> it's not saving us. I'll dig in there, but I don't think it's an RCU
> problem.
Whew! ;-)
Thanx, Paul
^ permalink raw reply
* Re: [PATCH v2 15/45] rcu: Use get/put_online_cpus_atomic() to prevent CPU offline
From: Srivatsa S. Bhat @ 2013-06-26 14:09 UTC (permalink / raw)
To: paulmck
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, tj,
akpm, linuxppc-dev
In-Reply-To: <20130625220026.GG3828@linux.vnet.ibm.com>
On 06/26/2013 03:30 AM, Paul E. McKenney wrote:
> On Wed, Jun 26, 2013 at 01:57:55AM +0530, Srivatsa S. Bhat wrote:
>> 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.
>>
>> In RCU code, rcu_implicit_dynticks_qs() checks if a CPU is offline,
>> while being protected by a spinlock. Use the get/put_online_cpus_atomic()
>> APIs to prevent CPUs from going offline, while invoking from atomic context.
>
> I am not completely sure that this is needed. Here is a (quite possibly
> flawed) argument for its not being needed:
>
> o rcu_gp_init() holds off CPU-hotplug operations during
> grace-period initialization. Therefore, RCU will avoid
> looking for quiescent states from CPUs that were offline
> (and thus in an extended quiescent state) at the beginning
> of the grace period.
>
> o If force_qs_rnp() is looking for a quiescent state from
> a given CPU, and if it senses that CPU as being offline,
> then even without synchronization we know that the CPU
> was offline some time during the current grace period.
>
> After all, it was online at the beginning of the grace
> period (otherwise, we would not be looking at it at all),
> and our later sampling of its state must have therefore
> happened after the start of the grace period. Given that
> the grace period has not yet ended, it also has to happened
> before the end of the grace period.
>
> o Therefore, we should be able to sample the offline state
> without synchronization.
>
Thanks a lot for explaining the synchronization design in detail, Paul!
I agree that get/put_online_cpus_atomic() is not necessary here.
Regarding the debug checks under CONFIG_DEBUG_HOTPLUG_CPU, to avoid
false-positives, I'm thinking of introducing a few _nocheck() variants,
on a case-by-case basis, like cpu_is_offline_nocheck() (useful here in RCU)
and for_each_online_cpu_nocheck() (useful in percpu-counter code, as
pointed out by Tejun Heo). These fine synchronization details are kinda
hard to encapsulate in that debug logic, so we can use the _nocheck()
variants here to avoid getting splats when running with DEBUG_HOTPLUG_CPU
enabled.
Regards,
Srivatsa S. Bhat
^ permalink raw reply
* Re: [PATCH v2 38/45] MIPS: Use get/put_online_cpus_atomic() to prevent CPU offline
From: Ralf Baechle @ 2013-06-26 13:39 UTC (permalink / raw)
To: Srivatsa S. Bhat
Cc: linux-mips, David Daney, peterz, fweisbec, linux-kernel, walken,
mingo, linux-arch, vincent.guittot, xiaoguangrong, Steven J. Hill,
wangyun, paulmck, nikunj, linux-pm, rusty, rostedt, Yong Zhang,
namhyung, tglx, Florian Fainelli, John Crispin, laijs, zhong,
Sanjay Lal, netdev, oleg, sbw, tj, akpm, linuxppc-dev
In-Reply-To: <20130625203257.16593.15358.stgit@srivatsabhat.in.ibm.com>
On Wed, Jun 26, 2013 at 02:02:57AM +0530, Srivatsa S. Bhat wrote:
> 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.
I think the same change also needs to be applied to r4k_on_each_cpu() in
arch/mips/mm/c-r4k.c which currently looks like:
static inline void r4k_on_each_cpu(void (*func) (void *info), void *info)
{
preempt_disable();
#if !defined(CONFIG_MIPS_MT_SMP) && !defined(CONFIG_MIPS_MT_SMTC)
smp_call_function(func, info, 1);
#endif
func(info);
preempt_enable();
}
This is a slightly specialized version of on_each_cpu() which only calls
out to other CPUs in actual multi-core environments and also - unlike
on_each_cpu() doesn't disable interrupts for the sake of better
interrupt latencies.
Which reminds me ...
Andrew, I was wondering why did 78eef01b0fae087c5fadbd85dd4fe2918c3a015f
[[PATCH] on_each_cpu(): disable local interrupts] disable interrupts?
The log is:
----- snip -----
When on_each_cpu() runs the callback on other CPUs, it runs with local
interrupts disabled. So we should run the function with local interrupts
disabled on this CPU, too.
And do the same for UP, so the callback is run in the same environment on bo
UP and SMP. (strictly it should do preempt_disable() too, but I think
local_irq_disable is sufficiently equivalent).
[...]
----- snip -----
I'm not entirely convinced the symmetry between UP and SMP environments is
really worth it. Would anybody mind removing the local_irq_disable() ...
local_irq_enable() from on_each_cpu()?
Ralf
^ permalink raw reply
* Re: linux-next: manual merge of the akpm tree with the powerpc tree
From: Andrew Morton @ 2013-06-26 13:13 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Stephen Rothwell, Michael Neuling, linux-kernel, Oleg Nesterov,
linux-next, linuxppc-dev
In-Reply-To: <1372234231.18612.6.camel@pasglop>
On Wed, 26 Jun 2013 18:10:31 +1000 Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> On Wed, 2013-06-26 at 16:56 +1000, Stephen Rothwell wrote:
> > Today's linux-next merge of the akpm tree got a conflict in
> > arch/powerpc/kernel/ptrace.c between commit b0b0aa9c7faf
> > ("powerpc/hw_brk: Fix setting of length for exact mode breakpoints") from
> > the powerpc tree and commit "ptrace/powerpc: revert "hw_breakpoints: Fix
> > racy access to ptrace breakpoints"" from the akpm tree.
> >
> > I fixed it up (see below) and can carry the fix as necessary (no action
> > is required).
>
> Where does the latter come from ? Why didn't it go through the powerpc tree ?
um, the most rudimentary search will take you to Oleg's patch series.
You were cc'ed on the whole thing two or three times.
^ permalink raw reply
* Re: powerpc/85xx: Move ePAPR paravirt initialization earlier
From: Tudor Laurentiu @ 2013-06-26 12:36 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <20130625231404.GA29587@home.buserror.net>
On 06/26/2013 02:14 AM, Scott Wood wrote:
> On Tue, Mar 05, 2013 at 05:52:36PM +0200, Laurentiu TUDOR wrote:
>> From: Tudor Laurentiu<laurentiu.tudor@freescale.com>
>>
>> The ePAPR para-virtualization needs to happen very early
>> otherwise the bytechannel based console will silently
>> drop some of the early boot messages.
>>
>> Before this patch, this is how the kernel log started:
>> -----------------------------------------------------
>> > Brought up 2 CPUs
>> > devtmpfs: initialized
>> > NET: Registered protocol family 16
>> [...]
>> -----------------------------------------------------
>>
>> After the patch the early messages show up:
>> -----------------------------------------------------
>> > Using P5020 DS machine description
>> > MMU: Supported page sizes
>> > 4 KB as direct
>> > 4096 KB as direct
>> [...]
>> -----------------------------------------------------
>>
>> At console init, the kernel tried to flush the log buffer.
>> Since the paravirt was not yet initialized the console write
>> function failed silently, thus losing the buffered messages.
> [snip]
>> diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
>> index 6da881b..ce092ac 100644
>> --- a/arch/powerpc/kernel/setup_64.c
>> +++ b/arch/powerpc/kernel/setup_64.c
>> @@ -66,6 +66,7 @@
>> #include<asm/code-patching.h>
>> #include<asm/kvm_ppc.h>
>> #include<asm/hugetlb.h>
>> +#include<asm/epapr_hcalls.h>
>>
>> #include "setup.h"
>>
>> @@ -599,6 +600,8 @@ void __init setup_arch(char **cmdline_p)
>> /* Initialize the MMU context management stuff */
>> mmu_context_init();
>>
>> + epapr_paravirt_init();
>> +
>> kvm_linear_init();
>>
>> /* Interrupt code needs to be 64K-aligned */
>
> Is this early enough? There's udbg activity before this. Maybe it
> should even go before udbg_early_init...
Right. I'll try moving it in machine_init().
> This would require converting
> the code to use the early device tree functions.
>
I see. Had a look at that api and it seems pretty limited.
I couldn't find a simple way of reading a property other than scanning
the entire tree with of_scan_flat_dt().
---
Best Regards, Laurentiu
^ permalink raw reply
* Re: [PATCH v2] powerpc/vfio: Add missing locks for take/release TCE table ownership
From: Alexey Kardashevskiy @ 2013-06-26 12:19 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Alexey Kardashevskiy, linux-kernel, Paul Mackerras,
Anton Blanchard, linuxppc-dev, David Gibson
In-Reply-To: <1372227662-28302-1-git-send-email-aik@ozlabs.ru>
Ben, please ignore this.
Need some more code there.
On 06/26/2013 04:21 PM, Alexey Kardashevskiy wrote:
> VFIO IOMMU driver for sPAPR TCE locks the whole DMA window by setting
> ones to iommu_table.it_map. However this was not protected by the locks
> which other clients of iommu_table use.
>
> The patch fixes this.
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>
> v1->v2:
> * Fixed a potential warning from lockdep.
>
> ---
> arch/powerpc/kernel/iommu.c | 25 +++++++++++++++++++++++--
> 1 file changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
> index b20ff17..5c1fc89 100644
> --- a/arch/powerpc/kernel/iommu.c
> +++ b/arch/powerpc/kernel/iommu.c
> @@ -1076,25 +1076,42 @@ EXPORT_SYMBOL_GPL(iommu_put_tce_user_mode);
> int iommu_take_ownership(struct iommu_table *tbl)
> {
> unsigned long sz = (tbl->it_size + 7) >> 3;
> + unsigned long i, flags;
> + int ret = 0;
> +
> + spin_lock_irqsave(&tbl->large_pool.lock, flags);
> + for (i = 0; i < tbl->nr_pools; i++)
> + spin_lock(&tbl->pools[i].lock);
>
> if (tbl->it_offset == 0)
> clear_bit(0, tbl->it_map);
>
> if (!bitmap_empty(tbl->it_map, tbl->it_size)) {
> pr_err("iommu_tce: it_map is not empty");
> - return -EBUSY;
> + ret = -EBUSY;
> + goto unlock_exit;
> }
>
> memset(tbl->it_map, 0xff, sz);
> iommu_clear_tces_and_put_pages(tbl, tbl->it_offset, tbl->it_size);
>
> - return 0;
> +unlock_exit:
> + for (i = 0; i < tbl->nr_pools; i++)
> + spin_unlock(&tbl->pools[i].lock);
> + spin_unlock_irqrestore(&tbl->large_pool.lock, flags);
> +
> + return ret;
> }
> EXPORT_SYMBOL_GPL(iommu_take_ownership);
>
> void iommu_release_ownership(struct iommu_table *tbl)
> {
> unsigned long sz = (tbl->it_size + 7) >> 3;
> + unsigned long i, flags;
> +
> + spin_lock_irqsave(&tbl->large_pool.lock, flags);
> + for (i = 0; i < tbl->nr_pools; i++)
> + spin_lock(&tbl->pools[i].lock);
>
> iommu_clear_tces_and_put_pages(tbl, tbl->it_offset, tbl->it_size);
> memset(tbl->it_map, 0, sz);
> @@ -1102,6 +1119,10 @@ void iommu_release_ownership(struct iommu_table *tbl)
> /* Restore bit#0 set by iommu_init_table() */
> if (tbl->it_offset == 0)
> set_bit(0, tbl->it_map);
> +
> + for (i = 0; i < tbl->nr_pools; i++)
> + spin_unlock(&tbl->pools[i].lock);
> + spin_unlock_irqrestore(&tbl->large_pool.lock, flags);
> }
> EXPORT_SYMBOL_GPL(iommu_release_ownership);
>
>
--
Alexey
^ permalink raw reply
* Re: [PATCH 4/6 v5] KVM: PPC: exit to user space on "ehpriv" instruction
From: Alexander Graf @ 2013-06-26 10:33 UTC (permalink / raw)
To: Bhushan Bharat-R65777
Cc: Wood Scott-B07421, mikey@neuling.org, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, kvm-ppc@vger.kernel.org,
tiejun.chen, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <6A3DF150A5B70D4F9B66A25E3F7C888D07086E00@039-SN2MPN1-013.039d.mgd.msft.net>
On 26.06.2013, at 11:27, Bhushan Bharat-R65777 wrote:
>=20
>=20
>> -----Original Message-----
>> From: tiejun.chen [mailto:tiejun.chen@windriver.com]
>> Sent: Wednesday, June 26, 2013 2:47 PM
>> To: Bhushan Bharat-R65777
>> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; agraf@suse.de; Wood =
Scott-
>> B07421; benh@kernel.crashing.org; linuxppc-dev@lists.ozlabs.org; =
linux-
>> kernel@vger.kernel.org; mikey@neuling.org
>> Subject: Re: [PATCH 4/6 v5] KVM: PPC: exit to user space on "ehpriv" =
instruction
>>=20
>> On 06/26/2013 04:44 PM, Bhushan Bharat-R65777 wrote:
>>>=20
>>>=20
>>>> -----Original Message-----
>>>> From: tiejun.chen [mailto:tiejun.chen@windriver.com]
>>>> Sent: Wednesday, June 26, 2013 12:25 PM
>>>> To: Bhushan Bharat-R65777
>>>> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; agraf@suse.de; =
Wood
>>>> Scott- B07421; benh@kernel.crashing.org;
>>>> linuxppc-dev@lists.ozlabs.org; linux- kernel@vger.kernel.org;
>>>> mikey@neuling.org; Bhushan Bharat-R65777
>>>> Subject: Re: [PATCH 4/6 v5] KVM: PPC: exit to user space on =
"ehpriv"
>>>> instruction
>>>>=20
>>>> On 06/26/2013 01:42 PM, Bharat Bhushan wrote:
>>>>> "ehpriv" instruction is used for setting software breakpoints by
>>>>> user space. This patch adds support to exit to user space with
>>>>> "run->debug" have relevant information.
>>>>>=20
>>>>> As this is the first point we are using run->debug, also defined =
the
>>>>> run->debug structure.
>>>>>=20
>>>>> Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
>>>>> ---
>>>>> arch/powerpc/include/asm/disassemble.h | 4 ++++
>>>>> arch/powerpc/include/uapi/asm/kvm.h | 21 =
+++++++++++++++++----
>>>>> arch/powerpc/kvm/e500_emulate.c | 27 =
+++++++++++++++++++++++++++
>>>>> 3 files changed, 48 insertions(+), 4 deletions(-)
>>>>>=20
>>>>> diff --git a/arch/powerpc/include/asm/disassemble.h
>>>> b/arch/powerpc/include/asm/disassemble.h
>>>>> index 9b198d1..856f8de 100644
>>>>> --- a/arch/powerpc/include/asm/disassemble.h
>>>>> +++ b/arch/powerpc/include/asm/disassemble.h
>>>>> @@ -77,4 +77,8 @@ static inline unsigned int get_d(u32 inst)
>>>>> return inst & 0xffff;
>>>>> }
>>>>>=20
>>>>> +static inline unsigned int get_oc(u32 inst) {
>>>>> + return (inst >> 11) & 0x7fff;
>>>>> +}
>>>>> #endif /* __ASM_PPC_DISASSEMBLE_H__ */ diff --git
>>>>> a/arch/powerpc/include/uapi/asm/kvm.h
>>>> b/arch/powerpc/include/uapi/asm/kvm.h
>>>>> index 0fb1a6e..ded0607 100644
>>>>> --- a/arch/powerpc/include/uapi/asm/kvm.h
>>>>> +++ b/arch/powerpc/include/uapi/asm/kvm.h
>>>>> @@ -269,7 +269,24 @@ struct kvm_fpu {
>>>>> __u64 fpr[32];
>>>>> };
>>>>>=20
>>>>> +/*
>>>>> + * Defines for h/w breakpoint, watchpoint (read, write or both) =
and
>>>>> + * software breakpoint.
>>>>> + * These are used as "type" in KVM_SET_GUEST_DEBUG ioctl and =
"status"
>>>>> + * for KVM_DEBUG_EXIT.
>>>>> + */
>>>>> +#define KVMPPC_DEBUG_NONE 0x0
>>>>> +#define KVMPPC_DEBUG_BREAKPOINT (1UL << 1)
>>>>> +#define KVMPPC_DEBUG_WATCH_WRITE (1UL << 2)
>>>>> +#define KVMPPC_DEBUG_WATCH_READ (1UL << 3)
>>>>> struct kvm_debug_exit_arch {
>>>>> + __u64 address;
>>>>> + /*
>>>>> + * exiting to userspace because of h/w breakpoint, watchpoint
>>>>> + * (read, write or both) and software breakpoint.
>>>>> + */
>>>>> + __u32 status;
>>>>> + __u32 reserved;
>>>>> };
>>>>>=20
>>>>> /* for KVM_SET_GUEST_DEBUG */
>>>>> @@ -281,10 +298,6 @@ struct kvm_guest_debug_arch {
>>>>> * Type denotes h/w breakpoint, read watchpoint, =
write
>>>>> * watchpoint or watchpoint (both read and =
write).
>>>>> */
>>>>> -#define KVMPPC_DEBUG_NONE 0x0
>>>>> -#define KVMPPC_DEBUG_BREAKPOINT (1UL << 1)
>>>>> -#define KVMPPC_DEBUG_WATCH_WRITE (1UL << 2)
>>>>> -#define KVMPPC_DEBUG_WATCH_READ (1UL << 3)
>>>>> __u32 type;
>>>>> __u32 reserved;
>>>>> } bp[16];
>>>>> diff --git a/arch/powerpc/kvm/e500_emulate.c
>>>>> b/arch/powerpc/kvm/e500_emulate.c index b10a012..dab9d07 100644
>>>>> --- a/arch/powerpc/kvm/e500_emulate.c
>>>>> +++ b/arch/powerpc/kvm/e500_emulate.c
>>>>> @@ -26,6 +26,8 @@
>>>>> #define XOP_TLBRE 946
>>>>> #define XOP_TLBWE 978
>>>>> #define XOP_TLBILX 18
>>>>> +#define XOP_EHPRIV 270
>>>>> +#define EHPRIV_OC_DEBUG 0
>>>>=20
>>>> As I think the case, "OC =3D 0", is a bit specific since IIRC, if =
the
>>>> OC operand is omitted, its equal 0 by default. So I think we should
>>>> start this OC value from 1 or other magic number.
>>>=20
>>> ehpriv instruction is defined to be used as:
>>> ehpriv OC // where OC can be 0,1, ... n and in extended for it =
can be
>>> used as
>>> ehpriv // With no OC, and here it assumes OC =3D 0 So OC =3D 0 =
is not
>>> specific but "ehpriv" is same as "ehpriv 0".
>>=20
>> Yes, this is just what I mean.
>>=20
>>>=20
>>> I do not think of any special reason to reserve "ehpriv" and "ehpriv =
0".
>>=20
>> So I still prefer we can reserve the 'ehpriv' without OC operand as =
one simple
>> approach to test or develop something for KVM quickly because its =
really
>> convenient to trap into the hypervisor only with one 'ehpriv' =
instruction
>> easily.
>>=20
>> But I have no further objection if you guys are fine to this ;-)
>=20
> I can see the using "ehpriv" can be a default choice. But all ehvpriv =
trap is handled at one place (in a single function) so the accidently =
overlap with debug should not be an issue.
>=20
> I too do not have any strong opinion to keep either way, so want to =
leave as is ;-).
Seconded. On x86 we also just use int3 for soft breakpoints IIRC.
Alex
^ permalink raw reply
* Re: [PATCH 3/6] powerpc/powernv: Replace variables with flags
From: Gavin Shan @ 2013-06-26 10:08 UTC (permalink / raw)
To: David Laight; +Cc: linuxppc-dev, Gavin Shan
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B72A6@saturn3.aculab.com>
On Wed, Jun 26, 2013 at 10:12:16AM +0100, David Laight wrote:
>> We have 2 fields in "struct pnv_phb" to trace the states. The patch
>> replace the fields with one and introduces flags for that. The patch
>> doesn't impact the logic.
>
>What is the benefit of this change?
>
There might have more flags coming in. Putting all flags together
could be maintained more easily. It doesn't save much memory as
you pointed.
>> +
>> +#define PNV_EEH_STATE_ENABLED (1 << 0) /* EEH enabled */
>> +#define PNV_EEH_STATE_REMOVED (1 << 1) /* PHB removed */
>> +
>> #endif /* CONFIG_EEH */
>>
>> struct pnv_phb {
>> @@ -92,8 +96,7 @@ struct pnv_phb {
>>
>> #ifdef CONFIG_EEH
>> struct pnv_eeh_ops *eeh_ops;
>> - int eeh_enabled;
>> - int removed;
>> + int eeh_state;
>> #endif
>>
>> #ifdef CONFIG_DEBUG_FS
>
>All I can see is that it possibly reduces a structure by 4 bytes
>while adding extra code.
>(On 64 bit systems there might be a 4 byte pad added.)
>
Thanks,
Gavin
^ permalink raw reply
* Re: [PATCH 8/8] powerpc/perf: Add power8 EBB support
From: Anshuman Khandual @ 2013-06-26 9:58 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev, sukadev, Paul Mackerras
In-Reply-To: <1372073336-8189-8-git-send-email-michael@ellerman.id.au>
> @@ -117,6 +117,7 @@
> (EVENT_UNIT_MASK << EVENT_UNIT_SHIFT) | \
> (EVENT_COMBINE_MASK << EVENT_COMBINE_SHIFT) | \
> (EVENT_MARKED_MASK << EVENT_MARKED_SHIFT) | \
> + (1ull << EVENT_CONFIG_EBB_SHIFT) | \
We should define this macro like EVENT_MARKED_MASK
#define EVENT_EBB_MASK 0x1
Numeric value of "1ull" stands out odd in the scheme.
> EVENT_PSEL_MASK)
>
> + * EBB -* | |
> + * | | Count of events for each PMC.
> + * L1 I/D qualifier -* | p1, p2, p3, p4, p5, p6.
> * nc - number of counters -*
> *
> * The PMC fields P1..P6, and NC, are adder fields. As we accumulate constraints
> @@ -159,6 +160,9 @@
> #define CNST_THRESH_VAL(v) (((v) & EVENT_THRESH_MASK) << 32)
> #define CNST_THRESH_MASK CNST_THRESH_VAL(EVENT_THRESH_MASK)
>
> +#define CNST_EBB_VAL(v) (((v) & 1) << 24)
EVENT_EBB_MASK can be used here as well.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox