* [PATCH v2 06/11] coresight-etm4x: Controls pertaining to the address comparator functions
From: Mathieu Poirier @ 2015-04-29 17:16 UTC (permalink / raw)
To: gregkh
Cc: linux-arm-kernel, linux-api, linux-kernel, kaixu.xia,
zhang.chunyan, mathieu.poirier
In-Reply-To: <1430327795-10710-1-git-send-email-mathieu.poirier@linaro.org>
From: Pratik Patel <pratikp@codeaurora.org>
Adding sysfs entries to control the various mode the address comparator
registers can enact, i.e, start/top, single, and range. Also supplementing
with address comparator types configuration registers access, mandatory
to complete the configuration of the comparator functions.
Signed-off-by: Pratik Patel <pratikp@codeaurora.org>
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
.../ABI/testing/sysfs-bus-coresight-devices-etm4x | 25 ++
drivers/hwtracing/coresight/coresight-etm4x.c | 423 +++++++++++++++++++++
2 files changed, 448 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x b/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
index d7409c3d58e6..8cdc4ad10bd6 100644
--- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
+++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
@@ -166,3 +166,28 @@ KernelVersion: 4.01
Contact: Mathieu Poirier <mathieu.poirier@linaro.org>
Description: (RW) In non-secure state, each bit controls whether instruction
tracing is enabled for the corresponding exception level.
+
+What: /sys/bus/coresight/devices/<memory_map>.etm/addr_idx
+Date: April 2015
+KernelVersion: 4.01
+Contact: Mathieu Poirier <mathieu.poirier@linaro.org>
+Description: (RW) Select which address comparator or pair (of comparators) to
+ work with.
+
+What: /sys/bus/coresight/devices/<memory_map>.etm/addr_instdatatype
+Date: April 2015
+KernelVersion: 4.01
+Contact: Mathieu Poirier <mathieu.poirier@linaro.org>
+Description: (RW) Controls what type of comparison the trace unit performs.
+
+What: /sys/bus/coresight/devices/<memory_map>.etm/addr_single
+Date: April 2015
+KernelVersion: 4.01
+Contact: Mathieu Poirier <mathieu.poirier@linaro.org>
+Description: (RW) Used to setup single address comparator values.
+
+What: /sys/bus/coresight/devices/<memory_map>.etm/addr_range
+Date: April 2015
+KernelVersion: 4.01
+Contact: Mathieu Poirier <mathieu.poirier@linaro.org>
+Description: (RW) Used to setup address range comparator values.
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
index 9f2a13dd1280..47cc68b8cc43 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x.c
@@ -1027,6 +1027,421 @@ static ssize_t ns_exlevel_vinst_store(struct device *dev,
}
static DEVICE_ATTR_RW(ns_exlevel_vinst);
+static ssize_t addr_idx_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ val = drvdata->addr_idx;
+ return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
+}
+
+static ssize_t addr_idx_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ if (kstrtoul(buf, 16, &val))
+ return -EINVAL;
+ if (val >= drvdata->nr_addr_cmp * 2)
+ return -EINVAL;
+
+ /*
+ * Use spinlock to ensure index doesn't change while it gets
+ * dereferenced multiple times within a spinlock block elsewhere.
+ */
+ spin_lock(&drvdata->spinlock);
+ drvdata->addr_idx = val;
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_RW(addr_idx);
+
+static ssize_t addr_instdatatype_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ ssize_t len;
+ u8 val, idx;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ spin_lock(&drvdata->spinlock);
+ idx = drvdata->addr_idx;
+ val = BMVAL(drvdata->addr_acc[idx], 0, 1);
+ len = scnprintf(buf, PAGE_SIZE, "%s\n",
+ val == ETM_INSTR_ADDR ? "instr" :
+ (val == ETM_DATA_LOAD_ADDR ? "data_load" :
+ (val == ETM_DATA_STORE_ADDR ? "data_store" :
+ "data_load_store")));
+ spin_unlock(&drvdata->spinlock);
+ return len;
+}
+
+static ssize_t addr_instdatatype_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ u8 idx;
+ char str[20] = "";
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ if (strlen(buf) >= 20)
+ return -EINVAL;
+ if (sscanf(buf, "%s", str) != 1)
+ return -EINVAL;
+
+ spin_lock(&drvdata->spinlock);
+ idx = drvdata->addr_idx;
+ if (!strcmp(str, "instr"))
+ /* TYPE, bits[1:0] */
+ drvdata->addr_acc[idx] &= ~(BIT(0) | BIT(1));
+
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_RW(addr_instdatatype);
+
+static ssize_t addr_single_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ u8 idx;
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ idx = drvdata->addr_idx;
+ spin_lock(&drvdata->spinlock);
+ if (!(drvdata->addr_type[idx] == ETM_ADDR_TYPE_NONE ||
+ drvdata->addr_type[idx] == ETM_ADDR_TYPE_SINGLE)) {
+ spin_unlock(&drvdata->spinlock);
+ return -EPERM;
+ }
+ val = (unsigned long)drvdata->addr_val[idx];
+ spin_unlock(&drvdata->spinlock);
+ return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
+}
+
+static ssize_t addr_single_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ u8 idx;
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ if (kstrtoul(buf, 16, &val))
+ return -EINVAL;
+
+ spin_lock(&drvdata->spinlock);
+ idx = drvdata->addr_idx;
+ if (!(drvdata->addr_type[idx] == ETM_ADDR_TYPE_NONE ||
+ drvdata->addr_type[idx] == ETM_ADDR_TYPE_SINGLE)) {
+ spin_unlock(&drvdata->spinlock);
+ return -EPERM;
+ }
+
+ drvdata->addr_val[idx] = (u64)val;
+ drvdata->addr_type[idx] = ETM_ADDR_TYPE_SINGLE;
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_RW(addr_single);
+
+static ssize_t addr_range_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ u8 idx;
+ unsigned long val1, val2;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ spin_lock(&drvdata->spinlock);
+ idx = drvdata->addr_idx;
+ if (idx % 2 != 0) {
+ spin_unlock(&drvdata->spinlock);
+ return -EPERM;
+ }
+ if (!((drvdata->addr_type[idx] == ETM_ADDR_TYPE_NONE &&
+ drvdata->addr_type[idx + 1] == ETM_ADDR_TYPE_NONE) ||
+ (drvdata->addr_type[idx] == ETM_ADDR_TYPE_RANGE &&
+ drvdata->addr_type[idx + 1] == ETM_ADDR_TYPE_RANGE))) {
+ spin_unlock(&drvdata->spinlock);
+ return -EPERM;
+ }
+
+ val1 = (unsigned long)drvdata->addr_val[idx];
+ val2 = (unsigned long)drvdata->addr_val[idx + 1];
+ spin_unlock(&drvdata->spinlock);
+ return scnprintf(buf, PAGE_SIZE, "%#lx %#lx\n", val1, val2);
+}
+
+static ssize_t addr_range_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ u8 idx;
+ unsigned long val1, val2;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ if (sscanf(buf, "%lx %lx", &val1, &val2) != 2)
+ return -EINVAL;
+ /* lower address comparator cannot have a higher address value */
+ if (val1 > val2)
+ return -EINVAL;
+
+ spin_lock(&drvdata->spinlock);
+ idx = drvdata->addr_idx;
+ if (idx % 2 != 0) {
+ spin_unlock(&drvdata->spinlock);
+ return -EPERM;
+ }
+
+ if (!((drvdata->addr_type[idx] == ETM_ADDR_TYPE_NONE &&
+ drvdata->addr_type[idx + 1] == ETM_ADDR_TYPE_NONE) ||
+ (drvdata->addr_type[idx] == ETM_ADDR_TYPE_RANGE &&
+ drvdata->addr_type[idx + 1] == ETM_ADDR_TYPE_RANGE))) {
+ spin_unlock(&drvdata->spinlock);
+ return -EPERM;
+ }
+
+ drvdata->addr_val[idx] = (u64)val1;
+ drvdata->addr_type[idx] = ETM_ADDR_TYPE_RANGE;
+ drvdata->addr_val[idx + 1] = (u64)val2;
+ drvdata->addr_type[idx + 1] = ETM_ADDR_TYPE_RANGE;
+ /*
+ * Program include or exclude control bits for vinst or vdata
+ * whenever we change addr comparators to ETM_ADDR_TYPE_RANGE
+ */
+ if (drvdata->mode & ETM_MODE_EXCLUDE)
+ etm4_set_mode_exclude(drvdata, true);
+ else
+ etm4_set_mode_exclude(drvdata, false);
+
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_RW(addr_range);
+
+static ssize_t addr_start_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ u8 idx;
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ spin_lock(&drvdata->spinlock);
+ idx = drvdata->addr_idx;
+
+ if (!(drvdata->addr_type[idx] == ETM_ADDR_TYPE_NONE ||
+ drvdata->addr_type[idx] == ETM_ADDR_TYPE_START)) {
+ spin_unlock(&drvdata->spinlock);
+ return -EPERM;
+ }
+
+ val = (unsigned long)drvdata->addr_val[idx];
+ spin_unlock(&drvdata->spinlock);
+ return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
+}
+
+static ssize_t addr_start_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ u8 idx;
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ if (kstrtoul(buf, 16, &val))
+ return -EINVAL;
+
+ spin_lock(&drvdata->spinlock);
+ idx = drvdata->addr_idx;
+ if (!drvdata->nr_addr_cmp) {
+ spin_unlock(&drvdata->spinlock);
+ return -EINVAL;
+ }
+ if (!(drvdata->addr_type[idx] == ETM_ADDR_TYPE_NONE ||
+ drvdata->addr_type[idx] == ETM_ADDR_TYPE_START)) {
+ spin_unlock(&drvdata->spinlock);
+ return -EPERM;
+ }
+
+ drvdata->addr_val[idx] = (u64)val;
+ drvdata->addr_type[idx] = ETM_ADDR_TYPE_START;
+ drvdata->vissctlr |= BIT(idx);
+ /* SSSTATUS, bit[9] - turn on start/stop logic */
+ drvdata->vinst_ctrl |= BIT(9);
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_RW(addr_start);
+
+static ssize_t addr_stop_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ u8 idx;
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ spin_lock(&drvdata->spinlock);
+ idx = drvdata->addr_idx;
+
+ if (!(drvdata->addr_type[idx] == ETM_ADDR_TYPE_NONE ||
+ drvdata->addr_type[idx] == ETM_ADDR_TYPE_STOP)) {
+ spin_unlock(&drvdata->spinlock);
+ return -EPERM;
+ }
+
+ val = (unsigned long)drvdata->addr_val[idx];
+ spin_unlock(&drvdata->spinlock);
+ return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
+}
+
+static ssize_t addr_stop_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ u8 idx;
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ if (kstrtoul(buf, 16, &val))
+ return -EINVAL;
+
+ spin_lock(&drvdata->spinlock);
+ idx = drvdata->addr_idx;
+ if (!drvdata->nr_addr_cmp) {
+ spin_unlock(&drvdata->spinlock);
+ return -EINVAL;
+ }
+ if (!(drvdata->addr_type[idx] == ETM_ADDR_TYPE_NONE ||
+ drvdata->addr_type[idx] == ETM_ADDR_TYPE_STOP)) {
+ spin_unlock(&drvdata->spinlock);
+ return -EPERM;
+ }
+
+ drvdata->addr_val[idx] = (u64)val;
+ drvdata->addr_type[idx] = ETM_ADDR_TYPE_STOP;
+ drvdata->vissctlr |= BIT(idx + 16);
+ /* SSSTATUS, bit[9] - turn on start/stop logic */
+ drvdata->vinst_ctrl |= BIT(9);
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_RW(addr_stop);
+
+static ssize_t addr_ctxtype_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ ssize_t len;
+ u8 idx, val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ spin_lock(&drvdata->spinlock);
+ idx = drvdata->addr_idx;
+ /* CONTEXTTYPE, bits[3:2] */
+ val = BMVAL(drvdata->addr_acc[idx], 2, 3);
+ len = scnprintf(buf, PAGE_SIZE, "%s\n", val == ETM_CTX_NONE ? "none" :
+ (val == ETM_CTX_CTXID ? "ctxid" :
+ (val == ETM_CTX_VMID ? "vmid" : "all")));
+ spin_unlock(&drvdata->spinlock);
+ return len;
+}
+
+static ssize_t addr_ctxtype_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ u8 idx;
+ char str[10] = "";
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ if (strlen(buf) >= 10)
+ return -EINVAL;
+ if (sscanf(buf, "%s", str) != 1)
+ return -EINVAL;
+
+ spin_lock(&drvdata->spinlock);
+ idx = drvdata->addr_idx;
+ if (!strcmp(str, "none"))
+ /* start by clearing context type bits */
+ drvdata->addr_acc[idx] &= ~(BIT(2) | BIT(3));
+ else if (!strcmp(str, "ctxid")) {
+ /* 0b01 The trace unit performs a Context ID */
+ if (drvdata->numcidc) {
+ drvdata->addr_acc[idx] |= BIT(2);
+ drvdata->addr_acc[idx] &= ~BIT(3);
+ }
+ } else if (!strcmp(str, "vmid")) {
+ /* 0b10 The trace unit performs a VMID */
+ if (drvdata->numvmidc) {
+ drvdata->addr_acc[idx] &= ~BIT(2);
+ drvdata->addr_acc[idx] |= BIT(3);
+ }
+ } else if (!strcmp(str, "all")) {
+ /*
+ * 0b11 The trace unit performs a Context ID
+ * comparison and a VMID
+ */
+ if (drvdata->numcidc)
+ drvdata->addr_acc[idx] |= BIT(2);
+ if (drvdata->numvmidc)
+ drvdata->addr_acc[idx] |= BIT(3);
+ }
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_RW(addr_ctxtype);
+
+static ssize_t addr_context_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ u8 idx;
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ spin_lock(&drvdata->spinlock);
+ idx = drvdata->addr_idx;
+ /* context ID comparator bits[6:4] */
+ val = BMVAL(drvdata->addr_acc[idx], 4, 6);
+ spin_unlock(&drvdata->spinlock);
+ return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
+}
+
+static ssize_t addr_context_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ u8 idx;
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ if (kstrtoul(buf, 16, &val))
+ return -EINVAL;
+ if ((drvdata->numcidc <= 1) && (drvdata->numvmidc <= 1))
+ return -EINVAL;
+ if (val >= (drvdata->numcidc >= drvdata->numvmidc ?
+ drvdata->numcidc : drvdata->numvmidc))
+ return -EINVAL;
+
+ spin_lock(&drvdata->spinlock);
+ idx = drvdata->addr_idx;
+ /* clear context ID comparator bits[6:4] */
+ drvdata->addr_acc[idx] &= ~(BIT(4) | BIT(5) | BIT(6));
+ drvdata->addr_acc[idx] |= (val << 4);
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_RW(addr_context);
+
static ssize_t status_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -1191,6 +1606,14 @@ static struct attribute *coresight_etmv4_attrs[] = {
&dev_attr_event_vinst.attr,
&dev_attr_s_exlevel_vinst.attr,
&dev_attr_ns_exlevel_vinst.attr,
+ &dev_attr_addr_idx.attr,
+ &dev_attr_addr_instdatatype.attr,
+ &dev_attr_addr_single.attr,
+ &dev_attr_addr_range.attr,
+ &dev_attr_addr_start.attr,
+ &dev_attr_addr_stop.attr,
+ &dev_attr_addr_ctxtype.attr,
+ &dev_attr_addr_context.attr,
&dev_attr_status.attr,
&dev_attr_mgmt.attr,
&dev_attr_trcidr.attr,
--
1.9.1
^ permalink raw reply related
* [PATCH v2 07/11] coresight-etm4x: Controls pertaining to the sequencer functions
From: Mathieu Poirier @ 2015-04-29 17:16 UTC (permalink / raw)
To: gregkh
Cc: linux-arm-kernel, linux-api, linux-kernel, kaixu.xia,
zhang.chunyan, mathieu.poirier
In-Reply-To: <1430327795-10710-1-git-send-email-mathieu.poirier@linaro.org>
From: Pratik Patel <pratikp@codeaurora.org>
Adding sysfs entries to access the sequencers related registers,
more specifically the sequencer state, the sequencer state
transition and the sequencer reset control registers.
Signed-off-by: Pratik Patel <pratikp@codeaurora.org>
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
.../ABI/testing/sysfs-bus-coresight-devices-etm4x | 25 ++++
drivers/hwtracing/coresight/coresight-etm4x.c | 129 +++++++++++++++++++++
2 files changed, 154 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x b/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
index 8cdc4ad10bd6..44723f2e107e 100644
--- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
+++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
@@ -191,3 +191,28 @@ Date: April 2015
KernelVersion: 4.01
Contact: Mathieu Poirier <mathieu.poirier@linaro.org>
Description: (RW) Used to setup address range comparator values.
+
+What: /sys/bus/coresight/devices/<memory_map>.etm/seq_idx
+Date: April 2015
+KernelVersion: 4.01
+Contact: Mathieu Poirier <mathieu.poirier@linaro.org>
+Description: (RW) Select which sequensor.
+
+What: /sys/bus/coresight/devices/<memory_map>.etm/seq_state
+Date: April 2015
+KernelVersion: 4.01
+Contact: Mathieu Poirier <mathieu.poirier@linaro.org>
+Description: (RW) Use this to set, or read, the sequencer state.
+
+What: /sys/bus/coresight/devices/<memory_map>.etm/seq_event
+Date: April 2015
+KernelVersion: 4.01
+Contact: Mathieu Poirier <mathieu.poirier@linaro.org>
+Description: (RW) Moves the sequencer state to a specific state.
+
+What: /sys/bus/coresight/devices/<memory_map>.etm/seq_reset_event
+Date: April 2015
+KernelVersion: 4.01
+Contact: Mathieu Poirier <mathieu.poirier@linaro.org>
+Description: (RW) Moves the sequencer to state 0 when a programmed event
+ occurs.
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
index 47cc68b8cc43..2ab64e2333e4 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x.c
@@ -1442,6 +1442,131 @@ static ssize_t addr_context_store(struct device *dev,
}
static DEVICE_ATTR_RW(addr_context);
+static ssize_t seq_idx_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ val = drvdata->seq_idx;
+ return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
+}
+
+static ssize_t seq_idx_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ if (kstrtoul(buf, 16, &val))
+ return -EINVAL;
+ if (val >= drvdata->nrseqstate - 1)
+ return -EINVAL;
+
+ /*
+ * Use spinlock to ensure index doesn't change while it gets
+ * dereferenced multiple times within a spinlock block elsewhere.
+ */
+ spin_lock(&drvdata->spinlock);
+ drvdata->seq_idx = val;
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_RW(seq_idx);
+
+static ssize_t seq_state_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ val = drvdata->seq_state;
+ return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
+}
+
+static ssize_t seq_state_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ if (kstrtoul(buf, 16, &val))
+ return -EINVAL;
+ if (val >= drvdata->nrseqstate)
+ return -EINVAL;
+
+ drvdata->seq_state = val;
+ return size;
+}
+static DEVICE_ATTR_RW(seq_state);
+
+static ssize_t seq_event_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ u8 idx;
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ spin_lock(&drvdata->spinlock);
+ idx = drvdata->seq_idx;
+ val = drvdata->seq_ctrl[idx];
+ spin_unlock(&drvdata->spinlock);
+ return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
+}
+
+static ssize_t seq_event_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ u8 idx;
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ if (kstrtoul(buf, 16, &val))
+ return -EINVAL;
+
+ spin_lock(&drvdata->spinlock);
+ idx = drvdata->seq_idx;
+ /* RST, bits[7:0] */
+ drvdata->seq_ctrl[idx] = val & 0xFF;
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_RW(seq_event);
+
+static ssize_t seq_reset_event_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ val = drvdata->seq_rst;
+ return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
+}
+
+static ssize_t seq_reset_event_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ if (kstrtoul(buf, 16, &val))
+ return -EINVAL;
+ if (!(drvdata->nrseqstate))
+ return -EINVAL;
+
+ drvdata->seq_rst = val & ETMv4_EVENT_MASK;
+ return size;
+}
+static DEVICE_ATTR_RW(seq_reset_event);
+
static ssize_t status_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -1614,6 +1739,10 @@ static struct attribute *coresight_etmv4_attrs[] = {
&dev_attr_addr_stop.attr,
&dev_attr_addr_ctxtype.attr,
&dev_attr_addr_context.attr,
+ &dev_attr_seq_idx.attr,
+ &dev_attr_seq_state.attr,
+ &dev_attr_seq_event.attr,
+ &dev_attr_seq_reset_event.attr,
&dev_attr_status.attr,
&dev_attr_mgmt.attr,
&dev_attr_trcidr.attr,
--
1.9.1
^ permalink raw reply related
* [PATCH v2 08/11] coresight-etm4x: Controls pertaining to the counter functions
From: Mathieu Poirier @ 2015-04-29 17:16 UTC (permalink / raw)
To: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
kaixu.xia-QSEj5FYQhm4dnm+yROfE0A,
zhang.chunyan-QSEj5FYQhm4dnm+yROfE0A,
mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A
In-Reply-To: <1430327795-10710-1-git-send-email-mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
From: Pratik Patel <pratikp-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Adding sysfs entries related to the counter functionality, more specifically
to set, control and reload the counters.
Signed-off-by: Pratik Patel <pratikp-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Signed-off-by: Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
.../ABI/testing/sysfs-bus-coresight-devices-etm4x | 26 ++++
drivers/hwtracing/coresight/coresight-etm4x.c | 144 +++++++++++++++++++++
2 files changed, 170 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x b/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
index 44723f2e107e..b4581c9426d3 100644
--- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
+++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
@@ -216,3 +216,29 @@ KernelVersion: 4.01
Contact: Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Description: (RW) Moves the sequencer to state 0 when a programmed event
occurs.
+
+What: /sys/bus/coresight/devices/<memory_map>.etm/cntr_idx
+Date: April 2015
+KernelVersion: 4.01
+Contact: Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
+Description: (RW) Select which counter unit to work with.
+
+What: /sys/bus/coresight/devices/<memory_map>.etm/cntrldvr
+Date: April 2015
+KernelVersion: 4.01
+Contact: Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
+Description: (RW) This sets or returns the reload count value of the
+ specific counter.
+
+What: /sys/bus/coresight/devices/<memory_map>.etm/cntr_val
+Date: April 2015
+KernelVersion: 4.01
+Contact: Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
+Description: (RW) This sets or returns the current count value of the
+ specific counter.
+
+What: /sys/bus/coresight/devices/<memory_map>.etm/cntr_ctrl
+Date: April 2015
+KernelVersion: 4.01
+Contact: Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
+Description: (RW) Controls the operation of the selected counter.
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
index 2ab64e2333e4..cd6a1d51ad66 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x.c
@@ -1567,6 +1567,146 @@ static ssize_t seq_reset_event_store(struct device *dev,
}
static DEVICE_ATTR_RW(seq_reset_event);
+static ssize_t cntr_idx_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ val = drvdata->cntr_idx;
+ return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
+}
+
+static ssize_t cntr_idx_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ if (kstrtoul(buf, 16, &val))
+ return -EINVAL;
+ if (val >= drvdata->nr_cntr)
+ return -EINVAL;
+
+ /*
+ * Use spinlock to ensure index doesn't change while it gets
+ * dereferenced multiple times within a spinlock block elsewhere.
+ */
+ spin_lock(&drvdata->spinlock);
+ drvdata->cntr_idx = val;
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_RW(cntr_idx);
+
+static ssize_t cntrldvr_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ u8 idx;
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ spin_lock(&drvdata->spinlock);
+ idx = drvdata->cntr_idx;
+ val = drvdata->cntrldvr[idx];
+ spin_unlock(&drvdata->spinlock);
+ return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
+}
+
+static ssize_t cntrldvr_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ u8 idx;
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ if (kstrtoul(buf, 16, &val))
+ return -EINVAL;
+ if (val > ETM_CNTR_MAX_VAL)
+ return -EINVAL;
+
+ spin_lock(&drvdata->spinlock);
+ idx = drvdata->cntr_idx;
+ drvdata->cntrldvr[idx] = val;
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_RW(cntrldvr);
+
+static ssize_t cntr_val_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ u8 idx;
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ spin_lock(&drvdata->spinlock);
+ idx = drvdata->cntr_idx;
+ val = drvdata->cntr_val[idx];
+ spin_unlock(&drvdata->spinlock);
+ return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
+}
+
+static ssize_t cntr_val_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ u8 idx;
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ if (kstrtoul(buf, 16, &val))
+ return -EINVAL;
+ if (val > ETM_CNTR_MAX_VAL)
+ return -EINVAL;
+
+ spin_lock(&drvdata->spinlock);
+ idx = drvdata->cntr_idx;
+ drvdata->cntr_val[idx] = val;
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_RW(cntr_val);
+
+static ssize_t cntr_ctrl_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ u8 idx;
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ spin_lock(&drvdata->spinlock);
+ idx = drvdata->cntr_idx;
+ val = drvdata->cntr_ctrl[idx];
+ spin_unlock(&drvdata->spinlock);
+ return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
+}
+
+static ssize_t cntr_ctrl_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ u8 idx;
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ if (kstrtoul(buf, 16, &val))
+ return -EINVAL;
+
+ spin_lock(&drvdata->spinlock);
+ idx = drvdata->cntr_idx;
+ drvdata->cntr_ctrl[idx] = val;
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_RW(cntr_ctrl);
+
static ssize_t status_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -1743,6 +1883,10 @@ static struct attribute *coresight_etmv4_attrs[] = {
&dev_attr_seq_state.attr,
&dev_attr_seq_event.attr,
&dev_attr_seq_reset_event.attr,
+ &dev_attr_cntr_idx.attr,
+ &dev_attr_cntrldvr.attr,
+ &dev_attr_cntr_val.attr,
+ &dev_attr_cntr_ctrl.attr,
&dev_attr_status.attr,
&dev_attr_mgmt.attr,
&dev_attr_trcidr.attr,
--
1.9.1
^ permalink raw reply related
* [PATCH v2 09/11] coresight-etm4x: Controls pertaining to the selection of resources
From: Mathieu Poirier @ 2015-04-29 17:16 UTC (permalink / raw)
To: gregkh
Cc: linux-arm-kernel, linux-api, linux-kernel, kaixu.xia,
zhang.chunyan, mathieu.poirier
In-Reply-To: <1430327795-10710-1-git-send-email-mathieu.poirier@linaro.org>
From: Pratik Patel <pratikp@codeaurora.org>
Adding sysfs entries to control the selection of the resources the
trace unit will use as triggers to perform a trace run.
Signed-off-by: Pratik Patel <pratikp@codeaurora.org>
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
.../ABI/testing/sysfs-bus-coresight-devices-etm4x | 12 ++++
drivers/hwtracing/coresight/coresight-etm4x.c | 75 ++++++++++++++++++++++
2 files changed, 87 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x b/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
index b4581c9426d3..b5c0456290ab 100644
--- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
+++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
@@ -242,3 +242,15 @@ Date: April 2015
KernelVersion: 4.01
Contact: Mathieu Poirier <mathieu.poirier@linaro.org>
Description: (RW) Controls the operation of the selected counter.
+
+What: /sys/bus/coresight/devices/<memory_map>.etm/res_idx
+Date: April 2015
+KernelVersion: 4.01
+Contact: Mathieu Poirier <mathieu.poirier@linaro.org>
+Description: (RW) Select which resource selection unit to work with.
+
+What: /sys/bus/coresight/devices/<memory_map>.etm/res_ctrl
+Date: April 2015
+KernelVersion: 4.01
+Contact: Mathieu Poirier <mathieu.poirier@linaro.org>
+Description: (RW) Controls the selection of the resources in the trace unit.
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
index cd6a1d51ad66..a9fda8ad8c2e 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x.c
@@ -1707,6 +1707,79 @@ static ssize_t cntr_ctrl_store(struct device *dev,
}
static DEVICE_ATTR_RW(cntr_ctrl);
+static ssize_t res_idx_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ val = drvdata->res_idx;
+ return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
+}
+
+static ssize_t res_idx_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ if (kstrtoul(buf, 16, &val))
+ return -EINVAL;
+ /* Resource selector pair 0 is always implemented and reserved */
+ if ((val == 0) || (val >= drvdata->nr_resource))
+ return -EINVAL;
+
+ /*
+ * Use spinlock to ensure index doesn't change while it gets
+ * dereferenced multiple times within a spinlock block elsewhere.
+ */
+ spin_lock(&drvdata->spinlock);
+ drvdata->res_idx = val;
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_RW(res_idx);
+
+static ssize_t res_ctrl_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ u8 idx;
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ spin_lock(&drvdata->spinlock);
+ idx = drvdata->res_idx;
+ val = drvdata->res_ctrl[idx];
+ spin_unlock(&drvdata->spinlock);
+ return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
+}
+
+static ssize_t res_ctrl_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ u8 idx;
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ if (kstrtoul(buf, 16, &val))
+ return -EINVAL;
+
+ spin_lock(&drvdata->spinlock);
+ idx = drvdata->res_idx;
+ /* For odd idx pair inversal bit is RES0 */
+ if (idx % 2 != 0)
+ /* PAIRINV, bit[21] */
+ val &= ~BIT(21);
+ drvdata->res_ctrl[idx] = val;
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_RW(res_ctrl);
+
static ssize_t status_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -1887,6 +1960,8 @@ static struct attribute *coresight_etmv4_attrs[] = {
&dev_attr_cntrldvr.attr,
&dev_attr_cntr_val.attr,
&dev_attr_cntr_ctrl.attr,
+ &dev_attr_res_idx.attr,
+ &dev_attr_res_ctrl.attr,
&dev_attr_status.attr,
&dev_attr_mgmt.attr,
&dev_attr_trcidr.attr,
--
1.9.1
^ permalink raw reply related
* [PATCH v2 10/11] coresight-etm4x: Controls pertaining to the context ID functions
From: Mathieu Poirier @ 2015-04-29 17:16 UTC (permalink / raw)
To: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
kaixu.xia-QSEj5FYQhm4dnm+yROfE0A,
zhang.chunyan-QSEj5FYQhm4dnm+yROfE0A,
mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A
In-Reply-To: <1430327795-10710-1-git-send-email-mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
From: Pratik Patel <pratikp-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Adding sysfs entries to access and configure specifics about the
context ID comparator functions.
Signed-off-by: Pratik Patel <pratikp-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Signed-off-by: Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
.../ABI/testing/sysfs-bus-coresight-devices-etm4x | 19 +++
drivers/hwtracing/coresight/coresight-etm4x.c | 187 +++++++++++++++++++++
2 files changed, 206 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x b/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
index b5c0456290ab..8b32cb7b9723 100644
--- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
+++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
@@ -254,3 +254,22 @@ Date: April 2015
KernelVersion: 4.01
Contact: Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Description: (RW) Controls the selection of the resources in the trace unit.
+
+What: /sys/bus/coresight/devices/<memory_map>.etm/ctxid_idx
+Date: April 2015
+KernelVersion: 4.01
+Contact: Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
+Description: (RW) Select which context ID comparator to work with.
+
+What: /sys/bus/coresight/devices/<memory_map>.etm/ctxid_val
+Date: April 2015
+KernelVersion: 4.01
+Contact: Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
+Description: (RW) Get/Set the context ID comparator value to trigger on.
+
+What: /sys/bus/coresight/devices/<memory_map>.etm/ctxid_masks
+Date: April 2015
+KernelVersion: 4.01
+Contact: Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
+Description: (RW) Mask for all 8 context ID comparator value
+ registers (if implemented).
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
index a9fda8ad8c2e..58ae33980b12 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x.c
@@ -1780,6 +1780,190 @@ static ssize_t res_ctrl_store(struct device *dev,
}
static DEVICE_ATTR_RW(res_ctrl);
+static ssize_t ctxid_idx_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ val = drvdata->ctxid_idx;
+ return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
+}
+
+static ssize_t ctxid_idx_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ if (kstrtoul(buf, 16, &val))
+ return -EINVAL;
+ if (val >= drvdata->numcidc)
+ return -EINVAL;
+
+ /*
+ * Use spinlock to ensure index doesn't change while it gets
+ * dereferenced multiple times within a spinlock block elsewhere.
+ */
+ spin_lock(&drvdata->spinlock);
+ drvdata->ctxid_idx = val;
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_RW(ctxid_idx);
+
+static ssize_t ctxid_val_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ u8 idx;
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ spin_lock(&drvdata->spinlock);
+ idx = drvdata->ctxid_idx;
+ val = (unsigned long)drvdata->ctxid_val[idx];
+ spin_unlock(&drvdata->spinlock);
+ return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
+}
+
+static ssize_t ctxid_val_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ u8 idx;
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ /*
+ * only implemented when ctxid tracing is enabled, i.e. at least one
+ * ctxid comparator is implemented and ctxid is greater than 0 bits
+ * in length
+ */
+ if (!drvdata->ctxid_size || !drvdata->numcidc)
+ return -EINVAL;
+ if (kstrtoul(buf, 16, &val))
+ return -EINVAL;
+
+ spin_lock(&drvdata->spinlock);
+ idx = drvdata->ctxid_idx;
+ drvdata->ctxid_val[idx] = (u64)val;
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_RW(ctxid_val);
+
+static ssize_t ctxid_masks_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ unsigned long val1, val2;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ spin_lock(&drvdata->spinlock);
+ val1 = drvdata->ctxid_mask0;
+ val2 = drvdata->ctxid_mask1;
+ spin_unlock(&drvdata->spinlock);
+ return scnprintf(buf, PAGE_SIZE, "%#lx %#lx\n", val1, val2);
+}
+
+static ssize_t ctxid_masks_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ u8 i, j, maskbyte;
+ unsigned long val1, val2, mask;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ /*
+ * only implemented when ctxid tracing is enabled, i.e. at least one
+ * ctxid comparator is implemented and ctxid is greater than 0 bits
+ * in length
+ */
+ if (!drvdata->ctxid_size || !drvdata->numcidc)
+ return -EINVAL;
+ if (sscanf(buf, "%lx %lx", &val1, &val2) != 2)
+ return -EINVAL;
+
+ spin_lock(&drvdata->spinlock);
+ /*
+ * each byte[0..3] controls mask value applied to ctxid
+ * comparator[0..3]
+ */
+ switch (drvdata->numcidc) {
+ case 0x1:
+ /* COMP0, bits[7:0] */
+ drvdata->ctxid_mask0 = val1 & 0xFF;
+ break;
+ case 0x2:
+ /* COMP1, bits[15:8] */
+ drvdata->ctxid_mask0 = val1 & 0xFFFF;
+ break;
+ case 0x3:
+ /* COMP2, bits[23:16] */
+ drvdata->ctxid_mask0 = val1 & 0xFFFFFF;
+ break;
+ case 0x4:
+ /* COMP3, bits[31:24] */
+ drvdata->ctxid_mask0 = val1;
+ break;
+ case 0x5:
+ /* COMP4, bits[7:0] */
+ drvdata->ctxid_mask0 = val1;
+ drvdata->ctxid_mask1 = val2 & 0xFF;
+ break;
+ case 0x6:
+ /* COMP5, bits[15:8] */
+ drvdata->ctxid_mask0 = val1;
+ drvdata->ctxid_mask1 = val2 & 0xFFFF;
+ break;
+ case 0x7:
+ /* COMP6, bits[23:16] */
+ drvdata->ctxid_mask0 = val1;
+ drvdata->ctxid_mask1 = val2 & 0xFFFFFF;
+ break;
+ case 0x8:
+ /* COMP7, bits[31:24] */
+ drvdata->ctxid_mask0 = val1;
+ drvdata->ctxid_mask1 = val2;
+ break;
+ default:
+ break;
+ }
+ /*
+ * If software sets a mask bit to 1, it must program relevant byte
+ * of ctxid comparator value 0x0, otherwise behavior is unpredictable.
+ * For example, if bit[3] of ctxid_mask0 is 1, we must clear bits[31:24]
+ * of ctxid comparator0 value (corresponding to byte 0) register.
+ */
+ mask = drvdata->ctxid_mask0;
+ for (i = 0; i < drvdata->numcidc; i++) {
+ /* mask value of corresponding ctxid comparator */
+ maskbyte = mask & ETMv4_EVENT_MASK;
+ /*
+ * each bit corresponds to a byte of respective ctxid comparator
+ * value register
+ */
+ for (j = 0; j < 8; j++) {
+ if (maskbyte & 1)
+ drvdata->ctxid_val[i] &= ~(0xFF << (j * 8));
+ maskbyte >>= 1;
+ }
+ /* Select the next ctxid comparator mask value */
+ if (i == 3)
+ /* ctxid comparators[4-7] */
+ mask = drvdata->ctxid_mask1;
+ else
+ mask >>= 0x8;
+ }
+
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_RW(ctxid_masks);
+
static ssize_t status_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -1962,6 +2146,9 @@ static struct attribute *coresight_etmv4_attrs[] = {
&dev_attr_cntr_ctrl.attr,
&dev_attr_res_idx.attr,
&dev_attr_res_ctrl.attr,
+ &dev_attr_ctxid_idx.attr,
+ &dev_attr_ctxid_val.attr,
+ &dev_attr_ctxid_masks.attr,
&dev_attr_status.attr,
&dev_attr_mgmt.attr,
&dev_attr_trcidr.attr,
--
1.9.1
^ permalink raw reply related
* [PATCH v2 11/11] coresight-etm4x: Controls pertaining to the VM ID functions
From: Mathieu Poirier @ 2015-04-29 17:16 UTC (permalink / raw)
To: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
kaixu.xia-QSEj5FYQhm4dnm+yROfE0A,
zhang.chunyan-QSEj5FYQhm4dnm+yROfE0A,
mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A
In-Reply-To: <1430327795-10710-1-git-send-email-mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
From: Pratik Patel <pratikp-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Adding sysfs entries to access and configure specifics about the
virtual machine ID comparator functions.
Signed-off-by: Pratik Patel <pratikp-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Signed-off-by: Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
.../ABI/testing/sysfs-bus-coresight-devices-etm4x | 20 +++
drivers/hwtracing/coresight/coresight-etm4x.c | 178 +++++++++++++++++++++
2 files changed, 198 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x b/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
index 8b32cb7b9723..eb5153891919 100644
--- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
+++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-etm4x
@@ -273,3 +273,23 @@ KernelVersion: 4.01
Contact: Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Description: (RW) Mask for all 8 context ID comparator value
registers (if implemented).
+
+What: /sys/bus/coresight/devices/<memory_map>.etm/vmid_idx
+Date: April 2015
+KernelVersion: 4.01
+Contact: Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
+Description: (RW) Select which virtual machine ID comparator to work with.
+
+What: /sys/bus/coresight/devices/<memory_map>.etm/vmid_val
+Date: April 2015
+KernelVersion: 4.01
+Contact: Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
+Description: (RW) Get/Set the virtual machine ID comparator value to
+ trigger on.
+
+What: /sys/bus/coresight/devices/<memory_map>.etm/vmid_masks
+Date: April 2015
+KernelVersion: 4.01
+Contact: Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
+Description: (RW) Mask for all 8 virtual machine ID comparator value
+ registers (if implemented).
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
index 58ae33980b12..cdcfa019a82c 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x.c
@@ -1964,6 +1964,181 @@ static ssize_t ctxid_masks_store(struct device *dev,
}
static DEVICE_ATTR_RW(ctxid_masks);
+static ssize_t vmid_idx_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ val = drvdata->vmid_idx;
+ return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
+}
+
+static ssize_t vmid_idx_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ if (kstrtoul(buf, 16, &val))
+ return -EINVAL;
+ if (val >= drvdata->numvmidc)
+ return -EINVAL;
+
+ /*
+ * Use spinlock to ensure index doesn't change while it gets
+ * dereferenced multiple times within a spinlock block elsewhere.
+ */
+ spin_lock(&drvdata->spinlock);
+ drvdata->vmid_idx = val;
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_RW(vmid_idx);
+
+static ssize_t vmid_val_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ val = (unsigned long)drvdata->vmid_val[drvdata->vmid_idx];
+ return scnprintf(buf, PAGE_SIZE, "%#lx\n", val);
+}
+
+static ssize_t vmid_val_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ unsigned long val;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ /*
+ * only implemented when vmid tracing is enabled, i.e. at least one
+ * vmid comparator is implemented and at least 8 bit vmid size
+ */
+ if (!drvdata->vmid_size || !drvdata->numvmidc)
+ return -EINVAL;
+ if (kstrtoul(buf, 16, &val))
+ return -EINVAL;
+
+ spin_lock(&drvdata->spinlock);
+ drvdata->vmid_val[drvdata->vmid_idx] = (u64)val;
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_RW(vmid_val);
+
+static ssize_t vmid_masks_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ unsigned long val1, val2;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ spin_lock(&drvdata->spinlock);
+ val1 = drvdata->vmid_mask0;
+ val2 = drvdata->vmid_mask1;
+ spin_unlock(&drvdata->spinlock);
+ return scnprintf(buf, PAGE_SIZE, "%#lx %#lx\n", val1, val2);
+}
+
+static ssize_t vmid_masks_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t size)
+{
+ u8 i, j, maskbyte;
+ unsigned long val1, val2, mask;
+ struct etmv4_drvdata *drvdata = dev_get_drvdata(dev->parent);
+ /*
+ * only implemented when vmid tracing is enabled, i.e. at least one
+ * vmid comparator is implemented and at least 8 bit vmid size
+ */
+ if (!drvdata->vmid_size || !drvdata->numvmidc)
+ return -EINVAL;
+ if (sscanf(buf, "%lx %lx", &val1, &val2) != 2)
+ return -EINVAL;
+
+ spin_lock(&drvdata->spinlock);
+
+ /*
+ * each byte[0..3] controls mask value applied to vmid
+ * comparator[0..3]
+ */
+ switch (drvdata->numvmidc) {
+ case 0x1:
+ /* COMP0, bits[7:0] */
+ drvdata->vmid_mask0 = val1 & 0xFF;
+ break;
+ case 0x2:
+ /* COMP1, bits[15:8] */
+ drvdata->vmid_mask0 = val1 & 0xFFFF;
+ break;
+ case 0x3:
+ /* COMP2, bits[23:16] */
+ drvdata->vmid_mask0 = val1 & 0xFFFFFF;
+ break;
+ case 0x4:
+ /* COMP3, bits[31:24] */
+ drvdata->vmid_mask0 = val1;
+ break;
+ case 0x5:
+ /* COMP4, bits[7:0] */
+ drvdata->vmid_mask0 = val1;
+ drvdata->vmid_mask1 = val2 & 0xFF;
+ break;
+ case 0x6:
+ /* COMP5, bits[15:8] */
+ drvdata->vmid_mask0 = val1;
+ drvdata->vmid_mask1 = val2 & 0xFFFF;
+ break;
+ case 0x7:
+ /* COMP6, bits[23:16] */
+ drvdata->vmid_mask0 = val1;
+ drvdata->vmid_mask1 = val2 & 0xFFFFFF;
+ break;
+ case 0x8:
+ /* COMP7, bits[31:24] */
+ drvdata->vmid_mask0 = val1;
+ drvdata->vmid_mask1 = val2;
+ break;
+ default:
+ break;
+ }
+
+ /*
+ * If software sets a mask bit to 1, it must program relevant byte
+ * of vmid comparator value 0x0, otherwise behavior is unpredictable.
+ * For example, if bit[3] of vmid_mask0 is 1, we must clear bits[31:24]
+ * of vmid comparator0 value (corresponding to byte 0) register.
+ */
+ mask = drvdata->vmid_mask0;
+ for (i = 0; i < drvdata->numvmidc; i++) {
+ /* mask value of corresponding vmid comparator */
+ maskbyte = mask & ETMv4_EVENT_MASK;
+ /*
+ * each bit corresponds to a byte of respective vmid comparator
+ * value register
+ */
+ for (j = 0; j < 8; j++) {
+ if (maskbyte & 1)
+ drvdata->vmid_val[i] &= ~(0xFF << (j * 8));
+ maskbyte >>= 1;
+ }
+ /* Select the next vmid comparator mask value */
+ if (i == 3)
+ /* vmid comparators[4-7] */
+ mask = drvdata->vmid_mask1;
+ else
+ mask >>= 0x8;
+ }
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_RW(vmid_masks);
+
static ssize_t status_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -2149,6 +2324,9 @@ static struct attribute *coresight_etmv4_attrs[] = {
&dev_attr_ctxid_idx.attr,
&dev_attr_ctxid_val.attr,
&dev_attr_ctxid_masks.attr,
+ &dev_attr_vmid_idx.attr,
+ &dev_attr_vmid_val.attr,
+ &dev_attr_vmid_masks.attr,
&dev_attr_status.attr,
&dev_attr_mgmt.attr,
&dev_attr_trcidr.attr,
--
1.9.1
^ permalink raw reply related
* Re: Regression: Requiring CAP_SYS_ADMIN for /proc/<pid>/pagemap causes application-level breakage
From: Mark Williamson @ 2015-04-29 18:44 UTC (permalink / raw)
To: Mark Seaborn
Cc: kernel list, Kirill A. Shutemov, Pavel Emelyanov,
Konstantin Khlebnikov, Andrew Morton, Linus Torvalds,
Andy Lutomirski, Linux API, Finn Grimwood, Daniel James
In-Reply-To: <CAEVpBa+vjfR8p_fJQPXyrQCMYJOocQpTHmkTYmj7nOpGcce=Pg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi all,
We've been investigating further and found a snag with the PFN-hiding
approach discussed last week - looks like it won't be enough on all
the architectures we support. Our product runs on x86_32, x86_64 and
ARM. For now, it looks like soft-dirty is only available on x86_64.
A patch that simply zeros out the physical addresses in
/proc/PID/pagemap will therefore help us on x86_64 but we'll still
have problems on other platforms[1].
For context, we were previously using pagemap as a cross-platform way
to get soft-dirty-like functionality. Specifically, to ask "did a
process write to any pages since fork()" by comparing addresses and
deducing where CoW must have occurred. In the absence of soft-dirty
and the physical addresses, it looks like we can't figure that out
with the remaining information in pagemap.
If the pagemap file included the "writeable" bit from the PTE, we
think we'd have all the information required to deduce what we need
(although I realise that's a bit of a nasty workaround). If I
proposed including the PTE protection bits in pagemap, would that be
controversial? I'm guessing yes but thought it was worth a shot ;-)
Would anybody be able to suggest a more tasteful approach?
Thanks,
Mark
[1] I'd note that using soft-dirty is clearly the right approach for
us on x64, where available and that ideally we'd use it on other
architectures - cross-arch support for soft-dirty is a slightly
different discussion, which I hope to post another thread for.
On Fri, Apr 24, 2015 at 5:43 PM, Mark Williamson
<mwilliamson-/4lU09Eg6ahx67MzidHQgQC/G2K4zDHf@public.gmane.org> wrote:
> Hi Mark,
>
> On Fri, Apr 24, 2015 at 4:26 PM, Mark Seaborn <mseaborn-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
>> I'm curious, what do you use the physical page addresses for?
>>
>> Since you pointed to http://undo-software.com, which talks about
>> reversible debugging tools, I can guess you would use the soft-dirty
>> flag to implement copy-on-write snapshotting. I'm guessing you might
>> use physical page addresses for determining when the same page is
>> mapped twice (in the same process or different processes)?
>
> That's pretty much it. Actually, we're effectively using the physical
> addresses to emulate soft-dirty. For certain operations (e.g. some
> system calls) we need to track what memory has changed since we last
> looked at the process state. We have a mechanism that forks a child
> process, runs the system call, then refers to pagemap to figure out
> what's been modified.
>
> Currently, our mechanism compares the physical addresses of pages
> before and after the syscall so that we can see which pages got CoWed.
> This is perhaps a slightly "unconventional" use of the interface but
> we support kernels that predate the soft-dirty mechanism and (as far
> as we know) this is probably the best way we can answer "What got
> changed?" on those releases.
>
> Using the soft-dirty mechanism where available should make our code
> both cleaner and faster, so if we can fix the pagemap file to allow
> that then we'll be quite happy!
>
> Cheers,
> Mark
^ permalink raw reply
* Re: Regression: Requiring CAP_SYS_ADMIN for /proc/<pid>/pagemap causes application-level breakage
From: Mark Williamson @ 2015-04-29 19:23 UTC (permalink / raw)
To: Mark Seaborn
Cc: kernel list, Kirill A. Shutemov, Pavel Emelyanov,
Konstantin Khlebnikov, Andrew Morton, Linus Torvalds,
Andy Lutomirski, Linux API, Finn Grimwood, Daniel James
In-Reply-To: <CAEVpBa+DPbhZeRsA=+Jnn7f1BqVZm9SU=ABjy6bAHbbMi_yn+g@mail.gmail.com>
Hi again,
On Wed, Apr 29, 2015 at 7:44 PM, Mark Williamson
<mwilliamson@undo-software.com> wrote:
> We've been investigating further and found a snag with the PFN-hiding
> approach discussed last week - looks like it won't be enough on all
> the architectures we support. Our product runs on x86_32, x86_64 and
> ARM. For now, it looks like soft-dirty is only available on x86_64.
> A patch that simply zeros out the physical addresses in
> /proc/PID/pagemap will therefore help us on x86_64 but we'll still
> have problems on other platforms[1].
Another thought occurs - although we *strictly* want to know "what got
written to", we might be able to get by with a superset of that, such
as "what got accessed, read or write"...
Thus, we could investigate clearing the Referenced bit (which I
understand we can do through /proc/PID/clear_refs) and then just treat
any subsequently-referenced pages as being potentially modified. It's
not ideal but it might be enough to get by...
I still feel a little nervous with this, since we support distros
(e.g. RHEL5) that are too old to have clear_refs. Still, it would
result in less disruption to the format of pagemap.
Thanks,
Mark
> For context, we were previously using pagemap as a cross-platform way
> to get soft-dirty-like functionality. Specifically, to ask "did a
> process write to any pages since fork()" by comparing addresses and
> deducing where CoW must have occurred. In the absence of soft-dirty
> and the physical addresses, it looks like we can't figure that out
> with the remaining information in pagemap.
>
> If the pagemap file included the "writeable" bit from the PTE, we
> think we'd have all the information required to deduce what we need
> (although I realise that's a bit of a nasty workaround). If I
> proposed including the PTE protection bits in pagemap, would that be
> controversial? I'm guessing yes but thought it was worth a shot ;-)
> Would anybody be able to suggest a more tasteful approach?
>
> Thanks,
> Mark
>
> [1] I'd note that using soft-dirty is clearly the right approach for
> us on x64, where available and that ideally we'd use it on other
> architectures - cross-arch support for soft-dirty is a slightly
> different discussion, which I hope to post another thread for.
>
> On Fri, Apr 24, 2015 at 5:43 PM, Mark Williamson
> <mwilliamson@undo-software.com> wrote:
>> Hi Mark,
>>
>> On Fri, Apr 24, 2015 at 4:26 PM, Mark Seaborn <mseaborn@chromium.org> wrote:
>>> I'm curious, what do you use the physical page addresses for?
>>>
>>> Since you pointed to http://undo-software.com, which talks about
>>> reversible debugging tools, I can guess you would use the soft-dirty
>>> flag to implement copy-on-write snapshotting. I'm guessing you might
>>> use physical page addresses for determining when the same page is
>>> mapped twice (in the same process or different processes)?
>>
>> That's pretty much it. Actually, we're effectively using the physical
>> addresses to emulate soft-dirty. For certain operations (e.g. some
>> system calls) we need to track what memory has changed since we last
>> looked at the process state. We have a mechanism that forks a child
>> process, runs the system call, then refers to pagemap to figure out
>> what's been modified.
>>
>> Currently, our mechanism compares the physical addresses of pages
>> before and after the syscall so that we can see which pages got CoWed.
>> This is perhaps a slightly "unconventional" use of the interface but
>> we support kernels that predate the soft-dirty mechanism and (as far
>> as we know) this is probably the best way we can answer "What got
>> changed?" on those releases.
>>
>> Using the soft-dirty mechanism where available should make our code
>> both cleaner and faster, so if we can fix the pagemap file to allow
>> that then we'll be quite happy!
>>
>> Cheers,
>> Mark
^ permalink raw reply
* Re: Regression: Requiring CAP_SYS_ADMIN for /proc/<pid>/pagemap causes application-level breakage
From: Kirill A. Shutemov @ 2015-04-29 19:36 UTC (permalink / raw)
To: Mark Williamson
Cc: Mark Seaborn, kernel list, Kirill A. Shutemov, Pavel Emelyanov,
Konstantin Khlebnikov, Andrew Morton, Linus Torvalds,
Andy Lutomirski, Linux API, Finn Grimwood, Daniel James
In-Reply-To: <CAEVpBa+DPbhZeRsA=+Jnn7f1BqVZm9SU=ABjy6bAHbbMi_yn+g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Wed, Apr 29, 2015 at 07:44:57PM +0100, Mark Williamson wrote:
> Hi all,
>
> We've been investigating further and found a snag with the PFN-hiding
> approach discussed last week - looks like it won't be enough on all
> the architectures we support. Our product runs on x86_32, x86_64 and
> ARM. For now, it looks like soft-dirty is only available on x86_64.
> A patch that simply zeros out the physical addresses in
> /proc/PID/pagemap will therefore help us on x86_64 but we'll still
> have problems on other platforms[1].
>
> For context, we were previously using pagemap as a cross-platform way
> to get soft-dirty-like functionality. Specifically, to ask "did a
> process write to any pages since fork()" by comparing addresses and
> deducing where CoW must have occurred. In the absence of soft-dirty
> and the physical addresses, it looks like we can't figure that out
> with the remaining information in pagemap.
>
> If the pagemap file included the "writeable" bit from the PTE, we
> think we'd have all the information required to deduce what we need
> (although I realise that's a bit of a nasty workaround). If I
> proposed including the PTE protection bits in pagemap, would that be
> controversial? I'm guessing yes but thought it was worth a shot ;-)
> Would anybody be able to suggest a more tasteful approach?
Emm.. I have hard time to understand how writable bit is enough to get
soft-dirty-alike functionality.
Let's say we have anon-mapping with COW setup after the fork(). It's not
writable PTEs to trigger COW on wp faults. But you can easily get to the
same non-writable PTE after breaking COW: fork() again or
mprotect(PROT_READ) and mprotect(PROT_READ|PROT_WRITE) back.
?
>
> Thanks,
> Mark
>
> [1] I'd note that using soft-dirty is clearly the right approach for
> us on x64, where available and that ideally we'd use it on other
> architectures - cross-arch support for soft-dirty is a slightly
> different discussion, which I hope to post another thread for.
--
Kirill A. Shutemov
^ permalink raw reply
* Re: [PATCH] usb: core: add usb3 lpm sysfs
From: Greg KH @ 2015-04-29 19:57 UTC (permalink / raw)
To: Zhuang Jin Can
Cc: rafael.j.wysocki-ral2JQCrhuEAvxtiuMwx3w,
stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz,
dan.j.williams-ral2JQCrhuEAvxtiuMwx3w, pmladek-AlSwsSmVLrQ,
peter.chen-KZfg59tc24xl57MIdRCFDg, jwerner-F7+t8E8rja9g9hUCZPvPmw,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150429162132.GA21141-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
On Thu, Apr 30, 2015 at 12:21:32AM +0800, Zhuang Jin Can wrote:
> On Wed, Apr 29, 2015 at 01:06:22PM +0200, Greg KH wrote:
> > On Wed, Apr 29, 2015 at 06:57:30PM +0800, Zhuang Jin Can wrote:
> > > On Wed, Apr 29, 2015 at 11:01:48AM +0200, Greg KH wrote:
> > > > On Wed, Apr 29, 2015 at 03:20:04PM +0800, Zhuang Jin Can wrote:
> > > > > On Tue, Apr 28, 2015 at 11:11:10PM +0200, Greg KH wrote:
> > > > > > On Wed, Apr 29, 2015 at 12:51:27AM +0800, Zhuang Jin Can wrote:
> > > > > > > Hi Greg KH,
> > > > > > >
> > > > > > > On Tue, Apr 28, 2015 at 12:42:24PM +0200, Greg KH wrote:
> > > > > > > > On Sun, Apr 19, 2015 at 11:46:12AM +0800, Zhuang Jin Can wrote:
> > > > > > > > > Some usb3 devices may not support usb3 lpm well.
> > > > > > > > > The patch adds a sysfs to enable/disable u1 or u2 of the port.The
> > > > > > > > > settings apply to both before and after device enumeration.
> > > > > > > > > Supported values are "0" - u1 and u2 are disabled, "u1" - only u1 is
> > > > > > > > > enabled, "u2" - only u2 is enabled, "u1_u2" - u1 and u2 are enabled.
> > > > > > > > >
> > > > > > > > > The interface is useful for testing some USB3 devices during
> > > > > > > > > development, and provides a way to disable usb3 lpm if the issues can
> > > > > > > > > not be fixed in final products.
> > > > > > > >
> > > > > > > > How is a user supposed to "know" to make this setting for a device? Why
> > > > > > > > can't the kernel automatically set this value properly? Why does it
> > > > > > > > need to be a kernel issue at all?
> > > > > > > >
> > > > > > > By default kernel enables u1 u2 of all USB3 devices. This interface
> > > > > > > provides the user to change this policy. User may set the policy
> > > > > > > according to PID/VID of uevent or according to the platform information
> > > > > > > known by userspace.
> > > > > >
> > > > > > And why would they ever want to do that?
> > > > > >
> > > > > > > It's not a kernel issue, as u1 u2 is mandatory by USB3 compliance. But
> > > > > > > for some internal hardwired USB3 connection, e.g. SSIC, passing USB3
> > > > > > > compliance is not mandatory. So the interface provides a way for vendor
> > > > > > > to ship with u1 or u2 broken products. Of course, this is not encouraged :).
> > > > > >
> > > > > > If the state is broken for those devices, we can't require the user to
> > > > > > fix it for us, the kernel should do it automatically.
> > > > > >
> > > > > > > > And when you are doing development of broken devices, the kernel doesn't
> > > > > > > > have to support you, you can run with debugging patches of your own
> > > > > > > > until you fix your firmware :)
> > > > > > > >
> > > > > > > Understood. But I think other vendor or developer may face the same
> > > > > > > issue in final product shipment or during development. Moreover, the
> > > > > > > interface provide the flexibility for developer to separately
> > > > > > > disable/enable u1 or u2, e.g. If they're debugging an u2 issue, they
> > > > > > > can disable u1 to simplify the situtation.
> > > > > >
> > > > > > For debugging only, perhaps, but for a "normal" user, please let's
> > > > > > handle this automatically and don't create a switch that never gets used
> > > > > > by anyone or anything.
> > > > > >
> > > > > Thanks Greg. Since so far the patch has no interesting value to the
> > > > > community, I'll drop the patch.
> > > >
> > > > I didn't say that, I said it needed some more work to be accepted.
> > > Sorry for misunderstanding. Let me explain more why we need this interface.
> > >
> > > We have a modem USB3 device (in stepping C) hardwired to one specific port of xHCI.
> > > The device was expected to work with u1 u2, however, due to a HW issue, it doesn't
> > > work stably. To workaround the issue, we let the init.rc script disable u1 u2 for
> > > this specific port.
> >
> > Modern Linux systems don't have init.rc scripts anymore :)
> >
> In Android, the init process still reads an init.rc where vendor can
> define their own policies. Vendors normally provides a whole reference
> design (including HW, FW, Kernel, BSP, AOSP) to OEMs. BSP contains
> vendor specific configurations including its own init.rc.
And that's generally not a good idea for companies to do, as they
shouldn't need special hardware workarounds in an init script, but I
understand :(
You are also going to be giving them a kernel patch that is not accepted
upstream which is really NOT the way to do things, and something that
many of us are working quite hard to keep from happening.
> > > Then maybe we want to start debug u1 issue first, to avoid hitting u2 issue,
> > > we can disable u2. After u1 issue is resolved, we can enable back u2 to continue to
> > > debug u2 issue. This provides the flexibility to isolate u1 u2 debugging.
> > > This is valuable I think :)
> >
> > I agree.
> >
> > > The HW issue will be fixed in stepping D, however C and D will have the same PID/VID.
> > > There's no way for kernel to know the difference between C and D.
> > > Even after fixing in D, C will still be used for development (to save money..)
> >
> > That sounds like a big design flaw, what about looking at the version of
> > the device? That's what that field in the USB descriptors is for.
> >
> You're right. bcdDevice should be used for this purpose.
Why can't it?
> However, since we need to live with what we have, we just used the init.rc to disable
> u1/u2. Definitely, it's a ugly hack in userspace to make it "automatically" work.
Why can't you put a quirk in the kernel for that bcdDevice value and
then not need any userspace hacks?
> A better solution is to use monitor uevent, reading bcdDevice + PID/VID, and define
> a rule in to disable u1/u2 of this device. Android provides an uevent machanism to do this.
It's a horrid uevent mechanism in that it duplicates what udev did years
ago :(
> However, how to do it automatically, it's out of the scope of the patch.
Not at all, what if you don't want to run Android on your hardware? You
still want it to work, so get the kernel fix upstream properly.
> Without the patch, the only choice is to add a quirk in usb core to do it automatically. And
> this should be in another separate patch.
Please send that patch.
> With the patch:
> 1. Userspace can also do the quirk with the help of uevent and rules
But it has no idea how or when to do that. Please don't provide hooks
that no one knows how to use.
> 2. Developer can isolate u1 u2 debugging.
That only developer seems to be you, and you've already debugged this :)
> And I don't think it's necessary for kernel to support this broken modem. Because, the modem
> is integrated with the SoC, and SoC goes with init.rc to OEMs. Thus, it doesn't make sense we
> add a quirk in kernel to long term support it. The SoC/Modem is going to be replaced by its next
> generation, especially in mobile area.
Again, someone wants to run a mainline kernel.org release on that
hardware, like they should. Some companies even are pushing to require
OEMs to have all of their changes upstream before they will buy their
chip, so please, make this a quirk and have it "just work" properly. No
need to rely on a magic init.rc value that no one notices or
understands.
thanks,
greg k-h
^ permalink raw reply
* Re: Regression: Requiring CAP_SYS_ADMIN for /proc/<pid>/pagemap causes application-level breakage
From: Mark Williamson @ 2015-04-29 20:24 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: Mark Seaborn, kernel list, Kirill A. Shutemov, Pavel Emelyanov,
Konstantin Khlebnikov, Andrew Morton, Linus Torvalds,
Andy Lutomirski, Linux API, Finn Grimwood, Daniel James
In-Reply-To: <20150429193622.GA11892-nhfs4B5ZimeFUdmeq17FyvUpdFzICT1y@public.gmane.org>
Hi,
On Wed, Apr 29, 2015 at 8:36 PM, Kirill A. Shutemov
<kirill-oKw7cIdHH8eLwutG50LtGA@public.gmane.org> wrote:
> On Wed, Apr 29, 2015 at 07:44:57PM +0100, Mark Williamson wrote:
>> Hi all,
... snip ...
>> For context, we were previously using pagemap as a cross-platform way
>> to get soft-dirty-like functionality. Specifically, to ask "did a
>> process write to any pages since fork()" by comparing addresses and
>> deducing where CoW must have occurred. In the absence of soft-dirty
>> and the physical addresses, it looks like we can't figure that out
>> with the remaining information in pagemap.
>>
>> If the pagemap file included the "writeable" bit from the PTE, we
>> think we'd have all the information required to deduce what we need
>> (although I realise that's a bit of a nasty workaround). If I
>> proposed including the PTE protection bits in pagemap, would that be
>> controversial? I'm guessing yes but thought it was worth a shot ;-)
>> Would anybody be able to suggest a more tasteful approach?
>
> Emm.. I have hard time to understand how writable bit is enough to get
> soft-dirty-alike functionality.
In the general case, you are of course correct - in our specific case
I *think* we'd be able to manage OK ... (see below).
> Let's say we have anon-mapping with COW setup after the fork(). It's not
> writable PTEs to trigger COW on wp faults. But you can easily get to the
> same non-writable PTE after breaking COW: fork() again or
> mprotect(PROT_READ) and mprotect(PROT_READ|PROT_WRITE) back.
I believe we'll be able to get away with this in our particular
usecase. The process is running in our debugger at the time and so we
can interpose on the system calls that are happening. That should
give us the opportunity to check for CoW-breaking before the debuggee
is allowed to alter page protections itself.
It ends up not being full soft-dirty behaviour but it's similar enough
to tell us what we need to know.
Cheers,
Mark
> ?
>
>>
>> Thanks,
>> Mark
>>
>> [1] I'd note that using soft-dirty is clearly the right approach for
>> us on x64, where available and that ideally we'd use it on other
>> architectures - cross-arch support for soft-dirty is a slightly
>> different discussion, which I hope to post another thread for.
>
> --
> Kirill A. Shutemov
^ permalink raw reply
* Re: Regression: Requiring CAP_SYS_ADMIN for /proc/<pid>/pagemap causes application-level breakage
From: Linus Torvalds @ 2015-04-29 20:33 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: Mark Williamson, Mark Seaborn, kernel list, Kirill A. Shutemov,
Pavel Emelyanov, Konstantin Khlebnikov, Andrew Morton,
Andy Lutomirski, Linux API, Finn Grimwood, Daniel James
In-Reply-To: <20150429193622.GA11892-nhfs4B5ZimeFUdmeq17FyvUpdFzICT1y@public.gmane.org>
On Wed, Apr 29, 2015 at 12:36 PM, Kirill A. Shutemov
<kirill-oKw7cIdHH8eLwutG50LtGA@public.gmane.org> wrote:
>
> Emm.. I have hard time to understand how writable bit is enough to get
> soft-dirty-alike functionality.
I don't think it is.
For anonymous pages, maybe you can play tricks with comparing the page
'anon_vma' with the vma->anon_vma.
I haven't really thought that through, but does something like
static inline bool page_is_dirty_in_vma(struct page *page, struct
vm_area_struct *vma)
{
struct anon_vma *anon_vma = vma->anon_vma;
return page->mapping == (void *)anon_vma + PAGE_MAPPING_ANON;
}
end up working as a "page has been dirtied in this mapping"?
If the page came from another process and hasn't been written to, it
will have the anon_vma pointing to the originalting vma.
I may be high on some bad drugs, though. As mentioned, I didn't really
think this through.
Linus
^ permalink raw reply
* Re: Regression: Requiring CAP_SYS_ADMIN for /proc/<pid>/pagemap causes application-level breakage
From: Konstantin Khlebnikov @ 2015-04-29 20:44 UTC (permalink / raw)
To: Linus Torvalds
Cc: Kirill A. Shutemov, Mark Williamson, Mark Seaborn, kernel list,
Kirill A. Shutemov, Pavel Emelyanov, Andrew Morton,
Andy Lutomirski, Linux API, Finn Grimwood, Daniel James
In-Reply-To: <CA+55aFx67hC3RxvAXYXWp=6mN_cfyMVqh_TDJVEqiV17o8+PtA@mail.gmail.com>
On Wed, Apr 29, 2015 at 11:33 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Wed, Apr 29, 2015 at 12:36 PM, Kirill A. Shutemov
> <kirill@shutemov.name> wrote:
>>
>> Emm.. I have hard time to understand how writable bit is enough to get
>> soft-dirty-alike functionality.
>
> I don't think it is.
>
> For anonymous pages, maybe you can play tricks with comparing the page
> 'anon_vma' with the vma->anon_vma.
>
> I haven't really thought that through, but does something like
>
> static inline bool page_is_dirty_in_vma(struct page *page, struct
> vm_area_struct *vma)
> {
> struct anon_vma *anon_vma = vma->anon_vma;
>
> return page->mapping == (void *)anon_vma + PAGE_MAPPING_ANON;
> }
>
> end up working as a "page has been dirtied in this mapping"?
This's no longer true. After recent fixes for "anon_vma endless growing" new vma
might reuse old anon_vma from grandparent vma.
>
> If the page came from another process and hasn't been written to, it
> will have the anon_vma pointing to the originalting vma.
>
> I may be high on some bad drugs, though. As mentioned, I didn't really
> think this through.
>
> Linus
^ permalink raw reply
* Re: Regression: Requiring CAP_SYS_ADMIN for /proc/<pid>/pagemap causes application-level breakage
From: Linus Torvalds @ 2015-04-29 21:02 UTC (permalink / raw)
To: Konstantin Khlebnikov
Cc: Kirill A. Shutemov, Mark Williamson, Mark Seaborn, kernel list,
Kirill A. Shutemov, Pavel Emelyanov, Andrew Morton,
Andy Lutomirski, Linux API, Finn Grimwood, Daniel James
In-Reply-To: <CALYGNiN6kU5xgEAfbs4VVXsVKmMN+uCStrQoLh6gFCzBxJfy+A@mail.gmail.com>
On Wed, Apr 29, 2015 at 1:44 PM, Konstantin Khlebnikov <koct9i@gmail.com> wrote:
>
> This's no longer true. After recent fixes for "anon_vma endless growing" new vma
> might reuse old anon_vma from grandparent vma.
Oh well. I guess that was too simple.
If Mark is ok with the rule that "it's not reliably if you have two
nested forks" (ie it only works if you exec for every fork you do), it
should still work, right? It sounds like Mark doesn't necessarily need
to handle the *generic* case.
Linus
^ permalink raw reply
* Re: Regression: Requiring CAP_SYS_ADMIN for /proc/<pid>/pagemap causes application-level breakage
From: Kirill A. Shutemov @ 2015-04-29 21:05 UTC (permalink / raw)
To: Linus Torvalds
Cc: Konstantin Khlebnikov, Mark Williamson, Mark Seaborn, kernel list,
Kirill A. Shutemov, Pavel Emelyanov, Andrew Morton,
Andy Lutomirski, Linux API, Finn Grimwood, Daniel James
In-Reply-To: <CA+55aFzW52u-LahoBA5kDs47rgeTGU_KF_GUrx+XSUDzeQD3=g@mail.gmail.com>
On Wed, Apr 29, 2015 at 02:02:01PM -0700, Linus Torvalds wrote:
> On Wed, Apr 29, 2015 at 1:44 PM, Konstantin Khlebnikov <koct9i-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >
> > This's no longer true. After recent fixes for "anon_vma endless growing" new vma
> > might reuse old anon_vma from grandparent vma.
>
> Oh well. I guess that was too simple.
>
> If Mark is ok with the rule that "it's not reliably if you have two
> nested forks" (ie it only works if you exec for every fork you do), it
> should still work, right? It sounds like Mark doesn't necessarily need
> to handle the *generic* case.
This sounds too ugly to be exposed it as ABI.
--
Kirill A. Shutemov
^ permalink raw reply
* Re: Regression: Requiring CAP_SYS_ADMIN for /proc/<pid>/pagemap causes application-level breakage
From: Linus Torvalds @ 2015-04-29 21:18 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: Konstantin Khlebnikov, Mark Williamson, Mark Seaborn, kernel list,
Kirill A. Shutemov, Pavel Emelyanov, Andrew Morton,
Andy Lutomirski, Linux API, Finn Grimwood, Daniel James
In-Reply-To: <20150429210513.GA12431-nhfs4B5ZimeFUdmeq17FyvUpdFzICT1y@public.gmane.org>
On Wed, Apr 29, 2015 at 2:05 PM, Kirill A. Shutemov
<kirill-oKw7cIdHH8eLwutG50LtGA@public.gmane.org> wrote:
>
> This sounds too ugly to be exposed it as ABI.
Oh, pretty it ain't. However, regressions in many ways are worse. If
it makes it possible to not regress...
Linus
^ permalink raw reply
* Re: Regression: Requiring CAP_SYS_ADMIN for /proc/<pid>/pagemap causes application-level breakage
From: Kirill A. Shutemov @ 2015-04-29 21:37 UTC (permalink / raw)
To: Linus Torvalds
Cc: Konstantin Khlebnikov, Mark Williamson, Mark Seaborn, kernel list,
Kirill A. Shutemov, Pavel Emelyanov, Andrew Morton,
Andy Lutomirski, Linux API, Finn Grimwood, Daniel James,
Cyrill Gorcunov
In-Reply-To: <CA+55aFw-dK4U3Z_gi5wEdGsPz0ZaQMizETKqS-=ajpDAu03Fnw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Wed, Apr 29, 2015 at 02:18:49PM -0700, Linus Torvalds wrote:
> On Wed, Apr 29, 2015 at 2:05 PM, Kirill A. Shutemov
> <kirill-oKw7cIdHH8eLwutG50LtGA@public.gmane.org> wrote:
> >
> > This sounds too ugly to be exposed it as ABI.
>
> Oh, pretty it ain't. However, regressions in many ways are worse. If
> it makes it possible to not regress...
One idea is to extend kcmp(2) with KCMP_PAGE. idx1 and idx2 are virtual
addresses in two processes. It returns 0 if addresses points to the same
page and 3 otherwise.
Would it be enough for the use case?
I guess it could be too slow to check one page a time...
Invent new kcmpv(2)? ;)
--
Kirill A. Shutemov
^ permalink raw reply
* Re: [RFC PATCH] mmap.2: clarify MAP_LOCKED semantic (was: Re: Should mmap MAP_LOCKED fail if mm_poppulate fails?)
From: David Rientjes @ 2015-04-30 0:28 UTC (permalink / raw)
To: Michal Hocko
Cc: Linus Torvalds, Michael Kerrisk, linux-mm, Cyril Hrubis,
Andrew Morton, Hugh Dickins, Michel Lespinasse, Rik van Riel,
LKML, Linux API
In-Reply-To: <20150429113818.GC16097@dhcp22.suse.cz>
On Wed, 29 Apr 2015, Michal Hocko wrote:
> MAP_LOCKED had a subtly different semantic from mmap(2)+mlock(2) since
> it has been introduced.
> mlock(2) fails if the memory range cannot get populated to guarantee
> that no future major faults will happen on the range. mmap(MAP_LOCKED) on
> the other hand silently succeeds even if the range was populated only
> partially.
>
> Fixing this subtle difference in the kernel is rather awkward because
> the memory population happens after mm locks have been dropped and so
> the cleanup before returning failure (munlock) could operate on something
> else than the originally mapped area.
>
> E.g. speculative userspace page fault handler catching SEGV and doing
> mmap(fault_addr, MAP_FIXED|MAP_LOCKED) might discard portion of a racing
> mmap and lead to lost data. Although it is not clear whether such a
> usage would be valid, mmap page doesn't explicitly describe requirements
> for threaded applications so we cannot exclude this possibility.
>
> This patch makes the semantic of MAP_LOCKED explicit and suggest using
> mmap + mlock as the only way to guarantee no later major page faults.
>
> Signed-off-by: Michal Hocko <mhocko@suse.cz>
> ---
> man2/mmap.2 | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/man2/mmap.2 b/man2/mmap.2
> index 54d68cf87e9e..1486be2e96b3 100644
> --- a/man2/mmap.2
> +++ b/man2/mmap.2
> @@ -235,8 +235,19 @@ See the Linux kernel source file
> for further information.
> .TP
> .BR MAP_LOCKED " (since Linux 2.5.37)"
> -Lock the pages of the mapped region into memory in the manner of
> +Mark the mmaped region to be locked in the same way as
> .BR mlock (2).
> +This implementation will try to populate (prefault) the whole range but
> +the mmap call doesn't fail with
> +.B ENOMEM
> +if this fails. Therefore major faults might happen later on. So the semantic
> +is not as strong as
> +.BR mlock (2).
> +.BR mmap (2)
> ++
> +.BR mlock (2)
> +should be used when major faults are not acceptable after the initialization
> +of the mapping.
> This flag is ignored in older kernels.
> .\" If set, the mapped pages will not be swapped out.
> .TP
The wording of this begs the question on the behavior of
MAP_LOCKED | MAP_POPULATE since this same man page specifies that
accesses to memory mapped with MAP_POPULATE will not block on page faults
later.
I think Documentation/vm/unevictable-lru.txt would benefit from an update
under the mmap(MAP_LOCKED) section where all this can be laid out and
perhaps reference it from the man page?
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH v3 3/3] proc: add kpageidle file
From: Minchan Kim @ 2015-04-30 6:55 UTC (permalink / raw)
To: Vladimir Davydov
Cc: Andrew Morton, Johannes Weiner, Michal Hocko, Greg Thelen,
Michel Lespinasse, David Rientjes, Pavel Emelyanov,
Cyrill Gorcunov, Jonathan Corbet,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
cgroups-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150429083148.GA11497@esperanza>
Hi,
On Wed, Apr 29, 2015 at 11:31:49AM +0300, Vladimir Davydov wrote:
> On Wed, Apr 29, 2015 at 01:57:59PM +0900, Minchan Kim wrote:
> > On Tue, Apr 28, 2015 at 03:24:42PM +0300, Vladimir Davydov wrote:
> > > @@ -69,6 +69,14 @@ There are four components to pagemap:
> > > memory cgroup each page is charged to, indexed by PFN. Only available when
> > > CONFIG_MEMCG is set.
> > >
> > > + * /proc/kpageidle. For each page this file contains a 64-bit number, which
> > > + equals 1 if the page is idle or 0 otherwise, indexed by PFN. A page is
> > > + considered idle if it has not been accessed since it was marked idle. To
> > > + mark a page idle one should write 1 to this file at the offset corresponding
> > > + to the page. Only user memory pages can be marked idle, for other page types
> > > + input is silently ignored. Writing to this file beyond max PFN results in
> > > + the ENXIO error. Only available when CONFIG_IDLE_PAGE_TRACKING is set.
> > > +
> >
> > How about using kpageflags for reading part?
> >
> > I mean PG_idle is one of the page flags and we already have a feature to
> > parse of each PFN flag so we could reuse existing feature for reading
> > idleness.
>
> Reading PG_idle implies clearing all pte references to make sure the
> page was not referenced via a pte. This means that exporting it via
> /proc/kpageflags would increase the cost of reading this file, even for
> users that don't care about PG_idle. I'm not sure all users of
> /proc/kpageflags will be fine with it.
It triggers rmap traverse so it would be horrible overhead sometime
so I agree every kpageflags users don't want it but I didn't mean
reading of PG_idle via kpageflags should clear all pte references.
Reset should be still part of kpageidle but we can just read idlenss
without reset by kpageflags(IOW, Reset and reading is orthogoal)
A benefit via reading kpageflags, we could parse it's idle page
and not dirty page so we could reclaim it easy.
Anyway, it could be further improvement.
>
> Thanks,
> Vladimir
--
Kind regards,
Minchan Kim
^ permalink raw reply
* Re: [RFC v2 1/4] fs: Add generic file system event notifications
From: Beata Michalska @ 2015-04-30 8:21 UTC (permalink / raw)
To: Greg KH
Cc: Jan Kara, linux-kernel, linux-fsdevel, linux-api, tytso,
adilger.kernel, hughd, lczerner, hch, linux-ext4, linux-mm,
kyungmin.park, kmpark
In-Reply-To: <20150429155522.GA14723@kroah.com>
Hi,
On 04/29/2015 05:55 PM, Greg KH wrote:
> On Wed, Apr 29, 2015 at 05:48:14PM +0200, Beata Michalska wrote:
>> On 04/29/2015 03:45 PM, Greg KH wrote:
>>> On Wed, Apr 29, 2015 at 01:10:34PM +0200, Beata Michalska wrote:
>>>>>>> It needs to be done internally by the app but is doable.
>>>>>>> The app knows what it is watching, so it can maintain the mappings.
>>>>>>> So prior to activating the notifications it can call 'stat' on the mount point.
>>>>>>> Stat struct gives the 'st_dev' which is the device id. Same will be reported
>>>>>>> within the message payload (through major:minor numbers). So having this,
>>>>>>> the app is able to get any other information it needs.
>>>>>>> Note that the events refer to the file system as a whole and they may not
>>>>>>> necessarily have anything to do with the actual block device.
>>>>>
>>>>> How are you going to show an event for a filesystem that is made up of
>>>>> multiple block devices?
>>>>
>>>> AFAIK, for such filesystems there will be similar case with the anonymous
>>>> major:minor numbers - at least the btrfs is doing so. Not sure we can
>>>> differentiate here the actual block device. So in this case such events
>>>> serves merely as a hint for the userspace.
>>>
>>> "hint" seems like this isn't really going to work well.
>>>
>>> Do you have userspace code that can properly map this back to the "real"
>>> device that is causing problems? Without that, this doesn't seem all
>>> that useful as no one would be able to use those events.
>>
>> I'm not sure we are on the same page here.
>> This is about watching the file system rather than the 'real' device.
>> Like the threshold notifications: you would like to know when you
>> will be approaching certain level of available space for the tmpfs
>> mounted on /tmp. You do know you are watching the /tmp
>> and you know that the dev numbers for this are 0:20 (or so).
>> (either through calling stat on /tmp or through reading the /proc/$$/mountinfo)
>> With this interface you can setup threshold levels
>> for /tmp. Then, once the limit is reached the event will be
>> sent with those anonymous major:minor numbers.
>>
>> I can provide a sample code which will demonstrate how this
>> can be achieved.
>
> Yes, example code would be helpful to understand this, thanks.
>
> greg k-h
>
Below is an absolutely *simplified* sample application.
Hope this will be helpful.
---------------
#include <netlink/cli/utils.h>
#include <fs_event.h>
#include <string.h>
#include <regex.h>
#define ARRAY_SIZE(x) (sizeof(x)/sizeof((x)[0]))
#define LOG(args...) fprintf(stderr, args)
#define BUFF_SIZE 256
struct list_node {
struct list_node *next;
struct list_node *prev;
};
#define MBITS 20
#define MAKE_DEV(major, minor) \
((major) << MBITS | ((minor) & ((1U << MBITS) -1)))
struct mount_data {
struct list_node link;
dev_t dev;
char *dname;
};
static struct list_node mount_list = {&mount_list, &mount_list};
static void list_add(struct list_node *new_node, struct list_node *head)
{
struct list_node *node;
node = head->next;
head->next = new_node;
new_node->prev = head;
new_node->next = node;
node->prev = new_node;
}
static struct mount_data *find_mount(struct list_node *mlist, dev_t dev)
{
struct list_node *node;
struct mount_data *mdata;
for (node = mlist->prev; node != mlist; node = node->prev) {
mdata = (char*)node - ((size_t) &((struct mount_data*)0)->link);
if (mdata->dev == dev)
return mdata;
}
return NULL;
}
static void create_mount_base(struct list_node *mlist)
{
FILE *f;
char entry[BUFF_SIZE];
regex_t re;
if (!(f = fopen("/proc/self/mountinfo", "r")))
return;
if (regcomp(&re, "[0-9]*:[0-9]*", REG_EXTENDED))
goto leave;
while (fgets(entry, BUFF_SIZE, f)) {
regmatch_t pmatch;
int dev_major, dev_minor;
char *s;
if (regexec(&re, entry, 1, &pmatch, 0))
continue;
if (pmatch.rm_so == -1)
continue;
sscanf(entry + pmatch.rm_so, "%d:%d",
&dev_major, &dev_minor);
s = entry + pmatch.rm_eo;
s = strtok(++s, " ");
if (!s)
continue;
if (s = strtok(NULL, " ")) {
struct mount_data *data = malloc(sizeof(*data));
if (!data)
continue;
data->dev = MAKE_DEV(dev_major, dev_minor);
data->dname = strdup(s);
list_add(&data->link, mlist);
}
}
regfree(&re);
leave:
close(f);
return;
}
static int parse_event(struct nl_cache_ops *unused, struct genl_cmd *cmd,
struct genl_info *info, void *arg)
{
struct mount_data *mdata;
int dev_major, dev_minor;
dev_major = info->attrs[FS_NL_A_DEV_MAJOR]
? nla_get_u32(info->attrs[FS_NL_A_DEV_MAJOR])
: 0;
dev_minor = info->attrs[FS_NL_A_DEV_MINOR]
? nla_get_u32(info->attrs[FS_NL_A_DEV_MINOR])
: 0;
mdata = find_mount(&mount_list, MAKE_DEV(dev_major, dev_minor));
if (!mdata) {
LOG("Unable to identify file system\n");
return 0;
}
LOG("Notification received for %s \n", mdata->dname);
LOG("Event ID: %d\n", nla_get_u32(info->attrs[FS_NL_A_EVENT_ID]));
LOG("Owner: %d\n", nla_get_u32(info->attrs[FS_NL_A_CAUSED_ID]));
LOG("Threshold data: %llu\n", info->attrs[FS_NL_A_DATA]
? nla_get_u64(info->attrs[FS_NL_A_DATA])
: 0);
return 0;
}
static struct genl_cmd cmd[] = {
{
.c_id = 1 ,
.c_name = "event",
.c_maxattr = 5,
.c_msg_parser = parse_event,
},
};
static struct genl_ops ops = {
.o_id = GENL_ID_FS_EVENT,
.o_name = "FS_EVENT",
.o_hdrsize = 0,
.o_cmds = cmd,
.o_ncmds = ARRAY_SIZE(cmd),
};
int events_cb(struct nl_msg *msg, void *arg)
{
return genl_handle_msg(msg, arg);
}
int main(int argc, char **argv)
{
struct nl_sock *sock;
int ret;
create_mount_base(&mount_list);
sock = nl_cli_alloc_socket();
nl_socket_set_local_port(sock, 0);
nl_socket_disable_seq_check(sock);
nl_socket_modify_cb(sock, NL_CB_VALID, NL_CB_CUSTOM, events_cb, NULL);
nl_cli_connect(sock, NETLINK_GENERIC);
if ((ret = nl_socket_add_membership(sock, GENL_ID_FS_EVENT))) {
LOG("Failed to add membership\n");
goto leave;
}
if((ret = genl_register_family(&ops))) {
LOG("Failed to register protocol family\n");
goto leave;
}
if ((ret = genl_ops_resolve(sock, &ops) < 0)) {
LOG("Unable to resolve the family name\n");
goto leave;
}
if (genl_ctrl_resolve(sock, "FS_EVENT") < 0) {
LOG("Failed to resolve the family name\n");
goto leave;
}
while (1) {
if ((ret = nl_recvmsgs_default(sock)) < 0)
LOG("Unable to receive message: %s\n",
nl_geterror(ret));
}
leave:
nl_close(sock);
nl_socket_free(sock);
return 0;
}
----------------------------
The configuration setup for the app:
# echo /tmp T 50000 10000 > /sys/fs/events/config;
# echo /opt/usr G T 710000 500000 > /sys/fs/events/config;
(tmpfs and ext4 as the support for those is part of the patchset)
And the output after playing around with the 'dd':
Notification received for /tmp
Event ID: 3 /* FS_THR_LRBELOW */
Owner: 3128
Threshold data: 50000
Notification received for /opt/usr
Event ID: 3 /* FS_THR_LRBELOW */
Owner: 3127
Threshold data: 710000
Notification received for /tmp
Event ID: 5 /* FS_THR_URBELOW */
Owner: 3128
Threshold data: 10000
Notification received for /opt/usr
Event ID: 5 /* FS_THR_URBELOW */
Owner: 3127
Threshold data: 500000
Notification received for /opt/usr
Event ID: 1 /* FS_WARN_ENOSPC */
Owner: 3127
Threshold data: 0
Notification received for /opt/usr
Event ID: 1 /* FS_WARN_ENOSPC */
Owner: 3127
Threshold data: 0
-------------------------
BR
Beata
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH v3 3/3] proc: add kpageidle file
From: Minchan Kim @ 2015-04-30 8:25 UTC (permalink / raw)
To: Vladimir Davydov
Cc: Andrew Morton, Johannes Weiner, Michal Hocko, Greg Thelen,
Michel Lespinasse, David Rientjes, Pavel Emelyanov,
Cyrill Gorcunov, Jonathan Corbet, linux-api, linux-doc, linux-mm,
cgroups, linux-kernel
In-Reply-To: <20150429091248.GD1694@esperanza>
On Wed, Apr 29, 2015 at 12:12:48PM +0300, Vladimir Davydov wrote:
> On Wed, Apr 29, 2015 at 01:35:36PM +0900, Minchan Kim wrote:
> > On Tue, Apr 28, 2015 at 03:24:42PM +0300, Vladimir Davydov wrote:
> > > diff --git a/fs/proc/page.c b/fs/proc/page.c
> > > index 70d23245dd43..cfc55ba7fee6 100644
> > > --- a/fs/proc/page.c
> > > +++ b/fs/proc/page.c
> > > @@ -275,6 +275,156 @@ static const struct file_operations proc_kpagecgroup_operations = {
> > > };
> > > #endif /* CONFIG_MEMCG */
> > >
> > > +#ifdef CONFIG_IDLE_PAGE_TRACKING
> > > +static struct page *kpageidle_get_page(unsigned long pfn)
> > > +{
> > > + struct page *page;
> > > +
> > > + if (!pfn_valid(pfn))
> > > + return NULL;
> > > + page = pfn_to_page(pfn);
> > > + /*
> > > + * We are only interested in user memory pages, i.e. pages that are
> > > + * allocated and on an LRU list.
> > > + */
> > > + if (!page || page_count(page) == 0 || !PageLRU(page))
> >
> > Why do you check (page_count == 0) even if we check it with get_page_unless_zero
> > below?
>
> I intended to avoid overhead of cmpxchg in case page_count is 0, but
> diving deeper into get_page_unless_zero, I see that it already handles
> such a scenario, so this check is useless. I'll remove it.
>
> >
> > > + return NULL;
> > > + if (!get_page_unless_zero(page))
> > > + return NULL;
> > > + if (unlikely(!PageLRU(page))) {
> >
> > What lock protect the check PageLRU?
> > If it is racing ClearPageLRU, what happens?
>
> If we hold a reference to a page and see that it's on an LRU list, it
> will surely remain a user memory page at least until we release the
> reference to it, so it must be safe to play with idle/young flags. If we
The problem is that you pass the page in rmap reverse logic(ie, page_referenced)
once you judge it's LRU page so if it is false-positive, what happens?
A question is SetPageLRU, PageLRU, ClearPageLRU keeps memory ordering?
IOW, all of fields from struct page rmap can acccess should be set up completely
before LRU checking. Otherwise, something will be broken.
Thanks.
> race with isolate_lru_page, or any similar function temporarily clearing
> PG_lru, we will silently skip the page w/o touching its idle/young
> flags. We could consider isolated pages too, but that would increase the
> cost of this function.
>
> If you find this explanation OK, I'll add it to the comment to this
> function.
>
> >
> > > + put_page(page);
> > > + return NULL;
> > > + }
> > > + return page;
> > > +}
> > > +
> > > +static void kpageidle_clear_refs(struct page *page)
> > > +{
> > > + unsigned long dummy;
> > > +
> > > + if (page_referenced(page, 0, NULL, &dummy))
> > > + /*
> > > + * This page was referenced. To avoid interference with the
> > > + * reclaimer, mark it young so that the next call will also
> >
> > next what call?
> >
> > It just works with mapped page so kpageidle_clear_pte_refs as function name
> > is more clear.
> >
> > One more, kpageidle_clear_refs removes PG_idle via page_referenced which
> > is important feature for the function. Please document it so we could
> > understand why we need double check for PG_idle after calling
> > kpageidle_clear_refs for pte access bit.
>
> Sounds reasonable, will do.
>
> > > diff --git a/mm/rmap.c b/mm/rmap.c
> > > index 24dd3f9fee27..12e73b758d9e 100644
> > > --- a/mm/rmap.c
> > > +++ b/mm/rmap.c
> > > @@ -784,6 +784,13 @@ static int page_referenced_one(struct page *page, struct vm_area_struct *vma,
> > > if (referenced) {
> > > pra->referenced++;
> > > pra->vm_flags |= vma->vm_flags;
> > > + if (page_is_idle(page))
> > > + clear_page_idle(page);
> > > + }
> > > +
> > > + if (page_is_young(page)) {
> > > + clear_page_young(page);
> > > + pra->referenced++;
> >
> > If a page was page_is_young and referenced recenlty,
> > pra->referenced is increased doubly and it changes current
> > behavior for file-backed page promotion. Look at page_check_references.
>
> Yeah, you're quite right, I missed that. Something like this should get
> rid of this extra reference:
>
> diff --git a/mm/rmap.c b/mm/rmap.c
> index 24dd3f9fee27..eca7416f55d7 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -781,6 +781,14 @@ static int page_referenced_one(struct page *page, struct vm_area_struct *vma,
> pte_unmap_unlock(pte, ptl);
> }
>
> + if (referenced && page_is_idle(page))
> + clear_page_idle(page);
> +
> + if (page_is_young(page)) {
> + clear_page_young(page);
> + referenced++;
> + }
> +
> if (referenced) {
> pra->referenced++;
> pra->vm_flags |= vma->vm_flags;
>
> Thanks,
> Vladimir
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
--
Kind regards,
Minchan Kim
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] usb: core: add usb3 lpm sysfs
From: Zhuang Jin Can @ 2015-04-30 8:49 UTC (permalink / raw)
To: Greg KH
Cc: rafael.j.wysocki-ral2JQCrhuEAvxtiuMwx3w,
stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz,
dan.j.williams-ral2JQCrhuEAvxtiuMwx3w, pmladek-AlSwsSmVLrQ,
peter.chen-KZfg59tc24xl57MIdRCFDg, jwerner-F7+t8E8rja9g9hUCZPvPmw,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150429195733.GA19183-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
On Wed, Apr 29, 2015 at 09:57:33PM +0200, Greg KH wrote:
> On Thu, Apr 30, 2015 at 12:21:32AM +0800, Zhuang Jin Can wrote:
> > On Wed, Apr 29, 2015 at 01:06:22PM +0200, Greg KH wrote:
> > > On Wed, Apr 29, 2015 at 06:57:30PM +0800, Zhuang Jin Can wrote:
> > > > On Wed, Apr 29, 2015 at 11:01:48AM +0200, Greg KH wrote:
> > > > > On Wed, Apr 29, 2015 at 03:20:04PM +0800, Zhuang Jin Can wrote:
> > > > > > On Tue, Apr 28, 2015 at 11:11:10PM +0200, Greg KH wrote:
> > > > > > > On Wed, Apr 29, 2015 at 12:51:27AM +0800, Zhuang Jin Can wrote:
> > > > > > > > Hi Greg KH,
> > > > > > > >
> > > > > > > > On Tue, Apr 28, 2015 at 12:42:24PM +0200, Greg KH wrote:
> > > > > > > > > On Sun, Apr 19, 2015 at 11:46:12AM +0800, Zhuang Jin Can wrote:
> > > > > > > > > > Some usb3 devices may not support usb3 lpm well.
> > > > > > > > > > The patch adds a sysfs to enable/disable u1 or u2 of the port.The
> > > > > > > > > > settings apply to both before and after device enumeration.
> > > > > > > > > > Supported values are "0" - u1 and u2 are disabled, "u1" - only u1 is
> > > > > > > > > > enabled, "u2" - only u2 is enabled, "u1_u2" - u1 and u2 are enabled.
> > > > > > > > > >
> > > > > > > > > > The interface is useful for testing some USB3 devices during
> > > > > > > > > > development, and provides a way to disable usb3 lpm if the issues can
> > > > > > > > > > not be fixed in final products.
> > > > > > > > >
> > > > > > > > > How is a user supposed to "know" to make this setting for a device? Why
> > > > > > > > > can't the kernel automatically set this value properly? Why does it
> > > > > > > > > need to be a kernel issue at all?
> > > > > > > > >
> > > > > > > > By default kernel enables u1 u2 of all USB3 devices. This interface
> > > > > > > > provides the user to change this policy. User may set the policy
> > > > > > > > according to PID/VID of uevent or according to the platform information
> > > > > > > > known by userspace.
> > > > > > >
> > > > > > > And why would they ever want to do that?
> > > > > > >
> > > > > > > > It's not a kernel issue, as u1 u2 is mandatory by USB3 compliance. But
> > > > > > > > for some internal hardwired USB3 connection, e.g. SSIC, passing USB3
> > > > > > > > compliance is not mandatory. So the interface provides a way for vendor
> > > > > > > > to ship with u1 or u2 broken products. Of course, this is not encouraged :).
> > > > > > >
> > > > > > > If the state is broken for those devices, we can't require the user to
> > > > > > > fix it for us, the kernel should do it automatically.
> > > > > > >
> > > > > > > > > And when you are doing development of broken devices, the kernel doesn't
> > > > > > > > > have to support you, you can run with debugging patches of your own
> > > > > > > > > until you fix your firmware :)
> > > > > > > > >
> > > > > > > > Understood. But I think other vendor or developer may face the same
> > > > > > > > issue in final product shipment or during development. Moreover, the
> > > > > > > > interface provide the flexibility for developer to separately
> > > > > > > > disable/enable u1 or u2, e.g. If they're debugging an u2 issue, they
> > > > > > > > can disable u1 to simplify the situtation.
> > > > > > >
> > > > > > > For debugging only, perhaps, but for a "normal" user, please let's
> > > > > > > handle this automatically and don't create a switch that never gets used
> > > > > > > by anyone or anything.
> > > > > > >
> > > > > > Thanks Greg. Since so far the patch has no interesting value to the
> > > > > > community, I'll drop the patch.
> > > > >
> > > > > I didn't say that, I said it needed some more work to be accepted.
> > > > Sorry for misunderstanding. Let me explain more why we need this interface.
> > > >
> > > > We have a modem USB3 device (in stepping C) hardwired to one specific port of xHCI.
> > > > The device was expected to work with u1 u2, however, due to a HW issue, it doesn't
> > > > work stably. To workaround the issue, we let the init.rc script disable u1 u2 for
> > > > this specific port.
> > >
> > > Modern Linux systems don't have init.rc scripts anymore :)
> > >
> > In Android, the init process still reads an init.rc where vendor can
> > define their own policies. Vendors normally provides a whole reference
> > design (including HW, FW, Kernel, BSP, AOSP) to OEMs. BSP contains
> > vendor specific configurations including its own init.rc.
>
> And that's generally not a good idea for companies to do, as they
> shouldn't need special hardware workarounds in an init script, but I
> understand :(
>
> You are also going to be giving them a kernel patch that is not accepted
> upstream which is really NOT the way to do things, and something that
> many of us are working quite hard to keep from happening.
>
> > > > Then maybe we want to start debug u1 issue first, to avoid hitting u2 issue,
> > > > we can disable u2. After u1 issue is resolved, we can enable back u2 to continue to
> > > > debug u2 issue. This provides the flexibility to isolate u1 u2 debugging.
> > > > This is valuable I think :)
> > >
> > > I agree.
> > >
> > > > The HW issue will be fixed in stepping D, however C and D will have the same PID/VID.
> > > > There's no way for kernel to know the difference between C and D.
> > > > Even after fixing in D, C will still be used for development (to save money..)
> > >
> > > That sounds like a big design flaw, what about looking at the version of
> > > the device? That's what that field in the USB descriptors is for.
> > >
> > You're right. bcdDevice should be used for this purpose.
>
> Why can't it?
>
> > However, since we need to live with what we have, we just used the init.rc to disable
> > u1/u2. Definitely, it's a ugly hack in userspace to make it "automatically" work.
>
> Why can't you put a quirk in the kernel for that bcdDevice value and
> then not need any userspace hacks?
>
Let me double confirm with our modem HW engineers to see if anything we
can differentiate the modem variants, and add the quirks in kernel
accordingly.
> > A better solution is to use monitor uevent, reading bcdDevice + PID/VID, and define
> > a rule in to disable u1/u2 of this device. Android provides an uevent machanism to do this.
>
> It's a horrid uevent mechanism in that it duplicates what udev did years
> ago :(
>
OK. I think I got your point now: kernel should handle broken devices automatically without
userspace's attention.
> > However, how to do it automatically, it's out of the scope of the patch.
>
> Not at all, what if you don't want to run Android on your hardware? You
> still want it to work, so get the kernel fix upstream properly.
>
Got it.
> > Without the patch, the only choice is to add a quirk in usb core to do it automatically. And
> > this should be in another separate patch.
>
> Please send that patch.
>
Sure. Will need some time before sending the patch.
> > With the patch:
> > 1. Userspace can also do the quirk with the help of uevent and rules
>
> But it has no idea how or when to do that. Please don't provide hooks
> that no one knows how to use.
>
OK.
> > 2. Developer can isolate u1 u2 debugging.
>
> That only developer seems to be you, and you've already debugged this :)
>
Do you think the interface has no much value to other developers, and I should
remove it?
> > And I don't think it's necessary for kernel to support this broken modem. Because, the modem
> > is integrated with the SoC, and SoC goes with init.rc to OEMs. Thus, it doesn't make sense we
> > add a quirk in kernel to long term support it. The SoC/Modem is going to be replaced by its next
> > generation, especially in mobile area.
>
> Again, someone wants to run a mainline kernel.org release on that
> hardware, like they should. Some companies even are pushing to require
> OEMs to have all of their changes upstream before they will buy their
> chip, so please, make this a quirk and have it "just work" properly. No
> need to rely on a magic init.rc value that no one notices or
> understands.
>
Got it. Thanks your explainations Greg.
Regards
Jincan
^ permalink raw reply
* Re: [PATCH 2/3] uffd: Introduce the v2 API
From: Pavel Emelyanov @ 2015-04-30 9:50 UTC (permalink / raw)
To: Andrea Arcangeli
Cc: Linux Kernel Mailing List, Linux MM, Linux API, Sanidhya Kashyap
In-Reply-To: <20150427211236.GB24035@redhat.com>
On 04/28/2015 12:12 AM, Andrea Arcangeli wrote:
> Hello,
>
> On Thu, Apr 23, 2015 at 09:29:07AM +0300, Pavel Emelyanov wrote:
>> So your proposal is to always report 16 bytes per PF from read() and
>> let userspace decide itself how to handle the result?
>
> Reading 16bytes for each userfault (instead of 8) and sharing the same
> read(2) protocol (UFFD_API) for both the cooperative and
> non-cooperative usages, is something I just suggested for
> consideration after reading your patchset.
>
> The pros of using a single protocol for both is that it would reduce
> amount of code and there would be just one file operation for the
> .read method. The cons is that it will waste 8 bytes per userfault in
> terms of memory footprint. The other major cons is that it would force
> us to define the format of the non cooperative protocol now despite it's
> not fully finished yet.
>
> I'm also ok with two protocols if nobody else objects, but if we use
> two protocols, we should at least use different file operation methods
> and use __always_inline with constants passed as parameter to optimize
> away the branches at build time. This way we get the reduced memory
> footprint in the read syscall without other runtime overhead
> associated with it.
OK. I would go with two protocols then and will reshuffle the code to
use two ops.
>>>> +struct uffd_v2_msg {
>>>> + __u64 type;
>>>> + __u64 arg;
>>>> +};
>>>> +
>>>> +#define UFFD_PAGEFAULT 0x1
>>>> +
>>>> +#define UFFD_PAGEFAULT_BIT (1 << (UFFD_PAGEFAULT - 1))
>>>> +#define __UFFD_API_V2_BITS (UFFD_PAGEFAULT_BIT)
>>>> +
>>>> +/*
>>>> + * Lower PAGE_SHIFT bits are used to report those supported
>>>> + * by the pagefault message itself. Other bits are used to
>>>> + * report the message types v2 API supports
>>>> + */
>>>> +#define UFFD_API_V2_BITS (__UFFD_API_V2_BITS << 12)
>>>> +
>>>
>>> And why exactly is this 12 hardcoded?
>>
>> Ah, it should have been the PAGE_SHIFT one, but I was unsure whether it
>> would be OK to have different shifts in different arches.
>>
>> But taking into account your comment that bits field id bad for these
>> values, if we introduce the new .features one for api message, then this
>> 12 will just go away.
>
> Ok.
>
>>> And which field should be masked
>>> with the bits? In the V1 protocol it was the "arg" (userfault address)
>>> not the "type". So this is a bit confusing and probably requires
>>> simplification.
>>
>> I see. Actually I decided that since bits higher than 12th (for x86) is
>> always 0 in api message (no bits allowed there, since pfn sits in this
>> place), it would be OK to put non-PF bits there.
>
> That was ok yes.
>
>> Should I better introduce another .features field in uffd API message?
>
> What about renaming "uffdio_api.bits" to "uffdio_api.features"?
Yup, agreed, will do.
> And then we set uffdio_api.features to
> UFFD_FEATURE_WRITE|UFFD_FEATURE_WP|UFFD_FEATURE_FORK as needed.
>
> UFFD_FEATURE_WRITE would always be enabled, it's there only in case we
> want to disable it later (mostly if some arch has trouble with it,
> which is unlikely, but qemu doesn't need that bit of information at
> all for example so qemu would be fine if UFFD_FEATURE_WRITE
> disappears).
>
> UFFD_FEATURE_WP would signal also that the wrprotection feature (not
> implemented yet) is available (then later the register ioctl would
> also show the new wrprotection ioctl numbers available to mangle the
> wrprotection). The UFFD_FEATURE_WP feature in the cooperative usage
> (qemu live snapshotting) can use the UFFD_API first protocol too.
>
> UFFD_FEATURE_FORK would be returned if the UFFD_API_V2 was set in
> uffdio.api, and it would be part of the incremental non-cooperative
> patchset.
>
> We could also not define "UFFD_FEATURE_FORK" at all and imply that
> fork/mremap/MADV_DONTNEED are all available if UFFD_API_V2 uffdio_api
> ioctl succeeds... That's only doable if we keep two different read
> protocols though. UFFD_FEATURE_FORK (or UFFD_FEATURE_NON_COOPERATIVE)
> are really strictly needed only if we share the same read(2) protocol
> for both the cooperative and non-cooperative usages.
>
> The idea is that there's not huge benefit of only having the "fork"
> feature supported but missing "mremap" and "madv_dontneed".
>
> In fact if a new syscall that works like mremap is added later (call
> it mremap2), we would need to fail the UFFDIO_API_V2 and require a
> UFFDIO_API_V3 for such kernel that can return a new mremap2 type of
> event. Userland couldn't just assume it is ok to use postcopy live
> migration for containers, because
> UFFD_FEATURE_FORK|MREMAP|MADV_DONTNEED are present in the
> uffdio.features when it asked for API_V2. There shall be something
> that tells userland "hey there's a new mremap2 that the software
> inside the container can run on top of this kernel, so you are going
> to get a new mremap2 type of userfault event too".
But that's why I assumed to use per-sycall bits -- UFFD_FEATURE_FORK,
_MREMAP, _MWHATEVER so that userspace can read those bits and make sure
it contains only bits it understands with other bits set to zero.
If we had only one UFFD_API_NON_COOPERATIVE userspace would have no idea
what kind of messages it may receive.
> In any case, regardless of how we solve the above,
> "uffdio_api.features" sounds better than ".bits".
>
> If we retain two different UFFD_API, we'll be able to freeze the
> current one and decide later if
> UFFD_FEATURE_FORK|UFFD_FEATURE_MREMAP|UFFD_FEATURE_MADV_DONTNEED shall
> be added to the .features, or if to rely on UFFD_API_V2 succeeding to
> let userland know that the non-cooperative usage is fully supported by
> the kernel.
>
> Not having to freeze these details now is the main benefit of having
> two different UFFD_API after all...
> .
>
-- Pavel
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: Regression: Requiring CAP_SYS_ADMIN for /proc/<pid>/pagemap causes application-level breakage
From: Konstantin Khlebnikov @ 2015-04-30 11:43 UTC (permalink / raw)
To: Linus Torvalds
Cc: Kirill A. Shutemov, Mark Williamson, Mark Seaborn, kernel list,
Kirill A. Shutemov, Pavel Emelyanov, Andrew Morton,
Andy Lutomirski, Linux API, Finn Grimwood, Daniel James
In-Reply-To: <CA+55aFzW52u-LahoBA5kDs47rgeTGU_KF_GUrx+XSUDzeQD3=g@mail.gmail.com>
On Thu, Apr 30, 2015 at 12:02 AM, Linus Torvalds
<torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org> wrote:
> On Wed, Apr 29, 2015 at 1:44 PM, Konstantin Khlebnikov <koct9i-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>
>> This's no longer true. After recent fixes for "anon_vma endless growing" new vma
>> might reuse old anon_vma from grandparent vma.
>
> Oh well. I guess that was too simple.
>
> If Mark is ok with the rule that "it's not reliably if you have two
> nested forks" (ie it only works if you exec for every fork you do), it
> should still work, right? It sounds like Mark doesn't necessarily need
> to handle the *generic* case.
What about exposing shared/exclusive bit in pagemap == 1 if
page_mapcount() > 1, otherwise 0 (or vise versa).
Seems like this should work for detecting CoWed pages in child mm.
^ permalink raw reply
* Re: Regression: Requiring CAP_SYS_ADMIN for /proc/<pid>/pagemap causes application-level breakage
From: Mark Williamson @ 2015-04-30 11:50 UTC (permalink / raw)
To: Linus Torvalds
Cc: Konstantin Khlebnikov, Kirill A. Shutemov, Mark Seaborn,
kernel list, Kirill A. Shutemov, Pavel Emelyanov, Andrew Morton,
Andy Lutomirski, Linux API, Finn Grimwood, Daniel James
In-Reply-To: <CA+55aFzW52u-LahoBA5kDs47rgeTGU_KF_GUrx+XSUDzeQD3=g@mail.gmail.com>
On Wed, Apr 29, 2015 at 10:02 PM, Linus Torvalds
<torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org> wrote:
> On Wed, Apr 29, 2015 at 1:44 PM, Konstantin Khlebnikov <koct9i-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>
>> This's no longer true. After recent fixes for "anon_vma endless growing" new vma
>> might reuse old anon_vma from grandparent vma.
>
> Oh well. I guess that was too simple.
>
> If Mark is ok with the rule that "it's not reliably if you have two
> nested forks" (ie it only works if you exec for every fork you do), it
> should still work, right? It sounds like Mark doesn't necessarily need
> to handle the *generic* case.
Yes, it sounds like that should be OK for us. Our usecase is pretty
restricted, so we're a long way off requiring a generic solution.
Our code will always fork() a fresh child in which to monitor memory
changes. We run the operations we're interested in, use pagemap to
figure out "what changed" (by comparing whether the pagemap_entry_t
values are different from their parent) and then throw away the child
process.
Currently our code does an entry-by-entry compare of pagemap, so
anything that exposes writes as a change to values in there would
allow us to run unmodified. That would be really nice. That said, I
think we'd still be OK to modify our own code too if we can find a
solution that would continue to function on older kernel releases,
-stable trees, etc.
Thanks,
Mark
^ 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