Hi,

quoted

+int sprint_kallsym_common(char *buffer, unsigned long address, int build_id,
+			    int backtrace, int symbol)
+{
+	if (backtrace)
+		return __sprint_symbol(buffer, address, -1, 1, build_id);

quoted

+	else if (symbol)
+		return __sprint_symbol(buffer, address, 0, 1, build_id);
+	else
+		return __sprint_symbol(buffer, address, 0, 0, 0);

Redundant 'else' in both cases.

Ok, will change it

quoted

+}

...

quoted

+static int sprint_module_info(char *buf, char *end, unsigned long value,
+			     const char *fmt, int modbuildid, int backtrace, int symbol)

fmt is not used.

Yes, did not notice it.(will remove both end and gmt)

quoted

+{
+	struct module *mod;
+	unsigned long offset = 0;

quoted

+	unsigned long base;

Can it be the same type as core_layout.base? Why not?

quoted

+	char *modname;
+	int len;
+	const unsigned char *buildid = NULL;
+
+	if (is_ksym_addr(value))
+		return 0;
+
+	if (backtrace || symbol)
+		offset = 1;

I would expect here to have

	else
		offset = 0;

But see below.

quoted

+	preempt_disable();
+	mod = __module_address(value);
+	if (mod) {
+		modname = mod->name;
+#if IS_ENABLED(CONFIG_STACKTRACE_BUILD_ID)
+		if (modbuildid)
+			buildid = mod->build_id;
+#endif

quoted

+		if (offset) {

This seems quite confusing because semantically you use offset as a boolean
flag and offset. Why not add a boolean variable with a clear name?

Ok, will add 2 separate variables.

quoted

+			base = (unsigned long)mod->core_layout.base;
+			offset = value - base;
+		}
+	}

quoted

+

Probably you can drop this blank line to group entire critical section,
or add a blank line above.

quoted

+	preempt_enable();
+	if (!mod)
+		return 0;
+
+	/* address belongs to module */
+	if (offset)
+		len = sprintf(buf, "0x%p+0x%lx", (void *)base, offset);
+	else

quoted

+		len = sprintf(buf, "0x%lx", (void *)value);

What this casting is for? Don't you have a compilation warning?

My Bad, earlier I made patch with hashing this value also (%p), but after that
changed it to %lx to have same original behavior in case of %ps, forgot to update final patch
to remove typecast.

quoted

+	len += fill_name_build_id(buf, modname, modbuildid, buildid, len);
+	return len;

	return len + ...;

?

quoted

+}

Will modify patch with all changes.

Thanks,
Maninder Singh