On Sat, Nov 25, 2023 at 03:49:54AM +0530, Mukesh Ojha wrote:
> Client like minidump, is only interested in ramoops
> region addresses/size so that it could register them
> with its table and also it is only deals with ram
> backend and does not use pstorefs to read the records.
> Let's introduce a client notifier in ramoops which
> gets called when ramoops driver probes successfully
> and it passes the ramoops region information to the
> passed callback by the client and If the call for
> ramoops ready register comes after ramoops probe
> than call the callback directly.
>
> Signed-off-by: Mukesh Ojha <quic_mojha@xxxxxxxxxxx>
> ---
> fs/pstore/ram.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/pstore_ram.h | 6 ++++
> 2 files changed, 83 insertions(+)
>
> diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
> index a6c0da8cfdd4..72341fd21aec 100644
> --- a/fs/pstore/ram.c
> +++ b/fs/pstore/ram.c
> @@ -22,6 +22,7 @@
> #include <linux/of_address.h>
> #include <linux/memblock.h>
> #include <linux/mm.h>
> +#include <linux/mutex.h>
>
> #include "internal.h"
> #include "ram_internal.h"
> @@ -101,6 +102,14 @@ struct ramoops_context {
> unsigned int ftrace_read_cnt;
> unsigned int pmsg_read_cnt;
> struct pstore_info pstore;
> + /*
> + * Lock to serialize calls to register_ramoops_ready_notifier,
> + * ramoops_ready_notifier and read/modification of 'ramoops_ready'.
> + */
> + struct mutex lock;
> + bool ramoops_ready;
> + int (*callback)(const char *name, int id, void *vaddr,
> + phys_addr_t paddr, size_t size);
> };
>
> static struct platform_device *dummy;
> @@ -488,6 +497,7 @@ static int ramoops_pstore_erase(struct pstore_record *record)
> }
>
> static struct ramoops_context oops_cxt = {
> + .lock = __MUTEX_INITIALIZER(oops_cxt.lock),
> .pstore = {
> .owner = THIS_MODULE,
> .name = "ramoops",
> @@ -662,6 +672,68 @@ static int ramoops_init_prz(const char *name,
> return 0;
> }
>
> +void ramoops_ready_notifier(struct ramoops_context *cxt)
> +{
> + struct persistent_ram_zone *prz;
> + int i;
> +
> + if (!cxt->callback)
> + return;
> +
> + for (i = 0; i < cxt->max_dump_cnt; i++) {
> + prz = cxt->dprzs[i];
> + cxt->callback("dmesg", i, prz->vaddr, prz->paddr, prz->size);
> + }
> +
> + if (cxt->console_size) {
> + prz = cxt->cprz;
> + cxt->callback("console", 0, prz->vaddr, prz->paddr, prz->size);
> + }
> +
> + for (i = 0; i < cxt->max_ftrace_cnt; i++) {
> + prz = cxt->fprzs[i];
> + cxt->callback("ftrace", i, prz->vaddr, prz->paddr, prz->size);
> + }
> +
> + if (cxt->pmsg_size) {
> + prz = cxt->mprz;
> + cxt->callback("pmsg", 0, prz->vaddr, prz->paddr, prz->size);
> + }
> +}
> +
> +int register_ramoops_ready_notifier(int (*fn)(const char *, int,
> + void *, phys_addr_t, size_t))
> +{
> + struct ramoops_context *cxt = &oops_cxt;
> +
> + mutex_lock(&cxt->lock);
> + if (cxt->callback) {
> + mutex_unlock(&cxt->lock);
> + return -EEXIST;
> + }
> +
> + cxt->callback = fn;
> + if (cxt->ramoops_ready)
> + ramoops_ready_notifier(cxt);
> +
> + mutex_unlock(&cxt->lock);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(register_ramoops_ready_notifier);
> +
Can you please elaborate on why do we need this custom notifier logic?
why would not a standard notifier (include/linux/notifier.h) work here?
The notifier_call callback can recieve custom data from the
notifier chain implementer. All we need is to define a custom struct like
struct pstore_ramoops_zone_data {
const char *name;
int id;
void *vaddr;
phys_addr_t paddr;
size_t size;
};
and pass the pointer to array of this struct.
btw, the current logic only supports just one client and this limitation
is not highlighted any where.
Thanks,
Pavan
