* Re: [PATCH 5/9] powerpc/eeh: Rename frozen_bus to bus in eeh_handle_normal_event()
From: Russell Currey @ 2018-03-06 0:57 UTC (permalink / raw)
To: Sam Bobroff, linuxppc-dev
In-Reply-To: <a8d5f7792b761721852b68363c68519c6c8b62ff.1520294174.git.sam.bobroff@au1.ibm.com>
On Tue, 2018-03-06 at 10:59 +1100, Sam Bobroff wrote:
> The name "frozen_bus" is misleading: it's not necessarily frozen,
> it's
> just the PE's PCI bus.
>
> Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
Reviewed-by: Russell Currey <ruscur@russell.cc>
^ permalink raw reply
* Re: [PATCH 1/9] powerpc/eeh: Remove eeh_handle_event()
From: Daniel Axtens @ 2018-03-06 1:08 UTC (permalink / raw)
To: Sam Bobroff, linuxppc-dev
In-Reply-To: <d3d01073da02a5eb844cc8998fdac602b8121b62.1520294174.git.sam.bobroff@au1.ibm.com>
Hi Sam,
> The function eeh_handle_event(pe) does nothing other than switching
> between calling eeh_handle_normal_event(pe) and
> eeh_handle_special_event(). However it is only called in two places,
> one where pe can't be NULL and the other where it must be NULL (see
> eeh_event_handler()) so it does nothing but obscure the flow of
> control.
I've verified this.
>
> So, remove it.
Sounds good.
> + * While PHB detects address or data parity errors on particular PCI
> + * slot, the associated PE will be frozen. Besides, DMA's occurring
> + * to wild addresses (which usually happen due to bugs in device
> + * drivers or in PCI adapter firmware) can cause EEH error. #SERR,
> + * #PERR or other misc PCI-related errors also can trigger EEH errors.
> + *
> + * Recovery process consists of unplugging the device driver (which
> + * generated hotplug events to userspace), then issuing a PCI #RST to
> + * the device, then reconfiguring the PCI config space for all bridges
> + * & devices under this slot, and then finally restarting the device
> + * drivers (which cause a second set of hotplug events to go out to
> + * userspace).
> + *
So this is the comment from eeh_handle_event. This seems as good a place
as any to put it. (At some point someone should check if it lines up
well with Documentation/powerpc/eeh-pci-error-recovery.txt but it can wait.)
In conclusion:
Reviewed-by: Daniel Axtens <dja@axtens.net>
Thanks Sam!
Regards,
Daniel
> * Returns true if @pe should no longer be used, else false.
> */
> -static bool eeh_handle_normal_event(struct eeh_pe *pe)
> +bool eeh_handle_normal_event(struct eeh_pe *pe)
> {
> struct pci_bus *frozen_bus;
> struct eeh_dev *edev, *tmp;
> @@ -942,7 +955,7 @@ static bool eeh_handle_normal_event(struct eeh_pe *pe)
> * specific PE. Iterates through possible failures and handles them as
> * necessary.
> */
> -static void eeh_handle_special_event(void)
> +void eeh_handle_special_event(void)
> {
> struct eeh_pe *pe, *phb_pe;
> struct pci_bus *bus;
> @@ -1049,28 +1062,3 @@ static void eeh_handle_special_event(void)
> break;
> } while (rc != EEH_NEXT_ERR_NONE);
> }
> -
> -/**
> - * eeh_handle_event - Reset a PCI device after hard lockup.
> - * @pe: EEH PE
> - *
> - * While PHB detects address or data parity errors on particular PCI
> - * slot, the associated PE will be frozen. Besides, DMA's occurring
> - * to wild addresses (which usually happen due to bugs in device
> - * drivers or in PCI adapter firmware) can cause EEH error. #SERR,
> - * #PERR or other misc PCI-related errors also can trigger EEH errors.
> - *
> - * Recovery process consists of unplugging the device driver (which
> - * generated hotplug events to userspace), then issuing a PCI #RST to
> - * the device, then reconfiguring the PCI config space for all bridges
> - * & devices under this slot, and then finally restarting the device
> - * drivers (which cause a second set of hotplug events to go out to
> - * userspace).
> - */
> -void eeh_handle_event(struct eeh_pe *pe)
> -{
> - if (pe)
> - eeh_handle_normal_event(pe);
> - else
> - eeh_handle_special_event();
> -}
> diff --git a/arch/powerpc/kernel/eeh_event.c b/arch/powerpc/kernel/eeh_event.c
> index accbf8b5fd46..872bcfe8f90e 100644
> --- a/arch/powerpc/kernel/eeh_event.c
> +++ b/arch/powerpc/kernel/eeh_event.c
> @@ -81,10 +81,10 @@ static int eeh_event_handler(void * dummy)
> pr_info("EEH: Detected PCI bus error on "
> "PHB#%x-PE#%x\n",
> pe->phb->global_number, pe->addr);
> - eeh_handle_event(pe);
> + eeh_handle_normal_event(pe);
> eeh_pe_state_clear(pe, EEH_PE_RECOVERING);
> } else {
> - eeh_handle_event(NULL);
> + eeh_handle_special_event();
> }
>
> kfree(event);
> --
> 2.16.1.74.g9b0b1f47b
^ permalink raw reply
* Re: [PATCH 2/9] powerpc/eeh: Manage EEH_PE_RECOVERING inside eeh_handle_normal_event()
From: Daniel Axtens @ 2018-03-06 1:47 UTC (permalink / raw)
To: Sam Bobroff, linuxppc-dev
In-Reply-To: <163b24ef504d753f4abf1510d9193962bbac32b7.1520294174.git.sam.bobroff@au1.ibm.com>
Hi Sam,
> Currently the EEH_PE_RECOVERING flag for a PE is managed by both the
> caller and callee of eeh_handle_normal_event() (among other places not
> considered here). This is complicated by the fact that the PE may
> or may not have been invalidated by the call.
So, I applied this patch and looked at those places. They're now
restricted to eeh_driver.c and eeh.c.
The only other place it's marked is
eeh_driver.c::eeh_pe_reset_and_recover(), which is itself only called
from eeh.c::eeh_pe_change_owner(). That is only called from 2 places,
both in eeh.c - eeh_dev_open() and eeh_dev_release(). I have not yet
wrapped my head around the logic of that.
I suspect the entire logic can have some more simplifications, but this
is definitely a good step.
I also read through your patch and have no other comments, so:
Reviewed-by: Daniel Axtens <dja@axtens.net>
Regards,
Daniel
>
> So move the callee's handling into eeh_handle_normal_event(), which
> clarifies it and allows the return type to be changed to void (because
> it no longer needs to indicate at the PE has been invalidated).
>
> This should not change behaviour except in eeh_event_handler() where
> it was previously possible to cause eeh_pe_state_clear() to be called
> on an invalid PE, which is now avoided.
>
> Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
> ---
> arch/powerpc/include/asm/eeh_event.h | 2 +-
> arch/powerpc/kernel/eeh_driver.c | 29 +++++++++++------------------
> arch/powerpc/kernel/eeh_event.c | 2 --
> 3 files changed, 12 insertions(+), 21 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/eeh_event.h b/arch/powerpc/include/asm/eeh_event.h
> index 0a168038882d..9884e872686f 100644
> --- a/arch/powerpc/include/asm/eeh_event.h
> +++ b/arch/powerpc/include/asm/eeh_event.h
> @@ -34,7 +34,7 @@ struct eeh_event {
> int eeh_event_init(void);
> int eeh_send_failure_event(struct eeh_pe *pe);
> void eeh_remove_event(struct eeh_pe *pe, bool force);
> -bool eeh_handle_normal_event(struct eeh_pe *pe);
> +void eeh_handle_normal_event(struct eeh_pe *pe);
> void eeh_handle_special_event(void);
>
> #endif /* __KERNEL__ */
> diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
> index 51b21c97910f..5b7a5ed4db4d 100644
> --- a/arch/powerpc/kernel/eeh_driver.c
> +++ b/arch/powerpc/kernel/eeh_driver.c
> @@ -733,7 +733,8 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
>
> /**
> * eeh_handle_normal_event - Handle EEH events on a specific PE
> - * @pe: EEH PE
> + * @pe: EEH PE - which should not be used after we return, as it may
> + * have been invalidated.
> *
> * Attempts to recover the given PE. If recovery fails or the PE has failed
> * too many times, remove the PE.
> @@ -750,10 +751,8 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus,
> * & devices under this slot, and then finally restarting the device
> * drivers (which cause a second set of hotplug events to go out to
> * userspace).
> - *
> - * Returns true if @pe should no longer be used, else false.
> */
> -bool eeh_handle_normal_event(struct eeh_pe *pe)
> +void eeh_handle_normal_event(struct eeh_pe *pe)
> {
> struct pci_bus *frozen_bus;
> struct eeh_dev *edev, *tmp;
> @@ -765,9 +764,11 @@ bool eeh_handle_normal_event(struct eeh_pe *pe)
> if (!frozen_bus) {
> pr_err("%s: Cannot find PCI bus for PHB#%x-PE#%x\n",
> __func__, pe->phb->global_number, pe->addr);
> - return false;
> + return;
> }
>
> + eeh_pe_state_mark(pe, EEH_PE_RECOVERING);
> +
> eeh_pe_update_time_stamp(pe);
> pe->freeze_count++;
> if (pe->freeze_count > eeh_max_freezes) {
> @@ -904,7 +905,7 @@ bool eeh_handle_normal_event(struct eeh_pe *pe)
> pr_info("EEH: Notify device driver to resume\n");
> eeh_pe_dev_traverse(pe, eeh_report_resume, NULL);
>
> - return false;
> + goto final;
>
> hard_fail:
> /*
> @@ -940,12 +941,12 @@ bool eeh_handle_normal_event(struct eeh_pe *pe)
> pci_lock_rescan_remove();
> pci_hp_remove_devices(frozen_bus);
> pci_unlock_rescan_remove();
> -
> /* The passed PE should no longer be used */
> - return true;
> + return;
> }
> }
> - return false;
> +final:
> + eeh_pe_state_clear(pe, EEH_PE_RECOVERING);
> }
>
> /**
> @@ -1018,15 +1019,7 @@ void eeh_handle_special_event(void)
> */
> if (rc == EEH_NEXT_ERR_FROZEN_PE ||
> rc == EEH_NEXT_ERR_FENCED_PHB) {
> - /*
> - * eeh_handle_normal_event() can make the PE stale if it
> - * determines that the PE cannot possibly be recovered.
> - * Don't modify the PE state if that's the case.
> - */
> - if (eeh_handle_normal_event(pe))
> - continue;
> -
> - eeh_pe_state_clear(pe, EEH_PE_RECOVERING);
> + eeh_handle_normal_event(pe);
> } else {
> pci_lock_rescan_remove();
> list_for_each_entry(hose, &hose_list, list_node) {
> diff --git a/arch/powerpc/kernel/eeh_event.c b/arch/powerpc/kernel/eeh_event.c
> index 872bcfe8f90e..61c9356bf9c9 100644
> --- a/arch/powerpc/kernel/eeh_event.c
> +++ b/arch/powerpc/kernel/eeh_event.c
> @@ -73,7 +73,6 @@ static int eeh_event_handler(void * dummy)
> /* We might have event without binding PE */
> pe = event->pe;
> if (pe) {
> - eeh_pe_state_mark(pe, EEH_PE_RECOVERING);
> if (pe->type & EEH_PE_PHB)
> pr_info("EEH: Detected error on PHB#%x\n",
> pe->phb->global_number);
> @@ -82,7 +81,6 @@ static int eeh_event_handler(void * dummy)
> "PHB#%x-PE#%x\n",
> pe->phb->global_number, pe->addr);
> eeh_handle_normal_event(pe);
> - eeh_pe_state_clear(pe, EEH_PE_RECOVERING);
> } else {
> eeh_handle_special_event();
> }
> --
> 2.16.1.74.g9b0b1f47b
^ permalink raw reply
* Re: [PATCH 4/9] powerpc/eeh: Remove misleading test in eeh_handle_normal_event()
From: Daniel Axtens @ 2018-03-06 2:14 UTC (permalink / raw)
To: Sam Bobroff, linuxppc-dev
In-Reply-To: <52c15c0414d248981e251386384ed2a716315003.1520294174.git.sam.bobroff@au1.ibm.com>
Sam Bobroff <sam.bobroff@au1.ibm.com> writes:
> Remove a test that checks if "frozen_bus" is NULL, because it cannot
> have changed since it was tested at the start of the function and so
> must be true here.
As far as I can tell this was added back in 2012 in 9b3c76f08122f. As
far as I can tell it couldn't be NULL back then either. Weird.
LGTM, but I'm not super comfortable chnging something I don't understand
so I will leave the formal review for now.
Regards,
Daniel
>
> Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
> ---
> arch/powerpc/kernel/eeh_driver.c | 24 +++++++++++-------------
> 1 file changed, 11 insertions(+), 13 deletions(-)
>
> diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
> index 5b7a5ed4db4d..04a5d9db5499 100644
> --- a/arch/powerpc/kernel/eeh_driver.c
> +++ b/arch/powerpc/kernel/eeh_driver.c
> @@ -930,20 +930,18 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
> * all removed devices correctly to avoid access
> * the their PCI config any more.
> */
> - if (frozen_bus) {
> - if (pe->type & EEH_PE_VF) {
> - eeh_pe_dev_traverse(pe, eeh_rmv_device, NULL);
> - eeh_pe_dev_mode_mark(pe, EEH_DEV_REMOVED);
> - } else {
> - eeh_pe_state_clear(pe, EEH_PE_PRI_BUS);
> - eeh_pe_dev_mode_mark(pe, EEH_DEV_REMOVED);
> + if (pe->type & EEH_PE_VF) {
> + eeh_pe_dev_traverse(pe, eeh_rmv_device, NULL);
> + eeh_pe_dev_mode_mark(pe, EEH_DEV_REMOVED);
> + } else {
> + eeh_pe_state_clear(pe, EEH_PE_PRI_BUS);
> + eeh_pe_dev_mode_mark(pe, EEH_DEV_REMOVED);
>
> - pci_lock_rescan_remove();
> - pci_hp_remove_devices(frozen_bus);
> - pci_unlock_rescan_remove();
> - /* The passed PE should no longer be used */
> - return;
> - }
> + pci_lock_rescan_remove();
> + pci_hp_remove_devices(frozen_bus);
> + pci_unlock_rescan_remove();
> + /* The passed PE should no longer be used */
> + return;
> }
> final:
> eeh_pe_state_clear(pe, EEH_PE_RECOVERING);
> --
> 2.16.1.74.g9b0b1f47b
^ permalink raw reply
* Re: [PATCH 6/9] powerpc/eeh: Clarify arguments to eeh_reset_device()
From: Russell Currey @ 2018-03-06 4:37 UTC (permalink / raw)
To: Sam Bobroff, linuxppc-dev
In-Reply-To: <c9191611543d221d5f003fad4a68dd5f86839404.1520294174.git.sam.bobroff@au1.ibm.com>
On Tue, 2018-03-06 at 10:59 +1100, Sam Bobroff wrote:
> It is currently difficult to understand the behaviour of
> eeh_reset_device() due to the way it's parameters are used. In
> particular, when 'bus' is NULL, it's value is still necessary so the
> same value is looked up again locally under a different name
> ('frozen_bus') but behaviour is changed.
>
> To clarify this, add a new parameter 'eeh_aware_driver', and have the
> caller set it when it would have passed NULL for 'bus' and always
> pass
> a value for 'bus'. Then change any test that was on 'bus' to one on
> '!eeh_aware_driver' and replace uses of 'frozen_bus' with 'bus'.
>
> Also update the function's comment.
>
> This should not change behaviour.
>
> Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
> ---
> arch/powerpc/kernel/eeh_driver.c | 22 ++++++++++++----------
> 1 file changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/arch/powerpc/kernel/eeh_driver.c
> b/arch/powerpc/kernel/eeh_driver.c
> index cb584d72b0a5..6c3577133223 100644
> --- a/arch/powerpc/kernel/eeh_driver.c
> +++ b/arch/powerpc/kernel/eeh_driver.c
> @@ -619,17 +619,19 @@ int eeh_pe_reset_and_recover(struct eeh_pe *pe)
>
> /**
> * eeh_reset_device - Perform actual reset of a pci slot
> + * @eeh_aware_driver: Does the device's driver provide EEH support?
> * @pe: EEH PE
> * @bus: PCI bus corresponding to the isolcated slot
> + * @rmv_data: Optional, list to record removed devices
> *
> * This routine must be called to do reset on the indicated PE.
> * During the reset, udev might be invoked because those affected
> * PCI devices will be removed and then added.
> */
Just a nitpick here, I would prefer that the PE remain the first
argument of the function since that's one thing that's pretty standard
across the EEH implementation. I'd also say that "driver_aware" is
clearer than "aware_driver".
Reviewed-by: Russell Currey <ruscur@russell.cc>
^ permalink raw reply
* Re: [PATCH 7/9] powerpc/eeh: Remove always-true tests in eeh_reset_device()
From: Russell Currey @ 2018-03-06 4:38 UTC (permalink / raw)
To: Sam Bobroff, linuxppc-dev
In-Reply-To: <62fba6f13022a6d3cfb1e6468560abce10b0abfc.1520294174.git.sam.bobroff@au1.ibm.com>
On Tue, 2018-03-06 at 10:59 +1100, Sam Bobroff wrote:
> eeh_reset_device() tests the value of 'bus' more than once but the
> only caller, eeh_handle_normal_device() does this test itself and
> will
> never pass NULL.
>
> So, remove the dead tests.
>
> This should not change behaviour.
>
> Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
Reviewed-by: Russell Currey <ruscur@russell.cc>
^ permalink raw reply
* RE: [PATCH 5/6] dma-mapping: support fsl-mc bus
From: Nipun Gupta @ 2018-03-06 4:41 UTC (permalink / raw)
To: Robin Murphy, Christoph Hellwig
Cc: will.deacon@arm.com, mark.rutland@arm.com,
catalin.marinas@arm.com, iommu@lists.linux-foundation.org,
robh+dt@kernel.org, m.szyprowski@samsung.com,
gregkh@linuxfoundation.org, joro@8bytes.org, Leo Li,
shawnguo@kernel.org, linux-kernel@vger.kernel.org,
devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linuxppc-dev@lists.ozlabs.org, Bharat Bhushan, stuyoder@gmail.com,
Laurentiu Tudor
In-Reply-To: <1729ae21-d08c-b413-51a3-f22c394b388d@arm.com>
DQoNCj4gRnJvbTogUm9iaW4gTXVycGh5IFttYWlsdG86cm9iaW4ubXVycGh5QGFybS5jb21dDQo+
IFNlbnQ6IFR1ZXNkYXksIE1hcmNoIDA2LCAyMDE4IDA6MjINCj4gDQo+IE9uIDA1LzAzLzE4IDE4
OjM5LCBDaHJpc3RvcGggSGVsbHdpZyB3cm90ZToNCj4gPiBPbiBNb24sIE1hciAwNSwgMjAxOCBh
dCAwMzo0ODozMlBNICswMDAwLCBSb2JpbiBNdXJwaHkgd3JvdGU6DQo+ID4+IFVuZm9ydHVuYXRl
bHkgZm9yIHVzLCBmc2wtbWMgaXMgY29uY2VwdHVhbGx5IHJhdGhlciBsaWtlIFBDSSBpbiB0aGF0
IGl0J3MNCj4gPj4gc29mdHdhcmUtZGlzY292ZXJhYmxlIGFuZCB0aGUgb25seSB0aGluZyBkZXNj
cmliZWQgaW4gRFQgaXMgdGhlIGJ1cyAiaG9zdCIsDQo+ID4+IHRodXMgd2UgbmVlZCB0aGUgc2Ft
ZSBzb3J0IG9mIHRoaW5nIGFzIGZvciBQQ0kgdG8gbWFwIGZyb20gdGhlIGNoaWxkDQo+ID4+IGRl
dmljZXMgYmFjayB0byB0aGUgYnVzIHJvb3QgaW4gb3JkZXIgdG8gZmluZCB0aGUgYXBwcm9wcmlh
dGUgZmlybXdhcmUNCj4gPj4gbm9kZS4gV29yc2UgdGhhbiBQQ0ksIHRob3VnaCwgd2Ugd291bGRu
J3QgZXZlbiBoYXZlIHRoZSBvcHRpb24gb2YNCj4gPj4gZGVzY3JpYmluZyBjaGlsZCBkZXZpY2Vz
IHN0YXRpY2FsbHkgaW4gZmlybXdhcmUgYXQgYWxsLCBzaW5jZSBpdCdzIGFjdHVhbGx5DQo+ID4+
IG9uZSBvZiB0aGVzZSBydW50aW1lLWNvbmZpZ3VyYWJsZSAiYnVpbGQgeW91ciBvd24gbmV0d29y
ayBhY2NlbGVyYXRvciINCj4gPj4gaGFyZHdhcmUgcG9vbHMgd2hlcmUgdXNlcnNwYWNlIGdldHMg
dG8gY3JlYXRlIGFuZCBkZXN0cm95ICJkZXZpY2VzIiBhcyBpdA0KPiA+PiBsaWtlcy4NCj4gPg0K
PiA+IEkgcmVhbGx5IGhhdGUgdGhlIFBDSSBzcGVjaWFsIGNhc2UganVzdCBhcyBtdWNoLiAgTWF5
YmUgd2UganVzdA0KPiA+IG5lZWQgYSBkbWFfY29uZmlndXJlIG1ldGhvZCBvbiB0aGUgYnVzLCBh
bmQgbW92ZSBQQ0kgYXMgd2VsbCBhcyBmc2wtbWMNCj4gPiB0byBpdC4NCj4gDQo+IEhtbSwgb24g
cmVmbGVjdGlvbiwgMTAwJSBhY2sgdG8gdGhhdCBpZGVhLiBJdCB3b3VsZCBuZWF0bHkgc3VwZXJz
ZWRlDQo+IGJ1cy0+Zm9yY2VfZG1hICphbmQqIG1lYW4gdGhhdCB3ZSBkb24ndCBoYXZlIHRvIGVm
ZmVjdGl2ZWx5IHB1bGwgcGNpLmgNCj4gaW50byBldmVyeXRoaW5nLCB3aGljaCBJJ3ZlIG5ldmVy
IGxpa2VkLiBJbiBoaW5kc2lnaHQgZG1hX2NvbmZpZ3VyZSgpDQo+IGRvZXMgZmVlbCBsaWtlIGl0
J3MgZ3Jvd24gaW50byB0aGlzIG9kZCBjaG9rZSBwb2ludCB3aGVyZSB3ZSBtdW5nZQ0KPiBldmVy
eXRoaW5nIGluIGp1c3QgZm9yIGl0IHRvIGF3a3dhcmRseSB1bnBpY2sgdGhpbmdzIGFnYWluLg0K
PiANCj4gUm9iaW4uDQoNCisxIHRvIHRoZSBpZGVhLg0KDQpTb3JyeSBmb3IgYXNraW5nIGEgdHJp
dmlhbCBxdWVzdGlvbiAtIGxvb2tpbmcgaW50byBkbWFfY29uZmlndXJlKCkgSSBzZWUgdGhhdA0K
UENJIGlzIHVzZWQgaW4gdGhlIHN0YXJ0IGFuZCB0aGUgZW5kIG9mIHRoZSBBUEkuDQpJbiB0aGUg
ZW5kIHBhcnQgcGNpX3B1dF9ob3N0X2JyaWRnZV9kZXZpY2UoKSBpcyBjYWxsZWQuDQpTbyBhcmUg
dHdvIGJ1cyBjYWxsYmFja3Mgc29tZXRoaW5nIGxpa2UgJ2RtYV9jb25maWdfc3RhcnQnICYgJ2Rt
YV9jb25maWdfZW5kJw0Kd2lsbCBiZSByZXF1aXJlZCB3aGVyZSB0aGUgZm9ybWVyIG9uZSB3aWxs
IHJldHVybiAiZG1hX2RldiI/DQoNClJlZ2FyZHMsDQpOaXB1bg0K
^ permalink raw reply
* Re: [PATCH 8/9] powerpc/eeh: Factor out common code eeh_reset_device()
From: Russell Currey @ 2018-03-06 5:48 UTC (permalink / raw)
To: Sam Bobroff, linuxppc-dev
In-Reply-To: <fe5673642328c2e11fee269bfc7987095311f25c.1520294174.git.sam.bobroff@au1.ibm.com>
On Tue, 2018-03-06 at 10:59 +1100, Sam Bobroff wrote:
> The caller will always pass NULL for 'rmv_data' when
> 'eeh_aware_driver' is true, so the first two calls to
> eeh_pe_dev_traverse() can be combined without changing behaviour as
> can the two arms of the final 'if' block.
>
> This should not change behaviour.
>
> Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
> ---
Reviewed-by: Russell Currey <ruscur@russell.cc>
minor nitpick below
> arch/powerpc/kernel/eeh_driver.c | 36 ++++++++++++----------------
> --------
> 1 file changed, 12 insertions(+), 24 deletions(-)
>
> diff --git a/arch/powerpc/kernel/eeh_driver.c
> b/arch/powerpc/kernel/eeh_driver.c
> index 1272f2c8cbd2..39d560e9f071 100644
> --- a/arch/powerpc/kernel/eeh_driver.c
> +++ b/arch/powerpc/kernel/eeh_driver.c
> @@ -647,16 +647,12 @@ static int eeh_reset_device(bool
> eeh_aware_driver,
> * into pci_hp_add_devices().
> */
> eeh_pe_state_mark(pe, EEH_PE_KEEP);
> - if (!eeh_aware_driver) {
> - if (pe->type & EEH_PE_VF) {
> - eeh_pe_dev_traverse(pe, eeh_rmv_device,
> NULL);
> - } else {
> - pci_lock_rescan_remove();
> - pci_hp_remove_devices(bus);
> - pci_unlock_rescan_remove();
> - }
> - } else {
> + if (eeh_aware_driver || (pe->type & EEH_PE_VF)) {
> eeh_pe_dev_traverse(pe, eeh_rmv_device, rmv_data);
> + } else {
> + pci_lock_rescan_remove();
> + pci_hp_remove_devices(bus);
> + pci_unlock_rescan_remove();
> }
>
> /*
> @@ -691,8 +687,9 @@ static int eeh_reset_device(bool
> eeh_aware_driver,
> * the device up before the scripts have taken it down,
> * potentially weird things happen.
> */
> - if (!eeh_aware_driver) {
> - pr_info("EEH: Sleep 5s ahead of complete
> hotplug\n");
> + if (!eeh_aware_driver || rmv_data->removed) {
> + pr_info("EEH: Sleep 5s ahead of %s hotplug\n",
> + (eeh_aware_driver ? "partial" :
> "complete"));
> ssleep(5);
>
> /*
> @@ -700,24 +697,15 @@ static int eeh_reset_device(bool
> eeh_aware_driver,
> * PE. We should disconnect it so the binding can be
> * rebuilt when adding PCI devices.
> */
> - edev = list_first_entry(&pe->edevs, struct eeh_dev,
> list);
> - eeh_pe_traverse(pe, eeh_pe_detach_dev, NULL);
> - if (pe->type & EEH_PE_VF) {
> - eeh_add_virt_device(edev, NULL);
> - } else {
> - eeh_pe_state_clear(pe, EEH_PE_PRI_BUS);
> - pci_hp_add_devices(bus);
> - }
> - } else if (rmv_data->removed) {
> - pr_info("EEH: Sleep 5s ahead of partial hotplug\n");
> - ssleep(5);
> -
> edev = list_first_entry(&pe->edevs, struct eeh_dev,
> list);
> eeh_pe_traverse(pe, eeh_pe_detach_dev, NULL);
> if (pe->type & EEH_PE_VF)
> eeh_add_virt_device(edev, NULL);
> - else
> + else {
Put brackets around the if block now that the else block isn't a one
liner
> + if (!eeh_aware_driver)
> + eeh_pe_state_clear(pe,
> EEH_PE_PRI_BUS);
> pci_hp_add_devices(bus);
> + }
> }
> eeh_pe_state_clear(pe, EEH_PE_KEEP);
>
^ permalink raw reply
* Re: [PATCH 9/9] powerpc/eeh: Add eeh_state_active() helper
From: Russell Currey @ 2018-03-06 5:49 UTC (permalink / raw)
To: Sam Bobroff, linuxppc-dev
In-Reply-To: <2c6264797f9efd44679fa8c8ac4f8beb11a38c18.1520294174.git.sam.bobroff@au1.ibm.com>
On Tue, 2018-03-06 at 11:00 +1100, Sam Bobroff wrote:
> Checking for a "fully active" device state requires testing two flag
> bits, which is open coded in several places, so add a function to do
> it.
>
> Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
Thanks for the patches.
Reviewed-by: Russell Currey <ruscur@russell.cc>
^ permalink raw reply
* [FIX PATCH] powerpc/pseries: Fix vector5 in ibm architecture vector table
From: Bharata B Rao @ 2018-03-06 8:14 UTC (permalink / raw)
To: linuxppc-dev; +Cc: nfont, mwb, mpe, Bharata B Rao
With ibm,dynamic-memory-v2 and ibm,drc-info coming around the
same time, byte22 in vector5 of ibm architecture vector table
got set twice separately. The end result is that guest kernel
isn't advertising support for ibm,dynamic-memory-v2.
Fix this by removing the duplicate assignment of byte22.
Fixes: 02ef6dd8109b5 (powerpc: Enable support for ibm,drc-info devtree property)
Fixes: c7a3275e0f9e4 (powerpc/pseries: Revert support for ibm,drc-info devtree property)
Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
---
arch/powerpc/kernel/prom_init.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index d22c41c26bb3..acf4b2e0530c 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -874,7 +874,6 @@ struct ibm_arch_vec __cacheline_aligned ibm_architecture_vec = {
.mmu = 0,
.hash_ext = 0,
.radix_ext = 0,
- .byte22 = 0,
},
/* option vector 6: IBM PAPR hints */
--
2.14.3
^ permalink raw reply related
* Re: [PATCH 2/3] rfi-flush: Make it possible to call setup_rfi_flush() again
From: Michal Suchánek @ 2018-03-06 10:21 UTC (permalink / raw)
To: Mauricio Faria de Oliveira; +Cc: Michael Ellerman, linuxppc-dev
In-Reply-To: <2bb5a110-3ea4-af5c-81fb-cf5350587dff@linux.vnet.ibm.com>
On Mon, 5 Mar 2018 19:46:12 -0300
Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com> wrote:
> Hi Michael, Michal,
>=20
> I got back from vacation. Checking this one.
>=20
> On 02/20/2018 02:06 PM, Michal Such=C3=A1nek wrote:
> >> I did it the way I did because otherwise we waste memory on every
> >> system on earth just to support a use case that we don't actually
> >> intend for anyone to ever use - ie. migrating from a patched
> >> machine to an unpatched machine. =20
>=20
> If this thread eventually closes in 'ok, so that memory has to be
> reserved/wasted anyway', that can be done only in pseries, right?
>=20
> It seems not so much memory for this particular platform/hardware.
Yes, that is what I think too.
>=20
> > If you have multiple hosts running some LPMs and want to update them
> > without shutting down the whole thing I suppose it might easily
> > happen that a machine (re)started on a patched host is migrated to
> > unpatched host. =20
>=20
> Right, but that should be temporary, I think -- after updating some of
> the hosts, the LPAR(s) can be migrated back to one of them, where the
> fallback flush is not required anymore.
Temporary, yes. But for how long?
Also is it possible to migrate between P8 and P9 in P8 compat mode?
AFAIK on P9 fallback mode is used exclusively so far.
>=20
> >> I think I'm inclined to leave it the way it is, unless you feel
> >> strongly about it Michal? =20
>=20
> > I think it would be more user friendly to either support the
> > fallback method 100% or remove it and require patched firmware. =20
>=20
> I beg to disagree. Since the matter is a security issue, the option
> of still have some sort of fix that works on unpatched firmware does
> look good and friendly to users (rather than require 'you _must_ get
> the firmware update') IMHO.
Specifically because this is a security issue user friendly is that
user can tell clearly if the fix is applied or not. If the kernel tells
the user the fix is applied and later it tells "sorry, could not
allocate more memory. disabling fix" this is really unfriendly.=20
Thanks
Michal
^ permalink raw reply
* Re: [PATCH v3] powerpc/kernel/sysfs: Export ldbar spr to sysfs
From: Michael Ellerman @ 2018-03-06 11:05 UTC (permalink / raw)
To: Anju T Sudhakar; +Cc: linuxppc-dev, linux-kernel, maddy, anju
In-Reply-To: <1510232445-10869-1-git-send-email-anju@linux.vnet.ibm.com>
Anju T Sudhakar <anju@linux.vnet.ibm.com> writes:
> diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
> index 4437c70..caefb64 100644
> --- a/arch/powerpc/kernel/sysfs.c
> +++ b/arch/powerpc/kernel/sysfs.c
> @@ -757,6 +759,9 @@ static int register_cpu_online(unsigned int cpu)
> device_create_file(s, &pmc_attrs[i]);
>
> #ifdef CONFIG_PPC64
> + if (cpu_has_feature(CPU_FTR_ARCH_300))
> + device_create_file(s, &dev_attr_ldbar);
Is this register readable in supervisor state?
cheers
^ permalink raw reply
* Re: [PATCH 2/3] rfi-flush: Make it possible to call setup_rfi_flush() again
From: Michael Ellerman @ 2018-03-06 12:15 UTC (permalink / raw)
To: Mauricio Faria de Oliveira, Michal Suchánek; +Cc: linuxppc-dev
In-Reply-To: <2bb5a110-3ea4-af5c-81fb-cf5350587dff@linux.vnet.ibm.com>
Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com> writes:
> Hi Michael, Michal,
>
> I got back from vacation. Checking this one.
Yeah it got stuck in a rut.
> On 02/20/2018 02:06 PM, Michal Such=C3=A1nek wrote:
>>> I did it the way I did because otherwise we waste memory on every
>>> system on earth just to support a use case that we don't actually
>>> intend for anyone to ever use - ie. migrating from a patched machine
>>> to an unpatched machine.
>
> If this thread eventually closes in 'ok, so that memory has to be
> reserved/wasted anyway', that can be done only in pseries, right?
>
> It seems not so much memory for this particular platform/hardware.
Yes and no. We certainly have pseries machines with 10s of TBs of
memory, but pseries is also used when running on KVM/Openstack where
some people are interested in running small VMs - so as usual it's not
so simple :)
But it's not *that* much memory so we should probably stop worrying
about it and just always allocate the fallback on pseries.
I *think* the patch below is all we need, as well as some tweaking of
patch 2, are you able to test and repost?
cheers
diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platform=
s/pseries/setup.c
index 1a527625acf7..9116824bd7c5 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -468,26 +468,18 @@ static void pseries_setup_rfi_flush(void)
=20
/* Enable by default */
enable =3D true;
+ types =3D L1D_FLUSH_FALLBACK;
=20
rc =3D plpar_get_cpu_characteristics(&result);
if (rc =3D=3D H_SUCCESS) {
- types =3D L1D_FLUSH_NONE;
-
if (result.character & H_CPU_CHAR_L1D_FLUSH_TRIG2)
types |=3D L1D_FLUSH_MTTRIG;
if (result.character & H_CPU_CHAR_L1D_FLUSH_ORI30)
types |=3D L1D_FLUSH_ORI;
=20
- /* Use fallback if nothing set in hcall */
- if (types =3D=3D L1D_FLUSH_NONE)
- types =3D L1D_FLUSH_FALLBACK;
-
if ((!(result.behaviour & H_CPU_BEHAV_L1D_FLUSH_PR)) ||
(!(result.behaviour & H_CPU_BEHAV_FAVOUR_SECURITY)))
enable =3D false;
- } else {
- /* Default to fallback if case hcall is not available */
- types =3D L1D_FLUSH_FALLBACK;
}
=20
setup_rfi_flush(types, enable);
^ permalink raw reply related
* Re: [PATCH v3 02/10] include: Move compat_timespec/ timeval to compat_time.h
From: Arnd Bergmann @ 2018-03-06 12:46 UTC (permalink / raw)
To: Christian Borntraeger
Cc: Deepa Dinamani, Thomas Gleixner, John Stultz, Mark Rutland,
open list:RALINK MIPS ARCHITECTURE, Peter Zijlstra,
Heiko Carstens, Paul Mackerras, H. Peter Anvin, sparclinux, devel,
linux-s390, y2038 Mailman List, Helge Deller,
the arch/x86 maintainers, sebott, James E.J. Bottomley,
Will Deacon, Ingo Molnar, oprofile-list, Catalin Marinas,
Robert Richter, Chris Metcalf, Peter Oberparleiter,
Arnaldo Carvalho de Melo, Julian Wiedmann, Steven Rostedt,
Ursula Braun, gerald.schaefer, Parisc List, gregkh, cohuck,
Linux Kernel Mailing List, Ralf Baechle, Jan Hoeppner,
Stefan Haberland, Martin Schwidefsky, linuxppc-dev, David Miller
In-Reply-To: <c6fb6676-a8d3-8893-660c-2b9899c5d5ab@de.ibm.com>
On Mon, Mar 5, 2018 at 10:30 AM, Christian Borntraeger
<borntraeger@de.ibm.com> wrote:
> On 01/16/2018 03:18 AM, Deepa Dinamani wrote:
>> All the current architecture specific defines for these
>> are the same. Refactor these common defines to a common
>> header file.
>>
>> The new common linux/compat_time.h is also useful as it
>> will eventually be used to hold all the defines that
>> are needed for compat time types that support non y2038
>> safe types. New architectures need not have to define these
>> new types as they will only use new y2038 safe syscalls.
>> This file can be deleted after y2038 when we stop supporting
>> non y2038 safe syscalls.
>
> You are now include a <linux/*.h> from several asm files
> (
> arch/arm64/include/asm/stat.h
> arch/s390/include/asm/elf.h
> arch/x86/include/asm/ftrace.h
> arch/x86/include/asm/sys_ia32.h
> )
> It works, and it is done in many places, but it looks somewhat weird.
> Would it make sense to have an asm-generic/compate-time.h instead? Asking for
> opinions here.
I don't think we have such a rule. If a header file is common to all
architectures (i.e. no architecture uses a different implementation),
it should be in include/linux rather than include/asm-generic, regardless
of whether it can be used by assembler files or not.
>> --- a/drivers/s390/net/qeth_core_main.c
>> +++ b/drivers/s390/net/qeth_core_main.c
>> @@ -32,7 +32,7 @@
>> #include <asm/chpid.h>
>> #include <asm/io.h>
>> #include <asm/sysinfo.h>
>> -#include <asm/compat.h>
>> +#include <linux/compat.h>
>> #include <asm/diag.h>
>> #include <asm/cio.h>
>> #include <asm/ccwdev.h>
>
> Can you move that into the other includes (where all the other <linux/*> includes are.
Good catch, this is definitely a rule we have ;-)
Arnd
^ permalink raw reply
* Re: [PATCH v3 02/10] include: Move compat_timespec/ timeval to compat_time.h
From: Christian Borntraeger @ 2018-03-06 12:48 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Deepa Dinamani, Thomas Gleixner, John Stultz, Mark Rutland,
open list:RALINK MIPS ARCHITECTURE, Peter Zijlstra,
Heiko Carstens, Paul Mackerras, H. Peter Anvin, sparclinux, devel,
linux-s390, y2038 Mailman List, Helge Deller,
the arch/x86 maintainers, sebott, James E.J. Bottomley,
Will Deacon, Ingo Molnar, oprofile-list, Catalin Marinas,
Robert Richter, Chris Metcalf, Peter Oberparleiter,
Arnaldo Carvalho de Melo, Julian Wiedmann, Steven Rostedt,
Ursula Braun, gerald.schaefer, Parisc List, gregkh, cohuck,
Linux Kernel Mailing List, Ralf Baechle, Jan Hoeppner,
Stefan Haberland, Martin Schwidefsky, linuxppc-dev, David Miller
In-Reply-To: <CAK8P3a0Gm1L70EaFzJBk0drRNKtX0FE22BHOSrXBgH1wNfKZ5A@mail.gmail.com>
On 03/06/2018 01:46 PM, Arnd Bergmann wrote:
> On Mon, Mar 5, 2018 at 10:30 AM, Christian Borntraeger
> <borntraeger@de.ibm.com> wrote:
>> On 01/16/2018 03:18 AM, Deepa Dinamani wrote:
>>> All the current architecture specific defines for these
>>> are the same. Refactor these common defines to a common
>>> header file.
>>>
>>> The new common linux/compat_time.h is also useful as it
>>> will eventually be used to hold all the defines that
>>> are needed for compat time types that support non y2038
>>> safe types. New architectures need not have to define these
>>> new types as they will only use new y2038 safe syscalls.
>>> This file can be deleted after y2038 when we stop supporting
>>> non y2038 safe syscalls.
>>
>> You are now include a <linux/*.h> from several asm files
>> (
>> arch/arm64/include/asm/stat.h
>> arch/s390/include/asm/elf.h
>> arch/x86/include/asm/ftrace.h
>> arch/x86/include/asm/sys_ia32.h
>> )
>> It works, and it is done in many places, but it looks somewhat weird.
>> Would it make sense to have an asm-generic/compate-time.h instead? Asking for
>> opinions here.
>
> I don't think we have such a rule. If a header file is common to all
> architectures (i.e. no architecture uses a different implementation),
> it should be in include/linux rather than include/asm-generic, regardless
> of whether it can be used by assembler files or not.
>
>>> --- a/drivers/s390/net/qeth_core_main.c
>>> +++ b/drivers/s390/net/qeth_core_main.c
>>> @@ -32,7 +32,7 @@
>>> #include <asm/chpid.h>
>>> #include <asm/io.h>
>>> #include <asm/sysinfo.h>
>>> -#include <asm/compat.h>
>>> +#include <linux/compat.h>
>>> #include <asm/diag.h>
>>> #include <asm/cio.h>
>>> #include <asm/ccwdev.h>
>>
>> Can you move that into the other includes (where all the other <linux/*> includes are.
>
> Good catch, this is definitely a rule we have ;-)
FWIW, this was also broken for
arch/x86/include/asm/sys_ia32.h
^ permalink raw reply
* Re: [PATCH 2/3] rfi-flush: Make it possible to call setup_rfi_flush() again
From: Michal Suchánek @ 2018-03-06 12:55 UTC (permalink / raw)
To: Michael Ellerman; +Cc: Mauricio Faria de Oliveira, linuxppc-dev
In-Reply-To: <876069flhq.fsf@concordia.ellerman.id.au>
On Tue, 06 Mar 2018 23:15:45 +1100
Michael Ellerman <mpe@ellerman.id.au> wrote:
> Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com> writes:
>=20
> > Hi Michael, Michal,
> >
> > I got back from vacation. Checking this one. =20
>=20
> Yeah it got stuck in a rut.
>=20
> > On 02/20/2018 02:06 PM, Michal Such=C3=A1nek wrote: =20
> >>> I did it the way I did because otherwise we waste memory on every
> >>> system on earth just to support a use case that we don't actually
> >>> intend for anyone to ever use - ie. migrating from a patched
> >>> machine to an unpatched machine. =20
> >
> > If this thread eventually closes in 'ok, so that memory has to be
> > reserved/wasted anyway', that can be done only in pseries, right?
> >
> > It seems not so much memory for this particular platform/hardware. =20
>=20
> Yes and no. We certainly have pseries machines with 10s of TBs of
> memory, but pseries is also used when running on KVM/Openstack where
> some people are interested in running small VMs - so as usual it's not
> so simple :)
>=20
> But it's not *that* much memory so we should probably stop worrying
> about it and just always allocate the fallback on pseries.
>=20
> I *think* the patch below is all we need, as well as some tweaking of
> patch 2, are you able to test and repost?
>=20
> cheers
>=20
> diff --git a/arch/powerpc/platforms/pseries/setup.c
> b/arch/powerpc/platforms/pseries/setup.c index
> 1a527625acf7..9116824bd7c5 100644 ---
> a/arch/powerpc/platforms/pseries/setup.c +++
> b/arch/powerpc/platforms/pseries/setup.c @@ -468,26 +468,18 @@ static
> void pseries_setup_rfi_flush(void)=20
> /* Enable by default */
> enable =3D true;
> + types =3D L1D_FLUSH_FALLBACK;
> =20
> rc =3D plpar_get_cpu_characteristics(&result);
> if (rc =3D=3D H_SUCCESS) {
> - types =3D L1D_FLUSH_NONE;
> -
> if (result.character & H_CPU_CHAR_L1D_FLUSH_TRIG2)
> types |=3D L1D_FLUSH_MTTRIG;
> if (result.character & H_CPU_CHAR_L1D_FLUSH_ORI30)
> types |=3D L1D_FLUSH_ORI;
> =20
> - /* Use fallback if nothing set in hcall */
> - if (types =3D=3D L1D_FLUSH_NONE)
> - types =3D L1D_FLUSH_FALLBACK;
> -
> if ((!(result.behaviour & H_CPU_BEHAV_L1D_FLUSH_PR))
> || (!(result.behaviour & H_CPU_BEHAV_FAVOUR_SECURITY)))
> enable =3D false;
> - } else {
> - /* Default to fallback if case hcall is not
> available */
> - types =3D L1D_FLUSH_FALLBACK;
> }
> =20
> setup_rfi_flush(types, enable);
>=20
Enabling the fallback flush always looks a bit dodgy but
do_rfi_flush_fixups will overwrite the jump so long any other fixup is
enabled.
Reviewed-by: Michal Such=C3=A1nek <msuchanek@suse.de>
Thanks
Michal
^ permalink raw reply
* [PATCH 00/10] powerpc/mm/slice: improve slice speed and stack use
From: Nicholas Piggin @ 2018-03-06 13:24 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin, Aneesh Kumar K . V, Christophe Leroy
Since this was last posted, it's been ported on top of Christophe's
8xx slice implementation that is merged in powerpc next, also taken
into account some feedback and bugs from Aneesh and Christophe --
thanks.
A few significant changes, first is refactoring slice_set_user_psize,
which makes it more obvious how the slice state is initialized, which
makes it easier to reason about using dynamic high slice size limits I
think.
Second is a significant change to how the slice masks are kept. No
longer are they bolted on the side and hit with a big recalculation
call that redoes everything whenever something changes. Now they are
just maintained as part of slice conversion.
This now passes vm selftests including the 128TB boundary case tests.
I also added a process microbenchmark and redid benchmarks and stack
measurements.
Overall on POWER8, this series increases vfork+exec+exit
microbenchmark rate by 15.6%, and mmap+munmap rate by 81%. Slice
code/data size is reduced by 1kB, and max stack overhead through
slice_get_unmapped_area call goes rom 992 to 448 bytes. The cost is
288 bytes added to the mm_context_t per mm for the slice masks on
Book3S.
Thanks,
Nick
Nicholas Piggin (10):
selftests/powerpc: add process creation benchmark
powerpc/mm/slice: Simplify and optimise slice context initialisation
powerpc/mm/slice: tidy lpsizes and hpsizes update loops
powerpc/mm/slice: pass pointers to struct slice_mask where possible
powerpc/mm/slice: implement a slice mask cache
powerpc/mm/slice: implement slice_check_range_fits
powerpc/mm/slice: Switch to 3-operand slice bitops helpers
powerpc/mm/slice: Use const pointers to cached slice masks where
possible
powerpc/mm/slice: use the dynamic high slice size to limit bitmap
operations
powerpc/mm/slice: remove radix calls to the slice code
arch/powerpc/include/asm/book3s/64/mmu.h | 18 +
arch/powerpc/include/asm/hugetlb.h | 9 +-
arch/powerpc/include/asm/mmu-8xx.h | 14 +
arch/powerpc/include/asm/slice.h | 8 +-
arch/powerpc/mm/hugetlbpage.c | 5 +-
arch/powerpc/mm/mmu_context_book3s64.c | 9 +-
arch/powerpc/mm/mmu_context_nohash.c | 5 +-
arch/powerpc/mm/slice.c | 458 +++++++++++----------
.../selftests/powerpc/benchmarks/.gitignore | 2 +
.../testing/selftests/powerpc/benchmarks/Makefile | 8 +-
.../selftests/powerpc/benchmarks/exec_target.c | 5 +
tools/testing/selftests/powerpc/benchmarks/fork.c | 339 +++++++++++++++
12 files changed, 632 insertions(+), 248 deletions(-)
create mode 100644 tools/testing/selftests/powerpc/benchmarks/exec_target.c
create mode 100644 tools/testing/selftests/powerpc/benchmarks/fork.c
--
2.16.1
^ permalink raw reply
* [PATCH 01/10] selftests/powerpc: add process creation benchmark
From: Nicholas Piggin @ 2018-03-06 13:24 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin, Aneesh Kumar K . V, Christophe Leroy
In-Reply-To: <20180306132507.10649-1-npiggin@gmail.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
.../selftests/powerpc/benchmarks/.gitignore | 2 +
.../testing/selftests/powerpc/benchmarks/Makefile | 8 +-
.../selftests/powerpc/benchmarks/exec_target.c | 5 +
tools/testing/selftests/powerpc/benchmarks/fork.c | 339 +++++++++++++++++++++
4 files changed, 353 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/powerpc/benchmarks/exec_target.c
create mode 100644 tools/testing/selftests/powerpc/benchmarks/fork.c
diff --git a/tools/testing/selftests/powerpc/benchmarks/.gitignore b/tools/testing/selftests/powerpc/benchmarks/.gitignore
index 04dc1e6ef2ce..9161679b1e1a 100644
--- a/tools/testing/selftests/powerpc/benchmarks/.gitignore
+++ b/tools/testing/selftests/powerpc/benchmarks/.gitignore
@@ -1,5 +1,7 @@
gettimeofday
context_switch
+fork
+exec_target
mmap_bench
futex_bench
null_syscall
diff --git a/tools/testing/selftests/powerpc/benchmarks/Makefile b/tools/testing/selftests/powerpc/benchmarks/Makefile
index a35058e3766c..61189a0b8285 100644
--- a/tools/testing/selftests/powerpc/benchmarks/Makefile
+++ b/tools/testing/selftests/powerpc/benchmarks/Makefile
@@ -1,5 +1,9 @@
# SPDX-License-Identifier: GPL-2.0
-TEST_GEN_PROGS := gettimeofday context_switch mmap_bench futex_bench null_syscall
+TEST_GEN_PROGS := gettimeofday context_switch fork mmap_bench futex_bench null_syscall
+TEST_GEN_FILES := exec_target
+
+$(OUTPUT)/exec_target: exec_target.c
+ $(CC) -O2 -static -nostartfiles -oexec_target exec_target.c
CFLAGS += -O2
@@ -10,3 +14,5 @@ $(TEST_GEN_PROGS): ../harness.c
$(OUTPUT)/context_switch: ../utils.c
$(OUTPUT)/context_switch: CFLAGS += -maltivec -mvsx -mabi=altivec
$(OUTPUT)/context_switch: LDLIBS += -lpthread
+
+$(OUTPUT)/fork: LDLIBS += -lpthread
diff --git a/tools/testing/selftests/powerpc/benchmarks/exec_target.c b/tools/testing/selftests/powerpc/benchmarks/exec_target.c
new file mode 100644
index 000000000000..5e2a6e917c1a
--- /dev/null
+++ b/tools/testing/selftests/powerpc/benchmarks/exec_target.c
@@ -0,0 +1,5 @@
+void _exit(int);
+void _start(void)
+{
+ _exit(0);
+}
diff --git a/tools/testing/selftests/powerpc/benchmarks/fork.c b/tools/testing/selftests/powerpc/benchmarks/fork.c
new file mode 100644
index 000000000000..c68a7c360fd2
--- /dev/null
+++ b/tools/testing/selftests/powerpc/benchmarks/fork.c
@@ -0,0 +1,339 @@
+/*
+ * Context switch microbenchmark.
+ *
+ * Copyright (C) 2018 Anton Blanchard <anton@au.ibm.com>, IBM
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#define _GNU_SOURCE
+#include <errno.h>
+#include <sched.h>
+#include <string.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <getopt.h>
+#include <signal.h>
+#include <assert.h>
+#include <pthread.h>
+#include <limits.h>
+#include <sys/time.h>
+#include <sys/syscall.h>
+#include <sys/types.h>
+#include <sys/shm.h>
+#include <sys/wait.h>
+#include <linux/futex.h>
+
+static unsigned int timeout = 30;
+
+static void set_cpu(int cpu)
+{
+ cpu_set_t cpuset;
+
+ if (cpu == -1)
+ return;
+
+ CPU_ZERO(&cpuset);
+ CPU_SET(cpu, &cpuset);
+
+ if (sched_setaffinity(0, sizeof(cpuset), &cpuset)) {
+ perror("sched_setaffinity");
+ exit(1);
+ }
+}
+
+static void start_process_on(void *(*fn)(void *), void *arg, int cpu)
+{
+ int pid;
+
+ pid = fork();
+ if (pid == -1) {
+ perror("fork");
+ exit(1);
+ }
+
+ if (pid)
+ return;
+
+ set_cpu(cpu);
+
+ fn(arg);
+
+ exit(0);
+}
+
+static int cpu;
+static int do_fork = 0;
+static int do_vfork = 0;
+static int do_exec = 0;
+static char *exec_file;
+static int exec_target = 0;
+static unsigned long iterations;
+static unsigned long iterations_prev;
+
+static void run_exec(void)
+{
+#if 0
+ char *const argv[] = { exec_file, "--exec-target", NULL };
+
+ if (execve(exec_file, argv, NULL) == -1) {
+ perror("execve");
+ exit(1);
+ }
+#else
+ char *const argv[] = { "./exec_target", NULL };
+
+ if (execve("./exec_target", argv, NULL) == -1) {
+ perror("execve");
+ exit(1);
+ }
+#endif
+}
+
+static void bench_fork(void)
+{
+ while (1) {
+ pid_t pid = fork();
+ if (pid == -1) {
+ perror("fork");
+ exit(1);
+ }
+ if (pid == 0) {
+ if (do_exec)
+ run_exec();
+ _exit(0);
+ }
+ pid = waitpid(pid, NULL, 0);
+ if (pid == -1) {
+ perror("waitpid");
+ exit(1);
+ }
+ iterations++;
+ }
+}
+
+static void bench_vfork(void)
+{
+ while (1) {
+ pid_t pid = vfork();
+ if (pid == -1) {
+ perror("fork");
+ exit(1);
+ }
+ if (pid == 0) {
+ if (do_exec)
+ run_exec();
+ _exit(0);
+ }
+ pid = waitpid(pid, NULL, 0);
+ if (pid == -1) {
+ perror("waitpid");
+ exit(1);
+ }
+ iterations++;
+ }
+}
+
+
+static void *null_fn(void *arg)
+{
+ pthread_exit(NULL);
+}
+
+static void bench_thread(void)
+{
+ pthread_t tid;
+ cpu_set_t cpuset;
+ pthread_attr_t attr;
+ int rc;
+
+ rc = pthread_attr_init(&attr);
+ if (rc) {
+ errno = rc;
+ perror("pthread_attr_init");
+ exit(1);
+ }
+
+ if (cpu != -1) {
+ CPU_ZERO(&cpuset);
+ CPU_SET(cpu, &cpuset);
+
+ rc = pthread_attr_setaffinity_np(&attr, sizeof(cpu_set_t), &cpuset);
+ if (rc) {
+ errno = rc;
+ perror("pthread_attr_setaffinity_np");
+ exit(1);
+ }
+ }
+
+ while (1) {
+ rc = pthread_create(&tid, &attr, null_fn, NULL);
+ if (rc) {
+ errno = rc;
+ perror("pthread_create");
+ exit(1);
+ }
+ rc = pthread_join(tid, NULL);
+ if (rc) {
+ errno = rc;
+ perror("pthread_join");
+ exit(1);
+ }
+ iterations++;
+ }
+}
+
+
+static void sigalrm_handler(int junk)
+{
+ unsigned long i = iterations;
+
+ printf("%ld\n", i - iterations_prev);
+ iterations_prev = i;
+
+ if (--timeout == 0)
+ kill(0, SIGUSR1);
+
+ alarm(1);
+}
+
+static void sigusr1_handler(int junk)
+{
+ exit(0);
+}
+
+static void *bench_proc(void *arg)
+{
+ signal(SIGALRM, sigalrm_handler);
+ alarm(1);
+
+ if (do_fork)
+ bench_fork();
+ else if (do_vfork)
+ bench_vfork();
+ else
+ bench_thread();
+
+ return NULL;
+}
+
+static struct option options[] = {
+ { "fork", no_argument, &do_fork, 1 },
+ { "vfork", no_argument, &do_vfork, 1 },
+ { "exec", no_argument, &do_exec, 1 },
+ { "timeout", required_argument, 0, 's' },
+ { "exec-target", no_argument, &exec_target, 1 },
+ { 0, },
+};
+
+static void usage(void)
+{
+ fprintf(stderr, "Usage: fork <options> CPU\n\n");
+ fprintf(stderr, "\t\t--fork\tUse fork() (default threads)\n");
+ fprintf(stderr, "\t\t--vfork\tUse vfork() (default threads)\n");
+ fprintf(stderr, "\t\t--exec\tAlso exec() (default no exec)\n");
+ fprintf(stderr, "\t\t--timeout=X\tDuration in seconds to run (default 30)\n");
+ fprintf(stderr, "\t\t--exec-target\tInternal option for exec workload\n");
+}
+
+int main(int argc, char *argv[])
+{
+ signed char c;
+
+ while (1) {
+ int option_index = 0;
+
+ c = getopt_long(argc, argv, "", options, &option_index);
+
+ if (c == -1)
+ break;
+
+ switch (c) {
+ case 0:
+ if (options[option_index].flag != 0)
+ break;
+
+ usage();
+ exit(1);
+ break;
+
+ case 's':
+ timeout = atoi(optarg);
+ break;
+
+ default:
+ usage();
+ exit(1);
+ }
+ }
+
+ if (do_fork && do_vfork) {
+ usage();
+ exit(1);
+ }
+ if (do_exec && !do_fork && !do_vfork) {
+ usage();
+ exit(1);
+ }
+
+ if (do_exec) {
+ char *dirname = strdup(argv[0]);
+ int i;
+ i = strlen(dirname) - 1;
+ while (i) {
+ if (dirname[i] == '/') {
+ dirname[i] = '\0';
+ if (chdir(dirname) == -1) {
+ perror("chdir");
+ exit(1);
+ }
+ break;
+ }
+ i--;
+ }
+ }
+
+ if (exec_target) {
+ exit(0);
+ }
+
+ if (((argc - optind) != 1)) {
+ cpu = -1;
+ } else {
+ cpu = atoi(argv[optind++]);
+ }
+
+ if (do_exec)
+ exec_file = argv[0];
+
+ set_cpu(cpu);
+
+ printf("Using ");
+ if (do_fork)
+ printf("fork");
+ else if (do_vfork)
+ printf("vfork");
+ else
+ printf("clone");
+
+ if (do_exec)
+ printf(" + exec");
+
+ printf(" on cpu %d\n", cpu);
+
+ /* Create a new process group so we can signal everyone for exit */
+ setpgid(getpid(), getpid());
+
+ signal(SIGUSR1, sigusr1_handler);
+
+ start_process_on(bench_proc, NULL, cpu);
+
+ while (1)
+ sleep(3600);
+
+ return 0;
+}
--
2.16.1
^ permalink raw reply related
* [PATCH 02/10] powerpc/mm/slice: Simplify and optimise slice context initialisation
From: Nicholas Piggin @ 2018-03-06 13:24 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin, Aneesh Kumar K . V, Christophe Leroy
In-Reply-To: <20180306132507.10649-1-npiggin@gmail.com>
The slice state of an mm gets zeroed then initialised upon exec.
This is the only caller of slice_set_user_psize now, so that can be
removed and instead implement a faster and simplified approach that
requires no locking or checking existing state.
This speeds up vfork+exec+exit performance on POWER8 by 3%.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/slice.h | 8 ++--
arch/powerpc/mm/mmu_context_book3s64.c | 9 +----
arch/powerpc/mm/mmu_context_nohash.c | 5 +--
arch/powerpc/mm/slice.c | 69 +++++++---------------------------
4 files changed, 19 insertions(+), 72 deletions(-)
diff --git a/arch/powerpc/include/asm/slice.h b/arch/powerpc/include/asm/slice.h
index 172711fadb1c..e40406cf5628 100644
--- a/arch/powerpc/include/asm/slice.h
+++ b/arch/powerpc/include/asm/slice.h
@@ -28,15 +28,13 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
unsigned int get_slice_psize(struct mm_struct *mm, unsigned long addr);
-void slice_set_user_psize(struct mm_struct *mm, unsigned int psize);
void slice_set_range_psize(struct mm_struct *mm, unsigned long start,
unsigned long len, unsigned int psize);
-#endif /* __ASSEMBLY__ */
-#else /* CONFIG_PPC_MM_SLICES */
+void slice_init_new_context_exec(struct mm_struct *mm);
+
+#endif /* __ASSEMBLY__ */
-#define slice_set_range_psize(mm, start, len, psize) \
- slice_set_user_psize((mm), (psize))
#endif /* CONFIG_PPC_MM_SLICES */
#endif /* _ASM_POWERPC_SLICE_H */
diff --git a/arch/powerpc/mm/mmu_context_book3s64.c b/arch/powerpc/mm/mmu_context_book3s64.c
index 929d9ef7083f..80acad52b006 100644
--- a/arch/powerpc/mm/mmu_context_book3s64.c
+++ b/arch/powerpc/mm/mmu_context_book3s64.c
@@ -93,13 +93,6 @@ static int hash__init_new_context(struct mm_struct *mm)
if (index < 0)
return index;
- /*
- * In the case of exec, use the default limit,
- * otherwise inherit it from the mm we are duplicating.
- */
- if (!mm->context.slb_addr_limit)
- mm->context.slb_addr_limit = DEFAULT_MAP_WINDOW_USER64;
-
/*
* The old code would re-promote on fork, we don't do that when using
* slices as it could cause problem promoting slices that have been
@@ -115,7 +108,7 @@ static int hash__init_new_context(struct mm_struct *mm)
* check against 0 is OK.
*/
if (mm->context.id == 0)
- slice_set_user_psize(mm, mmu_virtual_psize);
+ slice_init_new_context_exec(mm);
subpage_prot_init_new_context(mm);
diff --git a/arch/powerpc/mm/mmu_context_nohash.c b/arch/powerpc/mm/mmu_context_nohash.c
index d98f7e5c141b..be8f5c9d4d08 100644
--- a/arch/powerpc/mm/mmu_context_nohash.c
+++ b/arch/powerpc/mm/mmu_context_nohash.c
@@ -332,9 +332,6 @@ int init_new_context(struct task_struct *t, struct mm_struct *mm)
pr_hard("initing context for mm @%p\n", mm);
#ifdef CONFIG_PPC_MM_SLICES
- if (!mm->context.slb_addr_limit)
- mm->context.slb_addr_limit = DEFAULT_MAP_WINDOW;
-
/*
* We have MMU_NO_CONTEXT set to be ~0. Hence check
* explicitly against context.id == 0. This ensures that we properly
@@ -343,7 +340,7 @@ int init_new_context(struct task_struct *t, struct mm_struct *mm)
* will have id != 0).
*/
if (mm->context.id == 0)
- slice_set_user_psize(mm, mmu_virtual_psize);
+ slice_init_new_context_exec(mm);
#endif
mm->context.id = MMU_NO_CONTEXT;
mm->context.active = 0;
diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
index 5e9e1e57d580..af4351b15d01 100644
--- a/arch/powerpc/mm/slice.c
+++ b/arch/powerpc/mm/slice.c
@@ -671,70 +671,29 @@ unsigned int get_slice_psize(struct mm_struct *mm, unsigned long addr)
}
EXPORT_SYMBOL_GPL(get_slice_psize);
-/*
- * This is called by hash_page when it needs to do a lazy conversion of
- * an address space from real 64K pages to combo 4K pages (typically
- * when hitting a non cacheable mapping on a processor or hypervisor
- * that won't allow them for 64K pages).
- *
- * This is also called in init_new_context() to change back the user
- * psize from whatever the parent context had it set to
- * N.B. This may be called before mm->context.id has been set.
- *
- * This function will only change the content of the {low,high)_slice_psize
- * masks, it will not flush SLBs as this shall be handled lazily by the
- * caller.
- */
-void slice_set_user_psize(struct mm_struct *mm, unsigned int psize)
+void slice_init_new_context_exec(struct mm_struct *mm)
{
- int index, mask_index;
unsigned char *hpsizes, *lpsizes;
- unsigned long flags;
- unsigned int old_psize;
- int i;
-
- slice_dbg("slice_set_user_psize(mm=%p, psize=%d)\n", mm, psize);
-
- VM_BUG_ON(radix_enabled());
- spin_lock_irqsave(&slice_convert_lock, flags);
+ unsigned int psize = mmu_virtual_psize;
- old_psize = mm->context.user_psize;
- slice_dbg(" old_psize=%d\n", old_psize);
- if (old_psize == psize)
- goto bail;
+ slice_dbg("slice_init_new_context_exec(mm=%p)\n", mm);
+ /*
+ * In the case of exec, use the default limit. In the
+ * case of fork it is just inherited from the mm being
+ * duplicated.
+ */
+ mm->context.slb_addr_limit = DEFAULT_MAP_WINDOW_USER64;
mm->context.user_psize = psize;
- wmb();
+ /*
+ * Set all slice psizes to the default.
+ */
lpsizes = mm->context.low_slices_psize;
- for (i = 0; i < SLICE_NUM_LOW; i++) {
- mask_index = i & 0x1;
- index = i >> 1;
- if (((lpsizes[index] >> (mask_index * 4)) & 0xf) == old_psize)
- lpsizes[index] = (lpsizes[index] &
- ~(0xf << (mask_index * 4))) |
- (((unsigned long)psize) << (mask_index * 4));
- }
+ memset(lpsizes, (psize << 4) | psize, SLICE_NUM_LOW >> 1);
hpsizes = mm->context.high_slices_psize;
- for (i = 0; i < SLICE_NUM_HIGH; i++) {
- mask_index = i & 0x1;
- index = i >> 1;
- if (((hpsizes[index] >> (mask_index * 4)) & 0xf) == old_psize)
- hpsizes[index] = (hpsizes[index] &
- ~(0xf << (mask_index * 4))) |
- (((unsigned long)psize) << (mask_index * 4));
- }
-
-
-
-
- slice_dbg(" lsps=%lx, hsps=%lx\n",
- (unsigned long)mm->context.low_slices_psize,
- (unsigned long)mm->context.high_slices_psize);
-
- bail:
- spin_unlock_irqrestore(&slice_convert_lock, flags);
+ memset(hpsizes, (psize << 4) | psize, SLICE_NUM_HIGH >> 1);
}
void slice_set_range_psize(struct mm_struct *mm, unsigned long start,
--
2.16.1
^ permalink raw reply related
* [PATCH 03/10] powerpc/mm/slice: tidy lpsizes and hpsizes update loops
From: Nicholas Piggin @ 2018-03-06 13:25 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin, Aneesh Kumar K . V, Christophe Leroy
In-Reply-To: <20180306132507.10649-1-npiggin@gmail.com>
Make these loops look the same, and change their form so the
important part is not wrapped over so many lines.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/mm/slice.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
index af4351b15d01..9625ceb35685 100644
--- a/arch/powerpc/mm/slice.c
+++ b/arch/powerpc/mm/slice.c
@@ -232,22 +232,24 @@ static void slice_convert(struct mm_struct *mm, struct slice_mask mask, int psiz
spin_lock_irqsave(&slice_convert_lock, flags);
lpsizes = mm->context.low_slices_psize;
- for (i = 0; i < SLICE_NUM_LOW; i++)
- if (mask.low_slices & (1u << i)) {
- mask_index = i & 0x1;
- index = i >> 1;
- lpsizes[index] = (lpsizes[index] &
- ~(0xf << (mask_index * 4))) |
+ for (i = 0; i < SLICE_NUM_LOW; i++) {
+ if (!(mask.low_slices & (1u << i)))
+ continue;
+
+ mask_index = i & 0x1;
+ index = i >> 1;
+ lpsizes[index] = (lpsizes[index] & ~(0xf << (mask_index * 4))) |
(((unsigned long)psize) << (mask_index * 4));
- }
+ }
hpsizes = mm->context.high_slices_psize;
for (i = 0; i < GET_HIGH_SLICE_INDEX(mm->context.slb_addr_limit); i++) {
+ if (!test_bit(i, mask.high_slices))
+ continue;
+
mask_index = i & 0x1;
index = i >> 1;
- if (test_bit(i, mask.high_slices))
- hpsizes[index] = (hpsizes[index] &
- ~(0xf << (mask_index * 4))) |
+ hpsizes[index] = (hpsizes[index] & ~(0xf << (mask_index * 4))) |
(((unsigned long)psize) << (mask_index * 4));
}
--
2.16.1
^ permalink raw reply related
* [PATCH 04/10] powerpc/mm/slice: pass pointers to struct slice_mask where possible
From: Nicholas Piggin @ 2018-03-06 13:25 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin, Aneesh Kumar K . V, Christophe Leroy
In-Reply-To: <20180306132507.10649-1-npiggin@gmail.com>
Pass around const pointers to struct slice_mask where possible, rather
than copies of slice_mask, to reduce stack and call overhead.
checkstack.pl gives, before:
0x00000d1c slice_get_unmapped_area [slice.o]: 592
0x00001864 is_hugepage_only_range [slice.o]: 448
0x00000754 slice_find_area_topdown [slice.o]: 400
0x00000484 slice_find_area_bottomup.isra.1 [slice.o]: 272
0x000017b4 slice_set_range_psize [slice.o]: 224
0x00000a4c slice_find_area [slice.o]: 128
0x00000160 slice_check_fit [slice.o]: 112
after:
0x00000ad0 slice_get_unmapped_area [slice.o]: 448
0x00001464 is_hugepage_only_range [slice.o]: 288
0x000006c0 slice_find_area [slice.o]: 144
0x0000016c slice_check_fit [slice.o]: 128
0x00000528 slice_find_area_bottomup.isra.2 [slice.o]: 128
0x000013e4 slice_set_range_psize [slice.o]: 128
This increases vfork+exec+exit performance by 1.5%.
Reduces time to mmap+munmap a 64kB page by 17%.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/mm/slice.c | 87 ++++++++++++++++++++++++++-----------------------
1 file changed, 47 insertions(+), 40 deletions(-)
diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
index 9625ceb35685..233c42d593dc 100644
--- a/arch/powerpc/mm/slice.c
+++ b/arch/powerpc/mm/slice.c
@@ -50,19 +50,21 @@ struct slice_mask {
#ifdef DEBUG
int _slice_debug = 1;
-static void slice_print_mask(const char *label, struct slice_mask mask)
+static void slice_print_mask(const char *label, const struct slice_mask *mask)
{
if (!_slice_debug)
return;
- pr_devel("%s low_slice: %*pbl\n", label, (int)SLICE_NUM_LOW, &mask.low_slices);
- pr_devel("%s high_slice: %*pbl\n", label, (int)SLICE_NUM_HIGH, mask.high_slices);
+ pr_devel("%s low_slice: %*pbl\n", label,
+ (int)SLICE_NUM_LOW, &mask->low_slices);
+ pr_devel("%s high_slice: %*pbl\n", label,
+ (int)SLICE_NUM_HIGH, mask->high_slices);
}
#define slice_dbg(fmt...) do { if (_slice_debug) pr_devel(fmt); } while (0)
#else
-static void slice_print_mask(const char *label, struct slice_mask mask) {}
+static void slice_print_mask(const char *label, const struct slice_mask *mask) {}
#define slice_dbg(fmt...)
#endif
@@ -147,7 +149,8 @@ static void slice_mask_for_free(struct mm_struct *mm, struct slice_mask *ret,
__set_bit(i, ret->high_slices);
}
-static void slice_mask_for_size(struct mm_struct *mm, int psize, struct slice_mask *ret,
+static void slice_mask_for_size(struct mm_struct *mm, int psize,
+ struct slice_mask *ret,
unsigned long high_limit)
{
unsigned char *hpsizes, *lpsizes;
@@ -179,7 +182,8 @@ static void slice_mask_for_size(struct mm_struct *mm, int psize, struct slice_ma
}
static int slice_check_fit(struct mm_struct *mm,
- struct slice_mask mask, struct slice_mask available)
+ const struct slice_mask *mask,
+ const struct slice_mask *available)
{
DECLARE_BITMAP(result, SLICE_NUM_HIGH);
/*
@@ -189,14 +193,14 @@ static int slice_check_fit(struct mm_struct *mm,
unsigned long slice_count = GET_HIGH_SLICE_INDEX(mm->context.slb_addr_limit);
if (!SLICE_NUM_HIGH)
- return (mask.low_slices & available.low_slices) ==
- mask.low_slices;
+ return (mask->low_slices & available->low_slices) ==
+ mask->low_slices;
- bitmap_and(result, mask.high_slices,
- available.high_slices, slice_count);
+ bitmap_and(result, mask->high_slices,
+ available->high_slices, slice_count);
- return (mask.low_slices & available.low_slices) == mask.low_slices &&
- bitmap_equal(result, mask.high_slices, slice_count);
+ return (mask->low_slices & available->low_slices) == mask->low_slices &&
+ bitmap_equal(result, mask->high_slices, slice_count);
}
static void slice_flush_segments(void *parm)
@@ -216,7 +220,8 @@ static void slice_flush_segments(void *parm)
#endif
}
-static void slice_convert(struct mm_struct *mm, struct slice_mask mask, int psize)
+static void slice_convert(struct mm_struct *mm,
+ const struct slice_mask *mask, int psize)
{
int index, mask_index;
/* Write the new slice psize bits */
@@ -233,7 +238,7 @@ static void slice_convert(struct mm_struct *mm, struct slice_mask mask, int psiz
lpsizes = mm->context.low_slices_psize;
for (i = 0; i < SLICE_NUM_LOW; i++) {
- if (!(mask.low_slices & (1u << i)))
+ if (!(mask->low_slices & (1u << i)))
continue;
mask_index = i & 0x1;
@@ -244,7 +249,7 @@ static void slice_convert(struct mm_struct *mm, struct slice_mask mask, int psiz
hpsizes = mm->context.high_slices_psize;
for (i = 0; i < GET_HIGH_SLICE_INDEX(mm->context.slb_addr_limit); i++) {
- if (!test_bit(i, mask.high_slices))
+ if (!test_bit(i, mask->high_slices))
continue;
mask_index = i & 0x1;
@@ -270,26 +275,25 @@ static void slice_convert(struct mm_struct *mm, struct slice_mask mask, int psiz
* 'available' slice_mark.
*/
static bool slice_scan_available(unsigned long addr,
- struct slice_mask available,
- int end,
- unsigned long *boundary_addr)
+ const struct slice_mask *available,
+ int end, unsigned long *boundary_addr)
{
unsigned long slice;
if (addr < SLICE_LOW_TOP) {
slice = GET_LOW_SLICE_INDEX(addr);
*boundary_addr = (slice + end) << SLICE_LOW_SHIFT;
- return !!(available.low_slices & (1u << slice));
+ return !!(available->low_slices & (1u << slice));
} else {
slice = GET_HIGH_SLICE_INDEX(addr);
*boundary_addr = (slice + end) ?
((slice + end) << SLICE_HIGH_SHIFT) : SLICE_LOW_TOP;
- return !!test_bit(slice, available.high_slices);
+ return !!test_bit(slice, available->high_slices);
}
}
static unsigned long slice_find_area_bottomup(struct mm_struct *mm,
unsigned long len,
- struct slice_mask available,
+ const struct slice_mask *available,
int psize, unsigned long high_limit)
{
int pshift = max_t(int, mmu_psize_defs[psize].shift, PAGE_SHIFT);
@@ -335,7 +339,7 @@ static unsigned long slice_find_area_bottomup(struct mm_struct *mm,
static unsigned long slice_find_area_topdown(struct mm_struct *mm,
unsigned long len,
- struct slice_mask available,
+ const struct slice_mask *available,
int psize, unsigned long high_limit)
{
int pshift = max_t(int, mmu_psize_defs[psize].shift, PAGE_SHIFT);
@@ -393,7 +397,7 @@ static unsigned long slice_find_area_topdown(struct mm_struct *mm,
static unsigned long slice_find_area(struct mm_struct *mm, unsigned long len,
- struct slice_mask mask, int psize,
+ const struct slice_mask *mask, int psize,
int topdown, unsigned long high_limit)
{
if (topdown)
@@ -402,7 +406,8 @@ static unsigned long slice_find_area(struct mm_struct *mm, unsigned long len,
return slice_find_area_bottomup(mm, len, mask, psize, high_limit);
}
-static inline void slice_or_mask(struct slice_mask *dst, struct slice_mask *src)
+static inline void slice_or_mask(struct slice_mask *dst,
+ const struct slice_mask *src)
{
dst->low_slices |= src->low_slices;
if (!SLICE_NUM_HIGH)
@@ -411,7 +416,8 @@ static inline void slice_or_mask(struct slice_mask *dst, struct slice_mask *src)
SLICE_NUM_HIGH);
}
-static inline void slice_andnot_mask(struct slice_mask *dst, struct slice_mask *src)
+static inline void slice_andnot_mask(struct slice_mask *dst,
+ const struct slice_mask *src)
{
dst->low_slices &= ~src->low_slices;
@@ -501,7 +507,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
* already
*/
slice_mask_for_size(mm, psize, &good_mask, high_limit);
- slice_print_mask(" good_mask", good_mask);
+ slice_print_mask(" good_mask", &good_mask);
/*
* Here "good" means slices that are already the right page size,
@@ -535,12 +541,12 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
if (addr != 0 || fixed) {
/* Build a mask for the requested range */
slice_range_to_mask(addr, len, &mask);
- slice_print_mask(" mask", mask);
+ slice_print_mask(" mask", &mask);
/* Check if we fit in the good mask. If we do, we just return,
* nothing else to do
*/
- if (slice_check_fit(mm, mask, good_mask)) {
+ if (slice_check_fit(mm, &mask, &good_mask)) {
slice_dbg(" fits good !\n");
return addr;
}
@@ -548,7 +554,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
/* Now let's see if we can find something in the existing
* slices for that size
*/
- newaddr = slice_find_area(mm, len, good_mask,
+ newaddr = slice_find_area(mm, len, &good_mask,
psize, topdown, high_limit);
if (newaddr != -ENOMEM) {
/* Found within the good mask, we don't have to setup,
@@ -564,9 +570,10 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
*/
slice_mask_for_free(mm, &potential_mask, high_limit);
slice_or_mask(&potential_mask, &good_mask);
- slice_print_mask(" potential", potential_mask);
+ slice_print_mask(" potential", &potential_mask);
- if ((addr != 0 || fixed) && slice_check_fit(mm, mask, potential_mask)) {
+ if ((addr != 0 || fixed) &&
+ slice_check_fit(mm, &mask, &potential_mask)) {
slice_dbg(" fits potential !\n");
goto convert;
}
@@ -581,7 +588,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
* anywhere in the good area.
*/
if (addr) {
- addr = slice_find_area(mm, len, good_mask,
+ addr = slice_find_area(mm, len, &good_mask,
psize, topdown, high_limit);
if (addr != -ENOMEM) {
slice_dbg(" found area at 0x%lx\n", addr);
@@ -592,14 +599,14 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
/* Now let's see if we can find something in the existing slices
* for that size plus free slices
*/
- addr = slice_find_area(mm, len, potential_mask,
+ addr = slice_find_area(mm, len, &potential_mask,
psize, topdown, high_limit);
#ifdef CONFIG_PPC_64K_PAGES
if (addr == -ENOMEM && psize == MMU_PAGE_64K) {
/* retry the search with 4k-page slices included */
slice_or_mask(&potential_mask, &compat_mask);
- addr = slice_find_area(mm, len, potential_mask,
+ addr = slice_find_area(mm, len, &potential_mask,
psize, topdown, high_limit);
}
#endif
@@ -609,7 +616,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
slice_range_to_mask(addr, len, &mask);
slice_dbg(" found potential area at 0x%lx\n", addr);
- slice_print_mask(" mask", mask);
+ slice_print_mask(" mask", &mask);
convert:
slice_andnot_mask(&mask, &good_mask);
@@ -617,7 +624,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
if (mask.low_slices ||
(SLICE_NUM_HIGH &&
!bitmap_empty(mask.high_slices, SLICE_NUM_HIGH))) {
- slice_convert(mm, mask, psize);
+ slice_convert(mm, &mask, psize);
if (psize > MMU_PAGE_BASE)
on_each_cpu(slice_flush_segments, mm, 1);
}
@@ -706,7 +713,7 @@ void slice_set_range_psize(struct mm_struct *mm, unsigned long start,
VM_BUG_ON(radix_enabled());
slice_range_to_mask(start, len, &mask);
- slice_convert(mm, mask, psize);
+ slice_convert(mm, &mask, psize);
}
#ifdef CONFIG_HUGETLB_PAGE
@@ -753,9 +760,9 @@ int is_hugepage_only_range(struct mm_struct *mm, unsigned long addr,
#if 0 /* too verbose */
slice_dbg("is_hugepage_only_range(mm=%p, addr=%lx, len=%lx)\n",
mm, addr, len);
- slice_print_mask(" mask", mask);
- slice_print_mask(" available", available);
+ slice_print_mask(" mask", &mask);
+ slice_print_mask(" available", &available);
#endif
- return !slice_check_fit(mm, mask, available);
+ return !slice_check_fit(mm, &mask, &available);
}
#endif
--
2.16.1
^ permalink raw reply related
* [PATCH 05/10] powerpc/mm/slice: implement a slice mask cache
From: Nicholas Piggin @ 2018-03-06 13:25 UTC (permalink / raw)
To: linuxppc-dev
Cc: Nicholas Piggin, Aneesh Kumar K . V, Christophe Leroy,
Benjamin Herrenschmidt, Anton Blanchard
In-Reply-To: <20180306132507.10649-1-npiggin@gmail.com>
Calculating the slice mask can become a signifcant overhead for
get_unmapped_area. This patch adds a struct slice_mask for
each page size in the mm_context, and keeps these in synch with
the slices psize arrays and slb_addr_limit.
On Book3S/64 this adds 288 bytes to the mm_context_t for the
slice mask caches.
On POWER8, this increases vfork+exec+exit performance by 9.9%
and reduces time to mmap+munmap a 64kB page by 28%.
Reduces time to mmap+munmap by about 10% on 8xx.
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Anton Blanchard <anton@samba.org>
---
arch/powerpc/include/asm/book3s/64/mmu.h | 18 +++++
arch/powerpc/include/asm/mmu-8xx.h | 14 ++++
arch/powerpc/mm/slice.c | 118 ++++++++++++++++++++-----------
3 files changed, 107 insertions(+), 43 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/64/mmu.h b/arch/powerpc/include/asm/book3s/64/mmu.h
index bef6e39ed63a..777778579305 100644
--- a/arch/powerpc/include/asm/book3s/64/mmu.h
+++ b/arch/powerpc/include/asm/book3s/64/mmu.h
@@ -80,6 +80,16 @@ struct spinlock;
/* Maximum possible number of NPUs in a system. */
#define NV_MAX_NPUS 8
+/*
+ * One bit per slice. We have lower slices which cover 256MB segments
+ * upto 4G range. That gets us 16 low slices. For the rest we track slices
+ * in 1TB size.
+ */
+struct slice_mask {
+ u64 low_slices;
+ DECLARE_BITMAP(high_slices, SLICE_NUM_HIGH);
+};
+
typedef struct {
mm_context_id_t id;
u16 user_psize; /* page size index */
@@ -95,6 +105,14 @@ typedef struct {
unsigned char low_slices_psize[BITS_PER_LONG / BITS_PER_BYTE];
unsigned char high_slices_psize[SLICE_ARRAY_SIZE];
unsigned long slb_addr_limit;
+# ifdef CONFIG_PPC_64K_PAGES
+ struct slice_mask mask_64k;
+# endif
+ struct slice_mask mask_4k;
+# ifdef CONFIG_HUGETLB_PAGE
+ struct slice_mask mask_16m;
+ struct slice_mask mask_16g;
+# endif
#else
u16 sllp; /* SLB page size encoding */
#endif
diff --git a/arch/powerpc/include/asm/mmu-8xx.h b/arch/powerpc/include/asm/mmu-8xx.h
index d3d7e79140c6..4c3b14703b3e 100644
--- a/arch/powerpc/include/asm/mmu-8xx.h
+++ b/arch/powerpc/include/asm/mmu-8xx.h
@@ -192,6 +192,11 @@
#endif
#ifndef __ASSEMBLY__
+struct slice_mask {
+ u64 low_slices;
+ DECLARE_BITMAP(high_slices, 0);
+};
+
typedef struct {
unsigned int id;
unsigned int active;
@@ -201,6 +206,15 @@ typedef struct {
unsigned char low_slices_psize[SLICE_ARRAY_SIZE];
unsigned char high_slices_psize[0];
unsigned long slb_addr_limit;
+# ifdef CONFIG_PPC_16K_PAGES
+ struct slice_mask mask_16k;
+# else
+ struct slice_mask mask_4k;
+# endif
+# ifdef CONFIG_HUGETLB_PAGE
+ struct slice_mask mask_512k;
+ struct slice_mask mask_8m;
+# endif
#endif
} mm_context_t;
diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
index 233c42d593dc..2115efe5e869 100644
--- a/arch/powerpc/mm/slice.c
+++ b/arch/powerpc/mm/slice.c
@@ -37,15 +37,6 @@
#include <asm/hugetlb.h>
static DEFINE_SPINLOCK(slice_convert_lock);
-/*
- * One bit per slice. We have lower slices which cover 256MB segments
- * upto 4G range. That gets us 16 low slices. For the rest we track slices
- * in 1TB size.
- */
-struct slice_mask {
- u64 low_slices;
- DECLARE_BITMAP(high_slices, SLICE_NUM_HIGH);
-};
#ifdef DEBUG
int _slice_debug = 1;
@@ -149,37 +140,44 @@ static void slice_mask_for_free(struct mm_struct *mm, struct slice_mask *ret,
__set_bit(i, ret->high_slices);
}
-static void slice_mask_for_size(struct mm_struct *mm, int psize,
- struct slice_mask *ret,
- unsigned long high_limit)
+#ifdef CONFIG_PPC_BOOK3S_64
+static struct slice_mask *slice_mask_for_size(struct mm_struct *mm, int psize)
{
- unsigned char *hpsizes, *lpsizes;
- int index, mask_index;
- unsigned long i;
-
- ret->low_slices = 0;
- if (SLICE_NUM_HIGH)
- bitmap_zero(ret->high_slices, SLICE_NUM_HIGH);
-
- lpsizes = mm->context.low_slices_psize;
- for (i = 0; i < SLICE_NUM_LOW; i++) {
- mask_index = i & 0x1;
- index = i >> 1;
- if (((lpsizes[index] >> (mask_index * 4)) & 0xf) == psize)
- ret->low_slices |= 1u << i;
- }
-
- if (high_limit <= SLICE_LOW_TOP)
- return;
-
- hpsizes = mm->context.high_slices_psize;
- for (i = 0; i < GET_HIGH_SLICE_INDEX(high_limit); i++) {
- mask_index = i & 0x1;
- index = i >> 1;
- if (((hpsizes[index] >> (mask_index * 4)) & 0xf) == psize)
- __set_bit(i, ret->high_slices);
- }
+#ifdef CONFIG_PPC_64K_PAGES
+ if (psize == MMU_PAGE_64K)
+ return &mm->context.mask_64k;
+#endif
+ if (psize == MMU_PAGE_4K)
+ return &mm->context.mask_4k;
+#ifdef CONFIG_HUGETLB_PAGE
+ if (psize == MMU_PAGE_16M)
+ return &mm->context.mask_16m;
+ if (psize == MMU_PAGE_16G)
+ return &mm->context.mask_16g;
+#endif
+ BUG();
}
+#elif defined(CONFIG_PPC_8xx)
+static struct slice_mask *slice_mask_for_size(struct mm_struct *mm, int psize)
+{
+#ifdef CONFIG_PPC_16K_PAGES
+ if (psize == MMU_PAGE_16K)
+ return &mm->context.mask_16k;
+#else
+ if (psize == MMU_PAGE_4K)
+ return &mm->context.mask_4k;
+#endif
+#ifdef CONFIG_HUGETLB_PAGE
+ if (psize == MMU_PAGE_512K)
+ return &mm->context.mask_512k;
+ if (psize == MMU_PAGE_8M)
+ return &mm->context.mask_8m;
+#endif
+ BUG();
+}
+#else
+#error "Must define the slice masks for page sizes supported by the platform"
+#endif
static int slice_check_fit(struct mm_struct *mm,
const struct slice_mask *mask,
@@ -226,11 +224,15 @@ static void slice_convert(struct mm_struct *mm,
int index, mask_index;
/* Write the new slice psize bits */
unsigned char *hpsizes, *lpsizes;
+ struct slice_mask *psize_mask, *old_mask;
unsigned long i, flags;
+ int old_psize;
slice_dbg("slice_convert(mm=%p, psize=%d)\n", mm, psize);
slice_print_mask(" mask", mask);
+ psize_mask = slice_mask_for_size(mm, psize);
+
/* We need to use a spinlock here to protect against
* concurrent 64k -> 4k demotion ...
*/
@@ -243,6 +245,14 @@ static void slice_convert(struct mm_struct *mm,
mask_index = i & 0x1;
index = i >> 1;
+
+ /* Update the slice_mask */
+ old_psize = (lpsizes[index] >> (mask_index * 4)) & 0xf;
+ old_mask = slice_mask_for_size(mm, old_psize);
+ old_mask->low_slices &= ~(1u << i);
+ psize_mask->low_slices |= 1u << i;
+
+ /* Update the sizes array */
lpsizes[index] = (lpsizes[index] & ~(0xf << (mask_index * 4))) |
(((unsigned long)psize) << (mask_index * 4));
}
@@ -254,6 +264,14 @@ static void slice_convert(struct mm_struct *mm,
mask_index = i & 0x1;
index = i >> 1;
+
+ /* Update the slice_mask */
+ old_psize = (hpsizes[index] >> (mask_index * 4)) & 0xf;
+ old_mask = slice_mask_for_size(mm, old_psize);
+ __clear_bit(i, old_mask->high_slices);
+ __set_bit(i, psize_mask->high_slices);
+
+ /* Update the sizes array */
hpsizes[index] = (hpsizes[index] & ~(0xf << (mask_index * 4))) |
(((unsigned long)psize) << (mask_index * 4));
}
@@ -464,7 +482,13 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
}
if (high_limit > mm->context.slb_addr_limit) {
+ /*
+ * Increasing the slb_addr_limit does not require
+ * slice mask cache to be recalculated because it should
+ * be already initialised beyond the old address limit.
+ */
mm->context.slb_addr_limit = high_limit;
+
on_each_cpu(slice_flush_segments, mm, 1);
}
@@ -506,7 +530,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
/* First make up a "good" mask of slices that have the right size
* already
*/
- slice_mask_for_size(mm, psize, &good_mask, high_limit);
+ good_mask = *slice_mask_for_size(mm, psize);
slice_print_mask(" good_mask", &good_mask);
/*
@@ -531,7 +555,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
#ifdef CONFIG_PPC_64K_PAGES
/* If we support combo pages, we can allow 64k pages in 4k slices */
if (psize == MMU_PAGE_64K) {
- slice_mask_for_size(mm, MMU_PAGE_4K, &compat_mask, high_limit);
+ compat_mask = *slice_mask_for_size(mm, MMU_PAGE_4K);
if (fixed)
slice_or_mask(&good_mask, &compat_mask);
}
@@ -683,6 +707,7 @@ EXPORT_SYMBOL_GPL(get_slice_psize);
void slice_init_new_context_exec(struct mm_struct *mm)
{
unsigned char *hpsizes, *lpsizes;
+ struct slice_mask *mask;
unsigned int psize = mmu_virtual_psize;
slice_dbg("slice_init_new_context_exec(mm=%p)\n", mm);
@@ -703,6 +728,14 @@ void slice_init_new_context_exec(struct mm_struct *mm)
hpsizes = mm->context.high_slices_psize;
memset(hpsizes, (psize << 4) | psize, SLICE_NUM_HIGH >> 1);
+
+ /*
+ * Slice mask cache starts zeroed, fill the default size cache.
+ */
+ mask = slice_mask_for_size(mm, psize);
+ mask->low_slices = ~0UL;
+ if (SLICE_NUM_HIGH)
+ bitmap_fill(mask->high_slices, SLICE_NUM_HIGH);
}
void slice_set_range_psize(struct mm_struct *mm, unsigned long start,
@@ -741,18 +774,17 @@ int is_hugepage_only_range(struct mm_struct *mm, unsigned long addr,
{
struct slice_mask mask, available;
unsigned int psize = mm->context.user_psize;
- unsigned long high_limit = mm->context.slb_addr_limit;
if (radix_enabled())
return 0;
slice_range_to_mask(addr, len, &mask);
- slice_mask_for_size(mm, psize, &available, high_limit);
+ available = *slice_mask_for_size(mm, psize);
#ifdef CONFIG_PPC_64K_PAGES
/* We need to account for 4k slices too */
if (psize == MMU_PAGE_64K) {
struct slice_mask compat_mask;
- slice_mask_for_size(mm, MMU_PAGE_4K, &compat_mask, high_limit);
+ compat_mask = *slice_mask_for_size(mm, MMU_PAGE_4K);
slice_or_mask(&available, &compat_mask);
}
#endif
--
2.16.1
^ permalink raw reply related
* [PATCH 06/10] powerpc/mm/slice: implement slice_check_range_fits
From: Nicholas Piggin @ 2018-03-06 13:25 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin, Aneesh Kumar K . V, Christophe Leroy
In-Reply-To: <20180306132507.10649-1-npiggin@gmail.com>
Rather than build slice masks from a range then use that to check for
fit in a candidate mask, implement slice_check_range_fits that checks
if a range fits in a mask directly.
This allows several structures to be removed from stacks, and also we
don't expect a huge range in a lot of these cases, so building and
comparing a full mask is going to be more expensive than testing just
one or two bits of the range.
On POWER8, this increases vfork+exec+exit performance by 0.3%
and reduces time to mmap+munmap a 64kB page by 5%.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/mm/slice.c | 63 ++++++++++++++++++++++++++-----------------------
1 file changed, 34 insertions(+), 29 deletions(-)
diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
index 2115efe5e869..3841fca75006 100644
--- a/arch/powerpc/mm/slice.c
+++ b/arch/powerpc/mm/slice.c
@@ -179,26 +179,35 @@ static struct slice_mask *slice_mask_for_size(struct mm_struct *mm, int psize)
#error "Must define the slice masks for page sizes supported by the platform"
#endif
-static int slice_check_fit(struct mm_struct *mm,
- const struct slice_mask *mask,
- const struct slice_mask *available)
+static bool slice_check_range_fits(struct mm_struct *mm,
+ const struct slice_mask *available,
+ unsigned long start, unsigned long len)
{
- DECLARE_BITMAP(result, SLICE_NUM_HIGH);
- /*
- * Make sure we just do bit compare only to the max
- * addr limit and not the full bit map size.
- */
- unsigned long slice_count = GET_HIGH_SLICE_INDEX(mm->context.slb_addr_limit);
+ unsigned long end = start + len - 1;
+ u64 low_slices = 0;
- if (!SLICE_NUM_HIGH)
- return (mask->low_slices & available->low_slices) ==
- mask->low_slices;
+ if (start < SLICE_LOW_TOP) {
+ unsigned long mend = min(end, (SLICE_LOW_TOP - 1));
+
+ low_slices = (1u << (GET_LOW_SLICE_INDEX(mend) + 1))
+ - (1u << GET_LOW_SLICE_INDEX(start));
+ }
+ if ((low_slices & available->low_slices) != low_slices)
+ return false;
+
+ if (SLICE_NUM_HIGH && ((start + len) > SLICE_LOW_TOP)) {
+ unsigned long start_index = GET_HIGH_SLICE_INDEX(start);
+ unsigned long align_end = ALIGN(end, (1UL << SLICE_HIGH_SHIFT));
+ unsigned long count = GET_HIGH_SLICE_INDEX(align_end) - start_index;
+ unsigned long i;
- bitmap_and(result, mask->high_slices,
- available->high_slices, slice_count);
+ for (i = start_index; i < start_index + count; i++) {
+ if (!test_bit(i, available->high_slices))
+ return false;
+ }
+ }
- return (mask->low_slices & available->low_slices) == mask->low_slices &&
- bitmap_equal(result, mask->high_slices, slice_count);
+ return true;
}
static void slice_flush_segments(void *parm)
@@ -562,15 +571,11 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
#endif
/* First check hint if it's valid or if we have MAP_FIXED */
- if (addr != 0 || fixed) {
- /* Build a mask for the requested range */
- slice_range_to_mask(addr, len, &mask);
- slice_print_mask(" mask", &mask);
-
+ if (addr || fixed) {
/* Check if we fit in the good mask. If we do, we just return,
* nothing else to do
*/
- if (slice_check_fit(mm, &mask, &good_mask)) {
+ if (slice_check_range_fits(mm, &good_mask, addr, len)) {
slice_dbg(" fits good !\n");
return addr;
}
@@ -596,10 +601,11 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
slice_or_mask(&potential_mask, &good_mask);
slice_print_mask(" potential", &potential_mask);
- if ((addr != 0 || fixed) &&
- slice_check_fit(mm, &mask, &potential_mask)) {
- slice_dbg(" fits potential !\n");
- goto convert;
+ if (addr || fixed) {
+ if (slice_check_range_fits(mm, &potential_mask, addr, len)) {
+ slice_dbg(" fits potential !\n");
+ goto convert;
+ }
}
/* If we have MAP_FIXED and failed the above steps, then error out */
@@ -772,13 +778,12 @@ void slice_set_range_psize(struct mm_struct *mm, unsigned long start,
int is_hugepage_only_range(struct mm_struct *mm, unsigned long addr,
unsigned long len)
{
- struct slice_mask mask, available;
+ struct slice_mask available;
unsigned int psize = mm->context.user_psize;
if (radix_enabled())
return 0;
- slice_range_to_mask(addr, len, &mask);
available = *slice_mask_for_size(mm, psize);
#ifdef CONFIG_PPC_64K_PAGES
/* We need to account for 4k slices too */
@@ -795,6 +800,6 @@ int is_hugepage_only_range(struct mm_struct *mm, unsigned long addr,
slice_print_mask(" mask", &mask);
slice_print_mask(" available", &available);
#endif
- return !slice_check_fit(mm, &mask, &available);
+ return !slice_check_range_fits(mm, &available, addr, len);
}
#endif
--
2.16.1
^ permalink raw reply related
* [PATCH 07/10] powerpc/mm/slice: Switch to 3-operand slice bitops helpers
From: Nicholas Piggin @ 2018-03-06 13:25 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin, Aneesh Kumar K . V, Christophe Leroy
In-Reply-To: <20180306132507.10649-1-npiggin@gmail.com>
This converts the slice_mask bit operation helpers to be the usual
3-operand kind, which is clearer to work with.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/mm/slice.c | 38 +++++++++++++++++++++++---------------
1 file changed, 23 insertions(+), 15 deletions(-)
diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
index 3841fca75006..46daa1d1794f 100644
--- a/arch/powerpc/mm/slice.c
+++ b/arch/powerpc/mm/slice.c
@@ -433,25 +433,33 @@ static unsigned long slice_find_area(struct mm_struct *mm, unsigned long len,
return slice_find_area_bottomup(mm, len, mask, psize, high_limit);
}
-static inline void slice_or_mask(struct slice_mask *dst,
+static inline void slice_copy_mask(struct slice_mask *dst,
const struct slice_mask *src)
{
- dst->low_slices |= src->low_slices;
+ dst->low_slices = src->low_slices;
if (!SLICE_NUM_HIGH)
return;
- bitmap_or(dst->high_slices, dst->high_slices, src->high_slices,
- SLICE_NUM_HIGH);
+ bitmap_copy(dst->high_slices, src->high_slices, SLICE_NUM_HIGH);
}
-static inline void slice_andnot_mask(struct slice_mask *dst,
- const struct slice_mask *src)
+static inline void slice_or_mask(struct slice_mask *dst,
+ const struct slice_mask *src1,
+ const struct slice_mask *src2)
{
- dst->low_slices &= ~src->low_slices;
+ dst->low_slices = src1->low_slices | src2->low_slices;
+ if (!SLICE_NUM_HIGH)
+ return;
+ bitmap_or(dst->high_slices, src1->high_slices, src2->high_slices, SLICE_NUM_HIGH);
+}
+static inline void slice_andnot_mask(struct slice_mask *dst,
+ const struct slice_mask *src1,
+ const struct slice_mask *src2)
+{
+ dst->low_slices = src1->low_slices & ~src2->low_slices;
if (!SLICE_NUM_HIGH)
return;
- bitmap_andnot(dst->high_slices, dst->high_slices, src->high_slices,
- SLICE_NUM_HIGH);
+ bitmap_andnot(dst->high_slices, src1->high_slices, src2->high_slices, SLICE_NUM_HIGH);
}
#ifdef CONFIG_PPC_64K_PAGES
@@ -566,7 +574,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
if (psize == MMU_PAGE_64K) {
compat_mask = *slice_mask_for_size(mm, MMU_PAGE_4K);
if (fixed)
- slice_or_mask(&good_mask, &compat_mask);
+ slice_or_mask(&good_mask, &good_mask, &compat_mask);
}
#endif
@@ -598,7 +606,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
* empty and thus can be converted
*/
slice_mask_for_free(mm, &potential_mask, high_limit);
- slice_or_mask(&potential_mask, &good_mask);
+ slice_or_mask(&potential_mask, &potential_mask, &good_mask);
slice_print_mask(" potential", &potential_mask);
if (addr || fixed) {
@@ -635,7 +643,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
#ifdef CONFIG_PPC_64K_PAGES
if (addr == -ENOMEM && psize == MMU_PAGE_64K) {
/* retry the search with 4k-page slices included */
- slice_or_mask(&potential_mask, &compat_mask);
+ slice_or_mask(&potential_mask, &potential_mask, &compat_mask);
addr = slice_find_area(mm, len, &potential_mask,
psize, topdown, high_limit);
}
@@ -649,8 +657,8 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
slice_print_mask(" mask", &mask);
convert:
- slice_andnot_mask(&mask, &good_mask);
- slice_andnot_mask(&mask, &compat_mask);
+ slice_andnot_mask(&mask, &mask, &good_mask);
+ slice_andnot_mask(&mask, &mask, &compat_mask);
if (mask.low_slices ||
(SLICE_NUM_HIGH &&
!bitmap_empty(mask.high_slices, SLICE_NUM_HIGH))) {
@@ -790,7 +798,7 @@ int is_hugepage_only_range(struct mm_struct *mm, unsigned long addr,
if (psize == MMU_PAGE_64K) {
struct slice_mask compat_mask;
compat_mask = *slice_mask_for_size(mm, MMU_PAGE_4K);
- slice_or_mask(&available, &compat_mask);
+ slice_or_mask(&available, &available, &compat_mask);
}
#endif
--
2.16.1
^ permalink raw reply related
* [PATCH 08/10] powerpc/mm/slice: Use const pointers to cached slice masks where possible
From: Nicholas Piggin @ 2018-03-06 13:25 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin, Aneesh Kumar K . V, Christophe Leroy
In-Reply-To: <20180306132507.10649-1-npiggin@gmail.com>
The slice_mask cache was a basic conversion which copied the slice
mask into caller's structures, because that's how the original code
worked. In most cases the pointer can be used directly instead, saving
a copy and an on-stack structure.
On POWER8, this increases vfork+exec+exit performance by 0.3%
and reduces time to mmap+munmap a 64kB page by 2%.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/mm/slice.c | 77 +++++++++++++++++++++----------------------------
1 file changed, 33 insertions(+), 44 deletions(-)
diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
index 46daa1d1794f..086c31b8b982 100644
--- a/arch/powerpc/mm/slice.c
+++ b/arch/powerpc/mm/slice.c
@@ -472,10 +472,10 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
unsigned long flags, unsigned int psize,
int topdown)
{
- struct slice_mask mask;
struct slice_mask good_mask;
struct slice_mask potential_mask;
- struct slice_mask compat_mask;
+ const struct slice_mask *maskp;
+ const struct slice_mask *compat_maskp = NULL;
int fixed = (flags & MAP_FIXED);
int pshift = max_t(int, mmu_psize_defs[psize].shift, PAGE_SHIFT);
unsigned long page_size = 1UL << pshift;
@@ -509,22 +509,6 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
on_each_cpu(slice_flush_segments, mm, 1);
}
- /*
- * init different masks
- */
- mask.low_slices = 0;
-
- /* silence stupid warning */;
- potential_mask.low_slices = 0;
-
- compat_mask.low_slices = 0;
-
- if (SLICE_NUM_HIGH) {
- bitmap_zero(mask.high_slices, SLICE_NUM_HIGH);
- bitmap_zero(potential_mask.high_slices, SLICE_NUM_HIGH);
- bitmap_zero(compat_mask.high_slices, SLICE_NUM_HIGH);
- }
-
/* Sanity checks */
BUG_ON(mm->task_size == 0);
BUG_ON(mm->context.slb_addr_limit == 0);
@@ -547,8 +531,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
/* First make up a "good" mask of slices that have the right size
* already
*/
- good_mask = *slice_mask_for_size(mm, psize);
- slice_print_mask(" good_mask", &good_mask);
+ maskp = slice_mask_for_size(mm, psize);
/*
* Here "good" means slices that are already the right page size,
@@ -572,11 +555,19 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
#ifdef CONFIG_PPC_64K_PAGES
/* If we support combo pages, we can allow 64k pages in 4k slices */
if (psize == MMU_PAGE_64K) {
- compat_mask = *slice_mask_for_size(mm, MMU_PAGE_4K);
+ compat_maskp = slice_mask_for_size(mm, MMU_PAGE_4K);
if (fixed)
- slice_or_mask(&good_mask, &good_mask, &compat_mask);
- }
+ slice_or_mask(&good_mask, maskp, compat_maskp);
+ else
+ slice_copy_mask(&good_mask, maskp);
+ } else
#endif
+ {
+ slice_copy_mask(&good_mask, maskp);
+ }
+ slice_print_mask(" good_mask", &good_mask);
+ if (compat_maskp)
+ slice_print_mask(" compat_mask", compat_maskp);
/* First check hint if it's valid or if we have MAP_FIXED */
if (addr || fixed) {
@@ -643,7 +634,7 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
#ifdef CONFIG_PPC_64K_PAGES
if (addr == -ENOMEM && psize == MMU_PAGE_64K) {
/* retry the search with 4k-page slices included */
- slice_or_mask(&potential_mask, &potential_mask, &compat_mask);
+ slice_or_mask(&potential_mask, &potential_mask, compat_maskp);
addr = slice_find_area(mm, len, &potential_mask,
psize, topdown, high_limit);
}
@@ -652,17 +643,18 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
if (addr == -ENOMEM)
return -ENOMEM;
- slice_range_to_mask(addr, len, &mask);
+ slice_range_to_mask(addr, len, &potential_mask);
slice_dbg(" found potential area at 0x%lx\n", addr);
- slice_print_mask(" mask", &mask);
+ slice_print_mask(" mask", &potential_mask);
convert:
- slice_andnot_mask(&mask, &mask, &good_mask);
- slice_andnot_mask(&mask, &mask, &compat_mask);
- if (mask.low_slices ||
- (SLICE_NUM_HIGH &&
- !bitmap_empty(mask.high_slices, SLICE_NUM_HIGH))) {
- slice_convert(mm, &mask, psize);
+ slice_andnot_mask(&potential_mask, &potential_mask, &good_mask);
+ if (compat_maskp && !fixed)
+ slice_andnot_mask(&potential_mask, &potential_mask, compat_maskp);
+ if (potential_mask.low_slices ||
+ (SLICE_NUM_HIGH &&
+ !bitmap_empty(potential_mask.high_slices, SLICE_NUM_HIGH))) {
+ slice_convert(mm, &potential_mask, psize);
if (psize > MMU_PAGE_BASE)
on_each_cpu(slice_flush_segments, mm, 1);
}
@@ -786,28 +778,25 @@ void slice_set_range_psize(struct mm_struct *mm, unsigned long start,
int is_hugepage_only_range(struct mm_struct *mm, unsigned long addr,
unsigned long len)
{
- struct slice_mask available;
+ const struct slice_mask *maskp;
unsigned int psize = mm->context.user_psize;
if (radix_enabled())
return 0;
- available = *slice_mask_for_size(mm, psize);
+ maskp = slice_mask_for_size(mm, psize);
#ifdef CONFIG_PPC_64K_PAGES
/* We need to account for 4k slices too */
if (psize == MMU_PAGE_64K) {
- struct slice_mask compat_mask;
- compat_mask = *slice_mask_for_size(mm, MMU_PAGE_4K);
- slice_or_mask(&available, &available, &compat_mask);
+ const struct slice_mask *compat_maskp;
+ struct slice_mask available;
+
+ compat_maskp = slice_mask_for_size(mm, MMU_PAGE_4K);
+ slice_or_mask(&available, maskp, compat_maskp);
+ return !slice_check_range_fits(mm, &available, addr, len);
}
#endif
-#if 0 /* too verbose */
- slice_dbg("is_hugepage_only_range(mm=%p, addr=%lx, len=%lx)\n",
- mm, addr, len);
- slice_print_mask(" mask", &mask);
- slice_print_mask(" available", &available);
-#endif
- return !slice_check_range_fits(mm, &available, addr, len);
+ return !slice_check_range_fits(mm, maskp, addr, len);
}
#endif
--
2.16.1
^ permalink raw reply related
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