Re: [PATCH v2 3/4] power: bq25890_charger.c: Add the BQ25896 part
On 25 July 2018 at 21:46, Angus Ainslie (Purism) wrote: > The BQ25896 is almost identical the the BQ25890 Please use full stop on sentences. This patch (strip from other addons) nicely shows that driver does not need to do anything more to support BQ25896. These two devices are just compatible. You do not change behavior based on matching to compatible, Therefore you should not add new compatible but use old/existing one. If you wish to notify users that they should use this driver, mention in bindings that "ti,bq25896" compatible is for BQ2589x (BQ25890. BQ25896) devices. Best regards, Krzysztof > > Signed-off-by: Angus Ainslie (Purism) > --- > .../bindings/power/supply/bq25890.txt | 1 + > drivers/power/supply/bq25890_charger.c| 25 +++ > 2 files changed, 21 insertions(+), 5 deletions(-) > > diff --git a/Documentation/devicetree/bindings/power/supply/bq25890.txt > b/Documentation/devicetree/bindings/power/supply/bq25890.txt > index c9dd17d142ad..e98312fe6e26 100644 > --- a/Documentation/devicetree/bindings/power/supply/bq25890.txt > +++ b/Documentation/devicetree/bindings/power/supply/bq25890.txt > @@ -3,6 +3,7 @@ Binding for TI bq25890 Li-Ion Charger > Required properties: > - compatible: Should contain one of the following: > * "ti,bq25890" > +* "ti,bq25896" > - reg: integer, i2c address of the device. > - ti,battery-regulation-voltage: integer, maximum charging voltage (in uV); > - ti,charge-current: integer, maximum charging current (in uA); > diff --git a/drivers/power/supply/bq25890_charger.c > b/drivers/power/supply/bq25890_charger.c > index 641f7d779e2f..f4cf2987996b 100644 > --- a/drivers/power/supply/bq25890_charger.c > +++ b/drivers/power/supply/bq25890_charger.c > @@ -32,6 +32,7 @@ > #define BQ25890_IRQ_PIN"bq25890_irq" > > #define BQ25890_ID 3 > +#define BQ25896_ID 0 > > enum bq25890_fields { > F_EN_HIZ, F_EN_ILIM, F_IILIM,/* Reg00 > */ > @@ -153,8 +154,8 @@ static const struct reg_field bq25890_reg_fields[] = { > [F_CONV_RATE] = REG_FIELD(0x02, 6, 6), > [F_BOOSTF] = REG_FIELD(0x02, 5, 5), > [F_ICO_EN] = REG_FIELD(0x02, 4, 4), > - [F_HVDCP_EN]= REG_FIELD(0x02, 3, 3), > - [F_MAXC_EN] = REG_FIELD(0x02, 2, 2), > + [F_HVDCP_EN]= REG_FIELD(0x02, 3, 3), // reserved on > BQ25896 > + [F_MAXC_EN] = REG_FIELD(0x02, 2, 2), // reserved on > BQ25896 > [F_FORCE_DPM] = REG_FIELD(0x02, 1, 1), > [F_AUTO_DPDM_EN]= REG_FIELD(0x02, 0, 0), > /* REG03 */ > @@ -163,6 +164,7 @@ static const struct reg_field bq25890_reg_fields[] = { > [F_OTG_CFG] = REG_FIELD(0x03, 5, 5), > [F_CHG_CFG] = REG_FIELD(0x03, 4, 4), > [F_SYSVMIN] = REG_FIELD(0x03, 1, 3), > + /* MIN_VBAT_SEL on BQ25896 */ > /* REG04 */ > [F_PUMPX_EN]= REG_FIELD(0x04, 7, 7), > [F_ICHG]= REG_FIELD(0x04, 0, 6), > @@ -181,7 +183,7 @@ static const struct reg_field bq25890_reg_fields[] = { > [F_CHG_TMR] = REG_FIELD(0x07, 1, 2), > [F_JEITA_ISET] = REG_FIELD(0x07, 0, 0), > /* REG08 */ > - [F_BATCMP] = REG_FIELD(0x08, 6, 7), > + [F_BATCMP] = REG_FIELD(0x08, 6, 7), // 5-7 on BQ25896 > [F_VCLAMP] = REG_FIELD(0x08, 2, 4), > [F_TREG]= REG_FIELD(0x08, 0, 1), > /* REG09 */ > @@ -195,12 +197,13 @@ static const struct reg_field bq25890_reg_fields[] = { > [F_PUMPX_DN]= REG_FIELD(0x09, 0, 0), > /* REG0A */ > [F_BOOSTV] = REG_FIELD(0x0A, 4, 7), > + /* PFM_OTG_DIS 3 on BQ25896 */ > [F_BOOSTI] = REG_FIELD(0x0A, 0, 2), > /* REG0B */ > [F_VBUS_STAT] = REG_FIELD(0x0B, 5, 7), > [F_CHG_STAT]= REG_FIELD(0x0B, 3, 4), > [F_PG_STAT] = REG_FIELD(0x0B, 2, 2), > - [F_SDP_STAT]= REG_FIELD(0x0B, 1, 1), > + [F_SDP_STAT]= REG_FIELD(0x0B, 1, 1), // reserved on > BQ25896 > [F_VSYS_STAT] = REG_FIELD(0x0B, 0, 0), > /* REG0C */ > [F_WD_FAULT]= REG_FIELD(0x0C, 7, 7), > @@ -399,6 +402,16 @@ static int bq25890_power_supply_get_property(struct > power_supply *psy, > val->strval = BQ25890_MANUFACTURER; > break; > > + case POWER_SUPPLY_PROP_MODEL_NAME: > + if (bq->chip_id == BQ25890_ID) > + val->strval = "BQ25890"; > + else if (bq->chip_id == BQ25896_ID) > + val->strval = "BQ25896"; > + else > + val->strval = "UNKNOWN"; > + > + break; > + >
Re: kernel BUG at mm/shmem.c:LINE!
On Tue, 24 Jul 2018, Hugh Dickins wrote: > On Mon, 23 Jul 2018, Matthew Wilcox wrote: > > On Mon, Jul 23, 2018 at 03:42:22PM -0700, Hugh Dickins wrote: > > > On Mon, 23 Jul 2018, Matthew Wilcox wrote: > > > > I figured out a fix and pushed it to the 'ida' branch in > > > > git://git.infradead.org/users/willy/linux-dax.git > > > > > > Great, thanks a lot for sorting that out so quickly. But I've cloned > > > the tree and don't see today's patch, so assume you've folded the fix > > > into an existing commit? If possible, please append the diff of today's > > > fix to this thread so that we can try it out. Or if that's difficult, > > > please at least tell which files were modified, then I can probably > > > work it out from the diff of those files against mmotm. > > > > Sure! It's just this: > > > > diff --git a/lib/xarray.c b/lib/xarray.c > > index 32a9c2a6a9e9..383c410997eb 100644 > > --- a/lib/xarray.c > > +++ b/lib/xarray.c > > @@ -660,6 +660,8 @@ void xas_create_range(struct xa_state *xas) > > unsigned char sibs = xas->xa_sibs; > > > > xas->xa_index |= ((sibs + 1) << shift) - 1; > > + if (!xas_top(xas->xa_node) && xas->xa_node->shift == xas->xa_shift) > > + xas->xa_offset |= sibs; > > xas->xa_shift = 0; > > xas->xa_sibs = 0; > > Yes, that's a big improvement, the huge "cp" is now fine, thank you. > > I've updated my xfstests tree, and tried that on mmotm with this patch. > The few failures are exactly the same as on 4.18-rc6, whether mounting > tmpfs as huge or not. But four of the tests, generic/{340,345,346,354} > crash (oops) on 4.18-rc5-mm1 + your patch above, but pass on 4.18-rc6. Now I've learnt that an oops on 0xffbe points to EEXIST, not to EREMOTE, it's easy: patch below fixes those four xfstests (and no doubt a similar oops I've seen occasionally under swapping load): so gives clean xfstests runs for non-huge and huge tmpfs. I can reproduce a kernel BUG at mm/khugepaged.c:1358! - that's the VM_BUG_ON(index != xas.xa_index) in collapse_shmem() - but it will take too long to describe how to reproduce that one, so I'm running it past you just in case you have a quick idea on it, otherwise I'll try harder. I did just try an xas_set(&xas, index) before the loop, in case the xas_create_range(&xas) had interfered with initial state; but if that made any difference at all, it only delayed the crash. Hugh --- mmotm/mm/shmem.c2018-07-20 17:54:42.002805461 -0700 +++ linux/mm/shmem.c2018-07-25 23:32:39.170892551 -0700 @@ -597,8 +597,10 @@ static int shmem_add_to_page_cache(struc void *entry; xas_lock_irq(&xas); entry = xas_find_conflict(&xas); - if (entry != expected) + if (entry != expected) { xas_set_err(&xas, -EEXIST); + goto unlock; + } xas_create_range(&xas); if (xas_error(&xas)) goto unlock;
Re: [PATCH v4 14/35] mtd: rawnand: mtk: convert driver to nand_scan()
On Thu, 2018-07-26 at 08:49 +0200, Miquel Raynal wrote: > Hi xiaolei, > > xiaolei li wrote on Thu, 26 Jul 2018 14:46:29 > +0800: > > > On Thu, 2018-07-26 at 08:14 +0200, Boris Brezillon wrote: > > > On Thu, 26 Jul 2018 14:06:41 +0800 > > > xiaolei li wrote: > > > > > > > On Sat, 2018-07-21 at 19:10 +0200, Boris Brezillon wrote: > > > > > On Fri, 20 Jul 2018 17:15:06 +0200 > > > > > Miquel Raynal wrote: > > > > > > > > > > > Two helpers have been added to the core to make ECC-related > > > > > > configuration between the detection phase and the final NAND scan. > > > > > > Use > > > > > > these hooks and convert the driver to just use nand_scan() instead > > > > > > of > > > > > > both nand_scan_ident() and nand_scan_tail(). > > > > > > > > > > > > Signed-off-by: Miquel Raynal > > > > > > --- > > > > > > drivers/mtd/nand/raw/mtk_nand.c | 75 > > > > > > - > > > > > > 1 file changed, 44 insertions(+), 31 deletions(-) > > > > > > > > > > > > diff --git a/drivers/mtd/nand/raw/mtk_nand.c > > > > > > b/drivers/mtd/nand/raw/mtk_nand.c > > > > > > index 7bc6be3f6ec0..967418f945ea 100644 > > > > > > --- a/drivers/mtd/nand/raw/mtk_nand.c > > > > > > +++ b/drivers/mtd/nand/raw/mtk_nand.c > > > > > > @@ -1250,13 +1250,54 @@ static int mtk_nfc_ecc_init(struct device > > > > > > *dev, struct mtd_info *mtd) > > > > > > return 0; > > > > > > } > > > > > > > > > > > > +static int mtk_nfc_attach_chip(struct nand_chip *chip) > > > > > > +{ > > > > > > + struct mtd_info *mtd = nand_to_mtd(chip); > > > > > > + struct device *dev = mtd->dev.parent; > > > > > > + struct mtk_nfc *nfc = nand_get_controller_data(chip); > > > > > > + struct mtk_nfc_nand_chip *mtk_nand = to_mtk_nand(chip); > > > > > > + int len; > > > > > > + int ret; > > > > > > + > > > > > > + if (chip->options & NAND_BUSWIDTH_16) { > > > > > > + dev_err(dev, "16bits buswidth not supported"); > > > > > > + return -EINVAL; > > > > > > + } > > > > > > + > > > > > > + /* store bbt magic in page, cause OOB is not protected */ > > > > > > + if (chip->bbt_options & NAND_BBT_USE_FLASH) > > > > > > + chip->bbt_options |= NAND_BBT_NO_OOB; > > > > > > + > > > > > > + ret = mtk_nfc_ecc_init(dev, mtd); > > > > > > + if (ret) > > > > > > + return ret; > > > > > > + > > > > > > + ret = mtk_nfc_set_spare_per_sector(&mtk_nand->spare_per_sector, > > > > > > mtd); > > > > > > + if (ret) > > > > > > + return ret; > > > > > > + > > > > > > + mtk_nfc_set_fdm(&mtk_nand->fdm, mtd); > > > > > > + mtk_nfc_set_bad_mark_ctl(&mtk_nand->bad_mark, mtd); > > > > > > + > > > > > > + len = mtd->writesize + mtd->oobsize; > > > > > > + nfc->buffer = devm_kzalloc(dev, len, GFP_KERNEL); > > > > > > + if (!nfc->buffer) > > > > > > + return -ENOMEM; > > > > > > + > > > > > > + return 0; > > > > > > +} > > > > > > + > > > > > > +static const struct nand_controller_ops mtk_nfc_controller_ops = { > > > > > > + .attach_chip = mtk_nfc_attach_chip, > > > > > > +}; > > > > > > + > > > > > > static int mtk_nfc_nand_chip_init(struct device *dev, struct > > > > > > mtk_nfc *nfc, > > > > > > struct device_node *np) > > > > > > { > > > > > > struct mtk_nfc_nand_chip *chip; > > > > > > struct nand_chip *nand; > > > > > > struct mtd_info *mtd; > > > > > > - int nsels, len; > > > > > > + int nsels; > > > > > > u32 tmp; > > > > > > int ret; > > > > > > int i; > > > > > > @@ -1287,6 +1328,7 @@ static int mtk_nfc_nand_chip_init(struct > > > > > > device *dev, struct mtk_nfc *nfc, > > > > > > > > > > > > nand = &chip->nand; > > > > > > nand->controller = &nfc->controller; > > > > > > + nand->controller->ops = &mtk_nfc_controller_ops; > > > > > > > > > > Just like for the marvell driver, this assignment should be moved here > > > > > [1]. > > > > Agree to this. > > > > > > > > > > > > Also, it looks like this driver is open-coding > > > > > nand_controller_init(), probably something we should fix (in a > > > > > separate > > > > > patch). > > > > May I ask if you mean driver should use nand_hw_control_init helper to > > > > do controller_init? > > > > > > Well, now it's named nand_controller_init(), but yes, that's what I > > > meant. > > > > OK. I will fix it base on this patch series later. > > Nice! > > Then you can just wait for the next -rc1, this series should be > available. > OK. Thanks for your improvement! Assuming ->controller->ops assignment will be fixed, Acked-by: Xiaolei Li > Thanks, > Miquèl
Re: cgroups iptables-restor: vmalloc: allocation failure
Hello, Servers are with 64bit kernel and 32bit userland. here is the log: [ 485.467359] iptables-restor: vmalloc: allocation failure, allocated 2367488 of 3608576 bytes, mode:0x14010c0(GFP_KERNEL|__GFP_NORETRY), nodemask=(null) [ 485.467871] iptables-restor cpuset=/ mems_allowed=0 [ 485.468089] CPU: 0 PID: 18093 Comm: iptables-restor Not tainted 4.14.50 #1 [ 485.468399] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014 [ 485.468725] 81ed2b8b 8333a330 bb5cfe6a5a821e59 [ 485.469114] c90004aaba80 811fa3ea c90004aaba90 [ 485.469422] 825e11d0 014010c0 825e11d0 c90004aaba18 [ 485.469735] Call Trace: [ 485.469941] [] dump_stack+0x9a/0x10f [ 485.470159] [] warn_alloc+0x18a/0x270 [ 485.470379] [] __vmalloc_node_range+0x251/0x470 [ 485.470614] [] ? __vmalloc_node_flags_caller+0x3e/0x70 [ 485.470881] [] __vmalloc_node_flags_caller+0x3e/0x70 [ 485.471160] [] ? xt_alloc_table_info+0x4e/0xa0 [ 485.471395] [] kvmalloc_node+0x7a/0xd0 [ 485.471618] [] xt_alloc_table_info+0x4e/0xa0 [ 485.471849] [] ? translate_compat_table.constprop.19+0x447/0x870 [ 485.472197] [] translate_compat_table.constprop.19+0x447/0x870 [ 485.472514] [] ? compat_do_replace.isra.10.constprop.18+0x197/0x370 [ 485.472833] [] compat_do_replace.isra.10.constprop.18+0x197/0x370 [ 485.473230] [] compat_do_ipt_set_ctl+0x8b/0xb0 [ 485.473465] [] compat_nf_setsockopt+0x6a/0xd0 [ 485.473701] [] compat_ip_setsockopt+0x73/0xd0 [ 485.473936] [] compat_raw_setsockopt+0x7c/0xd0 [ 485.474208] [] ? handle_mm_fault+0xec/0x240 [ 485.474441] [] compat_sock_common_setsockopt+0x32/0x80 [ 485.474689] [] compat_sys_setsockopt+0x9a/0x2e0 [ 485.474926] [] compat_sys_socketcall+0x415/0x570 [ 485.475197] [] ? page_fault+0x5c/0x90 [ 485.475416] [] ? retint_user+0x2b/0x2b [ 485.475638] [] ? rap_compat_sys_socketcall+0x14/0x40 [ 485.475884] [] ? do_fast_syscall_32+0xe1/0x2f0 [ 485.476170] [] ? entry_SYSENTER_compat+0x93/0xa5 [ 485.476414] Mem-Info: [ 485.476579] active_anon:79884 inactive_anon:103007 isolated_anon:0 active_file:54010 inactive_file:219014 isolated_file:0 unevictable:1822 dirty:22 writeback:0 unstable:0 slab_reclaimable:8637 slab_unreclaimable:25015 mapped:16094 shmem:2271 pagetables:1481 bounce:0 free:3459 free_pcp:180 free_cma:0 [ 485.477691] Node 0 active_anon:320376kB inactive_anon:412028kB active_file:216040kB inactive_file:875844kB unevictable:7288kB isolated(anon):0kB isolated(file):0kB mapped:64376kB dirty:88kB writeback:0kB shmem:9084kB writeback_tmp:0kB unstable:0kB all_unreclaimable? no [ 485.478457] Node 0 DMA free:7896kB min:44kB low:56kB high:68kB active_anon:2324kB inactive_anon:0kB active_file:12kB inactive_file:5228kB unevictable:0kB writepending:0kB present:15992kB managed:15892kB mlocked:0kB kernel_stack:0kB pagetables:0kB bounce:0kB free_pcp:0kB local_pcp:0kB free_cma:0kB [ 485.479233] lowmem_reserve[]: 0 1963 1963 1963 [ 485.479483] Node 0 DMA32 free:5364kB min:5644kB low:7652kB high:9660kB active_anon:317672kB inactive_anon:412028kB active_file:216028kB inactive_file:871340kB unevictable:7288kB writepending:88kB present:2080624kB managed:2015900kB mlocked:7288kB kernel_stack:5584kB pagetables:5924kB bounce:0kB free_pcp:1052kB local_pcp:720kB free_cma:0kB [ 485.480425] lowmem_reserve[]: 0 0 0 0 [ 485.480612] Node 0 DMA: 38*4kB (UMEH) 54*8kB (MEH) 51*16kB (UMEH) 35*32kB (UMEH) 16*64kB (MEH) 12*128kB (MH) 7*256kB (UMEH) 0*512kB 1*1024kB (E) 0*2048kB 0*4096kB = 7896kB [ 485.481173] Node 0 DMA32: 12*4kB (MH) 16*8kB (H) 264*16kB (UH) 7*32kB (H) 6*64kB (H) 1*128kB (H) 0*256kB 1*512kB (H) 0*1024kB 0*2048kB 0*4096kB = 5648kB [ 485.481642] Node 0 hugepages_total=0 hugepages_free=0 hugepages_surp=0 hugepages_size=1048576kB [ 485.481975] Node 0 hugepages_total=0 hugepages_free=0 hugepages_surp=0 hugepages_size=2048kB [ 485.482309] 275972 total pagecache pages [ 485.482535] 0 pages in swap cache [ 485.482717] Swap cache stats: add 0, delete 0, find 0/0 [ 485.482937] Free swap = 1976316kB [ 485.483118] Total swap = 1976316kB [ 485.483300] 524154 pages RAM [ 485.483492] 0 pages HighMem/MovableOnly [ 485.483711] 16206 pages reserved [ 485.483889] 0 pages hwpoisoned Regards, -- Georgi Nikolov On 07/25/2018 10:42 PM, David Rientjes wrote: > On Wed, 25 Jul 2018, Georgi Nikolov wrote: > >> Hello, >> >> I posted a kernel bug https://bugzilla.kernel.org/show_bug.cgi?id=200651 and >> i hope this is the correct place to discuss this. >> > Could you post the full allocation failure from the kernel log? It's not > possible to vmalloc any additional memory due to the memory pressure, so > this is expected to fail but I'm curious as to the actual allocation that > is triggering it.
[PATCH v7 1/6] dt-bindings: mailbox: allow mbox-cells to be equal to 0
From: Dong Aisheng Mailbox devices may have only one channel which means the mbox-cells at least 1 does not make sense for this type devices. Let's remove that limitation to allow the mbox-cells to be equal to 0. Cc: Mark Rutland Cc: Sudeep Holla Cc: devicet...@vger.kernel.org Cc: linux-kernel@vger.kernel.org Reviewed-by: Rob Herring Signed-off-by: Dong Aisheng --- Documentation/devicetree/bindings/mailbox/mailbox.txt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/mailbox/mailbox.txt b/Documentation/devicetree/bindings/mailbox/mailbox.txt index af8ecee2ac68..c2fcd054141a 100644 --- a/Documentation/devicetree/bindings/mailbox/mailbox.txt +++ b/Documentation/devicetree/bindings/mailbox/mailbox.txt @@ -6,8 +6,7 @@ assign appropriate mailbox channel to client drivers. * Mailbox Controller Required property: -- #mbox-cells: Must be at least 1. Number of cells in a mailbox - specifier. +- #mbox-cells: Number of cells in a mailbox specifier. Example: mailbox: mailbox { -- 2.18.0
[PATCH] arm64: dts: allwinner: activate spi flash on pine64 LTS board
This board has SPI flash. add spi flash support in device tree. Tested on pine64 LTS. Signed-off-by: Akash Gajjar --- .../arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts | 12 1 file changed, 12 insertions(+) diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts index abe179d..9af07db 100644 --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts @@ -134,6 +134,18 @@ regulator-name = "vcc-wifi"; }; +&spi0 { + status = "okay"; + + flash@0 { + #address-cells = <1>; + #size-cells = <1>; + compatible = "winbond,w25q128", "jedec,spi-nor"; + reg = <0x0>; + spi-max-frequency = <4000>; + }; +}; + &uart0 { pinctrl-names = "default"; pinctrl-0 = <&uart0_pins_a>; -- 2.7.4
Re: [PATCH v2 2/4] power: bq25890_charger.c: Remove unused table
On 25 July 2018 at 21:46, Angus Ainslie (Purism) wrote: > The BATCMP table isn't used so drop it TBL_VCLAMP also looks unused. TBL_IPRECHG does not have table entry. Best regards, Krzysztof
Re: [PATCH v4 14/35] mtd: rawnand: mtk: convert driver to nand_scan()
Hi xiaolei, xiaolei li wrote on Thu, 26 Jul 2018 14:46:29 +0800: > On Thu, 2018-07-26 at 08:14 +0200, Boris Brezillon wrote: > > On Thu, 26 Jul 2018 14:06:41 +0800 > > xiaolei li wrote: > > > > > On Sat, 2018-07-21 at 19:10 +0200, Boris Brezillon wrote: > > > > On Fri, 20 Jul 2018 17:15:06 +0200 > > > > Miquel Raynal wrote: > > > > > > > > > Two helpers have been added to the core to make ECC-related > > > > > configuration between the detection phase and the final NAND scan. Use > > > > > these hooks and convert the driver to just use nand_scan() instead of > > > > > both nand_scan_ident() and nand_scan_tail(). > > > > > > > > > > Signed-off-by: Miquel Raynal > > > > > --- > > > > > drivers/mtd/nand/raw/mtk_nand.c | 75 > > > > > - > > > > > 1 file changed, 44 insertions(+), 31 deletions(-) > > > > > > > > > > diff --git a/drivers/mtd/nand/raw/mtk_nand.c > > > > > b/drivers/mtd/nand/raw/mtk_nand.c > > > > > index 7bc6be3f6ec0..967418f945ea 100644 > > > > > --- a/drivers/mtd/nand/raw/mtk_nand.c > > > > > +++ b/drivers/mtd/nand/raw/mtk_nand.c > > > > > @@ -1250,13 +1250,54 @@ static int mtk_nfc_ecc_init(struct device > > > > > *dev, struct mtd_info *mtd) > > > > > return 0; > > > > > } > > > > > > > > > > +static int mtk_nfc_attach_chip(struct nand_chip *chip) > > > > > +{ > > > > > + struct mtd_info *mtd = nand_to_mtd(chip); > > > > > + struct device *dev = mtd->dev.parent; > > > > > + struct mtk_nfc *nfc = nand_get_controller_data(chip); > > > > > + struct mtk_nfc_nand_chip *mtk_nand = to_mtk_nand(chip); > > > > > + int len; > > > > > + int ret; > > > > > + > > > > > + if (chip->options & NAND_BUSWIDTH_16) { > > > > > + dev_err(dev, "16bits buswidth not supported"); > > > > > + return -EINVAL; > > > > > + } > > > > > + > > > > > + /* store bbt magic in page, cause OOB is not protected */ > > > > > + if (chip->bbt_options & NAND_BBT_USE_FLASH) > > > > > + chip->bbt_options |= NAND_BBT_NO_OOB; > > > > > + > > > > > + ret = mtk_nfc_ecc_init(dev, mtd); > > > > > + if (ret) > > > > > + return ret; > > > > > + > > > > > + ret = mtk_nfc_set_spare_per_sector(&mtk_nand->spare_per_sector, > > > > > mtd); > > > > > + if (ret) > > > > > + return ret; > > > > > + > > > > > + mtk_nfc_set_fdm(&mtk_nand->fdm, mtd); > > > > > + mtk_nfc_set_bad_mark_ctl(&mtk_nand->bad_mark, mtd); > > > > > + > > > > > + len = mtd->writesize + mtd->oobsize; > > > > > + nfc->buffer = devm_kzalloc(dev, len, GFP_KERNEL); > > > > > + if (!nfc->buffer) > > > > > + return -ENOMEM; > > > > > + > > > > > + return 0; > > > > > +} > > > > > + > > > > > +static const struct nand_controller_ops mtk_nfc_controller_ops = { > > > > > + .attach_chip = mtk_nfc_attach_chip, > > > > > +}; > > > > > + > > > > > static int mtk_nfc_nand_chip_init(struct device *dev, struct mtk_nfc > > > > > *nfc, > > > > > struct device_node *np) > > > > > { > > > > > struct mtk_nfc_nand_chip *chip; > > > > > struct nand_chip *nand; > > > > > struct mtd_info *mtd; > > > > > - int nsels, len; > > > > > + int nsels; > > > > > u32 tmp; > > > > > int ret; > > > > > int i; > > > > > @@ -1287,6 +1328,7 @@ static int mtk_nfc_nand_chip_init(struct device > > > > > *dev, struct mtk_nfc *nfc, > > > > > > > > > > nand = &chip->nand; > > > > > nand->controller = &nfc->controller; > > > > > + nand->controller->ops = &mtk_nfc_controller_ops; > > > > > > > > Just like for the marvell driver, this assignment should be moved here > > > > [1]. > > > Agree to this. > > > > > > > > > Also, it looks like this driver is open-coding > > > > nand_controller_init(), probably something we should fix (in a separate > > > > patch). > > > May I ask if you mean driver should use nand_hw_control_init helper to > > > do controller_init? > > > > Well, now it's named nand_controller_init(), but yes, that's what I > > meant. > > OK. I will fix it base on this patch series later. Nice! Then you can just wait for the next -rc1, this series should be available. Thanks, Miquèl
Re: [PATCH v4 14/35] mtd: rawnand: mtk: convert driver to nand_scan()
On Thu, 2018-07-26 at 08:14 +0200, Boris Brezillon wrote: > On Thu, 26 Jul 2018 14:06:41 +0800 > xiaolei li wrote: > > > On Sat, 2018-07-21 at 19:10 +0200, Boris Brezillon wrote: > > > On Fri, 20 Jul 2018 17:15:06 +0200 > > > Miquel Raynal wrote: > > > > > > > Two helpers have been added to the core to make ECC-related > > > > configuration between the detection phase and the final NAND scan. Use > > > > these hooks and convert the driver to just use nand_scan() instead of > > > > both nand_scan_ident() and nand_scan_tail(). > > > > > > > > Signed-off-by: Miquel Raynal > > > > --- > > > > drivers/mtd/nand/raw/mtk_nand.c | 75 > > > > - > > > > 1 file changed, 44 insertions(+), 31 deletions(-) > > > > > > > > diff --git a/drivers/mtd/nand/raw/mtk_nand.c > > > > b/drivers/mtd/nand/raw/mtk_nand.c > > > > index 7bc6be3f6ec0..967418f945ea 100644 > > > > --- a/drivers/mtd/nand/raw/mtk_nand.c > > > > +++ b/drivers/mtd/nand/raw/mtk_nand.c > > > > @@ -1250,13 +1250,54 @@ static int mtk_nfc_ecc_init(struct device *dev, > > > > struct mtd_info *mtd) > > > > return 0; > > > > } > > > > > > > > +static int mtk_nfc_attach_chip(struct nand_chip *chip) > > > > +{ > > > > + struct mtd_info *mtd = nand_to_mtd(chip); > > > > + struct device *dev = mtd->dev.parent; > > > > + struct mtk_nfc *nfc = nand_get_controller_data(chip); > > > > + struct mtk_nfc_nand_chip *mtk_nand = to_mtk_nand(chip); > > > > + int len; > > > > + int ret; > > > > + > > > > + if (chip->options & NAND_BUSWIDTH_16) { > > > > + dev_err(dev, "16bits buswidth not supported"); > > > > + return -EINVAL; > > > > + } > > > > + > > > > + /* store bbt magic in page, cause OOB is not protected */ > > > > + if (chip->bbt_options & NAND_BBT_USE_FLASH) > > > > + chip->bbt_options |= NAND_BBT_NO_OOB; > > > > + > > > > + ret = mtk_nfc_ecc_init(dev, mtd); > > > > + if (ret) > > > > + return ret; > > > > + > > > > + ret = mtk_nfc_set_spare_per_sector(&mtk_nand->spare_per_sector, > > > > mtd); > > > > + if (ret) > > > > + return ret; > > > > + > > > > + mtk_nfc_set_fdm(&mtk_nand->fdm, mtd); > > > > + mtk_nfc_set_bad_mark_ctl(&mtk_nand->bad_mark, mtd); > > > > + > > > > + len = mtd->writesize + mtd->oobsize; > > > > + nfc->buffer = devm_kzalloc(dev, len, GFP_KERNEL); > > > > + if (!nfc->buffer) > > > > + return -ENOMEM; > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +static const struct nand_controller_ops mtk_nfc_controller_ops = { > > > > + .attach_chip = mtk_nfc_attach_chip, > > > > +}; > > > > + > > > > static int mtk_nfc_nand_chip_init(struct device *dev, struct mtk_nfc > > > > *nfc, > > > > struct device_node *np) > > > > { > > > > struct mtk_nfc_nand_chip *chip; > > > > struct nand_chip *nand; > > > > struct mtd_info *mtd; > > > > - int nsels, len; > > > > + int nsels; > > > > u32 tmp; > > > > int ret; > > > > int i; > > > > @@ -1287,6 +1328,7 @@ static int mtk_nfc_nand_chip_init(struct device > > > > *dev, struct mtk_nfc *nfc, > > > > > > > > nand = &chip->nand; > > > > nand->controller = &nfc->controller; > > > > + nand->controller->ops = &mtk_nfc_controller_ops; > > > > > > Just like for the marvell driver, this assignment should be moved here > > > [1]. > > Agree to this. > > > > > > Also, it looks like this driver is open-coding > > > nand_controller_init(), probably something we should fix (in a separate > > > patch). > > May I ask if you mean driver should use nand_hw_control_init helper to > > do controller_init? > > Well, now it's named nand_controller_init(), but yes, that's what I > meant. OK. I will fix it base on this patch series later. Thanks!
Re: [PATCH v2 1/4] power: bq25890_charger.c: Add debugging output of failed initialization
On 25 July 2018 at 21:46, Angus Ainslie (Purism) wrote: > To ease adding a new part variant some debugging is handy > > Signed-off-by: Angus Ainslie (Purism) > --- > drivers/power/supply/bq25890_charger.c | 23 ++- > 1 file changed, 18 insertions(+), 5 deletions(-) Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: Showing /sys/fs/cgroup/memory/memory.stat very slow on some machines
On 26 July 2018 at 02:55, Singh, Balbir wrote: > Do you by any chance have use_hierarch=1? memcg_stat_show should just rely on > counters inside the memory cgroup and the the LRU sizes for each node. Yes, /sys/fs/cgroup/memory/memory.use_hierarchy is 1. I assume systemd is doing that. Bruce -- Bruce Merry Senior Science Processing Developer SKA South Africa
Re: [PATCH] bq25890_charger.c : add the BQ25896 part
On 25 July 2018 at 14:17, Angus Ainslie wrote: > Hi Krzysztof, > > On 2018-07-25 03:58, Krzysztof Kozlowski wrote: >> >> On 23 July 2018 at 15:51, Angus Ainslie wrote: >>> >>> Add some debugging to be able to check the proper initialization >>> of the BQ25896 part. >> >> >> Hi, >> >> This should be split into separate patchset. Do not mix two features >> in one commit. >> > > Ok, I'll take it apart > >>> Enable the BQ25896 part. >>> >>> Add 2 new parameters "voltage_now" and "model_name". >>> >>> Signed-off-by: Angus Ainslie >> >> >> Your signed-off-by does not match From address. >> > > That was intentional as I wanted Purism to get credit for it. I'm guessing > that's not the correct way of doing it. The author of patch (appearing as "From") should match Signed-off-by. You might however easily create commit and signed it with your Purism email. If you send such email, you will notice that "From" field is visible in the mail message and it differs from Sender. Best regards, Krzysztof
Re: Zram writeback feature unstable with heavy swap utilization - BUG: Bad page state in process...
On 07/26/2018 08:21 AM, Minchan Kim wrote: That means you could reproduce it without writeback feature? If so, it would be more reasoanble to check [0bcac06f27d75, skip swapcache for swapin of synchronous device] No, the bug only occurs with a backing device. The writeback feature is enabled in the default Ubuntu kernel configuration. -- Kind regards, Tino Lehnig
Re: [PATCH 4.4 15/47] ubi: fastmap: Correctly handle interrupted erasures in EBA
Ben, Am Donnerstag, 26. Juli 2018, 04:12:54 CEST schrieb Ben Hutchings: > On Tue, 2018-07-10 at 20:24 +0200, Greg Kroah-Hartman wrote: > > 4.4-stable review patch. If anyone has any objections, please let me know. > > > > -- > > > > From: Richard Weinberger > > > > commit 781932375ffc6411713ee0926ccae8596ed0261c upstream. > > > > Fastmap cannot track the LEB unmap operation, therefore it can > > happen that after an interrupted erasure the mapping still looks > > good from Fastmap's point of view, while reading from the PEB will > > cause an ECC error and confuses the upper layer. > > > > Instead of teaching users of UBI how to deal with that, we read back > > the VID header and check for errors. If the PEB is empty or shows ECC > > errors we fixup the mapping and schedule the PEB for erasure. > [...] > > Isn't there a risk here, that a read error leads to erasing data that > might be recoverable if the read is retried? Well, read error means that already something went very wrong. At other places in UBI wo also don't retry reading headers and consider it as fatal when we are unable to read it. We could also read the EC header, but what do we gain from that? If the VID header is not readable we cannot check fastmap either. What case exactly do you have in mind? Thanks, //richard
Re: [PATCH 3/4] perf tools: Fix check-headers.sh output file variables
On Tue, Jul 24, 2018 at 08:20:07AM +0100, Alexander Kapshuk wrote: SNIP > > > % file1=file1; file2=file2 > > > % cmd="echo diff $file1 $file2" > > > % test -f $file2 && > > > eval $cmd || echo "Warning: Kernel ABI header at 'tools/$file1' > > > differs from latest version at '$file2'" >&2 > > > Warning: Kernel ABI header at 'tools/file1' differs from latest > > > version at 'file2' > > > > > > Is this something you would rather leave as is, or perhaps use something > > > along the lines of the code below instead: > > > > > > test -f $file2 && { > > > eval $cmd || > > > echo "Warning: Kernel ABI header at 'tools/$file' differs from latest > > > version at '$file'" >&2 > > > } > > > > hi, > > yea, probably.. please feel free to post a patch.. just make sure all > > the displayed files paths are based on the kernel root > > > > thanks, > > jirka > > > > I'm away traveling till August 10th, and I may not be able to send the > patch in until I get back. Is that OK? sure, thanks jirka
Re: Zram writeback feature unstable with heavy swap utilization - BUG: Bad page state in process...
On Thu, Jul 26, 2018 at 08:10:41AM +0200, Tino Lehnig wrote: > Hi, > > On 07/26/2018 04:03 AM, Minchan Kim wrote: > > A thing I could imagine is > > [0bcac06f27d75, skip swapcache for swapin of synchronous device] > > It was merged into v4.15. Could you check it by bisecting? > > Thanks, I will check that. > > > > My operating system is a minimal install of Debian 9. I took the kernel > > > configuration from the default Debian kernel and built my own kernel with > > > "make oldconfig" leaving all settings at their defaults. The only thing I > > > changed in the configuration was enabling the zram writeback feature. > > > > You mean you changed host kernel configuration? > > > > > > > > All my tests were done on bare-metal hardware with Xeon processors and > > > lots > > > of RAM. I encounter the bug quite quickly, but it still takes several GBs > > > of > > > swap usage. Below is my /proc/meminfo with enough KVM instances running (3 > > > in my case) to trigger the bug on my test machine. > > > > Aha.. you did writeback feature into your bare-metal host machine and > > execute > > kvm with window images as a guest. So, PG_uptodate warning happens on host > > side, > > not guest? Right? > > Yes, I am only talking about the host kernel. Zram swap is set up on the > host. I just used Windows guests to fill up the host RAM and force it into > swap. > > > > I will also try to reproduce the problem on some different hardware next. > > Just to confirm, I was able to reproduce the problem on another machine > running Ubuntu 18.04 with the Ubuntu stock kernel (4.15) and no > modifications to the kernel configuration whatsoever. The host had 8 GB of That means you could reproduce it without writeback feature? If so, it would be more reasoanble to check [0bcac06f27d75, skip swapcache for swapin of synchronous device] > RAM, 32 GB of swap with zram and a 32 GB SSD as backing device. I had to > start only one Windows VM with "-m 32768" to trigger the bug. Thanks. I will try it later today.
Re: [PATCH v4 14/35] mtd: rawnand: mtk: convert driver to nand_scan()
On Thu, 26 Jul 2018 14:06:41 +0800 xiaolei li wrote: > On Sat, 2018-07-21 at 19:10 +0200, Boris Brezillon wrote: > > On Fri, 20 Jul 2018 17:15:06 +0200 > > Miquel Raynal wrote: > > > > > Two helpers have been added to the core to make ECC-related > > > configuration between the detection phase and the final NAND scan. Use > > > these hooks and convert the driver to just use nand_scan() instead of > > > both nand_scan_ident() and nand_scan_tail(). > > > > > > Signed-off-by: Miquel Raynal > > > --- > > > drivers/mtd/nand/raw/mtk_nand.c | 75 > > > - > > > 1 file changed, 44 insertions(+), 31 deletions(-) > > > > > > diff --git a/drivers/mtd/nand/raw/mtk_nand.c > > > b/drivers/mtd/nand/raw/mtk_nand.c > > > index 7bc6be3f6ec0..967418f945ea 100644 > > > --- a/drivers/mtd/nand/raw/mtk_nand.c > > > +++ b/drivers/mtd/nand/raw/mtk_nand.c > > > @@ -1250,13 +1250,54 @@ static int mtk_nfc_ecc_init(struct device *dev, > > > struct mtd_info *mtd) > > > return 0; > > > } > > > > > > +static int mtk_nfc_attach_chip(struct nand_chip *chip) > > > +{ > > > + struct mtd_info *mtd = nand_to_mtd(chip); > > > + struct device *dev = mtd->dev.parent; > > > + struct mtk_nfc *nfc = nand_get_controller_data(chip); > > > + struct mtk_nfc_nand_chip *mtk_nand = to_mtk_nand(chip); > > > + int len; > > > + int ret; > > > + > > > + if (chip->options & NAND_BUSWIDTH_16) { > > > + dev_err(dev, "16bits buswidth not supported"); > > > + return -EINVAL; > > > + } > > > + > > > + /* store bbt magic in page, cause OOB is not protected */ > > > + if (chip->bbt_options & NAND_BBT_USE_FLASH) > > > + chip->bbt_options |= NAND_BBT_NO_OOB; > > > + > > > + ret = mtk_nfc_ecc_init(dev, mtd); > > > + if (ret) > > > + return ret; > > > + > > > + ret = mtk_nfc_set_spare_per_sector(&mtk_nand->spare_per_sector, mtd); > > > + if (ret) > > > + return ret; > > > + > > > + mtk_nfc_set_fdm(&mtk_nand->fdm, mtd); > > > + mtk_nfc_set_bad_mark_ctl(&mtk_nand->bad_mark, mtd); > > > + > > > + len = mtd->writesize + mtd->oobsize; > > > + nfc->buffer = devm_kzalloc(dev, len, GFP_KERNEL); > > > + if (!nfc->buffer) > > > + return -ENOMEM; > > > + > > > + return 0; > > > +} > > > + > > > +static const struct nand_controller_ops mtk_nfc_controller_ops = { > > > + .attach_chip = mtk_nfc_attach_chip, > > > +}; > > > + > > > static int mtk_nfc_nand_chip_init(struct device *dev, struct mtk_nfc > > > *nfc, > > > struct device_node *np) > > > { > > > struct mtk_nfc_nand_chip *chip; > > > struct nand_chip *nand; > > > struct mtd_info *mtd; > > > - int nsels, len; > > > + int nsels; > > > u32 tmp; > > > int ret; > > > int i; > > > @@ -1287,6 +1328,7 @@ static int mtk_nfc_nand_chip_init(struct device > > > *dev, struct mtk_nfc *nfc, > > > > > > nand = &chip->nand; > > > nand->controller = &nfc->controller; > > > + nand->controller->ops = &mtk_nfc_controller_ops; > > > > Just like for the marvell driver, this assignment should be moved here > > [1]. > Agree to this. > > > Also, it looks like this driver is open-coding > > nand_controller_init(), probably something we should fix (in a separate > > patch). > May I ask if you mean driver should use nand_hw_control_init helper to > do controller_init? Well, now it's named nand_controller_init(), but yes, that's what I meant.
Re: [PATCH] mm/vmscan: fix page_freeze_refs in comment.
On Thu 26-07-18 07:34:17, Jiang Biao wrote: > page_freeze_refs has already been relplaced by page_ref_freeze, but > it is not modified in the comment. Hmm $ git grep page_refs_freeze origin/master $ The same is the case in the linux-next tree. Which tree are you looking at? > > Signed-off-by: Jiang Biao > --- > mm/vmscan.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/mm/vmscan.c b/mm/vmscan.c > index 03822f8..d29e207 100644 > --- a/mm/vmscan.c > +++ b/mm/vmscan.c > @@ -744,7 +744,7 @@ static int __remove_mapping(struct address_space > *mapping, struct page *page, > refcount = 2; > if (!page_ref_freeze(page, refcount)) > goto cannot_free; > - /* note: atomic_cmpxchg in page_freeze_refs provides the smp_rmb */ > + /* note: atomic_cmpxchg in page_refs_freeze provides the smp_rmb */ > if (unlikely(PageDirty(page))) { > page_ref_unfreeze(page, refcount); > goto cannot_free; > -- > 2.7.4 > -- Michal Hocko SUSE Labs
Re: Zram writeback feature unstable with heavy swap utilization - BUG: Bad page state in process...
Hi, On 07/26/2018 04:03 AM, Minchan Kim wrote: A thing I could imagine is [0bcac06f27d75, skip swapcache for swapin of synchronous device] It was merged into v4.15. Could you check it by bisecting? Thanks, I will check that. My operating system is a minimal install of Debian 9. I took the kernel configuration from the default Debian kernel and built my own kernel with "make oldconfig" leaving all settings at their defaults. The only thing I changed in the configuration was enabling the zram writeback feature. You mean you changed host kernel configuration? All my tests were done on bare-metal hardware with Xeon processors and lots of RAM. I encounter the bug quite quickly, but it still takes several GBs of swap usage. Below is my /proc/meminfo with enough KVM instances running (3 in my case) to trigger the bug on my test machine. Aha.. you did writeback feature into your bare-metal host machine and execute kvm with window images as a guest. So, PG_uptodate warning happens on host side, not guest? Right? Yes, I am only talking about the host kernel. Zram swap is set up on the host. I just used Windows guests to fill up the host RAM and force it into swap. I will also try to reproduce the problem on some different hardware next. Just to confirm, I was able to reproduce the problem on another machine running Ubuntu 18.04 with the Ubuntu stock kernel (4.15) and no modifications to the kernel configuration whatsoever. The host had 8 GB of RAM, 32 GB of swap with zram and a 32 GB SSD as backing device. I had to start only one Windows VM with "-m 32768" to trigger the bug. -- Kind regards, Tino Lehnig
[PATCH][RESEND 2] component: enhance handling of devres group for master
From: Banajit Goswami The devres group opened for a master is left open-ended (without devres_group_close) even after bind() is complete. Similarly, while releasing the devres resources for master, the most recently opened devres group is selected, and released without identifying the targeted group. As the devres group opened before master bind was never closed, there may have unintended consequences of releasing devres resources that were allocated after master bind() function was complete. Change adds a devres_group_close() after bind() call to master, to encapsulate the resources allocated during bind, and then use a group ID to specifically identify the group in release, so that during master unbind, only the resources that are part of that specific devres group, are released. Signed-off-by: Banajit Goswami --- drivers/base/component.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/base/component.c b/drivers/base/component.c index 89b032f..f9ce937 100644 --- a/drivers/base/component.c +++ b/drivers/base/component.c @@ -155,17 +155,19 @@ static int try_to_bring_up_master(struct master *master, return 0; } - if (!devres_open_group(master->dev, NULL, GFP_KERNEL)) + if (!devres_open_group(master->dev, master, GFP_KERNEL)) return -ENOMEM; /* Found all components */ ret = master->ops->bind(master->dev); if (ret < 0) { - devres_release_group(master->dev, NULL); + devres_release_group(master->dev, master); dev_info(master->dev, "master bind failed: %d\n", ret); return ret; } + devres_close_group(master->dev, master); + master->bound = true; return 1; } @@ -190,7 +192,7 @@ static void take_down_master(struct master *master) { if (master->bound) { master->ops->unbind(master->dev); - devres_release_group(master->dev, NULL); + devres_release_group(master->dev, master); master->bound = false; } } -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v4 14/35] mtd: rawnand: mtk: convert driver to nand_scan()
On Sat, 2018-07-21 at 19:10 +0200, Boris Brezillon wrote: > On Fri, 20 Jul 2018 17:15:06 +0200 > Miquel Raynal wrote: > > > Two helpers have been added to the core to make ECC-related > > configuration between the detection phase and the final NAND scan. Use > > these hooks and convert the driver to just use nand_scan() instead of > > both nand_scan_ident() and nand_scan_tail(). > > > > Signed-off-by: Miquel Raynal > > --- > > drivers/mtd/nand/raw/mtk_nand.c | 75 > > - > > 1 file changed, 44 insertions(+), 31 deletions(-) > > > > diff --git a/drivers/mtd/nand/raw/mtk_nand.c > > b/drivers/mtd/nand/raw/mtk_nand.c > > index 7bc6be3f6ec0..967418f945ea 100644 > > --- a/drivers/mtd/nand/raw/mtk_nand.c > > +++ b/drivers/mtd/nand/raw/mtk_nand.c > > @@ -1250,13 +1250,54 @@ static int mtk_nfc_ecc_init(struct device *dev, > > struct mtd_info *mtd) > > return 0; > > } > > > > +static int mtk_nfc_attach_chip(struct nand_chip *chip) > > +{ > > + struct mtd_info *mtd = nand_to_mtd(chip); > > + struct device *dev = mtd->dev.parent; > > + struct mtk_nfc *nfc = nand_get_controller_data(chip); > > + struct mtk_nfc_nand_chip *mtk_nand = to_mtk_nand(chip); > > + int len; > > + int ret; > > + > > + if (chip->options & NAND_BUSWIDTH_16) { > > + dev_err(dev, "16bits buswidth not supported"); > > + return -EINVAL; > > + } > > + > > + /* store bbt magic in page, cause OOB is not protected */ > > + if (chip->bbt_options & NAND_BBT_USE_FLASH) > > + chip->bbt_options |= NAND_BBT_NO_OOB; > > + > > + ret = mtk_nfc_ecc_init(dev, mtd); > > + if (ret) > > + return ret; > > + > > + ret = mtk_nfc_set_spare_per_sector(&mtk_nand->spare_per_sector, mtd); > > + if (ret) > > + return ret; > > + > > + mtk_nfc_set_fdm(&mtk_nand->fdm, mtd); > > + mtk_nfc_set_bad_mark_ctl(&mtk_nand->bad_mark, mtd); > > + > > + len = mtd->writesize + mtd->oobsize; > > + nfc->buffer = devm_kzalloc(dev, len, GFP_KERNEL); > > + if (!nfc->buffer) > > + return -ENOMEM; > > + > > + return 0; > > +} > > + > > +static const struct nand_controller_ops mtk_nfc_controller_ops = { > > + .attach_chip = mtk_nfc_attach_chip, > > +}; > > + > > static int mtk_nfc_nand_chip_init(struct device *dev, struct mtk_nfc *nfc, > > struct device_node *np) > > { > > struct mtk_nfc_nand_chip *chip; > > struct nand_chip *nand; > > struct mtd_info *mtd; > > - int nsels, len; > > + int nsels; > > u32 tmp; > > int ret; > > int i; > > @@ -1287,6 +1328,7 @@ static int mtk_nfc_nand_chip_init(struct device *dev, > > struct mtk_nfc *nfc, > > > > nand = &chip->nand; > > nand->controller = &nfc->controller; > > + nand->controller->ops = &mtk_nfc_controller_ops; > > Just like for the marvell driver, this assignment should be moved here > [1]. Agree to this. > Also, it looks like this driver is open-coding > nand_controller_init(), probably something we should fix (in a separate > patch). May I ask if you mean driver should use nand_hw_control_init helper to do controller_init? Thanks, Xiaolei > > With this fixed: > > Reviewed-by: Boris Brezillon > > > > > nand_set_flash_node(nand, np); > > nand_set_controller_data(nand, nfc); > > @@ -1324,36 +1366,7 @@ static int mtk_nfc_nand_chip_init(struct device > > *dev, struct mtk_nfc *nfc, > > > > mtk_nfc_hw_init(nfc); > > > > - ret = nand_scan_ident(mtd, nsels, NULL); > > - if (ret) > > - return ret; > > - > > - /* store bbt magic in page, cause OOB is not protected */ > > - if (nand->bbt_options & NAND_BBT_USE_FLASH) > > - nand->bbt_options |= NAND_BBT_NO_OOB; > > - > > - ret = mtk_nfc_ecc_init(dev, mtd); > > - if (ret) > > - return -EINVAL; > > - > > - if (nand->options & NAND_BUSWIDTH_16) { > > - dev_err(dev, "16bits buswidth not supported"); > > - return -EINVAL; > > - } > > - > > - ret = mtk_nfc_set_spare_per_sector(&chip->spare_per_sector, mtd); > > - if (ret) > > - return ret; > > - > > - mtk_nfc_set_fdm(&chip->fdm, mtd); > > - mtk_nfc_set_bad_mark_ctl(&chip->bad_mark, mtd); > > - > > - len = mtd->writesize + mtd->oobsize; > > - nfc->buffer = devm_kzalloc(dev, len, GFP_KERNEL); > > - if (!nfc->buffer) > > - return -ENOMEM; > > - > > - ret = nand_scan_tail(mtd); > > + ret = nand_scan(mtd, nsels); > > if (ret) > > return ret; > > >
Re: Re: [PATCH] [PATCH] mm: disable preemption before swapcache_free
On Thu 26-07-18 10:21:40, zhaowu...@wingtech.com wrote: [...] > Our project really needs a fix to this issue Could you be more specific why? My understanding is that RT tasks usually have all the memory mlocked otherwise all the real time expectations are gone already. -- Michal Hocko SUSE Labs
[PATCH v2] ASoC: AMD: Fix build warning
Fixes sound/soc/amd/acp-da7219-max98357a.c: In function 'cz_probe': sound/soc/amd/acp-da7219-max98357a.c:367:3: warning: 'ret' may be used uninitialized in this function [-Wmaybe-uninitialized] dev_err(&pdev->dev, "Failed to register regulator: %d\n", ret); Signed-off-by: Akshu Agrawal --- v2: closing braces missing sound/soc/amd/acp-da7219-max98357a.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/soc/amd/acp-da7219-max98357a.c b/sound/soc/amd/acp-da7219-max98357a.c index cd3cf6e..8e3275a 100644 --- a/sound/soc/amd/acp-da7219-max98357a.c +++ b/sound/soc/amd/acp-da7219-max98357a.c @@ -365,7 +365,7 @@ static int cz_probe(struct platform_device *pdev) &acp_da7219_cfg); if (IS_ERR(rdev)) { dev_err(&pdev->dev, "Failed to register regulator: %d\n", - ret); + (int)PTR_ERR(rdev)); return -EINVAL; } -- 1.9.1
Re: Linux 4.18-rc6
On Wed, 25 Jul 2018 12:40:26 -0700 Kees Cook wrote: > On Tue, Jul 24, 2018 at 12:24 AM, Martin Schwidefsky > wrote: > > On Tue, 24 Jul 2018 09:15:58 +0200 > > Christian Borntraeger wrote: > > > >> On 07/24/2018 08:18 AM, Martin Schwidefsky wrote: > >> > On Mon, 23 Jul 2018 16:17:22 -0700 > >> > Linus Torvalds wrote: > >> > > >> >> On Mon, Jul 23, 2018 at 2:23 PM Guenter Roeck > >> >> wrote: > >> >>> > >> > >> Martin - can we just remove the > >> > >> select HAVE_GCC_PLUGINS > >> > >> from the s390 Kconfig file (or perhaps add "if BROKEN" or something to > >> disable it). > >> > >> Because if it's not getting fixed, it shouldn't be exposed. > >> > >> >>> The problem only affects 4.18 - the code has been rearranged in -next. > >> >>> Only, in my builders, I can't disable a flag for individual releases, > >> >>> so I just disabled it completely for s390. > >> >> > >> >> Well, I'm not going to release a 4.18 with a known problem, so in 4.18 > >> >> this *will* be disabled if it's not fixed. > >> >> > >> >> The fact that it might be fixed in linux-next is entirely immaterial > >> >> to the release of 4.18. > >> > > >> > Ok, if gcc with the plugins and an allmodconfig is considered to be > >> > important enough to warrant a fix, it can be pulled from here: > >> > > >> > git://git.kernel.org/pub/scm/linux/kernel/git/s390/linux.git for-linus > >> > > >> > Martin Schwidefsky (1): > >> > s390: disable gcc plugins > >> > > >> > Once that is in I will create another patch to undo this one and place > >> > it after the early boot rework. > >> > >> On the list a different fix was proposed about 2 weeks ago, > > https://lkml.kernel.org/r/cagxu5jkyesypd1mym7brvfpkdy9+hqrcpxckg5ikczwdgzl...@mail.gmail.com > > >> > >> something like > >> > >> CFLAGS_als.o += $(DISABLE_LATENT_ENTROPY_PLUGIN) > >> > >> and I prefer that. Because your patch disables all gcc plugins. > > > > This change would cause trouble with patch dependencies as als.c is moved > > to a different directory. I would prefer to disable all gcc plugins for > > 4.18. > > I don't understand why not just add it to the Makefile. For -next and > 4.19, just drop the line again once you merge with 4.18? A CFLAGS_*.o > entry for a file that doesn't exist should be harmless... > > Of course, if no one is actually using the gcc plugins on s390, then > okay, disabling them isn't a problem. :) But since it's been working > on s390 since 4.15, it seems weird to turn all of them off just for > 4.18 when a trivial fix is available. The gcc plugins have not in wide-spread use, it is no problem to just disable them all. The advantage doing the fixing with the Kconfig file is that I avoid a merge conflict with the current features branch. And I do not want to rebase that branch this late in the -rcs just because of this problem. -- blue skies, Martin. "Reality continues to ruin my life." - Calvin.
Re: linux-next: build warning after merge of the sound-asoc tree
On 7/26/2018 7:49 AM, Stephen Rothwell wrote: > Hi all, > > After merging the sound-asoc tree, today's linux-next build (x86_64 > allmodconfig) produced this warning: > > sound/soc/amd/acp-da7219-max98357a.c: In function 'cz_probe': > sound/soc/amd/acp-da7219-max98357a.c:367:3: warning: 'ret' may be used > uninitialized in this function [-Wmaybe-uninitialized] >dev_err(&pdev->dev, "Failed to register regulator: %d\n", >^ > ret); > > > Introduced by commit > > 7b5317aa809f ("ASoC: AMD: Add a fix voltage regulator for DA7219 and > ADAU7002") > > This is a real bug. > Posted the fix for the above warning message: https://patchwork.kernel.org/patch/10545235/ Thanks, Akshu
[PATCH] ASoC: AMD: Fix build warning
Fixes sound/soc/amd/acp-da7219-max98357a.c: In function 'cz_probe': sound/soc/amd/acp-da7219-max98357a.c:367:3: warning: 'ret' may be used uninitialized in this function [-Wmaybe-uninitialized] dev_err(&pdev->dev, "Failed to register regulator: %d\n", ret); Signed-off-by: Akshu Agrawal --- sound/soc/amd/acp-da7219-max98357a.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/soc/amd/acp-da7219-max98357a.c b/sound/soc/amd/acp-da7219-max98357a.c index cd3cf6e..48c1d94 100644 --- a/sound/soc/amd/acp-da7219-max98357a.c +++ b/sound/soc/amd/acp-da7219-max98357a.c @@ -365,7 +365,7 @@ static int cz_probe(struct platform_device *pdev) &acp_da7219_cfg); if (IS_ERR(rdev)) { dev_err(&pdev->dev, "Failed to register regulator: %d\n", - ret); + (int)PTR_ERR(rdev); return -EINVAL; } -- 1.9.1
[PATCH v2 3/3] selftests/ftrace: Fix kprobe string testcase to not probe notrace function
Fix kprobe string argument testcase to not probe notrace function. Instead, it probes tracefs function which must be available with ftrace. Signed-off-by: Masami Hiramatsu --- .../ftrace/test.d/kprobe/kprobe_args_string.tc | 30 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_string.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_string.tc index a0002563e9ee..1ad70cdaf442 100644 --- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_string.tc +++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_string.tc @@ -9,28 +9,22 @@ echo > kprobe_events case `uname -m` in x86_64) - ARG2=%si - OFFS=8 + ARG1=%di ;; i[3456]86) - ARG2=%cx - OFFS=4 + ARG1=%ax ;; aarch64) - ARG2=%x1 - OFFS=8 + ARG1=%x0 ;; arm*) - ARG2=%r1 - OFFS=4 + ARG1=%r0 ;; ppc64*) - ARG2=%r4 - OFFS=8 + ARG1=%r3 ;; ppc*) - ARG2=%r4 - OFFS=4 + ARG1=%r3 ;; *) echo "Please implement other architecture here" @@ -38,17 +32,17 @@ ppc*) esac : "Test get argument (1)" -echo "p:testprobe create_trace_kprobe arg1=+0(+0(${ARG2})):string" > kprobe_events +echo "p:testprobe tracefs_create_dir arg1=+0(${ARG1}):string" > kprobe_events echo 1 > events/kprobes/testprobe/enable -! echo test >> kprobe_events -tail -n 1 trace | grep -qe "testprobe.* arg1=\"test\"" +echo "p:test _do_fork" >> kprobe_events +grep -qe "testprobe.* arg1=\"test\"" trace echo 0 > events/kprobes/testprobe/enable : "Test get argument (2)" -echo "p:testprobe create_trace_kprobe arg1=+0(+0(${ARG2})):string arg2=+0(+${OFFS}(${ARG2})):string" > kprobe_events +echo "p:testprobe tracefs_create_dir arg1=+0(${ARG1}):string arg2=+0(${ARG1}):string" > kprobe_events echo 1 > events/kprobes/testprobe/enable -! echo test1 test2 >> kprobe_events -tail -n 1 trace | grep -qe "testprobe.* arg1=\"test1\" arg2=\"test2\"" +echo "p:test _do_fork" >> kprobe_events +grep -qe "testprobe.* arg1=\"test\" arg2=\"test\"" trace echo 0 > events/enable echo > kprobe_events
[PATCH v2 2/3] selftest/ftrace: Move kprobe selftest function to separate compile unit
From: Francis Deslauriers Move selftest function to its own compile unit so it can be compiled with the ftrace cflags (CC_FLAGS_FTRACE) allowing it to be probed during the ftrace startup tests. Signed-off-by: Francis Deslauriers Acked-by: Masami Hiramatsu --- kernel/trace/Makefile|5 + kernel/trace/trace_kprobe.c | 12 +--- kernel/trace/trace_kprobe_selftest.c | 10 ++ kernel/trace/trace_kprobe_selftest.h |7 +++ 4 files changed, 23 insertions(+), 11 deletions(-) create mode 100644 kernel/trace/trace_kprobe_selftest.c create mode 100644 kernel/trace/trace_kprobe_selftest.h diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile index e2538c7638d4..e38771eccb2f 100644 --- a/kernel/trace/Makefile +++ b/kernel/trace/Makefile @@ -13,6 +13,11 @@ obj-y += trace_selftest_dynamic.o endif endif +ifdef CONFIG_FTRACE_STARTUP_TEST +CFLAGS_trace_kprobe_selftest.o = $(CC_FLAGS_FTRACE) +obj-$(CONFIG_KPROBE_EVENTS) += trace_kprobe_selftest.o +endif + # If unlikely tracing is enabled, do not trace these files ifdef CONFIG_TRACING_BRANCHES KBUILD_CFLAGS += -DDISABLE_BRANCH_PROFILING diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c index 1f1b4d712a7e..859f7c310a00 100644 --- a/kernel/trace/trace_kprobe.c +++ b/kernel/trace/trace_kprobe.c @@ -23,6 +23,7 @@ #include #include +#include "trace_kprobe_selftest.h" #include "trace_probe.h" #define KPROBE_EVENT_SYSTEM "kprobes" @@ -1573,17 +1574,6 @@ fs_initcall(init_kprobe_trace); #ifdef CONFIG_FTRACE_STARTUP_TEST -/* - * The "__used" keeps gcc from removing the function symbol - * from the kallsyms table. 'noinline' makes sure that there - * isn't an inlined version used by the test method below - */ -static __used __init noinline int -kprobe_trace_selftest_target(int a1, int a2, int a3, int a4, int a5, int a6) -{ - return a1 + a2 + a3 + a4 + a5 + a6; -} - static __init struct trace_event_file * find_trace_probe_file(struct trace_kprobe *tk, struct trace_array *tr) { diff --git a/kernel/trace/trace_kprobe_selftest.c b/kernel/trace/trace_kprobe_selftest.c new file mode 100644 index ..16548ee4c8c6 --- /dev/null +++ b/kernel/trace/trace_kprobe_selftest.c @@ -0,0 +1,10 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Function used during the kprobe self test. This function is in a separate + * compile unit so it can be compile with CC_FLAGS_FTRACE to ensure that it + * can be probed by the selftests. + */ +int kprobe_trace_selftest_target(int a1, int a2, int a3, int a4, int a5, int a6) +{ + return a1 + a2 + a3 + a4 + a5 + a6; +} diff --git a/kernel/trace/trace_kprobe_selftest.h b/kernel/trace/trace_kprobe_selftest.h new file mode 100644 index ..4e10ec41c013 --- /dev/null +++ b/kernel/trace/trace_kprobe_selftest.h @@ -0,0 +1,7 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Function used during the kprobe self test. This function is in a separate + * compile unit so it can be compile with CC_FLAGS_FTRACE to ensure that it + * can be probed by the selftests. + */ +int kprobe_trace_selftest_target(int a1, int a2, int a3, int a4, int a5, int a6);
[PATCH v2 1/3] tracing: kprobes: Prohibit probing on notrace function
Prohibit kprobe-events probing on notrace function. Since probing on the notrace function can cause recursive event call. In most case those are just skipped, but in some case it falls into infinit recursive call. This protection can be disabled by the kconfig CONFIG_KPROBE_EVENTS_ON_NOTRACE=y, but it is highly recommended to keep it "n" for normal kernel. Signed-off-by: Masami Hiramatsu Tested-by: Francis Deslauriers --- Changes from v1 - Add CONFIG_KPROBE_EVENTS_ON_NOTRACE kconfig for knocking down the protection. --- kernel/trace/Kconfig| 18 ++ kernel/trace/trace_kprobe.c | 23 +++ 2 files changed, 41 insertions(+) diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig index dcc0166d1997..24d5a58467a3 100644 --- a/kernel/trace/Kconfig +++ b/kernel/trace/Kconfig @@ -456,6 +456,24 @@ config KPROBE_EVENTS This option is also required by perf-probe subcommand of perf tools. If you want to use perf tools, this option is strongly recommended. +config KPROBE_EVENTS_ON_NOTRACE + bool "Do NOT protect notrace function from kprobe events" + depends on KPROBE_EVENTS + default n + help + This is only for the developers who want to debug ftrace itself + using kprobe events. + + Usually, ftrace related functions are protected from kprobe-events + to prevent an infinit recursion or any unexpected execution path + which leads to a kernel crash. + + This option disables such protection and allows you to put kprobe + events on ftrace functions for debugging ftrace by itself. + Note that this might let you shoot yourself in the foot. + + If unsure, say N. + config UPROBE_EVENTS bool "Enable uprobes-based dynamic events" depends on ARCH_SUPPORTS_UPROBES diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c index 27ace4513c43..1f1b4d712a7e 100644 --- a/kernel/trace/trace_kprobe.c +++ b/kernel/trace/trace_kprobe.c @@ -496,6 +496,23 @@ disable_trace_kprobe(struct trace_kprobe *tk, struct trace_event_file *file) return ret; } +#ifdef CONFIG_KPROBE_EVENTS_ON_NOTRACE +#define within_notrace_func(tk)(false) +#else +static bool within_notrace_func(struct trace_kprobe *tk) +{ + unsigned long offset, size, addr; + + addr = kallsyms_lookup_name(trace_kprobe_symbol(tk)); + addr += trace_kprobe_offset(tk); + + if (!kallsyms_lookup_size_offset(addr, &size, &offset)) + return true;/* Out of range. */ + + return !ftrace_location_range(addr - offset, addr - offset + size); +} +#endif + /* Internal register function - just handle k*probes and flags */ static int __register_trace_kprobe(struct trace_kprobe *tk) { @@ -504,6 +521,12 @@ static int __register_trace_kprobe(struct trace_kprobe *tk) if (trace_probe_is_registered(&tk->tp)) return -EINVAL; + if (within_notrace_func(tk)) { + pr_warn("Could not probe notrace function %s\n", + trace_kprobe_symbol(tk)); + return -EINVAL; + } + for (i = 0; i < tk->tp.nr_args; i++) traceprobe_update_arg(&tk->tp.args[i]);
BUG: soft lockup in snd_virmidi_output_trigger
Reporting the crash: BUG: soft lockup in snd_virmidi_output_trigger This crash has been found in v4.18-rc3 using RaceFuzzer (a modified version of Syzkaller), which we describe more at the end of this report. Note that this bug is previously reported by Syzkaller a few month ago. (https://syzkaller.appspot.com/bug?id=642c927972318775e0ea1b4779ccd8624ce9e6f5) Nonetheless, the crash still can be detected. We guess that the crash has not fixed yet. We report the crash log and the attached reproducer with a hope that they are helpful to diagnose the crash and to fix a bug. C Reproducer: https://kiwi.cs.purdue.edu/static/race-fuzzer/repro-dab082.c Kernel config: https://kiwi.cs.purdue.edu/static/race-fuzzer/config-dab082 (Please disregard all code related to "should_hypercall" in the C repro, as this is only for our debugging purposes using our own hypervisor.) Crash log: == watchdog: BUG: soft lockup - CPU#1 stuck for 22s! [syz-executor0:24261] Modules linked in: irq event stamp: 6692 hardirqs last enabled at (6691): [] __raw_spin_unlock_irqrestore include/linux/spinlock_api_smp.h:160 [inline] hardirqs last enabled at (6691): [] _raw_spin_unlock_irqrestore+0x62/0x90 kernel/locking/spinlock.c:184 hardirqs last disabled at (6692): [] interrupt_entry+0xb5/0xf0 arch/x86/entry/entry_64.S:625 softirqs last enabled at (1468): [] __do_softirq+0x5ff/0x7f4 kernel/softirq.c:314 softirqs last disabled at (1455): [] invoke_softirq kernel/softirq.c:368 [inline] softirqs last disabled at (1455): [] irq_exit+0x126/0x130 kernel/softirq.c:408 CPU: 1 PID: 24261 Comm: syz-executor0 Not tainted 4.18.0-rc3 #1 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.8.2-0-g33fbe13 by qemu-project.org 04/01/2014 RIP: 0010:arch_local_irq_restore arch/x86/include/asm/paravirt.h:783 [inline] RIP: 0010:__raw_spin_unlock_irqrestore include/linux/spinlock_api_smp.h:160 [inline] RIP: 0010:_raw_spin_unlock_irqrestore+0x7d/0x90 kernel/locking/spinlock.c:184 Code: 0d c8 ce 16 7b 5b 41 5c 5d c3 e8 3e 0f 52 fc 48 c7 c7 68 b0 71 86 e8 62 4c 8d fc 48 83 3d 42 91 86 01 00 74 0e 48 89 df 57 9d <0f> 1f 44 00 00 eb cd 0f 0b 0f 0b 0f 1f 84 00 00 00 00 00 55 48 89 RSP: 0018:8801b8e4f7e8 EFLAGS: 0282 ORIG_RAX: ff13 RAX: RBX: 0282 RCX: 84eb1f1e RDX: dc00 RSI: dc00 RDI: 0282 RBP: 8801b8e4f7f8 R08: ed002573a06a R09: R10: R11: R12: 88012b9d0348 R13: 8801e6417580 R14: 8801b8e4f868 R15: 8801e64175a8 FS: 7f3447a49700() GS:8801f6d0() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: 7f3447a48db8 CR3: 0001ee58d000 CR4: 06e0 Call Trace: spin_unlock_irqrestore include/linux/spinlock.h:365 [inline] snd_virmidi_output_trigger+0x37d/0x420 sound/core/seq/seq_virmidi.c:205 snd_rawmidi_output_trigger sound/core/rawmidi.c:150 [inline] snd_rawmidi_kernel_write1+0x383/0x410 sound/core/rawmidi.c:1288 snd_rawmidi_write+0x249/0xa50 sound/core/rawmidi.c:1338 __vfs_write+0xf9/0x5d0 fs/read_write.c:485 vfs_write+0x195/0x380 fs/read_write.c:549 ksys_write+0xdb/0x1d0 fs/read_write.c:598 __do_sys_write fs/read_write.c:610 [inline] __se_sys_write fs/read_write.c:607 [inline] __x64_sys_write+0x43/0x50 fs/read_write.c:607 do_syscall_64+0x182/0x540 arch/x86/entry/common.c:290 entry_SYSCALL_64_after_hwframe+0x49/0xbe RIP: 0033:0x44b939 Code: 8d 6b fc ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 5b 6b fc ff c3 66 2e 0f 1f 84 00 00 00 00 RSP: 002b:7f3447a48b48 EFLAGS: 0246 ORIG_RAX: 0001 RAX: ffda RBX: 0071bfa0 RCX: 0044b939 RDX: 210b RSI: 2100 RDI: 0013 RBP: b7a8 R08: R09: R10: R11: 0246 R12: 7f3447a496d4 R13: R14: 006f7848 R15: = About RaceFuzzer RaceFuzzer is a customized version of Syzkaller, specifically tailored to find race condition bugs in the Linux kernel. While we leverage many different technique, the notable feature of RaceFuzzer is in leveraging a custom hypervisor (QEMU/KVM) to interleave the scheduling. In particular, we modified the hypervisor to intentionally stall a per-core execution, which is similar to supporting per-core breakpoint functionality. This allows RaceFuzzer to force the kernel to deterministically trigger racy condition (which may rarely happen in practice due to randomness in scheduling).
[PATCH v2 0/3] tracing: kprobes: Prohibit probing on notrace functions
Hello, this is the 2nd version of the series to prohibit kprobe on notrace functions which Francis sent before. Here is v1 series: https://lkml.org/lkml/2018/7/12/678 In this version, I've add a kconfig option to remove notrace protection just for debugging, and also fix ftracetest testcase to not probe notrace function. Thank you, --- Francis Deslauriers (1): selftest/ftrace: Move kprobe selftest function to separate compile unit Masami Hiramatsu (2): tracing: kprobes: Prohibit probing on notrace function selftests/ftrace: Fix kprobe string testcase to not probe notrace function kernel/trace/Kconfig | 18 ++ kernel/trace/Makefile |5 +++ kernel/trace/trace_kprobe.c| 35 ++-- kernel/trace/trace_kprobe_selftest.c | 10 ++ kernel/trace/trace_kprobe_selftest.h |7 .../ftrace/test.d/kprobe/kprobe_args_string.tc | 30 +++-- 6 files changed, 76 insertions(+), 29 deletions(-) create mode 100644 kernel/trace/trace_kprobe_selftest.c create mode 100644 kernel/trace/trace_kprobe_selftest.h -- Masami Hiramatsu (Linaro)
Re: [PATCH 2/2] drivers: clk: st: address sparse warnings
On Wed, Jul 25, 2018 at 01:40:53PM -0700, Stephen Boyd wrote: > Quoting Nicholas Mc Guire (2018-07-15 03:18:24) > > Refactoring of code to make it more readable and at the same time make > > sparse happy again. > > > > Signed-off-by: Nicholas Mc Guire > > --- > > > > sparse complained about: > > drivers/clk/st/clkgen-pll.c:225:12: warning: context imbalance in > > 'clkgen_pll_enable' - different lock contexts for basic block > > drivers/clk/st/clkgen-pll.c:267:9: warning: context imbalance in > > 'clkgen_pll_disable' - different lock contexts for basic block > > drivers/clk/st/clkgen-pll.c:413:9: warning: context imbalance in > > 'set_rate_stm_pll3200c32' - different lock contexts for basic block > > drivers/clk/st/clkgen-pll.c:570:9: warning: context imbalance in > > 'set_rate_stm_pll4600c28' - different lock contexts for basic block > > > > Which are technically false positives as the pll->lock which is being > > checked is determined at configuration time, thus the locks are balanced. > > Never the less the refactored code seems more readable and was > > commented to make it clear. > > This stuff above should go into the commit text. ok - then I´ll resend that with that moved into the commit message. > > > > > Patch was compile tested with: multi_v7_defconfig (implies > > CONFIG_ARCH_STI=y) > > > > Patch is against 4.18-rc4 (localversion-next is next-20180713) > > > > drivers/clk/st/clkgen-pll.c | 51 > > + > > 1 file changed, 28 insertions(+), 23 deletions(-) > > I believe this driver is in stasis mode. Not sure anyone is testing this > right now. Please Cc people who have worked on this driver, like Gabriel > Fernandez. > > > > > diff --git a/drivers/clk/st/clkgen-pll.c b/drivers/clk/st/clkgen-pll.c > > index 7a7106dc..cbb5184 100644 > > --- a/drivers/clk/st/clkgen-pll.c > > +++ b/drivers/clk/st/clkgen-pll.c > > @@ -228,13 +228,13 @@ static int clkgen_pll_enable(struct clk_hw *hw) > > unsigned long flags = 0; > > int ret = 0; > > > > - if (pll->lock) > > + if (pll->lock) { > > + /* stih418 and stih407 */ > > spin_lock_irqsave(pll->lock, flags); > > - > > - ret = __clkgen_pll_enable(hw); > > - > > - if (pll->lock) > > + ret = __clkgen_pll_enable(hw); > > spin_unlock_irqrestore(pll->lock, flags); > > + } else > > + ret = __clkgen_pll_enable(hw); > > > > return ret; > > } > > @@ -259,13 +259,13 @@ static void clkgen_pll_disable(struct clk_hw *hw) > > struct clkgen_pll *pll = to_clkgen_pll(hw); > > unsigned long flags = 0; > > > > - if (pll->lock) > > + if (pll->lock) { > > + /* stih418 and stih407 */ > > spin_lock_irqsave(pll->lock, flags); > > - > > - __clkgen_pll_disable(hw); > > - > > - if (pll->lock) > > + __clkgen_pll_disable(hw); > > spin_unlock_irqrestore(pll->lock, flags); > > + } else > > + __clkgen_pll_disable(hw); > > } > > > > static int clk_pll3200c32_get_params(unsigned long input, unsigned long > > output, > > @@ -400,15 +400,18 @@ static int set_rate_stm_pll3200c32(struct clk_hw *hw, > > unsigned long rate, > > > > __clkgen_pll_disable(hw); > > > > - if (pll->lock) > > + if (pll->lock) { > > + /* stih407 and stih418 */ > > spin_lock_irqsave(pll->lock, flags); > > - > > - CLKGEN_WRITE(pll, ndiv, pll->ndiv); > > - CLKGEN_WRITE(pll, idf, pll->idf); > > - CLKGEN_WRITE(pll, cp, pll->cp); > > - > > - if (pll->lock) > > + CLKGEN_WRITE(pll, ndiv, pll->ndiv); > > + CLKGEN_WRITE(pll, idf, pll->idf); > > + CLKGEN_WRITE(pll, cp, pll->cp); > > spin_unlock_irqrestore(pll->lock, flags); > > + } else { > > + CLKGEN_WRITE(pll, ndiv, pll->ndiv); > > + CLKGEN_WRITE(pll, idf, pll->idf); > > + CLKGEN_WRITE(pll, cp, pll->cp); > > + } > > Please don't duplicate this code. The sparse warnings can be fixed by > adding a fake lock acquire to the else of the if condition. We do this > in drivers/clk/clk.c so you should be able to copy it from there. the duplication is not a mater of the sparse warning only - the inetnt was to improve readability - atleast for the one-line cases it seems more readable to have it this way than to have the two lock checks - notably as you can then comment what the difference here really is. I agree that for this case with the three lines in the body it is not that reasonable - this was simply a matter of consistency as the other lock checks were moved into a if-else construct for readability and commenting. thx! hofrat
Re: [PATCH 1/2] drivers: clk: st: warn on iomap failure
On Wed, Jul 25, 2018 at 01:41:38PM -0700, Stephen Boyd wrote: > Quoting Nicholas Mc Guire (2018-07-15 03:18:23) > > While the return value of clkgen_get_register_base() is being checked > > at the call site, there is no indication of failure cause thus making > > diagnosis of the issues hard. The WARN_ON() allows to determine the > > cause of failure. > > > > Signed-off-by: Nicholas Mc Guire > > --- > > > > Problem found by an experimental coccinelle script > > > > Patch was compile tested with: multi_v7_defconfig (implies > > CONFIG_ARCH_STI=y) > > > > Patch is against 4.18-rc4 (localversion-next is next-20180713) > > > > drivers/clk/st/clkgen-pll.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/clk/st/clkgen-pll.c b/drivers/clk/st/clkgen-pll.c > > index cbb5184..aeb30ab 100644 > > --- a/drivers/clk/st/clkgen-pll.c > > +++ b/drivers/clk/st/clkgen-pll.c > > @@ -647,6 +647,7 @@ static void __iomem * __init clkgen_get_register_base( > > return NULL; > > > > reg = of_iomap(pnode, 0); > > + WARN_ON(!reg); > > > > of_node_put(pnode); > > return reg; > > Shouldn't the caller blow up on NULL pointer access? This patch doesn't > seem useful, sorry. > if you look at the call chain then there is a check for !NULL along the way - but never any information - no pr_*/printk or the like so ultimately you would get a failure but not know where that failure came from - the intent of the WARN_ON() is to allow you to locate the trigger event. Blowing up with a BUG_ON() is not necessary as the call chain does check for !NULL along the way. thx! hofrat
Re: [PATCH] watchdog: sp805: Add clock-frequency property
Hi Guenter, Thank you.. I sent next version patch with modification. Regards, Srinath. On Thu, Jul 26, 2018 at 10:39 AM, Guenter Roeck wrote: > On 07/25/2018 09:47 PM, Srinath Mannam wrote: >> >> Hi Guenter, >> >> It was my mistake sorry for that. I thought to add as Rivewed-by your >> name.. >> Many changes are made based on your review comments and suggestions. >> So, Can I add your name in Reviewed list? > > > You did that already below, and I am ok with that. > > Guenter > >> I will modify and send next patchset. >> >> thank you. >> >> Regards, >> Srinath. >> >> On Thu, Jul 26, 2018 at 10:12 AM, Guenter Roeck >> wrote: >>> >>> On 07/23/2018 01:37 AM, Srinath Mannam wrote: Use clock-frequency property given in _DSD object of ACPI device to calculate Watchdog rate as binding clock devices are not available as device tree. Note: There is no formal review process for _DSD properties Signed-off-by: Srinath Mannam Signed-off-by: Guenter Roeck >>> >>> >>> >>> I just noticed this. This is completely inappropriate. It is >>> the equivalent of forging a signature on a check. >>> >>> _Never_ add a Signed-off-by: statement for anyone but yourself. >>> >>> Guenter >>> Reviewed-by: Guenter Roeck Reviewed-by: Ray Jui Reviewed-by: Scott Branden --- drivers/watchdog/sp805_wdt.c | 35 +-- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c index 9849db0..a896b1c 100644 --- a/drivers/watchdog/sp805_wdt.c +++ b/drivers/watchdog/sp805_wdt.c @@ -11,6 +11,7 @@ * warranty of any kind, whether express or implied. */ +#include #include #include #include @@ -22,6 +23,7 @@ #include #include #include +#include #include #include #include @@ -65,6 +67,7 @@ struct sp805_wdt { spinlock_t lock; void __iomem*base; struct clk *clk; + u64 rate; struct amba_device *adev; unsigned intload_val; }; @@ -80,7 +83,7 @@ static int wdt_setload(struct watchdog_device *wdd, unsigned int timeout) struct sp805_wdt *wdt = watchdog_get_drvdata(wdd); u64 load, rate; - rate = clk_get_rate(wdt->clk); + rate = wdt->rate; /* * sp805 runs counter with given value twice, after the end of first @@ -106,9 +109,7 @@ static int wdt_setload(struct watchdog_device *wdd, unsigned int timeout) static unsigned int wdt_timeleft(struct watchdog_device *wdd) { struct sp805_wdt *wdt = watchdog_get_drvdata(wdd); - u64 load, rate; - - rate = clk_get_rate(wdt->clk); + u64 load; spin_lock(&wdt->lock); load = readl_relaxed(wdt->base + WDTVALUE); @@ -118,7 +119,7 @@ static unsigned int wdt_timeleft(struct watchdog_device *wdd) load += wdt->load_val + 1; spin_unlock(&wdt->lock); - return div_u64(load, rate); + return div_u64(load, wdt->rate); } static int @@ -228,11 +229,25 @@ sp805_wdt_probe(struct amba_device *adev, const struct amba_id *id) if (IS_ERR(wdt->base)) return PTR_ERR(wdt->base); - wdt->clk = devm_clk_get(&adev->dev, NULL); - if (IS_ERR(wdt->clk)) { - dev_warn(&adev->dev, "Clock not found\n"); - ret = PTR_ERR(wdt->clk); - goto err; + if (adev->dev.of_node) { + wdt->clk = devm_clk_get(&adev->dev, NULL); + if (IS_ERR(wdt->clk)) { + dev_err(&adev->dev, "Clock not found\n"); + return PTR_ERR(wdt->clk); + } + wdt->rate = clk_get_rate(wdt->clk); + } else if (has_acpi_companion(&adev->dev)) { + /* +* When Driver probe with ACPI device, clock devices +* are not available, so watchdog rate get from +* clock-frequency property given in _DSD object. +*/ + device_property_read_u64(&adev->dev, "clock-frequency", +&wdt->rate); + if (!wdt->rate) { + dev_err(&adev->dev, "no clock-frequency property\n"); + return -ENODEV; + } } wdt->adev = adev; >>> >> >
RE: [PATCH v3 2/2] fpga: zynq-fpga: Add support for readback
Hi Alan, Thanks for the review... > > In Zynq Case it supports two types of the readback (Configuration registers, > Configuration data(fpga image)) which may not be the same case for other > vendors. > > Since I need to support both the use cases I have differentiated them using > module param in the zynq-fpga driver. > > > > As you said we shouldn't change the attribute's function, So in the > > zynq-fpga driver, I am planning to add another debugfs attribute for > reading back of configuration registers as it is vendor specific readback > type. > > (Configuration registers Usage: > /sys/kernel/debug/fpga/zynq-fpga/cfgreg_summary) > > Is it called '...summary' because it is not all the registers? Could just be > "cfg_regs"? Sure will make it cfg_regs... Are you ok with the above proposal?? if you are ok will make changes accordingly and will post v4... > > > (Configuration data(fpga image) Usage: > /sys/kernel/debug/fpga/fpga0/image) > > Please let me know your thoughts for the same... > > > >> > >> > One other option is sysfs. Do you want me to implement sysfs path??? > >> > Any other suggestions??? > >> > >> You could implement another sysfs attribute for reading FPGA registers > >> with a separate ops callback. Please clarify what it is doing (not > >> all FPGAs have this function) and what that will look like when the > >> user reads the from the sysfs > > > > AFAIK we shouldn't add another sysfs/debugfs attribute for vendor-specific > readback type in the FPGA manager ops. > > Please correct me if my understanding is wrong. > > You are not wrong :) Thanks 😊 Regards, Kedar. > > > > > Regards, > > Kedar. > > > >> > >> Alan
Re: linux-next: build warning after merge of the rdma tree
On Wed, 2018-07-25 at 21:05 -0600, Jason Gunthorpe wrote: > On Thu, Jul 26, 2018 at 10:55:53AM +1000, Stephen Rothwell wrote: > > Hi all, > > > > After merging the rdma tree, today's linux-next build (powerpc > > ppc64_defconfig) produced this warning: > > > > net/sunrpc/xprtrdma/svc_rdma_rw.c: In function 'svc_rdma_post_chunk_ctxt': > > net/sunrpc/xprtrdma/svc_rdma_rw.c:350:5: warning: 'bad_wr' may be used > > uninitialized in this function [-Wmaybe-uninitialized] > > if (bad_wr != first_wr) > > ^ > > Huh. I'm quite surprised 0-day build service didn't warn on this. > > > Introduced by commit > > > > ed288d74a9e5 ("net/xprtrdma: Simplify ib_post_(send|recv|srq_recv)() > > calls") > > > > This is an actual problem. > > Yes, for sure. Bart? Thanks Stephen for having reported this. I propose to revert the changes in net/sunrpc/xprtrdma/svc_rdma_rw.c. Jason, do you want me to submit the below as a formal patch? Thanks, Bart. diff --git a/net/sunrpc/xprtrdma/svc_rdma_rw.c b/net/sunrpc/xprtrdma/svc_rdma_rw.c index 80975427f523..ce3ea8419704 100644 --- a/net/sunrpc/xprtrdma/svc_rdma_rw.c +++ b/net/sunrpc/xprtrdma/svc_rdma_rw.c @@ -329,7 +329,7 @@ static int svc_rdma_post_chunk_ctxt(struct svc_rdma_chunk_ctxt *cc) do { if (atomic_sub_return(cc->cc_sqecount, &rdma->sc_sq_avail) > 0) { - ret = ib_post_send(rdma->sc_qp, first_wr, NULL); + ret = ib_post_send(rdma->sc_qp, first_wr, &bad_wr); trace_svcrdma_post_rw(&cc->cc_cqe, cc->cc_sqecount, ret); if (ret)
Re: [PATCH] watchdog: sp805: Add clock-frequency property
On 07/25/2018 09:47 PM, Srinath Mannam wrote: Hi Guenter, It was my mistake sorry for that. I thought to add as Rivewed-by your name.. Many changes are made based on your review comments and suggestions. So, Can I add your name in Reviewed list? You did that already below, and I am ok with that. Guenter I will modify and send next patchset. thank you. Regards, Srinath. On Thu, Jul 26, 2018 at 10:12 AM, Guenter Roeck wrote: On 07/23/2018 01:37 AM, Srinath Mannam wrote: Use clock-frequency property given in _DSD object of ACPI device to calculate Watchdog rate as binding clock devices are not available as device tree. Note: There is no formal review process for _DSD properties Signed-off-by: Srinath Mannam Signed-off-by: Guenter Roeck I just noticed this. This is completely inappropriate. It is the equivalent of forging a signature on a check. _Never_ add a Signed-off-by: statement for anyone but yourself. Guenter Reviewed-by: Guenter Roeck Reviewed-by: Ray Jui Reviewed-by: Scott Branden --- drivers/watchdog/sp805_wdt.c | 35 +-- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c index 9849db0..a896b1c 100644 --- a/drivers/watchdog/sp805_wdt.c +++ b/drivers/watchdog/sp805_wdt.c @@ -11,6 +11,7 @@ * warranty of any kind, whether express or implied. */ +#include #include #include #include @@ -22,6 +23,7 @@ #include #include #include +#include #include #include #include @@ -65,6 +67,7 @@ struct sp805_wdt { spinlock_t lock; void __iomem*base; struct clk *clk; + u64 rate; struct amba_device *adev; unsigned intload_val; }; @@ -80,7 +83,7 @@ static int wdt_setload(struct watchdog_device *wdd, unsigned int timeout) struct sp805_wdt *wdt = watchdog_get_drvdata(wdd); u64 load, rate; - rate = clk_get_rate(wdt->clk); + rate = wdt->rate; /* * sp805 runs counter with given value twice, after the end of first @@ -106,9 +109,7 @@ static int wdt_setload(struct watchdog_device *wdd, unsigned int timeout) static unsigned int wdt_timeleft(struct watchdog_device *wdd) { struct sp805_wdt *wdt = watchdog_get_drvdata(wdd); - u64 load, rate; - - rate = clk_get_rate(wdt->clk); + u64 load; spin_lock(&wdt->lock); load = readl_relaxed(wdt->base + WDTVALUE); @@ -118,7 +119,7 @@ static unsigned int wdt_timeleft(struct watchdog_device *wdd) load += wdt->load_val + 1; spin_unlock(&wdt->lock); - return div_u64(load, rate); + return div_u64(load, wdt->rate); } static int @@ -228,11 +229,25 @@ sp805_wdt_probe(struct amba_device *adev, const struct amba_id *id) if (IS_ERR(wdt->base)) return PTR_ERR(wdt->base); - wdt->clk = devm_clk_get(&adev->dev, NULL); - if (IS_ERR(wdt->clk)) { - dev_warn(&adev->dev, "Clock not found\n"); - ret = PTR_ERR(wdt->clk); - goto err; + if (adev->dev.of_node) { + wdt->clk = devm_clk_get(&adev->dev, NULL); + if (IS_ERR(wdt->clk)) { + dev_err(&adev->dev, "Clock not found\n"); + return PTR_ERR(wdt->clk); + } + wdt->rate = clk_get_rate(wdt->clk); + } else if (has_acpi_companion(&adev->dev)) { + /* +* When Driver probe with ACPI device, clock devices +* are not available, so watchdog rate get from +* clock-frequency property given in _DSD object. +*/ + device_property_read_u64(&adev->dev, "clock-frequency", +&wdt->rate); + if (!wdt->rate) { + dev_err(&adev->dev, "no clock-frequency property\n"); + return -ENODEV; + } } wdt->adev = adev;
[PATCH v3 4/4] MAINTAINERS: Add entry for Actions Semi Owl SoCs DMA driver
Add entry for Actions Semi Owl SoCs DMA driver under ARM/ACTIONS. Signed-off-by: Manivannan Sadhasivam --- MAINTAINERS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 09b54e9ebc6f..56d9c7715c2a 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1145,12 +1145,14 @@ F: arch/arm/boot/dts/owl-* F: arch/arm64/boot/dts/actions/ F: drivers/clk/actions/ F: drivers/clocksource/owl-* +F: drivers/dma/owl-dma.c F: drivers/pinctrl/actions/* F: drivers/soc/actions/ F: include/dt-bindings/power/owl-* F: include/linux/soc/actions/ F: Documentation/devicetree/bindings/arm/actions.txt F: Documentation/devicetree/bindings/clock/actions,s900-cmu.txt +F: Documentation/devicetree/bindings/dma/owl-dma.txt F: Documentation/devicetree/bindings/pinctrl/actions,s900-pinctrl.txt F: Documentation/devicetree/bindings/power/actions,owl-sps.txt F: Documentation/devicetree/bindings/timer/actions,owl-timer.txt -- 2.17.1
[PATCH v3 0/4] Add Actions Semi Owl family S900 DMA Controller support
This patchset adds DMA controller support for Actions Semi Owl family S900 SoC. This driver has been structured in a way such that there will be only one controller driver for the whole Owl family series (S500, S700 and S900 SoCs). There are 12 physical channels and 46 logical channels supported by the DMA controller. The DMA controller also supports 4 software configurable interrupt lines for the priority based DMA usecase. But the DMA driver supports only 1 interrupt for simplification. By default, the driver uses Linked list mode for all transfers. Right now only MEMCPY support has been added and the rest (SLAVE, CYCLIC) will be added in upcoming patches. The driver has been tested using dmatest utility on the Bubblegum-96 board. The DTS patches in this series depends on the pinctrl DTS patches submitted [1], which is yet to be merged by the platform maintainer Andreas. Since the DMA driver goes through DMA tree and the relevant DTS patches goes through ARM-SoC tree, Andreas will pick it up once it has been reviewed. For the reference, I have queued up all reviewed dts patches so far in my repo [2] from which Andreas is picking them for 4.19. Thanks, Mani [1] https://patchwork.kernel.org/patch/10322937/ [2] https://git.linaro.org/people/manivannan.sadhasivam/linux.git/log/?h=s900-for-next Changes in v3: As per Vinod's review: * Removed unused header and API's * Used GENMASK for defines * Removed per member comment for structs * Modified pchan* and dma* API's to use corresponding containers as arguments * Removed error messages regarding the no free channels * Added devm_free_irq in dma_remove * Used dmaengine instead of dma for commit titles Changes in v2: * Fixed up the multi-line alignments according to `checkpatch --strict` Manivannan Sadhasivam (4): dt-bindings: dmaengine: Add binding for Actions Semi Owl SoCs arm64: dts: actions: Add Actions Semi S900 DMA Controller dmaengine: Add Actions Semi Owl family S900 DMA driver MAINTAINERS: Add entry for Actions Semi Owl SoCs DMA driver .../devicetree/bindings/dma/owl-dma.txt | 47 + MAINTAINERS | 2 + arch/arm64/boot/dts/actions/s900.dtsi | 13 + drivers/dma/Kconfig | 8 + drivers/dma/Makefile | 1 + drivers/dma/owl-dma.c | 971 ++ 6 files changed, 1042 insertions(+) create mode 100644 Documentation/devicetree/bindings/dma/owl-dma.txt create mode 100644 drivers/dma/owl-dma.c -- 2.17.1
[PATCH v3 3/4] dmaengine: Add Actions Semi Owl family S900 DMA driver
Add Actions Semi Owl family S900 DMA driver. Signed-off-by: Manivannan Sadhasivam --- drivers/dma/Kconfig | 8 + drivers/dma/Makefile | 1 + drivers/dma/owl-dma.c | 971 ++ 3 files changed, 980 insertions(+) create mode 100644 drivers/dma/owl-dma.c diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig index ca1680afa20a..92a278e6618c 100644 --- a/drivers/dma/Kconfig +++ b/drivers/dma/Kconfig @@ -413,6 +413,14 @@ config NBPFAXI_DMA help Support for "Type-AXI" NBPF DMA IPs from Renesas +config OWL_DMA + tristate "Actions Semi Owl SoCs DMA support" + depends on ARCH_ACTIONS + select DMA_ENGINE + select DMA_VIRTUAL_CHANNELS + help + Enable support for the Actions Semi Owl SoCs DMA controller. + config PCH_DMA tristate "Intel EG20T PCH / LAPIS Semicon IOH(ML7213/ML7223/ML7831) DMA" depends on PCI && (X86_32 || COMPILE_TEST) diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile index 203a99d68315..c91702d88b95 100644 --- a/drivers/dma/Makefile +++ b/drivers/dma/Makefile @@ -52,6 +52,7 @@ obj-$(CONFIG_MV_XOR_V2) += mv_xor_v2.o obj-$(CONFIG_MXS_DMA) += mxs-dma.o obj-$(CONFIG_MX3_IPU) += ipu/ obj-$(CONFIG_NBPFAXI_DMA) += nbpfaxi.o +obj-$(CONFIG_OWL_DMA) += owl-dma.o obj-$(CONFIG_PCH_DMA) += pch_dma.o obj-$(CONFIG_PL330_DMA) += pl330.o obj-$(CONFIG_PPC_BESTCOMM) += bestcomm/ diff --git a/drivers/dma/owl-dma.c b/drivers/dma/owl-dma.c new file mode 100644 index ..7812a6338acd --- /dev/null +++ b/drivers/dma/owl-dma.c @@ -0,0 +1,971 @@ +// SPDX-License-Identifier: GPL-2.0+ +// +// Actions Semi Owl SoCs DMA driver +// +// Copyright (c) 2014 Actions Semi Inc. +// Author: David Liu +// +// Copyright (c) 2018 Linaro Ltd. +// Author: Manivannan Sadhasivam + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include "virt-dma.h" + +#define OWL_DMA_FRAME_MAX_LENGTH 0xf + +/* Global DMA Controller Registers */ +#define OWL_DMA_IRQ_PD00x00 +#define OWL_DMA_IRQ_PD10x04 +#define OWL_DMA_IRQ_PD20x08 +#define OWL_DMA_IRQ_PD30x0C +#define OWL_DMA_IRQ_EN00x10 +#define OWL_DMA_IRQ_EN10x14 +#define OWL_DMA_IRQ_EN20x18 +#define OWL_DMA_IRQ_EN30x1C +#define OWL_DMA_SECURE_ACCESS_CTL 0x20 +#define OWL_DMA_NIC_QOS0x24 +#define OWL_DMA_DBGSEL 0x28 +#define OWL_DMA_IDLE_STAT 0x2C + +/* Channel Registers */ +#define OWL_DMA_CHAN_BASE(i) (0x100 + (i) * 0x100) +#define OWL_DMAX_MODE 0x00 +#define OWL_DMAX_SOURCE0x04 +#define OWL_DMAX_DESTINATION 0x08 +#define OWL_DMAX_FRAME_LEN 0x0C +#define OWL_DMAX_FRAME_CNT 0x10 +#define OWL_DMAX_REMAIN_FRAME_CNT 0x14 +#define OWL_DMAX_REMAIN_CNT0x18 +#define OWL_DMAX_SOURCE_STRIDE 0x1C +#define OWL_DMAX_DESTINATION_STRIDE0x20 +#define OWL_DMAX_START 0x24 +#define OWL_DMAX_PAUSE 0x28 +#define OWL_DMAX_CHAINED_CTL 0x2C +#define OWL_DMAX_CONSTANT 0x30 +#define OWL_DMAX_LINKLIST_CTL 0x34 +#define OWL_DMAX_NEXT_DESCRIPTOR 0x38 +#define OWL_DMAX_CURRENT_DESCRIPTOR_NUM0x3C +#define OWL_DMAX_INT_CTL 0x40 +#define OWL_DMAX_INT_STATUS0x44 +#define OWL_DMAX_CURRENT_SOURCE_POINTER0x48 +#define OWL_DMAX_CURRENT_DESTINATION_POINTER 0x4C + +/* OWL_DMAX_MODE Bits */ +#define OWL_DMA_MODE_TS(x) (((x) & GENMASK(5, 0)) << 0) +#define OWL_DMA_MODE_ST(x) (((x) & GENMASK(1, 0)) << 8) +#defineOWL_DMA_MODE_ST_DEV OWL_DMA_MODE_ST(0) +#defineOWL_DMA_MODE_ST_DCU OWL_DMA_MODE_ST(2) +#defineOWL_DMA_MODE_ST_SRAMOWL_DMA_MODE_ST(3) +#define OWL_DMA_MODE_DT(x) (((x) & GENMASK(1, 0)) << 10) +#defineOWL_DMA_MODE_DT_DEV OWL_DMA_MODE_DT(0) +#defineOWL_DMA_MODE_DT_DCU OWL_DMA_MODE_DT(2) +#defineOWL_DMA_MODE_DT_SRAMOWL_DMA_MODE_DT(3) +#define OWL_DMA_MODE_SAM(x)(((x) & GENMASK(1, 0)) << 16) +#defineOWL_DMA_MODE_SAM_CONST OWL_DMA_MODE_SAM(0) +#defineOWL_DMA_MODE_SAM_INCOWL_DMA_MODE_SAM(1) +#defineOWL_DMA_MODE_SAM_STRIDE OWL_DMA_MODE_SAM(2)
[PATCH v3 1/4] dt-bindings: dmaengine: Add binding for Actions Semi Owl SoCs
Add devicetree binding for Actions Semi Owl SoCs DMA controller. Signed-off-by: Manivannan Sadhasivam --- .../devicetree/bindings/dma/owl-dma.txt | 47 +++ 1 file changed, 47 insertions(+) create mode 100644 Documentation/devicetree/bindings/dma/owl-dma.txt diff --git a/Documentation/devicetree/bindings/dma/owl-dma.txt b/Documentation/devicetree/bindings/dma/owl-dma.txt new file mode 100644 index ..03e9bb12b75f --- /dev/null +++ b/Documentation/devicetree/bindings/dma/owl-dma.txt @@ -0,0 +1,47 @@ +* Actions Semi Owl SoCs DMA controller + +This binding follows the generic DMA bindings defined in dma.txt. + +Required properties: +- compatible: Should be "actions,s900-dma". +- reg: Should contain DMA registers location and length. +- interrupts: Should contain 4 interrupts shared by all channel. +- #dma-cells: Must be <1>. Used to represent the number of integer + cells in the dmas property of client device. +- dma-channels: Physical channels supported. +- dma-requests: Number of DMA request signals supported by the controller. +Refer to Documentation/devicetree/bindings/dma/dma.txt +- clocks: Phandle and Specifier of the clock feeding the DMA controller. + +Example: + +Controller: +dma: dma-controller@e026 { +compatible = "actions,s900-dma"; +reg = <0x0 0xe026 0x0 0x1000>; +interrupts = , + , + , + ; +#dma-cells = <1>; +dma-channels = <12>; +dma-requests = <46>; +clocks = <&clock CLK_DMAC>; +}; + +Client: + +DMA clients connected to the Actions Semi Owl SoCs DMA controller must +use the format described in the dma.txt file, using a two-cell specifier +for each channel. + +The two cells in order are: +1. A phandle pointing to the DMA controller. +2. The channel id. + +uart5: serial@e012a000 { +... +dma-names = "tx", "rx"; +dmas = <&dma 26>, <&dma 27>; +... +}; -- 2.17.1
[PATCH v3 2/4] arm64: dts: actions: Add Actions Semi S900 DMA Controller
Add DMA controller node for Actions Semi S900 SoC. Signed-off-by: Manivannan Sadhasivam --- arch/arm64/boot/dts/actions/s900.dtsi | 13 + 1 file changed, 13 insertions(+) diff --git a/arch/arm64/boot/dts/actions/s900.dtsi b/arch/arm64/boot/dts/actions/s900.dtsi index 7ae8b931f000..2e8178e50832 100644 --- a/arch/arm64/boot/dts/actions/s900.dtsi +++ b/arch/arm64/boot/dts/actions/s900.dtsi @@ -191,6 +191,19 @@ ; }; + dma: dma-controller@e026 { + compatible = "actions,s900-dma"; + reg = <0x0 0xe026 0x0 0x1000>; + interrupts = , +, +, +; + #dma-cells = <1>; + dma-channels = <12>; + dma-requests = <46>; + clocks = <&cmu CLK_DMAC>; + }; + timer: timer@e0228000 { compatible = "actions,s900-timer"; reg = <0x0 0xe0228000 0x0 0x8000>; -- 2.17.1
Re: [PATCH] mmc: host: iproc: Add ACPI support to IPROC SDHCI
Hi Ulf Hansson, Kindly review this patch. Thank you, Regards, Srinath. On Tue, Jul 17, 2018 at 10:15 PM, Srinath Mannam wrote: > Add ACPI support to all IPROC SDHCI varients > > Signed-off-by: Srinath Mannam > Reviewed-by: Ray Jui > Reviewed-by: Scott Branden > Reviewed-by: Vladimir Olovyannikov > --- > drivers/mmc/host/Kconfig | 1 + > drivers/mmc/host/sdhci-iproc.c | 187 > - > 2 files changed, 128 insertions(+), 60 deletions(-) > > diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig > index 0581c19..bc6702e 100644 > --- a/drivers/mmc/host/Kconfig > +++ b/drivers/mmc/host/Kconfig > @@ -334,6 +334,7 @@ config MMC_SDHCI_IPROC > tristate "SDHCI support for the BCM2835 & iProc SD/MMC Controller" > depends on ARCH_BCM2835 || ARCH_BCM_IPROC || COMPILE_TEST > depends on MMC_SDHCI_PLTFM > + depends on OF || ACPI > default ARCH_BCM_IPROC > select MMC_SDHCI_IO_ACCESSORS > help > diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c > index d0e83db..7c5c923 100644 > --- a/drivers/mmc/host/sdhci-iproc.c > +++ b/drivers/mmc/host/sdhci-iproc.c > @@ -15,6 +15,7 @@ > * iProc SDHCI platform driver > */ > > +#include > #include > #include > #include > @@ -162,9 +163,19 @@ static void sdhci_iproc_writeb(struct sdhci_host *host, > u8 val, int reg) > sdhci_iproc_writel(host, newval, reg & ~3); > } > > +static unsigned int sdhci_iproc_get_max_clock(struct sdhci_host *host) > +{ > + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > + > + if (pltfm_host->clk) > + return sdhci_pltfm_clk_get_max_clock(host); > + else > + return pltfm_host->clock; > +} > + > static const struct sdhci_ops sdhci_iproc_ops = { > .set_clock = sdhci_set_clock, > - .get_max_clock = sdhci_pltfm_clk_get_max_clock, > + .get_max_clock = sdhci_iproc_get_max_clock, > .set_bus_width = sdhci_set_bus_width, > .reset = sdhci_reset, > .set_uhs_signaling = sdhci_set_uhs_signaling, > @@ -178,34 +189,24 @@ static const struct sdhci_ops sdhci_iproc_32only_ops = { > .write_w = sdhci_iproc_writew, > .write_b = sdhci_iproc_writeb, > .set_clock = sdhci_set_clock, > - .get_max_clock = sdhci_pltfm_clk_get_max_clock, > + .get_max_clock = sdhci_iproc_get_max_clock, > .set_bus_width = sdhci_set_bus_width, > .reset = sdhci_reset, > .set_uhs_signaling = sdhci_set_uhs_signaling, > }; > > +enum sdhci_pltfm_type { > + SDHCI_PLTFM_IPROC_CYGNUS, > + SDHCI_PLTFM_IPROC, > + SDHCI_PLTFM_BCM2835, > +}; > + > static const struct sdhci_pltfm_data sdhci_iproc_cygnus_pltfm_data = { > .quirks = SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK, > .quirks2 = SDHCI_QUIRK2_ACMD23_BROKEN | SDHCI_QUIRK2_HOST_OFF_CARD_ON, > .ops = &sdhci_iproc_32only_ops, > }; > > -static const struct sdhci_iproc_data iproc_cygnus_data = { > - .pdata = &sdhci_iproc_cygnus_pltfm_data, > - .caps = ((0x1 << SDHCI_MAX_BLOCK_SHIFT) > - & SDHCI_MAX_BLOCK_MASK) | > - SDHCI_CAN_VDD_330 | > - SDHCI_CAN_VDD_180 | > - SDHCI_CAN_DO_SUSPEND | > - SDHCI_CAN_DO_HISPD | > - SDHCI_CAN_DO_ADMA2 | > - SDHCI_CAN_DO_SDMA, > - .caps1 = SDHCI_DRIVER_TYPE_C | > -SDHCI_DRIVER_TYPE_D | > -SDHCI_SUPPORT_DDR50, > - .mmc_caps = MMC_CAP_1_8V_DDR, > -}; > - > static const struct sdhci_pltfm_data sdhci_iproc_pltfm_data = { > .quirks = SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK | > SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12, > @@ -213,21 +214,6 @@ static const struct sdhci_pltfm_data > sdhci_iproc_pltfm_data = { > .ops = &sdhci_iproc_ops, > }; > > -static const struct sdhci_iproc_data iproc_data = { > - .pdata = &sdhci_iproc_pltfm_data, > - .caps = ((0x1 << SDHCI_MAX_BLOCK_SHIFT) > - & SDHCI_MAX_BLOCK_MASK) | > - SDHCI_CAN_VDD_330 | > - SDHCI_CAN_VDD_180 | > - SDHCI_CAN_DO_SUSPEND | > - SDHCI_CAN_DO_HISPD | > - SDHCI_CAN_DO_ADMA2 | > - SDHCI_CAN_DO_SDMA, > - .caps1 = SDHCI_DRIVER_TYPE_C | > -SDHCI_DRIVER_TYPE_D | > -SDHCI_SUPPORT_DDR50, > -}; > - > static const struct sdhci_pltfm_data sdhci_bcm2835_pltfm_data = { > .quirks = SDHCI_QUIRK_BROKEN_CARD_DETECTION | > SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK | > @@ -237,38 +223,104 @@ static const struct sdhci_pltfm_data > sdhci_bcm2835_pltfm_data = { > .ops = &sdhci_iproc_32only_ops, > }; > > -static const struct sdhci_iproc_data bcm2835_data = { > - .pdata = &sdhci_bcm2835_pltfm_data, > - .caps = ((0x1 << SDHCI_MAX_BLOCK_SHIFT) > - & SDHCI_MAX_BLOCK_
[PATCH v2] watchdog: sp805: Add clock-frequency property
Use clock-frequency property given in _DSD object of ACPI device to calculate Watchdog rate as binding clock devices are not available as device tree. Note: There is no formal review process for _DSD properties Signed-off-by: Srinath Mannam Reviewed-by: Guenter Roeck Reviewed-by: Ray Jui Reviewed-by: Scott Branden --- drivers/watchdog/sp805_wdt.c | 35 +-- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c index 9849db0..a896b1c 100644 --- a/drivers/watchdog/sp805_wdt.c +++ b/drivers/watchdog/sp805_wdt.c @@ -11,6 +11,7 @@ * warranty of any kind, whether express or implied. */ +#include #include #include #include @@ -22,6 +23,7 @@ #include #include #include +#include #include #include #include @@ -65,6 +67,7 @@ struct sp805_wdt { spinlock_t lock; void __iomem*base; struct clk *clk; + u64 rate; struct amba_device *adev; unsigned intload_val; }; @@ -80,7 +83,7 @@ static int wdt_setload(struct watchdog_device *wdd, unsigned int timeout) struct sp805_wdt *wdt = watchdog_get_drvdata(wdd); u64 load, rate; - rate = clk_get_rate(wdt->clk); + rate = wdt->rate; /* * sp805 runs counter with given value twice, after the end of first @@ -106,9 +109,7 @@ static int wdt_setload(struct watchdog_device *wdd, unsigned int timeout) static unsigned int wdt_timeleft(struct watchdog_device *wdd) { struct sp805_wdt *wdt = watchdog_get_drvdata(wdd); - u64 load, rate; - - rate = clk_get_rate(wdt->clk); + u64 load; spin_lock(&wdt->lock); load = readl_relaxed(wdt->base + WDTVALUE); @@ -118,7 +119,7 @@ static unsigned int wdt_timeleft(struct watchdog_device *wdd) load += wdt->load_val + 1; spin_unlock(&wdt->lock); - return div_u64(load, rate); + return div_u64(load, wdt->rate); } static int @@ -228,11 +229,25 @@ sp805_wdt_probe(struct amba_device *adev, const struct amba_id *id) if (IS_ERR(wdt->base)) return PTR_ERR(wdt->base); - wdt->clk = devm_clk_get(&adev->dev, NULL); - if (IS_ERR(wdt->clk)) { - dev_warn(&adev->dev, "Clock not found\n"); - ret = PTR_ERR(wdt->clk); - goto err; + if (adev->dev.of_node) { + wdt->clk = devm_clk_get(&adev->dev, NULL); + if (IS_ERR(wdt->clk)) { + dev_err(&adev->dev, "Clock not found\n"); + return PTR_ERR(wdt->clk); + } + wdt->rate = clk_get_rate(wdt->clk); + } else if (has_acpi_companion(&adev->dev)) { + /* +* When Driver probe with ACPI device, clock devices +* are not available, so watchdog rate get from +* clock-frequency property given in _DSD object. +*/ + device_property_read_u64(&adev->dev, "clock-frequency", +&wdt->rate); + if (!wdt->rate) { + dev_err(&adev->dev, "no clock-frequency property\n"); + return -ENODEV; + } } wdt->adev = adev; -- 2.7.4
Re: [PATCH] net/rds/Kconfig: RDS should depend on IPV6
On 07/26/2018 06:36 AM, Santosh Shilimkar wrote: On 7/25/2018 3:20 PM, Anders Roxell wrote: Build error, implicit declaration of function __inet6_ehashfn shows up When RDS is enabled but not IPV6. net/rds/connection.c: In function ‘rds_conn_bucket’: net/rds/connection.c:67:9: error: implicit declaration of function ‘__inet6_ehashfn’; did you mean ‘__inet_ehashfn’? [-Werror=implicit-function-declaration] hash = __inet6_ehashfn(lhash, 0, fhash, 0, rds_hash_secret); ^~~ __inet_ehashfn Current code adds IPV6 as a depends on in config RDS. Fixes: eee2fa6ab322 ("rds: Changing IP address internal representation to struct in6_addr") Signed-off-by: Anders Roxell --- net/rds/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/rds/Kconfig b/net/rds/Kconfig index 41f75563b54b..607128f10bcd 100644 --- a/net/rds/Kconfig +++ b/net/rds/Kconfig @@ -1,7 +1,7 @@ config RDS tristate "The RDS Protocol" - depends on INET + depends on INET && CONFIG_IPV6 This should build without CONFIG_IPV6 too. Hi Ka-cheong, Can you please loot at it ? I know you modified lookup function to take always in6_addr now, but probably hashing with '__inet_ehashfn' should work too for non IPV6 address(s). I guess for now, let's add this dependency first. I will do a follow up patch to remove this dependency. Thanks. -- K. Poon ka-cheong.p...@oracle.com
linux-next: build failure after merge of the block tree
Hi all, After merging the block tree, today's linux-next build (x86_64 allmodconfig) failed like this: drivers/nvme/target/rdma.c: In function 'nvmet_rdma_find_get_device': drivers/nvme/target/rdma.c:894:26: error: 'struct ib_device_attr' has no member named 'max_sge'; did you mean 'max_cqe'? cm_id->device->attrs.max_sge) - 1; ^ drivers/nvme/target/rdma.c:893:21: note: in expansion of macro 'max' inline_sge_count = max(cm_id->device->attrs.max_sge_rd, ^~~ In file included from include/linux/list.h:9:0, from include/linux/module.h:9, from drivers/nvme/host/rdma.c:15: drivers/nvme/host/rdma.c: In function 'nvme_rdma_find_get_device': drivers/nvme/host/rdma.c:381:23: error: 'struct ib_device_attr' has no member named 'max_sge'; did you mean 'max_cqe'? ndev->dev->attrs.max_sge - 1); ^ drivers/nvme/host/rdma.c:380:30: note: in expansion of macro 'min' ndev->num_inline_segments = min(NVME_RDMA_MAX_INLINE_SEGMENTS, ^~~ Caused by commits 64a741c1eaa8 ("nvme-rdma: support up to 4 segments of inline data") 0d5ee2b2ab4f ("nvmet-rdma: support max(16KB, PAGE_SIZE) inline data") interacting with commit 33023fb85a42 ("IB/core: add max_send_sge and max_recv_sge attributes") from the rdma tree. I have applied the following merge fix patch for today: From: Stephen Rothwell Date: Thu, 26 Jul 2018 14:32:15 +1000 Subject: [PATCH] nvme-dma: merge fix up for replacement of max_sge Signed-off-by: Stephen Rothwell --- drivers/nvme/host/rdma.c | 2 +- drivers/nvme/target/rdma.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c index cfa0319fcd1c..368fe5ac0c6b 100644 --- a/drivers/nvme/host/rdma.c +++ b/drivers/nvme/host/rdma.c @@ -378,7 +378,7 @@ nvme_rdma_find_get_device(struct rdma_cm_id *cm_id) } ndev->num_inline_segments = min(NVME_RDMA_MAX_INLINE_SEGMENTS, - ndev->dev->attrs.max_sge - 1); + ndev->dev->attrs.max_send_sge - 1); list_add(&ndev->entry, &device_list); out_unlock: mutex_unlock(&device_list_mutex); diff --git a/drivers/nvme/target/rdma.c b/drivers/nvme/target/rdma.c index 86121a7a19b2..8c30ac7d8078 100644 --- a/drivers/nvme/target/rdma.c +++ b/drivers/nvme/target/rdma.c @@ -891,7 +891,7 @@ nvmet_rdma_find_get_device(struct rdma_cm_id *cm_id) inline_page_count = num_pages(port->inline_data_size); inline_sge_count = max(cm_id->device->attrs.max_sge_rd, - cm_id->device->attrs.max_sge) - 1; + cm_id->device->attrs.max_send_sge) - 1; if (inline_page_count > inline_sge_count) { pr_warn("inline_data_size %d cannot be supported by device %s. Reducing to %lu.\n", port->inline_data_size, cm_id->device->name, -- 2.18.0 -- Cheers, Stephen Rothwell pgpcIpnrcEv73.pgp Description: OpenPGP digital signature
Re: [PATCH v2 3/4] dma: Add Actions Semi Owl family S900 DMA driver
Hi Vinod, On Tue, Jul 24, 2018 at 06:39:43PM +0530, Vinod wrote: > somehow this got stuck so sending again... > > On 24-07-18, 18:16, Vinod wrote: > > On 23-07-18, 09:47, Manivannan Sadhasivam wrote: > > > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > +#include > > > > do you need this? > > Not now ;-) will remove this. > > > +/* OWL_DMAX_MODE Bits */ > > > +#define OWL_DMA_MODE_TS(x) (((x) & 0x3f) << 0) > > > +#define OWL_DMA_MODE_ST(x) (((x) & 0x3) << 8) > > > +#define OWL_DMA_MODE_ST_DEV OWL_DMA_MODE_ST(0) > > > +#define OWL_DMA_MODE_ST_DCU OWL_DMA_MODE_ST(2) > > > +#define OWL_DMA_MODE_ST_SRAMOWL_DMA_MODE_ST(3) > > > > what are you trying to do with this? Generally we would define register > > bits using BIT and GENMASK here.. > > Okay. Not sure about BIT() but I can use GENMASK() here. > > > +/* Extract the bit field to new shift */ > > > +#define BIT_FIELD(val, width, shift, newshift) \ > > > + val) >> (shift)) & ((BIT(width)) - 1)) << (newshift)) > > > > why new shift? I guess you want to extract bits from a register here and > > use those, right? > > No. Here we are trying to pack two bit fields in a single word. So, the `shift` is for the first Bit field and the `newshift` is for the second one. Will modify the comment accordingly! > > > +struct owl_dma_lli_hw { > > > + u32 next_lli; /* physical address of the next link list */ > > > + u32 saddr; /* source physical address */ > > > + u32 daddr; /* destination physical address */ > > > + u32 flen:20;/* frame length */ > > > + u32 fcnt:12;/* frame count */ > > > + u32 src_stride; /* source stride */ > > > + u32 dst_stride; /* destination stride */ > > > + u32 ctrla; /* dma_mode and linklist ctrl */ > > > + u32 ctrlb; /* interrupt control */ > > > + u32 const_num; /* data for constant fill */ > > > > i think you can skip comment here or kernel-doc style, please pick one > > and not both > > Ack. Will remove the per member comment. > > > +struct owl_dma_txd { > > > + struct virt_dma_descvd; > > > + struct list_headlli_list; > > > > why do you need this list. vd has its own list as well! > > Yes, but vd's list is named as node and that will create ambiguity since we will be using it as a list. So, I guess we would need lli_list. > > > +static void pchan_update(void __iomem *reg, u32 val, bool state) > > > > why does this not use pchan as arg as the name of API implies (you did > > that on the other two) > > I wanted to just update the reg without using too many arguments. Anyway, I can modify it to use pchan as the argument. > > > +static inline int owl_dma_cfg_lli(struct owl_dma_vchan *vchan, > > > + struct owl_dma_lli *lli, > > > + dma_addr_t src, dma_addr_t dst, > > > + u32 len, enum dma_transfer_direction dir) > > > +{ > > > + struct owl_dma_lli_hw *hw = &lli->hw; > > > + u32 mode; > > > + > > > + mode = OWL_DMA_MODE_PW(0); > > > + > > > + switch (dir) { > > > + case DMA_MEM_TO_MEM: > > > + mode |= OWL_DMA_MODE_TS(0) | OWL_DMA_MODE_ST_DCU | > > > + OWL_DMA_MODE_DT_DCU | OWL_DMA_MODE_SAM_INC | > > > + OWL_DMA_MODE_DAM_INC; > > > + > > > + break; > > > + default: > > > + return -EINVAL; > > > + } > > > + > > > + hw->next_lli = 0; /* One link list by default */ > > > + hw->saddr = src; > > > + hw->daddr = dst; > > > + > > > + hw->fcnt = 1; /* Frame count fixed as 1 */ > > > + hw->flen = len; /* Max frame length is 1MB */ > > > > are you checking that somewhere? > > No need to check since we allow only max size in the caller. The following line does the job: bytes = min_t(size_t, (len - offset), OWL_DMA_FRAME_MAX_LENGTH); > > > +static struct owl_dma_pchan *owl_dma_get_pchan(struct owl_dma *od, > > > +struct owl_dma_vchan *vchan) > > > +{ > > > + struct owl_dma_pchan *pchan; > > > + unsigned long flags; > > > + int i; > > > + > > > + for (i = 0; i < od->nr_pchans; i++) { > > > + pchan = &od->pchans[i]; > > > + > > > + spin_lock_irqsave(&pchan->lock, flags); > > > + if (!pchan->vchan) { > > > + pchan->vchan = vchan; > > > + spin_unlock_irqrestore(&pchan->lock, flags); > > > + break; > > > + } > > > + > > > + spin_unlock_irqrestore(&pchan->lock, flags); > > > + } > > > + > > > + if (i == od->nr_pchans) { > > > + /* No physical channel available, cope with it */ > > > + dev_dbg(od->dma.dev, "no physical channel available\n"); > > > >
Re: [PATCH] watchdog: sp805: Add clock-frequency property
Hi Guenter, It was my mistake sorry for that. I thought to add as Rivewed-by your name.. Many changes are made based on your review comments and suggestions. So, Can I add your name in Reviewed list? I will modify and send next patchset. thank you. Regards, Srinath. On Thu, Jul 26, 2018 at 10:12 AM, Guenter Roeck wrote: > On 07/23/2018 01:37 AM, Srinath Mannam wrote: >> >> Use clock-frequency property given in _DSD object >> of ACPI device to calculate Watchdog rate as binding >> clock devices are not available as device tree. >> >> Note: There is no formal review process for _DSD >> properties >> >> Signed-off-by: Srinath Mannam >> Signed-off-by: Guenter Roeck > > > I just noticed this. This is completely inappropriate. It is > the equivalent of forging a signature on a check. > > _Never_ add a Signed-off-by: statement for anyone but yourself. > > Guenter > >> Reviewed-by: Guenter Roeck >> Reviewed-by: Ray Jui >> Reviewed-by: Scott Branden >> --- >> drivers/watchdog/sp805_wdt.c | 35 +-- >> 1 file changed, 25 insertions(+), 10 deletions(-) >> >> diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c >> index 9849db0..a896b1c 100644 >> --- a/drivers/watchdog/sp805_wdt.c >> +++ b/drivers/watchdog/sp805_wdt.c >> @@ -11,6 +11,7 @@ >>* warranty of any kind, whether express or implied. >>*/ >> +#include >> #include >> #include >> #include >> @@ -22,6 +23,7 @@ >> #include >> #include >> #include >> +#include >> #include >> #include >> #include >> @@ -65,6 +67,7 @@ struct sp805_wdt { >> spinlock_t lock; >> void __iomem*base; >> struct clk *clk; >> + u64 rate; >> struct amba_device *adev; >> unsigned intload_val; >> }; >> @@ -80,7 +83,7 @@ static int wdt_setload(struct watchdog_device *wdd, >> unsigned int timeout) >> struct sp805_wdt *wdt = watchdog_get_drvdata(wdd); >> u64 load, rate; >> - rate = clk_get_rate(wdt->clk); >> + rate = wdt->rate; >> /* >> * sp805 runs counter with given value twice, after the end of >> first >> @@ -106,9 +109,7 @@ static int wdt_setload(struct watchdog_device *wdd, >> unsigned int timeout) >> static unsigned int wdt_timeleft(struct watchdog_device *wdd) >> { >> struct sp805_wdt *wdt = watchdog_get_drvdata(wdd); >> - u64 load, rate; >> - >> - rate = clk_get_rate(wdt->clk); >> + u64 load; >> spin_lock(&wdt->lock); >> load = readl_relaxed(wdt->base + WDTVALUE); >> @@ -118,7 +119,7 @@ static unsigned int wdt_timeleft(struct >> watchdog_device *wdd) >> load += wdt->load_val + 1; >> spin_unlock(&wdt->lock); >> - return div_u64(load, rate); >> + return div_u64(load, wdt->rate); >> } >> static int >> @@ -228,11 +229,25 @@ sp805_wdt_probe(struct amba_device *adev, const >> struct amba_id *id) >> if (IS_ERR(wdt->base)) >> return PTR_ERR(wdt->base); >> - wdt->clk = devm_clk_get(&adev->dev, NULL); >> - if (IS_ERR(wdt->clk)) { >> - dev_warn(&adev->dev, "Clock not found\n"); >> - ret = PTR_ERR(wdt->clk); >> - goto err; >> + if (adev->dev.of_node) { >> + wdt->clk = devm_clk_get(&adev->dev, NULL); >> + if (IS_ERR(wdt->clk)) { >> + dev_err(&adev->dev, "Clock not found\n"); >> + return PTR_ERR(wdt->clk); >> + } >> + wdt->rate = clk_get_rate(wdt->clk); >> + } else if (has_acpi_companion(&adev->dev)) { >> + /* >> +* When Driver probe with ACPI device, clock devices >> +* are not available, so watchdog rate get from >> +* clock-frequency property given in _DSD object. >> +*/ >> + device_property_read_u64(&adev->dev, "clock-frequency", >> +&wdt->rate); >> + if (!wdt->rate) { >> + dev_err(&adev->dev, "no clock-frequency >> property\n"); >> + return -ENODEV; >> + } >> } >> wdt->adev = adev; >> >
Re: [PATCH] watchdog: sp805: Add clock-frequency property
On 07/23/2018 01:37 AM, Srinath Mannam wrote: Use clock-frequency property given in _DSD object of ACPI device to calculate Watchdog rate as binding clock devices are not available as device tree. Note: There is no formal review process for _DSD properties Signed-off-by: Srinath Mannam Signed-off-by: Guenter Roeck I just noticed this. This is completely inappropriate. It is the equivalent of forging a signature on a check. _Never_ add a Signed-off-by: statement for anyone but yourself. Guenter Reviewed-by: Guenter Roeck Reviewed-by: Ray Jui Reviewed-by: Scott Branden --- drivers/watchdog/sp805_wdt.c | 35 +-- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c index 9849db0..a896b1c 100644 --- a/drivers/watchdog/sp805_wdt.c +++ b/drivers/watchdog/sp805_wdt.c @@ -11,6 +11,7 @@ * warranty of any kind, whether express or implied. */ +#include #include #include #include @@ -22,6 +23,7 @@ #include #include #include +#include #include #include #include @@ -65,6 +67,7 @@ struct sp805_wdt { spinlock_t lock; void __iomem*base; struct clk *clk; + u64 rate; struct amba_device *adev; unsigned intload_val; }; @@ -80,7 +83,7 @@ static int wdt_setload(struct watchdog_device *wdd, unsigned int timeout) struct sp805_wdt *wdt = watchdog_get_drvdata(wdd); u64 load, rate; - rate = clk_get_rate(wdt->clk); + rate = wdt->rate; /* * sp805 runs counter with given value twice, after the end of first @@ -106,9 +109,7 @@ static int wdt_setload(struct watchdog_device *wdd, unsigned int timeout) static unsigned int wdt_timeleft(struct watchdog_device *wdd) { struct sp805_wdt *wdt = watchdog_get_drvdata(wdd); - u64 load, rate; - - rate = clk_get_rate(wdt->clk); + u64 load; spin_lock(&wdt->lock); load = readl_relaxed(wdt->base + WDTVALUE); @@ -118,7 +119,7 @@ static unsigned int wdt_timeleft(struct watchdog_device *wdd) load += wdt->load_val + 1; spin_unlock(&wdt->lock); - return div_u64(load, rate); + return div_u64(load, wdt->rate); } static int @@ -228,11 +229,25 @@ sp805_wdt_probe(struct amba_device *adev, const struct amba_id *id) if (IS_ERR(wdt->base)) return PTR_ERR(wdt->base); - wdt->clk = devm_clk_get(&adev->dev, NULL); - if (IS_ERR(wdt->clk)) { - dev_warn(&adev->dev, "Clock not found\n"); - ret = PTR_ERR(wdt->clk); - goto err; + if (adev->dev.of_node) { + wdt->clk = devm_clk_get(&adev->dev, NULL); + if (IS_ERR(wdt->clk)) { + dev_err(&adev->dev, "Clock not found\n"); + return PTR_ERR(wdt->clk); + } + wdt->rate = clk_get_rate(wdt->clk); + } else if (has_acpi_companion(&adev->dev)) { + /* +* When Driver probe with ACPI device, clock devices +* are not available, so watchdog rate get from +* clock-frequency property given in _DSD object. +*/ + device_property_read_u64(&adev->dev, "clock-frequency", +&wdt->rate); + if (!wdt->rate) { + dev_err(&adev->dev, "no clock-frequency property\n"); + return -ENODEV; + } } wdt->adev = adev;
Re: [PATCH v2] hexagon: switch to NO_BOOTMEM
On Wed, Jul 25, 2018 at 08:44:57PM -0500, Richard Kuo wrote: > On Wed, Jul 25, 2018 at 08:29:54AM +0300, Mike Rapoport wrote: > > This patch adds registration of the system memory with memblock, eliminates > > bootmem initialization and converts early memory reservations from bootmem > > to memblock. > > > > Signed-off-by: Mike Rapoport > > --- > > v2: fix calculation of the reserved memory size > > > > arch/hexagon/Kconfig | 3 +++ > > arch/hexagon/mm/init.c | 20 > > 2 files changed, 11 insertions(+), 12 deletions(-) > > > > Looks good, I can take this through my tree. Thanks. The hexagon tree seems the most appropriate :) > Acked-by: Richard Kuo > -- Sincerely yours, Mike.
[PATCH 1/4] ARM: dts: mvebu: 98dx3236: Rename nand controller node
Update the 98dx3236 SoC and dependent boards to use "nand-controller" instead of "nand". Signed-off-by: Chris Packham --- arch/arm/boot/dts/armada-xp-98dx3236.dtsi | 2 +- arch/arm/boot/dts/armada-xp-db-dxbc2.dts | 2 +- arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/arm/boot/dts/armada-xp-98dx3236.dtsi b/arch/arm/boot/dts/armada-xp-98dx3236.dtsi index 8d708cc22495..eb03a5773903 100644 --- a/arch/arm/boot/dts/armada-xp-98dx3236.dtsi +++ b/arch/arm/boot/dts/armada-xp-98dx3236.dtsi @@ -189,7 +189,7 @@ }; }; - nand: nand@d { + nand_controller: nand-controller@d { clocks = <&dfx_coredivclk 0>; }; diff --git a/arch/arm/boot/dts/armada-xp-db-dxbc2.dts b/arch/arm/boot/dts/armada-xp-db-dxbc2.dts index f42fc6118b7c..944e0a9c9dac 100644 --- a/arch/arm/boot/dts/armada-xp-db-dxbc2.dts +++ b/arch/arm/boot/dts/armada-xp-db-dxbc2.dts @@ -68,7 +68,7 @@ status = "okay"; }; -&nand { +&nand_controller { status = "okay"; label = "pxa3xx_nand-0"; num-cs = <1>; diff --git a/arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts b/arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts index 8432f517e346..d58ea48cb151 100644 --- a/arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts +++ b/arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts @@ -67,7 +67,7 @@ status = "okay"; }; -&nand { +&nand_controller { status = "okay"; label = "pxa3xx_nand-0"; num-cs = <1>; -- 2.18.0
[PATCH 2/4] ARM: dts: mvebu: db-dxbc2: use new style nand binding
Update the nand flash binding to the new style. Signed-off-by: Chris Packham --- arch/arm/boot/dts/armada-xp-db-dxbc2.dts | 16 ++-- 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/arch/arm/boot/dts/armada-xp-db-dxbc2.dts b/arch/arm/boot/dts/armada-xp-db-dxbc2.dts index 944e0a9c9dac..8a3aa616bbd0 100644 --- a/arch/arm/boot/dts/armada-xp-db-dxbc2.dts +++ b/arch/arm/boot/dts/armada-xp-db-dxbc2.dts @@ -70,12 +70,16 @@ &nand_controller { status = "okay"; - label = "pxa3xx_nand-0"; - num-cs = <1>; - marvell,nand-keep-config; - nand-on-flash-bbt; - nand-ecc-strength = <4>; - nand-ecc-step-size = <512>; + + nand@0 { + reg = <0>; + label = "pxa3xx_nand-0"; + nand-rb = <0>; + marvell,nand-keep-config; + nand-on-flash-bbt; + nand-ecc-strength = <4>; + nand-ecc-step-size = <512>; + }; }; &sdio { -- 2.18.0
[PATCH 3/4] ARM: dts: mvebu: db-xc3-24g4: use new style nand binding
Update the nand flash binding to the new style. Signed-off-by: Chris Packham --- arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts | 16 ++-- 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts b/arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts index d58ea48cb151..df048050615f 100644 --- a/arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts +++ b/arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts @@ -69,12 +69,16 @@ &nand_controller { status = "okay"; - label = "pxa3xx_nand-0"; - num-cs = <1>; - marvell,nand-keep-config; - nand-on-flash-bbt; - nand-ecc-strength = <4>; - nand-ecc-step-size = <512>; + + nand@0 { + reg = <0>; + label = "pxa3xx_nand-0"; + nand-rb = <0>; + marvell,nand-keep-config; + nand-on-flash-bbt; + nand-ecc-strength = <4>; + nand-ecc-step-size = <512>; + }; }; &spi0 { -- 2.18.0
[PATCH 4/4] ARM: dts: mvebu: Add device tree for db-88f6820-amc board
This board is a plugin card for some of Marvell's switch development kits. It's similar to the non-amc board except that it has no SATA support. Signed-off-by: Chris Packham --- arch/arm/boot/dts/Makefile| 1 + .../boot/dts/armada-385-db-88f6820-amc.dts| 147 ++ 2 files changed, 148 insertions(+) create mode 100644 arch/arm/boot/dts/armada-385-db-88f6820-amc.dts diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile index 37a3de760d40..e4212b7b7d9e 100644 --- a/arch/arm/boot/dts/Makefile +++ b/arch/arm/boot/dts/Makefile @@ -1126,6 +1126,7 @@ dtb-$(CONFIG_MACH_ARMADA_370) += \ dtb-$(CONFIG_MACH_ARMADA_375) += \ armada-375-db.dtb dtb-$(CONFIG_MACH_ARMADA_38X) += \ + armada-385-db-88f6820-amc.dtb \ armada-385-db-ap.dtb \ armada-385-linksys-caiman.dtb \ armada-385-linksys-cobra.dtb \ diff --git a/arch/arm/boot/dts/armada-385-db-88f6820-amc.dts b/arch/arm/boot/dts/armada-385-db-88f6820-amc.dts new file mode 100644 index ..d87614057e3f --- /dev/null +++ b/arch/arm/boot/dts/armada-385-db-88f6820-amc.dts @@ -0,0 +1,147 @@ +// SPDX-License-Identifier: (GPL-2.0 OR MIT) +/* + * Device Tree file for Marvell Armada 385 AMC board + * (DB-88F6820-AMC) + * + * Copyright (C) 2017 Allied Telesis Labs + */ + +/dts-v1/; +#include "armada-385.dtsi" + +#include + +/ { + model = "Marvell Armada 385 AMC"; + compatible = "marvell,a385-db-amc", "marvell,armada385", "marvell,armada380"; + + chosen { + stdout-path = "serial0:115200n8"; + }; + + aliases { + ethernet0 = ð0; + ethernet1 = ð1; + spi1 = &spi1; + }; + + memory { + device_type = "memory"; + reg = <0x 0x8000>; /* 2GB */ + }; + + soc { + ranges = ; + }; +}; + +&i2c0 { + pinctrl-names = "default"; + pinctrl-0 = <&i2c0_pins>; + status = "okay"; +}; + +&uart0 { + /* +* Exported on the micro USB connector CON3 +* through an FTDI +*/ + + pinctrl-names = "default"; + pinctrl-0 = <&uart0_pins>; + status = "okay"; +}; + + +ð0 { + pinctrl-names = "default"; + /* +* The Reference Clock 0 is used to provide a +* clock to the PHY +*/ + pinctrl-0 = <&ge0_rgmii_pins>, <&ref_clk0_pins>; + status = "okay"; + phy = <&phy0>; + phy-mode = "rgmii-id"; +}; + +ð2 { + status = "okay"; + phy = <&phy1>; + phy-mode = "sgmii"; +}; + +&usb0 { + status = "okay"; +}; + + + +&mdio { + pinctrl-names = "default"; + pinctrl-0 = <&mdio_pins>; + + phy0: ethernet-phy@1 { + reg = <1>; + }; + + phy1: ethernet-phy@0 { + reg = <0>; + }; +}; + +&nand_controller { + status = "okay"; + + nand@0 { + reg = <0>; + label = "pxa3xx_nand-0"; + nand-rb = <0>; + nand-on-flash-bbt; + nand-ecc-strength = <4>; + nand-ecc-step-size = <512>; + + partition@user { + reg = <0x 0x4000>; + label = "user"; + }; + }; +}; + +&pciec { + status = "okay"; +}; + +&pcie1 { + /* Port 0, Lane 0 */ + status = "okay"; +}; + +&spi1 { + pinctrl-names = "default"; + pinctrl-0 = <&spi1_pins>; + status = "okay"; + + spi-flash@0 { + #address-cells = <1>; + #size-cells = <1>; + compatible = "jedec,spi-nor"; + reg = <0>; /* Chip select 0 */ + spi-max-frequency = <5000>; + m25p,fast-read; + + partition@u-boot { + reg = <0x 0x0010>; + label = "u-boot"; + }; + partition@u-boot-env { + reg = <0x0010 0x0004>; + label = "u-boot-env"; + }; + }; +}; + +&refclk { + clock-frequency = <2000>; +}; -- 2.18.0
[PATCH 0/4] ARM: dts: mvebu: updates and new board
This series updates the armada-xp-98dx3236 SoC and related boards to use the new style dts bindings for nand. I've also added a new db-88f6820-amc board which is an Armada-385 based reference board from Marvell's switch team. It's a plugin card for either the db-dxbc2 or db-xc3-24g4 which can be used if you disable the internal CPU on those platforms. Chris Packham (4): ARM: dts: mvebu: 98dx3236: Rename nand controller node ARM: dts: mvebu: db-dxbc2: use new style nand binding ARM: dts: mvebu: db-xc3-24g4: use new style nand binding ARM: dts: mvebu: Add device tree for db-88f6820-amc board arch/arm/boot/dts/Makefile| 1 + .../boot/dts/armada-385-db-88f6820-amc.dts| 184 ++ arch/arm/boot/dts/armada-xp-98dx3236.dtsi | 2 +- arch/arm/boot/dts/armada-xp-db-dxbc2.dts | 18 +- arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts | 18 +- 5 files changed, 208 insertions(+), 15 deletions(-) create mode 100644 arch/arm/boot/dts/armada-385-db-88f6820-amc.dts -- 2.18.0
linux-next: manual merge of the block tree with the rdma tree
Hi all, Today's linux-next merge of the block tree got a conflict in: drivers/nvme/target/rdma.c between commit: 23f96d1f15a7 ("nvmet-rdma: Simplify ib_post_(send|recv|srq_recv)() calls") 202093848cac ("nvmet-rdma: add an error flow for post_recv failures") from the rdma tree and commits: 2fc464e2162c ("nvmet-rdma: add unlikely check in the fast path") from the block tree. I fixed it up (see below) and can carry the fix as necessary. This is now fixed as far as linux-next is concerned, but any non trivial conflicts should be mentioned to your upstream maintainer when your tree is submitted for merging. You may also want to consider cooperating with the maintainer of the conflicting tree to minimise any particularly complex conflicts. -- Cheers, Stephen Rothwell diff --cc drivers/nvme/target/rdma.c index 1a642e214a4c,e7f43d1e1779.. --- a/drivers/nvme/target/rdma.c +++ b/drivers/nvme/target/rdma.c @@@ -382,13 -435,22 +435,21 @@@ static void nvmet_rdma_free_rsps(struc static int nvmet_rdma_post_recv(struct nvmet_rdma_device *ndev, struct nvmet_rdma_cmd *cmd) { - struct ib_recv_wr *bad_wr; + int ret; + ib_dma_sync_single_for_device(ndev->device, cmd->sge[0].addr, cmd->sge[0].length, DMA_FROM_DEVICE); if (ndev->srq) - return ib_post_srq_recv(ndev->srq, &cmd->wr, NULL); - return ib_post_recv(cmd->queue->cm_id->qp, &cmd->wr, NULL); - ret = ib_post_srq_recv(ndev->srq, &cmd->wr, &bad_wr); ++ ret = ib_post_srq_recv(ndev->srq, &cmd->wr, NULL); + else - ret = ib_post_recv(cmd->queue->cm_id->qp, &cmd->wr, &bad_wr); ++ ret = ib_post_recv(cmd->queue->cm_id->qp, &cmd->wr, NULL); + + if (unlikely(ret)) + pr_err("post_recv cmd failed\n"); + + return ret; } static void nvmet_rdma_process_wr_wait_list(struct nvmet_rdma_queue *queue) @@@ -491,7 -553,7 +552,7 @@@ static void nvmet_rdma_queue_response(s rsp->send_sge.addr, rsp->send_sge.length, DMA_TO_DEVICE); - if (ib_post_send(cm_id->qp, first_wr, NULL)) { - if (unlikely(ib_post_send(cm_id->qp, first_wr, &bad_wr))) { ++ if (unlikely(ib_post_send(cm_id->qp, first_wr, NULL))) { pr_err("sending cmd response failed\n"); nvmet_rdma_release_rsp(rsp); } pgpCvQwUaa8BX.pgp Description: OpenPGP digital signature
[PATCH v5] ARM: mvebu: use dt_fixup to provide fallback for enable-method
We need to maintain backwards compatibility with device trees that don't define an enable method. At the same time we want the device tree to be able to specify an enable-method and have it stick. Previously by having smp assigned in the DT_MACHINE definition this would be picked up by setup_arch() and override whatever arm_dt_init_cpu_maps() had configured. Now we move the initial assignment of default smp_ops to a dt_fixup and let arm_dt_init_cpu_maps() override that if the device tree defines an enable-method. Signed-off-by: Chris Packham --- Pervious versions v1: http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/300182.html v2: http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/300480.html v3: http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/302945.html v4: http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/303899.html Changes since v4: - drop "RFC" arch/arm/mach-mvebu/board-v7.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/arch/arm/mach-mvebu/board-v7.c b/arch/arm/mach-mvebu/board-v7.c index ccca95173e17..5bbde5e5258e 100644 --- a/arch/arm/mach-mvebu/board-v7.c +++ b/arch/arm/mach-mvebu/board-v7.c @@ -145,6 +145,11 @@ static void __init mvebu_dt_init(void) i2c_quirk(); } +static void __init armada_370_xp_dt_fixup(void) +{ + smp_set_ops(smp_ops(armada_xp_smp_ops)); +} + static const char * const armada_370_xp_dt_compat[] __initconst = { "marvell,armada-370-xp", NULL, @@ -153,17 +158,12 @@ static const char * const armada_370_xp_dt_compat[] __initconst = { DT_MACHINE_START(ARMADA_370_XP_DT, "Marvell Armada 370/XP (Device Tree)") .l2c_aux_val= 0, .l2c_aux_mask = ~0, -/* - * The following field (.smp) is still needed to ensure backward - * compatibility with old Device Trees that were not specifying the - * cpus enable-method property. - */ - .smp= smp_ops(armada_xp_smp_ops), .init_machine = mvebu_dt_init, .init_irq = mvebu_init_irq, .restart= mvebu_restart, .reserve= mvebu_memblock_reserve, .dt_compat = armada_370_xp_dt_compat, + .dt_fixup = armada_370_xp_dt_fixup, MACHINE_END static const char * const armada_375_dt_compat[] __initconst = { -- 2.18.0
Re: [PATCH 0/3] PTI for x86-32 Fixes and Updates
On Mon, 2018-07-23 at 09:29 +0200, Joerg Roedel wrote: > Hey David, > > On Sun, Jul 22, 2018 at 11:49:00PM -0400, David H. Gutteridge wrote: > > Unfortunately, I can trigger a bug in KVM+QEMU with the Bochs VGA > > driver. (This is the same VM definition I shared with you in a PM > > back on Feb. 20th, except note that 4.18 kernels won't successfully > > boot with QEMU's IDE device, so I'm using SATA instead. That's a > > regression totally unrelated to your change sets, or to the general > > booting issue with 4.18 RC5, since it occurs in vanilla RC4 as > > well.) > > Yes, this needs the fixes in the tip/x86/mm branch as well. Can you > that branch in and test again, please? Sorry, I didn't realize I needed those changes, too. I've re-tested with those applied and haven't encountered any issues. I'm now re-testing again with your newer patch set from the 25th. No issues so far with those, either; I'll confirm in that email thread after the laptop has seen some more use. Dave
Re: [PATCH] ACPI / LPSS: Avoid PM quirks on suspend and resume from S3
Hi Rafael, > On 2018Jul24, at 18:36, Rafael J. Wysocki wrote: > > On Tuesday, July 24, 2018 11:13:42 AM CEST Rafael J. Wysocki wrote: >> On Tuesday, July 24, 2018 10:46:09 AM CEST Kai Heng Feng wrote: >>> Hi Rafael, >>> On Jun 13, 2018, at 7:17 PM, Rafael J. Wysocki wrote: From: Rafael J. Wysocki It is reported that commit a192aa923b66a (ACPI / LPSS: Consolidate runtime PM and system sleep handling) introduced a system suspend regression on some machines, but the only functional change made by it was to cause the PM quirks in the LPSS to also be used during system suspend and resume. While that should always work for suspend-to-idle, it turns out to be problematic for S3 (suspend-to-RAM). To address that issue restore the previous S3 suspend and resume behavior of the LPSS to avoid applying PM quirks then. >>> >>> The users reported that this patch does fix the S3 issue, but the S4 still >>> fails. >>> >>> Please refer to [1] for more for user's testing result. >>> >>> [1] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1774950/comments/60 >> >> Please ask the users to test the patch below, then, on top of the $subject >> one. > > There was a mistake in the previous patch I posted, sorry about that. > > Please test this one instead: The result is positive, thanks! Kai-Heng > > --- > drivers/acpi/acpi_lpss.c | 26 +- > 1 file changed, 17 insertions(+), 9 deletions(-) > > Index: linux-pm/drivers/acpi/acpi_lpss.c > === > --- linux-pm.orig/drivers/acpi/acpi_lpss.c > +++ linux-pm/drivers/acpi/acpi_lpss.c > @@ -879,6 +879,7 @@ static void acpi_lpss_dismiss(struct dev > #define LPSS_GPIODEF0_DMA_LLP BIT(13) > > static DEFINE_MUTEX(lpss_iosf_mutex); > +static bool lpss_iosf_d3_entered; > > static void lpss_iosf_enter_d3_state(void) > { > @@ -921,6 +922,9 @@ static void lpss_iosf_enter_d3_state(voi > > iosf_mbi_modify(LPSS_IOSF_UNIT_LPIOEP, MBI_CR_WRITE, > LPSS_IOSF_GPIODEF0, value1, mask1); > + > + lpss_iosf_d3_entered = true; > + > exit: > mutex_unlock(&lpss_iosf_mutex); > } > @@ -935,6 +939,11 @@ static void lpss_iosf_exit_d3_state(void > > mutex_lock(&lpss_iosf_mutex); > > + if (!lpss_iosf_d3_entered) > + goto exit; > + > + lpss_iosf_d3_entered = false; > + > iosf_mbi_modify(LPSS_IOSF_UNIT_LPIOEP, MBI_CR_WRITE, > LPSS_IOSF_GPIODEF0, value1, mask1); > > @@ -944,13 +953,13 @@ static void lpss_iosf_exit_d3_state(void > iosf_mbi_modify(LPSS_IOSF_UNIT_LPIO1, MBI_CFG_WRITE, > LPSS_IOSF_PMCSR, value2, mask2); > > +exit: > mutex_unlock(&lpss_iosf_mutex); > } > > -static int acpi_lpss_suspend(struct device *dev, bool runtime) > +static int acpi_lpss_suspend(struct device *dev, bool wakeup) > { > struct lpss_private_data *pdata = acpi_driver_data(ACPI_COMPANION(dev)); > - bool wakeup = runtime || device_may_wakeup(dev); > int ret; > > if (pdata->dev_desc->flags & LPSS_SAVE_CTX) > @@ -963,14 +972,14 @@ static int acpi_lpss_suspend(struct devi >* wrong status for devices being about to be powered off. See >* lpss_iosf_enter_d3_state() for further information. >*/ > - if ((runtime || !pm_suspend_via_firmware()) && > + if (acpi_target_system_state() == ACPI_STATE_S0 && > lpss_quirks & LPSS_QUIRK_ALWAYS_POWER_ON && iosf_mbi_available()) > lpss_iosf_enter_d3_state(); > > return ret; > } > > -static int acpi_lpss_resume(struct device *dev, bool runtime) > +static int acpi_lpss_resume(struct device *dev) > { > struct lpss_private_data *pdata = acpi_driver_data(ACPI_COMPANION(dev)); > int ret; > @@ -979,8 +988,7 @@ static int acpi_lpss_resume(struct devic >* This call is kept first to be in symmetry with >* acpi_lpss_runtime_suspend() one. >*/ > - if ((runtime || !pm_resume_via_firmware()) && > - lpss_quirks & LPSS_QUIRK_ALWAYS_POWER_ON && iosf_mbi_available()) > + if (lpss_quirks & LPSS_QUIRK_ALWAYS_POWER_ON && iosf_mbi_available()) > lpss_iosf_exit_d3_state(); > > ret = acpi_dev_resume(dev); > @@ -1004,12 +1012,12 @@ static int acpi_lpss_suspend_late(struct > return 0; > > ret = pm_generic_suspend_late(dev); > - return ret ? ret : acpi_lpss_suspend(dev, false); > + return ret ? ret : acpi_lpss_suspend(dev, device_may_wakeup(dev)); > } > > static int acpi_lpss_resume_early(struct device *dev) > { > - int ret = acpi_lpss_resume(dev, false); > + int ret = acpi_lpss_resume(dev); > > return ret ? ret : pm_generic_resume_early(dev); > } > @@ -1024,7 +1032,7 @@ static int acpi_lpss_runtime_suspend(str > > static int acpi_lpss_runtime_resume(struct device *dev) > { > - int ret = acpi_lpss_resume(dev, true)
Re: [PATCH 3/6] irqchip: RISC-V Local Interrupt Controller Driver
On Wed, Jul 25, 2018 at 4:54 PM, Christoph Hellwig wrote: > On Wed, Jul 25, 2018 at 12:18:39PM +0100, Marc Zyngier wrote: >> This feels odd. It means that you cannot have the following sequence: >> >> local_irq_disable(); >> enable_irq(x); // where x is owned by a remote hart >> >> as smp_call_function_single() requires interrupts to be enabled. >> >> More fundamentally, why are you trying to make these interrupts look >> global while they aren't? arm/arm64 have similar restrictions with GICv2 >> and earlier, and treats these interrupts as per-cpu. >> >> Given that the drivers that deal with drivers connected to the per-hart >> irqchip are themselves likely to be aware of the per-cpu aspect, it >> would make sense to align things (we've been through that same >> discussion about the clocksource driver a few weeks back). > > Right now the only direct consumers are said clocksource, the PLIC > driver later in this series and the RISC-V arch IPI code. None of them > is going to do a manual enable_irq, so I guess the remote case of the > code is simply dead code. I'll take a look at converting them to > per-cpu. I guess the GICv2 driver is the best template? Actually, RISCV HLIC and PLIC are very similar to RPi2 and RPi3 SOCs. On RPi2 and RPi3, we have per-CPU BCM2836 local intc and the global interrupts are managed using BCM2835 intc. You should certainly have a look a this drivers because these very simple compared to GICv2 and GICv3 drivers. Regards, Anup
Re: [PATCH mmc-next 3/3] mmc: sdhci-of-dwcmshc: solve 128MB DMA boundary limitation
On Wed, 25 Jul 2018 17:47:49 +0800 Jisheng Zhang wrote: > When using DMA, if the DMA addr spans 128MB boundary, we have to split > the DMA transfer into two so that each one doesn't exceed the boundary. > > Signed-off-by: Jisheng Zhang > --- > drivers/mmc/host/sdhci-of-dwcmshc.c | 41 + > 1 file changed, 41 insertions(+) > > diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c > b/drivers/mmc/host/sdhci-of-dwcmshc.c > index 1b7cd144fb01..01b5cb772554 100644 > --- a/drivers/mmc/host/sdhci-of-dwcmshc.c > +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c > @@ -13,16 +13,45 @@ > > #include "sdhci-pltfm.h" > > +#define BOUNDARY_OK(addr, len) \ > + ((addr | (SZ_128M - 1)) == ((addr + len) | (SZ_128M - 1))) this should be ((addr | (SZ_128M - 1)) == ((addr + len - 1) | (SZ_128M - 1))) > + > struct dwcmshc_priv { > struct clk *bus_clk; > }; > > +/* > + * if DMA addr spans 128MB boundary, we split the DMA transfer into two > + * so that the DMA transfer doesn't exceed the boundary. > + */ > +static unsigned int dwcmshc_adma_write_desc(struct sdhci_host *host, > + void *desc, dma_addr_t addr, > + int len, unsigned int cmd) > +{ > + int tmplen, offset; > + > + if (BOUNDARY_OK(addr, len)) > + return _sdhci_adma_write_desc(host, desc, addr, len, cmd); > + > + offset = addr & (SZ_128M - 1); > + tmplen = SZ_128M - offset; > + _sdhci_adma_write_desc(host, desc, addr, tmplen, cmd); > + > + addr += tmplen; > + len -= tmplen; > + desc += host->desc_sz; > + _sdhci_adma_write_desc(host, desc, addr, len, cmd); > + > + return host->desc_sz * 2; > +} > + > static const struct sdhci_ops sdhci_dwcmshc_ops = { > .set_clock = sdhci_set_clock, > .set_bus_width = sdhci_set_bus_width, > .set_uhs_signaling = sdhci_set_uhs_signaling, > .get_max_clock = sdhci_pltfm_clk_get_max_clock, > .reset = sdhci_reset, > + .adma_write_desc= dwcmshc_adma_write_desc, > }; > > static const struct sdhci_pltfm_data sdhci_dwcmshc_pdata = { > @@ -36,12 +65,24 @@ static int dwcmshc_probe(struct platform_device *pdev) > struct sdhci_host *host; > struct dwcmshc_priv *priv; > int err; > + u32 extra; > > host = sdhci_pltfm_init(pdev, &sdhci_dwcmshc_pdata, > sizeof(struct dwcmshc_priv)); > if (IS_ERR(host)) > return PTR_ERR(host); > > + /* > + * The DMA descriptor table number is calculated as the maximum > + * number of segments times 2, to allow for an alignment > + * descriptor for each segment, plus 1 for a nop end descriptor, > + * plus extra number for cross 128M boundary handling. > + */ > + extra = totalram_pages / (SZ_128M / PAGE_SIZE) + 1; will use DIV_ROUND_UP instead. > + if (extra > SDHCI_MAX_SEGS) > + extra = SDHCI_MAX_SEGS; > + host->adma_table_num = SDHCI_MAX_SEGS * 2 + 1 + extra; > + > pltfm_host = sdhci_priv(host); > priv = sdhci_pltfm_priv(pltfm_host); >
Re: [PATCH 06/11] sched/irq: add irq utilization tracking
Hi Vincent, On Fri, 29 Jun 2018 at 03:07, Vincent Guittot wrote: > > interrupt and steal time are the only remaining activities tracked by > rt_avg. Like for sched classes, we can use PELT to track their average > utilization of the CPU. But unlike sched class, we don't track when > entering/leaving interrupt; Instead, we take into account the time spent > under interrupt context when we update rqs' clock (rq_clock_task). > This also means that we have to decay the normal context time and account > for interrupt time during the update. > > That's also important to note that because > rq_clock == rq_clock_task + interrupt time > and rq_clock_task is used by a sched class to compute its utilization, the > util_avg of a sched class only reflects the utilization of the time spent > in normal context and not of the whole time of the CPU. The utilization of > interrupt gives an more accurate level of utilization of CPU. > The CPU utilization is : > avg_irq + (1 - avg_irq / max capacity) * /Sum avg_rq > > Most of the time, avg_irq is small and neglictible so the use of the > approximation CPU utilization = /Sum avg_rq was enough > > Cc: Ingo Molnar > Cc: Peter Zijlstra > Signed-off-by: Vincent Guittot > --- > kernel/sched/core.c | 4 +++- > kernel/sched/fair.c | 13 ++--- > kernel/sched/pelt.c | 40 > kernel/sched/pelt.h | 16 > kernel/sched/sched.h | 3 +++ > 5 files changed, 72 insertions(+), 4 deletions(-) > > diff --git a/kernel/sched/core.c b/kernel/sched/core.c > index 78d8fac..e5263a4 100644 > --- a/kernel/sched/core.c > +++ b/kernel/sched/core.c > @@ -18,6 +18,8 @@ > #include "../workqueue_internal.h" > #include "../smpboot.h" > > +#include "pelt.h" > + > #define CREATE_TRACE_POINTS > #include > > @@ -186,7 +188,7 @@ static void update_rq_clock_task(struct rq *rq, s64 delta) > > #if defined(CONFIG_IRQ_TIME_ACCOUNTING) || > defined(CONFIG_PARAVIRT_TIME_ACCOUNTING) > if ((irq_delta + steal) && sched_feat(NONTASK_CAPACITY)) > - sched_rt_avg_update(rq, irq_delta + steal); > + update_irq_load_avg(rq, irq_delta + steal); I think we should not add steal time into irq load tracking, steal time is always 0 on native kernel which doesn't matter, what will happen when guest disables IRQ_TIME_ACCOUNTING and enables PARAVIRT_TIME_ACCOUNTING? Steal time is not the real irq util_avg. In addition, we haven't exposed power management for performance which means that e.g. schedutil governor can not cooperate with passive mode intel_pstate driver to tune the OPP. To decay the old steal time avg and add the new one just wastes cpu cycles. Regards, Wanpeng Li > #endif > } > > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c > index ffce4b2..d2758e3 100644 > --- a/kernel/sched/fair.c > +++ b/kernel/sched/fair.c > @@ -7289,7 +7289,7 @@ static inline bool cfs_rq_has_blocked(struct cfs_rq > *cfs_rq) > return false; > } > > -static inline bool others_rqs_have_blocked(struct rq *rq) > +static inline bool others_have_blocked(struct rq *rq) > { > if (READ_ONCE(rq->avg_rt.util_avg)) > return true; > @@ -7297,6 +7297,11 @@ static inline bool others_rqs_have_blocked(struct rq > *rq) > if (READ_ONCE(rq->avg_dl.util_avg)) > return true; > > +#if defined(CONFIG_IRQ_TIME_ACCOUNTING) || > defined(CONFIG_PARAVIRT_TIME_ACCOUNTING) > + if (READ_ONCE(rq->avg_irq.util_avg)) > + return true; > +#endif > + > return false; > } > > @@ -7361,8 +7366,9 @@ static void update_blocked_averages(int cpu) > } > update_rt_rq_load_avg(rq_clock_task(rq), rq, 0); > update_dl_rq_load_avg(rq_clock_task(rq), rq, 0); > + update_irq_load_avg(rq, 0); > /* Don't need periodic decay once load/util_avg are null */ > - if (others_rqs_have_blocked(rq)) > + if (others_have_blocked(rq)) > done = false; > > #ifdef CONFIG_NO_HZ_COMMON > @@ -7431,9 +7437,10 @@ static inline void update_blocked_averages(int cpu) > update_cfs_rq_load_avg(cfs_rq_clock_task(cfs_rq), cfs_rq); > update_rt_rq_load_avg(rq_clock_task(rq), rq, 0); > update_dl_rq_load_avg(rq_clock_task(rq), rq, 0); > + update_irq_load_avg(rq, 0); > #ifdef CONFIG_NO_HZ_COMMON > rq->last_blocked_load_update_tick = jiffies; > - if (!cfs_rq_has_blocked(cfs_rq) && !others_rqs_have_blocked(rq)) > + if (!cfs_rq_has_blocked(cfs_rq) && !others_have_blocked(rq)) > rq->has_blocked_load = 0; > #endif > rq_unlock_irqrestore(rq, &rf); > diff --git a/kernel/sched/pelt.c b/kernel/sched/pelt.c > index 8b78b63..ead6d8b 100644 > --- a/kernel/sched/pelt.c > +++ b/kernel/sched/pelt.c > @@ -357,3 +357,43 @@ int update_dl_rq_load_avg(u64 now, struct rq *rq, int > running) > > return 0; > } > + > +#if defined(CONFIG_IRQ_TIME_ACCOUNTING) || > defined(CONFIG_PARAVIRT_TIME_ACC
Re: linux-next: build warning after merge of the rdma tree
On Thu, Jul 26, 2018 at 10:55:53AM +1000, Stephen Rothwell wrote: > Hi all, > > After merging the rdma tree, today's linux-next build (powerpc > ppc64_defconfig) produced this warning: > > net/sunrpc/xprtrdma/svc_rdma_rw.c: In function 'svc_rdma_post_chunk_ctxt': > net/sunrpc/xprtrdma/svc_rdma_rw.c:350:5: warning: 'bad_wr' may be used > uninitialized in this function [-Wmaybe-uninitialized] > if (bad_wr != first_wr) > ^ Huh. I'm quite surprised 0-day build service didn't warn on this. > Introduced by commit > > ed288d74a9e5 ("net/xprtrdma: Simplify ib_post_(send|recv|srq_recv)() calls") > > This is an actual problem. Yes, for sure. Bart? Thanks, Jason
[PATCH] checkpatch: Warn when a patch doesn't have a description
Potential patches should have a commit description. Emit a warning when there isn't one. Suggested-by: Prakruthi Deepak Heragu Signed-off-by: Joe Perches --- scripts/checkpatch.pl | 13 + 1 file changed, 13 insertions(+) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index b5c875d7132b..8b5f3dae31c9 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2240,6 +2240,7 @@ sub process { my $in_header_lines = $file ? 0 : 1; my $in_commit_log = 0; #Scanning lines before patch my $has_commit_log = 0; #Encountered lines before patch + my $commit_log_lines = 0; #Number of commit log lines my $commit_log_possible_stack_dump = 0; my $commit_log_long_line = 0; my $commit_log_has_diff = 0; @@ -2497,6 +2498,18 @@ sub process { $cnt_lines++ if ($realcnt != 0); +# Verify the existence of a commit log if appropriate +# 2 is used because a $signature is counted in $commit_log_lines + if ($in_commit_log) { + if ($line !~ /^\s*$/) { + $commit_log_lines++;#could be a $signature + } + } else if ($has_commit_log && $commit_log_lines < 2) { + WARN("COMMIT_MESSAGE", +"Missing commit description - Add an appropriate one\n"); + $commit_log_lines = 2; #warn only once + } + # Check if the commit log has what seems like a diff which can confuse patch if ($in_commit_log && !$commit_log_has_diff && (($line =~ m@^\s+diff\b.*a/[\w/]+@ &&
linux-next: build warning after merge of the sound-asoc tree
Hi all, After merging the sound-asoc tree, today's linux-next build (x86_64 allmodconfig) produced this warning: sound/soc/amd/acp-da7219-max98357a.c: In function 'cz_probe': sound/soc/amd/acp-da7219-max98357a.c:367:3: warning: 'ret' may be used uninitialized in this function [-Wmaybe-uninitialized] dev_err(&pdev->dev, "Failed to register regulator: %d\n", ^ ret); Introduced by commit 7b5317aa809f ("ASoC: AMD: Add a fix voltage regulator for DA7219 and ADAU7002") This is a real bug. -- Cheers, Stephen Rothwell pgpVMAoECqoIX.pgp Description: OpenPGP digital signature
Re: [PATCH 4.4 15/47] ubi: fastmap: Correctly handle interrupted erasures in EBA
On Tue, 2018-07-10 at 20:24 +0200, Greg Kroah-Hartman wrote: > 4.4-stable review patch. If anyone has any objections, please let me know. > > -- > > From: Richard Weinberger > > commit 781932375ffc6411713ee0926ccae8596ed0261c upstream. > > Fastmap cannot track the LEB unmap operation, therefore it can > happen that after an interrupted erasure the mapping still looks > good from Fastmap's point of view, while reading from the PEB will > cause an ECC error and confuses the upper layer. > > Instead of teaching users of UBI how to deal with that, we read back > the VID header and check for errors. If the PEB is empty or shows ECC > errors we fixup the mapping and schedule the PEB for erasure. [...] Isn't there a risk here, that a read error leads to erasing data that might be recoverable if the read is retried? Ben. -- Ben Hutchings, Software Developer Codethink Ltd https://www.codethink.co.uk/ Dale House, 35 Dale Street Manchester, M1 2HF, United Kingdom
Re: Zram writeback feature unstable with heavy swap utilization - BUG: Bad page state in process...
Hi Tino, On Wed, Jul 25, 2018 at 05:12:13PM +0200, Tino Lehnig wrote: > Hi, > > On 07/25/2018 03:21 PM, Minchan Kim wrote: > > It would be much helpful if you could check more versions with git-bisect. > > I started bisecting today, but my results are not conclusive yet. It is > certain that the problem started with 4.15 though. I have not encountered A thing I could imagine is [0bcac06f27d75, skip swapcache for swapin of synchronous device] It was merged into v4.15. Could you check it by bisecting? With enabling writeback feature of zram, it's no more synchonous device so it could affect the unresponsive symptom you are seeing now. I should disable synchronous flag when the zram works with writeback feature. However, I should still see why the PG_uptodate WARN turning on. I guess there are some places where assume all of anonymous pages have PG_swapbacked and !PG_dirty are in swapcache so reference accounting could be corrupted. > the bug message in 4.15-rc1 so far, but the kvm processes always became > unresponsive after hitting swap and could not be killed there. I saw the > same behavior in rc2, rc3, and other builds in between, but the bad state > bug would only trigger occasionally there. The behavior in 4.15.18 is the > same as in newer kernels. > > > I also want to reproduce it. > > > > Today, I downloaded one window iso and execute it as cdrom with my owned > > compiled kernel on KVM but I couldn't reproduce. > > I also tested some heavy swap workload(kernel build with multiple CPU > > on small memory) but I failed to reproduce, too. > > > > Please could you told me your method more detail? > > I found that running Windows in KVM really is the only reliable method, > maybe because the zero pages are easily compressible. There is actually not > a lot of disk utilization on the backing device when running this test. Hmm, my testing was creating a tons of disk IO on backing device. Maybe, that's why I couldn't reproduce a lot. zero pages could never go to the writeback into the device so I don't understand how such kinds of testing reprodcue the problem. Anyway, I will try it again with zero pages with a few of random pages. > > My operating system is a minimal install of Debian 9. I took the kernel > configuration from the default Debian kernel and built my own kernel with > "make oldconfig" leaving all settings at their defaults. The only thing I > changed in the configuration was enabling the zram writeback feature. You mean you changed host kernel configuration? > > All my tests were done on bare-metal hardware with Xeon processors and lots > of RAM. I encounter the bug quite quickly, but it still takes several GBs of > swap usage. Below is my /proc/meminfo with enough KVM instances running (3 > in my case) to trigger the bug on my test machine. Aha.. you did writeback feature into your bare-metal host machine and execute kvm with window images as a guest. So, PG_uptodate warning happens on host side, not guest? Right? Thanks! > > I will also try to reproduce the problem on some different hardware next. > > -- > > MemTotal: 264033384 kB > MemFree: 1232968 kB > MemAvailable: 0 kB > Buffers:1152 kB > Cached: 5036 kB > SwapCached:49200 kB > Active: 249955744 kB > Inactive:5096148 kB > Active(anon): 249953396 kB > Inactive(anon): 5093084 kB > Active(file): 2348 kB > Inactive(file): 3064 kB > Unevictable: 0 kB > Mlocked: 0 kB > SwapTotal: 1073741820 kB > SwapFree: 938603260 kB > Dirty:68 kB > Writeback: 0 kB > AnonPages: 255007752 kB > Mapped: 4708 kB > Shmem: 1212 kB > Slab: 88500 kB > SReclaimable: 16096 kB > SUnreclaim:72404 kB > KernelStack:5040 kB > PageTables: 765560 kB > NFS_Unstable: 0 kB > Bounce:0 kB > WritebackTmp: 0 kB > CommitLimit:1205758512 kB > Committed_AS: 403586176 kB > VmallocTotal: 34359738367 kB > VmallocUsed: 0 kB > VmallocChunk: 0 kB > HardwareCorrupted: 0 kB > AnonHugePages: 254799872 kB > ShmemHugePages:0 kB > ShmemPmdMapped:0 kB > HugePages_Total: 0 > HugePages_Free:0 > HugePages_Rsvd:0 > HugePages_Surp:0 > Hugepagesize: 2048 kB > DirectMap4k: 75136 kB > DirectMap2M:10295296 kB > DirectMap1G:260046848 kB > > -- > Kind regards, > > Tino Lehnig
RE: [PATCH V4 0/9] clk: add imx7ulp clk support
Hi Stephen, Do you have a chance to look at it? This patch series has been pending for quite a long time without much comments. Regards Dong Aisheng > -Original Message- > From: A.s. Dong > Sent: Wednesday, July 18, 2018 9:37 PM > To: linux-...@vger.kernel.org > Cc: linux-kernel@vger.kernel.org; linux-arm-ker...@lists.infradead.org; > sb...@kernel.org; mturque...@baylibre.com; shawn...@kernel.org; > Anson Huang ; Jacky Bai ; dl- > linux-imx ; A.s. Dong > Subject: [PATCH V4 0/9] clk: add imx7ulp clk support > > This is a rebased version of below patch series against latest clk tree. > [PATCH RESEND V3 0/9] clk: add imx7ulp clk support > https://lkml.org/lkml/2018/3/16/310 > > This patch series intends to add imx7ulp clk support. > > i.MX7ULP Clock functions are under joint control of the System Clock > Generation (SCG) modules, Peripheral Clock Control (PCC) modules, and > Core Mode Controller (CMC)1 blocks > > The clocking scheme provides clear separation between M4 domain and A7 > domain. Except for a few clock sources shared between two domains, such > as the System Oscillator clock, the Slow IRC (SIRC), and and the Fast IRC > clock > (FIRCLK), clock sources and clock management are separated and contained > within each domain. > > M4 clock management consists of SCG0, PCC0, PCC1, and CMC0 modules. > A7 clock management consists of SCG1, PCC2, PCC3, and CMC1 modules. > > Note: this series only adds A7 clock domain support as M4 clock domain will > be handled by M4 seperately. > > Change Log: > v3->v4: > * rebased to latest kernel > * make scg and pcc separate nodes according to Rob's suggestion > > v2->v3: > * Patch 1 changed on: 1) split normal and gate ops 2) fix the possible racy >Others no changes. > > v1->v2: > * add enable/disable for the type of CLK_DIVIDER_ZERO_GATE dividers > * use clk_hw apis to register clocks > * use of_clk_add_hw_provider > * split the clocks register process into two parts: early part for possible >timers clocks registered by CLK_OF_DECLARE_DRIVER and the later part for >the left normal peripheral clocks registered by a platform driver. > > Dong Aisheng (9): > clk: clk-divider: add CLK_DIVIDER_ZERO_GATE clk support > clk: fractional-divider: add CLK_FRAC_DIVIDER_ZERO_BASED flag support > clk: imx: add pllv4 support > clk: imx: add pfdv2 support > clk: imx: add composite clk support > dt-bindings: clock: add imx7ulp clock binding doc > clk: imx: make mux parent strings const > clk: imx: implement new clk_hw based APIs > clk: imx: add imx7ulp clk driver > > .../devicetree/bindings/clock/imx7ulp-clock.txt| 87 + > drivers/clk/clk-divider.c | 152 +++ > drivers/clk/clk-fractional-divider.c | 10 + > drivers/clk/imx/Makefile | 6 +- > drivers/clk/imx/clk-busy.c | 2 +- > drivers/clk/imx/clk-composite.c| 85 + > drivers/clk/imx/clk-fixup-mux.c| 2 +- > drivers/clk/imx/clk-imx7ulp.c | 209 > + > drivers/clk/imx/clk-pfdv2.c| 201 > drivers/clk/imx/clk-pllv4.c| 182 ++ > drivers/clk/imx/clk.c | 22 +++ > drivers/clk/imx/clk.h | 92 - > include/dt-bindings/clock/imx7ulp-clock.h | 109 +++ > include/linux/clk-provider.h | 17 ++ > 14 files changed, 1166 insertions(+), 10 deletions(-) create mode 100644 > Documentation/devicetree/bindings/clock/imx7ulp-clock.txt > create mode 100644 drivers/clk/imx/clk-composite.c create mode 100644 > drivers/clk/imx/clk-imx7ulp.c create mode 100644 drivers/clk/imx/clk- > pfdv2.c create mode 100644 drivers/clk/imx/clk-pllv4.c create mode 100644 > include/dt-bindings/clock/imx7ulp-clock.h > > -- > 2.7.4
Re: [PATCH v2] hexagon: switch to NO_BOOTMEM
On Wed, Jul 25, 2018 at 08:29:54AM +0300, Mike Rapoport wrote: > This patch adds registration of the system memory with memblock, eliminates > bootmem initialization and converts early memory reservations from bootmem > to memblock. > > Signed-off-by: Mike Rapoport > --- > v2: fix calculation of the reserved memory size > > arch/hexagon/Kconfig | 3 +++ > arch/hexagon/mm/init.c | 20 > 2 files changed, 11 insertions(+), 12 deletions(-) > Looks good, I can take this through my tree. Acked-by: Richard Kuo -- Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v1] checkpatch: check for #if 0/#if 1
On Wed, 2018-07-25 at 16:14 -0700, Prakruthi Deepak Heragu wrote: > The #if 0 or #if 1 is used to toggle features. Warn if #if 0 or #if 1 > is present and suggest that they can be removed. > > Signed-off-by: Abhijeet Dharmapurikar > Signed-off-by: Prakruthi Deepak Heragu > --- > Changes in v1: > - Rephrase the warning message to fit in a single line without > 80 column limit > > scripts/checkpatch.pl | 11 --- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > index 3394ed8..72513c2 100755 > --- a/scripts/checkpatch.pl > +++ b/scripts/checkpatch.pl > @@ -5383,9 +5383,14 @@ sub process { > > # warn about #if 0 > if ($line =~ /^.\s*\#\s*if\s+0\b/) { > - CHK("REDUNDANT_CODE", > - "if this code is redundant consider removing it\n" . > - $herecurr); > + WARN("IF_0", > + "Consider removing the #if 0 and its #endif\n". > $herecurr); No, this wording is not correct at all. The entire block, i.e.: 'this code' should be considered to be removed, not just the #if 0 and its #endif. For #if 1 code, then just the #if 1 and the #endif should be removed. > + } > + > +# warn about #if 1 > + if ($line =~ /^.\s*\#\s*if\s+1\b/) { > + WARN("IF_1", > + "Consider removing the #if 1 and its #endif\n". > $herecurr); > } > > # check for needless "if () fn()" uses
Re: [RFC][PATCH 0/5] Mount, Filesystem and Keyrings notifications
On Wed, 2018-07-25 at 08:48 -0700, Casey Schaufler wrote: > On 7/24/2018 10:39 PM, Ian Kent wrote: > > On Tue, 2018-07-24 at 11:57 -0700, Casey Schaufler wrote: > > > On 7/24/2018 9:00 AM, David Howells wrote: > > > > Casey Schaufler wrote: > > > > > > > > > > (1) Mount topology and reconfiguration change events. > > > > > > > > > > With the possibility of unprivileged mounting you're going to have to > > > > > address access control on events. If root in a user namespace mounts > > > > > a > > > > > filesystem you may have a case where the "real" user wouldn't want the > > > > > listener to receive a notification. > > > > > > > > Can you clarify who the listener is in this case? > > > > > > That would be anyone with a watchpoint set. > > > > And that process would have had the privilege to do so ... > > Which isn't the point. The access control isn't on the watchpoint, > it's on delivering the event to the watchpoint. The access control > needs to be based on the process that created the event and the > process receiving the event. > > > > > > > Note that mount topology events don't leak outside of the mount > > > > namespace > > > > they're generated in. > > > > > > > > That said, if you, a random user, put a watchpoint on "/" you can see > > > > the > > > > mount events triggered by another random user in the same mount > > > > namespace. I > > > > don't see a way to control this except by resorting to the LSM since > > > > UNIX > > > > doesn't have 'notify' permission bits. > > > > > > I would call that a write operation from the process that triggered > > > the watchpoint to the one watching it. Like a signal. Signals have a > > > rudimentary DAC policy (write only to the same UID) that could be > > > your model. > > > > I'm not sure signals are a good comparison. > > In both cases you have one process sending information > to another. If you use killpg() instead of kill() you can > send to a number of processes without knowing them individually. > A process can chose what to do with (most) signals, including > ignore them. I'm just saying that the analogy isn't quite the same. In the this case notifications can't be just sent by a process, it's entirely controlled by the VFS so the notion of some process doing something out-off-band doesn't apply. > > > They can affect a process in significant ways whereas triggering > > a notification is less invasive so the security requirements > > should take that into consideration. > > I'm looking at this from a security model viewpoint. If process > A sends information to process B that's a write operation with > A as the subject and B as the object. Perhaps, but again there's no process A deciding to send something. It's the VFS saying to itself, I see this thing has changed, someone that had appropriate privilege asked me to tell it so ... I may be wrong but there isn't a similar access control mechanism on the proc file system mount table (pseudo) files, only the usual access control on opening them and what is seen is based on the mount name space of the opening process. This is also not quite as what we have here but it is similar. > > > But there is a problem here I think. > > > > How about the case where a user name space is created or entered > > without a newly created mount name space and mounts and umounts > > are done, the user name space necessarily expects the table of > > mounts it sees to be up to date. > > > > But, if the methods here are used by user space, say libmount > > was updated to use it, to gain the efficiency of not constantly > > re-reading the proc mount table then restrictions on notifications > > would mean the mount table seen in the user name space might not > > be updated and would no longer be correct. > > Hey, I'm not the one saying that containers don't need/want kernel > manifestation. Of course you may have troubles with access consistency > if you go around changing the access control attributes with namespaces. > One of the problems with signals is that you can't chmod() your process > to allow signals from other specified users. If you don't design an > access control scheme into the watchpoint mechanism you aren't going to > be able to deal with situations like this. Don't get me wrong, I'm not criticizing your suggestion that some sort of security model is needed. I'm saying it's not straight forward to work out what's actually needed and, in thinking about it, I ended up described an additional potential problem that's not even (necessarily) security related. > > > The converse is more interesting, where the user name space does > > create or enter a new mount name space, then libmount would see ???, > > probably not the updated mount information ... unless it opens a > > new file handle to get mount update information ... a long running > > daemon that uses libmount and dispenses or uses mount information > > would very likely have a problem ... > > > > The current proc file sys
Re: [PATCH 1/2] tracing: kprobes: Prohibit probing on notrace functions
On Thu, 26 Jul 2018 09:41:06 +0900 Masami Hiramatsu wrote: > On Fri, 13 Jul 2018 08:18:03 -0400 > Steven Rostedt wrote: > > > On Fri, 13 Jul 2018 11:53:01 +0900 > > Masami Hiramatsu wrote: > > > > > On Thu, 12 Jul 2018 13:54:12 -0400 > > > Francis Deslauriers wrote: > > > > > > > From: Masami Hiramatsu > > > > > > > > Prohibit kprobe-events probing on notrace function. > > > > Since probing on the notrace function can cause recursive > > > > event call. In most case those are just skipped, but > > > > in some case it falls into infinite recursive call. > > > > > > BTW, I'm considering to add an option to allow putting > > > kprobes on notrace function - just for debugging > > > ftrace by kprobes. That is "developer only" option > > > so generally it should be disabled, but for debugging > > > the ftrace, we still need it. Or should I introduce > > > another kprobes module for debugging it? > > > > No, I think the former is better (to add an option to allow putting > > kprobes on notrace functions). By default we let people protect > > themselves. But if then provide a switch that lets you do things that > > might let you shoot yourself in the foot. > > I'm adding CONFIG_KPROBE_EVENTS_ON_NOTRACE kconfig which allows > kprobes on notrace function. I think we don't need to make it > online switchable, since it is only good for ftrace developers. > Works for me. Thanks! -- Steve
Re: [PATCH v5 1/3] thermal: qcom-spmi: Use PMIC thermal stage 2 for critical trip points
Hi Doug, On Wed, Jul 25, 2018 at 04:19:56PM -0700, Doug Anderson wrote: > On Tue, Jul 24, 2018 at 4:46 PM, Matthias Kaehlcke wrote: > > +static int qpnp_tm_update_critical_trip_temp(struct qpnp_tm_chip *chip, > > +int temp) > > +{ > > + u8 reg; > > + bool disable_s2_shutdown = false; > > + int ret; > > + > > + WARN_ON(!mutex_is_locked(&chip->lock)); > > + > > + /* > > +* Default: S2 and S3 shutdown enabled, thresholds at > > +* 105C/125C/145C, monitoring at 25Hz > > +*/ > > + reg = SHUTDOWN_CTRL1_RATE_25HZ; > > + > > + if ((temp == THERMAL_TEMP_INVALID) || > > + (temp < STAGE2_THRESHOLD_MIN)) { > > + chip->thresh = THRESH_MIN; > > + goto skip; > > + } > > + > > + if (temp <= STAGE2_THRESHOLD_MAX) { > > + chip->thresh = THRESH_MAX - > > + ((STAGE2_THRESHOLD_MAX - temp) / > > +TEMP_THRESH_STEP); > > + disable_s2_shutdown = true; > > + } else { > > + chip->thresh = THRESH_MAX; > > + > > + if (!IS_ERR(chip->adc)) > > + disable_s2_shutdown = true; > > + else > > + dev_warn(chip->dev, > > +"No ADC is configured and critical > > temperature is above the maximum stage 2 threshold of 140°C! Configuring > > stage 2 shutdown at 140°C.\n"); > > Putting a non-ASCII character (the degree symbol) in your commit > message is one thing, but are you sure it's wise to put it in the > kernel logs? A few other drivers also do this (drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c, drivers/macintosh/windfarm_pm121.c), however that doesn't mean it's a good idea. Will change to degC or C. > > + } > > + > > +skip: > > + reg |= chip->thresh; > > + if (disable_s2_shutdown) > > + reg |= SHUTDOWN_CTRL1_OVERRIDE_S2; > > + > > + ret = qpnp_tm_write(chip, QPNP_TM_REG_SHUTDOWN_CTRL1, reg); > > + if (ret < 0) > > + return ret; > > + > > + return ret; > > Simplify the above lines to: > > return qpnp_tm_write(chip, QPNP_TM_REG_SHUTDOWN_CTRL1, reg); Ouch, my code is indeed dumb ... > > @@ -313,12 +441,7 @@ static int qpnp_tm_probe(struct platform_device *pdev) > > if (ret < 0) > > return ret; > > > > - chip->tz_dev = devm_thermal_zone_of_sensor_register(&pdev->dev, 0, > > chip, > > - > > &qpnp_tm_sensor_ops); > > - if (IS_ERR(chip->tz_dev)) { > > - dev_err(&pdev->dev, "failed to register sensor\n"); > > - return PTR_ERR(chip->tz_dev); > > - } > > + chip->initialized = true; > > Should we add "thermal_zone_device_update(chip->tz_dev, > THERMAL_EVENT_UNSPECIFIED);" here Seems reasonable, will do. > ...also: do we care about any type of locking for chip->initialized? > Technically we can be running on weakly ordered memory so if > qpnp_tm_update_temp_no_adc() is running on a different processor then > possibly it could still keep returning the default temperature for a > little while. We could try to analyze whether there's some sort of > implicit barrier or we could add manual memory barriers, but generally > I try to avoid that and just do the simple locking... What about just > setting chip-Initialized = true at the end of qpnp_tm_init() while the > mutex is still held? Thanks for pointing that out. I agree that we should keep things simple, chip->initialized to true at the end of qpnp_tm_init() sounds good to me. > I'd also love to hear from someone with more thermal framework > experience to make sure it's legit to return a default value if > someone calls us while we're initting. It seems sane to me but nice > to confirm it's OK. An alternative could be to return THERMAL_TEMP_INVALID, however I don't see this handled outside of thermal_core.c, not sure if it could throw some other code off. Comments from thermal folks on either approach (or alternatives) are definitely welcome :) > Overall I like the idea of this patch so hopefully others do too. > Thanks for sending it out! Thanks for the review! Matthias
Re: [PATCH 0/10] psi: pressure stall information for CPU, memory, and IO v2
On 7/25/18 1:15 AM, Johannes Weiner wrote: > Hi Balbir, > > On Tue, Jul 24, 2018 at 07:14:02AM +1000, Balbir Singh wrote: >> Does the mechanism scale? I am a little concerned about how frequently >> this infrastructure is monitored/read/acted upon. > > I expect most users to poll in the frequency ballpark of the running > averages (10s, 1m, 5m). Our OOMD defaults to 5s polling of the 10s > average; we collect the 1m average once per minute from our machines > and cgroups to log the system/workload health trends in our fleet. > > Suren has been experimenting with adaptive polling down to the > millisecond range on Android. > I think this is a bad way of doing things, polling only adds to overheads, there needs to be an event driven mechanism and the selection of the events need to happen in user space. >> Why aren't existing mechanisms sufficient > > Our existing stuff gives a lot of indication when something *may* be > an issue, like the rate of page reclaim, the number of refaults, the > average number of active processes, one task waiting on a resource. > > But the real difference between an issue and a non-issue is how much > it affects your overall goal of making forward progress or reacting to > a request in time. And that's the only thing users really care > about. It doesn't matter whether my system is doing 2314 or 6723 page > refaults per minute, or scanned 8495 pages recently. I need to know > whether I'm losing 1% or 20% of my time on overcommitted memory. > > Delayacct is time-based, so it's a step in the right direction, but it > doesn't aggregate tasks and CPUs into compound productivity states to > tell you if only parts of your workload are seeing delays (which is > often tolerable for the purpose of ensuring maximum HW utilization) or > your system overall is not making forward progress. That aggregation > isn't something you can do in userspace with polled delayacct data. By aggregation you mean cgroup aggregation? > >> -- why is the avg delay calculation in the kernel? > > For one, as per above, most users will probably be using the standard > averaging windows, and we already have this highly optimizd > infrastructure from the load average. I don't see why we shouldn't use > that instead of exporting an obscure number that requires most users > to have an additional library or copy-paste the loadavg code. > > I also mentioned the OOM killer as a likely in-kernel user of the > pressure percentages to protect from memory livelocks out of the box, > in which case we have to do this calculation in the kernel anyway. > >> There is no talk about the overhead this introduces in general, may be >> the details are in the patches. I'll read through them > > I sent an email on benchmarks and overhead in one of the subthreads, I > will include that information in the cover letter in v3. > > https://lore.kernel.org/lkml/20180718215644.gb2...@cmpxchg.org/ Thanks, I'll take a look Balbir Singh.
Re: [PATCH] tracing: do not leak kernel addresses
On Wed, 25 Jul 2018 13:22:36 -0700 Mark Salyzyn wrote: > From: Nick Desaulniers > > Switch from 0x%lx to 0x%pK to print the kernel addresses. > > Fixes: CVE-2017-0630 Wait This breaks perf and trace-cmd! They require this to be able to print various strings in trace events. This file is root read only, as the CVE says. NAK for this fix. Come up with something that doesn't break perf and trace-cmd. That will not be trivial, as the format is stored in the ring buffer with an address, then referenced directly. It also handles trace_printk() functions that simply point to the string format itself. A fix would require having a pointer be the same that is referenced inside the kernel as well as in this file. Maybe make the format string placed in a location that doesn't leak where the rest of the kernel exists? -- Steve > Signed-off-by: Mark Salyzyn > Cc: Nick Desaulniers > Cc: Steven Rostedt > Cc: Ingo Molnar > Cc: > Cc: # 3.18, 4.4, 4.9, 4.14 > Cc: > > --- > kernel/trace/trace_printk.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/kernel/trace/trace_printk.c b/kernel/trace/trace_printk.c > index ad1d6164e946..93698023baf1 100644 > --- a/kernel/trace/trace_printk.c > +++ b/kernel/trace/trace_printk.c > @@ -304,7 +304,7 @@ static int t_show(struct seq_file *m, void *v) > if (!*fmt) > return 0; > > - seq_printf(m, "0x%lx : \"", *(unsigned long *)fmt); > + seq_printf(m, "0x%pK : \"", *(unsigned long *)fmt); > > /* >* Tabs and new lines need to be converted.
linux-next: build warning after merge of the rdma tree
Hi all, After merging the rdma tree, today's linux-next build (powerpc ppc64_defconfig) produced this warning: net/sunrpc/xprtrdma/svc_rdma_rw.c: In function 'svc_rdma_post_chunk_ctxt': net/sunrpc/xprtrdma/svc_rdma_rw.c:350:5: warning: 'bad_wr' may be used uninitialized in this function [-Wmaybe-uninitialized] if (bad_wr != first_wr) ^ Introduced by commit ed288d74a9e5 ("net/xprtrdma: Simplify ib_post_(send|recv|srq_recv)() calls") This is an actual problem. -- Cheers, Stephen Rothwell pgpBpaF3FlVqx.pgp Description: OpenPGP digital signature
Re: Showing /sys/fs/cgroup/memory/memory.stat very slow on some machines
On 7/19/18 3:40 AM, Bruce Merry wrote: > On 18 July 2018 at 17:49, Shakeel Butt wrote: >> On Wed, Jul 18, 2018 at 8:37 AM Bruce Merry wrote: >>> That sounds promising. Is there any way to tell how many zombies there >>> are, and is there any way to deliberately create zombies? If I can >>> produce zombies that might give me a reliable way to reproduce the >>> problem, which could then sensibly be tested against newer kernel >>> versions. >>> >> >> Yes, very easy to produce zombies, though I don't think kernel >> provides any way to tell how many zombies exist on the system. >> >> To create a zombie, first create a memcg node, enter that memcg, >> create a tmpfs file of few KiBs, exit the memcg and rmdir the memcg. >> That memcg will be a zombie until you delete that tmpfs file. > > Thanks, that makes sense. I'll see if I can reproduce the issue. Do > you expect the same thing to happen with normal (non-tmpfs) files that > are sitting in the page cache, and/or dentries? > Do you by any chance have use_hierarch=1? memcg_stat_show should just rely on counters inside the memory cgroup and the the LRU sizes for each node. Balbir Singh.
Re: [PATCH v4 2/3] Input: edt-ft5x06 - Set wake/reset values on resume/suspend
Hi Mylène, On Wed, Jul 25, 2018 at 09:34:09AM +0200, Mylène Josserand wrote: > On resume and suspend, set the value of wake and reset gpios > to be sure that we are in a know state after suspending/resuming. > > Signed-off-by: Mylène Josserand > --- > drivers/input/touchscreen/edt-ft5x06.c | 12 > 1 file changed, 12 insertions(+) > > diff --git a/drivers/input/touchscreen/edt-ft5x06.c > b/drivers/input/touchscreen/edt-ft5x06.c > index dcde719094f7..dad2f1f8bf89 100644 > --- a/drivers/input/touchscreen/edt-ft5x06.c > +++ b/drivers/input/touchscreen/edt-ft5x06.c > @@ -1158,6 +1158,12 @@ static int __maybe_unused edt_ft5x06_ts_suspend(struct > device *dev) > else > regulator_disable(tsdata->vcc); > > + if (tsdata->wake_gpio) > + gpiod_set_value(tsdata->wake_gpio, 0); > + > + if (tsdata->reset_gpio) > + gpiod_set_value(tsdata->reset_gpio, 1); Ondřej mentioned in previous review that if you power off the controller it will not be able to wake up the system, and you had to move call to regulator_disable() into "else" branch of check whether the controller is a wakeup device. Guess what happens if you unconditionally put the device into reset state? Thanks. -- Dmitry
[PATCH v2] reset: hisilicon: fix potential NULL pointer dereference
There is a potential execution path in which function platform_get_resource() returns NULL. If this happens, we will end up having a NULL pointer dereference. Fix this by replacing devm_ioremap with devm_ioremap_resource, which has the NULL check and the memory region request. This code was detected with the help of Coccinelle. Cc: sta...@vger.kernel.org Fixes: 97b7129cd2af ("reset: hisilicon: change the definition of hisi_reset_init") Signed-off-by: Gustavo A. R. Silva --- Changes in v2: - Use devm_ioremap_resource. Thanks to Stephen Boyd for pointing it out. drivers/clk/hisilicon/reset.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/clk/hisilicon/reset.c b/drivers/clk/hisilicon/reset.c index 2a5015c..43e82fa 100644 --- a/drivers/clk/hisilicon/reset.c +++ b/drivers/clk/hisilicon/reset.c @@ -109,9 +109,8 @@ struct hisi_reset_controller *hisi_reset_init(struct platform_device *pdev) return NULL; res = platform_get_resource(pdev, IORESOURCE_MEM, 0); - rstc->membase = devm_ioremap(&pdev->dev, - res->start, resource_size(res)); - if (!rstc->membase) + rstc->membase = devm_ioremap_resource(&pdev->dev, res); + if (IS_ERR(rstc->membase)) return NULL; spin_lock_init(&rstc->lock); -- 2.7.4
Re: [PATCH v4 1/3] Input: edt-ft5x06 - Add support for regulator
Hi Mylène, On Wed, Jul 25, 2018 at 09:34:08AM +0200, Mylène Josserand wrote: > Add the support of regulator to use it as VCC source. > > Signed-off-by: Mylène Josserand > Reviewed-by: Rob Herring > --- > .../bindings/input/touchscreen/edt-ft5x06.txt | 1 + > drivers/input/touchscreen/edt-ft5x06.c | 43 > ++ > 2 files changed, 44 insertions(+) > > diff --git > a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt > b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt > index 025cf8c9324a..48e975b9c1aa 100644 > --- a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt > +++ b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt > @@ -30,6 +30,7 @@ Required properties: > Optional properties: > - reset-gpios: GPIO specification for the RESET input > - wake-gpios: GPIO specification for the WAKE input > + - vcc-supply: Regulator that supplies the touchscreen > > - pinctrl-names: should be "default" > - pinctrl-0: a phandle pointing to the pin settings for the > diff --git a/drivers/input/touchscreen/edt-ft5x06.c > b/drivers/input/touchscreen/edt-ft5x06.c > index 1e18ca0d1b4e..dcde719094f7 100644 > --- a/drivers/input/touchscreen/edt-ft5x06.c > +++ b/drivers/input/touchscreen/edt-ft5x06.c > @@ -39,6 +39,7 @@ > #include > #include > #include > +#include > > #define WORK_REGISTER_THRESHOLD 0x00 > #define WORK_REGISTER_REPORT_RATE0x08 > @@ -91,6 +92,7 @@ struct edt_ft5x06_ts_data { > struct touchscreen_properties prop; > u16 num_x; > u16 num_y; > + struct regulator *vcc; > > struct gpio_desc *reset_gpio; > struct gpio_desc *wake_gpio; > @@ -963,6 +965,13 @@ edt_ft5x06_ts_set_regs(struct edt_ft5x06_ts_data *tsdata) > } > } > > +static void edt_ft5x06_disable_regulator(void *arg) > +{ > + struct edt_ft5x06_ts_data *data = arg; > + > + regulator_disable(data->vcc); > +} > + > static int edt_ft5x06_ts_probe(struct i2c_client *client, >const struct i2c_device_id *id) > { > @@ -991,6 +1000,28 @@ static int edt_ft5x06_ts_probe(struct i2c_client > *client, > > tsdata->max_support_points = chip_data->max_support_points; > > + tsdata->vcc = devm_regulator_get(&client->dev, "vcc"); > + if (IS_ERR(tsdata->vcc)) { > + error = PTR_ERR(tsdata->vcc); > + if (error != -EPROBE_DEFER) > + dev_err(&client->dev, "failed to request regulator: > %d\n", > + error); > + return error; > + } > + > + error = regulator_enable(tsdata->vcc); > + if (error < 0) { > + dev_err(&client->dev, "failed to enable vcc: %d\n", > + error); > + return error; > + } It is better to put the chip into reset and then power up the regulatori and take it out of the reset, rather than power up and then toggle reset on and off. > + > + error = devm_add_action_or_reset(&client->dev, > + edt_ft5x06_disable_regulator, > + tsdata); > + if (error) > + return error; > + > tsdata->reset_gpio = devm_gpiod_get_optional(&client->dev, >"reset", GPIOD_OUT_HIGH); > if (IS_ERR(tsdata->reset_gpio)) { > @@ -1120,9 +1151,12 @@ static int edt_ft5x06_ts_remove(struct i2c_client > *client) > static int __maybe_unused edt_ft5x06_ts_suspend(struct device *dev) > { > struct i2c_client *client = to_i2c_client(dev); > + struct edt_ft5x06_ts_data *tsdata = i2c_get_clientdata(client); > > if (device_may_wakeup(dev)) > enable_irq_wake(client->irq); > + else > + regulator_disable(tsdata->vcc); > > return 0; > } > @@ -1130,9 +1164,18 @@ static int __maybe_unused edt_ft5x06_ts_suspend(struct > device *dev) > static int __maybe_unused edt_ft5x06_ts_resume(struct device *dev) > { > struct i2c_client *client = to_i2c_client(dev); > + struct edt_ft5x06_ts_data *tsdata = i2c_get_clientdata(client); > + int ret; > > if (device_may_wakeup(dev)) > disable_irq_wake(client->irq); > + else { > + ret = regulator_enable(tsdata->vcc); > + if (ret < 0) { > + dev_err(dev, "failed to enable vcc: %d\n", ret); > + return ret; > + } > + } I believe I mentioned in other review that once you powered up the device, you need to restore its settings, include switching to factory mode, if it was in factory mode, and restoring threshold/gain/offset settings. Thanks. -- Dmitry
Re: [PATCH v1] arm64: dts: sdm845: enable tsens thermal zones
Hi Amit, On Wed, Jul 18, 2018 at 01:19:17PM +0530, Amit Kucheria wrote: > One thermal zone per cpu is defined > > Signed-off-by: Amit Kucheria > --- > arch/arm64/boot/dts/qcom/sdm845.dtsi | 170 > +++ > 1 file changed, 170 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi > b/arch/arm64/boot/dts/qcom/sdm845.dtsi > index 01ff146..a75be7c 100644 > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi > @@ -340,4 +340,174 @@ > }; > }; > }; > + > + thermal-zones { > + cpu0-thermal { > + polling-delay-passive = <250>; > + polling-delay = <1000>; In the context of the TSENS patches you mentioned that you are working on interrupt support. Can the polling delays be removed once that is merged? > + thermal-sensors = <&tsens0 1>; > + > + trips { > + cpu_alert0: trip0 { > + temperature = <75000>; > + hysteresis = <2000>; > + type = "passive"; > + }; > + > + cpu_crit0: trip1 { > + temperature = <11>; > + hysteresis = <1000>; > + type = "critical"; > + }; > + }; > + }; > + > + cpu1-thermal { > + polling-delay-passive = <250>; > + polling-delay = <1000>; > + > + thermal-sensors = <&tsens0 2>; > + > + trips { > + cpu_alert1: trip0 { > + temperature = <75000>; > + hysteresis = <2000>; > + type = "passive"; > + }; > + > + cpu_crit1: trip1 { > + temperature = <11>; > + hysteresis = <1000>; > + type = "critical"; > + }; > + }; > + }; > + > + cpu2-thermal { > + polling-delay-passive = <250>; > + polling-delay = <1000>; > + > + thermal-sensors = <&tsens0 3>; > + > + trips { > + cpu_alert2: trip0 { > + temperature = <75000>; > + hysteresis = <2000>; > + type = "passive"; > + }; > + > + cpu_crit2: trip1 { > + temperature = <11>; > + hysteresis = <1000>; > + type = "critical"; > + }; > + }; > + }; > + > + cpu3-thermal { > + polling-delay-passive = <250>; > + polling-delay = <1000>; > + > + thermal-sensors = <&tsens0 4>; > + > + trips { > + cpu_alert3: trip0 { > + temperature = <75000>; > + hysteresis = <2000>; > + type = "passive"; > + }; > + > + cpu_crit3: trip1 { > + temperature = <11>; > + hysteresis = <1000>; > + type = "critical"; > + }; > + }; > + }; > + > + cpu4-thermal { > + polling-delay-passive = <250>; > + polling-delay = <1000>; > + > + thermal-sensors = <&tsens0 7>; > + > + trips { > + cpu_alert4: trip0 { > + temperature = <75000>; > + hysteresis = <2000>; > + type = "passive"; > + }; > + > + cpu_crit4: trip1 { > + temperature = <11>; > + hysteresis = <1000>; > + type = "critical"; > + }; > + }; > + }; > + > + cpu5-thermal { > + polling-delay-passive = <250>; > + polling-delay = <1000>; > + > +
Re: [PATCH 1/2] tracing: kprobes: Prohibit probing on notrace functions
On Fri, 13 Jul 2018 08:18:03 -0400 Steven Rostedt wrote: > On Fri, 13 Jul 2018 11:53:01 +0900 > Masami Hiramatsu wrote: > > > On Thu, 12 Jul 2018 13:54:12 -0400 > > Francis Deslauriers wrote: > > > > > From: Masami Hiramatsu > > > > > > Prohibit kprobe-events probing on notrace function. > > > Since probing on the notrace function can cause recursive > > > event call. In most case those are just skipped, but > > > in some case it falls into infinite recursive call. > > > > BTW, I'm considering to add an option to allow putting > > kprobes on notrace function - just for debugging > > ftrace by kprobes. That is "developer only" option > > so generally it should be disabled, but for debugging > > the ftrace, we still need it. Or should I introduce > > another kprobes module for debugging it? > > No, I think the former is better (to add an option to allow putting > kprobes on notrace functions). By default we let people protect > themselves. But if then provide a switch that lets you do things that > might let you shoot yourself in the foot. I'm adding CONFIG_KPROBE_EVENTS_ON_NOTRACE kconfig which allows kprobes on notrace function. I think we don't need to make it online switchable, since it is only good for ftrace developers. Thank you, > > BTW, I'm now leaving on vacation. I'll be back on the 23rd and will be > looking for patches that I should be pulling in then. > > Thanks! > > -- Steve -- Masami Hiramatsu
Re: Reminder to review a few patches sent two weeks ago
On 2018-07-24 17:33, Joe Perches wrote: On Tue, 2018-07-24 at 14:56 -0700, pher...@codeaurora.org wrote: A reminder to review a few patches I had sent last week. Below are the links for the patches. https://lkml.org/lkml/2018/7/5/798 I have no fundamental object to this one, but the 80 column use is unnecessary and should be coalesced before it can be applied. Perhaps: # warn about #if 1 if ($line =~ /^.\s*\#\s*if\s+1\b/) { WARN("IF_1", "Consider removing the #if 1 and its #endif\n" . $herecurr); } http://lists-archives.com/linux-kernel/29168320-checkpatch-check-for-invalid-return-codes.html This one has I think too many existing uses of things like "return -1;" $ git grep -P "return\s*\-\d+\s*;" | wc -l 9929 How many of these are actually appropriate? I did go through a few of the files which return -1 in their functions, I observed that most of them were inappropriate and there was a case where actually the use of return -1 was incorrect(kernel/arch/ia64/mm/contig.c in the function find_bootmap_location()). We could actually catch such errors from now on if we use this patch. Also, no space is required between return and -1 by c90 and this should use $Int so it should be: if ($line =~ /\breturn\s*\-\$Int\s*;/) { etc...
Re: [PATCH] firmware: Avoid caching firmware when FW_OPT_NOCACHE is set
On Thu, Jul 19, 2018 at 02:25:21PM -0700, Rishabh Bhatnagar wrote: > When calling request_firmware_into_buf(), we pass the FW_OPT_NOCACHE > flag with the intent of skipping the caching mechanism of the > firmware loader. Unfortunately, that doesn't work, because > alloc_lookup_fw_priv() isn't told to _not_ add the struct > firmware_buf to the firmware cache (fwc) list. So when we call > request_firmware_into_buf() the second time, we find the buffer in > the cache and return it immediately without reloading. The code in question is not dealing with the firmware cache though, it is batched requests. Unfortunately the code is not very clear given the structure used is the struct firmware_cache, however if you look carefully you will see there are two struct list_head used: struct firmware_cache { /* firmware_buf instance will be added into the below list */ spinlock_t lock; struct list_head head; ... #ifdef CONFIG_PM_SLEEP /* * Names of firmware images which have been cached successfully * will be added into the below list so that device uncache * helper can trace which firmware images have been cached * before. */ spinlock_t name_lock; struct list_head fw_names; ... }; So the first struct list_head is used for batched firmware requests. It was the first patch of the work, it was introduced via commit one commit, while the actual firmware cache used for suspend/resume in another. Respectively they are commits: 1f2b79599ee8f ("firmware loader: always let firmware_buf own the pages buffer") 37276a51f803e ("firmware: introduce device_cache/uncache_fw_images") The ifdefery above clarifies this is a bit, however it is rather easy to fall into the trap to easily review this code and think they are the same thing. > This may break users of request_firmware_into_buf that are expecting > a fresh copy of the firmware to be reloaded into memory. No, this is not the reason why this will break users of request_firmware_into_buf(), its because the call is by design using device driver specific memory, so as a matter of fact, it would actually be a security issue: *Any* kernel code which makes a request for the same firmware name, if the call is carefully timed, could end up getting a pointer to the device's own memory area used for this firmware, and since request_firmware_into_buf() was used, and since today's only driver using this is with IO memory this is a bigger issue. What you state above alludes that the issue is that we want a fresh copy, this has nothing to do with a copy, this is device driver specific memory. > The existing > copy may either be modified by drivers, remote processors or even > freed. This is *also* true, specially since the only single Qualcomm driver using this *also* uses IO memory. And this is a small example of why also LSMs *should* be abe to have knowledge when this type of memory is used. > Fix fw_lookup_and_allocate_buf to not add to the fwc list > if FW_OPT_NOCACHE is set, and also don't do the lookup in the list. > > Fixes: 0e742e9271 ("firmware: provide infrastructure to make fw caching > optional") This commit does not exist, you made a typo, the last 1 should be a 5: 0e742e9275 NACK for a few reasons: a) The commit ID identified as it fixing is wrong b) The commit is not accurate as to the reason why this is an issue c) The commit does not ackowledge the severity of the issue d) I was hoping we can make the fix simpler As for c) and d) -- come to think of it, the fact that we didn't implement batched firmware requests with a const pointer instead means we have similar potential issues possible with othe kernel code also requesting the same firmware and potentially modifying it on the fly prior to a device processing it, therefore causing possibly unintended behaviour. So, initially I was inclined to just rip the batched firmware implementation completely for all callers. Unfortunately trying to do this revealed to me just how broken this was. For instance, the private buf kept around for the firmware when caching it is kept track by the using the above mentioned struct list head. This means that any firmware which intends to be used for firmware caching *must* also be added to the fw cache head list. And unfortunately the search for this entry is indexed by just the name. So uncache_firmnware() for instance uses lookup_fw_priv() and that in turn which just look for the first private buf with the associated firmware name. This means firmware which is cached assumes only one copy is kept around during suspend/resume cycle, and the batched firmware feature was collateral. So I don't think we can easily disable batched firmware requests without breaking caching, as such I think your patch in turn is what we need for now. Can you re-spin, fix the commit log with a proper explanation and commit ID it fixes? Luis > Signed-off-by: Vikram Mulukutla > Signed
Re: [PATCH v2] EDAC, sb_edac: Add support for systems with segmented PCI buses
Hi Boris, On 07/25/2018 05:22 AM, Borislav Petkov wrote: > On Tue, Jul 24, 2018 at 03:02:13PM -0400, Masayoshi Mizuma wrote: >> [*] KASAN report is as follows. > > That KASAN report is an arbitrary side-effect from the missing segmented > support so I ripped it out from the commit message and ended up > committing this: Thank you so much! - Masa > > --- > From: Masayoshi Mizuma > Date: Tue, 24 Jul 2018 15:02:13 -0400 > Subject: [PATCH] EDAC, sb_edac: Add support for systems with segmented PCI > buses > > Extend the driver to check whether segment number and bus number matches > when deciding how to group memory controller PCI devices to CPU sockets. > > Signed-off-by: Masayoshi Mizuma > Reviewed-by: Tony Luck > Cc: Mauro Carvalho Chehab > Cc: linux-edac > Link: http://lkml.kernel.org/r/20180724190213.26359-1-msys.miz...@gmail.com > [ Cleanup commit message. ] > Signed-off-by: Borislav Petkov > --- > drivers/edac/sb_edac.c | 17 - > 1 file changed, 12 insertions(+), 5 deletions(-) > > diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c > index 4a89c8093307..07726fb00321 100644 > --- a/drivers/edac/sb_edac.c > +++ b/drivers/edac/sb_edac.c > @@ -352,6 +352,7 @@ struct pci_id_table { > > struct sbridge_dev { > struct list_headlist; > + int seg; > u8 bus, mc; > u8 node_id, source_id; > struct pci_dev **pdev; > @@ -729,7 +730,8 @@ static inline int numcol(u32 mtr) > return 1 << cols; > } > > -static struct sbridge_dev *get_sbridge_dev(u8 bus, enum domain dom, int > multi_bus, > +static struct sbridge_dev *get_sbridge_dev(int seg, u8 bus, enum domain dom, > +int multi_bus, > struct sbridge_dev *prev) > { > struct sbridge_dev *sbridge_dev; > @@ -747,14 +749,15 @@ static struct sbridge_dev *get_sbridge_dev(u8 bus, enum > domain dom, int multi_bu > : sbridge_edac_list.next, struct > sbridge_dev, list); > > list_for_each_entry_from(sbridge_dev, &sbridge_edac_list, list) { > - if (sbridge_dev->bus == bus && (dom == SOCK || dom == > sbridge_dev->dom)) > + if ((sbridge_dev->seg == seg) && (sbridge_dev->bus == bus) && > + (dom == SOCK || dom == sbridge_dev->dom)) > return sbridge_dev; > } > > return NULL; > } > > -static struct sbridge_dev *alloc_sbridge_dev(u8 bus, enum domain dom, > +static struct sbridge_dev *alloc_sbridge_dev(int seg, u8 bus, enum domain > dom, >const struct pci_id_table *table) > { > struct sbridge_dev *sbridge_dev; > @@ -771,6 +774,7 @@ static struct sbridge_dev *alloc_sbridge_dev(u8 bus, enum > domain dom, > return NULL; > } > > + sbridge_dev->seg = seg; > sbridge_dev->bus = bus; > sbridge_dev->dom = dom; > sbridge_dev->n_devs = table->n_devs_per_imc; > @@ -2246,6 +2250,7 @@ static int sbridge_get_onedevice(struct pci_dev **prev, > struct sbridge_dev *sbridge_dev = NULL; > const struct pci_id_descr *dev_descr = &table->descr[devno]; > struct pci_dev *pdev = NULL; > + int seg = 0; > u8 bus = 0; > int i = 0; > > @@ -2276,10 +2281,12 @@ static int sbridge_get_onedevice(struct pci_dev > **prev, > /* End of list, leave */ > return -ENODEV; > } > + seg = pci_domain_nr(pdev->bus); > bus = pdev->bus->number; > > next_imc: > - sbridge_dev = get_sbridge_dev(bus, dev_descr->dom, multi_bus, > sbridge_dev); > + sbridge_dev = get_sbridge_dev(seg, bus, dev_descr->dom, > + multi_bus, sbridge_dev); > if (!sbridge_dev) { > /* If the HA1 wasn't found, don't create EDAC second memory > controller */ > if (dev_descr->dom == IMC1 && devno != 1) { > @@ -2292,7 +2299,7 @@ static int sbridge_get_onedevice(struct pci_dev **prev, > if (dev_descr->dom == SOCK) > goto out_imc; > > - sbridge_dev = alloc_sbridge_dev(bus, dev_descr->dom, table); > + sbridge_dev = alloc_sbridge_dev(seg, bus, dev_descr->dom, > table); > if (!sbridge_dev) { > pci_dev_put(pdev); > return -ENOMEM; >
Re: [PATCH] reset: hisilicon: fix potential NULL pointer dereference
Hi Stephen, On 07/25/2018 06:45 PM, Stephen Boyd wrote: > Quoting Gustavo A. R. Silva (2018-07-18 18:58:45) >> There is a potential execution path in which function >> platform_get_resource() returns NULL. If this happens, >> we will end up having a NULL pointer dereference. >> >> Fix this by adding asanity check in order to avoid a >> NULL pointer dereference. >> >> This code was detected with the help of Coccinelle. >> >> Cc: sta...@vger.kernel.org >> Fixes: 97b7129cd2af ("reset: hisilicon: change the definition of >> hisi_reset_init") >> Signed-off-by: Gustavo A. R. Silva >> --- >> drivers/clk/hisilicon/reset.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/drivers/clk/hisilicon/reset.c b/drivers/clk/hisilicon/reset.c >> index 2a5015c..5dfb48b 100644 >> --- a/drivers/clk/hisilicon/reset.c >> +++ b/drivers/clk/hisilicon/reset.c >> @@ -109,6 +109,9 @@ struct hisi_reset_controller *hisi_reset_init(struct >> platform_device *pdev) >> return NULL; >> >> res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> + if (!res) >> + return NULL; >> + >> rstc->membase = devm_ioremap(&pdev->dev, >> res->start, resource_size(res)); > > Why can't we use devm_ioremap_resource() here instead? > You're right. I think we can perfectly use devm_ioremap_resource here and remove the null check. I'll send patch for this shortly. Thanks -- Gustavo
Re: [PATCH] fsi: master-ast-cf: Fix memory leak
On 07/25/2018 06:45 PM, Benjamin Herrenschmidt wrote: > On Wed, 2018-07-25 at 08:38 -0500, Gustavo A. R. Silva wrote: >> In case memory resources for *fw* were allocated, release them >> before return. >> >> Addresses-Coverity-ID: 1472044 ("Resource leak") >> Fixes: 6a794a27daca ("fsi: master-ast-cf: Add new FSI master using Aspeed >> ColdFire") >> Signed-off-by: Gustavo A. R. Silva > > Thanks Gustavo ! > Glad to help. :) > I'll apply before I send my next pull request to Greg ! > Thanks -- Gustavo
[LKP] [ovl] 24c944dd64: BUG:kernel_reboot-without-warning_in_boot_stage
FYI, we noticed the following commit (built with gcc-7): commit: 24c944dd64f807542a2ec72744c81f064d1a60da ("ovl: Modify ovl_lookup() and friends to lookup metacopy dentry") https://git.kernel.org/cgit/linux/kernel/git/mszeredi/vfs.git overlayfs-next in testcase: boot on test machine: qemu-system-x86_64 -enable-kvm -smp 2 -m 512M caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace): +-+++ | | 48b3292dcd | 24c944dd64 | +-+++ | boot_successes | 34 | 0 | | boot_failures | 10 | 48 | | BUG:kernel_hang_in_test_stage | 8 || | invoked_oom-killer:gfp_mask=0x | 2 || | Mem-Info| 2 || | Out_of_memory:Kill_process | 2 || | BUG:kernel_reboot-without-warning_in_boot_stage | 0 | 48 | +-+++ [0.00] BRK [0x1d3ee000, 0x1d3eefff] PGTABLE [0.00] RAMDISK: [mem 0x1e73c000-0x1ffd] [0.00] ACPI: Early table checksum verification disabled [0.00] ACPI: RSDP 0x000F6870 14 (v00 BOCHS ) [0.00] ACPI: RSDT 0x1FFE1628 30 (v01 BOCHS BXPCRSDT 0001 BXPC 0001) BUG: kernel reboot-without-warning in boot stage Elapsed time: 10 #!/bin/bash To reproduce: git clone https://github.com/intel/lkp-tests.git cd lkp-tests bin/lkp qemu -k job-script # job-script is attached in this email Thanks, Rong, Chen # # Automatically generated file; DO NOT EDIT. # Linux/x86_64 4.18.0-rc1 Kernel Configuration # # # Compiler: gcc-7 (Debian 7.3.0-16) 7.3.0 # CONFIG_64BIT=y CONFIG_X86_64=y CONFIG_X86=y CONFIG_INSTRUCTION_DECODER=y CONFIG_OUTPUT_FORMAT="elf64-x86-64" CONFIG_ARCH_DEFCONFIG="arch/x86/configs/x86_64_defconfig" CONFIG_LOCKDEP_SUPPORT=y CONFIG_STACKTRACE_SUPPORT=y CONFIG_MMU=y CONFIG_ARCH_MMAP_RND_BITS_MIN=28 CONFIG_ARCH_MMAP_RND_BITS_MAX=32 CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MIN=8 CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MAX=16 CONFIG_GENERIC_ISA_DMA=y CONFIG_GENERIC_BUG=y CONFIG_GENERIC_BUG_RELATIVE_POINTERS=y CONFIG_GENERIC_HWEIGHT=y CONFIG_ARCH_MAY_HAVE_PC_FDC=y CONFIG_RWSEM_XCHGADD_ALGORITHM=y CONFIG_GENERIC_CALIBRATE_DELAY=y CONFIG_ARCH_HAS_CPU_RELAX=y CONFIG_ARCH_HAS_CACHE_LINE_SIZE=y CONFIG_ARCH_HAS_FILTER_PGPROT=y CONFIG_HAVE_SETUP_PER_CPU_AREA=y CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK=y CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK=y CONFIG_ARCH_HIBERNATION_POSSIBLE=y CONFIG_ARCH_SUSPEND_POSSIBLE=y CONFIG_ARCH_WANT_HUGE_PMD_SHARE=y CONFIG_ARCH_WANT_GENERAL_HUGETLB=y CONFIG_ZONE_DMA32=y CONFIG_AUDIT_ARCH=y CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING=y CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y CONFIG_KASAN_SHADOW_OFFSET=0xdc00 CONFIG_ARCH_SUPPORTS_UPROBES=y CONFIG_FIX_EARLYCON_MEM=y CONFIG_PGTABLE_LEVELS=4 CONFIG_CC_IS_GCC=y CONFIG_GCC_VERSION=70300 CONFIG_CLANG_VERSION=0 CONFIG_CONSTRUCTORS=y CONFIG_IRQ_WORK=y CONFIG_BUILDTIME_EXTABLE_SORT=y CONFIG_THREAD_INFO_IN_TASK=y # # General setup # CONFIG_BROKEN_ON_SMP=y CONFIG_INIT_ENV_ARG_LIMIT=32 # CONFIG_COMPILE_TEST is not set CONFIG_LOCALVERSION="" CONFIG_LOCALVERSION_AUTO=y CONFIG_HAVE_KERNEL_GZIP=y CONFIG_HAVE_KERNEL_BZIP2=y CONFIG_HAVE_KERNEL_LZMA=y CONFIG_HAVE_KERNEL_XZ=y CONFIG_HAVE_KERNEL_LZO=y CONFIG_HAVE_KERNEL_LZ4=y # CONFIG_KERNEL_GZIP is not set # CONFIG_KERNEL_BZIP2 is not set CONFIG_KERNEL_LZMA=y # CONFIG_KERNEL_XZ is not set # CONFIG_KERNEL_LZO is not set # CONFIG_KERNEL_LZ4 is not set CONFIG_DEFAULT_HOSTNAME="(none)" # CONFIG_SYSVIPC is not set # CONFIG_POSIX_MQUEUE is not set CONFIG_CROSS_MEMORY_ATTACH=y # CONFIG_USELIB is not set # CONFIG_AUDIT is not set CONFIG_HAVE_ARCH_AUDITSYSCALL=y # # IRQ subsystem # CONFIG_GENERIC_IRQ_PROBE=y CONFIG_GENERIC_IRQ_SHOW=y CONFIG_GENERIC_IRQ_CHIP=y CONFIG_IRQ_DOMAIN=y CONFIG_IRQ_SIM=y CONFIG_IRQ_DOMAIN_HIERARCHY=y CONFIG_GENERIC_MSI_IRQ=y CONFIG_GENERIC_MSI_IRQ_DOMAIN=y CONFIG_GENERIC_IRQ_MATRIX_ALLOCATOR=y CONFIG_GENERIC_IRQ_RESERVATION_MODE=y CONFIG_IRQ_FORCED_THREADING=y CONFIG_SPARSE_IRQ=y CONFIG_GENERIC_IRQ_DEBUGFS=y CONFIG_CLOCKSOURCE_WATCHDOG=y CONFIG_ARCH_CLOCKSOURCE_DATA=y CONFIG_CLOCKSOURCE_VALIDATE_LAST_CYCLE=y CONFIG_GENERIC_TIME_VSYSCALL=y CONFIG_GENERIC_CLOCKEVENTS=y CONFIG_GENERIC_CLOCKEVENTS_BROADCAST=y CONFIG_GENERIC_CLOCKEVENTS_MIN_ADJUST=y CONFIG_GENERIC_CMOS_UPDATE=y # # Timers subsystem # CONFIG_TICK_ONESHOT=y CONFIG_NO_HZ_COMMON=y # CONFIG_HZ_PERIODIC is not set CONFIG_NO_HZ_IDLE=y CONFIG_NO_HZ=y CONFIG_HIGH_RES_TIMERS=y # # CPU/Task time and stats accounting # CONFIG_TICK_CPU_ACCOUNTING=y # CONFIG_VIRT_CPU_ACCOUNTING_GEN is not set # CON
Re: linux-next: Signed-off-by missing for commit in the xarray tree
On 07/25/2018 05:03 PM, Stephen Rothwell wrote: > Hi Matthew, > > On Wed, 25 Jul 2018 16:36:21 -0700 Matthew Wilcox wrote: >> >> On Thu, Jul 26, 2018 at 07:28:14AM +1000, Stephen Rothwell wrote: >>> >>> Commits >>> >>> 890e537e2b42 ("filesystem-dax: Introduce dax_lock_mapping_entry()") >>> aaf149902c79 ("filesystem-dax: Set page->index") >>> >>> are missing a Signed-off-by from their committers. >> >> Oh, hah. I assume this is an automated email? > > Well, semi-automatic :-) > >> These two commits I cherry-picked from the nvdimm tree so that XArray can >> be rebased on top of it. Is there some other way I should be doing this, >> like rebasing on top of the nvdimm tree? > > Ideally, the nvdimm tree would have just those two commits in a branch > that you could merge (so that you both have the same commits (as > opposed to patches)) that way these changes cannot cause conflicts when > the files are further modifed in either tree. Alternatively, if you do > have to cherry-pick them, then you need to add your Signed-off-by to > the copy that you commit. > > As things are now, you could merge commit > > c2a7d2a11552 ("filesystem-dax: Introduce dax_lock_mapping_entry()") > > from the nvdimm tree into your tree before the conflicting commits in > your tree (or just rebase your tree on top of that commit). You need > to make sure that Dan and/or Dave (cc'd) will never rebase that part of > their tree. Also, you will pick up some other commits (which may not > be a problem for you). > I have all the acks I need for that branch for Dan's patches. So that branch shouldn't change anymore AFAIK.
Re: linux-next: Signed-off-by missing for commit in the xarray tree
Hi Matthew, On Wed, 25 Jul 2018 16:36:21 -0700 Matthew Wilcox wrote: > > On Thu, Jul 26, 2018 at 07:28:14AM +1000, Stephen Rothwell wrote: > > > > Commits > > > > 890e537e2b42 ("filesystem-dax: Introduce dax_lock_mapping_entry()") > > aaf149902c79 ("filesystem-dax: Set page->index") > > > > are missing a Signed-off-by from their committers. > > Oh, hah. I assume this is an automated email? Well, semi-automatic :-) > These two commits I cherry-picked from the nvdimm tree so that XArray can > be rebased on top of it. Is there some other way I should be doing this, > like rebasing on top of the nvdimm tree? Ideally, the nvdimm tree would have just those two commits in a branch that you could merge (so that you both have the same commits (as opposed to patches)) that way these changes cannot cause conflicts when the files are further modifed in either tree. Alternatively, if you do have to cherry-pick them, then you need to add your Signed-off-by to the copy that you commit. As things are now, you could merge commit c2a7d2a11552 ("filesystem-dax: Introduce dax_lock_mapping_entry()") from the nvdimm tree into your tree before the conflicting commits in your tree (or just rebase your tree on top of that commit). You need to make sure that Dan and/or Dave (cc'd) will never rebase that part of their tree. Also, you will pick up some other commits (which may not be a problem for you). -- Cheers, Stephen Rothwell pgpfefLdYxXfn.pgp Description: OpenPGP digital signature
Re: [PATCH v2 2/2] pinctrl: nuvoton: add NPCM7xx pinctrl and GPIO driver
Hi Linus, Thanks a lot for your comments. Sorry for my late reply, I was on vacation. The last days I have been working to move NPCM pinctrl GPIO to GENERIC GPIO, most of the work have been done but I had the some issue. I initialize bgpio as follow: ret = bgpio_init(&pctrl->gpio_bank[id].gc, pctrl->dev, 4, pctrl->gpio_bank[id].base + NPCM7XX_GP_N_DIN, pctrl->gpio_bank[id].base + NPCM7XX_GP_N_DOUT, NULL, NULL, pctrl->gpio_bank[id].base + NPCM7XX_GP_N_IEM, BGPIOF_READ_OUTPUT_REG_SET); After doing it, the directions functions I used are: bgpio_dir_out_inv, bgpio_dir_in_inv, bgpio_get_dir_inv and the I/O get function is bgpio_get_set By using inv directions: direction out = 0 (gc->bgpio_dir &= ~bgpio_line2mask(gc, gpio)) direction in = 1 (gc->bgpio_dir |= bgpio_line2mask(gc, gpio)) The problem occur when reading the GPIO value from bgpio_get_set function, because the directions value are inverse it reading the wrong I/O registers For direction out it reading dat register (instead of set register) For direction in it calling set register (instead of dat register) if (gc->bgpio_dir & pinmask) return !!(gc->read_reg(gc->reg_set) & pinmask); else return !!(gc->read_reg(gc->reg_dat) & pinmask); the same issue occur at bgpio_get_set_multiple function. Maybe in bgpio_dir parameter direction out should be in both cases 1 and direction in = 0. for now i did a local fix in the npcm pinctrl driver by setting bgpio_dir parameters as direction out = 1 and direction in = 0. Thanks a lot, Tomer On 13 July 2018 at 11:51, Linus Walleij wrote: > On Thu, Jul 12, 2018 at 11:42 PM Tomer Maimon wrote: > > > Add Nuvoton BMC NPCM750/730/715/705 Pinmux and > > GPIO controller driver. > > > > Signed-off-by: Tomer Maimon > > (...) > > +++ b/drivers/pinctrl/nuvoton/pinctrl-npcm7xx.c > > @@ -0,0 +1,2089 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +// Copyright (c) 2016-2018 Nuvoton Technology corporation. > > +// Copyright (c) 2016, Dell Inc > > + > > +#include > > +#include > > As this is a driver you should only include > > > +#include > > +#include > > +#include > > If you need syscon then the driver should select or depend > on MFD_SYSCON in Kconfig. > > > +#include > > +#include > > +#include > > +#include > > Do you really need this include? > > > +/* Structure for register banks */ > > +struct NPCM7XX_GPIO { > > Can we have this lowercase? Please? > > > + void __iomem*base; > > + struct gpio_chipgc; > > + int irqbase; > > + int irq; > > + spinlock_t lock; > > + void*priv; > > + struct irq_chip irq_chip; > > + u32 pinctrl_id; > > +}; > > So each GPIO bank has its own gpio chip and register > base, that is NICE! Because then it looks like you can > select GPIO_GENERIC and use the MMIO GPIO helper > library to handle the registers. Let's look into that > option! > > > +struct NPCM7xx_pinctrl { > > + struct pinctrl_dev *pctldev; > > + struct device *dev; > > + struct NPCM7XX_GPIO gpio_bank[NPCM7XX_GPIO_BANK_NUM]; > > + struct irq_domain *domain; > > I wonder why the pin controller needs and IRQ domain but > I keep reading the code and I might find out... > > > +enum operand { > > + op_set, > > + op_getbit, > > + op_setbit, > > + op_clrbit, > > +}; > > This looks complicated. I suspect you can use GPIO_GENERIC > to set/get and clear bits in the register(s). > > > +/* Perform locked bit operations on GPIO registers */ > > +static int gpio_bitop(struct NPCM7XX_GPIO *bank, int op, unsigned int > offset, > > + int reg) > > +{ > > + unsigned long flags; > > + u32 mask, val; > > + > > + mask = (1L << offset); > > + spin_lock_irqsave(&bank->lock, flags); > > + switch (op) { > > + case op_set: > > + iowrite32(mask, bank->base + reg); > > + break; > > + case op_getbit: > > + mask &= ioread32(bank->base + reg); > > + break; > > + case op_setbit: > > + val = ioread32(bank->base + reg); > > + iowrite32(val | mask, bank->base + reg); > > + break; > > + case op_clrbit: > > + val = ioread32(bank->base + reg); > > + iowrite32(val & (~mask), bank->base + reg); > > + break; > > + } >
[PATCH v4 1/9] proc/kcore: don't grab lock for kclist_add()
From: Omar Sandoval kclist_add() is only called at init time, so there's no point in grabbing any locks. We're also going to replace the rwlock with a rwsem, which we don't want to try grabbing during early boot. While we're here, mark kclist_add() with __init so that we'll get a warning if it's called from non-init code. Reviewed-by: Andrew Morton Signed-off-by: Omar Sandoval --- fs/proc/kcore.c | 7 +++ include/linux/kcore.h | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/fs/proc/kcore.c b/fs/proc/kcore.c index 66c373230e60..b0b9a76f28d6 100644 --- a/fs/proc/kcore.c +++ b/fs/proc/kcore.c @@ -62,16 +62,15 @@ static LIST_HEAD(kclist_head); static DEFINE_RWLOCK(kclist_lock); static int kcore_need_update = 1; -void -kclist_add(struct kcore_list *new, void *addr, size_t size, int type) +/* This doesn't grab kclist_lock, so it should only be used at init time. */ +void __init kclist_add(struct kcore_list *new, void *addr, size_t size, + int type) { new->addr = (unsigned long)addr; new->size = size; new->type = type; - write_lock(&kclist_lock); list_add_tail(&new->list, &kclist_head); - write_unlock(&kclist_lock); } static size_t get_kcore_size(int *nphdr, size_t *elf_buflen) diff --git a/include/linux/kcore.h b/include/linux/kcore.h index 8de55e4b5ee9..c20f296438fb 100644 --- a/include/linux/kcore.h +++ b/include/linux/kcore.h @@ -35,7 +35,7 @@ struct vmcoredd_node { }; #ifdef CONFIG_PROC_KCORE -extern void kclist_add(struct kcore_list *, void *, size_t, int type); +void __init kclist_add(struct kcore_list *, void *, size_t, int type); #else static inline void kclist_add(struct kcore_list *new, void *addr, size_t size, int type) -- 2.18.0
[PATCH v4 3/9] proc/kcore: replace kclist_lock rwlock with rwsem
From: Omar Sandoval Now we only need kclist_lock from user context and at fs init time, and the following changes need to sleep while holding the kclist_lock. Signed-off-by: Omar Sandoval --- fs/proc/kcore.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/fs/proc/kcore.c b/fs/proc/kcore.c index e83f15a4f66d..ae43a97d511d 100644 --- a/fs/proc/kcore.c +++ b/fs/proc/kcore.c @@ -59,7 +59,7 @@ struct memelfnote }; static LIST_HEAD(kclist_head); -static DEFINE_RWLOCK(kclist_lock); +static DECLARE_RWSEM(kclist_lock); static int kcore_need_update = 1; /* This doesn't grab kclist_lock, so it should only be used at init time. */ @@ -117,7 +117,7 @@ static void __kcore_update_ram(struct list_head *list) struct kcore_list *tmp, *pos; LIST_HEAD(garbage); - write_lock(&kclist_lock); + down_write(&kclist_lock); if (xchg(&kcore_need_update, 0)) { list_for_each_entry_safe(pos, tmp, &kclist_head, list) { if (pos->type == KCORE_RAM @@ -128,7 +128,7 @@ static void __kcore_update_ram(struct list_head *list) } else list_splice(list, &garbage); proc_root_kcore->size = get_kcore_size(&nphdr, &size); - write_unlock(&kclist_lock); + up_write(&kclist_lock); free_kclist_ents(&garbage); } @@ -451,11 +451,11 @@ read_kcore(struct file *file, char __user *buffer, size_t buflen, loff_t *fpos) int nphdr; unsigned long start; - read_lock(&kclist_lock); + down_read(&kclist_lock); size = get_kcore_size(&nphdr, &elf_buflen); if (buflen == 0 || *fpos >= size) { - read_unlock(&kclist_lock); + up_read(&kclist_lock); return 0; } @@ -472,11 +472,11 @@ read_kcore(struct file *file, char __user *buffer, size_t buflen, loff_t *fpos) tsz = buflen; elf_buf = kzalloc(elf_buflen, GFP_ATOMIC); if (!elf_buf) { - read_unlock(&kclist_lock); + up_read(&kclist_lock); return -ENOMEM; } elf_kcore_store_hdr(elf_buf, nphdr, elf_buflen); - read_unlock(&kclist_lock); + up_read(&kclist_lock); if (copy_to_user(buffer, elf_buf + *fpos, tsz)) { kfree(elf_buf); return -EFAULT; @@ -491,7 +491,7 @@ read_kcore(struct file *file, char __user *buffer, size_t buflen, loff_t *fpos) if (buflen == 0) return acc; } else - read_unlock(&kclist_lock); + up_read(&kclist_lock); /* * Check to see if our file offset matches with any of @@ -504,12 +504,12 @@ read_kcore(struct file *file, char __user *buffer, size_t buflen, loff_t *fpos) while (buflen) { struct kcore_list *m; - read_lock(&kclist_lock); + down_read(&kclist_lock); list_for_each_entry(m, &kclist_head, list) { if (start >= m->addr && start < (m->addr+m->size)) break; } - read_unlock(&kclist_lock); + up_read(&kclist_lock); if (&m->list == &kclist_head) { if (clear_user(buffer, tsz)) -- 2.18.0
[PATCH v4 5/9] proc/kcore: hold lock during read
From: Omar Sandoval Now that we're using an rwsem, we can hold it during the entirety of read_kcore() and have a common return path. This is preparation for the next change. Signed-off-by: Omar Sandoval --- fs/proc/kcore.c | 70 - 1 file changed, 40 insertions(+), 30 deletions(-) diff --git a/fs/proc/kcore.c b/fs/proc/kcore.c index 95aa988c5b5d..dc34642bbdb7 100644 --- a/fs/proc/kcore.c +++ b/fs/proc/kcore.c @@ -440,19 +440,18 @@ static ssize_t read_kcore(struct file *file, char __user *buffer, size_t buflen, loff_t *fpos) { char *buf = file->private_data; - ssize_t acc = 0; size_t size, tsz; size_t elf_buflen; int nphdr; unsigned long start; + size_t orig_buflen = buflen; + int ret = 0; down_read(&kclist_lock); size = get_kcore_size(&nphdr, &elf_buflen); - if (buflen == 0 || *fpos >= size) { - up_read(&kclist_lock); - return 0; - } + if (buflen == 0 || *fpos >= size) + goto out; /* trim buflen to not go beyond EOF */ if (buflen > size - *fpos) @@ -465,28 +464,26 @@ read_kcore(struct file *file, char __user *buffer, size_t buflen, loff_t *fpos) tsz = elf_buflen - *fpos; if (buflen < tsz) tsz = buflen; - elf_buf = kzalloc(elf_buflen, GFP_ATOMIC); + elf_buf = kzalloc(elf_buflen, GFP_KERNEL); if (!elf_buf) { - up_read(&kclist_lock); - return -ENOMEM; + ret = -ENOMEM; + goto out; } elf_kcore_store_hdr(elf_buf, nphdr, elf_buflen); - up_read(&kclist_lock); if (copy_to_user(buffer, elf_buf + *fpos, tsz)) { kfree(elf_buf); - return -EFAULT; + ret = -EFAULT; + goto out; } kfree(elf_buf); buflen -= tsz; *fpos += tsz; buffer += tsz; - acc += tsz; /* leave now if filled buffer already */ if (buflen == 0) - return acc; - } else - up_read(&kclist_lock); + goto out; + } /* * Check to see if our file offset matches with any of @@ -499,25 +496,29 @@ read_kcore(struct file *file, char __user *buffer, size_t buflen, loff_t *fpos) while (buflen) { struct kcore_list *m; - down_read(&kclist_lock); list_for_each_entry(m, &kclist_head, list) { if (start >= m->addr && start < (m->addr+m->size)) break; } - up_read(&kclist_lock); if (&m->list == &kclist_head) { - if (clear_user(buffer, tsz)) - return -EFAULT; + if (clear_user(buffer, tsz)) { + ret = -EFAULT; + goto out; + } } else if (m->type == KCORE_VMALLOC) { vread(buf, (char *)start, tsz); /* we have to zero-fill user buffer even if no read */ - if (copy_to_user(buffer, buf, tsz)) - return -EFAULT; + if (copy_to_user(buffer, buf, tsz)) { + ret = -EFAULT; + goto out; + } } else if (m->type == KCORE_USER) { /* User page is handled prior to normal kernel page: */ - if (copy_to_user(buffer, (char *)start, tsz)) - return -EFAULT; + if (copy_to_user(buffer, (char *)start, tsz)) { + ret = -EFAULT; + goto out; + } } else { if (kern_addr_valid(start)) { /* @@ -525,26 +526,35 @@ read_kcore(struct file *file, char __user *buffer, size_t buflen, loff_t *fpos) * hardened user copy kernel text checks. */ if (probe_kernel_read(buf, (void *) start, tsz)) { - if (clear_user(buffer, tsz)) - return -EFAULT; + if (clear_user(buffer, tsz)) { + ret = -EFAULT; + goto out; + } } el
[PATCH v4 7/9] proc/kcore: optimize multiple page reads
From: Omar Sandoval The current code does a full search of the segment list every time for every page. This is wasteful, since it's almost certain that the next page will be in the same segment. Instead, check if the previous segment covers the current page before doing the list search. Signed-off-by: Omar Sandoval --- fs/proc/kcore.c | 14 +++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/fs/proc/kcore.c b/fs/proc/kcore.c index 808ef9afd084..758c14e46a44 100644 --- a/fs/proc/kcore.c +++ b/fs/proc/kcore.c @@ -428,10 +428,18 @@ read_kcore(struct file *file, char __user *buffer, size_t buflen, loff_t *fpos) if ((tsz = (PAGE_SIZE - (start & ~PAGE_MASK))) > buflen) tsz = buflen; + m = NULL; while (buflen) { - list_for_each_entry(m, &kclist_head, list) { - if (start >= m->addr && start < (m->addr+m->size)) - break; + /* +* If this is the first iteration or the address is not within +* the previous entry, search for a matching entry. +*/ + if (!m || start < m->addr || start >= m->addr + m->size) { + list_for_each_entry(m, &kclist_head, list) { + if (start >= m->addr && + start < m->addr + m->size) + break; + } } if (&m->list == &kclist_head) { -- 2.18.0
[PATCH v4 2/9] proc/kcore: don't grab lock for memory hotplug notifier
From: Omar Sandoval The memory hotplug notifier kcore_callback() only needs kclist_lock to prevent races with __kcore_update_ram(), but we can easily eliminate that race by using an atomic xchg() in __kcore_update_ram(). This is preparation for converting kclist_lock to an rwsem. Reviewed-by: Andrew Morton Signed-off-by: Omar Sandoval --- fs/proc/kcore.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/fs/proc/kcore.c b/fs/proc/kcore.c index b0b9a76f28d6..e83f15a4f66d 100644 --- a/fs/proc/kcore.c +++ b/fs/proc/kcore.c @@ -118,7 +118,7 @@ static void __kcore_update_ram(struct list_head *list) LIST_HEAD(garbage); write_lock(&kclist_lock); - if (kcore_need_update) { + if (xchg(&kcore_need_update, 0)) { list_for_each_entry_safe(pos, tmp, &kclist_head, list) { if (pos->type == KCORE_RAM || pos->type == KCORE_VMEMMAP) @@ -127,7 +127,6 @@ static void __kcore_update_ram(struct list_head *list) list_splice_tail(list, &kclist_head); } else list_splice(list, &garbage); - kcore_need_update = 0; proc_root_kcore->size = get_kcore_size(&nphdr, &size); write_unlock(&kclist_lock); @@ -593,9 +592,8 @@ static int __meminit kcore_callback(struct notifier_block *self, switch (action) { case MEM_ONLINE: case MEM_OFFLINE: - write_lock(&kclist_lock); kcore_need_update = 1; - write_unlock(&kclist_lock); + break; } return NOTIFY_OK; } -- 2.18.0
[PATCH v4 6/9] proc/kcore: clean up ELF header generation
From: Omar Sandoval Currently, the ELF file header, program headers, and note segment are allocated all at once, in some icky code dating back to 2.3. Programs tend to read the file header, then the program headers, then the note segment, all separately, so this is a waste of effort. It's cleaner and more efficient to handle the three separately. Signed-off-by: Omar Sandoval --- fs/proc/kcore.c | 350 +++- 1 file changed, 141 insertions(+), 209 deletions(-) diff --git a/fs/proc/kcore.c b/fs/proc/kcore.c index dc34642bbdb7..808ef9afd084 100644 --- a/fs/proc/kcore.c +++ b/fs/proc/kcore.c @@ -49,15 +49,6 @@ static struct proc_dir_entry *proc_root_kcore; #definekc_offset_to_vaddr(o) ((o) + PAGE_OFFSET) #endif -/* An ELF note in memory */ -struct memelfnote -{ - const char *name; - int type; - unsigned int datasz; - void *data; -}; - static LIST_HEAD(kclist_head); static DECLARE_RWSEM(kclist_lock); static int kcore_need_update = 1; @@ -73,7 +64,8 @@ void __init kclist_add(struct kcore_list *new, void *addr, size_t size, list_add_tail(&new->list, &kclist_head); } -static size_t get_kcore_size(int *nphdr, size_t *elf_buflen) +static size_t get_kcore_size(int *nphdr, size_t *phdrs_len, size_t *notes_len, +size_t *data_offset) { size_t try, size; struct kcore_list *m; @@ -87,15 +79,15 @@ static size_t get_kcore_size(int *nphdr, size_t *elf_buflen) size = try; *nphdr = *nphdr + 1; } - *elf_buflen = sizeof(struct elfhdr) + - (*nphdr + 2)*sizeof(struct elf_phdr) + - 3 * ((sizeof(struct elf_note)) + -roundup(sizeof(CORE_STR), 4)) + - roundup(sizeof(struct elf_prstatus), 4) + - roundup(sizeof(struct elf_prpsinfo), 4) + - roundup(arch_task_struct_size, 4); - *elf_buflen = PAGE_ALIGN(*elf_buflen); - return size + *elf_buflen; + + *phdrs_len = *nphdr * sizeof(struct elf_phdr); + *notes_len = (3 * (sizeof(struct elf_note) + ALIGN(sizeof(CORE_STR), 4)) + + ALIGN(sizeof(struct elf_prstatus), 4) + + ALIGN(sizeof(struct elf_prpsinfo), 4) + + ALIGN(arch_task_struct_size, 4)); + *data_offset = PAGE_ALIGN(sizeof(struct elfhdr) + *phdrs_len + + *notes_len); + return *data_offset + size; } #ifdef CONFIG_HIGHMEM @@ -241,7 +233,7 @@ static int kcore_update_ram(void) LIST_HEAD(list); LIST_HEAD(garbage); int nphdr; - size_t size; + size_t phdrs_len, notes_len, data_offset; struct kcore_list *tmp, *pos; int ret = 0; @@ -263,7 +255,8 @@ static int kcore_update_ram(void) } list_splice_tail(&list, &kclist_head); - proc_root_kcore->size = get_kcore_size(&nphdr, &size); + proc_root_kcore->size = get_kcore_size(&nphdr, &phdrs_len, ¬es_len, + &data_offset); out: up_write(&kclist_lock); @@ -274,228 +267,168 @@ static int kcore_update_ram(void) return ret; } -/*/ -/* - * determine size of ELF note - */ -static int notesize(struct memelfnote *en) +static void append_kcore_note(char *notes, size_t *i, const char *name, + unsigned int type, const void *desc, + size_t descsz) { - int sz; - - sz = sizeof(struct elf_note); - sz += roundup((strlen(en->name) + 1), 4); - sz += roundup(en->datasz, 4); - - return sz; -} /* end notesize() */ - -/*/ -/* - * store a note in the header buffer - */ -static char *storenote(struct memelfnote *men, char *bufp) -{ - struct elf_note en; - -#define DUMP_WRITE(addr,nr) do { memcpy(bufp,addr,nr); bufp += nr; } while(0) - - en.n_namesz = strlen(men->name) + 1; - en.n_descsz = men->datasz; - en.n_type = men->type; - - DUMP_WRITE(&en, sizeof(en)); - DUMP_WRITE(men->name, en.n_namesz); - - /* XXX - cast from long long to long to avoid need for libgcc.a */ - bufp = (char*) roundup((unsigned long)bufp,4); - DUMP_WRITE(men->data, men->datasz); - bufp = (char*) roundup((unsigned long)bufp,4); - -#undef DUMP_WRITE - - return bufp; -} /* end storenote() */ - -/* - * store an ELF coredump header in the supplied buffer - * nphdr is the number of elf_phdr to insert - */ -static void elf_kcore_store_hdr(char *bufp, int nphdr, int dataoff) -{ - struct elf_prstatus prstatus; /* NT_PRSTATUS */ - struct elf_prpsinfo prpsinfo; /* NT_PRPSINFO */ - struct elf_phdr *nhdr, *phdr; - struct elfhdr *elf;
[PATCH v4 9/9] proc/kcore: add vmcoreinfo note to /proc/kcore
From: Omar Sandoval The vmcoreinfo information is useful for runtime debugging tools, not just for crash dumps. A lot of this information can be determined by other means, but this is much more convenient, and it only adds a page at most to the file. Signed-off-by: Omar Sandoval --- fs/proc/Kconfig| 1 + fs/proc/kcore.c| 18 -- include/linux/crash_core.h | 2 ++ kernel/crash_core.c| 4 ++-- 4 files changed, 21 insertions(+), 4 deletions(-) diff --git a/fs/proc/Kconfig b/fs/proc/Kconfig index 0eaeb41453f5..817c02b13b1d 100644 --- a/fs/proc/Kconfig +++ b/fs/proc/Kconfig @@ -31,6 +31,7 @@ config PROC_FS config PROC_KCORE bool "/proc/kcore support" if !ARM depends on PROC_FS && MMU + select CRASH_CORE help Provides a virtual ELF core file of the live kernel. This can be read with gdb and other ELF tools. No modifications can be diff --git a/fs/proc/kcore.c b/fs/proc/kcore.c index 758c14e46a44..80464432dfe6 100644 --- a/fs/proc/kcore.c +++ b/fs/proc/kcore.c @@ -10,6 +10,7 @@ * Safe accesses to vmalloc/direct-mapped discontiguous areas, Kanoj Sarcar */ +#include #include #include #include @@ -81,10 +82,13 @@ static size_t get_kcore_size(int *nphdr, size_t *phdrs_len, size_t *notes_len, } *phdrs_len = *nphdr * sizeof(struct elf_phdr); - *notes_len = (3 * (sizeof(struct elf_note) + ALIGN(sizeof(CORE_STR), 4)) + + *notes_len = (4 * sizeof(struct elf_note) + + 3 * ALIGN(sizeof(CORE_STR), 4) + + VMCOREINFO_NOTE_NAME_BYTES + ALIGN(sizeof(struct elf_prstatus), 4) + ALIGN(sizeof(struct elf_prpsinfo), 4) + - ALIGN(arch_task_struct_size, 4)); + ALIGN(arch_task_struct_size, 4) + + ALIGN(vmcoreinfo_size, 4)); *data_offset = PAGE_ALIGN(sizeof(struct elfhdr) + *phdrs_len + *notes_len); return *data_offset + size; @@ -406,6 +410,16 @@ read_kcore(struct file *file, char __user *buffer, size_t buflen, loff_t *fpos) sizeof(prpsinfo)); append_kcore_note(notes, &i, CORE_STR, NT_TASKSTRUCT, current, arch_task_struct_size); + /* +* vmcoreinfo_size is mostly constant after init time, but it +* can be changed by crash_save_vmcoreinfo(). Racing here with a +* panic on another CPU before the machine goes down is insanely +* unlikely, but it's better to not leave potential buffer +* overflows lying around, regardless. +*/ + append_kcore_note(notes, &i, VMCOREINFO_NOTE_NAME, 0, + vmcoreinfo_data, + min(vmcoreinfo_size, notes_len - i)); tsz = min_t(size_t, buflen, notes_offset + notes_len - *fpos); if (copy_to_user(buffer, notes + *fpos - notes_offset, tsz)) { diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h index b511f6d24b42..525510a9f965 100644 --- a/include/linux/crash_core.h +++ b/include/linux/crash_core.h @@ -60,6 +60,8 @@ phys_addr_t paddr_vmcoreinfo_note(void); #define VMCOREINFO_CONFIG(name) \ vmcoreinfo_append_str("CONFIG_%s=y\n", #name) +extern unsigned char *vmcoreinfo_data; +extern size_t vmcoreinfo_size; extern u32 *vmcoreinfo_note; Elf_Word *append_elf_note(Elf_Word *buf, char *name, unsigned int type, diff --git a/kernel/crash_core.c b/kernel/crash_core.c index e7b4025c7b24..a683bfb0530f 100644 --- a/kernel/crash_core.c +++ b/kernel/crash_core.c @@ -14,8 +14,8 @@ #include /* vmcoreinfo stuff */ -static unsigned char *vmcoreinfo_data; -static size_t vmcoreinfo_size; +unsigned char *vmcoreinfo_data; +size_t vmcoreinfo_size; u32 *vmcoreinfo_note; /* trusted vmcoreinfo, e.g. we can make a copy in the crash memory */ -- 2.18.0
[PATCH v4 0/9] /proc/kcore improvements
From: Omar Sandoval Hi, This series makes a few improvements to /proc/kcore. It fixes a couple of small issues in v3 but is otherwise the same. Patches 1, 2, and 3 are prep patches. Patch 4 is a fix/cleanup. Patch 5 is another prep patch. Patches 6 and 7 are optimizations to ->read(). Patch 8 makes it possible to enable CRASH_CORE on any architecture, which is needed for patch 9. Patch 9 adds vmcoreinfo to /proc/kcore. Based on v4.18-rc6 + James' patch in the mm tree, and tested with crash and readelf. Thanks! Changes from v3: - Fixes a mixed up up_write() instead of up_read() in patch 5 reported by Tetsuo Handa - Added patch 8 to fix a build failure reported by Stephen Rothwell Changes from v2: - Add __init to kclist_add() as per Andrew - Got rid of conversion of kcore_need_update from int to atomic_t and just used xchg() instead of atomic_cmpxchg() (split out into a new patch instead of combining it with the rwlock -> rwsem conversion) - Add comment about the increase in file size to patch 8 Changes from v1: - Rebased onto v4.18-rc4 + James' patch (https://patchwork.kernel.org/patch/10519739/) in the mm tree - Fix spurious sparse warning (see the report and response in https://patchwork.kernel.org/patch/10512431/) Omar Sandoval (9): proc/kcore: don't grab lock for kclist_add() proc/kcore: don't grab lock for memory hotplug notifier proc/kcore: replace kclist_lock rwlock with rwsem proc/kcore: fix memory hotplug vs multiple opens race proc/kcore: hold lock during read proc/kcore: clean up ELF header generation proc/kcore: optimize multiple page reads crash_core: use VMCOREINFO_SYMBOL_ARRAY() for swapper_pg_dir proc/kcore: add vmcoreinfo note to /proc/kcore fs/proc/Kconfig| 1 + fs/proc/kcore.c| 534 + include/linux/crash_core.h | 2 + include/linux/kcore.h | 2 +- kernel/crash_core.c| 6 +- 5 files changed, 252 insertions(+), 293 deletions(-) -- 2.18.0
[PATCH v4 8/9] crash_core: use VMCOREINFO_SYMBOL_ARRAY() for swapper_pg_dir
From: Omar Sandoval This is preparation for allowing CRASH_CORE to be enabled for any architecture. swapper_pg_dir is always either an array or a macro expanding to NULL. In the latter case, VMCOREINFO_SYMBOL() won't work, as it tries to take the address of the given symbol: #define VMCOREINFO_SYMBOL(name) \ vmcoreinfo_append_str("SYMBOL(%s)=%lx\n", #name, (unsigned long)&name) Instead, use VMCOREINFO_SYMBOL_ARRAY(), which uses the value: #define VMCOREINFO_SYMBOL_ARRAY(name) \ vmcoreinfo_append_str("SYMBOL(%s)=%lx\n", #name, (unsigned long)name) This is the same thing for the array case but isn't an error for the macro case. Signed-off-by: Omar Sandoval --- kernel/crash_core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/crash_core.c b/kernel/crash_core.c index b66aced5e8c2..e7b4025c7b24 100644 --- a/kernel/crash_core.c +++ b/kernel/crash_core.c @@ -401,7 +401,7 @@ static int __init crash_save_vmcoreinfo_init(void) VMCOREINFO_SYMBOL(init_uts_ns); VMCOREINFO_SYMBOL(node_online_map); #ifdef CONFIG_MMU - VMCOREINFO_SYMBOL(swapper_pg_dir); + VMCOREINFO_SYMBOL_ARRAY(swapper_pg_dir); #endif VMCOREINFO_SYMBOL(_stext); VMCOREINFO_SYMBOL(vmap_area_list); -- 2.18.0