2020-05-27 20:30:02

by Rishabh Bhatnagar

[permalink] [raw]
Subject: [v4 PATCH 0/3] Extend coredump functionality

This patch series moves the coredump functionality to a separate
file and adds "inline" coredump feature. Inline coredump directly
copies segments from device memory during coredump to userspace.
This avoids extra memory usage at the cost of speed. Recovery is
stalled until all data is read by userspace.

Changelog:

v4 -> v3:
- Write a helper function to copy segment memory for every dump format
- Change segment dump fn to add offset and size adn covert mss driver

v3 -> v2:
- Move entire coredump functionality to remoteproc_coredump.c
- Modify rproc_coredump to perform dump according to conf. set by userspace
- Move the userspace configuration to debugfs from sysfs.
- Keep the default coredump implementation as is

v2 -> v1:
- Introduce new file for coredump.
- Add userspace sysfs configuration for dump type.

Rishabh Bhatnagar (3):
remoteproc: Move coredump functionality to a new file
remoteproc: Add inline coredump functionality
remoteproc: Add coredump debugfs entry

drivers/remoteproc/Makefile | 1 +
drivers/remoteproc/qcom_q6v5_mss.c | 9 +-
drivers/remoteproc/remoteproc_core.c | 191 ------------------
drivers/remoteproc/remoteproc_coredump.c | 328 +++++++++++++++++++++++++++++++
drivers/remoteproc/remoteproc_debugfs.c | 86 ++++++++
drivers/remoteproc/remoteproc_internal.h | 4 +
include/linux/remoteproc.h | 21 +-
7 files changed, 443 insertions(+), 197 deletions(-)
create mode 100644 drivers/remoteproc/remoteproc_coredump.c

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


2020-05-27 20:32:02

by Rishabh Bhatnagar

[permalink] [raw]
Subject: [PATCH v4 3/3] remoteproc: Add coredump debugfs entry

Add coredump debugfs entry to configure the type of dump that will
be collected during recovery. User can select between default or
inline coredump functionality. Also coredump collection can be
disabled through this interface.
This functionality can be configured differently for different
remote processors.

Signed-off-by: Rishabh Bhatnagar <[email protected]>
Reviewed-by: Bjorn Andersson <[email protected]>
Reviewed-by: Mathieu Poirier <[email protected]>
---
drivers/remoteproc/remoteproc_debugfs.c | 86 +++++++++++++++++++++++++++++++++
1 file changed, 86 insertions(+)

diff --git a/drivers/remoteproc/remoteproc_debugfs.c b/drivers/remoteproc/remoteproc_debugfs.c
index 732770e..cca0a91 100644
--- a/drivers/remoteproc/remoteproc_debugfs.c
+++ b/drivers/remoteproc/remoteproc_debugfs.c
@@ -28,6 +28,90 @@
static struct dentry *rproc_dbg;

/*
+ * A coredump-configuration-to-string lookup table, for exposing a
+ * human readable configuration via debugfs. Always keep in sync with
+ * enum rproc_coredump_mechanism
+ */
+static const char * const rproc_coredump_str[] = {
+ [RPROC_COREDUMP_DEFAULT] = "default",
+ [RPROC_COREDUMP_INLINE] = "inline",
+ [RPROC_COREDUMP_DISABLED] = "disabled",
+};
+
+/* Expose the current coredump configuration via debugfs */
+static ssize_t rproc_coredump_read(struct file *filp, char __user *userbuf,
+ size_t count, loff_t *ppos)
+{
+ struct rproc *rproc = filp->private_data;
+ const char *buf = rproc_coredump_str[rproc->dump_conf];
+
+ return simple_read_from_buffer(userbuf, count, ppos, buf, strlen(buf));
+}
+
+/*
+ * By writing to the 'coredump' debugfs entry, we control the behavior of the
+ * coredump mechanism dynamically. The default value of this entry is "default".
+ *
+ * The 'coredump' debugfs entry supports these commands:
+ *
+ * default: This is the default coredump mechanism. When the remoteproc
+ * crashes the entire coredump will be copied to a separate buffer
+ * and exposed to userspace.
+ *
+ * inline: The coredump will not be copied to a separate buffer and the
+ * recovery process will have to wait until data is read by
+ * userspace. But this avoid usage of extra memory.
+ *
+ * disabled: This will disable coredump. Recovery will proceed without
+ * collecting any dump.
+ */
+static ssize_t rproc_coredump_write(struct file *filp,
+ const char __user *user_buf, size_t count,
+ loff_t *ppos)
+{
+ struct rproc *rproc = filp->private_data;
+ int ret, err = 0;
+ char buf[20];
+
+ if (count > sizeof(buf))
+ return -EINVAL;
+
+ ret = copy_from_user(buf, user_buf, count);
+ if (ret)
+ return -EFAULT;
+
+ /* remove end of line */
+ if (buf[count - 1] == '\n')
+ buf[count - 1] = '\0';
+
+ if (rproc->state == RPROC_CRASHED) {
+ dev_err(&rproc->dev, "can't change coredump configuration\n");
+ err = -EBUSY;
+ goto out;
+ }
+
+ if (!strncmp(buf, "disable", count)) {
+ rproc->dump_conf = RPROC_COREDUMP_DISABLED;
+ } else if (!strncmp(buf, "inline", count)) {
+ rproc->dump_conf = RPROC_COREDUMP_INLINE;
+ } else if (!strncmp(buf, "default", count)) {
+ rproc->dump_conf = RPROC_COREDUMP_DEFAULT;
+ } else {
+ dev_err(&rproc->dev, "Invalid coredump configuration\n");
+ err = -EINVAL;
+ }
+out:
+ return err ? err : count;
+}
+
+static const struct file_operations rproc_coredump_fops = {
+ .read = rproc_coredump_read,
+ .write = rproc_coredump_write,
+ .open = simple_open,
+ .llseek = generic_file_llseek,
+};
+
+/*
* Some remote processors may support dumping trace logs into a shared
* memory buffer. We expose this trace buffer using debugfs, so users
* can easily tell what's going on remotely.
@@ -337,6 +421,8 @@ void rproc_create_debug_dir(struct rproc *rproc)
rproc, &rproc_rsc_table_fops);
debugfs_create_file("carveout_memories", 0400, rproc->dbg_dir,
rproc, &rproc_carveouts_fops);
+ debugfs_create_file("coredump", 0600, rproc->dbg_dir,
+ rproc, &rproc_coredump_fops);
}

void __init rproc_init_debugfs(void)
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

2020-06-22 22:28:40

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [v4 PATCH 0/3] Extend coredump functionality

On Wed 27 May 13:26 PDT 2020, Rishabh Bhatnagar wrote:

> This patch series moves the coredump functionality to a separate
> file and adds "inline" coredump feature. Inline coredump directly
> copies segments from device memory during coredump to userspace.
> This avoids extra memory usage at the cost of speed. Recovery is
> stalled until all data is read by userspace.
>
> Changelog:
>

Hi Rishabh,

This looks good to me, but it doesn't apply cleanly on linux-next. Can
you please take a look?

Regards,
Bjorn

> v4 -> v3:
> - Write a helper function to copy segment memory for every dump format
> - Change segment dump fn to add offset and size adn covert mss driver
>
> v3 -> v2:
> - Move entire coredump functionality to remoteproc_coredump.c
> - Modify rproc_coredump to perform dump according to conf. set by userspace
> - Move the userspace configuration to debugfs from sysfs.
> - Keep the default coredump implementation as is
>
> v2 -> v1:
> - Introduce new file for coredump.
> - Add userspace sysfs configuration for dump type.
>
> Rishabh Bhatnagar (3):
> remoteproc: Move coredump functionality to a new file
> remoteproc: Add inline coredump functionality
> remoteproc: Add coredump debugfs entry
>
> drivers/remoteproc/Makefile | 1 +
> drivers/remoteproc/qcom_q6v5_mss.c | 9 +-
> drivers/remoteproc/remoteproc_core.c | 191 ------------------
> drivers/remoteproc/remoteproc_coredump.c | 328 +++++++++++++++++++++++++++++++
> drivers/remoteproc/remoteproc_debugfs.c | 86 ++++++++
> drivers/remoteproc/remoteproc_internal.h | 4 +
> include/linux/remoteproc.h | 21 +-
> 7 files changed, 443 insertions(+), 197 deletions(-)
> create mode 100644 drivers/remoteproc/remoteproc_coredump.c
>
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>

2020-06-23 02:46:24

by Rishabh Bhatnagar

[permalink] [raw]
Subject: Re: [v4 PATCH 0/3] Extend coredump functionality

On 2020-06-22 15:23, Bjorn Andersson wrote:
> On Wed 27 May 13:26 PDT 2020, Rishabh Bhatnagar wrote:
>
>> This patch series moves the coredump functionality to a separate
>> file and adds "inline" coredump feature. Inline coredump directly
>> copies segments from device memory during coredump to userspace.
>> This avoids extra memory usage at the cost of speed. Recovery is
>> stalled until all data is read by userspace.
>>
>> Changelog:
>>
>
> Hi Rishabh,
>
> This looks good to me, but it doesn't apply cleanly on linux-next. Can
> you please take a look?
>
> Regards,
> Bjorn
>
Yes some more gerrits have been merged in the meantime. I'll rebase on
top
of Linux-next and send out another patchset.
>> v4 -> v3:
>> - Write a helper function to copy segment memory for every dump format
>> - Change segment dump fn to add offset and size adn covert mss driver
>>
>> v3 -> v2:
>> - Move entire coredump functionality to remoteproc_coredump.c
>> - Modify rproc_coredump to perform dump according to conf. set by
>> userspace
>> - Move the userspace configuration to debugfs from sysfs.
>> - Keep the default coredump implementation as is
>>
>> v2 -> v1:
>> - Introduce new file for coredump.
>> - Add userspace sysfs configuration for dump type.
>>
>> Rishabh Bhatnagar (3):
>> remoteproc: Move coredump functionality to a new file
>> remoteproc: Add inline coredump functionality
>> remoteproc: Add coredump debugfs entry
>>
>> drivers/remoteproc/Makefile | 1 +
>> drivers/remoteproc/qcom_q6v5_mss.c | 9 +-
>> drivers/remoteproc/remoteproc_core.c | 191 ------------------
>> drivers/remoteproc/remoteproc_coredump.c | 328
>> +++++++++++++++++++++++++++++++
>> drivers/remoteproc/remoteproc_debugfs.c | 86 ++++++++
>> drivers/remoteproc/remoteproc_internal.h | 4 +
>> include/linux/remoteproc.h | 21 +-
>> 7 files changed, 443 insertions(+), 197 deletions(-)
>> create mode 100644 drivers/remoteproc/remoteproc_coredump.c
>>
>> --
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
>> Forum,
>> a Linux Foundation Collaborative Project
>>