Re: [PATCH] spmi: pmic-arb: Always allocate ppid_to_apid table
On 2017-06-27 07:47, Stephen Boyd wrote: After commit 7f1d4e58dabb ("spmi: pmic-arb: optimize table lookups") we always need the ppid_to_apid table regardless of the version of pmic arbiter we have. Otherwise, we will try to deref the array when we don't allocate it on v2 hardware like the msm8974 SoCs. Cc: Abhijeet Dharmapurikar Cc: Kiran Gunda Fixes: 7f1d4e58dabb ("spmi: pmic-arb: optimize table lookups") Signed-off-by: Stephen Boyd Reviewed-by: Kiran Gunda --- drivers/spmi/spmi-pmic-arb.c | 17 +++-- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c index 2afe3597982e..f4b7a98a7913 100644 --- a/drivers/spmi/spmi-pmic-arb.c +++ b/drivers/spmi/spmi-pmic-arb.c @@ -134,7 +134,6 @@ struct apid_data { * @spmic: SPMI controller object * @ver_ops: version dependent operations. * @ppid_to_apid in-memory copy of PPID -> channel (APID) mapping table. - * v2 only. */ struct spmi_pmic_arb { void __iomem*rd_base; @@ -1016,6 +1015,13 @@ static int spmi_pmic_arb_probe(struct platform_device *pdev) goto err_put_ctrl; } + pa->ppid_to_apid = devm_kcalloc(&ctrl->dev, PMIC_ARB_MAX_PPID, + sizeof(*pa->ppid_to_apid), GFP_KERNEL); + if (!pa->ppid_to_apid) { + err = -ENOMEM; + goto err_put_ctrl; + } + hw_ver = readl_relaxed(core + PMIC_ARB_VERSION); if (hw_ver < PMIC_ARB_VERSION_V2_MIN) { @@ -1048,15 +1054,6 @@ static int spmi_pmic_arb_probe(struct platform_device *pdev) err = PTR_ERR(pa->wr_base); goto err_put_ctrl; } - - pa->ppid_to_apid = devm_kcalloc(&ctrl->dev, - PMIC_ARB_MAX_PPID, - sizeof(*pa->ppid_to_apid), - GFP_KERNEL); - if (!pa->ppid_to_apid) { - err = -ENOMEM; - goto err_put_ctrl; - } } dev_info(&ctrl->dev, "PMIC arbiter version %s (0x%x)\n", Thanks for the fix !
Re: [PATCH v3 15/15] spi: qup: support for qup v1 dma
On Fri, Jun 23, 2017 at 04:49:23PM -0500, Rob Herring wrote: > On Tue, Jun 20, 2017 at 02:40:57PM +0530, Varadarajan Narayanan wrote: > > Currently the QUP Version v1 does not work with DMA so added > > the support for the same. > > > > 1. It uses ADM DMA which requires TX and RX CRCI > > 2. DMA channel initialization need to be done after setting > >block size for having valid values in maxburst > > 3. QUP mode should be DMOV instead of BAM. > > > > Signed-off-by: Abhishek Sahu > > Signed-off-by: Varadarajan Narayanan > > --- > > .../devicetree/bindings/spi/qcom,spi-qup.txt | 6 > > drivers/spi/spi-qup.c | 35 > > +- > > 2 files changed, 34 insertions(+), 7 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/spi/qcom,spi-qup.txt > > b/Documentation/devicetree/bindings/spi/qcom,spi-qup.txt > > index 5c09077..e754181 100644 > > --- a/Documentation/devicetree/bindings/spi/qcom,spi-qup.txt > > +++ b/Documentation/devicetree/bindings/spi/qcom,spi-qup.txt > > @@ -38,6 +38,12 @@ Optional properties: > > - dma-names:Names for the dma channels, if present. There must be at > > least one channel named "tx" for transmit and named "rx" > > for > > receive. > > +- qcom,tx-crci: Identificator for Client Rate Control Interface (CRCI) to > > be > > Identificator is not a word. > > This sounds like something that should be a cell in the dmas property. Yes, the CRCI should be part of the dma cells for the ADM. That would make a channel + crci work as a virtual channel that is backed by a hardware channel. The only thing that has to be dealt with is the protocol difference between the BAM and ADM dma blocks, which is what the v1 compatible tells us. Regards, Andy
Re: [PATCH v2] spin loop primitives for busy waiting
Nicholas Piggin writes: > Current busy-wait loops are implemented by repeatedly calling cpu_relax() > to give an arch option for a low-latency option to improve power and/or > SMT resource contention. > > This poses some difficulties for powerpc, which has SMT priority setting > instructions (priorities determine how ifetch cycles are apportioned). > powerpc's cpu_relax() is implemented by setting a low priority then > setting normal priority. This has several problems: > > - Changing thread priority can have some execution cost and potential >impact to other threads in the core. It's inefficient to execute them >every time around a busy-wait loop. > > - Depending on implementation details, a `low ; medium` sequence may >not have much if any affect. Some software with similar pattern >actually inserts a lot of nops between, in order to cause a few fetch >cycles with the low priority. > > - The busy-wait loop runs with regular priority. This might only be a few >fetch cycles, but if there are several threads running such loops, they >could cause a noticable impact on a non-idle thread. > > Implement spin_begin, spin_end primitives that can be used around busy > wait loops, which default to no-ops. And spin_cpu_relax which defaults to > cpu_relax. > > This will allow architectures to hook the entry and exit of busy-wait > loops, and will allow powerpc to set low SMT priority at entry, and > normal priority at exit. > > Suggested-by: Linus Torvalds > Signed-off-by: Nicholas Piggin > --- > > Since last time: > - Fixed spin_do_cond with initial test as suggested by Linus. > - Renamed it to spin_until_cond, which reads a little better. > > include/linux/processor.h | 70 > +++ > 1 file changed, 70 insertions(+) > create mode 100644 include/linux/processor.h I'm gonna merge this via the powerpc tree unless anyone objects. cheers
[PATCH] clocksource: timer-imx-gpt: Free and unmap region obtained by kzalloc/of_iomap.
In case of error at init time, rollback iomapping and kfree. Signed-off-by: Arvind Yadav --- drivers/clocksource/timer-imx-gpt.c | 19 ++- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/drivers/clocksource/timer-imx-gpt.c b/drivers/clocksource/timer-imx-gpt.c index f595460..1a8786c 100644 --- a/drivers/clocksource/timer-imx-gpt.c +++ b/drivers/clocksource/timer-imx-gpt.c @@ -489,12 +489,16 @@ static int __init mxc_timer_init_dt(struct device_node *np, enum imx_gpt_type t return -ENOMEM; imxtm->base = of_iomap(np, 0); - if (!imxtm->base) - return -ENXIO; + if (!imxtm->base) { + ret = -ENXIO; + goto error; + } imxtm->irq = irq_of_parse_and_map(np, 0); - if (imxtm->irq <= 0) - return -EINVAL; + if (imxtm->irq <= 0) { + ret = -EINVAL; + goto error_free; + } imxtm->clk_ipg = of_clk_get_by_name(np, "ipg"); @@ -507,11 +511,16 @@ static int __init mxc_timer_init_dt(struct device_node *np, enum imx_gpt_type t ret = _mxc_timer_init(imxtm); if (ret) - return ret; + goto error_free; initialized = 1; return 0; +error_free: + iounmap(imxtm->base); +error: + kfree(imxtm); + return ret; } static int __init imx1_timer_init_dt(struct device_node *np) -- 1.9.1
linux-next: manual merge of the kspp tree with the block tree
Hi Kees, Today's linux-next merge of the kspp tree got a conflict in: include/linux/fs.h between commit: c75b1d9421f8 ("fs: add fcntl() interface for setting/getting write life time hints") from the block tree and commit: 3abc2b3fcf5c ("randstruct: Mark various structs for randomization") from the kspp 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 include/linux/fs.h index 0ddaac8f66d9,8f28143486c4.. --- a/include/linux/fs.h +++ b/include/linux/fs.h @@@ -293,8 -275,7 +293,8 @@@ struct kiocb void (*ki_complete)(struct kiocb *iocb, long ret, long ret2); void*private; int ki_flags; + enum rw_hintki_hint; - }; + } __randomize_layout; static inline bool is_sync_kiocb(struct kiocb *kiocb) { @@@ -401,8 -392,7 +401,8 @@@ struct address_space gfp_t gfp_mask; /* implicit gfp mask for allocations */ struct list_headprivate_list; /* ditto */ void*private_data; /* ditto */ + errseq_twb_err; - } __attribute__((aligned(sizeof(long; + } __attribute__((aligned(sizeof(long __randomize_layout; /* * On most architectures that alignment is already the case; but * must be enforced here for CRIS, to let the least significant bit @@@ -881,8 -868,8 +881,9 @@@ struct file struct list_headf_tfile_llink; #endif /* #ifdef CONFIG_EPOLL */ struct address_space*f_mapping; + errseq_tf_md_wb_err; /* metadata wb error tracking */ - } __attribute__((aligned(4)));/* lest something weird decides that 2 is OK */ + } __randomize_layout + __attribute__((aligned(4)));/* lest something weird decides that 2 is OK */ struct file_handle { __u32 handle_bytes;
[PATCH v3 2/2] fs/dcache.c: fix spin lockup issue on nlru->lock
__list_lru_walk_one() acquires nlru spin lock (nlru->lock) for longer duration if there are more number of items in the lru list. As per the current code, it can hold the spin lock for upto maximum UINT_MAX entries at a time. So if there are more number of items in the lru list, then "BUG: spinlock lockup suspected" is observed in the below path - [] spin_bug+0x90 [] do_raw_spin_lock+0xfc [] _raw_spin_lock+0x28 [] list_lru_add+0x28 [] dput+0x1c8 [] path_put+0x20 [] terminate_walk+0x3c [] path_lookupat+0x100 [] filename_lookup+0x6c [] user_path_at_empty+0x54 [] SyS_faccessat+0xd0 [] el0_svc_naked+0x24 This nlru->lock is acquired by another CPU in this path - [] d_lru_shrink_move+0x34 [] dentry_lru_isolate_shrink+0x48 [] __list_lru_walk_one.isra.10+0x94 [] list_lru_walk_node+0x40 [] shrink_dcache_sb+0x60 [] do_remount_sb+0xbc [] do_emergency_remount+0xb0 [] process_one_work+0x228 [] worker_thread+0x2e0 [] kthread+0xf4 [] ret_from_fork+0x10 Fix this lockup by reducing the number of entries to be shrinked from the lru list to 1024 at once. Also, add cond_resched() before processing the lru list again. Link: http://marc.info/?t=14972286491&r=1&w=2 Fix-suggested-by: Jan kara Fix-suggested-by: Vladimir Davydov Signed-off-by: Sahitya Tummala --- v3: use list_lru_count() instead of freed in while loop to cover an extreme case where a single invocation of list_lru_walk() can skip all 1024 dentries, in which case 'freed' will be 0 forcing us to break the loop prematurely. v2: patch shrink_dcache_sb() instead of list_lru_walk() --- fs/dcache.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/fs/dcache.c b/fs/dcache.c index a9f995f..1161390 100644 --- a/fs/dcache.c +++ b/fs/dcache.c @@ -1133,11 +1133,12 @@ void shrink_dcache_sb(struct super_block *sb) LIST_HEAD(dispose); freed = list_lru_walk(&sb->s_dentry_lru, - dentry_lru_isolate_shrink, &dispose, UINT_MAX); + dentry_lru_isolate_shrink, &dispose, 1024); this_cpu_sub(nr_dentry_unused, freed); shrink_dentry_list(&dispose); - } while (freed > 0); + cond_resched(); + } while (list_lru_count(&sb->s_dentry_lru) > 0); } EXPORT_SYMBOL(shrink_dcache_sb); -- Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
[PATCH v3 1/2] mm/list_lru.c: fix list_lru_count_node() to be race free
list_lru_count_node() iterates over all memcgs to get the total number of entries on the node but it can race with memcg_drain_all_list_lrus(), which migrates the entries from a dead cgroup to another. This can return incorrect number of entries from list_lru_count_node(). Fix this by keeping track of entries per node and simply return it in list_lru_count_node(). Signed-off-by: Sahitya Tummala --- include/linux/list_lru.h | 1 + mm/list_lru.c| 14 ++ 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/include/linux/list_lru.h b/include/linux/list_lru.h index cb0ba9f..eff61bc 100644 --- a/include/linux/list_lru.h +++ b/include/linux/list_lru.h @@ -44,6 +44,7 @@ struct list_lru_node { /* for cgroup aware lrus points to per cgroup lists, otherwise NULL */ struct list_lru_memcg *memcg_lrus; #endif + long nr_count; } cacheline_aligned_in_smp; struct list_lru { diff --git a/mm/list_lru.c b/mm/list_lru.c index 234676e..d417b9f 100644 --- a/mm/list_lru.c +++ b/mm/list_lru.c @@ -117,6 +117,7 @@ bool list_lru_add(struct list_lru *lru, struct list_head *item) l = list_lru_from_kmem(nlru, item); list_add_tail(item, &l->list); l->nr_items++; + nlru->nr_count++; spin_unlock(&nlru->lock); return true; } @@ -136,6 +137,7 @@ bool list_lru_del(struct list_lru *lru, struct list_head *item) l = list_lru_from_kmem(nlru, item); list_del_init(item); l->nr_items--; + nlru->nr_count--; spin_unlock(&nlru->lock); return true; } @@ -183,15 +185,10 @@ unsigned long list_lru_count_one(struct list_lru *lru, unsigned long list_lru_count_node(struct list_lru *lru, int nid) { - long count = 0; - int memcg_idx; + struct list_lru_node *nlru; - count += __list_lru_count_one(lru, nid, -1); - if (list_lru_memcg_aware(lru)) { - for_each_memcg_cache_index(memcg_idx) - count += __list_lru_count_one(lru, nid, memcg_idx); - } - return count; + nlru = &lru->node[nid]; + return nlru->nr_count; } EXPORT_SYMBOL_GPL(list_lru_count_node); @@ -226,6 +223,7 @@ unsigned long list_lru_count_node(struct list_lru *lru, int nid) assert_spin_locked(&nlru->lock); case LRU_REMOVED: isolated++; + nlru->nr_count--; /* * If the lru lock has been dropped, our list * traversal is now invalid and so we have to -- Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
Re: arm64: build is broken on next-20170609 with merge-commit 9afca2c4e379 (arm64: allwinner: sun50i-a64: add dwmac-sun8i Ethernet driver)
Hi all, [Yes, top posted :-)] With the merge window approaching, this is just a reminder that this merge problem still exists. I assume that the sunxi tree will be merged into the arm-soc tree before going to Linus. On Sat, 10 Jun 2017 09:29:44 +1000 Stephen Rothwell wrote: > > On Fri, 9 Jun 2017 15:43:25 +0300 Yury Norov > wrote: > > > > Today's linux-next breaks build with: > > DTC arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dtb > > arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dtb: ERROR > > (duplicate_node_names): Duplicate node name /soc/ethernet@1c3 > > ERROR: Input tree has errors, aborting (use -f to force output) > > scripts/Makefile.lib:325: recipe for target > > 'arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dtb' failed > > make[2]: *** [arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dtb] > > Error 2 > > scripts/Makefile.build:561: recipe for target > > 'arch/arm64/boot/dts/allwinner' failed > > make[1]: *** [arch/arm64/boot/dts/allwinner] Error 2 > > arch/arm64/Makefile:130: recipe for target 'dtbs' failed > > make: *** [dtbs] Error 2 > > > > It seems, it's due to duplication error in merge-commit 9afca2c4e379. > > Removing it fixes the build. > > Thanks for the reports Yury and Sumit. I will apply the following > merge fixup patch when I next build linux-next (unless one of the > patches is removed in the mean time). > > From: Stephen Rothwell > Date: Sat, 10 Jun 2017 09:18:14 +1000 > Subject: [PATCH] remove duplicate ethernet noce intruduced by merge > > caused by patch "arm64: allwinner: sun50i-a64: add dwmac-sun8i Ethernet > driver" being merged into the sunxi and net-next trees as different > commits (e53f67e981bc and 103aefa01c1b respectively). Presumably the > former will turn up in the arm-soc tree before being sent to Linus. > > Signed-off-by: Stephen Rothwell > --- > arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 20 > 1 file changed, 20 deletions(-) > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi > b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi > index 1b36aab5afb6..9d00622ce845 100644 > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi > @@ -469,26 +469,6 @@ > }; > }; > > - emac: ethernet@1c3 { > - compatible = "allwinner,sun50i-a64-emac"; > - syscon = <&syscon>; > - reg = <0x01c3 0x100>; > - interrupts = ; > - interrupt-names = "macirq"; > - resets = <&ccu RST_BUS_EMAC>; > - reset-names = "stmmaceth"; > - clocks = <&ccu CLK_BUS_EMAC>; > - clock-names = "stmmaceth"; > - status = "disabled"; > - #address-cells = <1>; > - #size-cells = <0>; > - > - mdio: mdio { > - #address-cells = <1>; > - #size-cells = <0>; > - }; > - }; > - > gic: interrupt-controller@1c81000 { > compatible = "arm,gic-400"; > reg = <0x01c81000 0x1000>, > -- > 2.11.0 -- Cheers, Stephen Rothwell
Re: linux-next: manual merge of the akpm-current tree with the random tree
Hi Andrew, [Yes, top posting :-)] With the merge window approaching, just a reminder that this conflict still exists. On Thu, 8 Jun 2017 16:28:12 +1000 Stephen Rothwell wrote: > > Today's linux-next merge of the akpm-current tree got a conflict in: > > include/linux/random.h > > between commit: > > 60473a13020f ("random: add get_random_{bytes,u32,u64,int,long,once}_wait > family") > > from the random tree and commit: > > d7802aa82f66 ("random,stackprotect: introduce get_random_canary function") > > from the akpm-current 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 include/linux/random.h > index 4aecc339558d,1fa0dc880bd7.. > --- a/include/linux/random.h > +++ b/include/linux/random.h > @@@ -58,31 -57,27 +58,52 @@@ static inline unsigned long get_random_ > #endif > } > > +/* Calls wait_for_random_bytes() and then calls get_random_bytes(buf, > nbytes). > + * Returns the result of the call to wait_for_random_bytes. */ > +static inline int get_random_bytes_wait(void *buf, int nbytes) > +{ > +int ret = wait_for_random_bytes(); > +if (unlikely(ret)) > +return ret; > +get_random_bytes(buf, nbytes); > +return 0; > +} > + > +#define declare_get_random_var_wait(var) \ > +static inline int get_random_ ## var ## _wait(var *out) { \ > +int ret = wait_for_random_bytes(); \ > +if (unlikely(ret)) \ > +return ret; \ > +*out = get_random_ ## var(); \ > +return 0; \ > +} > +declare_get_random_var_wait(u32) > +declare_get_random_var_wait(u64) > +declare_get_random_var_wait(int) > +declare_get_random_var_wait(long) > +#undef declare_get_random_var > + > + /* > + * On 64-bit architectures, protect against non-terminated C string > overflows > + * by zeroing out the first byte of the canary; this leaves 56 bits of > entropy. > + */ > + #ifdef CONFIG_64BIT > + # ifdef __LITTLE_ENDIAN > + # define CANARY_MASK 0xff00UL > + # else /* big endian, 64 bits: */ > + # define CANARY_MASK 0x00ffUL > + # endif > + #else /* 32 bits: */ > + # define CANARY_MASK 0xUL > + #endif > + > + static inline unsigned long get_random_canary(void) > + { > + unsigned long val = get_random_long(); > + > + return val & CANARY_MASK; > + } > + > unsigned long randomize_page(unsigned long start, unsigned long range); > > u32 prandom_u32(void); -- Cheers, Stephen Rothwell
Re: [kernel-sched-cputime] question about probable bug in cputime_adjust()
On Wed, Jun 28, 2017 at 7:35 AM, Frans Klaver wrote: > On Wed, Jun 28, 2017 at 1:03 AM, Gustavo A. R. Silva > wrote: >> >> Hello everybody, >> >> While looking into Coverity ID 1371643 I ran into the following piece of >> code at kernel/sched/cputime.c:568: >> >> 568/* >> 569 * Adjust tick based cputime random precision against scheduler runtime >> 570 * accounting. >> 571 * >> 572 * Tick based cputime accounting depend on random scheduling timeslices >> of a >> 573 * task to be interrupted or not by the timer. Depending on these >> 574 * circumstances, the number of these interrupts may be over or >> 575 * under-optimistic, matching the real user and system cputime with a >> variable >> 576 * precision. >> 577 * >> 578 * Fix this by scaling these tick based values against the total runtime >> 579 * accounted by the CFS scheduler. >> 580 * >> 581 * This code provides the following guarantees: >> 582 * >> 583 * stime + utime == rtime >> 584 * stime_i+1 >= stime_i, utime_i+1 >= utime_i >> 585 * >> 586 * Assuming that rtime_i+1 >= rtime_i. >> 587 */ >> 588static void cputime_adjust(struct task_cputime *curr, >> 589 struct prev_cputime *prev, >> 590 u64 *ut, u64 *st) >> 591{ >> 592u64 rtime, stime, utime; >> 593unsigned long flags; >> 594 >> 595/* Serialize concurrent callers such that we can honour our >> guarantees */ >> 596raw_spin_lock_irqsave(&prev->lock, flags); >> 597rtime = curr->sum_exec_runtime; >> 598 >> 599/* >> 600 * This is possible under two circumstances: >> 601 * - rtime isn't monotonic after all (a bug); >> 602 * - we got reordered by the lock. >> 603 * >> 604 * In both cases this acts as a filter such that the rest of the >> code >> 605 * can assume it is monotonic regardless of anything else. >> 606 */ >> 607if (prev->stime + prev->utime >= rtime) >> 608goto out; >> 609 >> 610stime = curr->stime; >> 611utime = curr->utime; >> 612 >> 613/* >> 614 * If either stime or both stime and utime are 0, assume all >> runtime is >> 615 * userspace. Once a task gets some ticks, the monotonicy code at >> 616 * 'update' will ensure things converge to the observed ratio. >> 617 */ >> 618if (stime == 0) { >> 619utime = rtime; >> 620goto update; >> 621} >> 622 >> 623if (utime == 0) { >> 624stime = rtime; >> 625goto update; >> 626} >> 627 >> 628stime = scale_stime(stime, rtime, stime + utime); >> 629 >> 630update: >> 631/* >> 632 * Make sure stime doesn't go backwards; this preserves >> monotonicity >> 633 * for utime because rtime is monotonic. >> 634 * >> 635 * utime_i+1 = rtime_i+1 - stime_i >> 636 *= rtime_i+1 - (rtime_i - utime_i) >> 637 *= (rtime_i+1 - rtime_i) + utime_i >> 638 *>= utime_i >> 639 */ >> 640if (stime < prev->stime) >> 641stime = prev->stime; >> 642utime = rtime - stime; >> 643 >> 644/* >> 645 * Make sure utime doesn't go backwards; this still preserves >> 646 * monotonicity for stime, analogous argument to above. >> 647 */ >> 648if (utime < prev->utime) { >> 649utime = prev->utime; >> 650stime = rtime - utime; >> 651} >> 652 >> 653prev->stime = stime; >> 654prev->utime = utime; >> 655out: >> 656*ut = prev->utime; >> 657*st = prev->stime; >> 658raw_spin_unlock_irqrestore(&prev->lock, flags); >> 659} >> >> >> The issue here is that the value assigned to variable utime at line 619 is >> overwritten at line 642, which would make such variable assignment useless. > > It isn't completely useless, since utime is used in line 628 to calculate > stime. Oh, I missed 'goto update'. Never mind about this one. >> But I'm suspicious that such assignment is actually correct and that line >> 642 should be included into the IF block at line 640. Something similar to >> the following patch: >> >> --- a/kernel/sched/cputime.c >> +++ b/kernel/sched/cputime.c >> @@ -637,9 +637,10 @@ static void cputime_adjust(struct task_cputime *curr, >> *= (rtime_i+1 - rtime_i) + utime_i >> *>= utime_i >> */ >> - if (stime < prev->stime) >> + if (stime < prev->stime) { >> stime = prev->stime; >> - utime = rtime - stime; >> + utime = rtime - stime; >> + } >> >> >> If you confirm this, I will send a patch in a full and proper form. >> >> I'd really appreciate your comments. > > If you do that, how would you meet the guarantee made in line 583? > > Frans
Re: linux-next: manual merge of the kvms390 tree with the kvm-arm tree
Hi all, On Fri, 9 Jun 2017 14:28:56 +1000 Stephen Rothwell wrote: > > Today's linux-next merge of the kvms390 tree got a conflict in: > > arch/s390/include/asm/kvm_host.h > > between commit: > > 2387149eade2 ("KVM: improve arch vcpu request defining") > > from the kvm-arm tree and commit: > > 8611a6a64661 ("KVM: s390: CMMA tracking, ESSA emulation, migration mode") > > from the kvms390 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 arch/s390/include/asm/kvm_host.h > index 9c3bd94204ac,a8cafed79eb4.. > --- a/arch/s390/include/asm/kvm_host.h > +++ b/arch/s390/include/asm/kvm_host.h > @@@ -42,9 -42,11 +42,11 @@@ > #define KVM_HALT_POLL_NS_DEFAULT 8 > > /* s390-specific vcpu->requests bit members */ > -#define KVM_REQ_ENABLE_IBS 8 > -#define KVM_REQ_DISABLE_IBS9 > -#define KVM_REQ_ICPT_OPEREXC 10 > -#define KVM_REQ_START_MIGRATION 11 > -#define KVM_REQ_STOP_MIGRATION12 > +#define KVM_REQ_ENABLE_IBS KVM_ARCH_REQ(0) > +#define KVM_REQ_DISABLE_IBS KVM_ARCH_REQ(1) > +#define KVM_REQ_ICPT_OPEREXCKVM_ARCH_REQ(2) > ++#define KVM_REQ_START_MIGRATION KVM_ARCH_REQ(3) > ++#define KVM_REQ_STOP_MIGRATION KVM_ARCH_REQ(4) > > #define SIGP_CTRL_C 0x80 > #define SIGP_CTRL_SCN_MASK 0x3f With the merge window appraoching, I assume that these 2 trees will merge in the kvm tree soon. This is just a reminder that this conflict still exists (I think). -- Cheers, Stephen Rothwell
linux-next: build warnings after merge of the scsi-mkp tree
Hi Martin, After merging the scsi-mkp tree, today's linux-next build (powerpc_ppc64_defconfig) produced these warnings: In file included from include/linux/byteorder/big_endian.h:4:0, from arch/powerpc/include/uapi/asm/byteorder.h:13, from include/asm-generic/bitops/le.h:5, from arch/powerpc/include/asm/bitops.h:246, from include/linux/bitops.h:36, from include/linux/kernel.h:10, from include/asm-generic/bug.h:15, from arch/powerpc/include/asm/bug.h:127, from include/linux/bug.h:4, from arch/powerpc/include/asm/mmu.h:125, from arch/powerpc/include/asm/lppaca.h:36, from arch/powerpc/include/asm/paca.h:21, from arch/powerpc/include/asm/current.h:16, from include/linux/sched.h:11, from include/linux/blkdev.h:4, from include/linux/blk-mq.h:4, from drivers/scsi/qla2xxx/qla_nvme.h:10, from drivers/scsi/qla2xxx/qla_nvme.c:7: drivers/scsi/qla2xxx/qla_nvme.c: In function 'qla2x00_start_nvme_mq': include/uapi/linux/byteorder/big_endian.h:32:26: warning: large integer implicitly truncated to unsigned type [-Woverflow] #define __cpu_to_le32(x) ((__force __le32)__swab32((x))) ^ include/linux/byteorder/generic.h:87:21: note: in expansion of macro '__cpu_to_le32' #define cpu_to_le32 __cpu_to_le32 ^ drivers/scsi/qla2xxx/qla_nvme.c:444:27: note: in expansion of macro 'cpu_to_le32' cont_pkt->entry_type = cpu_to_le32(CONTINUE_A64_TYPE); ^ drivers/scsi/qla2xxx/qla_nvme.c: At top level: drivers/scsi/qla2xxx/qla_nvme.c:667:13: warning: 'qla_nvme_unregister_remote_port' defined but not used [-Wunused-function] static void qla_nvme_unregister_remote_port(struct work_struct *work) ^ drivers/scsi/qla2xxx/qla_nvme.c:604:12: warning: 'qla_nvme_wait_on_rport_del' defined but not used [-Wunused-function] static int qla_nvme_wait_on_rport_del(fc_port_t *fcport) ^ drivers/scsi/qla2xxx/qla_nvme.c:634:13: warning: 'qla_nvme_abort_all' defined but not used [-Wunused-function] static void qla_nvme_abort_all(fc_port_t *fcport) ^ Introduced by commit e84067d74301 ("scsi: qla2xxx: Add FC-NVMe F/W initialization and transport registration") -- Cheers, Stephen Rothwell
[PATCH] clocksource: timer-ti-32k: Unmap region obtained by of_iomap.
In case of error at init time, rollback iomapping. Signed-off-by: Arvind Yadav --- drivers/clocksource/timer-ti-32k.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/clocksource/timer-ti-32k.c b/drivers/clocksource/timer-ti-32k.c index 6240677..658d759 100644 --- a/drivers/clocksource/timer-ti-32k.c +++ b/drivers/clocksource/timer-ti-32k.c @@ -116,6 +116,7 @@ static int __init ti_32k_timer_init(struct device_node *np) ret = clocksource_register_hz(&ti_32k_timer.cs, 32768); if (ret) { pr_err("32k_counter: can't register clocksource\n"); + iounmap(ti_32k_timer.base); return ret; } -- 1.9.1
Re: [PATCH 2/6] drivers: perf: hisi: Add support for HiSilicon SoC uncore PMU driver
Hi, On 2017/6/28 9:49, kbuild test robot wrote: > Hi Shaokun, > > [auto build test ERROR on next-20170619] > [also build test ERROR on v4.12-rc7] > [cannot apply to linus/master linux/master arm64/for-next/core v4.12-rc6 > v4.12-rc5 v4.12-rc4] > [if your patch is applied to the wrong git tree, please drop us a note to > help improve the system] > > url: > https://github.com/0day-ci/linux/commits/Shaokun-Zhang/Add-HiSilicon-SoC-uncore-Performance-Monitoring-Unit-driver/20170628-070841 > config: x86_64-allmodconfig (attached as .config) > compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 > reproduce: > # save the attached .config to linux build tree > make ARCH=x86_64 > > All errors (new ones prefixed by >>): > >drivers/perf/hisilicon/hisi_uncore_pmu.c: In function > 'hisi_read_scl_and_ccl_id': >>> drivers/perf/hisilicon/hisi_uncore_pmu.c:72:10: error: implicit declaration >>> of function 'read_cpuid_mpidr' [-Werror=implicit-function-declaration] > mpidr = read_cpuid_mpidr(); > ^~~~ >>> drivers/perf/hisilicon/hisi_uncore_pmu.c:73:14: error: 'MPIDR_MT_BITMASK' >>> undeclared (first use in this function) > if (mpidr & MPIDR_MT_BITMASK) { > ^~~~ >drivers/perf/hisilicon/hisi_uncore_pmu.c:73:14: note: each undeclared > identifier is reported only once for each function it appears in >>> drivers/perf/hisilicon/hisi_uncore_pmu.c:75:14: error: implicit declaration >>> of function 'MPIDR_AFFINITY_LEVEL' [-Werror=implicit-function-declaration] >*scl_id = MPIDR_AFFINITY_LEVEL(mpidr, 3); > ^~~~ >cc1: some warnings being treated as errors > Apologies for my modified drivers/perf/Kconfig for HISI_PMU. It depends on ARM64 and i shall remove COMPILE_TEST to fix it. Thanks. Shaokun > vim +/read_cpuid_mpidr +72 drivers/perf/hisilicon/hisi_uncore_pmu.c > > 66 > 67/* Read Super CPU cluster and CPU cluster ID from MPIDR_EL1 */ > 68void hisi_read_scl_and_ccl_id(u32 *scl_id, u32 *ccl_id) > 69{ > 70u64 mpidr; > 71 > > 72mpidr = read_cpuid_mpidr(); > > 73if (mpidr & MPIDR_MT_BITMASK) { > 74if (scl_id) > > 75*scl_id = MPIDR_AFFINITY_LEVEL(mpidr, > 3); > 76if (ccl_id) > 77*ccl_id = MPIDR_AFFINITY_LEVEL(mpidr, > 2); > 78} else { > > --- > 0-DAY kernel test infrastructureOpen Source Technology Center > https://lists.01.org/pipermail/kbuild-all Intel Corporation > > > > ___ > linuxarm mailing list > linux...@huawei.com > http://rnd-openeuler.huawei.com/mailman/listinfo/linuxarm >
linux-next: build warning after merge of the libata tree
Hi Tejun, After merging the libata tree, today's linux-next build (arm multi_v7_defconfig) produced this warning: drivers/ata/libata-scsi.c: In function 'ata_scsi_var_len_cdb_xlat': drivers/ata/libata-scsi.c:4194:1: warning: label 'unspprt_sa' defined but not used [-Wunused-label] unspprt_sa: ^ Introduced by commit b1ffbf854e08 ("libata: Support for an ATA PASS-THROUGH(32) command.") -- Cheers, Stephen Rothwell
Re: linux-next: build failure after merge of the kspp tree
On Tue, 27 Jun 2017, Kees Cook wrote: > On Mon, Jun 26, 2017 at 8:33 PM, James Morris wrote: > > On Mon, 26 Jun 2017, Kees Cook wrote: > > > >> >> Fixes: 8014370f1257 ("apparmor: move path_link mediation to using > >> >> labels") > >> >> Signed-off-by: Stephen Rothwell > >> > Acked-by: John Johansen > >> > >> Hi James, > >> > >> Just a ping; this needs to get into -next to avoid build errors. > > > > Surely Linus will resolve this when he pulls the trees in? > > It's not a merge glitch, it's a refactoring glitch. John's commit in > security-next ("apparmor: move path_link mediation to using labels") > undid an earlier commit 8486adf0d755 ("apparmor: use designated > initializers") from v4.11. This patch is needed for security-next. > Thanks. Applied to git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git next -- James Morris
Re: linux-next: manual merge of the target-updates tree with the scsi tree
Hi all, On Wed, 28 Jun 2017 15:40:59 +1000 Stephen Rothwell wrote: > > Today's linux-next merge of the target-updates tree got a conflict in: > > drivers/scsi/qla2xxx/qla_target.c > > between commits: > > f775bd14e44d ("scsi: qla2xxx: Convert 32-bit LUN usage to 64-bit") > 82de802ad46e ("scsi: qla2xxx: Preparation for Target MQ.") > > from the scsi tree and commit: > > 6b26726af699 ("qla2xxx: Convert QLA_TGT_ABTS to > TARGET_SCF_LOOKUP_LUN_FROM_TAG") > > from the target-updates tree. > > I fixed it up (I think - see below) and can carry the fix as OK, that was no right :-( I have added the following fix patch as well (I suggest someone provide me with the correct fixup for tomorrow, please). From: Stephen Rothwell Date: Wed, 28 Jun 2017 15:44:27 +1000 Subject: [PATCH] qla2xxx: fix up for bad merge fix Signed-off-by: Stephen Rothwell --- drivers/scsi/qla2xxx/qla_target.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c index 9a054439560d..42a3f5be818c 100644 --- a/drivers/scsi/qla2xxx/qla_target.c +++ b/drivers/scsi/qla2xxx/qla_target.c @@ -1875,7 +1875,6 @@ static int __qlt_24xx_handle_abts(struct scsi_qla_host *vha, { struct qla_hw_data *ha = vha->hw; struct qla_tgt_mgmt_cmd *mcmd; - struct qla_tgt_cmd *cmd; int rc; if (abort_cmd_for_tag(vha, abts->exchange_addr_to_abort)) { @@ -1898,14 +1897,13 @@ static int __qlt_24xx_handle_abts(struct scsi_qla_host *vha, } memset(mcmd, 0, sizeof(*mcmd)); - cmd = container_of(se_cmd, struct qla_tgt_cmd, se_cmd); mcmd->sess = sess; memcpy(&mcmd->orig_iocb.abts, abts, sizeof(mcmd->orig_iocb.abts)); mcmd->reset_count = ha->base_qpair->chip_reset; mcmd->tmr_func = QLA_TGT_ABTS; mcmd->qpair = ha->base_qpair; - rc = ha->tgt.tgt_ops->handle_tmr(mcmd, cmd->unpacked_lun, mcmd->tmr_func, + rc = ha->tgt.tgt_ops->handle_tmr(mcmd, 0, mcmd->tmr_func, abts->exchange_addr_to_abort); if (rc != 0) { ql_dbg(ql_dbg_tgt_mgt, vha, 0xf052, -- 2.11.0 -- Cheers, Stephen Rothwell
[PATCH v2] drm: tilcdc: tilcdc_tfp410: make of_device_ids const.
of_device_ids are not supposed to change at runtime. All functions working with of_device_ids provided by work with const of_device_ids. So mark the non-const structs as const. File size before: textdata bss dec hex filename 1496 592 02088 828 drivers/gpu/drm/tilcdc/tilcdc_tfp410.o File size after constify: textdata bss dec hex filename 1880 176 02056 808 drivers/gpu/drm/tilcdc/tilcdc_tfp410.o Signed-off-by: Arvind Yadav --- Changes in v1: Subject was not correct. drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c index aabfad8..236ca1e 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c @@ -290,8 +290,6 @@ static int tfp410_modeset_init(struct tilcdc_module *mod, struct drm_device *dev * Device: */ -static struct of_device_id tfp410_of_match[]; - static int tfp410_probe(struct platform_device *pdev) { struct device_node *node = pdev->dev.of_node; @@ -376,7 +374,7 @@ static int tfp410_remove(struct platform_device *pdev) return 0; } -static struct of_device_id tfp410_of_match[] = { +static const struct of_device_id tfp410_of_match[] = { { .compatible = "ti,tilcdc,tfp410", }, { }, }; -- 1.9.1
Re: [PATCH 0/3] Enable namespaced file capabilities
On Fri, Jun 23, 2017 at 10:01:46AM +0300, Amir Goldstein wrote: > On Thu, Jun 22, 2017 at 9:59 PM, Stefan Berger > wrote: > > This series of patches primary goal is to enable file capabilities > > in user namespaces without affecting the file capabilities that are > > effective on the host. This is to prevent that any unprivileged user > > on the host maps his own uid to root in a private namespace, writes > > the xattr, and executes the file with privilege on the host. > > > > We achieve this goal by writing extended attributes with a different > > name when a user namespace is used. If for example the root user > > in a user namespace writes the security.capability xattr, the name > > of the xattr that is actually written is encoded as > > security.capability@uid=1000 for root mapped to uid 1000 on the host. > > When listing the xattrs on the host, the existing security.capability > > as well as the security.capability@uid=1000 will be shown. Inside the > > namespace only 'security.capability', with the value of > > security.capability@uid=1000, is visible. > > > > Am I the only one who thinks that suffix is perhaps not the best grammar > to use for this namespace? > xattrs are clearly namespaced by prefix, so it seems right to me to keep > it that way - define a new special xattr namespace "ns" and only if that > prefix exists, the @uid suffix will be parsed. > This could be either ns.security.capability@uid=1000 or > ns@uid=1000.security.capability. The latter seems more correct to me, > because then we will be able to namespace any xattr without having to > protect from "unprivileged xattr injection", i.e.: > setfattr -n "user.whatever.foo@uid=0" > > Amir. Hi Amir, I was liking the prefix at first, but I'm actually not sure it's worth it. THe main advantage would be so that checking for namespace or other tags could be done always at the same offset simplifying the parser. But since we will want to only handle namespacing for some tags, and potentially differently for each task, it won't actually be simpler, I don't think. On the other hand we do want to make sure that the syntax we use is generally usable, so I think simply specifying that >1 tags can each be separate by '@' should suffice. So for now we'd only have security.capability@uid=10 soon we'd hopefully have security.ima@uid=10 and eventually trusted.blarb@foo=bar -serge
linux-next: manual merge of the target-updates tree with the scsi tree
Hi Nicholas, Today's linux-next merge of the target-updates tree got a conflict in: drivers/scsi/qla2xxx/qla_target.c between commits: f775bd14e44d ("scsi: qla2xxx: Convert 32-bit LUN usage to 64-bit") 82de802ad46e ("scsi: qla2xxx: Preparation for Target MQ.") from the scsi tree and commit: 6b26726af699 ("qla2xxx: Convert QLA_TGT_ABTS to TARGET_SCF_LOOKUP_LUN_FROM_TAG") from the target-updates tree. I fixed it up (I think - 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/scsi/qla2xxx/qla_target.c index 2a0173e5d10e,401e245477d4.. --- a/drivers/scsi/qla2xxx/qla_target.c +++ b/drivers/scsi/qla2xxx/qla_target.c @@@ -1874,36 -1847,13 +1874,15 @@@ static int __qlt_24xx_handle_abts(struc struct abts_recv_from_24xx *abts, struct fc_port *sess) { struct qla_hw_data *ha = vha->hw; - struct se_session *se_sess = sess->se_sess; struct qla_tgt_mgmt_cmd *mcmd; + struct qla_tgt_cmd *cmd; - struct se_cmd *se_cmd; int rc; - bool found_lun = false; - unsigned long flags; - spin_lock_irqsave(&se_sess->sess_cmd_lock, flags); - list_for_each_entry(se_cmd, &se_sess->sess_cmd_list, se_cmd_list) { - if (se_cmd->tag == abts->exchange_addr_to_abort) { - found_lun = true; - break; - } - } - spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags); - - /* cmd not in LIO lists, look in qla list */ - if (!found_lun) { - if (abort_cmd_for_tag(vha, abts->exchange_addr_to_abort)) { - /* send TASK_ABORT response immediately */ - qlt_24xx_send_abts_resp(ha->base_qpair, abts, - FCP_TMF_CMPL, false); - return 0; - } else { - ql_dbg(ql_dbg_tgt_mgt, vha, 0xf081, - "unable to find cmd in driver or LIO for tag 0x%x\n", - abts->exchange_addr_to_abort); - return -ENOENT; - } + if (abort_cmd_for_tag(vha, abts->exchange_addr_to_abort)) { + /* send TASK_ABORT response immediately */ - qlt_24xx_send_abts_resp(vha, abts, FCP_TMF_CMPL, false); ++ qlt_24xx_send_abts_resp(ha->base_qpair, abts, ++ FCP_TMF_CMPL, false); + return 0; } ql_dbg(ql_dbg_tgt_mgt, vha, 0xf00f,
[PATCH v2] drm: tilcdc: tilcdc_panel: make of_device_ids const.
of_device_ids are not supposed to change at runtime. All functions working with of_device_ids provided by work with const of_device_ids. So mark the non-const structs as const. File size before: textdata bss dec hex filename 1531 592 02123 84b drivers/gpu/drm/tilcdc/tilcdc_panel.o File size after constify: textdata bss dec hex filename 1915 176 02091 82b drivers/gpu/drm/tilcdc/tilcdc_panel.o Signed-off-by: Arvind Yadav --- Changes in v1: Subject was not correct. drivers/gpu/drm/tilcdc/tilcdc_panel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/tilcdc/tilcdc_panel.c b/drivers/gpu/drm/tilcdc/tilcdc_panel.c index 28c3e2f..2abe7c4 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_panel.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_panel.c @@ -419,7 +419,7 @@ static int panel_remove(struct platform_device *pdev) return 0; } -static struct of_device_id panel_of_match[] = { +static const struct of_device_id panel_of_match[] = { { .compatible = "ti,tilcdc,panel", }, { }, }; -- 1.9.1
Re: [PATCH BUGFIX V2] block, bfq: update wr_busy_queues if needed on a queue split
> Il giorno 27 giu 2017, alle ore 20:29, Jens Axboe ha > scritto: > > On 06/27/2017 12:27 PM, Paolo Valente wrote: >> >>> Il giorno 27 giu 2017, alle ore 16:41, Jens Axboe ha >>> scritto: >>> >>> On 06/27/2017 12:09 AM, Paolo Valente wrote: > Il giorno 19 giu 2017, alle ore 13:43, Paolo Valente > ha scritto: > > This commit fixes a bug triggered by a non-trivial sequence of > events. These events are briefly described in the next two > paragraphs. The impatiens, or those who are familiar with queue > merging and splitting, can jump directly to the last paragraph. > > On each I/O-request arrival for a shared bfq_queue, i.e., for a > bfq_queue that is the result of the merge of two or more bfq_queues, > BFQ checks whether the shared bfq_queue has become seeky (i.e., if too > many random I/O requests have arrived for the bfq_queue; if the device > is non rotational, then random requests must be also small for the > bfq_queue to be tagged as seeky). If the shared bfq_queue is actually > detected as seeky, then a split occurs: the bfq I/O context of the > process that has issued the request is redirected from the shared > bfq_queue to a new non-shared bfq_queue. As a degenerate case, if the > shared bfq_queue actually happens to be shared only by one process > (because of previous splits), then no new bfq_queue is created: the > state of the shared bfq_queue is just changed from shared to non > shared. > > Regardless of whether a brand new non-shared bfq_queue is created, or > the pre-existing shared bfq_queue is just turned into a non-shared > bfq_queue, several parameters of the non-shared bfq_queue are set > (restored) to the original values they had when the bfq_queue > associated with the bfq I/O context of the process (that has just > issued an I/O request) was merged with the shared bfq_queue. One of > these parameters is the weight-raising state. > > If, on the split of a shared bfq_queue, > 1) a pre-existing shared bfq_queue is turned into a non-shared > bfq_queue; > 2) the previously shared bfq_queue happens to be busy; > 3) the weight-raising state of the previously shared bfq_queue happens > to change; > the number of weight-raised busy queues changes. The field > wr_busy_queues must then be updated accordingly, but such an update > was missing. This commit adds the missing update. > Hi Jens, any idea of the possible fate of this fix? >>> >>> I sort of missed this one. It looks trivial enough for 4.12, or we >>> can defer until 4.13. What do you think? >>> >> >> It should actually be something trivial, and hopefully correct, >> because a further throughput improvement (for BFQ), which depends on >> this fix, is now working properly, and we didn't see any regression so >> far. In addition, since this improvement is virtually ready for >> submission, further steps may be probably easier if this fix gets in >> sooner (whatever the luck of the improvement will be). > > OK, let's queue it up for 4.13 then. > My arguments was in favor of 4.12 actually. Maybe you did mean 4.12 here? Thanks, Paolo > -- > Jens Axboe
Re: [PATCH v3 1/2] spi: rockchip: Set GPIO_SS flag to enable Slave Select with GPIO CS
Hi, On Tue, Jun 27, 2017 at 9:38 PM, Jeffy Chen wrote: > The rockchip spi still requires slave selection when using GPIO CS. > > Signed-off-by: Jeffy Chen > --- > > Changes in v3: None > Changes in v2: None > > drivers/spi/spi-rockchip.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c > index bab9b13..52ea160 100644 > --- a/drivers/spi/spi-rockchip.c > +++ b/drivers/spi/spi-rockchip.c > @@ -749,6 +749,7 @@ static int rockchip_spi_probe(struct platform_device > *pdev) > master->transfer_one = rockchip_spi_transfer_one; > master->max_transfer_size = rockchip_spi_max_transfer_size; > master->handle_err = rockchip_spi_handle_err; > + master->flags = SPI_MASTER_GPIO_SS; > > rs->dma_tx.ch = dma_request_chan(rs->dev, "tx"); > if (IS_ERR(rs->dma_tx.ch)) { Reviewed-by: Douglas Anderson
Re: [REGRESSION] perf/core: PMU interrupts dropped if we entered the kernel in the "skid" region
Hi Kyle, I understand your requirement. Sorry I don't know the detail of rr debugger, but I guess if it just uses counter overflow to deliver a signal, could it set the counter without "exclude_kernel"? Another consideration is, I'm not sure if the kernel can accurately realize that if the interrupt is to trigger sampling or just deliver a signal. Userspace may know that but kernel may not. Anyway let's listen to more comments from community. Thanks Jin Yao On 6/28/2017 12:51 PM, Kyle Huey wrote: On Tue, Jun 27, 2017 at 7:09 PM, Jin, Yao wrote: Hi, In theory, the PMI interrupts in skid region should be dropped, right? No, why would they be dropped? My understanding of the situation is as follows: There is some time, call it t_0, where the hardware counter overflows. The PMU triggers an interrupt, but this is not instantaneous. Call the time when the interrupt is actually delivered t_1. Then t_1 - t_0 is the "skid". Note that if the counter is `exclude_kernel`, then at t_0 the CPU *must* be running a userspace program. But by t_1, the CPU may be doing something else. Your patch changed things so that if at t_1 the CPU is in the kernel, then the interrupt is discarded. But rr has programmed the counter to deliver a signal on overflow (via F_SETSIG on the fd returned by perf_event_open). This change results in the signal never being delivered, because the interrupt was ignored. (More accurately, the signal is delivered the *next* time the counter overflows, which is far past where we wanted to inject our asynchronous event into our tracee. It seems to me that it might be reasonable to ignore the interrupt if the purpose of the interrupt is to trigger sampling of the CPUs register state. But if the interrupt will trigger some other operation, such as a signal on an fd, then there's no reason to drop it. For a userspace debugger, is it the only choice that relies on the *skid* PMI interrupt? I don't understand this question, but hopefully the above clarified things. - Kyle Thanks Jin Yao On 6/28/2017 9:01 AM, Kyle Huey wrote: Sent again with LKML CCd, sorry for the noise. - Kyle On Tue, Jun 27, 2017 at 5:38 PM, Kyle Huey wrote: cc1582c231ea introduced a regression in v4.12.0-rc5, and appears to be a candidate for backporting to stable branches. rr, a userspace record and replay debugger[0], uses the PMU interrupt to stop a program during replay to inject asynchronous events such as signals. We are counting retired conditional branches in userspace only. This changeset causes the kernel to drop interrupts on the floor if, during the PMU interrupt's "skid" region, the CPU enters kernel mode for whatever reason. When replaying traces of complex programs such as Firefox, we intermittently fail to deliver asynchronous events on time, leading the replay to diverge from the recorded state. It seems like this change should, at a bare minimum, be limited to counters that actually perform sampling of register state when the interrupt fires. In our case, with the retired conditional branches counter restricted to counting userspace events only, it makes no difference that the PMU interrupt happened to be delivered in the kernel. As this makes rr unusable on complex applications and cannot be efficiently worked around, we would appreciate this being addressed before 4.12 is finalized, and the regression not being introduced to stable branches. Thanks, - Kyle [0] http://rr-project.org/
Re: [kernel-sched-cputime] question about probable bug in cputime_adjust()
On Wed, Jun 28, 2017 at 1:03 AM, Gustavo A. R. Silva wrote: > > Hello everybody, > > While looking into Coverity ID 1371643 I ran into the following piece of > code at kernel/sched/cputime.c:568: > > 568/* > 569 * Adjust tick based cputime random precision against scheduler runtime > 570 * accounting. > 571 * > 572 * Tick based cputime accounting depend on random scheduling timeslices > of a > 573 * task to be interrupted or not by the timer. Depending on these > 574 * circumstances, the number of these interrupts may be over or > 575 * under-optimistic, matching the real user and system cputime with a > variable > 576 * precision. > 577 * > 578 * Fix this by scaling these tick based values against the total runtime > 579 * accounted by the CFS scheduler. > 580 * > 581 * This code provides the following guarantees: > 582 * > 583 * stime + utime == rtime > 584 * stime_i+1 >= stime_i, utime_i+1 >= utime_i > 585 * > 586 * Assuming that rtime_i+1 >= rtime_i. > 587 */ > 588static void cputime_adjust(struct task_cputime *curr, > 589 struct prev_cputime *prev, > 590 u64 *ut, u64 *st) > 591{ > 592u64 rtime, stime, utime; > 593unsigned long flags; > 594 > 595/* Serialize concurrent callers such that we can honour our > guarantees */ > 596raw_spin_lock_irqsave(&prev->lock, flags); > 597rtime = curr->sum_exec_runtime; > 598 > 599/* > 600 * This is possible under two circumstances: > 601 * - rtime isn't monotonic after all (a bug); > 602 * - we got reordered by the lock. > 603 * > 604 * In both cases this acts as a filter such that the rest of the > code > 605 * can assume it is monotonic regardless of anything else. > 606 */ > 607if (prev->stime + prev->utime >= rtime) > 608goto out; > 609 > 610stime = curr->stime; > 611utime = curr->utime; > 612 > 613/* > 614 * If either stime or both stime and utime are 0, assume all > runtime is > 615 * userspace. Once a task gets some ticks, the monotonicy code at > 616 * 'update' will ensure things converge to the observed ratio. > 617 */ > 618if (stime == 0) { > 619utime = rtime; > 620goto update; > 621} > 622 > 623if (utime == 0) { > 624stime = rtime; > 625goto update; > 626} > 627 > 628stime = scale_stime(stime, rtime, stime + utime); > 629 > 630update: > 631/* > 632 * Make sure stime doesn't go backwards; this preserves > monotonicity > 633 * for utime because rtime is monotonic. > 634 * > 635 * utime_i+1 = rtime_i+1 - stime_i > 636 *= rtime_i+1 - (rtime_i - utime_i) > 637 *= (rtime_i+1 - rtime_i) + utime_i > 638 *>= utime_i > 639 */ > 640if (stime < prev->stime) > 641stime = prev->stime; > 642utime = rtime - stime; > 643 > 644/* > 645 * Make sure utime doesn't go backwards; this still preserves > 646 * monotonicity for stime, analogous argument to above. > 647 */ > 648if (utime < prev->utime) { > 649utime = prev->utime; > 650stime = rtime - utime; > 651} > 652 > 653prev->stime = stime; > 654prev->utime = utime; > 655out: > 656*ut = prev->utime; > 657*st = prev->stime; > 658raw_spin_unlock_irqrestore(&prev->lock, flags); > 659} > > > The issue here is that the value assigned to variable utime at line 619 is > overwritten at line 642, which would make such variable assignment useless. It isn't completely useless, since utime is used in line 628 to calculate stime. > But I'm suspicious that such assignment is actually correct and that line > 642 should be included into the IF block at line 640. Something similar to > the following patch: > > --- a/kernel/sched/cputime.c > +++ b/kernel/sched/cputime.c > @@ -637,9 +637,10 @@ static void cputime_adjust(struct task_cputime *curr, > *= (rtime_i+1 - rtime_i) + utime_i > *>= utime_i > */ > - if (stime < prev->stime) > + if (stime < prev->stime) { > stime = prev->stime; > - utime = rtime - stime; > + utime = rtime - stime; > + } > > > If you confirm this, I will send a patch in a full and proper form. > > I'd really appreciate your comments. If you do that, how would you meet the guarantee made in line 583? Frans
Re: [Intel-wired-lan] [PATCH v2 1/1] e1000e: Undo e1000e_pm_freeze if __e1000_shutdown fails
On Tue, Jun 27, 2017 at 10:51 PM, Jeff Kirsher wrote: > This was submitted and accepted into David Miller's net-next tree. I can > see if Dave can pull it into his net tree. DOes stable need to pick this > up as well? Nah if it landed somewhere at least I'm happy, we can carry the fixup for a while longer locally. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
Re: [PATCH v3 2/2] spi: rockchip: Disable Runtime PM when chip select is asserted
Jeffy, On Tue, Jun 27, 2017 at 9:38 PM, Jeffy Chen wrote: > The rockchip spi would stop driving pins when runtime suspended, which > might break slave's xfer(for example cros_ec). > > Since we have pullups on those pins, we only need to care about this > when the CS asserted. > > So let's keep the spi alive when chip select is asserted. > > Also use pm_runtime_put instead of pm_runtime_put_sync. > > Suggested-by: Doug Anderson > Signed-off-by: Jeffy Chen > > --- > > Changes in v3: > Store cs states in struct rockchip_spi, and use it to detect cs state > instead of hw register. > > Changes in v2: > Improve commit message and comments and coding style. > > drivers/spi/spi-rockchip.c | 51 > +++--- > 1 file changed, 26 insertions(+), 25 deletions(-) > > diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c > index 52ea160..0b4a52b 100644 > --- a/drivers/spi/spi-rockchip.c > +++ b/drivers/spi/spi-rockchip.c > @@ -25,6 +25,11 @@ > > #define DRIVER_NAME "rockchip-spi" > > +#define ROCKCHIP_SPI_CLR_BITS(reg, bits) \ > + writel_relaxed(readl_relaxed(reg) & ~(bits), reg) > +#define ROCKCHIP_SPI_SET_BITS(reg, bits) \ > + writel_relaxed(readl_relaxed(reg) | (bits), reg) > + > /* SPI register offsets */ > #define ROCKCHIP_SPI_CTRLR00x > #define ROCKCHIP_SPI_CTRLR10x0004 > @@ -149,6 +154,8 @@ > */ > #define ROCKCHIP_SPI_MAX_TRANLEN 0x > > +#define ROCKCHIP_SPI_MAX_CS_NUM2 > + > enum rockchip_ssi_type { > SSI_MOTO_SPI = 0, > SSI_TI_SSP, > @@ -193,6 +200,8 @@ struct rockchip_spi { > /* protect state */ > spinlock_t lock; > > + bool cs_asserted[ROCKCHIP_SPI_MAX_CS_NUM]; > + > u32 use_dma; > struct sg_table tx_sg; > struct sg_table rx_sg; > @@ -264,37 +273,29 @@ static inline u32 rx_max(struct rockchip_spi *rs) > > static void rockchip_spi_set_cs(struct spi_device *spi, bool enable) > { > - u32 ser; > struct spi_master *master = spi->master; > struct rockchip_spi *rs = spi_master_get_devdata(master); > + bool cs_asserted = !enable; > > - pm_runtime_get_sync(rs->dev); > + /* Return immediately for no-op */ > + if (cs_asserted == rs->cs_asserted[spi->chip_select]) > + return; > > - ser = readl_relaxed(rs->regs + ROCKCHIP_SPI_SER) & SER_MASK; > + if (cs_asserted) { > + /* Keep things powered as long as CS is asserted */ > + pm_runtime_get_sync(rs->dev); > > - /* > -* drivers/spi/spi.c: > -* static void spi_set_cs(struct spi_device *spi, bool enable) > -* { > -* if (spi->mode & SPI_CS_HIGH) > -* enable = !enable; > -* > -* if (spi->cs_gpio >= 0) > -* gpio_set_value(spi->cs_gpio, !enable); > -* else if (spi->master->set_cs) > -* spi->master->set_cs(spi, !enable); > -* } > -* > -* Note: enable(rockchip_spi_set_cs) = !enable(spi_set_cs) > -*/ > - if (!enable) > - ser |= 1 << spi->chip_select; > - else > - ser &= ~(1 << spi->chip_select); > + ROCKCHIP_SPI_SET_BITS(rs->regs + ROCKCHIP_SPI_SER, > + BIT(spi->chip_select)); > + } else { > + ROCKCHIP_SPI_CLR_BITS(rs->regs + ROCKCHIP_SPI_SER, > + BIT(spi->chip_select)); > > - writel_relaxed(ser, rs->regs + ROCKCHIP_SPI_SER); > + /* Drop reference from when we first asserted CS */ > + pm_runtime_put(rs->dev); > + } > > - pm_runtime_put_sync(rs->dev); > + rs->cs_asserted[spi->chip_select] = cs_asserted; Looks great! > } > > static int rockchip_spi_prepare_message(struct spi_master *master, > @@ -739,7 +740,7 @@ static int rockchip_spi_probe(struct platform_device > *pdev) > master->auto_runtime_pm = true; > master->bus_num = pdev->id; > master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_LOOP; > - master->num_chipselect = 2; > + master->num_chipselect = ROCKCHIP_SPI_MAX_CS_NUM; Reviewed-by: Douglas Anderson
Re: [PATCH] lib/zstd: use div_u64() to let it build on 32-bit
> Please don't top post. Sorry about that. > Which function needs 1KB of stack space? That's quite a lot. FSE_buildCTable_wksp(), FSE_compress_wksp(), and HUF_readDTableX4() required over 1 KB of stack space. > I can see in [1] that there are some on-stack buffers replaced by > pointers to the workspace. That's good, but I would like to know if > there's any hidden gem that grags the precious stack space. I've been hunting down functions that use up the most stack trace and replacing buffers with pointers to the workspace. I compiled the code with -Wframe-larger-than=512 and reduced the stack usage of all offending functions. In the next version of the patch, no function uses more than 400 B of stack space. We'll be porting the changes back upstream as well. > Hm, I'd suggest to create a version optimized for kernel, eg. expecting > that 4+ GB buffer will never be used and you can use the most fittin in > type. This should affect only the function signatures, not the > algorithm implementation, so porting future zstd changes should be > straightforward. If the functions were exposed, then I would agree 100%. However, since these are internal functions, and the rest of zstd uses size_t to represent buffer sizes, I think it would be awkward to change just FSE/HUF functions. I also prefer size_t because it is friendlier to the optimizer, especially the loop optimizer, since the compiler doesn't have to worry about unsigned overflow. On a related note, zstd performs automatic optimizations to improve compression speed and reduce memory usage when given small sources, which is the common case in the kernel.
Re: [PATCH 2/3] usb: phy: Add USB charger support
Hi, On 28 June 2017 at 10:44, kbuild test robot wrote: > Hi Baolin, > > [auto build test ERROR on balbi-usb/next] > [also build test ERROR on next-20170627] > [cannot apply to v4.12-rc7] > [if your patch is applied to the wrong git tree, please drop us a note to > help improve the system] > > url: > https://github.com/0day-ci/linux/commits/Baolin-Wang/include-uapi-usb-Introduce-USB-charger-type-and-state-definition/20170628-093530 > base: https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next > config: i386-randconfig-x076-06262345 (attached as .config) > compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 > reproduce: > # save the attached .config to linux build tree > make ARCH=i386 > > All errors (new ones prefixed by >>): > >drivers/power/supply/rt9455_charger.o: In function > `usb_phy_set_charger_current': >>> rt9455_charger.c:(.text+0x1660): multiple definition of >>> `usb_phy_set_charger_current' >drivers/power/supply/pda_power.o:pda_power.c:(.text+0xad0): first defined > here >drivers/power/supply/rt9455_charger.o: In function > `usb_phy_get_charger_current': >>> rt9455_charger.c:(.text+0x1670): multiple definition of >>> `usb_phy_get_charger_current' >drivers/power/supply/pda_power.o:pda_power.c:(.text+0xae0): first defined > here >drivers/power/supply/rt9455_charger.o: In function > `usb_phy_set_charger_state': >>> rt9455_charger.c:(.text+0x1680): multiple definition of >>> `usb_phy_set_charger_state' >drivers/power/supply/pda_power.o:pda_power.c:(.text+0xaf0): first defined > here Sorry for missing "static inline" for these exported functions, will fix in next version. -- Baolin.wang Best Regards
Re: [PATCH v4 5/5] g_NCR5380: Re-work PDMA loops
I'm afraid I accidentally introduced a regression into v4 of this patch. Ondrej, please test the patch below instead. Sorry for the inconvenience. diff --git a/drivers/scsi/g_NCR5380.c b/drivers/scsi/g_NCR5380.c index b1e0a08e49c1..98d5360b0c78 100644 --- a/drivers/scsi/g_NCR5380.c +++ b/drivers/scsi/g_NCR5380.c @@ -482,6 +482,30 @@ static void generic_NCR5380_release_resources(struct Scsi_Host *instance) release_mem_region(base, region_size); } +/* wait_for_53c80_access - wait for 53C80 registers to become accessible + * @hostdata: scsi host private data + * + * The registers within the 53C80 logic block are inaccessible until + * bit 7 in the 53C400 control status register gets asserted. + */ + +static void wait_for_53c80_access(struct NCR5380_hostdata *hostdata) +{ + int count = 1; + + do { + if (hostdata->board == BOARD_DTC3181E) + udelay(4); /* DTC436 chip hangs without this */ + if (NCR5380_read(hostdata->c400_ctl_status) & CSR_53C80_REG) + return; + } while (--count > 0); + + scmd_printk(KERN_ERR, hostdata->connected, + "53c80 registers not accessible, device will be reset\n"); + NCR5380_write(hostdata->c400_ctl_status, CSR_RESET); + NCR5380_write(hostdata->c400_ctl_status, CSR_BASE); +} + /** * generic_NCR5380_pread - pseudo DMA receive * @hostdata: scsi host private data @@ -494,18 +518,22 @@ static void generic_NCR5380_release_resources(struct Scsi_Host *instance) static inline int generic_NCR5380_pread(struct NCR5380_hostdata *hostdata, unsigned char *dst, int len) { - int blocks = len / 128; int start = 0; NCR5380_write(hostdata->c400_ctl_status, CSR_BASE | CSR_TRANS_DIR); - NCR5380_write(hostdata->c400_blk_cnt, blocks); - while (1) { - if (NCR5380_read(hostdata->c400_blk_cnt) == 0) + NCR5380_write(hostdata->c400_blk_cnt, len / 128); + + while (start < len) { + if (NCR5380_poll_politely2(hostdata, hostdata->c400_ctl_status, + CSR_HOST_BUF_NOT_RDY, 0, + hostdata->c400_ctl_status, + CSR_GATED_53C80_IRQ, + CSR_GATED_53C80_IRQ, HZ / 64) < 0) + break; + + if (NCR5380_read(hostdata->c400_ctl_status) & + CSR_GATED_53C80_IRQ) break; - if (NCR5380_read(hostdata->c400_ctl_status) & CSR_GATED_53C80_IRQ) - goto out_wait; - while (NCR5380_read(hostdata->c400_ctl_status) & CSR_HOST_BUF_NOT_RDY) - ; /* FIXME - no timeout */ if (hostdata->io_port && hostdata->io_width == 2) insw(hostdata->io_port + hostdata->c400_host_buf, @@ -518,38 +546,19 @@ static inline int generic_NCR5380_pread(struct NCR5380_hostdata *hostdata, hostdata->io + NCR53C400_host_buffer, 128); start += 128; - blocks--; - } - - if (blocks) { - while (NCR5380_read(hostdata->c400_ctl_status) & CSR_HOST_BUF_NOT_RDY) - ; /* FIXME - no timeout */ - - if (hostdata->io_port && hostdata->io_width == 2) - insw(hostdata->io_port + hostdata->c400_host_buf, - dst + start, 64); - else if (hostdata->io_port) - insb(hostdata->io_port + hostdata->c400_host_buf, - dst + start, 128); - else - memcpy_fromio(dst + start, - hostdata->io + NCR53C400_host_buffer, 128); - - start += 128; - blocks--; } - if (!(NCR5380_read(hostdata->c400_ctl_status) & CSR_GATED_53C80_IRQ)) - printk("53C400r: no 53C80 gated irq after transfer"); + hostdata->pdma_residual = NCR5380_read(hostdata->c400_blk_cnt) * 128; -out_wait: - hostdata->pdma_residual = len - start; - - /* wait for 53C80 registers to be available */ - while (!(NCR5380_read(hostdata->c400_ctl_status) & CSR_53C80_REG)) - ; + if (start < len) { + /* Detected a 53c80 interrupt (or worse). Reset 53c400 logic. */ + NCR5380_write(hostdata->c400_ctl_status, CSR_RESET); + NCR5380_write(hostdata->c400_ctl_status, CSR_BASE); + } else + wait_for_53c80_access(hostdata); - if (NCR5380_poll_politely(hostdata, BUS_AND_STATUS_REG, + if (hostdata->pdma_residual == 0 && + NCR5380_poll_politely(hostdata, BUS_AND_STATUS_REG, BASR_END_DMA_TRANSF
Re: [Patch v5 04/12] [media] s5p-mfc: Support MFCv10.10 buffer requirements
On Tue, 2017-06-27 at 22:30 +0200, Kamil Debski wrote: > Hi, > > Please find my comments inline. > > On 19 June 2017 at 07:10, Smitha T Murthy wrote: > > Aligning the luma_dpb_size, chroma_dpb_size, mv_size and me_buffer_size > > for MFCv10.10. > > > > Signed-off-by: Smitha T Murthy > > Reviewed-by: Andrzej Hajda > > --- > > drivers/media/platform/s5p-mfc/regs-mfc-v10.h | 19 + > > drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c | 95 > > +++-- > > drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.h | 2 + > > 3 files changed, 95 insertions(+), 21 deletions(-) > > > > diff --git a/drivers/media/platform/s5p-mfc/regs-mfc-v10.h > > b/drivers/media/platform/s5p-mfc/regs-mfc-v10.h > > index 1ca09d6..3f0dab3 100644 > > --- a/drivers/media/platform/s5p-mfc/regs-mfc-v10.h > > +++ b/drivers/media/platform/s5p-mfc/regs-mfc-v10.h > > @@ -32,5 +32,24 @@ > > #define MFC_VERSION_V100xA0 > > #define MFC_NUM_PORTS_V10 1 > > > > +/* MFCv10 codec defines*/ > > +#define S5P_FIMV_CODEC_HEVC_ENC 26 > > + > > +/* Encoder buffer size for MFC v10.0 */ > > +#define ENC_V100_BASE_SIZE(x, y) \ > > + (((x + 3) * (y + 3) * 8) \ > > + + ((y * 64) + 1280) * DIV_ROUND_UP(x, 8)) > > + > > +#define ENC_V100_H264_ME_SIZE(x, y) \ > > + (ENC_V100_BASE_SIZE(x, y) \ > > + + (DIV_ROUND_UP(x * y, 64) * 32)) > > + > > +#define ENC_V100_MPEG4_ME_SIZE(x, y) \ > > + (ENC_V100_BASE_SIZE(x, y) \ > > + + (DIV_ROUND_UP(x * y, 128) * 16)) > > + > > +#define ENC_V100_VP8_ME_SIZE(x, y) \ > > + ENC_V100_BASE_SIZE(x, y) > > + > > #endif /*_REGS_MFC_V10_H*/ > > > > diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c > > b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c > > index f1a8c53..83ea733 100644 > > --- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c > > +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c > > @@ -64,6 +64,7 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct > > s5p_mfc_ctx *ctx) > > { > > struct s5p_mfc_dev *dev = ctx->dev; > > unsigned int mb_width, mb_height; > > + unsigned int lcu_width = 0, lcu_height = 0; > > int ret; > > > > mb_width = MB_WIDTH(ctx->img_width); > > @@ -74,7 +75,9 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct > > s5p_mfc_ctx *ctx) > > ctx->luma_size, ctx->chroma_size, ctx->mv_size); > > mfc_debug(2, "Totals bufs: %d\n", ctx->total_dpb_count); > > } else if (ctx->type == MFCINST_ENCODER) { > > - if (IS_MFCV8_PLUS(dev)) > > + if (IS_MFCV10(dev)) {IZE_V10 (15 * SZ_1K) > > + ctx->tmv_buffer_size = 0; > > It would look much better to surround the above with braces, even > though it's only a single line. > I will add the braces in the next version. > > + } else if (IS_MFCV8_PLUS(dev)) > > ctx->tmv_buffer_size = S5P_FIMV_NUM_TMV_BUFFERS_V6 * > > ALIGN(S5P_FIMV_TMV_BUFFER_SIZE_V8(mb_width, > > mb_height), > > S5P_FIMV_TMV_BUFFER_ALIGN_V6); > > @@ -82,13 +85,36 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct > > s5p_mfc_ctx *ctx) > > ctx->tmv_buffer_size = S5P_FIMV_NUM_TMV_BUFFERS_V6 * > > ALIGN(S5P_FIMV_TMV_BUFFER_SIZE_V6(mb_width, > > mb_height), > > S5P_FIMV_TMV_BUFFER_ALIGN_V6); > > - > > - ctx->luma_dpb_size = ALIGN((mb_width * mb_height) * > > - S5P_FIMV_LUMA_MB_TO_PIXEL_V6, > > - S5P_FIMV_LUMA_DPB_BUFFER_ALIGN_V6); > > - ctx->chroma_dpb_size = ALIGN((mb_width * mb_height) * > > - S5P_FIMV_CHROMA_MB_TO_PIXEL_V6, > > - S5P_FIMV_CHROMA_DPB_BUFFER_ALIGN_V6); > > + if (IS_MFCV10(dev)) { > > + lcu_width = enc_lcu_width(ctx->img_width); > > + lcu_height = enc_lcu_height(ctx->img_height); > > + if (ctx->codec_mode != S5P_FIMV_CODEC_HEVC_ENC) { > > + ctx->luma_dpb_size = > > + ALIGN((mb_width * 16), 64) > > + * ALIGN((mb_height * 16), 32) > > + + 64; > > + ctx->chroma_dpb_size = > > + ALIGN((mb_width * 16), 64) > > + * (mb_height * 8) > > + + 64; > > + } else { > > + ctx->luma_dpb_size = > > + ALIGN((lcu_width * 32), 64) > > + * ALIGN((lcu_height * 32), 32) > > + + 64; > > +
[PATCH] IB/hns: Fix a potential use after free in 'hns_roce_v1_destroy_qp_work_fn()'
The last 'dev_dbg' call uses 'hr_qp->qpn'. However, 'hr_qp' may have been freed a few lines above. In order to fix it, take the value of 'hr_qp->qpn' earlier in the function and use it instead. Fixes: d838c481e025 ("IB/hns: Fix the bug when destroy qp") Signed-off-by: Christophe JAILLET --- drivers/infiniband/hw/hns/hns_roce_hw_v1.c | 12 +++- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c index 37d5d29597a4..825a0fc2 100644 --- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c @@ -3641,6 +3641,7 @@ static void hns_roce_v1_destroy_qp_work_fn(struct work_struct *work) struct hns_roce_dev *hr_dev; struct hns_roce_qp *hr_qp; struct device *dev; + unsigned long qpn; int ret; qp_work_entry = container_of(work, struct hns_roce_qp_work, work); @@ -3648,8 +3649,9 @@ static void hns_roce_v1_destroy_qp_work_fn(struct work_struct *work) dev = &hr_dev->pdev->dev; priv = (struct hns_roce_v1_priv *)hr_dev->hw->priv; hr_qp = qp_work_entry->qp; + qpn = hr_qp->qpn; - dev_dbg(dev, "Schedule destroy QP(0x%lx) work.\n", hr_qp->qpn); + dev_dbg(dev, "Schedule destroy QP(0x%lx) work.\n", qpn); qp_work_entry->sche_cnt++; @@ -3660,7 +3662,7 @@ static void hns_roce_v1_destroy_qp_work_fn(struct work_struct *work) &qp_work_entry->db_wait_stage); if (ret) { dev_err(dev, "Check QP(0x%lx) db process status failed!\n", - hr_qp->qpn); + qpn); return; } @@ -3674,7 +3676,7 @@ static void hns_roce_v1_destroy_qp_work_fn(struct work_struct *work) ret = hns_roce_v1_modify_qp(&hr_qp->ibqp, NULL, 0, hr_qp->state, IB_QPS_RESET); if (ret) { - dev_err(dev, "Modify QP(0x%lx) to RST failed!\n", hr_qp->qpn); + dev_err(dev, "Modify QP(0x%lx) to RST failed!\n", qpn); return; } @@ -3683,14 +3685,14 @@ static void hns_roce_v1_destroy_qp_work_fn(struct work_struct *work) if (hr_qp->ibqp.qp_type == IB_QPT_RC) { /* RC QP, release QPN */ - hns_roce_release_range_qp(hr_dev, hr_qp->qpn, 1); + hns_roce_release_range_qp(hr_dev, qpn, 1); kfree(hr_qp); } else kfree(hr_to_hr_sqp(hr_qp)); kfree(qp_work_entry); - dev_dbg(dev, "Accomplished destroy QP(0x%lx) work.\n", hr_qp->qpn); + dev_dbg(dev, "Accomplished destroy QP(0x%lx) work.\n", qpn); } int hns_roce_v1_destroy_qp(struct ib_qp *ibqp) -- 2.11.0
[PATCH] drm: tilcdc: tilcdc_panel: fsl_raid: make of_device_ids const.
of_device_ids are not supposed to change at runtime. All functions working with of_device_ids provided by work with const of_device_ids. So mark the non-const structs as const. File size before: textdata bss dec hex filename 1531 592 02123 84b drivers/gpu/drm/tilcdc/tilcdc_panel.o File size after constify: textdata bss dec hex filename 1915 176 02091 82b drivers/gpu/drm/tilcdc/tilcdc_panel.o Signed-off-by: Arvind Yadav --- drivers/gpu/drm/tilcdc/tilcdc_panel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/tilcdc/tilcdc_panel.c b/drivers/gpu/drm/tilcdc/tilcdc_panel.c index 28c3e2f..2abe7c4 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_panel.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_panel.c @@ -419,7 +419,7 @@ static int panel_remove(struct platform_device *pdev) return 0; } -static struct of_device_id panel_of_match[] = { +static const struct of_device_id panel_of_match[] = { { .compatible = "ti,tilcdc,panel", }, { }, }; -- 1.9.1
Re: [RFC PATCH] char: misc: Init misc->list in a safe way
On Wed, Jun 28, 2017 at 09:54:32AM +0800, Orson Zhai wrote: > We found the device is "fm". We highly suspect that fm driver call > misc_register twice and reinitialize list to make ->pre & ->next > pointing to himself. > > Meanwhile, we checked fm driver and found nothing obviously wrong in the code. Do you have a pointer to this driver? Is it in the kernel tree? > Consider that this is a crash after 46 hours continuous power-on/off, > it maybe caused by some special cases we are hard to know for now. What would cause this driver to want to register/unregister itself? Is it "recycling" the misc structure, or creating it new each time? And what kernel version are you testing here? > We think it might make some sence to add protection code into > misc_register() at first. To protect from "foolish" callers? Usually we fix the calling code to not do foolish things. :) thanks, greg k-h
[PATCH] drm: tilcdc: tilcdc_tfp410: fsl_raid: make of_device_ids const.
of_device_ids are not supposed to change at runtime. All functions working with of_device_ids provided by work with const of_device_ids. So mark the non-const structs as const. File size before: textdata bss dec hex filename 1496 592 02088 828 drivers/gpu/drm/tilcdc/tilcdc_tfp410.o File size after constify: textdata bss dec hex filename 1880 176 02056 808 drivers/gpu/drm/tilcdc/tilcdc_tfp410.o Signed-off-by: Arvind Yadav --- drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c index aabfad8..236ca1e 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c @@ -290,8 +290,6 @@ static int tfp410_modeset_init(struct tilcdc_module *mod, struct drm_device *dev * Device: */ -static struct of_device_id tfp410_of_match[]; - static int tfp410_probe(struct platform_device *pdev) { struct device_node *node = pdev->dev.of_node; @@ -376,7 +374,7 @@ static int tfp410_remove(struct platform_device *pdev) return 0; } -static struct of_device_id tfp410_of_match[] = { +static const struct of_device_id tfp410_of_match[] = { { .compatible = "ti,tilcdc,tfp410", }, { }, }; -- 1.9.1
Re: [PATCH] cifs: hide unused functions
merged into cifs-2.6.git for-next On Tue, Jun 27, 2017 at 10:06 AM, Arnd Bergmann wrote: > Some functions are only referenced under an #ifdef, causing a harmless > warning: > > fs/cifs/smb2ops.c:1374:1: error: 'get_smb2_acl' defined but not used > [-Werror=unused-function] > > We could mark them __maybe_unused or add another #ifdef, I picked > the second approach here. > > Fixes: b3fdda4d1e1b ("cifs: Use smb 2 - 3 and cifsacl mount options getacl > functions") > Signed-off-by: Arnd Bergmann > --- > fs/cifs/smb2ops.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c > index 06494e11c570..941c40b7a870 100644 > --- a/fs/cifs/smb2ops.c > +++ b/fs/cifs/smb2ops.c > @@ -1288,6 +1288,7 @@ smb2_query_symlink(const unsigned int xid, struct > cifs_tcon *tcon, > return rc; > } > > +#ifdef CONFIG_CIFS_ACL > static struct cifs_ntsd * > get_smb2_acl_by_fid(struct cifs_sb_info *cifs_sb, > const struct cifs_fid *cifsfid, u32 *pacllen) > @@ -1387,7 +1388,7 @@ get_smb2_acl(struct cifs_sb_info *cifs_sb, > cifsFileInfo_put(open_file); > return pntsd; > } > - > +#endif > > static long smb3_zero_range(struct file *file, struct cifs_tcon *tcon, > loff_t offset, loff_t len, bool keep_size) > -- > 2.9.0 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-cifs" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Thanks, Steve
[PATCH] soc: mtk-pmic-wrap: make of_device_ids const.
of_device_ids are not supposed to change at runtime. All functions working with of_device_ids provided by work with const of_device_ids. So mark the non-const structs as const. Signed-off-by: Arvind Yadav --- drivers/soc/mediatek/mtk-pmic-wrap.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c index a5f1093..1205a671 100644 --- a/drivers/soc/mediatek/mtk-pmic-wrap.c +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c @@ -1067,7 +1067,7 @@ static irqreturn_t pwrap_interrupt(int irqno, void *dev_id) .init_soc_specific = pwrap_mt2701_init_soc_specific, }; -static struct pmic_wrapper_type pwrap_mt8135 = { +static const struct pmic_wrapper_type pwrap_mt8135 = { .regs = mt8135_regs, .type = PWRAP_MT8135, .arb_en_all = 0x1ff, @@ -1079,7 +1079,7 @@ static irqreturn_t pwrap_interrupt(int irqno, void *dev_id) .init_soc_specific = pwrap_mt8135_init_soc_specific, }; -static struct pmic_wrapper_type pwrap_mt8173 = { +static const struct pmic_wrapper_type pwrap_mt8173 = { .regs = mt8173_regs, .type = PWRAP_MT8173, .arb_en_all = 0x3f, @@ -1091,7 +1091,7 @@ static irqreturn_t pwrap_interrupt(int irqno, void *dev_id) .init_soc_specific = pwrap_mt8173_init_soc_specific, }; -static struct of_device_id of_pwrap_match_tbl[] = { +static const struct of_device_id of_pwrap_match_tbl[] = { { .compatible = "mediatek,mt2701-pwrap", .data = &pwrap_mt2701, -- 1.9.1
[PATCH v5] usb: host: ohci-pxa27x: Handle return value of clk_prepare_enable
clk_prepare_enable() can fail here and we must check its return value. Signed-off-by: Arvind Yadav --- Changes in v4: Change log was missing. Changes in v3: Change log correction. Added change log below '---'. Changes in v2: Remove useless initialization of retval. drivers/usb/host/ohci-pxa27x.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/drivers/usb/host/ohci-pxa27x.c b/drivers/usb/host/ohci-pxa27x.c index 79efde8f..21c010f 100644 --- a/drivers/usb/host/ohci-pxa27x.c +++ b/drivers/usb/host/ohci-pxa27x.c @@ -274,14 +274,16 @@ static inline void pxa27x_reset_hc(struct pxa27x_ohci *pxa_ohci) static int pxa27x_start_hc(struct pxa27x_ohci *pxa_ohci, struct device *dev) { - int retval = 0; + int retval; struct pxaohci_platform_data *inf; uint32_t uhchr; struct usb_hcd *hcd = dev_get_drvdata(dev); inf = dev_get_platdata(dev); - clk_prepare_enable(pxa_ohci->clk); + retval = clk_prepare_enable(pxa_ohci->clk); + if (retval) + return retval; pxa27x_reset_hc(pxa_ohci); @@ -296,8 +298,10 @@ static int pxa27x_start_hc(struct pxa27x_ohci *pxa_ohci, struct device *dev) if (inf->init) retval = inf->init(dev); - if (retval < 0) + if (retval < 0) { + clk_disable_unprepare(pxa_ohci->clk); return retval; + } if (cpu_is_pxa3xx()) pxa3xx_u2d_start_hc(&hcd->self); -- 1.9.1
Re: [REGRESSION] perf/core: PMU interrupts dropped if we entered the kernel in the "skid" region
On Tue, Jun 27, 2017 at 7:09 PM, Jin, Yao wrote: > Hi, > > In theory, the PMI interrupts in skid region should be dropped, right? No, why would they be dropped? My understanding of the situation is as follows: There is some time, call it t_0, where the hardware counter overflows. The PMU triggers an interrupt, but this is not instantaneous. Call the time when the interrupt is actually delivered t_1. Then t_1 - t_0 is the "skid". Note that if the counter is `exclude_kernel`, then at t_0 the CPU *must* be running a userspace program. But by t_1, the CPU may be doing something else. Your patch changed things so that if at t_1 the CPU is in the kernel, then the interrupt is discarded. But rr has programmed the counter to deliver a signal on overflow (via F_SETSIG on the fd returned by perf_event_open). This change results in the signal never being delivered, because the interrupt was ignored. (More accurately, the signal is delivered the *next* time the counter overflows, which is far past where we wanted to inject our asynchronous event into our tracee. It seems to me that it might be reasonable to ignore the interrupt if the purpose of the interrupt is to trigger sampling of the CPUs register state. But if the interrupt will trigger some other operation, such as a signal on an fd, then there's no reason to drop it. > For a userspace debugger, is it the only choice that relies on the *skid* > PMI interrupt? I don't understand this question, but hopefully the above clarified things. - Kyle > Thanks > Jin Yao > > > On 6/28/2017 9:01 AM, Kyle Huey wrote: >> >> Sent again with LKML CCd, sorry for the noise. >> >> - Kyle >> >> On Tue, Jun 27, 2017 at 5:38 PM, Kyle Huey wrote: >>> >>> cc1582c231ea introduced a regression in v4.12.0-rc5, and appears to be >>> a candidate for backporting to stable branches. >>> >>> rr, a userspace record and replay debugger[0], uses the PMU interrupt >>> to stop a program during replay to inject asynchronous events such as >>> signals. We are counting retired conditional branches in userspace >>> only. This changeset causes the kernel to drop interrupts on the >>> floor if, during the PMU interrupt's "skid" region, the CPU enters >>> kernel mode for whatever reason. When replaying traces of complex >>> programs such as Firefox, we intermittently fail to deliver >>> asynchronous events on time, leading the replay to diverge from the >>> recorded state. >>> >>> It seems like this change should, at a bare minimum, be limited to >>> counters that actually perform sampling of register state when the >>> interrupt fires. In our case, with the retired conditional branches >>> counter restricted to counting userspace events only, it makes no >>> difference that the PMU interrupt happened to be delivered in the >>> kernel. >>> >>> As this makes rr unusable on complex applications and cannot be >>> efficiently worked around, we would appreciate this being addressed >>> before 4.12 is finalized, and the regression not being introduced to >>> stable branches. >>> >>> Thanks, >>> >>> - Kyle >>> >>> [0] http://rr-project.org/ > >
Re: 4.12.0-rc5+git: kernel BUG at arch/x86/mm/highmem_32.c:47
> > > > This is 4.12.0-rc5-00137-ga090bd4ff838 on a dual AthlonMP server tha > > > > has > > > > been running fine until 4.11.0 included. 4.12.0-rc5-00137-ga090bd4ff838 > > > > was the first kernel after 4.11 that I tried and the problem happened > > > > while compiling next kernel from git. > > > > > > I can't reproduce this in a guest. Can you send .config please? > > > > Here it is. > > Thanks. > > So plain 4.12.0-rc7 booted all the way in the athlon guest here. So I > either can't reproduce in the guest or I need to try linux-next. Well, > tomorrow... It booted on my physical machine too, and trouble happened during compilation on both CPUs with make -j2. On next try it just rebooted. -- Meelis Roos (mr...@linux.ee)
[PATCH 1/1] xen/blkfront: always allocate grants first from per-queue persistent grants
This patch partially reverts 3df0e50 ("xen/blkfront: pseudo support for multi hardware queues/rings"). The xen-blkfront queue/ring might hang due to grants allocation failure in the situation when gnttab_free_head is almost empty while many persistent grants are reserved for this queue/ring. As persistent grants management was per-queue since 73716df ("xen/blkfront: make persistent grants pool per-queue"), we should always allocate from persistent grants first. Signed-off-by: Dongli Zhang --- drivers/block/xen-blkfront.c | 17 ++--- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c index 3945963..d2b759f 100644 --- a/drivers/block/xen-blkfront.c +++ b/drivers/block/xen-blkfront.c @@ -713,6 +713,7 @@ static int blkif_queue_rw_req(struct request *req, struct blkfront_ring_info *ri * existing persistent grants, or if we have to get new grants, * as there are not sufficiently many free. */ + bool new_persistent_gnts = false; struct scatterlist *sg; int num_sg, max_grefs, num_grant; @@ -724,12 +725,13 @@ static int blkif_queue_rw_req(struct request *req, struct blkfront_ring_info *ri */ max_grefs += INDIRECT_GREFS(max_grefs); - /* -* We have to reserve 'max_grefs' grants because persistent -* grants are shared by all rings. -*/ - if (max_grefs > 0) - if (gnttab_alloc_grant_references(max_grefs, &setup.gref_head) < 0) { + /* Check if we have enough persistent grants to allocate a requests */ + if (rinfo->persistent_gnts_c < max_grefs) { + new_persistent_gnts = true; + + if (gnttab_alloc_grant_references( + max_grefs - rinfo->persistent_gnts_c, + &setup.gref_head) < 0) { gnttab_request_free_callback( &rinfo->callback, blkif_restart_queue_callback, @@ -737,6 +739,7 @@ static int blkif_queue_rw_req(struct request *req, struct blkfront_ring_info *ri max_grefs); return 1; } + } /* Fill out a communications ring structure. */ id = blkif_ring_get_request(rinfo, req, &ring_req); @@ -837,7 +840,7 @@ static int blkif_queue_rw_req(struct request *req, struct blkfront_ring_info *ri if (unlikely(require_extra_req)) rinfo->shadow[extra_id].req = *extra_ring_req; - if (max_grefs > 0) + if (new_persistent_gnts) gnttab_free_grant_references(setup.gref_head); return 0; -- 2.7.4
Re: [PATCH] ACPI: nfit: constify *_attribute_group.
On Thu, Jun 22, 2017 at 3:14 AM, Arvind Yadav wrote: > File size before: >textdata bss dec hex filename > 207921580 994 233665b46 drivers/acpi/nfit/core.o > > File size After adding 'const': >textdata bss dec hex filename > 209681388 994 233505b36 drivers/acpi/nfit/core.o > > Signed-off-by: Arvind Yadav > --- > drivers/acpi/nfit/core.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) Looks good to me, applied.
Re: [PATCH v2] spi: rockchip: Disable Runtime PM when chip select is asserted
Hi doug, thanx for your comments. On 06/28/2017 03:27 AM, Doug Anderson wrote: Hi, On Mon, Jun 26, 2017 at 7:20 PM, Jeffy Chen wrote: The rockchip spi would stop driving pins when runtime suspended, which might break slave's xfer(for example cros_ec). Since we have pullups on those pins, we only need to care about the CS asserted case. So let's keep the spi alive when chip select is asserted for that. Also change use pm_runtime_put instead of pm_runtime_put_sync. Suggested-by: Doug Anderson Signed-off-by: Jeffy Chen --- Changes in v2: Improve commit message and comments and coding style. drivers/spi/spi-rockchip.c | 23 ++- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c index acf31f3..ea0edd7 100644 --- a/drivers/spi/spi-rockchip.c +++ b/drivers/spi/spi-rockchip.c @@ -264,7 +264,7 @@ static inline u32 rx_max(struct rockchip_spi *rs) static void rockchip_spi_set_cs(struct spi_device *spi, bool enable) { - u32 ser; + u32 ser, new_ser; struct spi_master *master = spi->master; struct rockchip_spi *rs = spi_master_get_devdata(master); @@ -288,13 +288,26 @@ static void rockchip_spi_set_cs(struct spi_device *spi, bool enable) * Note: enable(rockchip_spi_set_cs) = !enable(spi_set_cs) */ if (!enable) - ser |= 1 << spi->chip_select; + new_ser = ser | BIT(spi->chip_select); else - ser &= ~(1 << spi->chip_select); + new_ser = ser & ~BIT(spi->chip_select); - writel_relaxed(ser, rs->regs + ROCKCHIP_SPI_SER); + if (new_ser != ser) { IMHO it's not a great idea to use the state of the hardware register here. Using the state of the hardware register probably works OK, but it makes me just a little nervous. If something happened to the state of the register (someone used /dev/mem to tweak, or the peripherals got reset, or ...) then you could end up with an unbalanced set of PM Runtime calls. I know none of those things are common, but it still seems less than great to me. I'd rather we kept track in "struct rockchip_spi" whether we already called pm_runtime_get_sync(). AKA the following (untested): bool cs_asserted = !enable; /* Return immediately for no-op */ if (cs_asserted == rs->cs_asserted) return; /* Keep things powered as long as CS is asserted */ if (cs_asserted) { pm_runtime_get_sync(rs->dev); rs->cs_asserted = true; } ser = ... ... ... if (!cs_asserted) pm_runtime_put(rs->dev); NOTE: another advantage of storing the state in 'struct rockchip_spi' is that you can access it without pm_runtime_get_sync(). ok, that make sense :) + writel_relaxed(new_ser, rs->regs + ROCKCHIP_SPI_SER); - pm_runtime_put_sync(rs->dev); + /* +* The rockchip spi would stop driving all pins when powered +* down. +* So hold a runtime PM reference as long as CS is asserted. +*/ + if (!enable) + return; + + /* Drop reference from when we first asserted CS */ + pm_runtime_put(rs->dev); + } + + pm_runtime_put(rs->dev); One note is that you should still submit your patch to add "SPI_MASTER_GPIO_SS" to spi-rockchip.c. It's important that the SPI driver see the CS transitions so that it can do the PM Runtime get/put even when someone uses a GPIO chip select. Even though the GPIO chip select will keep state, we don't want the rest of the lines to stop being driven. ok, will do. -Doug
[PATCH v3 1/2] spi: rockchip: Set GPIO_SS flag to enable Slave Select with GPIO CS
The rockchip spi still requires slave selection when using GPIO CS. Signed-off-by: Jeffy Chen --- Changes in v3: None Changes in v2: None drivers/spi/spi-rockchip.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c index bab9b13..52ea160 100644 --- a/drivers/spi/spi-rockchip.c +++ b/drivers/spi/spi-rockchip.c @@ -749,6 +749,7 @@ static int rockchip_spi_probe(struct platform_device *pdev) master->transfer_one = rockchip_spi_transfer_one; master->max_transfer_size = rockchip_spi_max_transfer_size; master->handle_err = rockchip_spi_handle_err; + master->flags = SPI_MASTER_GPIO_SS; rs->dma_tx.ch = dma_request_chan(rs->dev, "tx"); if (IS_ERR(rs->dma_tx.ch)) { -- 2.1.4
[PATCH v3 2/2] spi: rockchip: Disable Runtime PM when chip select is asserted
The rockchip spi would stop driving pins when runtime suspended, which might break slave's xfer(for example cros_ec). Since we have pullups on those pins, we only need to care about this when the CS asserted. So let's keep the spi alive when chip select is asserted. Also use pm_runtime_put instead of pm_runtime_put_sync. Suggested-by: Doug Anderson Signed-off-by: Jeffy Chen --- Changes in v3: Store cs states in struct rockchip_spi, and use it to detect cs state instead of hw register. Changes in v2: Improve commit message and comments and coding style. drivers/spi/spi-rockchip.c | 51 +++--- 1 file changed, 26 insertions(+), 25 deletions(-) diff --git a/drivers/spi/spi-rockchip.c b/drivers/spi/spi-rockchip.c index 52ea160..0b4a52b 100644 --- a/drivers/spi/spi-rockchip.c +++ b/drivers/spi/spi-rockchip.c @@ -25,6 +25,11 @@ #define DRIVER_NAME "rockchip-spi" +#define ROCKCHIP_SPI_CLR_BITS(reg, bits) \ + writel_relaxed(readl_relaxed(reg) & ~(bits), reg) +#define ROCKCHIP_SPI_SET_BITS(reg, bits) \ + writel_relaxed(readl_relaxed(reg) | (bits), reg) + /* SPI register offsets */ #define ROCKCHIP_SPI_CTRLR00x #define ROCKCHIP_SPI_CTRLR10x0004 @@ -149,6 +154,8 @@ */ #define ROCKCHIP_SPI_MAX_TRANLEN 0x +#define ROCKCHIP_SPI_MAX_CS_NUM2 + enum rockchip_ssi_type { SSI_MOTO_SPI = 0, SSI_TI_SSP, @@ -193,6 +200,8 @@ struct rockchip_spi { /* protect state */ spinlock_t lock; + bool cs_asserted[ROCKCHIP_SPI_MAX_CS_NUM]; + u32 use_dma; struct sg_table tx_sg; struct sg_table rx_sg; @@ -264,37 +273,29 @@ static inline u32 rx_max(struct rockchip_spi *rs) static void rockchip_spi_set_cs(struct spi_device *spi, bool enable) { - u32 ser; struct spi_master *master = spi->master; struct rockchip_spi *rs = spi_master_get_devdata(master); + bool cs_asserted = !enable; - pm_runtime_get_sync(rs->dev); + /* Return immediately for no-op */ + if (cs_asserted == rs->cs_asserted[spi->chip_select]) + return; - ser = readl_relaxed(rs->regs + ROCKCHIP_SPI_SER) & SER_MASK; + if (cs_asserted) { + /* Keep things powered as long as CS is asserted */ + pm_runtime_get_sync(rs->dev); - /* -* drivers/spi/spi.c: -* static void spi_set_cs(struct spi_device *spi, bool enable) -* { -* if (spi->mode & SPI_CS_HIGH) -* enable = !enable; -* -* if (spi->cs_gpio >= 0) -* gpio_set_value(spi->cs_gpio, !enable); -* else if (spi->master->set_cs) -* spi->master->set_cs(spi, !enable); -* } -* -* Note: enable(rockchip_spi_set_cs) = !enable(spi_set_cs) -*/ - if (!enable) - ser |= 1 << spi->chip_select; - else - ser &= ~(1 << spi->chip_select); + ROCKCHIP_SPI_SET_BITS(rs->regs + ROCKCHIP_SPI_SER, + BIT(spi->chip_select)); + } else { + ROCKCHIP_SPI_CLR_BITS(rs->regs + ROCKCHIP_SPI_SER, + BIT(spi->chip_select)); - writel_relaxed(ser, rs->regs + ROCKCHIP_SPI_SER); + /* Drop reference from when we first asserted CS */ + pm_runtime_put(rs->dev); + } - pm_runtime_put_sync(rs->dev); + rs->cs_asserted[spi->chip_select] = cs_asserted; } static int rockchip_spi_prepare_message(struct spi_master *master, @@ -739,7 +740,7 @@ static int rockchip_spi_probe(struct platform_device *pdev) master->auto_runtime_pm = true; master->bus_num = pdev->id; master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_LOOP; - master->num_chipselect = 2; + master->num_chipselect = ROCKCHIP_SPI_MAX_CS_NUM; master->dev.of_node = pdev->dev.of_node; master->bits_per_word_mask = SPI_BPW_MASK(16) | SPI_BPW_MASK(8); -- 2.1.4
Re: [PATCH] powernv:idle: Clear r12 on wakeup from stop lite
On Wed, 28 Jun 2017 06:46:49 +0530 Akshay Adiga wrote: > pnv_wakeup_noloss expects R12 to contain SRR1 value to determine if > the wakeup reason is an HMI in CHECK_HMI_INTERRUPT. > > When we wakeup with ESL=0, SRR1 will not contain the wakeup reason, so > there is no point setting R12 to SRR1. > > However, we don't set R12 at all and R12 contains garbage, and still > being used to check HMI assuming that it had SRR1. causing the > OPAL msglog to be filled with the following print: > HMI: Received HMI interrupt: HMER = 0x0040 > > This patch clears R12 after waking up from stop with ESL=EC=0, so that > we don't accidentally enter the HMI handler in pnv_wakeup_noloss if > the R12[42:45] corresponds to HMI as wakeup reason. > > Bug existed prior to "commit 9d29250136f6 ("powerpc/64s/idle: Avoid SRR > usage in idle sleep/wake paths") but was never hit in practice > > Signed-off-by: Akshay Adiga > Fixes: 9d29250136f6 ("powerpc/64s/idle: Avoid SRR usage in idle > sleep/wake paths") Thanks guys, appreciate you finding and fixing my bug :) I think this looks like the best fix. Really minor nitpick but you could adjust the line widths on the comment slightly (mpe might do that when merging). Reviewed-by: Nicholas Piggin > --- > arch/powerpc/kernel/idle_book3s.S | 15 +++ > 1 file changed, 15 insertions(+) > > diff --git a/arch/powerpc/kernel/idle_book3s.S > b/arch/powerpc/kernel/idle_book3s.S > index 1ea14b9..34794fd 100644 > --- a/arch/powerpc/kernel/idle_book3s.S > +++ b/arch/powerpc/kernel/idle_book3s.S > @@ -256,6 +256,21 @@ power_enter_stop: > bne .Lhandle_esl_ec_set > IDLE_STATE_ENTER_SEQ(PPC_STOP) > li r3,0 /* Since we didn't lose state, return 0 */ > + /* > + * pnv_wakeup_noloss expects R12 to contain SRR1 value > + * to determine if the wakeup reason is an HMI in > + * CHECK_HMI_INTERRUPT. > + * > + * However, when we wakeup with ESL=0, > + * SRR1 will not contain the wakeup reason, > + * so there is no point setting R12 to SRR1. > + * > + * Further, we clear R12 here, so that we > + * don't accidentally enter the HMI > + * in pnv_wakeup_noloss if the > + * R12[42:45] == WAKE_HMI. > + */ > + li r12, 0 > b pnv_wakeup_noloss > > .Lhandle_esl_ec_set:
Re: [PATCH] futex: avoid undefined behaviour when shift exponent is negative
Hi, Ingo Thank you for the comment. On 2017/6/22 0:40, Ingo Molnar wrote: > * zhong jiang wrote: > >> when shift expoment is negative, left shift alway zero. therefore, we >> modify the logic to avoid the warining. >> >> Signed-off-by: zhong jiang >> --- >> arch/x86/include/asm/futex.h | 8 ++-- >> 1 file changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/arch/x86/include/asm/futex.h b/arch/x86/include/asm/futex.h >> index b4c1f54..2425fca 100644 >> --- a/arch/x86/include/asm/futex.h >> +++ b/arch/x86/include/asm/futex.h >> @@ -49,8 +49,12 @@ static inline int futex_atomic_op_inuser(int encoded_op, >> u32 __user *uaddr) >> int cmparg = (encoded_op << 20) >> 20; >> int oldval = 0, ret, tem; >> >> -if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28)) >> -oparg = 1 << oparg; >> +if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28)) { >> +if (oparg >= 0) >> +oparg = 1 << oparg; >> +else >> +oparg = 0; >> +} > Could we avoid all these complications by using an unsigned type? I think it is not feasible. a negative shift exponent is likely existence and reasonable. as the above case, oparg is a negative is common. I think it can be avoided by following change. diff --git a/arch/x86/include/asm/futex.h b/arch/x86/include/asm/futex.h index b4c1f54..3205e86 100644 --- a/arch/x86/include/asm/futex.h +++ b/arch/x86/include/asm/futex.h @@ -50,7 +50,7 @@ static inline int futex_atomic_op_inuser(int encoded_op, u32 __user *uaddr) int oldval = 0, ret, tem; if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28)) - oparg = 1 << oparg; + oparg = safe_shift(1, oparg); if (!access_ok(VERIFY_WRITE, uaddr, sizeof(u32))) return -EFAULT; diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c index 069fe79..b4edda3 100644 --- a/drivers/video/fbdev/core/fbmem.c +++ b/drivers/video/fbdev/core/fbmem.c @@ -190,11 +190,6 @@ char* fb_get_buffer_offset(struct fb_info *info, struct fb_pixmap *buf, u32 size #ifdef CONFIG_LOGO -static inline unsigned safe_shift(unsigned d, int n) -{ - return n < 0 ? d >> -n : d << n; -} - static void fb_set_logocmap(struct fb_info *info, const struct linux_logo *logo) { diff --git a/include/linux/kernel.h b/include/linux/kernel.h index d043ada..f3b8856 100644 --- a/include/linux/kernel.h +++ b/include/linux/kernel.h @@ -841,6 +841,10 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { } */ #define clamp_val(val, lo, hi) clamp_t(typeof(val), val, lo, hi) +static inline unsigned safe_shift(unsigned d, int n) +{ + return n < 0 ? d >> -n : d << n; +} Thansk zhongjiang > Thanks, > > Ingo > > . >
Re: selftests/capabilities: test FAIL on linux mainline and linux-next and PASS on linux-4.4.70+
On Tue, Jun 27, 2017 at 4:16 PM, Shuah Khan wrote: > On 06/27/2017 09:16 AM, Greg KH wrote: >> On Tue, Jun 27, 2017 at 05:13:59PM +0200, Greg KH wrote: >>> On Tue, Jun 27, 2017 at 02:10:32PM +0530, Naresh Kamboju wrote: selftest capabilities test failed on linux mainline and linux-next and PASS on linux-4.4.70+ >>> >>> Odd. Any chance you can use 'git bisect' to track down the offending >>> commit? >>> >>> Does this also fail on x86 or any other platform you have available? >>> Let me go try this on my laptop... >> >> Ok, Linus's current tree (4.12.0-rc7+) also fails on this. I'm guessing >> it's failing, it's hard to understand the output. If only we had TAP >> output for this test :) > > As far as the output, it isn't bad. Not TAP13 will help make it better. > The problem seems to with the individual messages error/info. messages > themselves. This test has the quality of a developer unit test and the > messages could be improved for non-developer use. > > I ran the test on 4.11.8-rc1+ and 4.9.35-rc1 see the same failure. > It would be difficult to bisect this since it spans multiple releases. > I am hoping Andy can give us some insight. I bisected this to: commit 380cf5ba6b0a0b307f4afb62b186ca801defb203 Author: Andy Lutomirski Date: Thu Jun 23 16:41:05 2016 -0500 fs: Treat foreign mounts as nosuid I assume the test needs updating, but I bet Andy knows for sure. I can look into this more closely in the morning. -Kees -- Kees Cook Pixel Security
Re: [PATCH 1/7] platform/x86: fujitsu-laptop: do not use kfifo for storing hotkey scancodes
> On Sat, Jun 24, 2017 at 02:25:46AM +0200, Rafael Wysocki wrote: > > On Wednesday, June 21, 2017 11:15:43 AM Darren Hart wrote: > > > On Fri, Jun 16, 2017 at 06:40:52AM +0200, Michał Kępień wrote: > > > > All ACPI device notify callbacks are invoked using acpi_os_execute(), > > > > which causes the supplied callback to be queued to a static workqueue > > > > which always executes on CPU 0. This means that there is no possibility > > > > for any ACPI device notify callback to be concurrently executed on > > > > multiple CPUs, which in the case of fujitsu-laptop means that using a > > > > locked kfifo for handling hotkeys is redundant: as hotkey scancodes are > > > > only pushed and popped from within acpi_fujitsu_laptop_notify(), no risk > > > > of concurrent pushing and popping exists. > > > > > > Was the kfifo causing a problem currently or for the migration to separate > > > modules? Is this purely a simplification? > > > > > > Rafael, the above rationale appears sound to me. Do you have any concerns? > > > > I actually do. > > > > While this is the case today, making the driver code depend on it in a hard > > way > > sort of makes it difficult to change in the future if need be. > > OK, if we aren't guaranteed for this to run on CPU 0 in the future, and this > will be annoying to debug if it does changes, let's skip the kfifo change. > > I have removed this patch, and fixed up the merge conflicts of the remaining 6 > patches here: > > http://git.infradead.org/linux-platform-drivers-x86.git/shortlog/refs/heads/fujitsu > > Michal / Jonathan, would you please review and let me know if this is what you > would have done / approve the rebase? The only issue I can see is that the push/pop debug messages in the last patch contain the word "buffer" instead of the original "ringbuffer". The dropped kfifo patch changed the wording from "ringbuffer" to "buffer" as applying it means there is no ringbuffer any more, but since it was not applied in the end, I guess the original wording should stay in place. The rest looks good to me. -- Best regards, Michał Kępień
Re: linux-next: build failure after merge of the tip tree
Hi jeffy, On Wed, 28 Jun 2017 12:00:19 +0800 jeffy wrote: > > commit 50816c48997af857d4bab3dca1aba90339705e96 > Author: Ingo Molnar > Date: Sun Mar 5 10:33:16 2017 +0100 > > sched/wait: Standardize internal naming of wait-queue entries > > > changed wait_queue_entry_t to struct wait_queue_entry, and also wait to > wq_entry, maybe we should do it too? Sure, but that can be done later. -- Cheers, Stephen Rothwell
linux-next: manual merge of the xen-tip tree with the tip tree
Hi all, Today's linux-next merge of the xen-tip tree got a conflict in: drivers/xen/events/events_base.c between commit: ef1c2cc88531 ("xen/events: Add support for effective affinity mask") from the tip tree and commit: c48f64ab4723 ("xen-evtchn: Bind dyn evtchn:qemu-dm interrupt to next online VCPU") from the xen-tip 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/xen/events/events_base.c index 2e567d8433b3,813f1e86a599.. --- a/drivers/xen/events/events_base.c +++ b/drivers/xen/events/events_base.c @@@ -1343,12 -1343,8 +1343,12 @@@ static int set_affinity_irq(struct irq_ bool force) { unsigned tcpu = cpumask_first_and(dest, cpu_online_mask); - int ret = rebind_irq_to_cpu(data->irq, tcpu); ++ int ret = xen_rebind_evtchn_to_cpu(evtchn_from_irq(data->irq), tcpu); - return xen_rebind_evtchn_to_cpu(evtchn_from_irq(data->irq), tcpu); + if (!ret) + irq_data_update_effective_affinity(data, cpumask_of(tcpu)); + + return ret; } static void enable_dynirq(struct irq_data *data)
Re: [PATCH 3/6] xfs: map KM_MAYFAIL to __GFP_RETRY_MAYFAIL
[add linux-xfs to cc] FYI this is a discussion of the patch "xfs: map KM_MAYFAIL to __GFP_RETRY_MAYFAIL" which was last discussed on the xfs list in March and now is in the -mm tree... https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20170627&id=43182d82c48fae80d31a9101b6bb06d75cee32c7 On Tue, Jun 27, 2017 at 04:06:54PM +0200, Michal Hocko wrote: > On Tue 27-06-17 06:47:51, Christoph Hellwig wrote: > > On Tue, Jun 27, 2017 at 10:49:50AM +0200, Michal Hocko wrote: > > > Christoph, Darrick > > > could you have a look at this patch please? Andrew has put it into mmotm > > > but I definitely do not want it passes your attention. > > > > I don't think what we have to gain from it. Callsite for KM_MAYFAIL > > should handler failures, but the current behavior seems to be doing fine > > too. > > Last time I've asked I didnd't get any reply so let me ask again. Some > of those allocations seem to be small (e.g. by a random look > xlog_cil_init allocates struct xfs_cil which is 576B and struct > xfs_cil_ctx 176B). Those do not fail currently under most conditions and > it will retry allocation with the OOM killer if there is no progress. As > you know that failing those is acceptable, wouldn't it be better to > simply fail them and do not disrupt the system with the oom killer? I remember the first time I saw this patch, and didn't have much of an opinion either way -- the current behavior is fine, so why mess around? I'd just as soon XFS not have to deal with errors if it doesn't have to. :) But, you've asked again, so I'll be less glib this time. I took a quick glance at all the MAYFAIL users in XFS. /Nearly/ all them seem to be cases either where we're mounting a filesystem or are collecting memory for some ioctl -- in either case it's not hard to just fail back out to userspace. The upcoming online fsck patches use it heavily, which is fine since we can always fail out to userspace and tell the admin to go run xfs_repair offline. The one user that caught my eye was xfs_iflush_cluster, which seems to want an array of pointers to a cluster's worth of struct xfs_inodes. On a 64k block fs with 256 byte pointers I guess that could be ~2k worth of pointers, but otoh it looks like that's an optimization: If we're going to flush an inode out to disk we opportunistically scan the inode tree to see if the adjacent inodes are also ready to flush; if we can't get the memory for this, then it just backs off to flushing the one inode. All the callers of MAYFAIL that I found actually /do/ check the return value and start bailing out... so, uh, I guess I'm fine with it. At worst it's easily reverted during -rc if it causes problems. Anyone have a stronger objection? Acked-by: Darrick J. Wong --D > -- > Michal Hocko > SUSE Labs
Re: [PATCH] cpufreq: dt: Set default policy->transition_delay_ns
On 27-06-17, 18:08, Rafael J. Wysocki wrote: > On Tue, Jun 27, 2017 at 6:20 AM, Viresh Kumar wrote: > > @Rafael: Will it be fine to lower down the value of LATENCY_MULTIPLIER? > > We can do that, but then I think we need to compensate for the change > in the old governors code or there may be surprises. Why shouldn't we change the value of LATENCY_MULTIPLIER for old governors as well? They use the same calculations and the sampling rate there is also this bad (like rate_limit_us). If we aren't going to change that for old governors, then we can create a local version of LATENCY_MULTIPLIER for schedutil I believe. -- viresh
Re: [PATCH v3 0/4] g_NCR5380: PDMA fixes and cleanup
On Tue, 27 Jun 2017, Ondrej Zary wrote: > On Tuesday 27 June 2017 14:42:29 Finn Thain wrote: > > > > ... it triggers sometimes: the value is 1 instead of 0. As we use > > > only 16-bit writes, I don't see how the value could ever be odd. > > > Looks like a bug in the chip. The index register corrupts during the > > > transfer, not after IRQ or timeout. The same check at beginning of > > > pwrite() did not trigger. > > > > Are you reading this register at the right moment? Have you tried > > waiting for it to reach zero, as in, > > > > if (NCR5380_poll_politely(hostdata, 13, 0xff, 0, HZ / 64) < 0) > > /* printk, reset etc */; > > I have not but will try (expecting that it will not change by itself). > Now that I know that it is the byte at the beginning of the block that went missing, I agree that there's no point waiting for the byte count to change. I've included a patch with your 512 B limit in v4. Thanks. > > Even if this is a reliable way to detect a short transfer, it would be > > nice to know the root cause. But I'm being unrealistic: the DTC436 > > vendor never responded to my requests for technical documentation. > > According to the data corruption observed, it's not a short transfer. > The corruption is always the same: one byte missing at the beginning of > a 128 B block. It happens only with slow Quantum LPS 240 drive, not with > faster IBM DORS-32160. > --
[no subject]
внимания; Ваши сообщения превысил лимит памяти, который составляет 5 Гб, определенных администратором, который в настоящее время работает на 10.9GB, Вы не сможете отправить или получить новую почту, пока вы повторно не проверить ваш почтовый ящик почты. Чтобы восстановить работоспособность Вашего почтового ящика, отправьте следующую информацию ниже: имя: Имя пользователя: пароль: Подтверждение пароля: Адрес электронной почты: телефон: Если вы не в состоянии перепроверить сообщения, ваш почтовый ящик будет отключен! Приносим извинения за неудобства. Проверочный код: EN: Ru...776774990..2017 Почты технической поддержки ©2017 спасибо системы администратор
Re: [PATCH v3 0/4] g_NCR5380: PDMA fixes and cleanup
On Tue, 27 Jun 2017, Ondrej Zary wrote: > On Tuesday 27 June 2017 03:49:16 Finn Thain wrote: > > > > ... As long as there's no gated IRQ, we poll for buffer readiness > > until timeout. And when there is a gated IRQ, we break both the > > polling loop and the transfer loop immediately. Your code and mine are > > basically in agreement here. > > Yes, it stops transfer when an IRQ arrives. But the host buffer could be > ready at the same time. The IRQ can be an "end-of-DMA" IRQ (IIRC DTC > chips assert this earlier than 53C400). Or just a disconnect that > occured right now but the chip already read a buffer full of data. > The IRQ should not normally arise during the loop. A BASR_END_DMA_TRANSFER interrupt could only happen after the loop has finished sending/receiving, which is when /EOP becomes active. The BASR_PHASE_MATCH interrupt could happen during the transfer if the target disconnects suddenly. It is possible that the 53c400 core would assert /EOP upon BASR_PHASE_MATCH interrupt, which could then cause the 53c80 to raise a BASR_END_DMA_TRANSFER interrupt too. But who knows? > > > According to my tests, buffer ready signal is most important - if > > > there is any data to read/write, do the transfer. If not, only then > > > check why - maybe we got an IRQ (that terminated PDMA). Or no IRQ, > > > sometimes the wait for buffer ready times out - we need to terminate > > > PDMA manually then (reset). > > > > > > Then 53C80 registers should become ready. > > > > You seem to be saying that we should ignore the IRQ signal if the > > buffers have become ready. Maybe so. Can we try simply resetting the > > block counter? (I could imagine that the 53c400 core might leave the > > 53c80 registers inaccessible unless we keep accessing the buffers in > > the 53c400 core until the transfer is done.) > > We can't reset the block counter because 0 means 256 blocks to transfer > (page 13 in datasheet). I forgot about that. How awful. > Yes, the 53C80 registers seem to become available only when the PDMA > transfer ends by either: > 1. transferring all blocks (block counter decrementing to zero) > 2. IRQ I don't think that Gated IRQ is sufficient to make the 53c80 registers available again. If it was, you probably wouldn't have seen "switching to slow handshake" when you tested my earlier patch series. > 3. reset > Maybe we need to do the reset whenever IRQ is detected. I'll put this in v4. Please try commenting it out, to see what difference that makes. > > BTW, with regard to your patch, note that this construct is race prone: > > > > while (1) { /* monitor IRQ while waiting for host buffer */ > > csr = NCR5380_read(hostdata->c400_ctl_status); > > if (!(csr & CSR_HOST_BUF_NOT_RDY)) > > break; > > if (csr & CSR_GATED_53C80_IRQ) { > > basr = NCR5380_read(BUS_AND_STATUS_REG); > > if (!(basr & BASR_PHASE_MATCH) || > >(basr & BASR_BUSY_ERROR)) { > > printk("basr=0x%02x csr=0x%02x at start=%d\n", basr, > > csr, start); > > goto out_wait; > > } > > } > > if (retries-- < 1) { > > shost_printk(KERN_ERR, hostdata->host, "53C400r: host buffer > > not ready in > > time\n"); NCR5380_write(hostdata->c400_ctl_status, CSR_RESET); > > NCR5380_write(hostdata->c400_ctl_status, CSR_BASE); > > goto out_wait; > > } > > } > > > > This code can "goto out_wait" when !(csr & CSR_HOST_BUF_NOT_RDY). It > > depends on timing. This would seem to be contrary to your stated aim. > > > > Moreover, this code can also "break" when (csr & CSR_GATED_53C80_IRQ). > > That depends on timing too. But this may be an improvement on my code > > if it allows the 53c80 registers to become accessible, by allowing the > > block counter to be decremented. > > Yes, it continue the transfer even if the IRQ is asserted - as long as > the buffer is ready. That's intended. > If we continue to try to send when there is a phase mismatch (i.e. sudden disconnection) we'll probably end up with a buffer ready timeout. And we may also have trouble calculating the residual correctly. Hence my version of your patch always breaks out of the transfer loop as soon as any Gated IRQ is detected. If that then means a compulsory reset of the 53c400 core, I guess I can live with that. > > The uncertainty here was one of the reasons I reworked this code. > > My version reads CSR only once per loop but that probably does not help > at all as the HW state could change anytime. The chip's design seems to > be very race-prone. > > > > This is a log from writing 230 MB file using my code with some debug > > > prints, on a NCR53C400. No 53C80 timeouts, only disconnects and some > > > host buffer timeouts (maybe the drive sometimes just slows down > > > without disconnecting?) > > > > > > [ 3378.503828] basr=0x10 csr=0xd5 at start=512 > > > [ 3461.257973] w basr=0x
[PATCH v4 2/5] g_NCR5380: End PDMA transfer correctly on target disconnection
From: Ondrej Zary When an IRQ arrives during PDMA transfer, pread() and pwrite() return without waiting for the 53C80 registers to be ready and this ends up messing up the chip state. This was observed with SONY CDU-55S which is slow enough to disconnect during 4096-byte reads. IRQ during PDMA is not an error so don't return -1. Instead, store the remaining byte count for use by NCR5380_dma_residual(). [Poll for the BASR_END_DMA_TRANSFER condition rather than remove the error message -- F.T.] Signed-off-by: Ondrej Zary Signed-off-by: Finn Thain --- drivers/scsi/g_NCR5380.c | 48 +++- 1 file changed, 31 insertions(+), 17 deletions(-) diff --git a/drivers/scsi/g_NCR5380.c b/drivers/scsi/g_NCR5380.c index 14ef4e8c4713..911a4300ea51 100644 --- a/drivers/scsi/g_NCR5380.c +++ b/drivers/scsi/g_NCR5380.c @@ -44,12 +44,13 @@ int c400_ctl_status; \ int c400_blk_cnt; \ int c400_host_buf; \ - int io_width + int io_width; \ + int pdma_residual #define NCR5380_dma_xfer_lengeneric_NCR5380_dma_xfer_len #define NCR5380_dma_recv_setup generic_NCR5380_pread #define NCR5380_dma_send_setup generic_NCR5380_pwrite -#define NCR5380_dma_residualNCR5380_dma_residual_none +#define NCR5380_dma_residualgeneric_NCR5380_dma_residual #define NCR5380_intrgeneric_NCR5380_intr #define NCR5380_queue_command generic_NCR5380_queue_command @@ -500,10 +501,8 @@ static inline int generic_NCR5380_pread(struct NCR5380_hostdata *hostdata, while (1) { if (NCR5380_read(hostdata->c400_blk_cnt) == 0) break; - if (NCR5380_read(hostdata->c400_ctl_status) & CSR_GATED_53C80_IRQ) { - printk(KERN_ERR "53C400r: Got 53C80_IRQ start=%d, blocks=%d\n", start, blocks); - return -1; - } + if (NCR5380_read(hostdata->c400_ctl_status) & CSR_GATED_53C80_IRQ) + goto out_wait; while (NCR5380_read(hostdata->c400_ctl_status) & CSR_HOST_BUF_NOT_RDY) ; /* FIXME - no timeout */ @@ -542,13 +541,19 @@ static inline int generic_NCR5380_pread(struct NCR5380_hostdata *hostdata, if (!(NCR5380_read(hostdata->c400_ctl_status) & CSR_GATED_53C80_IRQ)) printk("53C400r: no 53C80 gated irq after transfer"); +out_wait: + hostdata->pdma_residual = len - start; + /* wait for 53C80 registers to be available */ while (!(NCR5380_read(hostdata->c400_ctl_status) & CSR_53C80_REG)) ; - if (!(NCR5380_read(BUS_AND_STATUS_REG) & BASR_END_DMA_TRANSFER)) - printk(KERN_ERR "53C400r: no end dma signal\n"); - + if (NCR5380_poll_politely(hostdata, BUS_AND_STATUS_REG, + BASR_END_DMA_TRANSFER, BASR_END_DMA_TRANSFER, + HZ / 64) < 0) + scmd_printk(KERN_ERR, hostdata->connected, "%s: End of DMA timeout (%d)\n", + __func__, hostdata->pdma_residual); + return 0; } @@ -571,10 +576,8 @@ static inline int generic_NCR5380_pwrite(struct NCR5380_hostdata *hostdata, NCR5380_write(hostdata->c400_ctl_status, CSR_BASE); NCR5380_write(hostdata->c400_blk_cnt, blocks); while (1) { - if (NCR5380_read(hostdata->c400_ctl_status) & CSR_GATED_53C80_IRQ) { - printk(KERN_ERR "53C400w: Got 53C80_IRQ start=%d, blocks=%d\n", start, blocks); - return -1; - } + if (NCR5380_read(hostdata->c400_ctl_status) & CSR_GATED_53C80_IRQ) + goto out_wait; if (NCR5380_read(hostdata->c400_blk_cnt) == 0) break; @@ -612,18 +615,24 @@ static inline int generic_NCR5380_pwrite(struct NCR5380_hostdata *hostdata, blocks--; } +out_wait: + hostdata->pdma_residual = len - start; + /* wait for 53C80 registers to be available */ while (!(NCR5380_read(hostdata->c400_ctl_status) & CSR_53C80_REG)) { udelay(4); /* DTC436 chip hangs without this */ /* FIXME - no timeout */ } - if (!(NCR5380_read(BUS_AND_STATUS_REG) & BASR_END_DMA_TRANSFER)) { - printk(KERN_ERR "53C400w: no end dma signal\n"); - } - while (!(NCR5380_read(TARGET_COMMAND_REG) & TCR_LAST_BYTE_SENT)) ; // TIMEOUT + + if (NCR5380_poll_politely(hostdata, BUS_AND_STATUS_REG, + BASR_END_DMA_TRANSFER, BASR_END_DMA_TRANSFER, + HZ / 64) < 0) + scmd_printk(KERN_ERR, hostdata->connected, "%s: End of DMA timeout (%d)\n", + __func__, hostdata->pdma_residual); + return 0; } @@ -
[PATCH v4 4/5] g_NCR5380: Limit PDMA send to 512 B to avoid random corruption on DTC3181E
From: Ondrej Zary The corruption is always the same: one byte missing at the beginning of a 128 B block. It happens only with slow Quantum LPS 240 drive, not with faster IBM DORS-32160. It's not clear what causes this. Documentation for the DTC436 chip has not been made available. Hence this workaround. Signed-off-by: Finn Thain --- drivers/scsi/g_NCR5380.c | 11 +-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/g_NCR5380.c b/drivers/scsi/g_NCR5380.c index 9f18082415c4..b1e0a08e49c1 100644 --- a/drivers/scsi/g_NCR5380.c +++ b/drivers/scsi/g_NCR5380.c @@ -45,7 +45,8 @@ int c400_blk_cnt; \ int c400_host_buf; \ int io_width; \ - int pdma_residual + int pdma_residual; \ + int board #define NCR5380_dma_xfer_lengeneric_NCR5380_dma_xfer_len #define NCR5380_dma_recv_setup generic_NCR5380_pread @@ -316,6 +317,7 @@ static int generic_NCR5380_init_one(struct scsi_host_template *tpnt, } hostdata = shost_priv(instance); + hostdata->board = board; hostdata->io = iomem; hostdata->region_size = region_size; @@ -644,7 +646,12 @@ static int generic_NCR5380_dma_xfer_len(struct NCR5380_hostdata *hostdata, /* 53C400 datasheet: non-modulo-128-byte transfers should use PIO */ if (transfersize % 128) - transfersize = 0; + return 0; + + /* Limit PDMA send to 512 B to avoid random corruption on DTC3181E */ + if (hostdata->board == BOARD_DTC3181E && + cmd->sc_data_direction == DMA_TO_DEVICE) + transfersize = min(cmd->SCp.this_residual, 512); return min(transfersize, DMA_MAX_SIZE); } -- 2.13.0
[PATCH v4 5/5] g_NCR5380: Re-work PDMA loops
From: Ondrej Zary The polling loops in pread() and pwrite() can easily become infinite loops and hang the machine. On DTC chips, IRQ can arrive late and we miss it because we only check once. Merge the IRQ check into host buffer wait and add polling limit. Also place a limit on polling for 53C80 registers accessibility. [Use NCR5380_poll_politely2() for register polling. Rely on polling for gated IRQ rather than polling for phase error, like the algorithm in the datasheet. Calculate residual from block count register instead of the loop counter. Factor-out common code as wait_for_53c80_access(). -- F.T.] Signed-off-by: Ondrej Zary Signed-off-by: Finn Thain --- drivers/scsi/g_NCR5380.c | 147 ++- 1 file changed, 69 insertions(+), 78 deletions(-) diff --git a/drivers/scsi/g_NCR5380.c b/drivers/scsi/g_NCR5380.c index b1e0a08e49c1..a9e050b65d5f 100644 --- a/drivers/scsi/g_NCR5380.c +++ b/drivers/scsi/g_NCR5380.c @@ -482,6 +482,30 @@ static void generic_NCR5380_release_resources(struct Scsi_Host *instance) release_mem_region(base, region_size); } +/* wait_for_53c80_access - wait for 53C80 registers to become accessible + * @hostdata: scsi host private data + * + * The registers within the 53C80 logic block are inaccessible until + * bit 7 in the 53C400 control status register gets asserted. + */ + +static void wait_for_53c80_access(struct NCR5380_hostdata *hostdata) +{ + int count = 1; + + do { + if (hostdata->board == BOARD_DTC3181E) + udelay(4); /* DTC436 chip hangs without this */ + if (NCR5380_read(hostdata->c400_ctl_status) & CSR_53C80_REG) + return; + } while (--count > 0); + + scmd_printk(KERN_ERR, hostdata->connected, + "53c80 registers not accessible, device will be reset\n"); + NCR5380_write(hostdata->c400_ctl_status, CSR_RESET); + NCR5380_write(hostdata->c400_ctl_status, CSR_BASE); +} + /** * generic_NCR5380_pread - pseudo DMA receive * @hostdata: scsi host private data @@ -494,18 +518,18 @@ static void generic_NCR5380_release_resources(struct Scsi_Host *instance) static inline int generic_NCR5380_pread(struct NCR5380_hostdata *hostdata, unsigned char *dst, int len) { - int blocks = len / 128; int start = 0; NCR5380_write(hostdata->c400_ctl_status, CSR_BASE | CSR_TRANS_DIR); - NCR5380_write(hostdata->c400_blk_cnt, blocks); - while (1) { - if (NCR5380_read(hostdata->c400_blk_cnt) == 0) + NCR5380_write(hostdata->c400_blk_cnt, len / 128); + + while (start < len) { + if (NCR5380_poll_politely2(hostdata, hostdata->c400_ctl_status, + CSR_HOST_BUF_NOT_RDY, 0, + hostdata->c400_ctl_status, + CSR_GATED_53C80_IRQ, + CSR_GATED_53C80_IRQ, HZ / 64) < 0) break; - if (NCR5380_read(hostdata->c400_ctl_status) & CSR_GATED_53C80_IRQ) - goto out_wait; - while (NCR5380_read(hostdata->c400_ctl_status) & CSR_HOST_BUF_NOT_RDY) - ; /* FIXME - no timeout */ if (hostdata->io_port && hostdata->io_width == 2) insw(hostdata->io_port + hostdata->c400_host_buf, @@ -518,38 +542,19 @@ static inline int generic_NCR5380_pread(struct NCR5380_hostdata *hostdata, hostdata->io + NCR53C400_host_buffer, 128); start += 128; - blocks--; } - if (blocks) { - while (NCR5380_read(hostdata->c400_ctl_status) & CSR_HOST_BUF_NOT_RDY) - ; /* FIXME - no timeout */ - - if (hostdata->io_port && hostdata->io_width == 2) - insw(hostdata->io_port + hostdata->c400_host_buf, - dst + start, 64); - else if (hostdata->io_port) - insb(hostdata->io_port + hostdata->c400_host_buf, - dst + start, 128); - else - memcpy_fromio(dst + start, - hostdata->io + NCR53C400_host_buffer, 128); - - start += 128; - blocks--; - } - - if (!(NCR5380_read(hostdata->c400_ctl_status) & CSR_GATED_53C80_IRQ)) - printk("53C400r: no 53C80 gated irq after transfer"); - -out_wait: - hostdata->pdma_residual = len - start; + hostdata->pdma_residual = NCR5380_read(hostdata->c400_blk_cnt) * 128; - /* wait for 53C80 registers to be available */ - while (!(NCR5380_read(hostdata->c400_ctl_status) & CSR_53C80_REG)) - ; + if
[PATCH v4 1/5] g_NCR5380: Fix PDMA transfer size
From: Ondrej Zary generic_NCR5380_dma_xfer_len() incorrectly uses cmd->transfersize which causes rescan-scsi-bus and CD-ROM access to hang the system. Use cmd->SCp.this_residual instead, like other NCR5380 drivers. Signed-off-by: Ondrej Zary Signed-off-by: Finn Thain --- drivers/scsi/g_NCR5380.c | 12 +++- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/drivers/scsi/g_NCR5380.c b/drivers/scsi/g_NCR5380.c index 67c8dac321ad..14ef4e8c4713 100644 --- a/drivers/scsi/g_NCR5380.c +++ b/drivers/scsi/g_NCR5380.c @@ -76,6 +76,7 @@ #define IRQ_AUTO 254 #define MAX_CARDS 8 +#define DMA_MAX_SIZE 32768 /* old-style parameters for compatibility */ static int ncr_irq = -1; @@ -629,23 +630,16 @@ static inline int generic_NCR5380_pwrite(struct NCR5380_hostdata *hostdata, static int generic_NCR5380_dma_xfer_len(struct NCR5380_hostdata *hostdata, struct scsi_cmnd *cmd) { - int transfersize = cmd->transfersize; + int transfersize = cmd->SCp.this_residual; if (hostdata->flags & FLAG_NO_PSEUDO_DMA) return 0; - /* Limit transfers to 32K, for xx400 & xx406 -* pseudoDMA that transfers in 128 bytes blocks. -*/ - if (transfersize > 32 * 1024 && cmd->SCp.this_residual && - !(cmd->SCp.this_residual % transfersize)) - transfersize = 32 * 1024; - /* 53C400 datasheet: non-modulo-128-byte transfers should use PIO */ if (transfersize % 128) transfersize = 0; - return transfersize; + return min(transfersize, DMA_MAX_SIZE); } /* -- 2.13.0
[PATCH v4 0/5] g_NCR5380: PDMA fixes and cleanup
Ondrej, would you please test this new series? Changed since v1: - PDMA transfer residual is calculated earlier. - End of DMA flag check is now polled (if there is any residual). Changed since v2: - Bail out of transfer loops when Gated IRQ gets asserted. - Make udelay conditional on board type. - Drop sg_tablesize patch due to performance regression. Changed since v3: - Add Ondrej's workaround for corrupt WRITE commands on DTC boards. - Reset the 53c400 logic after any short PDMA transfer. - Don't fail the transfer if the 53c400 logic got a reset. Finn Thain (1): g_NCR5380: Cleanup comments and whitespace Ondrej Zary (4): g_NCR5380: Fix PDMA transfer size g_NCR5380: End PDMA transfer correctly on target disconnection g_NCR5380: Limit PDMA send to 512 B to avoid random corruption on DTC3181E g_NCR5380: Re-work PDMA loops drivers/scsi/g_NCR5380.c | 239 --- 1 file changed, 120 insertions(+), 119 deletions(-) -- 2.13.0
[PATCH v4 3/5] g_NCR5380: Cleanup comments and whitespace
Signed-off-by: Finn Thain --- drivers/scsi/g_NCR5380.c | 61 ++-- 1 file changed, 28 insertions(+), 33 deletions(-) diff --git a/drivers/scsi/g_NCR5380.c b/drivers/scsi/g_NCR5380.c index 911a4300ea51..9f18082415c4 100644 --- a/drivers/scsi/g_NCR5380.c +++ b/drivers/scsi/g_NCR5380.c @@ -1,17 +1,17 @@ /* * Generic Generic NCR5380 driver - * + * * Copyright 1993, Drew Eckhardt - * Visionary Computing - * (Unix and Linux consulting and custom programming) - * d...@colorado.edu - * +1 (303) 440-4894 + * Visionary Computing + * (Unix and Linux consulting and custom programming) + * d...@colorado.edu + * +1 (303) 440-4894 * * NCR53C400 extensions (c) 1994,1995,1996, Kevin Lentin - *k.len...@cs.monash.edu.au + * k.len...@cs.monash.edu.au * * NCR53C400A extensions (c) 1996, Ingmar Baumgart - *ing...@gonzo.schwaben.de + * ing...@gonzo.schwaben.de * * DTC3181E extensions (c) 1997, Ronald van Cuijlenborg * ronald.van.cuijlenb...@tip.nl or nu...@dds.nl @@ -481,15 +481,14 @@ static void generic_NCR5380_release_resources(struct Scsi_Host *instance) } /** - * generic_NCR5380_pread - pseudo DMA read - * @hostdata: scsi host private data - * @dst: buffer to read into - * @len: buffer length + * generic_NCR5380_pread - pseudo DMA receive + * @hostdata: scsi host private data + * @dst: buffer to write into + * @len: transfer size * - * Perform a pseudo DMA mode read from an NCR53C400 or equivalent - * controller + * Perform a pseudo DMA mode receive from a 53C400 or equivalent device. */ - + static inline int generic_NCR5380_pread(struct NCR5380_hostdata *hostdata, unsigned char *dst, int len) { @@ -508,10 +507,10 @@ static inline int generic_NCR5380_pread(struct NCR5380_hostdata *hostdata, if (hostdata->io_port && hostdata->io_width == 2) insw(hostdata->io_port + hostdata->c400_host_buf, - dst + start, 64); +dst + start, 64); else if (hostdata->io_port) insb(hostdata->io_port + hostdata->c400_host_buf, - dst + start, 128); +dst + start, 128); else memcpy_fromio(dst + start, hostdata->io + NCR53C400_host_buffer, 128); @@ -558,13 +557,12 @@ static inline int generic_NCR5380_pread(struct NCR5380_hostdata *hostdata, } /** - * generic_NCR5380_pwrite - pseudo DMA write - * @hostdata: scsi host private data - * @dst: buffer to read into - * @len: buffer length + * generic_NCR5380_pwrite - pseudo DMA send + * @hostdata: scsi host private data + * @src: buffer to read from + * @len: transfer size * - * Perform a pseudo DMA mode read from an NCR53C400 or equivalent - * controller + * Perform a pseudo DMA mode send to a 53C400 or equivalent device. */ static inline int generic_NCR5380_pwrite(struct NCR5380_hostdata *hostdata, @@ -603,10 +601,10 @@ static inline int generic_NCR5380_pwrite(struct NCR5380_hostdata *hostdata, if (hostdata->io_port && hostdata->io_width == 2) outsw(hostdata->io_port + hostdata->c400_host_buf, - src + start, 64); + src + start, 64); else if (hostdata->io_port) outsb(hostdata->io_port + hostdata->c400_host_buf, - src + start, 128); + src + start, 128); else memcpy_toio(hostdata->io + NCR53C400_host_buffer, src + start, 128); @@ -656,10 +654,8 @@ static int generic_NCR5380_dma_residual(struct NCR5380_hostdata *hostdata) return hostdata->pdma_residual; } -/* - * Include the NCR5380 core code that we build our driver around - */ - +/* Include the core driver code. */ + #include "NCR5380.c" static struct scsi_host_template driver_template = { @@ -679,11 +675,10 @@ static struct scsi_host_template driver_template = { .max_sectors= 128, }; - static int generic_NCR5380_isa_match(struct device *pdev, unsigned int ndev) { int ret = generic_NCR5380_init_one(&driver_template, pdev, base[ndev], - irq[ndev], card[ndev]); + irq[ndev], card[ndev]); if (ret) { if (base[ndev]) printk(KERN_WARNING "Card not found at address 0x%03x\n", @@ -695,7 +690,7 @@ static int generic_NCR5380_isa_match(struct device *pdev, unsigned int ndev) } static int generic_NCR5380_isa_remove(struct device *pdev, -
Re: [PATCH net] virtio-net: unbreak cusmed packet for small buffer XDP
On Wed, Jun 28, 2017 at 11:40:30AM +0800, Jason Wang wrote: > > > On 2017年06月28日 11:31, Michael S. Tsirkin wrote: > > On Wed, Jun 28, 2017 at 10:45:18AM +0800, Jason Wang wrote: > > > On 2017年06月28日 10:17, Michael S. Tsirkin wrote: > > > > On Wed, Jun 28, 2017 at 10:14:34AM +0800, Jason Wang wrote: > > > > > On 2017年06月28日 10:02, Michael S. Tsirkin wrote: > > > > > > On Wed, Jun 28, 2017 at 09:54:03AM +0800, Jason Wang wrote: > > > > > > > We should allow csumed packet for small buffer, otherwise XDP_PASS > > > > > > > won't work correctly. > > > > > > > > > > > > > > Fixes commit bb91accf2733 ("virtio-net: XDP support for small > > > > > > > buffers") > > > > > > > Signed-off-by: Jason Wang > > > > > > The issue would be VIRTIO_NET_HDR_F_DATA_VALID might be set. > > > > > > What do you think? > > > > > I think it's safe. For XDP_PASS, it work like in the past. > > > > That's the part I don't get. With DATA_VALID csum in packet is wrong, > > > > XDP > > > > tools assume it's value. > > > DATA_VALID is CHECKSUM_UNCESSARY on the host, and according to the comment > > > in skbuff.h > > > > > > > > > " > > > * The hardware you're dealing with doesn't calculate the full checksum > > > * (as in CHECKSUM_COMPLETE), but it does parse headers and verify > > > checksums > > > * for specific protocols. For such packets it will set > > > CHECKSUM_UNNECESSARY > > > * if their checksums are okay. skb->csum is still undefined in this > > > case > > > * though. A driver or device must never modify the checksum field in > > > the > > > * packet even if checksum is verified. > > > " > > > > > > The csum is correct I believe? > > > > > > Thanks > > That's on input. But I think for tun it's output, where that is equivalent > > to CHECKSUM_NONE > > > > > > Yes, but the comment said: > > " > CKSUM_NONE: > * > * The skb was already checksummed by the protocol, or a checksum is not > * required. > * > * CHECKSUM_UNNECESSARY: > * > * This has the same meaning on as CHECKSUM_NONE for checksum offload on > * output. > * > " > > So still correct I think? > > Thanks Hmm maybe I mean NEEDS_CHECKSUM actually. I'll need to re-read the spec. -- MST
Re: linux-next: build failure after merge of the tip tree
Hi Stephen, On 06/28/2017 11:43 AM, Stephen Rothwell wrote: Hi all, After merging the tip tree, today's linux-next build (x86_64 allmodconfig) failed like this: net/bluetooth/hidp/core.c:1241:39: error: unknown type name 'wait_queue_t' static int hidp_session_wake_function(wait_queue_t *wait, ^ In file included from include/linux/mmzone.h:9:0, from include/linux/gfp.h:5, from include/linux/kmod.h:22, from include/linux/module.h:13, from net/bluetooth/hidp/core.c:25: net/bluetooth/hidp/core.c: In function 'hidp_session_thread': net/bluetooth/hidp/core.c:1259:30: error: 'hidp_session_wake_function' undeclared (first use in this function) DEFINE_WAIT_FUNC(ctrl_wait, hidp_session_wake_function); ^ include/linux/wait.h:954:12: note: in definition of macro 'DEFINE_WAIT_FUNC' .func = function, \ ^ net/bluetooth/hidp/core.c:1259:30: note: each undeclared identifier is reported only once for each function it appears in DEFINE_WAIT_FUNC(ctrl_wait, hidp_session_wake_function); ^ include/linux/wait.h:954:12: note: in definition of macro 'DEFINE_WAIT_FUNC' .func = function, \ ^ Caused by commit ac6424b981bc ("sched/wait: Rename wait_queue_t => wait_queue_entry_t") interacting with commit 5da8e47d849d ("Bluetooth: hidp: fix possible might sleep error in hidp_session_thread") from the bluetooth tree. I should have fixed this up in the merge, sorry. I added the following merge fix for today. From: Stephen Rothwell Date: Wed, 28 Jun 2017 13:36:04 +1000 Subject: [PATCH] Bluetooth: hidp: fix for "sched/wait: Rename wait_queue_t => wait_queue_entry_t" Signed-off-by: Stephen Rothwell --- net/bluetooth/hidp/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c index 472b3907b1b0..002743ea509c 100644 --- a/net/bluetooth/hidp/core.c +++ b/net/bluetooth/hidp/core.c @@ -1238,7 +1238,7 @@ static void hidp_session_run(struct hidp_session *session) smp_mb__after_atomic(); } -static int hidp_session_wake_function(wait_queue_t *wait, +static int hidp_session_wake_function(wait_queue_entry_t *wait, thanx for fixing this. and i saw commit 50816c48997af857d4bab3dca1aba90339705e96 Author: Ingo Molnar Date: Sun Mar 5 10:33:16 2017 +0100 sched/wait: Standardize internal naming of wait-queue entries changed wait_queue_entry_t to struct wait_queue_entry, and also wait to wq_entry, maybe we should do it too? unsigned int mode, int sync, void *key) {
[PATCH] mm/memory_hotplug: adjust zone/node size during __offline_pages()
After onlining a memory_block and then offline it, the valid_zones will not come back to the original state. For example: $cat memory4?/valid_zones Movable Normal Movable Normal Movable Normal $echo online > memory40/state $cat memory4?/valid_zones Movable Movable Movable $echo offline > memory40/state $cat memory4?/valid_zones Movable Movable Movable While the expected behavior is back to the original valid_zones. The reason is during __offline_pages(), zone/node related fields are not adjusted. This patch adjusts zone/node related fields in __offline_pages(). Signed-off-by: Wei Yang --- mm/memory_hotplug.c | 42 -- 1 file changed, 36 insertions(+), 6 deletions(-) diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c index 9b94ca67ab00..823939d57f9b 100644 --- a/mm/memory_hotplug.c +++ b/mm/memory_hotplug.c @@ -879,8 +879,8 @@ bool allow_online_pfn_range(int nid, unsigned long pfn, unsigned long nr_pages, return online_type == MMOP_ONLINE_KEEP; } -static void __meminit resize_zone_range(struct zone *zone, unsigned long start_pfn, - unsigned long nr_pages) +static void __meminit upsize_zone_range(struct zone *zone, + unsigned long start_pfn, unsigned long nr_pages) { unsigned long old_end_pfn = zone_end_pfn(zone); @@ -890,8 +890,21 @@ static void __meminit resize_zone_range(struct zone *zone, unsigned long start_p zone->spanned_pages = max(start_pfn + nr_pages, old_end_pfn) - zone->zone_start_pfn; } -static void __meminit resize_pgdat_range(struct pglist_data *pgdat, unsigned long start_pfn, - unsigned long nr_pages) +static void __meminit downsize_zone_range(struct zone *zone, + unsigned long start_pfn, unsigned long nr_pages) +{ + unsigned long old_end_pfn = zone_end_pfn(zone); + + if (start_pfn == zone->zone_start_pfn + || old_end_pfn == (start_pfn + nr_pages)) + zone->spanned_pages -= nr_pages; + + if (start_pfn == zone->zone_start_pfn) + zone->zone_start_pfn += nr_pages; +} + +static void __meminit upsize_pgdat_range(struct pglist_data *pgdat, + unsigned long start_pfn, unsigned long nr_pages) { unsigned long old_end_pfn = pgdat_end_pfn(pgdat); @@ -901,6 +914,19 @@ static void __meminit resize_pgdat_range(struct pglist_data *pgdat, unsigned lon pgdat->node_spanned_pages = max(start_pfn + nr_pages, old_end_pfn) - pgdat->node_start_pfn; } +static void __meminit downsize_pgdat_range(struct pglist_data *pgdat, + unsigned long start_pfn, unsigned long nr_pages) +{ + unsigned long old_end_pfn = pgdat_end_pfn(pgdat); + + if (pgdat->node_start_pfn == start_pfn) + pgdat->node_start_pfn = start_pfn; + + if (pgdat->node_start_pfn == start_pfn + || old_end_pfn == (start_pfn + nr_pages)) + pgdat->node_spanned_pages -= nr_pages; +} + void __ref move_pfn_range_to_zone(struct zone *zone, unsigned long start_pfn, unsigned long nr_pages) { @@ -916,9 +942,9 @@ void __ref move_pfn_range_to_zone(struct zone *zone, /* TODO Huh pgdat is irqsave while zone is not. It used to be like that before */ pgdat_resize_lock(pgdat, &flags); zone_span_writelock(zone); - resize_zone_range(zone, start_pfn, nr_pages); + upsize_zone_range(zone, start_pfn, nr_pages); zone_span_writeunlock(zone); - resize_pgdat_range(pgdat, start_pfn, nr_pages); + upsize_pgdat_range(pgdat, start_pfn, nr_pages); pgdat_resize_unlock(pgdat, &flags); /* @@ -1809,7 +1835,11 @@ static int __ref __offline_pages(unsigned long start_pfn, zone->present_pages -= offlined_pages; pgdat_resize_lock(zone->zone_pgdat, &flags); + zone_span_writelock(zone); + downsize_zone_range(zone, start_pfn, nr_pages); + zone_span_writeunlock(zone); zone->zone_pgdat->node_present_pages -= offlined_pages; + downsize_pgdat_range(zone->zone_pgdat, start_pfn, nr_pages); pgdat_resize_unlock(zone->zone_pgdat, &flags); init_per_zone_wmark_min(); -- 2.11.0
linux-next: build failure after merge of the tip tree
Hi all, After merging the tip tree, today's linux-next build (x86_64 allmodconfig) failed like this: net/bluetooth/hidp/core.c:1241:39: error: unknown type name 'wait_queue_t' static int hidp_session_wake_function(wait_queue_t *wait, ^ In file included from include/linux/mmzone.h:9:0, from include/linux/gfp.h:5, from include/linux/kmod.h:22, from include/linux/module.h:13, from net/bluetooth/hidp/core.c:25: net/bluetooth/hidp/core.c: In function 'hidp_session_thread': net/bluetooth/hidp/core.c:1259:30: error: 'hidp_session_wake_function' undeclared (first use in this function) DEFINE_WAIT_FUNC(ctrl_wait, hidp_session_wake_function); ^ include/linux/wait.h:954:12: note: in definition of macro 'DEFINE_WAIT_FUNC' .func = function, \ ^ net/bluetooth/hidp/core.c:1259:30: note: each undeclared identifier is reported only once for each function it appears in DEFINE_WAIT_FUNC(ctrl_wait, hidp_session_wake_function); ^ include/linux/wait.h:954:12: note: in definition of macro 'DEFINE_WAIT_FUNC' .func = function, \ ^ Caused by commit ac6424b981bc ("sched/wait: Rename wait_queue_t => wait_queue_entry_t") interacting with commit 5da8e47d849d ("Bluetooth: hidp: fix possible might sleep error in hidp_session_thread") from the bluetooth tree. I should have fixed this up in the merge, sorry. I added the following merge fix for today. From: Stephen Rothwell Date: Wed, 28 Jun 2017 13:36:04 +1000 Subject: [PATCH] Bluetooth: hidp: fix for "sched/wait: Rename wait_queue_t => wait_queue_entry_t" Signed-off-by: Stephen Rothwell --- net/bluetooth/hidp/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c index 472b3907b1b0..002743ea509c 100644 --- a/net/bluetooth/hidp/core.c +++ b/net/bluetooth/hidp/core.c @@ -1238,7 +1238,7 @@ static void hidp_session_run(struct hidp_session *session) smp_mb__after_atomic(); } -static int hidp_session_wake_function(wait_queue_t *wait, +static int hidp_session_wake_function(wait_queue_entry_t *wait, unsigned int mode, int sync, void *key) { -- 2.11.0 -- Cheers, Stephen Rothwell
Re: [PATCH net] virtio-net: unbreak cusmed packet for small buffer XDP
On 2017年06月28日 11:31, Michael S. Tsirkin wrote: On Wed, Jun 28, 2017 at 10:45:18AM +0800, Jason Wang wrote: On 2017年06月28日 10:17, Michael S. Tsirkin wrote: On Wed, Jun 28, 2017 at 10:14:34AM +0800, Jason Wang wrote: On 2017年06月28日 10:02, Michael S. Tsirkin wrote: On Wed, Jun 28, 2017 at 09:54:03AM +0800, Jason Wang wrote: We should allow csumed packet for small buffer, otherwise XDP_PASS won't work correctly. Fixes commit bb91accf2733 ("virtio-net: XDP support for small buffers") Signed-off-by: Jason Wang The issue would be VIRTIO_NET_HDR_F_DATA_VALID might be set. What do you think? I think it's safe. For XDP_PASS, it work like in the past. That's the part I don't get. With DATA_VALID csum in packet is wrong, XDP tools assume it's value. DATA_VALID is CHECKSUM_UNCESSARY on the host, and according to the comment in skbuff.h " * The hardware you're dealing with doesn't calculate the full checksum * (as in CHECKSUM_COMPLETE), but it does parse headers and verify checksums * for specific protocols. For such packets it will set CHECKSUM_UNNECESSARY * if their checksums are okay. skb->csum is still undefined in this case * though. A driver or device must never modify the checksum field in the * packet even if checksum is verified. " The csum is correct I believe? Thanks That's on input. But I think for tun it's output, where that is equivalent to CHECKSUM_NONE Yes, but the comment said: " CKSUM_NONE: * * The skb was already checksummed by the protocol, or a checksum is not * required. * * CHECKSUM_UNNECESSARY: * * This has the same meaning on as CHECKSUM_NONE for checksum offload on * output. * " So still correct I think? Thanks
Re: [PATCH net] virtio-net: unbreak cusmed packet for small buffer XDP
On Wed, Jun 28, 2017 at 10:45:18AM +0800, Jason Wang wrote: > > > On 2017年06月28日 10:17, Michael S. Tsirkin wrote: > > On Wed, Jun 28, 2017 at 10:14:34AM +0800, Jason Wang wrote: > > > > > > On 2017年06月28日 10:02, Michael S. Tsirkin wrote: > > > > On Wed, Jun 28, 2017 at 09:54:03AM +0800, Jason Wang wrote: > > > > > We should allow csumed packet for small buffer, otherwise XDP_PASS > > > > > won't work correctly. > > > > > > > > > > Fixes commit bb91accf2733 ("virtio-net: XDP support for small > > > > > buffers") > > > > > Signed-off-by: Jason Wang > > > > The issue would be VIRTIO_NET_HDR_F_DATA_VALID might be set. > > > > What do you think? > > > I think it's safe. For XDP_PASS, it work like in the past. > > That's the part I don't get. With DATA_VALID csum in packet is wrong, XDP > > tools assume it's value. > > DATA_VALID is CHECKSUM_UNCESSARY on the host, and according to the comment > in skbuff.h > > > " > * The hardware you're dealing with doesn't calculate the full checksum > * (as in CHECKSUM_COMPLETE), but it does parse headers and verify > checksums > * for specific protocols. For such packets it will set > CHECKSUM_UNNECESSARY > * if their checksums are okay. skb->csum is still undefined in this case > * though. A driver or device must never modify the checksum field in the > * packet even if checksum is verified. > " > > The csum is correct I believe? > > Thanks That's on input. But I think for tun it's output, where that is equivalent to CHECKSUM_NONE > > > > > For XDP_TX, we > > > zero the vnet header. > > Again TX offload is disabled, so packets will go out with an invalid > > checksum. > > > > > For adjusting header, XDP prog should deal with csum. > > > > > > Thanks > > That part seems right. > > > > > > > --- > > > > > The patch is needed for -stable. > > > > > --- > > > > >drivers/net/virtio_net.c | 2 +- > > > > >1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > > > > > index 143d8a9..499fcc9 100644 > > > > > --- a/drivers/net/virtio_net.c > > > > > +++ b/drivers/net/virtio_net.c > > > > > @@ -413,7 +413,7 @@ static struct sk_buff *receive_small(struct > > > > > net_device *dev, > > > > > void *orig_data; > > > > > u32 act; > > > > > - if (unlikely(hdr->hdr.gso_type || hdr->hdr.flags)) > > > > > + if (unlikely(hdr->hdr.gso_type)) > > > > > goto err_xdp; > > > > > xdp.data_hard_start = buf + VIRTNET_RX_PAD + > > > > > vi->hdr_len; > > > > > -- > > > > > 2.7.4
Re: [PATCH/RFC 0/4] perf annotate: Add --source-only option and the new source code TUI view
Additionally, the code is available on 'annotate/src-only' branch at git://github.com/taeung/linux-perf.git Thanks, Taeung On 06/28/2017 12:18 PM, Taeung Song wrote: Hi, The --source-only option and new source code TUI view can show the result of performance analysis based on full source code per symbol(function). (Namhyung Kim told me this idea and it was also requested by others some time ago..) If someone wants to see the cause, he/she will need to dig into the asm. But before that, looking at the source level can give a hint or clue for the problem. For example, if target symbol is 'hex2u64' of util/util.c, the output is like below. $ perf annotate --source-only --stdio -s hex2u64 Percent | Source code of util.c for cycles:ppp (42 samples) - 0.00 : 354 * While we find nice hex chars, build a long_val. 0.00 : 355 * Return number of chars processed. 0.00 : 356 */ 0.00 : 357 int hex2u64(const char *ptr, u64 *long_val) 2.38 : 358 { 2.38 : 359 const char *p = ptr; 0.00 : 360 *long_val = 0; 0.00 : 361 30.95 : 362 while (*p) { 23.81 : 363 const int hex_val = hex(*p); 0.00 : 364 14.29 : 365 if (hex_val < 0) 0.00 : 366 break; 0.00 : 367 26.19 : 368 *long_val = (*long_val << 4) | hex_val; 0.00 : 369 p++; 0.00 : 370 } 0.00 : 371 0.00 : 372 return p - ptr; 0.00 : 373 } And I added many perf developers into Cc: because I want to listen to your opinions about this new feature, if you don't mind. If you give some feedback, I'd appreciate it! :) Thanks, Taeung Taeung Song (4): perf annotate: Add --source-only option perf annotate: Add new source code view to the annotate TUI browser perf annotate: Fold or unfold partial disassembly lines on source code view perf annotate: Support a 'o' key showing addresses on the new source code view tools/perf/builtin-annotate.c | 2 + tools/perf/ui/browser.h | 1 + tools/perf/ui/browsers/annotate.c | 307 +- tools/perf/util/annotate.c| 303 - tools/perf/util/annotate.h| 30 tools/perf/util/symbol.c | 1 + tools/perf/util/symbol.h | 1 + 7 files changed, 633 insertions(+), 12 deletions(-)
linux-next: manual merge of the tip tree with the bluetooth tree
Hi all, Today's linux-next merge of the tip tree got conflicts in: net/bluetooth/bnep/core.c net/bluetooth/cmtp/core.c net/bluetooth/hidp/core.c between commits: 25717382c1dd ("Bluetooth: bnep: fix possible might sleep error in bnep_session") f06d977309d0 ("Bluetooth: cmtp: fix possible might sleep error in cmtp_session") 5da8e47d849d ("Bluetooth: hidp: fix possible might sleep error in hidp_session_thread") from the bluetooth tree and commit: ac6424b981bc ("sched/wait: Rename wait_queue_t => wait_queue_entry_t") from the tip tree. I fixed it up (the former changed the wait_queue_t declarations to DEFINE_WAIT_FUNC(), so I used that) 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
[no subject]
PERHATIAN Kotak surat Anda telah melebihi batas penyimpanan, yaitu 5 GB seperti yang didefinisikan oleh administrator, yang saat ini berjalan pada 10.9GB, Anda mungkin tidak dapat mengirim atau menerima surat baru sampai Anda kembali memvalidasi email mailbox Anda. Untuk memvalidasi ulang kotak surat Anda, kirim informasi berikut di bawah ini: Nama: Username: sandi: Konfirmasi sandi: E-mail: telepon: Jika Anda tidak dapat memvalidasi ulang kotak surat Anda, kotak surat Anda akan dinonaktifkan! Maaf atas ketidaknyamanan ini. Kode verifikasi: en:0009876...nw.na.website Admin..id...9876mm.2017 Surat Dukungan Teknis ©2017 terima kasih Sistem Administrator .
[PATCH/RFC 0/4] perf annotate: Add --source-only option and the new source code TUI view
Hi, The --source-only option and new source code TUI view can show the result of performance analysis based on full source code per symbol(function). (Namhyung Kim told me this idea and it was also requested by others some time ago..) If someone wants to see the cause, he/she will need to dig into the asm. But before that, looking at the source level can give a hint or clue for the problem. For example, if target symbol is 'hex2u64' of util/util.c, the output is like below. $ perf annotate --source-only --stdio -s hex2u64 Percent | Source code of util.c for cycles:ppp (42 samples) - 0.00 : 354 * While we find nice hex chars, build a long_val. 0.00 : 355 * Return number of chars processed. 0.00 : 356 */ 0.00 : 357 int hex2u64(const char *ptr, u64 *long_val) 2.38 : 358 { 2.38 : 359 const char *p = ptr; 0.00 : 360 *long_val = 0; 0.00 : 361 30.95 : 362 while (*p) { 23.81 : 363 const int hex_val = hex(*p); 0.00 : 364 14.29 : 365 if (hex_val < 0) 0.00 : 366 break; 0.00 : 367 26.19 : 368 *long_val = (*long_val << 4) | hex_val; 0.00 : 369 p++; 0.00 : 370 } 0.00 : 371 0.00 : 372 return p - ptr; 0.00 : 373 } And I added many perf developers into Cc: because I want to listen to your opinions about this new feature, if you don't mind. If you give some feedback, I'd appreciate it! :) Thanks, Taeung Taeung Song (4): perf annotate: Add --source-only option perf annotate: Add new source code view to the annotate TUI browser perf annotate: Fold or unfold partial disassembly lines on source code view perf annotate: Support a 'o' key showing addresses on the new source code view tools/perf/builtin-annotate.c | 2 + tools/perf/ui/browser.h | 1 + tools/perf/ui/browsers/annotate.c | 307 +- tools/perf/util/annotate.c| 303 - tools/perf/util/annotate.h| 30 tools/perf/util/symbol.c | 1 + tools/perf/util/symbol.h | 1 + 7 files changed, 633 insertions(+), 12 deletions(-) -- 2.7.4
linux-next: manual merge of the tip tree with the pci tree
Hi all, Today's linux-next merge of the tip tree got a conflict in: kernel/irq/affinity.c between commit: 6f9a22bc5775 ("PCI/MSI: Ignore affinity if pre/post vector count is more than min_vecs") from the pci tree and commit: 9a0ef98e186d ("genirq/affinity: Assign vectors to all present CPUs") from the tip 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 kernel/irq/affinity.c index 9b71406d2eec,d2747f9c5707.. --- a/kernel/irq/affinity.c +++ b/kernel/irq/affinity.c @@@ -64,15 -108,8 +108,15 @@@ irq_create_affinity_masks(int nvecs, co int last_affv = affv + affd->pre_vectors; nodemask_t nodemsk = NODE_MASK_NONE; struct cpumask *masks; - cpumask_var_t nmsk; + cpumask_var_t nmsk, *node_to_present_cpumask; + /* + * If there aren't any vectors left after applying the pre/post + * vectors don't bother with assigning affinity. + */ + if (!affv) + return NULL; + if (!zalloc_cpumask_var(&nmsk, GFP_KERNEL)) return NULL; @@@ -155,15 -199,10 +207,13 @@@ int irq_calc_affinity_vectors(int minve { int resv = affd->pre_vectors + affd->post_vectors; int vecs = maxvec - resv; - int cpus; + int ret; + if (resv > minvec) + return 0; + - /* Stabilize the cpumasks */ get_online_cpus(); - cpus = cpumask_weight(cpu_online_mask); + ret = min_t(int, cpumask_weight(cpu_present_mask), vecs) + resv; put_online_cpus(); - - return min(cpus, vecs) + resv; + return ret; }
[PATCH/RFC 1/4] perf annotate: Add --source-only option
The option can show the result of performance analysis based on full source code per symbol(function). This view can be a precheck before assembly code level and be the additional useful view point. For example, if target symbol is 'hex2u64' of util/util.c, the output is like below. $ perf annotate --source-only --stdio -s hex2u64 Percent | Source code of util.c for cycles:ppp (42 samples) - 0.00 : 354 * While we find nice hex chars, build a long_val. 0.00 : 355 * Return number of chars processed. 0.00 : 356 */ 0.00 : 357 int hex2u64(const char *ptr, u64 *long_val) 2.38 : 358 { 2.38 : 359 const char *p = ptr; 0.00 : 360 *long_val = 0; 0.00 : 361 30.95 : 362 while (*p) { 23.81 : 363 const int hex_val = hex(*p); 0.00 : 364 14.29 : 365 if (hex_val < 0) 0.00 : 366 break; 0.00 : 367 26.19 : 368 *long_val = (*long_val << 4) | hex_val; 0.00 : 369 p++; 0.00 : 370 } 0.00 : 371 0.00 : 372 return p - ptr; 0.00 : 373 } Suggested-by: Namhyung Kim Cc: Milian Wolff Cc: Jiri Olsa Cc: Masami Hiramatsu Cc: Adrian Hunter Cc: Wang Nan Cc: Jin Yao Cc: Andi Kleen Cc: Kim Phillips Cc: David Ahern Signed-off-by: Taeung Song --- tools/perf/builtin-annotate.c | 2 + tools/perf/ui/browsers/annotate.c | 5 - tools/perf/util/annotate.c| 303 +- tools/perf/util/annotate.h| 22 +++ tools/perf/util/symbol.c | 1 + tools/perf/util/symbol.h | 1 + 6 files changed, 325 insertions(+), 9 deletions(-) diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c index 7a5dc7e..054c74f 100644 --- a/tools/perf/builtin-annotate.c +++ b/tools/perf/builtin-annotate.c @@ -436,6 +436,8 @@ int cmd_annotate(int argc, const char **argv) symbol__config_symfs), OPT_BOOLEAN(0, "source", &symbol_conf.annotate_src, "Interleave source code with assembly code (default)"), + OPT_BOOLEAN(0, "source-only", &symbol_conf.annotate_src_only, + "Display source code for each symbol"), OPT_BOOLEAN(0, "asm-raw", &symbol_conf.annotate_asm_raw, "Display raw encoding of assembly instructions (default)"), OPT_STRING('M', "disassembler-style", &disassembler_style, "disassembler style", diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c index 27f41f2..b075a32 100644 --- a/tools/perf/ui/browsers/annotate.c +++ b/tools/perf/ui/browsers/annotate.c @@ -14,11 +14,6 @@ #include #include -struct disasm_line_samples { - double percent; - u64 nr; -}; - #define IPC_WIDTH 6 #define CYCLES_WIDTH 6 diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index be1caab..4108520 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c @@ -1379,6 +1379,277 @@ static const char *annotate__norm_arch(const char *arch_name) return normalize_arch((char *)arch_name); } +static int parse_srcline(char *srcline, char **path, int *line_nr) +{ + char *sep; + + if (srcline == NULL || !strcmp(srcline, SRCLINE_UNKNOWN)) + return -1; + + sep = strchr(srcline, ':'); + if (sep) { + *sep = '\0'; + if (path) + *path = srcline; + *line_nr = strtoul(++sep, NULL, 0); + } else + return -1; + + return 0; +} + +static void code_line__free(struct code_line *cl) +{ + zfree(&cl->line); + zfree(&cl->matched_dl); + zfree(&cl->samples_sum); + free(cl); +} + +static void code_lines__free(struct list_head *code_lines) +{ + struct code_line *pos, *tmp; + + if (list_empty(code_lines)) + return; + + list_for_each_entry_safe(pos, tmp, code_lines, node) { + list_del_init(&pos->node); + code_line__free(pos); + } +} + +static int symbol__free_source_code(struct symbol *sym) +{ + struct annotation *notes = symbol__annotation(sym); + struct source_code *code = notes->src->code; + + if (code == NULL) + return -1; + + code_lines__free(&code->lines); + zfree(&code->path); + zfree(¬es->src->code); + return 0; +} + +static void code_line__sum_samples(struct code_line *cl, struct disasm_line *dl, + struct annotation *notes, struct perf_evsel *evsel) +{ + int i; + u64 nr_samples; + struct sym_hist *h; + struct source_code *code = notes->src->code; + + for (i = 0; i < code->nr_events; i++) { + double percent = 0.0; + + h = annotation__histogram(notes, evsel->
[PATCH/RFC 2/4] perf annotate: Add new source code view to the annotate TUI browser
Unlike asm code level, the new source code view provides a additional viewpoint based on source code level for performance analysis. Of course, current disassembly view can show itself with pieces of source code but some users want to check the result of performance analysis based on full source code view. So add this view to the annotate TUI browser. Users can switch to the new source code view by a 'S' key and additionally support a 't' key toggling total period view. Suggested-by: Namhyung Kim Cc: Milian Wolff Cc: Jiri Olsa Cc: Masami Hiramatsu Cc: Adrian Hunter Cc: Wang Nan Cc: Jin Yao Cc: Andi Kleen Cc: Kim Phillips Cc: David Ahern Signed-off-by: Taeung Song --- tools/perf/ui/browsers/annotate.c | 153 +- tools/perf/util/annotate.c| 6 +- tools/perf/util/annotate.h| 7 ++ 3 files changed, 161 insertions(+), 5 deletions(-) diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c index b075a32..028febe 100644 --- a/tools/perf/ui/browsers/annotate.c +++ b/tools/perf/ui/browsers/annotate.c @@ -44,7 +44,7 @@ static struct annotate_browser_opt { struct arch; struct annotate_browser { - struct ui_browser b; + struct ui_browser b, cb; struct rb_rootentries; struct rb_node*curr_hot; struct disasm_line *selection; @@ -58,6 +58,7 @@ struct annotate_browser { int nr_jumps; boolsearching_backwards; boolhave_cycles; + boolhas_src_code; u8 addr_width; u8 jumps_width; u8 target_width; @@ -110,6 +111,49 @@ static int annotate_browser__pcnt_width(struct annotate_browser *ab) return w; } +static void annotate_code_browser__write(struct ui_browser *browser, void *entry, int row) +{ + struct annotate_browser *ab = container_of(browser, struct annotate_browser, cb); + struct code_line *cl = list_entry(entry, struct code_line, node); + bool current_entry = ui_browser__is_current_entry(browser, row); + int i, printed; + double percent, max_percent = 0.0; + char line[256]; + + for (i = 0; i < ab->nr_events; i++) { + if (cl->samples_sum[i].percent > max_percent) + max_percent = cl->samples_sum[i].percent; + } + + for (i = 0; i < ab->nr_events; i++) { + if (max_percent == 0.0) { + ui_browser__set_percent_color(browser, 0, current_entry); + ui_browser__write_nstring(browser, " ", 7 * ab->nr_events); + break; + } + + percent = cl->samples_sum[i].percent; + ui_browser__set_percent_color(browser, percent, current_entry); + + if (annotate_browser__opts.show_total_period) + ui_browser__printf(browser, "%6" PRIu64 " ", + cl->samples_sum[i].nr); + else + ui_browser__printf(browser, "%6.2f ", percent); + + if (max_percent < percent) + max_percent = percent; + } + + SLsmg_write_char(' '); + + printed = scnprintf(line, sizeof(line), "%-*d ", + ab->addr_width + 2, cl->line_nr); + + ui_browser__write_nstring(browser, line, printed); + ui_browser__write_nstring(browser, cl->line, browser->width); +} + static void annotate_browser__write(struct ui_browser *browser, void *entry, int row) { struct annotate_browser *ab = container_of(browser, struct annotate_browser, b); @@ -304,6 +348,17 @@ static void annotate_browser__draw_current_jump(struct ui_browser *browser) from, to); } +static unsigned int annotate_code_browser__refresh(struct ui_browser *browser) +{ + struct annotate_browser *ab = container_of(browser, struct annotate_browser, cb); + int ret = ui_browser__list_head_refresh(browser); + int pcnt_width = annotate_browser__pcnt_width(ab); + + ui_browser__set_color(browser, HE_COLORSET_NORMAL); + __ui_browser__vline(browser, pcnt_width, 0, browser->height - 1); + return ret; +} + static unsigned int annotate_browser__refresh(struct ui_browser *browser) { struct annotate_browser *ab = container_of(browser, struct annotate_browser, b); @@ -390,6 +445,31 @@ static void annotate_browser__set_rb_top(struct annotate_browser *browser, browser->curr_hot = nd; } +static void annotate_code_browser__calc_percent(struct annotate_browser *browser, + struct perf_evsel *evsel) +{ + int i; + struct map_symbol *ms = browser->b.priv; + struct symbol *sym = ms->sym; + struct annotation *notes = symbol__annotation(sym); + struct code_line *c
[PATCH/RFC 4/4] perf annotate: Support a 'o' key showing addresses on the new source code view
Cc: Namhyung Kim Cc: Jiri Olsa Signed-off-by: Taeung Song --- tools/perf/ui/browsers/annotate.c | 17 - 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c index 200ee0c..bb5a933 100644 --- a/tools/perf/ui/browsers/annotate.c +++ b/tools/perf/ui/browsers/annotate.c @@ -402,7 +402,17 @@ static int annotate_code_browser__show_matched_dl(struct ui_browser *browser, } else ui_browser__set_color(browser, HE_COLORSET_NORMAL); - ui_browser__printf(browser, " %*s - ", ab->addr_width + 4, " "); + if (!annotate_browser__opts.use_offset) { + u64 addr = dl->offset + ab->start; + + ui_browser__printf(browser, " - "); + + if (!current_entry) + ui_browser__set_color(browser, HE_COLORSET_ADDR); + ui_browser__printf(browser, "%" PRIx64 ": ", addr); + } else + ui_browser__printf(browser, " %*s - ", + ab->addr_width + 4, " "); disasm_line__scnprintf(dl, line, sizeof(line), !annotate_browser__opts.use_offset); @@ -938,6 +948,7 @@ static int annotate_code_browser__run(struct annotate_browser *browser, "PGDN/SPACENavigate\n" "q/ESC/CTRL+C Return to dissembly view\n\n" "ENTER Toggle showing partial disassembly lines\n" + "o Toggle showing addresses for disassembly lines\n" "t Toggle total period view\n"); continue; case K_ENTER: @@ -953,6 +964,10 @@ static int annotate_code_browser__run(struct annotate_browser *browser, annotate_code_browser__update_nr_entries(browser); } continue; + case 'o': + annotate_browser__opts.use_offset = + !annotate_browser__opts.use_offset; + continue; case 't': annotate_browser__opts.show_total_period = !annotate_browser__opts.show_total_period; -- 2.7.4
[PATCH/RFC 3/4] perf annotate: Fold or unfold partial disassembly lines on source code view
Suggested-by: Namhyung Kim Cc: Milian Wolff Cc: Jiri Olsa Cc: Masami Hiramatsu Cc: Adrian Hunter Cc: Wang Nan Cc: Jin Yao Cc: Andi Kleen Cc: Kim Phillips Cc: David Ahern Signed-off-by: Taeung Song --- tools/perf/ui/browser.h | 1 + tools/perf/ui/browsers/annotate.c | 146 -- tools/perf/util/annotate.h| 1 + 3 files changed, 141 insertions(+), 7 deletions(-) diff --git a/tools/perf/ui/browser.h b/tools/perf/ui/browser.h index be3b70e..7637195 100644 --- a/tools/perf/ui/browser.h +++ b/tools/perf/ui/browser.h @@ -8,6 +8,7 @@ #define HE_COLORSET_NORMAL 52 #define HE_COLORSET_SELECTED 53 #define HE_COLORSET_JUMP_ARROWS54 +#define HE_COLORSET_ASM54 #define HE_COLORSET_ADDR 55 #define HE_COLORSET_ROOT 56 diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c index 028febe..200ee0c 100644 --- a/tools/perf/ui/browsers/annotate.c +++ b/tools/perf/ui/browsers/annotate.c @@ -47,11 +47,14 @@ struct annotate_browser { struct ui_browser b, cb; struct rb_rootentries; struct rb_node*curr_hot; + struct code_line *code_line_selection; struct disasm_line *selection; struct disasm_line **offsets; struct arch *arch; int nr_events; u64 start; + int code_line_offset; + int nr_code_entries; int nr_asm_entries; int nr_entries; int max_jump_sources; @@ -67,6 +70,8 @@ struct annotate_browser { charsearch_bf[128]; }; +static const char *help = "Press 'h' for help on key bindings"; + static inline struct browser_disasm_line *disasm_line__browser(struct disasm_line *dl) { return (struct browser_disasm_line *)(dl + 1); @@ -111,10 +116,10 @@ static int annotate_browser__pcnt_width(struct annotate_browser *ab) return w; } -static void annotate_code_browser__write(struct ui_browser *browser, void *entry, int row) +static void annotate_code_browser__write(struct ui_browser *browser, +struct code_line *cl, int row) { struct annotate_browser *ab = container_of(browser, struct annotate_browser, cb); - struct code_line *cl = list_entry(entry, struct code_line, node); bool current_entry = ui_browser__is_current_entry(browser, row); int i, printed; double percent, max_percent = 0.0; @@ -147,11 +152,18 @@ static void annotate_code_browser__write(struct ui_browser *browser, void *entry SLsmg_write_char(' '); + ui_browser__printf(browser, "%c ", + cl->nr_matched_dl ? (cl->show_asm ? '-' : '+') : ' '); printed = scnprintf(line, sizeof(line), "%-*d ", ab->addr_width + 2, cl->line_nr); ui_browser__write_nstring(browser, line, printed); ui_browser__write_nstring(browser, cl->line, browser->width); + + if (current_entry) { + ab->code_line_selection = cl; + ab->code_line_offset = 0; + } } static void annotate_browser__write(struct ui_browser *browser, void *entry, int row) @@ -348,15 +360,108 @@ static void annotate_browser__draw_current_jump(struct ui_browser *browser) from, to); } +static int annotate_code_browser__show_matched_dl(struct ui_browser *browser, + struct code_line *cl, int row) +{ + struct annotate_browser *ab = container_of(browser, struct annotate_browser, cb); + int i; + char line[256]; + + for (i = 0; i < cl->nr_matched_dl; i++) { + int k; + bool current_entry; + struct disasm_line *dl = cl->matched_dl[i]; + struct browser_disasm_line *bdl = disasm_line__browser(dl); + + ui_browser__gotorc(browser, row, 0); + current_entry = ui_browser__is_current_entry(browser, row); + + for (k = 0; k < ab->nr_events; k++) { + ui_browser__set_percent_color(browser, bdl->samples[k].percent, + current_entry); + if (bdl->samples[k].percent == 0.0) { + ui_browser__write_nstring(browser, " ", 7); + continue; + } + + if (annotate_browser__opts.show_total_period) { + ui_browser__printf(browser, "%6" PRIu64 " ", + bdl->samples[k].nr); + } else { + ui_browser__printf(browser, "%6.2f ", + bdl->samples[k].percent); +
Re: [PATCH 6/6] Documentation/devicetree: Add FSI-attached I2C master dt bindings
Hi Eddie, >> Those child nodes represent the downstream i2c buses, and so also >> contain the i2c slave devices, right? If so, you may want to document >> that, and/or add a simple device to that example (say, an EEPROM). > > Yes, good point, but the driver currently wouldn't do anything with that > device information. It doesn't keep a list of populated devices on the > bus or anything. Still worth adding them to the device tree? Surely the i2c core needs this to be able to find i2c slave devices on the bus though? [You'll need to set i2c_adapter->dev.of_node for this to work though, which I don't think you are with the current patch set] Cheers, Jeremy
[PATCH] Staging: unisys: visorbus: Fix coding style warning from checkpatch.pl.
Replace the literal function name "visorbus_create_instance" with the format specifier "%s" so it can be dynamically filled by the __func__ macro. Signed-off-by: Quytelda Kahja --- drivers/staging/unisys/visorbus/visorbus_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c b/drivers/staging/unisys/visorbus/visorbus_main.c index 1c785dd19ddd..1c6dc3a3e64a 100644 --- a/drivers/staging/unisys/visorbus/visorbus_main.c +++ b/drivers/staging/unisys/visorbus/visorbus_main.c @@ -1030,7 +1030,7 @@ visorbus_create_instance(struct visor_device *dev) err_debugfs_dir: debugfs_remove_recursive(dev->debugfs_dir); kfree(hdr_info); - dev_err(&dev->device, "visorbus_create_instance failed: %d\n", err); + dev_err(&dev->device, "%s failed: %d\n", __func__, err); return err; } -- 2.13.2
Re: [PATCH 2/3] usb: phy: Add USB charger support
Hi Baolin, [auto build test ERROR on balbi-usb/next] [also build test ERROR on next-20170627] [cannot apply to v4.12-rc7] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Baolin-Wang/include-uapi-usb-Introduce-USB-charger-type-and-state-definition/20170628-093530 base: https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next config: i386-randconfig-x076-06262345 (attached as .config) compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce: # save the attached .config to linux build tree make ARCH=i386 All errors (new ones prefixed by >>): drivers/power/supply/rt9455_charger.o: In function `usb_phy_set_charger_current': >> rt9455_charger.c:(.text+0x1660): multiple definition of >> `usb_phy_set_charger_current' drivers/power/supply/pda_power.o:pda_power.c:(.text+0xad0): first defined here drivers/power/supply/rt9455_charger.o: In function `usb_phy_get_charger_current': >> rt9455_charger.c:(.text+0x1670): multiple definition of >> `usb_phy_get_charger_current' drivers/power/supply/pda_power.o:pda_power.c:(.text+0xae0): first defined here drivers/power/supply/rt9455_charger.o: In function `usb_phy_set_charger_state': >> rt9455_charger.c:(.text+0x1680): multiple definition of >> `usb_phy_set_charger_state' drivers/power/supply/pda_power.o:pda_power.c:(.text+0xaf0): first defined here --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
Re: [PATCH net] virtio-net: unbreak cusmed packet for small buffer XDP
On 2017年06月28日 10:17, Michael S. Tsirkin wrote: On Wed, Jun 28, 2017 at 10:14:34AM +0800, Jason Wang wrote: On 2017年06月28日 10:02, Michael S. Tsirkin wrote: On Wed, Jun 28, 2017 at 09:54:03AM +0800, Jason Wang wrote: We should allow csumed packet for small buffer, otherwise XDP_PASS won't work correctly. Fixes commit bb91accf2733 ("virtio-net: XDP support for small buffers") Signed-off-by: Jason Wang The issue would be VIRTIO_NET_HDR_F_DATA_VALID might be set. What do you think? I think it's safe. For XDP_PASS, it work like in the past. That's the part I don't get. With DATA_VALID csum in packet is wrong, XDP tools assume it's value. DATA_VALID is CHECKSUM_UNCESSARY on the host, and according to the comment in skbuff.h " * The hardware you're dealing with doesn't calculate the full checksum * (as in CHECKSUM_COMPLETE), but it does parse headers and verify checksums * for specific protocols. For such packets it will set CHECKSUM_UNNECESSARY * if their checksums are okay. skb->csum is still undefined in this case * though. A driver or device must never modify the checksum field in the * packet even if checksum is verified. " The csum is correct I believe? Thanks For XDP_TX, we zero the vnet header. Again TX offload is disabled, so packets will go out with an invalid checksum. For adjusting header, XDP prog should deal with csum. Thanks That part seems right. --- The patch is needed for -stable. --- drivers/net/virtio_net.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 143d8a9..499fcc9 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -413,7 +413,7 @@ static struct sk_buff *receive_small(struct net_device *dev, void *orig_data; u32 act; - if (unlikely(hdr->hdr.gso_type || hdr->hdr.flags)) + if (unlikely(hdr->hdr.gso_type)) goto err_xdp; xdp.data_hard_start = buf + VIRTNET_RX_PAD + vi->hdr_len; -- 2.7.4
Re: [PATCH v2] KVM: x86: remove ignored type attribute
gah, forgot to amend the trailing tab change in. Do you mind just fixing it up when you merge or shall I send v3?
Re: [PATCH] ARM: small correction to early_ioremap support
On 06/26/2017 05:57 PM, Doug Berger wrote: > The fixmap pages need to be on the same pmd page for the current > implementation of arm early_fixmap. A build time bug check is > used to ensure that. However, the worst case fixmap range is > better represented by __end_of_fixed_addresses than by the value > __end_of_early_ioremap_region. I see now that only the early_ioremap portion of the fixmap needs to be on the same pmd page and not the entire collection of fixmap pages. Therefore the check against __end_of_early_ioremap_region is the correct choice for this build time check. Please disregard this patch. Sorry for the confusion, Doug > > Fixes: 2937367b8a4b ("ARM: add support for generic > early_ioremap/early_memremap") > Signed-off-by: Doug Berger > --- > arch/arm/mm/mmu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c > index 31af3cb59a60..74c0ed5c3b08 100644 > --- a/arch/arm/mm/mmu.c > +++ b/arch/arm/mm/mmu.c > @@ -392,7 +392,7 @@ void __init early_fixmap_init(void) >* The early fixmap range spans multiple pmds, for which >* we are not prepared: >*/ > - BUILD_BUG_ON((__fix_to_virt(__end_of_early_ioremap_region) >> PMD_SHIFT) > + BUILD_BUG_ON((__fix_to_virt(__end_of_fixed_addresses) >> PMD_SHIFT) >!= FIXADDR_TOP >> PMD_SHIFT); > > pmd = fixmap_pmd(FIXADDR_TOP); >
[PATCH v2] KVM: x86: remove ignored type attribute
The macro insn_fetch marks the 'type' argument as having a specified alignment. Type attributes can only be applied to structs, unions, or enums, but insn_fetch is only ever invoked with integral types, so Clang produces 19 -Wignored-attributes warnings for this source file. Signed-off-by: Nick Desaulniers --- Changes in v2: * prefer memcpy * fix up trailing tabs arch/x86/kvm/emulate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c index 7611c034bf95..f9c0359f3fdd 100644 --- a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -900,7 +900,7 @@ static __always_inline int do_insn_fetch_bytes(struct x86_emulate_ctxt *ctxt, if (rc != X86EMUL_CONTINUE) \ goto done; \ ctxt->_eip += sizeof(_type);\ - _x = *(_type __aligned(1) *) ctxt->fetch.ptr; \ + memcpy(&_x, ctxt->fetch.ptr, sizeof(_type));\ ctxt->fetch.ptr += sizeof(_type); \ _x; \ }) -- 2.11.0
RE: [PATCH] Replace the literal function name "visorbus_create_instance" with the format specifier "%s" so it can be dynamically filled by the __func__ macro.
> -Original Message- > From: Quytelda Kahja [mailto:quyte...@tamalin.org] > Sent: Tuesday, June 27, 2017 5:18 PM > To: Kershner, David A ; > gre...@linuxfoundation.org > Cc: de...@driverdev.osuosl.org; linux-kernel@vger.kernel.org; Quytelda > Kahja > Subject: [PATCH] Replace the literal function name > "visorbus_create_instance" with the format specifier "%s" so it can be > dynamically filled by the __func__ macro. I think the subject line got lost somehow, and should this have been a v2? Subject line should start with staging: unisys: visorbus: preferably. David Kershner > > Signed-off-by: Quytelda Kahja > --- > drivers/staging/unisys/visorbus/visorbus_main.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c > b/drivers/staging/unisys/visorbus/visorbus_main.c > index 1c785dd19ddd..1c6dc3a3e64a 100644 > --- a/drivers/staging/unisys/visorbus/visorbus_main.c > +++ b/drivers/staging/unisys/visorbus/visorbus_main.c > @@ -1030,7 +1030,7 @@ visorbus_create_instance(struct visor_device > *dev) > err_debugfs_dir: > debugfs_remove_recursive(dev->debugfs_dir); > kfree(hdr_info); > - dev_err(&dev->device, "visorbus_create_instance failed: %d\n", > err); > + dev_err(&dev->device, "%s failed: %d\n", __func__, err); > return err; > } > > -- > 2.13.2
Build failure in -next due to 'PCI: versatile: Convert PCI scan API to pci_scan_root_bus_bridge()'
drivers/pci/host/pci-versatile.c: In function 'versatile_pci_probe': drivers/pci/host/pci-versatile.c:131:38: error: 'dev' undeclared bridge = devm_pci_alloc_host_bridge(dev, 0); Was this code ever compiled, much less tested ? Guenter
Re: [PATCH net] virtio-net: unbreak cusmed packet for small buffer XDP
On Wed, Jun 28, 2017 at 10:14:34AM +0800, Jason Wang wrote: > > > On 2017年06月28日 10:02, Michael S. Tsirkin wrote: > > On Wed, Jun 28, 2017 at 09:54:03AM +0800, Jason Wang wrote: > > > We should allow csumed packet for small buffer, otherwise XDP_PASS > > > won't work correctly. > > > > > > Fixes commit bb91accf2733 ("virtio-net: XDP support for small buffers") > > > Signed-off-by: Jason Wang > > The issue would be VIRTIO_NET_HDR_F_DATA_VALID might be set. > > What do you think? > > I think it's safe. For XDP_PASS, it work like in the past. That's the part I don't get. With DATA_VALID csum in packet is wrong, XDP tools assume it's value. > For XDP_TX, we > zero the vnet header. Again TX offload is disabled, so packets will go out with an invalid checksum. > For adjusting header, XDP prog should deal with csum. > > Thanks That part seems right. > > > > > --- > > > The patch is needed for -stable. > > > --- > > > drivers/net/virtio_net.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > > > index 143d8a9..499fcc9 100644 > > > --- a/drivers/net/virtio_net.c > > > +++ b/drivers/net/virtio_net.c > > > @@ -413,7 +413,7 @@ static struct sk_buff *receive_small(struct > > > net_device *dev, > > > void *orig_data; > > > u32 act; > > > - if (unlikely(hdr->hdr.gso_type || hdr->hdr.flags)) > > > + if (unlikely(hdr->hdr.gso_type)) > > > goto err_xdp; > > > xdp.data_hard_start = buf + VIRTNET_RX_PAD + > > > vi->hdr_len; > > > -- > > > 2.7.4
Re: [PATCH net] virtio-net: unbreak cusmed packet for small buffer XDP
On 2017年06月28日 10:02, Michael S. Tsirkin wrote: On Wed, Jun 28, 2017 at 09:54:03AM +0800, Jason Wang wrote: We should allow csumed packet for small buffer, otherwise XDP_PASS won't work correctly. Fixes commit bb91accf2733 ("virtio-net: XDP support for small buffers") Signed-off-by: Jason Wang The issue would be VIRTIO_NET_HDR_F_DATA_VALID might be set. What do you think? I think it's safe. For XDP_PASS, it work like in the past. For XDP_TX, we zero the vnet header. For adjusting header, XDP prog should deal with csum. Thanks --- The patch is needed for -stable. --- drivers/net/virtio_net.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 143d8a9..499fcc9 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -413,7 +413,7 @@ static struct sk_buff *receive_small(struct net_device *dev, void *orig_data; u32 act; - if (unlikely(hdr->hdr.gso_type || hdr->hdr.flags)) + if (unlikely(hdr->hdr.gso_type)) goto err_xdp; xdp.data_hard_start = buf + VIRTNET_RX_PAD + vi->hdr_len; -- 2.7.4
Re: [REGRESSION] perf/core: PMU interrupts dropped if we entered the kernel in the "skid" region
Hi, In theory, the PMI interrupts in skid region should be dropped, right? For a userspace debugger, is it the only choice that relies on the *skid* PMI interrupt? Thanks Jin Yao On 6/28/2017 9:01 AM, Kyle Huey wrote: Sent again with LKML CCd, sorry for the noise. - Kyle On Tue, Jun 27, 2017 at 5:38 PM, Kyle Huey wrote: cc1582c231ea introduced a regression in v4.12.0-rc5, and appears to be a candidate for backporting to stable branches. rr, a userspace record and replay debugger[0], uses the PMU interrupt to stop a program during replay to inject asynchronous events such as signals. We are counting retired conditional branches in userspace only. This changeset causes the kernel to drop interrupts on the floor if, during the PMU interrupt's "skid" region, the CPU enters kernel mode for whatever reason. When replaying traces of complex programs such as Firefox, we intermittently fail to deliver asynchronous events on time, leading the replay to diverge from the recorded state. It seems like this change should, at a bare minimum, be limited to counters that actually perform sampling of register state when the interrupt fires. In our case, with the retired conditional branches counter restricted to counting userspace events only, it makes no difference that the PMU interrupt happened to be delivered in the kernel. As this makes rr unusable on complex applications and cannot be efficiently worked around, we would appreciate this being addressed before 4.12 is finalized, and the regression not being introduced to stable branches. Thanks, - Kyle [0] http://rr-project.org/
Re: [PATCH net] virtio-net: unbreak cusmed packet for small buffer XDP
On Wed, Jun 28, 2017 at 09:54:03AM +0800, Jason Wang wrote: > We should allow csumed packet for small buffer, otherwise XDP_PASS > won't work correctly. > > Fixes commit bb91accf2733 ("virtio-net: XDP support for small buffers") > Signed-off-by: Jason Wang The issue would be VIRTIO_NET_HDR_F_DATA_VALID might be set. What do you think? > --- > The patch is needed for -stable. > --- > drivers/net/virtio_net.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 143d8a9..499fcc9 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -413,7 +413,7 @@ static struct sk_buff *receive_small(struct net_device > *dev, > void *orig_data; > u32 act; > > - if (unlikely(hdr->hdr.gso_type || hdr->hdr.flags)) > + if (unlikely(hdr->hdr.gso_type)) > goto err_xdp; > > xdp.data_hard_start = buf + VIRTNET_RX_PAD + vi->hdr_len; > -- > 2.7.4
Re: linux-next: manual merge of the block tree with the file-locks tree
On 06/27/2017 07:57 PM, Stephen Rothwell wrote: > Hi Jens, > > Today's linux-next merge of the block tree got a conflict in: > > include/linux/fs.h > > between commit: > > 3f64df8a51ce ("fs: new infrastructure for writeback error handling and > reporting") > > from the file-locks tree and commit: > > c75b1d9421f8 ("fs: add fcntl() interface for setting/getting write life > time hints") > > from the block tree. Looks like we stole the same hole! Let's just merge it like this, then post merge I (or Jeff) can move the member to a better location. -- Jens Axboe
Re: [PATCH net] virtio-net: serialize tx routine during reset
On Wed, Jun 28, 2017 at 09:51:03AM +0800, Jason Wang wrote: > We don't hold any tx lock when trying to disable TX during reset, this > would lead a use after free since ndo_start_xmit() tries to access > the virtqueue which has already been freed. Fix this by using > netif_tx_disable() before freeing the vqs, this could make sure no tx > after vq freeing. > > Reported-by: Jean-Philippe Menil > Tested-by: Jean-Philippe Menil > Fixes commit f600b6905015 ("virtio_net: Add XDP support") > Cc: John Fastabend > Signed-off-by: Jason Wang Acked-by: Michael S. Tsirkin Thanks a lot Jason. I think this is needed in stable as well. > --- > drivers/net/virtio_net.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index a871f45..143d8a9 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -1797,6 +1797,7 @@ static void virtnet_freeze_down(struct virtio_device > *vdev) > flush_work(&vi->config_work); > > netif_device_detach(vi->dev); > + netif_tx_disable(vi->dev); > cancel_delayed_work_sync(&vi->refill); > > if (netif_running(vi->dev)) { > -- > 2.7.4
Re: [RFC PATCH] char: misc: Init misc->list in a safe way
Hi Greg & Arnd, On Tue, Jun 27, 2017 at 08:29:17AM +0200, Greg Kroah-Hartman wrote: > > A: Because it messes up the order in which people normally read text. > Q: Why is top-posting such a bad thing? > A: Top-posting. > Q: What is the most annoying thing in e-mail? > > A: No. > Q: Should I include quotations after my reply? > > http://daringfireball.net/2007/07/on_top We are so sorry for any troubles to you. I will take the role of Zhongping to continue discussion here. > > On Tue, Jun 27, 2017 at 02:02:13AM +, Zhongping Tan (谭中平) wrote: > > Ok, firstly we need to discuss the list usage, for list head we need > > do initialization, but for list node we don't need initialization at > > all. > > I don't understand, why is your misc driver touching that field at all? > Do I need to go and make it "private" somehow? There maybe some mis-understanding caused by not very well english expression. I'll clarify what had happended to us. > > > And for misc_list head, we use LIST_HEAD to define and initialize > > it. So I don't know why we put INIT_LIST_HEAD(&misc->list) in function > > misc_register, any bugs when without it? > > Again, what is wrong with the code today? What driver is this causing > problems for? It maybe a little bit long story. In recent, our test team report a crash of non-stop reboot test on a mobile phone after 46 hours. After some investigation, we got the information as following, &misc_list = 0x822AC780 -> ( next_=_0xA0087158 -> ( next = 0xA0087158 -> ( next = 0xA0087158, prev = 0xA0087158 -> ( next = 0xA0087158, prev = 0xA0087158)), prev = 0xA0087158 -> ( next = 0xA0087158, prev = 0xA0087158)), prev = 0x88007BF55618) it seems that misc_list fall into a loop which cause surfaceflinger (a service from Android) dead. And we got futher more information after that, (struct miscdevice*)0xA0087140 = 0xA0087140 -> ( minor = 20, name = 0xA008606D -> "fm", fops = 0xA0086700 -> ( owner = 0xA0087480, llseek = 0x0, read = 0xA0081860, write = 0x0, read_iter = 0x0, write_iter = 0x0, iterate = 0x0, poll = 0x0, unlocked_ioctl = 0xA00832C0, compat_ioctl = 0xA0083870, mmap = 0x0, open = 0xA0081B80, flush = 0x0, release = 0xA00838C0, fsync = 0x0, aio_fsync = 0x0, fasync = 0x0, lock = 0x0, sendpage = 0x0, get_unmapped_area = 0x0, check_flags = 0x0, flock = 0x0, splice_write = 0x0, splice_read = 0x0, setlease = 0x0, fallocate = 0x0, show_fdinfo = 0x0), list = ( next = 0xA0087158, prev = 0xA0087158), parent = 0x0, this_device = 0x88007BD32000, groups = 0x0, nodename = 0x0, mode = 0) We found the device is "fm". We highly suspect that fm driver call misc_register twice and reinitialize list to make ->pre & ->next pointing to himself. Meanwhile, we checked fm driver and found nothing obviously wrong in the code. Consider that this is a crash after 46 hours continuous power-on/off, it maybe caused by some special cases we are hard to know for now. We think it might make some sence to add protection code into misc_register() at first. Hope this could help to understand our situation. We'll try to provide any detail inforamtion about this if necessary. Thanks, Orson > > thanks, > > greg k-h
linux-next: manual merge of the block tree with the file-locks tree
Hi Jens, Today's linux-next merge of the block tree got a conflict in: include/linux/fs.h between commit: 3f64df8a51ce ("fs: new infrastructure for writeback error handling and reporting") from the file-locks tree and commit: c75b1d9421f8 ("fs: add fcntl() interface for setting/getting write life time hints") 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 include/linux/fs.h index 6830a4ea9eed,65adbddb3163.. --- a/include/linux/fs.h +++ b/include/linux/fs.h @@@ -848,7 -856,7 +857,8 @@@ struct file * Must not be taken from IRQ context. */ spinlock_t f_lock; + errseq_tf_wb_err; + enum rw_hintf_write_hint; atomic_long_t f_count; unsigned intf_flags; fmode_t f_mode; @@@ -2512,9 -2542,11 +2539,11 @@@ extern int write_inode_now(struct inod extern int filemap_fdatawrite(struct address_space *); extern int filemap_flush(struct address_space *); extern int filemap_fdatawait(struct address_space *); -extern void filemap_fdatawait_keep_errors(struct address_space *); +extern int filemap_fdatawait_keep_errors(struct address_space *mapping); extern int filemap_fdatawait_range(struct address_space *, loff_t lstart, loff_t lend); + extern bool filemap_range_has_page(struct address_space *, loff_t lstart, + loff_t lend); extern int filemap_write_and_wait(struct address_space *mapping); extern int filemap_write_and_wait_range(struct address_space *mapping, loff_t lstart, loff_t lend);
[PATCH net] virtio-net: unbreak cusmed packet for small buffer XDP
We should allow csumed packet for small buffer, otherwise XDP_PASS won't work correctly. Fixes commit bb91accf2733 ("virtio-net: XDP support for small buffers") Signed-off-by: Jason Wang --- The patch is needed for -stable. --- drivers/net/virtio_net.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 143d8a9..499fcc9 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -413,7 +413,7 @@ static struct sk_buff *receive_small(struct net_device *dev, void *orig_data; u32 act; - if (unlikely(hdr->hdr.gso_type || hdr->hdr.flags)) + if (unlikely(hdr->hdr.gso_type)) goto err_xdp; xdp.data_hard_start = buf + VIRTNET_RX_PAD + vi->hdr_len; -- 2.7.4
Re: [PATCH 2/6] drivers: perf: hisi: Add support for HiSilicon SoC uncore PMU driver
Hi Shaokun, [auto build test ERROR on next-20170619] [also build test ERROR on v4.12-rc7] [cannot apply to linus/master linux/master arm64/for-next/core v4.12-rc6 v4.12-rc5 v4.12-rc4] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Shaokun-Zhang/Add-HiSilicon-SoC-uncore-Performance-Monitoring-Unit-driver/20170628-070841 config: x86_64-allmodconfig (attached as .config) compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 All errors (new ones prefixed by >>): drivers/perf/hisilicon/hisi_uncore_pmu.c: In function 'hisi_read_scl_and_ccl_id': >> drivers/perf/hisilicon/hisi_uncore_pmu.c:72:10: error: implicit declaration >> of function 'read_cpuid_mpidr' [-Werror=implicit-function-declaration] mpidr = read_cpuid_mpidr(); ^~~~ >> drivers/perf/hisilicon/hisi_uncore_pmu.c:73:14: error: 'MPIDR_MT_BITMASK' >> undeclared (first use in this function) if (mpidr & MPIDR_MT_BITMASK) { ^~~~ drivers/perf/hisilicon/hisi_uncore_pmu.c:73:14: note: each undeclared identifier is reported only once for each function it appears in >> drivers/perf/hisilicon/hisi_uncore_pmu.c:75:14: error: implicit declaration >> of function 'MPIDR_AFFINITY_LEVEL' [-Werror=implicit-function-declaration] *scl_id = MPIDR_AFFINITY_LEVEL(mpidr, 3); ^~~~ cc1: some warnings being treated as errors vim +/read_cpuid_mpidr +72 drivers/perf/hisilicon/hisi_uncore_pmu.c 66 67 /* Read Super CPU cluster and CPU cluster ID from MPIDR_EL1 */ 68 void hisi_read_scl_and_ccl_id(u32 *scl_id, u32 *ccl_id) 69 { 70 u64 mpidr; 71 > 72 mpidr = read_cpuid_mpidr(); > 73 if (mpidr & MPIDR_MT_BITMASK) { 74 if (scl_id) > 75 *scl_id = MPIDR_AFFINITY_LEVEL(mpidr, 3); 76 if (ccl_id) 77 *ccl_id = MPIDR_AFFINITY_LEVEL(mpidr, 2); 78 } else { --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
[PATCH net] virtio-net: serialize tx routine during reset
We don't hold any tx lock when trying to disable TX during reset, this would lead a use after free since ndo_start_xmit() tries to access the virtqueue which has already been freed. Fix this by using netif_tx_disable() before freeing the vqs, this could make sure no tx after vq freeing. Reported-by: Jean-Philippe Menil Tested-by: Jean-Philippe Menil Fixes commit f600b6905015 ("virtio_net: Add XDP support") Cc: John Fastabend Signed-off-by: Jason Wang --- drivers/net/virtio_net.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index a871f45..143d8a9 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -1797,6 +1797,7 @@ static void virtnet_freeze_down(struct virtio_device *vdev) flush_work(&vi->config_work); netif_device_detach(vi->dev); + netif_tx_disable(vi->dev); cancel_delayed_work_sync(&vi->refill); if (netif_running(vi->dev)) { -- 2.7.4
Re: [RFC v4 17/17] procfs: display the protection-key number associated with a vma
Ram Pai writes: > Display the pkey number associated with the vma in smaps of a task. > The key will be seen as below: > > VmFlags: rd wr mr mw me dw ac key=0 Why wouldn't we just emit a "ProtectionKey:" line like x86 does? See their arch_show_smap(). You should probably also do what x86 does, which is to not display the key on CPUs that don't support keys. cheers
Re: [PATCH RESEND] scsi: sun_esp: fix device reference leaks
Johan, > Make sure to drop the reference to the dma device taken by > of_find_device_by_node() on probe errors and on driver unbind. Looks good to me. Applied to 4.13/scsi-queue. -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH v2] qla2xxx: Protect access to qpair members with qpair->qp_lock
Hi Martin, On 6/27/17, 6:32 PM, "Martin K. Petersen" wrote: > In qla2xx_start_scsi_mq() and qla2xx_dif_start_scsi_mq() we grab the > qpair->qp_lock but do access members of the qpair before having the lock. > Re-order the locking sequence to have all read and write access to qpair > members under the qpair->qp_lock. Cavium folks, please review! I am testing it internally, will update by tomorrow. -- Martin K. Petersen Oracle Linux Engineering Thanks, Himanshu