* [PATCH v2] powerpc/configs: Update skiroot defconfig
From: Joel Stanley @ 2018-09-18 3:36 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Stewart Smith
Disable new features from recent releases, and clean out some other
unused options:
- Enable EXPERT, so we can disable some things
- Disable non-powerpc BPF decoders
- Disable TASKSTATS
- Disable unused syscalls
- Set more things to be modules
- Turn off unused network vendors
- PPC_OF_BOOT_TRAMPOLINE and FB_OF are unused on powernv
- Drop unused Radeon and Matrox GPU drivers
- IPV6 support landed in petitboot
- Bringup related command line powersave=off dropped, switch to quiet
Set CONFIG_I2C_CHARDEV=y as the module is not loaded automatically, and
without this i2cget etc. will fail in the skiroot environment.
This defconfig gets us build coverage of KERNEL_XZ, which was broken in
the 4.19 merge window for powerpc.
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
v2: re-sync with version used in op-build
---
arch/powerpc/configs/skiroot_defconfig | 154 +++++++++++++++++--------
1 file changed, 108 insertions(+), 46 deletions(-)
diff --git a/arch/powerpc/configs/skiroot_defconfig b/arch/powerpc/configs/skiroot_defconfig
index 6bd5e7261335..cfdd08897a06 100644
--- a/arch/powerpc/configs/skiroot_defconfig
+++ b/arch/powerpc/configs/skiroot_defconfig
@@ -3,20 +3,17 @@ CONFIG_ALTIVEC=y
CONFIG_VSX=y
CONFIG_NR_CPUS=2048
CONFIG_CPU_LITTLE_ENDIAN=y
+CONFIG_KERNEL_XZ=y
# CONFIG_SWAP is not set
CONFIG_SYSVIPC=y
CONFIG_POSIX_MQUEUE=y
# CONFIG_CROSS_MEMORY_ATTACH is not set
CONFIG_NO_HZ=y
CONFIG_HIGH_RES_TIMERS=y
-CONFIG_TASKSTATS=y
-CONFIG_TASK_DELAY_ACCT=y
-CONFIG_TASK_XACCT=y
-CONFIG_TASK_IO_ACCOUNTING=y
+# CONFIG_CPU_ISOLATION is not set
CONFIG_IKCONFIG=y
CONFIG_IKCONFIG_PROC=y
CONFIG_LOG_BUF_SHIFT=20
-CONFIG_RELAY=y
CONFIG_BLK_DEV_INITRD=y
# CONFIG_RD_GZIP is not set
# CONFIG_RD_BZIP2 is not set
@@ -24,8 +21,14 @@ CONFIG_BLK_DEV_INITRD=y
# CONFIG_RD_LZO is not set
# CONFIG_RD_LZ4 is not set
CONFIG_CC_OPTIMIZE_FOR_SIZE=y
+CONFIG_EXPERT=y
+# CONFIG_SGETMASK_SYSCALL is not set
+# CONFIG_SYSFS_SYSCALL is not set
+# CONFIG_SHMEM is not set
+# CONFIG_AIO is not set
CONFIG_PERF_EVENTS=y
# CONFIG_COMPAT_BRK is not set
+CONFIG_SLAB_FREELIST_HARDENED=y
CONFIG_JUMP_LABEL=y
CONFIG_STRICT_KERNEL_RWX=y
CONFIG_MODULES=y
@@ -35,7 +38,9 @@ CONFIG_MODULE_SIG_FORCE=y
CONFIG_MODULE_SIG_SHA512=y
CONFIG_PARTITION_ADVANCED=y
# CONFIG_IOSCHED_DEADLINE is not set
+# CONFIG_PPC_VAS is not set
# CONFIG_PPC_PSERIES is not set
+# CONFIG_PPC_OF_BOOT_TRAMPOLINE is not set
CONFIG_CPU_FREQ_DEFAULT_GOV_ONDEMAND=y
CONFIG_CPU_IDLE=y
CONFIG_HZ_100=y
@@ -48,8 +53,9 @@ CONFIG_NUMA=y
CONFIG_PPC_64K_PAGES=y
CONFIG_SCHED_SMT=y
CONFIG_CMDLINE_BOOL=y
-CONFIG_CMDLINE="console=tty0 console=hvc0 powersave=off"
+CONFIG_CMDLINE="console=tty0 console=hvc0 ipr.fast_reboot=1 quiet"
# CONFIG_SECCOMP is not set
+# CONFIG_PPC_MEM_KEYS is not set
CONFIG_NET=y
CONFIG_PACKET=y
CONFIG_UNIX=y
@@ -60,7 +66,6 @@ CONFIG_SYN_COOKIES=y
# CONFIG_INET_XFRM_MODE_TRANSPORT is not set
# CONFIG_INET_XFRM_MODE_TUNNEL is not set
# CONFIG_INET_XFRM_MODE_BEET is not set
-# CONFIG_IPV6 is not set
CONFIG_DNS_RESOLVER=y
# CONFIG_WIRELESS is not set
CONFIG_UEVENT_HELPER_PATH="/sbin/hotplug"
@@ -73,8 +78,10 @@ CONFIG_BLK_DEV_RAM=y
CONFIG_BLK_DEV_RAM_SIZE=65536
CONFIG_VIRTIO_BLK=m
CONFIG_BLK_DEV_NVME=m
-CONFIG_EEPROM_AT24=y
+CONFIG_NVME_MULTIPATH=y
+CONFIG_EEPROM_AT24=m
# CONFIG_CXL is not set
+# CONFIG_OCXL is not set
CONFIG_BLK_DEV_SD=m
CONFIG_BLK_DEV_SR=m
CONFIG_BLK_DEV_SR_VENDOR=y
@@ -85,7 +92,6 @@ CONFIG_SCSI_FC_ATTRS=y
CONFIG_SCSI_CXGB3_ISCSI=m
CONFIG_SCSI_CXGB4_ISCSI=m
CONFIG_SCSI_BNX2_ISCSI=m
-CONFIG_BE2ISCSI=m
CONFIG_SCSI_AACRAID=m
CONFIG_MEGARAID_NEWGEN=y
CONFIG_MEGARAID_MM=m
@@ -102,7 +108,7 @@ CONFIG_SCSI_VIRTIO=m
CONFIG_SCSI_DH=y
CONFIG_SCSI_DH_ALUA=m
CONFIG_ATA=y
-CONFIG_SATA_AHCI=y
+CONFIG_SATA_AHCI=m
# CONFIG_ATA_SFF is not set
CONFIG_MD=y
CONFIG_BLK_DEV_MD=m
@@ -119,25 +125,72 @@ CONFIG_DM_SNAPSHOT=m
CONFIG_DM_MIRROR=m
CONFIG_DM_ZERO=m
CONFIG_DM_MULTIPATH=m
+# CONFIG_NET_VENDOR_3COM is not set
+# CONFIG_NET_VENDOR_ADAPTEC is not set
+# CONFIG_NET_VENDOR_AGERE is not set
+# CONFIG_NET_VENDOR_ALACRITECH is not set
CONFIG_ACENIC=m
CONFIG_ACENIC_OMIT_TIGON_I=y
-CONFIG_TIGON3=y
+# CONFIG_NET_VENDOR_AMAZON is not set
+# CONFIG_NET_VENDOR_AMD is not set
+# CONFIG_NET_VENDOR_AQUANTIA is not set
+# CONFIG_NET_VENDOR_ARC is not set
+# CONFIG_NET_VENDOR_ATHEROS is not set
+CONFIG_TIGON3=m
CONFIG_BNX2X=m
-CONFIG_CHELSIO_T1=y
+# CONFIG_NET_VENDOR_BROCADE is not set
+# CONFIG_NET_CADENCE is not set
+# CONFIG_NET_VENDOR_CAVIUM is not set
+CONFIG_CHELSIO_T1=m
+# CONFIG_NET_VENDOR_CISCO is not set
+# CONFIG_NET_VENDOR_CORTINA is not set
+# CONFIG_NET_VENDOR_DEC is not set
+# CONFIG_NET_VENDOR_DLINK is not set
CONFIG_BE2NET=m
-CONFIG_S2IO=m
-CONFIG_E100=m
+# CONFIG_NET_VENDOR_EZCHIP is not set
+# CONFIG_NET_VENDOR_HP is not set
+# CONFIG_NET_VENDOR_HUAWEI is not set
CONFIG_E1000=m
-CONFIG_E1000E=m
+CONFIG_IGB=m
CONFIG_IXGB=m
CONFIG_IXGBE=m
+CONFIG_I40E=m
+CONFIG_S2IO=m
+# CONFIG_NET_VENDOR_MARVELL is not set
CONFIG_MLX4_EN=m
+# CONFIG_MLX4_CORE_GEN2 is not set
CONFIG_MLX5_CORE=m
-CONFIG_MLX5_CORE_EN=y
+# CONFIG_NET_VENDOR_MICREL is not set
CONFIG_MYRI10GE=m
+# CONFIG_NET_VENDOR_NATSEMI is not set
+# CONFIG_NET_VENDOR_NETRONOME is not set
+# CONFIG_NET_VENDOR_NI is not set
+# CONFIG_NET_VENDOR_NVIDIA is not set
+# CONFIG_NET_VENDOR_OKI is not set
+# CONFIG_NET_PACKET_ENGINE is not set
CONFIG_QLGE=m
CONFIG_NETXEN_NIC=m
+# CONFIG_NET_VENDOR_QUALCOMM is not set
+# CONFIG_NET_VENDOR_RDC is not set
+# CONFIG_NET_VENDOR_REALTEK is not set
+# CONFIG_NET_VENDOR_RENESAS is not set
+# CONFIG_NET_VENDOR_ROCKER is not set
+# CONFIG_NET_VENDOR_SAMSUNG is not set
+# CONFIG_NET_VENDOR_SEEQ is not set
CONFIG_SFC=m
+# CONFIG_NET_VENDOR_SILAN is not set
+# CONFIG_NET_VENDOR_SIS is not set
+# CONFIG_NET_VENDOR_SMSC is not set
+# CONFIG_NET_VENDOR_SOCIONEXT is not set
+# CONFIG_NET_VENDOR_STMICRO is not set
+# CONFIG_NET_VENDOR_SUN is not set
+# CONFIG_NET_VENDOR_SYNOPSYS is not set
+# CONFIG_NET_VENDOR_TEHUTI is not set
+# CONFIG_NET_VENDOR_TI is not set
+# CONFIG_NET_VENDOR_VIA is not set
+# CONFIG_NET_VENDOR_WIZNET is not set
+# CONFIG_NET_VENDOR_XILINX is not set
+CONFIG_PHYLIB=y
# CONFIG_USB_NET_DRIVERS is not set
# CONFIG_WLAN is not set
CONFIG_INPUT_EVDEV=y
@@ -149,39 +202,51 @@ CONFIG_SERIAL_8250_CONSOLE=y
CONFIG_IPMI_HANDLER=y
CONFIG_IPMI_DEVICE_INTERFACE=y
CONFIG_IPMI_POWERNV=y
+CONFIG_IPMI_WATCHDOG=y
CONFIG_HW_RANDOM=y
+CONFIG_TCG_TPM=y
CONFIG_TCG_TIS_I2C_NUVOTON=y
+CONFIG_I2C=y
# CONFIG_I2C_COMPAT is not set
CONFIG_I2C_CHARDEV=y
# CONFIG_I2C_HELPER_AUTO is not set
-CONFIG_DRM=y
-CONFIG_DRM_RADEON=y
+CONFIG_I2C_ALGOBIT=y
+CONFIG_I2C_OPAL=m
+CONFIG_PPS=y
+CONFIG_SENSORS_IBMPOWERNV=m
+CONFIG_DRM=m
CONFIG_DRM_AST=m
+CONFIG_FB=y
CONFIG_FIRMWARE_EDID=y
-CONFIG_FB_MODE_HELPERS=y
-CONFIG_FB_OF=y
-CONFIG_FB_MATROX=y
-CONFIG_FB_MATROX_MILLENIUM=y
-CONFIG_FB_MATROX_MYSTIQUE=y
-CONFIG_FB_MATROX_G=y
-# CONFIG_LCD_CLASS_DEVICE is not set
-# CONFIG_BACKLIGHT_GENERIC is not set
# CONFIG_VGA_CONSOLE is not set
+CONFIG_FRAMEBUFFER_CONSOLE=y
CONFIG_LOGO=y
# CONFIG_LOGO_LINUX_MONO is not set
# CONFIG_LOGO_LINUX_VGA16 is not set
+CONFIG_HID_GENERIC=m
+CONFIG_HID_A4TECH=y
+CONFIG_HID_BELKIN=y
+CONFIG_HID_CHERRY=y
+CONFIG_HID_CHICONY=y
+CONFIG_HID_CYPRESS=y
+CONFIG_HID_EZKEY=y
+CONFIG_HID_ITE=y
+CONFIG_HID_KENSINGTON=y
+CONFIG_HID_LOGITECH=y
+CONFIG_HID_MICROSOFT=y
+CONFIG_HID_MONTEREY=y
CONFIG_USB_HIDDEV=y
-CONFIG_USB=y
-CONFIG_USB_MON=y
-CONFIG_USB_XHCI_HCD=y
-CONFIG_USB_EHCI_HCD=y
+CONFIG_USB=m
+CONFIG_USB_XHCI_HCD=m
+CONFIG_USB_EHCI_HCD=m
# CONFIG_USB_EHCI_HCD_PPC_OF is not set
-CONFIG_USB_OHCI_HCD=y
-CONFIG_USB_STORAGE=y
+CONFIG_USB_OHCI_HCD=m
+CONFIG_USB_STORAGE=m
CONFIG_RTC_CLASS=y
+CONFIG_RTC_DRV_OPAL=m
CONFIG_RTC_DRV_GENERIC=m
CONFIG_VIRT_DRIVERS=y
-CONFIG_VIRTIO_PCI=y
+CONFIG_VIRTIO_PCI=m
# CONFIG_IOMMU_SUPPORT is not set
CONFIG_EXT4_FS=m
CONFIG_EXT4_FS_POSIX_ACL=y
@@ -195,10 +260,9 @@ CONFIG_UDF_FS=m
CONFIG_MSDOS_FS=m
CONFIG_VFAT_FS=m
CONFIG_PROC_KCORE=y
-CONFIG_TMPFS=y
-CONFIG_TMPFS_POSIX_ACL=y
# CONFIG_MISC_FILESYSTEMS is not set
# CONFIG_NETWORK_FILESYSTEMS is not set
+CONFIG_NLS=y
CONFIG_NLS_DEFAULT="utf8"
CONFIG_NLS_CODEPAGE_437=y
CONFIG_NLS_ASCII=y
@@ -207,26 +271,24 @@ CONFIG_NLS_UTF8=y
CONFIG_CRC16=y
CONFIG_CRC_ITU_T=y
CONFIG_LIBCRC32C=y
+# CONFIG_XZ_DEC_X86 is not set
+# CONFIG_XZ_DEC_IA64 is not set
+# CONFIG_XZ_DEC_ARM is not set
+# CONFIG_XZ_DEC_ARMTHUMB is not set
+# CONFIG_XZ_DEC_SPARC is not set
CONFIG_PRINTK_TIME=y
CONFIG_MAGIC_SYSRQ=y
-CONFIG_DEBUG_KERNEL=y
CONFIG_DEBUG_STACKOVERFLOW=y
CONFIG_SOFTLOCKUP_DETECTOR=y
+CONFIG_BOOTPARAM_SOFTLOCKUP_PANIC=y
CONFIG_HARDLOCKUP_DETECTOR=y
CONFIG_BOOTPARAM_HARDLOCKUP_PANIC=y
-CONFIG_BOOTPARAM_SOFTLOCKUP_PANIC=y
CONFIG_WQ_WATCHDOG=y
-CONFIG_SCHEDSTATS=y
+# CONFIG_SCHED_DEBUG is not set
# CONFIG_FTRACE is not set
+# CONFIG_RUNTIME_TESTING_MENU is not set
CONFIG_XMON=y
CONFIG_XMON_DEFAULT=y
-CONFIG_SECURITY=y
-CONFIG_IMA=y
-CONFIG_EVM=y
+CONFIG_ENCRYPTED_KEYS=y
# CONFIG_CRYPTO_ECHAINIV is not set
-CONFIG_CRYPTO_ECB=y
-CONFIG_CRYPTO_CMAC=y
-CONFIG_CRYPTO_MD4=y
-CONFIG_CRYPTO_ARC4=y
-CONFIG_CRYPTO_DES=y
# CONFIG_CRYPTO_HW is not set
--
2.17.1
^ permalink raw reply related
* Re: [RFC PATCH 06/11] powerpc/tm: Refactor the __switch_to_tm code
From: Michael Neuling @ 2018-09-18 4:04 UTC (permalink / raw)
To: Breno Leitao, linuxppc-dev; +Cc: paulus, gromero, mpe, ldufour
In-Reply-To: <1536781219-13938-7-git-send-email-leitao@debian.org>
On Wed, 2018-09-12 at 16:40 -0300, Breno Leitao wrote:
> __switch_to_tm is the function that switches between two tasks which migh=
t
> have TM enabled. This function is clearly split in two parts, the task th=
at
> is leaving the CPU, known as 'prev' and the task that is being scheduled,
> known as new.
>=20
> It starts checking if the previous task had TM enable, if so, it increase=
s
> the load_tm (this is the only place we increment load_tm). It also saves
> the TM SPRs here.
>=20
> If the previous task was scheduled out with a transaction active, the
> failure cause needs to be updated, since it might contain the failure cau=
se
> that caused the exception, as TM_CAUSE_MISC. In this case, since there wa=
s
> a context switch, overwrite the failure cause.
>=20
> If the previous task has overflowed load_tm, disable TM, putting the
> facility save/restore lazy mechanism at lazy mode.
>=20
> Regarding the new task, when loading it, it basically restore the SPRs, a=
nd
> TIF_RESTORE_TM (already set by tm_reclaim_current if the transaction was
> active) would invoke the recheckpoint process later in restore_tm_state()
> if recheckpoint is somehow required.
This paragraph is a little awkwardly worded. Can you rewrite?
> On top of that, both tm_reclaim_task() and tm_recheckpoint_new_task()
> functions are not used anymore, removing them.
What about tm_reclaim_current(). This is being used in places like signals
which I would have thought we could avoid with this series
>=20
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
> arch/powerpc/kernel/process.c | 163 +++++++++++++++-------------------
> 1 file changed, 74 insertions(+), 89 deletions(-)
>=20
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.=
c
> index fe063c0142e3..5cace1b744b1 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -921,48 +921,6 @@ void tm_reclaim_current(uint8_t cause)
> tm_reclaim_thread(¤t->thread, cause);
> }
> =20
> -static inline void tm_reclaim_task(struct task_struct *tsk)
> -{
> - /* We have to work out if we're switching from/to a task that's in the
> - * middle of a transaction.
> - *
> - * In switching we need to maintain a 2nd register state as
> - * oldtask->thread.ckpt_regs. We tm_reclaim(oldproc); this saves the
> - * checkpointed (tbegin) state in ckpt_regs, ckfp_state and
> - * ckvr_state
> - *
> - * We also context switch (save) TFHAR/TEXASR/TFIAR in here.
> - */
> - struct thread_struct *thr =3D &tsk->thread;
> -
> - if (!thr->regs)
> - return;
> -
> - if (!MSR_TM_ACTIVE(thr->regs->msr))
> - goto out_and_saveregs;
> -
> - WARN_ON(tm_suspend_disabled);
> -
> - TM_DEBUG("--- tm_reclaim on pid %d (NIP=3D%lx, "
> - "ccr=3D%lx, msr=3D%lx, trap=3D%lx)\n",
> - tsk->pid, thr->regs->nip,
> - thr->regs->ccr, thr->regs->msr,
> - thr->regs->trap);
> -
> - tm_reclaim_thread(thr, TM_CAUSE_RESCHED);
> -
> - TM_DEBUG("--- tm_reclaim on pid %d complete\n",
> - tsk->pid);
> -
> -out_and_saveregs:
> - /* Always save the regs here, even if a transaction's not active.
> - * This context-switches a thread's TM info SPRs. We do it here to
> - * be consistent with the restore path (in recheckpoint) which
> - * cannot happen later in _switch().
> - */
> - tm_save_sprs(thr);
> -}
> -
> extern void __tm_recheckpoint(struct thread_struct *thread);
> =20
> void tm_recheckpoint(struct thread_struct *thread)
> @@ -997,59 +955,87 @@ static void tm_fix_failure_cause(struct task_struct
> *task, uint8_t cause)
> task->thread.tm_texasr |=3D (unsigned long) cause << 56;
> }
> =20
> -static inline void tm_recheckpoint_new_task(struct task_struct *new)
> +static inline void __switch_to_tm(struct task_struct *prev,
Can we just drop the __ ?
> + struct task_struct *new)
> {
> if (!cpu_has_feature(CPU_FTR_TM))
> return;
> =20
> - /* Recheckpoint the registers of the thread we're about to switch to.
> - *
> - * If the task was using FP, we non-lazily reload both the original and
> - * the speculative FP register states. This is because the kernel
> - * doesn't see if/when a TM rollback occurs, so if we take an FP
> - * unavailable later, we are unable to determine which set of FP regs
> - * need to be restored.
> - */
> - if (!tm_enabled(new))
> - return;
> -
> - if (!MSR_TM_ACTIVE(new->thread.regs->msr)){
> - tm_restore_sprs(&new->thread);
> - return;
> - }
> - /* Recheckpoint to restore original checkpointed register state. */
> - TM_DEBUG("*** tm_recheckpoint of pid %d (new->msr 0x%lx)\n",
> - new->pid, new->thread.regs->msr);
> -
> - tm_recheckpoint(&new->thread);
> -
> - /*
> - * The checkpointed state has been restored but the live state has
> - * not, ensure all the math functionality is turned off to trigger
> - * restore_math() to reload.
> - */
> - new->thread.regs->msr &=3D ~(MSR_FP | MSR_VEC | MSR_VSX);
> + /* The task leaving the CPU was using TM, let's handle it */
> + if (tm_enabled(prev)) {
> + /*
> + * Load_tm is incremented only when the task is scheduled out
> + */
> + prev->thread.load_tm++;
> =20
> - TM_DEBUG("*** tm_recheckpoint of pid %d complete "
> - "(kernel msr 0x%lx)\n",
> - new->pid, mfmsr());
> -}
> + /*
> + * If TM is enabled for the thread, it needs to, at least,
> + * save the SPRs
> + */
> + tm_enable();
> + tm_save_sprs(&prev->thread);
> =20
> -static inline void __switch_to_tm(struct task_struct *prev,
> - struct task_struct *new)
> -{
> - if (cpu_has_feature(CPU_FTR_TM)) {
> - if (tm_enabled(prev) || tm_enabled(new))
> - tm_enable();
> + /*
> + * If we got here with an active transaction, then, it was
> + * aborted by TM_KERNEL_ENTRY and the fix the failure case
> + * needs to be fixed, so, indepedently how we arrived here, the
> + * new TM abort case will be TM_CAUSE_RESCHED now.
What does "fix the failure case needs to be fixed" mean?
also s/indepedently/independently/
> + */
> + if (MSR_TM_ACTIVE(prev->thread.regs->msr)) {
> + /*
> + * If there was an IRQ during trecheckpoint, it will
> + * cause an IRQ to be replayed. This replayed IRQ can
> + * invoke SCHEDULE_USER, thus, we arrive here with a TM
> + * active transaction.
I don't think this can happen. trecheckpoint (and treclaim) are called with=
IRQs
hard off (since they change r1).
I think something else is going on here. I think this code and comment need=
s to
go but I assume it's here because you are seeing something.
> + * I.e, the task was leaving kernelspace to userspace,
> + * already trecheckpointed, but there was a IRQ during
> + * the trecheckpoint process (soft irq disabled), and
> + * on the IRQ replay, the process was de-scheduled, so,
> + * SCHEDULE_USER was called and here we are.
> + *
> + */
> + if (MSR_TM_ACTIVE(mfmsr())) {
> + /*
> + * This is the only other case other than
> + * TM_KERNEL_ENTRY that does a TM reclaim
> + */
> + tm_reclaim_current(TM_CAUSE_RESCHED);
> + }
> =20
> - if (tm_enabled(prev)) {
> - prev->thread.load_tm++;
> - tm_reclaim_task(prev);
> - if (!MSR_TM_ACTIVE(prev->thread.regs->msr) && prev-
> >thread.load_tm =3D=3D 0)
> + /*
> + * If rescheduled with TM active, update the
> + * failure cause
> + */
> + tm_fix_failure_cause(prev, TM_CAUSE_RESCHED);
> + } else {
> + /*
> + * TM enabled but not transactional. Just disable TM
> + * if load_tm overflows. This should be the only place
> + * that disables the TM and reenables the laziness
> + * save/restore
> + */
> + if (prev->thread.load_tm =3D=3D 0)
> prev->thread.regs->msr &=3D ~MSR_TM; =09
> }
> + }
> =20
> - tm_recheckpoint_new_task(new);
> + /*
> + * It is a *bug* if we arrived so late with a transaction active
> + * (more precisely suspended)
> + */
> + if (WARN_ON(MSR_TM_ACTIVE(mfmsr()))) {
> + /* Recovery path. 0x99 shouldn't be exported to UAPI */
> + tm_reclaim_current(0x99);
> + }
> +
> + /*
> + * If the next task has TM enabled, restore the SPRs. Do not need to
> + * care about recheckpoint at this time. It will be done later if
> + * TIF_RESTORE_TM was set when the task was scheduled out
> + */
> + if (tm_enabled(new)) {
> + tm_enable();
> + tm_restore_sprs(&new->thread);
> }
> }
> =20
> @@ -1101,7 +1087,6 @@ void restore_tm_state(struct pt_regs *regs)
> }
> =20
> #else
> -#define tm_recheckpoint_new_task(new)
> #define __switch_to_tm(prev, new)
> #endif /* CONFIG_PPC_TRANSACTIONAL_MEM */
> =20
> @@ -1588,9 +1573,9 @@ int arch_dup_task_struct(struct task_struct *dst, s=
truct
> task_struct *src)
> /*
> * Flush TM state out so we can copy it. __switch_to_tm() does this
> * flush but it removes the checkpointed state from the current CPU and
> - * transitions the CPU out of TM mode. Hence we need to call
> - * tm_recheckpoint_new_task() (on the same task) to restore the
> - * checkpointed state back and the TM mode.
> + * transitions the CPU out of TM mode. Hence we need to make sure
> + * TIF_RESTORE_TM is set so restore_tm_state is called to restore the
> + * checkpointed state and back to TM mode.
> *
> * Can't pass dst because it isn't ready. Doesn't matter, passing
> * dst is only important for __switch_to()
^ permalink raw reply
* Re: [RFC PATCH 3/3] powerpc/mm/iommu: Allow migration of cma allocated pages during mm_iommu_get
From: David Gibson @ 2018-09-18 3:51 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: npiggin, benh, paulus, mpe, Alexey Kardashevskiy, linuxppc-dev
In-Reply-To: <20180903163733.27965-4-aneesh.kumar@linux.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 9885 bytes --]
On Mon, Sep 03, 2018 at 10:07:33PM +0530, Aneesh Kumar K.V wrote:
> Current code doesn't do page migration if the page allocated is a compound page.
> With HugeTLB migration support, we can end up allocating hugetlb pages from
> CMA region. Also THP pages can be allocated from CMA region. This patch updates
> the code to handle compound pages correctly.
>
> This add a new helper get_user_pages_cma_migrate. It does one get_user_pages
> with right count, instead of doing one get_user_pages per page. That avoids
> reading page table multiple times. The helper could possibly used by other
> subystem if we have more users.
>
> The patch also convert the hpas member of mm_iommu_table_group_mem_t to a union.
> We use the same storage location to store pointers to struct page. We cannot
> update alll the code path use struct page *, because we access hpas in real mode
> and we can't do that struct page * to pfn conversion in real mode.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
This approach doesn't seem quite right to me. It's specific to pages
mapped into the IOMMU. It's true that will address the obvious case
we have, of vfio-using guests fragmenting the CMA for other guests.
But AFAICT, fragmenting the CMA coud happen with *any* locked memory,
not just things that are IOMMU mapped for VFIO. So, for example a
guest not using vfio, but using -realtime mlock=on, or an unrelated
program using locked memory (e.g. gpg or something else that locks
memory for security reasons).
AFAICT this approach won't fix the problem for that case.
> ---
> arch/powerpc/mm/mmu_context_iommu.c | 195 ++++++++++++++++++----------
> 1 file changed, 123 insertions(+), 72 deletions(-)
>
> diff --git a/arch/powerpc/mm/mmu_context_iommu.c b/arch/powerpc/mm/mmu_context_iommu.c
> index f472965f7638..597b88a0abce 100644
> --- a/arch/powerpc/mm/mmu_context_iommu.c
> +++ b/arch/powerpc/mm/mmu_context_iommu.c
> @@ -20,6 +20,7 @@
> #include <linux/swap.h>
> #include <asm/mmu_context.h>
> #include <asm/pte-walk.h>
> +#include <linux/mm_inline.h>
>
> static DEFINE_MUTEX(mem_list_mutex);
>
> @@ -30,8 +31,18 @@ struct mm_iommu_table_group_mem_t {
> atomic64_t mapped;
> unsigned int pageshift;
> u64 ua; /* userspace address */
> - u64 entries; /* number of entries in hpas[] */
> - u64 *hpas; /* vmalloc'ed */
> + u64 entries; /* number of entries in hpages[] */
> + /*
> + * in mm_iommu_get we temporarily use this to store
> + * struct page address.
> + *
> + * We need to convert ua to hpa in real mode. Make it
> + * simpler by storing physicall address.
> + */
> + union {
> + struct page **hpages; /* vmalloc'ed */
> + phys_addr_t *hpas;
> + };
> };
>
> static long mm_iommu_adjust_locked_vm(struct mm_struct *mm,
> @@ -75,62 +86,112 @@ bool mm_iommu_preregistered(struct mm_struct *mm)
> EXPORT_SYMBOL_GPL(mm_iommu_preregistered);
>
> /*
> - * Taken from alloc_migrate_target with changes to remove CMA allocations
> + * Taken from alloc_migrate_target/alloc_migrate_huge_page with changes to remove
> + * CMA allocations
> + * Is this the right allocator for hugetlb?
> */
> struct page *new_iommu_non_cma_page(struct page *page, unsigned long private)
> {
> - gfp_t gfp_mask = GFP_USER;
> - struct page *new_page;
> + /* is this the right nid? */
> + int nid = numa_mem_id();
> + gfp_t gfp_mask = GFP_HIGHUSER;
>
> - if (PageCompound(page))
> - return NULL;
> + if (PageHuge(page)) {
>
> - if (PageHighMem(page))
> - gfp_mask |= __GFP_HIGHMEM;
> + struct hstate *h = page_hstate(page);
> + /*
> + * We don't want to dequeue from the pool because pool pages will
> + * mostly be from the CMA region.
> + */
> + return alloc_migrate_huge_page(h, gfp_mask, nid, NULL);
>
> - /*
> - * We don't want the allocation to force an OOM if possibe
> - */
> - new_page = alloc_page(gfp_mask | __GFP_NORETRY | __GFP_NOWARN);
> - return new_page;
> + } else if (PageTransHuge(page)) {
> + struct page *thp;
> + gfp_t thp_gfpmask = GFP_TRANSHUGE & ~__GFP_MOVABLE;
> +
> + thp = __alloc_pages_node(nid, thp_gfpmask, HPAGE_PMD_ORDER);
> + if (!thp)
> + return NULL;
> + prep_transhuge_page(thp);
> + return thp;
> + }
> + return __alloc_pages_node(nid, gfp_mask, 0);
> }
>
> -static int mm_iommu_move_page_from_cma(struct page *page)
> +int get_user_pages_cma_migrate(unsigned long start, int nr_pages, int write,
> + struct page **pages)
> {
> - int ret = 0;
> - LIST_HEAD(cma_migrate_pages);
> -
> - /* Ignore huge pages for now */
> - if (PageCompound(page))
> - return -EBUSY;
> -
> - lru_add_drain();
> - ret = isolate_lru_page(page);
> - if (ret)
> + int i, ret;
> + bool drain_allow = true;
> + bool migrate_allow = true;
> + LIST_HEAD(cma_page_list);
> +
> +get_user_again:
> + ret = get_user_pages_fast(start, nr_pages, write, pages);
> + if (ret <= 0)
> return ret;
>
> - list_add(&page->lru, &cma_migrate_pages);
> - put_page(page); /* Drop the gup reference */
> -
> - ret = migrate_pages(&cma_migrate_pages, new_iommu_non_cma_page,
> - NULL, 0, MIGRATE_SYNC, MR_CONTIG_RANGE);
> - if (ret) {
> - if (!list_empty(&cma_migrate_pages))
> - putback_movable_pages(&cma_migrate_pages);
> + for (i = 0; i < ret; ++i) {
> + /*
> + * If we get a page from the CMA zone, since we are going to
> + * be pinning these entries, we might as well move them out
> + * of the CMA zone if possible.
> + */
> + if (is_migrate_cma_page(pages[i]) && migrate_allow) {
> + if (PageHuge(pages[i]))
> + isolate_huge_page(pages[i], &cma_page_list);
> + else {
> + struct page *head = compound_head(pages[i]);
> +
> + if (!PageLRU(head) && drain_allow) {
> + lru_add_drain_all();
> + drain_allow = false;
> + }
> +
> + if (!isolate_lru_page(head)) {
> + list_add_tail(&head->lru, &cma_page_list);
> + mod_node_page_state(page_pgdat(head),
> + NR_ISOLATED_ANON +
> + page_is_file_cache(head),
> + hpage_nr_pages(head));
> + }
> + }
> + }
> }
> -
> - return 0;
> + if (!list_empty(&cma_page_list)) {
> + /*
> + * drop the above get_user_pages reference.
> + */
> + for (i = 0; i < ret; ++i)
> + put_page(pages[i]);
> +
> + if (migrate_pages(&cma_page_list, new_iommu_non_cma_page,
> + NULL, 0, MIGRATE_SYNC, MR_CONTIG_RANGE)) {
> + /*
> + * some of the pages failed migration. Do get_user_pages
> + * without migration.
> + */
> + migrate_allow = false;
> +
> + if (!list_empty(&cma_page_list))
> + putback_movable_pages(&cma_page_list);
> + }
> + /*
> + * We did migrate all the pages, Try to get the page references again
> + * migrating any new CMA pages which we failed to isolate earlier.
> + */
> + drain_allow = true;
> + goto get_user_again;
> + }
> + return ret;
> }
>
> long mm_iommu_get(struct mm_struct *mm, unsigned long ua, unsigned long entries,
> struct mm_iommu_table_group_mem_t **pmem)
> {
> struct mm_iommu_table_group_mem_t *mem;
> - long i, j, ret = 0, locked_entries = 0;
> + long i, ret = 0, locked_entries = 0;
> unsigned int pageshift;
> - unsigned long flags;
> - unsigned long cur_ua;
> - struct page *page = NULL;
>
> mutex_lock(&mem_list_mutex);
>
> @@ -177,47 +238,37 @@ long mm_iommu_get(struct mm_struct *mm, unsigned long ua, unsigned long entries,
> goto unlock_exit;
> }
>
> + ret = get_user_pages_cma_migrate(ua, entries, 1, mem->hpages);
> + if (ret != entries) {
> + /* free the reference taken */
> + for (i = 0; i < ret; i++)
> + put_page(mem->hpages[i]);
> +
> + vfree(mem->hpas);
> + kfree(mem);
> + ret = -EFAULT;
> + goto unlock_exit;
> + } else
> + ret = 0;
> +
> + pageshift = PAGE_SHIFT;
> for (i = 0; i < entries; ++i) {
> - cur_ua = ua + (i << PAGE_SHIFT);
> - if (1 != get_user_pages_fast(cur_ua,
> - 1/* pages */, 1/* iswrite */, &page)) {
> - ret = -EFAULT;
> - for (j = 0; j < i; ++j)
> - put_page(pfn_to_page(mem->hpas[j] >>
> - PAGE_SHIFT));
> - vfree(mem->hpas);
> - kfree(mem);
> - goto unlock_exit;
> - }
> + struct page *page = mem->hpages[i];
> /*
> - * If we get a page from the CMA zone, since we are going to
> - * be pinning these entries, we might as well move them out
> - * of the CMA zone if possible. NOTE: faulting in + migration
> - * can be expensive. Batching can be considered later
> + * Allow to use larger than 64k IOMMU pages. Only do that
> + * if we are backed by hugetlb.
> */
> - if (is_migrate_cma_page(page)) {
> - if (mm_iommu_move_page_from_cma(page))
> - goto populate;
> - if (1 != get_user_pages_fast(cur_ua,
> - 1/* pages */, 1/* iswrite */,
> - &page)) {
> - ret = -EFAULT;
> - for (j = 0; j < i; ++j)
> - put_page(pfn_to_page(mem->hpas[j] >>
> - PAGE_SHIFT));
> - vfree(mem->hpas);
> - kfree(mem);
> - goto unlock_exit;
> - }
> - }
> -populate:
> - pageshift = PAGE_SHIFT;
> - if (mem->pageshift > PAGE_SHIFT && PageHuge(page)) {
> + if ((mem->pageshift > PAGE_SHIFT) && PageHuge(page)) {
> struct page *head = compound_head(page);
> pageshift = compound_order(head) + PAGE_SHIFT;
> }
> mem->pageshift = min(mem->pageshift, pageshift);
> + /*
> + * We don't need struct page reference any more, switch
> + * physicall address.
> + */
> mem->hpas[i] = page_to_pfn(page) << PAGE_SHIFT;
> +
> }
>
> atomic64_set(&mem->mapped, 1);
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [RFC PATCH 07/11] powerpc/tm: Do not recheckpoint at sigreturn
From: Michael Neuling @ 2018-09-18 5:32 UTC (permalink / raw)
To: Breno Leitao, linuxppc-dev; +Cc: paulus, gromero, mpe, ldufour
In-Reply-To: <1536781219-13938-8-git-send-email-leitao@debian.org>
On Wed, 2018-09-12 at 16:40 -0300, Breno Leitao wrote:
> Do not recheckpoint at signal code return. Just make sure TIF_RESTORE_TM =
is
> set, which will restore on the exit to userspace by restore_tm_state.
Cool, but what about the same for reclaim? Why not avoid treclaim since it'=
s
done on entry?
Mikey
>=20
> All the FP and VEC lazy restore was already done by tm_reclaim_current(),
> where it checked if FP/VEC was set, and filled out the ckfp and ckvr
> registers area to the expected value.
>=20
> The current FP/VEC restoration is not necessary, since the transaction wi=
ll
> be aborted and the checkpointed values will be restore.
>=20
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
> arch/powerpc/kernel/signal_32.c | 23 +++--------------------
> arch/powerpc/kernel/signal_64.c | 15 ++-------------
> 2 files changed, 5 insertions(+), 33 deletions(-)
>=20
> diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal=
_32.c
> index e6474a45cef5..4a1b17409bf3 100644
> --- a/arch/powerpc/kernel/signal_32.c
> +++ b/arch/powerpc/kernel/signal_32.c
> @@ -850,28 +850,11 @@ static long restore_tm_user_regs(struct pt_regs *re=
gs,
> return 1;
> /* Pull in the MSR TM bits from the user context */
> regs->msr =3D (regs->msr & ~MSR_TS_MASK) | (msr_hi & MSR_TS_MASK);
> - /* Now, recheckpoint. This loads up all of the checkpointed (older)
> - * registers, including FP and V[S]Rs. After recheckpointing, the
> - * transactional versions should be loaded.
> - */
> - tm_enable();
> +
> /* Make sure the transaction is marked as failed */
> current->thread.tm_texasr |=3D TEXASR_FS;
> - /* This loads the checkpointed FP/VEC state, if used */
> - tm_recheckpoint(¤t->thread);
> -
> - /* This loads the speculative FP/VEC state, if used */
> - msr_check_and_set(msr & (MSR_FP | MSR_VEC));
> - if (msr & MSR_FP) {
> - load_fp_state(¤t->thread.fp_state);
> - regs->msr |=3D (MSR_FP | current->thread.fpexc_mode);
> - }
> -#ifdef CONFIG_ALTIVEC
> - if (msr & MSR_VEC) {
> - load_vr_state(¤t->thread.vr_state);
> - regs->msr |=3D MSR_VEC;
> - }
> -#endif
> + /* Make sure restore_tm_state will be called */
> + set_thread_flag(TIF_RESTORE_TM);
> =20
> return 0;
> }
> diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal=
_64.c
> index 83d51bf586c7..32402aa23a5e 100644
> --- a/arch/powerpc/kernel/signal_64.c
> +++ b/arch/powerpc/kernel/signal_64.c
> @@ -569,21 +569,10 @@ static long restore_tm_sigcontexts(struct task_stru=
ct
> *tsk,
> }
> }
> #endif
> - tm_enable();
> /* Make sure the transaction is marked as failed */
> tsk->thread.tm_texasr |=3D TEXASR_FS;
> - /* This loads the checkpointed FP/VEC state, if used */
> - tm_recheckpoint(&tsk->thread);
> -
> - msr_check_and_set(msr & (MSR_FP | MSR_VEC));
> - if (msr & MSR_FP) {
> - load_fp_state(&tsk->thread.fp_state);
> - regs->msr |=3D (MSR_FP | tsk->thread.fpexc_mode);
> - }
> - if (msr & MSR_VEC) {
> - load_vr_state(&tsk->thread.vr_state);
> - regs->msr |=3D MSR_VEC;
> - }
> + /* Guarantee that restore_tm_state() will be called */
> + set_thread_flag(TIF_RESTORE_TM);
> =20
> return err;
> }
^ permalink raw reply
* Re: [RFC PATCH 08/11] powerpc/tm: Do not reclaim on ptrace
From: Michael Neuling @ 2018-09-18 5:36 UTC (permalink / raw)
To: Breno Leitao, linuxppc-dev; +Cc: paulus, gromero, mpe, ldufour
In-Reply-To: <1536781219-13938-9-git-send-email-leitao@debian.org>
On Wed, 2018-09-12 at 16:40 -0300, Breno Leitao wrote:
> Make sure that we are not suspended on ptrace and that the registers were
> already reclaimed.
>=20
> Since the data was already reclaimed, there is nothing to be done here
> except to restore the SPRs.
>=20
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
> arch/powerpc/kernel/ptrace.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>=20
> diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
> index 9667666eb18e..cf6ee9154b11 100644
> --- a/arch/powerpc/kernel/ptrace.c
> +++ b/arch/powerpc/kernel/ptrace.c
> @@ -136,12 +136,10 @@ static void flush_tmregs_to_thread(struct task_stru=
ct
> *tsk)
> if ((!cpu_has_feature(CPU_FTR_TM)) || (tsk !=3D current))
> return;
> =20
> - if (MSR_TM_SUSPENDED(mfmsr())) {
> - tm_reclaim_current(TM_CAUSE_SIGNAL);
> - } else {
> - tm_enable();
> - tm_save_sprs(&(tsk->thread));
> - }
> + WARN_ON(MSR_TM_SUSPENDED(mfmsr()));
> +
> + tm_enable();
> + tm_save_sprs(&(tsk->thread));
Do we need to check if TM was enabled in the task before saving the TM SPRs=
?
What happens if TM was lazily off and hence we had someone else's TM SPRs i=
n the
CPU currently? Wouldn't this flush the wrong values to the task_struct?
I think we need to check the processes MSR before doing this.
Mikey
> }
> #else
> static inline void flush_tmregs_to_thread(struct task_struct *tsk) { }
^ permalink raw reply
* Re: [RFC PATCH 09/11] powerpc/tm: Do not restore default DSCR
From: Michael Neuling @ 2018-09-18 5:41 UTC (permalink / raw)
To: Breno Leitao, linuxppc-dev; +Cc: paulus, gromero, mpe, ldufour
In-Reply-To: <1536781219-13938-10-git-send-email-leitao@debian.org>
On Wed, 2018-09-12 at 16:40 -0300, Breno Leitao wrote:
> In the previous TM code, trecheckpoint was being executed in the middle o=
f
> an exception, thus, DSCR was being restored to default kernel DSCR value
> after trecheckpoint was done.
>=20
> With this current patchset, trecheckpoint is executed just before getting
> to userspace, at ret_from_except_lite, for example. Thus, we do not need =
to
> set default kernel DSCR value anymore, as we are leaving kernel space. I=
t
> is OK to keep the checkpointed DSCR value into the live SPR, mainly becau=
se
> the transaction is doomed and it will fail soon (after RFID),=20
What if we are going back to a suspended transaction? It will remain live =
until
userspace does a tresume
> so,
> continuing with the pre-checkpointed DSCR value is what seems correct.
Reading this description suggests this patch isn't really needed. Right?
Mikey
> That said, we must set the DSCR value that will be used in userspace now.
> Current trecheckpoint() function sets it to the pre-checkpointed value
> prior to lines being changed in this patch, so, removing these lines woul=
d
> keep the pre-checkpointed values.
>=20
> Important to say that we do not need to do the same thing with tm_reclaim=
,
> since it already set the DSCR to the default value, after TRECLAIM is
> called, in the following lines:
>=20
> /* Load CPU's default DSCR */
> ld r0, PACA_DSCR_DEFAULT(r13)
> mtspr SPRN_DSCR, r0
>=20
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
> arch/powerpc/kernel/tm.S | 4 ----
> 1 file changed, 4 deletions(-)
>=20
> diff --git a/arch/powerpc/kernel/tm.S b/arch/powerpc/kernel/tm.S
> index 6bffbc5affe7..5427eda69846 100644
> --- a/arch/powerpc/kernel/tm.S
> +++ b/arch/powerpc/kernel/tm.S
> @@ -493,10 +493,6 @@ restore_gprs:
> mtlr r0
> ld r2, STK_GOT(r1)
> =20
> - /* Load CPU's default DSCR */
> - ld r0, PACA_DSCR_DEFAULT(r13)
> - mtspr SPRN_DSCR, r0
> -
> blr
> =20
> /* ****************************************************************** *=
/
^ permalink raw reply
* [PATCH] powerpc/makefile: remove check on obsolete GCC versions
From: Christophe Leroy @ 2018-09-18 5:43 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linux-kernel, linuxppc-dev
Since commit cafa0010cd51 ("Raise the minimum required gcc version
to 4.6"), it is not possible to build kernel with GCC lower than 4.6
This patch removes checkbin tests addressing older versions of GCC.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/Makefile | 28 ----------------------------
1 file changed, 28 deletions(-)
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index 8397c7bd5880..b33083bad840 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -405,35 +405,7 @@ archprepare: checkbin
TOUT := .tmp_gas_check
# Check gcc and binutils versions:
-# - gcc-3.4 and binutils-2.14 are a fatal combination
-# - Require gcc 4.0 or above on 64-bit
-# - gcc-4.2.0 has issues compiling modules on 64-bit
checkbin:
- @if test "$(cc-name)" != "clang" \
- && test "$(cc-version)" = "0304" ; then \
- if ! /bin/echo mftb 5 | $(AS) -v -mppc -many -o $(TOUT) >/dev/null 2>&1 ; then \
- echo -n '*** ${VERSION}.${PATCHLEVEL} kernels no longer build '; \
- echo 'correctly with gcc-3.4 and your version of binutils.'; \
- echo '*** Please upgrade your binutils or downgrade your gcc'; \
- false; \
- fi ; \
- fi
- @if test "$(cc-name)" != "clang" \
- && test "$(cc-version)" -lt "0400" \
- && test "x${CONFIG_PPC64}" = "xy" ; then \
- echo -n "Sorry, GCC v4.0 or above is required to build " ; \
- echo "the 64-bit powerpc kernel." ; \
- false ; \
- fi
- @if test "$(cc-name)" != "clang" \
- && test "$(cc-fullversion)" = "040200" \
- && test "x${CONFIG_MODULES}${CONFIG_PPC64}" = "xyy" ; then \
- echo -n '*** GCC-4.2.0 cannot compile the 64-bit powerpc ' ; \
- echo 'kernel with modules enabled.' ; \
- echo -n '*** Please use a different GCC version or ' ; \
- echo 'disable kernel modules' ; \
- false ; \
- fi
@if test "x${CONFIG_CPU_LITTLE_ENDIAN}" = "xy" \
&& $(LD) --version | head -1 | grep ' 2\.24$$' >/dev/null ; then \
echo -n '*** binutils 2.24 miscompiles weak symbols ' ; \
--
2.13.3
^ permalink raw reply related
* Re: [PATCH] powerpc/makefile: remove check on obsolete GCC versions
From: Joel Stanley @ 2018-09-18 5:48 UTC (permalink / raw)
To: christophe.leroy
Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
linuxppc-dev, Linux Kernel Mailing List
In-Reply-To: <82bbb913ffd8bdc040a012152ee968cd0c6ccf4e.1537218487.git.christophe.leroy@c-s.fr>
Hey Christophe,
On Tue, 18 Sep 2018 at 15:13, Christophe Leroy <christophe.leroy@c-s.fr> wrote:
>
> Since commit cafa0010cd51 ("Raise the minimum required gcc version
> to 4.6"), it is not possible to build kernel with GCC lower than 4.6
>
> This patch removes checkbin tests addressing older versions of GCC.
This is the same as Nick's patch:
https://patchwork.ozlabs.org/patch/969624/
Cheers,
Joel
^ permalink raw reply
* Re: [RFC PATCH 10/11] powerpc/tm: Set failure summary
From: Michael Neuling @ 2018-09-18 5:50 UTC (permalink / raw)
To: Breno Leitao, linuxppc-dev; +Cc: paulus, gromero, mpe, ldufour
In-Reply-To: <1536781219-13938-11-git-send-email-leitao@debian.org>
On Wed, 2018-09-12 at 16:40 -0300, Breno Leitao wrote:
> Since the transaction will be doomed with treckpt., the TEXASR[FS]
> should be set, to reflect that the transaction is a failure. This patch
> ensures it before recheckpointing, and remove changes from other places
> that were calling recheckpoint.
TEXASR[FS] should be set by the reclaim. I don't know why you'd need to set=
this
explicitly in process.c. The only case is when the user supplies a bad sign=
al
context, but we should check that in the signals code, not process.c
Hence I think this patch is wrong.
Also, according to the architecture, TEXASR[FS] HAS TO BE SET on trecheckpo=
int
otherwise you'll get a TM Bad Thing. You should say that rather than sugges=
ting
it's because the transaction is doomed. It's illegal to not do it. That's w=
hy we
have this check in arch/powerpc/kernel/tm.S.
/* Do final sanity check on TEXASR to make sure FS is set. Do this
* here before we load up the userspace r1 so any bugs we hit will get
* a call chain */
mfspr r5, SPRN_TEXASR
srdi r5, r5, 16
li r6, (TEXASR_FS)@h
and r6, r6, r5
1: tdeqi r6, 0
EMIT_BUG_ENTRY 1b,__FILE__,__LINE__,0
Mikey
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
> arch/powerpc/kernel/process.c | 6 ++++++
> arch/powerpc/kernel/signal_32.c | 2 --
> arch/powerpc/kernel/signal_64.c | 2 --
> 3 files changed, 6 insertions(+), 4 deletions(-)
>=20
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.=
c
> index 5cace1b744b1..77725b2e4dc1 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -937,6 +937,12 @@ void tm_recheckpoint(struct thread_struct *thread)
> local_irq_save(flags);
> hard_irq_disable();
> =20
> + /*
> + * Make sure the failure summary is set, since the transaction will be
> + * doomed.
> + */
> + thread->tm_texasr |=3D TEXASR_FS;
> +
> /* The TM SPRs are restored here, so that TEXASR.FS can be set
> * before the trecheckpoint and no explosion occurs.
> */
> diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal=
_32.c
> index 4a1b17409bf3..96956d50538e 100644
> --- a/arch/powerpc/kernel/signal_32.c
> +++ b/arch/powerpc/kernel/signal_32.c
> @@ -851,8 +851,6 @@ static long restore_tm_user_regs(struct pt_regs *regs=
,
> /* Pull in the MSR TM bits from the user context */
> regs->msr =3D (regs->msr & ~MSR_TS_MASK) | (msr_hi & MSR_TS_MASK);
> =20
> - /* Make sure the transaction is marked as failed */
> - current->thread.tm_texasr |=3D TEXASR_FS;
> /* Make sure restore_tm_state will be called */
> set_thread_flag(TIF_RESTORE_TM);
> =20
> diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal=
_64.c
> index 32402aa23a5e..c84501711b14 100644
> --- a/arch/powerpc/kernel/signal_64.c
> +++ b/arch/powerpc/kernel/signal_64.c
> @@ -569,8 +569,6 @@ static long restore_tm_sigcontexts(struct task_struct
> *tsk,
> }
> }
> #endif
> - /* Make sure the transaction is marked as failed */
> - tsk->thread.tm_texasr |=3D TEXASR_FS;
> /* Guarantee that restore_tm_state() will be called */
> set_thread_flag(TIF_RESTORE_TM);
> =20
^ permalink raw reply
* Re: [PATCH] powerpc/makefile: remove check on obsolete GCC versions
From: Christophe LEROY @ 2018-09-18 5:52 UTC (permalink / raw)
To: Joel Stanley
Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
linuxppc-dev, Linux Kernel Mailing List
In-Reply-To: <CACPK8XdeT_FF_PHjqOh3e18jDhsFOdECVDd+Z2vWt8pSk5damA@mail.gmail.com>
Le 18/09/2018 à 07:48, Joel Stanley a écrit :
> Hey Christophe,
>
> On Tue, 18 Sep 2018 at 15:13, Christophe Leroy <christophe.leroy@c-s.fr> wrote:
>>
>> Since commit cafa0010cd51 ("Raise the minimum required gcc version
>> to 4.6"), it is not possible to build kernel with GCC lower than 4.6
>>
>> This patch removes checkbin tests addressing older versions of GCC.
>
> This is the same as Nick's patch:
>
> https://patchwork.ozlabs.org/patch/969624/
>
Oops, thanks, I missed that.
And even before Nick's, there is this one
https://patchwork.ozlabs.org/patch/962319/
So I missed twice :(
Christophe
^ permalink raw reply
* [PATCH net-next] net: ibm: fix return type of ndo_start_xmit function
From: YueHaibing @ 2018-09-18 6:35 UTC (permalink / raw)
To: davem, dougmill, benh, paulus, mpe, tlfalcon, jallen, ivan,
chunkeey, keescook
Cc: linux-kernel, netdev, linuxppc-dev, YueHaibing
The method ndo_start_xmit() is defined as returning an 'netdev_tx_t',
which is a typedef for an enum type, so make sure the implementation in
this driver has returns 'netdev_tx_t' value, and change the function
return type to netdev_tx_t.
Found by coccinelle.
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/net/ethernet/ibm/ehea/ehea_main.c | 2 +-
drivers/net/ethernet/ibm/emac/core.c | 7 ++++---
drivers/net/ethernet/ibm/ibmvnic.c | 4 ++--
3 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ehea/ehea_main.c b/drivers/net/ethernet/ibm/ehea/ehea_main.c
index ba580bf..88128d3 100644
--- a/drivers/net/ethernet/ibm/ehea/ehea_main.c
+++ b/drivers/net/ethernet/ibm/ehea/ehea_main.c
@@ -2038,7 +2038,7 @@ static void ehea_xmit3(struct sk_buff *skb, struct net_device *dev,
dev_consume_skb_any(skb);
}
-static int ehea_start_xmit(struct sk_buff *skb, struct net_device *dev)
+static netdev_tx_t ehea_start_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct ehea_port *port = netdev_priv(dev);
struct ehea_swqe *swqe;
diff --git a/drivers/net/ethernet/ibm/emac/core.c b/drivers/net/ethernet/ibm/emac/core.c
index 7410a1d..5107c94 100644
--- a/drivers/net/ethernet/ibm/emac/core.c
+++ b/drivers/net/ethernet/ibm/emac/core.c
@@ -1409,7 +1409,7 @@ static inline u16 emac_tx_csum(struct emac_instance *dev,
return 0;
}
-static inline int emac_xmit_finish(struct emac_instance *dev, int len)
+static inline netdev_tx_t emac_xmit_finish(struct emac_instance *dev, int len)
{
struct emac_regs __iomem *p = dev->emacp;
struct net_device *ndev = dev->ndev;
@@ -1436,7 +1436,7 @@ static inline int emac_xmit_finish(struct emac_instance *dev, int len)
}
/* Tx lock BH */
-static int emac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
+static netdev_tx_t emac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
{
struct emac_instance *dev = netdev_priv(ndev);
unsigned int len = skb->len;
@@ -1494,7 +1494,8 @@ static inline int emac_xmit_split(struct emac_instance *dev, int slot,
}
/* Tx lock BH disabled (SG version for TAH equipped EMACs) */
-static int emac_start_xmit_sg(struct sk_buff *skb, struct net_device *ndev)
+static netdev_tx_t
+emac_start_xmit_sg(struct sk_buff *skb, struct net_device *ndev)
{
struct emac_instance *dev = netdev_priv(ndev);
int nr_frags = skb_shinfo(skb)->nr_frags;
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 4f0daf6..a8369ad 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1428,7 +1428,7 @@ static int ibmvnic_xmit_workarounds(struct sk_buff *skb,
return 0;
}
-static int ibmvnic_xmit(struct sk_buff *skb, struct net_device *netdev)
+static netdev_tx_t ibmvnic_xmit(struct sk_buff *skb, struct net_device *netdev)
{
struct ibmvnic_adapter *adapter = netdev_priv(netdev);
int queue_num = skb_get_queue_mapping(skb);
@@ -1452,7 +1452,7 @@ static int ibmvnic_xmit(struct sk_buff *skb, struct net_device *netdev)
u64 *handle_array;
int index = 0;
u8 proto = 0;
- int ret = 0;
+ netdev_tx_t ret = NETDEV_TX_OK;
if (adapter->resetting) {
if (!netif_subqueue_stopped(netdev, skb))
--
1.8.3.1
^ permalink raw reply related
* Re: [RFC PATCH 11/11] selftests/powerpc: Adapt the test
From: Michael Neuling @ 2018-09-18 6:36 UTC (permalink / raw)
To: Breno Leitao, linuxppc-dev; +Cc: paulus, gromero, mpe, ldufour
In-Reply-To: <1536781219-13938-12-git-send-email-leitao@debian.org>
On Wed, 2018-09-12 at 16:40 -0300, Breno Leitao wrote:
> The Documentation/powerpc/transactional_memory.txt says:
>=20
> "Syscalls made from within a suspended transaction are performed as norm=
al
> and the transaction is not explicitly doomed by the kernel. However,
> what the kernel does to perform the syscall may result in the transacti=
on
> being doomed by the hardware."
>=20
> With this new TM mechanism, the syscall will continue to be executed if t=
he
> syscall happens on a suspended syscall, but, the syscall will *fail* if t=
he
> transaction is still active during the syscall invocation.
Not sure I get this. This doesn't seem any different to before.
An active (not suspended) transaction *will* result in the syscall failing =
and
the transaction being doomed. =20
A syscall in a suspended transaction should succeed and the transaction.
You might need to clean up the language. I try to use:
Active =3D=3D transactional but not suspended (ie MSR[TS] =3D T)
Suspended =3D=3D suspended (ie MSR [TS] =3D S)
Doomed =3D=3D transaction to be rolled back at next opportinity (ie tche=
ck returns doomed)
(note: the kernel MSR_TM_ACTIVE() macro is not consistent with this since i=
t's
MSR[TS] =3D=3D S or T).
> On the syscall path, if the transaction is active and not suspended, it
> will call TM_KERNEL_ENTRY which will reclaim and recheckpoint the
> transaction, thus, dooming the transaction on userspace return, with
> failure code TM_CAUSE_SYSCALL.
But the test below is on a suspend transaction?
> This new model will break part of this test, but I understand that that t=
he
> documentation above didn't guarantee that the syscall would succeed, and =
it
> will never succeed anymore now on.
The syscall should pass in suspend (modulo the normal syscall checks). The
transaction may fail as a result.
> In fact, glibc is calling 'tabort' before every syscalls, thus, any sysca=
ll
> called through glibc from inside a transaction will be doomed anyhow.
>=20
> This patch updates the test case to not assume that a syscall inside a
> active transaction will succeed, because it will not anymore.
>=20
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
> tools/testing/selftests/powerpc/tm/tm-syscall.c | 6 ------
> 1 file changed, 6 deletions(-)
>=20
> diff --git a/tools/testing/selftests/powerpc/tm/tm-syscall.c
> b/tools/testing/selftests/powerpc/tm/tm-syscall.c
> index 454b965a2db3..1439a87eba3a 100644
> --- a/tools/testing/selftests/powerpc/tm/tm-syscall.c
> +++ b/tools/testing/selftests/powerpc/tm/tm-syscall.c
> @@ -78,12 +78,6 @@ int tm_syscall(void)
> timeradd(&end, &now, &end);
> =20
> for (count =3D 0; timercmp(&now, &end, <); count++) {
> - /*
> - * Test a syscall within a suspended transaction and verify
> - * that it succeeds.
> - */
> - FAIL_IF(getppid_tm(true) =3D=3D -1); /* Should succeed. */
> -
> /*
> * Test a syscall within an active transaction and verify that
> * it fails with the correct failure code.
^ permalink raw reply
* RE: [PATCH v11 00/26] Speculative page faults
From: Song, HaiyanX @ 2018-09-18 6:42 UTC (permalink / raw)
To: Laurent Dufour
Cc: akpm@linux-foundation.org, mhocko@kernel.org,
peterz@infradead.org, kirill@shutemov.name, ak@linux.intel.com,
dave@stgolabs.net, jack@suse.cz, Matthew Wilcox,
khandual@linux.vnet.ibm.com, aneesh.kumar@linux.vnet.ibm.com,
benh@kernel.crashing.org, mpe@ellerman.id.au, paulus@samba.org,
Thomas Gleixner, Ingo Molnar, hpa@zytor.com, Will Deacon,
Sergey Senozhatsky, sergey.senozhatsky.work@gmail.com,
Andrea Arcangeli, Alexei Starovoitov, Wang, Kemi, Daniel Jordan,
David Rientjes, Jerome Glisse, Ganesh Mahendran, Minchan Kim,
Punit Agrawal, vinayak menon, Yang Shi,
linux-kernel@vger.kernel.org, linux-mm@kvack.org,
haren@linux.vnet.ibm.com, npiggin@gmail.com,
bsingharora@gmail.com, paulmck@linux.vnet.ibm.com, Tim Chen,
linuxppc-dev@lists.ozlabs.org, x86@kernel.org
In-Reply-To: <f9bc4701-ef52-d2de-0d72-4b29736cb1eb@linux.vnet.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 46630 bytes --]
Hi Laurent,
I am sorry for replying you so late.
The previous LKP test for this case are running on the same Intel skylake 4s platform, but it need maintain recently.
So I changed to another test box to run the page_fault3 test case, it is Intel skylake 2s platform (nr_cpu: 104, memory: 64G).
I applied your patch to the SPF kernel (commit : a7a8993bfe3ccb54ad468b9f1799649e4ad1ff12), then triggered below 2 cases test.
a) Turn on the SPF handler by below command, then run page_fault3-thp-always test.
echo 1 > /proc/sys/vm/speculative_page_fault
b) Turn off the SPF handler by below command, then run page_fault3-thp-always test.
echo 0 > /proc/sys/vm/speculative_page_fault
Every test run 3 times, and then get test result and capture perf data.
Here is average result for will-it-scale.per_thread_ops:
SPF_turn_off SPF_turn_on
page_fault3-THP-Alwasys.will-it-scale.per_thread_ops 31963 26285
Best regards,
Haiyan Song
________________________________________
From: owner-linux-mm@kvack.org [owner-linux-mm@kvack.org] on behalf of Laurent Dufour [ldufour@linux.vnet.ibm.com]
Sent: Wednesday, August 22, 2018 10:23 PM
To: Song, HaiyanX
Cc: akpm@linux-foundation.org; mhocko@kernel.org; peterz@infradead.org; kirill@shutemov.name; ak@linux.intel.com; dave@stgolabs.net; jack@suse.cz; Matthew Wilcox; khandual@linux.vnet.ibm.com; aneesh.kumar@linux.vnet.ibm.com; benh@kernel.crashing.org; mpe@ellerman.id.au; paulus@samba.org; Thomas Gleixner; Ingo Molnar; hpa@zytor.com; Will Deacon; Sergey Senozhatsky; sergey.senozhatsky.work@gmail.com; Andrea Arcangeli; Alexei Starovoitov; Wang, Kemi; Daniel Jordan; David Rientjes; Jerome Glisse; Ganesh Mahendran; Minchan Kim; Punit Agrawal; vinayak menon; Yang Shi; linux-kernel@vger.kernel.org; linux-mm@kvack.org; haren@linux.vnet.ibm.com; npiggin@gmail.com; bsingharora@gmail.com; paulmck@linux.vnet.ibm.com; Tim Chen; linuxppc-dev@lists.ozlabs.org; x86@kernel.org
Subject: Re: [PATCH v11 00/26] Speculative page faults
On 03/08/2018 08:36, Song, HaiyanX wrote:
> Hi Laurent,
Hi Haiyan,
Sorry for the late answer, I was off a couple of days.
>
> Thanks for your analysis for the last perf results.
> Your mentioned ," the major differences at the head of the perf report is the 92% testcase which is weirdly not reported
> on the head side", which is a bug of 0-day,and it caused the item is not counted in perf.
>
> I've triggered the test page_fault2 and page_fault3 again only with thread mode of will-it-scale on 0-day (on the same test box,every case tested 3 times).
> I checked the perf report have no above mentioned problem.
>
> I have compared them, found some items have difference, such as below case:
> page_fault2-thp-always: handle_mm_fault, base: 45.22% head: 29.41%
> page_fault3-thp-always: handle_mm_fault, base: 22.95% head: 14.15%
These would mean that the system spends lees time running handle_mm_fault()
when SPF is in the picture in this 2 cases which is good. This should lead to
better results with the SPF series, and I can't find any values higher on the
head side.
>
> So i attached the perf result in mail again, could your have a look again for checking the difference between base and head commit.
I took a close look to all the perf result you sent, but I can't identify any
major difference. But the compiler optimization is getting rid of the
handle_pte_fault() symbol on the base kernel which add complexity to check the
differences.
To get rid of that, I'm proposing that you applied the attached patch to the
spf kernel. This patch is allowing to turn on/off the SPF handler through
/proc/sys/vm/speculative_page_fault.
This should ease the testing by limiting the reboot and avoid kernel's symbols
mismatch. Obviously there is still a small overhead due to the check but it
should not be viewable.
With this patch applied you can simply run
echo 1 > /proc/sys/vm/speculative_page_fault
to run a test with the speculative page fault handler activated. Or run
echo 0 > /proc/sys/vm/speculative_page_fault
to run a test without it.
I'm really sorry to asking that again, but could please run the test
page_fault3_base_THP-Always with and without SPF and capture the perf output.
I think we should focus on that test which showed the biggest regression.
Thanks,
Laurent.
>
> Thanks,
> Haiyan, Song
>
> ________________________________________
> From: owner-linux-mm@kvack.org [owner-linux-mm@kvack.org] on behalf of Laurent Dufour [ldufour@linux.vnet.ibm.com]
> Sent: Tuesday, July 17, 2018 5:36 PM
> To: Song, HaiyanX
> Cc: akpm@linux-foundation.org; mhocko@kernel.org; peterz@infradead.org; kirill@shutemov.name; ak@linux.intel.com; dave@stgolabs.net; jack@suse.cz; Matthew Wilcox; khandual@linux.vnet.ibm.com; aneesh.kumar@linux.vnet.ibm.com; benh@kernel.crashing.org; mpe@ellerman.id.au; paulus@samba.org; Thomas Gleixner; Ingo Molnar; hpa@zytor.com; Will Deacon; Sergey Senozhatsky; sergey.senozhatsky.work@gmail.com; Andrea Arcangeli; Alexei Starovoitov; Wang, Kemi; Daniel Jordan; David Rientjes; Jerome Glisse; Ganesh Mahendran; Minchan Kim; Punit Agrawal; vinayak menon; Yang Shi; linux-kernel@vger.kernel.org; linux-mm@kvack.org; haren@linux.vnet.ibm.com; npiggin@gmail.com; bsingharora@gmail.com; paulmck@linux.vnet.ibm.com; Tim Chen; linuxppc-dev@lists.ozlabs.org; x86@kernel.org
> Subject: Re: [PATCH v11 00/26] Speculative page faults
>
> On 13/07/2018 05:56, Song, HaiyanX wrote:
>> Hi Laurent,
>
> Hi Haiyan,
>
> Thanks a lot for sharing this perf reports.
>
> I looked at them closely, and I've to admit that I was not able to found a
> major difference between the base and the head report, except that
> handle_pte_fault() is no more in-lined in the head one.
>
> As expected, __handle_speculative_fault() is never traced since these tests are
> dealing with file mapping, not handled in the speculative way.
>
> When running these test did you seen a major differences in the test's result
> between base and head ?
>
> From the number of cycles counted, the biggest difference is page_fault3 when
> run with the THP enabled:
> BASE HEAD Delta
> page_fault2_base_thp_never 1142252426747 1065866197589 -6.69%
> page_fault2_base_THP-Alwasys 1124844374523 1076312228927 -4.31%
> page_fault3_base_thp_never 1099387298152 1134118402345 3.16%
> page_fault3_base_THP-Always 1059370178101 853985561949 -19.39%
>
>
> The very weird thing is the difference of the delta cycles reported between
> thp never and thp always, because the speculative way is aborted when checking
> for the vma->ops field, which is the same in both case, and the thp is never
> checked. So there is no code covering differnce, on the speculative path,
> between these 2 cases. This leads me to think that there are other interactions
> interfering in the measure.
>
> Looking at the perf-profile_page_fault3_*_THP-Always, the major differences at
> the head of the perf report is the 92% testcase which is weirdly not reported
> on the head side :
> 92.02% 22.33% page_fault3_processes [.] testcase
> 92.02% testcase
>
> Then the base reported 37.67% for __do_page_fault() where the head reported
> 48.41%, but the only difference in this function, between base and head, is the
> call to handle_speculative_fault(). But this is a macro checking for the fault
> flags, and mm->users and then calling __handle_speculative_fault() if needed.
> So this can't explain this difference, except if __handle_speculative_fault()
> is inlined in __do_page_fault().
> Is this the case on your build ?
>
> Haiyan, do you still have the output of the test to check those numbers too ?
>
> Cheers,
> Laurent
>
>> I attached the perf-profile.gz file for case page_fault2 and page_fault3. These files were captured during test the related test case.
>> Please help to check on these data if it can help you to find the higher change. Thanks.
>>
>> File name perf-profile_page_fault2_head_THP-Always.gz, means the perf-profile result get from page_fault2
>> tested for head commit (a7a8993bfe3ccb54ad468b9f1799649e4ad1ff12) with THP_always configuration.
>>
>> Best regards,
>> Haiyan Song
>>
>> ________________________________________
>> From: owner-linux-mm@kvack.org [owner-linux-mm@kvack.org] on behalf of Laurent Dufour [ldufour@linux.vnet.ibm.com]
>> Sent: Thursday, July 12, 2018 1:05 AM
>> To: Song, HaiyanX
>> Cc: akpm@linux-foundation.org; mhocko@kernel.org; peterz@infradead.org; kirill@shutemov.name; ak@linux.intel.com; dave@stgolabs.net; jack@suse.cz; Matthew Wilcox; khandual@linux.vnet.ibm.com; aneesh.kumar@linux.vnet.ibm.com; benh@kernel.crashing.org; mpe@ellerman.id.au; paulus@samba.org; Thomas Gleixner; Ingo Molnar; hpa@zytor.com; Will Deacon; Sergey Senozhatsky; sergey.senozhatsky.work@gmail.com; Andrea Arcangeli; Alexei Starovoitov; Wang, Kemi; Daniel Jordan; David Rientjes; Jerome Glisse; Ganesh Mahendran; Minchan Kim; Punit Agrawal; vinayak menon; Yang Shi; linux-kernel@vger.kernel.org; linux-mm@kvack.org; haren@linux.vnet.ibm.com; npiggin@gmail.com; bsingharora@gmail.com; paulmck@linux.vnet.ibm.com; Tim Chen; linuxppc-dev@lists.ozlabs.org; x86@kernel.org
>> Subject: Re: [PATCH v11 00/26] Speculative page faults
>>
>> Hi Haiyan,
>>
>> Do you get a chance to capture some performance cycles on your system ?
>> I still can't get these numbers on my hardware.
>>
>> Thanks,
>> Laurent.
>>
>> On 04/07/2018 09:51, Laurent Dufour wrote:
>>> On 04/07/2018 05:23, Song, HaiyanX wrote:
>>>> Hi Laurent,
>>>>
>>>>
>>>> For the test result on Intel 4s skylake platform (192 CPUs, 768G Memory), the below test cases all were run 3 times.
>>>> I check the test results, only page_fault3_thread/enable THP have 6% stddev for head commit, other tests have lower stddev.
>>>
>>> Repeating the test only 3 times seems a bit too low to me.
>>>
>>> I'll focus on the higher change for the moment, but I don't have access to such
>>> a hardware.
>>>
>>> Is possible to provide a diff between base and SPF of the performance cycles
>>> measured when running page_fault3 and page_fault2 when the 20% change is detected.
>>>
>>> Please stay focus on the test case process to see exactly where the series is
>>> impacting.
>>>
>>> Thanks,
>>> Laurent.
>>>
>>>>
>>>> And I did not find other high variation on test case result.
>>>>
>>>> a). Enable THP
>>>> testcase base stddev change head stddev metric
>>>> page_fault3/enable THP 10519 ± 3% -20.5% 8368 ±6% will-it-scale.per_thread_ops
>>>> page_fault2/enalbe THP 8281 ± 2% -18.8% 6728 will-it-scale.per_thread_ops
>>>> brk1/eanble THP 998475 -2.2% 976893 will-it-scale.per_process_ops
>>>> context_switch1/enable THP 223910 -1.3% 220930 will-it-scale.per_process_ops
>>>> context_switch1/enable THP 233722 -1.0% 231288 will-it-scale.per_thread_ops
>>>>
>>>> b). Disable THP
>>>> page_fault3/disable THP 10856 -23.1% 8344 will-it-scale.per_thread_ops
>>>> page_fault2/disable THP 8147 -18.8% 6613 will-it-scale.per_thread_ops
>>>> brk1/disable THP 957 -7.9% 881 will-it-scale.per_thread_ops
>>>> context_switch1/disable THP 237006 -2.2% 231907 will-it-scale.per_thread_ops
>>>> brk1/disable THP 997317 -2.0% 977778 will-it-scale.per_process_ops
>>>> page_fault3/disable THP 467454 -1.8% 459251 will-it-scale.per_process_ops
>>>> context_switch1/disable THP 224431 -1.3% 221567 will-it-scale.per_process_ops
>>>>
>>>>
>>>> Best regards,
>>>> Haiyan Song
>>>> ________________________________________
>>>> From: Laurent Dufour [ldufour@linux.vnet.ibm.com]
>>>> Sent: Monday, July 02, 2018 4:59 PM
>>>> To: Song, HaiyanX
>>>> Cc: akpm@linux-foundation.org; mhocko@kernel.org; peterz@infradead.org; kirill@shutemov.name; ak@linux.intel.com; dave@stgolabs.net; jack@suse.cz; Matthew Wilcox; khandual@linux.vnet.ibm.com; aneesh.kumar@linux.vnet.ibm.com; benh@kernel.crashing.org; mpe@ellerman.id.au; paulus@samba.org; Thomas Gleixner; Ingo Molnar; hpa@zytor.com; Will Deacon; Sergey Senozhatsky; sergey.senozhatsky.work@gmail.com; Andrea Arcangeli; Alexei Starovoitov; Wang, Kemi; Daniel Jordan; David Rientjes; Jerome Glisse; Ganesh Mahendran; Minchan Kim; Punit Agrawal; vinayak menon; Yang Shi; linux-kernel@vger.kernel.org; linux-mm@kvack.org; haren@linux.vnet.ibm.com; npiggin@gmail.com; bsingharora@gmail.com; paulmck@linux.vnet.ibm.com; Tim Chen; linuxppc-dev@lists.ozlabs.org; x86@kernel.org
>>>> Subject: Re: [PATCH v11 00/26] Speculative page faults
>>>>
>>>> On 11/06/2018 09:49, Song, HaiyanX wrote:
>>>>> Hi Laurent,
>>>>>
>>>>> Regression test for v11 patch serials have been run, some regression is found by LKP-tools (linux kernel performance)
>>>>> tested on Intel 4s skylake platform. This time only test the cases which have been run and found regressions on
>>>>> V9 patch serials.
>>>>>
>>>>> The regression result is sorted by the metric will-it-scale.per_thread_ops.
>>>>> branch: Laurent-Dufour/Speculative-page-faults/20180520-045126
>>>>> commit id:
>>>>> head commit : a7a8993bfe3ccb54ad468b9f1799649e4ad1ff12
>>>>> base commit : ba98a1cdad71d259a194461b3a61471b49b14df1
>>>>> Benchmark: will-it-scale
>>>>> Download link: https://github.com/antonblanchard/will-it-scale/tree/master
>>>>>
>>>>> Metrics:
>>>>> will-it-scale.per_process_ops=processes/nr_cpu
>>>>> will-it-scale.per_thread_ops=threads/nr_cpu
>>>>> test box: lkp-skl-4sp1(nr_cpu=192,memory=768G)
>>>>> THP: enable / disable
>>>>> nr_task:100%
>>>>>
>>>>> 1. Regressions:
>>>>>
>>>>> a). Enable THP
>>>>> testcase base change head metric
>>>>> page_fault3/enable THP 10519 -20.5% 836 will-it-scale.per_thread_ops
>>>>> page_fault2/enalbe THP 8281 -18.8% 6728 will-it-scale.per_thread_ops
>>>>> brk1/eanble THP 998475 -2.2% 976893 will-it-scale.per_process_ops
>>>>> context_switch1/enable THP 223910 -1.3% 220930 will-it-scale.per_process_ops
>>>>> context_switch1/enable THP 233722 -1.0% 231288 will-it-scale.per_thread_ops
>>>>>
>>>>> b). Disable THP
>>>>> page_fault3/disable THP 10856 -23.1% 8344 will-it-scale.per_thread_ops
>>>>> page_fault2/disable THP 8147 -18.8% 6613 will-it-scale.per_thread_ops
>>>>> brk1/disable THP 957 -7.9% 881 will-it-scale.per_thread_ops
>>>>> context_switch1/disable THP 237006 -2.2% 231907 will-it-scale.per_thread_ops
>>>>> brk1/disable THP 997317 -2.0% 977778 will-it-scale.per_process_ops
>>>>> page_fault3/disable THP 467454 -1.8% 459251 will-it-scale.per_process_ops
>>>>> context_switch1/disable THP 224431 -1.3% 221567 will-it-scale.per_process_ops
>>>>>
>>>>> Notes: for the above values of test result, the higher is better.
>>>>
>>>> I tried the same tests on my PowerPC victim VM (1024 CPUs, 11TB) and I can't
>>>> get reproducible results. The results have huge variation, even on the vanilla
>>>> kernel, and I can't state on any changes due to that.
>>>>
>>>> I tried on smaller node (80 CPUs, 32G), and the tests ran better, but I didn't
>>>> measure any changes between the vanilla and the SPF patched ones:
>>>>
>>>> test THP enabled 4.17.0-rc4-mm1 spf delta
>>>> page_fault3_threads 2697.7 2683.5 -0.53%
>>>> page_fault2_threads 170660.6 169574.1 -0.64%
>>>> context_switch1_threads 6915269.2 6877507.3 -0.55%
>>>> context_switch1_processes 6478076.2 6529493.5 0.79%
>>>> brk1 243391.2 238527.5 -2.00%
>>>>
>>>> Tests were run 10 times, no high variation detected.
>>>>
>>>> Did you see high variation on your side ? How many times the test were run to
>>>> compute the average values ?
>>>>
>>>> Thanks,
>>>> Laurent.
>>>>
>>>>
>>>>>
>>>>> 2. Improvement: not found improvement based on the selected test cases.
>>>>>
>>>>>
>>>>> Best regards
>>>>> Haiyan Song
>>>>> ________________________________________
>>>>> From: owner-linux-mm@kvack.org [owner-linux-mm@kvack.org] on behalf of Laurent Dufour [ldufour@linux.vnet.ibm.com]
>>>>> Sent: Monday, May 28, 2018 4:54 PM
>>>>> To: Song, HaiyanX
>>>>> Cc: akpm@linux-foundation.org; mhocko@kernel.org; peterz@infradead.org; kirill@shutemov.name; ak@linux.intel.com; dave@stgolabs.net; jack@suse.cz; Matthew Wilcox; khandual@linux.vnet.ibm.com; aneesh.kumar@linux.vnet.ibm.com; benh@kernel.crashing.org; mpe@ellerman.id.au; paulus@samba.org; Thomas Gleixner; Ingo Molnar; hpa@zytor.com; Will Deacon; Sergey Senozhatsky; sergey.senozhatsky.work@gmail.com; Andrea Arcangeli; Alexei Starovoitov; Wang, Kemi; Daniel Jordan; David Rientjes; Jerome Glisse; Ganesh Mahendran; Minchan Kim; Punit Agrawal; vinayak menon; Yang Shi; linux-kernel@vger.kernel.org; linux-mm@kvack.org; haren@linux.vnet.ibm.com; npiggin@gmail.com; bsingharora@gmail.com; paulmck@linux.vnet.ibm.com; Tim Chen; linuxppc-dev@lists.ozlabs.org; x86@kernel.org
>>>>> Subject: Re: [PATCH v11 00/26] Speculative page faults
>>>>>
>>>>> On 28/05/2018 10:22, Haiyan Song wrote:
>>>>>> Hi Laurent,
>>>>>>
>>>>>> Yes, these tests are done on V9 patch.
>>>>>
>>>>> Do you plan to give this V11 a run ?
>>>>>
>>>>>>
>>>>>>
>>>>>> Best regards,
>>>>>> Haiyan Song
>>>>>>
>>>>>> On Mon, May 28, 2018 at 09:51:34AM +0200, Laurent Dufour wrote:
>>>>>>> On 28/05/2018 07:23, Song, HaiyanX wrote:
>>>>>>>>
>>>>>>>> Some regression and improvements is found by LKP-tools(linux kernel performance) on V9 patch series
>>>>>>>> tested on Intel 4s Skylake platform.
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> Thanks for reporting this benchmark results, but you mentioned the "V9 patch
>>>>>>> series" while responding to the v11 header series...
>>>>>>> Were these tests done on v9 or v11 ?
>>>>>>>
>>>>>>> Cheers,
>>>>>>> Laurent.
>>>>>>>
>>>>>>>>
>>>>>>>> The regression result is sorted by the metric will-it-scale.per_thread_ops.
>>>>>>>> Branch: Laurent-Dufour/Speculative-page-faults/20180316-151833 (V9 patch series)
>>>>>>>> Commit id:
>>>>>>>> base commit: d55f34411b1b126429a823d06c3124c16283231f
>>>>>>>> head commit: 0355322b3577eeab7669066df42c550a56801110
>>>>>>>> Benchmark suite: will-it-scale
>>>>>>>> Download link:
>>>>>>>> https://github.com/antonblanchard/will-it-scale/tree/master/tests
>>>>>>>> Metrics:
>>>>>>>> will-it-scale.per_process_ops=processes/nr_cpu
>>>>>>>> will-it-scale.per_thread_ops=threads/nr_cpu
>>>>>>>> test box: lkp-skl-4sp1(nr_cpu=192,memory=768G)
>>>>>>>> THP: enable / disable
>>>>>>>> nr_task: 100%
>>>>>>>>
>>>>>>>> 1. Regressions:
>>>>>>>> a) THP enabled:
>>>>>>>> testcase base change head metric
>>>>>>>> page_fault3/ enable THP 10092 -17.5% 8323 will-it-scale.per_thread_ops
>>>>>>>> page_fault2/ enable THP 8300 -17.2% 6869 will-it-scale.per_thread_ops
>>>>>>>> brk1/ enable THP 957.67 -7.6% 885 will-it-scale.per_thread_ops
>>>>>>>> page_fault3/ enable THP 172821 -5.3% 163692 will-it-scale.per_process_ops
>>>>>>>> signal1/ enable THP 9125 -3.2% 8834 will-it-scale.per_process_ops
>>>>>>>>
>>>>>>>> b) THP disabled:
>>>>>>>> testcase base change head metric
>>>>>>>> page_fault3/ disable THP 10107 -19.1% 8180 will-it-scale.per_thread_ops
>>>>>>>> page_fault2/ disable THP 8432 -17.8% 6931 will-it-scale.per_thread_ops
>>>>>>>> context_switch1/ disable THP 215389 -6.8% 200776 will-it-scale.per_thread_ops
>>>>>>>> brk1/ disable THP 939.67 -6.6% 877.33 will-it-scale.per_thread_ops
>>>>>>>> page_fault3/ disable THP 173145 -4.7% 165064 will-it-scale.per_process_ops
>>>>>>>> signal1/ disable THP 9162 -3.9% 8802 will-it-scale.per_process_ops
>>>>>>>>
>>>>>>>> 2. Improvements:
>>>>>>>> a) THP enabled:
>>>>>>>> testcase base change head metric
>>>>>>>> malloc1/ enable THP 66.33 +469.8% 383.67 will-it-scale.per_thread_ops
>>>>>>>> writeseek3/ enable THP 2531 +4.5% 2646 will-it-scale.per_thread_ops
>>>>>>>> signal1/ enable THP 989.33 +2.8% 1016 will-it-scale.per_thread_ops
>>>>>>>>
>>>>>>>> b) THP disabled:
>>>>>>>> testcase base change head metric
>>>>>>>> malloc1/ disable THP 90.33 +417.3% 467.33 will-it-scale.per_thread_ops
>>>>>>>> read2/ disable THP 58934 +39.2% 82060 will-it-scale.per_thread_ops
>>>>>>>> page_fault1/ disable THP 8607 +36.4% 11736 will-it-scale.per_thread_ops
>>>>>>>> read1/ disable THP 314063 +12.7% 353934 will-it-scale.per_thread_ops
>>>>>>>> writeseek3/ disable THP 2452 +12.5% 2759 will-it-scale.per_thread_ops
>>>>>>>> signal1/ disable THP 971.33 +5.5% 1024 will-it-scale.per_thread_ops
>>>>>>>>
>>>>>>>> Notes: for above values in column "change", the higher value means that the related testcase result
>>>>>>>> on head commit is better than that on base commit for this benchmark.
>>>>>>>>
>>>>>>>>
>>>>>>>> Best regards
>>>>>>>> Haiyan Song
>>>>>>>>
>>>>>>>> ________________________________________
>>>>>>>> From: owner-linux-mm@kvack.org [owner-linux-mm@kvack.org] on behalf of Laurent Dufour [ldufour@linux.vnet.ibm.com]
>>>>>>>> Sent: Thursday, May 17, 2018 7:06 PM
>>>>>>>> To: akpm@linux-foundation.org; mhocko@kernel.org; peterz@infradead.org; kirill@shutemov.name; ak@linux.intel.com; dave@stgolabs.net; jack@suse.cz; Matthew Wilcox; khandual@linux.vnet.ibm.com; aneesh.kumar@linux.vnet.ibm.com; benh@kernel.crashing.org; mpe@ellerman.id.au; paulus@samba.org; Thomas Gleixner; Ingo Molnar; hpa@zytor.com; Will Deacon; Sergey Senozhatsky; sergey.senozhatsky.work@gmail.com; Andrea Arcangeli; Alexei Starovoitov; Wang, Kemi; Daniel Jordan; David Rientjes; Jerome Glisse; Ganesh Mahendran; Minchan Kim; Punit Agrawal; vinayak menon; Yang Shi
>>>>>>>> Cc: linux-kernel@vger.kernel.org; linux-mm@kvack.org; haren@linux.vnet.ibm.com; npiggin@gmail.com; bsingharora@gmail.com; paulmck@linux.vnet.ibm.com; Tim Chen; linuxppc-dev@lists.ozlabs.org; x86@kernel.org
>>>>>>>> Subject: [PATCH v11 00/26] Speculative page faults
>>>>>>>>
>>>>>>>> This is a port on kernel 4.17 of the work done by Peter Zijlstra to handle
>>>>>>>> page fault without holding the mm semaphore [1].
>>>>>>>>
>>>>>>>> The idea is to try to handle user space page faults without holding the
>>>>>>>> mmap_sem. This should allow better concurrency for massively threaded
>>>>>>>> process since the page fault handler will not wait for other threads memory
>>>>>>>> layout change to be done, assuming that this change is done in another part
>>>>>>>> of the process's memory space. This type page fault is named speculative
>>>>>>>> page fault. If the speculative page fault fails because of a concurrency is
>>>>>>>> detected or because underlying PMD or PTE tables are not yet allocating, it
>>>>>>>> is failing its processing and a classic page fault is then tried.
>>>>>>>>
>>>>>>>> The speculative page fault (SPF) has to look for the VMA matching the fault
>>>>>>>> address without holding the mmap_sem, this is done by introducing a rwlock
>>>>>>>> which protects the access to the mm_rb tree. Previously this was done using
>>>>>>>> SRCU but it was introducing a lot of scheduling to process the VMA's
>>>>>>>> freeing operation which was hitting the performance by 20% as reported by
>>>>>>>> Kemi Wang [2]. Using a rwlock to protect access to the mm_rb tree is
>>>>>>>> limiting the locking contention to these operations which are expected to
>>>>>>>> be in a O(log n) order. In addition to ensure that the VMA is not freed in
>>>>>>>> our back a reference count is added and 2 services (get_vma() and
>>>>>>>> put_vma()) are introduced to handle the reference count. Once a VMA is
>>>>>>>> fetched from the RB tree using get_vma(), it must be later freed using
>>>>>>>> put_vma(). I can't see anymore the overhead I got while will-it-scale
>>>>>>>> benchmark anymore.
>>>>>>>>
>>>>>>>> The VMA's attributes checked during the speculative page fault processing
>>>>>>>> have to be protected against parallel changes. This is done by using a per
>>>>>>>> VMA sequence lock. This sequence lock allows the speculative page fault
>>>>>>>> handler to fast check for parallel changes in progress and to abort the
>>>>>>>> speculative page fault in that case.
>>>>>>>>
>>>>>>>> Once the VMA has been found, the speculative page fault handler would check
>>>>>>>> for the VMA's attributes to verify that the page fault has to be handled
>>>>>>>> correctly or not. Thus, the VMA is protected through a sequence lock which
>>>>>>>> allows fast detection of concurrent VMA changes. If such a change is
>>>>>>>> detected, the speculative page fault is aborted and a *classic* page fault
>>>>>>>> is tried. VMA sequence lockings are added when VMA attributes which are
>>>>>>>> checked during the page fault are modified.
>>>>>>>>
>>>>>>>> When the PTE is fetched, the VMA is checked to see if it has been changed,
>>>>>>>> so once the page table is locked, the VMA is valid, so any other changes
>>>>>>>> leading to touching this PTE will need to lock the page table, so no
>>>>>>>> parallel change is possible at this time.
>>>>>>>>
>>>>>>>> The locking of the PTE is done with interrupts disabled, this allows
>>>>>>>> checking for the PMD to ensure that there is not an ongoing collapsing
>>>>>>>> operation. Since khugepaged is firstly set the PMD to pmd_none and then is
>>>>>>>> waiting for the other CPU to have caught the IPI interrupt, if the pmd is
>>>>>>>> valid at the time the PTE is locked, we have the guarantee that the
>>>>>>>> collapsing operation will have to wait on the PTE lock to move forward.
>>>>>>>> This allows the SPF handler to map the PTE safely. If the PMD value is
>>>>>>>> different from the one recorded at the beginning of the SPF operation, the
>>>>>>>> classic page fault handler will be called to handle the operation while
>>>>>>>> holding the mmap_sem. As the PTE lock is done with the interrupts disabled,
>>>>>>>> the lock is done using spin_trylock() to avoid dead lock when handling a
>>>>>>>> page fault while a TLB invalidate is requested by another CPU holding the
>>>>>>>> PTE.
>>>>>>>>
>>>>>>>> In pseudo code, this could be seen as:
>>>>>>>> speculative_page_fault()
>>>>>>>> {
>>>>>>>> vma = get_vma()
>>>>>>>> check vma sequence count
>>>>>>>> check vma's support
>>>>>>>> disable interrupt
>>>>>>>> check pgd,p4d,...,pte
>>>>>>>> save pmd and pte in vmf
>>>>>>>> save vma sequence counter in vmf
>>>>>>>> enable interrupt
>>>>>>>> check vma sequence count
>>>>>>>> handle_pte_fault(vma)
>>>>>>>> ..
>>>>>>>> page = alloc_page()
>>>>>>>> pte_map_lock()
>>>>>>>> disable interrupt
>>>>>>>> abort if sequence counter has changed
>>>>>>>> abort if pmd or pte has changed
>>>>>>>> pte map and lock
>>>>>>>> enable interrupt
>>>>>>>> if abort
>>>>>>>> free page
>>>>>>>> abort
>>>>>>>> ...
>>>>>>>> }
>>>>>>>>
>>>>>>>> arch_fault_handler()
>>>>>>>> {
>>>>>>>> if (speculative_page_fault(&vma))
>>>>>>>> goto done
>>>>>>>> again:
>>>>>>>> lock(mmap_sem)
>>>>>>>> vma = find_vma();
>>>>>>>> handle_pte_fault(vma);
>>>>>>>> if retry
>>>>>>>> unlock(mmap_sem)
>>>>>>>> goto again;
>>>>>>>> done:
>>>>>>>> handle fault error
>>>>>>>> }
>>>>>>>>
>>>>>>>> Support for THP is not done because when checking for the PMD, we can be
>>>>>>>> confused by an in progress collapsing operation done by khugepaged. The
>>>>>>>> issue is that pmd_none() could be true either if the PMD is not already
>>>>>>>> populated or if the underlying PTE are in the way to be collapsed. So we
>>>>>>>> cannot safely allocate a PMD if pmd_none() is true.
>>>>>>>>
>>>>>>>> This series add a new software performance event named 'speculative-faults'
>>>>>>>> or 'spf'. It counts the number of successful page fault event handled
>>>>>>>> speculatively. When recording 'faults,spf' events, the faults one is
>>>>>>>> counting the total number of page fault events while 'spf' is only counting
>>>>>>>> the part of the faults processed speculatively.
>>>>>>>>
>>>>>>>> There are some trace events introduced by this series. They allow
>>>>>>>> identifying why the page faults were not processed speculatively. This
>>>>>>>> doesn't take in account the faults generated by a monothreaded process
>>>>>>>> which directly processed while holding the mmap_sem. This trace events are
>>>>>>>> grouped in a system named 'pagefault', they are:
>>>>>>>> - pagefault:spf_vma_changed : if the VMA has been changed in our back
>>>>>>>> - pagefault:spf_vma_noanon : the vma->anon_vma field was not yet set.
>>>>>>>> - pagefault:spf_vma_notsup : the VMA's type is not supported
>>>>>>>> - pagefault:spf_vma_access : the VMA's access right are not respected
>>>>>>>> - pagefault:spf_pmd_changed : the upper PMD pointer has changed in our
>>>>>>>> back.
>>>>>>>>
>>>>>>>> To record all the related events, the easier is to run perf with the
>>>>>>>> following arguments :
>>>>>>>> $ perf stat -e 'faults,spf,pagefault:*' <command>
>>>>>>>>
>>>>>>>> There is also a dedicated vmstat counter showing the number of successful
>>>>>>>> page fault handled speculatively. I can be seen this way:
>>>>>>>> $ grep speculative_pgfault /proc/vmstat
>>>>>>>>
>>>>>>>> This series builds on top of v4.16-mmotm-2018-04-13-17-28 and is functional
>>>>>>>> on x86, PowerPC and arm64.
>>>>>>>>
>>>>>>>> ---------------------
>>>>>>>> Real Workload results
>>>>>>>>
>>>>>>>> As mentioned in previous email, we did non official runs using a "popular
>>>>>>>> in memory multithreaded database product" on 176 cores SMT8 Power system
>>>>>>>> which showed a 30% improvements in the number of transaction processed per
>>>>>>>> second. This run has been done on the v6 series, but changes introduced in
>>>>>>>> this new version should not impact the performance boost seen.
>>>>>>>>
>>>>>>>> Here are the perf data captured during 2 of these runs on top of the v8
>>>>>>>> series:
>>>>>>>> vanilla spf
>>>>>>>> faults 89.418 101.364 +13%
>>>>>>>> spf n/a 97.989
>>>>>>>>
>>>>>>>> With the SPF kernel, most of the page fault were processed in a speculative
>>>>>>>> way.
>>>>>>>>
>>>>>>>> Ganesh Mahendran had backported the series on top of a 4.9 kernel and gave
>>>>>>>> it a try on an android device. He reported that the application launch time
>>>>>>>> was improved in average by 6%, and for large applications (~100 threads) by
>>>>>>>> 20%.
>>>>>>>>
>>>>>>>> Here are the launch time Ganesh mesured on Android 8.0 on top of a Qcom
>>>>>>>> MSM845 (8 cores) with 6GB (the less is better):
>>>>>>>>
>>>>>>>> Application 4.9 4.9+spf delta
>>>>>>>> com.tencent.mm 416 389 -7%
>>>>>>>> com.eg.android.AlipayGphone 1135 986 -13%
>>>>>>>> com.tencent.mtt 455 454 0%
>>>>>>>> com.qqgame.hlddz 1497 1409 -6%
>>>>>>>> com.autonavi.minimap 711 701 -1%
>>>>>>>> com.tencent.tmgp.sgame 788 748 -5%
>>>>>>>> com.immomo.momo 501 487 -3%
>>>>>>>> com.tencent.peng 2145 2112 -2%
>>>>>>>> com.smile.gifmaker 491 461 -6%
>>>>>>>> com.baidu.BaiduMap 479 366 -23%
>>>>>>>> com.taobao.taobao 1341 1198 -11%
>>>>>>>> com.baidu.searchbox 333 314 -6%
>>>>>>>> com.tencent.mobileqq 394 384 -3%
>>>>>>>> com.sina.weibo 907 906 0%
>>>>>>>> com.youku.phone 816 731 -11%
>>>>>>>> com.happyelements.AndroidAnimal.qq 763 717 -6%
>>>>>>>> com.UCMobile 415 411 -1%
>>>>>>>> com.tencent.tmgp.ak 1464 1431 -2%
>>>>>>>> com.tencent.qqmusic 336 329 -2%
>>>>>>>> com.sankuai.meituan 1661 1302 -22%
>>>>>>>> com.netease.cloudmusic 1193 1200 1%
>>>>>>>> air.tv.douyu.android 4257 4152 -2%
>>>>>>>>
>>>>>>>> ------------------
>>>>>>>> Benchmarks results
>>>>>>>>
>>>>>>>> Base kernel is v4.17.0-rc4-mm1
>>>>>>>> SPF is BASE + this series
>>>>>>>>
>>>>>>>> Kernbench:
>>>>>>>> ----------
>>>>>>>> Here are the results on a 16 CPUs X86 guest using kernbench on a 4.15
>>>>>>>> kernel (kernel is build 5 times):
>>>>>>>>
>>>>>>>> Average Half load -j 8
>>>>>>>> Run (std deviation)
>>>>>>>> BASE SPF
>>>>>>>> Elapsed Time 1448.65 (5.72312) 1455.84 (4.84951) 0.50%
>>>>>>>> User Time 10135.4 (30.3699) 10148.8 (31.1252) 0.13%
>>>>>>>> System Time 900.47 (2.81131) 923.28 (7.52779) 2.53%
>>>>>>>> Percent CPU 761.4 (1.14018) 760.2 (0.447214) -0.16%
>>>>>>>> Context Switches 85380 (3419.52) 84748 (1904.44) -0.74%
>>>>>>>> Sleeps 105064 (1240.96) 105074 (337.612) 0.01%
>>>>>>>>
>>>>>>>> Average Optimal load -j 16
>>>>>>>> Run (std deviation)
>>>>>>>> BASE SPF
>>>>>>>> Elapsed Time 920.528 (10.1212) 927.404 (8.91789) 0.75%
>>>>>>>> User Time 11064.8 (981.142) 11085 (990.897) 0.18%
>>>>>>>> System Time 979.904 (84.0615) 1001.14 (82.5523) 2.17%
>>>>>>>> Percent CPU 1089.5 (345.894) 1086.1 (343.545) -0.31%
>>>>>>>> Context Switches 159488 (78156.4) 158223 (77472.1) -0.79%
>>>>>>>> Sleeps 110566 (5877.49) 110388 (5617.75) -0.16%
>>>>>>>>
>>>>>>>>
>>>>>>>> During a run on the SPF, perf events were captured:
>>>>>>>> Performance counter stats for '../kernbench -M':
>>>>>>>> 526743764 faults
>>>>>>>> 210 spf
>>>>>>>> 3 pagefault:spf_vma_changed
>>>>>>>> 0 pagefault:spf_vma_noanon
>>>>>>>> 2278 pagefault:spf_vma_notsup
>>>>>>>> 0 pagefault:spf_vma_access
>>>>>>>> 0 pagefault:spf_pmd_changed
>>>>>>>>
>>>>>>>> Very few speculative page faults were recorded as most of the processes
>>>>>>>> involved are monothreaded (sounds that on this architecture some threads
>>>>>>>> were created during the kernel build processing).
>>>>>>>>
>>>>>>>> Here are the kerbench results on a 80 CPUs Power8 system:
>>>>>>>>
>>>>>>>> Average Half load -j 40
>>>>>>>> Run (std deviation)
>>>>>>>> BASE SPF
>>>>>>>> Elapsed Time 117.152 (0.774642) 117.166 (0.476057) 0.01%
>>>>>>>> User Time 4478.52 (24.7688) 4479.76 (9.08555) 0.03%
>>>>>>>> System Time 131.104 (0.720056) 134.04 (0.708414) 2.24%
>>>>>>>> Percent CPU 3934 (19.7104) 3937.2 (19.0184) 0.08%
>>>>>>>> Context Switches 92125.4 (576.787) 92581.6 (198.622) 0.50%
>>>>>>>> Sleeps 317923 (652.499) 318469 (1255.59) 0.17%
>>>>>>>>
>>>>>>>> Average Optimal load -j 80
>>>>>>>> Run (std deviation)
>>>>>>>> BASE SPF
>>>>>>>> Elapsed Time 107.73 (0.632416) 107.31 (0.584936) -0.39%
>>>>>>>> User Time 5869.86 (1466.72) 5871.71 (1467.27) 0.03%
>>>>>>>> System Time 153.728 (23.8573) 157.153 (24.3704) 2.23%
>>>>>>>> Percent CPU 5418.6 (1565.17) 5436.7 (1580.91) 0.33%
>>>>>>>> Context Switches 223861 (138865) 225032 (139632) 0.52%
>>>>>>>> Sleeps 330529 (13495.1) 332001 (14746.2) 0.45%
>>>>>>>>
>>>>>>>> During a run on the SPF, perf events were captured:
>>>>>>>> Performance counter stats for '../kernbench -M':
>>>>>>>> 116730856 faults
>>>>>>>> 0 spf
>>>>>>>> 3 pagefault:spf_vma_changed
>>>>>>>> 0 pagefault:spf_vma_noanon
>>>>>>>> 476 pagefault:spf_vma_notsup
>>>>>>>> 0 pagefault:spf_vma_access
>>>>>>>> 0 pagefault:spf_pmd_changed
>>>>>>>>
>>>>>>>> Most of the processes involved are monothreaded so SPF is not activated but
>>>>>>>> there is no impact on the performance.
>>>>>>>>
>>>>>>>> Ebizzy:
>>>>>>>> -------
>>>>>>>> The test is counting the number of records per second it can manage, the
>>>>>>>> higher is the best. I run it like this 'ebizzy -mTt <nrcpus>'. To get
>>>>>>>> consistent result I repeated the test 100 times and measure the average
>>>>>>>> result. The number is the record processes per second, the higher is the
>>>>>>>> best.
>>>>>>>>
>>>>>>>> BASE SPF delta
>>>>>>>> 16 CPUs x86 VM 742.57 1490.24 100.69%
>>>>>>>> 80 CPUs P8 node 13105.4 24174.23 84.46%
>>>>>>>>
>>>>>>>> Here are the performance counter read during a run on a 16 CPUs x86 VM:
>>>>>>>> Performance counter stats for './ebizzy -mTt 16':
>>>>>>>> 1706379 faults
>>>>>>>> 1674599 spf
>>>>>>>> 30588 pagefault:spf_vma_changed
>>>>>>>> 0 pagefault:spf_vma_noanon
>>>>>>>> 363 pagefault:spf_vma_notsup
>>>>>>>> 0 pagefault:spf_vma_access
>>>>>>>> 0 pagefault:spf_pmd_changed
>>>>>>>>
>>>>>>>> And the ones captured during a run on a 80 CPUs Power node:
>>>>>>>> Performance counter stats for './ebizzy -mTt 80':
>>>>>>>> 1874773 faults
>>>>>>>> 1461153 spf
>>>>>>>> 413293 pagefault:spf_vma_changed
>>>>>>>> 0 pagefault:spf_vma_noanon
>>>>>>>> 200 pagefault:spf_vma_notsup
>>>>>>>> 0 pagefault:spf_vma_access
>>>>>>>> 0 pagefault:spf_pmd_changed
>>>>>>>>
>>>>>>>> In ebizzy's case most of the page fault were handled in a speculative way,
>>>>>>>> leading the ebizzy performance boost.
>>>>>>>>
>>>>>>>> ------------------
>>>>>>>> Changes since v10 (https://lkml.org/lkml/2018/4/17/572):
>>>>>>>> - Accounted for all review feedbacks from Punit Agrawal, Ganesh Mahendran
>>>>>>>> and Minchan Kim, hopefully.
>>>>>>>> - Remove unneeded check on CONFIG_SPECULATIVE_PAGE_FAULT in
>>>>>>>> __do_page_fault().
>>>>>>>> - Loop in pte_spinlock() and pte_map_lock() when pte try lock fails
>>>>>>>> instead
>>>>>>>> of aborting the speculative page fault handling. Dropping the now
>>>>>>>> useless
>>>>>>>> trace event pagefault:spf_pte_lock.
>>>>>>>> - No more try to reuse the fetched VMA during the speculative page fault
>>>>>>>> handling when retrying is needed. This adds a lot of complexity and
>>>>>>>> additional tests done didn't show a significant performance improvement.
>>>>>>>> - Convert IS_ENABLED(CONFIG_NUMA) back to #ifdef due to build error.
>>>>>>>>
>>>>>>>> [1] http://linux-kernel.2935.n7.nabble.com/RFC-PATCH-0-6-Another-go-at-speculative-page-faults-tt965642.html#none
>>>>>>>> [2] https://patchwork.kernel.org/patch/9999687/
>>>>>>>>
>>>>>>>>
>>>>>>>> Laurent Dufour (20):
>>>>>>>> mm: introduce CONFIG_SPECULATIVE_PAGE_FAULT
>>>>>>>> x86/mm: define ARCH_SUPPORTS_SPECULATIVE_PAGE_FAULT
>>>>>>>> powerpc/mm: set ARCH_SUPPORTS_SPECULATIVE_PAGE_FAULT
>>>>>>>> mm: introduce pte_spinlock for FAULT_FLAG_SPECULATIVE
>>>>>>>> mm: make pte_unmap_same compatible with SPF
>>>>>>>> mm: introduce INIT_VMA()
>>>>>>>> mm: protect VMA modifications using VMA sequence count
>>>>>>>> mm: protect mremap() against SPF hanlder
>>>>>>>> mm: protect SPF handler against anon_vma changes
>>>>>>>> mm: cache some VMA fields in the vm_fault structure
>>>>>>>> mm/migrate: Pass vm_fault pointer to migrate_misplaced_page()
>>>>>>>> mm: introduce __lru_cache_add_active_or_unevictable
>>>>>>>> mm: introduce __vm_normal_page()
>>>>>>>> mm: introduce __page_add_new_anon_rmap()
>>>>>>>> mm: protect mm_rb tree with a rwlock
>>>>>>>> mm: adding speculative page fault failure trace events
>>>>>>>> perf: add a speculative page fault sw event
>>>>>>>> perf tools: add support for the SPF perf event
>>>>>>>> mm: add speculative page fault vmstats
>>>>>>>> powerpc/mm: add speculative page fault
>>>>>>>>
>>>>>>>> Mahendran Ganesh (2):
>>>>>>>> arm64/mm: define ARCH_SUPPORTS_SPECULATIVE_PAGE_FAULT
>>>>>>>> arm64/mm: add speculative page fault
>>>>>>>>
>>>>>>>> Peter Zijlstra (4):
>>>>>>>> mm: prepare for FAULT_FLAG_SPECULATIVE
>>>>>>>> mm: VMA sequence count
>>>>>>>> mm: provide speculative fault infrastructure
>>>>>>>> x86/mm: add speculative pagefault handling
>>>>>>>>
>>>>>>>> arch/arm64/Kconfig | 1 +
>>>>>>>> arch/arm64/mm/fault.c | 12 +
>>>>>>>> arch/powerpc/Kconfig | 1 +
>>>>>>>> arch/powerpc/mm/fault.c | 16 +
>>>>>>>> arch/x86/Kconfig | 1 +
>>>>>>>> arch/x86/mm/fault.c | 27 +-
>>>>>>>> fs/exec.c | 2 +-
>>>>>>>> fs/proc/task_mmu.c | 5 +-
>>>>>>>> fs/userfaultfd.c | 17 +-
>>>>>>>> include/linux/hugetlb_inline.h | 2 +-
>>>>>>>> include/linux/migrate.h | 4 +-
>>>>>>>> include/linux/mm.h | 136 +++++++-
>>>>>>>> include/linux/mm_types.h | 7 +
>>>>>>>> include/linux/pagemap.h | 4 +-
>>>>>>>> include/linux/rmap.h | 12 +-
>>>>>>>> include/linux/swap.h | 10 +-
>>>>>>>> include/linux/vm_event_item.h | 3 +
>>>>>>>> include/trace/events/pagefault.h | 80 +++++
>>>>>>>> include/uapi/linux/perf_event.h | 1 +
>>>>>>>> kernel/fork.c | 5 +-
>>>>>>>> mm/Kconfig | 22 ++
>>>>>>>> mm/huge_memory.c | 6 +-
>>>>>>>> mm/hugetlb.c | 2 +
>>>>>>>> mm/init-mm.c | 3 +
>>>>>>>> mm/internal.h | 20 ++
>>>>>>>> mm/khugepaged.c | 5 +
>>>>>>>> mm/madvise.c | 6 +-
>>>>>>>> mm/memory.c | 612 +++++++++++++++++++++++++++++-----
>>>>>>>> mm/mempolicy.c | 51 ++-
>>>>>>>> mm/migrate.c | 6 +-
>>>>>>>> mm/mlock.c | 13 +-
>>>>>>>> mm/mmap.c | 229 ++++++++++---
>>>>>>>> mm/mprotect.c | 4 +-
>>>>>>>> mm/mremap.c | 13 +
>>>>>>>> mm/nommu.c | 2 +-
>>>>>>>> mm/rmap.c | 5 +-
>>>>>>>> mm/swap.c | 6 +-
>>>>>>>> mm/swap_state.c | 8 +-
>>>>>>>> mm/vmstat.c | 5 +-
>>>>>>>> tools/include/uapi/linux/perf_event.h | 1 +
>>>>>>>> tools/perf/util/evsel.c | 1 +
>>>>>>>> tools/perf/util/parse-events.c | 4 +
>>>>>>>> tools/perf/util/parse-events.l | 1 +
>>>>>>>> tools/perf/util/python.c | 1 +
>>>>>>>> 44 files changed, 1161 insertions(+), 211 deletions(-)
>>>>>>>> create mode 100644 include/trace/events/pagefault.h
>>>>>>>>
>>>>>>>> --
>>>>>>>> 2.7.4
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>
[-- Attachment #2: perf-profile_page_fault3-head-thp-always-SPF-off.gz --]
[-- Type: application/gzip, Size: 11278 bytes --]
[-- Attachment #3: perf-profile_page_fault3-head-thp-always-SPF-on.gz --]
[-- Type: application/gzip, Size: 11424 bytes --]
^ permalink raw reply
* Re: [PATCH] powerpc/time: Calculate proper wday
From: Joakim Tjernlund @ 2018-09-18 8:27 UTC (permalink / raw)
To: malat@debian.org; +Cc: linuxppc-dev@lists.ozlabs.org, arnd@arndb.de
In-Reply-To: <CA+7wUsxbYGAuwE--AukCCYA8H4CQj1tv31nAOv256u9tkDENKg@mail.gmail.com>
T24gVHVlLCAyMDE4LTA5LTE4IGF0IDEwOjA4ICswMjAwLCBNYXRoaWV1IE1hbGF0ZXJyZSB3cm90
ZToNCj4gDQo+IA0KPiBPbiBXZWQsIEF1ZyAyOSwgMjAxOCBhdCAxMDowMyBBTSBKb2FraW0gVGpl
cm5sdW5kIDxqb2FraW0udGplcm5sdW5kQGluZmluZXJhLmNvbT4gd3JvdGU6DQo+ID4NCj4gPiB0
b190bSgpIGhhcmRjb2RlcyB3ZGF5IHRvIC0xIGFzICJOby1vbmUgdXNlcyB0aGUgZGF5IG9mIHRo
ZSB3ZWVrIi4NCj4gPiBCdXQgcmVjZW50bHkgcnRjIGRyaXZlciBkczEzMDcgZG9lcyBjYXJlIGFu
ZCB0cmllcyB0byBjb3JyZWN0IHdkYXkuDQo+ID4NCj4gPiBBZGQgd2RheSBjYWxjdWxhdGlvbihz
dG9sZW4gZnJvbSBydGNfdGltZTY0X3RvX3RtKSB0byB0b190bSgpIHRvIHBsZWFzZSBkczEzMDcu
DQo+IA0KPiBJcyB0aGlzIHN0aWxsIGFuIGlzc3VlIGFmdGVyOg0KPiANCj4gMzRlZmFiZTQxODk1
IHBvd2VycGM6IHJlbW92ZSB1bnVzZWQgdG9fdG0oKSBoZWxwZXINCg0KTm8sIGl0IGlzIG5vdCBh
biBpc3N1ZSBhbnltb3JlLiBZb3UgY2FuIGRyb3AgdGhpcyBwYXRjaC4NCg0KIEpvY2tlDQoNCj4g
DQo+ID8NCj4gDQo+ID4gU2lnbmVkLW9mZi1ieTogSm9ha2ltIFRqZXJubHVuZCA8am9ha2ltLnRq
ZXJubHVuZEBpbmZpbmVyYS5jb20+DQo+ID4gLS0tDQo+ID4gIGFyY2gvcG93ZXJwYy9rZXJuZWwv
dGltZS5jIHwgOCArKystLS0tLQ0KPiA+ICAxIGZpbGUgY2hhbmdlZCwgMyBpbnNlcnRpb25zKCsp
LCA1IGRlbGV0aW9ucygtKQ0KPiA+DQo+ID4gZGlmZiAtLWdpdCBhL2FyY2gvcG93ZXJwYy9rZXJu
ZWwvdGltZS5jIGIvYXJjaC9wb3dlcnBjL2tlcm5lbC90aW1lLmMNCj4gPiBpbmRleCBmZTZmM2Ey
ODU0NTUuLmY0YTA5ZWUwMTk0NCAxMDA2NDQNCj4gPiAtLS0gYS9hcmNoL3Bvd2VycGMva2VybmVs
L3RpbWUuYw0KPiA+ICsrKyBiL2FyY2gvcG93ZXJwYy9rZXJuZWwvdGltZS5jDQo+ID4gQEAgLTEx
NjAsNiArMTE2MCw5IEBAIHZvaWQgdG9fdG0oaW50IHRpbSwgc3RydWN0IHJ0Y190aW1lICogdG0p
DQo+ID4gICAgICAgICBkYXkgPSB0aW0gLyBTRUNEQVk7DQo+ID4gICAgICAgICBobXMgPSB0aW0g
JSBTRUNEQVk7DQo+ID4NCj4gPiArICAgICAgIC8qIGRheSBvZiB0aGUgd2VlaywgMTk3MC0wMS0w
MSB3YXMgYSBUaHVyc2RheSAqLw0KPiA+ICsgICAgICAgdG0tPnRtX3dkYXkgPSAoZGF5ICsgNCkg
JSA3Ow0KPiA+ICsNCj4gPiAgICAgICAgIC8qIEhvdXJzLCBtaW51dGVzLCBzZWNvbmRzIGFyZSBl
YXN5ICovDQo+ID4gICAgICAgICB0bS0+dG1faG91ciA9IGhtcyAvIDM2MDA7DQo+ID4gICAgICAg
ICB0bS0+dG1fbWluID0gKGhtcyAlIDM2MDApIC8gNjA7DQo+ID4gQEAgLTExODAsMTEgKzExODMs
NiBAQCB2b2lkIHRvX3RtKGludCB0aW0sIHN0cnVjdCBydGNfdGltZSAqIHRtKQ0KPiA+DQo+ID4g
ICAgICAgICAvKiBEYXlzIGFyZSB3aGF0IGlzIGxlZnQgb3ZlciAoKzEpIGZyb20gYWxsIHRoYXQu
ICovDQo+ID4gICAgICAgICB0bS0+dG1fbWRheSA9IGRheSArIDE7DQo+ID4gLQ0KPiA+IC0gICAg
ICAgLyoNCj4gPiAtICAgICAgICAqIE5vLW9uZSB1c2VzIHRoZSBkYXkgb2YgdGhlIHdlZWsuDQo+
ID4gLSAgICAgICAgKi8NCj4gPiAtICAgICAgIHRtLT50bV93ZGF5ID0gLTE7DQo+ID4gIH0NCj4g
PiAgRVhQT1JUX1NZTUJPTCh0b190bSk7DQo+ID4NCj4gPiAtLQ0KPiA+IDIuMTYuNA0KDQo=
^ permalink raw reply
* Re: [PATCH v4] powerpc: Avoid code patching freed init sections
From: Christophe LEROY @ 2018-09-18 8:52 UTC (permalink / raw)
To: Nicholas Piggin, Michael Neuling
Cc: mpe, linuxppc-dev, paulus, Haren Myneni, Michal Suchánek
In-Reply-To: <20180914142201.379b14a4@roar.ozlabs.ibm.com>
Le 14/09/2018 à 06:22, Nicholas Piggin a écrit :
> On Fri, 14 Sep 2018 11:14:11 +1000
> Michael Neuling <mikey@neuling.org> wrote:
>
>> This stops us from doing code patching in init sections after they've
>> been freed.
>>
>> In this chain:
>> kvm_guest_init() ->
>> kvm_use_magic_page() ->
>> fault_in_pages_readable() ->
>> __get_user() ->
>> __get_user_nocheck() ->
>> barrier_nospec();
>>
>> We have a code patching location at barrier_nospec() and
>> kvm_guest_init() is an init function. This whole chain gets inlined,
>> so when we free the init section (hence kvm_guest_init()), this code
>> goes away and hence should no longer be patched.
>>
>> We seen this as userspace memory corruption when using a memory
>> checker while doing partition migration testing on powervm (this
>> starts the code patching post migration via
>> /sys/kernel/mobility/migration). In theory, it could also happen when
>> using /sys/kernel/debug/powerpc/barrier_nospec.
>>
>> cc: stable@vger.kernel.org # 4.13+
>> Signed-off-by: Michael Neuling <mikey@neuling.org>
>>
>> ---
>> For stable I've marked this as v4.13+ since that's when we refactored
>> code-patching.c but it could go back even further than that. In
>> reality though, I think we can only hit this since the first
>> spectre/meltdown changes.
>>
>> v4:
>> Feedback from Christophe Leroy:
>> - init_mem_free -> init_mem_is_free
>> - prlog %lx -> %px
>>
>> v3:
>> Add init_mem_free flag to avoid potential race.
>> Feedback from Christophe Leroy:
>> - use init_section_contains()
>> - change order of init test for performance
>> - use pr_debug()
>> - remove blank line
>>
>> v2:
>> Print when we skip an address
>> ---
>> arch/powerpc/include/asm/setup.h | 1 +
>> arch/powerpc/lib/code-patching.c | 6 ++++++
>> arch/powerpc/mm/mem.c | 2 ++
>> 3 files changed, 9 insertions(+)
>>
>> diff --git a/arch/powerpc/include/asm/setup.h b/arch/powerpc/include/asm/setup.h
>> index 1a951b0046..1fffbba8d6 100644
>> --- a/arch/powerpc/include/asm/setup.h
>> +++ b/arch/powerpc/include/asm/setup.h
>> @@ -9,6 +9,7 @@ extern void ppc_printk_progress(char *s, unsigned short hex);
>>
>> extern unsigned int rtas_data;
>> extern unsigned long long memory_limit;
>> +extern bool init_mem_is_free;
>> extern unsigned long klimit;
>> extern void *zalloc_maybe_bootmem(size_t size, gfp_t mask);
>>
>> diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
>> index 850f3b8f4d..6ae2777c22 100644
>> --- a/arch/powerpc/lib/code-patching.c
>> +++ b/arch/powerpc/lib/code-patching.c
>> @@ -28,6 +28,12 @@ static int __patch_instruction(unsigned int *exec_addr, unsigned int instr,
>> {
>> int err;
>>
>> + /* Make sure we aren't patching a freed init section */
>> + if (init_mem_is_free && init_section_contains(exec_addr, 4)) {
>> + pr_debug("Skipping init section patching addr: 0x%px\n", exec_addr);
>> + return 0;
>> + }
>
> What we should do is a whitelist, make sure it's only patching the
> sections we want it to.
>
> That's a bigger job when you consider modules and things too though,
> so this looks good for now. Thanks,
What about using kernel_text_address() for it then ? It also handles
modules, is it more complicated than that ?
Christophe
^ permalink raw reply
* [PATCH] powerpc/kgdb: add kgdb_arch_set/remove_breakpoint()
From: Christophe Leroy @ 2018-09-18 9:26 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linux-kernel, linuxppc-dev
Generic implementation fails to remove breakpoints after init
when CONFIG_STRICT_KERNEL_RWX is selected:
[ 13.251285] KGDB: BP remove failed: c001c338
[ 13.259587] kgdbts: ERROR PUT: end of test buffer on 'do_fork_test' line 8 expected OK got $E14#aa
[ 13.268969] KGDB: re-enter exception: ALL breakpoints killed
[ 13.275099] CPU: 0 PID: 1 Comm: init Not tainted 4.18.0-g82bbb913ffd8 #860
[ 13.282836] Call Trace:
[ 13.285313] [c60e1ba0] [c0080ef0] kgdb_handle_exception+0x6f4/0x720 (unreliable)
[ 13.292618] [c60e1c30] [c000e97c] kgdb_handle_breakpoint+0x3c/0x98
[ 13.298709] [c60e1c40] [c000af54] program_check_exception+0x104/0x700
[ 13.305083] [c60e1c60] [c000e45c] ret_from_except_full+0x0/0x4
[ 13.310845] [c60e1d20] [c02a22ac] run_simple_test+0x2b4/0x2d4
[ 13.316532] [c60e1d30] [c0081698] put_packet+0xb8/0x158
[ 13.321694] [c60e1d60] [c00820b4] gdb_serial_stub+0x230/0xc4c
[ 13.327374] [c60e1dc0] [c0080af8] kgdb_handle_exception+0x2fc/0x720
[ 13.333573] [c60e1e50] [c000e928] kgdb_singlestep+0xb4/0xcc
[ 13.339068] [c60e1e70] [c000ae1c] single_step_exception+0x90/0xac
[ 13.345100] [c60e1e80] [c000e45c] ret_from_except_full+0x0/0x4
[ 13.350865] [c60e1f40] [c000e11c] ret_from_syscall+0x0/0x38
[ 13.356346] Kernel panic - not syncing: Recursive entry to debugger
This patch creates powerpc specific version of
kgdb_arch_set_breakpoint() and kgdb_arch_remove_breakpoint()
using patch_instruction()
Fixes: 1e0fc9d1eb2b ("powerpc/Kconfig: Enable STRICT_KERNEL_RWX for some configs")
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/include/asm/kgdb.h | 5 ++++-
arch/powerpc/kernel/kgdb.c | 43 +++++++++++++++++++++++++++++++++--------
2 files changed, 39 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/include/asm/kgdb.h b/arch/powerpc/include/asm/kgdb.h
index 9db24e77b9f4..a9e098a3b881 100644
--- a/arch/powerpc/include/asm/kgdb.h
+++ b/arch/powerpc/include/asm/kgdb.h
@@ -26,9 +26,12 @@
#define BREAK_INSTR_SIZE 4
#define BUFMAX ((NUMREGBYTES * 2) + 512)
#define OUTBUFMAX ((NUMREGBYTES * 2) + 512)
+
+#define BREAK_INSTR 0x7d821008 /* twge r2, r2 */
+
static inline void arch_kgdb_breakpoint(void)
{
- asm(".long 0x7d821008"); /* twge r2, r2 */
+ asm(stringify_in_c(.long BREAK_INSTR));
}
#define CACHE_FLUSH_IS_SAFE 1
#define DBG_MAX_REG_NUM 70
diff --git a/arch/powerpc/kernel/kgdb.c b/arch/powerpc/kernel/kgdb.c
index 35e240a0a408..59c578f865aa 100644
--- a/arch/powerpc/kernel/kgdb.c
+++ b/arch/powerpc/kernel/kgdb.c
@@ -24,6 +24,7 @@
#include <asm/processor.h>
#include <asm/machdep.h>
#include <asm/debug.h>
+#include <asm/code-patching.h>
#include <linux/slab.h>
/*
@@ -144,7 +145,7 @@ static int kgdb_handle_breakpoint(struct pt_regs *regs)
if (kgdb_handle_exception(1, SIGTRAP, 0, regs) != 0)
return 0;
- if (*(u32 *) (regs->nip) == *(u32 *) (&arch_kgdb_ops.gdb_bpt_instr))
+ if (*(u32 *)regs->nip == BREAK_INSTR)
regs->nip += BREAK_INSTR_SIZE;
return 1;
@@ -441,16 +442,42 @@ int kgdb_arch_handle_exception(int vector, int signo, int err_code,
return -1;
}
+int kgdb_arch_set_breakpoint(struct kgdb_bkpt *bpt)
+{
+ int err;
+ unsigned int instr;
+ unsigned int *addr = (unsigned int *)bpt->bpt_addr;
+
+ err = probe_kernel_address(addr, instr);
+ if (err)
+ return err;
+
+ err = patch_instruction(addr, BREAK_INSTR);
+ if (err)
+ return -EFAULT;
+
+ *(unsigned int *)bpt->saved_instr = instr;
+
+ return 0;
+}
+
+int kgdb_arch_remove_breakpoint(struct kgdb_bkpt *bpt)
+{
+ int err;
+ unsigned int instr = *(unsigned int *)bpt->saved_instr;
+ unsigned int *addr = (unsigned int *)bpt->bpt_addr;
+
+ err = patch_instruction(addr, instr);
+ if (err)
+ return -EFAULT;
+
+ return 0;
+}
+
/*
* Global data
*/
-struct kgdb_arch arch_kgdb_ops = {
-#ifdef __LITTLE_ENDIAN__
- .gdb_bpt_instr = {0x08, 0x10, 0x82, 0x7d},
-#else
- .gdb_bpt_instr = {0x7d, 0x82, 0x10, 0x08},
-#endif
-};
+struct kgdb_arch arch_kgdb_ops;
static int kgdb_not_implemented(struct pt_regs *regs)
{
--
2.13.3
^ permalink raw reply related
* Checkpatch bad Warning (Re: [PATCH] powerpc/kgdb: add kgdb_arch_set/remove_breakpoint())
From: Christophe Leroy @ 2018-09-18 9:33 UTC (permalink / raw)
To: Joe Perches, Andy Whitcroft
Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
linuxppc-dev, linux-kernel
In-Reply-To: <872199441fd43b05fc1c7d049098ef7c0e83f4c5.1537262646.git.christophe.leroy@c-s.fr>
On the below patch, checkpatch reports
WARNING: struct kgdb_arch should normally be const
#127: FILE: arch/powerpc/kernel/kgdb.c:480:
+struct kgdb_arch arch_kgdb_ops;
But when I add 'const', I get compilation failure
CC arch/powerpc/kernel/kgdb.o
arch/powerpc/kernel/kgdb.c:480:24: error: conflicting type qualifiers
for ‘arch_kgdb_ops’
const struct kgdb_arch arch_kgdb_ops;
^
In file included from arch/powerpc/kernel/kgdb.c:18:0:
./include/linux/kgdb.h:284:26: note: previous declaration of
‘arch_kgdb_ops’ was here
extern struct kgdb_arch arch_kgdb_ops;
^
make[1]: *** [arch/powerpc/kernel/kgdb.o] Error 1
Christophe
On 09/18/2018 09:26 AM, Christophe Leroy wrote:
> Generic implementation fails to remove breakpoints after init
> when CONFIG_STRICT_KERNEL_RWX is selected:
>
> [ 13.251285] KGDB: BP remove failed: c001c338
> [ 13.259587] kgdbts: ERROR PUT: end of test buffer on 'do_fork_test' line 8 expected OK got $E14#aa
> [ 13.268969] KGDB: re-enter exception: ALL breakpoints killed
> [ 13.275099] CPU: 0 PID: 1 Comm: init Not tainted 4.18.0-g82bbb913ffd8 #860
> [ 13.282836] Call Trace:
> [ 13.285313] [c60e1ba0] [c0080ef0] kgdb_handle_exception+0x6f4/0x720 (unreliable)
> [ 13.292618] [c60e1c30] [c000e97c] kgdb_handle_breakpoint+0x3c/0x98
> [ 13.298709] [c60e1c40] [c000af54] program_check_exception+0x104/0x700
> [ 13.305083] [c60e1c60] [c000e45c] ret_from_except_full+0x0/0x4
> [ 13.310845] [c60e1d20] [c02a22ac] run_simple_test+0x2b4/0x2d4
> [ 13.316532] [c60e1d30] [c0081698] put_packet+0xb8/0x158
> [ 13.321694] [c60e1d60] [c00820b4] gdb_serial_stub+0x230/0xc4c
> [ 13.327374] [c60e1dc0] [c0080af8] kgdb_handle_exception+0x2fc/0x720
> [ 13.333573] [c60e1e50] [c000e928] kgdb_singlestep+0xb4/0xcc
> [ 13.339068] [c60e1e70] [c000ae1c] single_step_exception+0x90/0xac
> [ 13.345100] [c60e1e80] [c000e45c] ret_from_except_full+0x0/0x4
> [ 13.350865] [c60e1f40] [c000e11c] ret_from_syscall+0x0/0x38
> [ 13.356346] Kernel panic - not syncing: Recursive entry to debugger
>
> This patch creates powerpc specific version of
> kgdb_arch_set_breakpoint() and kgdb_arch_remove_breakpoint()
> using patch_instruction()
>
> Fixes: 1e0fc9d1eb2b ("powerpc/Kconfig: Enable STRICT_KERNEL_RWX for some configs")
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> arch/powerpc/include/asm/kgdb.h | 5 ++++-
> arch/powerpc/kernel/kgdb.c | 43 +++++++++++++++++++++++++++++++++--------
> 2 files changed, 39 insertions(+), 9 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/kgdb.h b/arch/powerpc/include/asm/kgdb.h
> index 9db24e77b9f4..a9e098a3b881 100644
> --- a/arch/powerpc/include/asm/kgdb.h
> +++ b/arch/powerpc/include/asm/kgdb.h
> @@ -26,9 +26,12 @@
> #define BREAK_INSTR_SIZE 4
> #define BUFMAX ((NUMREGBYTES * 2) + 512)
> #define OUTBUFMAX ((NUMREGBYTES * 2) + 512)
> +
> +#define BREAK_INSTR 0x7d821008 /* twge r2, r2 */
> +
> static inline void arch_kgdb_breakpoint(void)
> {
> - asm(".long 0x7d821008"); /* twge r2, r2 */
> + asm(stringify_in_c(.long BREAK_INSTR));
> }
> #define CACHE_FLUSH_IS_SAFE 1
> #define DBG_MAX_REG_NUM 70
> diff --git a/arch/powerpc/kernel/kgdb.c b/arch/powerpc/kernel/kgdb.c
> index 35e240a0a408..59c578f865aa 100644
> --- a/arch/powerpc/kernel/kgdb.c
> +++ b/arch/powerpc/kernel/kgdb.c
> @@ -24,6 +24,7 @@
> #include <asm/processor.h>
> #include <asm/machdep.h>
> #include <asm/debug.h>
> +#include <asm/code-patching.h>
> #include <linux/slab.h>
>
> /*
> @@ -144,7 +145,7 @@ static int kgdb_handle_breakpoint(struct pt_regs *regs)
> if (kgdb_handle_exception(1, SIGTRAP, 0, regs) != 0)
> return 0;
>
> - if (*(u32 *) (regs->nip) == *(u32 *) (&arch_kgdb_ops.gdb_bpt_instr))
> + if (*(u32 *)regs->nip == BREAK_INSTR)
> regs->nip += BREAK_INSTR_SIZE;
>
> return 1;
> @@ -441,16 +442,42 @@ int kgdb_arch_handle_exception(int vector, int signo, int err_code,
> return -1;
> }
>
> +int kgdb_arch_set_breakpoint(struct kgdb_bkpt *bpt)
> +{
> + int err;
> + unsigned int instr;
> + unsigned int *addr = (unsigned int *)bpt->bpt_addr;
> +
> + err = probe_kernel_address(addr, instr);
> + if (err)
> + return err;
> +
> + err = patch_instruction(addr, BREAK_INSTR);
> + if (err)
> + return -EFAULT;
> +
> + *(unsigned int *)bpt->saved_instr = instr;
> +
> + return 0;
> +}
> +
> +int kgdb_arch_remove_breakpoint(struct kgdb_bkpt *bpt)
> +{
> + int err;
> + unsigned int instr = *(unsigned int *)bpt->saved_instr;
> + unsigned int *addr = (unsigned int *)bpt->bpt_addr;
> +
> + err = patch_instruction(addr, instr);
> + if (err)
> + return -EFAULT;
> +
> + return 0;
> +}
> +
> /*
> * Global data
> */
> -struct kgdb_arch arch_kgdb_ops = {
> -#ifdef __LITTLE_ENDIAN__
> - .gdb_bpt_instr = {0x08, 0x10, 0x82, 0x7d},
> -#else
> - .gdb_bpt_instr = {0x7d, 0x82, 0x10, 0x08},
> -#endif
> -};
> +struct kgdb_arch arch_kgdb_ops;
>
> static int kgdb_not_implemented(struct pt_regs *regs)
> {
>
^ permalink raw reply
* Re: [PATCH] powerpc/pseries: Disable CPU hotplug across migrations
From: Gautham R Shenoy @ 2018-09-18 10:32 UTC (permalink / raw)
To: nfont, ego, tyreld; +Cc: linuxppc-dev
In-Reply-To: <153721164232.32706.4283915467151746975.stgit@ltcalpine2-lp14.aus.stglabs.ibm.com>
Hi Nathan,
On Tue, Sep 18, 2018 at 1:05 AM Nathan Fontenot
<nfont@linux.vnet.ibm.com> wrote:
>
> When performing partition migrations all present CPUs must be online
> as all present CPUs must make the H_JOIN call as part of the migration
> process. Once all present CPUs make the H_JOIN call, one CPU is returned
> to make the rtas call to perform the migration to the destination system.
>
> During testing of migration and changing the SMT state we have found
> instances where CPUs are offlined, as part of the SMT state change,
> before they make the H_JOIN call. This results in a hung system where
> every CPU is either in H_JOIN or offline.
>
> To prevent this this patch disables CPU hotplug during the migration
> process.
>
> Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
> ---
> arch/powerpc/kernel/rtas.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index 8afd146bc9c7..2c7ed31c736e 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -981,6 +981,7 @@ int rtas_ibm_suspend_me(u64 handle)
> goto out;
> }
>
> + cpu_hotplug_disable();
So, some of the onlined CPUs ( via
rtas_online_cpus_mask(offline_mask);) can go still offline,
if the userspace issues an offline command, just before we execute
cpu_hotplug_disable().
So we are narrowing down the race, but it still exists. Am I missing something ?
> stop_topology_update();
>
> /* Call function on all CPUs. One of us will make the
> @@ -995,6 +996,7 @@ int rtas_ibm_suspend_me(u64 handle)
> printk(KERN_ERR "Error doing global join\n");
>
> start_topology_update();
> + cpu_hotplug_enable();
>
> /* Take down CPUs not online prior to suspend */
> cpuret = rtas_offline_cpus_mask(offline_mask);
>
--
Thanks and Regards
gautham.
^ permalink raw reply
* Re: [PATCH] powerpc/makefile: remove check on obsolete GCC versions
From: Michael Ellerman @ 2018-09-18 10:50 UTC (permalink / raw)
To: Christophe LEROY, Joel Stanley
Cc: Benjamin Herrenschmidt, Paul Mackerras, linuxppc-dev,
Linux Kernel Mailing List
In-Reply-To: <1ede3510-df2a-183a-dd4b-3276041d8910@c-s.fr>
Christophe LEROY <christophe.leroy@c-s.fr> writes:
> Le 18/09/2018 =C3=A0 07:48, Joel Stanley a =C3=A9crit=C2=A0:
>> Hey Christophe,
>>=20
>> On Tue, 18 Sep 2018 at 15:13, Christophe Leroy <christophe.leroy@c-s.fr>=
wrote:
>>>
>>> Since commit cafa0010cd51 ("Raise the minimum required gcc version
>>> to 4.6"), it is not possible to build kernel with GCC lower than 4.6
>>>
>>> This patch removes checkbin tests addressing older versions of GCC.
>>=20
>> This is the same as Nick's patch:
>>=20
>> https://patchwork.ozlabs.org/patch/969624/
>>=20
>
> Oops, thanks, I missed that.
>
> And even before Nick's, there is this one=20
> https://patchwork.ozlabs.org/patch/962319/
>
> So I missed twice :(
Haha, I missed that one too.
Everyone loves deleting code ;)
I have Nick's patch queued.
cheers
^ permalink raw reply
* Re: [PATCH v2 2/5] powerpc/boot: Fix crt0.S syntax for clang
From: Michael Ellerman @ 2018-09-18 10:53 UTC (permalink / raw)
To: Joel Stanley, Nick Desaulniers; +Cc: linuxppc-dev
In-Reply-To: <CACPK8XeWojwrQ08gzr7=YgCKFfgLt3H81V699W=9FYyyF32UZw@mail.gmail.com>
Joel Stanley <joel@jms.id.au> writes:
> On Tue, 18 Sep 2018 at 06:11, Nick Desaulniers <ndesaulniers@google.com> wrote:
>>
>> On Fri, Sep 14, 2018 at 2:08 PM Segher Boessenkool
>> <segher@kernel.crashing.org> wrote:
>> >
>> > On Fri, Sep 14, 2018 at 10:47:08AM -0700, Nick Desaulniers wrote:
>> > > On Thu, Sep 13, 2018 at 9:07 PM Joel Stanley <joel@jms.id.au> wrote:
>> > > > 10: addis r12,r12,(-RELACOUNT)@ha
>> > > > - cmpdi r12,RELACOUNT@l
>> > > > + cmpdi r12,(RELACOUNT)@l
>> > >
>> > > Yep, as we can see above, when RELACOUNT is negated, it's wrapped in
>> > > parens.
>>
>> Looks like this was just fixed in Clang-8:
>> https://bugs.llvm.org/show_bug.cgi?id=38945
>> https://reviews.llvm.org/D52188
>
> Nice!
>
> mpe, given we need the local references to labels fix which is also in
> clang-8 I suggest we drop this patch.
OK, no worries.
cheers
^ permalink raw reply
* Re: [PATCH v4] powerpc: Avoid code patching freed init sections
From: Michal Suchánek @ 2018-09-18 11:35 UTC (permalink / raw)
To: Christophe LEROY
Cc: Nicholas Piggin, Michael Neuling, linuxppc-dev, Haren Myneni
In-Reply-To: <2d775c16-9dc9-efa4-f3fb-6a0f16297a3a@c-s.fr>
On Tue, 18 Sep 2018 10:52:09 +0200
Christophe LEROY <christophe.leroy@c-s.fr> wrote:
>=20
>=20
> Le 14/09/2018 =C3=A0 06:22, Nicholas Piggin a =C3=A9crit=C2=A0:
> > On Fri, 14 Sep 2018 11:14:11 +1000
> > Michael Neuling <mikey@neuling.org> wrote:
> >=20
> >> This stops us from doing code patching in init sections after
> >> they've been freed.
> >>
> >> In this chain:
> >> kvm_guest_init() ->
> >> kvm_use_magic_page() ->
> >> fault_in_pages_readable() ->
> >> __get_user() ->
> >> __get_user_nocheck() ->
> >> barrier_nospec();
> >>
> >> We have a code patching location at barrier_nospec() and
> >> kvm_guest_init() is an init function. This whole chain gets
> >> inlined, so when we free the init section (hence
> >> kvm_guest_init()), this code goes away and hence should no longer
> >> be patched.
> >>
> >> We seen this as userspace memory corruption when using a memory
> >> checker while doing partition migration testing on powervm (this
> >> starts the code patching post migration via
> >> /sys/kernel/mobility/migration). In theory, it could also happen
> >> when using /sys/kernel/debug/powerpc/barrier_nospec.
> >>
> >> cc: stable@vger.kernel.org # 4.13+
> >> Signed-off-by: Michael Neuling <mikey@neuling.org>
> >>
> >> ---
> >> For stable I've marked this as v4.13+ since that's when we
> >> refactored code-patching.c but it could go back even further than
> >> that. In reality though, I think we can only hit this since the
> >> first spectre/meltdown changes.
> >>
> >> v4:
> >> Feedback from Christophe Leroy:
> >> - init_mem_free -> init_mem_is_free
> >> - prlog %lx -> %px
> >>
> >> v3:
> >> Add init_mem_free flag to avoid potential race.
> >> Feedback from Christophe Leroy:
> >> - use init_section_contains()
> >> - change order of init test for performance
> >> - use pr_debug()
> >> - remove blank line
> >>
> >> v2:
> >> Print when we skip an address
> >> ---
> >> arch/powerpc/include/asm/setup.h | 1 +
> >> arch/powerpc/lib/code-patching.c | 6 ++++++
> >> arch/powerpc/mm/mem.c | 2 ++
> >> 3 files changed, 9 insertions(+)
> >>
> >> diff --git a/arch/powerpc/include/asm/setup.h
> >> b/arch/powerpc/include/asm/setup.h index 1a951b0046..1fffbba8d6
> >> 100644 --- a/arch/powerpc/include/asm/setup.h
> >> +++ b/arch/powerpc/include/asm/setup.h
> >> @@ -9,6 +9,7 @@ extern void ppc_printk_progress(char *s, unsigned
> >> short hex);=20
> >> extern unsigned int rtas_data;
> >> extern unsigned long long memory_limit;
> >> +extern bool init_mem_is_free;
> >> extern unsigned long klimit;
> >> extern void *zalloc_maybe_bootmem(size_t size, gfp_t mask);
> >> =20
> >> diff --git a/arch/powerpc/lib/code-patching.c
> >> b/arch/powerpc/lib/code-patching.c index 850f3b8f4d..6ae2777c22
> >> 100644 --- a/arch/powerpc/lib/code-patching.c
> >> +++ b/arch/powerpc/lib/code-patching.c
> >> @@ -28,6 +28,12 @@ static int __patch_instruction(unsigned int
> >> *exec_addr, unsigned int instr, {
> >> int err;
> >> =20
> >> + /* Make sure we aren't patching a freed init section */
> >> + if (init_mem_is_free && init_section_contains(exec_addr,
> >> 4)) {
> >> + pr_debug("Skipping init section patching addr:
> >> 0x%px\n", exec_addr);
> >> + return 0;
> >> + }
> >=20
> > What we should do is a whitelist, make sure it's only patching the
> > sections we want it to.
> >=20
> > That's a bigger job when you consider modules and things too though,
> > so this looks good for now. Thanks,
>=20
> What about using kernel_text_address() for it then ? It also handles=20
> modules, is it more complicated than that ?
Modules are patched separately so should not need to be excluded here.
There is a different problem with modules: when the mitigation type
changes the modules are not re-patched with the new settings.
Thanks
Michal
^ permalink raw reply
* Re: How to handle PTE tables with non contiguous entries ?
From: Aneesh Kumar K.V @ 2018-09-18 11:47 UTC (permalink / raw)
To: Christophe LEROY, akpm, linux-mm, aneesh.kumar, Nicholas Piggin,
Michael Ellerman, linuxppc-dev
Cc: LKML
In-Reply-To: <d1be61a4-8dc7-cfe0-e4e7-82ce5f57ced3@c-s.fr>
Christophe LEROY <christophe.leroy@c-s.fr> writes:
> Le 17/09/2018 =C3=A0 11:03, Aneesh Kumar K.V a =C3=A9crit=C2=A0:
>> Christophe Leroy <christophe.leroy@c-s.fr> writes:
>>=20
>>> Hi,
>>>
>>> I'm having a hard time figuring out the best way to handle the following
>>> situation:
>>>
>>> On the powerpc8xx, handling 16k size pages requires to have page tables
>>> with 4 identical entries.
>>=20
>> I assume that hugetlb page size? If so isn't that similar to FSL hugetlb
>> page table layout?
>
> No, it is not for 16k hugepage size with a standard page size of 4k.
>
> Here I'm trying to handle the case of CONFIG_PPC_16K_PAGES.
> As of today, it is implemented by using the standard Linux page layout,=20
> ie one PTE entry for each 16k page. This forbids the use the 8xx HW=20
> assistance.
>
>>=20
>>>
>>> Initially I was thinking about handling this by simply modifying
>>> pte_index() which changing pte_t type in order to have one entry every
>>> 16 bytes, then replicate the PTE value at *ptep, *ptep+1,*ptep+2 and
>>> *ptep+3 both in set_pte_at() and pte_update().
>>>
>>> However, this doesn't work because many many places in the mm core part
>>> of the kernel use loops on ptep with single ptep++ increment.
>>>
>>> Therefore did it with the following hack:
>>>
>>> /* PTE level */
>>> +#if defined(CONFIG_PPC_8xx) && defined(CONFIG_PPC_16K_PAGES)
>>> +typedef struct { pte_basic_t pte, pte1, pte2, pte3; } pte_t;
>>> +#else
>>> typedef struct { pte_basic_t pte; } pte_t;
>>> +#endif
>>>
>>> @@ -181,7 +192,13 @@ static inline unsigned long pte_update(pte_t *p,
>>> : "cc" );
>>> #else /* PTE_ATOMIC_UPDATES */
>>> unsigned long old =3D pte_val(*p);
>>> - *p =3D __pte((old & ~clr) | set);
>>> + unsigned long new =3D (old & ~clr) | set;
>>> +
>>> +#if defined(CONFIG_PPC_8xx) && defined(CONFIG_PPC_16K_PAGES)
>>> + p->pte =3D p->pte1 =3D p->pte2 =3D p->pte3 =3D new;
>>> +#else
>>> + *p =3D __pte(new);
>>> +#endif
>>> #endif /* !PTE_ATOMIC_UPDATES */
>>>
>>> #ifdef CONFIG_44x
>>>
>>>
>>> @@ -161,7 +161,11 @@ static inline void __set_pte_at(struct mm_struct
>>> *mm, unsigned long addr,
>>> /* Anything else just stores the PTE normally. That covers all
>>> 64-bit
>>> * cases, and 32-bit non-hash with 32-bit PTEs.
>>> */
>>> +#if defined(CONFIG_PPC_8xx) && defined(CONFIG_PPC_16K_PAGES)
>>> + ptep->pte =3D ptep->pte1 =3D ptep->pte2 =3D ptep->pte3 =3D pte_=
val(pte);
>>> +#else
>>> *ptep =3D pte;
>>> +#endif
>>>
>>>
>>>
>>> But I'm not too happy with it as it means pte_t is not a single type
>>> anymore so passing it from one function to the other is quite heavy.
>>>
>>>
>>> Would someone have an idea of an elegent way to handle that ?
>>>
>>> Thanks
>>> Christophe
>>=20
>> Why would pte_update bother about updating all the 4 entries?. Can you
>> help me understand the issue?
>
> Because the 8xx HW assistance expects 4 identical entries for each 16k=20
> page, so everytime a PTE is updated the 4 entries have to be updated.
>
What you suggested in the original mail is what matches that best isn't it?
That is a linux pte update involves updating 4 slot. Hence a linux pte
consist of 4 unsigned long?
-aneesh
^ permalink raw reply
* [PATCH v1 0/6] mm: online/offline_pages called w.o. mem_hotplug_lock
From: David Hildenbrand @ 2018-09-18 11:48 UTC (permalink / raw)
To: linux-mm
Cc: linux-kernel, linux-doc, linuxppc-dev, linux-acpi, xen-devel,
devel, David Hildenbrand, Andrew Morton, Balbir Singh,
Benjamin Herrenschmidt, Boris Ostrovsky, Dan Williams,
Greg Kroah-Hartman, Haiyang Zhang, Heiko Carstens, John Allen,
Jonathan Corbet, Joonsoo Kim, Juergen Gross, Kate Stewart,
K. Y. Srinivasan, Len Brown, Martin Schwidefsky,
Mathieu Malaterre, Michael Ellerman, Michael Neuling,
Michal Hocko, Nathan Fontenot, Oscar Salvador, Paul Mackerras,
Pavel Tatashin, Pavel Tatashin, Philippe Ombredanne,
Rafael J. Wysocki, Rashmica Gupta, Stephen Hemminger,
Thomas Gleixner, Vlastimil Babka, YASUAKI ISHIMATSU
Reading through the code and studying how mem_hotplug_lock is to be used,
I noticed that there are two places where we can end up calling
device_online()/device_offline() - online_pages()/offline_pages() without
the mem_hotplug_lock. And there are other places where we call
device_online()/device_offline() without the device_hotplug_lock.
While e.g.
echo "online" > /sys/devices/system/memory/memory9/state
is fine, e.g.
echo 1 > /sys/devices/system/memory/memory9/online
Will not take the mem_hotplug_lock. However the device_lock() and
device_hotplug_lock.
E.g. via memory_probe_store(), we can end up calling
add_memory()->online_pages() without the device_hotplug_lock. So we can
have concurrent callers in online_pages(). We e.g. touch in online_pages()
basically unprotected zone->present_pages then.
Looks like there is a longer history to that (see Patch #2 for details),
and fixing it to work the way it was intended is not really possible. We
would e.g. have to take the mem_hotplug_lock in device/base/core.c, which
sounds wrong.
Summary: We had a lock inversion on mem_hotplug_lock and device_lock().
More details can be found in patch 3 and patch 6.
I propose the general rules (documentation added in patch 6):
1. add_memory/add_memory_resource() must only be called with
device_hotplug_lock.
2. remove_memory() must only be called with device_hotplug_lock. This is
already documented and holds for all callers.
3. device_online()/device_offline() must only be called with
device_hotplug_lock. This is already documented and true for now in core
code. Other callers (related to memory hotplug) have to be fixed up.
4. mem_hotplug_lock is taken inside of add_memory/remove_memory/
online_pages/offline_pages.
To me, this looks way cleaner than what we have right now (and easier to
verify). And looking at the documentation of remove_memory, using
lock_device_hotplug also for add_memory() feels natural.
RFCv2 -> v1:
- Dropped an unnecessary _ref from remove_memory() in patch #1
- Minor patch description fixes.
- Added rb's
RFC -> RFCv2:
- Don't export device_hotplug_lock, provide proper remove_memory/add_memory
wrappers.
- Split up the patches a bit.
- Try to improve powernv memtrace locking
- Add some documentation for locking that matches my knowledge
David Hildenbrand (6):
mm/memory_hotplug: make remove_memory() take the device_hotplug_lock
mm/memory_hotplug: make add_memory() take the device_hotplug_lock
mm/memory_hotplug: fix online/offline_pages called w.o.
mem_hotplug_lock
powerpc/powernv: hold device_hotplug_lock when calling device_online()
powerpc/powernv: hold device_hotplug_lock in memtrace_offline_pages()
memory-hotplug.txt: Add some details about locking internals
Documentation/memory-hotplug.txt | 39 +++++++++++-
arch/powerpc/platforms/powernv/memtrace.c | 14 +++--
.../platforms/pseries/hotplug-memory.c | 8 +--
drivers/acpi/acpi_memhotplug.c | 4 +-
drivers/base/memory.c | 22 +++----
drivers/xen/balloon.c | 3 +
include/linux/memory_hotplug.h | 4 +-
mm/memory_hotplug.c | 59 +++++++++++++++----
8 files changed, 115 insertions(+), 38 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH v1 1/6] mm/memory_hotplug: make remove_memory() take the device_hotplug_lock
From: David Hildenbrand @ 2018-09-18 11:48 UTC (permalink / raw)
To: linux-mm
Cc: linux-kernel, linux-doc, linuxppc-dev, linux-acpi, xen-devel,
devel, David Hildenbrand, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, Rafael J. Wysocki, Len Brown, Rashmica Gupta,
Michael Neuling, Balbir Singh, Nathan Fontenot, John Allen,
Andrew Morton, Michal Hocko, Dan Williams, Joonsoo Kim,
Vlastimil Babka, Pavel Tatashin, Greg Kroah-Hartman,
Oscar Salvador, YASUAKI ISHIMATSU, Mathieu Malaterre
In-Reply-To: <20180918114822.21926-1-david@redhat.com>
remove_memory() is exported right now but requires the
device_hotplug_lock, which is not exported. So let's provide a variant
that takes the lock and only export that one.
The lock is already held in
arch/powerpc/platforms/pseries/hotplug-memory.c
drivers/acpi/acpi_memhotplug.c
So, let's use the locked variant.
The lock is not held (but taken in)
arch/powerpc/platforms/powernv/memtrace.c
So let's keep using the (now) locked variant.
Apart from that, there are not other users in the tree.
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Len Brown <lenb@kernel.org>
Cc: Rashmica Gupta <rashmica.g@gmail.com>
Cc: Michael Neuling <mikey@neuling.org>
Cc: Balbir Singh <bsingharora@gmail.com>
Cc: Nathan Fontenot <nfont@linux.vnet.ibm.com>
Cc: John Allen <jallen@linux.vnet.ibm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Pavel Tatashin <pasha.tatashin@oracle.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: YASUAKI ISHIMATSU <yasu.isimatu@gmail.com>
Cc: Mathieu Malaterre <malat@debian.org>
Reviewed-by: Pavel Tatashin <pavel.tatashin@microsoft.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
arch/powerpc/platforms/powernv/memtrace.c | 2 --
arch/powerpc/platforms/pseries/hotplug-memory.c | 6 +++---
drivers/acpi/acpi_memhotplug.c | 2 +-
include/linux/memory_hotplug.h | 3 ++-
mm/memory_hotplug.c | 9 ++++++++-
5 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/memtrace.c b/arch/powerpc/platforms/powernv/memtrace.c
index 51dc398ae3f7..8f1cd4f3bfd5 100644
--- a/arch/powerpc/platforms/powernv/memtrace.c
+++ b/arch/powerpc/platforms/powernv/memtrace.c
@@ -90,9 +90,7 @@ static bool memtrace_offline_pages(u32 nid, u64 start_pfn, u64 nr_pages)
walk_memory_range(start_pfn, end_pfn, (void *)MEM_OFFLINE,
change_memblock_state);
- lock_device_hotplug();
remove_memory(nid, start_pfn << PAGE_SHIFT, nr_pages << PAGE_SHIFT);
- unlock_device_hotplug();
return true;
}
diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
index c1578f54c626..b3f54466e25f 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -334,7 +334,7 @@ static int pseries_remove_memblock(unsigned long base, unsigned int memblock_siz
nid = memory_add_physaddr_to_nid(base);
for (i = 0; i < sections_per_block; i++) {
- remove_memory(nid, base, MIN_MEMORY_BLOCK_SIZE);
+ __remove_memory(nid, base, MIN_MEMORY_BLOCK_SIZE);
base += MIN_MEMORY_BLOCK_SIZE;
}
@@ -423,7 +423,7 @@ static int dlpar_remove_lmb(struct drmem_lmb *lmb)
block_sz = pseries_memory_block_size();
nid = memory_add_physaddr_to_nid(lmb->base_addr);
- remove_memory(nid, lmb->base_addr, block_sz);
+ __remove_memory(nid, lmb->base_addr, block_sz);
/* Update memory regions for memory remove */
memblock_remove(lmb->base_addr, block_sz);
@@ -710,7 +710,7 @@ static int dlpar_add_lmb(struct drmem_lmb *lmb)
rc = dlpar_online_lmb(lmb);
if (rc) {
- remove_memory(nid, lmb->base_addr, block_sz);
+ __remove_memory(nid, lmb->base_addr, block_sz);
dlpar_remove_device_tree_lmb(lmb);
} else {
lmb->flags |= DRCONF_MEM_ASSIGNED;
diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
index 6b0d3ef7309c..811148415993 100644
--- a/drivers/acpi/acpi_memhotplug.c
+++ b/drivers/acpi/acpi_memhotplug.c
@@ -282,7 +282,7 @@ static void acpi_memory_remove_memory(struct acpi_memory_device *mem_device)
nid = memory_add_physaddr_to_nid(info->start_addr);
acpi_unbind_memory_blocks(info);
- remove_memory(nid, info->start_addr, info->length);
+ __remove_memory(nid, info->start_addr, info->length);
list_del(&info->list);
kfree(info);
}
diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index 34a28227068d..1f096852f479 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -301,6 +301,7 @@ extern bool is_mem_section_removable(unsigned long pfn, unsigned long nr_pages);
extern void try_offline_node(int nid);
extern int offline_pages(unsigned long start_pfn, unsigned long nr_pages);
extern void remove_memory(int nid, u64 start, u64 size);
+extern void __remove_memory(int nid, u64 start, u64 size);
#else
static inline bool is_mem_section_removable(unsigned long pfn,
@@ -317,6 +318,7 @@ static inline int offline_pages(unsigned long start_pfn, unsigned long nr_pages)
}
static inline void remove_memory(int nid, u64 start, u64 size) {}
+static inline void __remove_memory(int nid, u64 start, u64 size) {}
#endif /* CONFIG_MEMORY_HOTREMOVE */
extern void __ref free_area_init_core_hotplug(int nid);
@@ -330,7 +332,6 @@ extern void move_pfn_range_to_zone(struct zone *zone, unsigned long start_pfn,
unsigned long nr_pages, struct vmem_altmap *altmap);
extern int offline_pages(unsigned long start_pfn, unsigned long nr_pages);
extern bool is_memblock_offlined(struct memory_block *mem);
-extern void remove_memory(int nid, u64 start, u64 size);
extern int sparse_add_one_section(struct pglist_data *pgdat,
unsigned long start_pfn, struct vmem_altmap *altmap);
extern void sparse_remove_one_section(struct zone *zone, struct mem_section *ms,
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 38d94b703e9d..b8b1bd970322 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1873,7 +1873,7 @@ EXPORT_SYMBOL(try_offline_node);
* and online/offline operations before this call, as required by
* try_offline_node().
*/
-void __ref remove_memory(int nid, u64 start, u64 size)
+void __ref __remove_memory(int nid, u64 start, u64 size)
{
int ret;
@@ -1902,5 +1902,12 @@ void __ref remove_memory(int nid, u64 start, u64 size)
mem_hotplug_done();
}
+
+void remove_memory(int nid, u64 start, u64 size)
+{
+ lock_device_hotplug();
+ __remove_memory(nid, start, size);
+ unlock_device_hotplug();
+}
EXPORT_SYMBOL_GPL(remove_memory);
#endif /* CONFIG_MEMORY_HOTREMOVE */
--
2.17.1
^ permalink raw reply related
* [PATCH v1 2/6] mm/memory_hotplug: make add_memory() take the device_hotplug_lock
From: David Hildenbrand @ 2018-09-18 11:48 UTC (permalink / raw)
To: linux-mm
Cc: linux-kernel, linux-doc, linuxppc-dev, linux-acpi, xen-devel,
devel, David Hildenbrand, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, Rafael J. Wysocki, Len Brown,
Greg Kroah-Hartman, Boris Ostrovsky, Juergen Gross,
Nathan Fontenot, John Allen, Andrew Morton, Michal Hocko,
Dan Williams, Joonsoo Kim, Vlastimil Babka, Oscar Salvador,
Mathieu Malaterre, Pavel Tatashin, YASUAKI ISHIMATSU
In-Reply-To: <20180918114822.21926-1-david@redhat.com>
add_memory() currently does not take the device_hotplug_lock, however
is aleady called under the lock from
arch/powerpc/platforms/pseries/hotplug-memory.c
drivers/acpi/acpi_memhotplug.c
to synchronize against CPU hot-remove and similar.
In general, we should hold the device_hotplug_lock when adding memory
to synchronize against online/offline request (e.g. from user space) -
which already resulted in lock inversions due to device_lock() and
mem_hotplug_lock - see 30467e0b3be ("mm, hotplug: fix concurrent memory
hot-add deadlock"). add_memory()/add_memory_resource() will create memory
block devices, so this really feels like the right thing to do.
Holding the device_hotplug_lock makes sure that a memory block device
can really only be accessed (e.g. via .online/.state) from user space,
once the memory has been fully added to the system.
The lock is not held yet in
drivers/xen/balloon.c
arch/powerpc/platforms/powernv/memtrace.c
drivers/s390/char/sclp_cmd.c
drivers/hv/hv_balloon.c
So, let's either use the locked variants or take the lock.
Don't export add_memory_resource(), as it once was exported to be used
by XEN, which is never built as a module. If somebody requires it, we
also have to export a locked variant (as device_hotplug_lock is never
exported).
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Len Brown <lenb@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Nathan Fontenot <nfont@linux.vnet.ibm.com>
Cc: John Allen <jallen@linux.vnet.ibm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: Mathieu Malaterre <malat@debian.org>
Cc: Pavel Tatashin <pavel.tatashin@microsoft.com>
Cc: YASUAKI ISHIMATSU <yasu.isimatu@gmail.com>
Reviewed-by: Pavel Tatashin <pavel.tatashin@microsoft.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
.../platforms/pseries/hotplug-memory.c | 2 +-
drivers/acpi/acpi_memhotplug.c | 2 +-
drivers/base/memory.c | 9 ++++++--
drivers/xen/balloon.c | 3 +++
include/linux/memory_hotplug.h | 1 +
mm/memory_hotplug.c | 22 ++++++++++++++++---
6 files changed, 32 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
index b3f54466e25f..2e6f41dc103a 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -702,7 +702,7 @@ static int dlpar_add_lmb(struct drmem_lmb *lmb)
nid = memory_add_physaddr_to_nid(lmb->base_addr);
/* Add the memory */
- rc = add_memory(nid, lmb->base_addr, block_sz);
+ rc = __add_memory(nid, lmb->base_addr, block_sz);
if (rc) {
dlpar_remove_device_tree_lmb(lmb);
return rc;
diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c
index 811148415993..8fe0960ea572 100644
--- a/drivers/acpi/acpi_memhotplug.c
+++ b/drivers/acpi/acpi_memhotplug.c
@@ -228,7 +228,7 @@ static int acpi_memory_enable_device(struct acpi_memory_device *mem_device)
if (node < 0)
node = memory_add_physaddr_to_nid(info->start_addr);
- result = add_memory(node, info->start_addr, info->length);
+ result = __add_memory(node, info->start_addr, info->length);
/*
* If the memory block has been used by the kernel, add_memory()
diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index 817320c7c4c1..40cac122ec73 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -519,15 +519,20 @@ memory_probe_store(struct device *dev, struct device_attribute *attr,
if (phys_addr & ((pages_per_block << PAGE_SHIFT) - 1))
return -EINVAL;
+ ret = lock_device_hotplug_sysfs();
+ if (ret)
+ goto out;
+
nid = memory_add_physaddr_to_nid(phys_addr);
- ret = add_memory(nid, phys_addr,
- MIN_MEMORY_BLOCK_SIZE * sections_per_block);
+ ret = __add_memory(nid, phys_addr,
+ MIN_MEMORY_BLOCK_SIZE * sections_per_block);
if (ret)
goto out;
ret = count;
out:
+ unlock_device_hotplug();
return ret;
}
diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index e12bb256036f..6bab019a82b1 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -395,7 +395,10 @@ static enum bp_state reserve_additional_memory(void)
* callers drop the mutex before trying again.
*/
mutex_unlock(&balloon_mutex);
+ /* add_memory_resource() requires the device_hotplug lock */
+ lock_device_hotplug();
rc = add_memory_resource(nid, resource, memhp_auto_online);
+ unlock_device_hotplug();
mutex_lock(&balloon_mutex);
if (rc) {
diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index 1f096852f479..ffd9cd10fcf3 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -324,6 +324,7 @@ static inline void __remove_memory(int nid, u64 start, u64 size) {}
extern void __ref free_area_init_core_hotplug(int nid);
extern int walk_memory_range(unsigned long start_pfn, unsigned long end_pfn,
void *arg, int (*func)(struct memory_block *, void *));
+extern int __add_memory(int nid, u64 start, u64 size);
extern int add_memory(int nid, u64 start, u64 size);
extern int add_memory_resource(int nid, struct resource *resource, bool online);
extern int arch_add_memory(int nid, u64 start, u64 size,
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index b8b1bd970322..ef5444145c88 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1111,7 +1111,12 @@ static int online_memory_block(struct memory_block *mem, void *arg)
return device_online(&mem->dev);
}
-/* we are OK calling __meminit stuff here - we have CONFIG_MEMORY_HOTPLUG */
+/*
+ * NOTE: The caller must call lock_device_hotplug() to serialize hotplug
+ * and online/offline operations (triggered e.g. by sysfs).
+ *
+ * we are OK calling __meminit stuff here - we have CONFIG_MEMORY_HOTPLUG
+ */
int __ref add_memory_resource(int nid, struct resource *res, bool online)
{
u64 start, size;
@@ -1180,9 +1185,9 @@ int __ref add_memory_resource(int nid, struct resource *res, bool online)
mem_hotplug_done();
return ret;
}
-EXPORT_SYMBOL_GPL(add_memory_resource);
-int __ref add_memory(int nid, u64 start, u64 size)
+/* requires device_hotplug_lock, see add_memory_resource() */
+int __ref __add_memory(int nid, u64 start, u64 size)
{
struct resource *res;
int ret;
@@ -1196,6 +1201,17 @@ int __ref add_memory(int nid, u64 start, u64 size)
release_memory_resource(res);
return ret;
}
+
+int add_memory(int nid, u64 start, u64 size)
+{
+ int rc;
+
+ lock_device_hotplug();
+ rc = __add_memory(nid, start, size);
+ unlock_device_hotplug();
+
+ return rc;
+}
EXPORT_SYMBOL_GPL(add_memory);
#ifdef CONFIG_MEMORY_HOTREMOVE
--
2.17.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