On Thu, Nov 21, 2024 at 12:12:17PM +0530, Ekansh Gupta wrote:
>
>
> On 11/18/2024 7:32 PM, Greg KH wrote:
> > On Mon, Nov 18, 2024 at 02:10:46PM +0530, Ekansh Gupta wrote:
> >> Add changes to support debugfs. The fastrpc directory will be
> >> created which will carry debugfs files for all fastrpc processes.
> >> The information of fastrpc user and channel contexts are getting
> >> captured as part of this change.
> >>
> >> Signed-off-by: Ekansh Gupta <quic_ekangupt@xxxxxxxxxxx>
> >> ---
> >> drivers/misc/fastrpc/Makefile | 3 +-
> >> drivers/misc/fastrpc/fastrpc_debug.c | 156 +++++++++++++++++++++++++++
> >> drivers/misc/fastrpc/fastrpc_debug.h | 31 ++++++
> >> drivers/misc/fastrpc/fastrpc_main.c | 18 +++-
> >> 4 files changed, 205 insertions(+), 3 deletions(-)
> >> create mode 100644 drivers/misc/fastrpc/fastrpc_debug.c
> >> create mode 100644 drivers/misc/fastrpc/fastrpc_debug.h
> >>
> >> diff --git a/drivers/misc/fastrpc/Makefile b/drivers/misc/fastrpc/Makefile
> >> index 020d30789a80..4ff6b64166ae 100644
> >> --- a/drivers/misc/fastrpc/Makefile
> >> +++ b/drivers/misc/fastrpc/Makefile
> >> @@ -1,3 +1,4 @@
> >> # SPDX-License-Identifier: GPL-2.0
> >> obj-$(CONFIG_QCOM_FASTRPC) += fastrpc.o
> >> -fastrpc-objs := fastrpc_main.o
> >> \ No newline at end of file
> >> +fastrpc-objs := fastrpc_main.o \
> >> + fastrpc_debug.o
> > Only build this file if debugfs is enabled.
> >
> > And again, "debug.c"?
> I'll add change to build this only if debugfs is enabled. Going forward I have plans to add
> few more debug specific changes, maybe then I'll need to change the build rules again.
> >
> >> diff --git a/drivers/misc/fastrpc/fastrpc_debug.c b/drivers/misc/fastrpc/fastrpc_debug.c
> >> new file mode 100644
> >> index 000000000000..cdb4fc6845a8
> >> --- /dev/null
> >> +++ b/drivers/misc/fastrpc/fastrpc_debug.c
> >> @@ -0,0 +1,156 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +// Copyright (c) 2024 Qualcomm Innovation Center.
> >> +
> >> +#include <linux/debugfs.h>
> >> +#include <linux/seq_file.h>
> >> +#include "fastrpc_shared.h"
> >> +#include "fastrpc_debug.h"
> >> +
> >> +#ifdef CONFIG_DEBUG_FS
> > Please put the #ifdef in the .h file, not in the .c file.
> Ack
> >
> >> +void fastrpc_create_user_debugfs(struct fastrpc_user *fl)
> >> +{
> >> + char cur_comm[TASK_COMM_LEN];
> >> + int domain_id, size;
> >> + char *debugfs_buf;
> >> + struct dentry *debugfs_dir = fl->cctx->debugfs_dir;
> >> +
> >> + memcpy(cur_comm, current->comm, TASK_COMM_LEN);
> >> + cur_comm[TASK_COMM_LEN-1] = '\0';
> >> + if (debugfs_dir != NULL) {
> >> + domain_id = fl->cctx->domain_id;
> >> + size = snprintf(NULL, 0, "%.10s_%d_%d_%d", cur_comm,
> >> + current->pid, fl->tgid, domain_id) + 1;
> >> + debugfs_buf = kzalloc(size, GFP_KERNEL);
> >> + if (debugfs_buf == NULL)
> >> + return;
> >> + /*
> >> + * Use HLOS process name, HLOS PID, fastrpc user TGID,
> >> + * domain_id in debugfs filename to create unique file name
> >> + */
> >> + snprintf(debugfs_buf, size, "%.10s_%d_%d_%d",
> >> + cur_comm, current->pid, fl->tgid, domain_id);
> >> + fl->debugfs_file = debugfs_create_file(debugfs_buf, 0644,
> >> + debugfs_dir, fl, &fastrpc_debugfs_fops);
> > Why are you saving the debugfs file? What do you need to do with it
> > that you can't just delete the whole directory, or look up the name
> > again in the future when removing it?
> fl structure is specific to a process using fastrpc driver. The reason to save
> this debugfs file is to delete is when the process releases fastrpc device.
> If the file is not deleted, it might flood multiple files in debugfs directory.
>
> As part of this change, only the file that is getting created by a process is
> getting removed when process is releasing device and I don't think we
> can clean up the whole directory at this point.
My 2c: it might be better to create a single file that conains
information for all the processes instead of that. Or use fdinfo data to
export process / FD information to userspace.
--
With best wishes
Dmitry
