Re: [RFC] kallsyms_lookup always requires buffers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Franck Bui-Huu wrote:
Hi,

Some uses of kallsyms_lookup() do not need to find out the name of
a symbol and its module's name it belongs. This is specially true
in arch specific code, which needs to unwind the stack to show the
back trace during opps (mips is an example). In this specific case,
we just need to retreive the function's size and the offset of the
active intruction inside it.

This simple patch adds a new entry "kallsyms_lookup_gently()". The
name actually sucks but I can't figure out a coherent name with the
rest of the file. If someone could give me a better idea...

kallsyms_lookup_size_offset() ?

Granted it is a little bigger, but it tells exactly what it does and there are not that many users that we have to write this name all that often.

This new entry does exactly the same as kallsyms_lookup() but does
not require any buffers to store any names. It returns 0 if it fails
otherwise 1.

Do you think this can be usefull ?

You tell me, since you're proposing the change ;)

diff --git a/include/linux/kallsyms.h b/include/linux/kallsyms.h
index 849043c..30f8d8d 100644
--- a/include/linux/kallsyms.h
+++ b/include/linux/kallsyms.h
@@ -12,6 +12,10 @@ #ifdef CONFIG_KALLSYMS
 /* Lookup the address for a symbol. Returns 0 if not found. */
 unsigned long kallsyms_lookup_name(const char *name);
+extern int kallsyms_lookup_gently(unsigned long addr,
+				  unsigned long *symbolsize,
+				  unsigned long *offset);
+
 /* Lookup an address.  modname is set to NULL if it's in the kernel. */
 const char *kallsyms_lookup(unsigned long addr,
 			    unsigned long *symbolsize,
@@ -28,6 +32,13 @@ static inline unsigned long kallsyms_loo
 	return 0;
 }
+static inline const int kallsyms_lookup_gently(unsigned long addr,
+					       unsigned long *symbolsize,
+					       unsigned long *offset)
+{
+	return 0;
+}
+
 static inline const char *kallsyms_lookup(unsigned long addr,
 					  unsigned long *symbolsize,
 					  unsigned long *offset,
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index ab16a5a..c398753 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -69,6 +69,15 @@ static inline int is_kernel(unsigned lon
 	return in_gate_area_no_task(addr);
 }
+static int is_kernel_addr(unsigned long addr)

Maybe change the name to kallsyms_is_kernel_addr, because this function does more things than what the generic name implies.

+{
+	if (all_var)
+		return is_kernel(addr);
+
+	return is_kernel_text(addr) || is_kernel_inittext(addr) ||
+		is_kernel_extratext(addr);
+}
+

[...]
+/*
+ * Lookup an address but don't bother to find any names.
+ */
+int kallsyms_lookup_gently(unsigned long addr, unsigned long *symbolsize,
+			   unsigned long *offset)
+{
+	int rv;
+
+	if (is_kernel_addr(addr))
+		rv = !!get_symbol_pos(addr, symbolsize, offset);
+	else
+		rv = !!module_address_lookup(addr, symbolsize, offset, NULL);
+
+	return rv;
+}

<minor nitpick>

Why not just:

> if (is_kernel_addr(addr))
>	return !!get_symbol_pos(addr, symbolsize, offset);
>
> return !!module_address_lookup(addr, symbolsize, offset, NULL);

and just get rid of "rv" completely?

</minor nitpick>

+EXPORT_SYMBOL_GPL(kallsyms_lookup_gently);

I agree with Arjan here. If kallsyms_lookup wasn't exported, I don't see a reason to export this either.

Everything else seems fine.

--
Paulo Marques - www.grupopie.com

"The face of a child can say it all, especially the
mouth part of the face."
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux