[00/13] replace print_symbol() with printk()-s

[00/13] replace print_symbol() with printk()-s

[Sergey Senozhatsky]

On (12/11/17 08:26), Joe Perches wrote:
> On Mon, 2017-12-11 at 21:50 +0900, Sergey Senozhatsky wrote:
> > print_symbol
> 
> Yay.
> 
> Just about exactly 5 years earlier...
> http://lists.infradead.org/pipermail/linux-arm-kernel/2012-December/137121.html

indeed :)

hopefully it won't take us another 5 years to finally
---

 include/linux/kallsyms.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)


---

	-ss
--
To unsubscribe from this list: send the line "unsubscribe linux-edac" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

diff --git a/include/linux/kallsyms.h b/include/linux/kallsyms.h
index 7288f9c395b6..794fd35cad4b 100644
--- a/include/linux/kallsyms.h
+++ b/include/linux/kallsyms.h
@@ -176,7 +176,8 @@ void __check_printsym_format(const char *fmt, ...)
 {
 }
 
-static inline void print_symbol(const char *fmt, unsigned long addr)
+static inline void __deprecated print_symbol(const char *fmt,
+					     unsigned long addr)
 {
 	__check_printsym_format(fmt, "");
 	__print_symbol(fmt, (unsigned long)

[00/13] replace print_symbol() with printk()-s

[Joe Perches]

On Tue, 2017-12-12 at 11:47 +0900, Sergey Senozhatsky wrote:
> On (12/11/17 08:26), Joe Perches wrote:
> > On Mon, 2017-12-11 at 21:50 +0900, Sergey Senozhatsky wrote:
> > > print_symbol
> > Yay.
> > Just about exactly 5 years earlier...
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2012-December/137121.html
> 
> indeed :)
> 
> hopefully it won't take us another 5 years to finally
[]
> diff --git a/include/linux/kallsyms.h b/include/linux/kallsyms.h
[]
> -static inline void print_symbol(const char *fmt, unsigned long addr)
> +static inline void __deprecated print_symbol(const char *fmt,
> +					     unsigned long addr)
>  {
>  	__check_printsym_format(fmt, "");
>  	__print_symbol(fmt, (unsigned long)

As far as I'm concerned, as soon as there is
no longer a single user in the kernel tree,
better to delete it instead.
---
To unsubscribe from this list: send the line "unsubscribe linux-edac" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[00/13] replace print_symbol() with printk()-s

[Sergey Senozhatsky]

On (12/11/17 19:10), Joe Perches wrote:
[..]
> As far as I'm concerned, as soon as there is
> no longer a single user in the kernel tree,
> better to delete it instead.

sounds good to me. can drop it, once the series upstreamed.

8< ---

From: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Subject: [PATCH] kallsyms: remove print_symbol() function

No more print_symbol()/__print_symbol() users left, remove these
symbols.

Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Suggested-by: Joe Perches <joe@perches.com>
---
 Documentation/filesystems/sysfs.txt                    |  4 ++--
 Documentation/translations/zh_CN/filesystems/sysfs.txt |  4 ++--
 include/linux/kallsyms.h                               | 18 ------------------
 kernel/kallsyms.c                                      | 11 -----------
 4 files changed, 4 insertions(+), 33 deletions(-)

diff --git a/Documentation/filesystems/sysfs.txt b/Documentation/filesystems/sysfs.txt
index 9a3658cc399e..a1426cabcef1 100644
--- a/Documentation/filesystems/sysfs.txt
+++ b/Documentation/filesystems/sysfs.txt
@@ -154,8 +154,8 @@ static ssize_t dev_attr_show(struct kobject *kobj, struct attribute *attr,
         if (dev_attr->show)
                 ret = dev_attr->show(dev, dev_attr, buf);
         if (ret >= (ssize_t)PAGE_SIZE) {
-                print_symbol("dev_attr_show: %s returned bad count\n",
-                                (unsigned long)dev_attr->show);
+                printk("dev_attr_show: %pS returned bad count\n",
+                                dev_attr->show);
         }
         return ret;
 }
diff --git a/Documentation/translations/zh_CN/filesystems/sysfs.txt b/Documentation/translations/zh_CN/filesystems/sysfs.txt
index 7d3b05edb8ce..452271dda141 100644
--- a/Documentation/translations/zh_CN/filesystems/sysfs.txt
+++ b/Documentation/translations/zh_CN/filesystems/sysfs.txt
@@ -167,8 +167,8 @@ static ssize_t dev_attr_show(struct kobject *kobj, struct attribute *attr,
         if (dev_attr->show)
                 ret = dev_attr->show(dev, dev_attr, buf);
         if (ret >= (ssize_t)PAGE_SIZE) {
-                print_symbol("dev_attr_show: %s returned bad count\n",
-                                (unsigned long)dev_attr->show);
+                printk("dev_attr_show: %pS returned bad count\n",
+                                dev_attr->show);
         }
         return ret;
 }
diff --git a/include/linux/kallsyms.h b/include/linux/kallsyms.h
index 7288f9c395b6..d79d1e7486bd 100644
--- a/include/linux/kallsyms.h
+++ b/include/linux/kallsyms.h
@@ -94,9 +94,6 @@ extern int sprint_symbol(char *buffer, unsigned long address);
 extern int sprint_symbol_no_offset(char *buffer, unsigned long address);
 extern int sprint_backtrace(char *buffer, unsigned long address);
 
-/* Look up a kernel symbol and print it to the kernel messages. */
-extern void __print_symbol(const char *fmt, unsigned long address);
-
 int lookup_symbol_name(unsigned long addr, char *symname);
 int lookup_symbol_attrs(unsigned long addr, unsigned long *size, unsigned long *offset, char *modname, char *name);
 
@@ -166,23 +163,8 @@ static inline int kallsyms_show_value(void)
 	return false;
 }
 
-/* Stupid that this does nothing, but I didn't create this mess. */
-#define __print_symbol(fmt, addr)
 #endif /*CONFIG_KALLSYMS*/
 
-/* This macro allows us to keep printk typechecking */
-static __printf(1, 2)
-void __check_printsym_format(const char *fmt, ...)
-{
-}
-
-static inline void print_symbol(const char *fmt, unsigned long addr)
-{
-	__check_printsym_format(fmt, "");
-	__print_symbol(fmt, (unsigned long)
-		       __builtin_extract_return_addr((void *)addr));
-}
-
 static inline void print_ip_sym(unsigned long ip)
 {
 	printk("[<%p>] %pS\n", (void *) ip, (void *) ip);
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 24f456689f9c..a23e21ada81b 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -429,17 +429,6 @@ int sprint_backtrace(char *buffer, unsigned long address)
 	return __sprint_symbol(buffer, address, -1, 1);
 }
 
-/* Look up a kernel symbol and print it to the kernel messages. */
-void __print_symbol(const char *fmt, unsigned long address)
-{
-	char buffer[KSYM_SYMBOL_LEN];
-
-	sprint_symbol(buffer, address);
-
-	printk(fmt, buffer);
-}
-EXPORT_SYMBOL(__print_symbol);
-
 /* To avoid using get_symbol_offset for every symbol, we carry prefix along. */
 struct kallsym_iter {
 	loff_t pos;

[00/13] replace print_symbol() with printk()-s

[Sergey Senozhatsky]

On (01/05/18 19:21), Sergey Senozhatsky wrote:
> On (01/05/18 11:03), Petr Mladek wrote:
> [..]
> > Anyway, print_symbol() is an old weird API and it would be nice
> > to eventually get rid of it. I could take this patches into
> > printk.git.
> 
> no objections from my side if the patch set will go through the printk tree.
> shall we wait for ACKs or can we move on? do you plan to land it in 4.16?
> 
> > Would you mind if I change the commit messages to something like?:
> > 
> >     print_symbol() is an old weird API. It has been
> >     obsoleted by printk() and %pS format specifier.
> 
> I wouldn't. let's drop the "weird" part. other than that looks
> good to me.

oh, one more thing. one extra patch, which gets rid of
print_symbol()/__print_symbol().

8< ===

From: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Subject: [PATCH] kallsyms: remove print_symbol() function

No more print_symbol()/__print_symbol() users left, remove these
symbols.

Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Suggested-by: Joe Perches <joe@perches.com>
---
 Documentation/filesystems/sysfs.txt                    |  4 ++--
 Documentation/translations/zh_CN/filesystems/sysfs.txt |  4 ++--
 include/linux/kallsyms.h                               | 18 ------------------
 kernel/kallsyms.c                                      | 11 -----------
 4 files changed, 4 insertions(+), 33 deletions(-)

diff --git a/Documentation/filesystems/sysfs.txt b/Documentation/filesystems/sysfs.txt
index 9a3658cc399e..a1426cabcef1 100644
--- a/Documentation/filesystems/sysfs.txt
+++ b/Documentation/filesystems/sysfs.txt
@@ -154,8 +154,8 @@ static ssize_t dev_attr_show(struct kobject *kobj, struct attribute *attr,
         if (dev_attr->show)
                 ret = dev_attr->show(dev, dev_attr, buf);
         if (ret >= (ssize_t)PAGE_SIZE) {
-                print_symbol("dev_attr_show: %s returned bad count\n",
-                                (unsigned long)dev_attr->show);
+                printk("dev_attr_show: %pS returned bad count\n",
+                                dev_attr->show);
         }
         return ret;
 }
diff --git a/Documentation/translations/zh_CN/filesystems/sysfs.txt b/Documentation/translations/zh_CN/filesystems/sysfs.txt
index 7d3b05edb8ce..452271dda141 100644
--- a/Documentation/translations/zh_CN/filesystems/sysfs.txt
+++ b/Documentation/translations/zh_CN/filesystems/sysfs.txt
@@ -167,8 +167,8 @@ static ssize_t dev_attr_show(struct kobject *kobj, struct attribute *attr,
         if (dev_attr->show)
                 ret = dev_attr->show(dev, dev_attr, buf);
         if (ret >= (ssize_t)PAGE_SIZE) {
-                print_symbol("dev_attr_show: %s returned bad count\n",
-                                (unsigned long)dev_attr->show);
+                printk("dev_attr_show: %pS returned bad count\n",
+                                dev_attr->show);
         }
         return ret;
 }
diff --git a/include/linux/kallsyms.h b/include/linux/kallsyms.h
index 23190e5c940b..657a83b943f0 100644
--- a/include/linux/kallsyms.h
+++ b/include/linux/kallsyms.h
@@ -94,9 +94,6 @@ extern int sprint_symbol(char *buffer, unsigned long address);
 extern int sprint_symbol_no_offset(char *buffer, unsigned long address);
 extern int sprint_backtrace(char *buffer, unsigned long address);
 
-/* Look up a kernel symbol and print it to the kernel messages. */
-extern void __print_symbol(const char *fmt, unsigned long address);
-
 int lookup_symbol_name(unsigned long addr, char *symname);
 int lookup_symbol_attrs(unsigned long addr, unsigned long *size, unsigned long *offset, char *modname, char *name);
 
@@ -166,23 +163,8 @@ static inline int kallsyms_show_value(void)
 	return false;
 }
 
-/* Stupid that this does nothing, but I didn't create this mess. */
-#define __print_symbol(fmt, addr)
 #endif /*CONFIG_KALLSYMS*/
 
-/* This macro allows us to keep printk typechecking */
-static __printf(1, 2)
-void __check_printsym_format(const char *fmt, ...)
-{
-}
-
-static inline void print_symbol(const char *fmt, unsigned long addr)
-{
-	__check_printsym_format(fmt, "");
-	__print_symbol(fmt, (unsigned long)
-		       __builtin_extract_return_addr((void *)addr));
-}
-
 static inline void print_ip_sym(unsigned long ip)
 {
 	printk("[<%px>] %pS\n", (void *) ip, (void *) ip);
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 24f456689f9c..a23e21ada81b 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -429,17 +429,6 @@ int sprint_backtrace(char *buffer, unsigned long address)
 	return __sprint_symbol(buffer, address, -1, 1);
 }
 
-/* Look up a kernel symbol and print it to the kernel messages. */
-void __print_symbol(const char *fmt, unsigned long address)
-{
-	char buffer[KSYM_SYMBOL_LEN];
-
-	sprint_symbol(buffer, address);
-
-	printk(fmt, buffer);
-}
-EXPORT_SYMBOL(__print_symbol);
-
 /* To avoid using get_symbol_offset for every symbol, we carry prefix along. */
 struct kallsym_iter {
 	loff_t pos;

[00/13] replace print_symbol() with printk()-s

[Petr Mladek]

On Fri 2018-01-05 19:25:38, Sergey Senozhatsky wrote:
> On (01/05/18 19:21), Sergey Senozhatsky wrote:
> > On (01/05/18 11:03), Petr Mladek wrote:
> > [..]
> > > Anyway, print_symbol() is an old weird API and it would be nice
> > > to eventually get rid of it. I could take this patches into
> > > printk.git.
> > 
> > no objections from my side if the patch set will go through the printk tree.
> > shall we wait for ACKs or can we move on? do you plan to land it in 4.16?
> > 
> > > Would you mind if I change the commit messages to something like?:
> > > 
> > >     print_symbol() is an old weird API. It has been
> > >     obsoleted by printk() and %pS format specifier.
> > 
> > I wouldn't. let's drop the "weird" part. other than that looks
> > good to me.
> 
> oh, one more thing. one extra patch, which gets rid of
> print_symbol()/__print_symbol().

I am all for it. But I would postpone this removal to 4.17.
The reason is rather ugly. 13th patch is already in arc tree.
We would need to shuffle the patch or coordinate pull requests.
I think that it is not worth it. There is no real hurry.
I doubt that the would be any new user in the meantime.

Best Regards,
Petr

PS: I have just pushed 12 patches into printk.git for-4.16 branch.
I will merge this to linux-next branch on Monday. I will not
be around the computer over the weekend...
---
To unsubscribe from this list: send the line "unsubscribe linux-edac" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[00/13] replace print_symbol() with printk()-s

[Sergey Senozhatsky]

On (01/05/18 15:42), Petr Mladek wrote:
[..]
> > oh, one more thing. one extra patch, which gets rid of
> > print_symbol()/__print_symbol().
> 
> I am all for it. But I would postpone this removal to 4.17.
> The reason is rather ugly. 13th patch is already in arc tree.
> We would need to shuffle the patch or coordinate pull requests.
> I think that it is not worth it. There is no real hurry.
> I doubt that the would be any new user in the meantime.
> 
> Best Regards,
> Petr
> 
> PS: I have just pushed 12 patches into printk.git for-4.16 branch.
> I will merge this to linux-next branch on Monday. I will not
> be around the computer over the weekend...

OK. thanks!

	-ss
---
To unsubscribe from this list: send the line "unsubscribe linux-edac" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[00/13] replace print_symbol() with printk()-s

[Sergey Senozhatsky]

On (01/05/18 15:42), Petr Mladek wrote:
> 
> I am all for it. But I would postpone this removal to 4.17.
> The reason is rather ugly. 13th patch is already in arc tree.
> We would need to shuffle the patch or coordinate pull requests.

JFI, the patch is in Linus's tree as of now (d0729bc6bee797fb).

	-ss
---
To unsubscribe from this list: send the line "unsubscribe linux-edac" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[00/13] replace print_symbol() with printk()-s

[Petr Mladek]

On Mon 2018-01-08 11:09:42, Sergey Senozhatsky wrote:
> On (01/05/18 15:42), Petr Mladek wrote:
> > 
> > I am all for it. But I would postpone this removal to 4.17.
> > The reason is rather ugly. 13th patch is already in arc tree.
> > We would need to shuffle the patch or coordinate pull requests.
> 
> JFI, the patch is in Linus's tree as of now (d0729bc6bee797fb).

Great. I have pushed the patch that removes printk_symbol()
into printk.git, branch for-4.16-print-symbol.

Note that I have updated the commit message similar way
like I did for the other commits. Especially I wanted
to mention what it was obsoleted by. The message is:

  kallsyms: remove print_symbol() function

  No more print_symbol()/__print_symbol() users left, remove these
  symbols.

  It was a very old API that encouraged people use continuous lines.
  It had been obsoleted by %pS format specifier in a normal printk()
  call.


See also
https://git.kernel.org/pub/scm/linux/kernel/git/pmladek/printk.git/commit/?h=for-4.16-print-symbol&id=d2279c9d7f7db7f97567368bfc4539b3411adf8d

Best Regards,
Petr
---
To unsubscribe from this list: send the line "unsubscribe linux-edac" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[00/13] replace print_symbol() with printk()-s

[Sergey Senozhatsky]

On (01/16/18 17:33), Petr Mladek wrote:
[..]
> > JFI, the patch is in Linus's tree as of now (d0729bc6bee797fb).
> 
> Great. I have pushed the patch that removes printk_symbol()
> into printk.git, branch for-4.16-print-symbol.
> 
> Note that I have updated the commit message similar way
> like I did for the other commits. Especially I wanted
> to mention what it was obsoleted by. The message is:
> 
>   kallsyms: remove print_symbol() function
> 
>   No more print_symbol()/__print_symbol() users left, remove these
>   symbols.
> 
>   It was a very old API that encouraged people use continuous lines.
>   It had been obsoleted by %pS format specifier in a normal printk()
>   call.
> 
> 
> See also
> https://git.kernel.org/pub/scm/linux/kernel/git/pmladek/printk.git/commit/?h=for-4.16-print-symbol&id=d2279c9d7f7db7f97567368bfc4539b3411adf8d

thanks!

	-ss
---
To unsubscribe from this list: send the line "unsubscribe linux-edac" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html