Re: [PATCH V5 4/4] gpio: tegra: Add support for gpio debounce
On Mon, Apr 25, 2016 at 7:38 PM, Laxman Dewanganwrote: > NVIDIA's Tegra210 support the HW debounce in the GPIO controller > for all its GPIO pins. > > Add support for setting debounce timing by implementing the > set_debounce callback of gpiochip. > > Signed-off-by: Laxman Dewangan > Reviewed-by: Stephen Warren Reviewed-by: Alexandre Courbot
Re: [PATCH V5 4/4] gpio: tegra: Add support for gpio debounce
On Mon, Apr 25, 2016 at 7:38 PM, Laxman Dewangan wrote: > NVIDIA's Tegra210 support the HW debounce in the GPIO controller > for all its GPIO pins. > > Add support for setting debounce timing by implementing the > set_debounce callback of gpiochip. > > Signed-off-by: Laxman Dewangan > Reviewed-by: Stephen Warren Reviewed-by: Alexandre Courbot
Re: [PATCH v6 04/10] regulator: fixed: add support for ACPI interface
Hi, On 04/27/2016 08:33 PM, Mark Brown wrote: > On Wed, Apr 27, 2016 at 09:54:10AM +0800, Lu Baolu wrote: > >> Please refer to Documentation/acpi/gpio-properties.txt. > That's not visibly what your driver is doing, that is also recommending > using a static name which is what I'm asking for. Yes, I agree that we should use a static name. > >>> Why does the device care?It's requesting the GPIO in >>> its own context and it's only requesting one GPIO, with DT we're just >>> always calling the GPIO "gpio" which works fine. >> This driver is not bound to an ACPI device node directly. It's a child >> of a mfd device, which is corresponding to a real ACPI device node. > If it's the child of a MFD it's got an ACPI device, the ACPI device is > the parent.It should use the parent device or the parent should map > the GPIO through to the child as many other MFDs do, the whole concept > of a MFD is a Linux internal implementation detail. Yes. The mapping of GPIO is done in the parent. And the parent passes the GPIO by setting ACPI companion to this device (done in mfd internal). This driver is able to get the gpio descriptor with a static name. How about below code? + gpiod = gpiod_get(dev, "vbus_en", GPIOD_ASIS); + if (IS_ERR(gpiod)) + return PTR_ERR(gpiod); + + config->gpio = desc_to_gpio(gpiod); + config->enable_high = device_property_read_bool(dev, + "enable-active-high"); + gpiod_put(gpiod); Best regards, Lu Baolu
Re: [PATCH v6 04/10] regulator: fixed: add support for ACPI interface
Hi, On 04/27/2016 08:33 PM, Mark Brown wrote: > On Wed, Apr 27, 2016 at 09:54:10AM +0800, Lu Baolu wrote: > >> Please refer to Documentation/acpi/gpio-properties.txt. > That's not visibly what your driver is doing, that is also recommending > using a static name which is what I'm asking for. Yes, I agree that we should use a static name. > >>> Why does the device care?It's requesting the GPIO in >>> its own context and it's only requesting one GPIO, with DT we're just >>> always calling the GPIO "gpio" which works fine. >> This driver is not bound to an ACPI device node directly. It's a child >> of a mfd device, which is corresponding to a real ACPI device node. > If it's the child of a MFD it's got an ACPI device, the ACPI device is > the parent.It should use the parent device or the parent should map > the GPIO through to the child as many other MFDs do, the whole concept > of a MFD is a Linux internal implementation detail. Yes. The mapping of GPIO is done in the parent. And the parent passes the GPIO by setting ACPI companion to this device (done in mfd internal). This driver is able to get the gpio descriptor with a static name. How about below code? + gpiod = gpiod_get(dev, "vbus_en", GPIOD_ASIS); + if (IS_ERR(gpiod)) + return PTR_ERR(gpiod); + + config->gpio = desc_to_gpio(gpiod); + config->enable_high = device_property_read_bool(dev, + "enable-active-high"); + gpiod_put(gpiod); Best regards, Lu Baolu
Re: [PATCH trivial] scripts/prune-kernel: remove kdump images
On 03/07/16 at 03:56pm, Xiong Zhou wrote: > Signed-off-by: Xiong Zhou> --- > scripts/prune-kernel | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/scripts/prune-kernel b/scripts/prune-kernel > index ab5034e..9c67be2 100755 > --- a/scripts/prune-kernel > +++ b/scripts/prune-kernel > @@ -14,7 +14,7 @@ do > echo "removing $f" > rm -f "/boot/initramfs-$f.img" "/boot/System.map-$f" > rm -f "/boot/vmlinuz-$f" "/boot/config-$f" > -rm -rf "/lib/modules/$f" > +rm -rf "/lib/modules/$f" "/boot/initramfs-${f}kdump.img" It makes sense to remove kdump initramfs file if its related kernel and modules directory have been removed. I like this fix, maybe it's better to add at least one line of description to tell why it deserves removal. Thanks Baoquan
Re: [PATCH trivial] scripts/prune-kernel: remove kdump images
On 03/07/16 at 03:56pm, Xiong Zhou wrote: > Signed-off-by: Xiong Zhou > --- > scripts/prune-kernel | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/scripts/prune-kernel b/scripts/prune-kernel > index ab5034e..9c67be2 100755 > --- a/scripts/prune-kernel > +++ b/scripts/prune-kernel > @@ -14,7 +14,7 @@ do > echo "removing $f" > rm -f "/boot/initramfs-$f.img" "/boot/System.map-$f" > rm -f "/boot/vmlinuz-$f" "/boot/config-$f" > -rm -rf "/lib/modules/$f" > +rm -rf "/lib/modules/$f" "/boot/initramfs-${f}kdump.img" It makes sense to remove kdump initramfs file if its related kernel and modules directory have been removed. I like this fix, maybe it's better to add at least one line of description to tell why it deserves removal. Thanks Baoquan
Re: [PATCH v2 2/3] ARM: dts: exynos: Define vqmmc for eMMC card on Odroid X/X2/U3
On 04/27/2016 11:29 PM, Javier Martinez Canillas wrote: > Hello Krzysztof, > > On 04/27/2016 10:00 AM, Krzysztof Kozlowski wrote: >> The eMMC card vmmc-supply contained incorrectly two regulators: LDO20 >> and buck8. The second one is ignored. Additionally the buck8 is a vqmmc >> supply only on X and X2. On U3 the buck8 is providing power to the LAN >> (SMSC95xx) so instead the LDO22 should be used. >> >> Fix this by defining proper vmmc and vqmmc supplies for respective >> boards. >> >> Signed-off-by: Krzysztof Kozlowski>> >> --- >> >> Changes since v1: >> 1. buck8 is used on X/X2 so differentiate the configuration (hint by >>Tobias Jakobi). >> --- >> arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 13 ++--- >> arch/arm/boot/dts/exynos4412-odroidu3.dts | 18 ++ >> arch/arm/boot/dts/exynos4412-odroidx.dts| 11 +++ >> arch/arm/boot/dts/exynos4412-odroidx2.dts | 11 +++ >> 4 files changed, 50 insertions(+), 3 deletions(-) >> >> diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi >> b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi >> index 3d0d44581fbd..34a5b3daced0 100644 >> --- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi >> +++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi >> @@ -347,6 +347,13 @@ >> regulator-boot-on; >> }; >> >> +ldo22_reg: LDO22 { >> +regulator-name = "LDO22"; >> +regulator-min-microvolt = <80>; >> +regulator-max-microvolt = <395>; >> +regulator-boot-on; >> +}; >> + > > I don't have a datasheet for the max77686 but I guess these min and max > values are the actual voltage range that the ldo22 regulator can support? Yes. > If that's the case, then I don't think setting these are correct since the > DT binding says that the regulator-{min,max}-microvolt properties describe > the voltage range that consumers may set. I've seen Mark mention this many > times, the last one I remember is at [0]. For LDO22 there is no consumer so these are the constraints. Leaving them undefined (here and in DTS for X/X2) would result in the same effect. The point is that these values are valid constraints. Please have in mind that the binding describe this as "smallest/largest voltage consumer may set" which is true for the code above. > > Probably would be better to leave the ldo22_reg definition to odroidu3.dts > to avoid setting these constraint in a .dtsi file. What about X/X2? >> ldo25_reg: LDO25 { >> regulator-name = "VDDQ_LCD_1.8V"; >> regulator-min-microvolt = <180>; >> @@ -411,8 +418,8 @@ >> >> buck8_reg: BUCK8 { >> regulator-name = "BUCK8_2.8V"; >> -regulator-min-microvolt = <280>; >> -regulator-max-microvolt = <280>; >> +regulator-min-microvolt = <75>; >> +regulator-max-microvolt = <390>; > > Same here, since not all boards have the same constraint for this regulator, > it would be better to remove it from the .dtsi and let each dts to define it. Since all of the boards define them, it does not bring any difference having it here... so I can remove them. >> }; >> }; >> }; >> @@ -456,7 +463,7 @@ >> _0 { >> pinctrl-0 = <_clk _cmd _bus4 _bus8>; >> pinctrl-names = "default"; >> -vmmc-supply = <_reg _reg>; >> +vmmc-supply = <_reg>; >> mmc-pwrseq = <_pwrseq>; >> status = "okay"; >> >> diff --git a/arch/arm/boot/dts/exynos4412-odroidu3.dts >> b/arch/arm/boot/dts/exynos4412-odroidu3.dts >> index dd89f7b37c9f..d73aa6c58fe3 100644 >> --- a/arch/arm/boot/dts/exynos4412-odroidu3.dts >> +++ b/arch/arm/boot/dts/exynos4412-odroidu3.dts >> @@ -69,6 +69,24 @@ >> }; >> }; >> >> +/* Supply for LAN9730/SMSC95xx */ >> +_reg { >> +regulator-name = "BUCK8_P3V3"; > > I think it would be better to name it "BUCK8_3.3V" for consistency. Consistency of naming is nice but in the same time we want the regulator names to match the hardware because it is easier to read the code and check the schematics. On the schematics this is unfortunately called P3V3. :) > >> +regulator-min-microvolt = <330>; >> +regulator-max-microvolt = <330>; >> +}; >> + >> +/* VDDQ for MSHC (eMMC card) */ >> +_reg { >> +regulator-name = "LDO22_VDDQ_MMC4_2.8V"; >> +regulator-min-microvolt = <280>; >> +regulator-max-microvolt = <280>; >> +}; >> + >> +_0 { >> +vqmmc-supply = <_reg>; >> +}; >> + >> { >> pinctrl-0 = <_out>; >> pinctrl-names = "default"; >> diff --git a/arch/arm/boot/dts/exynos4412-odroidx.dts >>
Re: [PATCH v2 2/3] ARM: dts: exynos: Define vqmmc for eMMC card on Odroid X/X2/U3
On 04/27/2016 11:29 PM, Javier Martinez Canillas wrote: > Hello Krzysztof, > > On 04/27/2016 10:00 AM, Krzysztof Kozlowski wrote: >> The eMMC card vmmc-supply contained incorrectly two regulators: LDO20 >> and buck8. The second one is ignored. Additionally the buck8 is a vqmmc >> supply only on X and X2. On U3 the buck8 is providing power to the LAN >> (SMSC95xx) so instead the LDO22 should be used. >> >> Fix this by defining proper vmmc and vqmmc supplies for respective >> boards. >> >> Signed-off-by: Krzysztof Kozlowski >> >> --- >> >> Changes since v1: >> 1. buck8 is used on X/X2 so differentiate the configuration (hint by >>Tobias Jakobi). >> --- >> arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 13 ++--- >> arch/arm/boot/dts/exynos4412-odroidu3.dts | 18 ++ >> arch/arm/boot/dts/exynos4412-odroidx.dts| 11 +++ >> arch/arm/boot/dts/exynos4412-odroidx2.dts | 11 +++ >> 4 files changed, 50 insertions(+), 3 deletions(-) >> >> diff --git a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi >> b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi >> index 3d0d44581fbd..34a5b3daced0 100644 >> --- a/arch/arm/boot/dts/exynos4412-odroid-common.dtsi >> +++ b/arch/arm/boot/dts/exynos4412-odroid-common.dtsi >> @@ -347,6 +347,13 @@ >> regulator-boot-on; >> }; >> >> +ldo22_reg: LDO22 { >> +regulator-name = "LDO22"; >> +regulator-min-microvolt = <80>; >> +regulator-max-microvolt = <395>; >> +regulator-boot-on; >> +}; >> + > > I don't have a datasheet for the max77686 but I guess these min and max > values are the actual voltage range that the ldo22 regulator can support? Yes. > If that's the case, then I don't think setting these are correct since the > DT binding says that the regulator-{min,max}-microvolt properties describe > the voltage range that consumers may set. I've seen Mark mention this many > times, the last one I remember is at [0]. For LDO22 there is no consumer so these are the constraints. Leaving them undefined (here and in DTS for X/X2) would result in the same effect. The point is that these values are valid constraints. Please have in mind that the binding describe this as "smallest/largest voltage consumer may set" which is true for the code above. > > Probably would be better to leave the ldo22_reg definition to odroidu3.dts > to avoid setting these constraint in a .dtsi file. What about X/X2? >> ldo25_reg: LDO25 { >> regulator-name = "VDDQ_LCD_1.8V"; >> regulator-min-microvolt = <180>; >> @@ -411,8 +418,8 @@ >> >> buck8_reg: BUCK8 { >> regulator-name = "BUCK8_2.8V"; >> -regulator-min-microvolt = <280>; >> -regulator-max-microvolt = <280>; >> +regulator-min-microvolt = <75>; >> +regulator-max-microvolt = <390>; > > Same here, since not all boards have the same constraint for this regulator, > it would be better to remove it from the .dtsi and let each dts to define it. Since all of the boards define them, it does not bring any difference having it here... so I can remove them. >> }; >> }; >> }; >> @@ -456,7 +463,7 @@ >> _0 { >> pinctrl-0 = <_clk _cmd _bus4 _bus8>; >> pinctrl-names = "default"; >> -vmmc-supply = <_reg _reg>; >> +vmmc-supply = <_reg>; >> mmc-pwrseq = <_pwrseq>; >> status = "okay"; >> >> diff --git a/arch/arm/boot/dts/exynos4412-odroidu3.dts >> b/arch/arm/boot/dts/exynos4412-odroidu3.dts >> index dd89f7b37c9f..d73aa6c58fe3 100644 >> --- a/arch/arm/boot/dts/exynos4412-odroidu3.dts >> +++ b/arch/arm/boot/dts/exynos4412-odroidu3.dts >> @@ -69,6 +69,24 @@ >> }; >> }; >> >> +/* Supply for LAN9730/SMSC95xx */ >> +_reg { >> +regulator-name = "BUCK8_P3V3"; > > I think it would be better to name it "BUCK8_3.3V" for consistency. Consistency of naming is nice but in the same time we want the regulator names to match the hardware because it is easier to read the code and check the schematics. On the schematics this is unfortunately called P3V3. :) > >> +regulator-min-microvolt = <330>; >> +regulator-max-microvolt = <330>; >> +}; >> + >> +/* VDDQ for MSHC (eMMC card) */ >> +_reg { >> +regulator-name = "LDO22_VDDQ_MMC4_2.8V"; >> +regulator-min-microvolt = <280>; >> +regulator-max-microvolt = <280>; >> +}; >> + >> +_0 { >> +vqmmc-supply = <_reg>; >> +}; >> + >> { >> pinctrl-0 = <_out>; >> pinctrl-names = "default"; >> diff --git a/arch/arm/boot/dts/exynos4412-odroidx.dts >> b/arch/arm/boot/dts/exynos4412-odroidx.dts
linux-next: build failure after merge of the tpmdd tree
Hi Jarkko, After merging the tpmdd tree, today's linux-next build (powerpc allyesconfig) failed like this: In file included from /home/sfr/next/next/include/linux/rcupdate.h:38:0, from /home/sfr/next/next/include/linux/idr.h:18, from /home/sfr/next/next/include/linux/kernfs.h:14, from /home/sfr/next/next/include/linux/sysfs.h:15, from /home/sfr/next/next/include/linux/kobject.h:21, from /home/sfr/next/next/include/linux/device.h:17, from /home/sfr/next/next/include/linux/dma-mapping.h:6, from /home/sfr/next/next/drivers/char/tpm/tpm_ibmvtpm.c:18: /home/sfr/next/next/drivers/char/tpm/tpm_ibmvtpm.c: In function 'tpm_ibmvtpm_probe': /home/sfr/next/next/include/linux/spinlock.h:295:1: error: expected ';' before 'do' do { \ ^ /home/sfr/next/next/drivers/char/tpm/tpm_ibmvtpm.c:632:2: note: in expansion of macro 'spin_lock_init' spin_lock_init(>rtce_lock); ^ Caused by commit 28157164b056 ("tpm: Remove useless priv field in struct tpm_vendor_specific") A ';' was missed. I added the following patch for today. From: Stephen RothwellDate: Thu, 28 Apr 2016 15:27:17 +1000 Subject: [PATCH] tpm: fix for typo in tpm/tpm_ibmvtpm.c Fixes: 28157164b056 ("tpm: Remove useless priv field in struct tpm_vendor_specific") Signed-off-by: Stephen Rothwell --- drivers/char/tpm/tpm_ibmvtpm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c index 6b22826f0e11..946025a7413b 100644 --- a/drivers/char/tpm/tpm_ibmvtpm.c +++ b/drivers/char/tpm/tpm_ibmvtpm.c @@ -627,7 +627,7 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev, crq_q->index = 0; - dev_set_drvdata(>dev, ibmvtpm) + dev_set_drvdata(>dev, ibmvtpm); spin_lock_init(>rtce_lock); -- 2.7.0 -- Cheers, Stephen Rothwell
linux-next: build failure after merge of the tpmdd tree
Hi Jarkko, After merging the tpmdd tree, today's linux-next build (powerpc allyesconfig) failed like this: In file included from /home/sfr/next/next/include/linux/rcupdate.h:38:0, from /home/sfr/next/next/include/linux/idr.h:18, from /home/sfr/next/next/include/linux/kernfs.h:14, from /home/sfr/next/next/include/linux/sysfs.h:15, from /home/sfr/next/next/include/linux/kobject.h:21, from /home/sfr/next/next/include/linux/device.h:17, from /home/sfr/next/next/include/linux/dma-mapping.h:6, from /home/sfr/next/next/drivers/char/tpm/tpm_ibmvtpm.c:18: /home/sfr/next/next/drivers/char/tpm/tpm_ibmvtpm.c: In function 'tpm_ibmvtpm_probe': /home/sfr/next/next/include/linux/spinlock.h:295:1: error: expected ';' before 'do' do { \ ^ /home/sfr/next/next/drivers/char/tpm/tpm_ibmvtpm.c:632:2: note: in expansion of macro 'spin_lock_init' spin_lock_init(>rtce_lock); ^ Caused by commit 28157164b056 ("tpm: Remove useless priv field in struct tpm_vendor_specific") A ';' was missed. I added the following patch for today. From: Stephen Rothwell Date: Thu, 28 Apr 2016 15:27:17 +1000 Subject: [PATCH] tpm: fix for typo in tpm/tpm_ibmvtpm.c Fixes: 28157164b056 ("tpm: Remove useless priv field in struct tpm_vendor_specific") Signed-off-by: Stephen Rothwell --- drivers/char/tpm/tpm_ibmvtpm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c index 6b22826f0e11..946025a7413b 100644 --- a/drivers/char/tpm/tpm_ibmvtpm.c +++ b/drivers/char/tpm/tpm_ibmvtpm.c @@ -627,7 +627,7 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev, crq_q->index = 0; - dev_set_drvdata(>dev, ibmvtpm) + dev_set_drvdata(>dev, ibmvtpm); spin_lock_init(>rtce_lock); -- 2.7.0 -- Cheers, Stephen Rothwell
Re: [Intel-gfx] [Announcement] 2016-Q1 release of XenGT - a Mediated Graphics Passthrough Solution from Intel
Hi all, We are pleased to announce another update of Intel GVT-g for Xen. Intel GVT-g is a full GPU virtualization solution with mediated pass-through, starting from 4th generation Intel Core(TM) processors with Intel Graphics processors. A virtual GPU instance is maintained for each VM, with part of performance critical resources directly assigned. The capability of running native graphics driver inside a VM, without hypervisor intervention in performance critical paths, achieves a good balance among performance, feature, and sharing capability. Xen is currently supported on Intel Processor Graphics (a.k.a. XenGT). Repositories - Kernel: https://github.com/01org/igvtg-kernel (2016q1-4.3.0 branch) Xen: https://github.com/01org/igvtg-xen (2016q1-4.6 branch) Qemu: https://github.com/01org/igvtg-qemu (2016q1-2.3.0 branch) This update consists of: -Windows 10 guest is preliminarily supported in this release. -Implemented vgtbuffer(Indirect display) feature on SKL platform. -Backward compatibility support 5th generation (Broadwell) -Increased VGT stability on SKL platform -Kernel updated from drm-intel 4.2.0 to drm-intel 4.3.0 -Xen updated from Xen 4.5.0 to Xen 4.6.0 -Qemu updated from 1.6 to 2.3 Known issues: -At least 2GB memory is suggested for VM(win7-32/64, win8.1 64) to run most 3D workloads. -Windows 7 GFX driver upgrading only works on Safe mode. -Some media decode can't work well (will be resolved in the next version Windows GFX driver). -Windows8 and later Windows fast boot is not supported, whose workaround is to disable power S3/S4 in HVM file by adding "acpi_s3=0, acpi_s4=0" -Sometimes when dom0 and guest have heavy workload, i915 in dom0 will trigger a false graphics reset. The workaround is to disable dom0 hangcheck in dom0 grub file by adding "i915.enable_hangcheck=0" Next update will be around early July, 2016. GVT-g project portal: https://01.org/igvt-g Please subscribe the mailing list: https://lists.01.org/mailman/listinfo/igvt-g More information about background, architecture and others about Intel GVT-g, can be found at: https://01.org/igvt-g https://www.usenix.org/conference/atc14/technical-sessions/presentation/tian http://events.linuxfoundation.org/sites/events/files/slides/XenGT-Xen%20Summit-v7_0.pdf http://events.linuxfoundation.org/sites/events/files/slides/XenGT-Xen%20Summit-REWRITE%203RD%20v4.pdf https://01.org/xen/blogs/srclarkx/2013/graphics-virtualization-xengt Note: The XenGT project should be considered a work in progress. As such it is not a complete product nor should it be considered one. Extra care should be taken when testing and configuring a system to use the XenGT project. -- Thanks, Jike On 01/27/2016 02:21 PM, Jike Song wrote: > Hi all, > > We are pleased to announce another update of Intel GVT-g for Xen. > > Intel GVT-g is a full GPU virtualization solution with mediated pass-through, > starting from 4th generation Intel Core(TM) processors with Intel Graphics > processors. A virtual GPU instance is maintained for each VM, with part of > performance critical resources directly assigned. The capability of running > native graphics driver inside a VM, without hypervisor intervention in > performance critical paths, achieves a good balance among performance, > feature, and sharing capability. Xen is currently supported on Intel > Processor Graphics (a.k.a. XenGT). > > Repositories > - > > Kernel: https://github.com/01org/igvtg-kernel (2015q4-4.2.0 branch) > Xen: https://github.com/01org/igvtg-xen (2015q4-4.5 branch) > Qemu: https://github.com/01org/igvtg-qemu (xengt_public2015q4 branch) > > This update consists of: > > - 6th generation Intel Core Processor (code name: Skylake) is > preliminarily supported in this release. Users could start run multiple > Windows / Linux virtual machines simultaneously, and switch display among > them. > - Backward compatibility support 4th generation Intel Core Processor > (code name: Haswell) and 5th generation Intel Core Processor (code name: > Broadwell). > - Kernel update from drm-intel 3.18.0 to drm-intel 4.2.0. > > Known issues: >- At least 2GB memory is suggested for a VM to run most 3D workloads. >- Keymap might be incorrect in guest. Config file may need to explicitly > specify "keymap='en-us'". Although it looks like the default value, earlier > we saw the problem of wrong keymap code if it is not explicitly set. >- Cannot move mouse pointer smoothly in guest by default launched by VNC > mode. Configuration file need to explicitly specify "usb=1" to enable a USB > bus, and "usbdevice='tablet'" to add pointer device using absolute > coordinates. >- Running heavy 3D workloads in multiple guests for couple of hours may > cause stability issue. >- There are still stability issues on Skylake > > >
Re: [Intel-gfx] [Announcement] 2016-Q1 release of XenGT - a Mediated Graphics Passthrough Solution from Intel
Hi all, We are pleased to announce another update of Intel GVT-g for Xen. Intel GVT-g is a full GPU virtualization solution with mediated pass-through, starting from 4th generation Intel Core(TM) processors with Intel Graphics processors. A virtual GPU instance is maintained for each VM, with part of performance critical resources directly assigned. The capability of running native graphics driver inside a VM, without hypervisor intervention in performance critical paths, achieves a good balance among performance, feature, and sharing capability. Xen is currently supported on Intel Processor Graphics (a.k.a. XenGT). Repositories - Kernel: https://github.com/01org/igvtg-kernel (2016q1-4.3.0 branch) Xen: https://github.com/01org/igvtg-xen (2016q1-4.6 branch) Qemu: https://github.com/01org/igvtg-qemu (2016q1-2.3.0 branch) This update consists of: -Windows 10 guest is preliminarily supported in this release. -Implemented vgtbuffer(Indirect display) feature on SKL platform. -Backward compatibility support 5th generation (Broadwell) -Increased VGT stability on SKL platform -Kernel updated from drm-intel 4.2.0 to drm-intel 4.3.0 -Xen updated from Xen 4.5.0 to Xen 4.6.0 -Qemu updated from 1.6 to 2.3 Known issues: -At least 2GB memory is suggested for VM(win7-32/64, win8.1 64) to run most 3D workloads. -Windows 7 GFX driver upgrading only works on Safe mode. -Some media decode can't work well (will be resolved in the next version Windows GFX driver). -Windows8 and later Windows fast boot is not supported, whose workaround is to disable power S3/S4 in HVM file by adding "acpi_s3=0, acpi_s4=0" -Sometimes when dom0 and guest have heavy workload, i915 in dom0 will trigger a false graphics reset. The workaround is to disable dom0 hangcheck in dom0 grub file by adding "i915.enable_hangcheck=0" Next update will be around early July, 2016. GVT-g project portal: https://01.org/igvt-g Please subscribe the mailing list: https://lists.01.org/mailman/listinfo/igvt-g More information about background, architecture and others about Intel GVT-g, can be found at: https://01.org/igvt-g https://www.usenix.org/conference/atc14/technical-sessions/presentation/tian http://events.linuxfoundation.org/sites/events/files/slides/XenGT-Xen%20Summit-v7_0.pdf http://events.linuxfoundation.org/sites/events/files/slides/XenGT-Xen%20Summit-REWRITE%203RD%20v4.pdf https://01.org/xen/blogs/srclarkx/2013/graphics-virtualization-xengt Note: The XenGT project should be considered a work in progress. As such it is not a complete product nor should it be considered one. Extra care should be taken when testing and configuring a system to use the XenGT project. -- Thanks, Jike On 01/27/2016 02:21 PM, Jike Song wrote: > Hi all, > > We are pleased to announce another update of Intel GVT-g for Xen. > > Intel GVT-g is a full GPU virtualization solution with mediated pass-through, > starting from 4th generation Intel Core(TM) processors with Intel Graphics > processors. A virtual GPU instance is maintained for each VM, with part of > performance critical resources directly assigned. The capability of running > native graphics driver inside a VM, without hypervisor intervention in > performance critical paths, achieves a good balance among performance, > feature, and sharing capability. Xen is currently supported on Intel > Processor Graphics (a.k.a. XenGT). > > Repositories > - > > Kernel: https://github.com/01org/igvtg-kernel (2015q4-4.2.0 branch) > Xen: https://github.com/01org/igvtg-xen (2015q4-4.5 branch) > Qemu: https://github.com/01org/igvtg-qemu (xengt_public2015q4 branch) > > This update consists of: > > - 6th generation Intel Core Processor (code name: Skylake) is > preliminarily supported in this release. Users could start run multiple > Windows / Linux virtual machines simultaneously, and switch display among > them. > - Backward compatibility support 4th generation Intel Core Processor > (code name: Haswell) and 5th generation Intel Core Processor (code name: > Broadwell). > - Kernel update from drm-intel 3.18.0 to drm-intel 4.2.0. > > Known issues: >- At least 2GB memory is suggested for a VM to run most 3D workloads. >- Keymap might be incorrect in guest. Config file may need to explicitly > specify "keymap='en-us'". Although it looks like the default value, earlier > we saw the problem of wrong keymap code if it is not explicitly set. >- Cannot move mouse pointer smoothly in guest by default launched by VNC > mode. Configuration file need to explicitly specify "usb=1" to enable a USB > bus, and "usbdevice='tablet'" to add pointer device using absolute > coordinates. >- Running heavy 3D workloads in multiple guests for couple of hours may > cause stability issue. >- There are still stability issues on Skylake > > >
Re: [PATCH perf/core v5 11/15] perf probe: Accept %sdt and %cached event name
On Thu, Apr 28, 2016 at 03:38:49AM +0900, Masami Hiramatsu wrote: > From: Masami Hiramatsu> > To improbe usability, support %[PROVIDER:]SDTEVENT format to > add new probes on SDT and cached events. > > e.g. > > # perf probe -x /lib/libc-2.17.so %lll_lock_wait_private > Added new event: > sdt_libc:lll_lock_wait_private (on %lll_lock_wait_private in > /usr/lib/libc-2.17.so) > > You can now use it in all perf tools, such as: > > perf record -e sdt_libc:lll_lock_wait_private -aR sleep 1 > > # perf probe -l | more > sdt_libc:lll_lock_wait_private (on __lll_lock_wait_private+21 >in /usr/lib/libc-2.17.so) > > > Note that this is not only for SDT events, but also normal > events with event-name. > > e.g. define "myevent" on cache (-n doesn't add the real probe) > > # perf probe -x ./perf --cache -n --add 'myevent=dso__load $params' > > Reuse the "myevent" from cache as below. > > # perf probe -x ./perf %myevent > > > TODO: > Wildcard is not supported yet. > > Signed-off-by: Masami Hiramatsu > --- > tools/perf/Documentation/perf-probe.txt |3 + > tools/perf/util/probe-event.c | 78 > ++- > tools/perf/util/probe-event.h |1 > tools/perf/util/probe-file.c|9 > 4 files changed, 69 insertions(+), 22 deletions(-) > > diff --git a/tools/perf/Documentation/perf-probe.txt > b/tools/perf/Documentation/perf-probe.txt > index 7a258e9..43523be 100644 > --- a/tools/perf/Documentation/perf-probe.txt > +++ b/tools/perf/Documentation/perf-probe.txt > @@ -151,6 +151,8 @@ Probe points are defined by following syntax. > 3) Define event based on source file with lazy pattern > [[GROUP:]EVENT=]SRC;PTN [ARG ...] > > +4) Pre-defined SDT events > + %[PROVIDER:]SDTEVENT How about changing it to: 4) Pre-defined (SDT) or cached events %EVENTNAME ? Btw, is it possible to use group name here? The SDT will use sdt_ for group name and _ for event name, right? So is it for cached events and does it support defining group name when added like below? # perf probe -x ./perf --cache --add 'mygroup:myevent=main' Thanks, Namhyung > > 'EVENT' specifies the name of new event, if omitted, it will be set the name > of the probed function. You can also specify a group name by 'GROUP', if > omitted, set 'probe' is used for kprobe and 'probe_' is used for uprobe. > Note that using existing group name can conflict with other events. > Especially, using the group name reserved for kernel modules can hide > embedded events in the > @@ -158,6 +160,7 @@ modules. > 'FUNC' specifies a probed function name, and it may have one of the > following options; '+OFFS' is the offset from function entry address in > bytes, ':RLN' is the relative-line number from function entry line, and > '%return' means that it probes function return. And ';PTN' means lazy > matching pattern (see LAZY MATCHING). Note that ';PTN' must be the end of the > probe point definition. In addition, '@SRC' specifies a source file which > has that function. > It is also possible to specify a probe point by the source line number or > lazy matching by using 'SRC:ALN' or 'SRC;PTN' syntax, where 'SRC' is the > source file path, ':ALN' is the line number and ';PTN' is the lazy matching > pattern. > 'ARG' specifies the arguments of this probe point, (see PROBE ARGUMENT). > +'SDTEVENT' and 'PROVIDER' is the pre-defined event name which is defined by > user SDT (Statically Defined Tracing) or the pre-cached probes with event > name. > > PROBE ARGUMENT > -- > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c > index b2c6a4a..1a9ea2b 100644 > --- a/tools/perf/util/probe-event.c > +++ b/tools/perf/util/probe-event.c > @@ -1199,6 +1199,34 @@ err: > return err; > } > > +static int parse_perf_probe_event_name(char **arg, struct perf_probe_event > *pev) > +{ > + char *ptr; > + > + ptr = strchr(*arg, ':'); > + if (ptr) { > + *ptr = '\0'; > + if (!is_c_func_name(*arg)) > + goto ng_name; > + pev->group = strdup(*arg); > + if (!pev->group) > + return -ENOMEM; > + *arg = ptr + 1; > + } else > + pev->group = NULL; > + if (!is_c_func_name(*arg)) { > +ng_name: > + semantic_error("%s is bad for event name -it must " > +"follow C symbol-naming rule.\n", *arg); > + return -EINVAL; > + } > + pev->event = strdup(*arg); > + if (pev->event == NULL) > + return -ENOMEM; > + > + return 0; > +} > + > /* Parse probepoint definition. */ > static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev) > { > @@ -1206,38 +1234,43 @@ static int
Re: [PATCH perf/core v5 11/15] perf probe: Accept %sdt and %cached event name
On Thu, Apr 28, 2016 at 03:38:49AM +0900, Masami Hiramatsu wrote: > From: Masami Hiramatsu > > To improbe usability, support %[PROVIDER:]SDTEVENT format to > add new probes on SDT and cached events. > > e.g. > > # perf probe -x /lib/libc-2.17.so %lll_lock_wait_private > Added new event: > sdt_libc:lll_lock_wait_private (on %lll_lock_wait_private in > /usr/lib/libc-2.17.so) > > You can now use it in all perf tools, such as: > > perf record -e sdt_libc:lll_lock_wait_private -aR sleep 1 > > # perf probe -l | more > sdt_libc:lll_lock_wait_private (on __lll_lock_wait_private+21 >in /usr/lib/libc-2.17.so) > > > Note that this is not only for SDT events, but also normal > events with event-name. > > e.g. define "myevent" on cache (-n doesn't add the real probe) > > # perf probe -x ./perf --cache -n --add 'myevent=dso__load $params' > > Reuse the "myevent" from cache as below. > > # perf probe -x ./perf %myevent > > > TODO: > Wildcard is not supported yet. > > Signed-off-by: Masami Hiramatsu > --- > tools/perf/Documentation/perf-probe.txt |3 + > tools/perf/util/probe-event.c | 78 > ++- > tools/perf/util/probe-event.h |1 > tools/perf/util/probe-file.c|9 > 4 files changed, 69 insertions(+), 22 deletions(-) > > diff --git a/tools/perf/Documentation/perf-probe.txt > b/tools/perf/Documentation/perf-probe.txt > index 7a258e9..43523be 100644 > --- a/tools/perf/Documentation/perf-probe.txt > +++ b/tools/perf/Documentation/perf-probe.txt > @@ -151,6 +151,8 @@ Probe points are defined by following syntax. > 3) Define event based on source file with lazy pattern > [[GROUP:]EVENT=]SRC;PTN [ARG ...] > > +4) Pre-defined SDT events > + %[PROVIDER:]SDTEVENT How about changing it to: 4) Pre-defined (SDT) or cached events %EVENTNAME ? Btw, is it possible to use group name here? The SDT will use sdt_ for group name and _ for event name, right? So is it for cached events and does it support defining group name when added like below? # perf probe -x ./perf --cache --add 'mygroup:myevent=main' Thanks, Namhyung > > 'EVENT' specifies the name of new event, if omitted, it will be set the name > of the probed function. You can also specify a group name by 'GROUP', if > omitted, set 'probe' is used for kprobe and 'probe_' is used for uprobe. > Note that using existing group name can conflict with other events. > Especially, using the group name reserved for kernel modules can hide > embedded events in the > @@ -158,6 +160,7 @@ modules. > 'FUNC' specifies a probed function name, and it may have one of the > following options; '+OFFS' is the offset from function entry address in > bytes, ':RLN' is the relative-line number from function entry line, and > '%return' means that it probes function return. And ';PTN' means lazy > matching pattern (see LAZY MATCHING). Note that ';PTN' must be the end of the > probe point definition. In addition, '@SRC' specifies a source file which > has that function. > It is also possible to specify a probe point by the source line number or > lazy matching by using 'SRC:ALN' or 'SRC;PTN' syntax, where 'SRC' is the > source file path, ':ALN' is the line number and ';PTN' is the lazy matching > pattern. > 'ARG' specifies the arguments of this probe point, (see PROBE ARGUMENT). > +'SDTEVENT' and 'PROVIDER' is the pre-defined event name which is defined by > user SDT (Statically Defined Tracing) or the pre-cached probes with event > name. > > PROBE ARGUMENT > -- > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c > index b2c6a4a..1a9ea2b 100644 > --- a/tools/perf/util/probe-event.c > +++ b/tools/perf/util/probe-event.c > @@ -1199,6 +1199,34 @@ err: > return err; > } > > +static int parse_perf_probe_event_name(char **arg, struct perf_probe_event > *pev) > +{ > + char *ptr; > + > + ptr = strchr(*arg, ':'); > + if (ptr) { > + *ptr = '\0'; > + if (!is_c_func_name(*arg)) > + goto ng_name; > + pev->group = strdup(*arg); > + if (!pev->group) > + return -ENOMEM; > + *arg = ptr + 1; > + } else > + pev->group = NULL; > + if (!is_c_func_name(*arg)) { > +ng_name: > + semantic_error("%s is bad for event name -it must " > +"follow C symbol-naming rule.\n", *arg); > + return -EINVAL; > + } > + pev->event = strdup(*arg); > + if (pev->event == NULL) > + return -ENOMEM; > + > + return 0; > +} > + > /* Parse probepoint definition. */ > static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev) > { > @@ -1206,38 +1234,43 @@ static int parse_perf_probe_point(char *arg, struct > perf_probe_event *pev) >
Re: [PATCH v2 2/2] phy: Group vendor specific phy drivers
Hi Kishon, On Thu, Apr 28, 2016 at 10:28 AM, Kishon Vijay Abraham Iwrote: > Hi, > > On Tuesday 26 April 2016 11:36 AM, Vivek Gautam wrote: >> Hi Kishon, >> >> >> On Wed, Apr 6, 2016 at 7:37 PM, Vivek Gautam >> wrote: >>> Adding vendor specific directories in phy to group >>> phy drivers under their respective vendor umbrella. >>> >>> Also updated the MAINTAINERS file to reflect the correct >>> directory structure for phy drivers. >>> >>> Signed-off-by: Vivek Gautam >>> Acked-by: Heiko Stuebner >>> Acked-by: Viresh Kumar >>> --- >>> >>> Changes from v1: >>> - Updated the MAINTAINERS file to reflect the same change >>>in directory structure. >>> - Removed PHY_PLAT config as pointed out by Kishon. >>>So we don't require the second patch in the v1 of this series: >>>[PATCH 2/2] arm: mach-spear: Enable PHY_PLAT to meet dependency >>> - Renamed sunxi --> allwinner; rcar --> renesas. >>> - Fixed error coming with qcom Makefile. >> >> Does this change looks okay now ? >> I think you must be planning to take this in after 4.7 rc1 is out, isn't it ? > > I'm not sure. I'm thinking which should be the right way to group the phy > drivers, by vendors or by type (usb, pcie, etc..). I'm more inclined to group > it by type. I guess there are atleast couple of phy drivers (PIPE and ST's MiPHY) that have multiple roles - USB or PCIe or SATA. So grouping such drivers in one of the phy-types will be tricky, WDUT > > Thanks > Kishon -- Best Regards Vivek Gautam Samsung R Institute, Bangalore India
Re: [PATCH v2 2/2] phy: Group vendor specific phy drivers
Hi Kishon, On Thu, Apr 28, 2016 at 10:28 AM, Kishon Vijay Abraham I wrote: > Hi, > > On Tuesday 26 April 2016 11:36 AM, Vivek Gautam wrote: >> Hi Kishon, >> >> >> On Wed, Apr 6, 2016 at 7:37 PM, Vivek Gautam >> wrote: >>> Adding vendor specific directories in phy to group >>> phy drivers under their respective vendor umbrella. >>> >>> Also updated the MAINTAINERS file to reflect the correct >>> directory structure for phy drivers. >>> >>> Signed-off-by: Vivek Gautam >>> Acked-by: Heiko Stuebner >>> Acked-by: Viresh Kumar >>> --- >>> >>> Changes from v1: >>> - Updated the MAINTAINERS file to reflect the same change >>>in directory structure. >>> - Removed PHY_PLAT config as pointed out by Kishon. >>>So we don't require the second patch in the v1 of this series: >>>[PATCH 2/2] arm: mach-spear: Enable PHY_PLAT to meet dependency >>> - Renamed sunxi --> allwinner; rcar --> renesas. >>> - Fixed error coming with qcom Makefile. >> >> Does this change looks okay now ? >> I think you must be planning to take this in after 4.7 rc1 is out, isn't it ? > > I'm not sure. I'm thinking which should be the right way to group the phy > drivers, by vendors or by type (usb, pcie, etc..). I'm more inclined to group > it by type. I guess there are atleast couple of phy drivers (PIPE and ST's MiPHY) that have multiple roles - USB or PCIe or SATA. So grouping such drivers in one of the phy-types will be tricky, WDUT > > Thanks > Kishon -- Best Regards Vivek Gautam Samsung R Institute, Bangalore India
Re: [LKP] [lkp] [mm, oom] faad2185f4: vm-scalability.throughput -11.8% regression
On Wed, Apr 27, 2016 at 11:17:19AM +0200, Michal Hocko wrote: > On Wed 27-04-16 16:44:31, Huang, Ying wrote: > > Michal Hockowrites: > > > > > On Wed 27-04-16 16:20:43, Huang, Ying wrote: > > >> Michal Hocko writes: > > >> > > >> > On Wed 27-04-16 11:15:56, kernel test robot wrote: > > >> >> FYI, we noticed vm-scalability.throughput -11.8% regression with the > > >> >> following commit: > > >> > > > >> > Could you be more specific what the test does please? > > >> > > >> The sub-testcase of vm-scalability is swap-w-rand. An RAM emulated pmem > > >> device is used as a swap device, and a test program will allocate/write > > >> anonymous memory randomly to exercise page allocation, reclaiming, and > > >> swapping in code path. > > > > > > Can I download the test with the setup to play with this? > > > > There are reproduce steps in the original report email. > > > > To reproduce: > > > > git clone > > git://git.kernel.org/pub/scm/linux/kernel/git/wfg/lkp-tests.git > > cd lkp-tests > > bin/lkp install job.yaml # job file is attached in this email > > bin/lkp run job.yaml > > > > > > The job.yaml and kconfig file are attached in the original report email. > > Thanks for the instructions. My bad I have overlooked that in the > initial email. I have checked the configuration file and it seems rather > hardcoded for a particular HW. It expects a machine with 128G and > reserves 96G!4G which might lead to different amount of memory in the > end depending on the particular memory layout. Indeed, the job file needs manual change. The attached job file is the one we used on the test machine. > > Before I go and try to recreate a similar setup, how stable are the > results from this test. Random access pattern sounds like rather > volatile to be consider for a throughput test. Or is there any other > side effect I am missing and something fails which didn't use to > previously. I have the same doubt too, but the results look really stable(only for commit 0da9597ac9c0, see below for more explanation). We did 8 runs for this report and the standard deviation(represented by the %stddev shown in the original report) is used to show exactly this. I just checked the results again and found that the 8 runs for your commit faad2185f482 all OOMed, only 1 of them is able to finish the test before the OOM occur and got a throughput value of 38653. The source code for this test is here: https://git.kernel.org/cgit/linux/kernel/git/wfg/vm-scalability.git/tree/usemem.c And it's started as: ./usemem --runtime 300 -n 16 --random 6368538624 which means to fork 16 processes, each dealing with 6GiB around data. By dealing here, I mean the process each will mmap an anonymous region of 6GiB size and then write data to that area at random place, thus will trigger swapouts and swapins after the memory is used up(since the system has 128GiB memory and 96GiB is used by the pmem driver as swap space, the memory will be used up after a little while). So I guess the question here is, after the OOM rework, is the OOM expected for such a case? If so, then we can ignore this report.
Re: [LKP] [lkp] [mm, oom] faad2185f4: vm-scalability.throughput -11.8% regression
On Wed, Apr 27, 2016 at 11:17:19AM +0200, Michal Hocko wrote: > On Wed 27-04-16 16:44:31, Huang, Ying wrote: > > Michal Hocko writes: > > > > > On Wed 27-04-16 16:20:43, Huang, Ying wrote: > > >> Michal Hocko writes: > > >> > > >> > On Wed 27-04-16 11:15:56, kernel test robot wrote: > > >> >> FYI, we noticed vm-scalability.throughput -11.8% regression with the > > >> >> following commit: > > >> > > > >> > Could you be more specific what the test does please? > > >> > > >> The sub-testcase of vm-scalability is swap-w-rand. An RAM emulated pmem > > >> device is used as a swap device, and a test program will allocate/write > > >> anonymous memory randomly to exercise page allocation, reclaiming, and > > >> swapping in code path. > > > > > > Can I download the test with the setup to play with this? > > > > There are reproduce steps in the original report email. > > > > To reproduce: > > > > git clone > > git://git.kernel.org/pub/scm/linux/kernel/git/wfg/lkp-tests.git > > cd lkp-tests > > bin/lkp install job.yaml # job file is attached in this email > > bin/lkp run job.yaml > > > > > > The job.yaml and kconfig file are attached in the original report email. > > Thanks for the instructions. My bad I have overlooked that in the > initial email. I have checked the configuration file and it seems rather > hardcoded for a particular HW. It expects a machine with 128G and > reserves 96G!4G which might lead to different amount of memory in the > end depending on the particular memory layout. Indeed, the job file needs manual change. The attached job file is the one we used on the test machine. > > Before I go and try to recreate a similar setup, how stable are the > results from this test. Random access pattern sounds like rather > volatile to be consider for a throughput test. Or is there any other > side effect I am missing and something fails which didn't use to > previously. I have the same doubt too, but the results look really stable(only for commit 0da9597ac9c0, see below for more explanation). We did 8 runs for this report and the standard deviation(represented by the %stddev shown in the original report) is used to show exactly this. I just checked the results again and found that the 8 runs for your commit faad2185f482 all OOMed, only 1 of them is able to finish the test before the OOM occur and got a throughput value of 38653. The source code for this test is here: https://git.kernel.org/cgit/linux/kernel/git/wfg/vm-scalability.git/tree/usemem.c And it's started as: ./usemem --runtime 300 -n 16 --random 6368538624 which means to fork 16 processes, each dealing with 6GiB around data. By dealing here, I mean the process each will mmap an anonymous region of 6GiB size and then write data to that area at random place, thus will trigger swapouts and swapins after the memory is used up(since the system has 128GiB memory and 96GiB is used by the pmem driver as swap space, the memory will be used up after a little while). So I guess the question here is, after the OOM rework, is the OOM expected for such a case? If so, then we can ignore this report.
Re: [PATCH v2 2/2] phy: Group vendor specific phy drivers
Hi, On Tuesday 26 April 2016 11:36 AM, Vivek Gautam wrote: > Hi Kishon, > > > On Wed, Apr 6, 2016 at 7:37 PM, Vivek Gautamwrote: >> Adding vendor specific directories in phy to group >> phy drivers under their respective vendor umbrella. >> >> Also updated the MAINTAINERS file to reflect the correct >> directory structure for phy drivers. >> >> Signed-off-by: Vivek Gautam >> Acked-by: Heiko Stuebner >> Acked-by: Viresh Kumar >> --- >> >> Changes from v1: >> - Updated the MAINTAINERS file to reflect the same change >>in directory structure. >> - Removed PHY_PLAT config as pointed out by Kishon. >>So we don't require the second patch in the v1 of this series: >>[PATCH 2/2] arm: mach-spear: Enable PHY_PLAT to meet dependency >> - Renamed sunxi --> allwinner; rcar --> renesas. >> - Fixed error coming with qcom Makefile. > > Does this change looks okay now ? > I think you must be planning to take this in after 4.7 rc1 is out, isn't it ? I'm not sure. I'm thinking which should be the right way to group the phy drivers, by vendors or by type (usb, pcie, etc..). I'm more inclined to group it by type. Thanks Kishon
Re: [PATCH v2 2/2] phy: Group vendor specific phy drivers
Hi, On Tuesday 26 April 2016 11:36 AM, Vivek Gautam wrote: > Hi Kishon, > > > On Wed, Apr 6, 2016 at 7:37 PM, Vivek Gautam wrote: >> Adding vendor specific directories in phy to group >> phy drivers under their respective vendor umbrella. >> >> Also updated the MAINTAINERS file to reflect the correct >> directory structure for phy drivers. >> >> Signed-off-by: Vivek Gautam >> Acked-by: Heiko Stuebner >> Acked-by: Viresh Kumar >> --- >> >> Changes from v1: >> - Updated the MAINTAINERS file to reflect the same change >>in directory structure. >> - Removed PHY_PLAT config as pointed out by Kishon. >>So we don't require the second patch in the v1 of this series: >>[PATCH 2/2] arm: mach-spear: Enable PHY_PLAT to meet dependency >> - Renamed sunxi --> allwinner; rcar --> renesas. >> - Fixed error coming with qcom Makefile. > > Does this change looks okay now ? > I think you must be planning to take this in after 4.7 rc1 is out, isn't it ? I'm not sure. I'm thinking which should be the right way to group the phy drivers, by vendors or by type (usb, pcie, etc..). I'm more inclined to group it by type. Thanks Kishon
Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource
On Fri, Apr 22, 2016 at 1:49 PM, Bjorn Helgaaswrote: > [+cc Ben, Michael] > I'm kind of confused here. There are two ways to mmap PCI BARs: > > /proc/bus/pci/00/02.0 (proc_bus_pci_mmap()): > all BARs in one file; MEM/IO determined by ioctl() > mmap offset is a CPU physical address in the PCI resource > > /sys/devices/pci:00/:00:02.0/resource0 (pci_mmap_resource()): > one file per BAR; MEM/IO determined by BAR type > mmap offset is between 0 and BAR size > > Both proc_bus_pci_mmap() and pci_mmap_resource() validate the > requested area with pci_mmap_fits() before calling pci_mmap_page_range(). > > In the proc_bus_pci_mmap() path, the offset in vma->vm_pgoff must be > within the pdev->resource[], so the user must be supplying a CPU > physical address (not an address obtained from pci_resource_to_user()). > That vma->vm_pgoff is passed unchanged to pci_mmap_page_range(). > > In the pci_mmap_resource() path, vma->vm_pgoff must be between 0 and > the BAR size. Then we add in the pci_resource_to_user() information > before passing it to pci_mmap_page_range(). The comment in > pci_mmap_resource() says pci_mmap_page_range() expects a "user > visible" address, but I don't really believe that based on how > proc_bus_pci_mmap() works. > > Do both proc_bus_pci_mmap() and pci_mmap_resource() work on sparc? > It looks like they call pci_mmap_page_range() with different > assumptions, so I don't see how they can both work. for sysfs path: in pci_mmap_resource pci_resource_to_user(pdev, i, res, , ); vma->vm_pgoff += start >> PAGE_SHIFT; then call pci_mmap_page_range() the fit checking in pci_mmap_fits(), pci_start = (mmap_api == PCI_MMAP_PROCFS) ? pci_resource_start(pdev, resno) >> PAGE_SHIFT : 0; if (start >= pci_start && start < pci_start + size && start + nr <= pci_start + size) so proc fs assume resource_start for vm_pgoff ? but current pci_mmap_page_range want to use bus address start aka BAR value. and we have /* pci_mmap_page_range() expects the same kind of entry as coming * from /proc/bus/pci/ which is a "user visible" value. If this is * different from the resource itself, arch will do necessary fixup. */ so we need to fix pci_mmap_fits(), please check if it is ok, will submit it as separated one. diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c index d319a9c..3768c6a 100644 --- a/drivers/pci/pci-sysfs.c +++ b/drivers/pci/pci-sysfs.c @@ -969,15 +969,20 @@ void pci_remove_legacy_files(struct pci_bus *b) int pci_mmap_fits(struct pci_dev *pdev, int resno, struct vm_area_struct *vma, enum pci_mmap_api mmap_api) { - unsigned long nr, start, size, pci_start; + unsigned long nr, start, size; + resource_size_t pci_start = 0, pci_end; if (pci_resource_len(pdev, resno) == 0) return 0; nr = vma_pages(vma); start = vma->vm_pgoff; size = ((pci_resource_len(pdev, resno) - 1) >> PAGE_SHIFT) + 1; - pci_start = (mmap_api == PCI_MMAP_PROCFS) ? - pci_resource_start(pdev, resno) >> PAGE_SHIFT : 0; + if (mmap_api == PCI_MMAP_PROCFS) { + struct resource *res = >resource[resno]; + + pci_resource_to_user(pdev, resno, res, _start, _end); + pci_start >>= PAGE_SHIFT; + } if (start >= pci_start && start < pci_start + size && start + nr <= pci_start + size) return 1;
Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource
On Fri, Apr 22, 2016 at 1:49 PM, Bjorn Helgaas wrote: > [+cc Ben, Michael] > I'm kind of confused here. There are two ways to mmap PCI BARs: > > /proc/bus/pci/00/02.0 (proc_bus_pci_mmap()): > all BARs in one file; MEM/IO determined by ioctl() > mmap offset is a CPU physical address in the PCI resource > > /sys/devices/pci:00/:00:02.0/resource0 (pci_mmap_resource()): > one file per BAR; MEM/IO determined by BAR type > mmap offset is between 0 and BAR size > > Both proc_bus_pci_mmap() and pci_mmap_resource() validate the > requested area with pci_mmap_fits() before calling pci_mmap_page_range(). > > In the proc_bus_pci_mmap() path, the offset in vma->vm_pgoff must be > within the pdev->resource[], so the user must be supplying a CPU > physical address (not an address obtained from pci_resource_to_user()). > That vma->vm_pgoff is passed unchanged to pci_mmap_page_range(). > > In the pci_mmap_resource() path, vma->vm_pgoff must be between 0 and > the BAR size. Then we add in the pci_resource_to_user() information > before passing it to pci_mmap_page_range(). The comment in > pci_mmap_resource() says pci_mmap_page_range() expects a "user > visible" address, but I don't really believe that based on how > proc_bus_pci_mmap() works. > > Do both proc_bus_pci_mmap() and pci_mmap_resource() work on sparc? > It looks like they call pci_mmap_page_range() with different > assumptions, so I don't see how they can both work. for sysfs path: in pci_mmap_resource pci_resource_to_user(pdev, i, res, , ); vma->vm_pgoff += start >> PAGE_SHIFT; then call pci_mmap_page_range() the fit checking in pci_mmap_fits(), pci_start = (mmap_api == PCI_MMAP_PROCFS) ? pci_resource_start(pdev, resno) >> PAGE_SHIFT : 0; if (start >= pci_start && start < pci_start + size && start + nr <= pci_start + size) so proc fs assume resource_start for vm_pgoff ? but current pci_mmap_page_range want to use bus address start aka BAR value. and we have /* pci_mmap_page_range() expects the same kind of entry as coming * from /proc/bus/pci/ which is a "user visible" value. If this is * different from the resource itself, arch will do necessary fixup. */ so we need to fix pci_mmap_fits(), please check if it is ok, will submit it as separated one. diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c index d319a9c..3768c6a 100644 --- a/drivers/pci/pci-sysfs.c +++ b/drivers/pci/pci-sysfs.c @@ -969,15 +969,20 @@ void pci_remove_legacy_files(struct pci_bus *b) int pci_mmap_fits(struct pci_dev *pdev, int resno, struct vm_area_struct *vma, enum pci_mmap_api mmap_api) { - unsigned long nr, start, size, pci_start; + unsigned long nr, start, size; + resource_size_t pci_start = 0, pci_end; if (pci_resource_len(pdev, resno) == 0) return 0; nr = vma_pages(vma); start = vma->vm_pgoff; size = ((pci_resource_len(pdev, resno) - 1) >> PAGE_SHIFT) + 1; - pci_start = (mmap_api == PCI_MMAP_PROCFS) ? - pci_resource_start(pdev, resno) >> PAGE_SHIFT : 0; + if (mmap_api == PCI_MMAP_PROCFS) { + struct resource *res = >resource[resno]; + + pci_resource_to_user(pdev, resno, res, _start, _end); + pci_start >>= PAGE_SHIFT; + } if (start >= pci_start && start < pci_start + size && start + nr <= pci_start + size) return 1;
Re: Misleading hint to select CONFIG_PERF_EVENTS if driver sets PERF_PMU_CAP_NO_INTERRUPT
On Wednesday 27 April 2016 08:43 PM, Lada Trimasova wrote: > On Tue, 2016-04-26 at 12:42 +, Vineet Gupta wrote: > > On Friday 22 April 2016 06:56 PM, Lada Trimasova wrote: > I think what we have now is sufficient - but u seem to want a prettier > failure output. > > Anyhow, this print is coming from util/evsel.c: perf_evsel__open_strerror() > At the very least you want another entry in switch case for ENOTSUPP and then > check if event was sampling one ( > evsel->attr.sample_period) - use that as a hint for saying sampling events not > supported. > > > > > ENOTSUPP is not visible for user programs. So it's impossible to add this > entry > > to mentioned switch. > > I think that there is no good way to make error message more understandable > > without breaking existing api. I meant EOPNOTSUPP. Any errno value has to be visible to user programs otherwise there is no point defining it.
Re: Misleading hint to select CONFIG_PERF_EVENTS if driver sets PERF_PMU_CAP_NO_INTERRUPT
On Wednesday 27 April 2016 08:43 PM, Lada Trimasova wrote: > On Tue, 2016-04-26 at 12:42 +, Vineet Gupta wrote: > > On Friday 22 April 2016 06:56 PM, Lada Trimasova wrote: > I think what we have now is sufficient - but u seem to want a prettier > failure output. > > Anyhow, this print is coming from util/evsel.c: perf_evsel__open_strerror() > At the very least you want another entry in switch case for ENOTSUPP and then > check if event was sampling one ( > evsel->attr.sample_period) - use that as a hint for saying sampling events not > supported. > > > > > ENOTSUPP is not visible for user programs. So it's impossible to add this > entry > > to mentioned switch. > > I think that there is no good way to make error message more understandable > > without breaking existing api. I meant EOPNOTSUPP. Any errno value has to be visible to user programs otherwise there is no point defining it.
Re: [PATCH 2/2] ARC: [axs10x] Specify reserved memory for frame buffer
On Wednesday 27 April 2016 08:05 PM, Alexey Brodkin wrote: > Allocation of a frame buffer memory in a special memory region > allows bypassing of so-called IO Coherency aperture > which is typically set as a range 0x8z-0xAz. > > I.e. all data traffic to PGU bypasses IO Coherency block > and saves its bandwidth for other peripherals. > > Even though for AXS101 (which sorts ARC770 CPU) IOC is not > an option for a sake of keeping one DT description for the > base-board (axs10x_mb.dtsi) we're still defining reserved > memory location in the very end of DDR. > > Signed-off-by: Alexey Brodkin> Cc: devicet...@vger.kernel.org > --- > arch/arc/boot/dts/axc001.dtsi | 20 +++- > arch/arc/boot/dts/axc003.dtsi | 14 ++ > arch/arc/boot/dts/axc003_idu.dtsi | 14 ++ > arch/arc/boot/dts/axs10x_mb.dtsi | 2 +- > 4 files changed, 48 insertions(+), 2 deletions(-) > > diff --git a/arch/arc/boot/dts/axc001.dtsi b/arch/arc/boot/dts/axc001.dtsi > index 420dcfd..ae6162d 100644 > --- a/arch/arc/boot/dts/axc001.dtsi > +++ b/arch/arc/boot/dts/axc001.dtsi > @@ -95,6 +95,24 @@ > #size-cells = <1>; > ranges = <0x 0x8000 0x4000>; > device_type = "memory"; > - reg = <0x8000 0x2000>; /* 512MiB */ > + reg = <0x8000 0x1f00>; /* 512 - 16 MiB */ Is 16MB fixed size or is this a function of display resolution / density etc. > + }; > + > + reserved-memory { > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; > + /* > + * We just move frame buffer area to the very end of > + * available DDR. And even though in case of ARC770 there's > + * no strict requirement for a frame-buffer to be in any > + * particular location it allows us to use the same > + * base board's DT node for ARC PGU as for ARc HS38. > + */ > + frame_buffer: frame_buffer@9f00 { > + compatible = "shared-dma-pool"; > + reg = <0x9f00 0x100>; > + no-map; > + }; > }; > }; > diff --git a/arch/arc/boot/dts/axc003.dtsi b/arch/arc/boot/dts/axc003.dtsi > index f90fadf..c7a95c2 100644 > --- a/arch/arc/boot/dts/axc003.dtsi > +++ b/arch/arc/boot/dts/axc003.dtsi > @@ -100,4 +100,18 @@ > device_type = "memory"; > reg = <0x8000 0x2000>; /* 512MiB */ > }; > + > + reserved-memory { > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; > + /* > + * Move frame buffer out of IOC aperture (0x8z-0xAz). > + */ > + frame_buffer: frame_buffer@a000 { > + compatible = "shared-dma-pool"; > + reg = <0xa000 0x100>; Can this be made a bit more future safe. AXS103 has 1 GB of DDR while kernel currently only uses 512M. Once we increase that, this will need fixing too. Better to make this as far possible. Note that the IOC start alignment needs to follow max(4k, size). What will be maximum size of frame buffer - 16M always ! Same for the idu DT below ! > + no-map; > + }; > + }; > }; > diff --git a/arch/arc/boot/dts/axc003_idu.dtsi > b/arch/arc/boot/dts/axc003_idu.dtsi > index 06a9f29..929ec8c 100644 > --- a/arch/arc/boot/dts/axc003_idu.dtsi > +++ b/arch/arc/boot/dts/axc003_idu.dtsi > @@ -123,4 +123,18 @@ > device_type = "memory"; > reg = <0x8000 0x2000>; /* 512MiB */ > }; > + > + reserved-memory { > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; > + /* > + * Move frame buffer out of IOC aperture (0x8z-0xAz). > + */ > + frame_buffer: frame_buffer@a000 { > + compatible = "shared-dma-pool"; > + reg = <0xa000 0x100>; > + no-map; > + }; > + }; > }; > diff --git a/arch/arc/boot/dts/axs10x_mb.dtsi > b/arch/arc/boot/dts/axs10x_mb.dtsi > index 823f15c..64b063d 100644 > --- a/arch/arc/boot/dts/axs10x_mb.dtsi > +++ b/arch/arc/boot/dts/axs10x_mb.dtsi > @@ -283,7 +283,7 @@ > encoder-slave = <>; > clocks = <>; > clock-names = "pxlclk"; > - > + memory-region = <_buffer>; > port { > pgu_output: endpoint { > remote-endpoint = <_input>; >
Re: [PATCH 2/2] ARC: [axs10x] Specify reserved memory for frame buffer
On Wednesday 27 April 2016 08:05 PM, Alexey Brodkin wrote: > Allocation of a frame buffer memory in a special memory region > allows bypassing of so-called IO Coherency aperture > which is typically set as a range 0x8z-0xAz. > > I.e. all data traffic to PGU bypasses IO Coherency block > and saves its bandwidth for other peripherals. > > Even though for AXS101 (which sorts ARC770 CPU) IOC is not > an option for a sake of keeping one DT description for the > base-board (axs10x_mb.dtsi) we're still defining reserved > memory location in the very end of DDR. > > Signed-off-by: Alexey Brodkin > Cc: devicet...@vger.kernel.org > --- > arch/arc/boot/dts/axc001.dtsi | 20 +++- > arch/arc/boot/dts/axc003.dtsi | 14 ++ > arch/arc/boot/dts/axc003_idu.dtsi | 14 ++ > arch/arc/boot/dts/axs10x_mb.dtsi | 2 +- > 4 files changed, 48 insertions(+), 2 deletions(-) > > diff --git a/arch/arc/boot/dts/axc001.dtsi b/arch/arc/boot/dts/axc001.dtsi > index 420dcfd..ae6162d 100644 > --- a/arch/arc/boot/dts/axc001.dtsi > +++ b/arch/arc/boot/dts/axc001.dtsi > @@ -95,6 +95,24 @@ > #size-cells = <1>; > ranges = <0x 0x8000 0x4000>; > device_type = "memory"; > - reg = <0x8000 0x2000>; /* 512MiB */ > + reg = <0x8000 0x1f00>; /* 512 - 16 MiB */ Is 16MB fixed size or is this a function of display resolution / density etc. > + }; > + > + reserved-memory { > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; > + /* > + * We just move frame buffer area to the very end of > + * available DDR. And even though in case of ARC770 there's > + * no strict requirement for a frame-buffer to be in any > + * particular location it allows us to use the same > + * base board's DT node for ARC PGU as for ARc HS38. > + */ > + frame_buffer: frame_buffer@9f00 { > + compatible = "shared-dma-pool"; > + reg = <0x9f00 0x100>; > + no-map; > + }; > }; > }; > diff --git a/arch/arc/boot/dts/axc003.dtsi b/arch/arc/boot/dts/axc003.dtsi > index f90fadf..c7a95c2 100644 > --- a/arch/arc/boot/dts/axc003.dtsi > +++ b/arch/arc/boot/dts/axc003.dtsi > @@ -100,4 +100,18 @@ > device_type = "memory"; > reg = <0x8000 0x2000>; /* 512MiB */ > }; > + > + reserved-memory { > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; > + /* > + * Move frame buffer out of IOC aperture (0x8z-0xAz). > + */ > + frame_buffer: frame_buffer@a000 { > + compatible = "shared-dma-pool"; > + reg = <0xa000 0x100>; Can this be made a bit more future safe. AXS103 has 1 GB of DDR while kernel currently only uses 512M. Once we increase that, this will need fixing too. Better to make this as far possible. Note that the IOC start alignment needs to follow max(4k, size). What will be maximum size of frame buffer - 16M always ! Same for the idu DT below ! > + no-map; > + }; > + }; > }; > diff --git a/arch/arc/boot/dts/axc003_idu.dtsi > b/arch/arc/boot/dts/axc003_idu.dtsi > index 06a9f29..929ec8c 100644 > --- a/arch/arc/boot/dts/axc003_idu.dtsi > +++ b/arch/arc/boot/dts/axc003_idu.dtsi > @@ -123,4 +123,18 @@ > device_type = "memory"; > reg = <0x8000 0x2000>; /* 512MiB */ > }; > + > + reserved-memory { > + #address-cells = <1>; > + #size-cells = <1>; > + ranges; > + /* > + * Move frame buffer out of IOC aperture (0x8z-0xAz). > + */ > + frame_buffer: frame_buffer@a000 { > + compatible = "shared-dma-pool"; > + reg = <0xa000 0x100>; > + no-map; > + }; > + }; > }; > diff --git a/arch/arc/boot/dts/axs10x_mb.dtsi > b/arch/arc/boot/dts/axs10x_mb.dtsi > index 823f15c..64b063d 100644 > --- a/arch/arc/boot/dts/axs10x_mb.dtsi > +++ b/arch/arc/boot/dts/axs10x_mb.dtsi > @@ -283,7 +283,7 @@ > encoder-slave = <>; > clocks = <>; > clock-names = "pxlclk"; > - > + memory-region = <_buffer>; > port { > pgu_output: endpoint { > remote-endpoint = <_input>; >
RFC tips-for-maintainers.txt (was Re: [GIT PULL] platform-drivers-x86 for 4.6-3)
On Wed, 2016-04-27 at 09:57 -0700, Darren Hart wrote: > On Wed, Apr 27, 2016 at 09:08:01AM -0700, Linus Torvalds wrote: > > On Tue, Apr 26, 2016 at 10:02 PM, Darren Hartwrote: > > > > > > Found myself not wanting to send a one patch pull request, but not > > > wanting to > > > wait until RC6 and possibly miss 4.6. > > > > > > Do you have a preference during the RC cycle in terms of balance between > > > patch > > > count and frequency for a small subsystem like platform-driver-x86? > > > > Once a week like this is fine, even if it's just a single trivial > > one-liner change. ... > Very helpful, thank you Linus. I believe I just inherited a TODO to find the > right spot in the documentation to record this. Actually I've been meaning to do that for ages, and I have a few more collected in my notes. How do these look? Git pulls should start "[GIT PULL]": http://lkml.kernel.org/r/CA+55aFxURVwV4NZcPsVzQ-Ui9Nmn2vdXVw==s5rkhxc1y4b...@mail.gmail.com just for your information: this pull request doesn't show up with my normal merge-window search pattern of "git pull", because you never actually say "pull" anywhere. Please use a subject with "[GIT PULL]" prefix (preferred), or just change the body of the email to have a "please pull" in it or something. I get too much email, and particularly during the merge window it really helps if I can keep my pull emails separate from other stuff by just having them match a simple pattern. Linus generally likes to see merge conflicts himself, for subtle conflicts you can provide a separate tree with the result of your merge: http://lkml.kernel.org/r/ca+55afwm2e9stvtyjrnpjh5rszafbxbj5ffykyi730ojon+...@mail.gmail.com On Sat, Sep 5, 2015 at 2:37 AM, Neil Brown wrote: > > Please pull these updates. I've already merged with the 'block' tree > to resolve a few simple conflicts. So for the future, I actually prefer to see and handle the conflicts myself. I really just prefer knowing what's going on, and merge conflicts are an indication of cross-maintainer issues which are *exactly* the kinds of things I want to be aware of. However, in this case I was "ok, I've already done several other merge resolutions with the wbole damn bio_endio error handling changes", so I felt I was aware enough about how that ended up being a cross-subsystem conflict, and just took your pre-merged version. If you feel that the conflicts are particularly subtle, or just generally worry about the merge, or just because you want to do some merge-testing, what some people end up doing is to send me their unmerged branch, and then send me a separate ".. and here's the merge I did". I'll then do the merge myself anyway, but then after doing the merge I'll switch to a temporary testing branch and re-do the merge with the pre-merged state just to verify. Generally the end result is identical, but when it isn't, that's actually usually interesting (sometimes it's just a ordering difference, but sometimes it's a merge error - and so far I think most merge errors have come from sub-maintainers, for the simple reason that they generally aren't as used to merging as I am - so even if they know the code better, I sometimes catch merge gotcha's better). Don't rebase your tree between linux-next and requesting a pull unless you absolutely must: http://lkml.kernel.org/r/CA+55aFyrep8hMApzYA4DyFgEoHc8o8KATTV=jqf6-fgtlsq...@mail.gmail.com Just stop [rebasing]. I'll handle the merge issues. If there are complicated merge problems that you really worry about, what you can do is (a) make a test-merge just to check (b) do NOT send me that test-merge (c) as part of the pull request, tell me that "there's a conflict because xyz, and this is how we think it should be handled". ... There are valid reasons to rebase, but those are along the line of "we messed up catastrophically, and we _have_ to clean up the history". A simple merge conflict due to a trivial duplicated commit is not a reason. Linus likes signed tags with a short blurb: http://lkml.kernel.org/r/CA+55aFwa9b5=aykxnkfcaqn27pfy8gchuhck+pkrqcfgn6f...@mail.gmail.com .. the above is a tag, but it's not signed, nor even annotated. So it has no message about what the fixes are in it, it's just the plain SHA1 id. Now, I don't really require signed tags from kernel.org, but it would still be nice. And I *do* want a little blurb about the fixes, even if it doesn't have to be all that exhaustive. Putting that in the tag is convenient, but I'm ok with it being just in the email message too, like you usually do. Linus really wants signed tags when
RFC tips-for-maintainers.txt (was Re: [GIT PULL] platform-drivers-x86 for 4.6-3)
On Wed, 2016-04-27 at 09:57 -0700, Darren Hart wrote: > On Wed, Apr 27, 2016 at 09:08:01AM -0700, Linus Torvalds wrote: > > On Tue, Apr 26, 2016 at 10:02 PM, Darren Hart wrote: > > > > > > Found myself not wanting to send a one patch pull request, but not > > > wanting to > > > wait until RC6 and possibly miss 4.6. > > > > > > Do you have a preference during the RC cycle in terms of balance between > > > patch > > > count and frequency for a small subsystem like platform-driver-x86? > > > > Once a week like this is fine, even if it's just a single trivial > > one-liner change. ... > Very helpful, thank you Linus. I believe I just inherited a TODO to find the > right spot in the documentation to record this. Actually I've been meaning to do that for ages, and I have a few more collected in my notes. How do these look? Git pulls should start "[GIT PULL]": http://lkml.kernel.org/r/CA+55aFxURVwV4NZcPsVzQ-Ui9Nmn2vdXVw==s5rkhxc1y4b...@mail.gmail.com just for your information: this pull request doesn't show up with my normal merge-window search pattern of "git pull", because you never actually say "pull" anywhere. Please use a subject with "[GIT PULL]" prefix (preferred), or just change the body of the email to have a "please pull" in it or something. I get too much email, and particularly during the merge window it really helps if I can keep my pull emails separate from other stuff by just having them match a simple pattern. Linus generally likes to see merge conflicts himself, for subtle conflicts you can provide a separate tree with the result of your merge: http://lkml.kernel.org/r/ca+55afwm2e9stvtyjrnpjh5rszafbxbj5ffykyi730ojon+...@mail.gmail.com On Sat, Sep 5, 2015 at 2:37 AM, Neil Brown wrote: > > Please pull these updates. I've already merged with the 'block' tree > to resolve a few simple conflicts. So for the future, I actually prefer to see and handle the conflicts myself. I really just prefer knowing what's going on, and merge conflicts are an indication of cross-maintainer issues which are *exactly* the kinds of things I want to be aware of. However, in this case I was "ok, I've already done several other merge resolutions with the wbole damn bio_endio error handling changes", so I felt I was aware enough about how that ended up being a cross-subsystem conflict, and just took your pre-merged version. If you feel that the conflicts are particularly subtle, or just generally worry about the merge, or just because you want to do some merge-testing, what some people end up doing is to send me their unmerged branch, and then send me a separate ".. and here's the merge I did". I'll then do the merge myself anyway, but then after doing the merge I'll switch to a temporary testing branch and re-do the merge with the pre-merged state just to verify. Generally the end result is identical, but when it isn't, that's actually usually interesting (sometimes it's just a ordering difference, but sometimes it's a merge error - and so far I think most merge errors have come from sub-maintainers, for the simple reason that they generally aren't as used to merging as I am - so even if they know the code better, I sometimes catch merge gotcha's better). Don't rebase your tree between linux-next and requesting a pull unless you absolutely must: http://lkml.kernel.org/r/CA+55aFyrep8hMApzYA4DyFgEoHc8o8KATTV=jqf6-fgtlsq...@mail.gmail.com Just stop [rebasing]. I'll handle the merge issues. If there are complicated merge problems that you really worry about, what you can do is (a) make a test-merge just to check (b) do NOT send me that test-merge (c) as part of the pull request, tell me that "there's a conflict because xyz, and this is how we think it should be handled". ... There are valid reasons to rebase, but those are along the line of "we messed up catastrophically, and we _have_ to clean up the history". A simple merge conflict due to a trivial duplicated commit is not a reason. Linus likes signed tags with a short blurb: http://lkml.kernel.org/r/CA+55aFwa9b5=aykxnkfcaqn27pfy8gchuhck+pkrqcfgn6f...@mail.gmail.com .. the above is a tag, but it's not signed, nor even annotated. So it has no message about what the fixes are in it, it's just the plain SHA1 id. Now, I don't really require signed tags from kernel.org, but it would still be nice. And I *do* want a little blurb about the fixes, even if it doesn't have to be all that exhaustive. Putting that in the tag is convenient, but I'm ok with it being just in the email message too, like you usually do. Linus really wants signed tags when pulling from non-kernel.org, even if your
Re: [PATCH] mm/zswap: use workqueue to destroy pool
On (04/28/16 10:40), Sergey Senozhatsky wrote: [..] > the bigger issue here (and I was thinking at some point of fixing it, > but then I grepped to see how many API users are in there, and I gave > up) is that it seems we have no way to check if the dir exists in debugfs. well, unless we want to do something like below. but I don't think Greg will not buy it and the basic rule is to be careful in the driver code and avoid any collisions. --- fs/debugfs/inode.c | 48 include/linux/debugfs.h | 7 +++ 2 files changed, 55 insertions(+) diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c index 8580831..76cf851 100644 --- a/fs/debugfs/inode.c +++ b/fs/debugfs/inode.c @@ -709,6 +709,54 @@ exit: EXPORT_SYMBOL_GPL(debugfs_rename); /** + * debugfs_entry_exists - lookup file/directory name + * + * @name: a pointer to a string containing the name of the file/directory + *to lookup. + * @parent: a pointer to the parent dentry. This should be a directory + * dentry if set. If this parameter is NULL, then the root of the + * debugfs filesystem will be used. + * + * This function lookup a file/directory name in debugfs. If the + * name corresponds to positive dentry, the function will return %0. + * + * If debugfs is not enabled in the kernel, the value -%ENODEV will be + * returned. + */ +int debugfs_entry_exists(const char *name, struct dentry *parent) +{ + struct dentry *dentry; + int error; + + if (IS_ERR(parent)) + return PTR_ERR(parent); + + error = simple_pin_fs(_fs_type, _mount, + _mount_count); + if (error) + return error; + + if (!parent) + parent = debugfs_mount->mnt_root; + + error = -EINVAL; + inode_lock(d_inode(parent)); + dentry = lookup_one_len(name, parent, strlen(name)); + if (IS_ERR(dentry)) { + error = PTR_ERR(dentry); + } else { + if (d_really_is_positive(dentry)) + error = 0; + dput(dentry); + } + + inode_unlock(d_inode(parent)); + simple_release_fs(_mount, _mount_count); + return error; +} +EXPORT_SYMBOL_GPL(debugfs_entry_exists); + +/** * debugfs_initialized - Tells whether debugfs has been registered */ bool debugfs_initialized(void) diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h index 981e53a..5b6321e 100644 --- a/include/linux/debugfs.h +++ b/include/linux/debugfs.h @@ -124,6 +124,8 @@ ssize_t debugfs_read_file_bool(struct file *file, char __user *user_buf, ssize_t debugfs_write_file_bool(struct file *file, const char __user *user_buf, size_t count, loff_t *ppos); +int debugfs_entry_exists(const char *name, struct dentry *parent); + #else #include @@ -312,6 +314,11 @@ static inline ssize_t debugfs_write_file_bool(struct file *file, return -ENODEV; } +static inline int debugfs_entry_exists(const char *name, struct dentry *parent) +{ + return -ENODEV; +} + #endif #endif
Re: [PATCHSET v5] Make background writeback great again for the first time
于 2016/4/28 4:59, Jens Axboe 写道: > On Wed, Apr 27 2016, Jens Axboe wrote: >> On Wed, Apr 27 2016, Jens Axboe wrote: >>> On 04/27/2016 12:01 PM, Jan Kara wrote: Hi, On Tue 26-04-16 09:55:23, Jens Axboe wrote: > Since the dawn of time, our background buffered writeback has sucked. > When we do background buffered writeback, it should have little impact > on foreground activity. That's the definition of background activity... > But for as long as I can remember, heavy buffered writers have not > behaved like that. For instance, if I do something like this: > > $ dd if=/dev/zero of=foo bs=1M count=10k > > on my laptop, and then try and start chrome, it basically won't start > before the buffered writeback is done. Or, for server oriented > workloads, where installation of a big RPM (or similar) adversely > impacts database reads or sync writes. When that happens, I get people > yelling at me. > > I have posted plenty of results previously, I'll keep it shorter > this time. Here's a run on my laptop, using read-to-pipe-async for > reading a 5g file, and rewriting it. You can find this test program > in the fio git repo. I have tested your patchset on my test system. Generally I have observed noticeable drop in average throughput for heavy background writes without any other disk activity and also somewhat increased variance in the runtimes. It is most visible on this simple testcases: dd if=/dev/zero of=/mnt/file bs=1M count=1 and dd if=/dev/zero of=/mnt/file bs=1M count=1 conv=fsync The machine has 4GB of ram, /mnt is an ext3 filesystem that is freshly created before each dd run on a dedicated disk. Without your patches I get pretty stable dd runtimes for both cases: dd if=/dev/zero of=/mnt/file bs=1M count=1 Runtimes: 87.9611 87.3279 87.2554 dd if=/dev/zero of=/mnt/file bs=1M count=1 conv=fsync Runtimes: 93.3502 93.2086 93.541 With your patches the numbers look like: dd if=/dev/zero of=/mnt/file bs=1M count=1 Runtimes: 108.183, 97.184, 99.9587 dd if=/dev/zero of=/mnt/file bs=1M count=1 conv=fsync Runtimes: 104.9, 102.775, 102.892 I have checked whether the variance is due to some interaction with CFQ which is used for the disk. When I switched the disk to deadline, I still get some variance although, the throughput is still ~10% lower: dd if=/dev/zero of=/mnt/file bs=1M count=1 Runtimes: 100.417 100.643 100.866 dd if=/dev/zero of=/mnt/file bs=1M count=1 conv=fsync Runtimes: 104.208 106.341 105.483 The disk is rotational SATA drive with writeback cache, queue depth of the disk reported in /sys/block/sdb/device/queue_depth is 1. So I think we still need some tweaking on the low end of the storage spectrum so that we don't lose 10% of throughput for simple cases like this. >>> >>> Thanks for testing, Jan! I haven't tried old QD=1 SATA. I wonder if >>> you are seeing smaller requests, and that is why it both varies and >>> you get lower throughput? I'll try and setup a test here similar to >>> yours. >> >> Jan, care to try the below patch? I can't fully reproduce your issue on >> a SCSI disk limited to QD=1, but I have a feeling this might help. It's >> a bit of a hack, but the general idea is to allow one more request to >> build up for QD=1 devices. That eliminates wait time between one request >> finishing, and the next being submitted. > > That accidentally added a potentially stall, this one is both cleaner > and should have that fixed. > > diff --git a/lib/wbt.c b/lib/wbt.c > index 650da911f24f..322f5e04e994 100644 > --- a/lib/wbt.c > +++ b/lib/wbt.c > @@ -98,18 +98,23 @@ void __wbt_done(struct rq_wb *rwb) > else > limit = rwb->wb_normal; Hi Jens, This statement 'limit = rwb->wb_normal' is executed twice, maybe once is enough. It is not a big deal anyway :) Another question about this if branch: if (rwb->wc && !atomic_read(>bdi->wb.dirty_sleeping)) limit = 0; I can't follow the logic of this if branch. why set limit equal to 0 when the device supports write back caches and there are tasks being limited in balance_dirty_pages(). Could you pelase give more info about this ? Thanks! > > + inflight = atomic_dec_return(>inflight); > + > /* > - * Don't wake anyone up if we are above the normal limit. If > - * throttling got disabled (limit == 0) with waiters, ensure > - * that we wake them up. > + * wbt got disabled with IO in flight. Wake up any potential > + * waiters, we don't have to do more than that. >*/ > - inflight = atomic_dec_return(>inflight); > - if (limit && inflight >= limit) { > - if (!rwb->wb_max) > - wake_up_all(>wait); > +
Re: [PATCH] mm/zswap: use workqueue to destroy pool
On (04/28/16 10:40), Sergey Senozhatsky wrote: [..] > the bigger issue here (and I was thinking at some point of fixing it, > but then I grepped to see how many API users are in there, and I gave > up) is that it seems we have no way to check if the dir exists in debugfs. well, unless we want to do something like below. but I don't think Greg will not buy it and the basic rule is to be careful in the driver code and avoid any collisions. --- fs/debugfs/inode.c | 48 include/linux/debugfs.h | 7 +++ 2 files changed, 55 insertions(+) diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c index 8580831..76cf851 100644 --- a/fs/debugfs/inode.c +++ b/fs/debugfs/inode.c @@ -709,6 +709,54 @@ exit: EXPORT_SYMBOL_GPL(debugfs_rename); /** + * debugfs_entry_exists - lookup file/directory name + * + * @name: a pointer to a string containing the name of the file/directory + *to lookup. + * @parent: a pointer to the parent dentry. This should be a directory + * dentry if set. If this parameter is NULL, then the root of the + * debugfs filesystem will be used. + * + * This function lookup a file/directory name in debugfs. If the + * name corresponds to positive dentry, the function will return %0. + * + * If debugfs is not enabled in the kernel, the value -%ENODEV will be + * returned. + */ +int debugfs_entry_exists(const char *name, struct dentry *parent) +{ + struct dentry *dentry; + int error; + + if (IS_ERR(parent)) + return PTR_ERR(parent); + + error = simple_pin_fs(_fs_type, _mount, + _mount_count); + if (error) + return error; + + if (!parent) + parent = debugfs_mount->mnt_root; + + error = -EINVAL; + inode_lock(d_inode(parent)); + dentry = lookup_one_len(name, parent, strlen(name)); + if (IS_ERR(dentry)) { + error = PTR_ERR(dentry); + } else { + if (d_really_is_positive(dentry)) + error = 0; + dput(dentry); + } + + inode_unlock(d_inode(parent)); + simple_release_fs(_mount, _mount_count); + return error; +} +EXPORT_SYMBOL_GPL(debugfs_entry_exists); + +/** * debugfs_initialized - Tells whether debugfs has been registered */ bool debugfs_initialized(void) diff --git a/include/linux/debugfs.h b/include/linux/debugfs.h index 981e53a..5b6321e 100644 --- a/include/linux/debugfs.h +++ b/include/linux/debugfs.h @@ -124,6 +124,8 @@ ssize_t debugfs_read_file_bool(struct file *file, char __user *user_buf, ssize_t debugfs_write_file_bool(struct file *file, const char __user *user_buf, size_t count, loff_t *ppos); +int debugfs_entry_exists(const char *name, struct dentry *parent); + #else #include @@ -312,6 +314,11 @@ static inline ssize_t debugfs_write_file_bool(struct file *file, return -ENODEV; } +static inline int debugfs_entry_exists(const char *name, struct dentry *parent) +{ + return -ENODEV; +} + #endif #endif
Re: [PATCHSET v5] Make background writeback great again for the first time
于 2016/4/28 4:59, Jens Axboe 写道: > On Wed, Apr 27 2016, Jens Axboe wrote: >> On Wed, Apr 27 2016, Jens Axboe wrote: >>> On 04/27/2016 12:01 PM, Jan Kara wrote: Hi, On Tue 26-04-16 09:55:23, Jens Axboe wrote: > Since the dawn of time, our background buffered writeback has sucked. > When we do background buffered writeback, it should have little impact > on foreground activity. That's the definition of background activity... > But for as long as I can remember, heavy buffered writers have not > behaved like that. For instance, if I do something like this: > > $ dd if=/dev/zero of=foo bs=1M count=10k > > on my laptop, and then try and start chrome, it basically won't start > before the buffered writeback is done. Or, for server oriented > workloads, where installation of a big RPM (or similar) adversely > impacts database reads or sync writes. When that happens, I get people > yelling at me. > > I have posted plenty of results previously, I'll keep it shorter > this time. Here's a run on my laptop, using read-to-pipe-async for > reading a 5g file, and rewriting it. You can find this test program > in the fio git repo. I have tested your patchset on my test system. Generally I have observed noticeable drop in average throughput for heavy background writes without any other disk activity and also somewhat increased variance in the runtimes. It is most visible on this simple testcases: dd if=/dev/zero of=/mnt/file bs=1M count=1 and dd if=/dev/zero of=/mnt/file bs=1M count=1 conv=fsync The machine has 4GB of ram, /mnt is an ext3 filesystem that is freshly created before each dd run on a dedicated disk. Without your patches I get pretty stable dd runtimes for both cases: dd if=/dev/zero of=/mnt/file bs=1M count=1 Runtimes: 87.9611 87.3279 87.2554 dd if=/dev/zero of=/mnt/file bs=1M count=1 conv=fsync Runtimes: 93.3502 93.2086 93.541 With your patches the numbers look like: dd if=/dev/zero of=/mnt/file bs=1M count=1 Runtimes: 108.183, 97.184, 99.9587 dd if=/dev/zero of=/mnt/file bs=1M count=1 conv=fsync Runtimes: 104.9, 102.775, 102.892 I have checked whether the variance is due to some interaction with CFQ which is used for the disk. When I switched the disk to deadline, I still get some variance although, the throughput is still ~10% lower: dd if=/dev/zero of=/mnt/file bs=1M count=1 Runtimes: 100.417 100.643 100.866 dd if=/dev/zero of=/mnt/file bs=1M count=1 conv=fsync Runtimes: 104.208 106.341 105.483 The disk is rotational SATA drive with writeback cache, queue depth of the disk reported in /sys/block/sdb/device/queue_depth is 1. So I think we still need some tweaking on the low end of the storage spectrum so that we don't lose 10% of throughput for simple cases like this. >>> >>> Thanks for testing, Jan! I haven't tried old QD=1 SATA. I wonder if >>> you are seeing smaller requests, and that is why it both varies and >>> you get lower throughput? I'll try and setup a test here similar to >>> yours. >> >> Jan, care to try the below patch? I can't fully reproduce your issue on >> a SCSI disk limited to QD=1, but I have a feeling this might help. It's >> a bit of a hack, but the general idea is to allow one more request to >> build up for QD=1 devices. That eliminates wait time between one request >> finishing, and the next being submitted. > > That accidentally added a potentially stall, this one is both cleaner > and should have that fixed. > > diff --git a/lib/wbt.c b/lib/wbt.c > index 650da911f24f..322f5e04e994 100644 > --- a/lib/wbt.c > +++ b/lib/wbt.c > @@ -98,18 +98,23 @@ void __wbt_done(struct rq_wb *rwb) > else > limit = rwb->wb_normal; Hi Jens, This statement 'limit = rwb->wb_normal' is executed twice, maybe once is enough. It is not a big deal anyway :) Another question about this if branch: if (rwb->wc && !atomic_read(>bdi->wb.dirty_sleeping)) limit = 0; I can't follow the logic of this if branch. why set limit equal to 0 when the device supports write back caches and there are tasks being limited in balance_dirty_pages(). Could you pelase give more info about this ? Thanks! > > + inflight = atomic_dec_return(>inflight); > + > /* > - * Don't wake anyone up if we are above the normal limit. If > - * throttling got disabled (limit == 0) with waiters, ensure > - * that we wake them up. > + * wbt got disabled with IO in flight. Wake up any potential > + * waiters, we don't have to do more than that. >*/ > - inflight = atomic_dec_return(>inflight); > - if (limit && inflight >= limit) { > - if (!rwb->wb_max) > - wake_up_all(>wait); > +
[PATCH v2 2/2] usb: misc: usbtest: fix error of urb allocation
urb allocation will fail when usbtest_alloc_urb() tries to allocate zero length buffer, but it doesn't need it in fact, so just skips buffer allocation in the case. Signed-off-by: Chunfeng Yun--- drivers/usb/misc/usbtest.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c index 92fdb6e..de485d8 100644 --- a/drivers/usb/misc/usbtest.c +++ b/drivers/usb/misc/usbtest.c @@ -287,6 +287,9 @@ static struct urb *usbtest_alloc_urb( if (usb_pipein(pipe)) urb->transfer_flags |= URB_SHORT_NOT_OK; + if ((bytes + offset) == 0) + return urb; + if (urb->transfer_flags & URB_NO_TRANSFER_DMA_MAP) urb->transfer_buffer = usb_alloc_coherent(udev, bytes + offset, GFP_KERNEL, >transfer_dma); -- 1.7.9.5
[PATCH v2 1/2] usb: core: buffer: avoid NULL pointer dereferrence
NULL pointer dereferrence will happen when class driver wants to allocate zero length buffer and pool_max[0] can't be used, so simply returns NULL in the case. Signed-off-by: Chunfeng Yun--- drivers/usb/core/buffer.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/usb/core/buffer.c b/drivers/usb/core/buffer.c index 2741566..98e39f9 100644 --- a/drivers/usb/core/buffer.c +++ b/drivers/usb/core/buffer.c @@ -122,6 +122,9 @@ void *hcd_buffer_alloc( struct usb_hcd *hcd = bus_to_hcd(bus); int i; + if (size == 0) + return NULL; + /* some USB hosts just use PIO */ if (!IS_ENABLED(CONFIG_HAS_DMA) || (!bus->controller->dma_mask && -- 1.7.9.5
[PATCH v2 2/2] usb: misc: usbtest: fix error of urb allocation
urb allocation will fail when usbtest_alloc_urb() tries to allocate zero length buffer, but it doesn't need it in fact, so just skips buffer allocation in the case. Signed-off-by: Chunfeng Yun --- drivers/usb/misc/usbtest.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c index 92fdb6e..de485d8 100644 --- a/drivers/usb/misc/usbtest.c +++ b/drivers/usb/misc/usbtest.c @@ -287,6 +287,9 @@ static struct urb *usbtest_alloc_urb( if (usb_pipein(pipe)) urb->transfer_flags |= URB_SHORT_NOT_OK; + if ((bytes + offset) == 0) + return urb; + if (urb->transfer_flags & URB_NO_TRANSFER_DMA_MAP) urb->transfer_buffer = usb_alloc_coherent(udev, bytes + offset, GFP_KERNEL, >transfer_dma); -- 1.7.9.5
[PATCH v2 1/2] usb: core: buffer: avoid NULL pointer dereferrence
NULL pointer dereferrence will happen when class driver wants to allocate zero length buffer and pool_max[0] can't be used, so simply returns NULL in the case. Signed-off-by: Chunfeng Yun --- drivers/usb/core/buffer.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/usb/core/buffer.c b/drivers/usb/core/buffer.c index 2741566..98e39f9 100644 --- a/drivers/usb/core/buffer.c +++ b/drivers/usb/core/buffer.c @@ -122,6 +122,9 @@ void *hcd_buffer_alloc( struct usb_hcd *hcd = bus_to_hcd(bus); int i; + if (size == 0) + return NULL; + /* some USB hosts just use PIO */ if (!IS_ENABLED(CONFIG_HAS_DMA) || (!bus->controller->dma_mask && -- 1.7.9.5
Re: [PATCH 7/8] wbt: add general throttling mechanism
于 2016/4/27 23:21, Jens Axboe 写道: > On 04/27/2016 06:06 AM, xiakaixu wrote: >>> +void __wbt_done(struct rq_wb *rwb) >>> +{ >>> +int inflight, limit = rwb->wb_normal; >>> + >>> +/* >>> + * If the device does write back caching, drop further down >>> + * before we wake people up. >>> + */ >>> +if (rwb->wc && !atomic_read(>bdi->wb.dirty_sleeping)) >>> +limit = 0; >>> +else >>> +limit = rwb->wb_normal; >>> + >>> +/* >>> + * Don't wake anyone up if we are above the normal limit. If >>> + * throttling got disabled (limit == 0) with waiters, ensure >>> + * that we wake them up. >>> + */ >>> +inflight = atomic_dec_return(>inflight); >>> +if (limit && inflight >= limit) { >>> +if (!rwb->wb_max) >>> +wake_up_all(>wait); >>> +return; >>> +} >>> + >> Hi Jens, >> >> Just a little confused about this. The rwb->wb_max can't be 0 if the variable >> 'limit' does not equal to 0. So the if (!rwb->wb_max) branch maybe does not >> make sense. > > You are right, it doesn't make a lot of sense. I think it suffers from code > shuffling. How about the attached? The important part is that we wake up > waiters, if wbt got disabled while we had tracked IO in flight. > Hi Jens, The modified patch in another mail looks better. Maybe there are still some places coube be improved. You can find them in that mail. -- Regards Kaixu Xia
Re: [PATCH 7/8] wbt: add general throttling mechanism
于 2016/4/27 23:21, Jens Axboe 写道: > On 04/27/2016 06:06 AM, xiakaixu wrote: >>> +void __wbt_done(struct rq_wb *rwb) >>> +{ >>> +int inflight, limit = rwb->wb_normal; >>> + >>> +/* >>> + * If the device does write back caching, drop further down >>> + * before we wake people up. >>> + */ >>> +if (rwb->wc && !atomic_read(>bdi->wb.dirty_sleeping)) >>> +limit = 0; >>> +else >>> +limit = rwb->wb_normal; >>> + >>> +/* >>> + * Don't wake anyone up if we are above the normal limit. If >>> + * throttling got disabled (limit == 0) with waiters, ensure >>> + * that we wake them up. >>> + */ >>> +inflight = atomic_dec_return(>inflight); >>> +if (limit && inflight >= limit) { >>> +if (!rwb->wb_max) >>> +wake_up_all(>wait); >>> +return; >>> +} >>> + >> Hi Jens, >> >> Just a little confused about this. The rwb->wb_max can't be 0 if the variable >> 'limit' does not equal to 0. So the if (!rwb->wb_max) branch maybe does not >> make sense. > > You are right, it doesn't make a lot of sense. I think it suffers from code > shuffling. How about the attached? The important part is that we wake up > waiters, if wbt got disabled while we had tracked IO in flight. > Hi Jens, The modified patch in another mail looks better. Maybe there are still some places coube be improved. You can find them in that mail. -- Regards Kaixu Xia
Re: [PATCH V4] audit: add tty field to LOGIN event
On 04/27/2016 06:31 PM, Richard Guy Briggs wrote: > On 16/04/22, Peter Hurley wrote: >> On 04/21/2016 11:14 AM, Richard Guy Briggs wrote: >>> The tty field was missing from AUDIT_LOGIN events. >>> >>> Refactor code to create a new function audit_get_tty(), using it to >>> replace the call in audit_log_task_info() and to add it to >>> audit_log_set_loginuid(). Lock and bump the kref to protect it, adding >>> audit_put_tty() alias to decrement it. >>> >>> Signed-off-by: Richard Guy Briggs>>> --- >>> >>> V4: Add missing prototype for audit_put_tty() when audit syscall is not >>> enabled (MIPS). >>> >>> V3: Introduce audit_put_tty() alias to decrement kref. >>> >>> V2: Use kref to protect tty signal struct while in use. >>> >>> --- >>> >>> include/linux/audit.h | 24 >>> kernel/audit.c| 18 +- >>> kernel/auditsc.c |8 ++-- >>> 3 files changed, 35 insertions(+), 15 deletions(-) >>> >>> diff --git a/include/linux/audit.h b/include/linux/audit.h >>> index b40ed5d..32cdafb 100644 >>> --- a/include/linux/audit.h >>> +++ b/include/linux/audit.h >>> @@ -26,6 +26,7 @@ >>> #include >>> #include >>> #include >>> +#include >>> >>> #define AUDIT_INO_UNSET ((unsigned long)-1) >>> #define AUDIT_DEV_UNSET ((dev_t)-1) >>> @@ -343,6 +344,23 @@ static inline unsigned int audit_get_sessionid(struct >>> task_struct *tsk) >>> return tsk->sessionid; >>> } >>> >>> +static inline struct tty_struct *audit_get_tty(struct task_struct *tsk) >>> +{ >>> + struct tty_struct *tty = NULL; >>> + unsigned long flags; >>> + >>> + spin_lock_irqsave(>sighand->siglock, flags); >>> + if (tsk->signal) >>> + tty = tty_kref_get(tsk->signal->tty); >>> + spin_unlock_irqrestore(>sighand->siglock, flags); >> >> >> Not that I'm objecting because I get that you're just refactoring >> existing code, but I thought I'd point out some stuff. >> >> 1. There's no need to check if signal_struct is NULL (ie. tsk->signal) >>because if it is, this will blow up trying to dereference the >>sighand_struct (ie tsk->sighand). > > Ok. This logic goes back 10 years and one month less two days. (45d9bb0e) > >> 2. The existing usage is always tsk==current > > My understanding is that when it is called via: > > copy_process() > audit_free() > __audit_free() > audit_log_exit() > audit_log_task_info() > > then tsk != current. While it's true that tsk != current here, everything relevant to tty in task_struct is the same because the nascent task is not even half-done. So tsk->sighand == current->sighand, tsk->signal == current->signal etc. If you're uncomfortable with pass-through execution like that, then the simple solution is: struct tty_struct *tty = NULL; /* tsk != current when copy_process() failed */ if (tsk == current) tty = get_current_tty(); because tty_kref_put(tty) accepts NULL tty and (obviously) so does tty_name(tty). Regards, Peter Hurley > This appears to be the only case which appears to > force lugging around tsk. This is noted in that commit referenced > above. > >> 3. If the idea is to make this invulnerable to tsk being gone, then >>the usage is unsafe anyway. >> >> >> So ultimately (but not necessarily for this patch) I'd prefer that either >> a. audit use existing tty api instead of open-coding, or >> b. add any tty api functions required. > > This latter option did cross my mind... > >> Peter Hurley >> >>> + return tty; >>> +} >>> + >>> +static inline void audit_put_tty(struct tty_struct *tty) >>> +{ >>> + tty_kref_put(tty); >>> +} >>> + >>> extern void __audit_ipc_obj(struct kern_ipc_perm *ipcp); >>> extern void __audit_ipc_set_perm(unsigned long qbytes, uid_t uid, gid_t >>> gid, umode_t mode); >>> extern void __audit_bprm(struct linux_binprm *bprm); >>> @@ -500,6 +518,12 @@ static inline unsigned int audit_get_sessionid(struct >>> task_struct *tsk) >>> { >>> return -1; >>> } >>> +static inline struct tty_struct *audit_get_tty(struct task_struct *tsk) >>> +{ >>> + return NULL; >>> +} >>> +static inline void audit_put_tty(struct tty_struct *tty) >>> +{ } >>> static inline void audit_ipc_obj(struct kern_ipc_perm *ipcp) >>> { } >>> static inline void audit_ipc_set_perm(unsigned long qbytes, uid_t uid, >>> diff --git a/kernel/audit.c b/kernel/audit.c >>> index 3a3e5de..7edd776 100644 >>> --- a/kernel/audit.c >>> +++ b/kernel/audit.c >>> @@ -64,7 +64,6 @@ >>> #include >>> #endif >>> #include >>> -#include >>> #include >>> #include >>> >>> @@ -1873,21 +1872,14 @@ void audit_log_task_info(struct audit_buffer *ab, >>> struct task_struct *tsk) >>> { >>> const struct cred *cred; >>> char comm[sizeof(tsk->comm)]; >>> - char *tty; >>> + struct tty_struct *tty; >>> >>> if (!ab) >>> return; >>>
Re: [PATCH V4] audit: add tty field to LOGIN event
On 04/27/2016 06:31 PM, Richard Guy Briggs wrote: > On 16/04/22, Peter Hurley wrote: >> On 04/21/2016 11:14 AM, Richard Guy Briggs wrote: >>> The tty field was missing from AUDIT_LOGIN events. >>> >>> Refactor code to create a new function audit_get_tty(), using it to >>> replace the call in audit_log_task_info() and to add it to >>> audit_log_set_loginuid(). Lock and bump the kref to protect it, adding >>> audit_put_tty() alias to decrement it. >>> >>> Signed-off-by: Richard Guy Briggs >>> --- >>> >>> V4: Add missing prototype for audit_put_tty() when audit syscall is not >>> enabled (MIPS). >>> >>> V3: Introduce audit_put_tty() alias to decrement kref. >>> >>> V2: Use kref to protect tty signal struct while in use. >>> >>> --- >>> >>> include/linux/audit.h | 24 >>> kernel/audit.c| 18 +- >>> kernel/auditsc.c |8 ++-- >>> 3 files changed, 35 insertions(+), 15 deletions(-) >>> >>> diff --git a/include/linux/audit.h b/include/linux/audit.h >>> index b40ed5d..32cdafb 100644 >>> --- a/include/linux/audit.h >>> +++ b/include/linux/audit.h >>> @@ -26,6 +26,7 @@ >>> #include >>> #include >>> #include >>> +#include >>> >>> #define AUDIT_INO_UNSET ((unsigned long)-1) >>> #define AUDIT_DEV_UNSET ((dev_t)-1) >>> @@ -343,6 +344,23 @@ static inline unsigned int audit_get_sessionid(struct >>> task_struct *tsk) >>> return tsk->sessionid; >>> } >>> >>> +static inline struct tty_struct *audit_get_tty(struct task_struct *tsk) >>> +{ >>> + struct tty_struct *tty = NULL; >>> + unsigned long flags; >>> + >>> + spin_lock_irqsave(>sighand->siglock, flags); >>> + if (tsk->signal) >>> + tty = tty_kref_get(tsk->signal->tty); >>> + spin_unlock_irqrestore(>sighand->siglock, flags); >> >> >> Not that I'm objecting because I get that you're just refactoring >> existing code, but I thought I'd point out some stuff. >> >> 1. There's no need to check if signal_struct is NULL (ie. tsk->signal) >>because if it is, this will blow up trying to dereference the >>sighand_struct (ie tsk->sighand). > > Ok. This logic goes back 10 years and one month less two days. (45d9bb0e) > >> 2. The existing usage is always tsk==current > > My understanding is that when it is called via: > > copy_process() > audit_free() > __audit_free() > audit_log_exit() > audit_log_task_info() > > then tsk != current. While it's true that tsk != current here, everything relevant to tty in task_struct is the same because the nascent task is not even half-done. So tsk->sighand == current->sighand, tsk->signal == current->signal etc. If you're uncomfortable with pass-through execution like that, then the simple solution is: struct tty_struct *tty = NULL; /* tsk != current when copy_process() failed */ if (tsk == current) tty = get_current_tty(); because tty_kref_put(tty) accepts NULL tty and (obviously) so does tty_name(tty). Regards, Peter Hurley > This appears to be the only case which appears to > force lugging around tsk. This is noted in that commit referenced > above. > >> 3. If the idea is to make this invulnerable to tsk being gone, then >>the usage is unsafe anyway. >> >> >> So ultimately (but not necessarily for this patch) I'd prefer that either >> a. audit use existing tty api instead of open-coding, or >> b. add any tty api functions required. > > This latter option did cross my mind... > >> Peter Hurley >> >>> + return tty; >>> +} >>> + >>> +static inline void audit_put_tty(struct tty_struct *tty) >>> +{ >>> + tty_kref_put(tty); >>> +} >>> + >>> extern void __audit_ipc_obj(struct kern_ipc_perm *ipcp); >>> extern void __audit_ipc_set_perm(unsigned long qbytes, uid_t uid, gid_t >>> gid, umode_t mode); >>> extern void __audit_bprm(struct linux_binprm *bprm); >>> @@ -500,6 +518,12 @@ static inline unsigned int audit_get_sessionid(struct >>> task_struct *tsk) >>> { >>> return -1; >>> } >>> +static inline struct tty_struct *audit_get_tty(struct task_struct *tsk) >>> +{ >>> + return NULL; >>> +} >>> +static inline void audit_put_tty(struct tty_struct *tty) >>> +{ } >>> static inline void audit_ipc_obj(struct kern_ipc_perm *ipcp) >>> { } >>> static inline void audit_ipc_set_perm(unsigned long qbytes, uid_t uid, >>> diff --git a/kernel/audit.c b/kernel/audit.c >>> index 3a3e5de..7edd776 100644 >>> --- a/kernel/audit.c >>> +++ b/kernel/audit.c >>> @@ -64,7 +64,6 @@ >>> #include >>> #endif >>> #include >>> -#include >>> #include >>> #include >>> >>> @@ -1873,21 +1872,14 @@ void audit_log_task_info(struct audit_buffer *ab, >>> struct task_struct *tsk) >>> { >>> const struct cred *cred; >>> char comm[sizeof(tsk->comm)]; >>> - char *tty; >>> + struct tty_struct *tty; >>> >>> if (!ab) >>> return; >>> >>> /* tsk ==
Re: [PATCHv2 4/7] Documentation: dt: socfpga: Add Arria10 Ethernet binding
On Mon, Apr 25, 2016 at 12:52:45PM -0500, ttha...@opensource.altera.com wrote: > From: Thor Thayer> > Add the device tree bindings needed to support the Altera Ethernet > FIFO buffers on the Arria10 chip. > > Signed-off-by: Thor Thayer > --- > v2 No Change > --- > .../bindings/arm/altera/socfpga-eccmgr.txt | 24 > > 1 file changed, 24 insertions(+) > > diff --git a/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt > b/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt > index 5a6b160..aa1c593 100644 > --- a/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt > +++ b/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt > @@ -76,6 +76,18 @@ Required Properties: > - compatible : Should be "altr,socfpga-a10-ocram-ecc" > - reg: Address and size for ECC block registers. > > +Ethernet FIFO ECC > +Required Properties: > +- compatible : Should be "altr,socfpga-a10-emac0-rx-ecc" for the 1st EMAC > + Receive buffer > + or "altr,socfpga-a10-emac0-tx-ecc" for the 1st EMAC Transmit buffer > + or "altr,socfpga-a10-emac1-rx-ecc" for the 2nd EMAC Receive buffer > + or "altr,socfpga-a10-emac1-tx-ecc" for the 2nd EMAC Transmit buffer > + or "altr,socfpga-a10-emac2-rx-ecc" for the 3rd EMAC Receive buffer > + or "altr,socfpga-a10-emac2-tx-ecc" for the 3rd EMAC Transmit buffer These blocks don't really appear to be different other than the interrupt mask (which is in another block?). I think they should be the same compatible with a property for the interrupt (perhaps a full interrupt-controller binding). > +- reg: Address and size for ECC block registers. > +- parent : phandle to parent Ethernet node. Needs a better name and altr prefix. Maybe altr,eth-mac? Rob
Re: [PATCHv2 4/7] Documentation: dt: socfpga: Add Arria10 Ethernet binding
On Mon, Apr 25, 2016 at 12:52:45PM -0500, ttha...@opensource.altera.com wrote: > From: Thor Thayer > > Add the device tree bindings needed to support the Altera Ethernet > FIFO buffers on the Arria10 chip. > > Signed-off-by: Thor Thayer > --- > v2 No Change > --- > .../bindings/arm/altera/socfpga-eccmgr.txt | 24 > > 1 file changed, 24 insertions(+) > > diff --git a/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt > b/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt > index 5a6b160..aa1c593 100644 > --- a/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt > +++ b/Documentation/devicetree/bindings/arm/altera/socfpga-eccmgr.txt > @@ -76,6 +76,18 @@ Required Properties: > - compatible : Should be "altr,socfpga-a10-ocram-ecc" > - reg: Address and size for ECC block registers. > > +Ethernet FIFO ECC > +Required Properties: > +- compatible : Should be "altr,socfpga-a10-emac0-rx-ecc" for the 1st EMAC > + Receive buffer > + or "altr,socfpga-a10-emac0-tx-ecc" for the 1st EMAC Transmit buffer > + or "altr,socfpga-a10-emac1-rx-ecc" for the 2nd EMAC Receive buffer > + or "altr,socfpga-a10-emac1-tx-ecc" for the 2nd EMAC Transmit buffer > + or "altr,socfpga-a10-emac2-rx-ecc" for the 3rd EMAC Receive buffer > + or "altr,socfpga-a10-emac2-tx-ecc" for the 3rd EMAC Transmit buffer These blocks don't really appear to be different other than the interrupt mask (which is in another block?). I think they should be the same compatible with a property for the interrupt (perhaps a full interrupt-controller binding). > +- reg: Address and size for ECC block registers. > +- parent : phandle to parent Ethernet node. Needs a better name and altr prefix. Maybe altr,eth-mac? Rob
Re: [PATCH perf/core v5 05/15] perf probe: Use cache entry if possible
On Thu, Apr 28, 2016 at 03:37:52AM +0900, Masami Hiramatsu wrote: > From: Masami Hiramatsu> > Before analyzing debuginfo, try to find a corresponding entry > from probe cache always. This does not depend on --cache, > the --cache enables to store/update cache, but looking up > the cache is always enabled. > > Signed-off-by: Masami Hiramatsu > Signed-off-by: Masami Hiramatsu > --- [SNIP] > +static int find_probe_trace_events_from_cache(struct perf_probe_event *pev, > + struct probe_trace_event **tevs) > +{ > + struct probe_cache *cache; > + struct probe_cache_entry *entry; > + struct probe_trace_event *tev; > + struct str_node *node; > + int ret, i; > + > + cache = probe_cache__new(pev->target); > + if (!cache) > + return 0; > + > + entry = probe_cache__find(cache, pev); > + > + if (!entry && !pev->event && > + (!pev->point.file && pev->point.function && !pev->point.retprobe && > + !pev->point.line && !pev->point.offset && !pev->point.lazy_line)) { > + entry = probe_cache__find_by_name(cache, pev->group, > + pev->point.function); > + } What about arguments? It seems that it only checks group and event names of the given pev and use the cached entry's tev if found. What if they have different argument spec? Thanks, Namhyung > + if (!entry) { > + ret = 0; > + goto out; > + } > + > + ret = strlist__nr_entries(entry->tevlist); > + if (ret > probe_conf.max_probes) { > + pr_debug("Too many entries matched in the cache of %s\n", > + pev->target ? : "kernel"); > + ret = -E2BIG; > + goto out; > + } > + > + *tevs = zalloc(ret * sizeof(*tev)); > + if (!*tevs) { > + ret = -ENOMEM; > + goto out; > + } > + > + i = 0; > + strlist__for_each(node, entry->tevlist) { > + tev = &(*tevs)[i++]; > + ret = parse_probe_trace_command(node->s, tev); > + if (ret < 0) > + goto out; > + /* Set the uprobes attribute as same as original */ > + tev->uprobes = pev->uprobes; > + } > + ret = i; > + > +out: > + probe_cache__delete(cache); > + return ret; > +}
Re: [PATCH perf/core v5 05/15] perf probe: Use cache entry if possible
On Thu, Apr 28, 2016 at 03:37:52AM +0900, Masami Hiramatsu wrote: > From: Masami Hiramatsu > > Before analyzing debuginfo, try to find a corresponding entry > from probe cache always. This does not depend on --cache, > the --cache enables to store/update cache, but looking up > the cache is always enabled. > > Signed-off-by: Masami Hiramatsu > Signed-off-by: Masami Hiramatsu > --- [SNIP] > +static int find_probe_trace_events_from_cache(struct perf_probe_event *pev, > + struct probe_trace_event **tevs) > +{ > + struct probe_cache *cache; > + struct probe_cache_entry *entry; > + struct probe_trace_event *tev; > + struct str_node *node; > + int ret, i; > + > + cache = probe_cache__new(pev->target); > + if (!cache) > + return 0; > + > + entry = probe_cache__find(cache, pev); > + > + if (!entry && !pev->event && > + (!pev->point.file && pev->point.function && !pev->point.retprobe && > + !pev->point.line && !pev->point.offset && !pev->point.lazy_line)) { > + entry = probe_cache__find_by_name(cache, pev->group, > + pev->point.function); > + } What about arguments? It seems that it only checks group and event names of the given pev and use the cached entry's tev if found. What if they have different argument spec? Thanks, Namhyung > + if (!entry) { > + ret = 0; > + goto out; > + } > + > + ret = strlist__nr_entries(entry->tevlist); > + if (ret > probe_conf.max_probes) { > + pr_debug("Too many entries matched in the cache of %s\n", > + pev->target ? : "kernel"); > + ret = -E2BIG; > + goto out; > + } > + > + *tevs = zalloc(ret * sizeof(*tev)); > + if (!*tevs) { > + ret = -ENOMEM; > + goto out; > + } > + > + i = 0; > + strlist__for_each(node, entry->tevlist) { > + tev = &(*tevs)[i++]; > + ret = parse_probe_trace_command(node->s, tev); > + if (ret < 0) > + goto out; > + /* Set the uprobes attribute as same as original */ > + tev->uprobes = pev->uprobes; > + } > + ret = i; > + > +out: > + probe_cache__delete(cache); > + return ret; > +}
Re: [PATCH v2 1/3] dt/bindings: display: Add DT bindings for Mali Display Processors.
On Mon, Apr 25, 2016 at 03:19:22PM +0100, Liviu Dudau wrote: > Add DT bindings documentation for the Mali Display Processor. The bindings > describe the Mali DP500, DP550 and DP650 processors from ARM Ltd. > > Cc: Rob Herring> Cc: Pawel Moll > Cc: Mark Rutland > Cc: Ian Campbell > Cc: Kumar Gala > > Signed-off-by: Liviu Dudau > --- > .../devicetree/bindings/display/arm,malidp.txt | 65 > ++ > 1 file changed, 65 insertions(+) > create mode 100644 Documentation/devicetree/bindings/display/arm,malidp.txt Acked-by: Rob Herring
Re: [PATCH v2 1/3] dt/bindings: display: Add DT bindings for Mali Display Processors.
On Mon, Apr 25, 2016 at 03:19:22PM +0100, Liviu Dudau wrote: > Add DT bindings documentation for the Mali Display Processor. The bindings > describe the Mali DP500, DP550 and DP650 processors from ARM Ltd. > > Cc: Rob Herring > Cc: Pawel Moll > Cc: Mark Rutland > Cc: Ian Campbell > Cc: Kumar Gala > > Signed-off-by: Liviu Dudau > --- > .../devicetree/bindings/display/arm,malidp.txt | 65 > ++ > 1 file changed, 65 insertions(+) > create mode 100644 Documentation/devicetree/bindings/display/arm,malidp.txt Acked-by: Rob Herring
Re: [PATCH v4 04/11] drm: sun4i: Add DT bindings documentation
On Mon, Apr 25, 2016 at 03:22:45PM +0200, Maxime Ripard wrote: > The display pipeline of the Allwinner A10 is involving several loosely > coupled components. > > Add a documentation for the bindings. > > Signed-off-by: Maxime Ripard> --- > .../bindings/display/sunxi/sun4i-drm.txt | 258 > + > 1 file changed, 258 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > +The display engine pipeline (and its entry point, since it can be > +either directly the backend or the frontend) is represented as an > +extra node. > + > +Required properties: > + - compatible: value must be one of: > +* allwinner,sun5i-a13-display-engine > + > + - allwinner,pipelines: list of phandle to the display engine > +frontends available. > +display-engine { > + compatible = "allwinner,sun5i-a13-display-engine"; > + pipelines = <>; Missing allwinner prefix. Otherwise, Acked-by: Rob Herring
Re: [PATCH v4 04/11] drm: sun4i: Add DT bindings documentation
On Mon, Apr 25, 2016 at 03:22:45PM +0200, Maxime Ripard wrote: > The display pipeline of the Allwinner A10 is involving several loosely > coupled components. > > Add a documentation for the bindings. > > Signed-off-by: Maxime Ripard > --- > .../bindings/display/sunxi/sun4i-drm.txt | 258 > + > 1 file changed, 258 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > +The display engine pipeline (and its entry point, since it can be > +either directly the backend or the frontend) is represented as an > +extra node. > + > +Required properties: > + - compatible: value must be one of: > +* allwinner,sun5i-a13-display-engine > + > + - allwinner,pipelines: list of phandle to the display engine > +frontends available. > +display-engine { > + compatible = "allwinner,sun5i-a13-display-engine"; > + pipelines = <>; Missing allwinner prefix. Otherwise, Acked-by: Rob Herring
Re: [PATCH perf/core v5 04/15] perf probe: Add --cache option to cache the probe definitions
On Thu, Apr 28, 2016 at 03:37:42AM +0900, Masami Hiramatsu wrote: > From: Masami Hiramatsu> > Add --cache option to cache the probe definitions. This > just saves the result of the dwarf analysis to probe cache. > > Signed-off-by: Masami Hiramatsu > Signed-off-by: Masami Hiramatsu > --- > Changes in v5: > - Move probe_cache* definitions. (code cleanup) > > Changes in v4: > - Remove cache saving failure message. > --- [SNIP] > +/* For the kernel probe caches, pass target = NULL */ > +static int probe_cache__open(struct probe_cache *pcache, const char *target) > +{ > + char cpath[PATH_MAX]; > + char sbuildid[SBUILD_ID_SIZE]; > + char *dir_name; > + bool is_kallsyms = !target; > + int ret, fd; > + > + if (target) > + ret = filename__sprintf_build_id(target, sbuildid); > + else { > + target = "[kernel.kallsyms]"; > + ret = sysfs__sprintf_build_id("/", sbuildid); > + } > + if (ret < 0) { > + pr_debug("Failed to get build-id from %s.\n", target ?: > "kernel"); > + return ret; > + } > + > + /* If we have no buildid cache, make it */ > + if (!build_id_cache__cached(sbuildid)) { > + ret = build_id_cache__add_s(sbuildid, target, > + is_kallsyms, NULL); > + if (ret < 0) { > + pr_debug("Failed to add build-id cache: %s\n", target); > + return ret; > + } > + } > + > + dir_name = build_id_cache__dirname_from_path(sbuildid, target, > + is_kallsyms, false); > + if (!dir_name) > + return -ENOMEM; > + > + snprintf(cpath, PATH_MAX, "%s/probes", dir_name); > + fd = open(cpath, O_CREAT | O_RDWR | O_APPEND, 0644); > + if (fd < 0) > + pr_debug("Failed to open cache(%d): %s\n", fd, cpath); > + free(dir_name); > + pcache->fd = fd; > + > + return fd; > +} [SNIP] > + > +int probe_cache__commit(struct probe_cache *pcache) > +{ > + struct probe_cache_entry *entry; > + int ret = 0; > + > + /* TBD: if we do not update existing entries, skip it */ > + ret = lseek(pcache->fd, 0, SEEK_SET); > + if (ret < 0) > + goto out; > + > + ret = ftruncate(pcache->fd, 0); > + if (ret < 0) > + goto out; What is this doing? Does it truncate old contents on write? Opening with O_APPEND looks strange to me.. Thanks, Namhyung > + > + list_for_each_entry(entry, >list, list) { > + ret = probe_cache_entry__write(entry, pcache->fd); > + pr_debug("Cache committed: %d\n", ret); > + if (ret < 0) > + break; > + } > +out: > + return ret; > +} > diff --git a/tools/perf/util/probe-file.h b/tools/perf/util/probe-file.h > index 18ac9cf..d2b8791d 100644 > --- a/tools/perf/util/probe-file.h > +++ b/tools/perf/util/probe-file.h > @@ -5,6 +5,19 @@ > #include "strfilter.h" > #include "probe-event.h" > > +/* Cache of probe definitions */ > +struct probe_cache_entry { > + struct list_headlist; > + struct perf_probe_event pev; > + char*spev; > + struct strlist *tevlist; > +}; > + > +struct probe_cache { > + int fd; > + struct list_head list; > +}; > + > #define PF_FL_UPROBE 1 > #define PF_FL_RW 2 > > @@ -18,5 +31,11 @@ int probe_file__get_events(int fd, struct strfilter > *filter, > struct strlist *plist); > int probe_file__del_strlist(int fd, struct strlist *namelist); > > +struct probe_cache *probe_cache__new(const char *target); > +int probe_cache__add_entry(struct probe_cache *pcache, > +struct perf_probe_event *pev, > +struct probe_trace_event *tevs, int ntevs); > +int probe_cache__commit(struct probe_cache *pcache); > +void probe_cache__delete(struct probe_cache *pcache); > > #endif >
Re: [PATCH perf/core v5 04/15] perf probe: Add --cache option to cache the probe definitions
On Thu, Apr 28, 2016 at 03:37:42AM +0900, Masami Hiramatsu wrote: > From: Masami Hiramatsu > > Add --cache option to cache the probe definitions. This > just saves the result of the dwarf analysis to probe cache. > > Signed-off-by: Masami Hiramatsu > Signed-off-by: Masami Hiramatsu > --- > Changes in v5: > - Move probe_cache* definitions. (code cleanup) > > Changes in v4: > - Remove cache saving failure message. > --- [SNIP] > +/* For the kernel probe caches, pass target = NULL */ > +static int probe_cache__open(struct probe_cache *pcache, const char *target) > +{ > + char cpath[PATH_MAX]; > + char sbuildid[SBUILD_ID_SIZE]; > + char *dir_name; > + bool is_kallsyms = !target; > + int ret, fd; > + > + if (target) > + ret = filename__sprintf_build_id(target, sbuildid); > + else { > + target = "[kernel.kallsyms]"; > + ret = sysfs__sprintf_build_id("/", sbuildid); > + } > + if (ret < 0) { > + pr_debug("Failed to get build-id from %s.\n", target ?: > "kernel"); > + return ret; > + } > + > + /* If we have no buildid cache, make it */ > + if (!build_id_cache__cached(sbuildid)) { > + ret = build_id_cache__add_s(sbuildid, target, > + is_kallsyms, NULL); > + if (ret < 0) { > + pr_debug("Failed to add build-id cache: %s\n", target); > + return ret; > + } > + } > + > + dir_name = build_id_cache__dirname_from_path(sbuildid, target, > + is_kallsyms, false); > + if (!dir_name) > + return -ENOMEM; > + > + snprintf(cpath, PATH_MAX, "%s/probes", dir_name); > + fd = open(cpath, O_CREAT | O_RDWR | O_APPEND, 0644); > + if (fd < 0) > + pr_debug("Failed to open cache(%d): %s\n", fd, cpath); > + free(dir_name); > + pcache->fd = fd; > + > + return fd; > +} [SNIP] > + > +int probe_cache__commit(struct probe_cache *pcache) > +{ > + struct probe_cache_entry *entry; > + int ret = 0; > + > + /* TBD: if we do not update existing entries, skip it */ > + ret = lseek(pcache->fd, 0, SEEK_SET); > + if (ret < 0) > + goto out; > + > + ret = ftruncate(pcache->fd, 0); > + if (ret < 0) > + goto out; What is this doing? Does it truncate old contents on write? Opening with O_APPEND looks strange to me.. Thanks, Namhyung > + > + list_for_each_entry(entry, >list, list) { > + ret = probe_cache_entry__write(entry, pcache->fd); > + pr_debug("Cache committed: %d\n", ret); > + if (ret < 0) > + break; > + } > +out: > + return ret; > +} > diff --git a/tools/perf/util/probe-file.h b/tools/perf/util/probe-file.h > index 18ac9cf..d2b8791d 100644 > --- a/tools/perf/util/probe-file.h > +++ b/tools/perf/util/probe-file.h > @@ -5,6 +5,19 @@ > #include "strfilter.h" > #include "probe-event.h" > > +/* Cache of probe definitions */ > +struct probe_cache_entry { > + struct list_headlist; > + struct perf_probe_event pev; > + char*spev; > + struct strlist *tevlist; > +}; > + > +struct probe_cache { > + int fd; > + struct list_head list; > +}; > + > #define PF_FL_UPROBE 1 > #define PF_FL_RW 2 > > @@ -18,5 +31,11 @@ int probe_file__get_events(int fd, struct strfilter > *filter, > struct strlist *plist); > int probe_file__del_strlist(int fd, struct strlist *namelist); > > +struct probe_cache *probe_cache__new(const char *target); > +int probe_cache__add_entry(struct probe_cache *pcache, > +struct perf_probe_event *pev, > +struct probe_trace_event *tevs, int ntevs); > +int probe_cache__commit(struct probe_cache *pcache); > +void probe_cache__delete(struct probe_cache *pcache); > > #endif >
RE: [PATCH v2 4/6] ACPI / osi: Fix default _OSI(Darwin) support
Hi, > From: linux-acpi-ow...@vger.kernel.org [mailto:linux-acpi- > ow...@vger.kernel.org] On Behalf Of Rafael J. Wysocki > Subject: Re: [PATCH v2 4/6] ACPI / osi: Fix default _OSI(Darwin) support > > On Wednesday, April 27, 2016 09:45:21 AM Chen, Yu C wrote: > > Hi Lv, > > > > > From: Zheng, Lv > > > Subject: [PATCH v2 4/6] ACPI / osi: Fix default _OSI(Darwin) support > > > > > > From: Chen Yu> > > > > > The following commit always reports positive value when Apple hardware > > > queries _OSI("Darwin"): > > > Commit: 7bc5a2bad0b8d9d1ac9f7b8b33150e4ddf197334 > > > Subject: ACPI: Support _OSI("Darwin") correctly However since this > > > implementation places the judgement in runtime, it breaks > acpi_osi=!Darwin > > > and cannot return unsupported for _OSI("WinXXX") invoked before invoking > > > _OSI("Darwin"). > > > > > > This patch fixes the issues by reverting the wrong support and > > > implementing > > > the default behavior of _OSI("Darwin")/_OSI("WinXXX") on Apple hardware > via > > > DMI matching. > > > > > > Fixes: 7bc5a2bad0b8 ("ACPI: Support _OSI("Darwin") correctly") > > > Cc: # 3.18+ > > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=92111 > > > Reported-by: Lukas Wunner > > > Signed-off-by: Chen Yu > > > Signed-off-by: Lv Zheng > > > --- > > > drivers/acpi/blacklist.c | 23 ++ > > > drivers/acpi/osl.c | 58 > --- > > > --- > > > include/linux/acpi.h |1 + > > > 3 files changed, 75 insertions(+), 7 deletions(-) > > > > > > diff --git a/drivers/acpi/blacklist.c b/drivers/acpi/blacklist.c index > > > 96809cd..e947d3e 100644 > > > --- a/drivers/acpi/blacklist.c > > > +++ b/drivers/acpi/blacklist.c > > > @@ -133,6 +133,11 @@ int __init acpi_blacklisted(void) > > > return blacklisted; > > > } > > > #ifdef CONFIG_DMI > > > +static int __init dmi_enable_osi_darwin(const struct dmi_system_id *d) > > > +{ > > > + acpi_dmi_osi_darwin(1, d); /* enable */ > > > + return 0; > > > +} > > > static int __init dmi_enable_osi_linux(const struct dmi_system_id *d) { > > > acpi_dmi_osi_linux(1, d); /* enable */ > > > @@ -331,6 +336,24 @@ static struct dmi_system_id acpi_osi_dmi_table[] > > > __initdata = { > > > }, > > > }, > > > > > > + /* > > > + * Enable _OSI("Darwin") for all apple platforms. > > > + */ > > > + { > > > + .callback = dmi_enable_osi_darwin, > > > + .ident = "Apple hardware", > > > + .matches = { > > > + DMI_MATCH(DMI_SYS_VENDOR, "Apple INC."), > > > + }, > > > + }, > > > + { > > > + .callback = dmi_enable_osi_darwin, > > > + .ident = "Apple hardware", > > > + .matches = { > > > + DMI_MATCH(DMI_SYS_VENDOR, "Apple Computer, INC."), > > > + }, > > > + }, > > > + > > The vendor id should be 'Apple Inc.' and 'Apple Computer, Inc.' instead. > > If this is the only problem with this patch, I can fix it up. No need to > resend. [Lv Zheng] This is the only problem. Thanks for the help. Best regards -Lv > > Thanks, > Rafael > > -- > To unsubscribe from this list: send the line "unsubscribe linux-acpi" 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 4/6] ACPI / osi: Fix default _OSI(Darwin) support
Hi, > From: linux-acpi-ow...@vger.kernel.org [mailto:linux-acpi- > ow...@vger.kernel.org] On Behalf Of Rafael J. Wysocki > Subject: Re: [PATCH v2 4/6] ACPI / osi: Fix default _OSI(Darwin) support > > On Wednesday, April 27, 2016 09:45:21 AM Chen, Yu C wrote: > > Hi Lv, > > > > > From: Zheng, Lv > > > Subject: [PATCH v2 4/6] ACPI / osi: Fix default _OSI(Darwin) support > > > > > > From: Chen Yu > > > > > > The following commit always reports positive value when Apple hardware > > > queries _OSI("Darwin"): > > > Commit: 7bc5a2bad0b8d9d1ac9f7b8b33150e4ddf197334 > > > Subject: ACPI: Support _OSI("Darwin") correctly However since this > > > implementation places the judgement in runtime, it breaks > acpi_osi=!Darwin > > > and cannot return unsupported for _OSI("WinXXX") invoked before invoking > > > _OSI("Darwin"). > > > > > > This patch fixes the issues by reverting the wrong support and > > > implementing > > > the default behavior of _OSI("Darwin")/_OSI("WinXXX") on Apple hardware > via > > > DMI matching. > > > > > > Fixes: 7bc5a2bad0b8 ("ACPI: Support _OSI("Darwin") correctly") > > > Cc: # 3.18+ > > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=92111 > > > Reported-by: Lukas Wunner > > > Signed-off-by: Chen Yu > > > Signed-off-by: Lv Zheng > > > --- > > > drivers/acpi/blacklist.c | 23 ++ > > > drivers/acpi/osl.c | 58 > --- > > > --- > > > include/linux/acpi.h |1 + > > > 3 files changed, 75 insertions(+), 7 deletions(-) > > > > > > diff --git a/drivers/acpi/blacklist.c b/drivers/acpi/blacklist.c index > > > 96809cd..e947d3e 100644 > > > --- a/drivers/acpi/blacklist.c > > > +++ b/drivers/acpi/blacklist.c > > > @@ -133,6 +133,11 @@ int __init acpi_blacklisted(void) > > > return blacklisted; > > > } > > > #ifdef CONFIG_DMI > > > +static int __init dmi_enable_osi_darwin(const struct dmi_system_id *d) > > > +{ > > > + acpi_dmi_osi_darwin(1, d); /* enable */ > > > + return 0; > > > +} > > > static int __init dmi_enable_osi_linux(const struct dmi_system_id *d) { > > > acpi_dmi_osi_linux(1, d); /* enable */ > > > @@ -331,6 +336,24 @@ static struct dmi_system_id acpi_osi_dmi_table[] > > > __initdata = { > > > }, > > > }, > > > > > > + /* > > > + * Enable _OSI("Darwin") for all apple platforms. > > > + */ > > > + { > > > + .callback = dmi_enable_osi_darwin, > > > + .ident = "Apple hardware", > > > + .matches = { > > > + DMI_MATCH(DMI_SYS_VENDOR, "Apple INC."), > > > + }, > > > + }, > > > + { > > > + .callback = dmi_enable_osi_darwin, > > > + .ident = "Apple hardware", > > > + .matches = { > > > + DMI_MATCH(DMI_SYS_VENDOR, "Apple Computer, INC."), > > > + }, > > > + }, > > > + > > The vendor id should be 'Apple Inc.' and 'Apple Computer, Inc.' instead. > > If this is the only problem with this patch, I can fix it up. No need to > resend. [Lv Zheng] This is the only problem. Thanks for the help. Best regards -Lv > > Thanks, > Rafael > > -- > To unsubscribe from this list: send the line "unsubscribe linux-acpi" 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] cpufreq: st: enable selective initialization based on the platform
On 27-04-16, 17:18, Sudeep Holla wrote: > The sti-cpufreq does unconditional registration of the cpufreq-dt driver > which causes issue on an multi-platform build. For example, on Vexpress > TC2 platform, we get the following error on boot: > > cpu cpu0: OPP-v2 not supported > cpu cpu0: Not doing voltage scaling > cpu: dev_pm_opp_of_cpumask_add_table: couldn't find opp table > for cpu:0, -19 > cpu cpu0: dev_pm_opp_get_max_volt_latency: Invalid regulator (-6) > ... > arm_big_little: bL_cpufreq_register: Failed registering platform driver: > vexpress-spc, err: -17 > > The actual driver fails to initialise as cpufreq-dt is probed > successfully, which is incorrect. This issue can happen to any platform > not using cpufreq-dt in a multi-platform build. > > This patch adds a check to do selective initialization of the driver. > > Cc: Lee Jones> Cc: "Rafael J. Wysocki" > Cc: Viresh Kumar > Cc: linux...@vger.kernel.org > Fixes: ab0ea257fc58 ("cpufreq: st: Provide runtime initialised driver for > ST's platforms") > Signed-off-by: Sudeep Holla > --- > drivers/cpufreq/sti-cpufreq.c | 4 > 1 file changed, 4 insertions(+) > > v1->v2: > - Updated compatible strings as per Lee's confirmation > > diff --git a/drivers/cpufreq/sti-cpufreq.c b/drivers/cpufreq/sti-cpufreq.c > index a9c659f58974..04042038ec4b 100644 > --- a/drivers/cpufreq/sti-cpufreq.c > +++ b/drivers/cpufreq/sti-cpufreq.c > @@ -259,6 +259,10 @@ static int sti_cpufreq_init(void) > { > int ret; > > + if ((!of_machine_is_compatible("st,stih407")) && > + (!of_machine_is_compatible("st,stih410"))) > + return -ENODEV; > + > ddata.cpu = get_cpu_device(0); > if (!ddata.cpu) { > dev_err(ddata.cpu, "Failed to get device for CPU0\n"); Acked-by: Viresh Kumar -- viresh
Re: [PATCH v2] cpufreq: st: enable selective initialization based on the platform
On 27-04-16, 17:18, Sudeep Holla wrote: > The sti-cpufreq does unconditional registration of the cpufreq-dt driver > which causes issue on an multi-platform build. For example, on Vexpress > TC2 platform, we get the following error on boot: > > cpu cpu0: OPP-v2 not supported > cpu cpu0: Not doing voltage scaling > cpu: dev_pm_opp_of_cpumask_add_table: couldn't find opp table > for cpu:0, -19 > cpu cpu0: dev_pm_opp_get_max_volt_latency: Invalid regulator (-6) > ... > arm_big_little: bL_cpufreq_register: Failed registering platform driver: > vexpress-spc, err: -17 > > The actual driver fails to initialise as cpufreq-dt is probed > successfully, which is incorrect. This issue can happen to any platform > not using cpufreq-dt in a multi-platform build. > > This patch adds a check to do selective initialization of the driver. > > Cc: Lee Jones > Cc: "Rafael J. Wysocki" > Cc: Viresh Kumar > Cc: linux...@vger.kernel.org > Fixes: ab0ea257fc58 ("cpufreq: st: Provide runtime initialised driver for > ST's platforms") > Signed-off-by: Sudeep Holla > --- > drivers/cpufreq/sti-cpufreq.c | 4 > 1 file changed, 4 insertions(+) > > v1->v2: > - Updated compatible strings as per Lee's confirmation > > diff --git a/drivers/cpufreq/sti-cpufreq.c b/drivers/cpufreq/sti-cpufreq.c > index a9c659f58974..04042038ec4b 100644 > --- a/drivers/cpufreq/sti-cpufreq.c > +++ b/drivers/cpufreq/sti-cpufreq.c > @@ -259,6 +259,10 @@ static int sti_cpufreq_init(void) > { > int ret; > > + if ((!of_machine_is_compatible("st,stih407")) && > + (!of_machine_is_compatible("st,stih410"))) > + return -ENODEV; > + > ddata.cpu = get_cpu_device(0); > if (!ddata.cpu) { > dev_err(ddata.cpu, "Failed to get device for CPU0\n"); Acked-by: Viresh Kumar -- viresh
Re: [PATCH] cpufreq: governor: Change confusing struct field and variable names
On 28-04-16, 01:19, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki> > The name of the prev_cpu_wall field in struct cpu_dbs_info is > confusing, because it doesn't represent wall time, but the previous > update time as returned by get_cpu_idle_time() (that may be the > current value of jiffies_64 in some cases, for example). > > Moreover, the names of some related variables in dbs_update() take > that confusion further. > > Rename all of those things to make their names reflect the purpose > more accurately. While at it, drop unnecessary parens from one of > the updated expressions. > > No functional changes. > > Signed-off-by: Rafael J. Wysocki > --- > drivers/cpufreq/cpufreq_governor.c | 22 +++--- > drivers/cpufreq/cpufreq_governor.h |2 +- > 2 files changed, 12 insertions(+), 12 deletions(-) Acked-by: Viresh Kumar -- viresh
Re: [PATCH] cpufreq: governor: Change confusing struct field and variable names
On 28-04-16, 01:19, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki > > The name of the prev_cpu_wall field in struct cpu_dbs_info is > confusing, because it doesn't represent wall time, but the previous > update time as returned by get_cpu_idle_time() (that may be the > current value of jiffies_64 in some cases, for example). > > Moreover, the names of some related variables in dbs_update() take > that confusion further. > > Rename all of those things to make their names reflect the purpose > more accurately. While at it, drop unnecessary parens from one of > the updated expressions. > > No functional changes. > > Signed-off-by: Rafael J. Wysocki > --- > drivers/cpufreq/cpufreq_governor.c | 22 +++--- > drivers/cpufreq/cpufreq_governor.h |2 +- > 2 files changed, 12 insertions(+), 12 deletions(-) Acked-by: Viresh Kumar -- viresh
Re: [PATCH perf/core v5 04/15] perf probe: Add --cache option to cache the probe definitions
On Thu, Apr 28, 2016 at 03:37:42AM +0900, Masami Hiramatsu wrote: > From: Masami Hiramatsu> > Add --cache option to cache the probe definitions. This > just saves the result of the dwarf analysis to probe cache. > > Signed-off-by: Masami Hiramatsu > Signed-off-by: Masami Hiramatsu > --- > Changes in v5: > - Move probe_cache* definitions. (code cleanup) > > Changes in v4: > - Remove cache saving failure message. > --- [snip] > +static int probe_cache__load(struct probe_cache *pcache) > +{ > + struct probe_cache_entry *entry = NULL; > + char buf[MAX_CMDLEN], *p; > + int ret = 0; > + FILE *fp; > + > + fp = fdopen(dup(pcache->fd), "r"); > + while (!feof(fp)) { > + if (!fgets(buf, MAX_CMDLEN, fp)) > + break; > + p = strchr(buf, '\n'); > + if (p) > + *p = '\0'; > + if (buf[0] == '#') {/* #perf_probe_event */ > + entry = probe_cache_entry__new(NULL); The probe_cache_entry__new() can fail. Thanks, Namhyung > + entry->spev = strdup(buf + 1); > + ret = parse_perf_probe_command(buf + 1, >pev); > + if (!entry->spev || ret < 0) { > + probe_cache_entry__delete(entry); > + goto out; > + } > + list_add_tail(>list, >list); > + } else {/* trace_probe_event */ > + if (!entry) { > + ret = -EINVAL; > + goto out; > + } > + strlist__add(entry->tevlist, buf); > + } > + } > +out: > + fclose(fp); > + return ret; > +}
Re: [PATCH perf/core v5 04/15] perf probe: Add --cache option to cache the probe definitions
On Thu, Apr 28, 2016 at 03:37:42AM +0900, Masami Hiramatsu wrote: > From: Masami Hiramatsu > > Add --cache option to cache the probe definitions. This > just saves the result of the dwarf analysis to probe cache. > > Signed-off-by: Masami Hiramatsu > Signed-off-by: Masami Hiramatsu > --- > Changes in v5: > - Move probe_cache* definitions. (code cleanup) > > Changes in v4: > - Remove cache saving failure message. > --- [snip] > +static int probe_cache__load(struct probe_cache *pcache) > +{ > + struct probe_cache_entry *entry = NULL; > + char buf[MAX_CMDLEN], *p; > + int ret = 0; > + FILE *fp; > + > + fp = fdopen(dup(pcache->fd), "r"); > + while (!feof(fp)) { > + if (!fgets(buf, MAX_CMDLEN, fp)) > + break; > + p = strchr(buf, '\n'); > + if (p) > + *p = '\0'; > + if (buf[0] == '#') {/* #perf_probe_event */ > + entry = probe_cache_entry__new(NULL); The probe_cache_entry__new() can fail. Thanks, Namhyung > + entry->spev = strdup(buf + 1); > + ret = parse_perf_probe_command(buf + 1, >pev); > + if (!entry->spev || ret < 0) { > + probe_cache_entry__delete(entry); > + goto out; > + } > + list_add_tail(>list, >list); > + } else {/* trace_probe_event */ > + if (!entry) { > + ret = -EINVAL; > + goto out; > + } > + strlist__add(entry->tevlist, buf); > + } > + } > +out: > + fclose(fp); > + return ret; > +}
Re: [PATCH v6 07/12] usb: otg: add OTG/dual-role core
On Wed, Apr 27, 2016 at 01:59:44PM +0300, Roger Quadros wrote: > Hi, > > On 27/04/16 06:15, Peter Chen wrote: > > On Tue, Apr 26, 2016 at 04:21:07PM +0800, Peter Chen wrote: > >> On Tue, Apr 26, 2016 at 07:00:22AM +, Jun Li wrote: > >>> Hi > >>> > -Original Message- > From: Peter Chen [mailto:hzpeterc...@gmail.com] > Sent: Tuesday, April 26, 2016 2:28 PM > To: Jun Li> Cc: Roger Quadros ; st...@rowland.harvard.edu; > ba...@kernel.org; gre...@linuxfoundation.org; peter.c...@freescale.com; > dan.j.willi...@intel.com; jun...@freescale.com; > mathias.ny...@linux.intel.com; t...@atomide.com; joao.pi...@synopsys.com; > abres...@chromium.org; r.bald...@samsung.com; linux-...@vger.kernel.org; > linux-kernel@vger.kernel.org; linux-o...@vger.kernel.org > Subject: Re: [PATCH v6 07/12] usb: otg: add OTG/dual-role core > > On Tue, Apr 26, 2016 at 05:11:36AM +, Jun Li wrote: > > Hi > > > >> -Original Message- > >> From: Peter Chen [mailto:hzpeterc...@gmail.com] > >> Sent: Tuesday, April 26, 2016 11:47 AM > >> To: Jun Li > >> Cc: Roger Quadros ; st...@rowland.harvard.edu; > >> ba...@kernel.org; gre...@linuxfoundation.org; > >> peter.c...@freescale.com; dan.j.willi...@intel.com; > >> jun...@freescale.com; mathias.ny...@linux.intel.com; > >> t...@atomide.com; joao.pi...@synopsys.com; abres...@chromium.org; > >> r.bald...@samsung.com; linux-...@vger.kernel.org; > >> linux-kernel@vger.kernel.org; linux-o...@vger.kernel.org > >> Subject: Re: [PATCH v6 07/12] usb: otg: add OTG/dual-role core > >> > >> On Tue, Apr 26, 2016 at 02:07:56AM +, Jun Li wrote: > +struct usb_otg *usb_otg_register(struct device *dev, > + struct usb_otg_config *config) { > +struct usb_otg *otg; > +struct otg_wait_data *wait; > +int ret = 0; > + > +if (!dev || !config || !config->fsm_ops) > +return ERR_PTR(-EINVAL); > + > +/* already in list? */ > +mutex_lock(_list_mutex); > +if (usb_otg_get_data(dev)) { > +dev_err(dev, "otg: %s: device already in otg list\n", > +__func__); > +ret = -EINVAL; > +goto unlock; > +} > + > +/* allocate and add to list */ > +otg = kzalloc(sizeof(*otg), GFP_KERNEL); > +if (!otg) { > +ret = -ENOMEM; > +goto unlock; > +} > + > +otg->dev = dev; > +otg->caps = config->otg_caps; > + > +if ((otg->caps->hnp_support || otg->caps->srp_support || > + otg->caps->adp_support) && !config->otg_work) > +dev_info(dev, "otg: limiting to dual-role\n"); > >>> > >>> dev_err, this should be an error. > >> > >> The condition may be wrong, but it is an information to show that > >> current OTG is dual-role. > > > > This should not happen in any correct design, I even doubt if we > > should try to continue by "downgrade" it to be duel role, currently > > the only example user is dual role, so doing like this can't be tested > > by real case, this downgrade is not so easy like we image, at least > > for chipidea otg driver, simply replace a queue worker may not work, > > as we have much more difference between the 2 configs. > > > > Would you show more why chipidea can't work just replace the work item, > and see if anything we still can improve for this framework? > >>> > >>> In real OTG, we need enable AVV irq, > >> > >> Enable and Handling AVV is platform stuff. In this framework, we are > >> focus on how otg device manages host and gadget together, and the state > >> machine when the related otg event occurs. > >> > >>> but for duel role, nobody care/handle, > >>> there are much more resource required for OTG: timers, hnp polling, > >>> otg test device handling... > >> > >> They are common things for fully OTG fsm, you can move them > >> to common code (In fact, hnp polling handling is already common code). > >> > >>> > >>> with current design, chipidea driver can support real OTG with its own > >>> queue worker, or DRD with Roger's drd work item if config is correct. > >>> > >>> But improve something to work on a *wrong* config will make it complicated > >>> and does not make much sense IMO. > >>> > >> > >> What does above "config" you mean? > >> > >> If the configure is fully OTG, you can choose different state machine, > >> eg otg_statemachine, if you find it is hard for chipidea to use this > >> framework, just list the reason, and see if we can improve. > >> > > > > Roger, after
Re: [PATCH v6 07/12] usb: otg: add OTG/dual-role core
On Wed, Apr 27, 2016 at 01:59:44PM +0300, Roger Quadros wrote: > Hi, > > On 27/04/16 06:15, Peter Chen wrote: > > On Tue, Apr 26, 2016 at 04:21:07PM +0800, Peter Chen wrote: > >> On Tue, Apr 26, 2016 at 07:00:22AM +, Jun Li wrote: > >>> Hi > >>> > -Original Message- > From: Peter Chen [mailto:hzpeterc...@gmail.com] > Sent: Tuesday, April 26, 2016 2:28 PM > To: Jun Li > Cc: Roger Quadros ; st...@rowland.harvard.edu; > ba...@kernel.org; gre...@linuxfoundation.org; peter.c...@freescale.com; > dan.j.willi...@intel.com; jun...@freescale.com; > mathias.ny...@linux.intel.com; t...@atomide.com; joao.pi...@synopsys.com; > abres...@chromium.org; r.bald...@samsung.com; linux-...@vger.kernel.org; > linux-kernel@vger.kernel.org; linux-o...@vger.kernel.org > Subject: Re: [PATCH v6 07/12] usb: otg: add OTG/dual-role core > > On Tue, Apr 26, 2016 at 05:11:36AM +, Jun Li wrote: > > Hi > > > >> -Original Message- > >> From: Peter Chen [mailto:hzpeterc...@gmail.com] > >> Sent: Tuesday, April 26, 2016 11:47 AM > >> To: Jun Li > >> Cc: Roger Quadros ; st...@rowland.harvard.edu; > >> ba...@kernel.org; gre...@linuxfoundation.org; > >> peter.c...@freescale.com; dan.j.willi...@intel.com; > >> jun...@freescale.com; mathias.ny...@linux.intel.com; > >> t...@atomide.com; joao.pi...@synopsys.com; abres...@chromium.org; > >> r.bald...@samsung.com; linux-...@vger.kernel.org; > >> linux-kernel@vger.kernel.org; linux-o...@vger.kernel.org > >> Subject: Re: [PATCH v6 07/12] usb: otg: add OTG/dual-role core > >> > >> On Tue, Apr 26, 2016 at 02:07:56AM +, Jun Li wrote: > +struct usb_otg *usb_otg_register(struct device *dev, > + struct usb_otg_config *config) { > +struct usb_otg *otg; > +struct otg_wait_data *wait; > +int ret = 0; > + > +if (!dev || !config || !config->fsm_ops) > +return ERR_PTR(-EINVAL); > + > +/* already in list? */ > +mutex_lock(_list_mutex); > +if (usb_otg_get_data(dev)) { > +dev_err(dev, "otg: %s: device already in otg list\n", > +__func__); > +ret = -EINVAL; > +goto unlock; > +} > + > +/* allocate and add to list */ > +otg = kzalloc(sizeof(*otg), GFP_KERNEL); > +if (!otg) { > +ret = -ENOMEM; > +goto unlock; > +} > + > +otg->dev = dev; > +otg->caps = config->otg_caps; > + > +if ((otg->caps->hnp_support || otg->caps->srp_support || > + otg->caps->adp_support) && !config->otg_work) > +dev_info(dev, "otg: limiting to dual-role\n"); > >>> > >>> dev_err, this should be an error. > >> > >> The condition may be wrong, but it is an information to show that > >> current OTG is dual-role. > > > > This should not happen in any correct design, I even doubt if we > > should try to continue by "downgrade" it to be duel role, currently > > the only example user is dual role, so doing like this can't be tested > > by real case, this downgrade is not so easy like we image, at least > > for chipidea otg driver, simply replace a queue worker may not work, > > as we have much more difference between the 2 configs. > > > > Would you show more why chipidea can't work just replace the work item, > and see if anything we still can improve for this framework? > >>> > >>> In real OTG, we need enable AVV irq, > >> > >> Enable and Handling AVV is platform stuff. In this framework, we are > >> focus on how otg device manages host and gadget together, and the state > >> machine when the related otg event occurs. > >> > >>> but for duel role, nobody care/handle, > >>> there are much more resource required for OTG: timers, hnp polling, > >>> otg test device handling... > >> > >> They are common things for fully OTG fsm, you can move them > >> to common code (In fact, hnp polling handling is already common code). > >> > >>> > >>> with current design, chipidea driver can support real OTG with its own > >>> queue worker, or DRD with Roger's drd work item if config is correct. > >>> > >>> But improve something to work on a *wrong* config will make it complicated > >>> and does not make much sense IMO. > >>> > >> > >> What does above "config" you mean? > >> > >> If the configure is fully OTG, you can choose different state machine, > >> eg otg_statemachine, if you find it is hard for chipidea to use this > >> framework, just list the reason, and see if we can improve. > >> > > > > Roger, after discussing with Jun off line, we think usb_otg_register > > should
linux-next: build failure after merge of the drm tree
Hi Dave, After merging the drm tree, today's linux-next build (x86_64 allmodconfig) failed like this: drivers/gpu/drm/i915/intel_ddi.c: In function 'intel_prepare_ddi_buffer': drivers/gpu/drm/i915/intel_ddi.c:447:15: error: 'struct drm_i915_private' has no member named 'edp_low_vswing' if (dev_priv->edp_low_vswing) { ^ Caused by commit 06411f08b3f3 ("drm/i915: move edp low vswing config to vbt data") interacting with commit 992e7a41f9fc ("drm/i915: Fix eDP low vswing for Broadwell") from the drm-intel-fixes tree. I applied the following merge fixup patch (which is suggested by the "v3: Change dev_priv->edp_low_vswing to use dev_priv->vbt.edp.low_vswing" comment in the drm-intel-fixes tree patch, but clearly could not be done there). From: Stephen RothwellDate: Thu, 28 Apr 2016 11:53:36 +1000 Subject: [PATCH] drm/i915: fix up for edp_low_vswing change Signed-off-by: Stephen Rothwell --- drivers/gpu/drm/i915/intel_ddi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 6080b5481984..e304857ba405 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -444,7 +444,7 @@ void intel_prepare_ddi_buffer(struct intel_encoder *encoder) ddi_translations_fdi = bdw_ddi_translations_fdi; ddi_translations_dp = bdw_ddi_translations_dp; - if (dev_priv->edp_low_vswing) { + if (dev_priv->vbt.edp.low_vswing) { ddi_translations_edp = bdw_ddi_translations_edp; n_edp_entries = ARRAY_SIZE(bdw_ddi_translations_edp); } else { -- 2.7.0 -- Cheers, Stephen Rothwell
linux-next: build failure after merge of the drm tree
Hi Dave, After merging the drm tree, today's linux-next build (x86_64 allmodconfig) failed like this: drivers/gpu/drm/i915/intel_ddi.c: In function 'intel_prepare_ddi_buffer': drivers/gpu/drm/i915/intel_ddi.c:447:15: error: 'struct drm_i915_private' has no member named 'edp_low_vswing' if (dev_priv->edp_low_vswing) { ^ Caused by commit 06411f08b3f3 ("drm/i915: move edp low vswing config to vbt data") interacting with commit 992e7a41f9fc ("drm/i915: Fix eDP low vswing for Broadwell") from the drm-intel-fixes tree. I applied the following merge fixup patch (which is suggested by the "v3: Change dev_priv->edp_low_vswing to use dev_priv->vbt.edp.low_vswing" comment in the drm-intel-fixes tree patch, but clearly could not be done there). From: Stephen Rothwell Date: Thu, 28 Apr 2016 11:53:36 +1000 Subject: [PATCH] drm/i915: fix up for edp_low_vswing change Signed-off-by: Stephen Rothwell --- drivers/gpu/drm/i915/intel_ddi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 6080b5481984..e304857ba405 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -444,7 +444,7 @@ void intel_prepare_ddi_buffer(struct intel_encoder *encoder) ddi_translations_fdi = bdw_ddi_translations_fdi; ddi_translations_dp = bdw_ddi_translations_dp; - if (dev_priv->edp_low_vswing) { + if (dev_priv->vbt.edp.low_vswing) { ddi_translations_edp = bdw_ddi_translations_edp; n_edp_entries = ARRAY_SIZE(bdw_ddi_translations_edp); } else { -- 2.7.0 -- Cheers, Stephen Rothwell
Re: [PATCH perf/core v5 02/15] perf-buildid-cache: Use path/to/bin/buildid/elf instead of path/to/bin/buildid
On Wed, 27 Apr 2016 18:12:32 -0300 Arnaldo Carvalho de Melowrote: > Em Thu, Apr 28, 2016 at 03:37:23AM +0900, Masami Hiramatsu escreveu: > > From: Masami Hiramatsu > > > > Use path/to/bin/buildid/elf instead of path/to/bin/buildid > > to store corresponding elf binary. > > This also stores vdso in buildid/vdso, kallsyms in buildid/kallsyms. > > > > Note that the existing caches are not updated until user adds > > or updates the cache. Anyway, if there is the old style build-id > > cache it falls back to use it. (IOW, it is backward compatible) > > I reworded this to: > > "If there is old style build-id cache content, i.e. links to > path/to/bin/buildid/aa/bbccdd, it will still be accessible, IOW, this is > backard compatible." > > This part "existing caches are not updated until the user updates de > cache" sounds confusing. Ah, I see. Thank you! > > Anyway, applied. > > - Arnaldo -- Masami Hiramatsu
Re: [PATCH perf/core v5 02/15] perf-buildid-cache: Use path/to/bin/buildid/elf instead of path/to/bin/buildid
On Wed, 27 Apr 2016 18:12:32 -0300 Arnaldo Carvalho de Melo wrote: > Em Thu, Apr 28, 2016 at 03:37:23AM +0900, Masami Hiramatsu escreveu: > > From: Masami Hiramatsu > > > > Use path/to/bin/buildid/elf instead of path/to/bin/buildid > > to store corresponding elf binary. > > This also stores vdso in buildid/vdso, kallsyms in buildid/kallsyms. > > > > Note that the existing caches are not updated until user adds > > or updates the cache. Anyway, if there is the old style build-id > > cache it falls back to use it. (IOW, it is backward compatible) > > I reworded this to: > > "If there is old style build-id cache content, i.e. links to > path/to/bin/buildid/aa/bbccdd, it will still be accessible, IOW, this is > backard compatible." > > This part "existing caches are not updated until the user updates de > cache" sounds confusing. Ah, I see. Thank you! > > Anyway, applied. > > - Arnaldo -- Masami Hiramatsu
Re: [PATCH perf/core v5 02/15] perf-buildid-cache: Use path/to/bin/buildid/elf instead of path/to/bin/buildid
On Wed, 27 Apr 2016 18:23:50 -0300 Arnaldo Carvalho de Melowrote: > Em Thu, Apr 28, 2016 at 03:37:23AM +0900, Masami Hiramatsu escreveu: > > From: Masami Hiramatsu > > > > Use path/to/bin/buildid/elf instead of path/to/bin/buildid > > to store corresponding elf binary. > > This also stores vdso in buildid/vdso, kallsyms in buildid/kallsyms. > > > > Note that the existing caches are not updated until user adds > > or updates the cache. Anyway, if there is the old style build-id > > cache it falls back to use it. (IOW, it is backward compatible) > > > > Signed-off-by: Masami Hiramatsu > > Signed-off-by: Masami Hiramatsu > > --- > > Changes in v5: > > - Support old style buildid caches. > > > + /* Remove old style build-id cache */ > > + if (__is_regular_file(dir_name)) > > + if (unlink(dir_name)) > > + goto out_free; > > Shouldn't this just fail and require the user to use --force/-f or purge > first? No, I don't think so, because the old style build-id is useless for probe-cache in anyway, and after applying this patch, perf can use new one seemlessly. So let just renew it :) Thank you, -- Masami Hiramatsu
Re: [PATCH perf/core v5 02/15] perf-buildid-cache: Use path/to/bin/buildid/elf instead of path/to/bin/buildid
On Wed, 27 Apr 2016 18:23:50 -0300 Arnaldo Carvalho de Melo wrote: > Em Thu, Apr 28, 2016 at 03:37:23AM +0900, Masami Hiramatsu escreveu: > > From: Masami Hiramatsu > > > > Use path/to/bin/buildid/elf instead of path/to/bin/buildid > > to store corresponding elf binary. > > This also stores vdso in buildid/vdso, kallsyms in buildid/kallsyms. > > > > Note that the existing caches are not updated until user adds > > or updates the cache. Anyway, if there is the old style build-id > > cache it falls back to use it. (IOW, it is backward compatible) > > > > Signed-off-by: Masami Hiramatsu > > Signed-off-by: Masami Hiramatsu > > --- > > Changes in v5: > > - Support old style buildid caches. > > > + /* Remove old style build-id cache */ > > + if (__is_regular_file(dir_name)) > > + if (unlink(dir_name)) > > + goto out_free; > > Shouldn't this just fail and require the user to use --force/-f or purge > first? No, I don't think so, because the old style build-id is useless for probe-cache in anyway, and after applying this patch, perf can use new one seemlessly. So let just renew it :) Thank you, -- Masami Hiramatsu
Re: powerpc: wire up preadv2 and pwritev2 syscalls
On Tue, 2016-19-04 at 12:23:36 UTC, Rui Salvaterra wrote: > Wire up preadv2/pwritev2 in the same way as preadv/pwritev. Fixes two > build warnings on ppc64. > > Signed-off-by: Rui SalvaterraApplied to powerpc fixes, thanks. https://git.kernel.org/powerpc/c/d701cca6744fe0d67c86346dcf cheers
Re: powerpc: wire up preadv2 and pwritev2 syscalls
On Tue, 2016-19-04 at 12:23:36 UTC, Rui Salvaterra wrote: > Wire up preadv2/pwritev2 in the same way as preadv/pwritev. Fixes two > build warnings on ppc64. > > Signed-off-by: Rui Salvaterra Applied to powerpc fixes, thanks. https://git.kernel.org/powerpc/c/d701cca6744fe0d67c86346dcf cheers
Re: [PATCH v2] livepatch: Add some basic LivePatch documentation
On 28/04/16 06:08, Jiri Kosina wrote: > On Wed, 27 Apr 2016, Jiri Kosina wrote: > >> I've incorporated most of the feedback (*) and pushed out to >> livepatching.git#for-4.7/livepatching-doc so everybody please send any >> followup documentation patches on top of that branch. > > (*) Balbir, some of your comments were a bit too vague; if you could turn > them into actual patch on top of what's currently in git, that'd be > helpful > Hi, Jiri Thanks! I'll try to do that -- I'll add it on my TODO list. Balbir Singh.
Re: [PATCH v2] livepatch: Add some basic LivePatch documentation
On 28/04/16 06:08, Jiri Kosina wrote: > On Wed, 27 Apr 2016, Jiri Kosina wrote: > >> I've incorporated most of the feedback (*) and pushed out to >> livepatching.git#for-4.7/livepatching-doc so everybody please send any >> followup documentation patches on top of that branch. > > (*) Balbir, some of your comments were a bit too vague; if you could turn > them into actual patch on top of what's currently in git, that'd be > helpful > Hi, Jiri Thanks! I'll try to do that -- I'll add it on my TODO list. Balbir Singh.
[PATCH net v3 5/5] drivers: net: cpsw: use of_phy_connect() in fixed-link case
From: David RivshinIf a fixed-link DT subnode is used, the phy_device was looked up so that a PHY ID string could be constructed and passed to phy_connect(). This is not necessary, as the device_node can be passed directly to of_phy_connect() instead. This reuses the same codepath as if the phy-handle DT property was used. Signed-off-by: David Rivshin Tested-by: Nicolas Chauvet Tested-by: Andrew Goodbody Reviewed-by: Mugunthan V N Reviewed-by: Grygorii Strashko --- Changes since v2 [1]: - Added Tested-by from Andrew Goodbody [3] - Added Reviewed-by from Mugunthan V N [4] - Added Reviewed-by from Grygorii Strashko [5] Changes since v1 [2]: - Rebased (trivial conflict, e5a03bfd modified the deleted snprintf) - Added Tested-by from Nicolas Chauvet [1] http://patchwork.ozlabs.org/patch/613276/ [2] http://patchwork.ozlabs.org/patch/560327/ [3] https://lkml.org/lkml/2016/4/22/537 [4] https://lkml.org/lkml/2016/4/22/63 [5] https://lkml.org/lkml/2016/4/22/529 drivers/net/ethernet/ti/cpsw.c | 11 +-- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c index 712bc6d..e2fcdf1 100644 --- a/drivers/net/ethernet/ti/cpsw.c +++ b/drivers/net/ethernet/ti/cpsw.c @@ -2044,30 +2044,21 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data, "phy-handle", 0); parp = of_get_property(slave_node, "phy_id", ); if (slave_data->phy_node) { dev_dbg(>dev, "slave[%d] using phy-handle=\"%s\"\n", i, slave_data->phy_node->full_name); } else if (of_phy_is_fixed_link(slave_node)) { - struct device_node *phy_node; - struct phy_device *phy_dev; - /* In the case of a fixed PHY, the DT node associated * to the PHY is the Ethernet MAC DT node. */ ret = of_phy_register_fixed_link(slave_node); if (ret) return ret; - phy_node = of_node_get(slave_node); - phy_dev = of_phy_find_device(phy_node); - if (!phy_dev) - return -ENODEV; - snprintf(slave_data->phy_id, sizeof(slave_data->phy_id), -PHY_ID_FMT, phy_dev->mdio.bus->id, -phy_dev->mdio.addr); + slave_data->phy_node = of_node_get(slave_node); } else if (parp) { u32 phyid; struct device_node *mdio_node; struct platform_device *mdio; if (lenp != (sizeof(__be32) * 2)) { dev_err(>dev, "Invalid slave[%d] phy_id property\n", i); -- 2.5.5
[PATCH net v3 5/5] drivers: net: cpsw: use of_phy_connect() in fixed-link case
From: David Rivshin If a fixed-link DT subnode is used, the phy_device was looked up so that a PHY ID string could be constructed and passed to phy_connect(). This is not necessary, as the device_node can be passed directly to of_phy_connect() instead. This reuses the same codepath as if the phy-handle DT property was used. Signed-off-by: David Rivshin Tested-by: Nicolas Chauvet Tested-by: Andrew Goodbody Reviewed-by: Mugunthan V N Reviewed-by: Grygorii Strashko --- Changes since v2 [1]: - Added Tested-by from Andrew Goodbody [3] - Added Reviewed-by from Mugunthan V N [4] - Added Reviewed-by from Grygorii Strashko [5] Changes since v1 [2]: - Rebased (trivial conflict, e5a03bfd modified the deleted snprintf) - Added Tested-by from Nicolas Chauvet [1] http://patchwork.ozlabs.org/patch/613276/ [2] http://patchwork.ozlabs.org/patch/560327/ [3] https://lkml.org/lkml/2016/4/22/537 [4] https://lkml.org/lkml/2016/4/22/63 [5] https://lkml.org/lkml/2016/4/22/529 drivers/net/ethernet/ti/cpsw.c | 11 +-- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c index 712bc6d..e2fcdf1 100644 --- a/drivers/net/ethernet/ti/cpsw.c +++ b/drivers/net/ethernet/ti/cpsw.c @@ -2044,30 +2044,21 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data, "phy-handle", 0); parp = of_get_property(slave_node, "phy_id", ); if (slave_data->phy_node) { dev_dbg(>dev, "slave[%d] using phy-handle=\"%s\"\n", i, slave_data->phy_node->full_name); } else if (of_phy_is_fixed_link(slave_node)) { - struct device_node *phy_node; - struct phy_device *phy_dev; - /* In the case of a fixed PHY, the DT node associated * to the PHY is the Ethernet MAC DT node. */ ret = of_phy_register_fixed_link(slave_node); if (ret) return ret; - phy_node = of_node_get(slave_node); - phy_dev = of_phy_find_device(phy_node); - if (!phy_dev) - return -ENODEV; - snprintf(slave_data->phy_id, sizeof(slave_data->phy_id), -PHY_ID_FMT, phy_dev->mdio.bus->id, -phy_dev->mdio.addr); + slave_data->phy_node = of_node_get(slave_node); } else if (parp) { u32 phyid; struct device_node *mdio_node; struct platform_device *mdio; if (lenp != (sizeof(__be32) * 2)) { dev_err(>dev, "Invalid slave[%d] phy_id property\n", i); -- 2.5.5
[PATCH net v3 4/5] dt: cpsw: phy-handle, phy_id, and fixed-link are mutually exclusive
From: David RivshinThe phy-handle, phy_id, and fixed-link properties are mutually exclusive, and only one need be specified. Make this clear in the binding doc. Also mark the phy_id property as deprecated, as phy-handle should be used instead. Signed-off-by: David Rivshin --- Changes since v2 [1]: - split from previous patch 2 - marked the phy_id property as deprecated [3] - removed Rob Herring's Acked-by due to above change Changes since v1 [2]: - Rebased (no conflicts) - Added Tested-by from Nicolas Chauvet - Added Acked-by from Rob Herring for the binding change [1] http://patchwork.ozlabs.org/patch/613260/ [2] http://patchwork.ozlabs.org/patch/560324/ [3] https://lkml.org/lkml/2016/4/22/494 Documentation/devicetree/bindings/net/cpsw.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt index 28a4781..0ae0649 100644 --- a/Documentation/devicetree/bindings/net/cpsw.txt +++ b/Documentation/devicetree/bindings/net/cpsw.txt @@ -41,21 +41,21 @@ Optional properties: Slave Properties: Required properties: - phy-mode : See ethernet.txt file in the same directory Optional properties: - dual_emac_res_vlan : Specifies VID to be used to segregate the ports - mac-address : See ethernet.txt file in the same directory -- phy_id : Specifies slave phy id +- phy_id : Specifies slave phy id (deprecated, use phy-handle) - phy-handle : See ethernet.txt file in the same directory Slave sub-nodes: - fixed-link : See fixed-link.txt file in the same directory - Either the property phy_id, or the sub-node - fixed-link can be specified + +Note: Exactly one of phy_id, phy-handle, or fixed-link must be specified. Note: "ti,hwmods" field is used to fetch the base address and irq resources from TI, omap hwmod data base during device registration. Future plan is to migrate hwmod data base contents into device tree blob so that, all the required data will be used from device tree dts file. -- 2.5.5
[PATCH net v3 4/5] dt: cpsw: phy-handle, phy_id, and fixed-link are mutually exclusive
From: David Rivshin The phy-handle, phy_id, and fixed-link properties are mutually exclusive, and only one need be specified. Make this clear in the binding doc. Also mark the phy_id property as deprecated, as phy-handle should be used instead. Signed-off-by: David Rivshin --- Changes since v2 [1]: - split from previous patch 2 - marked the phy_id property as deprecated [3] - removed Rob Herring's Acked-by due to above change Changes since v1 [2]: - Rebased (no conflicts) - Added Tested-by from Nicolas Chauvet - Added Acked-by from Rob Herring for the binding change [1] http://patchwork.ozlabs.org/patch/613260/ [2] http://patchwork.ozlabs.org/patch/560324/ [3] https://lkml.org/lkml/2016/4/22/494 Documentation/devicetree/bindings/net/cpsw.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt index 28a4781..0ae0649 100644 --- a/Documentation/devicetree/bindings/net/cpsw.txt +++ b/Documentation/devicetree/bindings/net/cpsw.txt @@ -41,21 +41,21 @@ Optional properties: Slave Properties: Required properties: - phy-mode : See ethernet.txt file in the same directory Optional properties: - dual_emac_res_vlan : Specifies VID to be used to segregate the ports - mac-address : See ethernet.txt file in the same directory -- phy_id : Specifies slave phy id +- phy_id : Specifies slave phy id (deprecated, use phy-handle) - phy-handle : See ethernet.txt file in the same directory Slave sub-nodes: - fixed-link : See fixed-link.txt file in the same directory - Either the property phy_id, or the sub-node - fixed-link can be specified + +Note: Exactly one of phy_id, phy-handle, or fixed-link must be specified. Note: "ti,hwmods" field is used to fetch the base address and irq resources from TI, omap hwmod data base during device registration. Future plan is to migrate hwmod data base contents into device tree blob so that, all the required data will be used from device tree dts file. -- 2.5.5
Re: [BUG] x86/efi: MMRs no longer properly mapped after switch to isolated page table
On Thu, Apr 28, 2016 at 12:51:22AM +0200, Borislav Petkov wrote: > On Wed, Apr 27, 2016 at 10:41:32AM -0500, Alex Thorlton wrote: > > A bit of digging will tell us that this is the failing line: > > > > m_n_config.v = uv_read_local_mmr(UVH_RH_GAM_CONFIG_MMR ); > > That looks like > > All code > >0: 65 48 03 05 1d b8 49add%gs:0x7e49b81d(%rip),%rax# > 0x7e49b825 >7: 7e >8: 80 78 14 02 cmpb $0x2,0x14(%rax) >c: ba 00 00 00 fa mov$0xfa00,%edx > 11: 76 0b jbe0x1e > 13: 48 89 c8mov%rcx,%rax > 16: 65 48 03 05 07 b8 49add%gs:0x7e49b807(%rip),%rax# > 0x7e49b825 > 1d: 7e > 1e: 48 b8 00 00 60 01 00movabs $0x88000160,%rax > 25: 88 ff ff > 28: 48 09 d0or %rdx,%rax > 2b:* 48 8b 00mov(%rax),%rax <-- trapping > instruction > 2e: 88 c3 mov%al,%bl > 30: 48 c1 e8 06 shr$0x6,%rax > 34: 41 bd 01 00 00 00 mov$0x1,%r13d > 3a: 88 c1 mov%al,%cl > 3c: 83 e3 3fand$0x3f,%ebx > > but why does this have anything to do with the EFI pagetable, at all? In this particular instance, it's not using the EFI page table - it's showing that the register isn't mapped into the main kernel page table, via the bad paging request. The issue isn't that there's something wrong with the EFI page table, but that something appears to be missing from the kernel table. > The MMRs should be mapped in the normal kernel page table, right? Well, quite a while back, these MMRs got mapped in using init_extra_mapping_uc in map_low_mmrs. Back then, yes, they were mapped straight into the kernel page table. After d2f7cbe7b26a74 ("x86/efi: Runtime services virtual mapping") was introduced (I'm sure you remember the BIOS issue we had a while back) we had to fall back to using EFI_OLD_MEMMAP, so for a while we relied on efi_ioremap. Eventually we got a BIOS fix that allowed us to start using the new memmap scheme, at which point we removed the init_extra_mapping_uc()s, since the efi_map_region code appeared to be doing what we needed. So, short answer is, they were mapped in and working up until now. > And your dirty fix of mapping into trampoline_pgd doesn't make any > sense... I *think* it does? This one took me a while to figure out - I wrote the fix at midnight last night, so please inform me if my logic sounds insane, haha. In efi_map_region we first do the __map_region for the physical address, which will get mapped into the lowest PGD entry of the trampoline_pgd. I believe the lowest PGD entry in the trampoline_pgd is actually the same as init_mm.pgd + pgd_index(PAGE_OFFSET) (see setup_real_mode), so you're effectively mapping into the kernel 1:1 space when you map into pgd_index 0 of the trampoline_pgd. By adding the mappings to the trampoline_pgd back in, I think I've also added them back into the kernel. > How do the MMRs get mapped on that box exactly? Believe I covered this above. Let me know if you have questions! > And why aren't they > mapped in the normal kernel page table all of a sudden? It looks to me like there might be a bit of a complicated chain of events that led us to this point. I thought I was close to having this worked out, but after looking back over it I'm starting to think, "how was it working in the first place?!" At this point, considering the fact that my fix definitely gets the UV to boot, I'm going to stick to my claim that adding the maps back into the trampoline_pgd fixes at least part of the issue - not saying it's the right way, but it's an intersting data point. Despite the fact that my fix might help remedy the situation, I will admit that my understanding of the mechanics behind said fix could be flawed :) I'm going to have to think about this some more tomorrow. Let me know if you have any ideas! - Alex
Re: [BUG] x86/efi: MMRs no longer properly mapped after switch to isolated page table
On Thu, Apr 28, 2016 at 12:51:22AM +0200, Borislav Petkov wrote: > On Wed, Apr 27, 2016 at 10:41:32AM -0500, Alex Thorlton wrote: > > A bit of digging will tell us that this is the failing line: > > > > m_n_config.v = uv_read_local_mmr(UVH_RH_GAM_CONFIG_MMR ); > > That looks like > > All code > >0: 65 48 03 05 1d b8 49add%gs:0x7e49b81d(%rip),%rax# > 0x7e49b825 >7: 7e >8: 80 78 14 02 cmpb $0x2,0x14(%rax) >c: ba 00 00 00 fa mov$0xfa00,%edx > 11: 76 0b jbe0x1e > 13: 48 89 c8mov%rcx,%rax > 16: 65 48 03 05 07 b8 49add%gs:0x7e49b807(%rip),%rax# > 0x7e49b825 > 1d: 7e > 1e: 48 b8 00 00 60 01 00movabs $0x88000160,%rax > 25: 88 ff ff > 28: 48 09 d0or %rdx,%rax > 2b:* 48 8b 00mov(%rax),%rax <-- trapping > instruction > 2e: 88 c3 mov%al,%bl > 30: 48 c1 e8 06 shr$0x6,%rax > 34: 41 bd 01 00 00 00 mov$0x1,%r13d > 3a: 88 c1 mov%al,%cl > 3c: 83 e3 3fand$0x3f,%ebx > > but why does this have anything to do with the EFI pagetable, at all? In this particular instance, it's not using the EFI page table - it's showing that the register isn't mapped into the main kernel page table, via the bad paging request. The issue isn't that there's something wrong with the EFI page table, but that something appears to be missing from the kernel table. > The MMRs should be mapped in the normal kernel page table, right? Well, quite a while back, these MMRs got mapped in using init_extra_mapping_uc in map_low_mmrs. Back then, yes, they were mapped straight into the kernel page table. After d2f7cbe7b26a74 ("x86/efi: Runtime services virtual mapping") was introduced (I'm sure you remember the BIOS issue we had a while back) we had to fall back to using EFI_OLD_MEMMAP, so for a while we relied on efi_ioremap. Eventually we got a BIOS fix that allowed us to start using the new memmap scheme, at which point we removed the init_extra_mapping_uc()s, since the efi_map_region code appeared to be doing what we needed. So, short answer is, they were mapped in and working up until now. > And your dirty fix of mapping into trampoline_pgd doesn't make any > sense... I *think* it does? This one took me a while to figure out - I wrote the fix at midnight last night, so please inform me if my logic sounds insane, haha. In efi_map_region we first do the __map_region for the physical address, which will get mapped into the lowest PGD entry of the trampoline_pgd. I believe the lowest PGD entry in the trampoline_pgd is actually the same as init_mm.pgd + pgd_index(PAGE_OFFSET) (see setup_real_mode), so you're effectively mapping into the kernel 1:1 space when you map into pgd_index 0 of the trampoline_pgd. By adding the mappings to the trampoline_pgd back in, I think I've also added them back into the kernel. > How do the MMRs get mapped on that box exactly? Believe I covered this above. Let me know if you have questions! > And why aren't they > mapped in the normal kernel page table all of a sudden? It looks to me like there might be a bit of a complicated chain of events that led us to this point. I thought I was close to having this worked out, but after looking back over it I'm starting to think, "how was it working in the first place?!" At this point, considering the fact that my fix definitely gets the UV to boot, I'm going to stick to my claim that adding the maps back into the trampoline_pgd fixes at least part of the issue - not saying it's the right way, but it's an intersting data point. Despite the fact that my fix might help remedy the situation, I will admit that my understanding of the mechanics behind said fix could be flawed :) I'm going to have to think about this some more tomorrow. Let me know if you have any ideas! - Alex
[PATCH 05/12] staging: lustre: obd: cleanup client import if client_obd_setup fails
From: Swapnil Pimpaleclient_obd_setup() allocates an obd_import which should be cleaned up if there is any failure afterwards in callers of client_obd_setup(). This patch fixes the bug in osc_setup(), mgc_setup(), mdc_setup() and lwp_setup(). The fix is to call obd_cleanup_client_import() before calling client_obd_cleanup() in case of an error. Signed-off-by: Swapnil Pimpale Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-3810 Reviewed-on: http://review.whamcloud.com/7561 Reviewed-by: John L. Hammond Reviewed-by: Lai Siyao Reviewed-by: Andreas Dilger Signed-off-by: James Simmons --- drivers/staging/lustre/lustre/ldlm/ldlm_lib.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_lib.c b/drivers/staging/lustre/lustre/ldlm/ldlm_lib.c index 932aef2..00c93f3 100644 --- a/drivers/staging/lustre/lustre/ldlm/ldlm_lib.c +++ b/drivers/staging/lustre/lustre/ldlm/ldlm_lib.c @@ -439,6 +439,7 @@ int client_obd_cleanup(struct obd_device *obddev) ldlm_namespace_free_post(obddev->obd_namespace); obddev->obd_namespace = NULL; + obd_cleanup_client_import(obddev); LASSERT(!obddev->u.cli.cl_import); ldlm_put_ref(); -- 2.7.4
[PATCH 05/12] staging: lustre: obd: cleanup client import if client_obd_setup fails
From: Swapnil Pimpale client_obd_setup() allocates an obd_import which should be cleaned up if there is any failure afterwards in callers of client_obd_setup(). This patch fixes the bug in osc_setup(), mgc_setup(), mdc_setup() and lwp_setup(). The fix is to call obd_cleanup_client_import() before calling client_obd_cleanup() in case of an error. Signed-off-by: Swapnil Pimpale Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-3810 Reviewed-on: http://review.whamcloud.com/7561 Reviewed-by: John L. Hammond Reviewed-by: Lai Siyao Reviewed-by: Andreas Dilger Signed-off-by: James Simmons --- drivers/staging/lustre/lustre/ldlm/ldlm_lib.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_lib.c b/drivers/staging/lustre/lustre/ldlm/ldlm_lib.c index 932aef2..00c93f3 100644 --- a/drivers/staging/lustre/lustre/ldlm/ldlm_lib.c +++ b/drivers/staging/lustre/lustre/ldlm/ldlm_lib.c @@ -439,6 +439,7 @@ int client_obd_cleanup(struct obd_device *obddev) ldlm_namespace_free_post(obddev->obd_namespace); obddev->obd_namespace = NULL; + obd_cleanup_client_import(obddev); LASSERT(!obddev->u.cli.cl_import); ldlm_put_ref(); -- 2.7.4
[PATCH 02/12] staging: lustre: obd: add newline for dumped config record
From: Amir ShehataThe function class_config_parse_rec() parses the llog record and places it into a buffer to be returned. That buffer needs to end with a newline which is currently missing. Signed-off-by: Amir Shehata Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-2149 Reviewed-on: http://review.whamcloud.com/4254 Reviewed-by: Alex Zhuravlev Reviewed-by: Andreas Dilger Signed-off-by: James Simmons --- drivers/staging/lustre/lustre/obdclass/obd_config.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/lustre/lustre/obdclass/obd_config.c b/drivers/staging/lustre/lustre/obdclass/obd_config.c index 084af1a..e60ef4a 100644 --- a/drivers/staging/lustre/lustre/obdclass/obd_config.c +++ b/drivers/staging/lustre/lustre/obdclass/obd_config.c @@ -1350,6 +1350,7 @@ static int class_config_parse_rec(struct llog_rec_hdr *rec, char *buf, lustre_cfg_string(lcfg, i)); } } + ptr += snprintf(ptr, end - ptr, "\n"); /* return consumed bytes */ rc = ptr - buf; return rc; -- 2.7.4
[PATCH 02/12] staging: lustre: obd: add newline for dumped config record
From: Amir Shehata The function class_config_parse_rec() parses the llog record and places it into a buffer to be returned. That buffer needs to end with a newline which is currently missing. Signed-off-by: Amir Shehata Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-2149 Reviewed-on: http://review.whamcloud.com/4254 Reviewed-by: Alex Zhuravlev Reviewed-by: Andreas Dilger Signed-off-by: James Simmons --- drivers/staging/lustre/lustre/obdclass/obd_config.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/lustre/lustre/obdclass/obd_config.c b/drivers/staging/lustre/lustre/obdclass/obd_config.c index 084af1a..e60ef4a 100644 --- a/drivers/staging/lustre/lustre/obdclass/obd_config.c +++ b/drivers/staging/lustre/lustre/obdclass/obd_config.c @@ -1350,6 +1350,7 @@ static int class_config_parse_rec(struct llog_rec_hdr *rec, char *buf, lustre_cfg_string(lcfg, i)); } } + ptr += snprintf(ptr, end - ptr, "\n"); /* return consumed bytes */ rc = ptr - buf; return rc; -- 2.7.4
Re: [PATCH] mm/zswap: use workqueue to destroy pool
Hello Dan, On (04/27/16 13:19), Dan Streetman wrote: [..] > > so in general the patch look good to me. > > > > it's either I didn't have enough coffee yet (which is true) or > > _IN THEORY_ it creates a tiny race condition; which is hard (and > > unlikely) to hit, but still. and the problem being is > > CONFIG_ZSMALLOC_STAT. > > Aha, thanks, I hadn't tested with that param enabled. However, the > patch doesn't create the race condition, that existed already. well, agree. it's not like zsmalloc race condition, but the way zsmalloc is managed (deferred destruction either via rcu or scheduled work). > It fails because the new zswap pool creates a new zpool using > zsmalloc, but it can't create the zsmalloc pool because there is > already one named 'zswap' so the stat dir can't be created. > > So...either zswap needs to provide a unique 'name' to each of its > zpools, or zsmalloc needs to modify its provided pool name in some way > (add a unique suffix maybe). Or both. > > It seems like zsmalloc should do the checking/modification - or, at > the very least, it should have consistent behavior regardless of the > CONFIG_ZSMALLOC_STAT setting. yes, zram guarantees that there won't be any name collisions. and the way it's working for zram, zram corresponds to zsmalloc. the bigger issue here (and I was thinking at some point of fixing it, but then I grepped to see how many API users are in there, and I gave up) is that it seems we have no way to check if the dir exists in debugfs. we call this function struct dentry *debugfs_create_dir(const char *name, struct dentry *parent) { struct dentry *dentry = start_creating(name, parent); struct inode *inode; if (IS_ERR(dentry)) return NULL; inode = debugfs_get_inode(dentry->d_sb); if (unlikely(!inode)) return failed_creating(dentry); inode->i_mode = S_IFDIR | S_IRWXU | S_IRUGO | S_IXUGO; inode->i_op = _dir_inode_operations; inode->i_fop = _dir_operations; /* directory inodes start off with i_nlink == 2 (for "." entry) */ inc_nlink(inode); d_instantiate(dentry, inode); inc_nlink(d_inode(dentry->d_parent)); fsnotify_mkdir(d_inode(dentry->d_parent), dentry); return end_creating(dentry); } and debugfs _does know_ that the directory ERR_PTR(-EEXIST), that's what start_creating()->lookup_one_len() return static struct dentry *start_creating(const char *name, struct dentry *parent) { struct dentry *dentry; int error; pr_debug("debugfs: creating file '%s'\n",name); if (IS_ERR(parent)) return parent; error = simple_pin_fs(_fs_type, _mount, _mount_count); if (error) return ERR_PTR(error); /* If the parent is not specified, we create it in the root. * We need the root dentry to do this, which is in the super * block. A pointer to that is in the struct vfsmount that we * have around. */ if (!parent) parent = debugfs_mount->mnt_root; inode_lock(d_inode(parent)); dentry = lookup_one_len(name, parent, strlen(name)); if (!IS_ERR(dentry) && d_really_is_positive(dentry)) { dput(dentry); dentry = ERR_PTR(-EEXIST); } if (IS_ERR(dentry)) { inode_unlock(d_inode(parent)); simple_release_fs(_mount, _mount_count); } return dentry; } but debugfs_create_dir() instead of propagating this error, it swallows it and simply return NULL, so we can't tell the difference between -EEXIST, OOM, or anything else. so doing this check in zsmalloc() is not so easy. /* well, I may be wrong here */ > However, it's easy to change zswap to provide a unique name for each > zpool creation, and zsmalloc's primary user (zram) guarantees to > provide a unique name for each pool created. So updating zswap is > probably best. if you can do it in zswap, then please do. -ss
[PATCH 06/12] staging: lustre: fid: add a connect flag for open by FID
From: Lai SiyaoAdd OBD_CONNECT_OPEN_BY_FID for open by FID, if MDS supports this, for open by FID, it won't retry with name if object with the FID doesn't exist; while if client supports this, client won't pack name in open request if FID is known. Signed-off-by: Lai Siyao Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-3544 Reviewed-on: http://review.whamcloud.com/8093 Reviewed-by: John L. Hammond Reviewed-by: Andreas Dilger Signed-off-by: James Simmons --- drivers/staging/lustre/lustre/include/lustre/lustre_idl.h | 3 +++ drivers/staging/lustre/lustre/ptlrpc/wiretest.c | 2 ++ 2 files changed, 5 insertions(+) diff --git a/drivers/staging/lustre/lustre/include/lustre/lustre_idl.h b/drivers/staging/lustre/lustre/include/lustre/lustre_idl.h index 26819ee..0ef540a 100644 --- a/drivers/staging/lustre/lustre/include/lustre/lustre_idl.h +++ b/drivers/staging/lustre/lustre/include/lustre/lustre_idl.h @@ -1257,6 +1257,9 @@ void lustre_swab_ptlrpc_body(struct ptlrpc_body *pb); #define OBD_CONNECT_PINGLESS 0x4ULL/* pings not required */ #define OBD_CONNECT_FLOCK_DEAD 0x8ULL/* flock deadlock detection */ #define OBD_CONNECT_DISP_STRIPE 0x10ULL/*create stripe disposition*/ +#define OBD_CONNECT_OPEN_BY_FID0x20ULL /* open by fid won't pack +* name in request +*/ /* XXX README XXX: * Please DO NOT add flag values here before first ensuring that this same diff --git a/drivers/staging/lustre/lustre/ptlrpc/wiretest.c b/drivers/staging/lustre/lustre/ptlrpc/wiretest.c index 87555e4..308b6b96 100644 --- a/drivers/staging/lustre/lustre/ptlrpc/wiretest.c +++ b/drivers/staging/lustre/lustre/ptlrpc/wiretest.c @@ -1069,6 +1069,8 @@ void lustre_assert_wire_constants(void) OBD_CONNECT_PINGLESS); LASSERTF(OBD_CONNECT_FLOCK_DEAD == 0x8ULL, "found 0x%.16llxULL\n", OBD_CONNECT_FLOCK_DEAD); + LASSERTF(OBD_CONNECT_OPEN_BY_FID == 0x20ULL, +"found 0x%.16llxULL\n", OBD_CONNECT_OPEN_BY_FID); LASSERTF(OBD_CKSUM_CRC32 == 0x0001UL, "found 0x%.8xUL\n", (unsigned)OBD_CKSUM_CRC32); LASSERTF(OBD_CKSUM_ADLER == 0x0002UL, "found 0x%.8xUL\n", -- 2.7.4
[PATCH 06/12] staging: lustre: fid: add a connect flag for open by FID
From: Lai Siyao Add OBD_CONNECT_OPEN_BY_FID for open by FID, if MDS supports this, for open by FID, it won't retry with name if object with the FID doesn't exist; while if client supports this, client won't pack name in open request if FID is known. Signed-off-by: Lai Siyao Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-3544 Reviewed-on: http://review.whamcloud.com/8093 Reviewed-by: John L. Hammond Reviewed-by: Andreas Dilger Signed-off-by: James Simmons --- drivers/staging/lustre/lustre/include/lustre/lustre_idl.h | 3 +++ drivers/staging/lustre/lustre/ptlrpc/wiretest.c | 2 ++ 2 files changed, 5 insertions(+) diff --git a/drivers/staging/lustre/lustre/include/lustre/lustre_idl.h b/drivers/staging/lustre/lustre/include/lustre/lustre_idl.h index 26819ee..0ef540a 100644 --- a/drivers/staging/lustre/lustre/include/lustre/lustre_idl.h +++ b/drivers/staging/lustre/lustre/include/lustre/lustre_idl.h @@ -1257,6 +1257,9 @@ void lustre_swab_ptlrpc_body(struct ptlrpc_body *pb); #define OBD_CONNECT_PINGLESS 0x4ULL/* pings not required */ #define OBD_CONNECT_FLOCK_DEAD 0x8ULL/* flock deadlock detection */ #define OBD_CONNECT_DISP_STRIPE 0x10ULL/*create stripe disposition*/ +#define OBD_CONNECT_OPEN_BY_FID0x20ULL /* open by fid won't pack +* name in request +*/ /* XXX README XXX: * Please DO NOT add flag values here before first ensuring that this same diff --git a/drivers/staging/lustre/lustre/ptlrpc/wiretest.c b/drivers/staging/lustre/lustre/ptlrpc/wiretest.c index 87555e4..308b6b96 100644 --- a/drivers/staging/lustre/lustre/ptlrpc/wiretest.c +++ b/drivers/staging/lustre/lustre/ptlrpc/wiretest.c @@ -1069,6 +1069,8 @@ void lustre_assert_wire_constants(void) OBD_CONNECT_PINGLESS); LASSERTF(OBD_CONNECT_FLOCK_DEAD == 0x8ULL, "found 0x%.16llxULL\n", OBD_CONNECT_FLOCK_DEAD); + LASSERTF(OBD_CONNECT_OPEN_BY_FID == 0x20ULL, +"found 0x%.16llxULL\n", OBD_CONNECT_OPEN_BY_FID); LASSERTF(OBD_CKSUM_CRC32 == 0x0001UL, "found 0x%.8xUL\n", (unsigned)OBD_CKSUM_CRC32); LASSERTF(OBD_CKSUM_ADLER == 0x0002UL, "found 0x%.8xUL\n", -- 2.7.4
Re: [PATCH] mm/zswap: use workqueue to destroy pool
Hello Dan, On (04/27/16 13:19), Dan Streetman wrote: [..] > > so in general the patch look good to me. > > > > it's either I didn't have enough coffee yet (which is true) or > > _IN THEORY_ it creates a tiny race condition; which is hard (and > > unlikely) to hit, but still. and the problem being is > > CONFIG_ZSMALLOC_STAT. > > Aha, thanks, I hadn't tested with that param enabled. However, the > patch doesn't create the race condition, that existed already. well, agree. it's not like zsmalloc race condition, but the way zsmalloc is managed (deferred destruction either via rcu or scheduled work). > It fails because the new zswap pool creates a new zpool using > zsmalloc, but it can't create the zsmalloc pool because there is > already one named 'zswap' so the stat dir can't be created. > > So...either zswap needs to provide a unique 'name' to each of its > zpools, or zsmalloc needs to modify its provided pool name in some way > (add a unique suffix maybe). Or both. > > It seems like zsmalloc should do the checking/modification - or, at > the very least, it should have consistent behavior regardless of the > CONFIG_ZSMALLOC_STAT setting. yes, zram guarantees that there won't be any name collisions. and the way it's working for zram, zram corresponds to zsmalloc. the bigger issue here (and I was thinking at some point of fixing it, but then I grepped to see how many API users are in there, and I gave up) is that it seems we have no way to check if the dir exists in debugfs. we call this function struct dentry *debugfs_create_dir(const char *name, struct dentry *parent) { struct dentry *dentry = start_creating(name, parent); struct inode *inode; if (IS_ERR(dentry)) return NULL; inode = debugfs_get_inode(dentry->d_sb); if (unlikely(!inode)) return failed_creating(dentry); inode->i_mode = S_IFDIR | S_IRWXU | S_IRUGO | S_IXUGO; inode->i_op = _dir_inode_operations; inode->i_fop = _dir_operations; /* directory inodes start off with i_nlink == 2 (for "." entry) */ inc_nlink(inode); d_instantiate(dentry, inode); inc_nlink(d_inode(dentry->d_parent)); fsnotify_mkdir(d_inode(dentry->d_parent), dentry); return end_creating(dentry); } and debugfs _does know_ that the directory ERR_PTR(-EEXIST), that's what start_creating()->lookup_one_len() return static struct dentry *start_creating(const char *name, struct dentry *parent) { struct dentry *dentry; int error; pr_debug("debugfs: creating file '%s'\n",name); if (IS_ERR(parent)) return parent; error = simple_pin_fs(_fs_type, _mount, _mount_count); if (error) return ERR_PTR(error); /* If the parent is not specified, we create it in the root. * We need the root dentry to do this, which is in the super * block. A pointer to that is in the struct vfsmount that we * have around. */ if (!parent) parent = debugfs_mount->mnt_root; inode_lock(d_inode(parent)); dentry = lookup_one_len(name, parent, strlen(name)); if (!IS_ERR(dentry) && d_really_is_positive(dentry)) { dput(dentry); dentry = ERR_PTR(-EEXIST); } if (IS_ERR(dentry)) { inode_unlock(d_inode(parent)); simple_release_fs(_mount, _mount_count); } return dentry; } but debugfs_create_dir() instead of propagating this error, it swallows it and simply return NULL, so we can't tell the difference between -EEXIST, OOM, or anything else. so doing this check in zsmalloc() is not so easy. /* well, I may be wrong here */ > However, it's easy to change zswap to provide a unique name for each > zpool creation, and zsmalloc's primary user (zram) guarantees to > provide a unique name for each pool created. So updating zswap is > probably best. if you can do it in zswap, then please do. -ss
[PATCH 10/12] staging: lustre: llog: we don't need vfsmount
From: Lai SiyaoThe patch for LU-3286 removed vfsmount instances used on the server side. Since this is server side only we can remove it from the upstream client. Signed-off-by: Lai Siyao Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-3286 Reviewed-on: http://review.whamcloud.com/8286 Reviewed-by: Alex Zhuravlev Reviewed-by: Mike Pershin Reviewed-by: Jian Yu Reviewed-by: Oleg Drokin Signed-off-by: James Simmons --- drivers/staging/lustre/lustre/include/lustre_disk.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/staging/lustre/lustre/include/lustre_disk.h b/drivers/staging/lustre/lustre/include/lustre_disk.h index 95fd360..b36821f 100644 --- a/drivers/staging/lustre/lustre/include/lustre_disk.h +++ b/drivers/staging/lustre/lustre/include/lustre_disk.h @@ -130,7 +130,6 @@ struct lustre_sb_info { struct lustre_mount_data *lsi_lmd; /* mount command info */ struct ll_sb_info *lsi_llsbi; /* add'l client sbi info */ struct dt_device *lsi_dt_dev; /* dt device to access disk fs*/ - struct vfsmount *lsi_srv_mnt; /* the one server mount */ atomic_t lsi_mounts; /* references to the srv_mnt */ char lsi_svname[MTI_NAME_MAXLEN]; char lsi_osd_obdname[64]; @@ -158,7 +157,6 @@ struct lustre_sb_info { struct lustre_mount_info { char *lmi_name; struct super_block *lmi_sb; - struct vfsmount *lmi_mnt; struct list_headlmi_list_chain; }; -- 2.7.4
[PATCH net v3 3/5] drivers: net: cpsw: don't ignore phy-mode if phy-handle is used
From: David RivshinThe phy-mode emac property was only being processed in the phy_id or fixed-link cases. However if phy-handle was specified instead, an error message would complain about the lack of phy_id or fixed-link, and then jump past the of_get_phy_mode(). This would result in the PHY mode defaulting to MII, regardless of what the devicetree specified. Fixes: 9e42f715264f ("drivers: net: cpsw: add phy-handle parsing") Signed-off-by: David Rivshin Tested-by: Nicolas Chauvet Tested-by: Andrew Goodbody Reviewed-by: Mugunthan V N --- I would suggest this for -stable. It should apply cleanly as far back as 4.4. Changes since v2 [1]: - split from previous patch 2 - Added Tested-by from Andrew Goodbody [3] - Added Reviewed-by from Mugunthan V N [4] - rewrote commit log to focus on the functional bug fixed, rather than the bogus error message Changes since v1 [2]: - Rebased (no conflicts) - Added Tested-by from Nicolas Chauvet - Added Acked-by from Rob Herring for the binding change [1] http://patchwork.ozlabs.org/patch/613260/ [2] http://patchwork.ozlabs.org/patch/560324/ [3] https://lkml.org/lkml/2016/4/22/537 [4] https://lkml.org/lkml/2016/4/22/63 drivers/net/ethernet/ti/cpsw.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c index 5903448..712bc6d 100644 --- a/drivers/net/ethernet/ti/cpsw.c +++ b/drivers/net/ethernet/ti/cpsw.c @@ -2039,15 +2039,19 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data, /* This is no slave child node, continue */ if (strcmp(slave_node->name, "slave")) continue; slave_data->phy_node = of_parse_phandle(slave_node, "phy-handle", 0); parp = of_get_property(slave_node, "phy_id", ); - if (of_phy_is_fixed_link(slave_node)) { + if (slave_data->phy_node) { + dev_dbg(>dev, + "slave[%d] using phy-handle=\"%s\"\n", + i, slave_data->phy_node->full_name); + } else if (of_phy_is_fixed_link(slave_node)) { struct device_node *phy_node; struct phy_device *phy_dev; /* In the case of a fixed PHY, the DT node associated * to the PHY is the Ethernet MAC DT node. */ ret = of_phy_register_fixed_link(slave_node); @@ -2076,15 +2080,17 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data, if (!mdio) { dev_err(>dev, "Missing mdio platform device\n"); return -EINVAL; } snprintf(slave_data->phy_id, sizeof(slave_data->phy_id), PHY_ID_FMT, mdio->name, phyid); } else { - dev_err(>dev, "No slave[%d] phy_id or fixed-link property\n", i); + dev_err(>dev, + "No slave[%d] phy_id, phy-handle, or fixed-link property\n", + i); goto no_phy_slave; } slave_data->phy_if = of_get_phy_mode(slave_node); if (slave_data->phy_if < 0) { dev_err(>dev, "Missing or malformed slave[%d] phy-mode property\n", i); return slave_data->phy_if; -- 2.5.5
[PATCH 08/12] staging: lustre: llite: debugging for ll_file_open LASSERT
From: Andreas DilgerAdd debugging for LASSERTF(it_disposition(it, DISP_ENQ_OPEN_REF) in ll_file_open(), since this is a rarely hit failure under racer, and it would be useful to get more information if this is hit again. Print the full intent disposition, as well as the status, in case Oleg's earlier comment about bailing out on any error is actually the case here. Signed-off-by: Andreas Dilger Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-1993 Reviewed-on: http://review.whamcloud.com/6250 Reviewed-by: Bob Glossman Reviewed-by: John L. Hammond Reviewed-by: Oleg Drokin Signed-off-by: James Simmons --- drivers/staging/lustre/lustre/llite/file.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/staging/lustre/lustre/llite/file.c b/drivers/staging/lustre/lustre/llite/file.c index a8de10e..f47f2ac 100644 --- a/drivers/staging/lustre/lustre/llite/file.c +++ b/drivers/staging/lustre/lustre/llite/file.c @@ -679,7 +679,9 @@ restart: if (rc) goto out_och_free; - LASSERT(it_disposition(it, DISP_ENQ_OPEN_REF)); + LASSERTF(it_disposition(it, DISP_ENQ_OPEN_REF), +"inode %p: disposition %x, status %d\n", inode, +it_disposition(it, ~0), it->d.lustre.it_status); rc = ll_local_open(file, it, fd, *och_p); if (rc) -- 2.7.4
[PATCH 11/12] staging: lustre: ptlrpc: initialize request session early
From: Mikhail PershinInitialize request session early to make it available in high-priority handlers Signed-off-by: Mikhail Pershin Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-3467 Reviewed-on: http://review.whamcloud.com/7350 Reviewed-by: Alex Zhuravlev Reviewed-by: Fan Yong Reviewed-by: Andreas Dilger Signed-off-by: James Simmons --- drivers/staging/lustre/lustre/ptlrpc/service.c | 53 +++--- 1 file changed, 31 insertions(+), 22 deletions(-) diff --git a/drivers/staging/lustre/lustre/ptlrpc/service.c b/drivers/staging/lustre/lustre/ptlrpc/service.c index fc2632f..17c7b97 100644 --- a/drivers/staging/lustre/lustre/ptlrpc/service.c +++ b/drivers/staging/lustre/lustre/ptlrpc/service.c @@ -838,6 +838,11 @@ static void ptlrpc_server_finish_request(struct ptlrpc_service_part *svcpt, { ptlrpc_server_hpreq_fini(req); + if (req->rq_session.lc_thread) { + lu_context_exit(>rq_session); + lu_context_fini(>rq_session); + } + ptlrpc_server_drop_request(req); } @@ -1579,6 +1584,21 @@ ptlrpc_server_handle_req_in(struct ptlrpc_service_part *svcpt, } req->rq_svc_thread = thread; + if (thread) { + /* initialize request session, it is needed for request +* processing by target +*/ + rc = lu_context_init(>rq_session, +LCT_SERVER_SESSION | LCT_NOREF); + if (rc) { + CERROR("%s: failure to initialize session: rc = %d\n", + thread->t_name, rc); + goto err_req; + } + req->rq_session.lc_thread = thread; + lu_context_enter(>rq_session); + req->rq_svc_thread->t_env->le_ses = >rq_session; + } ptlrpc_at_add_timed(req); @@ -1612,7 +1632,6 @@ ptlrpc_server_handle_request(struct ptlrpc_service_part *svcpt, struct timespec64 arrived; unsigned long timediff_usecs; unsigned long arrived_usecs; - int rc; int fail_opc = 0; request = ptlrpc_server_request_get(svcpt, false); @@ -1649,22 +1668,6 @@ ptlrpc_server_handle_request(struct ptlrpc_service_part *svcpt, at_get(>scp_at_estimate)); } - rc = lu_context_init(>rq_session, LCT_SERVER_SESSION | - LCT_NOREF); - if (rc) { - CERROR("Failure to initialize session: %d\n", rc); - goto out_req; - } - request->rq_session.lc_thread = thread; - request->rq_session.lc_cookie = 0x5; - lu_context_enter(>rq_session); - - CDEBUG(D_NET, "got req %llu\n", request->rq_xid); - - request->rq_svc_thread = thread; - if (thread) - request->rq_svc_thread->t_env->le_ses = >rq_session; - if (likely(request->rq_export)) { if (unlikely(ptlrpc_check_req(request))) goto put_conn; @@ -1696,14 +1699,21 @@ ptlrpc_server_handle_request(struct ptlrpc_service_part *svcpt, if (lustre_msg_get_opc(request->rq_reqmsg) != OBD_PING) CFS_FAIL_TIMEOUT_MS(OBD_FAIL_PTLRPC_PAUSE_REQ, cfs_fail_val); - rc = svc->srv_ops.so_req_handler(request); + CDEBUG(D_NET, "got req %llu\n", request->rq_xid); + + /* re-assign request and sesson thread to the current one */ + request->rq_svc_thread = thread; + if (thread) { + LASSERT(request->rq_session.lc_thread); + request->rq_session.lc_thread = thread; + request->rq_session.lc_cookie = 0x55; + thread->t_env->le_ses = >rq_session; + } + svc->srv_ops.so_req_handler(request); ptlrpc_rqphase_move(request, RQ_PHASE_COMPLETE); put_conn: - lu_context_exit(>rq_session); - lu_context_fini(>rq_session); - if (unlikely(ktime_get_real_seconds() > request->rq_deadline)) { DEBUG_REQ(D_WARNING, request, "Request took longer than estimated (%lld:%llds); " @@ -1757,7 +1767,6 @@ put_conn: request->rq_arrival_time.tv_sec); } -out_req: ptlrpc_server_finish_active_request(svcpt, request); return 1; -- 2.7.4
[PATCH 10/12] staging: lustre: llog: we don't need vfsmount
From: Lai Siyao The patch for LU-3286 removed vfsmount instances used on the server side. Since this is server side only we can remove it from the upstream client. Signed-off-by: Lai Siyao Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-3286 Reviewed-on: http://review.whamcloud.com/8286 Reviewed-by: Alex Zhuravlev Reviewed-by: Mike Pershin Reviewed-by: Jian Yu Reviewed-by: Oleg Drokin Signed-off-by: James Simmons --- drivers/staging/lustre/lustre/include/lustre_disk.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/staging/lustre/lustre/include/lustre_disk.h b/drivers/staging/lustre/lustre/include/lustre_disk.h index 95fd360..b36821f 100644 --- a/drivers/staging/lustre/lustre/include/lustre_disk.h +++ b/drivers/staging/lustre/lustre/include/lustre_disk.h @@ -130,7 +130,6 @@ struct lustre_sb_info { struct lustre_mount_data *lsi_lmd; /* mount command info */ struct ll_sb_info *lsi_llsbi; /* add'l client sbi info */ struct dt_device *lsi_dt_dev; /* dt device to access disk fs*/ - struct vfsmount *lsi_srv_mnt; /* the one server mount */ atomic_t lsi_mounts; /* references to the srv_mnt */ char lsi_svname[MTI_NAME_MAXLEN]; char lsi_osd_obdname[64]; @@ -158,7 +157,6 @@ struct lustre_sb_info { struct lustre_mount_info { char *lmi_name; struct super_block *lmi_sb; - struct vfsmount *lmi_mnt; struct list_headlmi_list_chain; }; -- 2.7.4
[PATCH net v3 3/5] drivers: net: cpsw: don't ignore phy-mode if phy-handle is used
From: David Rivshin The phy-mode emac property was only being processed in the phy_id or fixed-link cases. However if phy-handle was specified instead, an error message would complain about the lack of phy_id or fixed-link, and then jump past the of_get_phy_mode(). This would result in the PHY mode defaulting to MII, regardless of what the devicetree specified. Fixes: 9e42f715264f ("drivers: net: cpsw: add phy-handle parsing") Signed-off-by: David Rivshin Tested-by: Nicolas Chauvet Tested-by: Andrew Goodbody Reviewed-by: Mugunthan V N --- I would suggest this for -stable. It should apply cleanly as far back as 4.4. Changes since v2 [1]: - split from previous patch 2 - Added Tested-by from Andrew Goodbody [3] - Added Reviewed-by from Mugunthan V N [4] - rewrote commit log to focus on the functional bug fixed, rather than the bogus error message Changes since v1 [2]: - Rebased (no conflicts) - Added Tested-by from Nicolas Chauvet - Added Acked-by from Rob Herring for the binding change [1] http://patchwork.ozlabs.org/patch/613260/ [2] http://patchwork.ozlabs.org/patch/560324/ [3] https://lkml.org/lkml/2016/4/22/537 [4] https://lkml.org/lkml/2016/4/22/63 drivers/net/ethernet/ti/cpsw.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c index 5903448..712bc6d 100644 --- a/drivers/net/ethernet/ti/cpsw.c +++ b/drivers/net/ethernet/ti/cpsw.c @@ -2039,15 +2039,19 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data, /* This is no slave child node, continue */ if (strcmp(slave_node->name, "slave")) continue; slave_data->phy_node = of_parse_phandle(slave_node, "phy-handle", 0); parp = of_get_property(slave_node, "phy_id", ); - if (of_phy_is_fixed_link(slave_node)) { + if (slave_data->phy_node) { + dev_dbg(>dev, + "slave[%d] using phy-handle=\"%s\"\n", + i, slave_data->phy_node->full_name); + } else if (of_phy_is_fixed_link(slave_node)) { struct device_node *phy_node; struct phy_device *phy_dev; /* In the case of a fixed PHY, the DT node associated * to the PHY is the Ethernet MAC DT node. */ ret = of_phy_register_fixed_link(slave_node); @@ -2076,15 +2080,17 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data, if (!mdio) { dev_err(>dev, "Missing mdio platform device\n"); return -EINVAL; } snprintf(slave_data->phy_id, sizeof(slave_data->phy_id), PHY_ID_FMT, mdio->name, phyid); } else { - dev_err(>dev, "No slave[%d] phy_id or fixed-link property\n", i); + dev_err(>dev, + "No slave[%d] phy_id, phy-handle, or fixed-link property\n", + i); goto no_phy_slave; } slave_data->phy_if = of_get_phy_mode(slave_node); if (slave_data->phy_if < 0) { dev_err(>dev, "Missing or malformed slave[%d] phy-mode property\n", i); return slave_data->phy_if; -- 2.5.5
[PATCH 08/12] staging: lustre: llite: debugging for ll_file_open LASSERT
From: Andreas Dilger Add debugging for LASSERTF(it_disposition(it, DISP_ENQ_OPEN_REF) in ll_file_open(), since this is a rarely hit failure under racer, and it would be useful to get more information if this is hit again. Print the full intent disposition, as well as the status, in case Oleg's earlier comment about bailing out on any error is actually the case here. Signed-off-by: Andreas Dilger Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-1993 Reviewed-on: http://review.whamcloud.com/6250 Reviewed-by: Bob Glossman Reviewed-by: John L. Hammond Reviewed-by: Oleg Drokin Signed-off-by: James Simmons --- drivers/staging/lustre/lustre/llite/file.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/staging/lustre/lustre/llite/file.c b/drivers/staging/lustre/lustre/llite/file.c index a8de10e..f47f2ac 100644 --- a/drivers/staging/lustre/lustre/llite/file.c +++ b/drivers/staging/lustre/lustre/llite/file.c @@ -679,7 +679,9 @@ restart: if (rc) goto out_och_free; - LASSERT(it_disposition(it, DISP_ENQ_OPEN_REF)); + LASSERTF(it_disposition(it, DISP_ENQ_OPEN_REF), +"inode %p: disposition %x, status %d\n", inode, +it_disposition(it, ~0), it->d.lustre.it_status); rc = ll_local_open(file, it, fd, *och_p); if (rc) -- 2.7.4
[PATCH 11/12] staging: lustre: ptlrpc: initialize request session early
From: Mikhail Pershin Initialize request session early to make it available in high-priority handlers Signed-off-by: Mikhail Pershin Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-3467 Reviewed-on: http://review.whamcloud.com/7350 Reviewed-by: Alex Zhuravlev Reviewed-by: Fan Yong Reviewed-by: Andreas Dilger Signed-off-by: James Simmons --- drivers/staging/lustre/lustre/ptlrpc/service.c | 53 +++--- 1 file changed, 31 insertions(+), 22 deletions(-) diff --git a/drivers/staging/lustre/lustre/ptlrpc/service.c b/drivers/staging/lustre/lustre/ptlrpc/service.c index fc2632f..17c7b97 100644 --- a/drivers/staging/lustre/lustre/ptlrpc/service.c +++ b/drivers/staging/lustre/lustre/ptlrpc/service.c @@ -838,6 +838,11 @@ static void ptlrpc_server_finish_request(struct ptlrpc_service_part *svcpt, { ptlrpc_server_hpreq_fini(req); + if (req->rq_session.lc_thread) { + lu_context_exit(>rq_session); + lu_context_fini(>rq_session); + } + ptlrpc_server_drop_request(req); } @@ -1579,6 +1584,21 @@ ptlrpc_server_handle_req_in(struct ptlrpc_service_part *svcpt, } req->rq_svc_thread = thread; + if (thread) { + /* initialize request session, it is needed for request +* processing by target +*/ + rc = lu_context_init(>rq_session, +LCT_SERVER_SESSION | LCT_NOREF); + if (rc) { + CERROR("%s: failure to initialize session: rc = %d\n", + thread->t_name, rc); + goto err_req; + } + req->rq_session.lc_thread = thread; + lu_context_enter(>rq_session); + req->rq_svc_thread->t_env->le_ses = >rq_session; + } ptlrpc_at_add_timed(req); @@ -1612,7 +1632,6 @@ ptlrpc_server_handle_request(struct ptlrpc_service_part *svcpt, struct timespec64 arrived; unsigned long timediff_usecs; unsigned long arrived_usecs; - int rc; int fail_opc = 0; request = ptlrpc_server_request_get(svcpt, false); @@ -1649,22 +1668,6 @@ ptlrpc_server_handle_request(struct ptlrpc_service_part *svcpt, at_get(>scp_at_estimate)); } - rc = lu_context_init(>rq_session, LCT_SERVER_SESSION | - LCT_NOREF); - if (rc) { - CERROR("Failure to initialize session: %d\n", rc); - goto out_req; - } - request->rq_session.lc_thread = thread; - request->rq_session.lc_cookie = 0x5; - lu_context_enter(>rq_session); - - CDEBUG(D_NET, "got req %llu\n", request->rq_xid); - - request->rq_svc_thread = thread; - if (thread) - request->rq_svc_thread->t_env->le_ses = >rq_session; - if (likely(request->rq_export)) { if (unlikely(ptlrpc_check_req(request))) goto put_conn; @@ -1696,14 +1699,21 @@ ptlrpc_server_handle_request(struct ptlrpc_service_part *svcpt, if (lustre_msg_get_opc(request->rq_reqmsg) != OBD_PING) CFS_FAIL_TIMEOUT_MS(OBD_FAIL_PTLRPC_PAUSE_REQ, cfs_fail_val); - rc = svc->srv_ops.so_req_handler(request); + CDEBUG(D_NET, "got req %llu\n", request->rq_xid); + + /* re-assign request and sesson thread to the current one */ + request->rq_svc_thread = thread; + if (thread) { + LASSERT(request->rq_session.lc_thread); + request->rq_session.lc_thread = thread; + request->rq_session.lc_cookie = 0x55; + thread->t_env->le_ses = >rq_session; + } + svc->srv_ops.so_req_handler(request); ptlrpc_rqphase_move(request, RQ_PHASE_COMPLETE); put_conn: - lu_context_exit(>rq_session); - lu_context_fini(>rq_session); - if (unlikely(ktime_get_real_seconds() > request->rq_deadline)) { DEBUG_REQ(D_WARNING, request, "Request took longer than estimated (%lld:%llds); " @@ -1757,7 +1767,6 @@ put_conn: request->rq_arrival_time.tv_sec); } -out_req: ptlrpc_server_finish_active_request(svcpt, request); return 1; -- 2.7.4
[PATCH 00/12] staging: lustre: add patches from lustre 2.5.52 version
Assortment of bug fixes that are present in the 2.5.52 version of lustre that is missing in the upstream client. Amir Shehata (2): staging: lustre: obd: remove newline from LCONSOLE string staging: lustre: obd: add newline for dumped config record Andreas Dilger (1): staging: lustre: llite: debugging for ll_file_open LASSERT Andriy Skulysh (1): staging: lustre: libcfs: Fix NUMA emulated mode Dmitry Eremin (2): staging: lustre: ldlm: check all errors during ldlm_debugfs_setup() staging: lustre: llite: fixup return value ll_direct_IO_26 Lai Siyao (2): staging: lustre: fid: add a connect flag for open by FID staging: lustre: llog: we don't need vfsmount Mikhail Pershin (2): staging: lustre: ptlrpc: use unified handler for OST requests staging: lustre: ptlrpc: initialize request session early Swapnil Pimpale (2): staging: lustre: obd: cleanup client import if client_obd_setup fails staging: lustre: llite: check ret of ll_prep_md_op_data in ll_dir_filler .../staging/lustre/lnet/libcfs/linux/linux-cpu.c | 9 +++- .../lustre/lustre/include/lustre/lustre_idl.h | 3 ++ .../staging/lustre/lustre/include/lustre_disk.h| 2 - .../lustre/lustre/include/lustre_req_layout.h | 2 +- .../staging/lustre/lustre/include/obd_support.h| 1 + drivers/staging/lustre/lustre/ldlm/ldlm_lib.c | 1 + drivers/staging/lustre/lustre/ldlm/ldlm_resource.c | 6 +++ drivers/staging/lustre/lustre/llite/dir.c | 7 ++- drivers/staging/lustre/lustre/llite/file.c | 4 +- drivers/staging/lustre/lustre/llite/rw26.c | 2 +- .../staging/lustre/lustre/obdclass/obd_config.c| 3 +- drivers/staging/lustre/lustre/ptlrpc/layout.c | 16 +-- drivers/staging/lustre/lustre/ptlrpc/service.c | 53 +- drivers/staging/lustre/lustre/ptlrpc/wiretest.c| 2 + 14 files changed, 74 insertions(+), 37 deletions(-) -- 2.7.4
[PATCH 00/12] staging: lustre: add patches from lustre 2.5.52 version
Assortment of bug fixes that are present in the 2.5.52 version of lustre that is missing in the upstream client. Amir Shehata (2): staging: lustre: obd: remove newline from LCONSOLE string staging: lustre: obd: add newline for dumped config record Andreas Dilger (1): staging: lustre: llite: debugging for ll_file_open LASSERT Andriy Skulysh (1): staging: lustre: libcfs: Fix NUMA emulated mode Dmitry Eremin (2): staging: lustre: ldlm: check all errors during ldlm_debugfs_setup() staging: lustre: llite: fixup return value ll_direct_IO_26 Lai Siyao (2): staging: lustre: fid: add a connect flag for open by FID staging: lustre: llog: we don't need vfsmount Mikhail Pershin (2): staging: lustre: ptlrpc: use unified handler for OST requests staging: lustre: ptlrpc: initialize request session early Swapnil Pimpale (2): staging: lustre: obd: cleanup client import if client_obd_setup fails staging: lustre: llite: check ret of ll_prep_md_op_data in ll_dir_filler .../staging/lustre/lnet/libcfs/linux/linux-cpu.c | 9 +++- .../lustre/lustre/include/lustre/lustre_idl.h | 3 ++ .../staging/lustre/lustre/include/lustre_disk.h| 2 - .../lustre/lustre/include/lustre_req_layout.h | 2 +- .../staging/lustre/lustre/include/obd_support.h| 1 + drivers/staging/lustre/lustre/ldlm/ldlm_lib.c | 1 + drivers/staging/lustre/lustre/ldlm/ldlm_resource.c | 6 +++ drivers/staging/lustre/lustre/llite/dir.c | 7 ++- drivers/staging/lustre/lustre/llite/file.c | 4 +- drivers/staging/lustre/lustre/llite/rw26.c | 2 +- .../staging/lustre/lustre/obdclass/obd_config.c| 3 +- drivers/staging/lustre/lustre/ptlrpc/layout.c | 16 +-- drivers/staging/lustre/lustre/ptlrpc/service.c | 53 +- drivers/staging/lustre/lustre/ptlrpc/wiretest.c| 2 + 14 files changed, 74 insertions(+), 37 deletions(-) -- 2.7.4
[PATCH 12/12] staging: lustre: llite: fixup return value ll_direct_IO_26
From: Dmitry EreminReturn the correct values from ll_direct_IO_26. Signed-off-by: Dmitry Eremin Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-4069 Reviewed-on: http://review.whamcloud.com/8080 Reviewed-by: John L. Hammond Reviewed-by: James Simmons Reviewed-by: Oleg Drokin Signed-Off-by: James Simmons --- drivers/staging/lustre/lustre/llite/rw26.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/lustre/lustre/llite/rw26.c b/drivers/staging/lustre/lustre/llite/rw26.c index a740c7a..9341189 100644 --- a/drivers/staging/lustre/lustre/llite/rw26.c +++ b/drivers/staging/lustre/lustre/llite/rw26.c @@ -446,7 +446,7 @@ out: } cl_env_put(env, ); - return tot_bytes ? : result; + return tot_bytes ? tot_bytes : result; } /** -- 2.7.4
[PATCH 07/12] staging: lustre: libcfs: Fix NUMA emulated mode
From: Andriy SkulyshKernel commit c1c3443c9c5e9be92641029ed229a41563e44506 assigns all allowed cpus to emulated node. End cpt initialization loop when all CPUs are assigned. Signed-off-by: Andriy Skulysh Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-3992 Reviewed-on: http://review.whamcloud.com/7724 Reviewed-by: Liang Zhen Reviewed-by: Oleg Drokin Signed-off-by: James Simmons --- drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c b/drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c index 389fb9e..b52518c5 100644 --- a/drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c +++ b/drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c @@ -755,8 +755,13 @@ cfs_cpt_table_create(int ncpt) struct cfs_cpu_partition *part; intn; - if (cpt >= ncpt) - goto failed; + /* +* Each emulated NUMA node has all allowed CPUs in +* the mask. +* End loop when all partitions have assigned CPUs. +*/ + if (cpt == ncpt) + break; part = >ctb_parts[cpt]; -- 2.7.4
[PATCH 04/12] staging: lustre: ldlm: check all errors during ldlm_debugfs_setup()
From: Dmitry EreminFix ignoring errors from ldebugfs_add_vars() function. Signed-off-by: Dmitry Eremin Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-3885 Reviewed-on: http://review.whamcloud.com/8115 Reviewed-by: John L. Hammond Reviewed-by: Andreas Dilger Signed-off-by: James Simmons --- drivers/staging/lustre/lustre/ldlm/ldlm_resource.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_resource.c b/drivers/staging/lustre/lustre/ldlm/ldlm_resource.c index 475fabb..e99c89c 100644 --- a/drivers/staging/lustre/lustre/ldlm/ldlm_resource.c +++ b/drivers/staging/lustre/lustre/ldlm/ldlm_resource.c @@ -124,9 +124,15 @@ int ldlm_debugfs_setup(void) } rc = ldebugfs_add_vars(ldlm_debugfs_dir, ldlm_debugfs_list, NULL); + if (rc) { + CERROR("LProcFS failed in ldlm-init\n"); + goto err_svc; + } return 0; +err_svc: + ldebugfs_remove(_svc_debugfs_dir); err_ns: ldebugfs_remove(_ns_debugfs_dir); err_type: -- 2.7.4
[PATCH 03/12] staging: lustre: ptlrpc: use unified handler for OST requests
From: Mikhail PershinSwitch OST/OFD request processing to the unified request handle. Signed-off-by: Mikhail Pershin Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-3467 Reviewed-on: http://review.whamcloud.com/7130 Reviewed-by: Andreas Dilger Reviewed-by: Alex Zhuravlev Signed-off-by: James Simmons --- .../staging/lustre/lustre/include/lustre_req_layout.h| 2 +- drivers/staging/lustre/lustre/include/obd_support.h | 1 + drivers/staging/lustre/lustre/ptlrpc/layout.c| 16 +++- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/drivers/staging/lustre/lustre/include/lustre_req_layout.h b/drivers/staging/lustre/lustre/include/lustre_req_layout.h index b2e67fc..b0bc751 100644 --- a/drivers/staging/lustre/lustre/include/lustre_req_layout.h +++ b/drivers/staging/lustre/lustre/include/lustre_req_layout.h @@ -199,7 +199,7 @@ extern struct req_format RQF_OST_BRW_READ; extern struct req_format RQF_OST_BRW_WRITE; extern struct req_format RQF_OST_STATFS; extern struct req_format RQF_OST_SET_GRANT_INFO; -extern struct req_format RQF_OST_GET_INFO_GENERIC; +extern struct req_format RQF_OST_GET_INFO; extern struct req_format RQF_OST_GET_INFO_LAST_ID; extern struct req_format RQF_OST_GET_INFO_LAST_FID; extern struct req_format RQF_OST_SET_INFO_LAST_FID; diff --git a/drivers/staging/lustre/lustre/include/obd_support.h b/drivers/staging/lustre/lustre/include/obd_support.h index c7267b7..2aaa343 100644 --- a/drivers/staging/lustre/lustre/include/obd_support.h +++ b/drivers/staging/lustre/lustre/include/obd_support.h @@ -290,6 +290,7 @@ extern char obd_jobid_var[]; #define OBD_FAIL_OST_ENOINO 0x229 #define OBD_FAIL_OST_DQACQ_NET0x230 #define OBD_FAIL_OST_STATFS_EINPROGRESS 0x231 +#define OBD_FAIL_OST_SET_INFO_NET 0x232 #define OBD_FAIL_LDLM 0x300 #define OBD_FAIL_LDLM_NAMESPACE_NEW 0x301 diff --git a/drivers/staging/lustre/lustre/ptlrpc/layout.c b/drivers/staging/lustre/lustre/ptlrpc/layout.c index 5b06901..ccc1f3e 100644 --- a/drivers/staging/lustre/lustre/ptlrpc/layout.c +++ b/drivers/staging/lustre/lustre/ptlrpc/layout.c @@ -566,7 +566,7 @@ static const struct req_msg_field *ost_get_info_generic_server[] = { static const struct req_msg_field *ost_get_info_generic_client[] = { _PTLRPC_BODY, - _SETINFO_KEY + _GETINFO_KEY }; static const struct req_msg_field *ost_get_last_id_server[] = { @@ -574,6 +574,12 @@ static const struct req_msg_field *ost_get_last_id_server[] = { _OBD_ID }; +static const struct req_msg_field *ost_get_last_fid_client[] = { + _PTLRPC_BODY, + _GETINFO_KEY, + _FID, +}; + static const struct req_msg_field *ost_get_last_fid_server[] = { _PTLRPC_BODY, _FID, @@ -696,7 +702,7 @@ static struct req_format *req_formats[] = { _OST_BRW_WRITE, _OST_STATFS, _OST_SET_GRANT_INFO, - _OST_GET_INFO_GENERIC, + _OST_GET_INFO, _OST_GET_INFO_LAST_ID, _OST_GET_INFO_LAST_FID, _OST_SET_INFO_LAST_FID, @@ -1519,10 +1525,10 @@ struct req_format RQF_OST_SET_GRANT_INFO = ost_body_only); EXPORT_SYMBOL(RQF_OST_SET_GRANT_INFO); -struct req_format RQF_OST_GET_INFO_GENERIC = +struct req_format RQF_OST_GET_INFO = DEFINE_REQ_FMT0("OST_GET_INFO", ost_get_info_generic_client, ost_get_info_generic_server); -EXPORT_SYMBOL(RQF_OST_GET_INFO_GENERIC); +EXPORT_SYMBOL(RQF_OST_GET_INFO); struct req_format RQF_OST_GET_INFO_LAST_ID = DEFINE_REQ_FMT0("OST_GET_INFO_LAST_ID", ost_get_info_generic_client, @@ -1530,7 +1536,7 @@ struct req_format RQF_OST_GET_INFO_LAST_ID = EXPORT_SYMBOL(RQF_OST_GET_INFO_LAST_ID); struct req_format RQF_OST_GET_INFO_LAST_FID = - DEFINE_REQ_FMT0("OST_GET_INFO_LAST_FID", obd_set_info_client, + DEFINE_REQ_FMT0("OST_GET_INFO_LAST_FID", ost_get_last_fid_client, ost_get_last_fid_server); EXPORT_SYMBOL(RQF_OST_GET_INFO_LAST_FID); -- 2.7.4
[PATCH 01/12] staging: lustre: obd: remove newline from LCONSOLE string
From: Amir ShehataRemove the newline from the LCONSOLE debug macro in the function class_config_dump_handler(). Signed-off-by: Amir Shehata Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-2149 Reviewed-on: http://review.whamcloud.com/4254 Reviewed-by: Alex Zhuravlev Reviewed-by: Andreas Dilger Signed-off-by: James Simmons --- drivers/staging/lustre/lustre/obdclass/obd_config.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/lustre/lustre/obdclass/obd_config.c b/drivers/staging/lustre/lustre/obdclass/obd_config.c index 5395e99..084af1a 100644 --- a/drivers/staging/lustre/lustre/obdclass/obd_config.c +++ b/drivers/staging/lustre/lustre/obdclass/obd_config.c @@ -1368,7 +1368,7 @@ int class_config_dump_handler(const struct lu_env *env, if (rec->lrh_type == OBD_CFG_REC) { class_config_parse_rec(rec, outstr, 256); - LCONSOLE(D_WARNING, " %s\n", outstr); + LCONSOLE(D_WARNING, " %s", outstr); } else { LCONSOLE(D_WARNING, "unhandled lrh_type: %#x\n", rec->lrh_type); rc = -EINVAL; -- 2.7.4
[PATCH 12/12] staging: lustre: llite: fixup return value ll_direct_IO_26
From: Dmitry Eremin Return the correct values from ll_direct_IO_26. Signed-off-by: Dmitry Eremin Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-4069 Reviewed-on: http://review.whamcloud.com/8080 Reviewed-by: John L. Hammond Reviewed-by: James Simmons Reviewed-by: Oleg Drokin Signed-Off-by: James Simmons --- drivers/staging/lustre/lustre/llite/rw26.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/lustre/lustre/llite/rw26.c b/drivers/staging/lustre/lustre/llite/rw26.c index a740c7a..9341189 100644 --- a/drivers/staging/lustre/lustre/llite/rw26.c +++ b/drivers/staging/lustre/lustre/llite/rw26.c @@ -446,7 +446,7 @@ out: } cl_env_put(env, ); - return tot_bytes ? : result; + return tot_bytes ? tot_bytes : result; } /** -- 2.7.4