Re: [PATCH V2 1/2] Documentation/features: Add na key to arch-support.txt
On 2015/07/17 19:51, Ananth N Mavinakayanahalli wrote: > To be used for features we will not support on a particular architecture. > The git log that adds this needs to provide the justification 'why?' > > Signed-off-by: Ananth N Mavinakayanahalli > --- > Documentation/features/arch-support.txt |1 + > 1 file changed, 1 insertion(+) > > diff --git a/Documentation/features/arch-support.txt > b/Documentation/features/arch-support.txt > index d22a109..f2b272e 100644 > --- a/Documentation/features/arch-support.txt > +++ b/Documentation/features/arch-support.txt > @@ -8,4 +8,5 @@ The meaning of entries in the tables is: > | ok | # feature supported by the architecture > |TODO| # feature not yet supported by the architecture > | .. | # feature cannot be supported by the hardware > +| na | # feature will not be supported by the architecture "will not" sounds like someone's will. I guess it could be "feature is useless/needless on the architecture". Thank you, -- Masami HIRAMATSU Linux Technology Research Center, System Productivity Research Dept. Center for Technology Innovation - Systems Engineering Hitachi, Ltd., Research & Development Group E-mail: masami.hiramatsu...@hitachi.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Re: [RFC PATCH perf/core v2 00/16] perf-probe --cache and SDT support
Hi Masami, On Fri, Jul 17, 2015 at 12:21:42PM +0900, Masami Hiramatsu wrote: > Now I'm thinking that we should avoid using %event syntax for perf-list > and perf-record to avoid confusion. For example, suppose that we have > "libfoo:bar" SDT event, when we just scanned the libfoo binary and > use it via perf-record, we'll run perf record -e "%libfoo:bar". > However, after we set the probe via perf-probe, we have to run > perf record -e "libfoo:bar". That difference looks no good. > So, I think in both case it should accept -e "libfoo:bar" syntax. I don't remember how the SDT events should be shown to users. Sorry if I'm missing something here. AFAIK an SDT event consists of a provider and an event name. So it can be simply 'provider:event' like tracepoints or 'binary:provider_event' like uprobes. I like the former because it's simpler but it needs to guarantee that it doesn't clash with existing tracepoints/[ku]probes. So IIUC we chose the '%' sign to distinguish them. But after setting a probe at it, the group name should be the binary name. So the whole event name might be changed, and this is not good. So we should use the latter form. But in this case, I think we need a way to distinguish provider and event names. Since the provider name also can include '_' characters in it. And maybe it still needs to distinguish an SDT event and its probe for some reason. In that case, we might use 'sdt:provider_event' form or something like that. Thanks, Namhyung > > In this series I've introduced %event syntax only to recall cached event > setting explicitly, because perf-probe is a lower layer tool to set up > new event. IMO, perf-list and perf-record should be higher tools which > handle abstract events. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/7] pinctrl: UniPhier: add UniPhier pinctrl core support
Hi Linus, 2015-07-16 16:20 GMT+09:00 Linus Walleij : > On Tue, Jul 14, 2015 at 4:40 AM, Masahiro Yamada > wrote: > >> The core support for the pinctrl drivers for all the UniPhier SoCs. >> >> Signed-off-by: Masahiro Yamada >> --- >> >> Changes in v2: >> - drop vogus THIS_MODULE because this file is always built-in >> - drop vogus "include because this file is >> always built-in > > This looks nice and uses all generic facilities we have. > > Patch applied. > > Yours, > Linus Walleij > I saw the applied patch and noticed "Changes in v2" was moved to git-description. commit 6e908892025885b07e804dc6c05aab6ce1e06832 Author: Masahiro Yamada Date: Tue Jul 14 11:40:01 2015 +0900 pinctrl: UniPhier: add UniPhier pinctrl core support The core support for the pinctrl drivers for all the UniPhier SoCs. Changes in v2: - drop vogus THIS_MODULE because this file is always built-in - drop vogus "include because this file is always built-in Signed-off-by: Masahiro Yamada Signed-off-by: Linus Walleij Do we need to record such stuff in the git history? In my opinion, the difference between patch versions should be mentioned to help reviewers, but not included in the git repository. Early versions are often so immature that they must be improved in the review process. No point to record what was updated from those versions, I think. This is why I put "changed in v2" below the "---". -- Best Regards Masahiro Yamada -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[GIT PULL] ARM: SoC fixes
The following changes since commit bc0195aad0daa2ad5b0d76cce22b167bc3435590: Linux 4.2-rc2 (2015-07-12 15:10:30 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc.git tags/armsoc-fixes for you to fetch changes up to 3eae03daa538303e0d28f56412c6c51f6452e493: Merge tag 'pxa-fixes-v4.2-rc2' of https://github.com/rjarzmik/linux into fixesD (2015-07-18 21:06:10 -0700) ARM: SoC fixes for v4.2 By far most of the fixes here are updates to DTS files to deal with some mostly minor bugs. There's also a fix to deal with non-PM kernel configs on i.MX, a regression fix for ethernet on PXA platforms and a dependency fix for OMAP. Adam YH Lee (3): ARM: dts: omap3: overo: Update LCD panel names ARM: dts: configure regulators for Gumstix Pepper ARM: dts: Correct audio input route & set mic bias for am335x-pepper Arun Bharadwaj (1): ARM: dts: Fix frequency scaling on Gumstix Pepper Baruch Siach (1): MAINTAINERS: digicolor: add dts files Chris Zhong (1): ARM: dts: cros-ec-keyboard: Add support for some Japanese keys Dave Gerlach (1): ARM: OMAP2+: Add HAVE_ARM_SCU for AM43XX Fabio Estevam (1): ARM: dts: imx27: Adjust the GPT compatible string Linus Walleij (2): ARM: ux500: define serial port aliases ARM: ux500: fix MMC/SD card regression Lucas Stach (1): ARM: imx6: gpc: always enable PU domain if CONFIG_PM is not set Murali Karicheri (2): ARM: keystone: dts: fix dt bindings for PCIe ARM: keystone: dts: rename pcie nodes to help override status Olof Johansson (5): Merge tag 'socfpga_fixes_for_v4.2-rc1' of git://git.kernel.org/.../dinguyen/linux into fixes Merge tag 'imx-fixes-4.2' of git://git.kernel.org/.../shawnguo/linux into fixes Merge tag 'omap-for-v4.2/fixes-rc2-v2' of git://git.kernel.org/.../tmlind/linux-omap into fixes Merge tag 'keystone-dts-fixes' of git://git.kernel.org/.../ssantosh/linux-keystone into fixes Merge tag 'pxa-fixes-v4.2-rc2' of https://github.com/rjarzmik/linux into fixesD Philipp Zabel (1): ARM: dts: imx53-qsb: fix TVE entry Robert Jarzmik (1): ARM: pxa: fix dm9000 platform data regression Stefan Wahren (1): ARM: dts: mx23: fix iio-hwmon support Suman Anna (2): ARM: dts: OMAP4: Add #iommu-cells property to IOMMUs ARM: dts: OMAP5: Add #iommu-cells property to IOMMUs Walter Lozano (2): ARM: socfpga: dts: Fix adxl34x formating and compatible string ARM: socfpga: dts: Fix entries order MAINTAINERS | 1 + arch/arm/boot/dts/am335x-pepper.dts | 16 --- arch/arm/boot/dts/cros-ec-keyboard.dtsi | 4 arch/arm/boot/dts/imx23.dtsi| 1 + arch/arm/boot/dts/imx27.dtsi| 12 +-- arch/arm/boot/dts/imx53-qsb-common.dtsi | 5 +++-- arch/arm/boot/dts/k2e.dtsi | 3 ++- arch/arm/boot/dts/keystone.dtsi | 3 ++- arch/arm/boot/dts/omap3-overo-common-lcd35.dtsi | 2 +- arch/arm/boot/dts/omap3-overo-common-lcd43.dtsi | 2 +- arch/arm/boot/dts/omap4.dtsi| 2 ++ arch/arm/boot/dts/omap5.dtsi| 2 ++ arch/arm/boot/dts/socfpga_cyclone5_sockit.dts | 26 arch/arm/boot/dts/ste-ccu8540.dts | 7 +++ arch/arm/boot/dts/ste-ccu9540.dts | 7 +++ arch/arm/boot/dts/ste-dbx5x0.dtsi | 6 +++--- arch/arm/boot/dts/ste-href.dtsi | 2 +- arch/arm/boot/dts/ste-hrefprev60-stuib.dts | 7 +++ arch/arm/boot/dts/ste-hrefprev60-tvk.dts| 7 +++ arch/arm/boot/dts/ste-hrefprev60.dtsi | 5 + arch/arm/boot/dts/ste-hrefv60plus-stuib.dts | 7 +++ arch/arm/boot/dts/ste-hrefv60plus-tvk.dts | 7 +++ arch/arm/boot/dts/ste-hrefv60plus.dtsi | 25 +-- arch/arm/boot/dts/ste-snowball.dts | 25 +-- arch/arm/mach-imx/gpc.c | 27 ++--- arch/arm/mach-omap2/Kconfig | 1 + arch/arm/mach-pxa/capc7117.c| 3 +++ arch/arm/mach-pxa/cm-x2xx.c | 3 +++ arch/arm/mach-pxa/cm-x300.c | 2 ++ arch/arm/mach-pxa/colibri-pxa270.c | 3 +++ arch/arm/mach-pxa/em-x270.c | 2 ++ arch/arm/mach-pxa/icontrol.c| 3 +++ arch/arm/mach-pxa/trizeps4.c| 3 +++ arch/arm/mach-pxa/vpac270.c | 3 +++ arch/arm/mach-pxa/zeus.c| 2 ++ 35 files changed, 179 insertions(+), 57 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a
Re: [RFC PATCH 04/21] x86/hweight: Add stack frame dependency for __arch_hweight*()
On Sat, Jul 18, 2015 at 10:57:14AM -0500, Josh Poimboeuf wrote: > Currently, when stackvalidate sees an ALTERNATIVE, it assumes that > either code path is possible, so it follows both paths in parallel. > > If I understand right, you're proposing that stackvalidate should only > follow the POPCNT path and never follow the !POPCNT path? Actually, you don't even need to follow the POPCNT case either because it is a single instruction - no stack operations there. So yeah, either that or special-case the case where the original insn is CALL and the replacement is a POPCNT and ignore those CALL locations. The advantage is that the burden is put on the tool and not by adding markers to kernel code paths. > In general, I agree, and I like the original patch much better. IMO, it > achieved the goal of keeping the kernel code clean, while fixing the > frame pointer bug. And I think that in that case, adding that rSP dependency is too much because even though it fixes the "bug", it is very very unlikely any stack trace will have __sw_hweight* in it for reasons pointed out earlier and also because those functions can't fail and they get integral types as args which can't fail when deref-fing either. And even if they do, they don't call any other functions so rIP pointing to them is already enough. So even if we're not 100% correct wrt stack traces in this case, I think that's ok. -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. -- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] perf tools: Do not require /bin/sh be bash
Commit fcfd6611fbccdbf2593bd949097a5c0e45cd96da truncated .config-detected by piping echo -n to it. On systems where /bin/sh is not bash and echo is a builtin, echo -n prints "-n", which breaks the build: .config-detected:1: *** missing separator. Stop. This broke builds on my Gentoo systems where /bin/sh is dash and possibly other systems where /bin/sh is not bash as well. It is easily solved by piping cat /dev/null. Piping an empty command should have been a suitable substitute, but zsh hangs when asked to do that. This method of truncating a file works on bash, busybox ash, dash and zsh. Other shells were not tested. Signed-off-by: Richard Yao Reviewed-by: Emilio López --- tools/perf/config/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile index 094ddae..70426d6 100644 --- a/tools/perf/config/Makefile +++ b/tools/perf/config/Makefile @@ -11,7 +11,7 @@ ifneq ($(obj-perf),) obj-perf := $(abspath $(obj-perf))/ endif -$(shell echo -n > $(OUTPUT).config-detected) +$(shell cat /dev/null > $(OUTPUT).config-detected) detected = $(shell echo "$(1)=y" >> $(OUTPUT).config-detected) detected_var = $(shell echo "$(1)=$($(1))" >> $(OUTPUT).config-detected) -- 2.3.6 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC V5 2/4] perf,tool: per-event time support
Hi Jiri, On Sat, Jul 18, 2015 at 02:45:47PM +0200, Jiri Olsa wrote: > On Fri, Jul 17, 2015 at 07:30:53AM -0400, kan.li...@intel.com wrote: > > SNIP > > > diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c > > index a71eeb2..c9981df 100644 > > --- a/tools/perf/util/parse-events.c > > +++ b/tools/perf/util/parse-events.c > > @@ -25,6 +25,9 @@ > > #ifdef PARSER_DEBUG > > extern int parse_events_debug; > > #endif > > + > > +bool time_term_detected = false; > > + > > int parse_events_parse(void *data, void *scanner); > > > > static struct perf_pmu_event_symbol *perf_pmu_events_list; > > @@ -598,6 +601,14 @@ do { > >\ > > * attr->branch_sample_type = term->val.num; > > */ > > break; > > + case PARSE_EVENTS__TERM_TYPE_TIME: > > + CHECK_TYPE_VAL(NUM); > > + if (term->val.num > 1) > > + return -EINVAL; > > + time_term_detected = true; > > + if (term->val.num == 1) > > + attr->sample_type |= PERF_SAMPLE_TIME; > > +break; > > case PARSE_EVENTS__TERM_TYPE_NAME: > > CHECK_TYPE_VAL(STR); > > break; > > diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h > > index 131f29b..1083478 100644 > > --- a/tools/perf/util/parse-events.h > > +++ b/tools/perf/util/parse-events.h > > @@ -22,6 +22,8 @@ struct tracepoint_path { > > struct tracepoint_path *next; > > }; > > > > +extern bool time_term_detected; > > so I wasnt happy about this time_term_detected global variable, > and I tried to make it without and ended up with somewhat siplified > patch.. not tested very deeply, just the basics: > > > [jolsa@krava perf]$ ./perf record -e 'cpu/cpu-cycles/,cpu/instructions/' > kill > ... > [jolsa@krava perf]$ ./perf evlist -v > cpu/cpu-cycles/: type: 4, size: 112, config: 0x3c, { sample_period, > sample_freq }: 4000, sample_type: IP|TID|TIME|ID|PERIOD, read_format: ID, > disabled: 1, inherit: 1, mmap: 1, comm: 1, freq: 1, enable_on_exec: 1, task: > 1, sample_id_all: 1, exclude_guest: 1, mmap2: 1, comm_exec: 1 > cpu/instructions/: type: 4, size: 112, config: 0xc0, { sample_period, > sample_freq }: 4000, sample_type: IP|TID|TIME|ID|PERIOD, read_format: ID, > disabled: 1, inherit: 1, freq: 1, enable_on_exec: 1, sample_id_all: 1, > exclude_guest: 1 > > > > [jolsa@krava perf]$ ./perf record -e > 'cpu/cpu-cycles/,cpu/instructions,time/' kill > ... > [jolsa@krava perf]$ ./perf evlist -v > cpu/cpu-cycles/: type: 4, size: 112, config: 0x3c, { sample_period, > sample_freq }: 4000, sample_type: IP|TID|PERIOD|IDENTIFIER, read_format: ID, > disabled: 1, inherit: 1, mmap: 1, comm: 1, freq: 1, enable_on_exec: 1, task: > 1, sample_id_all: 1, exclude_guest: 1, mmap2: 1, comm_exec: 1 > cpu/instructions,time/: type: 4, size: 112, config: 0xc0, { sample_period, > sample_freq }: 4000, sample_type: IP|TID|TIME|PERIOD|IDENTIFIER, read_format: > ID, disabled: 1, inherit: 1, freq: 1, enable_on_exec: 1, sample_id_all: 1, > exclude_guest: 1 What about this case? $ perf record -e 'cpu/cpu-cycles/,cpu/instructions,time=0/' kill > > > --- > diff --git a/tools/perf/Documentation/perf-record.txt > b/tools/perf/Documentation/perf-record.txt > index 5b47b2c88223..df479077384d 100644 > --- a/tools/perf/Documentation/perf-record.txt > +++ b/tools/perf/Documentation/perf-record.txt > @@ -49,7 +49,9 @@ OPTIONS > These params can be used to set event defaults. > Here is a list of the params. > - 'period': Set event sampling period > - > + - 'time': Disable/enable time stamping. Acceptable values are 1 for > + enabling time stamping. 0 for disabling time stamping. > + The default is 1. > Note: If user explicitly sets options which conflict with the params, > the value set by the params will be overridden. > > diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c > index 83c08037e7e2..8e3a17845c37 100644 > --- a/tools/perf/util/evsel.c > +++ b/tools/perf/util/evsel.c > @@ -712,7 +712,8 @@ void perf_evsel__config(struct perf_evsel *evsel, struct > record_opts *opts) > perf_evsel__set_sample_bit(evsel, TIME); > > if (opts->raw_samples && !evsel->no_aux_samples) { > - perf_evsel__set_sample_bit(evsel, TIME); > + if (opts->sample_time) > + perf_evsel__set_sample_bit(evsel, TIME); > perf_evsel__set_sample_bit(evsel, RAW); > perf_evsel__set_sample_bit(evsel, CPU); > } > diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c > index a71eeb279ed2..95100478200a 100644 > --- a/tools/perf/util/parse-events.c > +++ b/tools/perf/util/parse-events.c > @@ -598,6 +598,13 @@ do { >\ >
Re: [PATCH tip/master 1/3] kprobes: Support blacklist functions in module
On 2015/07/17 21:10, Ingo Molnar wrote: > > * Masami Hiramatsu wrote: > >> To blacklist the functions in a module (e.g. user-defined >> kprobe handler and the functions invoked from it), expand >> blacklist support for modules. >> With this change, users can use NOKPROBE_SYMBOL() macro in >> their own modules. > > Btw., whatever happened with renaming '__kprobes' to '__nokprobe' and using > that > consistently to blacklist certain functions? Yes, in this part, __kprobes marked functions placed in .kprobes.text section are safely added to the blacklist :) - + if (err >= 0 && __kprobes_text_start != __kprobes_text_end) { + /* The __kprobes marked functions must not be probed */ + err = kprobe_blacklist_add_range( + (unsigned long)__kprobes_text_start, + (unsigned long)__kprobes_text_end); + } - > > Also, shouldn't we convert such instances: > > static int notifier_call_chain(struct notifier_block **nl, >unsigned long val, void *v, >int nr_to_call, int *nr_calls) > > ... > > NOKPROBE_SYMBOL(notifier_call_chain); > > to: > > static int __nokprobe notifier_call_chain(struct notifier_block **nl, >unsigned long val, void *v, >int nr_to_call, int *nr_calls) > > ? For some symbols we can do that. But it can conflict with other __section attributes e.g. __sched, since a function must be placed in only one section. So, IMHO, using section for expressing its attribute is not a good idea, but I couldn't find another option in common function attribute. https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#Common-Function-Attributes Thus I've introduced NOKPROBE_SYMBOL macro which stores the target function addresses (not the function itself) in the _kprobe_blacklist section. Thank you, > > I.e. instead of extending it to modules we should eliminate NOKPROBE_SYMBOL() > in > favor of marking functions as __nokprobe which is the standard syntax for > marking > functions. > > Thanks, > > Ingo > -- Masami HIRAMATSU Linux Technology Research Center, System Productivity Research Dept. Center for Technology Innovation - Systems Engineering Hitachi, Ltd., Research & Development Group E-mail: masami.hiramatsu...@hitachi.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Ghost Lid switch on Baytrail tablet
Hey, This tablet: http://www.amazon.com/WinBook-10-1-Inch-Windows-full-size-Display/dp/B00N9ZG5U2/ doesn't have a physical lid switch unless you plug the keyboard accessory to it. But it thinks that there is a lid, and that it's closed, making logind suspend the machine after about 30 seconds. I'm currently testing this on a 4.0 kernel, as there have been regressions on Baytrail with 4.1 and 4.2rcs. The DSDT for that tablet: https://people.gnome.org/~hadess/Winbook%20TW100%20DSDT.dsl Any ideas how to handle this sort of device? Cheers -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 3/4] mtd: pxa3xx_nand: rework flash detection and timing setup
On 07/07/2015 12:08 PM, Antoine Tenart wrote: Rework the pxa3xx_nand driver to allow using functions exported by the nand framework to detect the flash and the timings. Then setup the timings using the helpers previously added. Signed-off-by: Antoine Tenart --- drivers/mtd/nand/pxa3xx_nand.c | 137 - 1 file changed, 38 insertions(+), 99 deletions(-) diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c index 9a95c24ab2ce..513f8f6069f0 100644 --- a/drivers/mtd/nand/pxa3xx_nand.c +++ b/drivers/mtd/nand/pxa3xx_nand.c @@ -1320,48 +1320,16 @@ static int pxa3xx_nand_waitfunc(struct mtd_info *mtd, struct nand_chip *this) return NAND_STATUS_READY; } -static int pxa3xx_nand_config_flash(struct pxa3xx_nand_info *info, - const struct pxa3xx_nand_flash *f) +static int pxa3xx_nand_config_flash(struct pxa3xx_nand_info *info) { - struct platform_device *pdev = info->pdev; - struct pxa3xx_nand_platform_data *pdata = dev_get_platdata(>dev); struct pxa3xx_nand_host *host = info->host[info->cs]; - uint32_t ndcr = 0x0; /* enable all interrupts */ - - if (f->page_size != 2048 && f->page_size != 512) { - dev_err(>dev, "Current only support 2048 and 512 size\n"); - return -EINVAL; - } - - if (f->flash_width != 16 && f->flash_width != 8) { - dev_err(>dev, "Only support 8bit and 16 bit!\n"); - return -EINVAL; - } - - /* calculate flash information */ - host->read_id_bytes = (f->page_size == 2048) ? 4 : 2; - - /* calculate addressing information */ - host->col_addr_cycles = (f->page_size == 2048) ? 2 : 1; - - if (f->num_blocks * f->page_per_block > 65536) - host->row_addr_cycles = 3; - else - host->row_addr_cycles = 2; - - ndcr |= (pdata->enable_arbiter) ? NDCR_ND_ARB_EN : 0; - ndcr |= (host->col_addr_cycles == 2) ? NDCR_RA_START : 0; - ndcr |= (f->page_per_block == 64) ? NDCR_PG_PER_BLK : 0; - ndcr |= (f->page_size == 2048) ? NDCR_PAGE_SZ : 0; - ndcr |= (f->flash_width == 16) ? NDCR_DWIDTH_M : 0; - ndcr |= (f->dfc_width == 16) ? NDCR_DWIDTH_C : 0; - - ndcr |= NDCR_RD_ID_CNT(host->read_id_bytes); - ndcr |= NDCR_SPARE_EN; /* enable spare by default */ + struct mtd_info *mtd = host->mtd; + struct nand_chip *chip = mtd->priv; - info->reg_ndcr = ndcr; + info->reg_ndcr |= (host->col_addr_cycles == 2) ? NDCR_RA_START : 0; + info->reg_ndcr |= (chip->page_shift == 6) ? NDCR_PG_PER_BLK : 0; + info->reg_ndcr |= (mtd->writesize == 2048) ? NDCR_PAGE_SZ : 0; - pxa3xx_nand_set_timing(host, f->timing); return 0; } @@ -1456,19 +1424,31 @@ static void pxa3xx_nand_free_buff(struct pxa3xx_nand_info *info) } #endif -static int pxa3xx_nand_sensing(struct pxa3xx_nand_info *info) +static int pxa3xx_nand_sensing(struct pxa3xx_nand_host *host) { + struct pxa3xx_nand_info *info = host->info_data; + struct platform_device *pdev = info->pdev; + struct pxa3xx_nand_platform_data *pdata = dev_get_platdata(>dev); struct mtd_info *mtd; struct nand_chip *chip; + const struct nand_sdr_timings *timings; int ret; mtd = info->host[info->cs]->mtd; chip = mtd->priv; + /* configure default flash values */ + info->reg_ndcr = 0x0; /* enable all interrupts */ + info->reg_ndcr |= (pdata->enable_arbiter) ? NDCR_ND_ARB_EN : 0; + info->reg_ndcr |= NDCR_RD_ID_CNT(host->read_id_bytes); + info->reg_ndcr |= NDCR_SPARE_EN; /* enable spare by default */ + /* use the common timing to make a try */ - ret = pxa3xx_nand_config_flash(info, _flash_types[0]); - if (ret) - return ret; + timings = onfi_async_timing_mode_to_sdr_timings(0); + if (IS_ERR(timings)) + return PTR_ERR(timings); + + pxa3xx_nand_set_sdr_timing(host, timings); chip->cmdfunc(mtd, NAND_CMD_RESET, 0, 0); ret = chip->waitfunc(mtd, chip); @@ -1553,12 +1533,8 @@ static int pxa3xx_nand_scan(struct mtd_info *mtd) struct pxa3xx_nand_info *info = host->info_data; struct platform_device *pdev = info->pdev; struct pxa3xx_nand_platform_data *pdata = dev_get_platdata(>dev); - struct nand_flash_dev pxa3xx_flash_ids[2], *def = NULL; - const struct pxa3xx_nand_flash *f = NULL; struct nand_chip *chip = mtd->priv; - uint32_t id = -1; - uint64_t chipsize; - int i, ret, num; + int ret; uint16_t ecc_strength, ecc_step; if (pdata->keep_config && !pxa3xx_nand_detect_config(info)) @@ -1567,7 +1543,7 @@ static int pxa3xx_nand_scan(struct mtd_info *mtd) /* Set a default chunk size */ info->chunk_size = 512; - ret = pxa3xx_nand_sensing(info); + ret =
Re: [PATCH v2 2/4] mtd: pxa3xx_nand: add helpers to setup the timings
On 07/07/2015 12:08 PM, Antoine Tenart wrote: Add helpers to setup the timings in the pxa3xx driver. These helpers allow to either make use of the nand framework nand_sdr_timings or the pxa3xx specific pxa3xx_nand_host, for compatibility reasons. Signed-off-by: Antoine Tenart --- drivers/mtd/nand/pxa3xx_nand.c | 91 ++ 1 file changed, 91 insertions(+) diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c index 5465fa439c9e..9a95c24ab2ce 100644 --- a/drivers/mtd/nand/pxa3xx_nand.c +++ b/drivers/mtd/nand/pxa3xx_nand.c @@ -385,6 +385,97 @@ static void pxa3xx_nand_set_timing(struct pxa3xx_nand_host *host, nand_writel(info, NDTR1CS0, ndtr1); } +static void pxa3xx_nand_set_sdr_timing(struct pxa3xx_nand_host *host, + const struct nand_sdr_timings *t) +{ + struct pxa3xx_nand_info *info = host->info_data; + struct nand_chip *chip = >chip; + unsigned long nand_clk = clk_get_rate(info->clk); + uint32_t ndtr0, ndtr1; + + u32 tCH_min = DIV_ROUND_UP(t->tCH_min, 1000); + u32 tCS_min = DIV_ROUND_UP(t->tCS_min, 1000); + u32 tWH_min = DIV_ROUND_UP(t->tWH_min, 1000); + u32 tWP_min = DIV_ROUND_UP(t->tWC_min - tWH_min, 1000); Is the substraction above correct? You seem to be substracting picoseconds to nanoseconds. + u32 tREH_min = DIV_ROUND_UP(t->tREH_min, 1000); + u32 tRP_min = DIV_ROUND_UP(t->tRC_min - tREH_min, 1000); Ditto. + u32 tR = chip->chip_delay * 1000; + u32 tWHR_min = DIV_ROUND_UP(t->tWHR_min, 1000); + u32 tAR_min = DIV_ROUND_UP(t->tAR_min, 1000); + + /* fallback to a default value if tR = 0 */ + if (!tR) + tR = 2; + + ndtr0 = NDTR0_tCH(ns2cycle(tCH_min, nand_clk)) | + NDTR0_tCS(ns2cycle(tCS_min, nand_clk)) | + NDTR0_tWH(ns2cycle(tWH_min, nand_clk)) | + NDTR0_tWP(ns2cycle(tWP_min, nand_clk)) | + NDTR0_tRH(ns2cycle(tREH_min, nand_clk)) | + NDTR0_tRP(ns2cycle(tRP_min, nand_clk)); + + ndtr1 = NDTR1_tR(ns2cycle(tR, nand_clk)) | + NDTR1_tWHR(ns2cycle(tWHR_min, nand_clk)) | + NDTR1_tAR(ns2cycle(tAR_min, nand_clk)); + + info->ndtr0cs0 = ndtr0; + info->ndtr1cs0 = ndtr1; + nand_writel(info, NDTR0CS0, ndtr0); + nand_writel(info, NDTR1CS0, ndtr1); +} + +static int pxa3xx_nand_init_timings(struct pxa3xx_nand_host *host) +{ + const struct nand_sdr_timings *timings; + struct nand_chip *chip = >chip; + struct pxa3xx_nand_info *info = host->info_data; + const struct pxa3xx_nand_flash *f = NULL; + int mode, id, ntypes, i; + + mode = onfi_get_async_timing_mode(chip); + if (mode == ONFI_TIMING_MODE_UNKNOWN) { + ntypes = ARRAY_SIZE(builtin_flash_types); + + chip->cmdfunc(host->mtd, NAND_CMD_READID, 0x00, -1); + + id = chip->read_byte(host->mtd); + id |= chip->read_byte(host->mtd) << 0x8; + + for (i = 0; i < ntypes; i++) { + f = _flash_types[i]; + + if (f->chip_id == id) + break; + } + + if (i == ntypes) { + dev_err(>pdev->dev, "Error: timings not found\n"); + return -EINVAL; + } + + pxa3xx_nand_set_timing(host, f->timing); + + if (f->flash_width == 16) { + info->reg_ndcr |= NDCR_DWIDTH_M; + chip->options |= NAND_BUSWIDTH_16; + } + + info->reg_ndcr |= (f->dfc_width == 16) ? NDCR_DWIDTH_C : 0; Nitpick: the 16-bit flash configuration doesn't belong in a function called "xxx_init_timings". + } else { + mode = fls(mode) - 1; + if (mode < 0) + mode = 0; + + timings = onfi_async_timing_mode_to_sdr_timings(mode); + if (IS_ERR(timings)) + return PTR_ERR(timings); + + pxa3xx_nand_set_sdr_timing(host, timings); + } + + return 0; +} + /* * Set the data and OOB size, depending on the selected * spare and ECC configuration. Thanks, -- Ezequiel Garcia, VanguardiaSur www.vanguardiasur.com.ar -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Use after free bug in null_blk driver
2015-07-19 2:19 GMT+09:00 Mike Krinkin : > On Sun, Jul 19, 2015 at 01:18:44AM +0900, Akinobu Mita wrote: >> 2015-07-18 23:51 GMT+09:00 Mike Krinkin : >> > Hi, >> > >> > i noticed that loading null_blk with queue_mode=1 and irqmode=2 parameters >> > and slab poisoning enabled causes general protection fault: >> > >> > [ 20.671974] general protection fault: [#1] SMP >> > [ 20.678050] Modules linked in: null_blk(+) usbhid hid psmouse floppy >> > [ 20.688351] CPU: 0 PID: 147 Comm: modprobe Not tainted 4.2.0-rc2+ #76 >> > [ 20.695961] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS >> > Bochs 01/01/2011 >> > [ 20.705143] task: 88007f2ed100 ti: 88007f0a4000 task.ti: >> > 88007f0a4000 >> > [ 20.715511] RIP: 0010:[] [] >> > null_cmd_timer_expired+0x62/0xc0 [null_blk] >> > [ 20.730541] RSP: 0018:88007d203e78 EFLAGS: 00010082 >> > [ 20.737301] RAX: 88007ec16b80 RBX: 6b6b6b6b6b6b6b6b RCX: >> > >> > [ 20.747034] RDX: 88007f260888 RSI: RDI: >> > 88007f260850 >> > [ 20.762601] RBP: 88007d203e98 R08: 0001 R09: >> > >> > [ 20.771472] R10: 0001 R11: R12: >> > >> > [ 20.782522] R13: 88007d218780 R14: R15: >> > 88007d20fba8 >> > [ 20.792347] FS: 7f02e1500740() GS:88007d20() >> > knlGS: >> > [ 20.805319] CS: 0010 DS: ES: CR0: 8005003b >> > [ 20.813120] CR2: 7ffd52478dd8 CR3: 7f3dd000 CR4: >> > 07f0 >> > [ 20.825357] Stack: >> > [ 20.827764] 88007d218788 88007d20fa00 88007d20fac0 >> > a00280f0 >> > [ 20.838269] 88007d203f08 810f822b 00045dd35ed6 >> > 88007d20fa18 >> > [ 20.853377] 0001810f89e7 88007d20fa00 88007d20fb28 >> > 00045dd35ed6 >> > [ 20.863415] Call Trace: >> > [ 20.866398] >> > [ 20.869088] [] ? null_softirq_done_fn+0x30/0x30 >> > [null_blk] >> > [ 20.880219] [] __hrtimer_run_queues+0x11b/0x6e0 >> > [ 20.891048] [] hrtimer_interrupt+0xab/0x1b0 >> > [ 20.899542] [] local_apic_timer_interrupt+0x3c/0x70 >> > [ 20.911730] [] smp_apic_timer_interrupt+0x41/0x60 >> > [ 20.921854] [] apic_timer_interrupt+0x70/0x80 >> > [ 20.931084] >> > [ 20.933406] [] ? >> > _raw_spin_unlock_irqrestore+0x3b/0x60 >> > [ 20.945250] [] hrtimer_start_range_ns+0x1a7/0x540 >> > [ 20.955952] [] ? trace_hardirqs_on_caller+0x151/0x1e0 >> > [ 20.963974] [] null_request_fn+0xd3/0x100 [null_blk] >> > [ 20.977159] [] __blk_run_queue+0x37/0x50 >> > [ 20.983782] [] __elv_add_request+0x15d/0x440 >> > [ 20.991540] [] blk_queue_bio+0x415/0x520 >> > [ 20.998876] [] generic_make_request+0xcc/0x110 >> > [ 21.007095] [] submit_bio+0x67/0x150 >> > [ 21.013603] [] submit_bh_wbc+0x14c/0x180 >> > [ 21.019730] [] block_read_full_page+0x2b0/0x3a0 >> > [ 21.028023] [] ? I_BDEV+0x20/0x20 >> > [ 21.034575] [] ? blkdev_readpages+0x20/0x20 >> > [ 21.040947] [] blkdev_readpage+0x18/0x20 >> > [ 21.048237] [] do_read_cache_page+0x7d/0x1a0 >> > [ 21.056016] [] read_cache_page+0x1c/0x20 >> > [ 21.063300] [] read_dev_sector+0x30/0x90 >> > [ 21.069804] [] ? check_partition+0x1f0/0x1f0 >> > [ 21.076932] [] adfspart_check_ICS+0x42/0x250 >> > [ 21.084317] [] ? snprintf+0x34/0x40 >> > [ 21.090670] [] ? check_partition+0x1f0/0x1f0 >> > [ 21.097864] [] check_partition+0x100/0x1f0 >> > [ 21.104535] [] rescan_partitions+0xa8/0x2b0 >> > [ 21.112087] [] __blkdev_get+0x313/0x460 >> > [ 21.120037] [] blkdev_get+0x41/0x3a0 >> > [ 21.126266] [] ? unlock_new_inode+0x5d/0x90 >> > [ 21.134043] [] ? bdget+0x130/0x150 >> > [ 21.140369] [] ? disk_get_part+0xd/0x1d0 >> > [ 21.147357] [] add_disk+0x436/0x4d0 >> > [ 21.153846] [] ? sprintf+0x40/0x50 >> > [ 21.159867] [] null_init+0x38e/0x1000 [null_blk] >> > [ 21.167691] [] ? 0xa004e000 >> > [ 21.174266] [] do_one_initcall+0xb3/0x1e0 >> > [ 21.182276] [] ? kmem_cache_alloc_trace+0x36f/0x3b0 >> > [ 21.190291] [] do_init_module+0x61/0x1ec >> > [ 21.197112] [] load_module+0x24fe/0x2810 >> > [ 21.204237] [] ? m_show+0x1a0/0x1a0 >> > [ 21.213498] [] SyS_finit_module+0x80/0xb0 >> > [ 21.221360] [] entry_SYSCALL_64_fastpath+0x16/0x7a >> > [ 21.230181] Code: e8 24 40 3b e1 48 89 c3 eb 08 4d 85 e4 4c 89 e3 74 e2 >> > 48 8d 7b f0 4c 8b 23 e8 db fe ff ff 48 8b 43 28 48 85 c0 74 e3 48 8b 58 30 >> > <48> 83 bb 38 01 00 00 00 75 d5 48 8b 83 98 08 00 00 a8 04 74 ca >> > [ 21.281572] RIP [] null_cmd_timer_expired+0x62/0xc0 >> > [null_blk] >> > [ 21.289770] RSP >> > [ 21.294601] ---[ end trace cd40ca6c9001dd7e ]--- >> > [ 21.299719] Kernel panic - not syncing: Fatal exception in interrupt >> > [ 21.309176] Kernel Offset: disabled >> > [ 21.312773] ---[ end Kernel panic - not syncing: Fatal exception in >> > interrupt >> > >> >
Re: [PATCH 04/16] MIPS: use struct mips_abi offsets to save FP context
On Sat, Jul 18, 2015 at 08:21:53PM -0400, Paul Gortmaker wrote: > Date: Sat, 18 Jul 2015 20:21:53 -0400 > From: Paul Gortmaker > To: Paul Burton > Cc: "linux-m...@linux-mips.org" , Matthew > Fortune , Leonid Yegoshin > , Michael Ellerman , LKML > , Richard Weinberger , James > Hogan , Andy Lutomirski , > Markos Chandras , Ralf Baechle > , Manuel Lauss , "Maciej W. > Rozycki" , "linux-n...@vger.kernel.org" > > Subject: Re: [PATCH 04/16] MIPS: use struct mips_abi offsets to save FP > context > Content-Type: text/plain; charset=UTF-8 > > On Fri, Jul 10, 2015 at 11:00 AM, Paul Burton wrote: > > When saving FP state to struct sigcontext, make use of the offsets > > provided by struct mips_abi to obtain appropriate addresses for the > > sc_fpregs & sc_fpc_csr fields of the sigcontext. This is done only for > > the native struct sigcontext in this patch (ie. for O32 in CONFIG_32BIT > > kernels or for N64 in CONFIG_64BIT kernels) but is done in preparation > > for sharing this code with compat ABIs in further patches. > > > > Signed-off-by: Paul Burton > > --- > > > > arch/mips/kernel/r4k_fpu.S | 151 > > +-- > > arch/mips/kernel/signal-common.h | 6 ++ > > arch/mips/kernel/signal.c| 85 +++--- > > 3 files changed, 145 insertions(+), 97 deletions(-) > > > > The current version of this in linux-next picked up a booger in transit. > > $ git show 6775b4ea74d5922e5310b7b7a902a8fbe61a0c9d|diffstat > arch/mips/kernel/r4k_fpu.S | 151 > --- > arch/mips/kernel/signal-common.h |6 + > arch/mips/kernel/signal.c| 85 ++--- > index.html | 16 > 4 files changed, 161 insertions(+), 97 deletions(-) > > Guessing it happened at Ralf's end. Yes and I fixed a few hours ago. Should be ok in the next -next. Ralf -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 04/16] MIPS: use struct mips_abi offsets to save FP context
On Fri, Jul 10, 2015 at 11:00 AM, Paul Burton wrote: > When saving FP state to struct sigcontext, make use of the offsets > provided by struct mips_abi to obtain appropriate addresses for the > sc_fpregs & sc_fpc_csr fields of the sigcontext. This is done only for > the native struct sigcontext in this patch (ie. for O32 in CONFIG_32BIT > kernels or for N64 in CONFIG_64BIT kernels) but is done in preparation > for sharing this code with compat ABIs in further patches. > > Signed-off-by: Paul Burton > --- > > arch/mips/kernel/r4k_fpu.S | 151 > +-- > arch/mips/kernel/signal-common.h | 6 ++ > arch/mips/kernel/signal.c| 85 +++--- > 3 files changed, 145 insertions(+), 97 deletions(-) > The current version of this in linux-next picked up a booger in transit. $ git show 6775b4ea74d5922e5310b7b7a902a8fbe61a0c9d|diffstat arch/mips/kernel/r4k_fpu.S | 151 --- arch/mips/kernel/signal-common.h |6 + arch/mips/kernel/signal.c| 85 ++--- index.html | 16 4 files changed, 161 insertions(+), 97 deletions(-) Guessing it happened at Ralf's end. Paul. -- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[lkp] [netfilter] 579502f1088: WARNING: CPU: 0 PID: 1 at net/netfilter/nf_conntrack_extend.c:80 __nf_ct_ext_add_length+0x2a/0x230()
FYI, we noticed the below changes on git://internal_merge_and_test_tree revert-579502f10880fdb7ad296ff8e858d48518dc01a5-579502f10880fdb7ad296ff8e858d48518dc01a5 commit 579502f10880fdb7ad296ff8e858d48518dc01a5 ("netfilter: fix netns dependencies with conntrack templates") +--+++ | | 484836ec2d | 579502f108 | +--+++ | boot_successes | 11 | 0 | | boot_failures| 20 | 11 | | IP-Config:Auto-configuration_of_network_failed | 20 || | WARNING:at_net/netfilter/nf_conntrack_extend.c:#__nf_ct_ext_add_length() | 0 | 11 | | backtrace:__nf_ct_ext_add_length | 0 | 11 | | backtrace:synproxy_net_init | 0 | 11 | | backtrace:ops_init | 0 | 11 | | backtrace:register_pernet_subsys | 0 | 11 | | backtrace:synproxy_core_init | 0 | 11 | | backtrace:kernel_init_freeable | 0 | 11 | +--+++ [ 12.601369] ctnetlink v0.93: registering with nfnetlink. [ 12.602995] [ cut here ] [ 12.602995] [ cut here ] [ 12.604390] WARNING: CPU: 0 PID: 1 at net/netfilter/nf_conntrack_extend.c:80 __nf_ct_ext_add_length+0x2a/0x230() [ 12.604390] WARNING: CPU: 0 PID: 1 at net/netfilter/nf_conntrack_extend.c:80 __nf_ct_ext_add_length+0x2a/0x230() [ 12.607905] CPU: 0 PID: 1 Comm: swapper Not tainted 4.1.0-12250-g579502f #1 [ 12.607905] CPU: 0 PID: 1 Comm: swapper Not tainted 4.1.0-12250-g579502f #1 [ 12.609936] [ 12.609936] 4002be7c 4002be7c 41968a51 41968a51 4002be98 4002be98 4103046d 4103046d 0050 0050 417d7592 417d7592 [ 12.620695] 55ff3d30 [ 12.620695] 55ff3d30 41fe3480 41fe3480 4002bea8 4002bea8 4103050a 4103050a 0009 0009 4002bed0 4002bed0 [ 12.623069] 417d7592 [ 12.623069] 417d7592 41fe3480 41fe3480 4002bec4 4002bec4 410b31fe 410b31fe 0002 0002 55ff3d30 55ff3d30 55ff3d30 55ff3d30 55ff4ed8 55ff4ed8 [ 12.625838] Call Trace: [ 12.625838] Call Trace: [ 12.626537] [<41968a51>] dump_stack+0x16/0x18 [ 12.626537] [<41968a51>] dump_stack+0x16/0x18 [ 12.638016] [<4103046d>] warn_slowpath_common+0x74/0x8b [ 12.638016] [<4103046d>] warn_slowpath_common+0x74/0x8b [ 12.639567] [<417d7592>] ? __nf_ct_ext_add_length+0x2a/0x230 [ 12.639567] [<417d7592>] ? __nf_ct_ext_add_length+0x2a/0x230 [ 12.641371] [<4103050a>] warn_slowpath_null+0xf/0x13 [ 12.641371] [<4103050a>] warn_slowpath_null+0xf/0x13 [ 12.643232] [<417d7592>] __nf_ct_ext_add_length+0x2a/0x230 [ 12.643232] [<417d7592>] __nf_ct_ext_add_length+0x2a/0x230 [ 12.654502] [<410b31fe>] ? __kmalloc+0x36/0x66 [ 12.654502] [<410b31fe>] ? __kmalloc+0x36/0x66 [ 12.655797] [<4203d809>] synproxy_net_init+0x7e/0xf8 [ 12.655797] [<4203d809>] synproxy_net_init+0x7e/0xf8 [ 12.657306] [<4178fe55>] ops_init+0xbf/0xe5 [ 12.657306] [<4178fe55>] ops_init+0xbf/0xe5 [ 12.658872] [<4178fecd>] register_pernet_operations+0x52/0x7a [ 12.658872] [<4178fecd>] register_pernet_operations+0x52/0x7a [ 12.660833] [<4203d883>] ? synproxy_net_init+0xf8/0xf8 [ 12.660833] [<4203d883>] ? synproxy_net_init+0xf8/0xf8 [ 12.662370] [<41790345>] register_pernet_subsys+0x19/0x2a [ 12.662370] [<41790345>] register_pernet_subsys+0x19/0x2a [ 12.672803] [<4203d89f>] synproxy_core_init+0x1c/0x33 [ 12.672803] [<4203d89f>] synproxy_core_init+0x1c/0x33 [ 12.674341] [<42006b2c>] do_one_initcall+0xce/0x147 [ 12.674341] [<42006b2c>] do_one_initcall+0xce/0x147 [ 12.675749] [<420064b7>] ? repair_env_string+0x12/0x54 [ 12.675749] [<420064b7>] ? repair_env_string+0x12/0x54 [ 12.680004] [<410424d8>] ? parse_args+0x1b2/0x293 [ 12.680004] [<410424d8>] ? parse_args+0x1b2/0x293 [ 12.681404] [<42006c5e>] ? kernel_init_freeable+0xb9/0x156 [ 12.681404] [<42006c5e>] ? kernel_init_freeable+0xb9/0x156 [ 12.687240] [<42006c7e>] kernel_init_freeable+0xd9/0x156 [ 12.687240] [<42006c7e>] kernel_init_freeable+0xd9/0x156 [ 12.688805] [<41964610>] kernel_init+0x8/0xb5 [ 12.688805] [<41964610>] kernel_init+0x8/0xb5 [ 12.693641] [<41972040>]
Re: [PATCH] macintosh/ans-lcd: fix build failure after module_init/exit relocation
[[PATCH] macintosh/ans-lcd: fix build failure after module_init/exit relocation] On 17/07/2015 (Fri 14:20) Luis Henriques wrote: > After commit 0fd972a7d91d ("module: relocate module_init from init.h to > module.h") > ans-lcd module fails to build with: > > drivers/macintosh/ans-lcd.c:201:1: warning: data definition has no type or > storage class [enabled by default] > module_init(anslcd_init); > ^ > drivers/macintosh/ans-lcd.c:201:1: error: type defaults to 'int' in > declaration of 'module_init' [-Werror=implicit-int] > drivers/macintosh/ans-lcd.c:201:1: warning: parameter names (without types) > in function declaration [enabled by default] > drivers/macintosh/ans-lcd.c:202:1: warning: data definition has no type or > storage class [enabled by default] > module_exit(anslcd_exit); > ^ > drivers/macintosh/ans-lcd.c:202:1: error: type defaults to 'int' in > declaration of 'module_exit' [-Werror=implicit-int] > drivers/macintosh/ans-lcd.c:202:1: warning: parameter names (without types) > in function declaration [enabled by default] > drivers/macintosh/ans-lcd.c:155:1: warning: 'anslcd_init' defined but not > used [-Wunused-function] > anslcd_init(void) > ^ > drivers/macintosh/ans-lcd.c:195:1: warning: 'anslcd_exit' defined but not > used [-Wunused-function] > anslcd_exit(void) > ^ > > This commit fixes it by replacing linux/init.h by linux/module.h. Guess it was inevitable I miss at least one. This old beast must not get coverage under any of the ppc defconfigs or allmodconfig, since I did both... are you using a custom config for an older box? Anyway, the thing is controlled by a tristate, so this is the right fix. I don't have _any_ other fixes queued for that cleanup series, so I'm fine with this going via the ppc tree, since I'm sure there will be pending commits there. Acked-by: Paul Gortmaker Thanks, Paul. -- > > Fixes: 0fd972a7d91d ("module: relocate module_init from init.h to module.h") > Signed-off-by: Luis Henriques > --- > drivers/macintosh/ans-lcd.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/macintosh/ans-lcd.c b/drivers/macintosh/ans-lcd.c > index 1a57e88a38f7..cd35079c8c98 100644 > --- a/drivers/macintosh/ans-lcd.c > +++ b/drivers/macintosh/ans-lcd.c > @@ -7,7 +7,7 @@ > #include > #include > #include > -#include > +#include > #include > #include > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Build error due to "x86/fpu, sched: Dynamically allocate 'struct fpu'"
On Sat, Jul 18, 2015 at 04:27:17PM -0700, Guenter wrote: > Hi, > > Commit 0c8c0f03e3a2 ("x86/fpu, sched: Dynamically allocate 'struct fpu'") > causes s390 builds in mainline to fail as follows. > > arch/s390/kernel/traps.c: Assembler messages: > arch/s390/kernel/traps.c:262: Error: operand out of range > (0x23e8 is not between 0x and > 0x0fff) > arch/s390/kernel/traps.c:300: Error: operand out of range > (0x23e8 is not between 0x and > 0x0fff) > Also: arm64:allmodconfig: arch/arm64/kernel/entry.S: Assembler messages: arch/arm64/kernel/entry.S:588: Error: immediate out of range arch/arm64/kernel/entry.S:597: Error: immediate out of range make[1]: *** [arch/arm64/kernel/entry.o] Error 1 I didn't bisect that one, but it looks like the cause is the same. Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Build error due to "x86/fpu, sched: Dynamically allocate 'struct fpu'"
Hi, Commit 0c8c0f03e3a2 ("x86/fpu, sched: Dynamically allocate 'struct fpu'") causes s390 builds in mainline to fail as follows. arch/s390/kernel/traps.c: Assembler messages: arch/s390/kernel/traps.c:262: Error: operand out of range (0x23e8 is not between 0x and 0x0fff) arch/s390/kernel/traps.c:300: Error: operand out of range (0x23e8 is not between 0x and 0x0fff) Bisect log is as follows. # bad: [9d37e6679dfddbb5fa605fb2d7ff448f7cd6d038] Merge branch 'fixes' of git://ftp.arm.linux.org.uk/~rmk/linux-arm # good: [bc0195aad0daa2ad5b0d76cce22b167bc3435590] Linux 4.2-rc2 git bisect start 'HEAD' 'v4.2-rc2' # good: [a27507ca2d796cfa8d907de31ad730359c8a6d06] x86/nmi/64: Reorder nested NMI checks git bisect good a27507ca2d796cfa8d907de31ad730359c8a6d06 # good: [eb254374a30cc53f976f2302f2198813a3b687ea] Merge tag 'staging-4.2-rc3' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging git bisect good eb254374a30cc53f976f2302f2198813a3b687ea # good: [3a26a5b1513cddfc018c8e264979bc6e28f8ec1f] Merge branch 'akpm' (patches from Andrew) git bisect good 3a26a5b1513cddfc018c8e264979bc6e28f8ec1f # good: [f79a17bf268cc043eecffb65033b2e58fc037eef] Merge branch 'perf-urgent-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip git bisect good f79a17bf268cc043eecffb65033b2e58fc037eef # bad: [5aaeb5c01c5b6c0be7b7aadbf3ace9f3a4458c3d] x86/fpu, sched: Introduce CONFIG_ARCH_WANTS_DYNAMIC_TASK_STRUCT and use it on x86 git bisect bad 5aaeb5c01c5b6c0be7b7aadbf3ace9f3a4458c3d # good: [36f1a77b3aa57c5c2eb1ae2d67d07c4350a78345] x86/nmi/64: Make the "NMI executing" variable more consistent git bisect good 36f1a77b3aa57c5c2eb1ae2d67d07c4350a78345 # bad: [0c8c0f03e3a292e031596484275c14cf39c0ab7a] x86/fpu, sched: Dynamically allocate 'struct fpu' git bisect bad 0c8c0f03e3a292e031596484275c14cf39c0ab7a # good: [a97439aa1aec10387797b4abae3cf117de1c90d7] x86/entry/64, x86/nmi/64: Add CONFIG_DEBUG_ENTRY NMI testing code git bisect good a97439aa1aec10387797b4abae3cf117de1c90d7 # first bad commit: [0c8c0f03e3a292e031596484275c14cf39c0ab7a] x86/fpu, sched: Dynamically allocate 'struct fpu' Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 4/4] media: pxa_camera: conversion to dmaengine
Robert Jarzmik writes: > Guennadi Liakhovetski writes: > >>> /* init DMA for Y channel */ >> >> How about taking the loop over the sg list out of pxa_init_dma_channel() >> to avoid having to iterate it from the beginning each time? Then you would >> be able to split it into channels inside that global loop? Would that >> work? Of course you might need to rearrange functions to avoid too deep >> code nesting. > > Ok, will try that. > The more I think of it, the more it looks to me like a generic thing : take an > sglist, and an array of sizes, and split the sglist into several sglists, each > of the defined size in the array. > > Or more code-like speaking : > - sglist_split(struct scatterlist *sg_int, size_t *sizes, int nb_sizes, > struct scatterlist **sg_out) > - and sg_out is an array of nb_sizes (struct scatterlist *sg) > > So I will try that out. Maybe if that works out for pxa_camera, Jens or > Russell > would accept that into lib/scatterlist.c. Ok, I made the code ... and I hate it. It's in [1], which is an incremental patch over patch 4/4. If that's what you had in mind, tell me. Cheers. -- Robert [1] The despised patch ---<8--- commit 43bbb9a4e3ac Author: Robert Jarzmik Date: Tue Jul 14 20:17:51 2015 +0200 tmp: pxa_camera: working on sg_split Signed-off-by: Robert Jarzmik diff --git a/drivers/media/platform/soc_camera/pxa_camera.c b/drivers/media/platform/soc_camera/pxa_camera.c index 26a66b9ff570..83efd284e976 100644 --- a/drivers/media/platform/soc_camera/pxa_camera.c +++ b/drivers/media/platform/soc_camera/pxa_camera.c @@ -287,64 +287,110 @@ static void free_buffer(struct videobuf_queue *vq, struct pxa_buffer *buf) >vb, buf->vb.baddr, buf->vb.bsize); } -static struct scatterlist *videobuf_sg_cut(struct scatterlist *sglist, - int sglen, int offset, int size, - int *new_sg_len) + +struct sg_splitter { + struct scatterlist *in_sg0; + int nents; + off_t skip_sg0; + size_t len_last_sg; + struct scatterlist *out_sg; +}; + +static struct sg_splitter * +sg_calculate_split(struct scatterlist *in, off_t skip, + const size_t *sizes, int nb_splits, gfp_t gfp_mask) { - struct scatterlist *sg0, *sg, *sg_first = NULL; - int i, dma_len, dropped_xfer_len, dropped_remain, remain; - int nfirst = -1, nfirst_offset = 0, xfer_len; - - *new_sg_len = 0; - dropped_remain = offset; - remain = size; - for_each_sg(sglist, sg, sglen, i) { - dma_len = sg_dma_len(sg); - /* PXA27x Developer's Manual 27.4.4.1: round up to 8 bytes */ - dropped_xfer_len = roundup(min(dma_len, dropped_remain), 8); - if (dropped_remain) - dropped_remain -= dropped_xfer_len; - xfer_len = dma_len - dropped_xfer_len; - - if (nfirst < 0 && xfer_len > 0) { - sg_first = sg; - nfirst = i; - nfirst_offset = dropped_xfer_len; + int i, nents; + size_t size, len; + struct sg_splitter *splitters, *curr; + struct scatterlist *sg; + + splitters = kcalloc(nb_splits, sizeof(*splitters), gfp_mask); + if (!splitters) + return NULL; + + nents = 0; + size = *sizes; + curr = splitters; + for_each_sg(in, sg, sg_nents(in), i) { + if (skip > sg_dma_len(sg)) { + skip -= sg_dma_len(sg); + continue; + } + len = min_t(size_t, size, sg_dma_len(sg) - skip); + if (!curr->in_sg0) { + curr->in_sg0 = sg; + curr->skip_sg0 = sg_dma_len(sg) - len; } - if (xfer_len > 0) { - (*new_sg_len)++; - remain -= xfer_len; + size -= len; + nents++; + if (!size) { + curr->nents = nents; + curr->len_last_sg = len; + nents = 0; + size = *(++sizes); + + if (!--nb_splits) + break; + + if (len < curr->len_last_sg) { + (splitters + 1)->in_sg0 = sg; + (splitters + 1)->skip_sg0 = 0; + } + curr++; } - if (remain <= 0) - break; } - WARN_ON(nfirst >= sglen); - sg0 = kmalloc_array(*new_sg_len, sizeof(struct scatterlist), - GFP_KERNEL); - if (!sg0) - return NULL; + return splitters; +} - remain = size; - for_each_sg(sg_first, sg, *new_sg_len, i) { -
[PATCH] iio: Documentation: Remove bytes_per_datum attribute
Remove sysfs bytes_per_datum device attribute ABI documentation since the attribute is not present anymore. Signed-off-by: Cristina Opriceana --- Documentation/ABI/testing/sysfs-bus-iio | 7 --- 1 file changed, 7 deletions(-) diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio index 666a341..400d234 100644 --- a/Documentation/ABI/testing/sysfs-bus-iio +++ b/Documentation/ABI/testing/sysfs-bus-iio @@ -1040,13 +1040,6 @@ Contact: linux-...@vger.kernel.org Description: Number of scans contained by the buffer. -What: /sys/bus/iio/devices/iio:deviceX/buffer/bytes_per_datum -KernelVersion: 2.6.37 -Contact: linux-...@vger.kernel.org -Description: - Bytes per scan. Due to alignment fun, the scan may be larger - than implied directly by the scan_element parameters. - What: /sys/bus/iio/devices/iio:deviceX/buffer/enable KernelVersion: 2.6.35 Contact: linux-...@vger.kernel.org -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] hwmon: (lm70) add device tree support
Allow the lm70 to be probed from a device tree. Signed-off-by: Rabin Vincent --- Documentation/devicetree/bindings/hwmon/lm70.txt | 21 +++ drivers/hwmon/lm70.c | 34 +++- 2 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/hwmon/lm70.txt diff --git a/Documentation/devicetree/bindings/hwmon/lm70.txt b/Documentation/devicetree/bindings/hwmon/lm70.txt new file mode 100644 index 000..e7fd921 --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/lm70.txt @@ -0,0 +1,21 @@ +* LM70/TMP121/LM71/LM74 thermometer. + +Required properties: +- compatible: one of + "ti,lm70" + "ti,tmp121" + "ti,lm71" + "ti,lm74" + +See Documentation/devicetree/bindings/spi/spi-bus.txt for more required and +optional properties. + +Example: + +spi_master { + temperature-sensor@0 { + compatible = "ti,lm70"; + reg = <0>; + spi-max-frequency = <100>; + }; +}; diff --git a/drivers/hwmon/lm70.c b/drivers/hwmon/lm70.c index 97204dc..9296e9d 100644 --- a/drivers/hwmon/lm70.c +++ b/drivers/hwmon/lm70.c @@ -37,6 +37,7 @@ #include #include #include +#include #define DRVNAME"lm70" @@ -130,11 +131,41 @@ ATTRIBUTE_GROUPS(lm70); /*--*/ +#ifdef CONFIG_OF +static const struct of_device_id lm70_of_ids[] = { + { + .compatible = "ti,lm70", + .data = (void *) LM70_CHIP_LM70, + }, + { + .compatible = "ti,tmp121", + .data = (void *) LM70_CHIP_TMP121, + }, + { + .compatible = "ti,lm71", + .data = (void *) LM70_CHIP_LM71, + }, + { + .compatible = "ti,lm74", + .data = (void *) LM70_CHIP_LM74, + }, + {}, +}; +MODULE_DEVICE_TABLE(of, lm70_of_ids); +#endif + static int lm70_probe(struct spi_device *spi) { - int chip = spi_get_device_id(spi)->driver_data; + const struct of_device_id *match; struct device *hwmon_dev; struct lm70 *p_lm70; + int chip; + + match = of_match_device(lm70_of_ids, >dev); + if (match) + chip = (int)(uintptr_t)match->data; + else + chip = spi_get_device_id(spi)->driver_data; /* signaling is SPI_MODE_0 */ if (spi->mode & (SPI_CPOL | SPI_CPHA)) @@ -169,6 +200,7 @@ static struct spi_driver lm70_driver = { .driver = { .name = "lm70", .owner = THIS_MODULE, + .of_match_table = of_match_ptr(lm70_of_ids), }, .id_table = lm70_ids, .probe = lm70_probe, -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] clocksource: atmel-st: Remove irq handler when clock event is unused
On 18/07/2015 at 10:12:03 +0200, Thomas Gleixner wrote : > On Fri, 17 Jul 2015, Alexandre Belloni wrote: > > +static int atmel_st_request_irq(struct clock_event_device *dev) > > +{ > > + int ret; > > + > > + if (clockevent_state_periodic(dev) || clockevent_state_oneshot(dev)) > > + return 0; > > + > > + ret = request_irq(irq, at91rm9200_timer_interrupt, > > + IRQF_SHARED | IRQF_TIMER | IRQF_IRQPOLL, > > + "at91_tick", regmap_st); > > + if (ret) { > > + pr_alert("Unable to setup IRQ\n"); > > + return ret; > > + } > > + > > return 0; > > } > > > > static int clkevt32k_set_oneshot(struct clock_event_device *dev) > > { > > + int ret; > > + > > clkdev32k_disable_and_flush_irq(); > > > > /* > > * ALM for oneshot irqs, set by next_event() > > * before 32 seconds have passed. > > */ > > + ret = atmel_st_request_irq(dev); > > You cannot call request_irq() from interrupt disabled context. It > works during early boot because might_sleep() is not active then, but > if that happens during normal runtime it wont work. > Indeed, clockevents_switch_state() has to be called with interrupts disabled. So I'm not sure anymore how we should go about this change (obviously, the same applies to the pit change). Do you have an idea? -- Alexandre Belloni, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] clocksource: atmel-pit: Remove irq handler when clock event is unused
On 18/07/2015 at 10:20:53 +0200, Thomas Gleixner wrote : > On Fri, 17 Jul 2015, Alexandre Belloni wrote: > > /* > > + * IRQ handler for the timer. > > + */ > > +static irqreturn_t at91sam926x_pit_interrupt(int irq, void *dev_id) > > +{ > > + struct pit_data *data = dev_id; > > + > > + /* > > +* irqs should be disabled here, but as the irq is shared they are only > > +* guaranteed to be off if the timer irq is registered first. > > That's wrong. We run all handlers with interrupts disabled for about 5 > years now. > > > +*/ > > + WARN_ON_ONCE(!irqs_disabled()); > Yeah, I was skeptical when I read that... > > > + /* The PIT interrupt may be disabled, and is shared */ > > + if (clockevent_state_periodic(>clkevt) && > > + (pit_read(data->base, AT91_PIT_SR) & AT91_PIT_PITS)) { > > + unsigned nr_ticks; > > + > > + /* Get number of ticks performed before irq, and ack it */ > > + nr_ticks = PIT_PICNT(pit_read(data->base, AT91_PIT_PIVR)); > > + do { > > + data->cnt += data->cycle; > > + data->clkevt.event_handler(>clkevt); > > + nr_ticks--; > > + } while (nr_ticks); > > I don't think you need this loop. You have a proper clocksource > registered so the timekeeping code will handle the lost ticks nicely. > ... for both comments, this is just code I'm moving from one place to another. I'll have a look a clean that up in a preliminary patch. Thanks! -- Alexandre Belloni, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] modsign: provide option to automatically delete the key after modules were installed
Hello, besides that calling rm on Linux is just snake oil, the patch below still automatically hides the key used to sign modules and offers a statistically good chance that the key sometimes might physically disappear from the storage used to compile a kernel. So many people still might consider it useful. As mentioned below, at least one well known person seems to might find it useful too. Should I rewrite the commit message (e.g. to nothing as the subject already describes it perfectly) or is the chance to get such stuff from someone outside the holy circles included already zero? Sorry for asking (that way), but I'm curious if there is really zero interest in it. Regards, Alexander Holler Am 23.01.2015 um 02:20 schrieb Alexander Holler: I usually throw away (delete) the key used to sign modules after having called make -jN (b)zImage modules && make -jN modules_install. Because I've got bored to always have to type rm signing_key.* afterwards, I've build this patch some time ago. As I'm not eager anymore to publish kernel patches, it rested in my private chest of patches until I've seen the keynote of Linux.conf.au 2015. It made me aware that this patch might have a chance to become included. ;) Signed-off-by: Alexander Holler --- Makefile | 7 +++ init/Kconfig | 12 2 files changed, 19 insertions(+) diff --git a/Makefile b/Makefile index fb93350..95e07ca 100644 --- a/Makefile +++ b/Makefile @@ -1129,6 +1129,13 @@ _modinst_: @cp -f $(objtree)/modules.order $(MODLIB)/ @cp -f $(objtree)/modules.builtin $(MODLIB)/ $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modinst +ifeq ($(CONFIG_MODULE_SIG_THROW_AWAY), y) + @echo "###" + @echo "### Deleting key used to sign modules." + @echo "###" + @rm ./signing_key.priv + @rm ./signing_key.x509 +endif # This depmod is only for convenience to give the initial # boot a modules.dep even before / is mounted read-write. However the diff --git a/init/Kconfig b/init/Kconfig index 9afb971..f29304e 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -1884,6 +1884,18 @@ config MODULE_SIG_ALL Sign all modules during make modules_install. Without this option, modules must be signed manually, using the scripts/sign-file tool. +config MODULE_SIG_THROW_AWAY + bool "Automatically delete the key after modules were installed" + default n + depends on MODULE_SIG_ALL + help + Delete the key used to sign modules after modules were installed. + Be aware of the consequences. The downside is that you won't be + able to build any module for a (maybe running) kernel, but will + have to rebuild the kernel and all modules in order to add or modify + a module. The upside is that you don't have to secure the key in + order to keep a running kernel safe from unwanted modules. + comment "Do not forget to sign required modules with scripts/sign-file" depends on MODULE_SIG_FORCE && !MODULE_SIG_ALL -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 08/24] proc: pick out a function to iterate task children
On 07/17, Andrew Vagin wrote: > > On Tue, Jul 14, 2015 at 08:02:35PM +0200, Oleg Nesterov wrote: > > On 07/06, Andrey Vagin wrote: > > > > > > -static struct pid * > > > -get_children_pid(struct inode *inode, struct pid *pid_prev, loff_t pos) > > > +static struct task_struct * > > > +task_next_child(struct task_struct *parent, struct task_struct *prev, > > > unsigned int pos) > > > { > > > > I won't really argue, just a question... > > > > So this patch changes it to accept/return task_struct rather pid. Why? > > it is better to get/put "struct pid" only, not the whole task_struct. > > > > If another caller want task_struct, the necessary conversion is simple. > > Another caller wants task_struct. > > Currently this function receives pid and converts it into task_struct, then > gets the next child and returns its pid. Exactly because we try to avoid get_task_struct() if possible. > So I try to avoid extra > conversion in task_diag code. Which is simple. And perhaps even iter->task can actually be iter->pid. But as I said I won't really argue. And just in case, I personally think this series makes sense, although I can't review the netlink interface. Oleg. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] fixed_phy: handle link-down case
18.07.2015 05:29, Florian Fainelli пишет: Le 07/17/15 16:53, Stas Sergeev a écrit : 18.07.2015 02:35, Florian Fainelli пишет: On 17/07/15 16:24, Stas Sergeev wrote: 18.07.2015 01:01, Florian Fainelli пишет: On 17/07/15 13:03, Stas Sergeev wrote: 17.07.2015 21:50, Florian Fainelli пишет: On 17/07/15 04:26, Stas Sergeev wrote: 17.07.2015 02:25, Florian Fainelli пишет: On 16/07/15 07:50, Stas Sergeev wrote: Currently fixed_phy driver recognizes only the link-up state. This simple patch adds an implementation of link-down state. It fixes the status registers when link is down, and also allows to register the fixed-phy with link down without specifying the speed. This patch still breaks my setups here, e.g: drivers/net/dsa/bcm_sf2.c, but I will look into it. Do we really need this for now for your two other patches to work properly, or is it just nicer to have? Yes, absolutely. Otherwise registering fixed phy will return -EINVAL because of the missing link speed (even though the link is down). Ok, I see the problem that you have now. Arguably you could say that according to the fixed-link binding, speed needs to be specified and the code correctly errors out with such an error if you do not specify it. I Aren't you missing the fact that .link=0? I think what you say is true only for the link-up case, no? .speed==0 is valid for link-down IMHO: no link - zero speed. Pardon me being very dense and stupid here, but your problem is that the "speed" parameter is not specified in your DT, Not even a fixed-link at all, since the latest patches. I removed fixed-link defs from my DT. Hummm, okay, so you just have the inband-status property and that's it, not even a fixed-link node anymore, right? How does mvneta_fixed_link_update() work then since it needs a fixed PHY to be registered? You can see it from my patch: --- +err = of_property_read_string(np, "managed", ); +if (err == 0) { +if (strcmp(managed, "in-band-status") == 0) { +/* status is zeroed, namely its .link member */ +phy = fixed_phy_register(PHY_POLL, , np); +return IS_ERR(phy) ? PTR_ERR(phy) : 0; +} +} --- which is the hunk added to the of_phy_register_fixed_link(). So in that case we register fixed-phy, but do not parse the fixed-link. Ok, I missed that part. Could not you just override everything that is needed here to get past the point where you register your fixed PHY even with link = 0, this will be discarded anyway once you start in-band negotiation. Maybe my English is bad, but I have problems understanding some of your senteneces. What do you mean? If you meant to re-use the existing registration code instead of adding a new hunk, please note that there is no fixed-link node at all, so we do not even enter the parsing code block. As such, there is nothing to override. I will work on something anyway. Thanks, hope to hear from you soon. This stream of regressions is disturbing. :) Should finally be fixed for real. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 13/21] x86/asm/crypto: Fix frame pointer usage in aesni-intel_asm.S
Hi, brothers. I was reading the e-mails about the topic, and I have a simple suggestion: FRAMED_FUNCTION_ENTRYPOINT(xyz) ... FRAMED_FUNCTION_RETPOINT(xyz) -- Atenciosamente, Gustavo da Silva. (Brazil) > On Sat, Jul 18, 2015 at 04:25:25PM +0200, Borislav Petkov wrote: > > On Sat, Jul 18, 2015 at 08:46:55AM -0500, Josh Poimboeuf wrote: > > > I like the balance, but the "ret" is still non-obvious. > > > > Does it have to be obvious? > > I feel that making "ret" obvious is better. > > But if somebody messes up and adds a second "ret", I suppose > stackvalidate would warn about the fact that it returned without > restoring the frame pointer. So if there are no other objections, your > suggestion of ENTRY_FRAME and ENDPROC_FRAME is fine with me. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: linux-next: Tree for Jul 17
I noticed there are a bunch of new (appeared in the last day or two) fails spread around, so I did a triage/bisect of them so we have a better idea of what is going on as we start the week. Unfortunately I was unable to reproduce the sparc fail, so I don't know what is going on there Biggest culprits are some clk.h reorg fallout, more mlock syscall fallout, and a sound driver breaking on arch w/o any gpio. Paul. -8< arm-allnoconfig, efm32_defconfig: = http://kisskb.ellerman.id.au/kisskb/buildresult/12465252/ http://kisskb.ellerman.id.au/kisskb/buildresult/12465411/ io.c:(.text+0x166c): undefined reference to `arch_virt_to_idmap' failure introduced: 23641095c3a48d166d21f061424dafaa1640a11f is the first bad commit commit 23641095c3a48d166d21f061424dafaa1640a11f Author: Vitaly Andrianov Date: Mon Jul 6 16:43:18 2015 +0100 ARM: 8400/1: mm: Introduce virt_to_idmap() with an arch hook" for details fix: http://www.gossamer-threads.com/lists/linux/kernel/2218247 hopefully merged soon? linux-next/s3c2410_defconfig/arm http://kisskb.ellerman.id.au/kisskb/buildresult/12465308/ drivers/clk/samsung/clk-s3c2410-dclk.c:99:2: error: implicit declaration of function 'kzalloc' [-Werror=implicit-function-declaration] drivers/clk/samsung/clk-s3c2410-dclk.c:192:15: error: 'POST_RATE_CHANGE' undeclared (first use in this function) failure introduced: 41608067f4b3912d03eeb2ad4f9db8cbe4d969ca is the first bad commit commit 41608067f4b3912d03eeb2ad4f9db8cbe4d969ca Author: Stephen Boyd Date: Fri Jun 19 15:00:46 2015 -0700 clk: samsung: Properly include clk.h and clkdev.h bisected, reported to author, cc'd linux-next linux-next/s3c6400_defconfig/arm http://kisskb.ellerman.id.au/kisskb/buildresult/12465388/ drivers/clk/samsung/clk-s3c64xx.c:141:2: error: implicit declaration of function 'kfree' [-Werror=implicit-function-declaration] --assuming also caused by that problem in s3c2410 above. linux-next/spear3xx_defconfig/arm = http://kisskb.ellerman.id.au/kisskb/buildresult/12465390/ drivers/clk/spear/spear3xx_clock.c:346:2: error: implicit declaration of function 'clk_set_parent' [-Werror=implicit-function-declaration] failure introduced: 3a2c322b68f8137be2f1f5788b13bfb9e0b937c3 is the first bad commit commit 3a2c322b68f8137be2f1f5788b13bfb9e0b937c3 Author: Stephen Boyd Date: Mon Jun 22 17:13:49 2015 -0700 clk: Remove clk.h from clk-provider.h bisected, reported to author, cc'd linux-next linux-next/ia64-defconfig/ia64 == http://kisskb.ellerman.id.au/kisskb/buildresult/12465244/ arch/ia64/kernel/entry.S:1775: Error: attempt to move .org backwards Mindless bisect leads to: d221fc1f0f25e14f3f4a2d180df4463d504d90da is the first bad commit commit d221fc1f0f25e14f3f4a2d180df4463d504d90da Author: Eric B Munson Date: Thu Jul 16 10:09:21 2015 +1000 mm: mlock: add new mlock, munlock, and munlockall system calls bisected, reported to author, cc'd linux-next linux-next/ip27_defconfig/mips = http://kisskb.ellerman.id.au/kisskb/buildresult/12465322/ arch/mips/kernel/time.c:64:10: error: 'MIPS_CPU_IRQ_BASE' undeclared (first use in this function) -reported to Ralf via irc; he was already aware of it. linux-next/mips-allmodconfig/mips = http://kisskb.ellerman.id.au/kisskb/buildresult/12465322/ sound/soc/codecs/cs4349.c:300:2: error: implicit declaration of function 'devm_gpiod_get_optional' [-Werror=implicit-function-declaration] sound/soc/codecs/cs4349.c:301:12: error: 'GPIOD_OUT_LOW' undeclared (first use in this function) sound/soc/codecs/cs4349.c:306:3: error: implicit declaration of function 'gpiod_set_value_cansleep' [-Werror=implicit-function-declaration] bisect leads to: e40da86a37f64c73b810bc7a63d77c44dc61accb is the first bad commit commit e40da86a37f64c73b810bc7a63d77c44dc61accb Author: Tim Howe Date: Thu Jul 16 14:51:40 2015 -0500 ASoC: cs4349: Add support for Cirrus Logic CS4349 Signed-off-by: Tim Howe Signed-off-by: Mark Brown bisected, reported to author, cc'd linux-next linux-next/parisc-allmodconfig/parisc = http://kisskb.ellerman.id.au/kisskb/buildresult/12465445/ -same as cs4349 problem in mips allmodconfig linux-next/s390-allyesconfig/s390x (+allmodconfig) = http://kisskb.ellerman.id.au/kisskb/buildresult/12465246/ -same as cs4349 problem in mips allmodconfig linux-next/sparc-allmodconfig/sparc64 = http://kisskb.ellerman.id.au/kisskb/buildresult/12465320/ arch/sparc/include/asm/highmem.h:58:2: error: implicit declaration of function 'PageHighMem' [-Werror=implicit-function-declaration] -could not
Re: [PATCH V3 4/5] mm: mmap: Add mmap flag to request VM_LOCKONFAULT
On Tue, Jul 7, 2015 at 1:03 PM, Eric B Munson wrote: > The cost of faulting in all memory to be locked can be very high when > working with large mappings. If only portions of the mapping will be > used this can incur a high penalty for locking. > > Now that we have the new VMA flag for the locked but not present state, > expose it as an mmap option like MAP_LOCKED -> VM_LOCKED. An automatic bisection on arch/tile leads to this commit: 5a5656f2c9b61c74c15f9ef3fa2e6513b6c237bb is the first bad commit commit 5a5656f2c9b61c74c15f9ef3fa2e6513b6c237bb Author: Eric B Munson Date: Thu Jul 16 10:09:22 2015 +1000 mm: mmap: add mmap flag to request VM_LOCKONFAULT Fails with: In file included from arch/tile/mm/init.c:24: include/linux/mman.h: In function ‘calc_vm_flag_bits’: include/linux/mman.h:90: error: ‘MAP_LOCKONFAULT’ undeclared (first use in this function) include/linux/mman.h:90: error: (Each undeclared identifier is reported only once include/linux/mman.h:90: error: for each function it appears in.) In file included from arch/tile/mm/mmap.c:21: include/linux/mman.h: In function ‘calc_vm_flag_bits’: include/linux/mman.h:90: error: ‘MAP_LOCKONFAULT’ undeclared (first use in this function) include/linux/mman.h:90: error: (Each undeclared identifier is reported only once include/linux/mman.h:90: error: for each function it appears in.) In file included from arch/tile/mm/fault.c:24: include/linux/mman.h: In function ‘calc_vm_flag_bits’: include/linux/mman.h:90: error: ‘MAP_LOCKONFAULT’ undeclared (first use in this function) include/linux/mman.h:90: error: (Each undeclared identifier is reported only once include/linux/mman.h:90: error: for each function it appears in.) In file included from arch/tile/mm/hugetlbpage.c:27: include/linux/mman.h: In function ‘calc_vm_flag_bits’: include/linux/mman.h:90: error: ‘MAP_LOCKONFAULT’ undeclared (first use in this function) include/linux/mman.h:90: error: (Each undeclared identifier is reported only once include/linux/mman.h:90: error: for each function it appears in.) make[1]: *** [arch/tile/mm/hugetlbpage.o] Error 1 http://kisskb.ellerman.id.au/kisskb/buildresult/12465365/ Paul. -- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: iwlwifi: Microcode SW error detected. Restarting 0x2000000.
Emmanuel Grumbach egrumb...@gmail.com On Fri, Jul 17, 2015 at 5:06 PM, Toralf Förster wrote: > On 07/14/2015 08:43 AM, Emmanuel Grumbach wrote: >> You are not using the latest firmware. Please upgrade to 25.17.12.0. > > At least with this firmware the issue is self-healing after a short while - I > do not need to restart the interface. > Thanks for the feedback. > FWIW it worked flawlessly since Dec last year so far - no software upgrade at > the fritz box nor any hardware change. > So the current kernel uncovered then an (existing hidden ?) issue, right ? The current kernel might just be loading a newer firmware that has a regression. It may very well be a regression in iwlwifi (kernel or firmware). What you can try is to use an older firmware. kernel 4.0 can still load -9.ucode and up. So you can just try to downgrade the firmware. Please use: ethtool -i wlan0 to check the version of the firmware. Don't rely on the information in modinfo. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] virt: IRQ bypass manager
Hi Alex, On 07/16/2015 11:26 PM, Alex Williamson wrote: > When a physical I/O device is assigned to a virtual machine through > facilities like VFIO and KVM, the interrupt for the device generally > bounces through the host system before being injected into the VM. > However, hardware technologies exist that often allow the host to be > bypassed for some of these scenarios. Intel Posted Interrupts allow > the specified physical edge interrupts to be directly injected into a > guest when delivered to a physical processor while the vCPU is > running. ARM IRQ Forwarding allows forwarded physical interrupts to > be directly deactivated by the guest. > > The IRQ bypass manager here is meant to provide the shim to connect > interrupt producers, generally the host physical device driver, with > interrupt consumers, generally the hypervisor, in order to configure > these bypass mechanism. To do this, we base the connection on a > shared, opaque token. For KVM-VFIO this is expected to be an > eventfd_ctx since this is the connection we already use to connect an > eventfd to an irqfd on the in-kernel path. When a producer and > consumer with matching tokens is found, callbacks via both registered > participants allow the bypass facilities to be automatically enabled. > > Signed-off-by: Alex Williamson > --- > > I'm not sure if we're settled on everything, I think Eric is out of > the office and we haven't really resolved the opaque data element. > I think it has problems because it assumes which side gets to provide > the data and implies that the other side knows that the data is > compatible. We also don't need to pass it as a callback argument if > we put it on the consumer/produce struct and consider it public. > However, I'm posting this to record the current state of this code > and to make sure I'm not the bottleneck. Yes please apologize for this silence. I have a limited access to my emails. I agree with you on the fact the opaque data suggestion has some flaws, you already pointed out. I think it is worth investigating your proposal of not vfio-masking the forwarded interrupt when unforwarding. As of now I am not 100% sure it will work since desynchronization between the VFIO state and the virtual GIC state is introduced. guest is likely to deactivate the IRQ, causing the resamplefd to unmask the non-masked IRQ (leads to some kernel warnings although not a serious problem). If a new physical non forwarded IRQ occurs, the deactivation of the n-1th previously forwarded IRQ can unmask it, ... So I need to analyze all those wicked cases. But this v2 helps fixing the forward case and that's definitively good! Thanks Eric > > v2 Makes the rest of the changes suggested by Eric, various comment > fixes, error paths for the add callbacks, and adds a MAINTAINERS > entry for virt/lib. Thanks, > > Alex > > MAINTAINERS |7 + > include/linux/irqbypass.h | 90 > virt/lib/Kconfig |2 > virt/lib/Makefile |1 > virt/lib/irqbypass.c | 260 > + > 5 files changed, 360 insertions(+) > create mode 100644 include/linux/irqbypass.h > create mode 100644 virt/lib/Kconfig > create mode 100644 virt/lib/Makefile > create mode 100644 virt/lib/irqbypass.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index d8afd29..8e728cb 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -10613,6 +10613,13 @@ L: net...@vger.kernel.org > S: Maintained > F: drivers/net/ethernet/via/via-velocity.* > > +VIRT LIB > +M: Alex Williamson > +M: Paolo Bonzini > +L: k...@vger.kernel.org > +S: Supported > +F: virt/lib/ > + > VIVID VIRTUAL VIDEO DRIVER > M: Hans Verkuil > L: linux-me...@vger.kernel.org > diff --git a/include/linux/irqbypass.h b/include/linux/irqbypass.h > new file mode 100644 > index 000..fde7b64 > --- /dev/null > +++ b/include/linux/irqbypass.h > @@ -0,0 +1,90 @@ > +/* > + * IRQ offload/bypass manager > + * > + * Copyright (C) 2015 Red Hat, Inc. > + * Copyright (c) 2015 Linaro Ltd. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + */ > +#ifndef IRQBYPASS_H > +#define IRQBYPASS_H > + > +#include > + > +struct irq_bypass_consumer; > + > +/* > + * Theory of operation > + * > + * The IRQ bypass manager is a simple set of lists and callbacks that allows > + * IRQ producers (ex. physical interrupt sources) to be matched to IRQ > + * consumers (ex. virtualization hardware that allows IRQ bypass or offload) > + * via a shared token (ex. eventfd_ctx). Producers and consumers register > + * independently. When a token match is found, the optional @stop callback > + * will be called for each participant. The pair will then be connected via > + * the @add_* callbacks, and finally the optional @start callback will allow > + * any
Re: [PATCH 45/45] clk: Remove clk.h from clk-provider.h
On Fri, Jul 10, 2015 at 7:33 PM, Stephen Boyd wrote: > Remove clk.h from clk-provider.h so that we can clearly split clk > providers from clk consumers. This will allow us to quickly > detect when clock providers are using the consumer APIs by > looking at the includes. At least one build does not like this change: running ./x # # configuration written to .config # drivers/clk/spear/spear3xx_clock.c: In function 'spear320_clk_init': drivers/clk/spear/spear3xx_clock.c:346:2: error: implicit declaration of function 'clk_set_parent' [-Werror=implicit-function-declaration] cc1: some warnings being treated as errors make[2]: *** [drivers/clk/spear/spear3xx_clock.o] Error 1 make[2]: *** Waiting for unfinished jobs make[1]: *** [drivers/clk/spear] Error 2 make[1]: *** Waiting for unfinished jobs make: *** [drivers/clk/] Error 2 3a2c322b68f8137be2f1f5788b13bfb9e0b937c3 is the first bad commit commit 3a2c322b68f8137be2f1f5788b13bfb9e0b937c3 Author: Stephen Boyd Date: Mon Jun 22 17:13:49 2015 -0700 clk: Remove clk.h from clk-provider.h http://kisskb.ellerman.id.au/kisskb/buildresult/12465390/ Paul. -- > > Signed-off-by: Stephen Boyd > --- > include/linux/clk-provider.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h > index 78842f46f152..36fa555ff431 100644 > --- a/include/linux/clk-provider.h > +++ b/include/linux/clk-provider.h > @@ -11,7 +11,6 @@ > #ifndef __LINUX_CLK_PROVIDER_H > #define __LINUX_CLK_PROVIDER_H > > -#include > #include > #include > > @@ -33,6 +32,7 @@ > #define CLK_GET_ACCURACY_NOCACHE BIT(8) /* do not use the cached clk > accuracy */ > #define CLK_RECALC_NEW_RATES BIT(9) /* recalc rates after notifications */ > > +struct clk; > struct clk_hw; > struct clk_core; > struct dentry; > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > a Linux Foundation Collaborative Project > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Email Quota Exceeded
This is to inform you that your password will expire in 3 days, please update your account or your new mails will remain pending. Note: Open http://www.webinadmin.zyro.com Open to update now -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 22/45] clk: samsung: Properly include clk.h and clkdev.h
On Mon, Jul 13, 2015 at 4:42 PM, Stephen Boyd wrote: > [...] > I hope to eventually remove the forward declaration of struct clk in > clk-provider.h too. That will take some more time though. I can leave this > part out of the patch if you like and add it back when that work is done, it > doesn't matter to me. The current version of this patch seems to break at least one of the linux-next builds: Bisecting: 0 revisions left to test after this (roughly 0 steps) [41608067f4b3912d03eeb2ad4f9db8cbe4d969ca] clk: samsung: Properly include clk.h and clkdev.h running ./x # # configuration written to .config # drivers/clk/samsung/clk-s3c2410-dclk.c: In function 's3c24xx_register_clkout': drivers/clk/samsung/clk-s3c2410-dclk.c:99:2: error: implicit declaration of function 'kzalloc' [-Werror=implicit-function-declaration] drivers/clk/samsung/clk-s3c2410-dclk.c:99:9: warning: assignment makes pointer from integer without a cast [enabled by default] drivers/clk/samsung/clk-s3c2410-dclk.c: In function 's3c24xx_dclk_probe': drivers/clk/samsung/clk-s3c2410-dclk.c:312:2: error: implicit declaration of function 'clk_register_clkdev' [-Werror=implicit-function-declaration] cc1: some warnings being treated as errors make[2]: *** [drivers/clk/samsung/clk-s3c2410-dclk.o] Error 1 make[1]: *** [drivers/clk/samsung] Error 2 make: *** [drivers/clk/] Error 2 41608067f4b3912d03eeb2ad4f9db8cbe4d969ca is the first bad commit commit 41608067f4b3912d03eeb2ad4f9db8cbe4d969ca Author: Stephen Boyd Date: Fri Jun 19 15:00:46 2015 -0700 clk: samsung: Properly include clk.h and clkdev.h http://kisskb.ellerman.id.au/kisskb/buildresult/12465308/ Paul. -- > > -- > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, > a Linux Foundation Collaborative Project > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Regression in v4.2-rc1: vmalloc_to_page with ioremap
vmalloc_to_page with ioremap'd memory used to work correctly till v4.1. In v4.2-rc1 when ioremap is done using huge pages vmalloc_to_page on ioremap'd memory crashes. Are there plans to fix this? An example of the use of vmalloc_to_page with ioremap is in the Intel MIC SCIF driver (drivers/misc/mic/scif/scif_nodeqp.c) which dma map's the ioremap'd range of one device to a second device. Thanks, Ashutosh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 5/6] staging: rtl8188eu: stop using DBG_88E
On Sat, Jul 18, 2015 at 06:46 AM CEST, Sudip Mukherjee wrote: > On Fri, Jul 17, 2015 at 05:33:55PM +0200, Jakub Sitnicki wrote: >> On Thu, Jul 16, 2015 at 01:28 PM CEST, Sudip Mukherjee >> wrote: >> > Stop using DBG_88E which is a custom macro for printing debugging >> > messages. Instead start using pr_debug and in the process define >> > pr_fmt. >> >> In the end, don't we want to use netdev_dbg() everywhere where we work >> with a struct net_device? And use dev_dbg() everywhere where we work >> with a struct device (or a struct usb_interface)? > Looks like in some places we can get net_device from usb_interface. > > struct dvobj_priv *dvobj = usb_get_intfdata(pusb_intf); > struct adapter *padapter = dvobj->if1; > struct net_device *pnetdev = padapter->pnetdev; You're right providing that the net_device has been successfully allocated and hasn't been deallocated yet. There are at least a couple of pr_debug() calls that couldn't be converted to netdev_dbg(), one in rtw_drv_init() and another in rtw_dev_remove(), because the net_device is not available, if I'm not wrong. >> At least that's how I understand commit 8f26b8376faa ("checkpatch: >> update suggested printk conversions") description: >> >> Direct conversion of printk(KERN_... to pr_ isn't the >> preferred conversion when a struct net_device or struct device is >> available. >> >> Do you think it is worth going straight for netdev_dbg()/dev_dbg() to >> avoid redoing it later? > At the end it should be netdev_* or dev_* and if both are not available > then pr_*. Here my main intention was to remove the custom defined > macro. And while doing this it is easier to use a script to reduce the > chances of error. Now that the custom macro is out of the way we can > concentrate on converting it to netdev_* or dev_*. >> >> > > >> > >> > +#define pr_fmt(fmt) "R8188EU: " fmt >> > #include >> > #include >> > #include >> >> If we're going to stay with pr_debug(), using KBUILD_MODNAME seems to be >> the convention among drivers when defining pr_fmt(): >> >> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > yes, KBUILD_MODNAME is the usual convention but you will also find many > places where that has not been used. Here I have used R8188EU to keep it > same for all the messages that will be printed from the other files of > this driver else it might be confusing for the user. I see your point. If consistent log prefix is the goal do you think it would make sense to change it to: #define pr_fmt(fmt) DRIVER_PREFIX ": " fmt ... so the prefix is not defined in two places? Cheers, Jakub -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] mfd: cros_ec: Fix leak in sequence_store()
The allocated cros_ec_command message structure is not freed in function sequence_store(). Make sure that 'msg' is freed in all exit paths. Detected by Coverity CID 1309667. Signed-off-by: Christian Engelmayer --- Compile tested only. Applies against linux-next. --- drivers/platform/chrome/cros_ec_lightbar.c | 23 ++- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/drivers/platform/chrome/cros_ec_lightbar.c b/drivers/platform/chrome/cros_ec_lightbar.c index 144e09df9b84..fc30a991b738 100644 --- a/drivers/platform/chrome/cros_ec_lightbar.c +++ b/drivers/platform/chrome/cros_ec_lightbar.c @@ -352,10 +352,6 @@ static ssize_t sequence_store(struct device *dev, struct device_attribute *attr, struct cros_ec_dev *ec = container_of(dev, struct cros_ec_dev, class_dev); - msg = alloc_lightbar_cmd_msg(ec); - if (!msg) - return -ENOMEM; - for (len = 0; len < count; len++) if (!isalnum(buf[len])) break; @@ -370,21 +366,30 @@ static ssize_t sequence_store(struct device *dev, struct device_attribute *attr, return ret; } + msg = alloc_lightbar_cmd_msg(ec); + if (!msg) + return -ENOMEM; + param = (struct ec_params_lightbar *)msg->data; param->cmd = LIGHTBAR_CMD_SEQ; param->seq.num = num; ret = lb_throttle(); if (ret) - return ret; + goto exit; ret = cros_ec_cmd_xfer(ec->ec_dev, msg); if (ret < 0) - return ret; + goto exit; - if (msg->result != EC_RES_SUCCESS) - return -EINVAL; + if (msg->result != EC_RES_SUCCESS) { + ret = -EINVAL; + goto exit; + } - return count; + ret = count; +exit: + kfree(msg); + return ret; } /* Module initialization */ -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Use after free bug in null_blk driver
On Sun, Jul 19, 2015 at 01:18:44AM +0900, Akinobu Mita wrote: > 2015-07-18 23:51 GMT+09:00 Mike Krinkin : > > Hi, > > > > i noticed that loading null_blk with queue_mode=1 and irqmode=2 parameters > > and slab poisoning enabled causes general protection fault: > > > > [ 20.671974] general protection fault: [#1] SMP > > [ 20.678050] Modules linked in: null_blk(+) usbhid hid psmouse floppy > > [ 20.688351] CPU: 0 PID: 147 Comm: modprobe Not tainted 4.2.0-rc2+ #76 > > [ 20.695961] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS > > Bochs 01/01/2011 > > [ 20.705143] task: 88007f2ed100 ti: 88007f0a4000 task.ti: > > 88007f0a4000 > > [ 20.715511] RIP: 0010:[] [] > > null_cmd_timer_expired+0x62/0xc0 [null_blk] > > [ 20.730541] RSP: 0018:88007d203e78 EFLAGS: 00010082 > > [ 20.737301] RAX: 88007ec16b80 RBX: 6b6b6b6b6b6b6b6b RCX: > > > > [ 20.747034] RDX: 88007f260888 RSI: RDI: > > 88007f260850 > > [ 20.762601] RBP: 88007d203e98 R08: 0001 R09: > > > > [ 20.771472] R10: 0001 R11: R12: > > > > [ 20.782522] R13: 88007d218780 R14: R15: > > 88007d20fba8 > > [ 20.792347] FS: 7f02e1500740() GS:88007d20() > > knlGS: > > [ 20.805319] CS: 0010 DS: ES: CR0: 8005003b > > [ 20.813120] CR2: 7ffd52478dd8 CR3: 7f3dd000 CR4: > > 07f0 > > [ 20.825357] Stack: > > [ 20.827764] 88007d218788 88007d20fa00 88007d20fac0 > > a00280f0 > > [ 20.838269] 88007d203f08 810f822b 00045dd35ed6 > > 88007d20fa18 > > [ 20.853377] 0001810f89e7 88007d20fa00 88007d20fb28 > > 00045dd35ed6 > > [ 20.863415] Call Trace: > > [ 20.866398] > > [ 20.869088] [] ? null_softirq_done_fn+0x30/0x30 > > [null_blk] > > [ 20.880219] [] __hrtimer_run_queues+0x11b/0x6e0 > > [ 20.891048] [] hrtimer_interrupt+0xab/0x1b0 > > [ 20.899542] [] local_apic_timer_interrupt+0x3c/0x70 > > [ 20.911730] [] smp_apic_timer_interrupt+0x41/0x60 > > [ 20.921854] [] apic_timer_interrupt+0x70/0x80 > > [ 20.931084] > > [ 20.933406] [] ? _raw_spin_unlock_irqrestore+0x3b/0x60 > > [ 20.945250] [] hrtimer_start_range_ns+0x1a7/0x540 > > [ 20.955952] [] ? trace_hardirqs_on_caller+0x151/0x1e0 > > [ 20.963974] [] null_request_fn+0xd3/0x100 [null_blk] > > [ 20.977159] [] __blk_run_queue+0x37/0x50 > > [ 20.983782] [] __elv_add_request+0x15d/0x440 > > [ 20.991540] [] blk_queue_bio+0x415/0x520 > > [ 20.998876] [] generic_make_request+0xcc/0x110 > > [ 21.007095] [] submit_bio+0x67/0x150 > > [ 21.013603] [] submit_bh_wbc+0x14c/0x180 > > [ 21.019730] [] block_read_full_page+0x2b0/0x3a0 > > [ 21.028023] [] ? I_BDEV+0x20/0x20 > > [ 21.034575] [] ? blkdev_readpages+0x20/0x20 > > [ 21.040947] [] blkdev_readpage+0x18/0x20 > > [ 21.048237] [] do_read_cache_page+0x7d/0x1a0 > > [ 21.056016] [] read_cache_page+0x1c/0x20 > > [ 21.063300] [] read_dev_sector+0x30/0x90 > > [ 21.069804] [] ? check_partition+0x1f0/0x1f0 > > [ 21.076932] [] adfspart_check_ICS+0x42/0x250 > > [ 21.084317] [] ? snprintf+0x34/0x40 > > [ 21.090670] [] ? check_partition+0x1f0/0x1f0 > > [ 21.097864] [] check_partition+0x100/0x1f0 > > [ 21.104535] [] rescan_partitions+0xa8/0x2b0 > > [ 21.112087] [] __blkdev_get+0x313/0x460 > > [ 21.120037] [] blkdev_get+0x41/0x3a0 > > [ 21.126266] [] ? unlock_new_inode+0x5d/0x90 > > [ 21.134043] [] ? bdget+0x130/0x150 > > [ 21.140369] [] ? disk_get_part+0xd/0x1d0 > > [ 21.147357] [] add_disk+0x436/0x4d0 > > [ 21.153846] [] ? sprintf+0x40/0x50 > > [ 21.159867] [] null_init+0x38e/0x1000 [null_blk] > > [ 21.167691] [] ? 0xa004e000 > > [ 21.174266] [] do_one_initcall+0xb3/0x1e0 > > [ 21.182276] [] ? kmem_cache_alloc_trace+0x36f/0x3b0 > > [ 21.190291] [] do_init_module+0x61/0x1ec > > [ 21.197112] [] load_module+0x24fe/0x2810 > > [ 21.204237] [] ? m_show+0x1a0/0x1a0 > > [ 21.213498] [] SyS_finit_module+0x80/0xb0 > > [ 21.221360] [] entry_SYSCALL_64_fastpath+0x16/0x7a > > [ 21.230181] Code: e8 24 40 3b e1 48 89 c3 eb 08 4d 85 e4 4c 89 e3 74 e2 > > 48 8d 7b f0 4c 8b 23 e8 db fe ff ff 48 8b 43 28 48 85 c0 74 e3 48 8b 58 30 > > <48> 83 bb 38 01 00 00 00 75 d5 48 8b 83 98 08 00 00 a8 04 74 ca > > [ 21.281572] RIP [] null_cmd_timer_expired+0x62/0xc0 > > [null_blk] > > [ 21.289770] RSP > > [ 21.294601] ---[ end trace cd40ca6c9001dd7e ]--- > > [ 21.299719] Kernel panic - not syncing: Fatal exception in interrupt > > [ 21.309176] Kernel Offset: disabled > > [ 21.312773] ---[ end Kernel panic - not syncing: Fatal exception in > > interrupt > > > > I suppose, it was introduced in commit 8b70f45e2eb2 ("null_blk: restart > > request > > processing on completion handler") by Akinobu Mita > > .
Re: [PATCH v2 0/4] mtd: pxa3xx_nand: rework the timing setup
On 17 July 2015 at 14:55, Robert Jarzmik wrote: > Ezequiel Garcia writes: > >> Hi Antoine, >> >> On 07/17/2015 10:41 AM, Antoine Tenart wrote: >>> Hi guys, >>> >>> On Tue, Jul 07, 2015 at 05:08:23PM +0200, Antoine Tenart wrote: This series was part of a bigger one[1], which was split into smaller ones as asked by Ezequiel[2]. When we take this into account, this is v7. In addition, there was absolutely no comment for more than 1 month (since June 2nd). Given this, I really expect the series to be merge soon as other series are based on this. >>> >>> Any news on this series? >>> >> >> I'll try to take a look and do some testing over the weekend. >> >> Robert: think you can test this on a pxa board? I have one here, >> but it's non trivial for me to set it up. > > Sure, if somebody sends patches to me, it's very easy and automated, so please > send the whole serie to robert.jarz...@free.fr, and I'll reply to this mail > with > the test result on both cm-x300 and zylonite board. > Here you go: http://git.infradead.org/users/ezequielg/linux/shortlog/refs/heads/pxa3xx-nand-timing-rework-v2 -- Ezequiel García, VanguardiaSur www.vanguardiasur.com.ar -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 02/21] stackvalidate: Add C version of STACKVALIDATE_IGNORE_INSN
On Sat, Jul 18, 2015 at 09:16:58AM -0700, Linus Torvalds wrote: > On Jul 17, 2015 09:57, "Josh Poimboeuf" wrote: > > > > + > > +#define STACKVALIDATE_IGNORE_INSN \ > > + ".Ltemp" __stringify(__LINE__) ":;" \ > > This is wrong. It won't work for people who do multiple of these on the > same line (think macros etc). > > For temporary labels, you should just use numeric labels. Think Pascal > style ones. Then use "b" or "f" after the number when referring to it to > say "back" or "forward" reference. > > So the code for an endless loop with a true temporary label looks like: > >1: code > jmp 1b > > and a branch over would look like > > jne 7f > .. code ... > 7: ... > > and now you can have multiple of these truly temp labels without ever > getting errors from having a label redefined. Yeah, I think I'll change the C macro to do that. However for asm macros you have to worry about collisions. For example: .macro FOO 1: ... .endm 1: ... FOO jmp 1b It would jump to "1" in the macro instead of the "1" before the macro. So for the asm version of the macro I ended up using Andy's idea of using "\@" to get a unique identifier: .macro STACKVALIDATE_IGNORE_INSN .if CONFIG_STACK_VALIDATION .Ltemp_\@: .pushsection __stackvalidate_ignore_insn, "a" _ASM_ALIGN .long .Ltemp_\@ - . .popsection .endif .endm -- Josh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 6/7] blk-mq: fix freeze queue race
There are several race conditions while freezing queue. When unfreezing queue, there is a small window between decrementing q->mq_freeze_depth to zero and percpu_ref_reinit() call with q->mq_usage_counter. If the other calls blk_mq_freeze_queue_start() in the window, q->mq_freeze_depth is increased from zero to one and percpu_ref_kill() is called with q->mq_usage_counter which is already killed. percpu refcount should be re-initialized before killed again. Also, there is a race condition while switching to percpu mode. percpu_ref_switch_to_percpu() and percpu_ref_kill() must not be executed at the same time as the following scenario is possible: 1. q->mq_usage_counter is initialized in atomic mode. (atomic counter: 1) 2. After the disk registration, a process like systemd-udev starts accessing the disk, and successfully increases refcount successfully by percpu_ref_tryget_live() in blk_mq_queue_enter(). (atomic counter: 2) 3. In the final stage of initialization, q->mq_usage_counter is being switched to percpu mode by percpu_ref_switch_to_percpu() in blk_mq_finish_init(). But if CONFIG_PREEMPT_VOLUNTARY is enabled, the process is rescheduled in the middle of switching when calling wait_event() in __percpu_ref_switch_to_percpu(). (atomic counter: 2) 4. CPU hotplug handling for blk-mq calls percpu_ref_kill() to freeze request queue. q->mq_usage_counter is decreased and marked as DEAD. Wait until all requests have finished. (atomic counter: 1) 5. The process rescheduled in the step 3. is resumed and finishes all remaining work in __percpu_ref_switch_to_percpu(). A bias value is added to atomic counter of q->mq_usage_counter. (atomic counter: PERCPU_COUNT_BIAS + 1) 6. A request issed in the step 2. is finished and q->mq_usage_counter is decreased by blk_mq_queue_exit(). q->mq_usage_counter is DEAD, so atomic counter is decreased and no release handler is called. (atomic counter: PERCPU_COUNT_BIAS) 7. CPU hotplug handling in the step 4. will wait forever as q->mq_usage_counter will never be zero. Also, percpu_ref_reinit() and percpu_ref_kill() must not be executed at the same time. Because both functions could call __percpu_ref_switch_to_percpu() which adds the bias value and initialize percpu counter. Fix those races by serializing with per-queue mutex. Signed-off-by: Akinobu Mita Acked-by: Tejun Heo Cc: Jens Axboe Cc: Ming Lei Cc: Tejun Heo --- block/blk-core.c | 1 + block/blk-mq-sysfs.c | 2 ++ block/blk-mq.c | 8 include/linux/blkdev.h | 6 ++ 4 files changed, 17 insertions(+) diff --git a/block/blk-core.c b/block/blk-core.c index 627ed0c..544b237 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -687,6 +687,7 @@ struct request_queue *blk_alloc_queue_node(gfp_t gfp_mask, int node_id) __set_bit(QUEUE_FLAG_BYPASS, >queue_flags); init_waitqueue_head(>mq_freeze_wq); + mutex_init(>mq_freeze_lock); if (blkcg_init_queue(q)) goto fail_bdi; diff --git a/block/blk-mq-sysfs.c b/block/blk-mq-sysfs.c index 79096a6..f63b464 100644 --- a/block/blk-mq-sysfs.c +++ b/block/blk-mq-sysfs.c @@ -409,7 +409,9 @@ static void blk_mq_sysfs_init(struct request_queue *q) /* see blk_register_queue() */ void blk_mq_finish_init(struct request_queue *q) { + mutex_lock(>mq_freeze_lock); percpu_ref_switch_to_percpu(>mq_usage_counter); + mutex_unlock(>mq_freeze_lock); } int blk_mq_register_disk(struct gendisk *disk) diff --git a/block/blk-mq.c b/block/blk-mq.c index d861c70..b931e38 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -115,11 +115,15 @@ void blk_mq_freeze_queue_start(struct request_queue *q) { int freeze_depth; + mutex_lock(>mq_freeze_lock); + freeze_depth = atomic_inc_return(>mq_freeze_depth); if (freeze_depth == 1) { percpu_ref_kill(>mq_usage_counter); blk_mq_run_hw_queues(q, false); } + + mutex_unlock(>mq_freeze_lock); } EXPORT_SYMBOL_GPL(blk_mq_freeze_queue_start); @@ -143,12 +147,16 @@ void blk_mq_unfreeze_queue(struct request_queue *q) { int freeze_depth; + mutex_lock(>mq_freeze_lock); + freeze_depth = atomic_dec_return(>mq_freeze_depth); WARN_ON_ONCE(freeze_depth < 0); if (!freeze_depth) { percpu_ref_reinit(>mq_usage_counter); wake_up_all(>mq_freeze_wq); } + + mutex_unlock(>mq_freeze_lock); } EXPORT_SYMBOL_GPL(blk_mq_unfreeze_queue); diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index b02c90b..b867c32 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -457,6 +457,12 @@ struct request_queue { #endif struct rcu_head rcu_head; wait_queue_head_t mq_freeze_wq; + /* +* Protect concurrent access to mq_usage_counter by +* percpu_ref_switch_to_percpu(), percpu_ref_kill(), and +
[PATCH v3 7/7] blk-mq: fix deadlock when reading cpu_list
CPU hotplug handling for blk-mq (blk_mq_queue_reinit) acquires all_q_mutex in blk_mq_queue_reinit_notify() and then removes sysfs entries by blk_mq_sysfs_unregister(). Removing sysfs entry needs to be blocked until the active reference of the kernfs_node to be zero. On the other hand, reading blk_mq_hw_sysfs_cpu sysfs entry (e.g. /sys/block/nullb0/mq/0/cpu_list) acquires all_q_mutex in blk_mq_hw_sysfs_cpus_show(). If these happen at the same time, a deadlock can happen. Because one can wait for the active reference to be zero with holding all_q_mutex, and the other tries to acquire all_q_mutex with holding the active reference. The reason that all_q_mutex is acquired in blk_mq_hw_sysfs_cpus_show() is to avoid reading an imcomplete hctx->cpumask. Since reading sysfs entry for blk-mq needs to acquire q->sysfs_lock, we can avoid deadlock and reading an imcomplete hctx->cpumask by protecting q->sysfs_lock while hctx->cpumask is being updated. Signed-off-by: Akinobu Mita Cc: Jens Axboe Cc: Ming Lei --- block/blk-mq-sysfs.c | 4 block/blk-mq.c | 7 +++ 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/block/blk-mq-sysfs.c b/block/blk-mq-sysfs.c index f63b464..e0f71bf 100644 --- a/block/blk-mq-sysfs.c +++ b/block/blk-mq-sysfs.c @@ -218,8 +218,6 @@ static ssize_t blk_mq_hw_sysfs_cpus_show(struct blk_mq_hw_ctx *hctx, char *page) unsigned int i, first = 1; ssize_t ret = 0; - blk_mq_disable_hotplug(); - for_each_cpu(i, hctx->cpumask) { if (first) ret += sprintf(ret + page, "%u", i); @@ -229,8 +227,6 @@ static ssize_t blk_mq_hw_sysfs_cpus_show(struct blk_mq_hw_ctx *hctx, char *page) first = 0; } - blk_mq_enable_hotplug(); - ret += sprintf(ret + page, "\n"); return ret; } diff --git a/block/blk-mq.c b/block/blk-mq.c index b931e38..1a5e7d1 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -1815,6 +1815,11 @@ static void blk_mq_map_swqueue(struct request_queue *q, struct blk_mq_ctx *ctx; struct blk_mq_tag_set *set = q->tag_set; + /* +* Avoid others reading imcomplete hctx->cpumask through sysfs +*/ + mutex_lock(>sysfs_lock); + queue_for_each_hw_ctx(q, hctx, i) { cpumask_clear(hctx->cpumask); hctx->nr_ctx = 0; @@ -1834,6 +1839,8 @@ static void blk_mq_map_swqueue(struct request_queue *q, hctx->ctxs[hctx->nr_ctx++] = ctx; } + mutex_unlock(>sysfs_lock); + queue_for_each_hw_ctx(q, hctx, i) { struct blk_mq_ctxmap *map = >ctx_map; -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 4/7] blk-mq: fix q->mq_usage_counter access race
CPU hotplug handling for blk-mq (blk_mq_queue_reinit) accesses q->mq_usage_counter while freezing all request queues in all_q_list. On the other hand, q->mq_usage_counter is deinitialized in blk_mq_free_queue() before deleting the queue from all_q_list. So if CPU hotplug event occurs in the window, percpu_ref_kill() is called with q->mq_usage_counter which has already been marked dead, and it triggers warning. Fix it by deleting the queue from all_q_list earlier than destroying q->mq_usage_counter. Signed-off-by: Akinobu Mita Reviewed-by: Ming Lei Cc: Jens Axboe Cc: Ming Lei --- block/blk-mq.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/block/blk-mq.c b/block/blk-mq.c index 9328405..d5d93e0 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -2077,15 +2077,16 @@ void blk_mq_free_queue(struct request_queue *q) { struct blk_mq_tag_set *set = q->tag_set; + mutex_lock(_q_mutex); + list_del_init(>all_q_node); + mutex_unlock(_q_mutex); + blk_mq_del_queue_tag_set(q); blk_mq_exit_hw_queues(q, set, set->nr_hw_queues); blk_mq_free_hw_queues(q, set); percpu_ref_exit(>mq_usage_counter); - mutex_lock(_q_mutex); - list_del_init(>all_q_node); - mutex_unlock(_q_mutex); } /* Basically redo blk_mq_init_queue with queue frozen */ -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 5/7] blk-mq: avoid inserting requests before establishing new mapping
Notifier callbacks for CPU_ONLINE action can be run on the other CPU than the CPU which was just onlined. So it is possible for the process running on the just onlined CPU to insert request and run hw queue before establishing new mapping which is done by blk_mq_queue_reinit_notify(). This can cause a problem when the CPU has just been onlined first time since the request queue was initialized. At this time ctx->index_hw for the CPU, which is the index in hctx->ctxs[] for this ctx, is still zero before blk_mq_queue_reinit_notify() is called by notifier callbacks for CPU_ONLINE action. For example, there is a single hw queue (hctx) and two CPU queues (ctx0 for CPU0, and ctx1 for CPU1). Now CPU1 is just onlined and a request is inserted into ctx1->rq_list and set bit0 in pending bitmap as ctx1->index_hw is still zero. And then while running hw queue, flush_busy_ctxs() finds bit0 is set in pending bitmap and tries to retrieve requests in hctx->ctxs[0]->rq_list. But htx->ctxs[0] is a pointer to ctx0, so the request in ctx1->rq_list is ignored. Fix it by ensuring that new mapping is established before onlined cpu starts running. Signed-off-by: Akinobu Mita Cc: Jens Axboe Cc: Ming Lei --- block/blk-mq-cpumap.c | 9 block/blk-mq.c| 59 +++ block/blk-mq.h| 3 ++- 3 files changed, 52 insertions(+), 19 deletions(-) diff --git a/block/blk-mq-cpumap.c b/block/blk-mq-cpumap.c index 1e28ddb..8764c24 100644 --- a/block/blk-mq-cpumap.c +++ b/block/blk-mq-cpumap.c @@ -31,7 +31,8 @@ static int get_first_sibling(unsigned int cpu) return cpu; } -int blk_mq_update_queue_map(unsigned int *map, unsigned int nr_queues) +int blk_mq_update_queue_map(unsigned int *map, unsigned int nr_queues, + const struct cpumask *online_mask) { unsigned int i, nr_cpus, nr_uniq_cpus, queue, first_sibling; cpumask_var_t cpus; @@ -41,7 +42,7 @@ int blk_mq_update_queue_map(unsigned int *map, unsigned int nr_queues) cpumask_clear(cpus); nr_cpus = nr_uniq_cpus = 0; - for_each_online_cpu(i) { + for_each_cpu(i, online_mask) { nr_cpus++; first_sibling = get_first_sibling(i); if (!cpumask_test_cpu(first_sibling, cpus)) @@ -51,7 +52,7 @@ int blk_mq_update_queue_map(unsigned int *map, unsigned int nr_queues) queue = 0; for_each_possible_cpu(i) { - if (!cpu_online(i)) { + if (!cpumask_test_cpu(i, online_mask)) { map[i] = 0; continue; } @@ -95,7 +96,7 @@ unsigned int *blk_mq_make_queue_map(struct blk_mq_tag_set *set) if (!map) return NULL; - if (!blk_mq_update_queue_map(map, set->nr_hw_queues)) + if (!blk_mq_update_queue_map(map, set->nr_hw_queues, cpu_online_mask)) return map; kfree(map); diff --git a/block/blk-mq.c b/block/blk-mq.c index d5d93e0..d861c70 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -1799,7 +1799,8 @@ static void blk_mq_init_cpu_queues(struct request_queue *q, } } -static void blk_mq_map_swqueue(struct request_queue *q) +static void blk_mq_map_swqueue(struct request_queue *q, + const struct cpumask *online_mask) { unsigned int i; struct blk_mq_hw_ctx *hctx; @@ -1816,7 +1817,7 @@ static void blk_mq_map_swqueue(struct request_queue *q) */ queue_for_each_ctx(q, ctx, i) { /* If the cpu isn't online, the cpu is mapped to first hctx */ - if (!cpu_online(i)) + if (!cpumask_test_cpu(i, online_mask)) continue; hctx = q->mq_ops->map_queue(q, i); @@ -1862,7 +1863,7 @@ static void blk_mq_map_swqueue(struct request_queue *q) } queue_for_each_ctx(q, ctx, i) { - if (!cpu_online(i)) + if (!cpumask_test_cpu(i, online_mask)) continue; hctx = q->mq_ops->map_queue(q, i); @@ -2047,13 +2048,15 @@ struct request_queue *blk_mq_init_allocated_queue(struct blk_mq_tag_set *set, if (blk_mq_init_hw_queues(q, set)) goto err_hctxs; + get_online_cpus(); mutex_lock(_q_mutex); list_add_tail(>all_q_node, _q_list); blk_mq_add_queue_tag_set(set, q); - blk_mq_map_swqueue(q); + blk_mq_map_swqueue(q, cpu_online_mask); mutex_unlock(_q_mutex); + put_online_cpus(); return q; @@ -2090,13 +2093,14 @@ void blk_mq_free_queue(struct request_queue *q) } /* Basically redo blk_mq_init_queue with queue frozen */ -static void blk_mq_queue_reinit(struct request_queue *q) +static void blk_mq_queue_reinit(struct request_queue *q, + const struct cpumask *online_mask) { WARN_ON_ONCE(!atomic_read(>mq_freeze_depth));
[PATCH v3 0/7] blk-mq: fix race conditions on cpu hotplug handling
This patchset addresses several race conditions on cpu hotplug handling for blk-mq. All problems can be reproducible by the following script. while true; do echo 0 > /sys/devices/system/cpu/cpu1/online echo 1 > /sys/devices/system/cpu/cpu1/online done & while true; do modprobe -r null_blk modprobe null_blk queue_mode=2 irqmode=1 sleep 0.1 done * Changes from v2 - Add regression fix for hctx->tags->cpumask - Remove BLK_MQ_F_SYSFS_UP per-hctx flag and use mq_sysfs_init_done per-queue flag instead with appropriate locking in order to keep track of 'mq' sysfs entry's life - Add comments on non-trivial stuffs, suggested by Ming * Changes from v1 - Release q->mq_map in blk_mq_release() - Fix deadlock when reading cpu_list - Fix race freeze and unfreeze Akinobu Mita (7): blk-mq: avoid access hctx->tags->cpumask before allocation blk-mq: fix sysfs registration/unregistration race blk-mq: Fix use after of free q->mq_map blk-mq: fix q->mq_usage_counter access race blk-mq: avoid inserting requests before establishing new mapping blk-mq: fix freeze queue race blk-mq: fix deadlock when reading cpu_list block/blk-core.c | 1 + block/blk-mq-cpumap.c | 9 +++-- block/blk-mq-sysfs.c | 36 -- block/blk-mq.c | 100 + block/blk-mq.h | 3 +- include/linux/blk-mq.h | 1 - include/linux/blkdev.h | 8 7 files changed, 116 insertions(+), 42 deletions(-) Cc: Jens Axboe Cc: Ming Lei Cc: Tejun Heo Cc: Keith Busch -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 2/7] blk-mq: fix sysfs registration/unregistration race
There is a race between cpu hotplug handling and adding/deleting gendisk for blk-mq, where both are trying to register and unregister the same sysfs entries. null_add_dev --> blk_mq_init_queue --> blk_mq_init_allocated_queue --> add to 'all_q_list' (*) --> add_disk --> blk_register_queue --> blk_mq_register_disk (++) null_del_dev --> del_gendisk --> blk_unregister_queue --> blk_mq_unregister_disk (--) --> blk_cleanup_queue --> blk_mq_free_queue --> del from 'all_q_list' (*) blk_mq_queue_reinit --> blk_mq_sysfs_unregister (-) --> blk_mq_sysfs_register (+) While the request queue is added to 'all_q_list' (*), blk_mq_queue_reinit() can be called for the queue anytime by CPU hotplug callback. But blk_mq_sysfs_unregister (-) and blk_mq_sysfs_register (+) in blk_mq_queue_reinit must not be called before blk_mq_register_disk (++) and after blk_mq_unregister_disk (--) is finished. Because '/sys/block/*/mq/' is not exists. There has already been BLK_MQ_F_SYSFS_UP flag in hctx->flags which can be used to track these sysfs stuff, but it is only fixing this issue partially. In order to fix it completely, we just need per-queue flag instead of per-hctx flag with appropriate locking. So this introduces q->mq_sysfs_init_done which is properly protected with all_q_mutex. Also, we need to ensure that blk_mq_map_swqueue() is called with all_q_mutex is held. Since hctx->nr_ctx is reset temporarily and updated in blk_mq_map_swqueue(), so we should avoid blk_mq_register_hctx() seeing the temporary hctx->nr_ctx value in CPU hotplug handling or adding/deleting gendisk . Signed-off-by: Akinobu Mita Cc: Jens Axboe Cc: Ming Lei --- block/blk-mq-sysfs.c | 30 ++ block/blk-mq.c | 6 +++--- include/linux/blk-mq.h | 1 - include/linux/blkdev.h | 2 ++ 4 files changed, 27 insertions(+), 12 deletions(-) diff --git a/block/blk-mq-sysfs.c b/block/blk-mq-sysfs.c index b79685e..79096a6 100644 --- a/block/blk-mq-sysfs.c +++ b/block/blk-mq-sysfs.c @@ -332,7 +332,7 @@ static void blk_mq_unregister_hctx(struct blk_mq_hw_ctx *hctx) struct blk_mq_ctx *ctx; int i; - if (!hctx->nr_ctx || !(hctx->flags & BLK_MQ_F_SYSFS_UP)) + if (!hctx->nr_ctx) return; hctx_for_each_ctx(hctx, ctx, i) @@ -347,7 +347,7 @@ static int blk_mq_register_hctx(struct blk_mq_hw_ctx *hctx) struct blk_mq_ctx *ctx; int i, ret; - if (!hctx->nr_ctx || !(hctx->flags & BLK_MQ_F_SYSFS_UP)) + if (!hctx->nr_ctx) return 0; ret = kobject_add(>kobj, >mq_kobj, "%u", hctx->queue_num); @@ -370,6 +370,8 @@ void blk_mq_unregister_disk(struct gendisk *disk) struct blk_mq_ctx *ctx; int i, j; + blk_mq_disable_hotplug(); + queue_for_each_hw_ctx(q, hctx, i) { blk_mq_unregister_hctx(hctx); @@ -384,6 +386,9 @@ void blk_mq_unregister_disk(struct gendisk *disk) kobject_put(>mq_kobj); kobject_put(_to_dev(disk)->kobj); + + q->mq_sysfs_init_done = false; + blk_mq_enable_hotplug(); } static void blk_mq_sysfs_init(struct request_queue *q) @@ -414,27 +419,30 @@ int blk_mq_register_disk(struct gendisk *disk) struct blk_mq_hw_ctx *hctx; int ret, i; + blk_mq_disable_hotplug(); + blk_mq_sysfs_init(q); ret = kobject_add(>mq_kobj, kobject_get(>kobj), "%s", "mq"); if (ret < 0) - return ret; + goto out; kobject_uevent(>mq_kobj, KOBJ_ADD); queue_for_each_hw_ctx(q, hctx, i) { - hctx->flags |= BLK_MQ_F_SYSFS_UP; ret = blk_mq_register_hctx(hctx); if (ret) break; } - if (ret) { + if (ret) blk_mq_unregister_disk(disk); - return ret; - } + else + q->mq_sysfs_init_done = true; +out: + blk_mq_enable_hotplug(); - return 0; + return ret; } EXPORT_SYMBOL_GPL(blk_mq_register_disk); @@ -443,6 +451,9 @@ void blk_mq_sysfs_unregister(struct request_queue *q) struct blk_mq_hw_ctx *hctx; int i; + if (!q->mq_sysfs_init_done) + return; + queue_for_each_hw_ctx(q, hctx, i) blk_mq_unregister_hctx(hctx); } @@ -452,6 +463,9 @@ int blk_mq_sysfs_register(struct request_queue *q) struct blk_mq_hw_ctx *hctx; int i, ret = 0; + if (!q->mq_sysfs_init_done) + return ret; + queue_for_each_hw_ctx(q, hctx, i) { ret = blk_mq_register_hctx(hctx); if (ret) diff --git a/block/blk-mq.c b/block/blk-mq.c index f29f766..68921b7 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -2045,13 +2045,13 @@ struct request_queue *blk_mq_init_allocated_queue(struct blk_mq_tag_set *set, goto
[PATCH v3 3/7] blk-mq: Fix use after of free q->mq_map
CPU hotplug handling for blk-mq (blk_mq_queue_reinit) updates q->mq_map by blk_mq_update_queue_map() for all request queues in all_q_list. On the other hand, q->mq_map is released before deleting the queue from all_q_list. So if CPU hotplug event occurs in the window, invalid memory access can happen. Fix it by releasing q->mq_map in blk_mq_release() to make it happen latter than removal from all_q_list. Signed-off-by: Akinobu Mita Suggested-by: Ming Lei Reviewed-by: Ming Lei Cc: Jens Axboe Cc: Ming Lei --- block/blk-mq.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/block/blk-mq.c b/block/blk-mq.c index 68921b7..9328405 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -1935,6 +1935,9 @@ void blk_mq_release(struct request_queue *q) kfree(hctx); } + kfree(q->mq_map); + q->mq_map = NULL; + kfree(q->queue_hw_ctx); /* ctx kobj stays in queue_ctx */ @@ -2080,11 +2083,6 @@ void blk_mq_free_queue(struct request_queue *q) blk_mq_free_hw_queues(q, set); percpu_ref_exit(>mq_usage_counter); - - kfree(q->mq_map); - - q->mq_map = NULL; - mutex_lock(_q_mutex); list_del_init(>all_q_node); mutex_unlock(_q_mutex); -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 1/7] blk-mq: avoid access hctx->tags->cpumask before allocation
When unmapped hw queue is remapped after CPU topology is changed, hctx->tags->cpumask is set before hctx->tags is allocated in blk_mq_map_swqueue(). In order to fix this null pointer dereference, hctx->tags must be allocated before configuring hctx->tags->cpumask. Fixes: f26cdc8536 ("blk-mq: Shared tag enhancements") Signed-off-by: Akinobu Mita Cc: Keith Busch Cc: Jens Axboe Cc: Ming Lei --- block/blk-mq.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/block/blk-mq.c b/block/blk-mq.c index 7d842db..f29f766 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -1821,7 +1821,6 @@ static void blk_mq_map_swqueue(struct request_queue *q) hctx = q->mq_ops->map_queue(q, i); cpumask_set_cpu(i, hctx->cpumask); - cpumask_set_cpu(i, hctx->tags->cpumask); ctx->index_hw = hctx->nr_ctx; hctx->ctxs[hctx->nr_ctx++] = ctx; } @@ -1861,6 +1860,14 @@ static void blk_mq_map_swqueue(struct request_queue *q) hctx->next_cpu = cpumask_first(hctx->cpumask); hctx->next_cpu_batch = BLK_MQ_CPU_WORK_BATCH; } + + queue_for_each_ctx(q, ctx, i) { + if (!cpu_online(i)) + continue; + + hctx = q->mq_ops->map_queue(q, i); + cpumask_set_cpu(i, hctx->tags->cpumask); + } } static void blk_mq_update_tag_set_depth(struct blk_mq_tag_set *set) -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Use after free bug in null_blk driver
2015-07-18 23:51 GMT+09:00 Mike Krinkin : > Hi, > > i noticed that loading null_blk with queue_mode=1 and irqmode=2 parameters > and slab poisoning enabled causes general protection fault: > > [ 20.671974] general protection fault: [#1] SMP > [ 20.678050] Modules linked in: null_blk(+) usbhid hid psmouse floppy > [ 20.688351] CPU: 0 PID: 147 Comm: modprobe Not tainted 4.2.0-rc2+ #76 > [ 20.695961] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS > Bochs 01/01/2011 > [ 20.705143] task: 88007f2ed100 ti: 88007f0a4000 task.ti: > 88007f0a4000 > [ 20.715511] RIP: 0010:[] [] > null_cmd_timer_expired+0x62/0xc0 [null_blk] > [ 20.730541] RSP: 0018:88007d203e78 EFLAGS: 00010082 > [ 20.737301] RAX: 88007ec16b80 RBX: 6b6b6b6b6b6b6b6b RCX: > > [ 20.747034] RDX: 88007f260888 RSI: RDI: > 88007f260850 > [ 20.762601] RBP: 88007d203e98 R08: 0001 R09: > > [ 20.771472] R10: 0001 R11: R12: > > [ 20.782522] R13: 88007d218780 R14: R15: > 88007d20fba8 > [ 20.792347] FS: 7f02e1500740() GS:88007d20() > knlGS: > [ 20.805319] CS: 0010 DS: ES: CR0: 8005003b > [ 20.813120] CR2: 7ffd52478dd8 CR3: 7f3dd000 CR4: > 07f0 > [ 20.825357] Stack: > [ 20.827764] 88007d218788 88007d20fa00 88007d20fac0 > a00280f0 > [ 20.838269] 88007d203f08 810f822b 00045dd35ed6 > 88007d20fa18 > [ 20.853377] 0001810f89e7 88007d20fa00 88007d20fb28 > 00045dd35ed6 > [ 20.863415] Call Trace: > [ 20.866398] > [ 20.869088] [] ? null_softirq_done_fn+0x30/0x30 > [null_blk] > [ 20.880219] [] __hrtimer_run_queues+0x11b/0x6e0 > [ 20.891048] [] hrtimer_interrupt+0xab/0x1b0 > [ 20.899542] [] local_apic_timer_interrupt+0x3c/0x70 > [ 20.911730] [] smp_apic_timer_interrupt+0x41/0x60 > [ 20.921854] [] apic_timer_interrupt+0x70/0x80 > [ 20.931084] > [ 20.933406] [] ? _raw_spin_unlock_irqrestore+0x3b/0x60 > [ 20.945250] [] hrtimer_start_range_ns+0x1a7/0x540 > [ 20.955952] [] ? trace_hardirqs_on_caller+0x151/0x1e0 > [ 20.963974] [] null_request_fn+0xd3/0x100 [null_blk] > [ 20.977159] [] __blk_run_queue+0x37/0x50 > [ 20.983782] [] __elv_add_request+0x15d/0x440 > [ 20.991540] [] blk_queue_bio+0x415/0x520 > [ 20.998876] [] generic_make_request+0xcc/0x110 > [ 21.007095] [] submit_bio+0x67/0x150 > [ 21.013603] [] submit_bh_wbc+0x14c/0x180 > [ 21.019730] [] block_read_full_page+0x2b0/0x3a0 > [ 21.028023] [] ? I_BDEV+0x20/0x20 > [ 21.034575] [] ? blkdev_readpages+0x20/0x20 > [ 21.040947] [] blkdev_readpage+0x18/0x20 > [ 21.048237] [] do_read_cache_page+0x7d/0x1a0 > [ 21.056016] [] read_cache_page+0x1c/0x20 > [ 21.063300] [] read_dev_sector+0x30/0x90 > [ 21.069804] [] ? check_partition+0x1f0/0x1f0 > [ 21.076932] [] adfspart_check_ICS+0x42/0x250 > [ 21.084317] [] ? snprintf+0x34/0x40 > [ 21.090670] [] ? check_partition+0x1f0/0x1f0 > [ 21.097864] [] check_partition+0x100/0x1f0 > [ 21.104535] [] rescan_partitions+0xa8/0x2b0 > [ 21.112087] [] __blkdev_get+0x313/0x460 > [ 21.120037] [] blkdev_get+0x41/0x3a0 > [ 21.126266] [] ? unlock_new_inode+0x5d/0x90 > [ 21.134043] [] ? bdget+0x130/0x150 > [ 21.140369] [] ? disk_get_part+0xd/0x1d0 > [ 21.147357] [] add_disk+0x436/0x4d0 > [ 21.153846] [] ? sprintf+0x40/0x50 > [ 21.159867] [] null_init+0x38e/0x1000 [null_blk] > [ 21.167691] [] ? 0xa004e000 > [ 21.174266] [] do_one_initcall+0xb3/0x1e0 > [ 21.182276] [] ? kmem_cache_alloc_trace+0x36f/0x3b0 > [ 21.190291] [] do_init_module+0x61/0x1ec > [ 21.197112] [] load_module+0x24fe/0x2810 > [ 21.204237] [] ? m_show+0x1a0/0x1a0 > [ 21.213498] [] SyS_finit_module+0x80/0xb0 > [ 21.221360] [] entry_SYSCALL_64_fastpath+0x16/0x7a > [ 21.230181] Code: e8 24 40 3b e1 48 89 c3 eb 08 4d 85 e4 4c 89 e3 74 e2 48 > 8d 7b f0 4c 8b 23 e8 db fe ff ff 48 8b 43 28 48 85 c0 74 e3 48 8b 58 30 <48> > 83 bb 38 01 00 00 00 75 d5 48 8b 83 98 08 00 00 a8 04 74 ca > [ 21.281572] RIP [] null_cmd_timer_expired+0x62/0xc0 > [null_blk] > [ 21.289770] RSP > [ 21.294601] ---[ end trace cd40ca6c9001dd7e ]--- > [ 21.299719] Kernel panic - not syncing: Fatal exception in interrupt > [ 21.309176] Kernel Offset: disabled > [ 21.312773] ---[ end Kernel panic - not syncing: Fatal exception in > interrupt > > I suppose, it was introduced in commit 8b70f45e2eb2 ("null_blk: restart > request > processing on completion handler") by Akinobu Mita . > The Right. That commit introduced use-after-free bug. > following fix solves problem for me: > > diff --git a/drivers/block/null_blk.c b/drivers/block/null_blk.c > index 69de41a..910f354 100644 > --- a/drivers/block/null_blk.c > +++ b/drivers/block/null_blk.c > @@
[PATCH] clk: Add a basic factor clock
Some clocks are using a factor component, however, unlike their mux, gate or divider counterpart, these factors don't have a basic clock implementation. This leads to code duplication across platforms that want to use that kind of clocks, and the impossibility to use the composite clocks with such a clock without defining your own rate operations. Create such a driver in order to remove these issues, and hopefully factor the implementations, reducing code size across platforms and consolidating the various implementations. Signed-off-by: Maxime Ripard --- drivers/clk/Makefile | 1 + drivers/clk/clk-factor.c | 176 +++ include/linux/clk-provider.h | 41 ++ 3 files changed, 218 insertions(+) create mode 100644 drivers/clk/clk-factor.c diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile index c4cf075a2320..00f8c7fd1196 100644 --- a/drivers/clk/Makefile +++ b/drivers/clk/Makefile @@ -3,6 +3,7 @@ obj-$(CONFIG_HAVE_CLK) += clk-devres.o obj-$(CONFIG_CLKDEV_LOOKUP)+= clkdev.o obj-$(CONFIG_COMMON_CLK) += clk.o obj-$(CONFIG_COMMON_CLK) += clk-divider.o +obj-$(CONFIG_COMMON_CLK) += clk-factor.o obj-$(CONFIG_COMMON_CLK) += clk-fixed-factor.o obj-$(CONFIG_COMMON_CLK) += clk-fixed-rate.o obj-$(CONFIG_COMMON_CLK) += clk-gate.o diff --git a/drivers/clk/clk-factor.c b/drivers/clk/clk-factor.c new file mode 100644 index ..17f83852cdb6 --- /dev/null +++ b/drivers/clk/clk-factor.c @@ -0,0 +1,176 @@ +/* + * Copyright (C) 2015 Maxime Ripard + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ +#include +#include +#include +#include +#include + +#define to_clk_factor(_hw) container_of(_hw, struct clk_factor, hw) + +static unsigned long __get_mult(struct clk_factor *factor, + unsigned long rate, + unsigned long parent_rate) +{ + if (factor->flags & CLK_FACTOR_ROUND_CLOSEST) + return DIV_ROUND_CLOSEST(rate, parent_rate); + + return rate / parent_rate; +} + +static unsigned long clk_factor_recalc_rate(struct clk_hw *hw, + unsigned long parent_rate) +{ + struct clk_factor *factor = to_clk_factor(hw); + unsigned long val; + + val = clk_readl(factor->reg) >> factor->shift; + val &= GENMASK(factor->width, 0); + + if (!val && factor->flags & CLK_FACTOR_ZERO_BYPASS) + val = 1; + + return parent_rate * val; +} + +static bool __is_best_rate(unsigned long rate, unsigned long new, + unsigned long best, unsigned long flags) +{ + if (flags & CLK_FACTOR_ROUND_CLOSEST) + return abs(rate - new) < abs(rate - best); + + return new >= rate && new < best; +} + +static unsigned long clk_factor_bestmult(struct clk_hw *hw, unsigned long rate, +unsigned long *best_parent_rate, +u8 width, unsigned long flags) +{ + unsigned long orig_parent_rate = *best_parent_rate; + unsigned long parent_rate, current_rate, best_rate = ~0; + unsigned int i, bestmult = 0; + + if (!(__clk_get_flags(hw->clk) & CLK_SET_RATE_PARENT)) + return rate / *best_parent_rate; + + for (i = 1; i < ((1 << width) - 1); i++) { + if (rate * i == orig_parent_rate) { + /* +* This is the best case for us if we have a +* perfect match without changing the parent +* rate. +*/ + *best_parent_rate = orig_parent_rate; + return i; + } + + parent_rate = __clk_round_rate(__clk_get_parent(hw->clk), + rate / i); + current_rate = parent_rate * i; + + if (__is_best_rate(rate, current_rate, best_rate, flags)) { + bestmult = i; + best_rate = current_rate; + *best_parent_rate = parent_rate; + } + } + + return bestmult; +} + +static long clk_factor_round_rate(struct clk_hw *hw, unsigned long rate, + unsigned long *parent_rate) +{ + struct clk_factor *factor = to_clk_factor(hw); + unsigned long mult = clk_factor_bestmult(hw, rate, parent_rate, +factor->width, factor->flags); + + return *parent_rate * mult; +} + +static int clk_factor_set_rate(struct clk_hw *hw, unsigned long rate, + unsigned long parent_rate) +{ + struct clk_factor *factor = to_clk_factor(hw); +
Re: [RFC PATCH 02/21] stackvalidate: Add C version of STACKVALIDATE_IGNORE_INSN
On Sat, Jul 18, 2015 at 04:56:54PM +0200, Borislav Petkov wrote: > On Fri, Jul 17, 2015 at 11:47:18AM -0500, Josh Poimboeuf wrote: > > Add a C inline asm string version of the STACKVALIDATE_IGNORE_INSN macro > > which tells stackvalidate to ignore the subsequent instruction. > > > > Signed-off-by: Josh Poimboeuf > > --- > > arch/x86/include/asm/stackvalidate.h | 9 + > > 1 file changed, 9 insertions(+) > > > > diff --git a/arch/x86/include/asm/stackvalidate.h > > b/arch/x86/include/asm/stackvalidate.h > > index 2d23c23..dac935c 100644 > > --- a/arch/x86/include/asm/stackvalidate.h > > +++ b/arch/x86/include/asm/stackvalidate.h > > @@ -23,6 +23,15 @@ > > .endif > > .endm > > > > +#else /* !__ASSEMBLY__ */ > > + > > #ifdef CONFIG_STACK_VALIDATION > > > +#define STACKVALIDATE_IGNORE_INSN \ > > + ".Ltemp" __stringify(__LINE__) ":;" \ > > + ".pushsection __stackvalidate_ignore_insn, \"a\";" \ > > + _ASM_ALIGN ";" \ > > + ".long .Ltemp" __stringify(__LINE__) " - .;"\ > > + ".popsection;" > > + > > #endif > > > Also, you should end your lines with "\n" so that the .s output looks > a bit more readable, not like now: > > #APP > # 30 "./arch/x86/include/asm/arch_hweight.h" 1 > 661: > .Ltemp32:;.pushsection __stackvalidate_ignore_insn, "a"; .balign 8 > ;.long .Ltemp32 - .;.popsection;call __sw_hweight32 > 662: Ok, will fix both issues. Thanks. -- Josh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 04/21] x86/hweight: Add stack frame dependency for __arch_hweight*()
On Sat, Jul 18, 2015 at 04:56:29PM +0200, Borislav Petkov wrote: > On Sat, Jul 18, 2015 at 08:44:15AM -0500, Josh Poimboeuf wrote: > > Ok, so would you rather adding a whitelist to tell stackvalidate to > > ignore it? Something like this? > > I tried it and maybe I'm missing something but that doesn't work: > > $ make drivers/gpu/drm/i915/intel_ringbuffer.o > CHK include/config/kernel.release > CHK include/generated/uapi/linux/version.h > CHK include/generated/utsrelease.h > CHK include/generated/timeconst.h > CHK include/generated/bounds.h > CHK include/generated/asm-offsets.h > CALLscripts/checksyscalls.sh > CC drivers/gpu/drm/i915/intel_ringbuffer.o > ./arch/x86/include/asm/arch_hweight.h: Assembler messages: > ./arch/x86/include/asm/arch_hweight.h:31: Error: symbol `.Ltemp32' is already > defined > ./arch/x86/include/asm/arch_hweight.h:31: Error: symbol `.Ltemp32' is already > defined > ./arch/x86/include/asm/arch_hweight.h:31: Error: symbol `.Ltemp32' is already > defined > scripts/Makefile.build:258: recipe for target > 'drivers/gpu/drm/i915/intel_ringbuffer.o' failed > make[1]: *** [drivers/gpu/drm/i915/intel_ringbuffer.o] Error 1 > Makefile:1528: recipe for target 'drivers/gpu/drm/i915/intel_ringbuffer.o' > failed > make: *** [drivers/gpu/drm/i915/intel_ringbuffer.o] Error 2 Yeah, it doesn't actually support this particular example yet. I was just trying to figure out if that's what you were proposing. > Also, that label temp32 could be more descriptive. Yeah, that's from: ".Ltemp" __stringify(__LINE__) ":;" Which was intended to give a unique ID for each use of the macro, but apparently that didn't work as planned here. > so you see that a CALL instruction gets replaced with a POPCNT and > the feature bit used is 4*32+23 which is X86_FEATURE_POPCNT. This > information is enough to detect that particular case and add the offset > ".long 661b - ." to the list of instructions which stackvalidate should > ignore. Currently, when stackvalidate sees an ALTERNATIVE, it assumes that either code path is possible, so it follows both paths in parallel. If I understand right, you're proposing that stackvalidate should only follow the POPCNT path and never follow the !POPCNT path? > Anyway, this is what I'd do. > > IMNSVHO, we must be very conservative and not add some > markers/helpers/etc to code only so that tools can do their job. Not if > it can be helped. Instead, tools should do the hard work and we should > keep kernel code clean. In general, I agree, and I like the original patch much better. IMO, it achieved the goal of keeping the kernel code clean, while fixing the frame pointer bug. If you insist on breaking stack traces on !POPCNT, I can probably add some intelligence to stackvalidate to look for !POPCNT and ignore it. It seems less "clean" to me, though. -- Josh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] workqueue: avoiding unbounded wq on isolated CPUs by default
On Sat, 2015-07-18 at 15:36 +0200, Frederic Weisbecker wrote: > On Fri, Jul 17, 2015 at 07:15:48PM +0200, Mike Galbraith wrote: > > On Fri, 2015-07-17 at 11:27 -0400, Tejun Heo wrote: > > > > > I'm just curious whether there was any specific reason we didn't do > > > this before (ISTR people discussing it back then too). > > > > I'm dead set against all this auto-presume nonsense fwtw Allocating a > > pool of no_hz_full _capable_ CPUs should not entice the kernel to make > > any rash assumptions. Let users do the button poking, they know what > > they want, and when they want it. > > We need to make a choice then. Either we do all the affinity tuning from > userspace with a common tool, which is what I had wished before everybody > asked for pre-settings. Giving userspace what they need to do what they want seems right to me. > Or we do it in the kernel, now we should define some kind of CONFIG_ISOLATION > to make that proper and rule the various kinds of isolation people are > interested in. > > But we can't leave it half-way like it is currently with everything preset on > top of nohz: rcu nocb mask, watchdog mask, cpu_isolation_map and exclude > workqueue. Yeah. Hell, maybe I'm wrong. Maybe people really want this rigidity and hand-holding by the kernel, but it just seems dainbramaged to me. ATM, you pay a high price (the overhead) for the capability, but until that auto-assume isolcpus landed, those CPUs weren't forever more specialists, they were CPUs with an extra (costly) capability, could be disconnected/reconnected to load balancing on the fly, and used however the user saw fit. I can imagine an auto-everything kernel having a bit of trouble with an SGI beast from hell. Too bad I don't have access to one, I'd try to boot a tune for maximum hand holding kernel. -Mike -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 13/21] x86/asm/crypto: Fix frame pointer usage in aesni-intel_asm.S
On Sat, Jul 18, 2015 at 04:25:25PM +0200, Borislav Petkov wrote: > On Sat, Jul 18, 2015 at 08:46:55AM -0500, Josh Poimboeuf wrote: > > I like the balance, but the "ret" is still non-obvious. > > Does it have to be obvious? I feel that making "ret" obvious is better. But if somebody messes up and adds a second "ret", I suppose stackvalidate would warn about the fact that it returned without restoring the frame pointer. So if there are no other objections, your suggestion of ENTRY_FRAME and ENDPROC_FRAME is fine with me. -- Josh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] net: dsa: mv88e6xxx: sleep in _mv88e6xxx_stats_wait
On Sat, Jul 18, 2015 at 11:23:19AM -0400, Vivien Didelot wrote: > Hi all, > > - On Jul 18, 2015, at 10:58 AM, Andrew Lunn and...@lunn.ch wrote: > > >> Good point. The timeout is most definitely quite large and for sure on > >> the safe side. It might make sense to add some statistics gathering to > >> see how long the maximum observed delay actually is. > > > > Hi All > > > > Statistics are something which can be used a lot, i bursts and > > interactivily. ATU, VTU etc, are much less often used. So different > > delays might be justified. > > > > I agree about doing some statistics gathering to determine actual > > delays needed. > > > >Andrew > > What do you think about something like this? Hi Vivien Lets get some actually statistics first. I would suggest for testing you make _mv88e6xxx_wait() a busy loop and time how long it actually takes for different busy bits to clear. We should also test it on different families. Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/9] perf, tools: Add tools support for cycles, weight branch_info field
From: Andi Kleen cycles is a new branch_info field available on some CPUs that indicates the time deltas between branches in the LBR. Add a sort key and output code for the cycles to allow to display the basic block cycles individually in perf report. We also pass in the cycles for weight when LBRs are processed, which allows to get global and local weight, to get an estimate of the total cost. And also print the cycles information for perf report -D. I also added printing for the previously missing LBR flags (mispredict etc.) Acked-by: Jiri Olsa Signed-off-by: Andi Kleen --- tools/perf/Documentation/perf-report.txt | 1 + tools/perf/util/event.h | 3 ++- tools/perf/util/hist.c | 3 ++- tools/perf/util/hist.h | 1 + tools/perf/util/session.c| 16 tools/perf/util/sort.c | 24 tools/perf/util/sort.h | 1 + 7 files changed, 43 insertions(+), 6 deletions(-) diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt index c33b69f..960da20 100644 --- a/tools/perf/Documentation/perf-report.txt +++ b/tools/perf/Documentation/perf-report.txt @@ -109,6 +109,7 @@ OPTIONS - mispredict: "N" for predicted branch, "Y" for mispredicted branch - in_tx: branch in TSX transaction - abort: TSX transaction abort. + - cycles: Cycles in basic block And default sort keys are changed to comm, dso_from, symbol_from, dso_to and symbol_to, see '--branch-stack'. diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h index c53f363..ec175ca 100644 --- a/tools/perf/util/event.h +++ b/tools/perf/util/event.h @@ -134,7 +134,8 @@ struct branch_flags { u64 predicted:1; u64 in_tx:1; u64 abort:1; - u64 reserved:60; + u64 cycles:16; + u64 reserved:44; }; struct branch_entry { diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c index 6f28d53..54fc003 100644 --- a/tools/perf/util/hist.c +++ b/tools/perf/util/hist.c @@ -618,7 +618,8 @@ iter_add_next_branch_entry(struct hist_entry_iter *iter, struct addr_location *a * and not events sampled. Thus we use a pseudo period of 1. */ he = __hists__add_entry(hists, al, iter->parent, [i], NULL, - 1, 1, 0, true); + 1, bi->flags.cycles ? bi->flags.cycles : 1, + 0, true); if (he == NULL) return -ENOMEM; diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h index 5ed8d9c..3881d98 100644 --- a/tools/perf/util/hist.h +++ b/tools/perf/util/hist.h @@ -47,6 +47,7 @@ enum hist_column { HISTC_MEM_SNOOP, HISTC_MEM_DCACHELINE, HISTC_TRANSACTION, + HISTC_CYCLES, HISTC_NR_COLS, /* Last entry */ }; diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c index ed9dc25..e495127c 100644 --- a/tools/perf/util/session.c +++ b/tools/perf/util/session.c @@ -766,10 +766,18 @@ static void branch_stack__printf(struct perf_sample *sample) printf("... branch stack: nr:%" PRIu64 "\n", sample->branch_stack->nr); - for (i = 0; i < sample->branch_stack->nr; i++) - printf(". %2"PRIu64": %016" PRIx64 " -> %016" PRIx64 "\n", - i, sample->branch_stack->entries[i].from, - sample->branch_stack->entries[i].to); + for (i = 0; i < sample->branch_stack->nr; i++) { + struct branch_entry *e = >branch_stack->entries[i]; + + printf(". %2"PRIu64": %016" PRIx64 " -> %016" PRIx64 " %hu cycles %s%s%s%s %x\n", + i, e->from, e->to, + e->flags.cycles, + e->flags.mispred ? "M" : " ", + e->flags.predicted ? "P" : " ", + e->flags.abort ? "A" : " ", + e->flags.in_tx ? "T" : " ", + (unsigned)e->flags.reserved); + } } static void regs_dump__printf(u64 mask, u64 *regs) diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c index 4c65a14..5b7a50c 100644 --- a/tools/perf/util/sort.c +++ b/tools/perf/util/sort.c @@ -526,6 +526,29 @@ static int hist_entry__mispredict_snprintf(struct hist_entry *he, char *bf, return repsep_snprintf(bf, size, "%-*.*s", width, width, out); } +static int64_t +sort__cycles_cmp(struct hist_entry *left, struct hist_entry *right) +{ + return left->branch_info->flags.cycles - + right->branch_info->flags.cycles; +} + +static int hist_entry__cycles_snprintf(struct hist_entry *he, char *bf, + size_t size, unsigned int width) +{ + if (he->branch_info->flags.cycles == 0) + return repsep_snprintf(bf, size, "%-*s", width, "-"); + return repsep_snprintf(bf, size, "%-*hd",
[PATCH 3/9] perf, tools, report: Add infrastructure for a cycles histogram
From: Andi Kleen This adds the basic infrastructure to keep track of cycle counts per basic block for annotate. We allocate an array similar to the normal accounting, and then account branch cycles there. We handle two cases: cycles per basic block with start and cycles per branch (these are later used for either IPC or just cycles per BB) In the start case we cannot handle overlaps, so always the longest basic block wins. For the cycles per branch case everything is accurately accounted. v2: Remove unnecessary checks. Slight restructure. Move symbol__get_annotation to another patch. Move histogram allocation. v3: Merged with current tree Acked-by: Jiri Olsa Signed-off-by: Andi Kleen --- tools/perf/builtin-annotate.c | 1 + tools/perf/util/annotate.c| 127 +- tools/perf/util/annotate.h| 17 ++ 3 files changed, 142 insertions(+), 3 deletions(-) diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c index 2c1bec3..467a23b 100644 --- a/tools/perf/builtin-annotate.c +++ b/tools/perf/builtin-annotate.c @@ -187,6 +187,7 @@ find_next: * symbol, free he->ms.sym->src to signal we already * processed this symbol. */ + zfree(>src->cycles_hist); zfree(>src); } } diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index 03b7bc70..e0b6146 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c @@ -473,17 +473,73 @@ int symbol__alloc_hist(struct symbol *sym) return 0; } +/* The cycles histogram is lazily allocated. */ +static int symbol__alloc_hist_cycles(struct symbol *sym) +{ + struct annotation *notes = symbol__annotation(sym); + const size_t size = symbol__size(sym); + + notes->src->cycles_hist = calloc(size, sizeof(struct cyc_hist)); + if (notes->src->cycles_hist == NULL) + return -1; + return 0; +} + void symbol__annotate_zero_histograms(struct symbol *sym) { struct annotation *notes = symbol__annotation(sym); pthread_mutex_lock(>lock); - if (notes->src != NULL) + if (notes->src != NULL) { memset(notes->src->histograms, 0, notes->src->nr_histograms * notes->src->sizeof_sym_hist); + if (notes->src->cycles_hist) + memset(notes->src->cycles_hist, 0, + symbol__size(sym) * sizeof(struct cyc_hist)); + } pthread_mutex_unlock(>lock); } +static int __symbol__account_cycles(struct annotation *notes, + u64 start, + unsigned offset, unsigned cycles, + unsigned have_start) +{ + struct cyc_hist *ch; + + ch = notes->src->cycles_hist; + /* +* For now we can only account one basic block per +* final jump. But multiple could be overlapping. +* Always account the longest one. So when +* a shorter one has been already seen throw it away. +* +* We separately always account the full cycles. +*/ + ch[offset].num_aggr++; + ch[offset].cycles_aggr += cycles; + + if (!have_start && ch[offset].have_start) + return 0; + if (ch[offset].num) { + if (have_start && (!ch[offset].have_start || + ch[offset].start > start)) { + ch[offset].have_start = 0; + ch[offset].cycles = 0; + ch[offset].num = 0; + if (ch[offset].reset < 0x) + ch[offset].reset++; + } else if (have_start && + ch[offset].start < start) + return 0; + } + ch[offset].have_start = have_start; + ch[offset].start = start; + ch[offset].cycles += cycles; + ch[offset].num++; + return 0; +} + static int __symbol__inc_addr_samples(struct symbol *sym, struct map *map, struct annotation *notes, int evidx, u64 addr) { @@ -506,7 +562,7 @@ static int __symbol__inc_addr_samples(struct symbol *sym, struct map *map, return 0; } -static struct annotation *symbol__get_annotation(struct symbol *sym) +static struct annotation *symbol__get_annotation(struct symbol *sym, bool cycles) { struct annotation *notes = symbol__annotation(sym); @@ -514,6 +570,10 @@ static struct annotation *symbol__get_annotation(struct symbol *sym) if (symbol__alloc_hist(sym) < 0) return NULL; } + if (!notes->src->cycles_hist && cycles) { + if (symbol__alloc_hist_cycles(sym) < 0) + return NULL; + } return notes; } @@ -524,12
Cycles annotation support for perf tools v3
[v2: Addressed review comments. Fixed display problems and correctly compute IPC now. See patches for detailed changes.] [v3: Merged with current Arnaldo perf/core and added acked-by.] [Note the respective kernel patches to report cycles are in peterz's perf/core queue, but so far not in tip. The patchkit can be tested however with the "fake cycles" debug patch added at the end] The upcoming Skylake CPU has a new timed branch stack feature, that reports cycle counts for individual branches in the last branch record. This allows to get fine grained cost information for code, and also allows to compute fine grained IPC. Available from git://git.kernel.org/pub/scm/linux/kernel/git/ak/linux-misc.git perf/skl-tools3 This patchkit adds support for this in the perf tools: - Basic support for the cycles field like other branch fields - Show cycles in the standard branch sort view (no IPC here, as IPC needs the instruction counts from annotation) - Annotate cycles and IPC in the assembler annotate view - Add branch support to top, so we can do live annotation. - Misc support, like dumping it in perf report -D Example output for annotate (with made up numbers): The second column is the IPC and third average cycles for the basic block. │static int hex(char ch) ▒ │{ ▒ 0.12 │ push %rbp ◆ 0.12 │ mov%rsp,%rbp ▒ 0.12 │ sub$0x20,%rsp ▒ 0.12 │ mov%edi,%eax ▒ 0.12 │ mov%al,-0x14(%rbp) ▒ 0.12 │ mov%fs:0x28,%rax ▒ 0.12 │ mov%rax,-0x8(%rbp) ▒ 0.12 │ xor%eax,%eax ▒ │if ((ch >= '0') && (ch <= '9')) ▒ 0.12 │ cmpb $0x2f,-0x14(%rbp) ▒ 66.67 0.12 123 │↓ jle31 ▒ 0.12 │ cmpb $0x39,-0x14(%rbp) ▒ 0.12 123 │↓ jg 31 ▒ │return ch - '0'; ▒ 22.22 0.12 │ movsbl -0x14(%rbp),%eax ▒ 0.12 │ sub$0x30,%eax ▒ 0.12 123 │↓ jmp60 ▒ │if ((ch >= 'a') && (ch <= 'f')) ▒ 0.06 │31: cmpb $0x60,-0x14(%rbp) ▒ 0.06 123 │↓ jle46 ▒ 0.06 │ cmpb $0x66,-0x14(%rbp) ▒ 0.06 │↓ jg 46 ▒ │return ch - 'a' + 10;
[PATCH 9/9] test patch: Add fake branch cycles to input data in report/top
From: Andi Kleen Not to be merged, but useful for testing if you don't have hardware with cycles branch stack support. --- tools/perf/util/hist.c| 2 +- tools/perf/util/machine.c | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c index a6e9ddd..8a4bf84 100644 --- a/tools/perf/util/hist.c +++ b/tools/perf/util/hist.c @@ -1421,7 +1421,7 @@ void hist__account_cycles(struct branch_stack *bs, struct addr_location *al, struct branch_info *bi; /* If we have branch cycles always annotate them. */ - if (bs && bs->nr && bs->entries[0].flags.cycles) { + if (bs && bs->nr /* && bs->entries[0].flags.cycles */) { int i; bi = sample__resolve_bstack(sample, al); diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c index 7ff6827..1351f19 100644 --- a/tools/perf/util/machine.c +++ b/tools/perf/util/machine.c @@ -1597,6 +1597,8 @@ struct branch_info *sample__resolve_bstack(struct perf_sample *sample, ip__resolve_ams(al->thread, [i].to, bs->entries[i].to); ip__resolve_ams(al->thread, [i].from, bs->entries[i].from); bi[i].flags = bs->entries[i].flags; + if (bi[i].flags.cycles == 0) + bi[i].flags.cycles = 123; } return bi; } -- 2.4.3 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 7/9] perf, tools, top: Add branch annotation code to top
From: Andi Kleen Now that we can process branch data in annotate it makes sense to support enabling branch recording from top too. Most of the code needed for this is already in shared code with report. But we need to add: - The option parsing code (using shared code from the previous patch) - Document the options - Set up the IPC/cycles accounting state in the top session - Call the accounting code in the hist iter callback Signed-off-by: Andi Kleen --- tools/perf/Documentation/perf-top.txt | 21 + tools/perf/builtin-top.c | 10 +- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/tools/perf/Documentation/perf-top.txt b/tools/perf/Documentation/perf-top.txt index 776aec4..f6a23eb 100644 --- a/tools/perf/Documentation/perf-top.txt +++ b/tools/perf/Documentation/perf-top.txt @@ -208,6 +208,27 @@ Default is to monitor all CPUS. This option sets the time out limit. The default value is 500 ms. +-b:: +--branch-any:: + Enable taken branch stack sampling. Any type of taken branch may be sampled. + This is a shortcut for --branch-filter any. See --branch-filter for more infos. + +-j:: +--branch-filter:: + Enable taken branch stack sampling. Each sample captures a series of consecutive + taken branches. The number of branches captured with each sample depends on the + underlying hardware, the type of branches of interest, and the executed code. + It is possible to select the types of branches captured by enabling filters. + For a full list of modifiers please see the perf record manpage. + + The option requires at least one branch type among any, any_call, any_ret, ind_call, cond. + The privilege levels may be omitted, in which case, the privilege levels of the associated + event are applied to the branch filter. Both kernel (k) and hypervisor (hv) privilege + levels are subject to permissions. When sampling on multiple events, branch stack sampling + is enabled for all the sampling events. The sampled branch type is the same for all events. + The various filters must be specified as a comma separated list: --branch-filter any_ret,u,k + Note that this feature may not be available on all processors. + INTERACTIVE PROMPTING KEYS -- diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c index ecf3197..f0a5240 100644 --- a/tools/perf/builtin-top.c +++ b/tools/perf/builtin-top.c @@ -40,6 +40,7 @@ #include "util/xyarray.h" #include "util/sort.h" #include "util/intlist.h" +#include "util/parse-branch-options.h" #include "arch/common.h" #include "util/debug.h" @@ -695,6 +696,8 @@ static int hist_iter__top_callback(struct hist_entry_iter *iter, perf_top__record_precise_ip(top, he, evsel->idx, ip); } + hist__account_cycles(iter->sample->branch_stack, al, iter->sample, +!(top->record_opts.branch_stack & PERF_SAMPLE_BRANCH_ANY)); return 0; } @@ -932,7 +935,6 @@ static int perf_top__setup_sample_type(struct perf_top *top __maybe_unused) return -EINVAL; } } - return 0; } @@ -1171,6 +1173,12 @@ int cmd_top(int argc, const char **argv, const char *prefix __maybe_unused) "don't try to adjust column width, use these fixed values"), OPT_UINTEGER(0, "proc-map-timeout", >proc_map_timeout, "per thread proc mmap processing timeout in ms"), + OPT_CALLBACK_NOOPT('b', "branch-any", >branch_stack, +"branch any", "sample any taken branches", +parse_branch_stack), + OPT_CALLBACK('j', "branch-filter", >branch_stack, +"branch filter mask", "branch stack filter modes", +parse_branch_stack), OPT_END() }; const char * const top_usage[] = { -- 2.4.3 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 4/9] perf, tools, report: Add processing for cycle histograms
From: Andi Kleen Call the earlier added cycle histogram infrastructure from the perf report hist iter callback. For this we walk the branch records. This allows to use cycle histograms when browsing perf report annotate. v2: Rename flag Signed-off-by: Andi Kleen --- tools/perf/builtin-report.c | 3 +++ tools/perf/util/hist.c | 33 + tools/perf/util/hist.h | 3 +++ 3 files changed, 39 insertions(+) diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c index 3ba0e97..3a9d1b6 100644 --- a/tools/perf/builtin-report.c +++ b/tools/perf/builtin-report.c @@ -103,6 +103,9 @@ static int hist_iter__report_callback(struct hist_entry_iter *iter, if (!ui__has_annotation()) return 0; + hist__account_cycles(iter->sample->branch_stack, al, iter->sample, +rep->nonany_branch_mode); + if (sort__mode == SORT_MODE__BRANCH) { bi = he->branch_info; err = addr_map_symbol__inc_samples(>from, evsel->idx); diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c index 54fc003..a6e9ddd 100644 --- a/tools/perf/util/hist.c +++ b/tools/perf/util/hist.c @@ -1415,6 +1415,39 @@ int hists__link(struct hists *leader, struct hists *other) return 0; } +void hist__account_cycles(struct branch_stack *bs, struct addr_location *al, + struct perf_sample *sample, bool nonany_branch_mode) +{ + struct branch_info *bi; + + /* If we have branch cycles always annotate them. */ + if (bs && bs->nr && bs->entries[0].flags.cycles) { + int i; + + bi = sample__resolve_bstack(sample, al); + if (bi) { + struct addr_map_symbol *prev = NULL; + + /* +* Ignore errors, still want to process the +* other entries. +* +* For non standard branch modes always +* force no IPC (prev == NULL) +* +* Note that perf stores branches reversed from +* program order! +*/ + for (i = bs->nr - 1; i >= 0; i--) { + addr_map_symbol__account_cycles([i].from, + nonany_branch_mode ? NULL : prev, + bi[i].flags.cycles); + prev = [i].to; + } + free(bi); + } + } +} size_t perf_evlist__fprintf_nr_events(struct perf_evlist *evlist, FILE *fp) { diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h index 3881d98..e2f712f 100644 --- a/tools/perf/util/hist.h +++ b/tools/perf/util/hist.h @@ -350,6 +350,9 @@ static inline int script_browse(const char *script_opt __maybe_unused) unsigned int hists__sort_list_width(struct hists *hists); +void hist__account_cycles(struct branch_stack *bs, struct addr_location *al, + struct perf_sample *sample, bool nonany_branch_mode); + struct option; int parse_filter_percentage(const struct option *opt __maybe_unused, const char *arg, int unset __maybe_unused); -- 2.4.3 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 6/9] perf, tools, annotate: Finally display IPC and cycle accounting
From: Andi Kleen Add two new columns to the annotate display and display the average cycles and the compute IPC if available. When the LBR was not in any branch mode the IPC computation is automatically disabled. We still display the cycle information. Example output (with made up numbers): The second column is the IPC and third average cycles. │__attribute__((noinline)) f2() │{ 5.15 0.07 │ push %rbp 0.01 0.07 │ mov%rsp,%rbp │c = a / b; 9.87 0.07 │ mova,%eax 0.07 │ movb,%ecx 0.07 │ cltd 4.92 0.07 123│ idiv %ecx 70.79 0.07 │ mov%eax,__TMC_END__ │} 9.25 0.07 │ pop%rbp 0.01 0.07 123│ ← retq v2: Fix display problems. Signed-off-by: Andi Kleen --- tools/perf/ui/browsers/annotate.c | 57 +++ 1 file changed, 40 insertions(+), 17 deletions(-) diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c index 6ec1795..b5fc847 100644 --- a/tools/perf/ui/browsers/annotate.c +++ b/tools/perf/ui/browsers/annotate.c @@ -16,6 +16,9 @@ struct disasm_line_samples { u64 nr; }; +#define IPC_WIDTH 6 +#define CYCLES_WIDTH 6 + struct browser_disasm_line { struct rb_node rb_node; u32 idx; @@ -97,6 +100,15 @@ static int annotate_browser__set_jumps_percent_color(struct annotate_browser *br return ui_browser__set_color(>b, color); } +static int annotate_browser__pcnt_width(struct annotate_browser *ab) +{ + int w = 7 * ab->nr_events; + + if (ab->have_cycles) + w += IPC_WIDTH + CYCLES_WIDTH; + return w; +} + static void annotate_browser__write(struct ui_browser *browser, void *entry, int row) { struct annotate_browser *ab = container_of(browser, struct annotate_browser, b); @@ -107,7 +119,7 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int (!current_entry || (browser->use_navkeypressed && !browser->navkeypressed))); int width = browser->width, printed; - int i, pcnt_width = 7 * ab->nr_events; + int i, pcnt_width = annotate_browser__pcnt_width(ab); double percent_max = 0.0; char bf[256]; @@ -117,19 +129,34 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int } if (dl->offset != -1 && percent_max != 0.0) { - for (i = 0; i < ab->nr_events; i++) { - ui_browser__set_percent_color(browser, - bdl->samples[i].percent, - current_entry); - if (annotate_browser__opts.show_total_period) - slsmg_printf("%6" PRIu64 " ", -bdl->samples[i].nr); - else - slsmg_printf("%6.2f ", bdl->samples[i].percent); + if (percent_max != 0.0) { + for (i = 0; i < ab->nr_events; i++) { + ui_browser__set_percent_color(browser, + bdl->samples[i].percent, + current_entry); + if (annotate_browser__opts.show_total_period) + slsmg_printf("%6" PRIu64 " ", +bdl->samples[i].nr); + else + slsmg_printf("%6.2f ", bdl->samples[i].percent); + } + } else { + slsmg_write_nstring(" ", 7 * ab->nr_events); } } else { ui_browser__set_percent_color(browser, 0, current_entry); - slsmg_write_nstring(" ", pcnt_width); + slsmg_write_nstring(" ", 7 * ab->nr_events); + } + if (ab->have_cycles) { + if (dl->ipc) + slsmg_printf("%*.2f ", IPC_WIDTH - 1, dl->ipc); + else + slsmg_write_nstring(" ", IPC_WIDTH); + if (dl->cycles) + slsmg_printf("%*" PRIu64 " ", +CYCLES_WIDTH - 1, dl->cycles); + else + slsmg_write_nstring(" ", CYCLES_WIDTH); } SLsmg_write_char(' '); @@ -232,7 +259,7 @@ static void annotate_browser__draw_current_jump(struct ui_browser *browser) unsigned int from, to; struct map_symbol *ms = ab->b.priv; struct symbol *sym = ms->sym; - u8 pcnt_width = 7; +
[PATCH 8/9] perf, tools, report: Display cycles in branch sort mode
From: Andi Kleen Display the cycles by default in branch sort mode. To make enough room for the new column I removed dso_to. It is usually redundant with dso_from. Signed-off-by: Andi Kleen --- tools/perf/util/sort.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c index 5b7a50c..5177088 100644 --- a/tools/perf/util/sort.c +++ b/tools/perf/util/sort.c @@ -9,7 +9,7 @@ regex_t parent_regex; const char default_parent_pattern[] = "^sys_|^do_page_fault"; const char *parent_pattern = default_parent_pattern; const char default_sort_order[] = "comm,dso,symbol"; -const char default_branch_sort_order[] = "comm,dso_from,symbol_from,dso_to,symbol_to"; +const char default_branch_sort_order[] = "comm,dso_from,symbol_from,symbol_to,cycles"; const char default_mem_sort_order[] = "local_weight,mem,sym,dso,symbol_daddr,dso_daddr,snoop,tlb,locked"; const char default_top_sort_order[] = "dso,symbol"; const char default_diff_sort_order[] = "dso,symbol"; -- 2.4.3 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/9] perf, tools, report: Add flag for non ANY branch mode
From: Andi Kleen Later patches need to cheaply check that the branch mode is in ANY. Add a new function to check all event attrs and add a flag to the report state, which is then initialized. v2: Rename flag Acked-by: Jiri Olsa Signed-off-by: Andi Kleen --- tools/perf/builtin-report.c | 7 +++ tools/perf/util/evlist.c| 10 ++ tools/perf/util/evlist.h| 1 + 3 files changed, 18 insertions(+) diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c index 95a4771..3ba0e97 100644 --- a/tools/perf/builtin-report.c +++ b/tools/perf/builtin-report.c @@ -53,6 +53,7 @@ struct report { boolmem_mode; boolheader; boolheader_only; + boolnonany_branch_mode; int max_stack; struct perf_read_values show_threads_values; const char *pretty_printing_style; @@ -258,6 +259,12 @@ static int report__setup_sample_type(struct report *rep) else callchain_param.record_mode = CALLCHAIN_FP; } + + /* ??? handle more cases than just ANY? */ + if (!(perf_evlist__combined_branch_type(session->evlist) & + PERF_SAMPLE_BRANCH_ANY)) + rep->nonany_branch_mode = true; + return 0; } diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c index f7d9c77..cba8069 100644 --- a/tools/perf/util/evlist.c +++ b/tools/perf/util/evlist.c @@ -1255,6 +1255,16 @@ u64 perf_evlist__combined_sample_type(struct perf_evlist *evlist) return __perf_evlist__combined_sample_type(evlist); } +u64 perf_evlist__combined_branch_type(struct perf_evlist *evlist) +{ + struct perf_evsel *evsel; + u64 branch_type = 0; + + evlist__for_each(evlist, evsel) + branch_type |= evsel->attr.branch_sample_type; + return branch_type; +} + bool perf_evlist__valid_read_format(struct perf_evlist *evlist) { struct perf_evsel *first = perf_evlist__first(evlist), *pos = first; diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h index 037633c..c1cdb86 100644 --- a/tools/perf/util/evlist.h +++ b/tools/perf/util/evlist.h @@ -169,6 +169,7 @@ void perf_evlist__set_leader(struct perf_evlist *evlist); u64 perf_evlist__read_format(struct perf_evlist *evlist); u64 __perf_evlist__combined_sample_type(struct perf_evlist *evlist); u64 perf_evlist__combined_sample_type(struct perf_evlist *evlist); +u64 perf_evlist__combined_branch_type(struct perf_evlist *evlist); bool perf_evlist__sample_id_all(struct perf_evlist *evlist); u16 perf_evlist__id_hdr_size(struct perf_evlist *evlist); -- 2.4.3 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 5/9] perf, tools: Compute IPC and basic block cycles for annotate
From: Andi Kleen Compute the IPC and the basic block cycles for the annotate display. IPC is computed by counting the instructions, and then dividing the accounted cycles by that count. The actual IPC computation can only be done at annotate time, because we need to parse the objdump output first to know the number of instructions in the basic block. The cycles/IPC are also put into the perf function annotation so that the display code can show them. Again basic block overlaps are not handled, with the longest winning, but there are some heuristics to hide the IPC when the longest is not the most common. v2: Compute IPC correctly. Signed-off-by: Andi Kleen --- tools/perf/ui/browsers/annotate.c | 73 ++- tools/perf/util/annotate.h| 2 ++ 2 files changed, 74 insertions(+), 1 deletion(-) diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c index 5995a8b..6ec1795 100644 --- a/tools/perf/ui/browsers/annotate.c +++ b/tools/perf/ui/browsers/annotate.c @@ -53,6 +53,7 @@ struct annotate_browser { int max_jump_sources; int nr_jumps; boolsearching_backwards; + boolhave_cycles; u8 addr_width; u8 jumps_width; u8 target_width; @@ -390,7 +391,7 @@ static void annotate_browser__calc_percent(struct annotate_browser *browser, max_percent = bpos->samples[i].percent; } - if (max_percent < 0.01) { + if (max_percent < 0.01 && pos->ipc == 0) { RB_CLEAR_NODE(>rb_node); continue; } @@ -869,6 +870,75 @@ int hist_entry__tui_annotate(struct hist_entry *he, struct perf_evsel *evsel, return map_symbol__tui_annotate(>ms, evsel, hbt); } + +static unsigned count_insn(struct annotate_browser *browser, u64 start, u64 end) +{ + unsigned n_insn = 0; + u64 offset; + + for (offset = start; offset <= end; offset++) { + if (browser->offsets[offset]) + n_insn++; + } + return n_insn; +} + +static void count_and_fill(struct annotate_browser *browser, u64 start, u64 end, + struct cyc_hist *ch) +{ + unsigned n_insn; + u64 offset; + + n_insn = count_insn(browser, start, end); + if (n_insn && ch->num && ch->cycles) { + float ipc = n_insn / ((double)ch->cycles / (double)ch->num); + + /* Hide data when there are too many overlaps. */ + if (ch->reset >= 0x7fff || ch->reset >= ch->num / 2) + return; + + for (offset = start; offset <= end; offset++) { + struct disasm_line *dl = browser->offsets[offset]; + + if (dl) + dl->ipc = ipc; + } + } +} + +/* + * This should probably be in util/annotate.c to share with the tty + * annotate, but right now we need the per byte offsets arrays, + * which are only here. + */ +static void annotate__compute_ipc(struct annotate_browser *browser, size_t size, + struct symbol *sym) +{ + u64 offset; + struct annotation *notes = symbol__annotation(sym); + + if (!notes->src || !notes->src->cycles_hist) + return; + + pthread_mutex_lock(>lock); + for (offset = 0; offset < size; ++offset) { + struct cyc_hist *ch; + + ch = >src->cycles_hist[offset]; + if (ch && ch->cycles) { + struct disasm_line *dl; + + if (ch->have_start) + count_and_fill(browser, ch->start, offset, ch); + dl = browser->offsets[offset]; + if (dl && ch->num_aggr) + dl->cycles = ch->cycles_aggr / ch->num_aggr; + browser->have_cycles = true; + } + } + pthread_mutex_unlock(>lock); +} + static void annotate_browser__mark_jump_targets(struct annotate_browser *browser, size_t size) { @@ -991,6 +1061,7 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map, } annotate_browser__mark_jump_targets(, size); + annotate__compute_ipc(, size, sym); browser.addr_width = browser.target_width = browser.min_addr_width = hex_width(size); browser.max_addr_width = hex_width(sym->end); diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h index a06518d..e999609 100644 --- a/tools/perf/util/annotate.h +++ b/tools/perf/util/annotate.h @@ -59,6 +59,8 @@ struct disasm_line { char*name; struct ins *ins; int line_nr; +
Re: [PATCH 1/2] net: dsa: mv88e6xxx: sleep in _mv88e6xxx_stats_wait
Hi all, - On Jul 18, 2015, at 10:58 AM, Andrew Lunn and...@lunn.ch wrote: >> Good point. The timeout is most definitely quite large and for sure on >> the safe side. It might make sense to add some statistics gathering to >> see how long the maximum observed delay actually is. > > Hi All > > Statistics are something which can be used a lot, i bursts and > interactivily. ATU, VTU etc, are much less often used. So different > delays might be justified. > > I agree about doing some statistics gathering to determine actual > delays needed. > >Andrew What do you think about something like this? Thanks, -v diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c index 4f3701f..6471807 100644 --- a/drivers/net/dsa/mv88e6xxx.c +++ b/drivers/net/dsa/mv88e6xxx.c @@ -563,9 +563,10 @@ static bool mv88e6xxx_6352_family(struct dsa_switch *ds) /* Must be called with SMI lock held */ static int _mv88e6xxx_wait(struct dsa_switch *ds, int reg, int offset, - u16 mask) + u16 mask, unsigned int msecs) { unsigned long timeout = jiffies + HZ / 10; + unsigned long usecs = msecs * 1000; while (time_before(jiffies, timeout)) { int ret; @@ -576,7 +577,8 @@ static int _mv88e6xxx_wait(struct dsa_switch *ds, int reg, int offset, if (!(ret & mask)) return 0; - usleep_range(1000, 2000); + if (usecs) + usleep_range(usecs, usecs + 1000); } return -ETIMEDOUT; } @@ -585,7 +587,7 @@ static int _mv88e6xxx_wait(struct dsa_switch *ds, int reg, int offset, static int _mv88e6xxx_stats_wait(struct dsa_switch *ds) { return _mv88e6xxx_wait(ds, REG_GLOBAL, GLOBAL_STATS_OP, - GLOBAL_STATS_OP_BUSY); + GLOBAL_STATS_OP_BUSY, 0); } /* Must be called with SMI mutex held */ @@ -872,13 +874,14 @@ error: } #endif /* CONFIG_NET_DSA_HWMON */ -static int mv88e6xxx_wait(struct dsa_switch *ds, int reg, int offset, u16 mask) +static int mv88e6xxx_wait(struct dsa_switch *ds, int reg, int offset, u16 mask, + unsigned int msecs) { struct mv88e6xxx_priv_state *ps = ds_to_priv(ds); int ret; mutex_lock(>smi_mutex); - ret = _mv88e6xxx_wait(ds, reg, offset, mask); + ret = _mv88e6xxx_wait(ds, reg, offset, mask, msecs); mutex_unlock(>smi_mutex); return ret; @@ -887,33 +890,33 @@ static int mv88e6xxx_wait(struct dsa_switch *ds, int reg, int offset, u16 mask) static int _mv88e6xxx_phy_wait(struct dsa_switch *ds) { return _mv88e6xxx_wait(ds, REG_GLOBAL2, GLOBAL2_SMI_OP, - GLOBAL2_SMI_OP_BUSY); + GLOBAL2_SMI_OP_BUSY, 1); } int mv88e6xxx_eeprom_load_wait(struct dsa_switch *ds) { return mv88e6xxx_wait(ds, REG_GLOBAL2, GLOBAL2_EEPROM_OP, - GLOBAL2_EEPROM_OP_LOAD); + GLOBAL2_EEPROM_OP_LOAD, 1); } int mv88e6xxx_eeprom_busy_wait(struct dsa_switch *ds) { return mv88e6xxx_wait(ds, REG_GLOBAL2, GLOBAL2_EEPROM_OP, - GLOBAL2_EEPROM_OP_BUSY); + GLOBAL2_EEPROM_OP_BUSY, 1); } /* Must be called with SMI lock held */ static int _mv88e6xxx_atu_wait(struct dsa_switch *ds) { return _mv88e6xxx_wait(ds, REG_GLOBAL, GLOBAL_ATU_OP, - GLOBAL_ATU_OP_BUSY); + GLOBAL_ATU_OP_BUSY, 1); } /* Must be called with SMI lock held */ static int _mv88e6xxx_scratch_wait(struct dsa_switch *ds) { return _mv88e6xxx_wait(ds, REG_GLOBAL2, GLOBAL2_SCRATCH_MISC, - GLOBAL2_SCRATCH_BUSY); + GLOBAL2_SCRATCH_BUSY, 1); } /* Must be called with SMI mutex held */ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] scripts: decode_stacktrace: fix ARM architecture decoding
Fix the stack decoder for the ARM architecture. An ARM stack is designed as : [ 81.547704] [] (bucket_find_contain) from [] (check_sync+0x40/0x4f8) [ 81.559668] [] (check_sync) from [] (debug_dma_sync_sg_for_cpu+0x128/0x194) [ 81.571583] [] (debug_dma_sync_sg_for_cpu) from [] (__videobuf_s The current script doesn't expect the symbols to be bound by parenthesis, and triggers the following errors : awk: cmd. line:1: error: Unmatched ( or \(: / (check_sync$/ [ 81.547704] (bucket_find_contain) from (check_sync+0x40/0x4f8) Fix it by chopping starting and ending parenthesis from the each symbol name. As a side note, this probably comes from the function dump_backtrace_entry(), which is implemented differently for each architecture. That makes a single decoding script a bit a challenge. Signed-off-by: Robert Jarzmik --- scripts/decode_stacktrace.sh | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/scripts/decode_stacktrace.sh b/scripts/decode_stacktrace.sh index 515c4c00e957..00d6d53c2681 100755 --- a/scripts/decode_stacktrace.sh +++ b/scripts/decode_stacktrace.sh @@ -14,11 +14,14 @@ declare -A cache parse_symbol() { # The structure of symbol at this point is: - # [name]+[offset]/[total length] + # ([name]+[offset]/[total length]) # # For example: # do_basic_setup+0x9c/0xbf + # Remove the englobing parenthesis + symbol=${symbol#\(} + symbol=${symbol%\)} # Strip the symbol name so that we could look it up local name=${symbol%+*} -- 2.1.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] net: dsa: mv88e6xxx: sleep in _mv88e6xxx_stats_wait
> Good point. The timeout is most definitely quite large and for sure on > the safe side. It might make sense to add some statistics gathering to > see how long the maximum observed delay actually is. Hi All Statistics are something which can be used a lot, i bursts and interactivily. ATU, VTU etc, are much less often used. So different delays might be justified. I agree about doing some statistics gathering to determine actual delays needed. Andrew -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 02/21] stackvalidate: Add C version of STACKVALIDATE_IGNORE_INSN
On Fri, Jul 17, 2015 at 11:47:18AM -0500, Josh Poimboeuf wrote: > Add a C inline asm string version of the STACKVALIDATE_IGNORE_INSN macro > which tells stackvalidate to ignore the subsequent instruction. > > Signed-off-by: Josh Poimboeuf > --- > arch/x86/include/asm/stackvalidate.h | 9 + > 1 file changed, 9 insertions(+) > > diff --git a/arch/x86/include/asm/stackvalidate.h > b/arch/x86/include/asm/stackvalidate.h > index 2d23c23..dac935c 100644 > --- a/arch/x86/include/asm/stackvalidate.h > +++ b/arch/x86/include/asm/stackvalidate.h > @@ -23,6 +23,15 @@ > .endif > .endm > > +#else /* !__ASSEMBLY__ */ > + #ifdef CONFIG_STACK_VALIDATION > +#define STACKVALIDATE_IGNORE_INSN\ > + ".Ltemp" __stringify(__LINE__) ":;" \ > + ".pushsection __stackvalidate_ignore_insn, \"a\";" \ > + _ASM_ALIGN ";" \ > + ".long .Ltemp" __stringify(__LINE__) " - .;"\ > + ".popsection;" > + #endif Also, you should end your lines with "\n" so that the .s output looks a bit more readable, not like now: #APP # 30 "./arch/x86/include/asm/arch_hweight.h" 1 661: .Ltemp32:;.pushsection __stackvalidate_ignore_insn, "a"; .balign 8 ;.long .Ltemp32 - .;.popsection;call __sw_hweight32 662: -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. -- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 04/21] x86/hweight: Add stack frame dependency for __arch_hweight*()
On Sat, Jul 18, 2015 at 08:44:15AM -0500, Josh Poimboeuf wrote: > Ok, so would you rather adding a whitelist to tell stackvalidate to > ignore it? Something like this? I tried it and maybe I'm missing something but that doesn't work: $ make drivers/gpu/drm/i915/intel_ringbuffer.o CHK include/config/kernel.release CHK include/generated/uapi/linux/version.h CHK include/generated/utsrelease.h CHK include/generated/timeconst.h CHK include/generated/bounds.h CHK include/generated/asm-offsets.h CALLscripts/checksyscalls.sh CC drivers/gpu/drm/i915/intel_ringbuffer.o ./arch/x86/include/asm/arch_hweight.h: Assembler messages: ./arch/x86/include/asm/arch_hweight.h:31: Error: symbol `.Ltemp32' is already defined ./arch/x86/include/asm/arch_hweight.h:31: Error: symbol `.Ltemp32' is already defined ./arch/x86/include/asm/arch_hweight.h:31: Error: symbol `.Ltemp32' is already defined scripts/Makefile.build:258: recipe for target 'drivers/gpu/drm/i915/intel_ringbuffer.o' failed make[1]: *** [drivers/gpu/drm/i915/intel_ringbuffer.o] Error 1 Makefile:1528: recipe for target 'drivers/gpu/drm/i915/intel_ringbuffer.o' failed make: *** [drivers/gpu/drm/i915/intel_ringbuffer.o] Error 2 Also, that label temp32 could be more descriptive. Regardless of the above, I don't like the idea of adding some compile-time checking and thus obfuscating what is already non-obvious code. And since your tool is already parsing ELF files and all that other fun, what I'd do is make that checking out-of-line *without* adding any new code to the kernel. In this particular case, you have: #APP # 28 "./arch/x86/include/asm/arch_hweight.h" 1 661: call __sw_hweight32 662: .skip -(((6651f-6641f)-(662b-661b)) > 0) * ((6651f-6641f)-(662b-661b)),0x90 663: .pushsection .altinstructions,"a" .long 661b - . .long 6641f - . .word ( 4*32+23) .byte 663b-661b .byte 6651f-6641f .byte 663b-662b .popsection .pushsection .altinstr_replacement, "ax" 6641: .byte 0xf3,0x40,0x0f,0xb8,0xc7 6651: .popsection # 0 "" 2 so you see that a CALL instruction gets replaced with a POPCNT and the feature bit used is 4*32+23 which is X86_FEATURE_POPCNT. This information is enough to detect that particular case and add the offset ".long 661b - ." to the list of instructions which stackvalidate should ignore. Anyway, this is what I'd do. IMNSVHO, we must be very conservative and not add some markers/helpers/etc to code only so that tools can do their job. Not if it can be helped. Instead, tools should do the hard work and we should keep kernel code clean. -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. -- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] x86, perf: Fix Skylake FRONTEND MSR extrareg mask
From: Andi Kleen Stephane pointed out that the extrareg mask was one bit too short. The bubble width field was truncated by one bit. Fix that here. Also add some extra comments on the reserved bits inside the event select code. Reported-by: Stephane Eranian Signed-off-by: Andi Kleen --- arch/x86/kernel/cpu/perf_event_intel.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/arch/x86/kernel/cpu/perf_event_intel.c b/arch/x86/kernel/cpu/perf_event_intel.c index caffb41..0861196 100644 --- a/arch/x86/kernel/cpu/perf_event_intel.c +++ b/arch/x86/kernel/cpu/perf_event_intel.c @@ -205,7 +205,11 @@ static struct extra_reg intel_skl_extra_regs[] __read_mostly = { INTEL_UEVENT_EXTRA_REG(0x01b7, MSR_OFFCORE_RSP_0, 0x3f8fffull, RSP_0), INTEL_UEVENT_EXTRA_REG(0x01bb, MSR_OFFCORE_RSP_1, 0x3f8fffull, RSP_1), INTEL_UEVENT_PEBS_LDLAT_EXTRA_REG(0x01cd), - INTEL_UEVENT_EXTRA_REG(0x01c6, MSR_PEBS_FRONTEND, 0x3fff17, FE), + /* +* Note the low 8 bits eventsel code is not a continuous field, containing +* some #GPing bits. These are masked out. +*/ + INTEL_UEVENT_EXTRA_REG(0x01c6, MSR_PEBS_FRONTEND, 0x7fff17, FE), EVENT_EXTRA_END }; -- 2.4.3 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Use after free bug in null_blk driver
Hi, i noticed that loading null_blk with queue_mode=1 and irqmode=2 parameters and slab poisoning enabled causes general protection fault: [ 20.671974] general protection fault: [#1] SMP [ 20.678050] Modules linked in: null_blk(+) usbhid hid psmouse floppy [ 20.688351] CPU: 0 PID: 147 Comm: modprobe Not tainted 4.2.0-rc2+ #76 [ 20.695961] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 [ 20.705143] task: 88007f2ed100 ti: 88007f0a4000 task.ti: 88007f0a4000 [ 20.715511] RIP: 0010:[] [] null_cmd_timer_expired+0x62/0xc0 [null_blk] [ 20.730541] RSP: 0018:88007d203e78 EFLAGS: 00010082 [ 20.737301] RAX: 88007ec16b80 RBX: 6b6b6b6b6b6b6b6b RCX: [ 20.747034] RDX: 88007f260888 RSI: RDI: 88007f260850 [ 20.762601] RBP: 88007d203e98 R08: 0001 R09: [ 20.771472] R10: 0001 R11: R12: [ 20.782522] R13: 88007d218780 R14: R15: 88007d20fba8 [ 20.792347] FS: 7f02e1500740() GS:88007d20() knlGS: [ 20.805319] CS: 0010 DS: ES: CR0: 8005003b [ 20.813120] CR2: 7ffd52478dd8 CR3: 7f3dd000 CR4: 07f0 [ 20.825357] Stack: [ 20.827764] 88007d218788 88007d20fa00 88007d20fac0 a00280f0 [ 20.838269] 88007d203f08 810f822b 00045dd35ed6 88007d20fa18 [ 20.853377] 0001810f89e7 88007d20fa00 88007d20fb28 00045dd35ed6 [ 20.863415] Call Trace: [ 20.866398] [ 20.869088] [] ? null_softirq_done_fn+0x30/0x30 [null_blk] [ 20.880219] [] __hrtimer_run_queues+0x11b/0x6e0 [ 20.891048] [] hrtimer_interrupt+0xab/0x1b0 [ 20.899542] [] local_apic_timer_interrupt+0x3c/0x70 [ 20.911730] [] smp_apic_timer_interrupt+0x41/0x60 [ 20.921854] [] apic_timer_interrupt+0x70/0x80 [ 20.931084] [ 20.933406] [] ? _raw_spin_unlock_irqrestore+0x3b/0x60 [ 20.945250] [] hrtimer_start_range_ns+0x1a7/0x540 [ 20.955952] [] ? trace_hardirqs_on_caller+0x151/0x1e0 [ 20.963974] [] null_request_fn+0xd3/0x100 [null_blk] [ 20.977159] [] __blk_run_queue+0x37/0x50 [ 20.983782] [] __elv_add_request+0x15d/0x440 [ 20.991540] [] blk_queue_bio+0x415/0x520 [ 20.998876] [] generic_make_request+0xcc/0x110 [ 21.007095] [] submit_bio+0x67/0x150 [ 21.013603] [] submit_bh_wbc+0x14c/0x180 [ 21.019730] [] block_read_full_page+0x2b0/0x3a0 [ 21.028023] [] ? I_BDEV+0x20/0x20 [ 21.034575] [] ? blkdev_readpages+0x20/0x20 [ 21.040947] [] blkdev_readpage+0x18/0x20 [ 21.048237] [] do_read_cache_page+0x7d/0x1a0 [ 21.056016] [] read_cache_page+0x1c/0x20 [ 21.063300] [] read_dev_sector+0x30/0x90 [ 21.069804] [] ? check_partition+0x1f0/0x1f0 [ 21.076932] [] adfspart_check_ICS+0x42/0x250 [ 21.084317] [] ? snprintf+0x34/0x40 [ 21.090670] [] ? check_partition+0x1f0/0x1f0 [ 21.097864] [] check_partition+0x100/0x1f0 [ 21.104535] [] rescan_partitions+0xa8/0x2b0 [ 21.112087] [] __blkdev_get+0x313/0x460 [ 21.120037] [] blkdev_get+0x41/0x3a0 [ 21.126266] [] ? unlock_new_inode+0x5d/0x90 [ 21.134043] [] ? bdget+0x130/0x150 [ 21.140369] [] ? disk_get_part+0xd/0x1d0 [ 21.147357] [] add_disk+0x436/0x4d0 [ 21.153846] [] ? sprintf+0x40/0x50 [ 21.159867] [] null_init+0x38e/0x1000 [null_blk] [ 21.167691] [] ? 0xa004e000 [ 21.174266] [] do_one_initcall+0xb3/0x1e0 [ 21.182276] [] ? kmem_cache_alloc_trace+0x36f/0x3b0 [ 21.190291] [] do_init_module+0x61/0x1ec [ 21.197112] [] load_module+0x24fe/0x2810 [ 21.204237] [] ? m_show+0x1a0/0x1a0 [ 21.213498] [] SyS_finit_module+0x80/0xb0 [ 21.221360] [] entry_SYSCALL_64_fastpath+0x16/0x7a [ 21.230181] Code: e8 24 40 3b e1 48 89 c3 eb 08 4d 85 e4 4c 89 e3 74 e2 48 8d 7b f0 4c 8b 23 e8 db fe ff ff 48 8b 43 28 48 85 c0 74 e3 48 8b 58 30 <48> 83 bb 38 01 00 00 00 75 d5 48 8b 83 98 08 00 00 a8 04 74 ca [ 21.281572] RIP [] null_cmd_timer_expired+0x62/0xc0 [null_blk] [ 21.289770] RSP [ 21.294601] ---[ end trace cd40ca6c9001dd7e ]--- [ 21.299719] Kernel panic - not syncing: Fatal exception in interrupt [ 21.309176] Kernel Offset: disabled [ 21.312773] ---[ end Kernel panic - not syncing: Fatal exception in interrupt I suppose, it was introduced in commit 8b70f45e2eb2 ("null_blk: restart request processing on completion handler") by Akinobu Mita . The following fix solves problem for me: diff --git a/drivers/block/null_blk.c b/drivers/block/null_blk.c index 69de41a..910f354 100644 --- a/drivers/block/null_blk.c +++ b/drivers/block/null_blk.c @@ -242,7 +242,6 @@ static enum hrtimer_restart null_cmd_timer_expired(struct hrtimer *timer) do { cmd = container_of(entry, struct nullb_cmd, ll_list); entry = entry->next; - end_cmd(cmd); if (cmd->rq) {
xhci_hcd 0000:00:14.0: ERROR Transfer event TRB DMA ptr not part of current TD ep_index 2 comp_code 1
Hi. I'm on 4.2.0-rc2-00077-gf760b87 kernel and while trying to copy some file from usb storage (sata disk behind sata-usb bridge or pendrive; hapens in both cases) copying process hangs just early after start with: [ 77.372137] usb 2-1: new SuperSpeed USB device number 2 using xhci_hcd [ 77.388945] usb 2-1: New USB device found, idVendor=1f75, idProduct=0611 [ 77.388952] usb 2-1: New USB device strings: Mfr=4, Product=5, SerialNumber=6 [ 77.388956] usb 2-1: SerialNumber: 20130514 [ 77.402599] usb-storage 2-1:1.0: USB Mass Storage device detected [ 77.403177] scsi host6: usb-storage 2-1:1.0 [ 77.403318] usbcore: registered new interface driver usb-storage [ 77.407529] usbcore: registered new interface driver uas [ 78.400954] scsi 6:0:0:0: scsi scan: INQUIRY result too short (5), using 36 [ 78.400961] scsi 6:0:0:0: Direct-Access Hitachi HDS723020BLA642 PQ: 0 ANSI: 0 [ 78.401401] sd 6:0:0:0: [sdb] 3907029168 512-byte logical blocks: (2.00 TB/1.81 TiB) [ 78.402126] sd 6:0:0:0: [sdb] Write Protect is off [ 78.402130] sd 6:0:0:0: [sdb] Mode Sense: 3b 00 00 00 [ 78.402876] sd 6:0:0:0: [sdb] No Caching mode page found [ 78.402882] sd 6:0:0:0: [sdb] Assuming drive cache: write through [ 78.444310] sd 6:0:0:0: [sdb] Attached SCSI disk [ 85.907972] EXT4-fs (sdb): mounted filesystem with ordered data mode. Opts: (null) [ 113.556376] xhci_hcd :00:14.0: ERROR Transfer event TRB DMA ptr not part of current TD ep_index 2 comp_code 1 [ 113.556383] xhci_hcd :00:14.0: Looking for event-dma fffa4000 trb-start fffa5fe0 trb-end fffa6000 seg-start fffa5000 seg-end fffa5ff0 [ 234.236311] usb 2-1: reset SuperSpeed USB device number 2 using xhci_hcd [ 234.890862] xhci_hcd :00:14.0: ERROR Transfer event TRB DMA ptr not part of current TD ep_index 2 comp_code 1 [ 234.890869] xhci_hcd :00:14.0: Looking for event-dma fff94000 trb-start fff95fd0 trb-end fff96000 seg-start fff95000 seg-end fff95ff0 [ 355.339935] usb 2-1: reset SuperSpeed USB device number 2 using xhci_hcd [ 355.574012] xhci_hcd :00:14.0: ERROR Transfer event TRB DMA ptr not part of current TD ep_index 2 comp_code 1 [ 355.574018] xhci_hcd :00:14.0: Looking for event-dma fff84000 trb-start fff85fb0 trb-end fff86000 seg-start fff85000 seg-end fff85ff0 [ 476.430339] usb 2-1: reset SuperSpeed USB device number 2 using xhci_hcd [ 476.728729] xhci_hcd :00:14.0: ERROR Transfer event TRB DMA ptr not part of current TD ep_index 2 comp_code 1 [ 476.728738] xhci_hcd :00:14.0: Looking for event-dma fff74000 trb-start fff75fe0 trb-end fff76000 seg-start fff75000 seg-end fff75ff0 3.19.3 works fine 4.0.8 fails 4.2.0-rc2-00077-gf760b87 fails There was some similar thread in march 2015 but the issue there got resolved by reverting one usb patch, so my issue has to be different (that revert is included in 4.0 final I think). .config -> http://sprunge.us/bACC lsusb -> http://sprunge.us/DOWb dmesg -> http://sprunge.us/UbTF machine is dell xps 15 9530 -- Arkadiusz Miśkiewicz, arekm / ( maven.pl | pld-linux.org ) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] iio: Fix iio_validate_scan_mask_oneshot function name typo
Rename function to iio_validate_scan_mask_oneshot() since it's used to validate that only one channel is selected. Signed-off-by: Cristina Opriceana --- drivers/iio/adc/ad_sigma_delta.c | 2 +- drivers/iio/industrialio-buffer.c| 8 drivers/staging/iio/meter/ade7758_ring.c | 2 +- include/linux/iio/buffer.h | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/iio/adc/ad_sigma_delta.c b/drivers/iio/adc/ad_sigma_delta.c index d10bd0c..2fa4e42 100644 --- a/drivers/iio/adc/ad_sigma_delta.c +++ b/drivers/iio/adc/ad_sigma_delta.c @@ -400,7 +400,7 @@ static const struct iio_buffer_setup_ops ad_sd_buffer_setup_ops = { .postenable = _sd_buffer_postenable, .predisable = _triggered_buffer_predisable, .postdisable = _sd_buffer_postdisable, - .validate_scan_mask = _validate_scan_mask_onehot, + .validate_scan_mask = _validate_scan_mask_oneshot, }; static irqreturn_t ad_sd_data_rdy_trig_poll(int irq, void *private) diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c index f72be48..b13f941 100644 --- a/drivers/iio/industrialio-buffer.c +++ b/drivers/iio/industrialio-buffer.c @@ -1106,7 +1106,7 @@ void iio_buffer_free_sysfs_and_mask(struct iio_dev *indio_dev) } /** - * iio_validate_scan_mask_onehot() - Validates that exactly one channel is selected + * iio_validate_scan_mask_oneshot() - Validates that exactly one channel is selected * @indio_dev: the iio device * @mask: scan mask to be checked * @@ -1114,12 +1114,12 @@ void iio_buffer_free_sysfs_and_mask(struct iio_dev *indio_dev) * can be used for devices where only one channel can be active for sampling at * a time. */ -bool iio_validate_scan_mask_onehot(struct iio_dev *indio_dev, - const unsigned long *mask) +bool iio_validate_scan_mask_oneshot(struct iio_dev *indio_dev, + const unsigned long *mask) { return bitmap_weight(mask, indio_dev->masklength) == 1; } -EXPORT_SYMBOL_GPL(iio_validate_scan_mask_onehot); +EXPORT_SYMBOL_GPL(iio_validate_scan_mask_oneshot); int iio_scan_mask_query(struct iio_dev *indio_dev, struct iio_buffer *buffer, int bit) diff --git a/drivers/staging/iio/meter/ade7758_ring.c b/drivers/staging/iio/meter/ade7758_ring.c index 9a24e02..d2fa9ef 100644 --- a/drivers/staging/iio/meter/ade7758_ring.c +++ b/drivers/staging/iio/meter/ade7758_ring.c @@ -103,7 +103,7 @@ static const struct iio_buffer_setup_ops ade7758_ring_setup_ops = { .preenable = _ring_preenable, .postenable = _triggered_buffer_postenable, .predisable = _triggered_buffer_predisable, - .validate_scan_mask = _validate_scan_mask_onehot, + .validate_scan_mask = _validate_scan_mask_oneshot, }; void ade7758_unconfigure_ring(struct iio_dev *indio_dev) diff --git a/include/linux/iio/buffer.h b/include/linux/iio/buffer.h index 1600c55..78d27b7 100644 --- a/include/linux/iio/buffer.h +++ b/include/linux/iio/buffer.h @@ -152,8 +152,8 @@ static inline int iio_push_to_buffers_with_timestamp(struct iio_dev *indio_dev, int iio_update_demux(struct iio_dev *indio_dev); -bool iio_validate_scan_mask_onehot(struct iio_dev *indio_dev, - const unsigned long *mask); +bool iio_validate_scan_mask_oneshot(struct iio_dev *indio_dev, + const unsigned long *mask); struct iio_buffer *iio_buffer_get(struct iio_buffer *buffer); void iio_buffer_put(struct iio_buffer *buffer); -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 13/21] x86/asm/crypto: Fix frame pointer usage in aesni-intel_asm.S
On Sat, Jul 18, 2015 at 08:46:55AM -0500, Josh Poimboeuf wrote: > I like the balance, but the "ret" is still non-obvious. Does it have to be obvious? -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. -- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] x86, perf: Add PEBS frontend profiling for Skylake
On Fri, Jul 17, 2015 at 04:52:56PM -0700, Stephane Eranian wrote: > On Fri, Jul 17, 2015 at 4:31 PM, Andi Kleen wrote: > > On Fri, Jul 17, 2015 at 03:00:18PM -0700, Stephane Eranian wrote: > >> Andi, > >> > >> On Fri, Jul 17, 2015 at 2:19 PM, Andi Kleen wrote: > >> >> But then, the SDM is misleading. It is not describing what's > >> >> implemented for SKL. > >> > > >> > Actually it has a list of valid values you can put into the various > >> > fields. > >> > None of them have the bits set you're trying to set. > >> > > >> You are talking about the events (bit 0-7). I am talking about the bubble > >> thresholds. I am okay with the event list for bits 0-7. > > > > Fair enough. There's a one-off in the MSR table and table 18-54. The IDQ > > bubble width is only 21:20. I'll ask for that to be fixed in both places > > that document them. > > > There is still something broken here, if bit 22 is not implemented, > then you have > a bunch of frontend events in the SKL event table (download.01.org) which are > bogus: I double checked and bit 22 is actually implemented. I'll send a patch to update the mask in the driver. Thanks for the report. -Andi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [tip:x86/asm] x86/irq, context_tracking: Document how IRQ context tracking works and add an RCU assertion
On Sat, Jul 18, 2015 at 03:23:57PM +0200, Frederic Weisbecker wrote: > On Tue, Jul 14, 2015 at 04:33:39PM -0700, Andy Lutomirski wrote: > > On Tue, Jul 14, 2015 at 4:26 PM, Frederic Weisbecker > > wrote: > > > On Tue, Jul 07, 2015 at 03:54:32AM -0700, tip-bot for Andy Lutomirski > > > wrote: > > >> Commit-ID: 0333a209cbf600e980fc55c24878a56f25f48b65 > > >> Gitweb: > > >> http://git.kernel.org/tip/0333a209cbf600e980fc55c24878a56f25f48b65 > > >> Author: Andy Lutomirski > > >> AuthorDate: Fri, 3 Jul 2015 12:44:34 -0700 > > >> Committer: Ingo Molnar > > >> CommitDate: Tue, 7 Jul 2015 10:59:10 +0200 > > >> > > >> x86/irq, context_tracking: Document how IRQ context tracking works and > > >> add an RCU assertion > > >> > > >> Signed-off-by: Andy Lutomirski > > >> Cc: Andy Lutomirski > > >> Cc: Borislav Petkov > > >> Cc: Brian Gerst > > >> Cc: Denys Vlasenko > > >> Cc: Denys Vlasenko > > >> Cc: Frederic Weisbecker > > >> Cc: H. Peter Anvin > > >> Cc: Kees Cook > > >> Cc: Linus Torvalds > > >> Cc: Oleg Nesterov > > >> Cc: Paul E. McKenney > > >> Cc: Peter Zijlstra > > >> Cc: Rik van Riel > > >> Cc: Thomas Gleixner > > >> Cc: paul...@linux.vnet.ibm.com > > >> Link: > > >> http://lkml.kernel.org/r/e8bdc4ed0193fb2fd130f3d6b7b8023e2ec1ab62.1435952415.git.l...@kernel.org > > >> Signed-off-by: Ingo Molnar > > >> --- > > >> arch/x86/kernel/irq.c | 15 +++ > > >> 1 file changed, 15 insertions(+) > > >> > > >> diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c > > >> index 88b36648..6233de0 100644 > > >> --- a/arch/x86/kernel/irq.c > > >> +++ b/arch/x86/kernel/irq.c > > >> @@ -216,8 +216,23 @@ __visible unsigned int __irq_entry do_IRQ(struct > > >> pt_regs *regs) > > >> unsigned vector = ~regs->orig_ax; > > >> unsigned irq; > > >> > > >> + /* > > >> + * NB: Unlike exception entries, IRQ entries do not reliably > > >> + * handle context tracking in the low-level entry code. This is > > >> + * because syscall entries execute briefly with IRQs on before > > >> + * updating context tracking state, so we can take an IRQ from > > >> + * kernel mode with CONTEXT_USER. The low-level entry code only > > >> + * updates the context if we came from user mode, so we won't > > >> + * switch to CONTEXT_KERNEL. We'll fix that once the syscall > > >> + * code is cleaned up enough that we can cleanly defer enabling > > >> + * IRQs. > > >> + */ > > >> + > > > > > > Now is it a problem to take interrupts in kernel mode with CONTEXT_USER? > > > I'm not sure it's worth trying to make it not happen. > > > > It's not currently a problem, but it would be nice if we could do the > > equivalent of: > > > > if (user_mode(regs)) { > > user_exit(); (or enter_from_user_mode or whatever) > > } else { > > // don't bother -- already in CONTEXT_KERNEL > > } > > This was the initial implementation of context tracking but it was terribly > buggy. What if we enter the kernel, we haven't yet got a change to call > context_tracking_user_exit() and we get an exception in the kernel entry > path? user_mode(regs) will return the wrong value and bad things happen. > > This is why context tracking needs its own tracking state, because we are > always > out of sync with the real processor context anyway. > > > > > i.e. the same thing that do_general_protection, etc do in -tip. That > > would get rid of any need to store the previous context. > > > > Currently we can't because of syscalls and maybe because of KVM. KVM > > has a weird fake interrupt thing. > > > > > > > >> entering_irq(); > > >> > > >> + /* entering_irq() tells RCU that we're not quiescent. Check it. */ > > >> + rcu_lockdep_assert(rcu_is_watching(), "IRQ failed to wake up RCU"); > > > > > > Why do we need to check that? > > > > Sanity check. If we're changing a bunch of context tracking details, > > I want to assert that it actually works. > > But we call rcu_irq_enter() right before. > > It's more or less like doing: > > local_irq_disable(); > WARN_ON(!irqs_disabled()); If we end up in a world where RCU sometimes uses context-tracking state and sometimes uses its own state (for example, for architecture that do not support context tracking), such a check might make more sense. It would be all too easy for someone to accidentailly manage to disable both somehow, and things would sort of work but have strange undebuggable failure cases. Sometimes. Thanx, Paul -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RESEND PATCH v3] ARM: tegra124: pmu support
On Fri, Jul 17, 2015 at 4:59 PM, Thierry Reding wrote: > On Mon, Jul 13, 2015 at 10:35:45AM -0700, Kyle Huey wrote: >> This patch modifies the device tree for tegra124 based devices to enable >> the Cortex A15 PMU. The interrupt numbers are taken from NVIDIA TRM >> DP-06905-001_v03p. This patch was tested on a Jetson TK1. >> >> Updated for proper ordering and to add interrupt-affinity values. >> >> Signed-off-by: Kyle Huey >> --- >> arch/arm/boot/dts/tegra124.dtsi | 17 + >> 1 file changed, 13 insertions(+), 4 deletions(-) > > Is there any way to test this? What are the effects of adding this? Yes. This enables the ARM PMU driver for the Cortex A15, which allows one to use hardware performance counters via the perf_event_open API. For a simple test program, see https://github.com/khuey/perf-counter-test/. Without this patch, the perf_event_open syscall will fail. With this patch, the program will print out the performance counter value for each iteration of the loop. (IIRC on the A15 the branch counter was removed, so you may want to replace 0xD with 0x8 which counts instructions executed if you want to see a non-zero number there). You also will see a message about the PMU in the kernel log at startup after applying this patch. I have also tested this extensively (including the interrupt features of the PMU) on a more complex program. > Does it enable using perf for profiling? I have not tested it, but I believe you can use perf without this patch if you do not use features that require hardware performance counter support. This patch would enable those features. >> diff --git a/arch/arm/boot/dts/tegra124.dtsi >> b/arch/arm/boot/dts/tegra124.dtsi >> index 13cc7ca..de07d7e 100644 >> --- a/arch/arm/boot/dts/tegra124.dtsi >> +++ b/arch/arm/boot/dts/tegra124.dtsi >> @@ -918,31 +918,40 @@ >> #address-cells = <1>; >> #size-cells = <0>; >> >> - cpu@0 { >> + A15_0: cpu@0 { >> device_type = "cpu"; >> compatible = "arm,cortex-a15"; >> reg = <0>; >> }; >> >> - cpu@1 { >> + A15_1: cpu@1 { >> device_type = "cpu"; >> compatible = "arm,cortex-a15"; >> reg = <1>; >> }; >> >> - cpu@2 { >> + A15_2: cpu@2 { >> device_type = "cpu"; >> compatible = "arm,cortex-a15"; >> reg = <2>; >> }; >> >> - cpu@3 { >> + A15_3: cpu@3 { >> device_type = "cpu"; >> compatible = "arm,cortex-a15"; >> reg = <3>; >> }; >> }; >> >> + pmu { >> + compatible = "arm,cortex-a15-pmu"; >> + interrupts = , >> + , >> + , >> + ; >> + interrupt-affinity = <_0>, <_1>, <_2>, <_3>; > > These labels look somewhat artificial to me, perhaps we could do > something like the following instead? > > interrupt-affinity = <&{/cpus/cpu@0}>, ...; > > That's slightly more obvious and avoids the need to "invent" labels for > the CPUs. > > No need to respin, I can fix that up when applying if nobody objects to > using the alternative notation. > > Thierry I have no objections. I was not aware that the device tree syntax supported that. FWIW I cargo-culted my way to victory from vexpress-v2p-ca9.dts here. - Kyle -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 13/21] x86/asm/crypto: Fix frame pointer usage in aesni-intel_asm.S
On Sat, Jul 18, 2015 at 10:42:36AM +0200, Borislav Petkov wrote: > On Fri, Jul 17, 2015 at 04:10:19PM -0500, Josh Poimboeuf wrote: > > Actually I'm not done painting. Personally it seems a little too > > verbose. I still like ENTRY_FRAME and ENDPROC_FRAME_RETURN :p > > Let's balance it out even more: > > ENTRY_FRAME(..) > > ... > > ENDPROC_FRAME(..) I like the balance, but the "ret" is still non-obvious. How about: FRAME_ENTRY(...) FRAME_RETURN(...) -- Josh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH 04/21] x86/hweight: Add stack frame dependency for __arch_hweight*()
On Sat, Jul 18, 2015 at 07:05:36AM +0200, Borislav Petkov wrote: > On Fri, Jul 17, 2015 at 12:32:20PM -0500, Josh Poimboeuf wrote: > > Well, but this isn't some whitelist code to make stackvalidate happy. > > > > It's actually a real runtime frame pointer bug, and the rsp dependency > > is real. If it does the call without first creating the stack frame > > then it breaks frame pointer based stack traces. > > I think we can live with the stack trace being a little wrong in those > __sw_* variants. And besides, we're talking about the very very small > percentage of machines (which keeps getting smaller) which don't > support POPCNT. And from those, only for the cases where the arg is not > __builtin_constant_p() because there we do the __const_hweight* thing. > > I'd prefer to not clutter the code more in that case. Ok, so would you rather adding a whitelist to tell stackvalidate to ignore it? Something like this? diff --git a/arch/x86/include/asm/arch_hweight.h b/arch/x86/include/asm/arch_hweight.h index 9686c3d..d604691 100644 --- a/arch/x86/include/asm/arch_hweight.h +++ b/arch/x86/include/asm/arch_hweight.h @@ -1,6 +1,8 @@ #ifndef _ASM_X86_HWEIGHT_H #define _ASM_X86_HWEIGHT_H +#include + #ifdef CONFIG_64BIT /* popcnt %edi, %eax -- redundant REX prefix for alignment */ #define POPCNT32 ".byte 0xf3,0x40,0x0f,0xb8,0xc7" @@ -25,7 +27,9 @@ static inline unsigned int __arch_hweight32(unsigned int w) { unsigned int res = 0; - asm (ALTERNATIVE("call __sw_hweight32", POPCNT32, X86_FEATURE_POPCNT) + asm (ALTERNATIVE(STACKVALIDATE_IGNORE_INSN +"call __sw_hweight32", +POPCNT32, X86_FEATURE_POPCNT) : "="REG_OUT (res) : REG_IN (w)); @@ -50,7 +54,9 @@ static inline unsigned long __arch_hweight64(__u64 w) return __arch_hweight32((u32)w) + __arch_hweight32((u32)(w >> 32)); #else - asm (ALTERNATIVE("call __sw_hweight64", POPCNT64, X86_FEATURE_POPCNT) + asm (ALTERNATIVE(STACKVALIDATE_IGNORE_INSN +"call __sw_hweight64", +POPCNT64, X86_FEATURE_POPCNT) : "="REG_OUT (res) : REG_IN (w)); #endif /* CONFIG_X86_32 */ -- Josh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC] workqueue: avoiding unbounded wq on isolated CPUs by default
On Fri, Jul 17, 2015 at 07:15:48PM +0200, Mike Galbraith wrote: > On Fri, 2015-07-17 at 11:27 -0400, Tejun Heo wrote: > > > I'm just curious whether there was any specific reason we didn't do > > this before (ISTR people discussing it back then too). > > I'm dead set against all this auto-presume nonsense fwtw Allocating a > pool of no_hz_full _capable_ CPUs should not entice the kernel to make > any rash assumptions. Let users do the button poking, they know what > they want, and when they want it. We need to make a choice then. Either we do all the affinity tuning from userspace with a common tool, which is what I had wished before everybody asked for pre-settings. Or we do it in the kernel, now we should define some kind of CONFIG_ISOLATION to make that proper and rule the various kinds of isolation people are interested in. But we can't leave it half-way like it is currently with everything preset on top of nohz: rcu nocb mask, watchdog mask, cpu_isolation_map and exclude workqueue. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: CONFIG_NO_HZ_FULL restricts cpu usage to the equivalent of one in 4.2
On Thu, Jul 16, 2015 at 01:32:18AM +0100, Ken Moffat wrote: > On Wed, Jul 15, 2015 at 09:11:46PM +0200, Frederic Weisbecker wrote: > > 2015-07-15 18:27 GMT+02:00 Ken Moffat : > > > > > > The config differences follow. Perhaps it is actually one of the > > > subsequent choices that is the problem. And I guess it could still > > > be a gcc-5.1 issue. > > > > > > --- config-4.2-initial 2015-07-15 16:25:12.548005751 +0100 > > > +++ config-4.2-speed-ok 2015-07-15 17:00:50.919998703 +0100 > > > @@ -104,11 +104,8 @@ > > > CONFIG_TICK_ONESHOT=y > > > CONFIG_NO_HZ_COMMON=y > > > # CONFIG_HZ_PERIODIC is not set > > > -# CONFIG_NO_HZ_IDLE is not set > > > -CONFIG_NO_HZ_FULL=y > > > -CONFIG_NO_HZ_FULL_ALL=y > > > > You had CONFIG_NO_HZ_FULL_ALL enabled? Because that would indeed > > produce that effect since it isolates all CPUs but 0 off sched > > domains. > > > > Which means that basically only CPU 0 runs user tasks unless you > > forces these otherwise. > > Thanks. I'll put it down to a bad .config choice, although it was > fine on early 4.1. Yeah we decided to include the nohz_full on cpu_isolated_map recently, except CPU 0. > While I was starting to bisect, I noticed that > on the A10 everything was happening on CPU 0 - not sure if that was > happening on the original box, but for the moment it sounds likely. > > ĸen > -- > This one goes up to eleven! -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [tip:x86/asm] x86/irq, context_tracking: Document how IRQ context tracking works and add an RCU assertion
On Tue, Jul 14, 2015 at 04:33:39PM -0700, Andy Lutomirski wrote: > On Tue, Jul 14, 2015 at 4:26 PM, Frederic Weisbecker > wrote: > > On Tue, Jul 07, 2015 at 03:54:32AM -0700, tip-bot for Andy Lutomirski wrote: > >> Commit-ID: 0333a209cbf600e980fc55c24878a56f25f48b65 > >> Gitweb: > >> http://git.kernel.org/tip/0333a209cbf600e980fc55c24878a56f25f48b65 > >> Author: Andy Lutomirski > >> AuthorDate: Fri, 3 Jul 2015 12:44:34 -0700 > >> Committer: Ingo Molnar > >> CommitDate: Tue, 7 Jul 2015 10:59:10 +0200 > >> > >> x86/irq, context_tracking: Document how IRQ context tracking works and add > >> an RCU assertion > >> > >> Signed-off-by: Andy Lutomirski > >> Cc: Andy Lutomirski > >> Cc: Borislav Petkov > >> Cc: Brian Gerst > >> Cc: Denys Vlasenko > >> Cc: Denys Vlasenko > >> Cc: Frederic Weisbecker > >> Cc: H. Peter Anvin > >> Cc: Kees Cook > >> Cc: Linus Torvalds > >> Cc: Oleg Nesterov > >> Cc: Paul E. McKenney > >> Cc: Peter Zijlstra > >> Cc: Rik van Riel > >> Cc: Thomas Gleixner > >> Cc: paul...@linux.vnet.ibm.com > >> Link: > >> http://lkml.kernel.org/r/e8bdc4ed0193fb2fd130f3d6b7b8023e2ec1ab62.1435952415.git.l...@kernel.org > >> Signed-off-by: Ingo Molnar > >> --- > >> arch/x86/kernel/irq.c | 15 +++ > >> 1 file changed, 15 insertions(+) > >> > >> diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c > >> index 88b36648..6233de0 100644 > >> --- a/arch/x86/kernel/irq.c > >> +++ b/arch/x86/kernel/irq.c > >> @@ -216,8 +216,23 @@ __visible unsigned int __irq_entry do_IRQ(struct > >> pt_regs *regs) > >> unsigned vector = ~regs->orig_ax; > >> unsigned irq; > >> > >> + /* > >> + * NB: Unlike exception entries, IRQ entries do not reliably > >> + * handle context tracking in the low-level entry code. This is > >> + * because syscall entries execute briefly with IRQs on before > >> + * updating context tracking state, so we can take an IRQ from > >> + * kernel mode with CONTEXT_USER. The low-level entry code only > >> + * updates the context if we came from user mode, so we won't > >> + * switch to CONTEXT_KERNEL. We'll fix that once the syscall > >> + * code is cleaned up enough that we can cleanly defer enabling > >> + * IRQs. > >> + */ > >> + > > > > Now is it a problem to take interrupts in kernel mode with CONTEXT_USER? > > I'm not sure it's worth trying to make it not happen. > > It's not currently a problem, but it would be nice if we could do the > equivalent of: > > if (user_mode(regs)) { > user_exit(); (or enter_from_user_mode or whatever) > } else { > // don't bother -- already in CONTEXT_KERNEL > } This was the initial implementation of context tracking but it was terribly buggy. What if we enter the kernel, we haven't yet got a change to call context_tracking_user_exit() and we get an exception in the kernel entry path? user_mode(regs) will return the wrong value and bad things happen. This is why context tracking needs its own tracking state, because we are always out of sync with the real processor context anyway. > > i.e. the same thing that do_general_protection, etc do in -tip. That > would get rid of any need to store the previous context. > > Currently we can't because of syscalls and maybe because of KVM. KVM > has a weird fake interrupt thing. > > > > >> entering_irq(); > >> > >> + /* entering_irq() tells RCU that we're not quiescent. Check it. */ > >> + rcu_lockdep_assert(rcu_is_watching(), "IRQ failed to wake up RCU"); > > > > Why do we need to check that? > > Sanity check. If we're changing a bunch of context tracking details, > I want to assert that it actually works. But we call rcu_irq_enter() right before. It's more or less like doing: local_irq_disable(); WARN_ON(!irqs_disabled()); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Reconciling rcu_irq_enter()/rcu_nmi_enter() with context tracking
On Fri, Jul 17, 2015 at 11:59:18AM -0700, Andy Lutomirski wrote: > On Thu, Jul 16, 2015 at 9:49 PM, Paul E. McKenney > wrote: > > On Thu, Jul 16, 2015 at 09:29:07PM -0700, Paul E. McKenney wrote: > >> On Thu, Jul 16, 2015 at 06:53:15PM -0700, Andy Lutomirski wrote: > >> > For reasons that mystify me a bit, we currently track context tracking > >> > state separately from rcu's watching state. This results in strange > >> > artifacts: nothing generic cause IRQs to enter CONTEXT_KERNEL, and we > >> > can nest exceptions inside the IRQ handler (an example would be > >> > wrmsr_safe failing), and, in -next, we splat a warning: > >> > > >> > https://gist.github.com/sashalevin/a006a44989312f6835e7 > >> > > >> > I'm trying to make context tracking more exact, which will fix this > >> > issue (the particular splat that Sasha hit shouldn't be possible when > >> > I'm done), but I think it would be nice to unify all of this stuff. > >> > Would it be plausible for us to guarantee that RCU state is always in > >> > sync with context tracking state? If so, we could maybe simplify > >> > things and have fewer state variables. > >> > >> A noble goal. Might even be possible, and maybe even advantageous. > >> > >> But it is usually easier to say than to do. RCU really does need to make > >> some adjustments when the state changes, as do the other subsystems. > >> It might or might not be possible to do the transitions atomically. > >> And if the transitions are not atomic, there will still be weird code > >> paths where (say) the processor is considered non-idle, but RCU doesn't > >> realize it yet. Such a code path could not safely use rcu_read_lock(), > >> so you still need RCU to be able to scream if someone tries it. > >> Contrariwise, if there is a code path where the processor is considered > >> idle, but RCU thinks it is non-idle, that code path can stall > >> grace periods. (Yes, not a problem if the code path is short enough. > >> At least if the underlying VCPU is making progres...) > >> > >> Still, I cannot prove that it is impossible, and if it is possible, > >> then as you say, there might well be benefits. > >> > >> > Doing this for NMIs might be weird. Would it make sense to have a > >> > CONTEXT_NMI that's somehow valid even if the NMI happened while > >> > changing context tracking state. > >> > >> Face it, NMIs are weird. ;-) > >> > >> > Thoughts? As it stands, I think we might already be broken for real: > >> > > >> > Syscall -> user_exit. Perf NMI hits *during* user_exit. Perf does > >> > copy_from_user_nmi, which can fault, causing do_page_fault to get > >> > called, which calls exception_enter(), which can't be a good thing. > >> > > >> > RCU is okay (sort of) because of rcu_nmi_enter, but this seems very > >> > fragile. > >> > >> Actually, I see more cases where people forget irq_enter() than > >> rcu_nmi_enter(). "We will just nip in quickly and do something without > >> actually letting the irq system know. Oh, and we want some event tracing > >> in that code path." Boom! > >> > >> > Thoughts? As it stands, I need to do something because -tip and thus > >> > -next spews occasional warnings. > >> > >> Tell me more? > > > > And for completeness, RCU also has the following requirements on the > > state-transition mechanism: > > > > 1. It must be possible to reliably sample some other CPU's state. > > This is an energy-efficiency requirement, as RCU is not normally > > permitted to wake up idle CPUs. Nor nohz CPUs, for that matter. > > NOHZ needs this for vtime accounting, too. I think Rik might be > thinking about this. Maybe the underlying state could be shared? > > > > > 2. RCU must be able to track passage through idle and nohz states. > > In other words, if RCU samples at t=0 and finds that the CPU > > is executing (say) in kernel mode, and RCU samples again at > > t=10 and again finds that the CPU is executing in kernel mode, > > RCU needs to be able to determine whether or not that CPU passed > > through idle or nohz betweentimes. > > And RCU can do this for CONTEXT_KERNEL vs CONTEXT_USER because the > context tracking stuff notifies RCU. The think I'm less than happy > with is that we can currently be CONTEXT_USER but still rcu-awake. > This is manageable, but it seems messy. When we interrupt userspace, right? I don't see that much as a problem, until we use a unified context tracking for both RCU and context tracking. > > > > > 3. In some configurations, RCU needs to be able to block entry into > > nohz state, both for idle and userspace. > > > > Hmm. I suppose we could be CONTEXT_USER but still have RCU awake, > although the tick would have to stay on. Well 3) is handled by the tick nohz code so it's still external. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at
Re: Reconciling rcu_irq_enter()/rcu_nmi_enter() with context tracking
On Thu, Jul 16, 2015 at 06:53:15PM -0700, Andy Lutomirski wrote: > For reasons that mystify me a bit, we currently track context tracking > state separately from rcu's watching state. This results in strange > artifacts: nothing generic cause IRQs to enter CONTEXT_KERNEL, and we > can nest exceptions inside the IRQ handler (an example would be > wrmsr_safe failing), and, in -next, we splat a warning: > > https://gist.github.com/sashalevin/a006a44989312f6835e7 I don't know how it happened. But the context tracking code should be able to handle exceptions on irqs. They are supposed to be simply ignored with the in_interrupt() check on context_tracking_enter/exit(). > > I'm trying to make context tracking more exact, which will fix this > issue (the particular splat that Sasha hit shouldn't be possible when > I'm done), but I think it would be nice to unify all of this stuff. > Would it be plausible for us to guarantee that RCU state is always in > sync with context tracking state? If so, we could maybe simplify > things and have fewer state variables. RCU uses the same variables for idle and user tracking whereas context tracking only tracks user. So they are at least decoupled there. And we probably don't want RCU to use a different variable due to the overhead it brings on readers. But it could be a shifted count on the same variable. > > Doing this for NMIs might be weird. Would it make sense to have a > CONTEXT_NMI that's somehow valid even if the NMI happened while > changing context tracking state. > > Thoughts? As it stands, I think we might already be broken for real: > > Syscall -> user_exit. Perf NMI hits *during* user_exit. Perf does > copy_from_user_nmi, which can fault, causing do_page_fault to get > called, which calls exception_enter(), which can't be a good thing. I think the in_interrupt() handles that. Besides NMI has its own counter. > > RCU is okay (sort of) because of rcu_nmi_enter, but this seems very fragile. > > Thoughts? As it stands, I need to do something because -tip and thus > -next spews occasional warnings. But yeah if we can, it would be nice to use context tracking as the sole tracker that RCU can safely use. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v9 07/22] tracing: Add lock-free tracing_map
On Sat, 2015-07-18 at 02:40 +, Mathieu Desnoyers wrote: > - On Jul 17, 2015, at 7:44 PM, Tom Zanussi tom.zanu...@linux.intel.com > wrote: > > > On Fri, 2015-07-17 at 15:48 +, Mathieu Desnoyers wrote: > >> - On Jul 16, 2015, at 9:35 PM, Tom Zanussi tom.zanu...@linux.intel.com > >> wrote: > >> > >> > Hi Mathieu, > >> > > >> > On Thu, 2015-07-16 at 23:25 +, Mathieu Desnoyers wrote: > >> >> * Tom Zanussi wrote: > >> >> >> Add tracing_map, a special-purpose lock-free map for tracing. > >> >> >> > >> >> >> tracing_map is designed to aggregate or 'sum' one or more values > >> >> >> associated with a specific object of type tracing_map_elt, which > >> >> >> is associated by the map to a given key. > >> >> >> > >> >> >> It provides various hooks allowing per-tracer customization and is > >> >> >> separated out into a separate file in order to allow it to be shared > >> >> >> between multiple tracers, but isn't meant to be generally used > >> >> >> outside > >> >> >> of that context. > >> >> >> > >> >> >> The tracing_map implementation was inspired by lock-free map > >> >> >> algorithms originated by Dr. Cliff Click: > >> >> >> > >> >> >> http://www.azulsystems.com/blog/cliff/2007-03-26-non-blocking-hashtable > >> >> >> http://www.azulsystems.com/events/javaone_2007/2007_LockFreeHash.pdf > >> >> > >> >> Hi Tom, > >> >> > >> >> First question: what is the rationale for implementing another > >> >> hash table from scratch here ? What is missing in the pre-existing > >> >> hash table implementations ? > >> >> > >> > > >> > None of the other hash tables allow for lock-free insertion (and I > >> > didn't see an easy way to add it). > >> > >> This is one of the nice things about the Userspace RCU lock-free hash > >> table we've done a few years ago: it provides lock-free add, add_unique, > >> removal, and replace, as well as RCU wait-free lookups and traversals. > >> Resize can be done concurrently by a worker thread. I ported it to the > >> Linux kernel for Julien's work on latency tracker. You can find the > >> implementation here: https://github.com/jdesfossez/latency_tracker > >> (see rculfhash*) > >> It is a simplified version that has the "resize" feature removed for > >> simplicity sake. The "insert and lookup" feature you need is called > >> "add_unique" in our API: it behaves both as a lookup and as an atomic > >> insert if the key is not found. > >> > > > > Interesting, but it's just as much not upstream as mine is. ;-) > > Fair point, although the userspace RCU lock-free hash table has been > heavily tested and used in user-space in the context of routing tables > (I remember Stephen Hemminger uses it at Vyatta). So being in userspace > RCU library got it some exposure and use over years. > > > > > From the perspective of the hist triggers, it doesn't matter what hash > > table implementation it uses as long as whatever it is supports > > insertion in any context. In fact the current tracing_map > > implementation is already the second completely different implementation > > it's plugged into (see v2 of this patchset for the first). If yours is > > better and going upstream, I'd be happy to make it the third and forget > > about mine. > > I'm a big fan of awaiting that people show a need before proposing > something for Linux mainline. Having a lock-free hash table for the > sake of tracing triggers appears to be the perfect use-case. If you > think it can be useful to you, I can look into doing a proper port > to the Linux kernel. > And I'm a big fan of creating and reusing common components for multiple use-cases (see my v2 attempt, and I'm guessing you might have some uses for it too ;-) So yeah, I'd be happy to try it out as a replacement for tracing_map in the hist triggers. Tom > Thanks, > > Mathieu > > > > > Tom > > > >> Thanks, > >> > >> Mathieu > >> > >> > > >> >> Moreover, you might want to handle the case where jhash() returns > >> >> 0. AFAIU, there is a race on "insert" in this scenario. > >> >> > >> > > >> > You're right, in that case you'd accidentally overwrite an already > >> > claimed slot. Thanks for pointing that out. > >> > > >> > Tom > >> > > >> >> Thanks, > >> >> > >> >> Mathieu > >> >> > >> >> >> > >> >> >> Signed-off-by: Tom Zanussi > >> >> >> --- > >> >> >> kernel/trace/Makefile | 1 + > >> >> >> kernel/trace/tracing_map.c | 935 > >> >> >> + > >> >> >> kernel/trace/tracing_map.h | 258 + > >> >> >> 3 files changed, 1194 insertions(+) > >> >> >> create mode 100644 kernel/trace/tracing_map.c > >> >> >> create mode 100644 kernel/trace/tracing_map.h > >> >> >> > >> >> >> diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile > >> >> >> index 9b1044e..3b26cfb 100644 > >> >> >> --- a/kernel/trace/Makefile > >> >> >> +++ b/kernel/trace/Makefile > >> >> >> @@ -31,6 +31,7 @@ obj-$(CONFIG_TRACING) += trace_output.o > >> >> >> obj-$(CONFIG_TRACING) += trace_seq.o > >>
Your Trust
My name is Gatan Magsino, I work with Mediterranean Bank in Malta. Can i trust you with a business worth 8.3 million USD? Please reply ONLY to my private email: magsin...@yahoo.com.hk for more information. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC V5 2/4] perf,tool: per-event time support
On Fri, Jul 17, 2015 at 07:30:53AM -0400, kan.li...@intel.com wrote: SNIP > diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c > index a71eeb2..c9981df 100644 > --- a/tools/perf/util/parse-events.c > +++ b/tools/perf/util/parse-events.c > @@ -25,6 +25,9 @@ > #ifdef PARSER_DEBUG > extern int parse_events_debug; > #endif > + > +bool time_term_detected = false; > + > int parse_events_parse(void *data, void *scanner); > > static struct perf_pmu_event_symbol *perf_pmu_events_list; > @@ -598,6 +601,14 @@ do { >\ >* attr->branch_sample_type = term->val.num; >*/ > break; > + case PARSE_EVENTS__TERM_TYPE_TIME: > + CHECK_TYPE_VAL(NUM); > + if (term->val.num > 1) > + return -EINVAL; > + time_term_detected = true; > + if (term->val.num == 1) > + attr->sample_type |= PERF_SAMPLE_TIME; > + break; > case PARSE_EVENTS__TERM_TYPE_NAME: > CHECK_TYPE_VAL(STR); > break; > diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h > index 131f29b..1083478 100644 > --- a/tools/perf/util/parse-events.h > +++ b/tools/perf/util/parse-events.h > @@ -22,6 +22,8 @@ struct tracepoint_path { > struct tracepoint_path *next; > }; > > +extern bool time_term_detected; so I wasnt happy about this time_term_detected global variable, and I tried to make it without and ended up with somewhat siplified patch.. not tested very deeply, just the basics: [jolsa@krava perf]$ ./perf record -e 'cpu/cpu-cycles/,cpu/instructions/' kill ... [jolsa@krava perf]$ ./perf evlist -v cpu/cpu-cycles/: type: 4, size: 112, config: 0x3c, { sample_period, sample_freq }: 4000, sample_type: IP|TID|TIME|ID|PERIOD, read_format: ID, disabled: 1, inherit: 1, mmap: 1, comm: 1, freq: 1, enable_on_exec: 1, task: 1, sample_id_all: 1, exclude_guest: 1, mmap2: 1, comm_exec: 1 cpu/instructions/: type: 4, size: 112, config: 0xc0, { sample_period, sample_freq }: 4000, sample_type: IP|TID|TIME|ID|PERIOD, read_format: ID, disabled: 1, inherit: 1, freq: 1, enable_on_exec: 1, sample_id_all: 1, exclude_guest: 1 [jolsa@krava perf]$ ./perf record -e 'cpu/cpu-cycles/,cpu/instructions,time/' kill ... [jolsa@krava perf]$ ./perf evlist -v cpu/cpu-cycles/: type: 4, size: 112, config: 0x3c, { sample_period, sample_freq }: 4000, sample_type: IP|TID|PERIOD|IDENTIFIER, read_format: ID, disabled: 1, inherit: 1, mmap: 1, comm: 1, freq: 1, enable_on_exec: 1, task: 1, sample_id_all: 1, exclude_guest: 1, mmap2: 1, comm_exec: 1 cpu/instructions,time/: type: 4, size: 112, config: 0xc0, { sample_period, sample_freq }: 4000, sample_type: IP|TID|TIME|PERIOD|IDENTIFIER, read_format: ID, disabled: 1, inherit: 1, freq: 1, enable_on_exec: 1, sample_id_all: 1, exclude_guest: 1 thoughts? jirka --- diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt index 5b47b2c88223..df479077384d 100644 --- a/tools/perf/Documentation/perf-record.txt +++ b/tools/perf/Documentation/perf-record.txt @@ -49,7 +49,9 @@ OPTIONS These params can be used to set event defaults. Here is a list of the params. - 'period': Set event sampling period - + - 'time': Disable/enable time stamping. Acceptable values are 1 for + enabling time stamping. 0 for disabling time stamping. + The default is 1. Note: If user explicitly sets options which conflict with the params, the value set by the params will be overridden. diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c index 83c08037e7e2..8e3a17845c37 100644 --- a/tools/perf/util/evsel.c +++ b/tools/perf/util/evsel.c @@ -712,7 +712,8 @@ void perf_evsel__config(struct perf_evsel *evsel, struct record_opts *opts) perf_evsel__set_sample_bit(evsel, TIME); if (opts->raw_samples && !evsel->no_aux_samples) { - perf_evsel__set_sample_bit(evsel, TIME); + if (opts->sample_time) + perf_evsel__set_sample_bit(evsel, TIME); perf_evsel__set_sample_bit(evsel, RAW); perf_evsel__set_sample_bit(evsel, CPU); } diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c index a71eeb279ed2..95100478200a 100644 --- a/tools/perf/util/parse-events.c +++ b/tools/perf/util/parse-events.c @@ -598,6 +598,13 @@ do { \ * attr->branch_sample_type = term->val.num; */ break; + case PARSE_EVENTS__TERM_TYPE_TIME: + CHECK_TYPE_VAL(NUM); + if (term->val.num > 1) + return -EINVAL; + if (term->val.num == 1) +
Re: tw5864 driver development, help needed
An update: see there: http://ffmpeg.org/pipermail/ffmpeg-devel/2015-July/175796.html -- Bluecherry developer. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 3/4] touchscreen: colibri-vf50-ts: Add touchscreen support for Colibri VF50
Hello Dmitry, On 15-07-17 16:42:42, Dmitry Torokhov wrote: > Hi Sanchayan, > > > On Thu, Jul 16, 2015 at 08:43:21PM +0530, Sanchayan Maity wrote: > > The Colibri Vybrid VF50 module supports 4-wire touchscreens using > > FETs and ADC inputs. This driver uses the IIO consumer interface > > and relies on the vf610_adc driver based on the IIO framework. > > This looks pretty good, thank you. Just a few comments below. > > > > > Signed-off-by: Sanchayan Maity > > --- > > drivers/input/touchscreen/Kconfig | 12 + > > drivers/input/touchscreen/Makefile | 1 + > > drivers/input/touchscreen/colibri-vf50-ts.c | 451 > > > > 3 files changed, 464 insertions(+) > > create mode 100644 drivers/input/touchscreen/colibri-vf50-ts.c > > > > diff --git a/drivers/input/touchscreen/Kconfig > > b/drivers/input/touchscreen/Kconfig > > index 80f6386..28948ca 100644 > > --- a/drivers/input/touchscreen/Kconfig > > +++ b/drivers/input/touchscreen/Kconfig > > @@ -1027,4 +1027,16 @@ config TOUCHSCREEN_ZFORCE > > To compile this driver as a module, choose M here: the > > module will be called zforce_ts. > > > > +config TOUCHSCREEN_COLIBRI_VF50 > > + tristate "Toradex Colibri on board touchscreen driver" > > + depends on GPIOLIB && IIO && VF610_ADC > > + help > > + Say Y here if you have a Colibri VF50 and plan to use > > + the on-board provided 4-wire touchscreen driver. > > + > > + If unsure, say N. > > + > > + To compile this driver as a module, choose M here: the > > + module will be called colibri_vf50_ts. > > + > > endif > > diff --git a/drivers/input/touchscreen/Makefile > > b/drivers/input/touchscreen/Makefile > > index 44deea7..93746a0 100644 > > --- a/drivers/input/touchscreen/Makefile > > +++ b/drivers/input/touchscreen/Makefile > > @@ -84,3 +84,4 @@ obj-$(CONFIG_TOUCHSCREEN_W90X900) += w90p910_ts.o > > obj-$(CONFIG_TOUCHSCREEN_SX8654) += sx8654.o > > obj-$(CONFIG_TOUCHSCREEN_TPS6507X) += tps6507x-ts.o > > obj-$(CONFIG_TOUCHSCREEN_ZFORCE) += zforce_ts.o > > +obj-$(CONFIG_TOUCHSCREEN_COLIBRI_VF50) += colibri-vf50-ts.o > > diff --git a/drivers/input/touchscreen/colibri-vf50-ts.c > > b/drivers/input/touchscreen/colibri-vf50-ts.c > > new file mode 100644 > > index 000..eb16bdc > > --- /dev/null > > +++ b/drivers/input/touchscreen/colibri-vf50-ts.c > > @@ -0,0 +1,451 @@ > > +/* Copyright 2015 Toradex AG > > + * > > + * Toradex Colibri VF50 Touchscreen driver > > + * > > + * Originally authored by Stefan Agner for 3.0 kernel > > + * > > + * This program is free software; you can redistribute it and/or modify > > + * it under the terms of the GNU General Public License as published by > > + * the Free Software Foundation; either version 2 of the License, or > > + * (at your option) any later version. > > + */ > > + > > +#include > > Why? Remnant of old usage. Will fix. > > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > Is not used as far as I can see. Right. Crept in from old usage of extracting gpio information from DT. Should have removed when I switched to using gpiod_* and removal of legacy gpio API. Will fix. > > > +#include > > +#include > > +#include > > +#include > > + > > +#define DRIVER_NAME "colibri-vf50-ts" > > +#define DRV_VERSION "1.0" > > + > > +#define VF_ADC_MAX ((1 << 12) - 1) > > + > > +#define COLI_TOUCH_MIN_DELAY_US 1000 > > +#define COLI_TOUCH_MAX_DELAY_US 2000 > > + > > +static int min_pressure = 200; > > + > > +struct vf50_touch_device { > > + struct platform_device *pdev; > > + struct input_dev*ts_input; > > + struct workqueue_struct *ts_workqueue; > > + struct work_struct ts_work; > > + struct iio_channel *channels; > > + struct gpio_desc *gpio_xp; > > + struct gpio_desc *gpio_xm; > > + struct gpio_desc *gpio_yp; > > + struct gpio_desc *gpio_ym; > > + struct gpio_desc *gpio_pen_detect; > > + struct gpio_desc *gpio_pen_detect_pullup; > > + int pen_irq; > > + bool stop_touchscreen; > > +}; > > + > > +/* > > + * Enables given plates and measures touch parameters using ADC > > + */ > > +static int adc_ts_measure(struct iio_channel *channel, > > + struct gpio_desc *plate_p, struct gpio_desc *plate_m) > > +{ > > + int i, value = 0, val = 0; > > + int ret; > > + > > + gpiod_set_value(plate_p, 1); > > + gpiod_set_value(plate_m, 1); > > + > > + usleep_range(COLI_TOUCH_MIN_DELAY_US, COLI_TOUCH_MAX_DELAY_US); > > + > > + for (i = 0; i < 5; i++) { > > + ret = iio_read_channel_raw(channel, ); > > + if (ret < 0) > > + return -EINVAL; > > + > > + value += val; > > + } > > + > > + value /= 5; > > + > > + gpiod_set_value(plate_p, 0); > > + gpiod_set_value(plate_m, 0); > > + > > + return value; > > +} > > + > > +/* > > + * Enable touch detection using falling edge
[PATCH] Doc: fix trivial typo in SubmittingPatches
This patch changes the tense of a verb in SubmittingPatches to ensure grammatical validity of the containing sentence. Signed-off-by: Benjamin Herr --- Documentation/SubmittingPatches | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches index 27e7e5e..186b2d8 100644 --- a/Documentation/SubmittingPatches +++ b/Documentation/SubmittingPatches @@ -90,7 +90,7 @@ patch. Make sure your patch does not include any extra files which do not belong in a patch submission. Make sure to review your patch -after- -generated it with diff(1), to ensure accuracy. +generating it with diff(1), to ensure accuracy. If your changes produce a lot of deltas, you need to split them into individual patches which modify things in logical stages; see section -- 2.4.6 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 3/4] touchscreen: colibri-vf50-ts: Add touchscreen support for Colibri VF50
On 15-07-18 14:03:25, Nicolae Rosia wrote: > Hi, > > On Thu, Jul 16, 2015 at 6:13 PM, Sanchayan Maity > wrote: > > The Colibri Vybrid VF50 module supports 4-wire touchscreens using > > FETs and ADC inputs. This driver uses the IIO consumer interface > > and relies on the vf610_adc driver based on the IIO framework. > > > > Signed-off-by: Sanch > > +static const struct of_device_id vf50_touch_of_match[] = { > > + { .compatible = "toradex,vf50-touchctrl", }, > > + { } > > +}; > > +MODULE_DEVICE_TABLE(of, vf50_touch_of_match); > > + > > +static struct platform_driver __refdata vf50_touch_driver = { > > + .driver = { > > + .name = "toradex,vf50_touchctrl", > > + .of_match_table = vf50_touch_of_match, > > + }, > > + .probe = vf50_ts_probe, > > + .remove = vf50_ts_remove, > > + .prevent_deferred_probe = false, > > +}; > Why Toradex ? Isn't this a Freescale IP ? The 4 wire touchscreen support is provided by using on module circuitry mainly comprising of FET's and leveraging the GPIOs and on chip ADC of the Vybrid SoC. This is specific to our Colibri Vybrid VF50 module and not the Freescale IP. While I guess one could certainly use the driver for their own needs if one were to replicate a similar circuitry and change the DT properties concerning GPIO and ADC's as per their own board, as of now this is only use on our Toradex VF50 modules and was done by us specifically to provide touchscreen support for VF50. Regards, Sanchayan. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/5] drivers: staging: rtl8192u: fixing coding style issues in r8192U_core.c
On Fri, Jul 17, 2015 at 05:11:34PM -0700, Joe Perches wrote: > On Fri, 2015-07-17 at 15:59 +0200, Joseph-Eugene Winzer wrote: > > Reformatting the code without introducing other warnings > > like 'Avoid unnecessary line continuations' or breaking strings. > > Very few of these seem to be improvements and many > of them should likely be in separate patches. > > > diff --git a/drivers/staging/rtl8192u/r8192U_core.c > > b/drivers/staging/rtl8192u/r8192U_core.c > [] > > @@ -143,17 +143,35 @@ struct CHANNEL_LIST { > > }; > > > > static struct CHANNEL_LIST ChannelPlan[] = { > > - {{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 36, 40, 44, 48, 52, 56, 60, 64, > > 149, 153, 157, 161, 165}, 24}, /* FCC */ > > - {{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}, 11}, /* IC */ > > - {{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 36, 40, 44, 48, 52, 56, > > 60, 64}, 21}, /* ETSI */ > > - {{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13}, 13}, /* Spain. Change to > > ETSI. */ > > - {{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13}, 13}, /* France. Change to > > ETSI. */ > > - {{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 36, 40, 44, 48, 52, > > 56, 60, 64}, 22}, /* MKK //MKK */ > > - {{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 36, 40, 44, 48, 52, > > 56, 60, 64}, 22}, /* MKK1 */ > > - {{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13}, 13}, /* Israel. */ > > - {{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 36, 40, 44, 48, 52, > > 56, 60, 64}, 22}, /* For 11a , TELEC */ > > - {{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 36, 40, 44, 48, 52, > > 56, 60, 64}, 22}, /* MIC */ > > - {{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14}, 14} /* For Global > > Domain. 1-11:active scan, 12-14 passive scan. //+YJ, 080626 */ > > If these were to be reformatted, it'd probably be better > to use 10 per line so that the Len value that follows is > more obvious. Likely this struct should be const too. > Maybe something like: > > static const struct CHANNEL_LIST ChannelPlan[] = { > { > .Channel = { > 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, >11, 36, 40, 44, 48, 52, 56, 60, 64,149, > 153,157,161,165 > }, > .Len = 24, > }, > > etc. Add more spaces between channels if you really want. > > > @@ -179,7 +197,9 @@ static void rtl819x_set_channel_map(u8 channel_plan, > > struct r8192_priv *priv) > > min_chan = 1; > > max_chan = 14; > > } else { > > - RT_TRACE(COMP_ERR, "unknown rf chip, can't set channel > > map in function:%s()\n", __func__); > > + RT_TRACE(COMP_ERR, > > +"unknown rf chip, can't set channel map in > > function:%s()\n" > > +, __func__); > > No leading commas please. Put the comma after the format. > > > @@ -187,7 +207,10 @@ static void rtl819x_set_channel_map(u8 channel_plan, > > struct r8192_priv *priv) > >sizeof(GET_DOT11D_INFO(ieee)->channel_map)); > > /* Set new channel map */ > > for (i = 0; i < ChannelPlan[channel_plan].Len; i++) { > > - if (ChannelPlan[channel_plan].Channel[i] < > > min_chan || ChannelPlan[channel_plan].Channel[i] > max_chan) > > + if (ChannelPlan[channel_plan].Channel[i] < > > + min_chan || > > + ChannelPlan[channel_plan].Channel[i] > > > + max_chan) > > Likely better to use a temporary for ChannelPlan[channel_plan] > so these could become: > > if (cp->Channel[i] < min_chan || > cp->Channel[i] > max_chan) > etc... > > > @@ -1490,7 +1523,8 @@ static u8 QueryIsShort(u8 TxHT, u8 TxRate, cb_desc > > *tcb_desc) > > { > > u8 tmp_Short; > > > > - tmp_Short = (TxHT == 1) ? ((tcb_desc->bUseShortGI) ? 1 : 0) : > > ((tcb_desc->bUseShortPreamble) ? 1 : 0); > > + tmp_Short = (TxHT == 1) ? ((tcb_desc->bUseShortGI) ? 1 : 0) : > > + ((tcb_desc->bUseShortPreamble) ? 1 : 0); > > > > if (TxHT == 1 && TxRate != DESC90_RATEMCS15) > > tmp_Short = 0; > > Maybe using bool would be better. > > > @@ -1673,7 +1711,8 @@ short rtl8192_tx(struct net_device *dev, struct > > sk_buff *skb) > > 0, tx_zero_isr, dev); > > status = usb_submit_urb(tx_urb_zero, GFP_ATOMIC); > > if (status) { > > - RT_TRACE(COMP_ERR, "Error TX URB for zero byte > > %d, error %d", atomic_read(>tx_pending[tcb_desc->queue_index]), > > status); > > + RT_TRACE(COMP_ERR, > > +"Error TX URB
Re: [PATCH v5] arm DMA: Fix allocation from CMA for coherent DMA
On Fri, Jul 3, 2015 at 11:03 AM, Russell King - ARM Linux wrote: > On Wed, Jul 01, 2015 at 12:12:51PM +0100, Catalin Marinas wrote: >> On Tue, Jun 30, 2015 at 11:29:06PM +0200, Lorenzo Nava wrote: >> > This patch allows the use of CMA for DMA coherent memory allocation. >> > At the moment if the input parameter "is_coherent" is set to true >> > the allocation is not made using the CMA, which I think is not the >> > desired behaviour. >> > >> > Signed-off-by: Lorenzo Nava >> > Reviewed-by: Catalin Marinas >> >> If Russell doesn't have any objections, you can send the patch to >> his patch system. See here for more information: > > I'm left wondering whether this patch is really want Lorenzo wants. > From my reading of it, while this has the effect of allocating from > CMA for coherent devices, it's no different from the non-coherent > case, because by calling __alloc_from_contiguous(), we end up > remapping the allocated memory, removing the cacheability status > from the allocated pages. > > This brings up an interesting point: presumably, it's been tested, and > people are happy with the performance it's giving, inspite of it not > returning cacheable memory... or maybe it hasn't been tested that much? > Hello Russell, I was wondering if you have any other observations about this patch or if you need any further investigation about some aspects. It's not clear to me if the last comments from me and Catalin answer your questions or if you see any other problem that we didn't consider. Thank you. Lorenzo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC v2 0/2] hwspinlock: Introduce raw capability for hwspinlock_device
Hi Lina, On Thu, Jul 2, 2015 at 11:30 PM, Lina Iyer wrote: > You are right, RAW capability is not lock specific. But we dont want to > impose this on every lock in the bank either. I'm not sure I'm following your concern here: drivers still need to explicitly indicate RAW in order for this to kick in, so this lenient approach is not being imposed on them. Your original patch allowed every driver on all platforms to disable the sw spinlock mechanism. What I'm merely suggesting is that the underlying platform-specific driver should first allow this before it is being used, as some vendors prohibit this completely. Let's not make this more complicated than needed, so please add the hwcaps member to hwspinlock_device instead of to hwspinlock struct. We could always change this later if it proves to be insufficient. Thanks, Ohad. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 3/4] touchscreen: colibri-vf50-ts: Add touchscreen support for Colibri VF50
Hi, On Thu, Jul 16, 2015 at 6:13 PM, Sanchayan Maity wrote: > The Colibri Vybrid VF50 module supports 4-wire touchscreens using > FETs and ADC inputs. This driver uses the IIO consumer interface > and relies on the vf610_adc driver based on the IIO framework. > > Signed-off-by: Sanch > +static const struct of_device_id vf50_touch_of_match[] = { > + { .compatible = "toradex,vf50-touchctrl", }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, vf50_touch_of_match); > + > +static struct platform_driver __refdata vf50_touch_driver = { > + .driver = { > + .name = "toradex,vf50_touchctrl", > + .of_match_table = vf50_touch_of_match, > + }, > + .probe = vf50_ts_probe, > + .remove = vf50_ts_remove, > + .prevent_deferred_probe = false, > +}; Why Toradex ? Isn't this a Freescale IP ? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/2] tracing: gpio: add Kconfig option for enabling/disabling trace events
On second thought, I wonder if the config option shouldn't be in kernel/trace/Kconfig Imagine there's an option for every subsystem. Disabling all of them but the one you actually care about is going to be a mess if they're scattered all over the place. Your thoughts? On Fri, Jul 17, 2015 at 9:36 PM, Steven Rostedt wrote: > On Thu, 16 Jul 2015 20:39:36 +0300 > Tal Shorer wrote: > >> Add a new options to gpio Kconfig, CONFIG_GPIO_TRACING, that is used >> for enabling/disabling compilation of gpio function trace events. >> > > If I can get acks from the gpio maintainers, I can take this in my > tree, as it depends on code that modifies the core tracing facility. > > -- Steve > >> Signed-off-by: Tal Shorer >> --- >> drivers/gpio/Kconfig| 7 +++ >> include/trace/events/gpio.h | 4 >> 2 files changed, 11 insertions(+) >> >> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig >> index c1e2ca3..2829e8e 100644 >> --- a/drivers/gpio/Kconfig >> +++ b/drivers/gpio/Kconfig >> @@ -88,6 +88,13 @@ config GPIO_SYSFS >> config GPIO_GENERIC >> tristate >> >> +config GPIO_TRACING >> + bool "gpio tracing" >> + depends on TRACING >> + help >> + Enable tracing for gpio subsystem >> + >> + >> # put drivers in the right section, in alphabetical order >> >> config GPIO_DA9052 >> diff --git a/include/trace/events/gpio.h b/include/trace/events/gpio.h >> index 927a8ad..09af636 100644 >> --- a/include/trace/events/gpio.h >> +++ b/include/trace/events/gpio.h >> @@ -1,6 +1,10 @@ >> #undef TRACE_SYSTEM >> #define TRACE_SYSTEM gpio >> >> +#ifndef CONFIG_GPIO_TRACING >> +#define NOTRACE >> +#endif >> + >> #if !defined(_TRACE_GPIO_H) || defined(TRACE_HEADER_MULTI_READ) >> #define _TRACE_GPIO_H >> > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH V2 2/2] staging: rtl8188eu: core: Use tabs for indentation
Signed-off-by: Guillaume Bienkowski --- drivers/staging/rtl8188eu/core/rtw_mlme_ext.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c index 1a00d1b0..23bc1bf 100644 --- a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c +++ b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c @@ -4709,7 +4709,7 @@ void _linked_rx_signal_strehgth_display(struct adapter *padapter); void _linked_rx_signal_strehgth_display(struct adapter *padapter) { struct mlme_ext_priv*pmlmeext = >mlmeextpriv; - struct mlme_ext_info*pmlmeinfo = &(pmlmeext->mlmext_info); + struct mlme_ext_info*pmlmeinfo = &(pmlmeext->mlmext_info); u8 mac_id; int UndecoratedSmoothedPWDB; if ((pmlmeinfo->state&0x03) == WIFI_FW_STATION_STATE) -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH V2 1/2] staging: rtl8188eu: Fix style errors and warnings
Fix spaces before comma and indentation. Signed-off-by: Guillaume Bienkowski --- drivers/staging/rtl8188eu/core/rtw_mlme_ext.c | 42 +-- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c index a0b8f66..27d7561 100644 --- a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c +++ b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c @@ -672,10 +672,10 @@ static int _issue_probereq(struct adapter *padapter, struct ndis_802_11_ssid *ps get_rate_set(padapter, bssrate, _len); if (bssrate_len > 8) { - pframe = rtw_set_ie(pframe, _SUPPORTEDRATES_IE_ , 8, bssrate, &(pattrib->pktlen)); - pframe = rtw_set_ie(pframe, _EXT_SUPPORTEDRATES_IE_ , (bssrate_len - 8), (bssrate + 8), &(pattrib->pktlen)); + pframe = rtw_set_ie(pframe, _SUPPORTEDRATES_IE_, 8, bssrate, &(pattrib->pktlen)); + pframe = rtw_set_ie(pframe, _EXT_SUPPORTEDRATES_IE_, (bssrate_len - 8), (bssrate + 8), &(pattrib->pktlen)); } else { - pframe = rtw_set_ie(pframe, _SUPPORTEDRATES_IE_ , bssrate_len , bssrate, &(pattrib->pktlen)); + pframe = rtw_set_ie(pframe, _SUPPORTEDRATES_IE_, bssrate_len, bssrate, &(pattrib->pktlen)); } /* add wps_ie for wps2.0 */ @@ -944,13 +944,13 @@ static void issue_asocrsp(struct adapter *padapter, unsigned short status, /* capability */ val = *(unsigned short *)rtw_get_capability_from_ie(ie); - pframe = rtw_set_fixed_ie(pframe, _CAPABILITY_ , (unsigned char *), &(pattrib->pktlen)); + pframe = rtw_set_fixed_ie(pframe, _CAPABILITY_, (unsigned char *), &(pattrib->pktlen)); lestatus = cpu_to_le16(status); - pframe = rtw_set_fixed_ie(pframe , _STATUS_CODE_ , (unsigned char *), &(pattrib->pktlen)); + pframe = rtw_set_fixed_ie(pframe, _STATUS_CODE_, (unsigned char *), &(pattrib->pktlen)); leval = cpu_to_le16(pstat->aid | BIT(14) | BIT(15)); - pframe = rtw_set_fixed_ie(pframe, _ASOC_ID_ , (unsigned char *), &(pattrib->pktlen)); + pframe = rtw_set_fixed_ie(pframe, _ASOC_ID_, (unsigned char *), &(pattrib->pktlen)); if (pstat->bssratelen <= 8) { pframe = rtw_set_ie(pframe, _SUPPORTEDRATES_IE_, pstat->bssratelen, pstat->bssrateset, &(pattrib->pktlen)); @@ -999,7 +999,7 @@ static void issue_asocrsp(struct adapter *padapter, unsigned short status, } if (pmlmeinfo->assoc_AP_vendor == HT_IOT_PEER_REALTEK) - pframe = rtw_set_ie(pframe, _VENDOR_SPECIFIC_IE_, 6 , REALTEK_96B_IE, &(pattrib->pktlen)); + pframe = rtw_set_ie(pframe, _VENDOR_SPECIFIC_IE_, 6, REALTEK_96B_IE, &(pattrib->pktlen)); /* add WPS IE ie for wps 2.0 */ if (pmlmepriv->wps_assoc_resp_ie && pmlmepriv->wps_assoc_resp_ie_len > 0) { @@ -1069,7 +1069,7 @@ static void issue_assocreq(struct adapter *padapter) /* listen interval */ /* todo: listen interval for power saving */ le_tmp = cpu_to_le16(3); - memcpy(pframe , (unsigned char *)_tmp, 2); + memcpy(pframe, (unsigned char *)_tmp, 2); pframe += 2; pattrib->pktlen += 2; @@ -1122,10 +1122,10 @@ static void issue_assocreq(struct adapter *padapter) if (bssrate_len > 8) { - pframe = rtw_set_ie(pframe, _SUPPORTEDRATES_IE_ , 8, bssrate, &(pattrib->pktlen)); - pframe = rtw_set_ie(pframe, _EXT_SUPPORTEDRATES_IE_ , (bssrate_len - 8), (bssrate + 8), &(pattrib->pktlen)); + pframe = rtw_set_ie(pframe, _SUPPORTEDRATES_IE_, 8, bssrate, &(pattrib->pktlen)); + pframe = rtw_set_ie(pframe, _EXT_SUPPORTEDRATES_IE_, (bssrate_len - 8), (bssrate + 8), &(pattrib->pktlen)); } else { - pframe = rtw_set_ie(pframe, _SUPPORTEDRATES_IE_ , bssrate_len , bssrate, &(pattrib->pktlen)); + pframe = rtw_set_ie(pframe, _SUPPORTEDRATES_IE_, bssrate_len, bssrate, &(pattrib->pktlen)); } /* RSN */ @@ -1167,7 +1167,7 @@ static void issue_assocreq(struct adapter *padapter) memcpy(pmlmeinfo->HT_caps.u.HT_cap_element.MCS_rate, MCS_rate_2R, 16); break; } - pframe = rtw_set_ie(pframe, _HT_CAPABILITY_IE_, ie_len , (u8 *)(&(pmlmeinfo->HT_caps)), &(pattrib->pktlen)); + pframe = rtw_set_ie(pframe, _HT_CAPABILITY_IE_, ie_len, (u8 *)(&(pmlmeinfo->HT_caps)), &(pattrib->pktlen)); } } @@ -1197,7 +1197,7 @@ static void issue_assocreq(struct adapter *padapter) } if (pmlmeinfo->assoc_AP_vendor == HT_IOT_PEER_REALTEK) - pframe = rtw_set_ie(pframe, _VENDOR_SPECIFIC_IE_, 6 , REALTEK_96B_IE, &(pattrib->pktlen)); + pframe = rtw_set_ie(pframe, _VENDOR_SPECIFIC_IE_, 6, REALTEK_96B_IE,
Re: [PATCH v2 1/2] ASoC: rockchip: Add machine driver for max98090 codec
On Sat, Jul 18, 2015 at 01:08:43PM +0800, Xing Zheng wrote: > + ret = devm_snd_soc_register_card(>dev, card); > + if (ret) { > + dev_err(>dev, > + "Soc register card failed %d\n", ret); > + return ret; > + } > + > + return ret; > +} > + > +static int snd_rk_mc_remove(struct platform_device *pdev) > +{ > + struct snd_soc_card *soc_card = platform_get_drvdata(pdev); > + > + snd_soc_unregister_card(soc_card); The point with using devm_snd_soc_register_card() is that you don't need to manually unregister the card - devm_ ensures that the card is freed. signature.asc Description: Digital signature