* Re: [RFC PATCH v2 3/4] powerpc: Don't bolt the hpte in kernel_map_linear_page()
From: Benjamin Herrenschmidt @ 2013-04-15 6:56 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev, Li Zhong
In-Reply-To: <20130415035031.GA7494@iris.ozlabs.ibm.com>
On Mon, 2013-04-15 at 13:50 +1000, Paul Mackerras wrote:
> On Fri, Apr 12, 2013 at 10:16:59AM +0800, Li Zhong wrote:
> > It seems that in kernel_unmap_linear_page(), it only checks whether there
> > is a map in the linear_map_hash_slots array, so seems we don't need bolt
> > the hpte.
>
> I don't exactly understand your rationale here, but I don't think it's
> safe not to have linear mapping pages bolted. Basically, if a page
> will be used in the process of calling hash_page to demand-fault an
> HPTE into the hash table, then that page needs to be bolted, otherwise
> we can get an infinite recursion of HPT misses. That includes all
> kernel stack pages, among other things, so I think we need to leave
> the HPTE_V_BOLTED in there.
I suspect Li's confusion comes from the fact that he doesn't realizes
that we might evict random hash slots. If the linear mapping hash
entries could only be thrown out via kernel_unmap_linear_page() then his
comment would make sense. However this isn't the case.
Li: When faulting something in, if both the primary and secondary
buckets are full, we "somewhat randomly" evict the content of a slot and
replace it. However we only do that on non-bolted slots.
This is why the linear mapping (and the vmemmap) must be bolted.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH 1/8] Remove syslog prefix in uncompressed oops text
From: Michael Ellerman @ 2013-04-15 7:20 UTC (permalink / raw)
To: Aruna Balakrishnaiah
Cc: jkenisto, mahesh, linux-kernel, linuxppc-dev, paulus, anton
In-Reply-To: <20130410072100.20150.74661.stgit@aruna-ThinkPad-T420>
On Wed, Apr 10, 2013 at 12:51:00PM +0530, Aruna Balakrishnaiah wrote:
> Removal of syslog prefix in the uncompressed oops text will
> help in capturing more oops data.
Why does it help? Does this effect any existing tools?
cheers
^ permalink raw reply
* Re: [PATCH] powerpc/perf: Power8 PMU support
From: Benjamin Herrenschmidt @ 2013-04-15 7:31 UTC (permalink / raw)
To: Michael Ellerman
Cc: linux-kernel, acme, linuxppc-dev, Paul Mackerras, Anton Blanchard,
sukadev
In-Reply-To: <1365999438-20578-1-git-send-email-michael@ellerman.id.au>
On Mon, 2013-04-15 at 14:17 +1000, Michael Ellerman wrote:
> This patch adds preliminary support for the power8 PMU to perf.
Might be worthwhile to have a small blurb explaining roughly what you
mean by "preliminary" :-)
Cheers,
Ben.
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> ---
> arch/powerpc/perf/Makefile | 3 +-
> arch/powerpc/perf/power8-pmu.c | 454 ++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 456 insertions(+), 1 deletion(-)
> create mode 100644 arch/powerpc/perf/power8-pmu.c
>
> diff --git a/arch/powerpc/perf/Makefile b/arch/powerpc/perf/Makefile
> index af3fac2..472db18 100644
> --- a/arch/powerpc/perf/Makefile
> +++ b/arch/powerpc/perf/Makefile
> @@ -4,7 +4,8 @@ obj-$(CONFIG_PERF_EVENTS) += callchain.o
>
> obj-$(CONFIG_PPC_PERF_CTRS) += core-book3s.o
> obj64-$(CONFIG_PPC_PERF_CTRS) += power4-pmu.o ppc970-pmu.o power5-pmu.o \
> - power5+-pmu.o power6-pmu.o power7-pmu.o
> + power5+-pmu.o power6-pmu.o power7-pmu.o \
> + power8-pmu.o
> obj32-$(CONFIG_PPC_PERF_CTRS) += mpc7450-pmu.o
>
> obj-$(CONFIG_FSL_EMB_PERF_EVENT) += core-fsl-emb.o
> diff --git a/arch/powerpc/perf/power8-pmu.c b/arch/powerpc/perf/power8-pmu.c
> new file mode 100644
> index 0000000..106ae0b
> --- /dev/null
> +++ b/arch/powerpc/perf/power8-pmu.c
> @@ -0,0 +1,454 @@
> +/*
> + * Performance counter support for POWER8 processors.
> + *
> + * Copyright 2009 Paul Mackerras, IBM Corporation.
> + * Copyright 2013 Michael Ellerman, IBM Corporation.
> + *
> + * 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 <linux/kernel.h>
> +#include <linux/perf_event.h>
> +#include <asm/firmware.h>
> +
> +
> +/*
> + * Some power8 event codes.
> + */
> +#define PM_CYC 0x0001e
> +#define PM_GCT_NOSLOT_CYC 0x100f8
> +#define PM_CMPLU_STALL 0x4000a /* or 0x1e054 */
> +#define PM_INST_CMPL 0x00002
> +#define PM_BRU_FIN 0x10068
> +#define PM_BR_MPRED_CMPL 0x400f6
> +
> +
> +/*
> + * Raw event encoding for POWER8:
> + *
> + * 60 56 52 48 44 40 36 32
> + * | - - - - | - - - - | - - - - | - - - - | - - - - | - - - - | - - - - | - - - - |
> + * [ thresh_cmp ] [ thresh_ctl ]
> + * |
> + * thresh start/stop OR FAB match -*
> + *
> + * 28 24 20 16 12 8 4 0
> + * | - - - - | - - - - | - - - - | - - - - | - - - - | - - - - | - - - - | - - - - |
> + * [ ] [ sample ] [cache] [ pmc ] [unit ] c m [ pmcxsel ]
> + * | | | | |
> + * | | | | *- mark
> + * | | *- L1/L2/L3 cache_sel |
> + * | | |
> + * | *- sampling mode for marked events *- combine
> + * |
> + * *- thresh_sel
> + *
> + * Below uses IBM bit numbering.
> + *
> + * MMCR1[x:y] = unit (PMCxUNIT)
> + * MMCR1[x] = combine (PMCxCOMB)
> + *
> + * if pmc == 3 and unit == 0 and pmcxsel[0:6] == 0b0101011
> + * # PM_MRK_FAB_RSP_MATCH
> + * MMCR1[20:27] = thresh_ctl (FAB_CRESP_MATCH / FAB_TYPE_MATCH)
> + * else if pmc == 4 and unit == 0xf and pmcxsel[0:6] == 0b0101001
> + * # PM_MRK_FAB_RSP_MATCH_CYC
> + * MMCR1[20:27] = thresh_ctl (FAB_CRESP_MATCH / FAB_TYPE_MATCH)
> + * else
> + * MMCRA[48:55] = thresh_ctl (THRESH START/END)
> + *
> + * if thresh_sel:
> + * MMCRA[45:47] = thresh_sel
> + *
> + * if thresh_cmp:
> + * MMCRA[22:24] = thresh_cmp[0:2]
> + * MMCRA[25:31] = thresh_cmp[3:9]
> + *
> + * if unit == 6 or unit == 7
> + * MMCRC[53:55] = cache_sel[1:3] (L2EVENT_SEL)
> + * else if unit == 8 or unit == 9:
> + * if cache_sel[0] == 0: # L3 bank
> + * MMCRC[47:49] = cache_sel[1:3] (L3EVENT_SEL0)
> + * else if cache_sel[0] == 1:
> + * MMCRC[50:51] = cache_sel[2:3] (L3EVENT_SEL1)
> + * else if cache_sel[1]: # L1 event
> + * MMCR1[16] = cache_sel[2]
> + * MMCR1[17] = cache_sel[3]
> + *
> + * if mark:
> + * MMCRA[63] = 1 (SAMPLE_ENABLE)
> + * MMCRA[57:59] = sample[0:2] (RAND_SAMP_ELIG)
> + * MMCRA[61:62] = sample[3:4] (RAND_SAMP_MODE)
> + *
> + */
> +
> +#define EVENT_THR_CMP_SHIFT 40 /* Threshold CMP value */
> +#define EVENT_THR_CMP_MASK 0x3ff
> +#define EVENT_THR_CTL_SHIFT 32 /* Threshold control value (start/stop) */
> +#define EVENT_THR_CTL_MASK 0xffull
> +#define EVENT_THR_SEL_SHIFT 29 /* Threshold select value */
> +#define EVENT_THR_SEL_MASK 0x7
> +#define EVENT_THRESH_SHIFT 29 /* All threshold bits */
> +#define EVENT_THRESH_MASK 0x1fffffull
> +#define EVENT_SAMPLE_SHIFT 24 /* Sampling mode & eligibility */
> +#define EVENT_SAMPLE_MASK 0x1f
> +#define EVENT_CACHE_SEL_SHIFT 20 /* L2/L3 cache select */
> +#define EVENT_CACHE_SEL_MASK 0xf
> +#define EVENT_IS_L1 (4 << EVENT_CACHE_SEL_SHIFT)
> +#define EVENT_PMC_SHIFT 16 /* PMC number (1-based) */
> +#define EVENT_PMC_MASK 0xf
> +#define EVENT_UNIT_SHIFT 12 /* Unit */
> +#define EVENT_UNIT_MASK 0xf
> +#define EVENT_COMBINE_SHIFT 11 /* Combine bit */
> +#define EVENT_COMBINE_MASK 0x1
> +#define EVENT_MARKED_SHIFT 8 /* Marked bit */
> +#define EVENT_MARKED_MASK 0x1
> +#define EVENT_IS_MARKED (EVENT_MARKED_MASK << EVENT_MARKED_SHIFT)
> +#define EVENT_PSEL_MASK 0xff /* PMCxSEL value */
> +
> +/*
> + * Layout of constraint bits:
> + *
> + * 60 56 52 48 44 40 36 32
> + * | - - - - | - - - - | - - - - | - - - - | - - - - | - - - - | - - - - | - - - - |
> + * [ fab_match ] [ thresh_cmp ] [ thresh_ctl ] [ ]
> + * |
> + * thresh_sel -*
> + *
> + * 28 24 20 16 12 8 4 0
> + * | - - - - | - - - - | - - - - | - - - - | - - - - | - - - - | - - - - | - - - - |
> + * [ ] [ sample ] [ ] [6] [5] [4] [3] [2] [1]
> + * | |
> + * L1 I/D qualifier -* | Count of events for each PMC.
> + * | p1, p2, p3, p4, p5, p6.
> + * nc - number of counters -*
> + *
> + * The PMC fields P1..P6, and NC, are adder fields. As we accumulate constraints
> + * we want the low bit of each field to be added to any existing value.
> + *
> + * Everything else is a value field.
> + */
> +
> +#define CNST_FAB_MATCH_VAL(v) (((v) & EVENT_THR_CTL_MASK) << 56)
> +#define CNST_FAB_MATCH_MASK CNST_FAB_MATCH_VAL(EVENT_THR_CTL_MASK)
> +
> +/* We just throw all the threshold bits into the constraint */
> +#define CNST_THRESH_VAL(v) (((v) & EVENT_THRESH_MASK) << 32)
> +#define CNST_THRESH_MASK CNST_THRESH_VAL(EVENT_THRESH_MASK)
> +
> +#define CNST_L1_QUAL_VAL(v) (((v) & 3) << 22)
> +#define CNST_L1_QUAL_MASK CNST_L1_QUAL_VAL(3)
> +
> +#define CNST_SAMPLE_VAL(v) (((v) & EVENT_SAMPLE_MASK) << 16)
> +#define CNST_SAMPLE_MASK CNST_SAMPLE_VAL(EVENT_SAMPLE_MASK)
> +
> +/*
> + * For NC we are counting up to 4 events. This requires three bits, and we need
> + * the fifth event to overflow and set the 4th bit. To achieve that we bias the
> + * fields by 3 in test_adder.
> + */
> +#define CNST_NC_SHIFT 12
> +#define CNST_NC_VAL (1 << CNST_NC_SHIFT)
> +#define CNST_NC_MASK (8 << CNST_NC_SHIFT)
> +#define POWER8_TEST_ADDER (3 << CNST_NC_SHIFT)
> +
> +/*
> + * For the per-PMC fields we have two bits. The low bit is added, so if two
> + * events ask for the same PMC the sum will overflow, setting the high bit,
> + * indicating an error. So our mask sets the high bit.
> + */
> +#define CNST_PMC_SHIFT(pmc) ((pmc - 1) * 2)
> +#define CNST_PMC_VAL(pmc) (1 << CNST_PMC_SHIFT(pmc))
> +#define CNST_PMC_MASK(pmc) (2 << CNST_PMC_SHIFT(pmc))
> +
> +/* Our add_fields is defined as: */
> +#define POWER8_ADD_FIELDS \
> + CNST_PMC_VAL(1) | CNST_PMC_VAL(2) | CNST_PMC_VAL(3) | \
> + CNST_PMC_VAL(4) | CNST_PMC_VAL(5) | CNST_PMC_VAL(6) | CNST_NC_VAL
> +
> +
> +/* Bits in MMCR1 for POWER8 */
> +#define MMCR1_UNIT_SHIFT(pmc) (60 - (4 * ((pmc) - 1)))
> +#define MMCR1_COMBINE_SHIFT(pmc) (35 - ((pmc) - 1))
> +#define MMCR1_PMCSEL_SHIFT(pmc) (24 - (((pmc) - 1)) * 8)
> +#define MMCR1_DC_QUAL_SHIFT 47
> +#define MMCR1_IC_QUAL_SHIFT 46
> +
> +/* Bits in MMCRA for POWER8 */
> +#define MMCRA_SAMP_MODE_SHIFT 1
> +#define MMCRA_SAMP_ELIG_SHIFT 4
> +#define MMCRA_THR_CTL_SHIFT 8
> +#define MMCRA_THR_SEL_SHIFT 16
> +#define MMCRA_THR_CMP_SHIFT 32
> +#define MMCRA_SDAR_MODE_TLB (1ull << 42)
> +
> +
> +static inline bool event_is_fab_match(u64 event)
> +{
> + /* Only check pmc, unit and pmcxsel, ignore the edge bit (0) */
> + event &= 0xff0fe;
> +
> + /* PM_MRK_FAB_RSP_MATCH & PM_MRK_FAB_RSP_MATCH_CYC */
> + return (event == 0x30056 || event == 0x4f052);
> +}
> +
> +static int power8_get_constraint(u64 event, unsigned long *maskp, unsigned long *valp)
> +{
> + unsigned int unit, pmc, cache;
> + unsigned long mask, value;
> +
> + mask = value = 0;
> +
> + pmc = (event >> EVENT_PMC_SHIFT) & EVENT_PMC_MASK;
> + unit = (event >> EVENT_UNIT_SHIFT) & EVENT_UNIT_MASK;
> + cache = (event >> EVENT_CACHE_SEL_SHIFT) & EVENT_CACHE_SEL_MASK;
> +
> + if (pmc) {
> + if (pmc > 6)
> + return -1;
> +
> + mask |= CNST_PMC_MASK(pmc);
> + value |= CNST_PMC_VAL(pmc);
> +
> + if (pmc >= 5 && event != 0x500fa && event != 0x600f4)
> + return -1;
> + }
> +
> + if (pmc <= 4) {
> + /*
> + * Add to number of counters in use. Note this includes events with
> + * a PMC of 0 - they still need a PMC, it's just assigned later.
> + * Don't count events on PMC 5 & 6, there is only one valid event
> + * on each of those counters, and they are handled above.
> + */
> + mask |= CNST_NC_MASK;
> + value |= CNST_NC_VAL;
> + }
> +
> + if (unit >= 6 && unit <= 9) {
> + /*
> + * L2/L3 events contain a cache selector field, which is
> + * supposed to be programmed into MMCRC. However MMCRC is only
> + * HV writable, and there is no API for guest kernels to modify
> + * it. The solution is for the hypervisor to initialise the
> + * field to zeroes, and for us to only ever allow events that
> + * have a cache selector of zero.
> + */
> + if (cache)
> + return -1;
> +
> + } else if (event & EVENT_IS_L1) {
> + mask |= CNST_L1_QUAL_MASK;
> + value |= CNST_L1_QUAL_VAL(cache);
> + }
> +
> + if (event & EVENT_IS_MARKED) {
> + mask |= CNST_SAMPLE_MASK;
> + value |= CNST_SAMPLE_VAL(event >> EVENT_SAMPLE_SHIFT);
> + }
> +
> + /*
> + * Special case for PM_MRK_FAB_RSP_MATCH and PM_MRK_FAB_RSP_MATCH_CYC,
> + * the threshold control bits are used for the match value.
> + */
> + if (event_is_fab_match(event)) {
> + mask |= CNST_FAB_MATCH_MASK;
> + value |= CNST_FAB_MATCH_VAL(event >> EVENT_THR_CTL_SHIFT);
> + } else {
> + /*
> + * Check the mantissa upper two bits are not zero, unless the
> + * exponent is also zero. See the THRESH_CMP_MANTISSA doc.
> + */
> + unsigned int cmp, exp;
> +
> + cmp = (event >> EVENT_THR_CMP_SHIFT) & EVENT_THR_CMP_MASK;
> + exp = cmp >> 7;
> +
> + if (exp && (cmp & 0x60) == 0)
> + return -1;
> +
> + mask |= CNST_THRESH_MASK;
> + value |= CNST_THRESH_VAL(event >> EVENT_THRESH_SHIFT);
> + }
> +
> + *maskp = mask;
> + *valp = value;
> +
> + return 0;
> +}
> +
> +static int power8_compute_mmcr(u64 event[], int n_ev,
> + unsigned int hwc[], unsigned long mmcr[])
> +{
> + unsigned long mmcra, mmcr1, unit, combine, psel, cache, val;
> + unsigned int pmc, pmc_inuse;
> + int i;
> +
> + pmc_inuse = 0;
> +
> + /* First pass to count resource use */
> + for (i = 0; i < n_ev; ++i) {
> + pmc = (event[i] >> EVENT_PMC_SHIFT) & EVENT_PMC_MASK;
> + if (pmc)
> + pmc_inuse |= 1 << pmc;
> + }
> +
> + /* In continous sampling mode, update SDAR on TLB miss */
> + mmcra = MMCRA_SDAR_MODE_TLB;
> + mmcr1 = 0;
> +
> + /* Second pass: assign PMCs, set all MMCR1 fields */
> + for (i = 0; i < n_ev; ++i) {
> + pmc = (event[i] >> EVENT_PMC_SHIFT) & EVENT_PMC_MASK;
> + unit = (event[i] >> EVENT_UNIT_SHIFT) & EVENT_UNIT_MASK;
> + combine = (event[i] >> EVENT_COMBINE_SHIFT) & EVENT_COMBINE_MASK;
> + psel = event[i] & EVENT_PSEL_MASK;
> +
> + if (!pmc) {
> + for (pmc = 1; pmc <= 4; ++pmc) {
> + if (!(pmc_inuse & (1 << pmc)))
> + break;
> + }
> +
> + pmc_inuse |= 1 << pmc;
> + }
> +
> + if (pmc <= 4) {
> + mmcr1 |= unit << MMCR1_UNIT_SHIFT(pmc);
> + mmcr1 |= combine << MMCR1_COMBINE_SHIFT(pmc);
> + mmcr1 |= psel << MMCR1_PMCSEL_SHIFT(pmc);
> + }
> +
> + if (event[i] & EVENT_IS_L1) {
> + cache = event[i] >> EVENT_CACHE_SEL_SHIFT;
> + mmcr1 |= (cache & 1) << MMCR1_IC_QUAL_SHIFT;
> + cache >>= 1;
> + mmcr1 |= (cache & 1) << MMCR1_DC_QUAL_SHIFT;
> + }
> +
> + if (event[i] & EVENT_IS_MARKED) {
> + mmcra |= MMCRA_SAMPLE_ENABLE;
> +
> + val = (event[i] >> EVENT_SAMPLE_SHIFT) & EVENT_SAMPLE_MASK;
> + if (val) {
> + mmcra |= (val & 3) << MMCRA_SAMP_MODE_SHIFT;
> + mmcra |= (val >> 2) << MMCRA_SAMP_ELIG_SHIFT;
> + }
> + }
> +
> + /*
> + * PM_MRK_FAB_RSP_MATCH and PM_MRK_FAB_RSP_MATCH_CYC,
> + * the threshold bits are used for the match value.
> + */
> + if (event_is_fab_match(event[i])) {
> + mmcr1 |= (event[i] >> EVENT_THR_CTL_SHIFT) &
> + EVENT_THR_CTL_MASK;
> + } else {
> + val = (event[i] >> EVENT_THR_CTL_SHIFT) & EVENT_THR_CTL_MASK;
> + mmcra |= val << MMCRA_THR_CTL_SHIFT;
> + val = (event[i] >> EVENT_THR_SEL_SHIFT) & EVENT_THR_SEL_MASK;
> + mmcra |= val << MMCRA_THR_SEL_SHIFT;
> + val = (event[i] >> EVENT_THR_CMP_SHIFT) & EVENT_THR_CMP_MASK;
> + mmcra |= val << MMCRA_THR_CMP_SHIFT;
> + }
> +
> + hwc[i] = pmc - 1;
> + }
> +
> + /* Return MMCRx values */
> + mmcr[0] = 0;
> +
> + /* pmc_inuse is 1-based */
> + if (pmc_inuse & 2)
> + mmcr[0] = MMCR0_PMC1CE;
> +
> + if (pmc_inuse & 0x7c)
> + mmcr[0] |= MMCR0_PMCjCE;
> +
> + mmcr[1] = mmcr1;
> + mmcr[2] = mmcra;
> +
> + return 0;
> +}
> +
> +static void power8_disable_pmc(unsigned int pmc, unsigned long mmcr[])
> +{
> + if (pmc <= 3)
> + mmcr[1] &= ~(0xffUL << MMCR1_PMCSEL_SHIFT(pmc + 1));
> +}
> +
> +PMU_FORMAT_ATTR(event, "config:0-49");
> +PMU_FORMAT_ATTR(pmcxsel, "config:0-7");
> +PMU_FORMAT_ATTR(mark, "config:8");
> +PMU_FORMAT_ATTR(combine, "config:11");
> +PMU_FORMAT_ATTR(unit, "config:12-15");
> +PMU_FORMAT_ATTR(pmc, "config:16-19");
> +PMU_FORMAT_ATTR(cache_sel, "config:20-23");
> +PMU_FORMAT_ATTR(sample_mode, "config:24-28");
> +PMU_FORMAT_ATTR(thresh_sel, "config:29-31");
> +PMU_FORMAT_ATTR(thresh_stop, "config:32-35");
> +PMU_FORMAT_ATTR(thresh_start, "config:36-39");
> +PMU_FORMAT_ATTR(thresh_cmp, "config:40-49");
> +
> +static struct attribute *power8_pmu_format_attr[] = {
> + &format_attr_event.attr,
> + &format_attr_pmcxsel.attr,
> + &format_attr_mark.attr,
> + &format_attr_combine.attr,
> + &format_attr_unit.attr,
> + &format_attr_pmc.attr,
> + &format_attr_cache_sel.attr,
> + &format_attr_sample_mode.attr,
> + &format_attr_thresh_sel.attr,
> + &format_attr_thresh_stop.attr,
> + &format_attr_thresh_start.attr,
> + &format_attr_thresh_cmp.attr,
> + NULL,
> +};
> +
> +struct attribute_group power8_pmu_format_group = {
> + .name = "format",
> + .attrs = power8_pmu_format_attr,
> +};
> +
> +static const struct attribute_group *power8_pmu_attr_groups[] = {
> + &power8_pmu_format_group,
> + NULL,
> +};
> +
> +static int power8_generic_events[] = {
> + [PERF_COUNT_HW_CPU_CYCLES] = PM_CYC,
> + [PERF_COUNT_HW_STALLED_CYCLES_FRONTEND] = PM_GCT_NOSLOT_CYC,
> + [PERF_COUNT_HW_STALLED_CYCLES_BACKEND] = PM_CMPLU_STALL,
> + [PERF_COUNT_HW_INSTRUCTIONS] = PM_INST_CMPL,
> + [PERF_COUNT_HW_BRANCH_INSTRUCTIONS] = PM_BRU_FIN,
> + [PERF_COUNT_HW_BRANCH_MISSES] = PM_BR_MPRED_CMPL,
> +};
> +
> +static struct power_pmu power8_pmu = {
> + .name = "POWER8",
> + .n_counter = 6,
> + .max_alternatives = 0,
> + .add_fields = POWER8_ADD_FIELDS,
> + .test_adder = POWER8_TEST_ADDER,
> + .compute_mmcr = power8_compute_mmcr,
> + .get_constraint = power8_get_constraint,
> + .disable_pmc = power8_disable_pmc,
> + .flags = PPMU_HAS_SSLOT | PPMU_HAS_SIER,
> + .n_generic = ARRAY_SIZE(power8_generic_events),
> + .generic_events = power8_generic_events,
> + .attr_groups = power8_pmu_attr_groups,
> +};
> +
> +static int __init init_power8_pmu(void)
> +{
> + if (!cur_cpu_spec->oprofile_cpu_type ||
> + strcmp(cur_cpu_spec->oprofile_cpu_type, "ppc64/power8"))
> + return -ENODEV;
> +
> + return register_power_pmu(&power8_pmu);
> +}
> +early_initcall(init_power8_pmu);
^ permalink raw reply
* Re: [PATCH 2/8] Add version and timestamp to oops header
From: Michael Ellerman @ 2013-04-15 7:31 UTC (permalink / raw)
To: Aruna Balakrishnaiah
Cc: jkenisto, mahesh, linux-kernel, linuxppc-dev, paulus, anton
In-Reply-To: <20130410072112.20150.10281.stgit@aruna-ThinkPad-T420>
On Wed, Apr 10, 2013 at 12:51:12PM +0530, Aruna Balakrishnaiah wrote:
> Introduce version and timestamp information in the oops header.
> oops_log_info (oops header) holds version (to distinguish between old
> and new format oops header), length of the oops text
> (compressed or uncompressed) and timestamp.
This needs a much more detailed explanation.
I think what you're doing is you're overlaying the new information so
that the version field in oops_log_info sits in the same location as the
length field in the old format. And then you're defining the version to
be a value that is an illegal length.
So existing tools will refuse to dump new style partitions,
because they'll think the length is too large. You've tested that?
Updated tools will know about both formats, so will be able to handle
either old or new style partitions.
Is that correct?
And we're adding the timestamp just because we can and it'd be nice to
have?
cheers
^ permalink raw reply
* Re: [PATCH 3/8] Introduce generic read function to read nvram-partitions
From: Michael Ellerman @ 2013-04-15 7:35 UTC (permalink / raw)
To: Aruna Balakrishnaiah
Cc: jkenisto, mahesh, linux-kernel, linuxppc-dev, paulus, anton
In-Reply-To: <20130410072125.20150.97063.stgit@aruna-ThinkPad-T420>
On Wed, Apr 10, 2013 at 12:51:25PM +0530, Aruna Balakrishnaiah wrote:
> Introduce generic read function to read nvram partitions other than rtas.
> nvram_read_error_log will be retained which is used to read rtas partition
> from rtasd. nvram_read_partition is the generic read function to read from
> any nvram partition.
>
> Signed-off-by: Aruna Balakrishnaiah <aruna@linux.vnet.ibm.com>
> Reviewed-by: Jim Keniston <jkenisto@us.ibm.com>
> ---
> arch/powerpc/platforms/pseries/nvram.c | 34 +++++++++++++++++++++++---------
> 1 file changed, 24 insertions(+), 10 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/nvram.c b/arch/powerpc/platforms/pseries/nvram.c
> index 742735a..6701b71 100644
> --- a/arch/powerpc/platforms/pseries/nvram.c
> +++ b/arch/powerpc/platforms/pseries/nvram.c
> @@ -293,34 +293,37 @@ int nvram_write_error_log(char * buff, int length,
> return rc;
> }
>
> -/* nvram_read_error_log
> +/* nvram_read_partition
> *
> - * Reads nvram for error log for at most 'length'
> + * Reads nvram partition for at most 'length'
> */
> -int nvram_read_error_log(char * buff, int length,
> - unsigned int * err_type, unsigned int * error_log_cnt)
> +int nvram_read_partition(struct nvram_os_partition *part, char *buff,
> + int length, unsigned int *err_type,
> + unsigned int *error_log_cnt)
> {
> int rc;
> loff_t tmp_index;
> struct err_log_info info;
>
> - if (rtas_log_partition.index == -1)
> + if (part->index == -1)
> return -1;
>
> - if (length > rtas_log_partition.size)
> - length = rtas_log_partition.size;
> + if (length > part->size)
> + length = part->size;
>
> - tmp_index = rtas_log_partition.index;
> + tmp_index = part->index;
>
> rc = ppc_md.nvram_read((char *)&info, sizeof(struct err_log_info), &tmp_index);
> if (rc <= 0) {
> - printk(KERN_ERR "nvram_read_error_log: Failed nvram_read (%d)\n", rc);
> + printk(KERN_ERR "nvram_read_partition: "
> + "Failed nvram_read (%d)\n", rc);
Should be:
pr_err("%s: Failed ..\n", __FUNCTION__, ..)
cheers
^ permalink raw reply
* Re: [PATCH 0/8] Nvram-to-pstore
From: Michael Ellerman @ 2013-04-15 7:36 UTC (permalink / raw)
To: Aruna Balakrishnaiah
Cc: jkenisto, mahesh, linux-kernel, linuxppc-dev, paulus, anton
In-Reply-To: <20130410071835.20150.56489.stgit@aruna-ThinkPad-T420>
On Wed, Apr 10, 2013 at 12:50:47PM +0530, Aruna Balakrishnaiah wrote:
> Currently the kernel provides the contents of p-series NVRAM only as a
> simple stream of bytes via /dev/nvram, which must be interpreted in user
> space by the nvram command in the powerpc-utils package. This patch set
> exploits the pstore subsystem to expose each partition in NVRAM as a
> separate file in /dev/pstore. For instance Oops messages will stored in a
> file named [dmesg-nvram-2].
Please try to fold some of that info into the commit messages for actual
patches. The 0th patch is lost when we commit the series into git.
Also all your patches should have a subject starting with
"powerpc/pseries:".
cheers
^ permalink raw reply
* Re: [PATCH 1/8] Remove syslog prefix in uncompressed oops text
From: aruna @ 2013-04-15 7:39 UTC (permalink / raw)
To: Michael Ellerman
Cc: jkenisto, mahesh, linux-kernel, linuxppc-dev, paulus, anton
In-Reply-To: <20130415072027.GA30156@concordia>
On Monday 15 April 2013 12:50 PM, Michael Ellerman wrote:
> On Wed, Apr 10, 2013 at 12:51:00PM +0530, Aruna Balakrishnaiah wrote:
>> Removal of syslog prefix in the uncompressed oops text will
>> help in capturing more oops data.
> Why does it help? Does this effect any existing tools?
>
> cheers
>
By setting the (2nd) syslog argument of kmsg_dump_get_buffer() to false,
we omit <n> line prefixes and thereby capture more of the printk buffer.
No this should not effect any existing tools.
Regards,
Aruna
^ permalink raw reply
* Re: [PATCH 4/8] Read/Write oops nvram partition via pstore
From: Michael Ellerman @ 2013-04-15 7:55 UTC (permalink / raw)
To: Aruna Balakrishnaiah
Cc: jkenisto, mahesh, linux-kernel, linuxppc-dev, paulus, anton
In-Reply-To: <20130410072303.20150.61382.stgit@aruna-ThinkPad-T420>
On Wed, Apr 10, 2013 at 12:53:03PM +0530, Aruna Balakrishnaiah wrote:
> This patch exploits pstore infrastructure in power systems.
> IBM's system p machines provide persistent storage for LPARs
In the kernel we use "pseries" instead of "system p".
> through NVRAM. NVRAM's lnx,oops-log partition is used to log
> oops messages. In case pstore registration fails it will
> fall back to kmsg_dump mechanism.
What are the implications of falling back to kmsg_dump()?
Is there any reason we would not want to enable CONFIG_PSTORE ? ie.
should the pseries platform just select it?
> diff --git a/arch/powerpc/platforms/pseries/nvram.c b/arch/powerpc/platforms/pseries/nvram.c
> index 6701b71..82d32a2 100644
> --- a/arch/powerpc/platforms/pseries/nvram.c
> +++ b/arch/powerpc/platforms/pseries/nvram.c
> @@ -18,6 +18,7 @@
> #include <linux/spinlock.h>
> #include <linux/slab.h>
> #include <linux/kmsg_dump.h>
> +#include <linux/pstore.h>
> #include <linux/ctype.h>
> #include <linux/zlib.h>
> #include <asm/uaccess.h>
> @@ -87,6 +88,25 @@ static struct kmsg_dumper nvram_kmsg_dumper = {
> .dump = oops_to_nvram
> };
>
> +static int nvram_pstore_open(struct pstore_info *psi);
> +
> +static ssize_t nvram_pstore_read(u64 *id, enum pstore_type_id *type,
> + int *count, struct timespec *time, char **buf,
> + struct pstore_info *psi);
> +
> +static int nvram_pstore_write(enum pstore_type_id type,
> + enum kmsg_dump_reason reason, u64 *id,
> + unsigned int part, int count, size_t size,
> + struct pstore_info *psi);
I think you should be able to rearrange this so that you don't need the
forward declarations.
> +
> +static struct pstore_info nvram_pstore_info = {
> + .owner = THIS_MODULE,
> + .name = "nvram",
> + .open = nvram_pstore_open,
> + .read = nvram_pstore_read,
> + .write = nvram_pstore_write,
> +};
> +
> /* See clobbering_unread_rtas_event() */
> #define NVRAM_RTAS_READ_TIMEOUT 5 /* seconds */
> static unsigned long last_unread_rtas_event; /* timestamp */
> @@ -121,6 +141,13 @@ static char *big_oops_buf, *oops_buf;
> static char *oops_data;
> static size_t oops_data_sz;
>
> +#ifdef CONFIG_PSTORE
If we are going to have CONFIG_PSTORE #ifdefs in this file, I don't see
why there can't be just a single block of code that is #ifdef'ed, rather
than several like you have.
> +static enum pstore_type_id nvram_type_ids[] = {
> + PSTORE_TYPE_DMESG,
> + -1
> +};
> +static int read_type;
I don't understand what you're doing with read_type. It looks fishy.
> +#endif
> /* Compression parameters */
> #define COMPR_LEVEL 6
> #define WINDOW_BITS 12
> @@ -455,6 +482,23 @@ static void __init nvram_init_oops_partition(int rtas_partition_exists)
> oops_data = oops_buf + sizeof(struct oops_log_info);
> oops_data_sz = oops_log_partition.size - sizeof(struct oops_log_info);
>
> + nvram_pstore_info.buf = oops_data;
> + nvram_pstore_info.bufsize = oops_data_sz;
> +
> + rc = pstore_register(&nvram_pstore_info);
> +
> + if (rc != 0) {
> + pr_err("nvram: pstore_register() failed, defaults to "
> + "kmsg_dump; returned %d\n", rc);
> + goto kmsg_dump;
You don't need the goto.
> + } else {
> + /*TODO: Support compression when pstore is configured */
What is the issue here?
> + pr_info("nvram: Compression of oops text supported only when "
> + "pstore is not configured");
> + return;
> + }
> +
> +kmsg_dump:
> /*
> * Figure compression (preceded by elimination of each line's <n>
> * severity prefix) will reduce the oops/panic report to at most
> @@ -663,3 +707,104 @@ static void oops_to_nvram(struct kmsg_dumper *dumper,
>
> spin_unlock_irqrestore(&lock, flags);
> }
> +
> +#ifdef CONFIG_PSTORE
Same comment about too many ifdefs.
> +static int nvram_pstore_open(struct pstore_info *psi)
> +{
> + read_type = -1;
Locking?
> + return 0;
> +}
> +
> +/*
Make it a kernel-doc style comment.
> + * Called by pstore_dump() when an oops or panic report is logged to the printk
> + * buffer. @size bytes have been written to oops_buf, starting after the
> + * oops_log_info header.
"@size bytes have", or "@size bytes should be written"?
> + */
> +static int nvram_pstore_write(enum pstore_type_id type,
> + enum kmsg_dump_reason reason,
> + u64 *id, unsigned int part, int count,
> + size_t size, struct pstore_info *psi)
> +{
> + struct oops_log_info *oops_hdr = (struct oops_log_info *) oops_buf;
> +
> + /* part 1 has the recent messages from printk buffer */
> + if (part > 1 || clobbering_unread_rtas_event())
> + return -1;
> +
> + BUG_ON(type != PSTORE_TYPE_DMESG);
> + BUG_ON(sizeof(*oops_hdr) + size > oops_log_partition.size);
Why would we be called with the wrong type? Would it be better to just
return an error, rather than causing another oops while we're trying to
write the oops?
And couldn't we just clamp the size, rather than BUG'ing.
> + oops_hdr->version = OOPS_HDR_VERSION;
> + oops_hdr->report_length = (u16) size;
> + oops_hdr->timestamp = get_seconds();
> + (void) nvram_write_os_partition(&oops_log_partition, oops_buf,
> + (int) (sizeof(*oops_hdr) + size), ERR_TYPE_KERNEL_PANIC,
> + count);
You definitely don't need the (void). But more to the point why aren't
you checking the return value?
> + *id = part;
What is this? Part of the API?
> + return 0;
> +}
> +
> +/*
> + * Reads the oops/panic report.
> + * Returns the length of the data we read from each partition.
> + * Returns 0 if we've been called before.
> + */
> +static ssize_t nvram_pstore_read(u64 *id, enum pstore_type_id *type,
> + int *count, struct timespec *time, char **buf,
> + struct pstore_info *psi)
> +{
> + struct oops_log_info *oops_hdr;
> + unsigned int err_type, id_no;
> + struct nvram_os_partition *part = NULL;
> + char *buff = NULL;
> +
> + read_type++;
> +
> + switch (nvram_type_ids[read_type]) {
> + case PSTORE_TYPE_DMESG:
> + part = &oops_log_partition;
> + *type = PSTORE_TYPE_DMESG;
> + break;
> + default:
> + return 0;
> + }
> +
> + buff = kmalloc(part->size, GFP_KERNEL);
> +
> + if (!buff)
> + return -ENOMEM;
> +
> + if (nvram_read_partition(part, buff, part->size, &err_type, &id_no)) {
> + kfree(buff);
> + return 0;
> + }
> +
> + *count = 0;
> + *id = id_no;
Can't you just cast in the call to nvram_read_partition() ?
> + oops_hdr = (struct oops_log_info *)buff;
> + *buf = buff + sizeof(*oops_hdr);
> + time->tv_sec = oops_hdr->timestamp;
> + time->tv_nsec = 0;
> + return oops_hdr->report_length;
> +}
> +#else
> +static int nvram_pstore_open(struct pstore_info *psi)
> +{
> + return 0;
> +}
> +
> +static int nvram_pstore_write(enum pstore_type_id type,
> + enum kmsg_dump_reason reason, u64 *id,
> + unsigned int part, int count, size_t size,
> + struct pstore_info *psi)
> +{
> + return 0;
> +}
> +
> +static ssize_t nvram_pstore_read(u64 *id, enum pstore_type_id *type,
> + int *count, struct timespec *time, char **buf,
> + struct pstore_info *psi)
> +{
> + return 0;
> +}
> +#endif
I don't understand why we have empty versions of these. If CONFIG_PSTORE
is disabled we should just not register with pstore at all.
cheers
^ permalink raw reply
* Re: [PATCH 2/8] Add version and timestamp to oops header
From: aruna @ 2013-04-15 7:51 UTC (permalink / raw)
To: Michael Ellerman
Cc: jkenisto, mahesh, linux-kernel, linuxppc-dev, paulus, anton
In-Reply-To: <20130415073138.GB30156@concordia>
On Monday 15 April 2013 01:01 PM, Michael Ellerman wrote:
> On Wed, Apr 10, 2013 at 12:51:12PM +0530, Aruna Balakrishnaiah wrote:
>> Introduce version and timestamp information in the oops header.
>> oops_log_info (oops header) holds version (to distinguish between old
>> and new format oops header), length of the oops text
>> (compressed or uncompressed) and timestamp.
> This needs a much more detailed explanation.
>
> I think what you're doing is you're overlaying the new information so
> that the version field in oops_log_info sits in the same location as the
> length field in the old format. And then you're defining the version to
> be a value that is an illegal length.
Thats right.
> So existing tools will refuse to dump new style partitions,
> because they'll think the length is too large. You've tested that?
Yeah, I have tested that.
>
> Updated tools will know about both formats, so will be able to handle
> either old or new style partitions.
>
> Is that correct?
Yeah, thats correct.
>
> And we're adding the timestamp just because we can and it'd be nice to
> have?
Thats right. And also, the main reason behind adding timestamp is
it will be used when we create a pstore file for oops messages.
The pstore file's timestamp will reflect the timestamp in the oops-header
added during the crash.
> cheers
>
^ permalink raw reply
* Re: [PATCH 5/8] Read rtas partition via pstore
From: Michael Ellerman @ 2013-04-15 8:01 UTC (permalink / raw)
To: Aruna Balakrishnaiah
Cc: jkenisto, mahesh, linux-kernel, linuxppc-dev, paulus, anton
In-Reply-To: <20130410072327.20150.42189.stgit@aruna-ThinkPad-T420>
On Wed, Apr 10, 2013 at 12:53:27PM +0530, Aruna Balakrishnaiah wrote:
> This patch exploits pstore infrastructure to read the details
> from NVRAM's rtas partition.
Does that mean it's exposed in the pstore filesystem?
> Signed-off-by: Aruna Balakrishnaiah <aruna@linux.vnet.ibm.com>
> Reviewed-by: Jim Keniston <jkenisto@us.ibm.com>
> ---
> arch/powerpc/platforms/pseries/nvram.c | 33 +++++++++++++++++++++++++-------
> fs/pstore/inode.c | 3 +++
> include/linux/pstore.h | 2 ++
> 3 files changed, 31 insertions(+), 7 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/nvram.c b/arch/powerpc/platforms/pseries/nvram.c
> index 82d32a2..d420b1d 100644
> --- a/arch/powerpc/platforms/pseries/nvram.c
> +++ b/arch/powerpc/platforms/pseries/nvram.c
> @@ -144,9 +144,11 @@ static size_t oops_data_sz;
> #ifdef CONFIG_PSTORE
> static enum pstore_type_id nvram_type_ids[] = {
> PSTORE_TYPE_DMESG,
> + PSTORE_TYPE_RTAS,
> -1
> };
> static int read_type;
> +static unsigned long last_rtas_event;
> #endif
> /* Compression parameters */
> #define COMPR_LEVEL 6
> @@ -315,8 +317,13 @@ int nvram_write_error_log(char * buff, int length,
> {
> int rc = nvram_write_os_partition(&rtas_log_partition, buff, length,
> err_type, error_log_cnt);
> - if (!rc)
> + if (!rc) {
> last_unread_rtas_event = get_seconds();
> +#ifdef CONFIG_PSTORE
> + last_rtas_event = get_seconds();
> +#endif
> + }
> +
> return rc;
> }
>
> @@ -745,7 +752,7 @@ static int nvram_pstore_write(enum pstore_type_id type,
> }
>
> /*
> - * Reads the oops/panic report.
> + * Reads the oops/panic report and ibm,rtas-log partition.
> * Returns the length of the data we read from each partition.
> * Returns 0 if we've been called before.
> */
> @@ -765,6 +772,12 @@ static ssize_t nvram_pstore_read(u64 *id, enum pstore_type_id *type,
> part = &oops_log_partition;
> *type = PSTORE_TYPE_DMESG;
> break;
> + case PSTORE_TYPE_RTAS:
> + part = &rtas_log_partition;
> + *type = PSTORE_TYPE_RTAS;
> + time->tv_sec = last_rtas_event;
> + time->tv_nsec = 0;
> + break;
> default:
> return 0;
> }
> @@ -781,11 +794,17 @@ static ssize_t nvram_pstore_read(u64 *id, enum pstore_type_id *type,
>
> *count = 0;
> *id = id_no;
> - oops_hdr = (struct oops_log_info *)buff;
> - *buf = buff + sizeof(*oops_hdr);
> - time->tv_sec = oops_hdr->timestamp;
> - time->tv_nsec = 0;
> - return oops_hdr->report_length;
> +
> + if (nvram_type_ids[read_type] == PSTORE_TYPE_DMESG) {
> + oops_hdr = (struct oops_log_info *)buff;
> + *buf = buff + sizeof(*oops_hdr);
> + time->tv_sec = oops_hdr->timestamp;
> + time->tv_nsec = 0;
> + return oops_hdr->report_length;
> + }
> +
> + *buf = buff;
> + return part->size;
> }
> #else
> static int nvram_pstore_open(struct pstore_info *psi)
> diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
> index e4bcb2c..59b1454 100644
> --- a/fs/pstore/inode.c
> +++ b/fs/pstore/inode.c
> @@ -324,6 +324,9 @@ int pstore_mkfile(enum pstore_type_id type, char *psname, u64 id, int count,
> case PSTORE_TYPE_MCE:
> sprintf(name, "mce-%s-%lld", psname, id);
> break;
> + case PSTORE_TYPE_RTAS:
> + sprintf(name, "rtas-%s-%lld", psname, id);
> + break;
> case PSTORE_TYPE_UNKNOWN:
> sprintf(name, "unknown-%s-%lld", psname, id);
> break;
> diff --git a/include/linux/pstore.h b/include/linux/pstore.h
> index 75d0176..4eb94c9 100644
> --- a/include/linux/pstore.h
> +++ b/include/linux/pstore.h
> @@ -35,6 +35,8 @@ enum pstore_type_id {
> PSTORE_TYPE_MCE = 1,
> PSTORE_TYPE_CONSOLE = 2,
> PSTORE_TYPE_FTRACE = 3,
> + /* PPC64 partition types */
> + PSTORE_TYPE_RTAS = 10,
> PSTORE_TYPE_UNKNOWN = 255
I think you should probably just continue at 4, and call it
PSTORE_TYPE_PPC_RTAS. But you must get an ACK from the pstore
maintainers for this and the previous hunk, and I don't see them on CC.
cheers
^ permalink raw reply
* Re: [RFC PATCH v2 3/4] powerpc: Don't bolt the hpte in kernel_map_linear_page()
From: Li Zhong @ 2013-04-15 8:15 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <1366008995.24994.25.camel@pasglop>
On Mon, 2013-04-15 at 08:56 +0200, Benjamin Herrenschmidt wrote:
> On Mon, 2013-04-15 at 13:50 +1000, Paul Mackerras wrote:
> > On Fri, Apr 12, 2013 at 10:16:59AM +0800, Li Zhong wrote:
> > > It seems that in kernel_unmap_linear_page(), it only checks whether there
> > > is a map in the linear_map_hash_slots array, so seems we don't need bolt
> > > the hpte.
> >
Hi Paul, Ben
Thank you both for the comments and detailed information. I'll keep it
bolted in the next version. If you have time, please help to check
whether my understanding below is correct.
Thanks, Zhong
> > I don't exactly understand your rationale here, but I don't think it's
> > safe not to have linear mapping pages bolted. Basically, if a page
> > will be used in the process of calling hash_page to demand-fault an
> > HPTE into the hash table, then that page needs to be bolted, otherwise
> > we can get an infinite recursion of HPT misses.
So the infinite recursion happens like below?
fault for PAGE A
hash_page for PAGE A
some page B needed by hash_page processing removed by others,
before inserting the HPTE
fault for PAGE B
hash_page for PAGE B and recursion for ever
> That includes all
> > kernel stack pages, among other things, so I think we need to leave
> > the HPTE_V_BOLTED in there.
>
> I suspect Li's confusion comes from the fact that he doesn't realizes
> that we might evict random hash slots. If the linear mapping hash
> entries could only be thrown out via kernel_unmap_linear_page() then his
> comment would make sense. However this isn't the case.
>
> Li: When faulting something in, if both the primary and secondary
> buckets are full, we "somewhat randomly" evict the content of a slot and
> replace it. However we only do that on non-bolted slots.
So the code is implemented in ppc_md.hpte_remove(), may be called by
__hash_huge_page(), and asm code htab_call_hpte_remove?
> This is why the linear mapping (and the vmemmap) must be bolted.
If not, it would result the infinite recursion like above?
> Cheers,
> Ben.
>
>
^ permalink raw reply
* Re: [RFC PATCH v2 1/4] powerpc: Move the setting of rflags out of loop in __hash_page_huge
From: Li Zhong @ 2013-04-15 8:17 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev, paulus
In-Reply-To: <20130415063258.GE21147@concordia>
On Mon, 2013-04-15 at 16:32 +1000, Michael Ellerman wrote:
> On Fri, Apr 12, 2013 at 10:16:57AM +0800, Li Zhong wrote:
> > It seems that rflags don't get changed in the repeating loop, so move
> > it out of the loop.
> You've also changed the way new_pte is handled on repeat, but I think
> that's OK too.
OK, I'll add it in the description :)
Thanks, Zhong
> cheers
>
> > diff --git a/arch/powerpc/mm/hugetlbpage-hash64.c b/arch/powerpc/mm/hugetlbpage-hash64.c
> > index cecad34..edb4129 100644
> > --- a/arch/powerpc/mm/hugetlbpage-hash64.c
> > +++ b/arch/powerpc/mm/hugetlbpage-hash64.c
> > @@ -87,10 +87,6 @@ int __hash_page_huge(unsigned long ea, unsigned long access, unsigned long vsid,
> >
> > pa = pte_pfn(__pte(old_pte)) << PAGE_SHIFT;
> >
> > -repeat:
> > - hpte_group = ((hash & htab_hash_mask) *
> > - HPTES_PER_GROUP) & ~0x7UL;
> > -
> > /* clear HPTE slot informations in new PTE */
> > #ifdef CONFIG_PPC_64K_PAGES
> > new_pte = (new_pte & ~_PAGE_HPTEFLAGS) | _PAGE_HPTE_SUB0;
>
> ie. here new_pte was updated on repeat, but now it's not.
>
> > @@ -101,6 +97,10 @@ repeat:
> > rflags |= (new_pte & (_PAGE_WRITETHRU | _PAGE_NO_CACHE |
> > _PAGE_COHERENT | _PAGE_GUARDED));
> >
> > +repeat:
> > + hpte_group = ((hash & htab_hash_mask) *
> > + HPTES_PER_GROUP) & ~0x7UL;
> > +
> > /* Insert into the hash table, primary slot */
> > slot = ppc_md.hpte_insert(hpte_group, vpn, pa, rflags, 0,
> > mmu_psize, ssize);
>
^ permalink raw reply
* Re: [RFC PATCH v2 4/4] powerpc: Try to insert the hptes repeatedly in kernel_map_linear_page()
From: Li Zhong @ 2013-04-15 8:21 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev, paulus
In-Reply-To: <20130415063658.GF21147@concordia>
On Mon, 2013-04-15 at 16:36 +1000, Michael Ellerman wrote:
> On Fri, Apr 12, 2013 at 10:17:00AM +0800, Li Zhong wrote:
> > This patch tries to fix following issue when CONFIG_DEBUG_PAGEALLOC
> > is enabled:
>
> Please include a better changelog.
OK, I'll use the following as a template, thank you for the suggestion.
>
> This patch does fix (I hope?) the following oops, caused by xxx. Reproducible
> by doing yyy.
>
> cheers
>
> >
> > [ 543.075675] ------------[ cut here ]------------
> > [ 543.075701] kernel BUG at arch/powerpc/mm/hash_utils_64.c:1239!
> > [ 543.075714] Oops: Exception in kernel mode, sig: 5 [#1]
> > [ 543.075722] PREEMPT SMP NR_CPUS=16 DEBUG_PAGEALLOC NUMA pSeries
> > [ 543.075741] Modules linked in: binfmt_misc ehea
> > [ 543.075759] NIP: c000000000036eb0 LR: c000000000036ea4 CTR: c00000000005a594
> > [ 543.075771] REGS: c0000000a90832c0 TRAP: 0700 Not tainted (3.8.0-next-20130222)
> > [ 543.075781] MSR: 8000000000029032 <SF,EE,ME,IR,DR,RI> CR: 22224482 XER: 00000000
> > [ 543.075816] SOFTE: 0
> > [ 543.075823] CFAR: c00000000004c200
> > [ 543.075830] TASK = c0000000e506b750[23934] 'cc1' THREAD: c0000000a9080000 CPU: 1
> > GPR00: 0000000000000001 c0000000a9083540 c000000000c600a8 ffffffffffffffff
> > GPR04: 0000000000000050 fffffffffffffffa c0000000a90834e0 00000000004ff594
> > GPR08: 0000000000000001 0000000000000000 000000009592d4d8 c000000000c86854
> > GPR12: 0000000000000002 c000000006ead300 0000000000a51000 0000000000000001
> > GPR16: f000000003354380 ffffffffffffffff ffffffffffffff80 0000000000000000
> > GPR20: 0000000000000001 c000000000c600a8 0000000000000001 0000000000000001
> > GPR24: 0000000003354380 c000000000000000 0000000000000000 c000000000b65950
> > GPR28: 0000002000000000 00000000000cd50e 0000000000bf50d9 c000000000c7c230
> > [ 543.076005] NIP [c000000000036eb0] .kernel_map_pages+0x1e0/0x3f8
> > [ 543.076016] LR [c000000000036ea4] .kernel_map_pages+0x1d4/0x3f8
> > [ 543.076025] Call Trace:
> > [ 543.076033] [c0000000a9083540] [c000000000036ea4] .kernel_map_pages+0x1d4/0x3f8 (unreliable)
> > [ 543.076053] [c0000000a9083640] [c000000000167638] .get_page_from_freelist+0x6cc/0x8dc
> > [ 543.076067] [c0000000a9083800] [c000000000167a48] .__alloc_pages_nodemask+0x200/0x96c
> > [ 543.076082] [c0000000a90839c0] [c0000000001ade44] .alloc_pages_vma+0x160/0x1e4
> > [ 543.076098] [c0000000a9083a80] [c00000000018ce04] .handle_pte_fault+0x1b0/0x7e8
> > [ 543.076113] [c0000000a9083b50] [c00000000018d5a8] .handle_mm_fault+0x16c/0x1a0
> > [ 543.076129] [c0000000a9083c00] [c0000000007bf1dc] .do_page_fault+0x4d0/0x7a4
> > [ 543.076144] [c0000000a9083e30] [c0000000000090e8] handle_page_fault+0x10/0x30
> > [ 543.076155] Instruction dump:
> > [ 543.076163] 7c630038 78631d88 e80a0000 f8410028 7c0903a6 e91f01de e96a0010 e84a0008
> > [ 543.076192] 4e800421 e8410028 7c7107b4 7a200fe0 <0b000000> 7f63db78 48785781 60000000
> > [ 543.076224] ---[ end trace bd5807e8d6ae186b ]---
> >
> > Signed-off-by: Li Zhong <zhong@linux.vnet.ibm.com>
> > ---
> > arch/powerpc/mm/hash_utils_64.c | 10 ++++++----
> > 1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/powerpc/mm/hash_utils_64.c b/arch/powerpc/mm/hash_utils_64.c
> > index a7f54f0..4b449a0 100644
> > --- a/arch/powerpc/mm/hash_utils_64.c
> > +++ b/arch/powerpc/mm/hash_utils_64.c
> > @@ -1272,7 +1272,7 @@ static void kernel_map_linear_page(unsigned long vaddr, unsigned long lmi)
> > unsigned long vsid = get_kernel_vsid(vaddr, mmu_kernel_ssize);
> > unsigned long vpn = hpt_vpn(vaddr, vsid, mmu_kernel_ssize);
> > unsigned long mode = htab_convert_pte_flags(PAGE_KERNEL);
> > - int ret;
> > + long ret;
> >
> > hash = hpt_hash(vpn, PAGE_SHIFT, mmu_kernel_ssize);
> > hpteg = ((hash & htab_hash_mask) * HPTES_PER_GROUP);
> > @@ -1280,9 +1280,11 @@ static void kernel_map_linear_page(unsigned long vaddr, unsigned long lmi)
> > /* Don't create HPTE entries for bad address */
> > if (!vsid)
> > return;
> > - ret = ppc_md.hpte_insert(hpteg, vpn, __pa(vaddr),
> > - mode, 0,
> > - mmu_linear_psize, mmu_kernel_ssize);
> > +
> > + ret = hpte_insert_repeating(hash, vpn, __pa(vaddr),
> > + mode, mmu_linear_psize,
> > + mmu_kernel_ssize);
> > +
> > BUG_ON (ret < 0);
> > spin_lock(&linear_map_hash_lock);
> > BUG_ON(linear_map_hash_slots[lmi] & 0x80);
> > --
> > 1.7.9.5
> >
>
^ permalink raw reply
* Re: [PATCH] powerpc/perf: Power8 PMU support
From: Michael Ellerman @ 2013-04-15 10:26 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linux-kernel, acme, linuxppc-dev, Paul Mackerras, Anton Blanchard,
sukadev
In-Reply-To: <1366011086.24994.33.camel@pasglop>
On Mon, Apr 15, 2013 at 09:31:26AM +0200, Benjamin Herrenschmidt wrote:
> On Mon, 2013-04-15 at 14:17 +1000, Michael Ellerman wrote:
> > This patch adds preliminary support for the power8 PMU to perf.
>
> Might be worthwhile to have a small blurb explaining roughly what you
> mean by "preliminary" :-)
True.
There's no alternative handling, and no cache events. I need to work
with the HW folks on both of those.
Also missing is EBB support. I will hopefully post that in the next few
days.
cheers
^ permalink raw reply
* Re: [RFC PATCH v2 3/4] powerpc: Don't bolt the hpte in kernel_map_linear_page()
From: Benjamin Herrenschmidt @ 2013-04-15 11:27 UTC (permalink / raw)
To: Li Zhong; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <1366013711.12424.33.camel@ThinkPad-T5421.cn.ibm.com>
On Mon, 2013-04-15 at 16:15 +0800, Li Zhong wrote:
> So the code is implemented in ppc_md.hpte_remove(), may be called by
> __hash_huge_page(), and asm code htab_call_hpte_remove?
>
> > This is why the linear mapping (and the vmemmap) must be bolted.
>
> If not, it would result the infinite recursion like above?
Potentially, we don't expect to fault linear mapping or vmemmap entries
on demand. We aren't equipped to do it and we occasionally have code
path that access the linear mapping and cannot afford to have SRR0 and
SRR1 clobbered by a page fault.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCHv3 1/2] ppc64: perform proper max_bus_speed detection
From: Michael Ellerman @ 2013-04-15 11:42 UTC (permalink / raw)
To: Lucas Kannebley Tavares
Cc: David Airlie, Brian King, dri-devel, Kleber Sacilotto de Souza,
Alex Deucher, Jerome Glisse, Thadeu Lima de Souza Cascardo,
Bjorn Helgaas, linuxppc-dev
In-Reply-To: <1365685994-32603-2-git-send-email-lucaskt@linux.vnet.ibm.com>
On Thu, Apr 11, 2013 at 10:13:13AM -0300, Lucas Kannebley Tavares wrote:
> On pseries machines the detection for max_bus_speed should be done
> through an OpenFirmware property. This patch adds a function to perform this
> detection and a hook to perform dynamic adding of the function only for
> pseries.
This fails to build for me on ppc64_defconfig, with:
arch/powerpc/include/asm/machdep.h:111:5: error: 'struct pci_host_bridge' declared inside parameter list [-Werror]
Presumably you tested it using some other defconfig?
cheers
^ permalink raw reply
* Re: [PATCH 1/2] powerpc: remove section changes from _GLOBAL() and friends
From: Michael Ellerman @ 2013-04-15 12:00 UTC (permalink / raw)
To: Stephen Rothwell; +Cc: Alan Modra, ppc-dev
In-Reply-To: <20121129105525.0e47b3b3e51aebe7ded3b7b6@canb.auug.org.au>
On Thu, Nov 29, 2012 at 10:55:25AM +1100, Stephen Rothwell wrote:
> These sometimes produce unexpected results and make it hard to put the
> start up code (for 64 bit) into the .head.text section.
...
> diff --git a/arch/powerpc/kernel/head_fsl_booke.S b/arch/powerpc/kernel/head_fsl_booke.S
> index 6f62a73..4ec5625 100644
> --- a/arch/powerpc/kernel/head_fsl_booke.S
> +++ b/arch/powerpc/kernel/head_fsl_booke.S
> @@ -776,6 +776,8 @@ tlb_write_entry:
> mfspr r10, SPRN_SPRG_RSCRATCH0
> rfi /* Force context change */
>
> + ,text
> +
I'm assuming this should be .text ?
cheers
^ permalink raw reply
* Re: [PATCH 1/2] powerpc: remove section changes from _GLOBAL() and friends
From: Stephen Rothwell @ 2013-04-15 13:30 UTC (permalink / raw)
To: Michael Ellerman; +Cc: Alan Modra, ppc-dev
In-Reply-To: <20130415120016.GB12701@concordia>
[-- Attachment #1: Type: text/plain, Size: 836 bytes --]
Hi Michael,
On Mon, 15 Apr 2013 22:00:17 +1000 Michael Ellerman <michael@ellerman.id.au> wrote:
>
> On Thu, Nov 29, 2012 at 10:55:25AM +1100, Stephen Rothwell wrote:
> > These sometimes produce unexpected results and make it hard to put the
> > start up code (for 64 bit) into the .head.text section.
>
> ...
>
> > diff --git a/arch/powerpc/kernel/head_fsl_booke.S b/arch/powerpc/kernel/head_fsl_booke.S
> > index 6f62a73..4ec5625 100644
> > --- a/arch/powerpc/kernel/head_fsl_booke.S
> > +++ b/arch/powerpc/kernel/head_fsl_booke.S
> > @@ -776,6 +776,8 @@ tlb_write_entry:
> > mfspr r10, SPRN_SPRG_RSCRATCH0
> > rfi /* Force context change */
> >
> > + ,text
> > +
>
> I'm assuming this should be .text ?
Indeed. Oops :-)
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 1/3] iommu: Move swap_pci_ref function to pci.h.
From: Joerg Roedel @ 2013-04-15 14:58 UTC (permalink / raw)
To: Varun Sethi
Cc: linux-kernel, stuart.yoder, bhelgaas, iommu, scottwood,
linuxppc-dev
In-Reply-To: <1365966722-1804-1-git-send-email-Varun.Sethi@freescale.com>
On Mon, Apr 15, 2013 at 12:42:00AM +0530, Varun Sethi wrote:
> swap_pci_ref function is used by the IOMMU API code for swapping pci device
> pointers, while determining the iommu group for the device.
> Currently this function was being implemented for different IOMMU drivers.
> This patch moves the function to pci.h so that the implementation can be
> shared across various IOMMU drivers.
The function is only used in IOMMU code, so I think its fine to keep it
there (unless Bjorn disagrees and wants it in PCI code).
Joerg
^ permalink raw reply
* Re: [PATCH 1/3] iommu: Move swap_pci_ref function to pci.h.
From: Bjorn Helgaas @ 2013-04-15 15:34 UTC (permalink / raw)
To: Joerg Roedel
Cc: stuart.yoder, linux-kernel@vger.kernel.org,
open list:INTEL IOMMU (VT-d), scottwood, Varun Sethi,
linuxppc-dev
In-Reply-To: <20130415145834.GR6858@8bytes.org>
On Mon, Apr 15, 2013 at 8:58 AM, Joerg Roedel <joro@8bytes.org> wrote:
> On Mon, Apr 15, 2013 at 12:42:00AM +0530, Varun Sethi wrote:
>> swap_pci_ref function is used by the IOMMU API code for swapping pci device
>> pointers, while determining the iommu group for the device.
>> Currently this function was being implemented for different IOMMU drivers.
>> This patch moves the function to pci.h so that the implementation can be
>> shared across various IOMMU drivers.
>
> The function is only used in IOMMU code, so I think its fine to keep it
> there (unless Bjorn disagrees and wants it in PCI code).
I agree; I don't think there's much benefit in putting something under
#ifdef CONFIG_IOMMU_API into pci.h. Maybe there is or could be a
shared iommu header file?
Bjorn
^ permalink raw reply
* Re: [PATCH 1/2] powerpc: remove section changes from _GLOBAL() and friends
From: Stephen Rothwell @ 2013-04-15 16:17 UTC (permalink / raw)
To: Michael Ellerman; +Cc: Alan Modra, ppc-dev
In-Reply-To: <20130415233040.ca38e79ce08c519554baa865@canb.auug.org.au>
[-- Attachment #1: Type: text/plain, Size: 1092 bytes --]
Hi Michael,
On Mon, 15 Apr 2013 23:30:40 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> On Mon, 15 Apr 2013 22:00:17 +1000 Michael Ellerman <michael@ellerman.id.au> wrote:
> >
> > On Thu, Nov 29, 2012 at 10:55:25AM +1100, Stephen Rothwell wrote:
> > > These sometimes produce unexpected results and make it hard to put the
> > > start up code (for 64 bit) into the .head.text section.
> >
> > ...
> >
> > > diff --git a/arch/powerpc/kernel/head_fsl_booke.S b/arch/powerpc/kernel/head_fsl_booke.S
> > > index 6f62a73..4ec5625 100644
> > > --- a/arch/powerpc/kernel/head_fsl_booke.S
> > > +++ b/arch/powerpc/kernel/head_fsl_booke.S
> > > @@ -776,6 +776,8 @@ tlb_write_entry:
> > > mfspr r10, SPRN_SPRG_RSCRATCH0
> > > rfi /* Force context change */
> > >
> > > + ,text
> > > +
> >
> > I'm assuming this should be .text ?
>
> Indeed. Oops :-)
BTW, those patches are almost certainly stale by now and would need to be
redone before be included on the kernel proper.
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] powerpc: fix usage of setup_pci_atmu()
From: Kim Phillips @ 2013-04-15 16:30 UTC (permalink / raw)
To: Michael Neuling; +Cc: sfr, linux-next, linuxppc-dev
In-Reply-To: <5881.1366004521@ale.ozlabs.ibm.com>
On Mon, 15 Apr 2013 15:42:01 +1000
Michael Neuling <mikey@neuling.org> wrote:
> Linux next is currently failing to compile mpc85xx_defconfig with:
> arch/powerpc/sysdev/fsl_pci.c:944:2: error: too many arguments to function 'setup_pci_atmu'
>
> This is caused by (from Kumar's next branch):
> commit 34642bbb3d12121333efcf4ea7dfe66685e403a1
> Author: Kumar Gala <galak@kernel.crashing.org>
> powerpc/fsl-pci: Keep PCI SoC controller registers in pci_controller
>
> Which changed definition of setup_pci_atmu() but didn't update one of
> the callers. Below fixes this.
>
> Signed-off-by: Michael Neuling <mikey@neuling.org>
> ---
> Kumar: this is for your next tree
Reviewed-by: Kim Phillips <kim.phillips@freescale.com>
Thanks,
Kim
^ permalink raw reply
* RE: [PATCH 1/3] iommu: Move swap_pci_ref function to pci.h.
From: Sethi Varun-B16395 @ 2013-04-15 17:13 UTC (permalink / raw)
To: Joerg Roedel
Cc: Wood Scott-B07421, linux-kernel@vger.kernel.org,
Yoder Stuart-B08248, iommu@lists.linux-foundation.org,
bhelgaas@google.com, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20130415145834.GR6858@8bytes.org>
Fine, I will move it to iommu.h.
-Varun
> -----Original Message-----
> From: Joerg Roedel [mailto:joro@8bytes.org]
> Sent: Monday, April 15, 2013 8:29 PM
> To: Sethi Varun-B16395
> Cc: Yoder Stuart-B08248; Wood Scott-B07421; iommu@lists.linux-
> foundation.org; linuxppc-dev@lists.ozlabs.org; linux-
> kernel@vger.kernel.org; galak@kernel.crashing.org;
> benh@kernel.crashing.org; bhelgaas@google.com
> Subject: Re: [PATCH 1/3] iommu: Move swap_pci_ref function to pci.h.
>=20
> On Mon, Apr 15, 2013 at 12:42:00AM +0530, Varun Sethi wrote:
> > swap_pci_ref function is used by the IOMMU API code for swapping pci
> > device pointers, while determining the iommu group for the device.
> > Currently this function was being implemented for different IOMMU
> drivers.
> > This patch moves the function to pci.h so that the implementation can
> > be shared across various IOMMU drivers.
>=20
> The function is only used in IOMMU code, so I think its fine to keep it
> there (unless Bjorn disagrees and wants it in PCI code).
>=20
>=20
> Joerg
>=20
>=20
^ permalink raw reply
* Re: [PATCH v2 2/11] Add PRRN Event Handler
From: Nathan Fontenot @ 2013-04-15 20:12 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <20130410083012.GD24786@concordia>
On 04/10/2013 03:30 AM, Michael Ellerman wrote:
> On Mon, Mar 25, 2013 at 01:52:32PM -0500, Nathan Fontenot wrote:
>> From: Jesse Larrew <jlarrew@linux.vnet.ibm.com>
>>
>> A PRRN event is signaled via the RTAS event-scan mechanism, which
>> returns a Hot Plug Event message "fixed part" indicating "Platform
>> Resource Reassignment". In response to the Hot Plug Event message,
>> we must call ibm,update-nodes to determine which resources were
>> reassigned and then ibm,update-properties to obtain the new affinity
>> information about those resources.
> ..
>
>> Index: powerpc/arch/powerpc/kernel/rtasd.c
>> ===================================================================
>> --- powerpc.orig/arch/powerpc/kernel/rtasd.c 2013-03-20 08:24:14.000000000 -0500
>> +++ powerpc/arch/powerpc/kernel/rtasd.c 2013-03-20 08:52:08.000000000 -0500
>> @@ -87,6 +87,8 @@
>> return "Resource Deallocation Event";
>> case RTAS_TYPE_DUMP:
>> return "Dump Notification Event";
>> + case RTAS_TYPE_PRRN:
>> + return "Platform Resource Reassignment Event";
>> }
>>
>> return rtas_type[0];
>> @@ -265,7 +267,38 @@
>> spin_unlock_irqrestore(&rtasd_log_lock, s);
>> return;
>> }
>> +}
>> +
>> +static s32 update_scope;
>> +
>> +static void prrn_work_fn(struct work_struct *work)
>> +{
>> + /*
>> + * For PRRN, we must pass the negative of the scope value in
>> + * the RTAS event.
>> + */
>> + pseries_devicetree_update(-update_scope);
>> +}
>> +static DECLARE_WORK(prrn_work, prrn_work_fn);
>
> This breaks the 32-bit build (ppc6xx_defconfig):
>
> arch/powerpc/kernel/rtasd.c:280: undefined reference to `pseries_devicetree_update'
>
I'm not seeing this error. rtasd.c compilkes fine, but I am hitting another
error later in the build that keeps it from finishing.
arch/powerpc/platforms/52xx/mpc52xx_pic.c: In function ‘mpc52xx_irqhost_map’:
arch/powerpc/platforms/52xx/mpc52xx_pic.c:343: error: ‘irqchip’ may be used uninitialized in this function
-Nathan
^ permalink raw reply
* Re: [RFC PATCH v2 3/4] powerpc: Don't bolt the hpte in kernel_map_linear_page()
From: Li Zhong @ 2013-04-16 2:51 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <1366025246.24994.35.camel@pasglop>
On Mon, 2013-04-15 at 13:27 +0200, Benjamin Herrenschmidt wrote:
> On Mon, 2013-04-15 at 16:15 +0800, Li Zhong wrote:
>
> > So the code is implemented in ppc_md.hpte_remove(), may be called by
> > __hash_huge_page(), and asm code htab_call_hpte_remove?
> >
> > > This is why the linear mapping (and the vmemmap) must be bolted.
> >
> > If not, it would result the infinite recursion like above?
>
> Potentially, we don't expect to fault linear mapping or vmemmap entries
> on demand. We aren't equipped to do it and we occasionally have code
> path that access the linear mapping and cannot afford to have SRR0 and
> SRR1 clobbered by a page fault.
Thank you for the education :)
Thanks, Zhong
>
> Cheers,
> Ben.
>
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox