Re: 4.9.0 regression in pipe-backed iov_iter with systemd-nspawn
at 14:26 on Thu 12-Jan-2017 Linus Torvalds (torva...@linux-foundation.org) wrote: > > Strace shows that the processes are hanging in write() and read() calls. > > If this is splice-related, I'm assuming that they aren't actually the > two ends of the same pipe, and there is somebody doing splice in the > middle. > > I'm not seeing that process. I'm assuming it's systemd. Can you try > to find it and strace that one too? strace -f -o /var/tmp/strace.txt systemd-nspawn -q -D /work/chroot.32 --register=no date full output at https://wylie.me.uk/tmp/strace.txt last few lines are: 13766 execve("/bin/date", ["date"], [/* 7 vars */] ... 13766 close(2) = 0 13766 exit_group(0) = ? 13766 +++ exited with 0 +++ 13761 <... epoll_wait resumed> [{EPOLLIN, {u32=3843638720, u64=94179591538112}}], 7, -1) = 1 13761 clock_gettime(CLOCK_BOOTTIME, {9780, 528425388}) = 0 13761 read(13, "\21\0\0\0\0\0\0\0\1\0\0\0\3065\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 128) = 128 13761 epoll_ctl(11, EPOLL_CTL_DEL, 1, NULL) = 0 13761 epoll_ctl(11, EPOLL_CTL_DEL, 5, NULL) = 0 13761 signalfd4(13, [INT TERM CHLD], 8, SFD_CLOEXEC|SFD_NONBLOCK) = 13 13761 fcntl(0, F_GETFL) = 0 (flags O_RDONLY) 13761 fcntl(1, F_GETFL) = 0x1 (flags O_WRONLY) 13761 kill(13766, SIGKILL) = 0 13761 waitid(P_PID, 13766, {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=13766, si_uid=0, si_status=0, si_utime=0, si_stime=0}, WEXITED, NULL) = 0 13761 close(6) = 0 13761 close(7) = 0 13761 close(8) = 0 13761 close(9) = 0 13761 close(18) = 0 13761 close(16) = 0 13761 close(14) = 0 13761 close(10) = 0 13761 copy_file_range(5, NULL, 1, NULL, 9223372036854775807, 0) = -1 EXDEV (Invalid cross-device link) 13761 sendfile(1, 5, NULL, 9223372036854775807) = -1 EINVAL (Invalid argument) 13761 splice(5, NULL, 1, NULL, 9223372036854775807, 0) = -1 EAGAIN (Resource temporarily unavailable) 13761 open("/run/systemd/nspawn/propagate/chroot.32", O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_NOFOLLOW|O_NOATIME|O_CLOEXEC) = 6 13761 fstatfs(6, {f_type=TMPFS_MAGIC, f_bsize=4096, f_blocks=1020327, f_bfree=1015931, f_bavail=1015931, f_files=1020327, f_ffree=1019297, f_fsid={0, 0}, f_namelen=255, f_frsize=4096, f_flags=ST_VALID|ST_NOSUID|ST_NODEV}) = 0 13761 fstat(6, {st_mode=S_IFDIR|0600, st_size=40, ...}) = 0 13761 fcntl(6, F_GETFL) = 0x78800 (flags O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_DIRECTORY|O_NOFOLLOW|O_NOATIME) 13761 fcntl(6, F_SETFD, FD_CLOEXEC) = 0 13761 getdents(6, /* 2 entries */, 32768) = 48 13761 getdents(6, /* 0 entries */, 32768) = 0 13761 close(6) = 0 13761 rmdir("/run/systemd/nspawn/propagate/chroot.32") = 0 13761 unlink("/work/.#chroot.32.lck") = 0 13761 close(3) = 0 13761 unlink("/run/systemd/nspawn/locks/inode-65035:24641537") = 0 13761 close(4) = 0 13761 close(5) = 0 13761 exit_group(0) = ? 13761 +++ exited with 0 +++ I'm off to bed now with a stinking head cold I'm afraid. -- Alan J. Wylie http://www.wylie.me.uk/ Dance like no-one's watching. / Encrypt like everyone is. Security is inversely proportional to convenience
Re: 4.9.0 regression in pipe-backed iov_iter with systemd-nspawn
On Thu, Jan 12, 2017 at 10:37:18PM +, Al Viro wrote: > On Thu, Jan 12, 2017 at 02:26:42PM -0800, Linus Torvalds wrote: > > On Thu, Jan 12, 2017 at 12:26 PM, Alan J. Wylie wrote: > > > > > > Strace shows that the processes are hanging in write() and read() calls. > > > > If this is splice-related, I'm assuming that they aren't actually the > > two ends of the same pipe, and there is somebody doing splice in the > > middle. > > > > I'm not seeing that process. I'm assuming it's systemd. Can you try > > to find it and strace that one too? Because that middle man is likely > > the one that has problems (and is not able to splice from one pipe to > > the other). > > > > Ugh. That one commit has had a lot of bugs in it already. We do not > > have good splice test coverage, because almost nobody uses it. > > FWIW, I would really like to know what kind of files had been involved. > There are two paths that can lead to default_file_splice_read(): > splice_direct_to_actor() -> do_splice_to() -> default_file_splice_read() and > do_splice() -> do_splice_to() -> default_file_splice_read(). > > The former only gets there for regular files and block devices. The latter > is guaranteed that file is not a pipe. So > * not a socket (have ->splice_read() of their own) > * not a pipe or FIFO (neither path allows those) > * not a block device (have ->splice_read() of their own) > * not a regular file on a normal local fs (ditto) > > So what is it called for in that reproducer? PS: what about the /proc/mounts contents? If it's something 9p-backed kvm, your bisect might have been caught on the bug I'd mentioned - if the breakage you are seeing in 4.9.3 has started after that commit and before the backport of the fix, your bisect could converge there. Does the reproducer trigger on 523ac9afc73a + cherry-pick of 8e54cadab447?
[patch v2] mm, memcg: do not retry precharge charges
When memory.move_charge_at_immigrate is enabled and precharges are depleted during move, mem_cgroup_move_charge_pte_range() will attempt to increase the size of the precharge. This can be allowed to do reclaim, but should not call the oom killer to oom kill a process. It's better to fail the attach rather than oom kill a process attached to the memcg hierarchy. Prevent precharges from ever looping by setting __GFP_NORETRY. This was probably the intention of the GFP_KERNEL & ~__GFP_NORETRY, which is pointless as written. Fixes: 0029e19ebf84 ("mm: memcontrol: remove explicit OOM parameter in charge path") Signed-off-by: David Rientjes --- mm/memcontrol.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/mm/memcontrol.c b/mm/memcontrol.c --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -4353,9 +4353,12 @@ static int mem_cgroup_do_precharge(unsigned long count) return ret; } - /* Try charges one by one with reclaim */ + /* +* Try charges one by one with reclaim, but do not retry. This avoids +* calling the oom killer when the precharge should just fail. +*/ while (count--) { - ret = try_charge(mc.to, GFP_KERNEL & ~__GFP_NORETRY, 1); + ret = try_charge(mc.to, GFP_KERNEL | __GFP_NORETRY, 1); if (ret) return ret; mc.precharge++;
Re: [PATCH v2] tcp: fix tcp_fastopen unaligned access complaints on sparc
On Thu, 2017-01-12 at 14:24 -0800, Shannon Nelson wrote: > Fix up a data alignment issue on sparc by swapping the order > of the cookie byte array field with the length field in > struct tcp_fastopen_cookie, and making it a proper union > to clean up the typecasting. > > This addresses log complaints like these: > log_unaligned: 113 callbacks suppressed > Kernel unaligned access at TPC[976490] tcp_try_fastopen+0x2d0/0x360 > Kernel unaligned access at TPC[9764ac] tcp_try_fastopen+0x2ec/0x360 > Kernel unaligned access at TPC[9764c8] tcp_try_fastopen+0x308/0x360 > Kernel unaligned access at TPC[9764e4] tcp_try_fastopen+0x324/0x360 > Kernel unaligned access at TPC[976490] tcp_try_fastopen+0x2d0/0x360 > > Cc: Eric Dumazet > Signed-off-by: Shannon Nelson > --- > v2: Use Eric's suggestion for a union in the struct Acked-by: Eric Dumazet Thanks for fixing this !
Re: [PATCH net-next v2 05/10] drivers: base: Add device_find_class()
On 01/12/2017 01:21 PM, David Miller wrote: > From: Florian Fainelli > Date: Wed, 11 Jan 2017 19:41:16 -0800 > >> Add a helper function to lookup a device reference given a class name. >> This is a preliminary patch to remove adhoc code from net/dsa/dsa.c and >> make it more generic. >> >> Signed-off-by: Florian Fainelli >> --- >> drivers/base/core.c| 19 +++ >> include/linux/device.h | 1 + >> 2 files changed, 20 insertions(+) >> >> diff --git a/drivers/base/core.c b/drivers/base/core.c >> index 020ea7f05520..3dd6047c10d8 100644 >> --- a/drivers/base/core.c >> +++ b/drivers/base/core.c >> @@ -2065,6 +2065,25 @@ struct device *device_find_child(struct device >> *parent, void *data, >> } >> EXPORT_SYMBOL_GPL(device_find_child); >> >> +static int dev_is_class(struct device *dev, void *class) > > I know you are just moving code, but this class argumnet is a string > and thus should be "char *" or even "const char *". Well, this is really so that we don't need to cast the arguments passed to device_find_child(), which takes a void *data as well. If we made that a const char *class, we'd get warnings that look like these: drivers/base/core.c: In function 'device_find_class': drivers/base/core.c:2083:2: warning: passing argument 2 of 'device_find_child' discards 'const' qualifier from pointer target type [enabled by default] return device_find_child(parent, class, dev_is_class); ^ drivers/base/core.c:2050:16: note: expected 'void *' but argument is of type 'const char *' struct device *device_find_child(struct device *parent, void *data, ^ drivers/base/core.c:2083:2: warning: passing argument 3 of 'device_find_child' from incompatible pointer type [enabled by default] return device_find_child(parent, class, dev_is_class); ^ drivers/base/core.c:2050:16: note: expected 'int (*)(struct device *, void *)' but argument is of type 'int (*)(struct device *, const char *)' struct device *device_find_child(struct device *parent, void *data, ^ -- Florian
Re: [PATCH v4 1/2] eeprom: Add IDT 89HPESx EEPROM/CSR driver
On Wed, Jan 11, 2017 at 09:21:19AM +0100, Greg KH wrote: > On Tue, Dec 13, 2016 at 05:22:50PM +0300, Serge Semin wrote: > > +struct idt_89hpesx_dev { > > + u32 eesize; > > + bool eero; > > + u8 eeaddr; > > + > > + u8 inieecmd; > > + u8 inicsrcmd; > > + u8 iniccode; > > + > > + atomic_t csr; > > Why is this an atomic_t and not just a "normal" u32 or u64? I don't see > the need for an atomic variable at all here, do you? > Of course, I did. Since it was shared resource before it was necessary to have it atomically accessed. But since we moved "csr" sysfs node to DebugFS, it might not be necessary of atomic_t type. Ok, I'll make it unsigned int. > > > + > > + int (*smb_write)(struct idt_89hpesx_dev *, const struct idt_smb_seq *); > > + int (*smb_read)(struct idt_89hpesx_dev *, struct idt_smb_seq *); > > + struct mutex smb_mtx; > > + > > + struct i2c_client *client; > > + > > + struct bin_attribute *ee_file; > > + struct dentry *csr_dir; > > + struct dentry *csr_file; > > +}; > > > > > + > > +static int idt_create_dbgfs_files(struct idt_89hpesx_dev *pdev) > > +{ > > + struct device *dev = &pdev->client->dev; > > + struct i2c_client *cli = pdev->client; > > + char fname[CSRNAME_LEN]; > > + > > + /* Initialize basic value of CSR debugfs dentries */ > > + pdev->csr_dir = NULL; > > + pdev->csr_file = NULL; > > + > > + /* Return failure if root directory doesn't exist */ > > + if (!csr_dbgdir) { > > + dev_dbg(dev, "No Debugfs root directory"); > > + return -EINVAL; > > + } > > If debugfs is not enabled, don't error out, just keep going, it should > never stop kernel code from running properly. > > Also, this test isn't really doing what you think it is doing... > I see, it must be replaced with IS_ERR_OR_NULL() test. But I don't think, it would be good to get rid of dev_dbg() completely here. In case if debugging is enabled, user would understand why csr-node isn't created within DebugFS directory. I don't see the reasoning why one shouldn't know a source of possible problems. (See the next comment as continue of the discussion) > > + /* Create Debugfs directory for CSR file */ > > + snprintf(fname, CSRNAME_LEN, "%d-%04hx", cli->adapter->nr, cli->addr); > > + pdev->csr_dir = debugfs_create_dir(fname, csr_dbgdir); > > + if (IS_ERR_OR_NULL(pdev->csr_dir)) { > > + dev_err(dev, "Failed to create CSR node directory"); > > + return -EINVAL; > > Again, don't do this, you really don't care if debugfs worked or not. > Actually the driver doesn't stop the kernel code from running, if it finds out any problem with DebugFS CSR-node creation. The function just logs the error and return error status. Take a look the place the method is called: 1489/* Create debugfs files */ 1490(void)idt_create_dbgfs_files(pdev); The initialization code doesn't check the return value at all, so the driver will proceed with further code. Why did I make the function with return value? Because it's a good style to always return a status of function code execution if it may fail, but only caller will decide whether to check the return value or not. Regarding the error printing. In case if the code gets to this check, one can be sure the DebugFS works properly, so in case if the driver failed to create the corresponding sub-directory or node, it is really error to have any failure at this point, and a user should be notified. But still the driver won't stop functioning, since the caller doesn't check the return value. Hopefully you'll understand my point. > > + } > > + > > + /* Create Debugfs file for CSR read/write operations */ > > + pdev->csr_file = debugfs_create_file(cli->name, 0600, > > + pdev->csr_dir, pdev, &csr_dbgfs_ops); > > + if (IS_ERR_OR_NULL(pdev->csr_file)) { > > + dev_err(dev, "Failed to create CSR dbgfs-node"); > > + debugfs_remove_recursive(pdev->csr_dir); > > + return -EINVAL; > > Same here, just create the file and move on. > > > + } > > + > > + dev_dbg(dev, "Debugfs-files created"); > > You do know about ftrace, right? Please remove all of these > "trace-like" debugging lines, they aren't needed for anyone. > Ok, I'll remove all these prints, even though I do find these prints being handy to have initialization process printed on debugging stage. > thanks, > > greg k-h thanks, Sergey
Re: [PATCH] clk: stm32f4: avoid uninitialized variable access
On 01/12/2017 02:42 PM, Arnd Bergmann wrote: > On Thu, Jan 12, 2017 at 11:06 PM, Stephen Boyd wrote: >> Applied to clk-next. Seems I need to update my compiler to find >> these warnings. > I'm currently playing with gcc-7, which adds a lot of new warnings > (including many false positives). gcc-6 was supposed to have better > warnings than 5, but I didn't find the difference that noticeable. 5 > or 6 is probably best at the moment, and if you have at least 4.9 > there is no urgent need to upgrade. Thanks. I'm on gcc-4.7. It's been a long time since I upgraded my cross compiler. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
[�PATCH net-next] ARM: dts: vf610-zii-dev: add EEPROM entry to Rev B
The ZII Dev Rev B board has EEPROMs hanging the 88E6352 Ethernet switch chips. Add an "eeprom-length" property to allow access from ethtool. Signed-off-by: Vivien Didelot --- arch/arm/boot/dts/vf610-zii-dev-rev-b.dts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm/boot/dts/vf610-zii-dev-rev-b.dts b/arch/arm/boot/dts/vf610-zii-dev-rev-b.dts index 958b4c42d320..b60d3d03f58c 100644 --- a/arch/arm/boot/dts/vf610-zii-dev-rev-b.dts +++ b/arch/arm/boot/dts/vf610-zii-dev-rev-b.dts @@ -98,6 +98,7 @@ interrupts = <27 IRQ_TYPE_LEVEL_LOW>; interrupt-controller; #interrupt-cells = <2>; + eeprom-length = <512>; ports { #address-cells = <1>; @@ -181,6 +182,7 @@ interrupts = <26 IRQ_TYPE_LEVEL_LOW>; interrupt-controller; #interrupt-cells = <2>; + eeprom-length = <512>; ports { #address-cells = <1>; -- 2.11.0
Re: 4.9.0 regression in pipe-backed iov_iter with systemd-nspawn
On Thu, Jan 12, 2017 at 10:46:06PM +, Alan J. Wylie wrote: > at 14:26 on Thu 12-Jan-2017 Linus Torvalds (torva...@linux-foundation.org) > wrote: > 13761 copy_file_range(5, NULL, 1, NULL, 9223372036854775807, 0) = -1 EXDEV > (Invalid cross-device link) > 13761 sendfile(1, 5, NULL, 9223372036854775807) = -1 EINVAL (Invalid argument) > 13761 splice(5, NULL, 1, NULL, 9223372036854775807, 0) = -1 EAGAIN (Resource > temporarily unavailable) *bleep* splice from /dev/ptmx to stdout, whatever the latter had been. IOW, we are dealing with do_splice() -> do_splice_to() -> default_file_splice_read() for pseudo-tty as source...
Re: 4.9.0 regression in pipe-backed iov_iter with systemd-nspawn
On Thu, Jan 12, 2017 at 2:46 PM, Al Viro wrote: > > PS: what about the /proc/mounts contents? If it's something 9p-backed kvm, > your bisect might have been caught on the bug I'd mentioned - if the breakage > you are seeing in 4.9.3 has started after that commit and before the > backport of the fix, your bisect could converge there. Does the > reproducer trigger on 523ac9afc73a + cherry-pick of 8e54cadab447? Looking at the strace that Alan just posted, I really think it's the splice change that Alan bisected to. In particular, this line: splice(5, NULL, 1, NULL, 9223372036854775807, 0) = -1 EAGAIN (Resource temporarily unavailable) and note that the commit in question introduces that -EAGAIN error code. The old code never returned EAGAIN at all (well, it could do so later, if NONBLOCK was set, obviously, but that doesn't seem to be the case here). So that commit seems to have introduced a new error case, and I suspect systemd-nospawn simply doesn't handle it. It is expecting splice_to_pipe() to actually block. Ergo: I think we need to do a wait_for_space() somewhere, getting rid of the EAGAIN. Looking at the callers of "do_splice_to()", we already have the wait_for_space() in do_splice(), but we do *not* have it in the do_splice_from() case when both the input and output file descriptors are pipes. Hmm? Linus
Re: [PATCH] Input: egalax_ts - do not release gpio if probe successful
Hi Dmitri, On Thu, Jan 12, 2017 at 10:30:19AM -0800, Dmitry Torokhov wrote: > Hi Gary, > > On Wed, Jan 11, 2017 at 06:28:41PM +0100, Gary Bisson wrote: > > Thus preventing anyone to later modify the interrupt GPIO direction > > and/or state without the driver knowing. > > I am afraid not releasing gpio after waking up the controller will cause > request_irq to fail if we are using the same pin for interrupt and > wakeup (as majority of current DTSes do: see > arch/arm/boot/dts/imx53-tx53-x13x.dts for example). No, keeping the GPIO doesn't prevent from requesting the IRQ. However it keeps other drivers/users from changing the GPIO as output later on. > You'll need to figure out if irq is backed by the same gpio as wakeup > and act accordingly. > > What setup did you test this on? Was it shared pin or dedicated wakeup? I've tested on i.MX6Q Nitrogen6x [1] with Hannstar 10" display [2]. Regards, Gary [1] https://boundarydevices.com/product/nitrogen6x-board-imx6-arm-cortex-a9-sbc/ [2] https://boundarydevices.com/product/nit6x_10-1hannstar/
[PATCH] ARM: dts: vf610-zii-dev: add EEPROM entry to Rev C
The ZII Dev Rev C board has EEPROMs hanging the 88E6390 Ethernet switch chips. Add an "eeprom-length" property to allow access from ethtool. Signed-off-by: Vivien Didelot --- arch/arm/boot/dts/vf610-zii-dev-rev-c.dts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm/boot/dts/vf610-zii-dev-rev-c.dts b/arch/arm/boot/dts/vf610-zii-dev-rev-c.dts index fbedb7bb3628..6a45bd24ffe6 100644 --- a/arch/arm/boot/dts/vf610-zii-dev-rev-c.dts +++ b/arch/arm/boot/dts/vf610-zii-dev-rev-c.dts @@ -71,6 +71,7 @@ #size-cells = <0>; reg = <0>; dsa,member = <0 0>; + eeprom-length = <512>; ports { #address-cells = <1>; @@ -128,6 +129,7 @@ #size-cells = <0>; reg = <0>; dsa,member = <0 1>; + eeprom-length = <512>; ports { #address-cells = <1>; -- 2.11.0
[PATCH net-next] net: dsa: mv88e6xxx: add EEPROM support to 6390
The Marvell 6352 chip has a 8-bit address/16-bit data EEPROM access. The Marvell 6390 chip has a 16-bit address/8-bit data EEPROM access. This patch implements the 8-bit data EEPROM access in the mv88e6xxx driver and adds its support to chips of the 6390 family. Signed-off-by: Vivien Didelot --- drivers/net/dsa/mv88e6xxx/chip.c | 14 ++ drivers/net/dsa/mv88e6xxx/global2.c | 93 ++- drivers/net/dsa/mv88e6xxx/global2.h | 21 drivers/net/dsa/mv88e6xxx/mv88e6xxx.h | 1 + 4 files changed, 128 insertions(+), 1 deletion(-) diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c index eea8e0176e33..987b2dbbd35a 100644 --- a/drivers/net/dsa/mv88e6xxx/chip.c +++ b/drivers/net/dsa/mv88e6xxx/chip.c @@ -3453,6 +3453,8 @@ static const struct mv88e6xxx_ops mv88e6185_ops = { static const struct mv88e6xxx_ops mv88e6190_ops = { /* MV88E6XXX_FAMILY_6390 */ + .get_eeprom = mv88e6xxx_g2_get_eeprom8, + .set_eeprom = mv88e6xxx_g2_set_eeprom8, .set_switch_mac = mv88e6xxx_g2_set_switch_mac, .phy_read = mv88e6xxx_g2_smi_phy_read, .phy_write = mv88e6xxx_g2_smi_phy_write, @@ -3478,6 +3480,8 @@ static const struct mv88e6xxx_ops mv88e6190_ops = { static const struct mv88e6xxx_ops mv88e6190x_ops = { /* MV88E6XXX_FAMILY_6390 */ + .get_eeprom = mv88e6xxx_g2_get_eeprom8, + .set_eeprom = mv88e6xxx_g2_set_eeprom8, .set_switch_mac = mv88e6xxx_g2_set_switch_mac, .phy_read = mv88e6xxx_g2_smi_phy_read, .phy_write = mv88e6xxx_g2_smi_phy_write, @@ -3503,6 +3507,8 @@ static const struct mv88e6xxx_ops mv88e6190x_ops = { static const struct mv88e6xxx_ops mv88e6191_ops = { /* MV88E6XXX_FAMILY_6390 */ + .get_eeprom = mv88e6xxx_g2_get_eeprom8, + .set_eeprom = mv88e6xxx_g2_set_eeprom8, .set_switch_mac = mv88e6xxx_g2_set_switch_mac, .phy_read = mv88e6xxx_g2_smi_phy_read, .phy_write = mv88e6xxx_g2_smi_phy_write, @@ -3556,6 +3562,8 @@ static const struct mv88e6xxx_ops mv88e6240_ops = { static const struct mv88e6xxx_ops mv88e6290_ops = { /* MV88E6XXX_FAMILY_6390 */ + .get_eeprom = mv88e6xxx_g2_get_eeprom8, + .set_eeprom = mv88e6xxx_g2_set_eeprom8, .set_switch_mac = mv88e6xxx_g2_set_switch_mac, .phy_read = mv88e6xxx_g2_smi_phy_read, .phy_write = mv88e6xxx_g2_smi_phy_write, @@ -3714,6 +3722,8 @@ static const struct mv88e6xxx_ops mv88e6352_ops = { static const struct mv88e6xxx_ops mv88e6390_ops = { /* MV88E6XXX_FAMILY_6390 */ + .get_eeprom = mv88e6xxx_g2_get_eeprom8, + .set_eeprom = mv88e6xxx_g2_set_eeprom8, .set_switch_mac = mv88e6xxx_g2_set_switch_mac, .phy_read = mv88e6xxx_g2_smi_phy_read, .phy_write = mv88e6xxx_g2_smi_phy_write, @@ -3741,6 +3751,8 @@ static const struct mv88e6xxx_ops mv88e6390_ops = { static const struct mv88e6xxx_ops mv88e6390x_ops = { /* MV88E6XXX_FAMILY_6390 */ + .get_eeprom = mv88e6xxx_g2_get_eeprom8, + .set_eeprom = mv88e6xxx_g2_set_eeprom8, .set_switch_mac = mv88e6xxx_g2_set_switch_mac, .phy_read = mv88e6xxx_g2_smi_phy_read, .phy_write = mv88e6xxx_g2_smi_phy_write, @@ -3768,6 +3780,8 @@ static const struct mv88e6xxx_ops mv88e6390x_ops = { static const struct mv88e6xxx_ops mv88e6391_ops = { /* MV88E6XXX_FAMILY_6390 */ + .get_eeprom = mv88e6xxx_g2_get_eeprom8, + .set_eeprom = mv88e6xxx_g2_set_eeprom8, .set_switch_mac = mv88e6xxx_g2_set_switch_mac, .phy_read = mv88e6xxx_g2_smi_phy_read, .phy_write = mv88e6xxx_g2_smi_phy_write, diff --git a/drivers/net/dsa/mv88e6xxx/global2.c b/drivers/net/dsa/mv88e6xxx/global2.c index 3e77071949ab..ead2e265c9ef 100644 --- a/drivers/net/dsa/mv88e6xxx/global2.c +++ b/drivers/net/dsa/mv88e6xxx/global2.c @@ -218,7 +218,8 @@ static int mv88e6xxx_g2_clear_pot(struct mv88e6xxx_chip *chip) } /* Offset 0x14: EEPROM Command - * Offset 0x15: EEPROM Data + * Offset 0x15: EEPROM Data (for 16-bit data access) + * Offset 0x15: EEPROM Addr (for 8-bit data access) */ static int mv88e6xxx_g2_eeprom_wait(struct mv88e6xxx_chip *chip) @@ -239,6 +240,50 @@ static int mv88e6xxx_g2_eeprom_cmd(struct mv88e6xxx_chip *chip, u16 cmd) return mv88e6xxx_g2_eeprom_wait(chip); } +static int mv88e6xxx_g2_eeprom_read8(struct mv88e6xxx_chip *chip, +u16 addr, u8 *data) +{ + u16 cmd = GLOBAL2_EEPROM_CMD_OP_READ; + int err; + + err = mv88e6xxx_g2_eeprom_wait(chip); + if (err) + return err; + + err = mv88e6xxx_g2_write(chip, GLOBAL2_EEPROM_ADDR, addr); + if (err) + return err; + + err = mv88e6xxx_g2_eeprom_cmd(chip, cmd); + if (err) + return err; + + err = mv88e6xxx_g2_read(chip, GLOBAL2_EEPROM_CMD, &cmd); + if (err) + return err; + + *data = cmd & 0xff; + +
Re: [PATCH] xen-netfront: Fix Rx stall during network stress and OOM
On 01/12/2017 12:17 PM, David Miller wrote: From: Vineeth Remanan Pillai Date: Wed, 11 Jan 2017 23:17:17 + @@ -1054,7 +1059,11 @@ static int xennet_poll(struct napi_struct *napi, int budget) napi_complete(napi); RING_FINAL_CHECK_FOR_RESPONSES(&queue->rx, more_to_do); - if (more_to_do) + + /* If there is more work to do or could not allocate +* rx buffers, re-enable polling. +*/ + if (more_to_do || err != 0) napi_schedule(napi); Just polling endlessly in a loop retrying the SKB allocation over and over again until it succeeds is not very nice behavior. You already have that refill timer, so please use that to retry instead of wasting cpu cycles looping in NAPI poll. Thanks Dave for the inputs. On further look, I think I can fix it much simpler by correcting the test condition for minimum slots for pushing requests. Existing test is like this: /* Not enough requests? Try again later. */ if (req_prod - queue->rx.rsp_cons < NET_RX_SLOTS_MIN) { mod_timer(&queue->rx_refill_timer, jiffies + (HZ/10)); return; } Actually the above check counts more than the newly created request slots as it counts from rsp_cons. The actual count should be the difference between new req_prod and old req_prod(in the queue). If skbs cannot be created, this count remains small and hence we would schedule the timer. So the fix could be: /* Not enough requests? Try again later. */ - if (req_prod - queue->rx.rsp_cons < NET_RX_SLOTS_MIN) { + if (req_prod - queue->rx.sring->req_prod < NET_RX_SLOTS_MIN) { I have done some initial testing to verify the fix. Will send out v2 patch after couple more round of testing. Thanks, Vineeth
Re: [PATCH] can: Fix kernel panic at security_sock_rcv_skb
On Thu, 2017-01-12 at 14:40 -0800, william.c.robe...@intel.com wrote: > From: Zhang Yanmin > > The patch is for fix the below kernel panic: > BUG: unable to handle kernel NULL pointer dereference at (null) > IP: [] selinux_socket_sock_rcv_skb+0x65/0x2a0 Same patch was sent earlier, and we gave a feedback on it. Adding synchronize_rcu() calls is a step backward. https://patchwork.ozlabs.org/patch/714446/
Re: [PATCH 2/4] efi/x86: move efi bgrt init code to early init code
On Fri, Jan 13 2017, Dave Young wrote: > On 01/12/17 at 12:54pm, Nicolai Stange wrote: >> On Thu, Jan 12 2017, Dave Young wrote: >> >> > -void __init efi_bgrt_init(void) >> > +void __init efi_bgrt_init(struct acpi_table_header *table) >> > { >> > - acpi_status status; >> >void *image; >> >struct bmp_header bmp_header; >> > >> >if (acpi_disabled) >> >return; >> > >> > - status = acpi_get_table("BGRT", 0, >> > - (struct acpi_table_header **)&bgrt_tab); >> > - if (ACPI_FAILURE(status)) >> > - return; >> >> >> Not sure, but wouldn't it be safer to reverse the order of this assignment >> >> > + bgrt_tab = *(struct acpi_table_bgrt *)table; > > Nicolai, sorry, I'm not sure I understand the comment, is it about above line? > Could you elaborate a bit? > >> >> and this length check >> > > I also do not get this :( Ah sorry, my point is this: the length check should perhaps be made before doing the assignment to bgrt_tab because otherwise, we might end up reading from invalid memory. I.e. if (struct acpi_table_bgrt *)table->length < sizeof(bgrt_tab), then bgrt_tab = *(struct acpi_table_bgrt *)table; would read past the table's end. I'm not sure whether this is a real problem though -- that is, whether this read could ever hit some unmapped memory. >> > - if (bgrt_tab->header.length < sizeof(*bgrt_tab)) { >> > + if (bgrt_tab.header.length < sizeof(bgrt_tab)) { >> >pr_notice("Ignoring BGRT: invalid length %u (expected %zu)\n", >> > - bgrt_tab->header.length, sizeof(*bgrt_tab)); >> > + bgrt_tab.header.length, sizeof(bgrt_tab)); >> >return; >> >} >> >> ? Thanks, Nicolai
Re: 4.9.0 regression in pipe-backed iov_iter with systemd-nspawn
On Thu, Jan 12, 2017 at 03:02:13PM -0800, Linus Torvalds wrote: > splice(5, NULL, 1, NULL, 9223372036854775807, 0) = -1 EAGAIN > (Resource temporarily unavailable) > > and note that the commit in question introduces that -EAGAIN error code. > > The old code never returned EAGAIN at all (well, it could do so later, > if NONBLOCK was set, obviously, but that doesn't seem to be the case > here). > > So that commit seems to have introduced a new error case, and I > suspect systemd-nospawn simply doesn't handle it. It is expecting > splice_to_pipe() to actually block. > > Ergo: I think we need to do a wait_for_space() somewhere, getting rid > of the EAGAIN. > > Looking at the callers of "do_splice_to()", we already have the > wait_for_space() in do_splice(), but we do *not* have it in the > do_splice_from() case when both the input and output file descriptors > are pipes. >From the look of his strace, the source is /dev/ptmx. Pipe-to-pipe splice goes into splice_pipe_to_pipe() anyway. do_splice_from() is pipe-to-non-pipe, and it doesn't go anywhere near default_file_splice_read()... do_splice_to() is the only thing that can call default_file_splice_read() and there are only two callers - do_splice() (with its wait_for_space()) and splice_direct_to_actor(), which has internal pipe for destination. That certainly shouldn't be calling wait_for_space() - there's no other thread that could possibly read from the destination getting more space in there. IOW, it's do_splice() -> do_splice_to() -> default_file_splice_read().
Re: 4.9.0 regression in pipe-backed iov_iter with systemd-nspawn
On Thu, Jan 12, 2017 at 3:02 PM, Linus Torvalds wrote: > > Looking at the callers of "do_splice_to()", we already have the > wait_for_space() in do_splice(), but we do *not* have it in the > do_splice_from() case when both the input and output file descriptors > are pipes. Bah. That case doesn't even trigger the new code. I was lazy with my grep. The two cases are "do_splice()" (which does have the wait-for-space) and splice_direct_to_actor(). And splice_direct_to_actor() shouldn't even need it, should it? So ignore that. But I think there is something about the EAGAIN. Linus
Re: 4.9.0 regression in pipe-backed iov_iter with systemd-nspawn
On Thu, Jan 12, 2017 at 03:14:41PM -0800, Linus Torvalds wrote: > On Thu, Jan 12, 2017 at 3:02 PM, Linus Torvalds > wrote: > > > > Looking at the callers of "do_splice_to()", we already have the > > wait_for_space() in do_splice(), but we do *not* have it in the > > do_splice_from() case when both the input and output file descriptors > > are pipes. > > Bah. That case doesn't even trigger the new code. I was lazy with my > grep. The two cases are "do_splice()" (which does have the > wait-for-space) and splice_direct_to_actor(). And > splice_direct_to_actor() shouldn't even need it, should it? > > So ignore that. But I think there is something about the EAGAIN. It might, but I would really like to see where has that EAGAIN come from. I see several possibilities: * wait_for_space() with SPLICE_F_NONBLOCK in flags. Shouldn't happen with 0 in the last argument of splice(2). * default_file_splice_read() seeing pipe->nrbufs == pipe->buffers. Shouldn't be possible after successful wait_for_space(). * vfs_readv() returning -EAGAIN. That might be possible, actually - the damn thing has come from 13761 open("/dev/ptmx", O_RDWR|O_NOCTTY|O_NONBLOCK|O_CLOEXEC) = 5 and O_NONBLOCK had been present in open flags. What I'd like to see is strace of the same thing on the working kernel, ideally - just prior to the commit bisect has converged to.
Re: [PATCH v4 0/4] Introduce the initify gcc plugin
On Thu, Jan 12, 2017 at 1:41 PM, Emese Revfy wrote: > On Tue, 10 Jan 2017 17:09:31 -0800 > Kees Cook wrote: > >> WARNING: vmlinux.o(.text+0x1087e7): Section mismatch in reference from >> the function rebind_subsystems() to the variable >> .init.rodata.str:__func__.4400 >> The function rebind_subsystems() references >> the variable __initconst __func__.4400. >> This is often because rebind_subsystems lacks a __initconst >> annotation or the annotation of __func__.4400 is wrong. > > Thanks for the report, you can find the fix here: > https://github.com/ephox-gcc-plugins/initify/commit/25f34834e3373e067133bc5d39d42c50a3592d56 Awesome! I can confirm, it builds without warnings now. Thanks! -Kees -- Kees Cook Nexus Security
Re: 4.9.0 regression in pipe-backed iov_iter with systemd-nspawn
On Thu, Jan 12, 2017 at 2:46 PM, Alan J. Wylie wrote: > > I'm off to bed now with a stinking head cold I'm afraid. So assuming Al hasn't figured it out by the time you get back, can you try to send us the same strace for the working case? That fd5 (ptmx) does have O_NONBLOCK, so maybe the EAGAIN actually comes from reading the ptmx, rather than the new AGAIN in fs/splice.c. But if so, I'd have expected the old code to do the same thing. Ho humm. Linus
Re: [PATCH] MIPS: zboot: fix build failure
On Wednesday 04 January 2017 06:04 PM, Sudip Mukherjee wrote: The build of mips ar7_defconfig was failing with the error: arch/mips/boot/compressed/Makefile:21: *** insufficient number of arguments (1) to function `filter-out'. Stop. A ',' was missing while adding filter-out. Fixes: afca036d463c ("MIPS: zboot: Consolidate compiler flag filtering.") Signed-off-by: Sudip Mukherjee --- A gentle ping. Many of the mips builds are failing daily. regards sudip
Re: linux-next: build failure after merge of the akpm-current tree
On Thu, 12 Jan 2017 13:06:01 +0800 Eric Ren wrote: > On 01/12/2017 11:49 AM, Stephen Rothwell wrote: > > Hi Andrew, > > > > After merging the akpm tree, today's linux-next build (powerpc > > allyesconfig) failed like this: > > > > In file included from fs/ocfs2/file.c:49:0: > > fs/ocfs2/file.c: In function 'ocfs2_permission': > > fs/ocfs2/dlmglue.h:189:29: error: inlining failed in call to always_inline > > 'ocfs2_is_locked_by_me': function body not available > > inline struct ocfs2_holder *ocfs2_is_locked_by_me(struct ocfs2_lock_res > > *lockres); > > ^ > > fs/ocfs2/file.c:1345:16: error: called from here > >has_locked = (ocfs2_is_locked_by_me(lockres) != NULL); > > ^ > ... > > > > Caused by commits > > > >984c4659d463 ("ocfs2/dlmglue: prepare tracking logic to avoid recursive > > cluster lock") > >0ca17730270e ("ocfs2: fix deadlocks when taking inode lock at vfs entry > > points") > > (top-posting repaired. Please don't do that) > Hi Stephen, > > Thanks for your report and the fix for it. The 0-day project has reported > several days ago, > but this patch set is still in discussion, so I am waiting for more days to > see if other > developers > have any other questions. > > I am confused that how to deal with your patch if I need to work out the V2 > patch set. Perhaps, > pick up your fix and add your efforts in the change log? > I'll drop ocfs2-dlmglue-prepare-tracking-logic-to-avoid-recursive-cluster-lock.patch and ocfs2-fix-deadlocks-when-taking-inode-lock-at-vfs-entry-points.patch
Re: [PATCH v3 10/24] ARM: dts: imx6-sabreauto: add pinctrl for gpt input capture
On 01/12/2017 11:37 AM, Tim Harvey wrote: On Fri, Jan 6, 2017 at 6:11 PM, Steve Longerbeam wrote: Add pinctrl groups for both GPT input capture channels. Signed-off-by: Steve Longerbeam --- arch/arm/boot/dts/imx6qdl-sabreauto.dtsi | 12 1 file changed, 12 insertions(+) diff --git a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi index 967c3b8..495709f 100644 --- a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi +++ b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi @@ -457,6 +457,18 @@ >; }; + pinctrl_gpt_input_capture0: gptinputcapture0grp { + fsl,pins = < + MX6QDL_PAD_SD1_DAT0__GPT_CAPTURE1 0x1b0b0 + >; + }; + + pinctrl_gpt_input_capture1: gptinputcapture1grp { + fsl,pins = < + MX6QDL_PAD_SD1_DAT1__GPT_CAPTURE2 0x1b0b0 + >; + }; + pinctrl_spdif: spdifgrp { fsl,pins = < MX6QDL_PAD_KEY_COL3__SPDIF_IN 0x1b0b0 -- Steve, These are not used anywhere. Yes, maybe I should just remove this patch for now. I'm only keeping it because eventually it will be needed to support i.MX6 input capture. Steve
Re: [PATCH] Input: egalax_ts - do not release gpio if probe successful
On Fri, Jan 13, 2017 at 12:03:43AM +0100, Gary Bisson wrote: > Hi Dmitri, > > On Thu, Jan 12, 2017 at 10:30:19AM -0800, Dmitry Torokhov wrote: > > Hi Gary, > > > > On Wed, Jan 11, 2017 at 06:28:41PM +0100, Gary Bisson wrote: > > > Thus preventing anyone to later modify the interrupt GPIO direction > > > and/or state without the driver knowing. > > > > I am afraid not releasing gpio after waking up the controller will cause > > request_irq to fail if we are using the same pin for interrupt and > > wakeup (as majority of current DTSes do: see > > arch/arm/boot/dts/imx53-tx53-x13x.dts for example). > > No, keeping the GPIO doesn't prevent from requesting the IRQ. > > However it keeps other drivers/users from changing the GPIO as output > later on. Hmm, I think _gpiod_direction_output_raw() will not let them as it checks FLAG_USED_AS_IRQ, so as long as it's the same line it's OK. But I guess if they are separate some other component might try to grab it and mess with it. OK, in this case please: - use devm_gpiod_get - request with GPIOD_OUT_LOW - do not override the return value with -ENODEV (especially important with probe deferrals) - lose gpiod_direction_output() call - move everything into egalax_ts_probe() as egalax_wake_up_device() is no longer self-contained. Thanks! -- Dmitry
Re: [PATCH v4 0/4] Introduce the initify gcc plugin
On Thu, Jan 12, 2017 at 3:27 PM, Kees Cook wrote: > On Thu, Jan 12, 2017 at 1:41 PM, Emese Revfy wrote: >> On Tue, 10 Jan 2017 17:09:31 -0800 >> Kees Cook wrote: >> >>> WARNING: vmlinux.o(.text+0x1087e7): Section mismatch in reference from >>> the function rebind_subsystems() to the variable >>> .init.rodata.str:__func__.4400 >>> The function rebind_subsystems() references >>> the variable __initconst __func__.4400. >>> This is often because rebind_subsystems lacks a __initconst >>> annotation or the annotation of __func__.4400 is wrong. >> >> Thanks for the report, you can find the fix here: >> https://github.com/ephox-gcc-plugins/initify/commit/25f34834e3373e067133bc5d39d42c50a3592d56 > > Awesome! I can confirm, it builds without warnings now. Thanks! Hm, actually, with an "allyesconfig" build, I'm still seeing warnings (and possibly some nocapture verification failures). Most look like this: WARNING: drivers/clk/bcm/built-in.o(.text+0xec2): Section mismatch in reference from the function clk_gate() to the variable .init.rodata.str:__func__.29708 The function clk_gate() references the variable __initconst __func__.29708. This is often because clk_gate lacks a __initconst annotation or the annotation of __func__.29708 is wrong. And there's this (should KASAN be disabled for initify?) mm/kasan/kasan.c: In function ‘memmove’: mm/kasan/kasan.c:346:7: warning: ‘memmove’ captures its 2 (‘src’) parameter, please remove it from the nocapture attribute. void *memmove(void *dest, const void *src, size_t len) ^ mm/kasan/kasan.c: In function ‘memcpy’: mm/kasan/kasan.c:355:7: warning: ‘memcpy’ captures its 2 (‘src’) parameter, please remove it from the nocapture attribute. void *memcpy(void *dest, const void *src, size_t len) ^ And ACPI: drivers/acpi/acpica/utdebug.c: In function ‘acpi_debug_print’: drivers/acpi/acpica/utdebug.c:158:1: warning: ‘acpi_debug_print’ captures its 3 (‘function_name’) parameter, please remove it from the nocapture attribute. acpi_debug_print(u32 requested_debug_level, ^ I used my initify v5 development tree, with the following patch, with "make allyesconfig": http://git.kernel.org/cgit/linux/kernel/git/kees/linux.git/log/?h=for-next/gcc-plugin/initify diff --git a/arch/Kconfig b/arch/Kconfig index b6009a21ebea..5693ef5f22c8 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -359,7 +359,6 @@ config HAVE_GCC_PLUGINS menuconfig GCC_PLUGINS bool "GCC plugins" depends on HAVE_GCC_PLUGINS - depends on !COMPILE_TEST help GCC plugins are loadable modules that provide extra features to the compiler. They are useful for runtime instrumentation and static analysis. @@ -429,6 +428,7 @@ config GCC_PLUGIN_INITIFY config GCC_PLUGIN_INITIFY_VERBOSE bool "Report initification" depends on GCC_PLUGIN_INITIFY + depends on !COMPILE_TEST help Print all initified strings and all functions which should be __init/__exit. I'll see if acpi needs __noverified_nocapture ... -Kees -- Kees Cook Nexus Security
[PATCH] radix-tree: Fix private list warnings
From: Matthew Wilcox The newly introduced warning in radix_tree_free_nodes() was testing the wrong variable; it should have been 'old' instead of 'node'. Rather than replace that one instance, I noticed that we can simply put the WARN_ON_ONCE in radix_tree_node_free() and it will be just as effective. Fixes: ea07b862ac8e ("mm: workingset: fix use-after-free in shadow node shrinker") Signed-off-by: Matthew Wilcox --- lib/radix-tree.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/radix-tree.c b/lib/radix-tree.c index 4a4ed3ee4222..3c4577cabc57 100644 --- a/lib/radix-tree.c +++ b/lib/radix-tree.c @@ -449,6 +449,7 @@ static void radix_tree_node_rcu_free(struct rcu_head *head) static inline void radix_tree_node_free(struct radix_tree_node *node) { + WARN_ON_ONCE(!list_empty(&node->private_list)); call_rcu(&node->rcu_head, radix_tree_node_rcu_free); } @@ -734,7 +735,6 @@ static inline void radix_tree_shrink(struct radix_tree_root *root, update_node(node, private); } - WARN_ON_ONCE(!list_empty(&node->private_list)); radix_tree_node_free(node); } } @@ -766,7 +766,6 @@ static void delete_node(struct radix_tree_root *root, root->rnode = NULL; } - WARN_ON_ONCE(!list_empty(&node->private_list)); radix_tree_node_free(node); node = parent; @@ -868,7 +867,6 @@ static void radix_tree_free_nodes(struct radix_tree_node *node) struct radix_tree_node *old = child; offset = child->offset + 1; child = child->parent; - WARN_ON_ONCE(!list_empty(&node->private_list)); radix_tree_node_free(old); if (old == entry_to_node(node)) return; -- 2.11.0.296.g5800ad326.dirty
Re: [PATCH] usb: dwc2: gadget: Fix GUSBCFG.USBTRDTIM value
On 1/12/2017 7:41 AM, Amelie DELAUNAY wrote: > Hi all, > Sorry, I did not see Pengcheng Li patch which is exactly the same: > https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.kernel.org_patch_9347979_&d=DgIC-g&c=DPL6_X_6JkXFx7AXWqB0tg&r=U3o8uKoKhWme5_V9D-eeCkB11BFwt4KvWztBgdE9ZpA&m=6VylcKBQDS2RlneASNqb6vUEW48snVAZ1f_mhtzcSaE&s=iYEesI5W2GVN4jBs6pzyRC7Sb0RAng4gpapDsoZQG_s&e= > > > Regards > Hi Felipe, Could you take either one? Regards, John > On 01/12/2017 04:36 PM, Amelie Delaunay wrote: >> USBTrdTim must be programmed to 0x5 when phy has a UTMI+ 16-bit wide >> interface or 0x9 when it has a 8-bit wide interface. >> GUSBCFG reset value (Value After Reset: 0x1400) sets USBTrdTim to 0x5. >> In case of 8-bit UTMI+, without clearing GUSBCFG.USBTRDTIM mask, USBTrdTim >> results in 0xD (0x5 | 0x9). >> That's why we need to clear GUSBCFG.USBTRDTIM mask before setting USBTrdTim >> value, to ensure USBTrdTim is correctly set in case of 8-bit UTMI+. >> >> Signed-off-by: Amelie Delaunay >> --- >> drivers/usb/dwc2/gadget.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c >> index c55db4a..86b2076 100644 >> --- a/drivers/usb/dwc2/gadget.c >> +++ b/drivers/usb/dwc2/gadget.c >> @@ -3169,7 +3169,7 @@ void dwc2_hsotg_core_init_disconnected(struct >> dwc2_hsotg *hsotg, >> /* keep other bits untouched (so e.g. forced modes are not lost) */ >> usbcfg = dwc2_readl(hsotg->regs + GUSBCFG); >> usbcfg &= ~(GUSBCFG_TOUTCAL_MASK | GUSBCFG_PHYIF16 | GUSBCFG_SRPCAP | >> -GUSBCFG_HNPCAP); >> +GUSBCFG_HNPCAP | GUSBCFG_USBTRDTIM_MASK); >> >> if (hsotg->params.phy_type == DWC2_PHY_TYPE_PARAM_FS && >> (hsotg->params.speed == DWC2_SPEED_PARAM_FULL || >> @@ -4131,7 +4131,7 @@ static void dwc2_hsotg_init(struct dwc2_hsotg *hsotg) >> /* keep other bits untouched (so e.g. forced modes are not lost) */ >> usbcfg = dwc2_readl(hsotg->regs + GUSBCFG); >> usbcfg &= ~(GUSBCFG_TOUTCAL_MASK | GUSBCFG_PHYIF16 | GUSBCFG_SRPCAP | >> -GUSBCFG_HNPCAP); >> +GUSBCFG_HNPCAP | GUSBCFG_USBTRDTIM_MASK); >> >> /* set the PLL on, remove the HNP/SRP and set the PHY */ >> trdtim = (hsotg->phyif == GUSBCFG_PHYIF8) ? 9 : 5; >> >
Re: [PATCH] ARM: dts: NSP: Fix DT ranges error
On 01/12/2017 07:50 AM, Jon Mason wrote: > The range size for axi is 0x2 bytes too small, as the QSPI needs > 0x11c408 + 0x004 (which is 0x0011c40c, not 0x0011c40a). No errors have > been observed with this shortcoming, but fixing it for correctness. > > Signed-off-by: Jon Mason Applied to devicetree/fixes thanks Jon. -- Florian
[PATCH] PCI: iproc: fix resource allocation for BCMA PCIe
Resource allocated on stack was saved by 'devm_request_resource' to global 'iomem_resource' but become invalid after 'iproc_pcie_bcma_probe' exit. So the global 'iomem_resource' was poisoned. This may cause kernel crash or second PCIe bridge registration failure. Tested on Broadcom NorthStar machine ('Edgecore ECW7220-L') with two PCIe wifi adapters (b43 BCM4331 and ath10k QCA988X). Signed-off-by: Abylay Ospan --- drivers/pci/host/pcie-iproc-bcma.c | 18 -- drivers/pci/host/pcie-iproc.h | 2 ++ 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/pci/host/pcie-iproc-bcma.c b/drivers/pci/host/pcie-iproc-bcma.c index bd4c9ec..28f9b89 100644 --- a/drivers/pci/host/pcie-iproc-bcma.c +++ b/drivers/pci/host/pcie-iproc-bcma.c @@ -44,8 +44,6 @@ static int iproc_pcie_bcma_probe(struct bcma_device *bdev) { struct device *dev = &bdev->dev; struct iproc_pcie *pcie; - LIST_HEAD(res); - struct resource res_mem; int ret; pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL); @@ -62,21 +60,21 @@ static int iproc_pcie_bcma_probe(struct bcma_device *bdev) } pcie->base_addr = bdev->addr; + INIT_LIST_HEAD(&pcie->resources); - res_mem.start = bdev->addr_s[0]; - res_mem.end = bdev->addr_s[0] + SZ_128M - 1; - res_mem.name = "PCIe MEM space"; - res_mem.flags = IORESOURCE_MEM; - pci_add_resource(&res, &res_mem); + pcie->res_mem.start = bdev->addr_s[0]; + pcie->res_mem.end = bdev->addr_s[0] + SZ_128M - 1; + pcie->res_mem.name = "PCIe MEM space"; + pcie->res_mem.flags = IORESOURCE_MEM; + pcie->res_mem.child = NULL; + pci_add_resource(&pcie->resources, &pcie->res_mem); pcie->map_irq = iproc_pcie_bcma_map_irq; - ret = iproc_pcie_setup(pcie, &res); + ret = iproc_pcie_setup(pcie, &pcie->resources); if (ret) dev_err(dev, "PCIe controller setup failed\n"); - pci_free_resource_list(&res); - bcma_set_drvdata(bdev, pcie); return ret; } diff --git a/drivers/pci/host/pcie-iproc.h b/drivers/pci/host/pcie-iproc.h index 04fed8e..866d649 100644 --- a/drivers/pci/host/pcie-iproc.h +++ b/drivers/pci/host/pcie-iproc.h @@ -105,6 +105,8 @@ struct iproc_pcie { bool need_msi_steer; struct iproc_msi *msi; + struct resource res_mem; + struct list_head resources; }; int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res); -- 2.7.4
Re: [PATCH v4 06/15] lockdep: Make save_trace can skip stack tracing of the current
On Thu, Jan 12, 2017 at 05:37:57PM +0100, Peter Zijlstra wrote: > On Fri, Dec 09, 2016 at 02:12:02PM +0900, Byungchul Park wrote: > > Currently, save_trace() always performs save_stack_trace() for the > > current. However, crossrelease needs to use stack trace data of another > > context instead of the current. So add a parameter for skipping stack > > tracing of the current and make it use trace data, which is already > > saved by crossrelease framework. > > > > Signed-off-by: Byungchul Park > > --- > > kernel/locking/lockdep.c | 33 - > > 1 file changed, 20 insertions(+), 13 deletions(-) > > > > diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c > > index 3eaa11c..11580ec 100644 > > --- a/kernel/locking/lockdep.c > > +++ b/kernel/locking/lockdep.c > > @@ -387,15 +387,22 @@ static void print_lockdep_off(const char *bug_msg) > > #endif > > } > > > > -static int save_trace(struct stack_trace *trace) > > +static int save_trace(struct stack_trace *trace, int skip_tracing) > > { > > - trace->nr_entries = 0; > > - trace->max_entries = MAX_STACK_TRACE_ENTRIES - nr_stack_trace_entries; > > - trace->entries = stack_trace + nr_stack_trace_entries; > > + unsigned int nr_avail = MAX_STACK_TRACE_ENTRIES - > > nr_stack_trace_entries; > > > > - trace->skip = 3; > > - > > - save_stack_trace(trace); > > + if (skip_tracing) { > > + trace->nr_entries = min(trace->nr_entries, nr_avail); > > + memcpy(stack_trace + nr_stack_trace_entries, trace->entries, > > + trace->nr_entries * sizeof(trace->entries[0])); > > + trace->entries = stack_trace + nr_stack_trace_entries; > > + } else { > > + trace->nr_entries = 0; > > + trace->max_entries = nr_avail; > > + trace->entries = stack_trace + nr_stack_trace_entries; > > + trace->skip = 3; > > + save_stack_trace(trace); > > + } > > > > /* > > * Some daft arches put -1 at the end to indicate its a full trace. > > That's pretty nasty semantics.. so when skip_tracing it modifies trace > in-place. I agree. Let me think more and enhance it. Thank you, Byungchul
linux-next: build warning after merge of the wireless-drivers-next tree
Hi all, After merging the wireless-drivers-next tree, today's linux-next build (x86_64 allmodconfig) produced this warning: drivers/net/wireless/marvell/mwifiex/pcie.c: In function 'mwifiex_pcie_remove': drivers/net/wireless/marvell/mwifiex/pcie.c:303:5: warning: 'fw_status' may be used uninitialized in this function [-Wmaybe-uninitialized] if (fw_status == FIRMWARE_READY_PCIE && !adapter->mfg_mode) { ^ Introduced by commit 045f0c1b5e26 ("mwifiex: get rid of global user_rmmod flag") This is not a false positive since "reg" could be NULL just above (otherwise it would be tested for). -- Cheers, Stephen Rothwell
[PATCH] PCI: iproc: fix kernel crash if dev->of_node not defined
pcie->dev->of_node not always defined (NULL) and can cause crash: [ 19.053195] Unable to handle kernel NULL pointer dereference at virtual address 0020 [] (of_n_addr_cells) from [] (iproc_pcie_setup+0x30c/0xce0) this patch adds sanity check to prevent crash. Signed-off-by: Abylay Ospan --- drivers/pci/host/pcie-iproc.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c index 3ebc025..f2836a9 100644 --- a/drivers/pci/host/pcie-iproc.c +++ b/drivers/pci/host/pcie-iproc.c @@ -952,6 +952,9 @@ static int pci_dma_range_parser_init(struct of_pci_range_parser *parser, const int na = 3, ns = 2; int rlen; + if (!node) + return -ENOENT; + parser->node = node; parser->pna = of_n_addr_cells(node); parser->np = parser->pna + na + ns; -- 2.7.4
Re: [PATCH] PCI: iproc: fix kernel crash if dev->of_node not defined
On 01/12/2017 04:20 PM, Abylay Ospan wrote: > pcie->dev->of_node not always defined (NULL) and can cause crash: > > [ 19.053195] Unable to handle kernel NULL pointer dereference at > virtual address 0020 > [] (of_n_addr_cells) from [] > (iproc_pcie_setup+0x30c/0xce0) > > this patch adds sanity check to prevent crash. Humm, how can it not be defined based on your earlier comment that you are using this on NSP which is Device Tree exclusively? I would agree if this was seen on e.g: MIPS/BCMA (47xx). > > Signed-off-by: Abylay Ospan > --- > drivers/pci/host/pcie-iproc.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c > index 3ebc025..f2836a9 100644 > --- a/drivers/pci/host/pcie-iproc.c > +++ b/drivers/pci/host/pcie-iproc.c > @@ -952,6 +952,9 @@ static int pci_dma_range_parser_init(struct > of_pci_range_parser *parser, > const int na = 3, ns = 2; > int rlen; > > + if (!node) > + return -ENOENT; > + > parser->node = node; > parser->pna = of_n_addr_cells(node); > parser->np = parser->pna + na + ns; > -- Florian
Re: [PATCH v4 0/4] Application Data Integrity feature introduced by SPARC M7
On 01/12/2017 10:53 AM, Dave Hansen wrote: On 01/12/2017 08:50 AM, Khalid Aziz wrote: 2. Any shared page that has ADI protection enabled on it, must stay ADI protected across all processes sharing it. Is that true? What happens if a page with ADI tags set is accessed via a PTE without the ADI enablement bit set? ADI protection applies across all processes in terms of all of them must use the same tag to access the shared memory, but if a process accesses a shared page with TTE.mcde bit cleared, access will be granted. COW creates an intersection of the two. It creates a new copy of the shared data. It is a new data page and hence the process creating it must be the one responsible for enabling ADI protection on it. Do you mean that the application must be responsible? Or the kernel running in the context of the new process must be responsible? It is also a copy of what was ADI protected data, so should it inherit the protection instead? I think the COW'd copy must inherit the VMA bit, the PTE bits, and the tags on the cachelines. I misspoke earlier. I had misinterpreted the results of test I ran. Changing the tag on shared memory is allowed by memory controller. The requirement is every one sharing the page must switch to the new tag or else they get SIGSEGV. I asked this in the last mail, but I guess I'll ask it again. Please answer this directly. If we require that everyone coordinate their tags on the backing physical memory, and we allow a lower-privileged program to access the same data as a more-privileged one, then the lower-privilege app can cause arbitrary crashes in the privileged application. For instance, say sudo mmap()'s /etc/passwd and uses ADI tags to protect the mapping. Couldn't any other app in the system prevent sudo from working? How can we *EVER* allow tags to be set on non-writable mappings? I understand your quetion better now. That is a very valid concern. Using ADI tags to prevent an unauthorized process from just reading data in memory, say an in-memory copy of database, is one of the use cases for ADI. This means there is a reasonable case to allow enabling ADI and setting tags even on non-writable mappings. On the other hand, if an unauthorized process manages to map the right memory pages in its address space, it can read them any way by not setting TTE.mcd. Userspace app can set tag on any memory it has mapped in without requiring assistance from kernel. Can this problem be solved by not allowing setting TTE.mcd on non-writable mappings? Doesn't the same problem occur on writable mappings? If a privileged process mmap()'s a writable file with MAP_SHARED, enables ADI and sets tag on the mmap'd memory region, then another lower privilege process mmap's the same file writable (assuming file permissions allow it to), enables ADI and sets a different tag on it, the privileged process would get SIGSEGV when it tries to access the mmap'd file. Right? Thanks, Khalid
Re: [PATCH v2 RESEND] video: backlight: pwm_bl: Initialize fb_bl_on[x] and use_count during pwm_backlight_probe()
Dear All, > Thierry, Jingoo, > > Please respond to Lukasz. Yes, your response is more than welcome... :-) Thanks in advance, Łukasz Majewski > > On Mon, 26 Dec 2016, Lukasz Majewski wrote: > > > The commit a55944ca82d2 > > ("backlight: update bd state & fb_blank properties when necessary") > > has posed some extra restrictions on blanking and unblanking frame > > buffer device. > > > > Unfortunately, pwm_bl driver's probe did not initialize members of > > struct backlight_device necessary for further blank/unblank > > operation. > > > > This code in case of initial unblank of backlight device (default > > behaviour) sets use_count to 1 and marks this particular backlight > > device as used by all available fb devices (since it is not known > > during probe how much and which fb devices will be assigned). > > > > Without this code, the backlight works properly until one tries to > > blank it manually from sysfs with "echo 1 > > > /sys/class/graphics/fb0/blank". Since fb_bl_on[0] and use_count > > > were both set to 0, the logic at > > fb_notifier_callback (@backlight.c) thought that we didn't turn on > > (unblanked) the backlight device and refuses to disable (blank) it. > > As a result we see garbage from fb displayed. > > > > Signed-off-by: Lukasz Majewski > > --- > > The patch has been tested on i.MX6q with vanilla 4.9 kernel. > > It applies on 4.10-rc1. > > --- > > drivers/video/backlight/pwm_bl.c | 10 +- > > 1 file changed, 9 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/video/backlight/pwm_bl.c > > b/drivers/video/backlight/pwm_bl.c index 1261400..6859ba0 100644 > > --- a/drivers/video/backlight/pwm_bl.c > > +++ b/drivers/video/backlight/pwm_bl.c > > @@ -202,7 +202,7 @@ static int pwm_backlight_probe(struct > > platform_device *pdev) struct pwm_bl_data *pb; > > int initial_blank = FB_BLANK_UNBLANK; > > struct pwm_args pargs; > > - int ret; > > + int ret, i; > > > > if (!data) { > > ret = pwm_backlight_parse_dt(&pdev->dev, &defdata); > > @@ -348,6 +348,14 @@ static int pwm_backlight_probe(struct > > platform_device *pdev) > > bl->props.brightness = data->dft_brightness; > > bl->props.power = initial_blank; > > + > > + if (initial_blank == FB_BLANK_UNBLANK) { > > + for (i = 0; i < FB_MAX; i++) > > + bl->fb_bl_on[i] = true; > > + > > + bl->use_count = 1; > > + } > > + > > backlight_update_status(bl); > > > > platform_set_drvdata(pdev, bl); > Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
Re: [PATCH V2] usb: dwc2: host: fix Wmaybe-uninitialized warning
On 1/12/2017 8:32 AM, Nicholas Mc Guire wrote: > Uninitialized char* causes a sparse build-warning, fix it up by > initializing it to NULL. > > Signed-off-by: Nicholas Mc Guire > --- > > V2: add missing change-log as requested by Greg Kroah-Hartman > > > Problem reported by sparse > drivers/usb/dwc2/hcd.c: In function 'dwc2_dump_urb_info': > ./include/linux/dynamic_debug.h:134:3: warning: 'pipetype' may be used > uninitialized in this function [-Wmaybe-uninitialized] >__dynamic_dev_dbg(&descriptor, dev, fmt, \ >^ > drivers/usb/dwc2/hcd.c:4492:8: note: 'pipetype' was declared here > char *pipetype; > ^ > Patch was compile tested with: x86_64_defconfig + CONFIG_USB_DWC2=m + > CONFIG_USB_DWC2_VERBOSE=y > > Patch is against 4.10-rc3 (localversion-next is next-20170112) > > drivers/usb/dwc2/hcd.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c > index 911c3b3..9f66777 100644 > --- a/drivers/usb/dwc2/hcd.c > +++ b/drivers/usb/dwc2/hcd.c > @@ -4489,8 +4489,8 @@ static void dwc2_dump_urb_info(struct usb_hcd *hcd, > struct urb *urb, > { > #ifdef VERBOSE_DEBUG > struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd); > - char *pipetype; > - char *speed; > + char *pipetype = NULL; > + char *speed = NULL; > > dev_vdbg(hsotg->dev, "%s, urb %p\n", fn_name, urb); > dev_vdbg(hsotg->dev, " Device address: %d\n", > +Felipe Acked-by: John Youn John
Re: [PATCH RFC] usb: dwc2: host: use msleep() for long delay
On 1/12/2017 7:15 AM, Nicholas Mc Guire wrote: > ulseep_range() uses hrtimers and provides no advantage over msleep() > for larger delays. Fix up the 100ms delays here passing the adjusted "min" > value to msleep(). This helps reduce the load on the hrtimer subsystem. > > Link: > https://urldefense.proofpoint.com/v2/url?u=http-3A__lkml.org_lkml_2017_1_11_377&d=DgIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=U3o8uKoKhWme5_V9D-eeCkB11BFwt4KvWztBgdE9ZpA&m=dvQDbbH_QF2kZjARIDPimFAK95z8eOnw04feJuHCrJg&s=Prc0m1oUG4u8n8TFK-fdgzitMzE2ep6pM3L6y_DQeRs&e= > > Fixes: commit 2938fc63e0c2 ("usb: dwc2: Properly account for the force mode > delays") > Signed-off-by: Nicholas Mc Guire > --- > Problem was found by cocinelle script. > > Note that this originally was an msleep(25) then commit 2938fc63e0c2 > ("usb: dwc2: Properly account for the force mode delays") changed this > to usleep_range(10,11) with the comment in the function head: > > After clearing the bits, wait up to 100 ms to account for any > potential IDDIG filter delay... > > but without explaining why the API was switched to usleep_range() here > It does not look like there is any reason for using a high resolution > timer here - the stated requirement is clearly satisfied by msleep(100) > as well. But it originally was usleep_range(15, 16) in > commit 09c96980dc72 ("usb: dwc2: Add functions to set and clear force mode") > so not sure if I am not overlooking some reason for the highrestimer here ? > > This needs to be clarified by someone that knows the details and history > of this device/driver. > > Patch was compile tested with: x86_64_defconfig + CONFIG_USB_DWC2 > > Patch is aginast 4.10-rc3 (localversion-next is next-20170112) > > drivers/usb/dwc2/core.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c > index 11d8ae9..439a21b 100644 > --- a/drivers/usb/dwc2/core.c > +++ b/drivers/usb/dwc2/core.c > @@ -455,7 +455,7 @@ void dwc2_clear_force_mode(struct dwc2_hsotg *hsotg) > dwc2_writel(gusbcfg, hsotg->regs + GUSBCFG); > > if (dwc2_iddig_filter_enabled(hsotg)) > - usleep_range(10, 11); > + msleep(100); > } > > /* > +Felipe Acked-by: John Youn John
Re: [PATCHv5 3/8] rtc: add STM32 RTC driver
On 11/01/2017 at 14:46:43 +0100, Amelie Delaunay wrote : > This patch adds support for the STM32 RTC. > > Signed-off-by: Amelie Delaunay > --- > drivers/rtc/Kconfig | 11 + > drivers/rtc/Makefile| 1 + > drivers/rtc/rtc-stm32.c | 727 > > 3 files changed, 739 insertions(+) > create mode 100644 drivers/rtc/rtc-stm32.c > This didn't apply cleanly, please check rtc-next. I don't think I made any mistake as the issue was only in Kconfig. You probably based your patches on 4.9 instead of 4.10-rc1. -- Alexandre Belloni, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
Re: [PATCHv3 2/8] dt-bindings: document the STM32 RTC bindings
On 05/01/2017 at 14:43:23 +0100, Amelie Delaunay wrote : > This patch adds documentation of device tree bindings for the STM32 RTC. > > Signed-off-by: Amelie Delaunay > Acked-by: Rob Herring > --- > .../devicetree/bindings/rtc/st,stm32-rtc.txt | 27 > ++ > 1 file changed, 27 insertions(+) > create mode 100644 Documentation/devicetree/bindings/rtc/st,stm32-rtc.txt > Applied, thanks. -- Alexandre Belloni, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
Re: [PATCH] usb: dwc2: host: use msleep() for long delays
On 1/12/2017 7:52 AM, Nicholas Mc Guire wrote: > ulseep_range() uses hrtimers and provides no advantage over msleep() > for larger delays. Fix up the 20+ ms delays here passing the adjusted "min" > value to msleep(). This helps reduce the load on the hrtimer subsystem. > > Signed-off-by: Nicholas Mc Guire > --- > > Patch was compile tested with: x86_64_defconfig + CONFIG_USB_DWC2 > (1 sparse and 6 coccinelle warning - preparing separate patches for that) > > Patch is against 4.10-rc3 (localversion-next is next-20170112) > > drivers/usb/dwc2/hcd.c | 12 ++-- > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c > index 911c3b3..37ee72d 100644 > --- a/drivers/usb/dwc2/hcd.c > +++ b/drivers/usb/dwc2/hcd.c > @@ -2150,7 +2150,7 @@ static int dwc2_hcd_endpoint_disable(struct dwc2_hsotg > *hsotg, > } > > spin_unlock_irqrestore(&hsotg->lock, flags); > - usleep_range(2, 4); > + msleep(20); > spin_lock_irqsave(&hsotg->lock, flags); > qh = ep->hcpriv; > if (!qh) { > @@ -3240,7 +3240,7 @@ static void dwc2_conn_id_status_change(struct > work_struct *work) >"Waiting for Peripheral Mode, Mode=%s\n", >dwc2_is_host_mode(hsotg) ? "Host" : >"Peripheral"); > - usleep_range(2, 4); > + msleep(20); > if (++count > 250) > break; > } > @@ -3261,7 +3261,7 @@ static void dwc2_conn_id_status_change(struct > work_struct *work) > dev_info(hsotg->dev, "Waiting for Host Mode, Mode=%s\n", >dwc2_is_host_mode(hsotg) ? >"Host" : "Peripheral"); > - usleep_range(2, 4); > + msleep(20); > if (++count > 250) > break; > } > @@ -3354,7 +3354,7 @@ static void dwc2_port_suspend(struct dwc2_hsotg *hsotg, > u16 windex) > > spin_unlock_irqrestore(&hsotg->lock, flags); > > - usleep_range(20, 25); > + msleep(200); > } else { > spin_unlock_irqrestore(&hsotg->lock, flags); > } > @@ -3378,7 +3378,7 @@ static void dwc2_port_resume(struct dwc2_hsotg *hsotg) > pcgctl &= ~PCGCTL_STOPPCLK; > dwc2_writel(pcgctl, hsotg->regs + PCGCTL); > spin_unlock_irqrestore(&hsotg->lock, flags); > - usleep_range(2, 4); > + msleep(20); > spin_lock_irqsave(&hsotg->lock, flags); > } > > @@ -3691,7 +3691,7 @@ static int dwc2_hcd_hub_control(struct dwc2_hsotg > *hsotg, u16 typereq, > } > > /* Clear reset bit in 10ms (FS/LS) or 50ms (HS) */ > - usleep_range(5, 7); > + msleep(50); > hprt0 &= ~HPRT0_RST; > dwc2_writel(hprt0, hsotg->regs + HPRT0); > hsotg->lx_state = DWC2_L0; /* Now back to On state */ > +Felipe Acked-by: John Youn I've also fixed this up locally to apply against recent dwc2 patches for next. Hi Felipe, If/when you queue the pending dwc2 patches on testing/next I can resend this to you. Regards, John
Re: [PATCH] PCI: iproc: fix resource allocation for BCMA PCIe
Hi Abylay, On 1/12/2017 3:58 PM, Abylay Ospan wrote: > Resource allocated on stack was saved by 'devm_request_resource' to > global 'iomem_resource' but become invalid after 'iproc_pcie_bcma_probe' exit. > So the global 'iomem_resource' was poisoned. This may cause kernel crash > or second PCIe bridge registration failure. > > Tested on Broadcom NorthStar machine ('Edgecore ECW7220-L') with two PCIe wifi > adapters (b43 BCM4331 and ath10k QCA988X). > > Signed-off-by: Abylay Ospan I have not yet looked into this in great details. But if what you claimed is true, do we have the same problem with multiple PCIe host drivers that all have their resource allocated on the stack and have 'devm_request_resource' called to save it? Thanks, Ray > --- > drivers/pci/host/pcie-iproc-bcma.c | 18 -- > drivers/pci/host/pcie-iproc.h | 2 ++ > 2 files changed, 10 insertions(+), 10 deletions(-) > > diff --git a/drivers/pci/host/pcie-iproc-bcma.c > b/drivers/pci/host/pcie-iproc-bcma.c > index bd4c9ec..28f9b89 100644 > --- a/drivers/pci/host/pcie-iproc-bcma.c > +++ b/drivers/pci/host/pcie-iproc-bcma.c > @@ -44,8 +44,6 @@ static int iproc_pcie_bcma_probe(struct bcma_device *bdev) > { > struct device *dev = &bdev->dev; > struct iproc_pcie *pcie; > - LIST_HEAD(res); > - struct resource res_mem; > int ret; > > pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL); > @@ -62,21 +60,21 @@ static int iproc_pcie_bcma_probe(struct bcma_device *bdev) > } > > pcie->base_addr = bdev->addr; > + INIT_LIST_HEAD(&pcie->resources); > > - res_mem.start = bdev->addr_s[0]; > - res_mem.end = bdev->addr_s[0] + SZ_128M - 1; > - res_mem.name = "PCIe MEM space"; > - res_mem.flags = IORESOURCE_MEM; > - pci_add_resource(&res, &res_mem); > + pcie->res_mem.start = bdev->addr_s[0]; > + pcie->res_mem.end = bdev->addr_s[0] + SZ_128M - 1; > + pcie->res_mem.name = "PCIe MEM space"; > + pcie->res_mem.flags = IORESOURCE_MEM; > + pcie->res_mem.child = NULL; > + pci_add_resource(&pcie->resources, &pcie->res_mem); > > pcie->map_irq = iproc_pcie_bcma_map_irq; > > - ret = iproc_pcie_setup(pcie, &res); > + ret = iproc_pcie_setup(pcie, &pcie->resources); > if (ret) > dev_err(dev, "PCIe controller setup failed\n"); > > - pci_free_resource_list(&res); > - > bcma_set_drvdata(bdev, pcie); > return ret; > } > diff --git a/drivers/pci/host/pcie-iproc.h b/drivers/pci/host/pcie-iproc.h > index 04fed8e..866d649 100644 > --- a/drivers/pci/host/pcie-iproc.h > +++ b/drivers/pci/host/pcie-iproc.h > @@ -105,6 +105,8 @@ struct iproc_pcie { > > bool need_msi_steer; > struct iproc_msi *msi; > + struct resource res_mem; > + struct list_head resources; > }; > > int iproc_pcie_setup(struct iproc_pcie *pcie, struct list_head *res); >
Re: [PATCH] Input: synaptics-rmi4 - make F03 a tristate symbol
On 01/11/2017 11:27 AM, Christopher Heiny wrote: On Wed, 2017-01-11 at 18:48 +0100, Benjamin Tissoires wrote: On Jan 11 2017 or thereabouts, Arnd Bergmann wrote: On Wednesday, January 11, 2017 5:28:28 PM CET Benjamin Tissoires wrote: Yep, it was initially written that way, and IIRC there was some issues depending on how the drivers were compiled. For example, if rmi4_core is Y and some functions are m, you can't load the device initially, so you send a -EPROBE_DEFER, but how can you be sure that the function will ever be loaded? I'm not sure if I understand your problem correctly, but normally the way it's done is that the bus driver notifies user space that a new device has appeared on the bus, and udev looks for the right driver for the device, using a MODULE_DEVICE_TABLE list. Once the driver gets loaded, it binds to the device. I agree, but we never managed to make it properly working for RMI4. See https://lkml.org/lkml/2015/11/5/726 where we decided to switch to a static list of functions. Maybe we did not try hard enough, but we kept the current bus/functions_as_drivers to be able to switch back to the modular option, Actually, long, long ago (well before I got yanked off the RMI4 driver project for a 'short term emergency' two and a half years ago - fortunately Andrew was more than able to take it over) it worked that way. If you had the bus, a physical driver, and the sensor driver running, you could build the functions as modules and attach them via udev or insert them later. We had this working on the bench at one point, but fairly early in the submission process seem to have just assumed it would keep working and stopped regression testing on that feature. There have been an whole lot of changes since then, and somewhere along the line functions-as- loadable-modules stopped working. Since we weren't testing it anymore, it wasn't caught. We made the decision to disable support for function drivers as modules after running into a few technical challenges which happened during initialization. The priority was to get the driver upstreamed so we put off getting function drivers working as modules until there was a compelling reason to do so. But, we left the structure mostly intact if we decided to do so. Here are the messages which describe what we discovered at the time: https://lkml.org/lkml/2015/11/10/92 https://lkml.org/lkml/2015/11/12/652 I'm basically coming to the same conclusion I had back then. Getting loadable modules for function drivers to work would add complexity for not a lot of benefit based on the current state of the driver and how it is currently being used. I'm also not sure we will reach the critical mass of function drivers where there is a significant benefit of not having to load them all. Given that we need to have all the functions loaded during probe, we decided to switch to a monolithic rmi4_core driver that has everything it needs inside. If everything is in one module, you can probably get rid of at least part of the bus abstraction as well and just call the functions directly. Agree, though that means we won't be able to switch back. In the current form it's overly engineered. Well, I'm not sure I agree with that. :-) More on that later in this email. Looking through the driver some more, I also find the 'rmi_driver rmi_physical_driver' concept very odd, you seem to have a device on the bus that is actually just another representation of the parent device and that creates another set of devices for the functions. Either I misunderstand what this is for, or you have I think you have this right. a candidate for cleanup there and once you remove it (by calling rmi_driver_probe() instead of rmi_register_transport_device() to oversimplify the idea), the actual probing for the function drivers becomes much easier to do right. Agree, that would simplify the code a lot. I just don't know how important it is for other users of RMI4 to have a modular solution or if the monolithic approach is a consensus now. The modular solution is currently disabled, but we can "always" switch back with a small effort. My opinion on this matter is that there is no need for the modular functions, but others might have a different opinion. Just to clarify, this is proposing that the rmi_physical device be removed and we just calling the equivalent functions from the context of the transport? Basically, what Bjorn suggested here last year: https://lkml.org/lkml/2016/4/21/781 Andrew The primary impetus for the original modular function design was to allow phone/tablet/etc manufacturers to write their own handlers for some functions without having to write the entire driver or hack up an existing monolithic driver. This simplifies engineering for the device manufacturer a lot. We also hoped that other peripheral manufacturers would pick up RMI4 for other sorts of sensors and devices - it is an open standard after all. However, this didn't hap
Re: [PATCH] PCI: iproc: fix kernel crash if dev->of_node not defined
On 1/12/2017 4:20 PM, Abylay Ospan wrote: > pcie->dev->of_node not always defined (NULL) and can cause crash: Ah I guess this can happen with the BCMA based platforms that do not use device tree for PCIe? > > [ 19.053195] Unable to handle kernel NULL pointer dereference at > virtual address 0020 > [] (of_n_addr_cells) from [] > (iproc_pcie_setup+0x30c/0xce0) > > this patch adds sanity check to prevent crash. > > Signed-off-by: Abylay Ospan > --- > drivers/pci/host/pcie-iproc.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c > index 3ebc025..f2836a9 100644 > --- a/drivers/pci/host/pcie-iproc.c > +++ b/drivers/pci/host/pcie-iproc.c > @@ -952,6 +952,9 @@ static int pci_dma_range_parser_init(struct > of_pci_range_parser *parser, > const int na = 3, ns = 2; > int rlen; > > + if (!node) > + return -ENOENT; > + Looks like a valid check to me. Acked-by: Ray Jui > parser->node = node; > parser->pna = of_n_addr_cells(node); > parser->np = parser->pna + na + ns; >
Re: [PATCH] PCI: iproc: fix kernel crash if dev->of_node not defined
Hi Florian, On 1/12/2017 4:22 PM, Florian Fainelli wrote: > On 01/12/2017 04:20 PM, Abylay Ospan wrote: >> pcie->dev->of_node not always defined (NULL) and can cause crash: >> >> [ 19.053195] Unable to handle kernel NULL pointer dereference at >> virtual address 0020 >> [] (of_n_addr_cells) from [] >> (iproc_pcie_setup+0x30c/0xce0) >> >> this patch adds sanity check to prevent crash. > > Humm, how can it not be defined based on your earlier comment that you > are using this on NSP which is Device Tree exclusively? I would agree if > this was seen on e.g: MIPS/BCMA (47xx). I thought Abylay mentioned: "Tested on Broadcom NorthStar machine ('Edgecore ECW7220-L') with two PCIe wifi adapters (b43 BCM4331 and ath10k QCA988X)." That is a NorthStar device which is BCMA based? > >> >> Signed-off-by: Abylay Ospan >> --- >> drivers/pci/host/pcie-iproc.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c >> index 3ebc025..f2836a9 100644 >> --- a/drivers/pci/host/pcie-iproc.c >> +++ b/drivers/pci/host/pcie-iproc.c >> @@ -952,6 +952,9 @@ static int pci_dma_range_parser_init(struct >> of_pci_range_parser *parser, >> const int na = 3, ns = 2; >> int rlen; >> >> +if (!node) >> +return -ENOENT; >> + >> parser->node = node; >> parser->pna = of_n_addr_cells(node); >> parser->np = parser->pna + na + ns; >> > >
Re: [PATCH 2/3] perf/x86/amd/uncore: Dynamically allocate uncore counters
On 1/12/2017 3:20 AM, Peter Zijlstra wrote: On Wed, Jan 11, 2017 at 10:02:17AM -0600, Janakarajan Natarajan wrote: This patch updates the AMD uncore driver to support AMD Family17h processors. In Family17h, there are two extra last level cache counters. The counters are, therefore, allocated dynamically based on the family. The cpu hotplug up callback function is refactored to better manage failure conditions. Signed-off-by: Janakarajan Natarajan --- arch/x86/events/amd/uncore.c | 141 +++ 1 file changed, 104 insertions(+), 37 deletions(-) diff --git a/arch/x86/events/amd/uncore.c b/arch/x86/events/amd/uncore.c index 24c8537..7ab92f7 100644 --- a/arch/x86/events/amd/uncore.c +++ b/arch/x86/events/amd/uncore.c @@ -22,13 +22,16 @@ #define NUM_COUNTERS_NB 4 #define NUM_COUNTERS_L2 4 -#define MAX_COUNTERS NUM_COUNTERS_NB +#define NUM_COUNTERS_L36 #define RDPMC_BASE_NB 6 #define RDPMC_BASE_LLC10 #define COUNTER_SHIFT 16 +static int num_counters_llc; +static int num_counters_nb; + static HLIST_HEAD(uncore_unused_list); struct amd_uncore { @@ -40,7 +43,7 @@ struct amd_uncore { u32 msr_base; cpumask_t *active_mask; struct pmu *pmu; - struct perf_event *events[MAX_COUNTERS]; + struct perf_event **events; struct hlist_node node; }; Why bother with the dynamic allocation crud? Why not simply set MAX_COUNTERS to 6 and be happy? My reasoning behind using dynamic allocation was to prevent memory from being allocated when not needed on a per cpu basis. If memory isn't a consideration, I can send a v2 without the dynamic memory allocation.
Re: [PATCH] ARM64: dts: meson-gxbb-odroidc2: fix GbE tx link breakage
Jerome Brunet writes: > OdroidC2 GbE link breaks under heavy tx transfer. This happens even if the > MAC does not enable Energy Efficient Ethernet (No Low Power state Idle on > the Tx path). The problem seems to come from the phy Rx path, entering the > LPI state. > > Disabling EEE advertisement on the phy prevent this feature to be > negociated with the link partner and solve the issue. > > Signed-off-by: Jerome Brunet > --- > > This patch is based on Linus recent master branch [0] > This patch depends on the series [1] which has been merged in this branch. > > 0: ba6d973f78eb ("Merge > git://git.kernel.org/pub/scm/linux/kernel/git/davem/net") > 1: > http://lkml.kernel.org/r/1480326409-25419-1-git-send-email-jbru...@baylibre.com >Fix integration of eee-broken-modes > > arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 12 > 1 file changed, 12 insertions(+) > > diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts > b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts > index 238fbeacd330..d8933e9e9a5a 100644 > --- a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts > +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts > @@ -147,6 +147,18 @@ > status = "okay"; > pinctrl-0 = <ð_rgmii_pins>; > pinctrl-names = "default"; > + phy-handle = <ð_phy0>; > + > + mdio { > + compatible = "snps,dwmac-mdio"; > + #address-cells = <1>; > + #size-cells = <0>; > + > + eth_phy0: ethernet-phy@0 { > + reg = <0>; > + eee-broken-1000t; > + }; > + }; There's already an MDIO node in the meson-gx.dtsi (using the same compatible), shouldn't you just override that and add the new properties? What would make things easier is if the names were like Martin used in his reset patch, so that when I merge them together it's not a major conflict. Thanks, Kevin [1] https://patchwork.kernel.org/patch/9459409/
Re: [PATCH 5/6] treewide: use kv[mz]alloc* rather than opencoded variants
> On Jan 12, 2017, at 08:37, Michal Hocko wrote: > > From: Michal Hocko > > There are many code paths opencoding kvmalloc. Let's use the helper > instead. The main difference to kvmalloc is that those users are usually > not considering all the aspects of the memory allocator. E.g. allocation > requests < 64kB are basically never failing and invoke OOM killer to > satisfy the allocation. This sounds too disruptive for something that > has a reasonable fallback - the vmalloc. On the other hand those > requests might fallback to vmalloc even when the memory allocator would > succeed after several more reclaim/compaction attempts previously. There > is no guarantee something like that happens though. > > This patch converts many of those places to kv[mz]alloc* helpers because > they are more conservative. > > Signed-off-by: Michal Hocko Lustre part can be Acked-by: Andreas Dilger [snip] > diff --git a/drivers/staging/lustre/lnet/libcfs/linux/linux-mem.c > b/drivers/staging/lustre/lnet/libcfs/linux/linux-mem.c > index a6a76a681ea9..8f638267e704 100644 > --- a/drivers/staging/lustre/lnet/libcfs/linux/linux-mem.c > +++ b/drivers/staging/lustre/lnet/libcfs/linux/linux-mem.c > @@ -45,15 +45,6 @@ EXPORT_SYMBOL(libcfs_kvzalloc); > void *libcfs_kvzalloc_cpt(struct cfs_cpt_table *cptab, int cpt, size_t size, > gfp_t flags) > { > - void *ret; > - > - ret = kzalloc_node(size, flags | __GFP_NOWARN, > -cfs_cpt_spread_node(cptab, cpt)); > - if (!ret) { > - WARN_ON(!(flags & (__GFP_FS | __GFP_HIGH))); > - ret = vmalloc_node(size, cfs_cpt_spread_node(cptab, cpt)); > - } > - > - return ret; > + return kvzalloc_node(size, flags, cfs_cpt_spread_node(cptab, cpt)); > } > EXPORT_SYMBOL(libcfs_kvzalloc_cpt);
Re: [tpmdd-devel] [PATCH RFC v2 3/5] tpm: infrastructure for TPM spaces
On Thu, 2017-01-12 at 19:46 +0200, Jarkko Sakkinen wrote: > @@ -189,6 +190,12 @@ struct tpm_chip *tpm_chip_alloc(struct device > *pdev, > chip->cdev.owner = THIS_MODULE; > chip->cdev.kobj.parent = &chip->dev.kobj; > > + chip->work_space.context_buf = kzalloc(PAGE_SIZE, > GFP_KERNEL); > + if (!chip->work_space.context_buf) { > + rc = -ENOMEM; > + goto out; > + } > + I think the work_buf handling can be greatly simplified by making it a pointer to the space: it's only usable between tpm2_prepare_space() and tpm2_commit_space() which are protected by the chip mutex, so there's no need for it to exist outside of these calls (i.e. it can be NULL). Doing it this way also saves the allocation and copying overhead of work_space. The patch below can be folded to effect this. James --- diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c index 13cac09..770a8c0 100644 --- a/drivers/char/tpm/tpm-chip.c +++ b/drivers/char/tpm/tpm-chip.c @@ -131,7 +131,6 @@ static void tpm_dev_release(struct device *dev) mutex_unlock(&idr_lock); kfree(chip->log.bios_event_log); - kfree(chip->work_space.context_buf); kfree(chip); } @@ -206,12 +205,6 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev, chip->cdev.kobj.parent = &chip->dev.kobj; chip->cdevrm.kobj.parent = &chip->devrm.kobj; - chip->work_space.context_buf = kzalloc(PAGE_SIZE, GFP_KERNEL); - if (!chip->work_space.context_buf) { - rc = -ENOMEM; - goto out; - } - return chip; out: diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h index 8009ed4..adf7810 100644 --- a/drivers/char/tpm/tpm.h +++ b/drivers/char/tpm/tpm.h @@ -211,7 +211,7 @@ struct tpm_chip { char ppi_version[TPM_PPI_VERSION_LEN + 1]; #endif /* CONFIG_ACPI */ - struct tpm_space work_space; + struct tpm_space *work_space; u32 nr_commands; u32 *cc_attrs_tbl; }; diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c index 44e5501..285361e 100644 --- a/drivers/char/tpm/tpm2-space.c +++ b/drivers/char/tpm/tpm2-space.c @@ -27,7 +27,7 @@ enum tpm2_handle_types { static void tpm2_flush_space(struct tpm_chip *chip) { - struct tpm_space *space = &chip->work_space; + struct tpm_space *space = chip->work_space; int i; for (i = 0; i < ARRAY_SIZE(space->context_tbl); i++) @@ -45,7 +45,7 @@ struct tpm2_context { static int tpm2_load_space(struct tpm_chip *chip) { - struct tpm_space *space = &chip->work_space; + struct tpm_space *space = chip->work_space; struct tpm2_context *ctx; struct tpm_buf buf; int i; @@ -99,7 +99,7 @@ static int tpm2_load_space(struct tpm_chip *chip) static int tpm2_map_command(struct tpm_chip *chip, u32 cc, u8 *cmd, size_t len) { - struct tpm_space *space = &chip->work_space; + struct tpm_space *space = chip->work_space; unsigned int nr_handles; u32 vhandle; u32 phandle; @@ -147,9 +147,7 @@ int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, if (!space) return 0; - memcpy(&chip->work_space.context_tbl, &space->context_tbl, - sizeof(space->context_tbl)); - memcpy(chip->work_space.context_buf, space->context_buf, PAGE_SIZE); + chip->work_space = space; rc = tpm2_load_space(chip); if (rc) @@ -164,7 +162,7 @@ int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, static int tpm2_map_response(struct tpm_chip *chip, u32 cc, u8 *rsp, size_t len) { - struct tpm_space *space = &chip->work_space; + struct tpm_space *space = chip->work_space; u32 phandle; u32 vhandle; u32 attrs; @@ -222,7 +220,7 @@ static int tpm2_map_response(struct tpm_chip *chip, u32 cc, u8 *rsp, size_t len) static int tpm2_save_space(struct tpm_chip *chip) { - struct tpm_space *space = &chip->work_space; + struct tpm_space *space = chip->work_space; struct tpm_buf buf; int i; int j; @@ -295,9 +293,7 @@ int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space, if (rc) return rc; - memcpy(&space->context_tbl, &chip->work_space.context_tbl, - sizeof(space->context_tbl)); - memcpy(space->context_buf, chip->work_space.context_buf, PAGE_SIZE); + chip->work_space = NULL; return 0; }
Re: [lustre-devel] [PATCH 1/2] Fixed signedness check
On Dec 26, 2016, at 08:43, Guillermo O. Freschi wrote: > > Was `unsigned int`, but `enum`s are signed. Interesting that GCC didn't complain that the itree_overlap_cb() function signature didn't match the argument prototype for interval_search(): typedef enum interval_iter (*interval_callback_t)(struct interval_node *node, void *args); In the end it is a somewhat cosmetic fix, because the only enum values (1, 2) were unsigned, but good to be consistent. Thanks for the fix. > Signed-off-by: Guillermo O. Freschi Reviewed-by: Andreas Dilger > --- > drivers/staging/lustre/lustre/ldlm/ldlm_lock.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c > b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c > index a4a291acb659..f4cbc89b4f24 100644 > --- a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c > +++ b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c > @@ -1148,7 +1148,7 @@ static int lock_matches(struct ldlm_lock *lock, struct > lock_match_data *data) > return INTERVAL_ITER_STOP; > } > > -static unsigned int itree_overlap_cb(struct interval_node *in, void *args) > +static enum interval_iter itree_overlap_cb(struct interval_node *in, void > *args) > { > struct ldlm_interval *node = to_ldlm_interval(in); > struct lock_match_data *data = args; > -- > 2.11.0 > > ___ > lustre-devel mailing list > lustre-de...@lists.lustre.org > http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org
Re: 4.10-rc1: thinkpad x60: who ate my cpu?
Pavel Machek wrote: Hi! I used to have two cpus, and Thinkpad X60 should have two cores, but I only see one on 4.10-rc1. This machine went through many suspend/resume cycles. When backups finish, I'll try -rc2. Whoever did it, he seems to have returned the cpu in -rc3. All seems to be good now. Pavel Actually since you have mentioned - I have checked my x60 - same problem - only one CPU. However I was running 4.8.13 with uptime 33 days, multiple sleep/wake-ups. Installed a current EOL 4.8.17 and rebooted - I see 2 CPUs. So the issue is older then 4.10 kernel, and I suspect it is the CPU hotplug / wakeup related... Woody
Re: [lustre-devel] [PATCH 2/2] Style fixes
> On Dec 26, 2016, at 08:43, Guillermo O. Freschi wrote: > > Missing braces on `if` statement; misaligned parameter. > > Signed-off-by: Guillermo O. Freschi > --- > drivers/staging/lustre/lustre/ldlm/ldlm_lock.c | 12 +++- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c > b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c > index f4cbc89b4f24..a23e7ada3891 100644 > --- a/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c > +++ b/drivers/staging/lustre/lustre/ldlm/ldlm_lock.c > @@ -1024,11 +1024,11 @@ void ldlm_grant_lock(struct ldlm_lock *lock, struct > list_head *work_list) > if (work_list && lock->l_completion_ast) > ldlm_add_ast_work_item(lock, NULL, work_list); > > - if (res->lr_type == LDLM_PLAIN || res->lr_type == LDLM_IBITS) > + if (res->lr_type == LDLM_PLAIN || res->lr_type == LDLM_IBITS) { > ldlm_grant_lock_with_skiplist(lock); > - else if (res->lr_type == LDLM_EXTENT) > + } else if (res->lr_type == LDLM_EXTENT) { > ldlm_extent_add_lock(res, lock); > - else if (res->lr_type == LDLM_FLOCK) { > + } else if (res->lr_type == LDLM_FLOCK) { > /* >* We should not add locks to granted list in the following > cases: >* - this is an UNLOCK but not a real lock; This part is fine. > @@ -1040,8 +1040,9 @@ void ldlm_grant_lock(struct ldlm_lock *lock, struct > list_head *work_list) > ldlm_is_test_lock(lock) || ldlm_is_flock_deadlock(lock)) > return; > ldlm_resource_add_lock(res, &res->lr_granted, lock); > - } else > + } else { > LBUG(); > + } > > ldlm_pool_add(&ldlm_res_to_ns(res)->ns_pool, lock); > } Fine. > @@ -1481,7 +1482,8 @@ int ldlm_fill_lvb(struct ldlm_lock *lock, struct > req_capsule *pill, > lustre_swab_ost_lvb_v1); > else > lvb = req_capsule_server_sized_swab_get(pill, > - &RMF_DLM_LVB, size, > + > &RMF_DLM_LVB, > + size, > lustre_swab_ost_lvb_v1); Not so keen on this one, since "lustre_swab_ost_lvb_v1" can't fit after the '(' of the original function line, there isn't any benefit to aligning the other two arguments but not that one. Better to keep the indentation the same and save one line of whitespace. Can you please resubmit without this hunk. Cheers, Andreas
Re: [PATCH] PCI: iproc: fix kernel crash if dev->of_node not defined
On 01/12/2017 04:48 PM, Ray Jui wrote: > Hi Florian, > > On 1/12/2017 4:22 PM, Florian Fainelli wrote: >> On 01/12/2017 04:20 PM, Abylay Ospan wrote: >>> pcie->dev->of_node not always defined (NULL) and can cause crash: >>> >>> [ 19.053195] Unable to handle kernel NULL pointer dereference at >>> virtual address 0020 >>> [] (of_n_addr_cells) from [] >>> (iproc_pcie_setup+0x30c/0xce0) >>> >>> this patch adds sanity check to prevent crash. >> >> Humm, how can it not be defined based on your earlier comment that you >> are using this on NSP which is Device Tree exclusively? I would agree if >> this was seen on e.g: MIPS/BCMA (47xx). > > I thought Abylay mentioned: > > "Tested on Broadcom NorthStar machine ('Edgecore ECW7220-L') with two > PCIe wifi > adapters (b43 BCM4331 and ath10k QCA988X)." > > That is a NorthStar device which is BCMA based? Still, upstream Linux support for Northstar is Device Tree, and BCMA bus should fill in of_nodes accordingly, if not, that's a bug that must be fixed at the BCMA layer. > >> >>> >>> Signed-off-by: Abylay Ospan >>> --- >>> drivers/pci/host/pcie-iproc.c | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/drivers/pci/host/pcie-iproc.c b/drivers/pci/host/pcie-iproc.c >>> index 3ebc025..f2836a9 100644 >>> --- a/drivers/pci/host/pcie-iproc.c >>> +++ b/drivers/pci/host/pcie-iproc.c >>> @@ -952,6 +952,9 @@ static int pci_dma_range_parser_init(struct >>> of_pci_range_parser *parser, >>> const int na = 3, ns = 2; >>> int rlen; >>> >>> + if (!node) >>> + return -ENOENT; >>> + >>> parser->node = node; >>> parser->pna = of_n_addr_cells(node); >>> parser->np = parser->pna + na + ns; >>> >> >> -- Florian
Re: [lustre-devel] [PATCH] staging: lustre: selftest: Make brw_inject_one_error() static
On Dec 23, 2016, at 08:42, Karthik Nayak wrote: > > Since the function brw_inject_one_error() is used only within > brw_test.c, make it static. This was reported as a warning by sparse. > > Signed-off-by: Karthik Nayak Reviewed-by: Andreas Dilger > --- > drivers/staging/lustre/lnet/selftest/brw_test.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/lustre/lnet/selftest/brw_test.c > b/drivers/staging/lustre/lnet/selftest/brw_test.c > index 67b460f..b9ac34e 100644 > --- a/drivers/staging/lustre/lnet/selftest/brw_test.c > +++ b/drivers/staging/lustre/lnet/selftest/brw_test.c > @@ -136,7 +136,7 @@ brw_client_init(struct sfw_test_instance *tsi) > return 0; > } > > -int brw_inject_one_error(void) > +static int brw_inject_one_error(void) > { > struct timespec64 ts; > > -- > 2.10.2 > > ___ > lustre-devel mailing list > lustre-de...@lists.lustre.org > http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org
Re: [PATCH] usb: dwc2: host: use true/false for boolean
On 1/12/2017 7:53 AM, Nicholas Mc Guire wrote: > For boolean variables true/false is preferred over 1/0 for readability. > > Signed-off-by: Nicholas Mc Guire > --- > Problem reported by scripts/coccinelle/misc/boolinit.cocci: > ./drivers/usb/dwc2/hcd.c:5003:2-24: WARNING: Assignment of bool to 0/1 > ./drivers/usb/dwc2/hcd.c:3397:1-21: WARNING: Assignment of bool to 0/1 > ./drivers/usb/dwc2/hcd.c:3335:1-21: WARNING: Assignment of bool to 0/1 > ./drivers/usb/dwc2/hcd.c:2970:3-17: WARNING: Assignment of bool to 0/1 > ./drivers/usb/dwc2/hcd.c:2999:3-16: WARNING: Assignment of bool to 0/1 > ./drivers/usb/dwc2/hcd.c:3299:1-21: WARNING: Assignment of bool to 0/1 > > Patch was compile tested with: x86_64_defconfig + CONFIG_USB_DWC2 > > Patch is against 4.10-rc3 (localversion-next is next-20170112) > > drivers/usb/dwc2/hcd.c | 12 ++-- > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c > index 9f66777..7467a37 100644 > --- a/drivers/usb/dwc2/hcd.c > +++ b/drivers/usb/dwc2/hcd.c > @@ -2967,7 +2967,7 @@ static void dwc2_process_periodic_channels(struct > dwc2_hsotg *hsotg) > qspcavail = (tx_status & TXSTS_QSPCAVAIL_MASK) >> > TXSTS_QSPCAVAIL_SHIFT; > if (qspcavail == 0) { > - no_queue_space = 1; > + no_queue_space = true; > break; > } > > @@ -2996,7 +2996,7 @@ static void dwc2_process_periodic_channels(struct > dwc2_hsotg *hsotg) > TXSTS_FSPCAVAIL_SHIFT; > status = dwc2_queue_transaction(hsotg, qh->channel, fspcavail); > if (status < 0) { > - no_fifo_space = 1; > + no_fifo_space = true; > break; > } > > @@ -3296,7 +3296,7 @@ static void dwc2_wakeup_detected(unsigned long data) > dwc2_readl(hsotg->regs + HPRT0)); > > dwc2_hcd_rem_wakeup(hsotg); > - hsotg->bus_suspended = 0; > + hsotg->bus_suspended = false; > > /* Change to L0 state */ > hsotg->lx_state = DWC2_L0; > @@ -3332,7 +3332,7 @@ static void dwc2_port_suspend(struct dwc2_hsotg *hsotg, > u16 windex) > hprt0 |= HPRT0_SUSP; > dwc2_writel(hprt0, hsotg->regs + HPRT0); > > - hsotg->bus_suspended = 1; > + hsotg->bus_suspended = true; > > /* >* If hibernation is supported, Phy clock will be suspended > @@ -3394,7 +3394,7 @@ static void dwc2_port_resume(struct dwc2_hsotg *hsotg) > hprt0 = dwc2_read_hprt0(hsotg); > hprt0 &= ~(HPRT0_RES | HPRT0_SUSP); > dwc2_writel(hprt0, hsotg->regs + HPRT0); > - hsotg->bus_suspended = 0; > + hsotg->bus_suspended = false; > spin_unlock_irqrestore(&hsotg->lock, flags); > } > > @@ -5000,7 +5000,7 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq) > hsotg->dev->dma_mask == NULL) { > dev_warn(hsotg->dev, >"dma_mask not set, disabling DMA\n"); > - hsotg->params.host_dma = 0; > + hsotg->params.host_dma = false; > hsotg->params.dma_desc_enable = 0; > } > > +Felipe Acked-by: John Youn This one also required fix-up for next. Regards, John
Re: [PATCH v20 0/4] Mediatek MT8173 CMDQ support
On Wed, 2017-01-04 at 11:06 +0800, HS Liao wrote: > Hi, > > This is Mediatek MT8173 Command Queue(CMDQ) driver. The CMDQ is used > to help write registers with critical time limitation, such as > updating display configuration during the vblank. It controls Global > Command Engine (GCE) hardware to achieve this requirement. > > These patches have a build dependency on top of v4.10-rc2. > > Changes since v19: > - rebase to v4.10-rc2 > > Best regards, > HS Liao > > HS Liao (4): > dt-bindings: soc: Add documentation for the MediaTek GCE unit > mailbox: mediatek: Add Mediatek CMDQ driver > arm64: dts: mt8173: Add GCE node > soc: mediatek: Add Mediatek CMDQ helper > > .../devicetree/bindings/mailbox/mtk-gce.txt| 43 ++ > arch/arm64/boot/dts/mediatek/mt8173.dtsi | 10 + > drivers/mailbox/Kconfig| 10 + > drivers/mailbox/Makefile | 2 + > drivers/mailbox/mtk-cmdq-mailbox.c | 596 > + > drivers/soc/mediatek/Kconfig | 12 + > drivers/soc/mediatek/Makefile | 1 + > drivers/soc/mediatek/mtk-cmdq-helper.c | 310 +++ > include/linux/mailbox/mtk-cmdq-mailbox.h | 75 +++ > include/linux/soc/mediatek/mtk-cmdq.h | 174 ++ > 10 files changed, 1233 insertions(+) > create mode 100644 Documentation/devicetree/bindings/mailbox/mtk-gce.txt > create mode 100644 drivers/mailbox/mtk-cmdq-mailbox.c > create mode 100644 drivers/soc/mediatek/mtk-cmdq-helper.c > create mode 100644 include/linux/mailbox/mtk-cmdq-mailbox.h > create mode 100644 include/linux/soc/mediatek/mtk-cmdq.h > Hi Jassi, Matthias, Sorry to disturb you. Do you have any further comments on CMDQ v20? Thanks. HS
Re: [PATCH v2 7/7] uapi: export all headers under uapi directories
On Thu, Jan 12, 2017 at 05:32:09PM +0100, Nicolas Dichtel wrote: > What I was trying to say is that I export those directories like other are. > Removing those files is not related to that series. Perhaps the correct solution is to only copy files matching "*.h" to reduce the risk of copying files incidentally created by kbuild but which shouldn't be installed as uapi headers. jeff
Re: [PATCH v4 0/4] Application Data Integrity feature introduced by SPARC M7
On 01/12/2017 05:22 PM, Khalid Aziz wrote: On 01/12/2017 10:53 AM, Dave Hansen wrote: On 01/12/2017 08:50 AM, Khalid Aziz wrote: 2. Any shared page that has ADI protection enabled on it, must stay ADI protected across all processes sharing it. Is that true? What happens if a page with ADI tags set is accessed via a PTE without the ADI enablement bit set? ADI protection applies across all processes in terms of all of them must use the same tag to access the shared memory, but if a process accesses a shared page with TTE.mcde bit cleared, access will be granted. COW creates an intersection of the two. It creates a new copy of the shared data. It is a new data page and hence the process creating it must be the one responsible for enabling ADI protection on it. Do you mean that the application must be responsible? Or the kernel running in the context of the new process must be responsible? It is also a copy of what was ADI protected data, so should it inherit the protection instead? I think the COW'd copy must inherit the VMA bit, the PTE bits, and the tags on the cachelines. I misspoke earlier. I had misinterpreted the results of test I ran. Changing the tag on shared memory is allowed by memory controller. The requirement is every one sharing the page must switch to the new tag or else they get SIGSEGV. I asked this in the last mail, but I guess I'll ask it again. Please answer this directly. If we require that everyone coordinate their tags on the backing physical memory, and we allow a lower-privileged program to access the same data as a more-privileged one, then the lower-privilege app can cause arbitrary crashes in the privileged application. For instance, say sudo mmap()'s /etc/passwd and uses ADI tags to protect the mapping. Couldn't any other app in the system prevent sudo from working? How can we *EVER* allow tags to be set on non-writable mappings? I don't think you can write a tag to memory if you don't have write access in the TTE. Writing a tag requires a store instruction, and if the machine is at all sane, this will fault if you don't have write access. Rob I understand your quetion better now. That is a very valid concern. Using ADI tags to prevent an unauthorized process from just reading data in memory, say an in-memory copy of database, is one of the use cases for ADI. This means there is a reasonable case to allow enabling ADI and setting tags even on non-writable mappings. On the other hand, if an unauthorized process manages to map the right memory pages in its address space, it can read them any way by not setting TTE.mcd. Userspace app can set tag on any memory it has mapped in without requiring assistance from kernel. Can this problem be solved by not allowing setting TTE.mcd on non-writable mappings? Doesn't the same problem occur on writable mappings? If a privileged process mmap()'s a writable file with MAP_SHARED, enables ADI and sets tag on the mmap'd memory region, then another lower privilege process mmap's the same file writable (assuming file permissions allow it to), enables ADI and sets a different tag on it, the privileged process would get SIGSEGV when it tries to access the mmap'd file. Right?
Re: [PATCH v2 1/1] serial: 8250_dw: Allow hardware flow control to be used
On 2017/1/12 3:48, Jason Uy wrote: > In the most common use case, the Synopsys DW UART driver does not > set the set_termios callback function. This prevents UPSTAT_AUTOCTS > from being set when the UART flag CRTSCTS is set. As a result, the > driver will use software flow control as opposed to hardware flow > control. > > To fix the problem, the set_termios callback function is set to the > DW specific function. The logic to set UPSTAT_AUTOCTS is moved so > that any clock error will not affect setting the hardware flow > control. > > Reviewed-by: Scott Branden > Reviewed-by: Ray Jui > Signed-off-by: Jason Uy Reviewed-by: Kefeng Wang > --- > drivers/tty/serial/8250/8250_dw.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/tty/serial/8250/8250_dw.c > b/drivers/tty/serial/8250/8250_dw.c > index c89fafc..c89ae45 100644 > --- a/drivers/tty/serial/8250/8250_dw.c > +++ b/drivers/tty/serial/8250/8250_dw.c > @@ -248,11 +248,11 @@ static void dw8250_set_termios(struct uart_port *p, > struct ktermios *termios, > if (!ret) > p->uartclk = rate; > > +out: > p->status &= ~UPSTAT_AUTOCTS; > if (termios->c_cflag & CRTSCTS) > p->status |= UPSTAT_AUTOCTS; > > -out: > serial8250_do_set_termios(p, termios, old); > } > > @@ -326,13 +326,11 @@ static void dw8250_quirks(struct uart_port *p, struct > dw8250_data *data) > p->serial_in = dw8250_serial_in32; > data->uart_16550_compatible = true; > } > - p->set_termios = dw8250_set_termios; > } > > /* Platforms with iDMA */ > if (platform_get_resource_byname(to_platform_device(p->dev), >IORESOURCE_MEM, "lpss_priv")) { > - p->set_termios = dw8250_set_termios; > data->dma.rx_param = p->dev->parent; > data->dma.tx_param = p->dev->parent; > data->dma.fn = dw8250_idma_filter; > @@ -414,6 +412,7 @@ static int dw8250_probe(struct platform_device *pdev) > p->serial_in= dw8250_serial_in; > p->serial_out = dw8250_serial_out; > p->set_ldisc= dw8250_set_ldisc; > + p->set_termios = dw8250_set_termios; > > p->membase = devm_ioremap(dev, regs->start, resource_size(regs)); > if (!p->membase) >
Re: + mm-vmscan-add-mm_vmscan_inactive_list_is_low-tracepoint.patch added to -mm tree
Hello, On Thu, Jan 12, 2017 at 10:10:17AM +0100, Michal Hocko wrote: > On Thu 12-01-17 17:48:13, Minchan Kim wrote: > > On Thu, Jan 12, 2017 at 09:15:54AM +0100, Michal Hocko wrote: > > > On Thu 12-01-17 14:12:47, Minchan Kim wrote: > > > > Hello, > > > > > > > > On Wed, Jan 11, 2017 at 04:52:39PM +0100, Michal Hocko wrote: > > > > > On Wed 11-01-17 08:52:50, Minchan Kim wrote: > > > > > [...] > > > > > > > @@ -2055,8 +2055,8 @@ static bool inactive_list_is_low(struct > > > > > > > if (!file && !total_swap_pages) > > > > > > > return false; > > > > > > > > > > > > > > - inactive = lruvec_lru_size(lruvec, file * LRU_FILE); > > > > > > > - active = lruvec_lru_size(lruvec, file * LRU_FILE + LRU_ACTIVE); > > > > > > > + total_inactive = inactive = lruvec_lru_size(lruvec, file * > > > > > > > LRU_FILE); > > > > > > > + total_active = active = lruvec_lru_size(lruvec, file * LRU_FILE > > > > > > > + LRU_ACTIVE); > > > > > > > > > > > > > > > > > > > the decision of deactivating is based on eligible zone's LRU size, > > > > > > not whole zone so why should we need to get a trace of all zones's > > > > > > LRU? > > > > > > > > > > Strictly speaking, the total_ counters are not necessary for making > > > > > the > > > > > decision. I found reporting those numbers useful regardless because > > > > > this > > > > > will give us also an information how large is the eligible portion of > > > > > the LRU list. We do not have any other tracepoint which would report > > > > > that. > > > > > > > > The patch doesn't say anything why it's useful. Could you tell why it's > > > > useful and inactive_list_is_low should be right place? > > > > > > > > Don't get me wrong, please. I don't want to bother you. > > > > I really don't want to add random stuff although it's tracepoint for > > > > debugging. > > > > > > This doesn't sounds random to me. We simply do not have a full picture > > > on 32b systems without this information. Especially when memcgs are > > > involved and global numbers spread over different LRUs. > > > > Could you elaborate it? > > The problem with 32b systems is that you only can consider a part of the > LRU for the lowmem requests. While we have global counters to see how > much lowmem inactive/active pages we have, those get distributed to > memcg LRUs. And that distribution is impossible to guess. So my thinking > is that it can become a real head scratcher to realize why certain > active LRUs are aged while others are not. This was the case when I was > debugging the last issue which triggered all this. All of the sudden I > have seen many invocations when inactive and active were zero which > sounded weird, until I realized that those are memcg's lruvec which is > what total numbers told me... Hmm, it seems I miss something. AFAIU, what you need is just memcg identifier, not all lru size. If it isn't, please tell more detail usecase of all lru size in that particular tracepoint. > > Later on I would like to add an memcg identifier to the vmscan > tracepoints but I didn't get there yet. > > > " > > Currently we have tracepoints for both active and inactive LRU lists > > reclaim but we do not have any which would tell us why we we decided to > > age the active list. Without that it is quite hard to diagnose > > active/inactive lists balancing. Add mm_vmscan_inactive_list_is_low > > tracepoint to tell us this information. > > " > > > > Your description says "why we decided to age the active list". > > So, what's needed? > > > > > > > > > > [...] > > > > > > > @@ -2223,7 +2228,7 @@ static void get_scan_count(struct lruvec > > > > > > >* lruvec even if it has plenty of old anonymous pages unless > > > > > > > the > > > > > > >* system is under heavy pressure. > > > > > > >*/ > > > > > > > - if (!inactive_list_is_low(lruvec, true, sc) && > > > > > > > + if (!inactive_list_is_low(lruvec, true, sc, false) && > > > > > > > > > > > > Hmm, I was curious why you added trace boolean arguement and found > > > > > > it here. > > > > > > Yes, here is not related to deactivation directly but couldn't we > > > > > > help to > > > > > > trace it unconditionally? > > > > > > > > > > I've had it like that when I was debugging the mentioned bug and found > > > > > it a bit disturbing. It generated more output than I would like and it > > > > > wasn't really clear from which code path this has been called from. > > > > > > > > Indeed. > > > > > > > > Personally, I want to move inactive_list_is_low in shrink_active_list > > > > and shrink_active_list calls inactive_list_is_low(, true), > > > > unconditionally so that it can make code simple/clear but cannot remove > > > > trace boolean variable , which what I want. So, it's okay if you love > > > > your version. > > > > > > I am not sure I am following. Why is the additional parameter a problem? > > > > Well, to me, it's not a elegance. Is it? If we need such boolean variable > > to control show the trace, it me
Re: Duplicate inode number when mount --bind some directories to same mountpoint. (from Fedora18 to 4.10-rc3)
On 2017/01/12 19:24, Al Viro wrote: On Thu, Jan 12, 2017 at 06:16:35PM +0900, Nakajima Akira wrote: Bug: Duplicate inode number when mount --bind some directories to same mountpoint. (from Fedora18 to 4.10-rc3) Fedora17 and earlier works correctly. Explain, please. "Duplicate inode number" between what and what? Duplicate inode number between mounted directories. Example) # cd /home # mkdir a b # ls -i 100 a 999 b # mount --bind a /mnt # mount --bind b /mnt # ls -i 999 a 999 b Inode number of directory "a" is changed to "b". Then we see directory "b" when ls "a". And, Above kernel ver 3.6 (Fedora18 including 4.10-rc3) creates many structs of mount than ver 3.3 (Fedora17). Is this a correct specification? Looks kernel creates same many structs of mount. alloc_vfsmnt() and clone_mnt() are internal functions, no promises of stability had ever been given... As for the differences between these setups... almost certainly an effect of changed shared-subtree settings. Userland, not kernel. Systemtap script result on Fedora25 Kernel create many structs of mount. And, inode number of "a" changes to 547586 of "b". What I would like to see is the contents of /proc/self/mountinfo - systemtap be damned, there is a sane interface for getting the mount tree setup. BTW, what's in that /root/mnt.stp thing? /root/mnt.stp is following. In result of script, Kernel creates many same structs of mount, It looks waste of memory. But I don't know whether it is correct specification or not. # cat /root/mnt.stp #! /usr/bin/stap probe kernel.function("alloc_vfsmnt").return { printf("%s() new_mnt:%p\n", probefunc(), $return); } probe kernel.function("clone_mnt").return {// do_mount, copy_tree name = @cast($return, "mount")->mnt_mountpoint->d_iname; inode = @cast($return, "mount")->mnt_mountpoint->d_inode; ino = @cast($return, "mount")->mnt_mountpoint->d_inode->i_ino; printf("%s() mnt:%p d_iname:%s inode:%p ino:%u\n", probefunc(), $return, kernel_string(name), inode, ino); } /proc/self/mountinfo is following 17 61 0:17 / /sys rw,nosuid,nodev,noexec,relatime shared:6 - sysfs sysfs rw 18 61 0:4 / /proc rw,nosuid,nodev,noexec,relatime shared:5 - proc proc rw 19 61 0:6 / /dev rw,nosuid shared:2 - devtmpfs devtmpfs rw,size=2013132k,nr_inodes=503283,mode=755 20 17 0:18 / /sys/kernel/security rw,nosuid,nodev,noexec,relatime shared:7 - securityfs securityfs rw 21 19 0:19 / /dev/shm rw,nosuid,nodev shared:3 - tmpfs tmpfs rw 22 19 0:20 / /dev/pts rw,nosuid,noexec,relatime shared:4 - devpts devpts rw,gid=5,mode=620,ptmxmode=000 23 61 0:21 / /run rw,nosuid,nodev shared:22 - tmpfs tmpfs rw,mode=755 24 17 0:22 / /sys/fs/cgroup ro,nosuid,nodev,noexec shared:8 - tmpfs tmpfs ro,mode=755 25 24 0:23 / /sys/fs/cgroup/systemd rw,nosuid,nodev,noexec,relatime shared:9 - cgroup cgroup rw,xattr,release_agent=/usr/lib/systemd/systemd-cgroups-agent,name=systemd 26 17 0:24 / /sys/fs/pstore rw,nosuid,nodev,noexec,relatime shared:20 - pstore pstore rw 27 24 0:25 / /sys/fs/cgroup/cpu,cpuacct rw,nosuid,nodev,noexec,relatime shared:10 - cgroup cgroup rw,cpu,cpuacct 28 24 0:26 / /sys/fs/cgroup/blkio rw,nosuid,nodev,noexec,relatime shared:11 - cgroup cgroup rw,blkio 29 24 0:27 / /sys/fs/cgroup/perf_event rw,nosuid,nodev,noexec,relatime shared:12 - cgroup cgroup rw,perf_event 30 24 0:28 / /sys/fs/cgroup/pids rw,nosuid,nodev,noexec,relatime shared:13 - cgroup cgroup rw,pids 31 24 0:29 / /sys/fs/cgroup/freezer rw,nosuid,nodev,noexec,relatime shared:14 - cgroup cgroup rw,freezer 32 24 0:30 / /sys/fs/cgroup/net_cls,net_prio rw,nosuid,nodev,noexec,relatime shared:15 - cgroup cgroup rw,net_cls,net_prio 33 24 0:31 / /sys/fs/cgroup/cpuset rw,nosuid,nodev,noexec,relatime shared:16 - cgroup cgroup rw,cpuset 34 24 0:32 / /sys/fs/cgroup/devices rw,nosuid,nodev,noexec,relatime shared:17 - cgroup cgroup rw,devices 35 24 0:33 / /sys/fs/cgroup/hugetlb rw,nosuid,nodev,noexec,relatime shared:18 - cgroup cgroup rw,hugetlb 36 24 0:34 / /sys/fs/cgroup/memory rw,nosuid,nodev,noexec,relatime shared:19 - cgroup cgroup rw,memory 58 17 0:35 / /sys/kernel/config rw,relatime shared:21 - configfs configfs rw 61 0 252:1 / / rw,relatime shared:1 - ext4 /dev/vda1 rw,data=ordered 16 19 0:15 / /dev/mqueue rw,relatime shared:23 - mqueue mqueue rw 37 19 0:16 / /dev/hugepages rw,relatime shared:24 - hugetlbfs hugetlbfs rw 38 61 0:37 / /tmp rw,nosuid,nodev shared:25 - tmpfs tmpfs rw 39 18 0:38 / /proc/sys/fs/binfmt_misc rw,relatime shared:26 - autofs systemd-1 rw,fd=38,pgrp=1,timeout=0,minproto=5,maxproto=5,direct,pipe_ino=12648 40 17 0:7 / /sys/kernel/debug rw,relatime shared:27 - debugfs debugfs rw 72 61 0:39 / /var/lib/nfs/rpc_pipefs rw,relatime shared:28 - rpc_pipefs sunrpc rw 74 18 0:40 / /proc/fs/nfsd rw,relatime shared:29 - nfsd nfsd rw 111 23 0:41 / /run/user/0 rw,nosuid,nodev,re
Re: [PATCH] partially revert "xen: Remove event channel notification through Xen PCI platform device"
On Thu, 12 Jan 2017, Boris Ostrovsky wrote: > On 01/12/2017 04:33 PM, Stefano Stabellini wrote: > > On Thu, 12 Jan 2017, Boris Ostrovsky wrote: > >> On 01/11/2017 06:36 PM, Stefano Stabellini wrote: > >>> The following commit: > >>> > >>> commit 72a9b186292d98494f26cfd24a1621796209 > >>> Author: KarimAllah Ahmed > >>> Date: Fri Aug 26 23:55:36 2016 +0200 > >>> > >>> xen: Remove event channel notification through Xen PCI platform device > > Can you also replace this with > > "Commit 72a9b186292d ("xen: Remove event channel notification through > Xen PCI platform device")" ... ? Too late. But if you are Ok with the patch, please fix the message while committing.
Re: x86-64: Maintain 16-byte stack alignment
On Thu, Jan 12, 2017 at 12:15 PM, Josh Poimboeuf wrote: > On Thu, Jan 12, 2017 at 12:08:07PM -0800, Andy Lutomirski wrote: >> On Thu, Jan 12, 2017 at 11:51 AM, Linus Torvalds >> wrote: >> > On Thu, Jan 12, 2017 at 6:02 AM, Josh Poimboeuf >> > wrote: >> >> >> >> Just to clarify, I think you're asking if, for versions of gcc which >> >> don't support -mpreferred-stack-boundary=3, objtool can analyze all C >> >> functions to ensure their stacks are 16-byte aligned. >> >> >> >> It's certainly possible, but I don't see how that solves the problem. >> >> The stack will still be misaligned by entry code. Or am I missing >> >> something? >> > >> > I think the argument is that we *could* try to align things, if we >> > just had some tool that actually then verified that we aren't missing >> > anything. >> > >> > I'm not entirely happy with checking the generated code, though, >> > because as Ingo says, you have a 50:50 chance of just getting it right >> > by mistake. So I'd much rather have some static tool that checks >> > things at a code level (ie coccinelle or sparse). >> >> What I meant was checking the entry code to see if it aligns stack >> frames, and good luck getting sparse to do that. Hmm, getting 16-byte >> alignment for real may actually be entirely a lost cause. After all, >> I think we have some inline functions that do asm volatile ("call >> ..."), and I don't see any credible way of forcing alignment short of >> generating an entirely new stack frame and aligning that. > > Actually we already found all such cases and fixed them by forcing a new > stack frame, thanks to objtool. For example, see 55a76b59b5fe. What I mean is: what guarantees that the stack is properly aligned for the subroutine call? gcc promises to set up a stack frame, but does it promise that rsp will be properly aligned to call a C function?
[PATCH] libnvdimm, namespace: fix pmem namespace leak, delete when size set to zero
Commit 98a29c39dc68 ("libnvdimm, namespace: allow creation of multiple pmem-namespaces per region") added support for establishing additional pmem namespace beyond the seed device, similar to blk namespaces. However, it neglected to delete the namespace when the size is set to zero. Fixes: 98a29c39dc68 ("libnvdimm, namespace: allow creation of multiple pmem-namespaces per region") Cc: Signed-off-by: Dan Williams --- drivers/nvdimm/namespace_devs.c | 23 ++- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/drivers/nvdimm/namespace_devs.c b/drivers/nvdimm/namespace_devs.c index 6307088b375f..a518cb1b59d4 100644 --- a/drivers/nvdimm/namespace_devs.c +++ b/drivers/nvdimm/namespace_devs.c @@ -957,6 +957,7 @@ static ssize_t __size_store(struct device *dev, unsigned long long val) { resource_size_t allocated = 0, available = 0; struct nd_region *nd_region = to_nd_region(dev->parent); + struct nd_namespace_common *ndns = to_ndns(dev); struct nd_mapping *nd_mapping; struct nvdimm_drvdata *ndd; struct nd_label_id label_id; @@ -964,7 +965,7 @@ static ssize_t __size_store(struct device *dev, unsigned long long val) u8 *uuid = NULL; int rc, i; - if (dev->driver || to_ndns(dev)->claim) + if (dev->driver || ndns->claim) return -EBUSY; if (is_namespace_pmem(dev)) { @@ -1034,20 +1035,16 @@ static ssize_t __size_store(struct device *dev, unsigned long long val) nd_namespace_pmem_set_resource(nd_region, nspm, val * nd_region->ndr_mappings); - } else if (is_namespace_blk(dev)) { - struct nd_namespace_blk *nsblk = to_nd_namespace_blk(dev); - - /* -* Try to delete the namespace if we deleted all of its -* allocation, this is not the seed device for the -* region, and it is not actively claimed by a btt -* instance. -*/ - if (val == 0 && nd_region->ns_seed != dev - && !nsblk->common.claim) - nd_device_unregister(dev, ND_ASYNC); } + /* +* Try to delete the namespace if we deleted all of its +* allocation, this is not the seed device for the region, and +* it is not actively claimed by a btt instance. +*/ + if (val == 0 && nd_region->ns_seed != dev && !ndns->claim) + nd_device_unregister(dev, ND_ASYNC); + return rc; }
RE: [PATCH v8 1/1] crypto: add virtio-crypto driver
> > On Thu, Jan 12, 2017 at 03:10:25PM +0100, Christian Borntraeger wrote: > > On 01/10/2017 01:56 PM, Christian Borntraeger wrote: > > > On 01/10/2017 01:36 PM, Gonglei (Arei) wrote: > > >> Hi, > > >> > > >>> > > >>> On 12/15/2016 03:03 AM, Gonglei wrote: > > >>> [...] > > + > > +static struct crypto_alg virtio_crypto_algs[] = { { > > + .cra_name = "cbc(aes)", > > + .cra_driver_name = "virtio_crypto_aes_cbc", > > + .cra_priority = 501, > > >>> > > >>> > > >>> This is still higher than the hardware-accelerators (like intel aesni > > >>> or the > > >>> s390 cpacf functions or the arm hw). aesni and s390/cpacf are supported > by the > > >>> hardware virtualization and available to the guests. I do not see a way > how > > >>> virtio > > >>> crypto can be faster than that (in the end it might be cpacf/aesni + > overhead) > > >>> instead it will very likely be slower. > > >>> So we should use a number that is higher than software implementations > but > > >>> lower than the hw ones. > > >>> > > >>> Just grepping around, the software ones seem be be around 100 and the > > >>> hardware > > >>> ones around 200-400. So why was 150 not enough? > > >>> > > >> I didn't find a documentation about how we use the priority, and I > > >> assumed > > >> people use virtio-crypto will configure hardware accelerators in the > > >> host. So I choosed the number which bigger than aesni's priority. > > > > > > Yes, but the aesni driver will only bind if there is HW support in the > > > guest. > > > And if aesni is available in the guest (or the s390 aes function from > > > cpacf) > > > it will always be faster than the same in the host via virtio.So your > > > priority > > > should be smaller. > > > > > > any opinion on this? > > Going forward, we might add an emulated aesni device and that might > become slower than virtio. OTOH if or when this happens, we can solve it > by adding a priority or a feature flag to virtio to raise its priority. > > So I think I agree with Christian here, let's lower the priority. > Gonglei, could you send a patch like this? > OK, will do. Thanks, -Gonglei
Re: [PATCH] coresight: STM: Balance enable/disable
On 11 January 2017 at 21:59, Suzuki K Poulose wrote: > On 11/01/17 11:41, Chunyan Zhang wrote: >> >> On 11 January 2017 at 01:36, Mathieu Poirier >> wrote: >>> >>> On Tue, Jan 10, 2017 at 11:21:55AM +, Suzuki K Poulose wrote: The stm is automatically enabled when an application sets the policy via ->link() call back by using coresight_enable(), which keeps the refcount of the current users of the STM. However, the unlink() callback issues stm_disable() directly, which leaves the STM turned off, without the coresight layer knowing about it. This prevents any further uses of the STM hardware as the coresight layer still thinks the STM is turned on and doesn't issue an stm_enable(). Even manually enabling the STM via sysfs can't really enable the hw. e.g, $ echo 1 > $CS_DEVS/$ETR/enable_sink $ mkdir -p $CONFIG_FS/stp-policy/$source.0/stm_test/ $ echo 32768 65535 > $CONFIG_FS/stp-policy/$source.0/stm_test/channels $ echo 64 > $CS_DEVS/$source/traceid $ ./stm_app Sending 64000 byte blocks of pattern 0 at 0us intervals Success to map channel(32768~32783) to 0xa95fa000 Sending on channel 32768 $ dd if=/dev/$ETR of=~/trace.bin.1 597+1 records in 597+1 records out 305920 bytes (306 kB) copied, 0.399952 s, 765 kB/s $ ./stm_app Sending 64000 byte blocks of pattern 0 at 0us intervals Success to map channel(32768~32783) to 0x7e9e2000 Sending on channel 32768 $ dd if=/dev/$ETR of=~/trace.bin.2 0+0 records in 0+0 records out 0 bytes (0 B) copied, 0.0232083 s, 0.0 kB/s Note that we don't get any data from the ETR for the second session. Also dmesg shows : [ 77.520458] coresight-tmc 2080.etr: TMC-ETR enabled [ 77.537097] coresight-replicator etr_replicator@2089: REPLICATOR enabled [ 77.558828] coresight-replicator main_replicator@208a: REPLICATOR enabled [ 77.581068] coresight-funnel 208c.main_funnel: FUNNEL inport 0 enabled [ 77.602217] coresight-tmc 2084.etf: TMC-ETF enabled [ 77.618422] coresight-stm 2086.stm: STM tracing enabled [ 139.554252] coresight-stm 2086.stm: STM tracing disabled # End of first tracing session [ 146.351135] coresight-tmc 2080.etr: TMC read start [ 146.514486] coresight-tmc 2080.etr: TMC read end # Note that the STM is not turned on via stm_generic_link()->coresight_enable() # and hence none of the components are turned on. [ 152.479080] coresight-tmc 2080.etr: TMC read start [ 152.542632] coresight-tmc 2080.etr: TMC read end This patch balances the unlink operation by using the coresight_disable(), keeping the coresight layer in sync with the hardware state. Fixes: commit 237483aa5cf43 ("coresight: stm: adding driver for CoreSight STM component") Cc: Pratik Patel Cc: Mathieu Poirier Cc: Chunyan Zhang Cc: Greg Kroah-Hartman Cc: sta...@vger.kernel.org # 4.7+ Reported-by: Robert Walker Signed-off-by: Suzuki K Poulose --- drivers/hwtracing/coresight/coresight-stm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c index 3524452..57b7330 100644 --- a/drivers/hwtracing/coresight/coresight-stm.c +++ b/drivers/hwtracing/coresight/coresight-stm.c @@ -356,7 +356,7 @@ static void stm_generic_unlink(struct stm_data *stm_data, if (!drvdata || !drvdata->csdev) return; - stm_disable(drvdata->csdev, NULL); + coresight_disable(drvdata->csdev); >>> >>> >>> This looks valid to me. >>> >>> Chunyan, any reason to use stm_disable() directly rather than calling it >>> as part >>> of the device OPS in coresight_disable()? >> >> >> I don't think there's some special reason for this. I simply hadn't >> noticed that these two operations didn't use two balanced functions. > > > Please can I have an Ack/Reviewed -by on it, so that we can push it > as a fix. Sure, I've had a run with this patch, it works well, so, Reviewed-by: Chunyan Zhang Thanks, Chunyan > > > Suzuki >
Re: eMMC boot problem: switch to bus width 8 ddr failed
On 2017/1/13 0:51, Ulf Hansson wrote: + Haibo, Gary, Fabio, Shawn Gou On 6 January 2017 at 16:56, Clemens Gruber wrote: On Fri, Jan 06, 2017 at 10:33:49AM +0800, Shawn Lin wrote: On 2017/1/6 8:41, Clemens Gruber wrote: Hi, with the current mainline 4.10-rc2 kernel, I can no longer boot from the eMMC on my i.MX6Q board. Details: The eMMC is a Micron MTFC4GACAJCN-1M WT but as the i.MX6Q only supports eMMC 4.41 features and we did not implement voltage switching from 3.3V to 1.8V or lower, I did add no-1-8-v; (but none of the mmc-ddr or mmc-hs options) to the device tree. The bus-width is 8. With 4.9 the board booted fine, now with the current mainline 4.10 tree, I get the following (repeating) errors at boot: [4.326834] Waiting for root device /dev/mmcblk0p2... [ 14.563861] mmc0: Timeout waiting for hardware cmd interrupt. [ 14.569619] sdhci: === REGISTER DUMP (mmc0)=== [ 14.575461] sdhci: Sys addr: 0x4e726000 | Version: 0x0002 [ 14.581300] sdhci: Blk size: 0x0200 | Blk cnt: 0x0001 [ 14.587140] sdhci: Argument: 0x0001 | Trn mode: 0x0013 [ 14.592979] sdhci: Present: 0x01fd8009 | Host ctl: 0x0031 [ 14.598816] sdhci: Power:0x0002 | Blk gap: 0x0080 [ 14.604654] sdhci: Wake-up: 0x0008 | Clock:0x001f [ 14.610493] sdhci: Timeout: 0x008f | Int stat: 0x [ 14.616332] sdhci: Int enab: 0x107f100b | Sig enab: 0x107f100b [ 14.622168] sdhci: AC12 err: 0x | Slot int: 0x0003 [ 14.628007] sdhci: Caps: 0x07eb | Caps_1: 0xa007 [ 14.633845] sdhci: Cmd: 0x0d1a | Max curr: 0x00ff it shows you always fail to get resp of sending status within the expected period of time. [ 14.639682] sdhci: Host ctl2: 0x [ 14.643611] sdhci: ADMA Err: 0x | ADMA Ptr: 0x4e6f7208 [ 14.649447] sdhci: === This repeats a few times, then more information is shown at the bottom: [ 86.893859] mmc0: Timeout waiting for hardware cmd interrupt. [ 86.899615] sdhci: === REGISTER DUMP (mmc0)=== [ 86.905453] sdhci: Sys addr: 0x | Version: 0x0002 [ 86.911291] sdhci: Blk size: 0x0200 | Blk cnt: 0x0001 [ 86.917129] sdhci: Argument: 0x0001 | Trn mode: 0x0013 [ 86.922967] sdhci: Present: 0x01fd8009 | Host ctl: 0x0031 [ 86.928804] sdhci: Power:0x0002 | Blk gap: 0x0080 [ 86.934642] sdhci: Wake-up: 0x0008 | Clock:0x001f [ 86.940479] sdhci: Timeout: 0x008f | Int stat: 0x [ 86.946316] sdhci: Int enab: 0x107f100b | Sig enab: 0x107f100b [ 86.952154] sdhci: AC12 err: 0x | Slot int: 0x0003 [ 86.957992] sdhci: Caps: 0x07eb | Caps_1: 0xa007 [ 86.963830] sdhci: Cmd: 0x0d1a | Max curr: 0x00ff [ 86.969668] sdhci: Host ctl2: 0x [ 86.973596] sdhci: ADMA Err: 0x | ADMA Ptr: 0x [ 86.979433] sdhci: === [ 86.986356] mmc0: switch to bus width 8 ddr failed [ 86.991163] mmc0: error -110 whilst initialising MMC card [ 97.773859] mmc0: Timeout waiting for hardware cmd interrupt. -- After looking through the latest commits to mmc/core, I found the culprit: Commit e173f8911f091fa50ccf8cc1fa316dd5569bc470 ("mmc: core: Update CMD13 polling policy when switch to HS DDR mode") Reverting it fixes the problem. But I am unsure if that's the right course of action? Feel free to send me patches for testing! By looking the changes itself, it should be good from the view of spec. Maybe you could try the patch below, but don't beat me if that doesn't help at all. :) --- a/drivers/mmc/core/mmc.c +++ b/drivers/mmc/core/mmc.c @@ -1074,7 +1074,7 @@ static int mmc_select_hs_ddr(struct mmc_card *card) EXT_CSD_BUS_WIDTH, ext_csd_bits, card->ext_csd.generic_cmd6_time, - MMC_TIMING_MMC_DDR52, + 0, true, true, true); if (err) { pr_err("%s: switch to bus width %d ddr failed\n", @@ -1118,6 +1118,9 @@ static int mmc_select_hs_ddr(struct mmc_card *card) if (err) err = __mmc_set_signal_voltage(host, MMC_SIGNAL_VOLTAGE_330); + if (!err) + mmc_set_timing(host, MMC_TIMING_MMC_DDR52); + Hi, thank you. This patch solves the problem! :) Tested-by: Clemens Gruber Regards, Clemens Everybody involved, thanks for looking into this! I think the above approach seems like a reasonable fix for the 4.10 rcs. Shawn Lin, would you mind re-posting a proper patch with a change-log? Sure. In the meantime, I will follow the process of Haibo Chen's debugging around the voltage switch issue and look into what Dong's suggesting around this may be. Just to be clear, I would definitely prefer a fix in the sdhci driver, yup, I prefer to fix the sdhc
Re: [PATCH 2/4] efi/x86: move efi bgrt init code to early init code
On 01/13/17 at 12:11am, Nicolai Stange wrote: > On Fri, Jan 13 2017, Dave Young wrote: > > > On 01/12/17 at 12:54pm, Nicolai Stange wrote: > >> On Thu, Jan 12 2017, Dave Young wrote: > >> > >> > -void __init efi_bgrt_init(void) > >> > +void __init efi_bgrt_init(struct acpi_table_header *table) > >> > { > >> > -acpi_status status; > >> > void *image; > >> > struct bmp_header bmp_header; > >> > > >> > if (acpi_disabled) > >> > return; > >> > > >> > -status = acpi_get_table("BGRT", 0, > >> > -(struct acpi_table_header **)&bgrt_tab); > >> > -if (ACPI_FAILURE(status)) > >> > -return; > >> > >> > >> Not sure, but wouldn't it be safer to reverse the order of this assignment > >> > >> > +bgrt_tab = *(struct acpi_table_bgrt *)table; > > > > Nicolai, sorry, I'm not sure I understand the comment, is it about above > > line? > > Could you elaborate a bit? > > > >> > >> and this length check > >> > > > > I also do not get this :( > > Ah sorry, my point is this: the length check should perhaps be made > before doing the assignment to bgrt_tab because otherwise, we might end > up reading from invalid memory. > > I.e. if (struct acpi_table_bgrt *)table->length < sizeof(bgrt_tab), then > > bgrt_tab = *(struct acpi_table_bgrt *)table; > > would read past the table's end. > > I'm not sure whether this is a real problem though -- that is, whether > this read could ever hit some unmapped memory. Nicolai, thanks for the explanation. It make sense to move it to even later at the end of the function. Thanks Dave
Re: [PATCH] mm/page_alloc: Wait for oom_lock before retrying.
On (01/12/17 15:18), Petr Mladek wrote: > On Mon 2016-12-26 20:34:07, Sergey Senozhatsky wrote: > > console_trylock() used to always forbid rescheduling; but it got changed > > like a yaer ago. > > > > the other thing is... do we really need to console_conditional_schedule() > > from fbcon_*()? console_unlock() does cond_resched() after every line it > > prints. wouldn't that be enough? > > > > so may be we can drop some of console_conditional_schedule() > > call sites in fbcon. or update console_conditional_schedule() > > function to always return the current preemption value, not the > > one we saw in console_trylock(). > > I was curious if it makes sense to remove > console_conditional_schedule() completely. I was looking at this option at some point as well. > In practice, it never allows rescheduling when the console driver > is called via console_unlock(). It is since 2006 and the commit > 78944e549d36673eb62 ("vt: printk: Fix framebuffer console > triggering might_sleep assertion"). This commit added > that > > console_may_schedule = 0; > > into console_unlock() before the console drivers are called. > > > On the other hand, it seems that the rescheduling was always > enabled when some console operations were called via > tty_operations. For example: > > struct tty_operations con_ops > > con_ops->con_write() > -> do_con_write() #calls console_lock() >-> do_con_trol() > -> fbcon_scroll() > -> fbcon_redraw_move() > -> console_conditional_schedule() > > , where console_lock() sets console_may_schedule = 1; > > > A complete console scroll/redraw might take a while. The rescheduling > would make sense => IMHO, we should keep console_conditional_schedule() > or some alternative in the console drivers as well. > > But I am afraid that we could not use the automatic detection. > We are not able to detect preemption when CONFIG_PREEMPT_COUNT can one actually have a preemptible kernel with !CONFIG_PREEMPT_COUNT? how? it's not even possible to change CONFIG_PREEMPT_COUNT in menuconfig. the option is automatically selected by PREEMPT. and if PREEMPT is not selected then _cond_resched() is just "{ rcu_all_qs(); return 0; }" ... > We cannot put the automatic detection into console_conditional_schedule(). why can't we? > I am going to prepare a patch for this. I'm on it. -ss
[PATCH] arm64: dts: mt8173: Fix cpu_thermal cooling-maps contributions
According to [0], the contribution field for each cooling-device express their relative power efficiency. Higher weights express higher power efficiency. Weighting is relative such that if each cooling device has a weight of 1 they are considered equal. This is particularly useful in heterogeneous systems where two cooling devices may perform the same kind of compute, but with different efficiency. [0] Documentation/thermal/power_allocator.txt According to Mediatek IC designer, the power efficiency ratio between the LITTLE core cluster (cooling-device cpu0) and big core cluster (cooling-device cpu1) is around 3:1 (3072:1024). Signed-off-by: Daniel Kurtz --- arch/arm64/boot/dts/mediatek/mt8173.dtsi | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi index 12e702771f5c..9a3b0d20f7a8 100644 --- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi +++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi @@ -182,12 +182,12 @@ map@0 { trip = <&target>; cooling-device = <&cpu0 0 0>; - contribution = <1024>; + contribution = <3072>; }; map@1 { trip = <&target>; cooling-device = <&cpu2 0 0>; - contribution = <2048>; + contribution = <1024>; }; }; }; -- 2.11.0.390.gc69c2f50cf-goog
Re: [PATCH 0/3] ata: add m68k/Atari Falcon PATA support
On Wed, 11 Jan 2017, Michael Schmitz wrote: > What is still correct is that the IDE driver does use the interrupt > only, not the ST-DMA chip. And a single IDE interrupt can be correctly > assigned to IDE by looking at the status register. > > With the SCSI (and IIRC also floppy) interrupts, we don't have direct > access to the status registers without disturbing the state of the DMA > though. Unless we know for definite that either chips have raised the > interrupt (and DMA ops are in flight), we must not touch the DMA chip at > all. > > The case I'm worried about is both IDE and SCSI raising an interrupt. We > don't currently mask the IDE/ST-DMA interrupt so a stacked interrupt > must be processed in the same pass as the initial interrupt or it will > get dropped. We'd have to peek at the DMA registers to check the SCSI or > floppy interrupt status, and we just can't safely do that. So races of > this kind are currently prevented by including IDE in the IRQ locking > process. > > Whether it's possible to mask the interrupt, do one pass, unmask and > process the second interrupt I don't know. Would that require handling the SCSI DMA interrupt in the first pass? Or handling IDE first, and ensuring that the IDE handler does not access ST-DMA registers? What about FDC? The atari_scsi handler accesses the ST-DMA registers; it can do so because it knows that any DMA must have completed -- it can infer this because a simultaneous pending interrupt from FDC or IDE is impossible due to stdma_lock(). Your suggestion would seem to allow other pending interrupts, hence the atari_scsi interrupt handler logic has to be tossed out. What logic would replace it? If all else fails, perhaps we could inhibit DMA entirely when the new ATA driver is loaded. Then we can just dispatch the ST-DMA irq like a shared irq. I'm sure that atari_scsi can work without DMA. No idea about the FDC driver though (ataflop.c). Another solution would be to dedicate the DMA function to atari_scsi, and then mask the FDC and IDE interrupts during each DMA transfer. But once again, this would mean changing the FDC driver to eliminate DMA, if that is possible. From the schematic it looks the the FDC chip, "AJAX", is another custom ... http://dev-docs.atariforge.org/files/Falcon030_Schematic.pdf Unfortunately my grasp of the ST hardware reflects my inability to read German; those who can may want to take a look at "ATARI Profibuch ST-STE-TT.pdf". -- > Maybe Andreas does? > > Cheers, > > Michael > > > > it should be okay to use IDE at the same time as SCSI/Floppy which is > > what the new driver does (the old one is serialized operations by > > ST-DMA related IRQ handling magic). > > > > Also the comment itself may need some fixups as on Falcon it is SCSI > > not ACSI (according to the earlier comment in same file) and the old > > IDE host driver name is not falhd.c but falconide.c. > > > > Best regards, > > -- > > Bartlomiej Zolnierkiewicz > > Samsung R&D Institute Poland > > Samsung Electronics > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-m68k" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
Re: [PATCH net-next v2 00/10] net: dsa: Support for pdata in dsa2
Hi Florian, Florian Fainelli writes: > Hi all, > > This is not exactly new, and was sent before, although back then, I did not > have an user of the pre-declared MDIO board information, but now we do. Note > that I have additional changes queued up to have b53 register platform data > for > MIPS bcm47xx and bcm63xx. > > Yes I know that we should have the Orion platforms eventually be converted to > Device Tree, but until that happens, I don't want any remaining users of the > old "dsa" platform device (hence the previous DTS submissions for ARM/mvebu) > and, there will be platforms out there that most likely won't never see DT > coming their way (BCM47xx is almost 100% sure, BCM63xx maybe not in a distant > future). > > We would probably want the whole series to be merged via David Miller's tree > to simplify things. > > Greg, can you Ack/Nack patch 5 since it touched the core LDD? > > Thanks! I've tested this patchset on my mv88e6xxx (DTS) boards to make sure nothing was broken, since it touches the driver. Looks good! Tested-by: Vivien Didelot Thanks, Vivien
Re: [PATCH] usb: hcd: initialize hcd->flags to 0 when rm hcd
Hi Roger, 在 2017年01月12日 23:38, Roger Quadros 写道: On 12/01/17 17:33, Alan Stern wrote: On Thu, 12 Jan 2017, Roger Quadros wrote: William, On 12/01/17 14:03, William Wu wrote: From: William wu On some platforms(e.g. rk3399 board), we can call hcd_add/remove consecutively without calling usb_put_hcd/usb_create_hcd in between, so hcd->flags can be stale. If the HC dies due to whatever reason then without this patch we get the below error on next hcd_add. [173.296154] xhci-hcd xhci-hcd.2.auto: HC died; cleaning up [173.296209] xhci-hcd xhci-hcd.2.auto: xHCI Host Controller [173.296762] xhci-hcd xhci-hcd.2.auto: new USB bus registered, assigned bus number 6 [173.296931] usb usb6: We don't know the algorithms for LPM for this host, disabling LPM. [173.297179] usb usb6: New USB device found, idVendor=1d6b, idProduct=0003 [173.297203] usb usb6: New USB device strings: Mfr=3, Product=2, SerialNumber=1 [173.297222] usb usb6: Product: xHCI Host Controller [173.297240] usb usb6: Manufacturer: Linux 4.4.21 xhci-hcd [173.297257] usb usb6: SerialNumber: xhci-hcd.2.auto [173.298680] hub 6-0:1.0: USB hub found [173.298749] hub 6-0:1.0: 1 port detected [173.299382] rockchip-dwc3 usb@fe80: USB HOST connected [173.395418] hub 5-0:1.0: activate --> -19 [173.603447] irq 228: nobody cared (try booting with the "irqpoll" option) [173.603493] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.4.21 #9 [173.603513] Hardware name: Google Kevin (DT) [173.603531] Call trace: [173.603568] [] dump_backtrace+0x0/0x160 [173.603596] [] show_stack+0x20/0x28 [173.603623] [] dump_stack+0x90/0xb0 [173.603650] [] __report_bad_irq+0x48/0xe8 [173.603674] [] note_interrupt+0x1e8/0x28c [173.603698] [] handle_irq_event_percpu+0x1d4/0x25c [173.603722] [] handle_irq_event+0x4c/0x7c [173.603748] [] handle_fasteoi_irq+0xb4/0x124 [173.603777] [] generic_handle_irq+0x30/0x44 [173.603804] [] __handle_domain_irq+0x90/0xbc [173.603827] [] gic_handle_irq+0xcc/0x188 ... [173.604500] [] el1_irq+0x80/0xf8 [173.604530] [] cpu_startup_entry+0x38/0x3cc [173.604558] [] rest_init+0x8c/0x94 [173.604585] [] start_kernel+0x3d0/0x3fc [173.604607] [<00b16000>] 0xb16000 [173.604622] handlers: [173.604648] [] usb_hcd_irq [173.604673] Disabling IRQ #228 Signed-off-by: William wu Signed-off-by: William wu --- drivers/usb/core/hcd.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index 479e223..612fab6 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -3017,6 +3017,7 @@ void usb_remove_hcd(struct usb_hcd *hcd) } usb_put_invalidate_rhdev(hcd); + hcd->flags = 0; } EXPORT_SYMBOL_GPL(usb_remove_hcd); I suggest to initialize hcd->flags to 0 in usb_add_hcd() instead. cheers, -roger Roger, didn't you originally propose this very same patch in http://marc.info/?l=linux-usb&m=146556415503583&w=2 (and of course, the earlier versions before v10)? What happened to it? Alan, You are right. That patch got lost in the ML. Looks like I didn't push it hard enough and then forgot about it. Sorry. William, You don't need to do any changes. My earlier version was clearing the flag in usb_add_hcd() and I guess Alan suggested to move it to usb_remove_hcd(). So your patch is good. You can add my. Acked-by: Roger Quadros cheers, -roger Thanks very much for your suggestion,I will add acked-by immediately.
[PATCH] rcu: Narrow early boot window of illegal synchronous grace periods
The current preemptible RCU implementation goes through three phases during bootup. In the first phase, there is only one CPU that is running with preemption disabled, so that a no-op is a synchronous grace period. In the second mid-boot phase, the scheduler is running, but RCU has not yet gotten its kthreads spawned (and, for expedited grace periods, workqueues are not yet running. During this time, any attempt to do a synchronous grace period will hang the system (or complain bitterly, depending). In the third and final phase, RCU is fully operational and everything works normally. This has been OK for some time, but there has recently been some synchronous grace periods showing up during the second mid-boot phase. This commit therefore reworks RCU to permit synchronous grace periods to proceed during this mid-boot phase. This commit accomplishes this by setting a flag from the existing rcu_scheduler_starting() function which causes all synchronous grace periods to take the expedited path. The expedited path now checks this flag, using the requesting task to drive the expedited grace period forward during the mid-boot phase. Finally, this flag is updated by a core_initcall() function named rcu_exp_runtime_mode(), which causes the runtime codepaths to be used. Note that this arrangement assumes that tasks are not sent POSIX signals (or anything similar) from the time that the first task is spawned through core_initcall() time. Reported-by: "Zheng, Lv" Reported-by: Borislav Petkov Signed-off-by: Paul E. McKenney diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h index 321f9ed552a9..01f71e1d2e94 100644 --- a/include/linux/rcupdate.h +++ b/include/linux/rcupdate.h @@ -444,6 +444,10 @@ bool __rcu_is_watching(void); #error "Unknown RCU implementation specified to kernel configuration" #endif +#define RCU_SCHEDULER_INACTIVE 0 +#define RCU_SCHEDULER_INIT 1 +#define RCU_SCHEDULER_RUNNING 2 + /* * init_rcu_head_on_stack()/destroy_rcu_head_on_stack() are needed for dynamic * initialization and destruction of rcu_head on the stack. rcu_head structures diff --git a/kernel/rcu/rcu.h b/kernel/rcu/rcu.h index 80adef7d4c3d..0d6ff3e471be 100644 --- a/kernel/rcu/rcu.h +++ b/kernel/rcu/rcu.h @@ -136,6 +136,7 @@ int rcu_jiffies_till_stall_check(void); #define TPS(x) tracepoint_string(x) void rcu_early_boot_tests(void); +void rcu_test_sync_prims(void); /* * This function really isn't for public consumption, but RCU is special in diff --git a/kernel/rcu/tiny_plugin.h b/kernel/rcu/tiny_plugin.h index 196f0302e2f4..e3953bdee383 100644 --- a/kernel/rcu/tiny_plugin.h +++ b/kernel/rcu/tiny_plugin.h @@ -65,7 +65,7 @@ EXPORT_SYMBOL_GPL(rcu_scheduler_active); void __init rcu_scheduler_starting(void) { WARN_ON(nr_context_switches() > 0); - rcu_scheduler_active = 1; + rcu_scheduler_active = RCU_SCHEDULER_RUNNING; } #endif /* #ifdef CONFIG_DEBUG_LOCK_ALLOC */ diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c index 96c52e43f7ca..7bcce4607863 100644 --- a/kernel/rcu/tree.c +++ b/kernel/rcu/tree.c @@ -127,13 +127,16 @@ int rcu_num_nodes __read_mostly = NUM_RCU_NODES; /* Total # rcu_nodes in use. */ int sysctl_panic_on_rcu_stall __read_mostly; /* - * The rcu_scheduler_active variable transitions from zero to one just - * before the first task is spawned. So when this variable is zero, RCU + * The rcu_scheduler_active variable transitions from + * RCU_SCHEDULER_INACTIVE to RCU_SCHEDULER_INIT just before the first + * task is spawned. So when this variable is RCU_SCHEDULER_INACTIVE, RCU * can assume that there is but one task, allowing RCU to (for example) * optimize synchronize_rcu() to a simple barrier(). When this variable * is one, RCU must actually do all the hard work required to detect real * grace periods. This variable is also used to suppress boot-time false - * positives from lockdep-RCU error checking. + * positives from lockdep-RCU error checking. Finally, this variable + * transitions from RCU_SCHEDULER_INIT to RCU_SCHEDULER_RUNNING after RCU + * is fully initialized, including all of its tasks having been spawned. */ int rcu_scheduler_active __read_mostly; EXPORT_SYMBOL_GPL(rcu_scheduler_active); @@ -3980,18 +3983,22 @@ static int __init rcu_spawn_gp_kthread(void) early_initcall(rcu_spawn_gp_kthread); /* - * This function is invoked towards the end of the scheduler's initialization - * process. Before this is called, the idle task might contain - * RCU read-side critical sections (during which time, this idle - * task is booting the system). After this function is called, the - * idle tasks are prohibited from containing RCU read-side critical - * sections. This function also enables RCU lockdep checking. + * This function is invoked towards the end of the scheduler's + * initialization process. Before this is called, the idle task might + * contain synchronous grace-period primitives (during which time, this idle + * task
Re: [PATCH v4 05/15] lockdep: Make check_prev_add can use a separate stack_trace
On Thu, Jan 12, 2017 at 05:16:43PM +0100, Peter Zijlstra wrote: > On Fri, Dec 09, 2016 at 02:12:01PM +0900, Byungchul Park wrote: > > check_prev_add() saves a stack trace of the current. But crossrelease > > feature needs to use a separate stack trace of another context in > > check_prev_add(). So make it use a separate stack trace instead of one > > of the current. > > > > So I was thinking, can't we make check_prevs_add() create the stack > trace unconditionally but record if we used it or not, and then return > the entries when unused. All that is serialized by graph_lock anyway and > that way we already pass a stack into check_prev_add() so we can easily > pass in a different one. > > I think that removes a bunch of tricky and avoids all the new tricky. Looks very good.
Re: [PATCH] mm/page_alloc: Wait for oom_lock before retrying.
On (01/12/17 14:10), Petr Mladek wrote: [..] > > /** > > * console_lock - lock the console system for exclusive use. > > * > > @@ -2316,7 +2321,7 @@ EXPORT_SYMBOL(console_unlock); > > */ > > void __sched console_conditional_schedule(void) > > { > > - if (console_may_schedule) > > + if (get_console_may_schedule()) > > Note that console_may_schedule should be zero when > the console drivers are called. See the following lines in > console_unlock(): > > /* >* Console drivers are called under logbuf_lock, so >* @console_may_schedule should be cleared before; however, we may >* end up dumping a lot of lines, for example, if called from >* console registration path, and should invoke cond_resched() >* between lines if allowable. Not doing so can cause a very long >* scheduling stall on a slow console leading to RCU stall and >* softlockup warnings which exacerbate the issue with more >* messages practically incapacitating the system. >*/ > do_cond_resched = console_may_schedule; > console_may_schedule = 0; console drivers are never-ever-ever getting called under logbuf lock. never. with disabled local IRQs - yes. under logbuf lock - no. that would soft lockup systems in really bad ways, otherwise. the reason why we set console_may_schedule to zero in console_unlock() is VT. and lf() function in particular. commit 78944e549d36673eb6265a2411574e79c28e23dc Author: Antonino A. Daplas Date: Sat Aug 5 12:14:16 2006 -0700 [PATCH] vt: printk: Fix framebuffer console triggering might_sleep assertion Reported by: Dave Jones Whilst printk'ing to both console and serial console, I got this... (2.6.18rc1) BUG: sleeping function called from invalid context at kernel/sched.c:4438 in_atomic():0, irqs_disabled():1 Call Trace: [] show_trace+0xaa/0x23d [] dump_stack+0x15/0x17 [] __might_sleep+0xb2/0xb4 [] __cond_resched+0x15/0x55 [] cond_resched+0x3b/0x42 [] console_conditional_schedule+0x12/0x14 [] fbcon_redraw+0xf6/0x160 [] fbcon_scroll+0x5d9/0xb52 [] scrup+0x6b/0xd6 [] lf+0x24/0x44 [] vt_console_print+0x166/0x23d [] __call_console_drivers+0x65/0x76 [] _call_console_drivers+0x5e/0x62 [] release_console_sem+0x14b/0x232 [] fb_flashcursor+0x279/0x2a6 [] run_workqueue+0xa8/0xfb [] worker_thread+0xef/0x122 [] kthread+0x100/0x136 [] child_rip+0x8/0x12 and we really don't want to cond_resched() when we are in panic. that's why console_flush_on_panic() sets it to zero explicitly. console_trylock() checks oops_in_progress, so re-taking the semaphore when we are in panic() console_flush_on_panic() console_unlock() console_trylock() should be OK. as well as doing get_console_conditional_schedule() somewhere in console driver code. I still don't understand why do you guys think we can't simply do get_console_conditional_schedule() and get the actual value. [..] > Sergey, if you agree with the above paragraph. Do you want to prepare > the patch or should I do so? I'm on it. -ss
RE: [PATCH v2 1/4] ASoC: rt5660: Add ACPI support
> -Original Message- > From: Shrirang Bagul [mailto:shrirang.ba...@canonical.com] > Sent: Thursday, January 12, 2017 8:01 PM > To: alsa-de...@alsa-project.org > Cc: linux-kernel@vger.kernel.org; Bard Liao; Oder Chiou; Liam Girdwood; Mark > Brown; Jaroslav Kysela; Takashi Iwai > Subject: [PATCH v2 1/4] ASoC: rt5660: Add ACPI support > > On Dell IoT Gateways, RT5660 codec is available with ACPI ID 10EC3277. > Also, GPIO's are only available by index, so we register mappings to allow > machine drivers to access them by name. > > Signed-off-by: Shrirang Bagul > --- > +static const struct acpi_gpio_params audio_wake_intr_gpio = > { RT5660_GPIO_WAKE_INTR, 0, false }; > +static const struct acpi_gpio_params lineout_mute_gpio = > { RT5660_GPIO_LINEOUT_MUTE, 0, true }; > + > +static const struct acpi_gpio_mapping byt_rt5660_gpios[] = { > + { "audio-wake-intr-gpios", &audio_wake_intr_gpio, 1 }, > + { "lineout-mute-gpios", &lineout_mute_gpio, 1 }, > + { NULL }, > +}; > + I am thinking if it is more suitable to move the gpio params to machine driver? They are not codec's gpios and are only used by machine driver. > -- > 2.9.3
[PATCH v3] ocfs2/journal: fix umount hang after flushing journal failure
Hi Joseph, Do you think my last version of patch to fix umount hang after journal flushing failure is OK? If so, I 'd like to ask Andrew's help to merge this patch into his test tree. Thanks, Br. Changwei From 686b52ee2f06395c53e36e2c7515c276dc7541fb Mon Sep 17 00:00:00 2001 From: Changwei Ge Date: Wed, 11 Jan 2017 09:05:35 +0800 Subject: [PATCH] fix umount hang after journal flushing failure Signed-off-by: Changwei Ge --- fs/ocfs2/journal.c | 18 ++ 1 file changed, 18 insertions(+) diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c index a244f14..5f3c862 100644 --- a/fs/ocfs2/journal.c +++ b/fs/ocfs2/journal.c @@ -2315,6 +2315,24 @@ static int ocfs2_commit_thread(void *arg) "commit_thread: %u transactions pending on " "shutdown\n", atomic_read(&journal->j_num_trans)); + + if (status < 0) { + mlog(ML_ERROR, "journal is already abort and cannot be " +"flushed any more. So ignore the pending " +"transactions to avoid blocking ocfs2 unmount.\n"); + /* +* This may a litte hacky, however, no chance +* for ocfs2/journal to decrease this variable +* thourgh commit-thread. I have to do so to +* avoid umount hang after journal flushing +* failure. Since jounral has been marked ABORT +* within jbd2_journal_flush, commit cache will +* never do any real work to flush journal to +* disk.Set it to ZERO so that umount will +* continue during shutting down journal +*/ + atomic_set(&journal->j_num_trans, 0); + } } } -- 1.7.9.5 - 本邮件及其附件含有杭州华三通信技术有限公司的保密信息,仅限于发送给上面地址中列出 的个人或群组。禁止任何其他人以任何形式使用(包括但不限于全部或部分地泄露、复制、 或散发)本邮件中的信息。如果您错收了本邮件,请您立即电话或邮件通知发件人并删除本 邮件! This e-mail and its attachments contain confidential information from H3C, which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction, or dissemination) by persons other than the intended recipient(s) is prohibited. If you receive this e-mail in error, please notify the sender by phone or email immediately and delete it!
[PATCH] lockdep: Fix wrong condition to print bug msgs for MAX_LOCKDEP_CHAIN_HLOCKS
Bug messages and stack dump for MAX_LOCKDEP_CHAIN_HLOCKS should be printed only at the first time. Signed-off-by: Byungchul Park --- kernel/locking/lockdep.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c index f37156f..a143eb4 100644 --- a/kernel/locking/lockdep.c +++ b/kernel/locking/lockdep.c @@ -2166,7 +2166,7 @@ static inline int add_chain_cache(struct task_struct *curr, * Important for check_no_collision(). */ if (unlikely(nr_chain_hlocks > MAX_LOCKDEP_CHAIN_HLOCKS)) { - if (debug_locks_off_graph_unlock()) + if (!debug_locks_off_graph_unlock()) return 0; print_lockdep_off("BUG: MAX_LOCKDEP_CHAIN_HLOCKS too low!"); -- 1.9.1
Re: [PATCH 2/4] efi/x86: move efi bgrt init code to early init code
On 01/13/17 at 10:21am, Dave Young wrote: > On 01/13/17 at 12:11am, Nicolai Stange wrote: > > On Fri, Jan 13 2017, Dave Young wrote: > > > > > On 01/12/17 at 12:54pm, Nicolai Stange wrote: > > >> On Thu, Jan 12 2017, Dave Young wrote: > > >> > > >> > -void __init efi_bgrt_init(void) > > >> > +void __init efi_bgrt_init(struct acpi_table_header *table) > > >> > { > > >> > - acpi_status status; > > >> >void *image; > > >> >struct bmp_header bmp_header; > > >> > > > >> >if (acpi_disabled) > > >> >return; > > >> > > > >> > - status = acpi_get_table("BGRT", 0, > > >> > - (struct acpi_table_header **)&bgrt_tab); > > >> > - if (ACPI_FAILURE(status)) > > >> > - return; > > >> > > >> > > >> Not sure, but wouldn't it be safer to reverse the order of this > > >> assignment > > >> > > >> > + bgrt_tab = *(struct acpi_table_bgrt *)table; > > > > > > Nicolai, sorry, I'm not sure I understand the comment, is it about above > > > line? > > > Could you elaborate a bit? > > > > > >> > > >> and this length check > > >> > > > > > > I also do not get this :( > > > > Ah sorry, my point is this: the length check should perhaps be made > > before doing the assignment to bgrt_tab because otherwise, we might end > > up reading from invalid memory. > > > > I.e. if (struct acpi_table_bgrt *)table->length < sizeof(bgrt_tab), then > > > > bgrt_tab = *(struct acpi_table_bgrt *)table; > > > > would read past the table's end. > > > > I'm not sure whether this is a real problem though -- that is, whether > > this read could ever hit some unmapped memory. > > Nicolai, thanks for the explanation. It make sense to move it to even later > at the end of the function. Indeed assignment should be after the length checking, but with another tmp variable the assignment to global var can be moved to the end to avoid clear the image_address field.. > > Thanks > Dave
Re: [PATCH] virtio_mmio: expose header to userspace
On 2017年01月13日 05:37, Michael S. Tsirkin wrote: It's handy for userspace emulators like QEMU. Signed-off-by: Michael S. Tsirkin --- drivers/virtio/virtio_mmio.c | 2 +- include/{ => uapi}/linux/virtio_mmio.h | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename include/{ => uapi}/linux/virtio_mmio.h (100%) diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c index d47a2fc..7a10aa2 100644 --- a/drivers/virtio/virtio_mmio.c +++ b/drivers/virtio/virtio_mmio.c @@ -69,7 +69,7 @@ #include #include #include -#include +#include #include diff --git a/include/linux/virtio_mmio.h b/include/uapi/linux/virtio_mmio.h similarity index 100% rename from include/linux/virtio_mmio.h rename to include/uapi/linux/virtio_mmio.h Acked-by: Jason Wang
Re: x86-64: Maintain 16-byte stack alignment
On Thu, Jan 12, 2017 at 05:46:55PM -0800, Andy Lutomirski wrote: > On Thu, Jan 12, 2017 at 12:15 PM, Josh Poimboeuf wrote: > > On Thu, Jan 12, 2017 at 12:08:07PM -0800, Andy Lutomirski wrote: > >> On Thu, Jan 12, 2017 at 11:51 AM, Linus Torvalds > >> wrote: > >> > On Thu, Jan 12, 2017 at 6:02 AM, Josh Poimboeuf > >> > wrote: > >> >> > >> >> Just to clarify, I think you're asking if, for versions of gcc which > >> >> don't support -mpreferred-stack-boundary=3, objtool can analyze all C > >> >> functions to ensure their stacks are 16-byte aligned. > >> >> > >> >> It's certainly possible, but I don't see how that solves the problem. > >> >> The stack will still be misaligned by entry code. Or am I missing > >> >> something? > >> > > >> > I think the argument is that we *could* try to align things, if we > >> > just had some tool that actually then verified that we aren't missing > >> > anything. > >> > > >> > I'm not entirely happy with checking the generated code, though, > >> > because as Ingo says, you have a 50:50 chance of just getting it right > >> > by mistake. So I'd much rather have some static tool that checks > >> > things at a code level (ie coccinelle or sparse). > >> > >> What I meant was checking the entry code to see if it aligns stack > >> frames, and good luck getting sparse to do that. Hmm, getting 16-byte > >> alignment for real may actually be entirely a lost cause. After all, > >> I think we have some inline functions that do asm volatile ("call > >> ..."), and I don't see any credible way of forcing alignment short of > >> generating an entirely new stack frame and aligning that. > > > > Actually we already found all such cases and fixed them by forcing a new > > stack frame, thanks to objtool. For example, see 55a76b59b5fe. > > What I mean is: what guarantees that the stack is properly aligned for > the subroutine call? gcc promises to set up a stack frame, but does > it promise that rsp will be properly aligned to call a C function? Yes, I did an experiment and you're right. I had naively assumed that all stack frames would be aligned. -- Josh
[RESEND PATCH] usb: hcd: initialize hcd->flags to 0 when rm hcd
From: William wu On some platforms(e.g. rk3399 board), we can call hcd_add/remove consecutively without calling usb_put_hcd/usb_create_hcd in between, so hcd->flags can be stale. If the HC dies due to whatever reason then without this patch we get the below error on next hcd_add. [173.296154] xhci-hcd xhci-hcd.2.auto: HC died; cleaning up [173.296209] xhci-hcd xhci-hcd.2.auto: xHCI Host Controller [173.296762] xhci-hcd xhci-hcd.2.auto: new USB bus registered, assigned bus number 6 [173.296931] usb usb6: We don't know the algorithms for LPM for this host, disabling LPM. [173.297179] usb usb6: New USB device found, idVendor=1d6b, idProduct=0003 [173.297203] usb usb6: New USB device strings: Mfr=3, Product=2, SerialNumber=1 [173.297222] usb usb6: Product: xHCI Host Controller [173.297240] usb usb6: Manufacturer: Linux 4.4.21 xhci-hcd [173.297257] usb usb6: SerialNumber: xhci-hcd.2.auto [173.298680] hub 6-0:1.0: USB hub found [173.298749] hub 6-0:1.0: 1 port detected [173.299382] rockchip-dwc3 usb@fe80: USB HOST connected [173.395418] hub 5-0:1.0: activate --> -19 [173.603447] irq 228: nobody cared (try booting with the "irqpoll" option) [173.603493] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.4.21 #9 [173.603513] Hardware name: Google Kevin (DT) [173.603531] Call trace: [173.603568] [] dump_backtrace+0x0/0x160 [173.603596] [] show_stack+0x20/0x28 [173.603623] [] dump_stack+0x90/0xb0 [173.603650] [] __report_bad_irq+0x48/0xe8 [173.603674] [] note_interrupt+0x1e8/0x28c [173.603698] [] handle_irq_event_percpu+0x1d4/0x25c [173.603722] [] handle_irq_event+0x4c/0x7c [173.603748] [] handle_fasteoi_irq+0xb4/0x124 [173.603777] [] generic_handle_irq+0x30/0x44 [173.603804] [] __handle_domain_irq+0x90/0xbc [173.603827] [] gic_handle_irq+0xcc/0x188 ... [173.604500] [] el1_irq+0x80/0xf8 [173.604530] [] cpu_startup_entry+0x38/0x3cc [173.604558] [] rest_init+0x8c/0x94 [173.604585] [] start_kernel+0x3d0/0x3fc [173.604607] [<00b16000>] 0xb16000 [173.604622] handlers: [173.604648] [] usb_hcd_irq [173.604673] Disabling IRQ #228 Signed-off-by: William wu Acked-by: Roger Quadros --- drivers/usb/core/hcd.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index 479e223..612fab6 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -3017,6 +3017,7 @@ void usb_remove_hcd(struct usb_hcd *hcd) } usb_put_invalidate_rhdev(hcd); + hcd->flags = 0; } EXPORT_SYMBOL_GPL(usb_remove_hcd); -- 2.7.4
RE: eMMC boot problem: switch to bus width 8 ddr failed
> -Original Message- > From: Shawn Lin [mailto:shawn@rock-chips.com] > Sent: Friday, January 13, 2017 10:11 AM > To: Ulf Hansson ; Clemens Gruber > > Cc: shawn@rock-chips.com; linux-...@vger.kernel.org; Linus Walleij > ; Adrian Hunter ; A.S. > Dong ; linux-kernel@vger.kernel.org; Bough Chen > ; Gary Bisson ; > Fabio Estevam ; Shawn Guo > Subject: Re: eMMC boot problem: switch to bus width 8 ddr failed > > On 2017/1/13 0:51, Ulf Hansson wrote: > > + Haibo, Gary, Fabio, Shawn Gou > > > > On 6 January 2017 at 16:56, Clemens Gruber > wrote: > >> On Fri, Jan 06, 2017 at 10:33:49AM +0800, Shawn Lin wrote: > >>> On 2017/1/6 8:41, Clemens Gruber wrote: > Hi, > > with the current mainline 4.10-rc2 kernel, I can no longer boot > from the eMMC on my i.MX6Q board. > > Details: > The eMMC is a Micron MTFC4GACAJCN-1M WT but as the i.MX6Q only > supports eMMC 4.41 features and we did not implement voltage > switching from 3.3V to 1.8V or lower, I did add no-1-8-v; (but none > of the mmc-ddr or mmc-hs > options) to the device tree. The bus-width is 8. > > With 4.9 the board booted fine, now with the current mainline 4.10 > tree, I get the following (repeating) errors at boot: > > [4.326834] Waiting for root device /dev/mmcblk0p2... > [ 14.563861] mmc0: Timeout waiting for hardware cmd interrupt. > [ 14.569619] sdhci: === REGISTER DUMP > (mmc0)=== > [ 14.575461] sdhci: Sys addr: 0x4e726000 | Version: 0x0002 > [ 14.581300] sdhci: Blk size: 0x0200 | Blk cnt: 0x0001 > [ 14.587140] sdhci: Argument: 0x0001 | Trn mode: 0x0013 > [ 14.592979] sdhci: Present: 0x01fd8009 | Host ctl: 0x0031 > [ 14.598816] sdhci: Power:0x0002 | Blk gap: 0x0080 > [ 14.604654] sdhci: Wake-up: 0x0008 | Clock:0x001f > [ 14.610493] sdhci: Timeout: 0x008f | Int stat: 0x > [ 14.616332] sdhci: Int enab: 0x107f100b | Sig enab: 0x107f100b > [ 14.622168] sdhci: AC12 err: 0x | Slot int: 0x0003 > [ 14.628007] sdhci: Caps: 0x07eb | Caps_1: 0xa007 > [ 14.633845] sdhci: Cmd: 0x0d1a | Max curr: 0x00ff > >>> > >>> it shows you always fail to get resp of sending status within the > >>> expected period of time. > >>> > >>> > [ 14.639682] sdhci: Host ctl2: 0x > [ 14.643611] sdhci: ADMA Err: 0x | ADMA Ptr: 0x4e6f7208 > [ 14.649447] sdhci: > === > > This repeats a few times, then more information is shown at the bottom: > > [ 86.893859] mmc0: Timeout waiting for hardware cmd interrupt. > [ 86.899615] sdhci: === REGISTER DUMP > (mmc0)=== > [ 86.905453] sdhci: Sys addr: 0x | Version: 0x0002 > [ 86.911291] sdhci: Blk size: 0x0200 | Blk cnt: 0x0001 > [ 86.917129] sdhci: Argument: 0x0001 | Trn mode: 0x0013 > [ 86.922967] sdhci: Present: 0x01fd8009 | Host ctl: 0x0031 > [ 86.928804] sdhci: Power:0x0002 | Blk gap: 0x0080 > [ 86.934642] sdhci: Wake-up: 0x0008 | Clock:0x001f > [ 86.940479] sdhci: Timeout: 0x008f | Int stat: 0x > [ 86.946316] sdhci: Int enab: 0x107f100b | Sig enab: 0x107f100b > [ 86.952154] sdhci: AC12 err: 0x | Slot int: 0x0003 > [ 86.957992] sdhci: Caps: 0x07eb | Caps_1: 0xa007 > [ 86.963830] sdhci: Cmd: 0x0d1a | Max curr: 0x00ff > [ 86.969668] sdhci: Host ctl2: 0x > [ 86.973596] sdhci: ADMA Err: 0x | ADMA Ptr: 0x > [ 86.979433] sdhci: > === > [ 86.986356] mmc0: switch to bus width 8 ddr failed > [ 86.991163] mmc0: error -110 whilst initialising MMC card > [ 97.773859] mmc0: Timeout waiting for hardware cmd interrupt. > > -- > > After looking through the latest commits to mmc/core, I found the > culprit: > Commit e173f8911f091fa50ccf8cc1fa316dd5569bc470 ("mmc: core: > Update > CMD13 polling policy when switch to HS DDR mode") > > Reverting it fixes the problem. But I am unsure if that's the right > course of action? > > Feel free to send me patches for testing! > >>> > >>> By looking the changes itself, it should be good from the view of spec. > >>> Maybe you could try the patch below, but don't beat me if that > >>> doesn't help at all. :) > >>> > >>> --- a/drivers/mmc/core/mmc.c > >>> +++ b/drivers/mmc/core/mmc.c > >>> @@ -1074,7 +1074,7 @@ static int mmc_select_hs_ddr(struct mmc_card > *card) > >>>EXT_CSD_BUS_WIDTH, > >>>ext_csd_bits, > >>>card->ext_csd.generic_cmd6_time, > >>> -
linux-next: build warning after merge of the rtc tree
Hi Alexandre, After merging the rtc tree, today's linux-next build (x86_64 allmodconfig) produced this warning: In file included from drivers/rtc/rtc-stm32.c:14:0: drivers/rtc/rtc-stm32.c: In function 'stm32_rtc_probe': drivers/rtc/rtc-stm32.c:653:51: warning: large integer implicitly truncated to unsigned type [-Woverflow] regmap_update_bits(rtc->dbp, PWR_CR, PWR_CR_DBP, ~PWR_CR_DBP); ^ include/linux/regmap.h:73:42: note: in definition of macro 'regmap_update_bits' regmap_update_bits_base(map, reg, mask, val, NULL, false, false) ^ drivers/rtc/rtc-stm32.c: In function 'stm32_rtc_remove': drivers/rtc/rtc-stm32.c:675:51: warning: large integer implicitly truncated to unsigned type [-Woverflow] regmap_update_bits(rtc->dbp, PWR_CR, PWR_CR_DBP, ~PWR_CR_DBP); ^ include/linux/regmap.h:73:42: note: in definition of macro 'regmap_update_bits' regmap_update_bits_base(map, reg, mask, val, NULL, false, false) ^ Introduced by commit 4e64350f42e2 ("rtc: add STM32 RTC driver") -- Cheers, Stephen Rothwell
[PATCH] usb: host: xhci: plat: check hcc_params after add hcd
From: William wu The commit 4ac53087d6d4 ("usb: xhci: plat: Create both HCDs before adding them") move add hcd to the end of probe, this cause hcc_params uninitiated, because xHCI driver sets hcc_params in xhci_gen_setup() called from usb_add_hcd(). This patch checks the Maximum Primary Stream Array Size in the hcc_params register after add hcd. Signed-off-by: William wu --- drivers/usb/host/xhci-plat.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c index ddfab30..52ce697 100644 --- a/drivers/usb/host/xhci-plat.c +++ b/drivers/usb/host/xhci-plat.c @@ -232,9 +232,6 @@ static int xhci_plat_probe(struct platform_device *pdev) if (device_property_read_bool(&pdev->dev, "usb3-lpm-capable")) xhci->quirks |= XHCI_LPM_SUPPORT; - if (HCC_MAX_PSA(xhci->hcc_params) >= 4) - xhci->shared_hcd->can_do_streams = 1; - hcd->usb_phy = devm_usb_get_phy_by_phandle(&pdev->dev, "usb-phy", 0); if (IS_ERR(hcd->usb_phy)) { ret = PTR_ERR(hcd->usb_phy); @@ -255,6 +252,9 @@ static int xhci_plat_probe(struct platform_device *pdev) if (ret) goto dealloc_usb2_hcd; + if (HCC_MAX_PSA(xhci->hcc_params) >= 4) + xhci->shared_hcd->can_do_streams = 1; + return 0; -- 2.7.4
Re: x86-64: Maintain 16-byte stack alignment
On Thu, Jan 12, 2017 at 7:11 PM, Josh Poimboeuf wrote: > On Thu, Jan 12, 2017 at 05:46:55PM -0800, Andy Lutomirski wrote: >> On Thu, Jan 12, 2017 at 12:15 PM, Josh Poimboeuf wrote: >> > On Thu, Jan 12, 2017 at 12:08:07PM -0800, Andy Lutomirski wrote: >> >> On Thu, Jan 12, 2017 at 11:51 AM, Linus Torvalds >> >> wrote: >> >> > On Thu, Jan 12, 2017 at 6:02 AM, Josh Poimboeuf >> >> > wrote: >> >> >> >> >> >> Just to clarify, I think you're asking if, for versions of gcc which >> >> >> don't support -mpreferred-stack-boundary=3, objtool can analyze all C >> >> >> functions to ensure their stacks are 16-byte aligned. >> >> >> >> >> >> It's certainly possible, but I don't see how that solves the problem. >> >> >> The stack will still be misaligned by entry code. Or am I missing >> >> >> something? >> >> > >> >> > I think the argument is that we *could* try to align things, if we >> >> > just had some tool that actually then verified that we aren't missing >> >> > anything. >> >> > >> >> > I'm not entirely happy with checking the generated code, though, >> >> > because as Ingo says, you have a 50:50 chance of just getting it right >> >> > by mistake. So I'd much rather have some static tool that checks >> >> > things at a code level (ie coccinelle or sparse). >> >> >> >> What I meant was checking the entry code to see if it aligns stack >> >> frames, and good luck getting sparse to do that. Hmm, getting 16-byte >> >> alignment for real may actually be entirely a lost cause. After all, >> >> I think we have some inline functions that do asm volatile ("call >> >> ..."), and I don't see any credible way of forcing alignment short of >> >> generating an entirely new stack frame and aligning that. >> > >> > Actually we already found all such cases and fixed them by forcing a new >> > stack frame, thanks to objtool. For example, see 55a76b59b5fe. >> >> What I mean is: what guarantees that the stack is properly aligned for >> the subroutine call? gcc promises to set up a stack frame, but does >> it promise that rsp will be properly aligned to call a C function? > > Yes, I did an experiment and you're right. I had naively assumed that > all stack frames would be aligned. Just to check: did you do your experiment with -mpreferred-stack-boundary=4? --Andy
Re: Problem on SCTP
Hi All below is the packet trace in text format. Packet received (From client to router) [ Source IP: 192.168.206.83, Destination IP: 192.168.206.65. the IPv4 address parameter is 192.168.206.83 and 192.168.1.83 ] == No. Time SourceSPort Destination Protocol DPort Length Info DSCP 1 2017-01-06 08:52:49.662142192.168.206.8350001 192.168.206.65SCTP 3868 98 INIT CS0 Frame 1: 98 bytes on wire (784 bits), 98 bytes captured (784 bits) Encapsulation type: Ethernet (1) Arrival Time: Jan 6, 2017 16:52:49.662142000 Malay Peninsula Standard Time [Time shift for this packet: 0.0 seconds] Epoch Time: 1483692769.662142000 seconds [Time delta from previous captured frame: 0.0 seconds] [Time delta from previous displayed frame: 0.0 seconds] [Time since reference or first frame: 0.0 seconds] Frame Number: 1 Frame Length: 98 bytes (784 bits) Capture Length: 98 bytes (784 bits) [Frame is marked: False] [Frame is ignored: False] [Protocols in frame: eth:ethertype:ip:sctp] Ethernet II, Src: RealtekU_54:81:87 (52:54:00:54:81:87), Dst: Vmware_81:41:6b (00:50:56:81:41:6b) Destination: Vmware_81:41:6b (00:50:56:81:41:6b) Address: Vmware_81:41:6b (00:50:56:81:41:6b) ..0. = LG bit: Globally unique address (factory default) ...0 = IG bit: Individual address (unicast) Source: RealtekU_54:81:87 (52:54:00:54:81:87) Address: RealtekU_54:81:87 (52:54:00:54:81:87) ..1. = LG bit: Locally administered address (this is NOT the factory default) ...0 = IG bit: Individual address (unicast) Type: IPv4 (0x0800) Internet Protocol Version 4, Src: 192.168.206.83, Dst: 192.168.206.65 0100 = Version: 4 0101 = Header Length: 20 bytes (5) Differentiated Services Field: 0x02 (DSCP: CS0, ECN: ECT(0)) 00.. = Differentiated Services Codepoint: Default (0) ..10 = Explicit Congestion Notification: ECN-Capable Transport codepoint '10' (2) Total Length: 84 Identification: 0x (0) Flags: 0x02 (Don't Fragment) 0... = Reserved bit: Not set .1.. = Don't fragment: Set ..0. = More fragments: Not set Fragment offset: 0 Time to live: 64 Protocol: SCTP (132) Header checksum: 0x1c3e [validation disabled] [Good: False] [Bad: False] Source: 192.168.206.83 Destination: 192.168.206.65 [Source GeoIP: Unknown] [Destination GeoIP: Unknown] Stream Control Transmission Protocol, Src Port: 50001 (50001), Dst Port: 3868 (3868) Source port: 50001 Destination port: 3868 Verification tag: 0x [Assocation index: 0] Checksum: 0xbaea49e5 (not verified) INIT chunk (Outbound streams: 3000, inbound streams: 3000) Chunk type: INIT (1) 0... = Bit: Stop processing of the packet .0.. = Bit: Do not report Chunk flags: 0x00 Chunk length: 52 Initiate tag: 0xe79f40cb Advertised receiver window credit (a_rwnd): 62464 Number of outbound streams: 3000 Number of inbound streams: 3000 Initial TSN: 176990880 IPv4 address parameter (Address: 192.168.206.83) Parameter type: IPv4 address (0x0005) 0... = Bit: Stop processing of chunk .0.. = Bit: Do not report Parameter length: 8 IP Version 4 address: 192.168.206.83 IPv4 address parameter (Address: 192.168.1.83) Parameter type: IPv4 address (0x0005) 0... = Bit: Stop processing of chunk .0.. = Bit: Do not report Parameter length: 8 IP Version 4 address: 192.168.1.83 Supported address types parameter (Supported types: IPv6, IPv4) Parameter type: Supported address types (0x000c) 0... = Bit: Stop processing of chunk .0.. = Bit: Do not report Parameter length: 8 Supported address type: IPv6 address (6) Supported address type: IPv4 address (5) ECN parameter Parameter type: ECN (0x8000) 1... = Bit: Skip parameter and continue processing of the chunk .0.. = Bit: Do not report Parameter length: 4 Forward TSN supported parameter Parameter type: Forward TSN supported (0xc000) 1... = Bit: Skip parameter and continue processing of the chunk .1.. = Bit: Do report
Re: Duplicate inode number when mount --bind some directories to same mountpoint. (from Fedora18 to 4.10-rc3)
On Fri, Jan 13, 2017 at 10:40:08AM +0900, Nakajima Akira wrote: > On 2017/01/12 19:24, Al Viro wrote: > > On Thu, Jan 12, 2017 at 06:16:35PM +0900, Nakajima Akira wrote: > > > Bug: > > > Duplicate inode number when mount --bind some directories to same > > > mountpoint. (from Fedora18 to 4.10-rc3) > > > Fedora17 and earlier works correctly. > > > > Explain, please. "Duplicate inode number" between what and what? > > Duplicate inode number between mounted directories. > > Example) > # cd /home > # mkdir a b > # ls -i > 100 a 999 b > # mount --bind a /mnt > # mount --bind b /mnt > # ls -i > 999 a 999 b > > Inode number of directory "a" is changed to "b". > Then we see directory "b" when ls "a". 61 0 252:1 / / rw,relatime shared:1 - ext4 /dev/vda1 rw,data=ordered Root, marked shared (peer group 1). /home is not a mountpoint, /mnt wasn't one until your mounts (i.e. both are within the same mount as /). Since /home/a is a subtree of a shared mount, any clone of it will, by default, join the same peer group. Which means that binding it on /mnt results in 116 61 252:1 /home/a /mnt rw,relatime shared:1 - ext4 /dev/vda1 rw,data=ordered i.e. ext4[vda1]home/a being mounted on /mnt and marked peer of root mount. Accordingly, any mount/umount event in either will be duplicated to all peers (provided that they contain a counterpart of affected mountpoint). In particular, binding /home/b on /mnt (i.e. on top of ext4[vda1]home/mnt) propagates to the corresponding points in all peers - including the root mount, where it corresponds to /home/a. Result: 120 116 252:1 /home/b /mnt rw,relatime shared:1 - ext4 /dev/vda1 rw,data=ordered 121 61 252:1 /home/b /home/a rw,relatime shared:1 - ext4 /dev/vda1 rw,data=ordered The same tree (ext4[vda1]home/b) is mounted on root in mount 116 (i.e. the thing found on /mnt) and on /home/a in mount 61 (i.e. /home/a). Since /home/b is on a shared mount, both clones are put in the same peer group (i.e. the same group 1). You asked for it, you've got it... Well, fedora folks did, actually. I'm none too fond of their default setup (root made shared), but that has nothing to do with the kernel. Userland (systemd, as far as I can tell) is setting the things up that way, and it's even documented in fedora release notes... Kernel mechanisms involved in that had been there for a long time and they are also documented (man 2 mount, look for MS_SHARED and related flags in there). Take it up with fedora folks...
Re: lkp: make.cross: old aarch64-gcc has been removed
On Thu, 12 Jan 2017 11:31:57 +0800 Fengguang Wu wrote: > Hi Masami, > > On Wed, Jan 11, 2017 at 07:08:40PM +0900, Masami Hiramatsu wrote: > >Ping? > > Oops, sorry for overlooking your previous email! > > >BTW, I found that the old toolchain is archived in release.linaro.org. > >So I made a patch for that. > > So there are gcc 4.9, 5, 6 versions available for aarch64. Since the > build robot nowadays uses debian's packaged gcc-6-aarch64-linux-gnu, > it should be better to use gcc-6 in make.coss to better reproduce > reported regressions: OK, that'll be good too. Would it use debian cross-arch gcc for other architectures too? I would like to make similar environment for build test internally. Thank you, > > http://releases.linaro.org/components/toolchain/binaries/latest/aarch64-linux-gnu/gcc-linaro-6.2.1-2016.11-x86_64_aarch64-linux-gnu.tar.xz > > There are some other files there and I'm not quite sure if that's the > right URL to use. CC aarch64 maintainers for possible inputs. > > Thanks, > Fengguang > > >On Fri, 30 Dec 2016 07:43:45 +0900 > >Masami Hiramatsu wrote: > > > >> Hi Fengguang, > >> > >> Recently I tried to build a cross-build environment ( > >> https://github.com/mhiramat/linux-cross ) by using your make.cross ( > >> https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross > >> ). > >> And I've found that it failed to setup cross gcc for aarch64, because it > >> is no more provided by linaro. > >> > >> Could you update the url to the newer one from linaro or use crosstool > >> as like as other archs? > >> > >> The latest stable version (gcc-5) can be found here. > >> http://releases.linaro.org/components/toolchain/binaries/latest-5/ > >> > >> There are also gcc-6 series for development, and you can find 4.9 binaries > >> too. But I would like to recommend you to use stable one for testing. > >> > >> Thank you, > >> > >> -- > >> Masami Hiramatsu > > > > > >-- > >Masami Hiramatsu > > -- Masami Hiramatsu
Re: Problem on SCTP
Packet to SERVER (from router to SERVER)[ Source IP: 192.168.206.83, Destination IP: 192.168.206.66. the IPv4 address parameter is 192.168.206.83 and 192.168.1.83 ] No. Time SourceSPort Destination Protocol DPort Length Info DSCP 2 2017-01-06 08:52:49.662321192.168.206.8350001 192.168.206.66SCTP 3868 98 INIT CS0 Frame 2: 98 bytes on wire (784 bits), 98 bytes captured (784 bits) Encapsulation type: Ethernet (1) Arrival Time: Jan 6, 2017 16:52:49.662321000 Malay Peninsula Standard Time [Time shift for this packet: 0.0 seconds] Epoch Time: 1483692769.662321000 seconds [Time delta from previous captured frame: 0.000179000 seconds] [Time delta from previous displayed frame: 0.000179000 seconds] [Time since reference or first frame: 0.000179000 seconds] Frame Number: 2 Frame Length: 98 bytes (784 bits) Capture Length: 98 bytes (784 bits) [Frame is marked: False] [Frame is ignored: False] [Protocols in frame: eth:ethertype:ip:sctp] Ethernet II, Src: Vmware_81:41:6b (00:50:56:81:41:6b), Dst: Vmware_81:a6:a3 (00:50:56:81:a6:a3) Destination: Vmware_81:a6:a3 (00:50:56:81:a6:a3) Address: Vmware_81:a6:a3 (00:50:56:81:a6:a3) ..0. = LG bit: Globally unique address (factory default) ...0 = IG bit: Individual address (unicast) Source: Vmware_81:41:6b (00:50:56:81:41:6b) Address: Vmware_81:41:6b (00:50:56:81:41:6b) ..0. = LG bit: Globally unique address (factory default) ...0 = IG bit: Individual address (unicast) Type: IPv4 (0x0800) Internet Protocol Version 4, Src: 192.168.206.83, Dst: 192.168.206.66 0100 = Version: 4 0101 = Header Length: 20 bytes (5) Differentiated Services Field: 0x02 (DSCP: CS0, ECN: ECT(0)) 00.. = Differentiated Services Codepoint: Default (0) ..10 = Explicit Congestion Notification: ECN-Capable Transport codepoint '10' (2) Total Length: 84 Identification: 0x (0) Flags: 0x02 (Don't Fragment) 0... = Reserved bit: Not set .1.. = Don't fragment: Set ..0. = More fragments: Not set Fragment offset: 0 Time to live: 63 Protocol: SCTP (132) Header checksum: 0x1d3d [validation disabled] [Good: False] [Bad: False] Source: 192.168.206.83 Destination: 192.168.206.66 [Source GeoIP: Unknown] [Destination GeoIP: Unknown] Stream Control Transmission Protocol, Src Port: 50001 (50001), Dst Port: 3868 (3868) Source port: 50001 Destination port: 3868 Verification tag: 0x [Assocation index: 0] Checksum: 0xa9a86d3f (not verified) On Fri, Jan 13, 2017 at 11:27 AM, Sun Paul wrote: > Hi All > > below is the packet trace in text format. > > > Packet received (From client to router) [ Source IP: 192.168.206.83, > Destination IP: 192.168.206.65. the IPv4 address parameter is > 192.168.206.83 and 192.168.1.83 ] > == > > No. Time SourceSPort > Destination Protocol DPort Length Info > DSCP > 1 2017-01-06 08:52:49.662142192.168.206.8350001 > 192.168.206.65SCTP 3868 98 INIT > CS0 > > Frame 1: 98 bytes on wire (784 bits), 98 bytes captured (784 bits) > Encapsulation type: Ethernet (1) > Arrival Time: Jan 6, 2017 16:52:49.662142000 Malay Peninsula Standard > Time > [Time shift for this packet: 0.0 seconds] > Epoch Time: 1483692769.662142000 seconds > [Time delta from previous captured frame: 0.0 seconds] > [Time delta from previous displayed frame: 0.0 seconds] > [Time since reference or first frame: 0.0 seconds] > Frame Number: 1 > Frame Length: 98 bytes (784 bits) > Capture Length: 98 bytes (784 bits) > [Frame is marked: False] > [Frame is ignored: False] > [Protocols in frame: eth:ethertype:ip:sctp] > Ethernet II, Src: RealtekU_54:81:87 (52:54:00:54:81:87), Dst: > Vmware_81:41:6b (00:50:56:81:41:6b) > Destination: Vmware_81:41:6b (00:50:56:81:41:6b) > Address: Vmware_81:41:6b (00:50:56:81:41:6b) > ..0. = LG bit: Globally unique > address (factory default) > ...0 = IG bit: Individual address (unicast) > Source: RealtekU_54:81:87 (52:54:00:54:81:87) > Address: RealtekU_54:81:87 (52:54:00:54:81:87) > ..1. = LG bit: Locally administered > address (this is NOT the factory default) > ...0 = IG bit: Individual address (unicast) > Typ
Re: Problem on SCTP
Packet to SERVER (From router to SERVER) [ Source IP: 192.168.206.83, Destination IP: 192.168.206.66. the IPv4 address parameter is 192.168.206.83 and 192.168.1.83 ] No. Time SourceSPort Destination Protocol DPort Length Info DSCP 2 2017-01-06 08:52:49.662321192.168.206.8350001 192.168.206.66SCTP 3868 98 INIT CS0 Frame 2: 98 bytes on wire (784 bits), 98 bytes captured (784 bits) Encapsulation type: Ethernet (1) Arrival Time: Jan 6, 2017 16:52:49.662321000 Malay Peninsula Standard Time [Time shift for this packet: 0.0 seconds] Epoch Time: 1483692769.662321000 seconds [Time delta from previous captured frame: 0.000179000 seconds] [Time delta from previous displayed frame: 0.000179000 seconds] [Time since reference or first frame: 0.000179000 seconds] Frame Number: 2 Frame Length: 98 bytes (784 bits) Capture Length: 98 bytes (784 bits) [Frame is marked: False] [Frame is ignored: False] [Protocols in frame: eth:ethertype:ip:sctp] Ethernet II, Src: Vmware_81:41:6b (00:50:56:81:41:6b), Dst: Vmware_81:a6:a3 (00:50:56:81:a6:a3) Destination: Vmware_81:a6:a3 (00:50:56:81:a6:a3) Address: Vmware_81:a6:a3 (00:50:56:81:a6:a3) ..0. = LG bit: Globally unique address (factory default) ...0 = IG bit: Individual address (unicast) Source: Vmware_81:41:6b (00:50:56:81:41:6b) Address: Vmware_81:41:6b (00:50:56:81:41:6b) ..0. = LG bit: Globally unique address (factory default) ...0 = IG bit: Individual address (unicast) Type: IPv4 (0x0800) Internet Protocol Version 4, Src: 192.168.206.83, Dst: 192.168.206.66 0100 = Version: 4 0101 = Header Length: 20 bytes (5) Differentiated Services Field: 0x02 (DSCP: CS0, ECN: ECT(0)) 00.. = Differentiated Services Codepoint: Default (0) ..10 = Explicit Congestion Notification: ECN-Capable Transport codepoint '10' (2) Total Length: 84 Identification: 0x (0) Flags: 0x02 (Don't Fragment) 0... = Reserved bit: Not set .1.. = Don't fragment: Set ..0. = More fragments: Not set Fragment offset: 0 Time to live: 63 Protocol: SCTP (132) Header checksum: 0x1d3d [validation disabled] [Good: False] [Bad: False] Source: 192.168.206.83 Destination: 192.168.206.66 [Source GeoIP: Unknown] [Destination GeoIP: Unknown] Stream Control Transmission Protocol, Src Port: 50001 (50001), Dst Port: 3868 (3868) Source port: 50001 Destination port: 3868 Verification tag: 0x [Assocation index: 0] Checksum: 0xa9a86d3f (not verified) INIT chunk (Outbound streams: 3000, inbound streams: 3000) Chunk type: INIT (1) 0... = Bit: Stop processing of the packet .0.. = Bit: Do not report Chunk flags: 0x00 Chunk length: 52 Initiate tag: 0xe79f40cb Advertised receiver window credit (a_rwnd): 62464 Number of outbound streams: 3000 Number of inbound streams: 3000 Initial TSN: 176990880 IPv4 address parameter (Address: 192.168.206.83) Parameter type: IPv4 address (0x0005) 0... = Bit: Stop processing of chunk .0.. = Bit: Do not report Parameter length: 8 IP Version 4 address: 192.168.206.83 IPv4 address parameter (Address: 192.168.1.83) Parameter type: IPv4 address (0x0005) 0... = Bit: Stop processing of chunk .0.. = Bit: Do not report Parameter length: 8 IP Version 4 address: 192.168.1.83 Supported address types parameter (Supported types: IPv6, IPv4) Parameter type: Supported address types (0x000c) 0... = Bit: Stop processing of chunk .0.. = Bit: Do not report Parameter length: 8 Supported address type: IPv6 address (6) Supported address type: IPv4 address (5) ECN parameter Parameter type: ECN (0x8000) 1... = Bit: Skip parameter and continue processing of the chunk .0.. = Bit: Do not report Parameter length: 4 Forward TSN supported parameter Parameter type: Forward TSN supported (0xc000) 1... = Bit: Skip parameter and continue processing of the chunk .1.. = Bit: Do report Parameter length: 4 On Fri, Jan 13, 2017 at 11:27 AM, Sun Paul wrote: > Hi All
Re: kvm: use-after-free in process_srcu
I'm not that familiar with the kernel's workqueues, but this seems like the classic "callback outlives the memory it references" use-after-free, where the process_srcu callback is outliving struct kvm (which contains the srcu_struct). If that's right, then calling srcu_barrier (which should wait for all of the call_srcu callbacks to complete, which are what enqueue the process_srcu callbacks) before cleanup_srcu_struct in kvm_destroy_vm probably fixes this. The corresponding patch to virt/kvm/kvm_main.c looks something like: static void kvm_destroy_vm(struct kvm *kvm) { ... for (i = 0; i < KVM_ADDRESS_SPACE_NUM; i++) kvm_free_memslots(kvm, kvm->memslots[i]); + srcu_barrier(&kvm->irq_srcu); cleanup_srcu_struct(&kvm->irq_srcu); + srcu_barrier(&kvm->srcu); cleanup_srcu_struct(&kvm->srcu); ... Since we don't have a repro, this obviously won't be readily testable. I find srcu subtle enough that I don't trust my reasoning fully (in particular, I don't trust that waiting for all of the call_srcu callbacks to complete also waits for all of the process_srcu callbacks). Someone else know if that's the case? Steve On Sun, Dec 11, 2016 at 12:49 AM, Dmitry Vyukov wrote: > On Sun, Dec 11, 2016 at 9:40 AM, Vegard Nossum > wrote: >> On 11 December 2016 at 07:46, Dmitry Vyukov wrote: >>> Hello, >>> >>> I am getting the following use-after-free reports while running >>> syzkaller fuzzer. >>> On commit 318c8932ddec5c1c26a4af0f3c053784841c598e (Dec 7). >>> Unfortunately it is not reproducible, but all reports look sane and >>> very similar, so I would assume that it is some hard to trigger race. >>> In all cases the use-after-free offset within struct kvm is 344 bytes. >>> This points to srcu field, which starts at 208 with size 360 (I have >>> some debug configs enabled). >> [...] >>> [ 376.024345] [] __fput+0x34e/0x910 fs/file_table.c:208 >>> [ 376.024345] [] fput+0x1a/0x20 fs/file_table.c:244 >> >> I've been hitting what I think is a struct file refcounting bug which >> causes similar symptoms as you have here (the struct file is freed >> while somebody still has an active reference to it). >> >>> [ 376.024345] [] task_work_run+0x1a0/0x280 >>> kernel/task_work.c:116 >>> [ 376.024345] [< inline >] exit_task_work >>> include/linux/task_work.h:21 >>> [ 376.024345] [] do_exit+0x1842/0x2650 kernel/exit.c:828 >>> [ 376.024345] [] do_group_exit+0x14e/0x420 >>> kernel/exit.c:932 >>> [ 376.024345] [] get_signal+0x663/0x1880 >>> kernel/signal.c:2307 >>> [ 376.024345] [] do_signal+0xc5/0x2190 >>> arch/x86/kernel/signal.c:807 >> >> Was this or any other process by any chance killed by the OOM killer? >> That seems to be a pattern in the crashes I've seen. If not, do you >> know what killed this process? > > > Difficult to say as I can't reproduce them. > I've looked at the logs I have and there are no OOM kills, only some > kvm-related messages: > > [ 372.188708] kvm [12528]: vcpu0, guest rIP: 0xfff0 > kvm_set_msr_common: MSR_IA32_DEBUGCTLMSR 0x2, nop > [ 372.321334] kvm [12528]: vcpu0, guest rIP: 0xfff0 unhandled wrmsr: > 0x0 data 0x0 > [ 372.426831] kvm [12593]: vcpu512, guest rIP: 0xfff0 unhandled > wrmsr: 0x5 data 0x200 > [ 372.646417] irq bypass consumer (token 880052f74780) > registration fails: -16 > [ 373.001273] pit: kvm: requested 1676 ns i8254 timer period limited > to 50 ns > [ 375.541449] kvm [13011]: vcpu0, guest rIP: 0x11 unhandled > wrmsr: 0x0 data 0x2 > [ 376.005387] > == > [ 376.024345] BUG: KASAN: use-after-free in process_srcu+0x27a/0x280 > at addr 88005e29a418 > > [ 720.214985] kvm: vcpu 0: requested 244148 ns lapic timer period > limited to 50 ns > [ 720.271334] kvm: vcpu 0: requested 244148 ns lapic timer period > limited to 50 ns > [ 720.567985] kvm_vm_ioctl_assign_device: host device not found > [ 721.094589] kvm [22114]: vcpu0, guest rIP: 0x2 unhandled wrmsr: 0x6 data > 0x8 > [ 723.829467] > == > [ 723.829467] BUG: KASAN: use-after-free in process_srcu+0x27a/0x280 > at addr 88005a4d10d8 > > Logs capture ~3-4 seconds before the crash. > However, syzkaller test processes tend to consume lots of memory from > time to time and cause low memory conditions. > > Kills are usually caused by my test driver that kills test processes > after short time. > > However, I do see other assorted bugs caused by kvm that are induced > by OOM kills: > https://groups.google.com/d/msg/syzkaller/ytVPh93HLnI/KhZdengZBwAJ
Re: [PATCH v6 23/25] usb: chipidea: Pullup D+ in device mode via phy APIs
On Thu, Jan 12, 2017 at 02:49:51PM -0800, Stephen Boyd wrote: > Quoting Peter Chen (2017-01-12 01:50:40) > > On Wed, Jan 11, 2017 at 04:19:53PM -0800, Stephen Boyd wrote: > > > Quoting Peter Chen (2017-01-02 22:53:19) > > > > On Wed, Dec 28, 2016 at 02:57:09PM -0800, Stephen Boyd wrote: > > > > > If the phy supports it, call phy_set_mode() to pull up D+ when > > > > > required by setting the mode to PHY_MODE_USB_DEVICE. If we want > > > > > to remove the pullup, set the mode to PHY_MODE_USB_HOST. > > > > > > > > [..] > > > > > diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c > > > > > index 0d532a724d48..6d61fa0689b0 100644 > > > > > --- a/drivers/usb/chipidea/udc.c > > > > > +++ b/drivers/usb/chipidea/udc.c > > > > > @@ -1609,10 +1610,15 @@ static int ci_udc_pullup(struct usb_gadget > > > > > *_gadget, int is_on) > > > > > return 0; > > > > > > > > > > pm_runtime_get_sync(&ci->gadget.dev); > > > > > - if (is_on) > > > > > + if (is_on) { > > > > > + if (ci->phy) > > > > > + phy_set_mode(ci->phy, PHY_MODE_USB_DEVICE); > > > > > hw_write(ci, OP_USBCMD, USBCMD_RS, USBCMD_RS); > > > > > - else > > > > > + } else { > > > > > hw_write(ci, OP_USBCMD, USBCMD_RS, 0); > > > > > + if (ci->phy) > > > > > + phy_set_mode(ci->phy, PHY_MODE_USB_HOST); > > > > > + } > > > > > pm_runtime_put_sync(&ci->gadget.dev); > > > > > > > > > > return 0; > > > > > > > > Would you describe the use case for it? Why not adding it at > > > > role switch routine? > > > > > > > > > > This is about pulling up D+. The phy I have requires that we manually > > > pull up D+ by writing a ULPI register before we set the run/stop bit. > > > > Afaik, only controller can pull up dp when it is at device mode by > > setting USBCMD_RS. At host mode, clear USBCMD_RS will only stopping > > sending SoF from controller side. > > > > I am puzzled why you can pull up D+ by writing an ULPI register, perhaps, > > your phy needs DP to change before switching the mode? Would you > > double confirm that? > > With the boards I have, vbus is not routed to the phy. Instead, there's > a vbus comparator on the PMIC where the vbus line from the usb > receptacle is sent. The vbus extcon driver probes the comparator on the > PMIC to see if vbus is present or not and then notifies extcon users > when vbus changes. > > The ULPI register we write in the phy is a vendor specific register > (called MISC_A) that has two bits. If you look at > qcom_usb_hs_phy_set_mode() in this series you'll see that we set > VBUSVLDEXTSEL and VBUSVLDEXT. VBUSVLDEXTSEL controls a mux in the phy > that chooses between an internal comparator, in the case where vbus goes > to the phy, or an external signal input to the phy, VBUSVLDEXT, to > consider as the "session valid" signal. It looks like the session valid > signal drives the D+ pullup resistor in the phy. These bits in MISC_A > don't matter when the phy is in host mode. > > So when the board doesn't route vbus to the phy, we have to toggle the > VBUSVLDEXT bit to signal to the phy that the vbus is there or not. I > also see that we're not supposed to toggle the VBUSVLDEXTSEL bit when in > "normal" operating mode. So perhaps we should do everything in the > qcom_usb_hs_phy_set_mode() routine during the role switch as you > suggest, except toggle the VBUSVLDEXT bit. Toggling the VBUSVLDEXT bit > can be done via some new phy op when the extcon triggers? Why not call phy_set_mode(phy, DEVICE) directly at ci_handle_vbus_change when you get extcon vbus event? -- Best Regards, Peter Chen
Re: linux-next: build failure after merge of the akpm-current tree
Hi Andrew, On Thu, 12 Jan 2017 15:33:53 -0800 Andrew Morton wrote: > > I'll drop > > ocfs2-dlmglue-prepare-tracking-logic-to-avoid-recursive-cluster-lock.patch > and > ocfs2-fix-deadlocks-when-taking-inode-lock-at-vfs-entry-points.patch I removed them from linux-next today. -- Cheers, Stephen Rothwell
[PATCH] libfc: Fix variable name in fc_set_wwpn
The parameter name should be wwpn instead of wwnn. Signed-off-by: Fam Zheng --- include/scsi/libfc.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/scsi/libfc.h b/include/scsi/libfc.h index 96dd0b3..da5033d 100644 --- a/include/scsi/libfc.h +++ b/include/scsi/libfc.h @@ -809,11 +809,11 @@ static inline void fc_set_wwnn(struct fc_lport *lport, u64 wwnn) /** * fc_set_wwpn() - Set the World Wide Port Name of a local port * @lport: The local port whose WWPN is to be set - * @wwnn: The new WWPN + * @wwpn: The new WWPN */ -static inline void fc_set_wwpn(struct fc_lport *lport, u64 wwnn) +static inline void fc_set_wwpn(struct fc_lport *lport, u64 wwpn) { - lport->wwpn = wwnn; + lport->wwpn = wwpn; } /** -- 2.9.3
Re: linux-next: build failure after merge of the akpm-current tree
Hi Eric, On Thu, 12 Jan 2017 13:06:01 +0800 Eric Ren wrote: > > Thanks for your report and the fix for it. The 0-day project has reported > several days ago, > but this patch set is still in discussion, so I am waiting for more days to > see if other > developers > have any other questions. > > I am confused that how to deal with your patch if I need to work out the V2 > patch set. Perhaps, > pick up your fix and add your efforts in the change log? If you had already fixed the problem, then just submit your new version. You only need to give credit when you use someone's work. If you want to give credit, then maybe a line like: [s...@canb.auug.org.au remove some inlines] among the Signed-off-by: lines -- Cheers, Stephen Rothwell
Re: [RFC PATCH] ext4: increase the protection of drop nlink and ext4 inode destroy
On Thu, Jan 12, 2017 at 12:03:28PM -0500, Theodore Ts'o wrote: > On Thu, Jan 12, 2017 at 04:00:16PM +0800, zhangyi (F) wrote: > > > > At the same time, I think other file systems may have the same problem, do > > you think we should put these detections on the VFS layer? Thus other file > > systems no need to do the same things, but the disadvantage is that we can > > not call ext4_error to report ext4 inconsistency. > > There are file systems which don't have inodes per-se where the > i_nlinks could be a something which is simulated by the file system. > So it's not *necessarily* an on-disk inconsistency. > > We'll have to see if Al and other file system developers are > agreeable, but one thing that we could do is to do the detection in > the VFS layer (which it is actually easier to do), and if they find an > issue, they can just pass a report via a callback function found in > the struct_operations structure. If there isn't such a function > defined, or the function returns 0, the VFS could just do nothing; if > it returns an error code, then that would get reflected back up to > userspace, plus whatever other action the file system sees fit to do. Detection of what? Zero ->i_nlink on inode of dentry that passes e.g. may_delete()?
Re: [PATCH 2/3] gpio: davinci: Redesign driver to accommodate ngpios in one gpio chip
On Friday 13 January 2017 12:36 AM, Grygorii Strashko wrote: On 01/11/2017 08:00 PM, Keerthy wrote: On Wednesday 11 January 2017 11:23 PM, Grygorii Strashko wrote: On 01/10/2017 11:00 PM, Keerthy wrote: The Davinci GPIO driver is implemented to work with one monolithic Davinci GPIO platform device which may have up to Y(144) gpios. The Davinci GPIO driver instantiates number of GPIO chips with max 32 gpio pins per each during initialization and one IRQ domain. So, the current GPIO's opjects structure is: Davinci GPIO controller |- --| ...|--- irq_domain (hwirq [0..143]) |- --| Current driver creates one chip for every 32 GPIOs in a controller. This was a limitation earlier now there is no need for that. Hence redesigning the driver to create one gpio chip for all the ngpio in the controller. |- --|--- irq_domain (hwirq [0..143]). The previous discussion on this can be found here: https://www.spinics.net/lists/linux-omap/msg132869.html nice rework. Thanks! Signed-off-by: Keerthy --- Boot tested on Davinci platform. drivers/gpio/gpio-davinci.c| 127 + [...] #ifdef CONFIG_OF_GPIO -chips[i].chip.of_gpio_n_cells = 2; -chips[i].chip.of_xlate = davinci_gpio_of_xlate; -chips[i].chip.parent = dev; -chips[i].chip.of_node = dev->of_node; +chips->chip.of_gpio_n_cells = 2; +chips->chip.of_xlate = davinci_gpio_of_xlate; I think It's not necessary to have custom .xlate() and it can be removed, then gpiolib will assign default one of_gpio_simple_xlate(). Okay. Can i do that as a separate patch? I think it's ok. +chips->chip.parent = dev; +chips->chip.of_node = dev->of_node; #endif -spin_lock_init(&chips[i].lock); - [...] irq_set_chip_and_handler_name(irq, &gpio_irqchip, handle_simple_irq, "davinci_gpio"); @@ -459,6 +468,7 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev) struct irq_domain*irq_domain = NULL; const struct of_device_id *match; struct irq_chip *irq_chip; +struct davinci_gpio_irq_data *irqdata[MAX_BANKED_IRQS]; You declare irqdata as array here but it has not been used anywhere except for assignment. Could you remove this array and MAX_BANKED_IRQS define? irq_set_chained_handler_and_data(bank_irq, gpio_irq_handler, &chips[gpio / 32]); irqdata[bank]); That is hooked as data for each bank. As there is only one controller now and the differentiating parameters per bank is the irqdata data structure with the registers pointer and the bank number. This structure makes it very easy in the irq handler to identify the register sets that need to be modified and the bank irqs. That I understood, but why do you need array here? Seems you can just use struct davinci_gpio_irq_data *irqdata; why can't you use (see below): struct davinci_gpio_irq_data *irqdata; Understood your comment now thanks for clarifying. I will do like you have suggested. gpio_get_irq_chip_cb_t gpio_get_irq_chip; /* @@ -514,10 +524,8 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev) * IRQs, while the others use banked IRQs, would need some setup * tweaks to recognize hardware which can do that. */ [...] @@ -567,8 +575,19 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev) * gpio irqs. Pass the irq bank's corresponding controller to * the chained irq handler. */ +irqdata[bank] = devm_kzalloc(&pdev->dev, + sizeof(struct +davinci_gpio_irq_data), + GFP_KERNEL); irqdata = devm_kzalloc(&pdev->dev, sizeof(struct davinci_gpio_irq_data), GFP_KERNEL); Okay. +if (!irqdata[bank]) +return -ENOMEM; + +irqdata[bank]->regs = g; +irqdata[bank]->bank_num = bank; +irqdata[bank]->chip = chips; + irq_set_chained_handler_and_data(bank_irq, gpio_irq_handler, - &chips[gpio / 32]); + irqdata[bank]); irq_set_chained_handler_and_data(bank_irq, gpio_irq_handler, irqdata); I will post the new set with the above changes. [...]
Re: [PATCH v5 2/5] powernv:stop: Uniformly rename power9 to arch300
On Thu, Jan 12, 2017 at 03:17:33PM +0530, Balbir Singh wrote: > On Tue, Jan 10, 2017 at 02:37:01PM +0530, Gautham R. Shenoy wrote: > > From: "Gautham R. Shenoy" > > > > Balbir pointed out that in idle_book3s.S and powernv/idle.c some > > functions and variables had power9 in their names while some others > > had arch300. > > > > I would prefer power9 to arch300 > I don't have a strong preference for arch300 vs power9, will change it to power9 if that looks better. -- Thanks and Regards gautham.