[PATCH net-next 2/6] net: dsa: return after mdb prepare phase
The current code does not return after successfully preparing the MDB addition on every ports member of a multicast group. Fix this. Fixes: a1a6b7ea7f2d ("net: dsa: add cross-chip multicast support") Reported-by: Egil Hjelmeland Signed-off-by: Vivien Didelot --- net/dsa/switch.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/net/dsa/switch.c b/net/dsa/switch.c index 04e8ad2c3d5d..0041aba5c339 100644 --- a/net/dsa/switch.c +++ b/net/dsa/switch.c @@ -133,6 +133,8 @@ static int dsa_switch_mdb_add(struct dsa_switch *ds, if (err) return err; } + + return 0; } for_each_set_bit(port, group, ds->num_ports) -- 2.15.0
[PATCH net-next 6/6] net: dsa: add switch vlan bitmap functions
This patch brings no functional changes. It moves out the VLAN code iterating on a list of VLAN members into new dsa_switch_vlan_{prepare,add}_bitmap() functions. This gives us a better isolation of the two switchdev phases. Signed-off-by: Vivien Didelot --- net/dsa/switch.c | 49 ++--- 1 file changed, 34 insertions(+), 15 deletions(-) diff --git a/net/dsa/switch.c b/net/dsa/switch.c index 0ed60456b669..fa99624f76ab 100644 --- a/net/dsa/switch.c +++ b/net/dsa/switch.c @@ -175,13 +175,43 @@ static int dsa_switch_mdb_del(struct dsa_switch *ds, return 0; } +static int +dsa_switch_vlan_prepare_bitmap(struct dsa_switch *ds, + const struct switchdev_obj_port_vlan *vlan, + const unsigned long *bitmap) +{ + int port, err; + + if (!ds->ops->port_vlan_prepare || !ds->ops->port_vlan_add) + return -EOPNOTSUPP; + + for_each_set_bit(port, bitmap, ds->num_ports) { + err = ds->ops->port_vlan_prepare(ds, port, vlan); + if (err) + return err; + } + + return 0; +} + +static void +dsa_switch_vlan_add_bitmap(struct dsa_switch *ds, + const struct switchdev_obj_port_vlan *vlan, + const unsigned long *bitmap) +{ + int port; + + for_each_set_bit(port, bitmap, ds->num_ports) + ds->ops->port_vlan_add(ds, port, vlan); +} + static int dsa_switch_vlan_add(struct dsa_switch *ds, struct dsa_notifier_vlan_info *info) { const struct switchdev_obj_port_vlan *vlan = info->vlan; struct switchdev_trans *trans = info->trans; DECLARE_BITMAP(members, ds->num_ports); - int port, err; + int port; /* Build a mask of VLAN members */ bitmap_zero(members, ds->num_ports); @@ -191,21 +221,10 @@ static int dsa_switch_vlan_add(struct dsa_switch *ds, if (dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port)) set_bit(port, members); - if (switchdev_trans_ph_prepare(trans)) { - if (!ds->ops->port_vlan_prepare || !ds->ops->port_vlan_add) - return -EOPNOTSUPP; + if (switchdev_trans_ph_prepare(trans)) + return dsa_switch_vlan_prepare_bitmap(ds, vlan, members); - for_each_set_bit(port, members, ds->num_ports) { - err = ds->ops->port_vlan_prepare(ds, port, vlan); - if (err) - return err; - } - - return 0; - } - - for_each_set_bit(port, members, ds->num_ports) - ds->ops->port_vlan_add(ds, port, vlan); + dsa_switch_vlan_add_bitmap(ds, vlan, members); return 0; } -- 2.15.0
Re: [PATCH 2/2] kbuild: handle dtb-y and CONFIG_OF_ALL_DTBS natively in Makefile.lib
On Sun, Nov 5, 2017 at 7:49 AM, Arnd Bergmann wrote: > On Sun, Nov 5, 2017 at 6:30 AM, Masahiro Yamada > wrote: >> If CONFIG_OF_ALL_DTBS is enabled, "make ARCH=arm64 dtbs" compiles each >> DTB twice; one from arch/arm64/boot/dts/*/Makefile and the other from >> the dtb-$(CONFIG_OF_ALL_DTBS) line in arch/arm64/boot/dts/Makefile. >> It could be a race problem when building DTBS in parallel. >> >> Another minor issue is CONFIG_OF_ALL_DTBS covers only *.dts in vendor >> sub-directories, so this broke when Broadcom added one more hierarchy >> in arch/arm64/boot/dts/broadcom//. >> >> One idea to fix the issues in a clean way is to move DTB handling >> to Kbuild core scripts. Makefile.dtbinst already recognizes dtb-y >> natively, so it should not hurt to do so. >> >> Add $(dtb-y) to extra-y, and $(dtb-) as well if CONFIG_OF_ALL_DTBS is >> enabled. All clutter things in Makefiles go away. >> >> As a bonus clean-up, I also removed dts-dirs. Just use subdir-y >> directly to traverse sub-directories. >> >> Signed-off-by: Masahiro Yamada > > Nice, that's much better than the hack I had. > > Acked-by: Arnd Bergmann I applied this, but it has a few conflicts with SPDX tags from Greg and some board additions in arm-soc. Please take a look. Rob
Re: [PATCH v7 1/6] x86/tsc: remove tsc_disabled flag
> > IMO, we already have a message by > > mark_tsc_unstable("boot parameter notsc"); > > and we will use 'notsc' in case of CONFIG_X86_TSC = no > > So, I guess there is no need to print this msg. > OK, removed the warning.
Re: [tip:x86/asm] x86/umip: Add emulation code for UMIP instructions
On 11/08/2017 06:14 PM, Paolo Bonzini wrote: On 08/11/2017 18:09, Denys Vlasenko wrote: On 11/08/2017 05:57 PM, Linus Torvalds wrote: On Wed, Nov 8, 2017 at 8:53 AM, Denys Vlasenko wrote: We can postpone enabling UMIP by default by a year or so. By this time, new Wine will be on majority of users' machines. So you are suggesting we run unnecessarily insecure, only in order to not do the emulation that we already have the code for and that the patch implements? We ran insecure in this way for ~25 years. Why? To avoid having to maintain more obscure, rarely executed code. As a start, you could propose a patch to disable the emulation code through a sysctl or Kconfig symbol. This way, the emulation code will still be in the kernel, and still need to be maintained. In my opinion, if we do emulate these insns, then adding even more code to disable that emulation is not worth doing. I would be surprised if it takes more time than what you've spent writing emails in this thread. Yes, I not only f**ing retarded, I'm also lazy. Thanks guys...
Re: [PATCH v7 6/6] x86/tsc: use tsc early
Hi Dou, > I have tested it based on tip tree. it is OK for me. Execllent, Thank you very much for spending time testing this project. >> x86_init.timers.timer_init(); >> tsc_init(); >> + tsc_early_fini(); > > > Can we put this into tsc_init(), So we can remove the definitions in > tsc.h Sure, done. >> +static u64 sched_clock_early(void) > > > This function is only called during boot time. Should it > be a "__init" function too? While it is guranteed that this function is never going to be called once system is booted, and we indeed can unload it. I do not think this is possible, because this function is called from sched_clock(), which is not part of __init section. Is there a way to do it and not to have a warning about section missmatch? I will send out new patches with Dou's comments addressed soon. Thank you, Pavel
[PATCH] net: ethernet: bgmac: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Addresses-Coverity-ID: 1397972 Signed-off-by: Gustavo A. R. Silva --- drivers/net/ethernet/broadcom/bgmac-platform.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/ethernet/broadcom/bgmac-platform.c b/drivers/net/ethernet/broadcom/bgmac-platform.c index d937083d..894eda5 100644 --- a/drivers/net/ethernet/broadcom/bgmac-platform.c +++ b/drivers/net/ethernet/broadcom/bgmac-platform.c @@ -131,6 +131,7 @@ static void bgmac_nicpm_speed_set(struct net_device *net_dev) switch (bgmac->net_dev->phydev->speed) { default: netdev_err(net_dev, "Unsupported speed. Defaulting to 1000Mb\n"); + /* fall through */ case SPEED_1000: val |= NICPM_IOMUX_CTRL_SPD_1000M << NICPM_IOMUX_CTRL_SPD_SHIFT; break; -- 2.7.4
[PATCH] Fixes issue with bit shift in rf69_get_modulation
Fixes issue with bit shift in rf69_get_modulation Signed-off-by: Marcus Wolf --- drivers/staging/pi433/rf69.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c index 290b419..c945b4b 100644 --- a/drivers/staging/pi433/rf69.c +++ b/drivers/staging/pi433/rf69.c @@ -102,7 +102,7 @@ enum modulation rf69_get_modulation(struct spi_device *spi) currentValue = READ_REG(REG_DATAMODUL); - switch (currentValue & MASK_DATAMODUL_MODULATION_TYPE >> 3) { // TODO improvement: change 3 to define + switch (currentValue & MASK_DATAMODUL_MODULATION_TYPE) { case DATAMODUL_MODULATION_TYPE_OOK: return OOK; case DATAMODUL_MODULATION_TYPE_FSK: return FSK; default:return undefined; -- 1.7.10.4
Re: [PATCH] rtc: x-gene: mark PM functions as __maybe_unused
Hi, > The new xgene_rtc_alarm_irq_enabled() function is only accessed > from PM code, which is inside of an #ifdef; this causes a harmless > build warning when CONFIG_PM is disabled: > > drivers/rtc/rtc-xgene.c:108:12: error: 'xgene_rtc_alarm_irq_enabled' defined > but not used [-Werror=unused-function] > > Just remove the #ifdef and use __maybe_unused annotations instead, > to make the code more robust here. It sounds like desire to merge in the suspend/resume support as this is in linux-next 14 hours ago. Reviewed-by: Loc Ho -Loc
Re: [tip:x86/asm] x86/umip: Add emulation code for UMIP instructions
On Wed, 8 Nov 2017 17:53:12 +0100 Denys Vlasenko wrote: > On 11/08/2017 05:34 PM, Linus Torvalds wrote: > > On Wed, Nov 8, 2017 at 8:14 AM, Denys Vlasenko wrote: > > > >> > >> Can we avoid maintain emulation of these isns, by asking Wine to remove > >> their use instead? > > > > If we ask the Wine people to remove the instruction use, that may mean > > that we can avoid the emulation in four or five _years_ once everybody > > has updated. > > > > But it wouldn't mean that we could avoid it today. > > We can postpone enabling UMIP by default by a year or so. > By this time, new Wine will be on majority of users' machines. And because Wine compatibility is a bit random at times many users will have multiple wine versiosn at once on their system which they expect to continue to work as they used to. For gaming in particularl that isn't going to change. There are even front ends that let you package up games with the right matching wine they were tested with. Alan
Re: [PATCH v4 01/17] PCI: dwc: Use the DMA-API to get the MSI address
On Wed, Nov 08, 2017 at 12:45:49PM +, Joao Pinto wrote: > Hello to all, > > Às 12:56 AM de 11/8/2017, Bjorn Helgaas escreveu: > > On Fri, Nov 03, 2017 at 02:47:05PM +0100, Niklas Cassel wrote: > >> Use the DMA-API to get the MSI address. This address will be written to > >> our PCI config space and to the register which determines which AXI > >> address the DWC IP will spoof for incoming MSI irqs. > >> > >> Since it is a PCIe endpoint device, rather than the CPU, that is supposed > >> to write to the MSI address, the proper way to get the MSI address is by > >> using the DMA API, not by using virt_to_phys(). > >> > >> Using virt_to_phys() might work on some systems, but using the DMA API > >> should work on all systems. > >> > >> This is essentially the same thing as allocating a buffer in a driver > >> to which the endpoint will write to. To do this, we use the DMA API. > >> > >> Signed-off-by: Niklas Cassel > > > > I'm expecting Jingoo and/or Joao to chime in and ack the > > DesignWare-related patches. I think all the others are in > > pretty good shape. > > > >> --- > >> drivers/pci/dwc/pcie-designware-host.c | 15 --- > >> drivers/pci/dwc/pcie-designware.h | 3 ++- > >> 2 files changed, 14 insertions(+), 4 deletions(-) > >> ... > > Let me test this patch-set in my setup first! > I will give feedback until Friday. OK, thanks. This may push this series out until v4.16 because the v4.15 merge window will probably open Sunday, and I don't like to add code during the window and merge it immediately. Bjorn
Re: [PATCH v2] s390/virtio: mark headers as BSD licensed
On Wed, 8 Nov 2017 19:01:07 +0200 "Michael S. Tsirkin" wrote: > On Wed, Nov 08, 2017 at 06:59:21PM +0200, Michael S. Tsirkin wrote: > > Virtio UAPI headers aren't just for UAPI, it's for guests/hypervisors as > > well. The s390 ones need to be BSD as well. > > > > Fixes: e2be04c7f995 ("License cleanup: add SPDX license identifier to uapi > > header files with a license") > > Cc: Greg Kroah-Hartman > > Signed-off-by: Michael S. Tsirkin > > A word of warning - this is completely untested. I lost my s390 > cross-build setup. Tested-by would be nice, otherwise > I will cross-build in the next couple of days and report here. FWIW, it survives my cross build.
Re: [PATCH] s390/virtio: mark headers as BSD licensed
On Wed, Nov 08, 2017 at 06:47:18PM +0200, Michael S. Tsirkin wrote: > Virtio UAPI headers aren't just for UAPI, it's for guests/hypervisors as > well. The s390 ones need to be BSD as well. > > Fixes: e2be04c7f995 ("License cleanup: add SPDX license identifier to uapi > header files with a license") > Cc: Greg Kroah-Hartman > Signed-off-by: Michael S. Tsirkin > --- > > Christian, I would appreciate your ack if you agree. You are changing the license on this file, you can't just slap a BSD one on the top, and have GPL below it without saying something about both covering the file, right? > > arch/s390/include/uapi/asm/kvm_virtio.h | 26 ++- > arch/s390/include/uapi/asm/virtio-ccw.h | 37 > - > 2 files changed, 61 insertions(+), 2 deletions(-) > > diff --git a/arch/s390/include/uapi/asm/kvm_virtio.h > b/arch/s390/include/uapi/asm/kvm_virtio.h > index 7328367..1773693 100644 > --- a/arch/s390/include/uapi/asm/kvm_virtio.h > +++ b/arch/s390/include/uapi/asm/kvm_virtio.h > @@ -1,4 +1,28 @@ > -/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ There is a dual-license SPDX identifier you can use, please use that, if you agree to relicense this file. thanks, greg k-h
Re: [PATCH] staging: fsl-dpaa2: Fix multiple assignments should be avoided
On Wed, 2017-11-08 at 09:16 -0800, Randy Dunlap wrote: > On 11/08/2017 03:39 AM, Joe Perches wrote: > > On Wed, 2017-11-08 at 12:40 +0300, Dan Carpenter wrote: > > > On Wed, Nov 08, 2017 at 10:20:48AM +0100, Greg KH wrote: > > > > On Tue, Nov 07, 2017 at 07:45:03PM -0500, Joshua Abraham wrote: > > > > > This patch fixes the checkpatch.pl warning: > > > > > "CHECK: multiple assignments should be avoided" [] > > > > > diff --git a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c > > > > > b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c [] > > > > > @@ -1661,7 +1661,8 @@ static void set_fq_affinity(struct > > > > > dpaa2_eth_priv *priv) > > > > >* This may well change at runtime, either through irqbalance or > > > > >* through direct user intervention. > > > > >*/ > > > > > - rx_cpu = txc_cpu = cpumask_first(&priv->dpio_cpumask); > > > > > + rx_cpu = cpumask_first(&priv->dpio_cpumask); > > > > > + txc_cpu = rx_cpu; > > > > > > > > The original code here makes much more sense, doesn't it? > > > > > > > > Sometimes checkpatch is wrong :) > > > > > > It feels like the majority of these multiple assignment warnings are > > > wrong. I thought it would be a good idea at first but after looking at > > > a bunch of the patches it feels like we should just remove the check. > > > > I don't have a particular opinion one way or another. > > > > That bit was added to CodingStyle by Randy Dunlap back > > in 2006 by > > > > commit b3fc9941fbc6efe5cb77728adb0fb12be363e73e > > Author: Randy Dunlap > > Date: Sun Dec 10 02:18:56 2006 -0800 > > > > [PATCH] CodingStyle updates > > > > Add some kernel coding style comments, mostly pulled from emails > > by Andrew Morton, Jesper Juhl, and Randy Dunlap. [] > > - add paragraph on multiple-assignments [] > > Signed-off-by: Randy Dunlap > > Acked-by: Jesper Juhl > > Acked-by: Jan Engelhardt > > Signed-off-by: Andrew Morton > > So it was. I would certainly support removing it. > > I suspect I copied that from someone else's email. My personal style > would be to allow that. My personal style too actually. foo = bar = baz(); is rather more sensible to me than bar = baz(); foo = bar; I think this style stems from the desire to be similar to single line definitions, but if the identifier lengths are short it really doesn't make the code better to read. Though I personally do prefer int i; int *j; over int i, *j;
Re: [PATCH V1 1/1] serial: 8250_fintek: Fix crash with baud rate B0
On Wed, 8 Nov 2017 11:05:46 +0800 "Ji-Ze Hong (Peter Hong)" wrote: > The 8250_fintek.c is support the Fintek F81866/F81216 with dynamic clock. > But It'll generate "division by zero" exception and crash in > fintek_8250_set_termios() with baud rate 0 on baudrate_table[i] % baud. 0 means "hang up", so this looks good providing it leaves the device in a sane state. Reviewed-by: Alan Cox
Re: [PATCH v2] checkpatch: do not check missing blank line before builtin_*_driver
On Wed, 2017-11-08 at 22:32 +0900, Masahiro Yamada wrote: > Hi Joe, Andrew, > > (sorry, Andrew was missing from the list...) > > 2017-09-18 11:01 GMT+09:00 Masahiro Yamada : > > checkpatch.pl does not check missing blank line before module_*_driver. > > I want it to behave likewise for builtin_*_driver. > > > > Signed-off-by: Masahiro Yamada > > --- > > > > Changes in v2: > > - Improve the matching pattern as suggested by Joe Perches > > > > scripts/checkpatch.pl | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > > index dd2c262aebbf..5c6179c63cf6 100755 > > --- a/scripts/checkpatch.pl > > +++ b/scripts/checkpatch.pl > > @@ -3103,6 +3103,7 @@ sub process { > > $line =~ /^\+[a-z_]*init/ || > > $line =~ /^\+\s*(?:static\s+)?[A-Z_]*ATTR/ || > > $line =~ /^\+\s*DECLARE/ || > > + $line =~ /^\+\s*builtin_[\w_]*driver/ || > > $line =~ /^\+\s*__setup/)) { > > if (CHK("LINE_SPACING", > > "Please use a blank line after > > function/struct/union/enum declarations\n" . $hereprev) && > > -- > > 2.7.4 > > > > > Is this patch good for 4.15-rc1? > > "git log scripts/checkpatch.pl" shows > this kind of patch is generally picked up by Andrew. > > > If Joe issues Acked-by, that will be very helpful > for applying this patch. Acked-by: Joe Perches
[PATCH] io: core: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Addresses-Coverity-ID: 1397962 Signed-off-by: Gustavo A. R. Silva --- drivers/iio/industrialio-core.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c index 9c4cfd1..2e8e36f 100644 --- a/drivers/iio/industrialio-core.c +++ b/drivers/iio/industrialio-core.c @@ -588,6 +588,7 @@ static ssize_t __iio_format_value(char *buf, size_t len, unsigned int type, return snprintf(buf, len, "%d", vals[0]); case IIO_VAL_INT_PLUS_MICRO_DB: scale_db = true; + /* fall through */ case IIO_VAL_INT_PLUS_MICRO: if (vals[1] < 0) return snprintf(buf, len, "-%d.%06u%s", abs(vals[0]), -- 2.7.4
Re: [PATCHv3 1/1] locking/qspinlock/x86: Avoid test-and-set when PV_DEDICATED is set
2017-11-06 12:26-0800, Eduardo Valentin: > Currently, the existing qspinlock implementation will fallback to > test-and-set if the hypervisor has not set the PV_UNHALT flag. > > This patch gives the opportunity to guest kernels to select > between test-and-set and the regular queueu fair lock implementation > based on the PV_DEDICATED KVM feature flag. When the PV_DEDICATED > flag is not set, the code will still fall back to test-and-set, > but when the PV_DEDICATED flag is set, the code will use > the regular queue spinlock implementation. > > With this patch, when in autoselect mode, the guest will > use the default spinlock implementation based on host feature > flags as follows: > > PV_DEDICATED = 1, PV_UNHALT = anything: default is qspinlock > PV_DEDICATED = 0, PV_UNHALT = 1: default is pvqspinlock > PV_DEDICATED = 0, PV_UNHALT = 0: default is tas > > Cc: Paolo Bonzini > Cc: "Radim Krčmář" > Cc: Jonathan Corbet > Cc: Thomas Gleixner > Cc: Ingo Molnar > Cc: "H. Peter Anvin" > Cc: x...@kernel.org > Cc: Peter Zijlstra > Cc: Waiman Long > Cc: k...@vger.kernel.org > Cc: linux-...@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Cc: Jan H. Schoenherr > Cc: Anthony Liguori > Suggested-by: Matt Wilson > Signed-off-by: Eduardo Valentin > --- > V3: > - When PV_DEDICATED is set (1), qspinlock is selected, >regardless of the value of PV_UNHAULT. Suggested by Paolo Bonzini. > - Refreshed on top of tip/master. > V2: > - rebase on top of tip/master > > Documentation/virtual/kvm/cpuid.txt | 6 ++ > arch/x86/include/asm/qspinlock.h | 4 > arch/x86/include/uapi/asm/kvm_para.h | 1 + > arch/x86/kernel/kvm.c| 2 ++ > 4 files changed, 13 insertions(+) > > diff --git a/Documentation/virtual/kvm/cpuid.txt > b/Documentation/virtual/kvm/cpuid.txt > index 3c65feb..117066a 100644 > --- a/Documentation/virtual/kvm/cpuid.txt > +++ b/Documentation/virtual/kvm/cpuid.txt > @@ -54,6 +54,12 @@ KVM_FEATURE_PV_UNHALT || 7 || guest > checks this feature bit > || || before enabling > paravirtualized > || || spinlock support. > > -- > +KVM_FEATURE_PV_DEDICATED || 8 || guest checks this feature bit > + || || to determine if they run on > + || || dedicated vCPUs, allowing > opti- > + || || mizations such as usage of > + || || qspinlocks. > +-- > KVM_FEATURE_CLOCKSOURCE_STABLE_BIT ||24 || host will warn if no > guest-side > || || per-cpu warps are expected in > || || kvmclock. > diff --git a/arch/x86/include/asm/qspinlock.h > b/arch/x86/include/asm/qspinlock.h > index 5e16b5d..de42694 100644 > --- a/arch/x86/include/asm/qspinlock.h > +++ b/arch/x86/include/asm/qspinlock.h > @@ -3,6 +3,8 @@ > #define _ASM_X86_QSPINLOCK_H > > #include > +#include > + > #include > #include > #include > @@ -58,6 +60,8 @@ static inline bool virt_spin_lock(struct qspinlock *lock) > if (!static_branch_likely(&virt_spin_lock_key)) > return false; > > + if (kvm_para_has_feature(KVM_FEATURE_PV_DEDICATED)) > + return false; Hm, every spinlock slowpath calls cpuid, which causes a VM exit, so I wouldn't expect it to be faster than the existing implementations. (Using the static key would be better.) How does this patch perform compared to user-forced qspinlock and hybrid pvqspinlock? Thanks.
Re: [PATCH V2 2/3] dmaengine: qcom_hidma: add support for the new revision
On 11/8/2017 12:12 PM, Timur Tabi wrote: > On 11/08/2017 10:58 AM, Sinan Kaya wrote: >> Besides, C compiler also won't let me put two arrays together like this. >> >> struct my_struct { >> struct some_struct array1[] >> struct some_struct array2[] >> } > > Why not this: > > const struct of_device_id hidma_msi_of_ids[] = { > {.compatible = "qcom,hidma-1.1",}, > {.compatible = "qcom,hidma-1.2",}, > {}, > }, > > static const struct acpi_device_id hidma_msi_acpi_ids[] = { > {"QCOM8001", QDF2XXX_V1}, > {"QCOM8002", QDF2XXX_V2}, > {}, > }; > > struct hidma_cap { > const struct of_device_id *of; > const struct acpi_device_id *acpi; > }; > > static struct hidma_cap hidma_msi_cap = { > hidma_msi_of_ids, > hidma_msi_acpi_ids > } > I think we are talking styles here. I started with your proposal and wanted to group the settings together as much as I can for maintenance reasons only because I don't have to remember that there are two different arrays that I need to take care of when I add a new HW in the future. +static struct hidma_cap hidma_msi_cap = { + .of = { + {.compatible = "qcom,hidma-1.1",}, + {.compatible = "qcom,hidma-1.2",}, + {}, + }, + + .acpi = { + {"QCOM8062"}, + {"QCOM8063"}, + {}, + } +}; I like this better than what you are proposing. > Keep in mind that you also need to add MODULE_DEVICE_TABLE entries, which you > can't really do with your approach. > > MODULE_DEVICE_TABLE(of, hidma_msi_of_ids); > MODULE_DEVICE_TABLE(acpi, hidma_msi_acpi_ids); > HIDMA capable devices are a subset of the devices that need to be probed. That's also why I don't touch the device_table. In the end both approaches work. It is a choice between what is more manageable. That was the initial objection. I tried to close on this request. -- Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
[PATCH v2] mm, shrinker: make shrinker_list lockless
In our production, we have observed that the job loader gets stuck for 10s of seconds while doing mount operation. It turns out that it was stuck in register_shrinker() and some unrelated job was under memory pressure and spending time in shrink_slab(). Our machines have a lot of shrinkers registered and jobs under memory pressure has to traverse all of those memcg-aware shrinkers and do affect unrelated jobs which want to register their own shrinkers. This patch has made the shrinker_list traversal lockless and shrinker register remain fast. For the shrinker unregister, atomic counter has been introduced to avoid synchronize_rcu() call. The fields of struct shrinker has been rearraged to make sure that the size does not increase for x86_64. The shrinker functions are allowed to reschedule() and thus can not be called with rcu read lock. One way to resolve that is to use srcu read lock but then ifdefs has to be used as SRCU is behind CONFIG_SRCU. Another way is to just release the rcu read lock before calling the shrinker and reacquire on the return. The atomic counter will make sure that the shrinker entry will not be freed under us. Signed-off-by: Shakeel Butt --- Changelog since v1: - release and reacquire rcu lock across shrinker call. include/linux/shrinker.h | 4 +++- mm/vmscan.c | 54 ++-- 2 files changed, 37 insertions(+), 21 deletions(-) diff --git a/include/linux/shrinker.h b/include/linux/shrinker.h index 388ff2936a87..434b76ef9367 100644 --- a/include/linux/shrinker.h +++ b/include/linux/shrinker.h @@ -60,14 +60,16 @@ struct shrinker { unsigned long (*scan_objects)(struct shrinker *, struct shrink_control *sc); + unsigned int flags; int seeks; /* seeks to recreate an obj */ long batch; /* reclaim batch size, 0 = default */ - unsigned long flags; /* These are for internal use */ struct list_head list; /* objs pending delete, per node */ atomic_long_t *nr_deferred; + /* Number of active do_shrink_slab calls to this shrinker */ + atomic_t nr_active; }; #define DEFAULT_SEEKS 2 /* A good number if you don't know better. */ diff --git a/mm/vmscan.c b/mm/vmscan.c index eb2f0315b8c0..6cec46ac6d95 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c @@ -157,7 +157,7 @@ int vm_swappiness = 60; unsigned long vm_total_pages; static LIST_HEAD(shrinker_list); -static DECLARE_RWSEM(shrinker_rwsem); +static DEFINE_SPINLOCK(shrinker_lock); #ifdef CONFIG_MEMCG static bool global_reclaim(struct scan_control *sc) @@ -285,21 +285,42 @@ int register_shrinker(struct shrinker *shrinker) if (!shrinker->nr_deferred) return -ENOMEM; - down_write(&shrinker_rwsem); - list_add_tail(&shrinker->list, &shrinker_list); - up_write(&shrinker_rwsem); + atomic_set(&shrinker->nr_active, 0); + spin_lock(&shrinker_lock); + list_add_tail_rcu(&shrinker->list, &shrinker_list); + spin_unlock(&shrinker_lock); return 0; } EXPORT_SYMBOL(register_shrinker); +static void get_shrinker(struct shrinker *shrinker) +{ + atomic_inc(&shrinker->nr_active); + rcu_read_unlock(); +} + +static void put_shrinker(struct shrinker *shrinker) +{ + rcu_read_lock(); + if (!atomic_dec_return(&shrinker->nr_active)) + wake_up_atomic_t(&shrinker->nr_active); +} + +static int shrinker_wait_atomic_t(atomic_t *p) +{ + schedule(); + return 0; +} /* * Remove one */ void unregister_shrinker(struct shrinker *shrinker) { - down_write(&shrinker_rwsem); - list_del(&shrinker->list); - up_write(&shrinker_rwsem); + spin_lock(&shrinker_lock); + list_del_rcu(&shrinker->list); + spin_unlock(&shrinker_lock); + wait_on_atomic_t(&shrinker->nr_active, shrinker_wait_atomic_t, +TASK_UNINTERRUPTIBLE); kfree(shrinker->nr_deferred); } EXPORT_SYMBOL(unregister_shrinker); @@ -468,18 +489,9 @@ static unsigned long shrink_slab(gfp_t gfp_mask, int nid, if (nr_scanned == 0) nr_scanned = SWAP_CLUSTER_MAX; - if (!down_read_trylock(&shrinker_rwsem)) { - /* -* If we would return 0, our callers would understand that we -* have nothing else to shrink and give up trying. By returning -* 1 we keep it going and assume we'll be able to shrink next -* time. -*/ - freed = 1; - goto out; - } + rcu_read_lock(); - list_for_each_entry(shrinker, &shrinker_list, list) { + list_for_each_entry_rcu(shrinker, &shrinker_list, list) { struct shrink_control sc = { .gfp_mask = gfp_mask, .nid = nid, @@ -498,11 +510,13 @@ static unsigned long shrink_slab(gfp_t gfp_mask, int nid,
Re: [PATCH] locktorture: Fix Oops when reader/writer count is 0
On Wed, Nov 08, 2017 at 10:57:07AM -0600, Jeremy Linton wrote: > On 11/08/2017 10:48 AM, Paul E. McKenney wrote: > >On Wed, Nov 08, 2017 at 06:45:23AM -0800, Davidlohr Bueso wrote: > >>On Tue, 07 Nov 2017, Paul E. McKenney wrote: > >> > >>>On Tue, Nov 07, 2017 at 10:07:48PM +0100, Peter Zijlstra wrote: > On Tue, Nov 07, 2017 at 02:01:58PM -0600, Jeremy Linton wrote: > >Hi, > > > >On 10/10/2017 10:52 AM, Jeremy Linton wrote: > >>If nwriters_stress=0 is passed to the lock torture test > >>it will panic in: > > > >Ping? > > > >Has anyone had a chance to look at this? > > Helps if you Cc the people actually working on this stuff of course... > >>> > >>>Thank you for the forward, Peter, I have queued Jeremy's patch for > >>>testing and review. > >> > >>fyi I had proposed the following a while back, which I think is more > >>complete than this patch: > >> > >>https://lkml.org/lkml/2017/5/15/201 > >> > >>Ah, there's also this (unrelated) fix: > >>https://lkml.org/lkml/2017/5/15/203 > >> > >>>But Jeremy's list of email addresses is what you would expect from > >>>looking at MAINTAINERS, so how about the following patch? > > > >And it looks like Jeremy was not alone -- I was not CCed on either > >of those patches, either. > > > >Dave's patch does look more complete, and it certainly was submitted > >first. Let's see if it still applies... And they both do. > > Yes, avoiding the zero length allocations is probably a good plan, > and complaining if both the reader and writer are zero is doesn't > hurt either. > > So, I'm good with that patch too.. > > Reviewed-by: Jeremy Linton Thank you! > >Jeremy, could you please test Dave's patches and make sure that they > >work for you? That way I can apply your Tested-by. Dave, any objection > >to my adding Jeremy's Reported-by to your /201 patch? > > I will give it a spin.. Very good! Looking forward to seeing the results. Thanx, Paul > >>> > >>> > >>>commit 58322063498c8f5a3cc88f95bee237a0ce81f70a > >>>Author: Paul E. McKenney > >>>Date: Tue Nov 7 14:10:03 2017 -0800 > >>> > >>> torture: Place all torture-test modules in one MAINTAINERS group > >>> > >>> There is some confusion about where patches to kernel/torture.c > >>> and kernel/locking/locktorture.c should be sent. This commit > >>> therefore updates MAINTAINERS appropriately. > >>> > >>> Reported-by: Peter Zijlstra > >>> Signed-off-by: Paul E. McKenney > >>> > >>>diff --git a/MAINTAINERS b/MAINTAINERS > >>>index 2d3d750b19c0..eab868adedc6 100644 > >>>--- a/MAINTAINERS > >>>+++ b/MAINTAINERS > >>>@@ -8091,6 +8091,7 @@ F: arch/*/include/asm/rwsem.h > >>>F: include/linux/seqlock.h > >>>F: lib/locking*.[ch] > >>>F: kernel/locking/ > >>>+X:kernel/locking/locktorture.c > >>> > >>>LOGICAL DISK MANAGER SUPPORT (LDM, Windows 2000/XP/Vista Dynamic Disks) > >>>M: "Richard Russon (FlatCap)" > >>>@@ -11318,15 +11319,6 @@ L:linux-wirel...@vger.kernel.org > >>>S: Orphan > >>>F: drivers/net/wireless/ray* > >>> > >>>-RCUTORTURE MODULE > >>>-M:Josh Triplett > >>>-M:"Paul E. McKenney" > >>>-L:linux-kernel@vger.kernel.org > >>>-S:Supported > >>>-T:git > >>>git://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git > >>>-F:Documentation/RCU/torture.txt > >>>-F:kernel/rcu/rcutorture.c > >>>- > >>>RCUTORTURE TEST FRAMEWORK > >>>M: "Paul E. McKenney" > >>>M: Josh Triplett > >>>@@ -13558,6 +13550,18 @@ L:platform-driver-...@vger.kernel.org > >>>S: Maintained > >>>F: drivers/platform/x86/topstar-laptop.c > >>> > >>>+TORTURE-TEST MODULES > >>>+M:Davidlohr Bueso > >>>+M:"Paul E. McKenney" > >>>+M:Josh Triplett > >>>+L:linux-kernel@vger.kernel.org > >>>+S:Supported > >>>+T:git > >>>git://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git > >>>+F:Documentation/RCU/torture.txt > >>>+F:kernel/torture.c > >>>+F:kernel/rcu/rcutorture.c > >>>+F:kernel/locking/locktorture.c > >> > >>Sure, if you think this is the best way to go, I have no problem. > >> > >>Thanks, > >>Davidlohr > >> > > >
Re: [Xen-devel] [PATCH] Xen/pciback: Implement PCI slot or bus reset with 'do_flr' SysFS attribute
Hi, On Wed, Nov 08, 2017 at 09:44:48AM -0600, Govinda Tatti wrote: > Thanks Jan for your review comments. Please see below for my comments. > > On 11/7/2017 8:40 AM, Jan Beulich wrote: > On 06.11.17 at 18:48, wrote: > >>--- a/Documentation/ABI/testing/sysfs-driver-pciback > >>+++ b/Documentation/ABI/testing/sysfs-driver-pciback > >>@@ -11,3 +11,15 @@ Description: > >> #echo 00:19.0-E0:2:FF > > >> /sys/bus/pci/drivers/pciback/quirks > >> will allow the guest to read and write to the > >> configuration > >> register 0x0E. > >>+ > >>+What: /sys/bus/pci/drivers/pciback/do_flr > >>+Date: Nov 2017 > >>+KernelVersion: 4.15 > >>+Contact:xen-de...@lists.xenproject.org > >>+Description: > >>+An option to perform a slot or bus reset when a PCI device > >>+ is owned by Xen PCI backend. Writing a string of :BB:DD.F > >>+ will cause the pciback driver to perform a slot or bus reset > >>+ if the device supports it. It also checks to make sure that > >>+ all of the devices under the bridge are owned by Xen PCI > >>+ backend. > >Why do you name this "do_flr" when you don't even try FLR, but > >go to slot or then bus reset right away. > Yes, I agree but xen toolstack has already been modified to consume"do_flr" > attribute. Hence, we are using the > function that matches with sysfs attribute. > Hmm.. I remember some discussion from ages ago related to this. Back then it was suggested to "emulate" the flr capability (by doing slot or bus reset) for devices which don't have *native* flr available? So is this patch perhaps related to that? If the PCI device in question has native flr capability, then native flr is used, right ? I guess I should read the full patch.. -- Pasi
Re: [PATCH net-next 4/6] net: dsa: remove trans argument from vlan ops
On Wed, 2017-11-08 at 12:19 -0500, Vivien Didelot wrote: > The DSA switch VLAN ops pass the switchdev_trans structure down to the > drivers, but no one is using them and they aren't supposed to anyway. [] > diff --git a/include/net/dsa.h b/include/net/dsa.h [] > @@ -410,12 +410,10 @@ struct dsa_switch_ops { >*/ > int (*port_vlan_filtering)(struct dsa_switch *ds, int port, > bool vlan_filtering); > - int (*port_vlan_prepare)(struct dsa_switch *ds, int port, > - const struct switchdev_obj_port_vlan *vlan, > - struct switchdev_trans *trans); > - void(*port_vlan_add)(struct dsa_switch *ds, int port, > - const struct switchdev_obj_port_vlan *vlan, > - struct switchdev_trans *trans); > + int (*port_vlan_prepare)(struct dsa_switch *ds, int port, > + const struct switchdev_obj_port_vlan *vlan); > + void (*port_vlan_add)(struct dsa_switch *ds, int port, > + const struct switchdev_obj_port_vlan *vlan); > int (*port_vlan_del)(struct dsa_switch *ds, int port, >const struct switchdev_obj_port_vlan *vlan); I think this bit is slightly worse. Mixing alignment styles seems odd. I think it's better to either align all the (*func) uses on a tabstop or none of them.
Re: [PATCH V2 2/3] dmaengine: qcom_hidma: add support for the new revision
On 08/11/17 16:29, Sinan Kaya wrote: Add support for probing the newer HW and also organize MSI capable hardware into an array for maintenance reasons. Signed-off-by: Sinan Kaya --- drivers/dma/qcom/hidma.c | 41 + 1 file changed, 29 insertions(+), 12 deletions(-) diff --git a/drivers/dma/qcom/hidma.c b/drivers/dma/qcom/hidma.c index e366985..4ef7d6f 100644 --- a/drivers/dma/qcom/hidma.c +++ b/drivers/dma/qcom/hidma.c @@ -50,6 +50,7 @@ #include #include #include +#include #include #include #include @@ -104,6 +105,26 @@ static void hidma_free(struct hidma_dev *dmadev) module_param(nr_desc_prm, uint, 0644); MODULE_PARM_DESC(nr_desc_prm, "number of descriptors (default: 0)"); +#define HIDMA_MAX_DEV_MATCH 10 + +struct hidma_cap { + const struct of_device_id of[HIDMA_MAX_DEV_MATCH]; + const struct acpi_device_id acpi[HIDMA_MAX_DEV_MATCH]; +}; + +static struct hidma_cap hidma_msi_cap = { + .of = { + {.compatible = "qcom,hidma-1.1",}, + {.compatible = "qcom,hidma-1.2",}, + {}, + }, + + .acpi = { + {"QCOM8062"}, + {"QCOM8063"}, + {}, + } +}; Yikes, I dread to imagine where this is going... Apologies if I wasn't very clear, but what I meant to imply by dropping the of_device_get_match_data() hint was to follow one of the common patterns where you either just have some version token: enum foo_ver { FOO_V1, ... } struct acpi_device_id foo_acpi_ids[] = { { "_FOO0001", FOO_V1 }, ... } struct of_device_id foo_of_match[] = { { .compatible = "foo,v1", .data = (void *)FOO_V1 }, ... } int foo_probe(struct device *dev) { ... foodev->version = (enum foo_ver) of_device_get_match_data(&dev->of_node) ... } int foo_reset(struct foodev *foodev) { if (foodev->version == FOO_V1) writel(0, foodev->base + 0x20); else writel(0, foodev->base + 0x30); } or if it fits the code better, encapsulate the relevant details directly: struct foodata { .offset = 0x20, } foo_v1_data; struct acpi_device_id foo_acpi_ids[] = { { "_FOO0001", (unsigned long)&foo_v1_data }, ... } struct of_device_id foo_of_match[] = { { .compatible = "foo,v1", .data = &foo_v1_data }, ... } int foo_probe(struct device *dev) { ... foodev->data = of_device_get_match_data(dev->of_node) ... } int foo_reset(struct foodev *foodev) { writel(0, foodev->base + foodev->data->offset); } Creating multiple sets of match tables seems completely backwards, and frankly looks worse than the open-coded strcmps IMO. Robin. /* process completed descriptors */ static void hidma_process_completed(struct hidma_chan *mchan) @@ -739,22 +760,16 @@ static int hidma_request_msi(struct hidma_dev *dmadev, static bool hidma_msi_capable(struct device *dev) { struct acpi_device *adev = ACPI_COMPANION(dev); - const char *of_compat; - int ret = -EINVAL; - - if (!adev || acpi_disabled) { - ret = device_property_read_string(dev, "compatible", - &of_compat); - if (ret) - return false; + int ret; - ret = strcmp(of_compat, "qcom,hidma-1.1"); - } else { + if (!adev || acpi_disabled) + ret = of_match_device(hidma_msi_cap.of, dev) != NULL; + else { #ifdef CONFIG_ACPI - ret = strcmp(acpi_device_hid(adev), "QCOM8062"); + ret = acpi_match_device(hidma_msi_cap.acpi, dev) != NULL; #endif } - return ret == 0; + return ret; } static int hidma_probe(struct platform_device *pdev) @@ -954,6 +969,7 @@ static int hidma_remove(struct platform_device *pdev) static const struct acpi_device_id hidma_acpi_ids[] = { {"QCOM8061"}, {"QCOM8062"}, + {"QCOM8063"}, {}, }; MODULE_DEVICE_TABLE(acpi, hidma_acpi_ids); @@ -962,6 +978,7 @@ static int hidma_remove(struct platform_device *pdev) static const struct of_device_id hidma_match[] = { {.compatible = "qcom,hidma-1.0",}, {.compatible = "qcom,hidma-1.1",}, + {.compatible = "qcom,hidma-1.2",}, {}, }; MODULE_DEVICE_TABLE(of, hidma_match);
Re: [PATCH V2 2/3] dmaengine: qcom_hidma: add support for the new revision
On 11/08/2017 11:37 AM, Sinan Kaya wrote: I think we are talking styles here. I don't think my suggestions are stylistic. Your version wastes space. However, if you really insist on your approach, that's fine with me. I'm not the maintainer. -- Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
[PATCH] fsl/fman_port: mark expected switch fall-throughs
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Addresses-Coverity-ID: 1397960 Signed-off-by: Gustavo A. R. Silva --- drivers/net/ethernet/freescale/fman/fman_port.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/net/ethernet/freescale/fman/fman_port.c b/drivers/net/ethernet/freescale/fman/fman_port.c index 1789b20..6552d68 100644 --- a/drivers/net/ethernet/freescale/fman/fman_port.c +++ b/drivers/net/ethernet/freescale/fman/fman_port.c @@ -1339,8 +1339,10 @@ int fman_port_config(struct fman_port *port, struct fman_port_params *params) switch (port->port_type) { case FMAN_PORT_TYPE_RX: set_rx_dflt_cfg(port, params); + /* fall through */ case FMAN_PORT_TYPE_TX: set_tx_dflt_cfg(port, params, &port->dts_params); + /* fall through */ default: set_dflt_cfg(port, params); } -- 2.7.4
Re: [PATCH 0/2] mmc: renesas_sdhi: clean-up and fix CONFIG stuff for renesas_sdhi
On Tue, Nov 07, 2017 at 04:57:20PM +0900, Masahiro Yamada wrote: Ulf, Yamada-san: All of Yamada-san's patches look good from a glimpse. However, I am currently swamped with work and will probably need 1-2 weeks for review. I am sorry about that! However, since 4.16 is far away enough, I don't think this will cause a miss for the next merge window. I'll try to review it earlier but please bear with me. > > > > Masahiro Yamada (2): > mmc: renesas_sdhi: consilidate DMAC CONFIG options > mmc: renesas_shci: remove wrong depends on to enable compile test > > drivers/mmc/host/Kconfig | 5 ++--- > drivers/mmc/host/Makefile | 8 ++-- > 2 files changed, 4 insertions(+), 9 deletions(-) > > -- > 2.7.4 > signature.asc Description: PGP signature
Re: [PATCH v2] mm, shrinker: make shrinker_list lockless
(off list) Shakeel Butt wrote: > In our production, we have observed that the job loader gets stuck for > 10s of seconds while doing mount operation. It turns out that it was > stuck in register_shrinker() and some unrelated job was under memory > pressure and spending time in shrink_slab(). Our machines have a lot > of shrinkers registered and jobs under memory pressure has to traverse > all of those memcg-aware shrinkers and do affect unrelated jobs which > want to register their own shrinkers. > > This patch has made the shrinker_list traversal lockless and shrinker > register remain fast. For the shrinker unregister, atomic counter > has been introduced to avoid synchronize_rcu() call. The fields of > struct shrinker has been rearraged to make sure that the size does > not increase for x86_64. > > The shrinker functions are allowed to reschedule() and thus can not > be called with rcu read lock. One way to resolve that is to use > srcu read lock but then ifdefs has to be used as SRCU is behind > CONFIG_SRCU. Another way is to just release the rcu read lock before > calling the shrinker and reacquire on the return. The atomic counter > will make sure that the shrinker entry will not be freed under us. > > Signed-off-by: Shakeel Butt > --- > Changelog since v1: > - release and reacquire rcu lock across shrinker call. > > include/linux/shrinker.h | 4 +++- > mm/vmscan.c | 54 > ++-- > 2 files changed, 37 insertions(+), 21 deletions(-) > > diff --git a/include/linux/shrinker.h b/include/linux/shrinker.h > index 388ff2936a87..434b76ef9367 100644 > --- a/include/linux/shrinker.h > +++ b/include/linux/shrinker.h > @@ -60,14 +60,16 @@ struct shrinker { > unsigned long (*scan_objects)(struct shrinker *, > struct shrink_control *sc); > > + unsigned int flags; > int seeks; /* seeks to recreate an obj */ > long batch; /* reclaim batch size, 0 = default */ > - unsigned long flags; > > /* These are for internal use */ > struct list_head list; > /* objs pending delete, per node */ > atomic_long_t *nr_deferred; > + /* Number of active do_shrink_slab calls to this shrinker */ > + atomic_t nr_active; > }; > #define DEFAULT_SEEKS 2 /* A good number if you don't know better. */ > > diff --git a/mm/vmscan.c b/mm/vmscan.c > index eb2f0315b8c0..6cec46ac6d95 100644 > --- a/mm/vmscan.c > +++ b/mm/vmscan.c > @@ -157,7 +157,7 @@ int vm_swappiness = 60; > unsigned long vm_total_pages; > > static LIST_HEAD(shrinker_list); > -static DECLARE_RWSEM(shrinker_rwsem); > +static DEFINE_SPINLOCK(shrinker_lock); > > #ifdef CONFIG_MEMCG > static bool global_reclaim(struct scan_control *sc) > @@ -285,21 +285,42 @@ int register_shrinker(struct shrinker *shrinker) > if (!shrinker->nr_deferred) > return -ENOMEM; > > - down_write(&shrinker_rwsem); > - list_add_tail(&shrinker->list, &shrinker_list); > - up_write(&shrinker_rwsem); > + atomic_set(&shrinker->nr_active, 0); > + spin_lock(&shrinker_lock); > + list_add_tail_rcu(&shrinker->list, &shrinker_list); > + spin_unlock(&shrinker_lock); > return 0; > } > EXPORT_SYMBOL(register_shrinker); > > +static void get_shrinker(struct shrinker *shrinker) > +{ > + atomic_inc(&shrinker->nr_active); > + rcu_read_unlock(); > +} > + > +static void put_shrinker(struct shrinker *shrinker) > +{ > + rcu_read_lock(); > + if (!atomic_dec_return(&shrinker->nr_active)) > + wake_up_atomic_t(&shrinker->nr_active); > +} > + > +static int shrinker_wait_atomic_t(atomic_t *p) > +{ > + schedule(); > + return 0; > +} > /* > * Remove one > */ > void unregister_shrinker(struct shrinker *shrinker) > { > - down_write(&shrinker_rwsem); > - list_del(&shrinker->list); > - up_write(&shrinker_rwsem); > + spin_lock(&shrinker_lock); > + list_del_rcu(&shrinker->list); > + spin_unlock(&shrinker_lock); > + wait_on_atomic_t(&shrinker->nr_active, shrinker_wait_atomic_t, > + TASK_UNINTERRUPTIBLE); What keeps us from returning to the caller which could kfree the shrinker before shrink_slab() uses it for list iteration? > kfree(shrinker->nr_deferred); > } > EXPORT_SYMBOL(unregister_shrinker); > @@ -468,18 +489,9 @@ static unsigned long shrink_slab(gfp_t gfp_mask, int nid, > if (nr_scanned == 0) > nr_scanned = SWAP_CLUSTER_MAX; > > - if (!down_read_trylock(&shrinker_rwsem)) { > - /* > - * If we would return 0, our callers would understand that we > - * have nothing else to shrink and give up trying. By returning > - * 1 we keep it going and assume we'll be able to shrink next > - * time. > - */ > - freed = 1; > - goto out; > - } > + rcu_read_lock(); > > - list_for_each_
Re: [PATCH v2 2/3] drivers: hwmon: Add W83773G driver
On Wed, Nov 08, 2017 at 02:03:34PM +0800, Lei YU wrote: > On Tue, Nov 7, 2017 at 10:33 PM, Guenter Roeck wrote: > > On 11/07/2017 12:27 AM, Lei YU wrote: > >> > >> Nuvoton W83773G is a hardware monitor IC providing one local > >> temperature and two remote temperature sensors. > >> > >> Signed-off-by: Lei YU > >> --- > >> v2: > >> - Rewrite the driver using regmap > >> - Add offset and update_interval [ ... ] > >> + hwmon_dev = devm_hwmon_device_register_with_groups(dev, > >> client->name, > >> + regmap, > >> w83773g_groups); > > > > > > You lost me here. Why did you stop using the new API ? > > I was referencing some other hwmon driver (e.g. tmp401.c) and find out > the usage of SENSOR_DEVICE_ATTR, which I feel more clear and straightforward. > The new API is supposed to replace the old API to make it independent from sysfs and to simplify the actual driver code. On a driver like this, using the new API should reduce code and data size by 30% or more. If you find the new API to be less clear and less straigtforward to use than the old API, we need to improve it. Do you have any suggestions ? Thanks, Guenter
Re: [PATCH] io: core: mark expected switch fall-through
> In preparation to enabling -Wimplicit-fallthrough, mark switch cases > where we are expecting to fall through. please fix the subject line, should be iio: core: Mark... thx, p. > Addresses-Coverity-ID: 1397962 > Signed-off-by: Gustavo A. R. Silva > --- > drivers/iio/industrialio-core.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c > index 9c4cfd1..2e8e36f 100644 > --- a/drivers/iio/industrialio-core.c > +++ b/drivers/iio/industrialio-core.c > @@ -588,6 +588,7 @@ static ssize_t __iio_format_value(char *buf, size_t len, > unsigned int type, > return snprintf(buf, len, "%d", vals[0]); > case IIO_VAL_INT_PLUS_MICRO_DB: > scale_db = true; > + /* fall through */ > case IIO_VAL_INT_PLUS_MICRO: > if (vals[1] < 0) > return snprintf(buf, len, "-%d.%06u%s", abs(vals[0]), > -- Peter Meerwald-Stadler Mobile: +43 664 24 44 418
Re: [PATCH] io: core: mark expected switch fall-through
Quoting Peter Meerwald-Stadler : In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. please fix the subject line, should be iio: core: Mark... Oh yeah. I'll send v2 shortly. Thanks! -- Gustavo A. R. Silva
Re: [vlan_device_event] BUG: unable to handle kernel paging request at 6b6b6ccf
On Wed, Nov 8, 2017 at 9:12 AM, Fengguang Wu wrote: > > OK. Here is the original faddr2line output: > > $ ~/linux/scripts/faddr2line vmlinux vlan_device_event+0x7f5/0xa40 > vlan_device_event+0x7f5/0xa40: > vlan_device_event at net/8021q/vlan.h:60 Hmm. Yes, that's not what I hoped for. > I notice that this trace shows no additional inline files at all. > Is it because I did some kconfig option wrong, so that inline info is > lost? Eg. > > CONFIG_OPTIMIZE_INLINING=y (reading lib/Kconfig.debug, it looks better set > to N) > CONFIG_DEBUG_INFO_REDUCED=y > CONFIG_DEBUG_INFO_SPLIT=y Hmm. It might also be a compiler/linker issue. Debug info is sadly often a second class citizen, because it's generally not something that gets tested nearly as much as the actual _code_ that a compiler generates. So we've certainly had issues before with incomplete debug info. That said, I'm not sure how well that CONFIG_DEBUG_INFO_SPLIT works, I've only ever tested with CONFIG_DEBUG_INFO=y CONFIG_DEBUG_INFO_REDUCED=y # CONFIG_DEBUG_INFO_SPLIT is not set # CONFIG_DEBUG_INFO_DWARF4 is not set which was the config that my scheduler example was generated from. It might be an issue with the particular version of 'addr2line' too, for all I know. The one I have is GNU addr2line version 2.27-24.fc26 and who knows what else could influence it. I *thought* that as long as you just had DEBUG_INFO enabled, it would automatically be ok, but I was clearly wrong. > [ 745.719623] BUG: unable to handle kernel paging request at 6b6b6f4f > [ 745.732871] IP: vlan_device_event+0x7f5/0xa40: > vlan_device_event at net/8021q/vlan.h:60 Ok, at least now there's no lost information, even if it doesn't have that nice inlining information that I was hoping for. Linus
[PATCH] sh: migor: Reserve memory block for CEU
A memory region for CEU video buffer has to be reserved during machine initialization. Originally, it was allocated through DMA API helpers and stored in the second IORESOURCE_MEM entry, to be later remapped by the CEU driver with a call to 'dma_declare_coherent_memory()' As Linux does not allow anymore to remap system RAM regions with 'memremap' function, sh_mobile_ceu driver fails when trying to remap the memory area: WARN_ONCE(1, "memremap attempted on ram %pa size: %#lx\n", &offset, (unsigned long) size) from 'memremap()' function in kernel/memremap.c To avoid hitting that WARN_ONCE() and have memory successfully remapped, reserve a region using '.mv_mem_reserve' member of SH's 'struct sh_machine_vector' to make sure memory is reserved early enough, and removed from the available system RAM. This is similar to what happens on ARM architecture with 'arm_memblock_steal()' function. Suggested-by: Laurent Pinchart Signed-off-by: Jacopo Mondi --- arch/sh/boards/mach-migor/setup.c | 28 ++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/arch/sh/boards/mach-migor/setup.c b/arch/sh/boards/mach-migor/setup.c index 0bcbe58..080627c 100644 --- a/arch/sh/boards/mach-migor/setup.c +++ b/arch/sh/boards/mach-migor/setup.c @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -45,6 +46,9 @@ * 0x1800 8GB8 NAND Flash (K9K8G08U0A) */ +#define CEU_BUFFER_MEMORY_SIZE (4 << 20) +static phys_addr_t ceu_dma_membase; + static struct smc91x_platdata smc91x_info = { .flags = SMC91X_USE_16BIT | SMC91X_NOWAIT, }; @@ -501,6 +505,8 @@ extern char migor_sdram_leave_end; static int __init migor_devices_setup(void) { + struct resource *r = &migor_ceu_device.resource[2]; + /* register board specific self-refresh code */ sh_mobile_register_self_refresh(SUSP_SH_STANDBY | SUSP_SH_SF, &migor_sdram_enter_start, @@ -632,8 +638,6 @@ static int __init migor_devices_setup(void) #endif __raw_writew(__raw_readw(PORT_MSELCRB) | 0x2000, PORT_MSELCRB); /* D15->D8 */ - platform_resource_setup_memory(&migor_ceu_device, "ceu", 4 << 20); - /* SIU: Port B */ gpio_request(GPIO_FN_SIUBOLR, NULL); gpio_request(GPIO_FN_SIUBOBT, NULL); @@ -647,6 +651,12 @@ static int __init migor_devices_setup(void) */ __raw_writew(__raw_readw(PORT_MSELCRA) | 1, PORT_MSELCRA); + /* Setup additional memory resources for Migo-R */ + r->flags = IORESOURCE_MEM; + r->start = ceu_dma_membase; + r->end = r->start + CEU_BUFFER_MEMORY_SIZE - 1; + r->name = "ceu"; + i2c_register_board_info(0, migor_i2c_devices, ARRAY_SIZE(migor_i2c_devices)); @@ -665,10 +675,24 @@ static int migor_mode_pins(void) return MODE_PIN0 | MODE_PIN1 | MODE_PIN5; } +/* Reserve a portion of memory for CEU buffers */ +static void __init migor_mv_mem_reserve(void) +{ + phys_addr_t phys; + phys_addr_t size = CEU_BUFFER_MEMORY_SIZE; + + phys = memblock_alloc_base(size, PAGE_SIZE, MEMBLOCK_ALLOC_ANYWHERE); + memblock_free(phys, size); + memblock_remove(phys, size); + + ceu_dma_membase = phys; +} + /* * The Machine Vector */ static struct sh_machine_vector mv_migor __initmv = { .mv_name= "Migo-R", .mv_mode_pins = migor_mode_pins, + .mv_mem_reserve = migor_mv_mem_reserve, }; -- 2.7.4
Re: WTF? Re: [PATCH] License cleanup: add SPDX GPL-2.0 license identifier to files with no license
And I swear I wrote the previous mail before I read this announcement: https://fsfe.org/news/2017/news-20171108-01.en.html But the actual practices there are pretty much what I'd expect as a start (plus a list of acceptable licenses), they even use the same identifiers as you are adding, but they also back them up with the actual license texts in the source tree, which makes them a whole lot more useful. On Wed, Nov 08, 2017 at 07:11:21AM -0800, Christoph Hellwig wrote: > On Wed, Nov 08, 2017 at 01:35:46PM +0100, Philippe Ombredanne wrote: > > The benefits now and later: > > - no distraction with licensing boilerplate cr*p in patches and files > > - no guessing licensing needed when sending a patch > > - anyone can grep the kernel tree for licensing, no extra tool needed > > - Greg must feel really good about deleting so much things for once > > This patch didn't delete anything, it added random notes. > > I see now Greg deletes things from files he maintains which is even > worse, given that the kernel tree doesn't document anywhere what these > tags actually mean. > > So he deletes a lot of license tags and replaces them with tags he > puts a great significance on, but which aren't defined. A quick googles > shows some Linuxfoundation web page defines them, but they could change > them any time they want, nevermind that we don't even have a reference > to them either. > > > > > The downsides: > > - folks can no longer express their creativity in licensing texts like > > licensing thermal code under the "therms" of the GPL [2] > > I'd love something like that to happen. But for that we don't need a > sneaky patch that doesn't talk to kernel contributors. > > For that we need to > > a) agree on which licensing schemes we accept for future contributions > b) cleary document that policy in the kernel tree > c) reject anything that doesn't matter the above policy by manual > and/or automated review > > An automated tag scheme might help with b) and c) above if done > properly. But for that we need to document it, agree on it, discuss > it with everyone involved, etc. None of that has happened. Instead > Greg farted arcane tags that he thinks have a legal singnificance > all over three three without talking to the people whos code he tagged, > without any RFC or public discussion, without documenting what his > tags mean or any future strategy towards making use of them. ---end quoted text---
[PATCH v2] iio: core: Mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Addresses-Coverity-ID: 1397962 Signed-off-by: Gustavo A. R. Silva --- Changes in v2: Fix subject line. drivers/iio/industrialio-core.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c index 9c4cfd1..2e8e36f 100644 --- a/drivers/iio/industrialio-core.c +++ b/drivers/iio/industrialio-core.c @@ -588,6 +588,7 @@ static ssize_t __iio_format_value(char *buf, size_t len, unsigned int type, return snprintf(buf, len, "%d", vals[0]); case IIO_VAL_INT_PLUS_MICRO_DB: scale_db = true; + /* fall through */ case IIO_VAL_INT_PLUS_MICRO: if (vals[1] < 0) return snprintf(buf, len, "-%d.%06u%s", abs(vals[0]), -- 2.7.4
Re: [cdrom_check_status] BUG: unable to handle kernel NULL pointer dereference at 000001c0
On Wednesday, November 08, 2017 05:28:16 PM Bartlomiej Zolnierkiewicz wrote: > Something is very wrong here as pci_request_selected_regions() in > drivers/ide/setup-pci.c:ide_pci_enable() should allocate PCI resources > so the second probe attempt should not happen. Also interface/device > names reuse should be prevented by ide_find_port_slot().. OK, I see now what is going on here: ... CONFIG_DEBUG_TEST_DRIVER_REMOVE=y ... config DEBUG_TEST_DRIVER_REMOVE bool "Test driver remove calls during probe (UNSTABLE)" depends on DEBUG_KERNEL help Say Y here if you want the Driver core to test driver remove functions by calling probe, remove, probe. This tests the remove path without having to unbind the driver or unload the driver module. This option is expected to find errors and may render your system unusable. You should say N here unless you are explicitly looking to test this functionality. We actually see race on ->remove inside IDE's ide-cd.c driver related to disk_check_events() handling.. It is not worth to continue with fixing IDE but from the quick look SCSI sr.c may have similar problem - it may be worth to try to reproduce it using libata's piix driver (disable CONFIG_IDE and enable CONFIG_BLK_DEV_SR, CONFIG_ATA_PIIX is already enabled). Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics
Re: [PATCH v2] s390/virtio: mark headers as BSD licensed
On Wed, Nov 08, 2017 at 06:59:21PM +0200, Michael S. Tsirkin wrote: > Virtio UAPI headers aren't just for UAPI, it's for guests/hypervisors as > well. The s390 ones need to be BSD as well. > > Fixes: e2be04c7f995 ("License cleanup: add SPDX license identifier to uapi > header files with a license") > Cc: Greg Kroah-Hartman > Signed-off-by: Michael S. Tsirkin I'm not sure why you say your patch fixes the above commit. It just added a tag that described reality, while you are _proposing_ to relicense the header file. That's a completely different story. > --- > > since v1: > drop an extra comment chunk, reported by Cornelia > > arch/s390/include/uapi/asm/kvm_virtio.h | 26 +- > arch/s390/include/uapi/asm/virtio-ccw.h | 26 +- > 2 files changed, 50 insertions(+), 2 deletions(-) > > diff --git a/arch/s390/include/uapi/asm/kvm_virtio.h > b/arch/s390/include/uapi/asm/kvm_virtio.h > index 7328367..1773693 100644 > --- a/arch/s390/include/uapi/asm/kvm_virtio.h > +++ b/arch/s390/include/uapi/asm/kvm_virtio.h > @@ -1,4 +1,28 @@ > -/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ > +/* This header is BSD licensed so anyone can use the definitions to implement > + * compatible drivers/servers. > + * > + * Redistribution and use in source and binary forms, with or without > + * modification, are permitted provided that the following conditions > + * are met: > + * 1. Redistributions of source code must retain the above copyright > + *notice, this list of conditions and the following disclaimer. > + * 2. Redistributions in binary form must reproduce the above copyright > + *notice, this list of conditions and the following disclaimer in the > + *documentation and/or other materials provided with the distribution. > + * 3. Neither the name of IBM nor the names of its contributors > + *may be used to endorse or promote products derived from this software > + *without specific prior written permission. > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS ``AS > IS'' AND > + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE > + * ARE DISCLAIMED. IN NO EVENT SHALL IBM OR CONTRIBUTORS BE LIABLE > + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL > + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS > + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) > + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT > + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY > + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF > + * SUCH DAMAGE. */
Re: [PATCH] isa: Prevent NULL dereference in isa_bus driver callbacks
On Wed, Nov 08, 2017 at 09:11:22AM -0800, Linus Torvalds wrote: > On Wed, Nov 8, 2017 at 7:23 AM, William Breathitt Gray > wrote: > > > > This patch fixes a possible NULL pointer dereference if one of the > > isa_driver callbacks to attempted for an unsupported device. This error > > should not occur in practice since ISA devices are typically manually > > configured and loaded by the users, but we may as well prevent this > > error from popping up for the 0day testers. > > Thanks. Acked-by: Linus Torvalds > > And it doesn't seem critical enough for me to take directly this late > in the 4.14 game, so I guess I'll get it from the usual channels. I'll queue it up, given that this has been a problem for 10+ years :) thanks, greg k-h
Re: [PATCH v2] s390/virtio: mark headers as BSD licensed
On 11/08/2017 05:59 PM, Michael S. Tsirkin wrote: > Virtio UAPI headers aren't just for UAPI, it's for guests/hypervisors as > well. The s390 ones need to be BSD as well. > > Fixes: e2be04c7f995 ("License cleanup: add SPDX license identifier to uapi > header files with a license") > Cc: Greg Kroah-Hartman > Signed-off-by: Michael S. Tsirkin > --- > > since v1: > drop an extra comment chunk, reported by Cornelia > > arch/s390/include/uapi/asm/kvm_virtio.h | 26 +- FWIW, this file will go away anyway. Looks like the following commit in the s390 feature branch https://git.kernel.org/pub/scm/linux/kernel/git/s390/linux.git/commit/drivers/s390/virtio/Makefile?h=features&id=7fb2b2d512448cf0e914c4647a1cf02b52263702 missed this file. Regarding the virtio-ccw.h headers it is probably something that happened by accident. Back then Rusty (as an IBMer) proposed to change all virtio headers to BSD licence. > arch/s390/include/uapi/asm/virtio-ccw.h | 26 +- > 2 files changed, 50 insertions(+), 2 deletions(-) > > diff --git a/arch/s390/include/uapi/asm/kvm_virtio.h > b/arch/s390/include/uapi/asm/kvm_virtio.h > index 7328367..1773693 100644 > --- a/arch/s390/include/uapi/asm/kvm_virtio.h > +++ b/arch/s390/include/uapi/asm/kvm_virtio.h > @@ -1,4 +1,28 @@ > -/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ > +/* This header is BSD licensed so anyone can use the definitions to implement > + * compatible drivers/servers. > + * > + * Redistribution and use in source and binary forms, with or without > + * modification, are permitted provided that the following conditions > + * are met: > + * 1. Redistributions of source code must retain the above copyright > + *notice, this list of conditions and the following disclaimer. > + * 2. Redistributions in binary form must reproduce the above copyright > + *notice, this list of conditions and the following disclaimer in the > + *documentation and/or other materials provided with the distribution. > + * 3. Neither the name of IBM nor the names of its contributors > + *may be used to endorse or promote products derived from this software > + *without specific prior written permission. > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS ``AS > IS'' AND > + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE > + * ARE DISCLAIMED. IN NO EVENT SHALL IBM OR CONTRIBUTORS BE LIABLE > + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL > + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS > + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) > + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT > + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY > + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF > + * SUCH DAMAGE. */ > /* > * definition for virtio for kvm on s390 > * > diff --git a/arch/s390/include/uapi/asm/virtio-ccw.h > b/arch/s390/include/uapi/asm/virtio-ccw.h > index 967aad3..ae7a6eb 100644 > --- a/arch/s390/include/uapi/asm/virtio-ccw.h > +++ b/arch/s390/include/uapi/asm/virtio-ccw.h > @@ -1,4 +1,28 @@ > -/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ > +/* This header is BSD licensed so anyone can use the definitions to implement > + * compatible drivers/servers. > + * > + * Redistribution and use in source and binary forms, with or without > + * modification, are permitted provided that the following conditions > + * are met: > + * 1. Redistributions of source code must retain the above copyright > + *notice, this list of conditions and the following disclaimer. > + * 2. Redistributions in binary form must reproduce the above copyright > + *notice, this list of conditions and the following disclaimer in the > + *documentation and/or other materials provided with the distribution. > + * 3. Neither the name of IBM nor the names of its contributors > + *may be used to endorse or promote products derived from this software > + *without specific prior written permission. > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS ``AS > IS'' AND > + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE > + * ARE DISCLAIMED. IN NO EVENT SHALL IBM OR CONTRIBUTORS BE LIABLE > + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL > + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS > + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) > + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT > + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY W
Re: [PATCH v2] s390/virtio: mark headers as BSD licensed
On Wed, 8 Nov 2017 19:12:25 +0100 Christian Borntraeger wrote: > On 11/08/2017 05:59 PM, Michael S. Tsirkin wrote: > > Virtio UAPI headers aren't just for UAPI, it's for guests/hypervisors as > > well. The s390 ones need to be BSD as well. > > > > Fixes: e2be04c7f995 ("License cleanup: add SPDX license identifier to uapi > > header files with a license") > > Cc: Greg Kroah-Hartman > > Signed-off-by: Michael S. Tsirkin > > --- > > > > since v1: > > drop an extra comment chunk, reported by Cornelia > > > > arch/s390/include/uapi/asm/kvm_virtio.h | 26 +- > > FWIW, this file will go away anyway. > Looks like the following commit in the s390 feature branch > https://git.kernel.org/pub/scm/linux/kernel/git/s390/linux.git/commit/drivers/s390/virtio/Makefile?h=features&id=7fb2b2d512448cf0e914c4647a1cf02b52263702 > missed this file. Can we simply rip this out? > > > Regarding the virtio-ccw.h headers it is probably something that happened by > accident. > Back then Rusty (as an IBMer) proposed to change all virtio headers to BSD > licence. ISTR that the relicensing already went through... is that not the case? I thought this was just a fixup to align with reality? > > > arch/s390/include/uapi/asm/virtio-ccw.h | 26 +- > > 2 files changed, 50 insertions(+), 2 deletions(-)
Re: oops with 4.14-rc8 when opening and closing /dev/watchdog0
On Wed, Nov 08, 2017 at 02:26:34PM +0100, Rasmus Villemoes wrote: > Running current master (4.14.0-rc8-9-gfbc3edf) I can reproduce the > below quite consistently, though there are some variations in the stack > trace. It happens when I start and stop busybox watchdog on > /dev/watchdog0 a few times (sometimes on start, sometimes on stop, > almost always after at most 3 starts/stops). watchdog0 is a gpio > watchdog with the below DT entry. > > gpio-wdt { > status = "okay"; > compatible = "linux,wdt-gpio"; > hw_margin_ms = <0xfa>; > hw_algo = "toggle"; > gpios = <0x15 0x19 0x0>; > always-running; > }; > > The 6b6b6b6b suggests some kind of use-after-free, I think. > > This is a ARM board based on LS1021A. Unfortunately, I hit this in the > process of starting to use a mainline-based kernel for the board, so I > don't have any previous known-working kernel to start a bisection from. > I'll try to see if I can get a 4.13 one to boot with the same .dtb and > .config, but in the meantime perhaps someone can see something obvious. > Please let me know if the following two patches help. https://patchwork.kernel.org/patch/9970181/ https://patchwork.kernel.org/patch/9970187/ Thanks, Guenter > Thanks, > Rasmus > > > Unable to handle kernel paging request at virtual address 6b6b6d3b > pgd = 80003000 > [6b6b6d3b] *pgd=8080005003, *pmd= > Internal error: Oops: 206 [#1] SMP ARM > Modules linked in: bridge stp llc > CPU: 0 PID: 1931 Comm: watchdog Not tainted 4.14.0-rc8-9-gfbc3edf #1 > Hardware name: Freescale LS1021A > task: be4a8d40 task.stack: bd1d4000 > PC is at module_put+0x8/0x68 > LR is at __fput+0x108/0x1b0 > pc : [<8027f090>]lr : [<802ef898>]psr: 200d0013 > sp : bd1d5f20 ip : fp : > r10: bf0877c8 r9 : be805608 r8 : be04ccd0 > r7 : be3f57c8 r6 : 0008 r5 : bf0877c8 r4 : be3f57c0 > r3 : bf2241a0 r2 : 6b6b6b6a r1 : r0 : 6b6b6b6b > Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user > Control: 30c5387d Table: bd189340 DAC: fffd > Process watchdog (pid: 1931, stack limit = 0xbd1d4210) > Stack: (0xbd1d5f20 to 0xbd1d6000) > 5f20: 002b be4a917c be4a8d40 be412b80 81055310 > be49897c > 5f40: 044c 80235014 be4a8d40 be498940 bd1d4000 > bd1d5f70 > 5f60: be49897c 80220a10 be4a9068 be4a8d40 be4a9068 bd80c5c0 > 00f8 > 5f80: 80220fe8 000f4240 7ed9bccc 0007a154 00f8 80207544 > 80220ffc > 5fa0: 000f4240 80207380 000f4240 7ed9bccc 000874fe 0001 > > 5fc0: 000f4240 7ed9bccc 0007a154 00f8 0f00 76f89000 > > 5fe0: 76eb87e0 7ed9b9b4 00021c9c 76eb87f0 600d0010 > > [<8027f090>] (module_put) from [<>] ( (null)) > Code: e3a1 e12fff1e e350 012fff1e (e59021d0) > ---[ end trace d8b636b1833a6c9e ]--- > Kernel panic - not syncing: Fatal exception > CPU1: stopping > CPU: 1 PID: 64 Comm: kworker/u4:1 Tainted: G D > 4.14.0-rc8-9-gfbc3edf #1 > Hardware name: Freescale LS1021A > Workqueue: events_unbound flush_to_ldisc > [<8020c8e8>] (unwind_backtrace) from [<8020a728>] (show_stack+0x10/0x14) > [<8020a728>] (show_stack) from [<80661704>] (dump_stack+0x7c/0x98) > [<80661704>] (dump_stack) from [<8020bc24>] (handle_IPI+0xdc/0x184) > [<8020bc24>] (handle_IPI) from [<802013ac>] (gic_handle_irq+0x70/0x78) > [<802013ac>] (gic_handle_irq) from [<806776f8>] (__irq_svc+0x58/0x74) > Exception stack(0xbe0d1e58 to 0xbe0d1ea0) > 1e40: > 00fd > 1e60: 0ff8 be18ca40 c08a3000 bdb09c5b > bdb09c5b > 1e80: 0052 be18cbbc be0d1ea8 802502c0 8045cd2c 60010013 > > [<806776f8>] (__irq_svc) from [<8045cd2c>] > (n_tty_receive_buf_common+0x804/0x8bc) > [<8045cd2c>] (n_tty_receive_buf_common) from [<8045cdf4>] > (n_tty_receive_buf2+0x10/0x18) > [<8045cdf4>] (n_tty_receive_buf2) from [<8045f374>] > (tty_port_default_receive_buf+0x44/0x54) > [<8045f374>] (tty_port_default_receive_buf) from [<8045ebfc>] > (flush_to_ldisc+0x8c/0xac) > [<8045ebfc>] (flush_to_ldisc) from [<80231224>] > (process_one_work+0x1b0/0x314) > [<80231224>] (process_one_work) from [<80232118>] > (worker_thread+0x2cc/0x424) > [<80232118>] (worker_thread) from [<80236598>] (kthread+0x130/0x148) > [<80236598>] (kthread) from [<80207440>] (ret_from_fork+0x14/0x34) >
Re: [PATCH v2 02/12] hwrng: bcm2835-rng: Define a driver private context
Florian Fainelli writes: > Instead of making hwrng::priv host the base register address, define a > driver private context, make it per platform device instance and pass it > down the different functions. > > Signed-off-by: Florian Fainelli > --- > drivers/char/hw_random/bcm2835-rng.c | 55 > ++-- > 1 file changed, 34 insertions(+), 21 deletions(-) > > diff --git a/drivers/char/hw_random/bcm2835-rng.c > b/drivers/char/hw_random/bcm2835-rng.c > index a818418a7e4c..0d72147ab45b 100644 > --- a/drivers/char/hw_random/bcm2835-rng.c > +++ b/drivers/char/hw_random/bcm2835-rng.c > -static struct hwrng bcm2835_rng_ops = { > - .name = "bcm2835", > - .read = bcm2835_rng_read, > -}; > - > /* map peripheral */ > - rng_base = devm_ioremap_resource(dev, r); > - if (IS_ERR(rng_base)) { > + priv->base = devm_ioremap_resource(dev, r); > + if (IS_ERR(priv->base)) { > dev_err(dev, "failed to remap rng regs"); > - return PTR_ERR(rng_base); > + return PTR_ERR(priv->base); > } > - bcm2835_rng_ops.priv = (unsigned long)rng_base; > + > + priv->rng.name = "bcm2835-rng"; Looks like an unintentional change from the previous "bcm2835" here? Other than that, 1-2 are: Reviewed-by: Eric Anholt signature.asc Description: PGP signature
Re: [PATCH] dell-smbios: fix string overflow
Hi, I recommend using "platform/x86: dell-smbios:" in commit header. On 11/08/2017 04:08 AM, Arnd Bergmann wrote: The new sysfs code overwrites two fixed-length character arrays that are each one byte shorter than they need to be, to hold the trailing \0: drivers/platform/x86/dell-smbios.c: In function 'build_tokens_sysfs': drivers/platform/x86/dell-smbios.c:494:42: error: 'sprintf' writing a terminating nul past the end of the destination [-Werror=format-overflow=] sprintf(buffer_location, "%04x_location", drivers/platform/x86/dell-smbios.c:494:3: note: 'sprintf' output 14 bytes into a destination of size 13 drivers/platform/x86/dell-smbios.c:506:36: error: 'sprintf' writing a terminating nul past the end of the destination [-Werror=format-overflow=] sprintf(buffer_value, "%04x_value", drivers/platform/x86/dell-smbios.c:506:3: note: 'sprintf' output 11 bytes into a destination of size 10 Don't need to include the error log in commit message. Just explaining the issue is good enough. This changes it to just use kasprintf(), which always gets it right. Fixes: 33b9ca1e53b4 ("platform/x86: dell-smbios: Add a sysfs interface for SMBIOS tokens") Signed-off-by: Arnd Bergmann --- drivers/platform/x86/dell-smbios.c | 12 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-smbios.c index d99edd803c19..6a60db515bda 100644 --- a/drivers/platform/x86/dell-smbios.c +++ b/drivers/platform/x86/dell-smbios.c @@ -463,8 +463,6 @@ static struct platform_driver platform_driver = { static int build_tokens_sysfs(struct platform_device *dev) { - char buffer_location[13]; - char buffer_value[10]; char *location_name; char *value_name; size_t size; @@ -491,9 +489,8 @@ static int build_tokens_sysfs(struct platform_device *dev) if (da_tokens[i].tokenID == 0) continue; /* add location */ - sprintf(buffer_location, "%04x_location", - da_tokens[i].tokenID); - location_name = kstrdup(buffer_location, GFP_KERNEL); + location_name = kasprintf(GFP_KERNEL, "%04x_location", + da_tokens[i].tokenID); if (location_name == NULL) goto out_unwind_strings; sysfs_attr_init(&token_location_attrs[i].attr); @@ -503,9 +500,8 @@ static int build_tokens_sysfs(struct platform_device *dev) token_attrs[j++] = &token_location_attrs[i].attr; /* add value */ - sprintf(buffer_value, "%04x_value", - da_tokens[i].tokenID); - value_name = kstrdup(buffer_value, GFP_KERNEL); + value_name = kasprintf(GFP_KERNEL, "%04x_value", + da_tokens[i].tokenID); if (value_name == NULL) goto loop_fail_create_value; sysfs_attr_init(&token_value_attrs[i].attr); -- Sathyanarayanan Kuppuswamy Linux kernel developer
Re: [PATCH v2] nvme: compare NQN string with right size
Thanks, applied both patches to nvme-4.15.
Re: [PATCH v2 05/12] hwrng: bcm2835-rng: Use device managed helpers
Florian Fainelli writes: > Now that we have moved the RNG disabling into a hwrng::cleanup callback, > we can use the device managed registration operation and remove our > remove callback since it won't do anything necessary. 3-5 are: Reviewed-by: Eric Anholt signature.asc Description: PGP signature
[PATCH] tools/power turbostat: mark expected switch fall-through
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. Addresses-Coverity-ID: 1354683 Signed-off-by: Gustavo A. R. Silva --- tools/power/x86/turbostat/turbostat.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c index bd9c6b3..c8d9bf1 100644 --- a/tools/power/x86/turbostat/turbostat.c +++ b/tools/power/x86/turbostat/turbostat.c @@ -3515,6 +3515,7 @@ void perf_limit_reasons_probe(unsigned int family, unsigned int model) case INTEL_FAM6_HASWELL_ULT:/* HSW */ case INTEL_FAM6_HASWELL_GT3E: /* HSW */ do_gfx_perf_limit_reasons = 1; + /* fall through */ case INTEL_FAM6_HASWELL_X: /* HSX */ do_core_perf_limit_reasons = 1; do_ring_perf_limit_reasons = 1; -- 2.7.4
Re: [PATCH 5/5] driver core: Remove redundant license text
On Tue, Nov 07, 2017 at 05:30:09PM +0100, Greg Kroah-Hartman wrote: > Now that the SPDX tag is in all driver core files, that identifies the > license in a specific and legally-defined manner. Takashi and Jiri mentioned that the effort to add SPDX tags to files which did not have licensing was discussed at the maintainers summit and it was agreed upon there that this made sense. That is wonderful. Naturally, even despite this, some still have their own questions about this work [0]. And some others seem to actually have pointed out that the work might have some technical issues [1] likely worth considering. [0] https://lkml.kernel.org/r/20171108151121.gc10...@infradead.org [1] https://lkml.kernel.org/r/20171108171938.7df66c65@alans-desktop > So the extra GPL text wording can be removed Highlight *removed* > as it is no longer needed at all. This secondary however was not. The only sort of explain about this came from Philippe Ombredanne recently [2] on the generic effort questioned by Christoph. [2] https://lkml.kernel.org/r/CAOFm3uFLRCZwtw4F884Rd=5=yZnX_ibzBzyiC4f+=5iju4k...@mail.gmail.com But this begs the question that if there is still questions, issues pointed out, and request for a bit more open discussion about the *first* SPDX effort of adding a tag to files which have no license, if there was *any* due process for creating consensus for also going along with this *secondary* SPDX effort of license *simplification* by replacing old boiler plate license tags with an SPDX tag. At least internally within SUSE I can say so far that we are surprised by these patches and work. We did not know, and this is the first of communication of such effort. Don't get me wrong, these simplifications make perfect sense to me! But in dealing with licensing considerations before on Linux I've learned through feedback from you, Alan, and Ted and others to also be *extremely* careful and sensitive about licensing annotation matters, and this type of change seems to likely deserve a bit more community consensus than what this seems to be getting. Not even an RFC. So why rush this work in? > This is done on a quest to remove the 700+ different ways that files in > the kernel describe the GPL license text. And there's unneeded stuff > like the address (sometimes incorrect) for the FSF which is never > needed. Completely agreed, all this stuff is rather silly, however which tag is used, when, and how seems to have never been discussed and vetted anywhere to my knowledge. Below I leave two examples of the patch, but leave in place the diffstat. > No copyright headers or other non-license-description text was removed. > > Cc: Greg Kroah-Hartman > Cc: Johannes Berg > Cc: "Luis R. Rodriguez" > Signed-off-by: Greg Kroah-Hartman > --- > drivers/base/attribute_container.c | 2 -- > drivers/base/bus.c | 3 --- > drivers/base/cacheinfo.c| 12 > drivers/base/class.c| 3 --- > drivers/base/component.c| 4 > drivers/base/container.c| 4 > drivers/base/core.c | 3 --- > drivers/base/dd.c | 2 -- > drivers/base/devcoredump.c | 16 > drivers/base/devres.c | 2 -- > drivers/base/dma-contiguous.c | 5 - > drivers/base/dma-mapping.c | 2 -- > drivers/base/driver.c | 3 --- > drivers/base/firmware.c | 2 -- > drivers/base/hypervisor.c | 2 -- > drivers/base/init.c | 2 -- > drivers/base/map.c | 1 - > drivers/base/module.c | 3 --- > drivers/base/pinctrl.c | 2 -- > drivers/base/platform-msi.c | 12 > drivers/base/platform.c | 2 -- > drivers/base/property.c | 4 > drivers/base/soc.c | 1 - > drivers/base/syscore.c | 2 -- > drivers/base/test/test_async_driver_probe.c | 9 - > drivers/base/topology.c | 16 > drivers/base/transport_class.c | 2 -- > include/linux/device.h | 2 -- > 28 files changed, 123 deletions(-) <-- snip --> > diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c > index e321a7e66a1d..6e1000bff481 100644 > --- a/drivers/base/cacheinfo.c > +++ b/drivers/base/cacheinfo.c > @@ -4,18 +4,6 @@ > * > * Based on arch/x86/kernel/cpu/intel_cacheinfo.c > * Author: Sudeep Holla > - * > - * This program is free software; you can redistribute it and/or modify > - * it under the terms of the GNU General Public License version 2 as > - * published by the Free Software Foundation. > - * > - * This program is distributed "as is" WITHOUT ANY WARRANTY
Applied "ASoC: Intel: improve DMADEVICES dependency" to the asoc tree
The patch ASoC: Intel: improve DMADEVICES dependency has been applied to the asoc tree at https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark >From 326c4aa27a801918136df15d507f74968c7093fb Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Wed, 8 Nov 2017 14:03:19 +0100 Subject: [PATCH] ASoC: Intel: improve DMADEVICES dependency As pointed out by Pierre-Louis Bossart, the dependency I added was broader than necessary, only Baytrail and Haswell/Broadwell actually need it, the others don't. At the same time, we have individual entries for the codecs that all have the 'select' statement but now don't need it any more. Fixes: f7a88db6fffd ("ASoC: Intel: fix Kconfig dependencies") Signed-off-by: Arnd Bergmann Acked-By: Vinod Koul Signed-off-by: Mark Brown --- sound/soc/intel/Kconfig| 3 ++- sound/soc/intel/boards/Kconfig | 5 - 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig index e18118209b75..bb8be10b8437 100644 --- a/sound/soc/intel/Kconfig +++ b/sound/soc/intel/Kconfig @@ -32,19 +32,20 @@ config SND_SOC_ACPI_INTEL_MATCH config SND_SOC_INTEL_SST_TOPLEVEL tristate "Intel ASoC SST drivers" depends on X86 || COMPILE_TEST - depends on DMADEVICES select SND_SOC_INTEL_MACH select SND_SOC_INTEL_COMMON config SND_SOC_INTEL_HASWELL tristate "Intel ASoC SST driver for Haswell/Broadwell" depends on SND_SOC_INTEL_SST_TOPLEVEL && SND_DMA_SGBUF + depends on DMADEVICES select SND_SOC_INTEL_SST select SND_SOC_INTEL_SST_FIRMWARE config SND_SOC_INTEL_BAYTRAIL tristate "Intel ASoC SST driver for Baytrail (legacy)" depends on SND_SOC_INTEL_SST_TOPLEVEL + depends on DMADEVICES select SND_SOC_INTEL_SST select SND_SOC_INTEL_SST_FIRMWARE diff --git a/sound/soc/intel/boards/Kconfig b/sound/soc/intel/boards/Kconfig index 449bc8baaa60..5063f15b4ca4 100644 --- a/sound/soc/intel/boards/Kconfig +++ b/sound/soc/intel/boards/Kconfig @@ -19,7 +19,6 @@ config SND_SOC_INTEL_HASWELL_MACH tristate "ASoC Audio DSP support for Intel Haswell Lynxpoint" depends on SND_SOC_INTEL_MACH depends on X86_INTEL_LPSS && I2C && I2C_DESIGNWARE_PLATFORM - depends on DMADEVICES depends on SND_SOC_INTEL_HASWELL select SND_SOC_RT5640 help @@ -32,7 +31,6 @@ config SND_SOC_INTEL_BDW_RT5677_MACH tristate "ASoC Audio driver for Intel Broadwell with RT5677 codec" depends on SND_SOC_INTEL_MACH depends on X86_INTEL_LPSS && GPIOLIB && I2C - depends on DMADEVICES depends on SND_SOC_INTEL_HASWELL select SND_SOC_RT5677 help @@ -43,7 +41,6 @@ config SND_SOC_INTEL_BROADWELL_MACH tristate "ASoC Audio DSP support for Intel Broadwell Wildcatpoint" depends on SND_SOC_INTEL_MACH depends on X86_INTEL_LPSS && I2C && I2C_DESIGNWARE_PLATFORM - depends on DMADEVICES depends on SND_SOC_INTEL_HASWELL select SND_SOC_RT286 help @@ -56,7 +53,6 @@ config SND_SOC_INTEL_BYT_MAX98090_MACH tristate "ASoC Audio driver for Intel Baytrail with MAX98090 codec" depends on SND_SOC_INTEL_MACH depends on X86_INTEL_LPSS && I2C - depends on DMADEVICES depends on SND_SST_IPC_ACPI = n depends on SND_SOC_INTEL_BAYTRAIL select SND_SOC_MAX98090 @@ -68,7 +64,6 @@ config SND_SOC_INTEL_BYT_RT5640_MACH tristate "ASoC Audio driver for Intel Baytrail with RT5640 codec" depends on SND_SOC_INTEL_MACH depends on X86_INTEL_LPSS && I2C - depends on DMADEVICES depends on SND_SST_IPC_ACPI = n depends on SND_SOC_INTEL_BAYTRAIL select SND_SOC_RT5640 -- 2.15.0
Re: [PATCH v2 06/12] hwrng: bcm2835-rng: Rework interrupt masking
Florian Fainelli writes: > The interrupt masking done for Northstart Plus and Northstar (BCM5301X) > is moved from being a function pointer mapped to of_device_id::data into > a proper part of the hwrng::init callback. While at it, we also make the > of_data be a proper structure indicating the platform specifics, since > the day we need to add a second type of platform information, we would > have to do that anyway. I still think we should just unconditionally mask off the interrupt regardless of platform if we're not going to use it in the driver and some platforms need it. Looks like a fine refactor, though: Reviewed-by: Eric Anholt signature.asc Description: PGP signature
Applied "ASoC: Intel: improve SND_SOC_INTEL_MACH dependencies" to the asoc tree
The patch ASoC: Intel: improve SND_SOC_INTEL_MACH dependencies has been applied to the asoc tree at https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark >From 960115b842886999a64a87d8baadb81dce4293b4 Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Wed, 8 Nov 2017 14:03:20 +0100 Subject: [PATCH] ASoC: Intel: improve SND_SOC_INTEL_MACH dependencies I ran into a build error with CONFIG_SND_SOC_INTEL_COMMON=m and SND_SOC_INTEL_MACH=y: ERROR: "snd_soc_acpi_intel_broadwell_machines" [sound/soc/intel/common/snd-soc-sst-acpi.ko] undefined! ERROR: "snd_soc_acpi_intel_haswell_machines" [sound/soc/intel/common/snd-soc-sst-acpi.ko] undefined! ERROR: "snd_soc_acpi_intel_cherrytrail_machines" [sound/soc/intel/atom/sst/snd-intel-sst-acpi.ko] undefined! ERROR: "snd_soc_acpi_intel_baytrail_machines" [sound/soc/intel/atom/sst/snd-intel-sst-acpi.ko] undefined! The problem here is that the sound/soc/intel/common/ directory is then entered only for building modules, but the sst-acpi.o never gets built since it depends on a built-in Kconfig symbol. That configuration obviously makes no sense since all options below SND_SOC_INTEL_MACH also depend on something else that in turn depends on CONFIG_SND_SOC_INTEL_COMMON. Adding a SND_SOC_INTEL_SST_TOPLEVEL dependency to SND_SOC_INTEL_MACH solves the build error. I notice we can also consolidate the 'depends on SND_SOC_INTEL_MACH' lines by using an 'if' block to simplify it further and make sure the configuration stays sane. Signed-off-by: Arnd Bergmann Acked-By: Vinod Koul Reviewed-by: Pierre-Louis Bossart Signed-off-by: Mark Brown --- sound/soc/intel/boards/Kconfig | 26 +- 1 file changed, 5 insertions(+), 21 deletions(-) diff --git a/sound/soc/intel/boards/Kconfig b/sound/soc/intel/boards/Kconfig index 5063f15b4ca4..6f754708a48c 100644 --- a/sound/soc/intel/boards/Kconfig +++ b/sound/soc/intel/boards/Kconfig @@ -1,10 +1,12 @@ config SND_SOC_INTEL_MACH tristate "Intel Audio machine drivers" + depends on SND_SOC_INTEL_SST_TOPLEVEL select SND_SOC_ACPI_INTEL_MATCH if ACPI +if SND_SOC_INTEL_MACH + config SND_MFLD_MACHINE tristate "SOC Machine Audio driver for Intel Medfield MID platform" - depends on SND_SOC_INTEL_MACH depends on INTEL_SCU_IPC select SND_SOC_SN95031 depends on SND_SST_ATOM_HIFI2_PLATFORM @@ -17,7 +19,6 @@ config SND_MFLD_MACHINE config SND_SOC_INTEL_HASWELL_MACH tristate "ASoC Audio DSP support for Intel Haswell Lynxpoint" - depends on SND_SOC_INTEL_MACH depends on X86_INTEL_LPSS && I2C && I2C_DESIGNWARE_PLATFORM depends on SND_SOC_INTEL_HASWELL select SND_SOC_RT5640 @@ -29,7 +30,6 @@ config SND_SOC_INTEL_HASWELL_MACH config SND_SOC_INTEL_BDW_RT5677_MACH tristate "ASoC Audio driver for Intel Broadwell with RT5677 codec" - depends on SND_SOC_INTEL_MACH depends on X86_INTEL_LPSS && GPIOLIB && I2C depends on SND_SOC_INTEL_HASWELL select SND_SOC_RT5677 @@ -39,7 +39,6 @@ config SND_SOC_INTEL_BDW_RT5677_MACH config SND_SOC_INTEL_BROADWELL_MACH tristate "ASoC Audio DSP support for Intel Broadwell Wildcatpoint" - depends on SND_SOC_INTEL_MACH depends on X86_INTEL_LPSS && I2C && I2C_DESIGNWARE_PLATFORM depends on SND_SOC_INTEL_HASWELL select SND_SOC_RT286 @@ -51,7 +50,6 @@ config SND_SOC_INTEL_BROADWELL_MACH config SND_SOC_INTEL_BYT_MAX98090_MACH tristate "ASoC Audio driver for Intel Baytrail with MAX98090 codec" - depends on SND_SOC_INTEL_MACH depends on X86_INTEL_LPSS && I2C depends on SND_SST_IPC_ACPI = n depends on SND_SOC_INTEL_BAYTRAIL @@ -62,7 +60,6 @@ config SND_SOC_INTEL_BYT_MAX98090_MACH config SND_SOC_INTEL_BYT_RT5640_MACH tristate "ASoC Audio driver for Intel Baytrail with RT5640 codec" - depends on SND_SOC_INTEL_MACH depends on X86_INTEL_LPSS && I2C depends on SND_SST_IPC_ACPI = n depends on SND_SOC_INTEL_BAYTRAIL @@ -74,7 +71,6 @@ config SND_SOC_INTEL_BYT_RT5640_MACH config SND_SOC_INTEL_BYTCR_RT5640_MACH tristate "ASoC Audio driver for Intel Baytrail and Baytrail-CR with
RE: [PATCH] dell-smbios: fix string overflow
> -Original Message- > From: sathyanarayanan kuppuswamy > [mailto:sathyanarayanan.kuppusw...@linux.intel.com] > Sent: Wednesday, November 8, 2017 12:23 PM > To: Arnd Bergmann ; Pali Rohár ; > Limonciello, Mario ; Darren Hart > ; Andy Shevchenko > Cc: Edward O'Callaghan ; Hans de Goede > ; platform-driver-...@vger.kernel.org; linux- > ker...@vger.kernel.org > Subject: Re: [PATCH] dell-smbios: fix string overflow > > Hi, > > I recommend using "platform/x86: dell-smbios:" in commit header. > > On 11/08/2017 04:08 AM, Arnd Bergmann wrote: > > The new sysfs code overwrites two fixed-length character arrays > > that are each one byte shorter than they need to be, to hold > > the trailing \0: > > > > drivers/platform/x86/dell-smbios.c: In function 'build_tokens_sysfs': > > drivers/platform/x86/dell-smbios.c:494:42: error: 'sprintf' writing a > > terminating > nul past the end of the destination [-Werror=format-overflow=] > > sprintf(buffer_location, "%04x_location", > > drivers/platform/x86/dell-smbios.c:494:3: note: 'sprintf' output 14 bytes > > into a > destination of size 13 > > drivers/platform/x86/dell-smbios.c:506:36: error: 'sprintf' writing a > > terminating > nul past the end of the destination [-Werror=format-overflow=] > > sprintf(buffer_value, "%04x_value", > > drivers/platform/x86/dell-smbios.c:506:3: note: 'sprintf' output 11 bytes > > into a > destination of size 10 > Don't need to include the error log in commit message. Just explaining > the issue is good enough. > > > > This changes it to just use kasprintf(), which always gets it right. > > > > Fixes: 33b9ca1e53b4 ("platform/x86: dell-smbios: Add a sysfs interface for > SMBIOS tokens") > > Signed-off-by: Arnd Bergmann > > --- > > drivers/platform/x86/dell-smbios.c | 12 > > 1 file changed, 4 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell- > smbios.c > > index d99edd803c19..6a60db515bda 100644 > > --- a/drivers/platform/x86/dell-smbios.c > > +++ b/drivers/platform/x86/dell-smbios.c > > @@ -463,8 +463,6 @@ static struct platform_driver platform_driver = { > > > > static int build_tokens_sysfs(struct platform_device *dev) > > { > > - char buffer_location[13]; > > - char buffer_value[10]; > > char *location_name; > > char *value_name; > > size_t size; > > @@ -491,9 +489,8 @@ static int build_tokens_sysfs(struct platform_device > *dev) > > if (da_tokens[i].tokenID == 0) > > continue; > > /* add location */ > > - sprintf(buffer_location, "%04x_location", > > - da_tokens[i].tokenID); > > - location_name = kstrdup(buffer_location, GFP_KERNEL); > > + location_name = kasprintf(GFP_KERNEL, "%04x_location", > > + da_tokens[i].tokenID); > > if (location_name == NULL) > > goto out_unwind_strings; > > sysfs_attr_init(&token_location_attrs[i].attr); > > @@ -503,9 +500,8 @@ static int build_tokens_sysfs(struct platform_device > *dev) > > token_attrs[j++] = &token_location_attrs[i].attr; > > > > /* add value */ > > - sprintf(buffer_value, "%04x_value", > > - da_tokens[i].tokenID); > > - value_name = kstrdup(buffer_value, GFP_KERNEL); > > + value_name = kasprintf(GFP_KERNEL, "%04x_value", > > + da_tokens[i].tokenID); > > if (value_name == NULL) > > goto loop_fail_create_value; > > sysfs_attr_init(&token_value_attrs[i].attr); > > -- Arnd, Assuming Darren will do the fixup for title and message as recommended by Sathyanarayanan before committing: Acked-by: Mario Limonciello Thanks, for the fix. For my own information and improvement, would you share how you caught that? Just added "-Werror=format-overflow=" to CFLAGS? I wasn't seeing the compile errors with default flags in my own testing, so I'd like to make sure I'm doing better testing in the future.
Re: [PATCH v2 07/12] hwrng: bcm2835-rng: Manage an optional clock
Florian Fainelli writes: > One of the last steps before bcm63xx-rng can be eliminated is to manage > a clock during hwrng::init and hwrng::cleanup, so fetch it in the probe > function, and manage it during these two steps when valid. > > Signed-off-by: Florian Fainelli > --- > drivers/char/hw_random/bcm2835-rng.c | 17 - > 1 file changed, 16 insertions(+), 1 deletion(-) > > diff --git a/drivers/char/hw_random/bcm2835-rng.c > b/drivers/char/hw_random/bcm2835-rng.c > index ed20e0b6b7ae..99b56fd5482c 100644 > --- a/drivers/char/hw_random/bcm2835-rng.c > +++ b/drivers/char/hw_random/bcm2835-rng.c > @@ -15,6 +15,7 @@ > #include > #include > #include > +#include > > #define RNG_CTRL 0x0 > #define RNG_STATUS 0x4 > @@ -33,6 +34,7 @@ struct bcm2835_rng_priv { > struct hwrng rng; > void __iomem *base; > bool mask_interrupts; > + struct clk *clk; > }; > > static inline struct bcm2835_rng_priv *to_rng_priv(struct hwrng *rng) > @@ -66,8 +68,15 @@ static int bcm2835_rng_read(struct hwrng *rng, void *buf, > size_t max, > static int bcm2835_rng_init(struct hwrng *rng) > { > struct bcm2835_rng_priv *priv = to_rng_priv(rng); > + int ret = 0; > u32 val; > > + if (!IS_ERR(priv->clk)) { > + ret = clk_prepare_enable(priv->clk); > + if (ret) > + return ret; > + } clk_prepare_enable() is protected by IS_ERR_OR_NULL() checks and will return 0. > + > if (priv->mask_interrupts) { > /* mask the interrupt */ > val = readl(priv->base + RNG_INT_MASK); > @@ -79,7 +88,7 @@ static int bcm2835_rng_init(struct hwrng *rng) > __raw_writel(RNG_WARMUP_COUNT, priv->base + RNG_STATUS); > __raw_writel(RNG_RBGEN, priv->base + RNG_CTRL); > > - return 0; > + return ret; > } > > static void bcm2835_rng_cleanup(struct hwrng *rng) > @@ -88,6 +97,9 @@ static void bcm2835_rng_cleanup(struct hwrng *rng) > > /* disable rng hardware */ > __raw_writel(0, priv->base + RNG_CTRL); > + > + if (!IS_ERR(priv->clk)) > + clk_disable_unprepare(priv->clk); Same. With those conditionals dropped, Reviewed-by: Eric Anholt signature.asc Description: PGP signature
Re: [PATCH] sh: migor: Reserve memory block for CEU
Hi Jacopo, On Wed, Nov 8, 2017 at 7:05 PM, Jacopo Mondi wrote: > A memory region for CEU video buffer has to be reserved during machine > initialization. > > Originally, it was allocated through DMA API helpers and stored in the > second IORESOURCE_MEM entry, to be later remapped by the CEU driver with > a call to 'dma_declare_coherent_memory()' > > As Linux does not allow anymore to remap system RAM regions with > 'memremap' function, sh_mobile_ceu driver fails when trying to remap the > memory area: > > WARN_ONCE(1, "memremap attempted on ram %pa size: %#lx\n", > &offset, (unsigned long) size) > > from 'memremap()' function in kernel/memremap.c > > To avoid hitting that WARN_ONCE() and have memory successfully remapped, > reserve a region using '.mv_mem_reserve' member of SH's 'struct > sh_machine_vector' to make sure memory is reserved early enough, and > removed from the available system RAM. > > This is similar to what happens on ARM architecture with > 'arm_memblock_steal()' function. > > Suggested-by: Laurent Pinchart > Signed-off-by: Jacopo Mondi I assume this failure isnot limited to CEU on Migo-R, but applies to the other 24 callers of platform_resource_setup_memory(), too? Can platform_resource_setup_memory() be fixed instead, or is that difficult due to the need to reserve the memory very early in the boot process? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [PATCH v2 08/12] hwrng: bcm2835-rng: Abstract I/O accessors
Florian Fainelli writes: > In preparation for allowing BCM63xx to use this driver, we abstract I/O > accessors such that we can easily change those later on. This may even be a fix on rpi, since i/o from different devices can get reordered and you're using the barrier variants now! Reviewed-by: Eric Anholt signature.asc Description: PGP signature
Re: [PATCH v2] s390/virtio: mark headers as BSD licensed
On 11/08/2017 07:15 PM, Cornelia Huck wrote: > On Wed, 8 Nov 2017 19:12:25 +0100 > Christian Borntraeger wrote: > >> On 11/08/2017 05:59 PM, Michael S. Tsirkin wrote: >>> Virtio UAPI headers aren't just for UAPI, it's for guests/hypervisors as >>> well. The s390 ones need to be BSD as well. >>> >>> Fixes: e2be04c7f995 ("License cleanup: add SPDX license identifier to uapi >>> header files with a license") >>> Cc: Greg Kroah-Hartman >>> Signed-off-by: Michael S. Tsirkin >>> --- >>> >>> since v1: >>> drop an extra comment chunk, reported by Cornelia >>> >>> arch/s390/include/uapi/asm/kvm_virtio.h | 26 +- >> >> FWIW, this file will go away anyway. >> Looks like the following commit in the s390 feature branch >> https://git.kernel.org/pub/scm/linux/kernel/git/s390/linux.git/commit/drivers/s390/virtio/Makefile?h=features&id=7fb2b2d512448cf0e914c4647a1cf02b52263702 >> missed this file. > > Can we simply rip this out? > >> >> >> Regarding the virtio-ccw.h headers it is probably something that happened by >> accident. >> Back then Rusty (as an IBMer) proposed to change all virtio headers to BSD >> licence. > > ISTR that the relicensing already went through... is that not the case? The original kvm_virtio.h code was added in 2008 commit e976a2b997fc4ad70ccc53acfe62811c4aaec851 Author: Christian Borntraeger Date: Tue Mar 25 18:47:46 2008 +0100 s390: KVM guest: virtio device support, and kvm hypercalls The BSD re-licencing happened in 2008 commit 674bfc23c585b34c42263d73fb51710d49762a23 Author: Rusty Russell Date: Fri Jul 25 12:06:03 2008 -0500 virtio: clarify that ABI is usable by any implementations We want others to implement and use virtio, so it makes sense to BSD license the non-__KERNEL__ parts of the headers to make this crystal clear. and it seems that it missed arch/include/uapi/asm/kvm_virtio.h arch/include/uapi/asm/virtio_ccw.h probably just copied what kvm_virtio.h had in 2013. So its somewhat fair to assume that both files should be BSD, but they are not according to the licence statement. Gregs SPDX change just changed the GPL text in the header files to a SPDX statement. Now Michael wants to change this to BSD as it probably was intended but not written down. Sigh. > I thought this was just a fixup to align with reality? > >> >>> arch/s390/include/uapi/asm/virtio-ccw.h | 26 +- >>> 2 files changed, 50 insertions(+), 2 deletions(-) >
RE: [Revised PATCH v2] hv: kvp: Avoid reading past allocated blocks from KVP file
> -Original Message- > From: Haiyang Zhang > Sent: Wednesday, November 8, 2017 6:22 AM > To: Long Li ; KY Srinivasan ; > Stephen Hemminger ; > de...@linuxdriverproject.org; linux-kernel@vger.kernel.org > Cc: Paul Meyer ; Long Li > > Subject: RE: [Revised PATCH v2] hv: kvp: Avoid reading past allocated blocks > from KVP file > > > > > -Original Message- > > From: Long Li [mailto:lon...@exchange.microsoft.com] > > Sent: Wednesday, November 1, 2017 2:45 PM > > To: KY Srinivasan ; Haiyang Zhang > > ; Stephen Hemminger > > ; de...@linuxdriverproject.org; linux- > > ker...@vger.kernel.org > > Cc: Paul Meyer ; Long Li > > > > Subject: [Revised PATCH v2] hv: kvp: Avoid reading past allocated blocks > > from KVP file > > > > [This sender failed our fraud detection checks and may not be who they > > appear to be. Learn about spoofing at http://aka.ms/LearnAboutSpoofing] > > > > From: Paul Meyer > > > > While reading in more than one block (50) of KVP records, the allocation > > goes per block, but the reads used the total number of allocated records > > (without resetting the pointer/stream). This causes the records buffer to > > overrun when the refresh reads more than one block over the previous > > capacity (e.g. reading more than 100 KVP records whereas the in-memory > > database was empty before). > > > > Fix this by reading the correct number of KVP records from file each time. > > > > Changes since v1: > > 1. Properly wrapped comment texts. > > 2. Added the 2nd Signed-off-by. > > > > Signed-off-by: Paul Meyer > > Signed-off-by: Long Li > > Reviewed-by: Haiyang Zhang I will take this patch. K. Y
[RFC PATCH for 4.15 4/6] membarrier: Provide core serializing command
Provide core serializing membarrier command to support memory reclaim by JIT. Each architecture needs to explicitly opt into that support by documenting in their architecture code how they provide the core serializing instructions required when returning from the membarrier IPI, and after the scheduler has updated the curr->mm pointer (before going back to user-space). They should then define ARCH_HAS_MEMBARRIER_SYNC_CORE to enable support for that command on their architecture. Signed-off-by: Mathieu Desnoyers CC: Peter Zijlstra CC: Andy Lutomirski CC: Paul E. McKenney CC: Boqun Feng CC: Andrew Hunter CC: Maged Michael CC: gro...@google.com CC: Avi Kivity CC: Benjamin Herrenschmidt CC: Paul Mackerras CC: Michael Ellerman CC: Dave Watson CC: Thomas Gleixner CC: Ingo Molnar CC: "H. Peter Anvin" CC: Andrea Parri --- include/linux/sched/mm.h| 9 +-- include/uapi/linux/membarrier.h | 14 --- init/Kconfig| 3 +++ kernel/sched/membarrier.c | 56 ++--- 4 files changed, 61 insertions(+), 21 deletions(-) diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h index ff1d510a9c51..a888da398517 100644 --- a/include/linux/sched/mm.h +++ b/include/linux/sched/mm.h @@ -219,8 +219,13 @@ static inline void memalloc_noreclaim_restore(unsigned int flags) #ifdef CONFIG_MEMBARRIER enum { - MEMBARRIER_STATE_PRIVATE_EXPEDITED_READY= (1U << 0), - MEMBARRIER_STATE_SWITCH_MM = (1U << 1), + MEMBARRIER_STATE_PRIVATE_EXPEDITED_READY= (1U << 0), + MEMBARRIER_STATE_SWITCH_MM = (1U << 1), + MEMBARRIER_STATE_PRIVATE_EXPEDITED_SYNC_CORE_READY = (1U << 2), +}; + +enum { + MEMBARRIER_FLAG_SYNC_CORE = (1U << 0), }; #ifdef CONFIG_ARCH_HAS_MEMBARRIER_HOOKS diff --git a/include/uapi/linux/membarrier.h b/include/uapi/linux/membarrier.h index 4e01ad7ffe98..4dd4715a6c08 100644 --- a/include/uapi/linux/membarrier.h +++ b/include/uapi/linux/membarrier.h @@ -64,18 +64,24 @@ * Register the process intent to use * MEMBARRIER_CMD_PRIVATE_EXPEDITED. Always * returns 0. + * @MEMBARRIER_CMD_PRIVATE_EXPEDITED_SYNC_CORE: + * TODO + * @MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED_SYNC_CORE: + * TODO * * Command to be passed to the membarrier system call. The commands need to * be a single bit each, except for MEMBARRIER_CMD_QUERY which is assigned to * the value 0. */ enum membarrier_cmd { - MEMBARRIER_CMD_QUERY= 0, - MEMBARRIER_CMD_SHARED = (1 << 0), + MEMBARRIER_CMD_QUERY= 0, + MEMBARRIER_CMD_SHARED = (1 << 0), /* reserved for MEMBARRIER_CMD_SHARED_EXPEDITED (1 << 1) */ /* reserved for MEMBARRIER_CMD_PRIVATE (1 << 2) */ - MEMBARRIER_CMD_PRIVATE_EXPEDITED= (1 << 3), - MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED = (1 << 4), + MEMBARRIER_CMD_PRIVATE_EXPEDITED= (1 << 3), + MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED = (1 << 4), + MEMBARRIER_CMD_PRIVATE_EXPEDITED_SYNC_CORE = (1 << 5), + MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED_SYNC_CORE = (1 << 6), }; #endif /* _UAPI_LINUX_MEMBARRIER_H */ diff --git a/init/Kconfig b/init/Kconfig index ba7f17971554..ea4a7ce10328 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -1403,6 +1403,9 @@ config MEMBARRIER config ARCH_HAS_MEMBARRIER_HOOKS bool +config ARCH_HAS_MEMBARRIER_SYNC_CORE + bool + config EMBEDDED bool "Embedded system" option allnoconfig_y diff --git a/kernel/sched/membarrier.c b/kernel/sched/membarrier.c index 23bd256f8458..6b3cbaa03ed7 100644 --- a/kernel/sched/membarrier.c +++ b/kernel/sched/membarrier.c @@ -26,24 +26,41 @@ * Bitmask made from a "or" of all commands within enum membarrier_cmd, * except MEMBARRIER_CMD_QUERY. */ +#ifdef CONFIG_ARCH_HAS_MEMBARRIER_SYNC_CORE +#define MEMBARRIER_PRIVATE_EXPEDITED_SYNC_CORE_BITMASK \ + (MEMBARRIER_CMD_PRIVATE_EXPEDITED_SYNC_CORE \ + | MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED_SYNC_CORE) +#else +#define MEMBARRIER_PRIVATE_EXPEDITED_SYNC_CORE_BITMASK 0 +#endif + #define MEMBARRIER_CMD_BITMASK \ (MEMBARRIER_CMD_SHARED | MEMBARRIER_CMD_PRIVATE_EXPEDITED \ - | MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED) + | MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED \ + | MEMBARRIER_PRIVATE_EXPEDITED_SYNC_CORE_BITMASK) static void ipi_mb(void *info) { smp_mb(); /* IPIs should be serializing but paranoid. */ } -static int membarrier_private_expedited(void) +static int membarrier_private_expedited(int flags) { int cpu; boo
[RFC PATCH for 4.15 0/6] membarrier updates for 4.15
Here are the membarrier changes I plan on sending for the 4.15 merge window. This series includes selftests improvements for sys_membarrier, improvement of powerpc handling of the memory barrier required by sys_membarrier in switch_mm(), and adds a new core serializing membarrier, currently only implemented on x86. Architectures wishing to provide the core serializing membarrier need to select ARCH_HAS_MEMBARRIER_SYNC_CORE and document how they provide the core serialization required by that command in their architecture code. Andy, I know you told me you had changes coming up in x86 entry.S for 4.15, but I figure that managing the merge conflict between your changes in 4.15 and those added comments should be straightforward. Anyway, I kind of suspect that at any given point in time you will always have changes of some sort to propose to entry.S, so now seems to be a time as appropriate as ever to push the core serializing membarrier comments. Feedback is welcome! Thanks, Mathieu Mathieu Desnoyers (6): membarrier: selftest: Test private expedited cmd membarrier: powerpc: Skip memory barrier in switch_mm() (v6) membarrier: Document scheduler barrier requirements (v5) membarrier: Provide core serializing command membarrier: x86: Provide core serializing command membarrier: selftest: Test private expedited sync core cmd MAINTAINERS| 2 + arch/powerpc/Kconfig | 1 + arch/powerpc/include/asm/membarrier.h | 32 arch/powerpc/kernel/Makefile | 2 + arch/powerpc/kernel/membarrier.c | 37 arch/powerpc/mm/mmu_context.c | 7 + arch/x86/Kconfig | 2 + arch/x86/entry/entry_32.S | 5 + arch/x86/entry/entry_64.S | 8 + arch/x86/include/asm/membarrier.h | 36 arch/x86/kernel/Makefile | 1 + arch/x86/kernel/membarrier.c | 39 + arch/x86/mm/tlb.c | 6 + include/linux/sched/mm.h | 36 +++- include/uapi/linux/membarrier.h| 14 +- init/Kconfig | 6 + kernel/sched/core.c| 53 +++--- kernel/sched/membarrier.c | 55 -- .../testing/selftests/membarrier/membarrier_test.c | 186 +++-- 19 files changed, 470 insertions(+), 58 deletions(-) create mode 100644 arch/powerpc/include/asm/membarrier.h create mode 100644 arch/powerpc/kernel/membarrier.c create mode 100644 arch/x86/include/asm/membarrier.h create mode 100644 arch/x86/kernel/membarrier.c -- 2.11.0
[RFC PATCH for 4.15 6/6] membarrier: selftest: Test private expedited sync core cmd
Test the new MEMBARRIER_CMD_PRIVATE_EXPEDITED_SYNC_CORE and MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED_SYNC_CORE commands. Signed-off-by: Mathieu Desnoyers CC: Shuah Khan CC: Greg Kroah-Hartman CC: Peter Zijlstra CC: Paul E. McKenney CC: Boqun Feng CC: Andrew Hunter CC: Maged Michael CC: gro...@google.com CC: Avi Kivity CC: Benjamin Herrenschmidt CC: Paul Mackerras CC: Michael Ellerman CC: Dave Watson CC: Alan Stern CC: Will Deacon CC: Andy Lutomirski CC: Alice Ferrazzi CC: Paul Elder CC: linux-kselft...@vger.kernel.org CC: linux-a...@vger.kernel.org --- .../testing/selftests/membarrier/membarrier_test.c | 77 +- 1 file changed, 76 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/membarrier/membarrier_test.c b/tools/testing/selftests/membarrier/membarrier_test.c index d7543a6d9030..a0eae8d51e72 100644 --- a/tools/testing/selftests/membarrier/membarrier_test.c +++ b/tools/testing/selftests/membarrier/membarrier_test.c @@ -132,6 +132,63 @@ static int test_membarrier_private_expedited_success(void) return 0; } +static int test_membarrier_private_expedited_sync_core_fail(void) +{ + int cmd = MEMBARRIER_CMD_PRIVATE_EXPEDITED_SYNC_CORE, flags = 0; + const char *test_name = "sys membarrier MEMBARRIER_CMD_PRIVATE_EXPEDITED_SYNC_CORE not registered failure"; + + if (sys_membarrier(cmd, flags) != -1) { + ksft_exit_fail_msg( + "%s test: flags = %d. Should fail, but passed\n", + test_name, flags); + } + if (errno != EPERM) { + ksft_exit_fail_msg( + "%s test: flags = %d. Should return (%d: \"%s\"), but returned (%d: \"%s\").\n", + test_name, flags, EPERM, strerror(EPERM), + errno, strerror(errno)); + } + + ksft_test_result_pass( + "%s test: flags = %d, errno = %d\n", + test_name, flags, errno); + return 0; +} + +static int test_membarrier_register_private_expedited_sync_core_success(void) +{ + int cmd = MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED_SYNC_CORE, flags = 0; + const char *test_name = "sys membarrier MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED_SYNC_CORE"; + + if (sys_membarrier(cmd, flags) != 0) { + ksft_exit_fail_msg( + "%s test: flags = %d, errno = %d\n", + test_name, flags, errno); + } + + ksft_test_result_pass( + "%s test: flags = %d\n", + test_name, flags); + return 0; +} + +static int test_membarrier_private_expedited_sync_core_success(void) +{ + int cmd = MEMBARRIER_CMD_PRIVATE_EXPEDITED, flags = 0; + const char *test_name = "sys membarrier MEMBARRIER_CMD_PRIVATE_EXPEDITED_SYNC_CORE"; + + if (sys_membarrier(cmd, flags) != 0) { + ksft_exit_fail_msg( + "%s test: flags = %d, errno = %d\n", + test_name, flags, errno); + } + + ksft_test_result_pass( + "%s test: flags = %d\n", + test_name, flags); + return 0; +} + static int test_membarrier(void) { int status; @@ -154,6 +211,22 @@ static int test_membarrier(void) status = test_membarrier_private_expedited_success(); if (status) return status; + status = sys_membarrier(MEMBARRIER_CMD_QUERY, 0); + if (status < 0) { + ksft_test_result_fail("sys_membarrier() failed\n"); + return status; + } + if (status & MEMBARRIER_CMD_PRIVATE_EXPEDITED_SYNC_CORE) { + status = test_membarrier_private_expedited_sync_core_fail(); + if (status) + return status; + status = test_membarrier_register_private_expedited_sync_core_success(); + if (status) + return status; + status = test_membarrier_private_expedited_sync_core_success(); + if (status) + return status; + } return 0; } @@ -173,8 +246,10 @@ static int test_membarrier_query(void) } ksft_exit_fail_msg("sys_membarrier() failed\n"); } - if (!(ret & MEMBARRIER_CMD_SHARED)) + if (!(ret & MEMBARRIER_CMD_SHARED)) { + ksft_test_result_fail("sys_membarrier() CMD_SHARED query failed\n"); ksft_exit_fail_msg("sys_membarrier is not supported.\n"); + } ksft_test_result_pass("sys_membarrier available\n"); return 0; -- 2.11.0
[RFC PATCH for 4.15 5/6] membarrier: x86: Provide core serializing command
There are two places where core serialization is needed by membarrier: 1) When returning from the membarrier IPI, 2) After scheduler updates curr to a thread with a different mm, before going back to user-space, since the curr->mm is used by membarrier to check whether it needs to send an IPI to that CPU. x86-32 uses only iret both as return from interrupt, and to go back to user-space. The iret instruction is core serializing. x86-64 uses iret as return from interrupt, which takes care of the IPI. However, it can return to user-space through either sysretl (compat code), sysretq, or iret. Given that sysret{l,q} is not core serializing, we rely instead on write_cr3() performed by switch_mm() to provide core serialization after changing the current mm, and deal with the special case of kthread -> uthread (temporarily keeping current mm into active_mm) by adding a sync_core() in that specific case. Signed-off-by: Mathieu Desnoyers CC: Peter Zijlstra CC: Andy Lutomirski CC: Paul E. McKenney CC: Boqun Feng CC: Andrew Hunter CC: Maged Michael CC: gro...@google.com CC: Avi Kivity CC: Benjamin Herrenschmidt CC: Paul Mackerras CC: Michael Ellerman CC: Dave Watson CC: Thomas Gleixner CC: Ingo Molnar CC: "H. Peter Anvin" CC: Andrea Parri CC: x...@kernel.org --- MAINTAINERS | 2 ++ arch/powerpc/include/asm/membarrier.h | 8 ++- arch/powerpc/kernel/membarrier.c | 3 ++- arch/x86/Kconfig | 2 ++ arch/x86/entry/entry_32.S | 5 + arch/x86/entry/entry_64.S | 8 +++ arch/x86/include/asm/membarrier.h | 36 arch/x86/kernel/Makefile | 1 + arch/x86/kernel/membarrier.c | 39 +++ arch/x86/mm/tlb.c | 7 --- include/linux/sched/mm.h | 9 +++- kernel/sched/core.c | 6 +- 12 files changed, 119 insertions(+), 7 deletions(-) create mode 100644 arch/x86/include/asm/membarrier.h create mode 100644 arch/x86/kernel/membarrier.c diff --git a/MAINTAINERS b/MAINTAINERS index 34687a0ec28c..ff564e5195fb 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -8831,6 +8831,8 @@ F:kernel/sched/membarrier.c F: include/uapi/linux/membarrier.h F: arch/powerpc/kernel/membarrier.c F: arch/powerpc/include/asm/membarrier.h +F: arch/x86/kernel/membarrier.c +F: arch/x86/include/asm/membarrier.h MEMORY MANAGEMENT L: linux...@kvack.org diff --git a/arch/powerpc/include/asm/membarrier.h b/arch/powerpc/include/asm/membarrier.h index 0951646253d9..018cf278dc93 100644 --- a/arch/powerpc/include/asm/membarrier.h +++ b/arch/powerpc/include/asm/membarrier.h @@ -21,6 +21,12 @@ static inline void membarrier_arch_switch_mm(struct mm_struct *prev, */ smp_mb(); } -void membarrier_arch_register_private_expedited(struct task_struct *t); + +static inline void membarrier_arch_mm_sync_core(void) +{ +} + +void membarrier_arch_register_private_expedited(struct task_struct *t, + int flags); #endif /* _ASM_POWERPC_MEMBARRIER_H */ diff --git a/arch/powerpc/kernel/membarrier.c b/arch/powerpc/kernel/membarrier.c index 4795ad59b833..0026d740e5a3 100644 --- a/arch/powerpc/kernel/membarrier.c +++ b/arch/powerpc/kernel/membarrier.c @@ -21,7 +21,8 @@ #include #include -void membarrier_arch_register_private_expedited(struct task_struct *p) +void membarrier_arch_register_private_expedited(struct task_struct *p, + int flags) { struct mm_struct *mm = p->mm; diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index 2fdb23313dd5..6ac32fe768a8 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -54,6 +54,8 @@ config X86 select ARCH_HAS_FORTIFY_SOURCE select ARCH_HAS_GCOV_PROFILE_ALL select ARCH_HAS_KCOVif X86_64 + select ARCH_HAS_MEMBARRIER_HOOKS + select ARCH_HAS_MEMBARRIER_SYNC_CORE select ARCH_HAS_PMEM_APIif X86_64 # Causing hangs/crashes, see the commit that added this change for details. select ARCH_HAS_REFCOUNTif BROKEN diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S index 4838037f97f6..04e5daba8456 100644 --- a/arch/x86/entry/entry_32.S +++ b/arch/x86/entry/entry_32.S @@ -553,6 +553,11 @@ restore_all: .Lrestore_nocheck: RESTORE_REGS 4 # skip orig_eax/error_code .Lirq_return: + /* +* ARCH_HAS_MEMBARRIER_SYNC_CORE rely on iret core serialization +* when returning from IPI handler and when returning from +* scheduler to user-space. +*/ INTERRUPT_RETURN .section .fixup, "ax" diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S index bcfc5668dcb2..4859f04e1695 100644 --- a/arch/x86/entry/entry_64.S +++ b/arch/x86/entry/entry_64.S @@ -642,6 +642,10 @@ GLOBAL(restore_regs_and_iret) restore_c
Re: [vlan_device_event] BUG: unable to handle kernel paging request at 6b6b6ccf
On Wed, Nov 8, 2017 at 9:12 AM, Fengguang Wu wrote: > Hi Linus, > > > On Wed, Nov 08, 2017 at 08:20:38AM -0800, Linus Torvalds wrote: >> >> On Wed, Nov 8, 2017 at 1:48 AM, Fengguang Wu >> wrote: >>> >>> >>> Now I got the faddr2line output. :) >> >> >> Thank you, but this also shows that you then compress the output too >> much for convenience. >> >>> [ 745.719623] BUG: unable to handle kernel paging request at 6b6b6f4f >>> [ 745.732871] IP: vlan_device_event at net/8021q/vlan.h:60 >> >> >> So this looks more legible than "vlan_device_event+0x7f5/0xa40" (or >> whatever), but in fact it's not. >> >> The reason faddr2line is great is because it gives inlining >> information, so that you can see exactly which access it is, even if >> it's inside some helper inline function (macros still obviously are >> going to be problematic). >> >> And you've cut the whole information down, to the point where there is >> _less_ information than there used to be. >> >> So the full faddr2line output is actually important, and should look >> something like this: >> >>__schedule+0x315/0x840: >>rq_sched_info_arrive at kernel/sched/stats.h:13 >> (inlined by) sched_info_arrive at kernel/sched/stats.h:99 >> (inlined by) __sched_info_switch at kernel/sched/stats.h:151 >> (inlined by) sched_info_switch at kernel/sched/stats.h:158 >> (inlined by) prepare_task_switch at kernel/sched/core.c:2582 >> (inlined by) context_switch at kernel/sched/core.c:2755 >> (inlined by) __schedule at kernel/sched/core.c:3366 >> >> which is admittedly a fairly extreme case, but it shows how involved >> things can be. > > > Indeed! I wasn't aware that faddr2line could have so informative > output! After checking several of its outputs, I decided to pipe its > output to |tail -1 :/ That's clearly due to my lacking of first hand > experience on oops analyzing! > > IMHO this kind of informative demo in your email could be very good > candidates to put in Documentation/. If you search faddr2line under > Documentation there's no single mention of it (addr2line does show up > 7 times). > > >> Note how in my example above the access itself is from a header file >> (that inlined rq_sched_info_arrive() function), and I happened to pick >> the >> >>rq->rq_sched_info.pcount++; >> >> line. But Many inline functions are inlined from different points, >> often many times in the same top-level function (think of the atomic >> helper inlines, for example). >> >> In your case, that net/8021q/vlan.h:60 information is literally not >> helping, because it only shows what I could already figure out from >> looking at the constants in the disassembly: the code comes from the >> >>vlan_group_for_each_dev(...) { >>.. >> >> loop setup (it's the inline __vlan_group_get_device() function, and >> the constants I could recognize are the VLAN_GROUP_ARRAY_PART_LEN >> things. >> >> But exactly like the constants didn't tell me *which* invocation of >> that loop it was, your "net/8021q/vlan.h:60" doesn't tell me which one >> it is. >> >> There's at least _eight_ different "vlan_group_for_each_dev() {" loops >> in vlan_device_event(), and maybe it's not all that meaningful which >> of the eight it is in this case, in other cases it's critical. >> >> And since you've actually replaced the "vlan_device_event+0x7f5/0xa40" >> with that "net/8021q/vlan.h:60" entirely, all the offset information >> that *could* maybe have been used to pick one case over another (but >> likely not) is also gone. >> >> That's why that inlining chain is important - so that we can see how >> it got to that access in __vlan_group_get_device(). >> >> So please do keep _all_ of the faddr2line output, at least for the >> "IP:" line (the stack traces might not be worth it). > > > OK. Here is the original faddr2line output: > > $ ~/linux/scripts/faddr2line vmlinux vlan_device_event+0x7f5/0xa40 > vlan_device_event+0x7f5/0xa40: > vlan_device_event at net/8021q/vlan.h:60 > > And below is call trace embedded with full faddr2line output. > > I notice that this trace shows no additional inline files at all. > Is it because I did some kconfig option wrong, so that inline info is > lost? Eg. > > CONFIG_OPTIMIZE_INLINING=y (reading lib/Kconfig.debug, it looks better set > to N) > CONFIG_DEBUG_INFO_REDUCED=y > CONFIG_DEBUG_INFO_SPLIT=y > (full .config attached) > > [ 745.719623] BUG: unable to handle kernel paging request at 6b6b6f4f > [ 745.732871] IP: vlan_device_event+0x7f5/0xa40: > vlan_device_event at net/8021q/vlan.h:60 > [ 745.742106] *pde = > [ 745.748587] Oops: [#1] PREEMPT > [ 745.756104] CPU: 0 PID: 786 Comm: netifd Not tainted 4.14.0-rc8 #1 > [ 745.769171] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS > 1.10.2-1 04/01/2014 > [ 745.786791] task: cf768780 task.stack: d187a000 > [ 745.796485] EIP: vlan_device_event+0x7f5/0xa40: > vlan_device_event at net/8021q/vlan.h:60 > [ 745.805877] EFLAGS: 00010206 CPU: 0 >
[RFC PATCH for 4.15 2/6] membarrier: powerpc: Skip memory barrier in switch_mm() (v6)
Allow PowerPC to skip the full memory barrier in switch_mm(), and only issue the barrier when scheduling into a task belonging to a process that has registered to use expedited private. Threads targeting the same VM but which belong to different thread groups is a tricky case. It has a few consequences: It turns out that we cannot rely on get_nr_threads(p) to count the number of threads using a VM. We can use (atomic_read(&mm->mm_users) == 1 && get_nr_threads(p) == 1) instead to skip the synchronize_sched() for cases where the VM only has a single user, and that user only has a single thread. It also turns out that we cannot use for_each_thread() to set thread flags in all threads using a VM, as it only iterates on the thread group. Therefore, test the membarrier state variable directly rather than relying on thread flags. This means membarrier_register_private_expedited() needs to set the MEMBARRIER_STATE_SWITCH_MM flag, issue synchronize_sched(), and only then set MEMBARRIER_STATE_PRIVATE_EXPEDITED_READY which allows private expedited membarrier commands to succeed. membarrier_arch_switch_mm() now tests for the MEMBARRIER_STATE_SWITCH_MM flag. Changes since v1: - Use test_ti_thread_flag(next, ...) instead of test_thread_flag() in powerpc membarrier_arch_sched_in(), given that we want to specifically check the next thread state. - Add missing ARCH_HAS_MEMBARRIER_HOOKS in Kconfig. - Use task_thread_info() to pass thread_info from task to *_ti_thread_flag(). Changes since v2: - Move membarrier_arch_sched_in() call to finish_task_switch(). - Check for NULL t->mm in membarrier_arch_fork(). - Use membarrier_sched_in() in generic code, which invokes the arch-specific membarrier_arch_sched_in(). This fixes allnoconfig build on PowerPC. - Move asm/membarrier.h include under CONFIG_MEMBARRIER, fixing allnoconfig build on PowerPC. - Build and runtime tested on PowerPC. Changes since v3: - Simply rely on copy_mm() to copy the membarrier_private_expedited mm field on fork. - powerpc: test thread flag instead of reading membarrier_private_expedited in membarrier_arch_fork(). - powerpc: skip memory barrier in membarrier_arch_sched_in() if coming from kernel thread, since mmdrop() implies a full barrier. - Set membarrier_private_expedited to 1 only after arch registration code, thus eliminating a race where concurrent commands could succeed when they should fail if issued concurrently with process registration. - Use READ_ONCE() for membarrier_private_expedited field access in membarrier_private_expedited. Matches WRITE_ONCE() performed in process registration. Changes since v4: - Move powerpc hook from sched_in() to switch_mm(), based on feedback from Nicholas Piggin. Changes since v5: - Rebase on v4.14-rc6. - Fold "Fix: membarrier: Handle CLONE_VM + !CLONE_THREAD correctly on powerpc (v2)" Signed-off-by: Mathieu Desnoyers CC: Peter Zijlstra CC: Paul E. McKenney CC: Boqun Feng CC: Andrew Hunter CC: Maged Michael CC: gro...@google.com CC: Avi Kivity CC: Benjamin Herrenschmidt CC: Paul Mackerras CC: Michael Ellerman CC: Dave Watson CC: Alan Stern CC: Will Deacon CC: Andy Lutomirski CC: Ingo Molnar CC: Alexander Viro CC: Nicholas Piggin CC: linuxppc-...@lists.ozlabs.org CC: linux-a...@vger.kernel.org --- MAINTAINERS | 2 ++ arch/powerpc/Kconfig | 1 + arch/powerpc/include/asm/membarrier.h | 26 + arch/powerpc/kernel/Makefile | 2 ++ arch/powerpc/kernel/membarrier.c | 36 +++ arch/powerpc/mm/mmu_context.c | 7 +++ include/linux/sched/mm.h | 15 +++ init/Kconfig | 3 +++ kernel/sched/core.c | 10 -- kernel/sched/membarrier.c | 1 + 10 files changed, 93 insertions(+), 10 deletions(-) create mode 100644 arch/powerpc/include/asm/membarrier.h create mode 100644 arch/powerpc/kernel/membarrier.c diff --git a/MAINTAINERS b/MAINTAINERS index 2f4e462aa4a2..34687a0ec28c 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -8829,6 +8829,8 @@ L:linux-kernel@vger.kernel.org S: Supported F: kernel/sched/membarrier.c F: include/uapi/linux/membarrier.h +F: arch/powerpc/kernel/membarrier.c +F: arch/powerpc/include/asm/membarrier.h MEMORY MANAGEMENT L: linux...@kvack.org diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index cb782ac1c35d..86077c4cd0a4 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -139,6 +139,7 @@ config PPC select ARCH_HAS_ELF_RANDOMIZE select ARCH_HAS_FORTIFY_SOURCE select ARCH_HAS_GCOV_PROFILE_ALL + select ARCH_HAS_MEMBARRIER_HOOKS select ARCH_HAS_SCALED_CPUTIME if VIRT_CPU_ACCOUNTING_NATIVE select ARCH_HAS_SG_CHAIN select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST diff --git a/arch/powerpc/include/asm/me
Re: [PATCH v2 12/12] hwrng: bcm63xx-rng: Remove since bcm2835-rng takes over
Florian Fainelli writes: > bcm2835-rng is now capable of supporting the BCM63xx hardware, so remove > the driver which duplicates the same functionality. > > Signed-off-by: Florian Fainelli Assuming Stefan's testing says that the .name handling you settled on works, patches 9, 11, 12 are: Reviewed-by: Eric Anholt signature.asc Description: PGP signature
[RFC PATCH for 4.15 3/6] membarrier: Document scheduler barrier requirements (v5)
Document the membarrier requirement on having a full memory barrier in __schedule() after coming from user-space, before storing to rq->curr. It is provided by smp_mb__after_spinlock() in __schedule(). Document that membarrier requires a full barrier on transition from kernel thread to userspace thread. We currently have an implicit barrier from atomic_dec_and_test() in mmdrop() that ensures this. The x86 switch_mm_irqs_off() full barrier is currently provided by many cpumask update operations as well as write_cr3(). Document that write_cr3() provides this barrier. Changes since v1: - Update comments to match reality for code paths which are after storing to rq->curr, before returning to user-space, based on feedback from Andrea Parri. Changes since v2: - Update changelog (smp_mb__before_spinlock -> smp_mb__after_spinlock). Based on feedback from Andrea Parri. Changes since v3: - Clarify comments following feeback from Peter Zijlstra. Changes since v4: - Update comment regarding powerpc barrier. Signed-off-by: Mathieu Desnoyers CC: Peter Zijlstra CC: Paul E. McKenney CC: Boqun Feng CC: Andrew Hunter CC: Maged Michael CC: gro...@google.com CC: Avi Kivity CC: Benjamin Herrenschmidt CC: Paul Mackerras CC: Michael Ellerman CC: Dave Watson CC: Thomas Gleixner CC: Ingo Molnar CC: "H. Peter Anvin" CC: Andrea Parri CC: x...@kernel.org --- arch/x86/mm/tlb.c| 5 + include/linux/sched/mm.h | 5 + kernel/sched/core.c | 37 ++--- 3 files changed, 36 insertions(+), 11 deletions(-) diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c index 3118392cdf75..5abf9bfcca1f 100644 --- a/arch/x86/mm/tlb.c +++ b/arch/x86/mm/tlb.c @@ -146,6 +146,11 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next, #endif this_cpu_write(cpu_tlbstate.is_lazy, false); + /* +* The membarrier system call requires a full memory barrier +* before returning to user-space, after storing to rq->curr. +* Writing to CR3 provides that full memory barrier. +*/ if (real_prev == next) { VM_WARN_ON(this_cpu_read(cpu_tlbstate.ctxs[prev_asid].ctx_id) != next->context.ctx_id); diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h index bbc3861d14c8..ff1d510a9c51 100644 --- a/include/linux/sched/mm.h +++ b/include/linux/sched/mm.h @@ -39,6 +39,11 @@ static inline void mmgrab(struct mm_struct *mm) extern void __mmdrop(struct mm_struct *); static inline void mmdrop(struct mm_struct *mm) { + /* +* The implicit full barrier implied by atomic_dec_and_test is +* required by the membarrier system call before returning to +* user-space, after storing to rq->curr. +*/ if (unlikely(atomic_dec_and_test(&mm->mm_count))) __mmdrop(mm); } diff --git a/kernel/sched/core.c b/kernel/sched/core.c index f29525e453c4..8a176892b4f0 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -2648,6 +2648,12 @@ static struct rq *finish_task_switch(struct task_struct *prev) finish_arch_post_lock_switch(); fire_sched_in_preempt_notifiers(current); + /* +* When transitioning from a kernel thread to a userspace +* thread, mmdrop()'s implicit full barrier is required by the +* membarrier system call, because the current active_mm can +* become the current mm without going through switch_mm(). +*/ if (mm) mmdrop(mm); if (unlikely(prev_state == TASK_DEAD)) { @@ -2753,6 +2759,13 @@ context_switch(struct rq *rq, struct task_struct *prev, */ arch_start_context_switch(prev); + /* +* If mm is non-NULL, we pass through switch_mm(). If mm is +* NULL, we will pass through mmdrop() in finish_task_switch(). +* Both of these contain the full memory barrier required by +* membarrier after storing to rq->curr, before returning to +* user-space. +*/ if (!mm) { next->active_mm = oldmm; mmgrab(oldmm); @@ -3289,6 +3302,9 @@ static void __sched notrace __schedule(bool preempt) * Make sure that signal_pending_state()->signal_pending() below * can't be reordered with __set_current_state(TASK_INTERRUPTIBLE) * done by the caller to avoid the race with signal_wake_up(). +* +* The membarrier system call requires a full memory barrier +* after coming from user-space, before storing to rq->curr. */ rq_lock(rq, &rf); smp_mb__after_spinlock(); @@ -3336,17 +3352,16 @@ static void __sched notrace __schedule(bool preempt) /* * The membarrier system call requires each architecture * to have a full memory barrier after updating -* rq->curr, before returning to user-space. For TSO -* (e.g. x
[RFC PATCH for 4.15 1/6] membarrier: selftest: Test private expedited cmd
Test the new MEMBARRIER_CMD_PRIVATE_EXPEDITED and MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED commands. Add checks expecting specific error values on system calls expected to fail. Signed-off-by: Mathieu Desnoyers Acked-by: Shuah Khan Acked-by: Greg Kroah-Hartman CC: Peter Zijlstra CC: Paul E. McKenney CC: Boqun Feng CC: Andrew Hunter CC: Maged Michael CC: gro...@google.com CC: Avi Kivity CC: Benjamin Herrenschmidt CC: Paul Mackerras CC: Michael Ellerman CC: Dave Watson CC: Alan Stern CC: Will Deacon CC: Andy Lutomirski CC: Alice Ferrazzi CC: Paul Elder CC: linux-kselft...@vger.kernel.org CC: linux-a...@vger.kernel.org --- .../testing/selftests/membarrier/membarrier_test.c | 109 ++--- 1 file changed, 94 insertions(+), 15 deletions(-) diff --git a/tools/testing/selftests/membarrier/membarrier_test.c b/tools/testing/selftests/membarrier/membarrier_test.c index 9e674d9514d1..d7543a6d9030 100644 --- a/tools/testing/selftests/membarrier/membarrier_test.c +++ b/tools/testing/selftests/membarrier/membarrier_test.c @@ -16,49 +16,119 @@ static int sys_membarrier(int cmd, int flags) static int test_membarrier_cmd_fail(void) { int cmd = -1, flags = 0; + const char *test_name = "sys membarrier invalid command"; if (sys_membarrier(cmd, flags) != -1) { ksft_exit_fail_msg( - "sys membarrier invalid command test: command = %d, flags = %d. Should fail, but passed\n", - cmd, flags); + "%s test: command = %d, flags = %d. Should fail, but passed\n", + test_name, cmd, flags); + } + if (errno != EINVAL) { + ksft_exit_fail_msg( + "%s test: flags = %d. Should return (%d: \"%s\"), but returned (%d: \"%s\").\n", + test_name, flags, EINVAL, strerror(EINVAL), + errno, strerror(errno)); } ksft_test_result_pass( - "sys membarrier invalid command test: command = %d, flags = %d. Failed as expected\n", - cmd, flags); + "%s test: command = %d, flags = %d, errno = %d. Failed as expected\n", + test_name, cmd, flags, errno); return 0; } static int test_membarrier_flags_fail(void) { int cmd = MEMBARRIER_CMD_QUERY, flags = 1; + const char *test_name = "sys membarrier MEMBARRIER_CMD_QUERY invalid flags"; if (sys_membarrier(cmd, flags) != -1) { ksft_exit_fail_msg( - "sys membarrier MEMBARRIER_CMD_QUERY invalid flags test: flags = %d. Should fail, but passed\n", - flags); + "%s test: flags = %d. Should fail, but passed\n", + test_name, flags); + } + if (errno != EINVAL) { + ksft_exit_fail_msg( + "%s test: flags = %d. Should return (%d: \"%s\"), but returned (%d: \"%s\").\n", + test_name, flags, EINVAL, strerror(EINVAL), + errno, strerror(errno)); } ksft_test_result_pass( - "sys membarrier MEMBARRIER_CMD_QUERY invalid flags test: flags = %d. Failed as expected\n", - flags); + "%s test: flags = %d, errno = %d. Failed as expected\n", + test_name, flags, errno); return 0; } -static int test_membarrier_success(void) +static int test_membarrier_shared_success(void) { int cmd = MEMBARRIER_CMD_SHARED, flags = 0; - const char *test_name = "sys membarrier MEMBARRIER_CMD_SHARED\n"; + const char *test_name = "sys membarrier MEMBARRIER_CMD_SHARED"; + + if (sys_membarrier(cmd, flags) != 0) { + ksft_exit_fail_msg( + "%s test: flags = %d, errno = %d\n", + test_name, flags, errno); + } + + ksft_test_result_pass( + "%s test: flags = %d\n", test_name, flags); + return 0; +} + +static int test_membarrier_private_expedited_fail(void) +{ + int cmd = MEMBARRIER_CMD_PRIVATE_EXPEDITED, flags = 0; + const char *test_name = "sys membarrier MEMBARRIER_CMD_PRIVATE_EXPEDITED not registered failure"; + + if (sys_membarrier(cmd, flags) != -1) { + ksft_exit_fail_msg( + "%s test: flags = %d. Should fail, but passed\n", + test_name, flags); + } + if (errno != EPERM) { + ksft_exit_fail_msg( + "%s test: flags = %d. Should return (%d: \"%s\"), but returned (%d: \"%s\").\n", + test_name, flags, EPERM, strerror(EPERM), + errno, strerror(errno)); + } + + ksft_test_result_pass( + "%s test: flags = %d, errno = %d\n", + test_name, flags, errno); + return 0; +} + +static int test_membarrier_register_private_expedited
[PATCH 0/3] net/sched/cls_fw: Fine-tuning for seven function implementations
From: Markus Elfring Date: Wed, 8 Nov 2017 19:36:54 +0100 Three update suggestions were taken into account from static source code analysis. Markus Elfring (3): Use common error handling code in fw_change() Improve two size determinations in fw_change() Adjust nine checks for null pointers net/sched/cls_fw.c | 27 +-- 1 file changed, 13 insertions(+), 14 deletions(-) -- 2.15.0
[PATCH v2] sched: fix sched_feat for !SCHED_DEBUG builds
When the kernel is compiled with !SCHED_DEBUG support, we expect that all SCHED_FEAT are turned into compile time constants being propagated to support compiler optimizations. Specifically, we expect that code blocks like this: if (sched_feat(FEATURE_NAME) [&& ]) { /* FEATURE CODE */ } are turned into dead-code in case FEATURE_NAME defaults to FALSE, and thus being removed by the compiler from the finale image. For this mechanism to properly work it's required for the compiler to have full access, from each translation unit, to whatever is the value defined by the sched_feat macro. This macro is defined as: #define sched_feat(x) (sysctl_sched_features & (1UL << __SCHED_FEAT_##x)) and thus, the compiler can optimize that code only if the value of sysctl_sched_features is visible within each translation unit. Since: 029632fbb sched: Make separate sched*.c translation units the scheduler code has been split into separate translation units however the definition of sysctl_sched_features is part of kernel/sched/core.c while, for all the other scheduler modules, it is visible only via kernel/sched/sched.h as an: extern const_debug unsigned int sysctl_sched_features Unfortunately, an extern reference does not allow the compiler to apply constants propagation. Thus, on !SCHED_DEBUG kernel we still end up with code to load a memory reference and (eventually) doing an unconditional jump of a chunk of code. This mechanism is unavoidable when sched_features can be turned on and off at run-time. However, this is not the case for "production" kernels compiled with !SCHED_DEBUG. In this case, sysctl_sched_features is just a constant value which cannot be changed at run-time and thus memory loads and jumps can be avoided altogether. This patch fixes the case of !SCHED_DEBUG kernel by declaring a local version of the sysctl_sched_features constant for each translation unit. This will ultimately allow the compiler to perform constants propagation and dead-code pruning. Tests have been done, with !SCHED_DEBUG on a v4.14-rc8 with and without the patch, by running 30 iterations of: perf bench sched messaging --pipe --thread --group 4 --loop 5 on a 40 cores Intel(R) Xeon(R) CPU E5-2690 v2 @ 3.00GHz using the powersave governor to rule out variations due to frequency scaling. Statistics on the reported completion time: count mean std min 99% max v4.14-rc8 30.0 15.7831 0.176032 15.442 16.01226 16.014 v4.14-rc8+patch 30.0 15.5033 0.189681 15.232 15.93938 15.962 show a 1.8% speedup on average completion time and 0.5% speedup in the 99 percentile. Signed-off-by: Patrick Bellasi Signed-off-by: Chris Redpath Reviewed-by: Dietmar Eggemann Reviewed-by: Brendan Jackman Cc: Ingo Molnar Cc: Peter Zijlstra Cc: linux-kernel@vger.kernel.org --- kernel/sched/core.c | 9 ++--- kernel/sched/sched.h | 25 ++--- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/kernel/sched/core.c b/kernel/sched/core.c index d17c5da523a0..0edb08716555 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -42,18 +42,21 @@ DEFINE_PER_CPU_SHARED_ALIGNED(struct rq, runqueues); +#if defined(CONFIG_SCHED_DEBUG) && defined(HAVE_JUMP_LABEL) /* * Debugging: various feature bits + * + * If SCHED_DEBUG is disabled, each compilation unit has its own copy of + * sysctl_sched_features, defined in sched.h, to allow constants propagation + * at compile time and compiler optimization based on features default. */ - #define SCHED_FEAT(name, enabled) \ (1UL << __SCHED_FEAT_##name) * enabled | - const_debug unsigned int sysctl_sched_features = #include "features.h" 0; - #undef SCHED_FEAT +#endif /* * Number of tasks to iterate in a single balance run. diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h index 3b448ba82225..2b8647a3f61f 100644 --- a/kernel/sched/sched.h +++ b/kernel/sched/sched.h @@ -1219,8 +1219,6 @@ static inline void __set_task_cpu(struct task_struct *p, unsigned int cpu) # define const_debug const #endif -extern const_debug unsigned int sysctl_sched_features; - #define SCHED_FEAT(name, enabled) \ __SCHED_FEAT_##name , @@ -1232,6 +1230,13 @@ enum { #undef SCHED_FEAT #if defined(CONFIG_SCHED_DEBUG) && defined(HAVE_JUMP_LABEL) + +/* + * To support run-time toggling of sched features, all the translation units + * (but core.c) reference the sysctl_sched_features defined in core.c. + */ +extern const_debug unsigned int sysctl_sched_features; + #define SCHED_FEAT(name, enabled) \ static __always_inline bool static_branch_##name(struct static_key *key) \ { \ @@ -1239,13 +1244,27 @@ static __always_inline bool static_branch_##name(struct static_key *key) \ } #include "features.h" - #undef SCHED_FEAT extern struct static_key sc
Re: [PATCH 1/1] locking/qspinlock/x86: Avoid test-and-set when PV_DEDICATED is set
2017-10-31 10:02-0700, Eduardo Valentin: > Hello Radim, > > On Tue, Oct 24, 2017 at 01:18:59PM +0200, Radim Krčmář wrote: > > 2017-10-23 17:44-0700, Eduardo Valentin: > > > Currently, the existing qspinlock implementation will fallback to > > > test-and-set if the hypervisor has not set the PV_UNHALT flag. > > > > Where have you detected the main source of overhead with pinned VCPUs? > > Makes me wonder if we couldn't improve general PV_UNHALT, > > This is essentially for cases of non-overcommitted vCPUs in which we want > the instance vCPUs to run uninterrupted as much as possible. Here by disabling > the PV_UNHALT, we avoid the accounting needed to properly do the PV_UNHALT > hypercall, as the lock holder won't be preempted anyway for the 1:1 pin case. Right, I would expect that the scenario should very rarely go into the halt/kick path -- is SPIN_THRESHOLD too low? We could also try abolishing the SPIN_THRESHOLD completely and only use vcpu_is_preempted() and state of the previous lock holder to enter the halt/kick path. (The drawback is that vcpu_is_preempted() currently gets set even when dropping into userspace.) > > > This patch gives the opportunity to guest kernels to select > > > between test-and-set and the regular queueu fair lock implementation > > > based on the PV_DEDICATED KVM feature flag. When the PV_DEDICATED > > > flag is not set, the code will still fall back to test-and-set, > > > but when the PV_DEDICATED flag is set, the code will use > > > the regular queue spinlock implementation. > > > > Some flag makes sense and we do want to make sure that userspaces don't > > enable it in pass-through-cpuid mode. > > Did you mean something like: > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c > index 0099e10..8ceb503 100644 > --- a/arch/x86/kvm/cpuid.c > +++ b/arch/x86/kvm/cpuid.c > @@ -211,7 +211,8 @@ int kvm_vcpu_ioctl_set_cpuid(struct kvm_vcpu *vcpu, > } > for (i = 0; i < cpuid->nent; i++) { > vcpu->arch.cpuid_entries[i].function = > cpuid_entries[i].function; > - vcpu->arch.cpuid_entries[i].eax = cpuid_entries[i].eax; > + vcpu->arch.cpuid_entries[i].eax = cpuid_entries[i].eax & > + > ~KVM_FEATURE_PV_DEDICATED; > vcpu->arch.cpuid_entries[i].ebx = cpuid_entries[i].ebx; > vcpu->arch.cpuid_entries[i].ecx = cpuid_entries[i].ecx; > vcpu->arch.cpuid_entries[i].edx = cpuid_entries[i].edx; > > > But I do not see any other KVM_FEATURE_* being enforced (e.g. PV_UNHALT). > Do you mind elaborating a bit here? Sorry, nothing is needed. I somehow though that we need to expose this to the userspace through CPUID, but KVM just needs to consider the flag as reserved.
[PATCH 1/3] net: sched: cls_fw: Use common error handling code in fw_change()
From: Markus Elfring Date: Wed, 8 Nov 2017 19:15:08 +0100 Add a jump target so that a bit of exception handling can be better reused in an if branch of this function. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring --- net/sched/cls_fw.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/net/sched/cls_fw.c b/net/sched/cls_fw.c index 5908f56f76da..567db4d2349f 100644 --- a/net/sched/cls_fw.c +++ b/net/sched/cls_fw.c @@ -281,14 +281,13 @@ static int fw_change(struct net *net, struct sk_buff *in_skb, fnew->tp = f->tp; err = tcf_exts_init(&fnew->exts, TCA_FW_ACT, TCA_FW_POLICE); - if (err < 0) { - kfree(fnew); - return err; - } + if (err < 0) + goto free_filter; err = fw_set_parms(net, tp, fnew, tb, tca, base, ovr); if (err < 0) { tcf_exts_destroy(&fnew->exts); +free_filter: kfree(fnew); return err; } -- 2.15.0
[PATCH 2/3] net: sched: cls_fw: Improve two size determinations in fw_change()
From: Markus Elfring Date: Wed, 8 Nov 2017 19:19:10 +0100 Replace the specification of data structures by pointer dereferences as the parameter for the operator "sizeof" to make the corresponding size determination a bit safer according to the Linux coding style convention. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring --- net/sched/cls_fw.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/net/sched/cls_fw.c b/net/sched/cls_fw.c index 567db4d2349f..53c1c8ae3e00 100644 --- a/net/sched/cls_fw.c +++ b/net/sched/cls_fw.c @@ -269,7 +269,7 @@ static int fw_change(struct net *net, struct sk_buff *in_skb, if (f->id != handle && handle) return -EINVAL; - fnew = kzalloc(sizeof(struct fw_filter), GFP_KERNEL); + fnew = kzalloc(sizeof(*fnew), GFP_KERNEL); if (!fnew) return -ENOBUFS; @@ -323,7 +323,7 @@ static int fw_change(struct net *net, struct sk_buff *in_skb, rcu_assign_pointer(tp->root, head); } - f = kzalloc(sizeof(struct fw_filter), GFP_KERNEL); + f = kzalloc(sizeof(*f), GFP_KERNEL); if (f == NULL) return -ENOBUFS; -- 2.15.0
[PATCH 3/3] net: sched: cls_fw: Adjust nine checks for null pointers
From: Markus Elfring Date: Wed, 8 Nov 2017 19:26:29 +0100 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The script “checkpatch.pl” pointed information out like the following. Comparison to NULL could be written … Thus fix the affected source code places. Signed-off-by: Markus Elfring --- net/sched/cls_fw.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/net/sched/cls_fw.c b/net/sched/cls_fw.c index 53c1c8ae3e00..2af2627d6f45 100644 --- a/net/sched/cls_fw.c +++ b/net/sched/cls_fw.c @@ -68,7 +68,7 @@ static int fw_classify(struct sk_buff *skb, const struct tcf_proto *tp, int r; u32 id = skb->mark; - if (head != NULL) { + if (head) { id &= head->mask; for (f = rcu_dereference_bh(head->ht[fw_hash(id)]); f; @@ -106,7 +106,7 @@ static void *fw_get(struct tcf_proto *tp, u32 handle) struct fw_head *head = rtnl_dereference(tp->root); struct fw_filter *f; - if (head == NULL) + if (!head) return NULL; f = rtnl_dereference(head->ht[fw_hash(handle)]); @@ -149,7 +149,7 @@ static void fw_destroy(struct tcf_proto *tp) struct fw_filter *f; int h; - if (head == NULL) + if (!head) return; for (h = 0; h < HTSIZE; h++) { @@ -172,7 +172,7 @@ static int fw_delete(struct tcf_proto *tp, void *arg, bool *last) int ret = -EINVAL; int h; - if (head == NULL || f == NULL) + if (!head || !f) goto out; fp = &head->ht[fw_hash(f->id)]; @@ -324,7 +324,7 @@ static int fw_change(struct net *net, struct sk_buff *in_skb, } f = kzalloc(sizeof(*f), GFP_KERNEL); - if (f == NULL) + if (!f) return -ENOBUFS; err = tcf_exts_init(&f->exts, TCA_FW_ACT, TCA_FW_POLICE); @@ -354,7 +354,7 @@ static void fw_walk(struct tcf_proto *tp, struct tcf_walker *arg) struct fw_head *head = rtnl_dereference(tp->root); int h; - if (head == NULL) + if (!head) arg->stop = 1; if (arg->stop) @@ -385,7 +385,7 @@ static int fw_dump(struct net *net, struct tcf_proto *tp, void *fh, struct fw_filter *f = fh; struct nlattr *nest; - if (f == NULL) + if (!f) return skb->len; t->tcm_handle = f->id; @@ -394,7 +394,7 @@ static int fw_dump(struct net *net, struct tcf_proto *tp, void *fh, return skb->len; nest = nla_nest_start(skb, TCA_OPTIONS); - if (nest == NULL) + if (!nest) goto nla_put_failure; if (f->res.classid && -- 2.15.0
Re: [RFC PATCH v2] ftrace: support very early function tracing
I implemented the ring_buffer_set_clock solution and I have some questions. void __init ftrace_early_fill_ringbuffer(void *data) { ... ring_buffer_set_clock(tr->trace_buffer.buffer, early_trace_clock); preempt_disable_notrace(); for (i = 0; i < vearly_entries_count; i++) { entry = &ftrace_vearly_entries[i]; early_timestamp = entry->clock; trace_function(tr, entry->ip, entry->parent_ip, 0, 0); } preempt_enable_notrace(); /* TODO: should set the previous clock */ ring_buffer_set_clock(tr->trace_buffer.buffer, trace_clock_local); I need also to save the current clock before calling "ring_buffer_set_clock()", is there a way to get the current clock or to set the clock from tr->clock_id ? There is one thing happening when using the trace_clock_local instead of rdtsc. I added different kernel params to trigger the issue : When using entry->clock = trace_clock_local(): kernel param "ftrace_vearly ftrace_filter=*_init" [0.312179] -0 0dp..0us : boot_cpu_init <-start_kernel ... [0.320280] -0 0dp..0us : numa_init <-x86_numa_init [0.320673] -0 0dp..0us : dummy_numa_init <-numa_init [0.321082] -0 0dp..6us : paging_init <-setup_arch [0.321471] -0 0dp..9us : sparse_init <-paging_init [0.321867] -0 0dp.. 1174us : zone_sizes_init <-paging_init [0.322283] -0 0dp.. 1201us : lruvec_init <-free_area_init_node [0.322714] -0 0dp.. 1524us : acpi_boot_init <-setup_arch [0.323120] -0 0dp.. 1595us : sfi_init <-setup_arch [0.323500] -0 0dp.. 1601us : kvm_guest_init <-setup_arch [0.323904] -0 0dp.. 1623us : mcheck_init <-setup_arch [0.324293] -0 0dp.. 1623us : mcheck_intel_therm_init <-mcheck_init [0.324772] -0 0dp.. 1724us : boot_cpu_state_init <-start_kernel [0.325259] -0 0dp.. 1724us : kvm_guest_cpu_init <-kvm_smp_prepare_boot_cpu [0.325789] -0 0dp.. 1733us : kvm_spinlock_init <-kvm_smp_prepare_boot_cpu [0.326300] -0 0dp.. 1733us : build_all_zonelists_init <-build_all_zonelists [0.326820] -0 0dp.. 1738us : page_alloc_init <-start_kernel [0.327252] -0 0dp.. 1906us : jump_label_init <-start_kernel [0.327673] -0 0dp.. 4913us : pidhash_init <-start_kernel [0.328076] -0 0dp.. 4917us : trap_init <-start_kernel [0.328466] -0 0dp.. 4927us : kvm_apf_trap_init <-trap_init [0.328877] -0 0dp.. 4928us : mem_init <-start_kernel [0.329263] -0 0dp.. 4930us : gart_iommu_hole_init <-pci_iommu_alloc [0.329745] -0 0dp.. 5129us : kmem_cache_init <-start_kernel [0.330162] -0 0dp.. 5300us : vmalloc_init <-start_kernel kernel param "ftrace=function ftrace_vearly ftrace_filter=*_init" [0.315455] -0 0dp.. 18446744073666366us : boot_cpu_init <-start_kernel ... [0.338102] -0 0dp.. 18446744073671436us : trap_init <-start_kernel [0.338584] -0 0dp.. 18446744073671446us : kvm_apf_trap_init <-trap_init [0.339089] -0 0dp.. 18446744073671447us : mem_init <-start_kernel [0.339564] -0 0dp.. 18446744073671449us : gart_iommu_hole_init <-pci_iommu_alloc [0.340111] -0 0dp.. 18446744073671635us : kmem_cache_init <-start_kernel [0.340622] -0 0dp.. 18446744073671775us : vmalloc_init <-start_kernel function tracing starts here << [0.341119] -0 0dp.1 48004us : sched_init <-start_kernel [0.341520] -0 0dp.1 48005us : wait_bit_init <-sched_init [0.341926] -0 0dp.1 48007us : hrtimer_init <-init_rt_bandwidth [0.342359] -0 0dp.1 48007us : __hrtimer_init <-hrtimer_init [0.342779] -0 0dp.1 48007us : cpudl_init <-init_rootdomain [0.343196] -0 0dp.1 48008us : cpupri_init <-init_rootdomain [0.343618] -0 0dp.1 48014us : autogroup_init <-sched_init When using entry->clock = rdtsc() [working well]: kernel param "ftrace=function ftrace_vearly ftrace_filter=*_init" [0.314067] -0 0dp.. 154207us : boot_cpu_init <-start_kernel ... [0.97] -0 0dp.. 180362us : mem_init <-start_kernel [0.333806] -0 0dp.. 180365us : gart_iommu_hole_init <-pci_iommu_alloc [0.334330] -0 0dp.. 180652us : kmem_cache_init <-start_kernel [0.334760] -0 0dp.. 180810us : vmalloc_init <-start_kernel function tracing starts here << [0.335225] -0 0dp.1 180810us : sched_init <-start_kernel [0.335626] -0 0dp.1 180810us : wait_bit_init <-sched_init [0.336015] -0 0dp.1 180810us : hrtimer_init <-init_rt_bandwidth [0.336500] -0 0dp.1 180810us : __hrtimer_init <-hrtimer_init [0.336981] -0 0dp.1 180810us :
Re: WTF? Re: [PATCH] License cleanup: add SPDX GPL-2.0 license identifier to files with no license
On Tue, Nov 07, 2017 at 09:26:48PM +0100, Greg Kroah-Hartman wrote: > On Tue, Nov 07, 2017 at 11:28:46AM -0800, Christoph Hellwig wrote: > > On Tue, Nov 07, 2017 at 02:15:26PM -0500, Theodore Ts'o wrote: > > > On Tue, Nov 07, 2017 at 06:46:58PM +, Alan Cox wrote: > > > > > Given that it had no license text on it at all, it "defaults" to > > > > > GPLv2, > > > > > so the GPLv2 SPDX identifier was added to it. > > > > > > > > > > No copyright was changed, nothing at all happened except we explicitly > > > > > list the license of the file, instead of it being "implicit" before. > > > > > > > > Well if Christoph owns the copyright (if there is one) and he has stated > > > > he believes it is too trivial to copyright then it needs an SPDX tag > > > > that > > > > indicates the rightsholder has stated it's too trivial to copyright and > > > > (by estoppel) revoked any right they might have to pursue a claim. > > > > > > If Cristoph has revoked any right to pursue a claim, then he's also > > > legally given up the right to complain if, say, Bradley Kuhn starting > > > distributing a version with a GPLv3 permission statement --- or if Greg > > > K-H adds a GPLv2 SPDX identifier. :-) > > > > > > First Christoph really appreciateѕ spelling his name right. > > > > Second Christoph really appreciates talking to him when trying to slap > > on licensing bits on his code. I'm not evil, but I'd really like to > > understand what you are doing and why, and I might be fairly agreeable > > if that makes sense. > > I already described it in the pull request, and in this patch itself, > and in this thread already, what is happening, why it was done, and how > it was done. I don't know how everyone who was on the original email > thread got dropped and the xfs mailing list added, that's just odd... This thread is a result of regular efforts to keep the xfsprogs libxfs code roughly in-sync with the kernel libxfs code. That's how the xfs mailing is on cc on this thread. That said, it's pretty obvious from Christoph's reaction that nobody emailed Christoph (the original author of xfs_cksum.h) about the original change to the kernel source. Nobody emailed the XFS maintainers for an ack to the kernel change, and nobody cc'd the linux-xfs mailing list as a heads-up. Neither XFS maintainer were invited to the summit where this change was discussed. I always thought the rules in SubmittingPatches[1] existed to facilitate cooperation with Linux subprojects and I strongly protest your decision to ignore us. Had anyone involved us, I would have suggested fixing the whole XFS tree to use the shortened tag instead of each file containing its own mutations of the GPL, and our broader XFS community could have worked with you on this. --D [1] submitting-patches.rst, section 5: 5) Select the recipients for your patch --- You should always copy the appropriate subsystem maintainer(s) on any patch to code that they maintain; look through the MAINTAINERS file and the source code revision history to see who those maintainers are. The script scripts/get_maintainer.pl can be very useful at this step. If you cannot find a maintainer for the subsystem you are working on, Andrew Morton (a...@linux-foundation.org) serves as a maintainer of last resort. > > Doing batch annotations of code where you do not the know any of > > the history of is a receipt for a desaster if we want to use that > > information anywhere. > > But we did research the history as well as we could when touching 11k > files at a single time. It's been months of research and work, as > described in the patch. > > If we got something wrong, very sorry, not a problem, please, let's put > the proper license on the file and all will be good. What file are you > concerned about, and what license belongs on it? This patch only > touched files without any license header, so again, by default that > implied it was GPLv2. Again, no copyright was changed at all. > > thanks, > > greg k-h > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 06/12] hwrng: bcm2835-rng: Rework interrupt masking
Hi Florian, > Florian Fainelli hat am 8. November 2017 um 01:44 > geschrieben: > > > The interrupt masking done for Northstart Plus and Northstar (BCM5301X) > is moved from being a function pointer mapped to of_device_id::data into > a proper part of the hwrng::init callback. While at it, we also make the > of_data be a proper structure indicating the platform specifics, since > the day we need to add a second type of platform information, we would > have to do that anyway. in the lack of proper documentation for bcm2835 rng, is it possible that we should mask the interrupts for bcm2835 as well?
Re: [PATCH 0/2] ARM: dts: NSP fixes
On 11/07/2017 02:28 PM, Florian Fainelli wrote: > Hi all, > > This patch series contains a couple of fixes: > > - disables AHCI on NSP HR boards since that results in a hard hang > - fixes PPI interrupts specifiers Applied to devicetree/fixes. -- Florian
Re: [PATCH V3 2/2] It makes the code clearer and less error prone.
On Tue, 15 Aug 2017 09:25:10 +0200 Federico Vaga wrote: > On Monday, August 14, 2017 7:33:41 PM CEST Steven Rostedt wrote: > > On Thu, 3 Aug 2017 00:15:58 +0200 > > Federico Vaga wrote: > > > > Why did you change the subject? The previous patch had a much better > > one: "trace-cmd: Use asprintf when possible" > > > > Or was it the tool you used to send the patches that chopped it off? > > It was not my intention. > I used git send-email, but probably I did something wrong by accident. Sorry > > > > > A quick scan of the patch looks good. I'll look a bit deeper at it, and > > if I don't find anything, I'll apply it (and fix the subject). > > Ok, thank you > Finally got around to applying your patches. Sorry for the long delay. :-/ I've been traveling too much lately. -- Steve
[PATCH] fs/xfs: Remove NULL check before kmem_cache_destroy
kmem_cache_destroy already checks for null values. Signed-off-by: Tim Hansen --- fs/xfs/kmem.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/fs/xfs/kmem.h b/fs/xfs/kmem.h index 7509019..4b87472 100644 --- a/fs/xfs/kmem.h +++ b/fs/xfs/kmem.h @@ -119,8 +119,7 @@ kmem_zone_free(kmem_zone_t *zone, void *ptr) static inline void kmem_zone_destroy(kmem_zone_t *zone) { - if (zone) - kmem_cache_destroy(zone); + kmem_cache_destroy(zone); } extern void *kmem_zone_alloc(kmem_zone_t *, xfs_km_flags_t); -- 2.1.4
Re: [PATCH] apparmor: fix off-by-one comparison on MAXMAPPED_SIG
On Wed, Nov 8, 2017 at 8:09 AM, John Johansen wrote: > > Signed-off-by: Colin Ian King > Signed-off-by: John Johansen This sign-off chain is odd. It implies that the patch came from Colin King, bnut it lacks authorship attribution. So who authored this? If it was Colin, there should have been a From: Colin Ian King at the top. So either the authorship is broken, or the sign-off chain is. Which is it? Linus
Re: [PATCH] fs/xfs: Remove NULL check before kmem_cache_destroy
On Wed, Nov 08, 2017 at 01:53:10PM -0500, Tim Hansen wrote: > kmem_cache_destroy already checks for null values. > > Signed-off-by: Tim Hansen > --- > fs/xfs/kmem.h | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/fs/xfs/kmem.h b/fs/xfs/kmem.h > index 7509019..4b87472 100644 > --- a/fs/xfs/kmem.h > +++ b/fs/xfs/kmem.h > @@ -119,8 +119,7 @@ kmem_zone_free(kmem_zone_t *zone, void *ptr) > static inline void > kmem_zone_destroy(kmem_zone_t *zone) > { > - if (zone) > - kmem_cache_destroy(zone); > + kmem_cache_destroy(zone); > } > > extern void *kmem_zone_alloc(kmem_zone_t *, xfs_km_flags_t); > -- > 2.1.4 > This patch was created on linux-next repo, tag next-20171108.
Re: [PATCH] apparmor: fix off-by-one comparison on MAXMAPPED_SIG
On Wed, Nov 8, 2017 at 10:53 AM, Linus Torvalds wrote: > So either the authorship is broken, or the sign-off chain is. Which is it? Ahh, looking around for emails, I see that Colin sent the first version with just one comparison, and you added the second one. So authorship is mixed, I guess. git doesn't support multiple authors, so I guess I'll just apply the patch as is, and Colin misses out. Sorry, Colin. Linus
Re: [PATCH v2] s390/virtio: mark headers as BSD licensed
So what about the following - leave kvm_virtio.h unchanged and we will delete this file via the s390 tree - change virtio_ccw.h to BSD license. The content of this file is really really trivial and it boils down to 2 defines, that can be easily reconstructed by looking at the virtio spec. Not even sure if something like this can be copyrighted. For reference the content of this file minus comments is - snip - #define KVM_VIRTIO_CCW_RING_ALIGN 4096 #define KVM_S390_VIRTIO_CCW_NOTIFY 3 - snip - On 11/08/2017 07:33 PM, Christian Borntraeger wrote: > > > On 11/08/2017 07:15 PM, Cornelia Huck wrote: >> On Wed, 8 Nov 2017 19:12:25 +0100his >> Christian Borntraeger wrote: >> >>> On 11/08/2017 05:59 PM, Michael S. Tsirkin wrote: Virtio UAPI headers aren't just for UAPI, it's for guests/hypervisors as well. The s390 ones need to be BSD as well. Fixes: e2be04c7f995 ("License cleanup: add SPDX license identifier to uapi header files with a license") Cc: Greg Kroah-Hartman Signed-off-by: Michael S. Tsirkin --- since v1: drop an extra comment chunk, reported by Cornelia arch/s390/include/uapi/asm/kvm_virtio.h | 26 +- >>> >>> FWIW, this file will go away anyway. >>> Looks like the following commit in the s390 feature branch >>> https://git.kernel.org/pub/scm/linux/kernel/git/s390/linux.git/commit/drivers/s390/virtio/Makefile?h=features&id=7fb2b2d512448cf0e914c4647a1cf02b52263702 >>> missed this file. >> >> Can we simply rip this out? >> >>> >>> >>> Regarding the virtio-ccw.h headers it is probably something that happened >>> by accident. >>> Back then Rusty (as an IBMer) proposed to change all virtio headers to BSD >>> licence. >> >> ISTR that the relicensing already went through... is that not the case? > > The original kvm_virtio.h code was added in 2008 > > commit e976a2b997fc4ad70ccc53acfe62811c4aaec851 > Author: Christian Borntraeger > Date: Tue Mar 25 18:47:46 2008 +0100 > > s390: KVM guest: virtio device support, and kvm hypercalls > > > > > The BSD re-licencing happened in 2008 > > commit 674bfc23c585b34c42263d73fb51710d49762a23 > Author: Rusty Russell > Date: Fri Jul 25 12:06:03 2008 -0500 > > virtio: clarify that ABI is usable by any implementations > > We want others to implement and use virtio, so it makes sense to BSD > license the non-__KERNEL__ parts of the headers to make this crystal > clear. > > and it seems that it missed arch/include/uapi/asm/kvm_virtio.h > > > > arch/include/uapi/asm/virtio_ccw.h probably just copied what > kvm_virtio.h had in 2013. > > So its somewhat fair to assume that both files should be BSD, but they are not > according to the licence statement. > > Gregs SPDX change just changed the GPL text in the header files to a SPDX > statement. Now Michael wants to change this to BSD as it probably was intended > but not written down. Sigh. > >> I thought this was just a fixup to align with reality? >> >>> arch/s390/include/uapi/asm/virtio-ccw.h | 26 +- 2 files changed, 50 insertions(+), 2 deletions(-) >>
[PATCH] media: coda: remove definition of CODA_STD_MJPG
According to i.MX VPU API Reference Manuals the MJPG video codec is refernced to by number 7, not 3. Also Philipp pointed out that this value is only meant to fill in CMD_ENC_SEQ_COD_STD for encoding, only on i.MX53. It was never written to any register, and even if defined correctly, wouldn't be needed for i.MX6. So avoid confusion and remove this definition. Signed-off-by: Martin Kepplinger --- drivers/media/platform/coda/coda_regs.h | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/media/platform/coda/coda_regs.h b/drivers/media/platform/coda/coda_regs.h index 38df5fd9a2fa..35e620c7f1f4 100644 --- a/drivers/media/platform/coda/coda_regs.h +++ b/drivers/media/platform/coda/coda_regs.h @@ -254,7 +254,6 @@ #defineCODA9_STD_H264 0 #defineCODA_STD_H263 1 #defineCODA_STD_H264 2 -#defineCODA_STD_MJPG 3 #defineCODA9_STD_MPEG4 3 #define CODA_CMD_ENC_SEQ_SRC_SIZE 0x190 -- 2.11.0
Re: [PATCH 0/3] zfcp: timer_setup() refactoring feature for v4.15-rc1
On Wed, Nov 8, 2017 at 6:17 AM, Steffen Maier wrote: > Hi all, > > here is a small series for the timer_setup() refactoring of zfcp. > We target it for the merge window to land in v4.15-rc1. > > Unfortunately, they don't seem to apply to the current state of neither > James' misc branch nor Martin's 4.15/scsi-queue branch, > because they depend on: > v4.14-rc3 686fef928bba ("timer: Prepare to change timer callback argument > type") > and > v4.14-rc7 ab31fd0ce65e ("scsi: zfcp: fix erp_action use-before-initialize in > REC action trace"). > > However, they do apply to Linus' tree for v4.14-rc7 or later and > thus they would also apply for the upcoming merge window. > > In http://www.spinics.net/lists/linux-scsi/msg114581.html I saw a decision > to have such changes go in via the timer tree. I would be happy with that. > > Kees Cook (1): > zfcp: convert timers to use timer_setup() > > Steffen Maier (2): > zfcp: purely mechanical update using timer API, plus blank lines > zfcp: drop open coded assignments of timer_list.function These looks great, thanks! -Kees > > drivers/s390/scsi/zfcp_erp.c | 15 +-- > drivers/s390/scsi/zfcp_ext.h | 2 +- > drivers/s390/scsi/zfcp_fsf.c | 13 ++--- > 3 files changed, 16 insertions(+), 14 deletions(-) > > -- > 2.13.5 > -- Kees Cook Pixel Security
Re: [PATCH v2] s390/virtio: mark headers as BSD licensed
On 11/08/2017 07:57 PM, Christian Borntraeger wrote: > So what about the following > > - leave kvm_virtio.h unchanged and we will delete this file via the s390 tree > > - change virtio_ccw.h to BSD license. The content of this file is really > really trivial GPLv2 and BSD dual licence might be better I guess. > and it boils down to 2 defines, that can be easily reconstructed by looking > at the virtio spec. > Not even sure if something like this can be copyrighted. > > > For reference the content of this file minus comments is > > - snip - > #define KVM_VIRTIO_CCW_RING_ALIGN 4096 > #define KVM_S390_VIRTIO_CCW_NOTIFY 3 > - snip -
[PATCH] arm64: cpu_errata: Add Kryo to Falkor 1003 errata
The Kryo CPUs are also affected by the Falkor 1003 errata, so we need to do the same workaround on Kryo CPUs. The MIDR is slightly more complicated here, where the PART number is not always the same when looking at all the bits from 15 to 4. Drop the lower 8 bits and just look at the top 4 to see if it's '2' and then consider those as Kryo CPUs. This covers all the combinations without having to list them all out. Signed-off-by: Stephen Boyd --- We may need to introduce another Kconfig option to block software PAN from being enabled when this errata is enabled (and then have software PAN depend on this new config being false). That's because some Kryo CPUs don't support HW PAN while others do. That's seems to be the best compromise, similar to what was mentioned in this thread[1]. Otherwise, I can take a look at updating the software PAN implementation to handle this errata, but that is probably a dead-end? [1] https://lkml.org/lkml/2017/2/1/591 Documentation/arm64/silicon-errata.txt | 2 +- arch/arm64/include/asm/cputype.h | 2 ++ arch/arm64/kernel/cpu_errata.c | 21 + 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt index 66e8ce14d23d..cd7d997063f6 100644 --- a/Documentation/arm64/silicon-errata.txt +++ b/Documentation/arm64/silicon-errata.txt @@ -71,6 +71,6 @@ stable kernels. | Hisilicon | Hip0{5,6,7} | #161010101 | HISILICON_ERRATUM_161010101 | | Hisilicon | Hip0{6,7} | #161010701 | N/A | || | | | -| Qualcomm Tech. | Falkor v1 | E1003 | QCOM_FALKOR_ERRATUM_1003| +| Qualcomm Tech. | Kryo/Falkor v1 | E1003 | QCOM_FALKOR_ERRATUM_1003| | Qualcomm Tech. | Falkor v1 | E1009 | QCOM_FALKOR_ERRATUM_1009| | Qualcomm Tech. | QDF2400 ITS | E0065 | QCOM_QDF2400_ERRATUM_0065 | diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h index 235e77d98261..b5afa6668646 100644 --- a/arch/arm64/include/asm/cputype.h +++ b/arch/arm64/include/asm/cputype.h @@ -91,6 +91,7 @@ #define BRCM_CPU_PART_VULCAN 0x516 #define QCOM_CPU_PART_FALKOR_V10x800 +#define QCOM_CPU_PART_KRYO 0x200 #define MIDR_CORTEX_A53 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A53) #define MIDR_CORTEX_A57 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A57) @@ -99,6 +100,7 @@ #define MIDR_THUNDERX_81XX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX_81XX) #define MIDR_THUNDERX_83XX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX_83XX) #define MIDR_QCOM_FALKOR_V1 MIDR_CPU_MODEL(ARM_CPU_IMP_QCOM, QCOM_CPU_PART_FALKOR_V1) +#define MIDR_QCOM_KRYO MIDR_CPU_MODEL(ARM_CPU_IMP_QCOM, QCOM_CPU_PART_KRYO) #ifndef __ASSEMBLY__ diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c index 0e27f86ee709..e4c78630a730 100644 --- a/arch/arm64/kernel/cpu_errata.c +++ b/arch/arm64/kernel/cpu_errata.c @@ -30,6 +30,20 @@ is_affected_midr_range(const struct arm64_cpu_capabilities *entry, int scope) entry->midr_range_max); } +static bool __maybe_unused +is_kryo_midr(const struct arm64_cpu_capabilities *entry, int scope) +{ + u32 model; + + WARN_ON(scope != SCOPE_LOCAL_CPU || preemptible()); + + model = read_cpuid_id(); + model &= MIDR_IMPLEMENTOR_MASK | (0xf00 << MIDR_PARTNUM_SHIFT) | +MIDR_ARCHITECTURE_MASK; + + return model == entry->midr_model; +} + static bool has_mismatched_cache_line_size(const struct arm64_cpu_capabilities *entry, int scope) @@ -169,6 +183,13 @@ const struct arm64_cpu_capabilities arm64_errata[] = { MIDR_CPU_VAR_REV(0, 0), MIDR_CPU_VAR_REV(0, 0)), }, + { + .desc = "Qualcomm Technologies Kryo erratum 1003", + .capability = ARM64_WORKAROUND_QCOM_FALKOR_E1003, + .def_scope = SCOPE_LOCAL_CPU, + .midr_model = MIDR_QCOM_KRYO, + .matches = is_kryo_midr, + }, #endif #ifdef CONFIG_QCOM_FALKOR_ERRATUM_1009 { -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [kernel-hardening] Re: [PATCH resend 2/2] userns: control capabilities of some user namespaces
On Wed, Nov 08, 2017 at 03:09:59AM -0800, Mahesh Bandewar (महेश बंडेवार) wrote: > Sorry folks I was traveling and seems like lot happened on this thread. :p > > I will try to response few of these comments selectively - > > > The thing that makes me hesitate with this set is that it is a > > permanent new feature to address what (I hope) is a temporary > > problem. > I agree this is permanent new feature but it's not solving a temporary > problem. It's impossible to assess what and when new vulnerability > that could show up. I think Daniel summed it up appropriately in his > response > > > Seems like there are two naive ways to do it, the first being to just > > look at all code under ns_capable() plus code called from there. It > > seems like looking at the result of that could be fruitful. > This is really hard. The main issue that there were features designed > and developed before user-ns days with an assumption that unprivileged > users will never get certain capabilities which only root user gets. > Now that is not true anymore with user-ns creation with mapping root > for any process. Also at the same time blocking user-ns creation for > eveyone is a big-hammer which is not needed too. So it's not that easy > to just perform a code-walk-though and correct those decisions now. > > > It seems to me that the existing control in > > /proc/sys/kernel/unprivileged_userns_clone might be the better duct tape > > in that case. > This solution is essentially blocking unprivileged users from using > the user-namespaces entirely. This is not really a solution that can > work. The solution that this patch-set adds allows unprivileged users > to create user-namespaces. Actually the proposed solution is more > fine-grained approach than the unprivileged_userns_clone solution > since you can selectively block capabilities rather than completely > blocking the functionality. I've been talking to Stéphane today about this and we should also keep in mind that we have: chb@conventiont|~ > ls -al /proc/sys/user/ total 0 dr-xr-xr-x 1 root root 0 Nov 6 23:32 . dr-xr-xr-x 1 root root 0 Nov 2 22:13 .. -rw-r--r-- 1 root root 0 Nov 8 19:48 max_cgroup_namespaces -rw-r--r-- 1 root root 0 Nov 8 19:48 max_inotify_instances -rw-r--r-- 1 root root 0 Nov 8 19:48 max_inotify_watches -rw-r--r-- 1 root root 0 Nov 8 19:48 max_ipc_namespaces -rw-r--r-- 1 root root 0 Nov 8 19:48 max_mnt_namespaces -rw-r--r-- 1 root root 0 Nov 8 19:48 max_net_namespaces -rw-r--r-- 1 root root 0 Nov 8 19:48 max_pid_namespaces -rw-r--r-- 1 root root 0 Nov 8 19:48 max_user_namespaces -rw-r--r-- 1 root root 0 Nov 8 19:48 max_uts_namespaces These files allow you to limit the number of namespaces that can be created *per namespace* type. So let's say your system runs a bunch of user namespaces you can do: chb@conventiont|~ > echo 0 > /proc/sys/user/max_user_namespaces So that the next time you try to create a user namespaces you'd see: chb@conventiont|~ > unshare -U unshare: unshare failed: No space left on device So there's not even a need to upstream a new sysctl since we have ways of blocking this. Also I'd like to point out that a lot of capability checks and actual security vulnerabilities are associated with CAP_SYS_ADMIN. So what you likely want to do is block CAP_SYS_ADMIN in user namespaces but at this point they become basically useless for a lot of interesting use cases. In addition, this patch would add another layer of complexity that is - imho - not really warranted given what we already have. The relationship between capabilities and user namespaces should stay as simply as possible so that it stays maintaineable. User namespaces already introduce a proper layer of complexity. Just my two cents. I might be totally off here of course. Christian
Re: WARNING in __check_heap_object
On Wed, Nov 8, 2017 at 12:23 AM, Dmitry Vyukov wrote: > On Tue, Nov 7, 2017 at 9:35 PM, Kees Cook wrote: >> On Tue, Nov 7, 2017 at 10:36 AM, syzbot >> >> wrote: >>> Hello, >>> >>> syzkaller hit the following crash on >>> 5a3517e009e979f21977d362212b7729c5165d92 >>> git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/master >>> compiler: gcc (GCC) 7.1.1 20170620 >>> .config is attached >>> Raw console output is attached. >>> C reproducer is attached >>> syzkaller reproducer is attached. See https://goo.gl/kgGztJ >>> for information about syzkaller reproducers >>> >>> >> >> Please include the line _before_ the "cut here" (dumb, I know, but >> that's where warnings show up...) >> >> Found in the raw.log: >> >> [ 44.227177] unexpected usercopy without slab whitelist from SCTPv6 >> offset 1648 size 11 >> >> This means some part of the SCTPv6 slab was being poked into userspace >> without a usercopy whitelist. >> >>> check_heap_object mm/usercopy.c:222 [inline] >>> __check_object_size+0x22c/0x4f0 mm/usercopy.c:248 >>> check_object_size include/linux/thread_info.h:112 [inline] >>> check_copy_size include/linux/thread_info.h:143 [inline] >>> copy_to_user include/linux/uaccess.h:154 [inline] >>> sctp_getsockopt_events net/sctp/socket.c:4972 [inline] >>> sctp_getsockopt+0x2b90/0x70b0 net/sctp/socket.c:7012 >>> sock_common_getsockopt+0x95/0xd0 net/core/sock.c:2924 >>> SYSC_getsockopt net/socket.c:1882 [inline] >>> SyS_getsockopt+0x178/0x340 net/socket.c:1864 >>> entry_SYSCALL_64_fastpath+0x1f/0xbe >> >> Looking at the SCTPv6 slab declaration, it seems David and I missed >> the usercopy whitelist for the sctpv6_sock struct. I'll update the >> usercopy whitelist patch with: >> >> #syz fix: sctp: Define usercopy region in SCTP proto slab cache >> >> diff --git a/net/sctp/socket.c b/net/sctp/socket.c >> index 5fd83974c5cc..8ac85877c0e4 100644 >> --- a/net/sctp/socket.c >> +++ b/net/sctp/socket.c >> @@ -8492,6 +8492,10 @@ struct proto sctpv6_prot = { >> .unhash = sctp_unhash, >> .get_port = sctp_get_port, >> .obj_size = sizeof(struct sctp6_sock), >> + .useroffset = offsetof(struct sctp_sock, subscribe), >> + .usersize = offsetof(struct sctp_sock, initmsg) - >> + offsetof(struct sctp_sock, subscribe) + >> + sizeof_field(struct sctp_sock, initmsg), >> .sysctl_mem = sysctl_sctp_mem, >> .sysctl_rmem= sysctl_sctp_rmem, >> .sysctl_wmem= sysctl_sctp_wmem, >> >> Thanks! > > > Kees, please also follow this part once the commit reaches any of > trees (title is settled): > >> syzbot will keep track of this bug report. >> Once a fix for this bug is committed, please reply to this email with: >> #syz fix: exact-commit-title >> Note: all commands must start from beginning of the line. > > This will greatly help to keep the whole process running and report > new bugs in future. I included that in my email reply already, and the commit will be in -next shortly. (Do you prefer something else?) -Kees -- Kees Cook Pixel Security
Re: [PATCH] dell-smbios: fix string overflow
On Wed, Nov 8, 2017 at 8:30 PM, wrote: >> I recommend using "platform/x86: dell-smbios:" in commit header. While we (maintainers) are fixing this manually, it would be better if contributors will do this themselves :-) >> On 11/08/2017 04:08 AM, Arnd Bergmann wrote: >> > The new sysfs code overwrites two fixed-length character arrays >> > that are each one byte shorter than they need to be, to hold >> > the trailing \0: >> > >> > drivers/platform/x86/dell-smbios.c: In function 'build_tokens_sysfs': >> > drivers/platform/x86/dell-smbios.c:494:42: error: 'sprintf' writing a >> > terminating >> nul past the end of the destination [-Werror=format-overflow=] >> > sprintf(buffer_location, "%04x_location", >> > drivers/platform/x86/dell-smbios.c:494:3: note: 'sprintf' output 14 bytes >> > into a >> destination of size 13 >> > drivers/platform/x86/dell-smbios.c:506:36: error: 'sprintf' writing a >> > terminating >> nul past the end of the destination [-Werror=format-overflow=] >> > sprintf(buffer_value, "%04x_value", >> > drivers/platform/x86/dell-smbios.c:506:3: note: 'sprintf' output 11 bytes >> > into a >> destination of size 10 >> Don't need to include the error log in commit message. Just explaining >> the issue is good enough. >> > >> > This changes it to just use kasprintf(), which always gets it right. Good catch followed by a fix! > Assuming Darren will do the fixup for title and message as recommended by > Sathyanarayanan before committing: > Acked-by: Mario Limonciello > Thanks, for the fix. For my own information and improvement, would you > share how you caught that? Just added "-Werror=format-overflow=" to CFLAGS? > I wasn't seeing the compile errors with default flags in my own testing, so > I'd like > to make sure I'm doing better testing in the future. gcc7 by its default [1], though you need to revert [2]. And always good to remember % make W=1 which implies some warning, W=2 a lot more. [1]: http://patches.linaro.org/cover/107779/ [2]: commit bd664f6b3e376a8ef4990f87d08271cc2d01ba9a Author: Linus Torvalds Date: Wed Jul 12 19:25:47 2017 -0700 disable new gcc-7.1.1 warnings for now -- With Best Regards, Andy Shevchenko
Re: [PATCH net-next 4/6] net: dsa: remove trans argument from vlan ops
Hi Joe, Joe Perches writes: > On Wed, 2017-11-08 at 12:19 -0500, Vivien Didelot wrote: >> The DSA switch VLAN ops pass the switchdev_trans structure down to the >> drivers, but no one is using them and they aren't supposed to anyway. > [] >> diff --git a/include/net/dsa.h b/include/net/dsa.h > [] >> @@ -410,12 +410,10 @@ struct dsa_switch_ops { >> */ >> int (*port_vlan_filtering)(struct dsa_switch *ds, int port, >> bool vlan_filtering); >> -int (*port_vlan_prepare)(struct dsa_switch *ds, int port, >> - const struct switchdev_obj_port_vlan *vlan, >> - struct switchdev_trans *trans); >> -void(*port_vlan_add)(struct dsa_switch *ds, int port, >> - const struct switchdev_obj_port_vlan *vlan, >> - struct switchdev_trans *trans); >> +int (*port_vlan_prepare)(struct dsa_switch *ds, int port, >> + const struct switchdev_obj_port_vlan *vlan); >> +void (*port_vlan_add)(struct dsa_switch *ds, int port, >> + const struct switchdev_obj_port_vlan *vlan); >> int (*port_vlan_del)(struct dsa_switch *ds, int port, >> const struct switchdev_obj_port_vlan *vlan); > > I think this bit is slightly worse. > Mixing alignment styles seems odd. > > I think it's better to either align all the (*func) uses > on a tabstop or > none of them. I couldn't use a tab here as it is done in the other functions, because of the 80-char limit. I will send a patch soon to remove all of this alignment style in this header because it doesn't bring any value. Thanks, Vivien
Re: [3/3] arm64: Add software workaround for Falkor erratum 1041
On Thu, 2 Nov 2017, Shanker Donthineni wrote: The ARM architecture defines the memory locations that are permitted to be accessed as the result of a speculative instruction fetch from an exception level for which all stages of translation are disabled. Specifically, the core is permitted to speculatively fetch from the 4KB region containing the current program counter and next 4KB. When translation is changed from enabled to disabled for the running exception level (SCTLR_ELn[M] changed from a value of 1 to 0), the Falkor core may errantly speculatively access memory locations outside of the 4KB region permitted by the architecture. The errant memory access may lead to one of the following unexpected behaviors. 1) A System Error Interrupt (SEI) being raised by the Falkor core due to the errant memory access attempting to access a region of memory that is protected by a slave-side memory protection unit. 2) Unpredictable device behavior due to a speculative read from device memory. This behavior may only occur if the instruction cache is disabled prior to or coincident with translation being changed from enabled to disabled. To avoid the errant behavior, software must execute an ISB immediately prior to executing the MSR that will change SCTLR_ELn[M] from 1 to 0. Signed-off-by: Shanker Donthineni --- Documentation/arm64/silicon-errata.txt | 1 + arch/arm64/Kconfig | 10 ++ arch/arm64/include/asm/assembler.h | 17 + arch/arm64/include/asm/cpucaps.h | 3 ++- arch/arm64/kernel/cpu_errata.c | 16 arch/arm64/kernel/efi-entry.S | 4 ++-- arch/arm64/kernel/head.S | 4 ++-- 7 files changed, 50 insertions(+), 5 deletions(-) diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt index 66e8ce1..704770c0 100644 --- a/Documentation/arm64/silicon-errata.txt +++ b/Documentation/arm64/silicon-errata.txt @@ -74,3 +74,4 @@ stable kernels. | Qualcomm Tech. | Falkor v1 | E1003 | QCOM_FALKOR_ERRATUM_1003 | | Qualcomm Tech. | Falkor v1 | E1009 | QCOM_FALKOR_ERRATUM_1009 | | Qualcomm Tech. | QDF2400 ITS | E0065 | QCOM_QDF2400_ERRATUM_0065 | +| Qualcomm Tech. | Falkor v{1,2} | E1041 | QCOM_FALKOR_ERRATUM_1041| diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 0df64a6..7e933fb 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -539,6 +539,16 @@ config QCOM_QDF2400_ERRATUM_0065 If unsure, say Y. +config QCOM_FALKOR_ERRATUM_1041 + bool "Falkor E1041: Speculative instruction fetches might cause errant memory access" + default y + help + Falkor CPU may speculatively fetch instructions from an improper + memory location when MMU translation is changed from SCTLR_ELn[M]=1 + to SCTLR_ELn[M]=0. Prefix an ISB instruction to fix the problem. + + If unsure, say Y. + endmenu diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h index b6dfb4f..4c91efb 100644 --- a/arch/arm64/include/asm/assembler.h +++ b/arch/arm64/include/asm/assembler.h @@ -30,6 +30,7 @@ #include #include #include +#include /* * Enable and disable interrupts. @@ -514,6 +515,22 @@ * reg: the value to be written. */ .macro write_sctlr, eln, reg +#ifdef CONFIG_QCOM_FALKOR_ERRATUM_1041 +alternative_if ARM64_WORKAROUND_QCOM_FALKOR_E1041 + tbnz\reg, #0, 8000f // enable MMU? + isb +8000: +alternative_else_nop_endif +#endif + msr sctlr_\eln, \reg + .endm + + .macro early_write_sctlr, eln, reg +#ifdef CONFIG_QCOM_FALKOR_ERRATUM_1041 + tbnz\reg, #0, 8000f // enable MMU? + isb +8000: +#endif msr sctlr_\eln, \reg .endm diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h index 8da6216..7f7a59d 100644 --- a/arch/arm64/include/asm/cpucaps.h +++ b/arch/arm64/include/asm/cpucaps.h @@ -40,7 +40,8 @@ #define ARM64_WORKAROUND_858921 19 #define ARM64_WORKAROUND_CAVIUM_30115 20 #define ARM64_HAS_DCPOP 21 +#define ARM64_WORKAROUND_QCOM_FALKOR_E1041 22 -#define ARM64_NCAPS22 +#define ARM64_NCAPS23 #endif /* __ASM_CPUCAPS_H */ diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c index 0e27f86..27f9a45 100644 --- a/arch/arm64/kernel/cpu_errata.c +++ b/arch/arm64/kernel/cpu_errata.c @@ -179,6 +179,22 @@ static int cpu_enable_trap_ctr_access(void *__unused) MIDR_CPU_VAR_REV(0, 0)), }, #endif +#ifdef CONFIG_QCOM_FALKOR_ERRATUM_1041 + { + .desc = "Qualcomm Technologies Falkor erratum 1041", + .capability = ARM64_WORKAROUND_QCOM_FALKOR_E1041, + MIDR_RANGE(MIDR_QCOM_FALKOR_V1, + MIDR_CPU_VAR_
Re: [RFC PATCH v2] ftrace: support very early function tracing
On Wed, Nov 08, 2017 at 11:07:57AM -0500, Steven Rostedt wrote: > On Wed, 8 Nov 2017 09:10:58 +0100 > Peter Zijlstra wrote: > > > On Tue, Nov 07, 2017 at 09:17:06PM -0500, Steven Rostedt wrote: > > > > +#ifdef CONFIG_X86_TSC > > > > + entry->clock = rdtsc(); > > > > +#else > > > > + entry->clock = trace_clock_local(); > > > > +#endif > > > > > > +#ifdef CONFIG_X86_TSC > > > > + cpu_khz = native_calibrate_cpu(); > > > > +#endif > > > > > > +#ifdef CONFIG_X86_TSC > > > > + ns = cycles_to_ns(entry->clock, cpu_khz); > > > > +#else > > > > + ns = entry->clock; > > > > +#endif > > > > Yeah, no, not going to happen... > > Agreed. Depends on how early you need things; ideally you'd be able to use the stuff from here: https://lkml.kernel.org/r/1504116205-355281-1-git-send-email-pasha.tatas...@oracle.com That is, fix the normal time sources to work earlier instead of building parallel infrastructure.
Re: WARNING in __check_heap_object
On Wed, Nov 8, 2017 at 8:02 PM, Kees Cook wrote: >>> On Tue, Nov 7, 2017 at 10:36 AM, syzbot >>> >>> wrote: Hello, syzkaller hit the following crash on 5a3517e009e979f21977d362212b7729c5165d92 git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/master compiler: gcc (GCC) 7.1.1 20170620 .config is attached Raw console output is attached. C reproducer is attached syzkaller reproducer is attached. See https://goo.gl/kgGztJ for information about syzkaller reproducers >>> >>> Please include the line _before_ the "cut here" (dumb, I know, but >>> that's where warnings show up...) >>> >>> Found in the raw.log: >>> >>> [ 44.227177] unexpected usercopy without slab whitelist from SCTPv6 >>> offset 1648 size 11 >>> >>> This means some part of the SCTPv6 slab was being poked into userspace >>> without a usercopy whitelist. >>> check_heap_object mm/usercopy.c:222 [inline] __check_object_size+0x22c/0x4f0 mm/usercopy.c:248 check_object_size include/linux/thread_info.h:112 [inline] check_copy_size include/linux/thread_info.h:143 [inline] copy_to_user include/linux/uaccess.h:154 [inline] sctp_getsockopt_events net/sctp/socket.c:4972 [inline] sctp_getsockopt+0x2b90/0x70b0 net/sctp/socket.c:7012 sock_common_getsockopt+0x95/0xd0 net/core/sock.c:2924 SYSC_getsockopt net/socket.c:1882 [inline] SyS_getsockopt+0x178/0x340 net/socket.c:1864 entry_SYSCALL_64_fastpath+0x1f/0xbe >>> >>> Looking at the SCTPv6 slab declaration, it seems David and I missed >>> the usercopy whitelist for the sctpv6_sock struct. I'll update the >>> usercopy whitelist patch with: >>> >>> #syz fix: sctp: Define usercopy region in SCTP proto slab cache >>> >>> diff --git a/net/sctp/socket.c b/net/sctp/socket.c >>> index 5fd83974c5cc..8ac85877c0e4 100644 >>> --- a/net/sctp/socket.c >>> +++ b/net/sctp/socket.c >>> @@ -8492,6 +8492,10 @@ struct proto sctpv6_prot = { >>> .unhash = sctp_unhash, >>> .get_port = sctp_get_port, >>> .obj_size = sizeof(struct sctp6_sock), >>> + .useroffset = offsetof(struct sctp_sock, subscribe), >>> + .usersize = offsetof(struct sctp_sock, initmsg) - >>> + offsetof(struct sctp_sock, subscribe) + >>> + sizeof_field(struct sctp_sock, initmsg), >>> .sysctl_mem = sysctl_sctp_mem, >>> .sysctl_rmem= sysctl_sctp_rmem, >>> .sysctl_wmem= sysctl_sctp_wmem, >>> >>> Thanks! >> >> >> Kees, please also follow this part once the commit reaches any of >> trees (title is settled): >> >>> syzbot will keep track of this bug report. >>> Once a fix for this bug is committed, please reply to this email with: >>> #syz fix: exact-commit-title >>> Note: all commands must start from beginning of the line. >> >> This will greatly help to keep the whole process running and report >> new bugs in future. > > I included that in my email reply already, and the commit will be in > -next shortly. (Do you prefer something else?) Doh! That's just me trying to handle 200 bugs at the same time. Sorry.
Re: [RFC PATCH v2] ftrace: support very early function tracing
On Wed, 08 Nov 2017 18:44:39 + Abderrahmane Benbachir wrote: > I implemented the ring_buffer_set_clock solution and I have some questions. > > > void __init ftrace_early_fill_ringbuffer(void *data) > { > ... > ring_buffer_set_clock(tr->trace_buffer.buffer, early_trace_clock); > preempt_disable_notrace(); > for (i = 0; i < vearly_entries_count; i++) { > entry = &ftrace_vearly_entries[i]; > > early_timestamp = entry->clock; > > trace_function(tr, entry->ip, entry->parent_ip, 0, 0); > } > preempt_enable_notrace(); > > /* TODO: should set the previous clock */ > ring_buffer_set_clock(tr->trace_buffer.buffer, trace_clock_local); > > I need also to save the current clock before calling > "ring_buffer_set_clock()", is there > a way to get the current clock or to set the clock from tr->clock_id ? No, but we could add one, although it is not really necessary. This is why I just defaulted to what it is set to at the start anyway. It's hard coded to trace_clock_local now, and it's only changed via the command line parameter after this code is run. > > > There is one thing happening when using the trace_clock_local instead > of rdtsc. > > I added different kernel params to trigger the issue : > > When using entry->clock = trace_clock_local(): > > kernel param "ftrace_vearly ftrace_filter=*_init" > > > [0.312179] -0 0dp..0us : boot_cpu_init <-start_kernel > ... > [0.320280] -0 0dp..0us : numa_init <-x86_numa_init > [0.320673] -0 0dp..0us : dummy_numa_init <-numa_init > [0.321082] -0 0dp..6us : paging_init <-setup_arch > [0.321471] -0 0dp..9us : sparse_init <-paging_init > [0.321867] -0 0dp.. 1174us : zone_sizes_init <-paging_init > [0.322283] -0 0dp.. 1201us : lruvec_init > <-free_area_init_node > [0.322714] -0 0dp.. 1524us : acpi_boot_init <-setup_arch > [0.323120] -0 0dp.. 1595us : sfi_init <-setup_arch > [0.323500] -0 0dp.. 1601us : kvm_guest_init <-setup_arch > [0.323904] -0 0dp.. 1623us : mcheck_init <-setup_arch > [0.324293] -0 0dp.. 1623us : mcheck_intel_therm_init > <-mcheck_init > [0.324772] -0 0dp.. 1724us : boot_cpu_state_init > <-start_kernel > [0.325259] -0 0dp.. 1724us : kvm_guest_cpu_init > <-kvm_smp_prepare_boot_cpu > [0.325789] -0 0dp.. 1733us : kvm_spinlock_init > <-kvm_smp_prepare_boot_cpu > [0.326300] -0 0dp.. 1733us : > build_all_zonelists_init <-build_all_zonelists > [0.326820] -0 0dp.. 1738us : page_alloc_init <-start_kernel > [0.327252] -0 0dp.. 1906us : jump_label_init <-start_kernel > [0.327673] -0 0dp.. 4913us : pidhash_init <-start_kernel > [0.328076] -0 0dp.. 4917us : trap_init <-start_kernel > [0.328466] -0 0dp.. 4927us : kvm_apf_trap_init <-trap_init > [0.328877] -0 0dp.. 4928us : mem_init <-start_kernel > [0.329263] -0 0dp.. 4930us : gart_iommu_hole_init > <-pci_iommu_alloc > [0.329745] -0 0dp.. 5129us : kmem_cache_init <-start_kernel > [0.330162] -0 0dp.. 5300us : vmalloc_init <-start_kernel > > > kernel param "ftrace=function ftrace_vearly ftrace_filter=*_init" > > [0.315455] -0 0dp.. 18446744073666366us : > boot_cpu_init <-start_kernel > ... > [0.338102] -0 0dp.. 18446744073671436us : trap_init > <-start_kernel > [0.338584] -0 0dp.. 18446744073671446us : > kvm_apf_trap_init <-trap_init > [0.339089] -0 0dp.. 18446744073671447us : mem_init > <-start_kernel > [0.339564] -0 0dp.. 18446744073671449us : > gart_iommu_hole_init <-pci_iommu_alloc > [0.340111] -0 0dp.. 18446744073671635us : > kmem_cache_init <-start_kernel > [0.340622] -0 0dp.. 18446744073671775us : > vmalloc_init <-start_kernel > function tracing starts here << > [0.341119] -0 0dp.1 48004us : sched_init <-start_kernel > [0.341520] -0 0dp.1 48005us : wait_bit_init <-sched_init > [0.341926] -0 0dp.1 48007us : hrtimer_init > <-init_rt_bandwidth > [0.342359] -0 0dp.1 48007us : __hrtimer_init <-hrtimer_init > [0.342779] -0 0dp.1 48007us : cpudl_init <-init_rootdomain > [0.343196] -0 0dp.1 48008us : cpupri_init <-init_rootdomain > [0.343618] -0 0dp.1 48014us : autogroup_init <-sched_init > I'd have to see the code to figure this out. -- Steve > > When using entry->clock = rdtsc() [working well]: > > kernel param "ftrace=function ftrace_vearly ftrace_filter=*_init" > > [0.314067] -0 0dp.. 154207us : boot_cpu_init <-start_kernel > ... > [0.97] -0 0dp.. 180362us : mem_init <-start_kernel > [0.333806] -0 0dp.. 180365us : ga
Re: [PATCH] apparmor: fix off-by-one comparison on MAXMAPPED_SIG
On 11/08/2017 10:53 AM, Linus Torvalds wrote: > On Wed, Nov 8, 2017 at 8:09 AM, John Johansen > wrote: >> >> Signed-off-by: Colin Ian King >> Signed-off-by: John Johansen > > This sign-off chain is odd. It implies that the patch came from Colin > King, bnut it lacks authorship attribution. > > So who authored this? > > If it was Colin, there should have been a > > From: Colin Ian King > > at the top. > > So either the authorship is broken, or the sign-off chain is. Which is it? > The initial patch is from Colin it covered the 1st of the 2 cases. I edited the patch, adding the second case and added a revision note then I messed up the mail. Rushing to get this out (I dumped it into the regular mail client instead of using git mail). I should have just sent the request pull The following changes since commit 8b50f0a6261b3b6e6c7ef9febe7ab8a973243ba3: apparmor: fix off-by-one comparison on MAXMAPPED_SIG (2017-11-08 07:49:53 -0800) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor aa-for-up for you to fetch changes up to 8b50f0a6261b3b6e6c7ef9febe7ab8a973243ba3: apparmor: fix off-by-one comparison on MAXMAPPED_SIG (2017-11-08 07:49:53 -0800)
[PATCH 0/2] Restructure and fix GHES PCIe AER handling
First, break the PCIe AER handling out into its own function to separate it from the standard GHES processing Then fix the AER handling to process all errors in the AER driver rather than only handling recoverable errors. Tyler Baicar (2): acpi: apei: handle PCIe AER errors in separate function acpi: apei: call into AER handling regardless of severity drivers/acpi/apei/ghes.c | 62 +--- 1 file changed, 32 insertions(+), 30 deletions(-) -- Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
[PATCH 1/2] acpi: apei: handle PCIe AER errors in separate function
Move PCIe AER error handling code into a separate function. Signed-off-by: Tyler Baicar --- drivers/acpi/apei/ghes.c | 64 +--- 1 file changed, 34 insertions(+), 30 deletions(-) diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c index 3c3a37b..839c3d5 100644 --- a/drivers/acpi/apei/ghes.c +++ b/drivers/acpi/apei/ghes.c @@ -458,6 +458,39 @@ static void ghes_handle_memory_failure(struct acpi_hest_generic_data *gdata, int #endif } +static void ghes_handle_aer(struct acpi_hest_generic_data *gdata, int sev, int sec_sev) +{ +#ifdef CONFIG_ACPI_APEI_PCIEAER + struct cper_sec_pcie *pcie_err = acpi_hest_get_payload(gdata); + + if (sev == GHES_SEV_RECOVERABLE && + sec_sev == GHES_SEV_RECOVERABLE && + pcie_err->validation_bits & CPER_PCIE_VALID_DEVICE_ID && + pcie_err->validation_bits & CPER_PCIE_VALID_AER_INFO) { + unsigned int devfn; + int aer_severity; + + devfn = PCI_DEVFN(pcie_err->device_id.device, + pcie_err->device_id.function); + aer_severity = cper_severity_to_aer(gdata->error_severity); + + /* +* If firmware reset the component to contain +* the error, we must reinitialize it before +* use, so treat it as a fatal AER error. +*/ + if (gdata->flags & CPER_SEC_RESET) + aer_severity = AER_FATAL; + + aer_recover_queue(pcie_err->device_id.segment, + pcie_err->device_id.bus, + devfn, aer_severity, + (struct aer_capability_regs *) + pcie_err->aer_info); + } +#endif +} + static void ghes_do_proc(struct ghes *ghes, const struct acpi_hest_generic_status *estatus) { @@ -485,38 +518,9 @@ static void ghes_do_proc(struct ghes *ghes, arch_apei_report_mem_error(sev, mem_err); ghes_handle_memory_failure(gdata, sev); } -#ifdef CONFIG_ACPI_APEI_PCIEAER else if (guid_equal(sec_type, &CPER_SEC_PCIE)) { - struct cper_sec_pcie *pcie_err = acpi_hest_get_payload(gdata); - - if (sev == GHES_SEV_RECOVERABLE && - sec_sev == GHES_SEV_RECOVERABLE && - pcie_err->validation_bits & CPER_PCIE_VALID_DEVICE_ID && - pcie_err->validation_bits & CPER_PCIE_VALID_AER_INFO) { - unsigned int devfn; - int aer_severity; - - devfn = PCI_DEVFN(pcie_err->device_id.device, - pcie_err->device_id.function); - aer_severity = cper_severity_to_aer(gdata->error_severity); - - /* -* If firmware reset the component to contain -* the error, we must reinitialize it before -* use, so treat it as a fatal AER error. -*/ - if (gdata->flags & CPER_SEC_RESET) - aer_severity = AER_FATAL; - - aer_recover_queue(pcie_err->device_id.segment, - pcie_err->device_id.bus, - devfn, aer_severity, - (struct aer_capability_regs *) - pcie_err->aer_info); - } - + ghes_handle_aer(gdata, sev, sec_sev); } -#endif else if (guid_equal(sec_type, &CPER_SEC_PROC_ARM)) { struct cper_sec_proc_arm *err = acpi_hest_get_payload(gdata); -- Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
[PATCH V3 2/2] acpi: apei: call into AER handling regardless of severity
Currently the GHES code only calls into the AER driver for recoverable type errors. This is incorrect because errors of other severities do not get logged by the AER driver and do not get exposed to user space via the AER trace event. So, call into the AER driver for PCIe errors regardless of the severity Signed-off-by: Tyler Baicar --- drivers/acpi/apei/ghes.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c index 839c3d5..bb65fa6 100644 --- a/drivers/acpi/apei/ghes.c +++ b/drivers/acpi/apei/ghes.c @@ -458,14 +458,12 @@ static void ghes_handle_memory_failure(struct acpi_hest_generic_data *gdata, int #endif } -static void ghes_handle_aer(struct acpi_hest_generic_data *gdata, int sev, int sec_sev) +static void ghes_handle_aer(struct acpi_hest_generic_data *gdata) { #ifdef CONFIG_ACPI_APEI_PCIEAER struct cper_sec_pcie *pcie_err = acpi_hest_get_payload(gdata); - if (sev == GHES_SEV_RECOVERABLE && - sec_sev == GHES_SEV_RECOVERABLE && - pcie_err->validation_bits & CPER_PCIE_VALID_DEVICE_ID && + if (pcie_err->validation_bits & CPER_PCIE_VALID_DEVICE_ID && pcie_err->validation_bits & CPER_PCIE_VALID_AER_INFO) { unsigned int devfn; int aer_severity; @@ -519,7 +517,7 @@ static void ghes_do_proc(struct ghes *ghes, ghes_handle_memory_failure(gdata, sev); } else if (guid_equal(sec_type, &CPER_SEC_PCIE)) { - ghes_handle_aer(gdata, sev, sec_sev); + ghes_handle_aer(gdata); } else if (guid_equal(sec_type, &CPER_SEC_PROC_ARM)) { struct cper_sec_proc_arm *err = acpi_hest_get_payload(gdata); -- Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
Re: [PATCH v2] lib/mpi: call cond_resched() from mpi_powm() loop
Hi David, On Wed, Nov 08, 2017 at 12:50:17PM +, David Howells wrote: > Eric Biggers wrote: > > > On a non-preemptible kernel, if KEYCTL_DH_COMPUTE is called with the > > largest permitted inputs (16384 bits), the kernel spends 10+ seconds > > doing modular exponentiation in mpi_powm() without rescheduling. If all > > threads do it, it locks up the system. Moreover, it can cause > > rcu_sched-stall warnings. > > Do you want this to go in immediately, or I should I push it to James for the > next merge window? > > David This probably should be grouped with my series "crypto: dh - input validation fixes", as this is also a fix for Diffie-Hellman. I was actually expecting Herbert Xu to take these patches, as Diffie-Hellman is now part of the crypto API (crypto/dh.c); none of the patches actually touch security/keys/. If you'd like to maintain the Diffie-Hellman implementation(s) you should get yourself added to MAINTAINERS for the relevant files. As for the criticality of the fixes, the code is new in v4.12, so I doubt many people have CONFIG_KEY_DH_OPERATIONS=y enabled yet. syzbot doesn't actually have it enabled either (we should turn it on); I just happened to find these bugs by running syzkaller myself. So it may not be particularly critical to take these fixes immediately. However, at least the fix for the double free bug ("crypto: dh - Fix double free of ctx->p") shouldn't wait for too long... Eric
Re: [RFC PATCH v2] ftrace: support very early function tracing
On Wed, 8 Nov 2017 20:07:27 +0100 Peter Zijlstra wrote: > Depends on how early you need things; ideally you'd be able to use the Well, he starts tracing at the entry of start_kernel(), would this handle that? -- Steve > stuff from here: > > > https://lkml.kernel.org/r/1504116205-355281-1-git-send-email-pasha.tatas...@oracle.com > > That is, fix the normal time sources to work earlier instead of building > parallel infrastructure.
Re: [PATCH v2 07/12] hwrng: bcm2835-rng: Manage an optional clock
Hi Florian, > Florian Fainelli hat am 8. November 2017 um 01:44 > geschrieben: > > > One of the last steps before bcm63xx-rng can be eliminated is to manage > a clock during hwrng::init and hwrng::cleanup, so fetch it in the probe > function, and manage it during these two steps when valid. > > Signed-off-by: Florian Fainelli > --- > drivers/char/hw_random/bcm2835-rng.c | 17 - > 1 file changed, 16 insertions(+), 1 deletion(-) > > diff --git a/drivers/char/hw_random/bcm2835-rng.c > b/drivers/char/hw_random/bcm2835-rng.c > index ed20e0b6b7ae..99b56fd5482c 100644 > --- a/drivers/char/hw_random/bcm2835-rng.c > +++ b/drivers/char/hw_random/bcm2835-rng.c > @@ -15,6 +15,7 @@ > #include > #include > #include > +#include > > #define RNG_CTRL 0x0 > #define RNG_STATUS 0x4 > @@ -33,6 +34,7 @@ struct bcm2835_rng_priv { > struct hwrng rng; > void __iomem *base; > bool mask_interrupts; > + struct clk *clk; > }; > > static inline struct bcm2835_rng_priv *to_rng_priv(struct hwrng *rng) > @@ -66,8 +68,15 @@ static int bcm2835_rng_read(struct hwrng *rng, void *buf, > size_t max, > static int bcm2835_rng_init(struct hwrng *rng) > { > struct bcm2835_rng_priv *priv = to_rng_priv(rng); > + int ret = 0; > u32 val; > > + if (!IS_ERR(priv->clk)) { > + ret = clk_prepare_enable(priv->clk); > + if (ret) > + return ret; > + } > + the clocks are optional to the binding, but not for the proper function of all RNG. So shouldn't we catch the case that we cannot get the clock during probe, but the clock is required for a specific SoC? > if (priv->mask_interrupts) { > /* mask the interrupt */ > val = readl(priv->base + RNG_INT_MASK); > @@ -79,7 +88,7 @@ static int bcm2835_rng_init(struct hwrng *rng) > __raw_writel(RNG_WARMUP_COUNT, priv->base + RNG_STATUS); > __raw_writel(RNG_RBGEN, priv->base + RNG_CTRL); > > - return 0; > + return ret; > } > > static void bcm2835_rng_cleanup(struct hwrng *rng) > @@ -88,6 +97,9 @@ static void bcm2835_rng_cleanup(struct hwrng *rng) > > /* disable rng hardware */ > __raw_writel(0, priv->base + RNG_CTRL); > + > + if (!IS_ERR(priv->clk)) > + clk_disable_unprepare(priv->clk); > } > > struct bcm2835_rng_of_data { > @@ -130,6 +142,9 @@ static int bcm2835_rng_probe(struct platform_device *pdev) > return PTR_ERR(priv->base); > } > > + /* Clock is optional on most platforms */ > + priv->clk = devm_clk_get(dev, NULL); > + At least EPROBE_DEFER should be handled here: if (IS_ERR(priv->clk) && PTR_ERR(priv->clk) == -EPROBE_DEFER) return -EPROBE_DEFER; Regards Stefan