linux-next: build warning after merge of the printk tree

linux-next: build warning after merge of the printk tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 354 bytes --]

Hi all,

After merging the printk tree, today's linux-next build (htmldocs)
produced this warning:

kernel/printk/printk.c:298: warning: Function parameter or member 'cookie' not described in 'console_srcu_read_unlock'

Introduced by commit

  8bdbdd7f43cd ("printk: Prepare for SRCU console list protection")

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: build warning after merge of the printk tree

[Chris Down]

Jonathan Corbet writes:
>The problem is that you moved printk(), but left the associated
>kerneldoc comment tied to _printk(), which isn't what you really want to
>document.  The fix should look something like the attached.

Ah, I see. Thank you for your help!

Re: linux-next: build warning after merge of the printk tree

[Petr Mladek]

On Sun 2021-07-25 15:16:00, Jonathan Corbet wrote:
> Chris Down <chris@chrisdown.name> writes:
> 
> > Chris Down writes:
> >>+Cc: Jonathan Corbet <corbet@lwn.net>, linux-doc@vger.kernel.org
> >
> > Well, let's actually Cc them this time...
> >
> >>Stephen Rothwell writes:
> >>>After merging the printk tree, today's linux-next build (htmldocs)
> >>>produced this warning:
> >>>
> >>>kernel/printk/printk.c:1: warning: 'printk' not found
> >>>
> >>>Introduced by commit
> >>>
> >>> 337015573718 ("printk: Userspace format indexing support")
> >>>
> >>>I presume that "printk" is referred to elsewhere in the documentation
> >>>as being in this file.
> >>
> >>Hmm, this is an interesting one, because I think we still generally 
> >>just want to refer to the API as being `printk()`. Changing it all 
> >>over the place seems wrong. As you'd imagine, there are quite a few 
> >>references to this name, so it requires a lot of noise all over the 
> >>docs and inline comments.
> >>
> >>Jonathan and other docs folks, how can one tell Sphinx that when it 
> >>sees printk() it's referring to a function-like macro, or otherwise 
> >>squelch this reasonably? :-)
> 
> The problem is that you moved printk(), but left the associated
> kerneldoc comment tied to _printk(), which isn't what you really want to
> document.  The fix should look something like the attached.
> 
> Thanks,
> 
> jon
> 
> --------snip here----------------------
> printk: Move the printk() kerneldoc comment to its new home
> 
> Commit 337015573718 ("printk: Userspace format indexing support") turned
> printk() into a macro, but left the kerneldoc comment for it with the (now)
> _printk() function, resulting in this docs-build warning:
> 
>   kernel/printk/printk.c:1: warning: 'printk' not found
> 
> Move the kerneldoc comment back next to the (now) macro it's meant to
> describe and have the docs build find it there.
> 
> Signed-off-by: Jonathan Corbet <corbet@lwn.net>

This looks better than my variant. I have pushed it into
printk/linux.git, branch for-5.15-printk-index.

Best Regards,
Petr

Re: linux-next: build warning after merge of the printk tree

[Jonathan Corbet]

Chris Down <chris@chrisdown.name> writes:

> Chris Down writes:
>>+Cc: Jonathan Corbet <corbet@lwn.net>, linux-doc@vger.kernel.org
>
> Well, let's actually Cc them this time...
>
>>Stephen Rothwell writes:
>>>After merging the printk tree, today's linux-next build (htmldocs)
>>>produced this warning:
>>>
>>>kernel/printk/printk.c:1: warning: 'printk' not found
>>>
>>>Introduced by commit
>>>
>>> 337015573718 ("printk: Userspace format indexing support")
>>>
>>>I presume that "printk" is referred to elsewhere in the documentation
>>>as being in this file.
>>
>>Hmm, this is an interesting one, because I think we still generally 
>>just want to refer to the API as being `printk()`. Changing it all 
>>over the place seems wrong. As you'd imagine, there are quite a few 
>>references to this name, so it requires a lot of noise all over the 
>>docs and inline comments.
>>
>>Jonathan and other docs folks, how can one tell Sphinx that when it 
>>sees printk() it's referring to a function-like macro, or otherwise 
>>squelch this reasonably? :-)

The problem is that you moved printk(), but left the associated
kerneldoc comment tied to _printk(), which isn't what you really want to
document.  The fix should look something like the attached.

Thanks,

jon

--------snip here----------------------
printk: Move the printk() kerneldoc comment to its new home

Commit 337015573718 ("printk: Userspace format indexing support") turned
printk() into a macro, but left the kerneldoc comment for it with the (now)
_printk() function, resulting in this docs-build warning:

  kernel/printk/printk.c:1: warning: 'printk' not found

Move the kerneldoc comment back next to the (now) macro it's meant to
describe and have the docs build find it there.

Signed-off-by: Jonathan Corbet <corbet@lwn.net>
---
 Documentation/core-api/printk-basics.rst |  5 +----
 include/linux/printk.h                   | 24 ++++++++++++++++++++++++
 kernel/printk/printk.c                   | 24 ------------------------
 3 files changed, 25 insertions(+), 28 deletions(-)

diff --git a/Documentation/core-api/printk-basics.rst b/Documentation/core-api/printk-basics.rst
index 965e4281eddd..2dde24ca7d9f 100644
--- a/Documentation/core-api/printk-basics.rst
+++ b/Documentation/core-api/printk-basics.rst
@@ -107,9 +107,6 @@ also ``CONFIG_DYNAMIC_DEBUG`` in the case of pr_debug()) is defined.
 Function reference
 ==================
 
-.. kernel-doc:: kernel/printk/printk.c
-   :functions: printk
-
 .. kernel-doc:: include/linux/printk.h
-   :functions: pr_emerg pr_alert pr_crit pr_err pr_warn pr_notice pr_info
+   :functions: printk pr_emerg pr_alert pr_crit pr_err pr_warn pr_notice pr_info
       pr_fmt pr_debug pr_devel pr_cont
diff --git a/include/linux/printk.h b/include/linux/printk.h
index 2651b82ed352..c1e176403967 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -431,6 +431,30 @@ struct pi_entry {
 	})
 
 
+/**
+ * printk - print a kernel message
+ * @fmt: format string
+ *
+ * This is printk(). It can be called from any context. We want it to work.
+ *
+ * If printk indexing is enabled, _printk() is called from printk_index_wrap.
+ * Otherwise, printk is simply #defined to _printk.
+ *
+ * We try to grab the console_lock. If we succeed, it's easy - we log the
+ * output and call the console drivers.  If we fail to get the semaphore, we
+ * place the output into the log buffer and return. The current holder of
+ * the console_sem will notice the new output in console_unlock(); and will
+ * send it to the consoles before releasing the lock.
+ *
+ * One effect of this deferred printing is that code which calls printk() and
+ * then changes console_loglevel may break. This is because console_loglevel
+ * is inspected when the actual printing occurs.
+ *
+ * See also:
+ * printf(3)
+ *
+ * See the vsnprintf() documentation for format string extensions over C99.
+ */
 #define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__)
 #define printk_deferred(fmt, ...)					\
 	printk_index_wrap(_printk_deferred, fmt, ##__VA_ARGS__)
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 9b3bd6e017f1..63176be3b50c 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2186,30 +2186,6 @@ int vprintk_default(const char *fmt, va_list args)
 }
 EXPORT_SYMBOL_GPL(vprintk_default);
 
-/**
- * _printk - print a kernel message
- * @fmt: format string
- *
- * This is _printk(). It can be called from any context. We want it to work.
- *
- * If printk indexing is enabled, _printk() is called from printk_index_wrap.
- * Otherwise, printk is simply #defined to _printk.
- *
- * We try to grab the console_lock. If we succeed, it's easy - we log the
- * output and call the console drivers.  If we fail to get the semaphore, we
- * place the output into the log buffer and return. The current holder of
- * the console_sem will notice the new output in console_unlock(); and will
- * send it to the consoles before releasing the lock.
- *
- * One effect of this deferred printing is that code which calls printk() and
- * then changes console_loglevel may break. This is because console_loglevel
- * is inspected when the actual printing occurs.
- *
- * See also:
- * printf(3)
- *
- * See the vsnprintf() documentation for format string extensions over C99.
- */
 asmlinkage __visible int _printk(const char *fmt, ...)
 {
 	va_list args;
-- 
2.31.1

Re: linux-next: build warning after merge of the printk tree

[Chris Down]

[-- Attachment #1: Type: text/plain, Size: 1010 bytes --]

Chris Down writes:
>+Cc: Jonathan Corbet <corbet@lwn.net>, linux-doc@vger.kernel.org

Well, let's actually Cc them this time...

>Stephen Rothwell writes:
>>After merging the printk tree, today's linux-next build (htmldocs)
>>produced this warning:
>>
>>kernel/printk/printk.c:1: warning: 'printk' not found
>>
>>Introduced by commit
>>
>> 337015573718 ("printk: Userspace format indexing support")
>>
>>I presume that "printk" is referred to elsewhere in the documentation
>>as being in this file.
>
>Hmm, this is an interesting one, because I think we still generally 
>just want to refer to the API as being `printk()`. Changing it all 
>over the place seems wrong. As you'd imagine, there are quite a few 
>references to this name, so it requires a lot of noise all over the 
>docs and inline comments.
>
>Jonathan and other docs folks, how can one tell Sphinx that when it 
>sees printk() it's referring to a function-like macro, or otherwise 
>squelch this reasonably? :-)



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 963 bytes --]

Re: linux-next: build warning after merge of the printk tree

[Chris Down]

[-- Attachment #1: Type: text/plain, Size: 917 bytes --]

+Cc: Jonathan Corbet <corbet@lwn.net>, linux-doc@vger.kernel.org

Stephen Rothwell writes:
>After merging the printk tree, today's linux-next build (htmldocs)
>produced this warning:
>
>kernel/printk/printk.c:1: warning: 'printk' not found
>
>Introduced by commit
>
>  337015573718 ("printk: Userspace format indexing support")
>
>I presume that "printk" is referred to elsewhere in the documentation
>as being in this file.

Hmm, this is an interesting one, because I think we still generally just want 
to refer to the API as being `printk()`. Changing it all over the place seems 
wrong. As you'd imagine, there are quite a few references to this name, so it 
requires a lot of noise all over the docs and inline comments.

Jonathan and other docs folks, how can one tell Sphinx that when it sees 
printk() it's referring to a function-like macro, or otherwise squelch this 
reasonably? :-)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 963 bytes --]

linux-next: build warning after merge of the printk tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 379 bytes --]

Hi all,

After merging the printk tree, today's linux-next build (htmldocs)
produced this warning:

kernel/printk/printk.c:1: warning: 'printk' not found

Introduced by commit

  337015573718 ("printk: Userspace format indexing support")

I presume that "printk" is referred to elsewhere in the documentation
as being in this file.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: linux-next: build warning after merge of the printk tree

[Sergey Senozhatsky]

On (07/10/18 11:19), Petr Mladek wrote:
> suppress_message_printing() is not longer called in console_unlock().
> Therefore it is not longer needed with disabled CONFIG_PRINTK.
> 
> This fixes the warning:
> 
> kernel/printk/printk.c:2033:13: warning: ‘suppress_message_printing’ defined but not used [-Wunused-function]
>  static bool suppress_message_printing(int level) { return false; }
> 
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Suggested-by: Maninder Singh <maninder1.s@samsung.com>
> Signed-off-by: Petr Mladek <pmladek@suse.com>

Acked-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>


Happens all the time to me as well. I'd rather remove CONFIG_PRINTK
at this point ;)


	-ss

Re: linux-next: build warning after merge of the printk tree

[Petr Mladek]

On Tue 2018-07-10 12:46:14, Maninder Singh wrote:
> Hi Petr,
>  
> > Hi all,
>  
> > After merging the printk tree, today's linux-next build (x86_64
> > allnoconfig) produced this warning:
>  
> > kernel/printk/printk.c:2033:13: warning: 'suppress_message_printing' defined but not used [-Wunused-function]
> > static bool suppress_message_printing(int level) { return false; }
>              ^~~~~~~~~~~~~~~~~~~~~~~~~
>  
> seems there is no user for suppress_message_printing when CONFIG_PRINTK is disabled,
> because earlier it was getting used in console_unlock and now we shifted this into vprintk_emit
> which will be disabled on disabling CONFIG_PRINTk.
> 
> should we remove this defination when CONFIG_PRINTK is disabled ?
> 
> > Introduced by commit
> >  375899cddcbb ("printk: make sure to print log on console.")

Grr, I wonder how I missed it. I usually try build with CONFIG_PRINTK
disabled.

Anyway, I have just pushed the following patch into printk.git,
branch for-4.19:


>From 7b83946e61a1e4a080d48c24396aeb18b6366203 Mon Sep 17 00:00:00 2001
From: Petr Mladek <pmladek@suse.com>
Date: Tue, 10 Jul 2018 10:44:53 +0200
Subject: [PATCH] printk: Fix warning about unused suppress_message_printing
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

suppress_message_printing() is not longer called in console_unlock().
Therefore it is not longer needed with disabled CONFIG_PRINTK.

This fixes the warning:

kernel/printk/printk.c:2033:13: warning: ‘suppress_message_printing’ defined but not used [-Wunused-function]
 static bool suppress_message_printing(int level) { return false; }

Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Suggested-by: Maninder Singh <maninder1.s@samsung.com>
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
 kernel/printk/printk.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index fcc1992c040a..e2cb0fc18e2d 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2021,7 +2021,6 @@ static void call_console_drivers(const char *ext_text, size_t ext_len,
 				 const char *text, size_t len) {}
 static size_t msg_print_text(const struct printk_log *msg,
 			     bool syslog, char *buf, size_t size) { return 0; }
-static bool suppress_message_printing(int level) { return false; }
 
 #endif /* CONFIG_PRINTK */
 
-- 
2.13.7

RE: linux-next: build warning after merge of the printk tree

[Maninder Singh]

[-- Attachment #1: Type: text/plain, Size: 736 bytes --]

Hi Petr,
 
> Hi all,
 
> After merging the printk tree, today's linux-next build (x86_64
> allnoconfig) produced this warning:
 
> kernel/printk/printk.c:2033:13: warning: 'suppress_message_printing' defined but not used [-Wunused-function]
> static bool suppress_message_printing(int level) { return false; }
             ^~~~~~~~~~~~~~~~~~~~~~~~~
 
seems there is no user for suppress_message_printing when CONFIG_PRINTK is disabled,
because earlier it was getting used in console_unlock and now we shifted this into vprintk_emit
which will be disabled on disabling CONFIG_PRINTk.

should we remove this defination when CONFIG_PRINTK is disabled ?

> Introduced by commit
>  375899cddcbb ("printk: make sure to print log on console.")

[-- Attachment #2: No File Name 0 --]
[-- Type: application/octet-stream, Size: 499 bytes --]

-----BEGIN PGP SIGNATURE-----

iQEzBAEBCAAdFiEENIC96giZ81tWdLgKAVBC80lX0GwFAltEVfoACgkQAVBC80lX
0GxQTwf9EZwdtrXMLQwD1/qJOadRHs1fyvT5nyUdMbCPctaMkdIm4Nsyx1lai8r/
oFMjjlMqmwO+YqneRwojhv+nAZ77+CtBURnV7k2riazf3mHcrUjHCzfQ2el5Fgsj
6p3Yaowl45+lTVYa9uEZxs59QWWFrZnfPKdDk8jDFeFppG1N3UHr9rQJm9pis6TA
9rZFucyeWAZAe7wMkbpeKPl8whpVUPXxwDgtgIjiotVg5tKluC7RNcQwLzMbBH0o
u2haX3SDfeqOdxi05Weh4QIXKo7kFAyX3os+wyyUPoIv40uWuhLAf1I6WhdPBdbi
gLCiuK5xrnNYmgxURg7moHUQ+0PzDw==
=bSrM
-----END PGP SIGNATURE-----

linux-next: build warning after merge of the printk tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 458 bytes --]

Hi all,

After merging the printk tree, today's linux-next build (x86_64
allnoconfig) produced this warning:

kernel/printk/printk.c:2033:13: warning: 'suppress_message_printing' defined but not used [-Wunused-function]
 static bool suppress_message_printing(int level) { return false; }
             ^~~~~~~~~~~~~~~~~~~~~~~~~

Introduced by commit

  375899cddcbb ("printk: make sure to print log on console.")

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]