From: Anurup M <anurupvasu@gmail.com> To: Mark Rutland <mark.rutland@arm.com> Cc: will.deacon@arm.com, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, anurup.m@huawei.com, zhangshaokun@hisilicon.com, tanxiaojun@huawei.com, xuwei5@hisilicon.com, sanil.kumar@hisilicon.com, john.garry@huawei.com, gabriele.paoloni@huawei.com, shiju.jose@huawei.com, huangdaode@hisilicon.com, linuxarm@huawei.com, dikshit.n@huawei.com, shyju.pv@huawei.com Subject: Re: [PATCH v6 08/11] drivers: perf: hisi: use poll method to avoid L3C counter overflow Date: Fri, 24 Mar 2017 12:13:18 +0530 [thread overview] Message-ID: <58D4C006.2010907@gmail.com> (raw) In-Reply-To: <20170321171605.GB29116@leverpostej> On Tuesday 21 March 2017 10:46 PM, Mark Rutland wrote: > On Fri, Mar 10, 2017 at 01:28:45AM -0500, Anurup M wrote: >> Add hrtimer support which use poll method to avoid counter overflow >> when overflow IRQ is not supported in hardware. >> The L3 cache PMU use N-N SPI interrupt which has no support in kernel >> mainline. So use hrtimer to poll and update event counter to avoid >> overflow condition for L3 cache PMU. >> An interval of 10 seconds is used for the hrtimer. > This should be folded with the previous patch, given that it is > necessary for counters to work correctly. I had separated for ease of review. I shall fold it to main L3C patch. > [...] > >> +/* >> + * Default timer frequency to poll and avoid counter overflow. >> + * CPU speed = 2.4Ghz, Therefore Access time = 0.4ns >> + * L1 cache - 2 way set associative >> + * L2 - 16 way set associative >> + * L3 - 16 way set associative. L3 cache has 4 banks. >> + * >> + * Overflow time = 2^31 * (access time L1 + access time L2 + access time L3) >> + * = 2^31 * ((2 * 0.4ns) + (16 * 0.4ns) + (4 * 16 * 0.4ns)) = 70 seconds >> + * >> + * L3 cache is also used by devices like PCIe, SAS etc. at >> + * the same time. So the overflow time could be even smaller. >> + * So on a safe side we use a timer interval of 10sec >> + */ >> +#define L3C_HRTIMER_INTERVAL (10LL * MSEC_PER_SEC) > This sounds fine. > > [...] > >> +/* >> + * sysfs hrtimer_interval attributes >> + */ >> +ssize_t hisi_hrtimer_interval_sysfs_show(struct device *dev, >> + struct device_attribute *attr, >> + char *buf) >> +{ >> + struct pmu *pmu = dev_get_drvdata(dev); >> + struct hisi_pmu *hisi_pmu = to_hisi_pmu(pmu); >> + >> + if (hisi_pmu->hrt_duration) >> + return sprintf(buf, "%llu\n", hisi_pmu->hrt_duration); >> + return 0; >> +} > I don't think that we need a sysfs property for this. Agreed. Shall remove it. > [...] > >> +/* The counter overflow IRQ is not supported for some PMUs >> + * use hrtimer to periodically poll and avoid overflow >> + */ >> +static enum hrtimer_restart hisi_hrtimer_callback(struct hrtimer *hrtimer) >> +{ >> + struct hisi_pmu *hisi_pmu = container_of(hrtimer, >> + struct hisi_pmu, hrtimer); >> + struct perf_event *event; >> + struct hw_perf_event *hwc; >> + unsigned long flags; >> + >> + /* Return if no active events */ >> + if (!hisi_pmu->num_active) >> + return HRTIMER_NORESTART; >> + >> + local_irq_save(flags); >> + >> + /* Update event count for each active event */ >> + list_for_each_entry(event, &hisi_pmu->active_list, active_entry) { >> + hwc = &event->hw; >> + /* Read hardware counter and update the Perf event counter */ >> + hisi_pmu->ops->event_update(event, hwc, GET_CNTR_IDX(hwc)); >> + } > How do we ensure that we don't take the interrupt in the middle of a > sequence of accesses to the HW? The L3 cache and MN PMU does not use the overflow IRQ and it does not occur here as the interrupt Mask register is by default masked in hardware. But yes I would modify it as hisi_pmu->ops->overflow_handler (which can mask IRQ (if required) and call event_update) Thanks, Anurup > Thanks, > Mark.
WARNING: multiple messages have this Message-ID (diff)
From: anurupvasu@gmail.com (Anurup M) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH v6 08/11] drivers: perf: hisi: use poll method to avoid L3C counter overflow Date: Fri, 24 Mar 2017 12:13:18 +0530 [thread overview] Message-ID: <58D4C006.2010907@gmail.com> (raw) In-Reply-To: <20170321171605.GB29116@leverpostej> On Tuesday 21 March 2017 10:46 PM, Mark Rutland wrote: > On Fri, Mar 10, 2017 at 01:28:45AM -0500, Anurup M wrote: >> Add hrtimer support which use poll method to avoid counter overflow >> when overflow IRQ is not supported in hardware. >> The L3 cache PMU use N-N SPI interrupt which has no support in kernel >> mainline. So use hrtimer to poll and update event counter to avoid >> overflow condition for L3 cache PMU. >> An interval of 10 seconds is used for the hrtimer. > This should be folded with the previous patch, given that it is > necessary for counters to work correctly. I had separated for ease of review. I shall fold it to main L3C patch. > [...] > >> +/* >> + * Default timer frequency to poll and avoid counter overflow. >> + * CPU speed = 2.4Ghz, Therefore Access time = 0.4ns >> + * L1 cache - 2 way set associative >> + * L2 - 16 way set associative >> + * L3 - 16 way set associative. L3 cache has 4 banks. >> + * >> + * Overflow time = 2^31 * (access time L1 + access time L2 + access time L3) >> + * = 2^31 * ((2 * 0.4ns) + (16 * 0.4ns) + (4 * 16 * 0.4ns)) = 70 seconds >> + * >> + * L3 cache is also used by devices like PCIe, SAS etc. at >> + * the same time. So the overflow time could be even smaller. >> + * So on a safe side we use a timer interval of 10sec >> + */ >> +#define L3C_HRTIMER_INTERVAL (10LL * MSEC_PER_SEC) > This sounds fine. > > [...] > >> +/* >> + * sysfs hrtimer_interval attributes >> + */ >> +ssize_t hisi_hrtimer_interval_sysfs_show(struct device *dev, >> + struct device_attribute *attr, >> + char *buf) >> +{ >> + struct pmu *pmu = dev_get_drvdata(dev); >> + struct hisi_pmu *hisi_pmu = to_hisi_pmu(pmu); >> + >> + if (hisi_pmu->hrt_duration) >> + return sprintf(buf, "%llu\n", hisi_pmu->hrt_duration); >> + return 0; >> +} > I don't think that we need a sysfs property for this. Agreed. Shall remove it. > [...] > >> +/* The counter overflow IRQ is not supported for some PMUs >> + * use hrtimer to periodically poll and avoid overflow >> + */ >> +static enum hrtimer_restart hisi_hrtimer_callback(struct hrtimer *hrtimer) >> +{ >> + struct hisi_pmu *hisi_pmu = container_of(hrtimer, >> + struct hisi_pmu, hrtimer); >> + struct perf_event *event; >> + struct hw_perf_event *hwc; >> + unsigned long flags; >> + >> + /* Return if no active events */ >> + if (!hisi_pmu->num_active) >> + return HRTIMER_NORESTART; >> + >> + local_irq_save(flags); >> + >> + /* Update event count for each active event */ >> + list_for_each_entry(event, &hisi_pmu->active_list, active_entry) { >> + hwc = &event->hw; >> + /* Read hardware counter and update the Perf event counter */ >> + hisi_pmu->ops->event_update(event, hwc, GET_CNTR_IDX(hwc)); >> + } > How do we ensure that we don't take the interrupt in the middle of a > sequence of accesses to the HW? The L3 cache and MN PMU does not use the overflow IRQ and it does not occur here as the interrupt Mask register is by default masked in hardware. But yes I would modify it as hisi_pmu->ops->overflow_handler (which can mask IRQ (if required) and call event_update) Thanks, Anurup > Thanks, > Mark.
next prev parent reply other threads:[~2017-03-24 6:43 UTC|newest] Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-03-10 6:28 [PATCH v6 08/11] drivers: perf: hisi: use poll method to avoid L3C counter overflow Anurup M 2017-03-10 6:28 ` Anurup M 2017-03-21 17:16 ` Mark Rutland 2017-03-21 17:16 ` Mark Rutland 2017-03-24 6:43 ` Anurup M [this message] 2017-03-24 6:43 ` Anurup M 2017-03-24 11:43 ` Mark Rutland 2017-03-24 11:43 ` Mark Rutland 2017-03-27 6:33 ` Anurup M 2017-03-27 6:33 ` Anurup M 2017-03-28 11:09 ` Mark Rutland 2017-03-28 11:09 ` Mark Rutland
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=58D4C006.2010907@gmail.com \ --to=anurupvasu@gmail.com \ --cc=anurup.m@huawei.com \ --cc=dikshit.n@huawei.com \ --cc=gabriele.paoloni@huawei.com \ --cc=huangdaode@hisilicon.com \ --cc=john.garry@huawei.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linuxarm@huawei.com \ --cc=mark.rutland@arm.com \ --cc=sanil.kumar@hisilicon.com \ --cc=shiju.jose@huawei.com \ --cc=shyju.pv@huawei.com \ --cc=tanxiaojun@huawei.com \ --cc=will.deacon@arm.com \ --cc=xuwei5@hisilicon.com \ --cc=zhangshaokun@hisilicon.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.