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

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

[Stephen Rothwell]

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

Hi all,

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

Documentation/RCU/rcubarrier.rst:205: WARNING: Literal block ends without a blank line; unexpected unindent.

Introduced by commit

  21c2e3909721 ("doc: Update rcubarrier.rst")

-- 
Cheers,
Stephen Rothwell

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

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

[Paul E. McKenney]

On Mon, Nov 07, 2022 at 02:26:41PM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the rcu tree, today's linux-next build (htmldocs)
> produced this warning:
> 
> Documentation/RCU/rcubarrier.rst:205: WARNING: Literal block ends without a blank line; unexpected unindent.
> 
> Introduced by commit
> 
>   21c2e3909721 ("doc: Update rcubarrier.rst")

Huh.  I guess that numbered code samples are not supposed to have more
than nine lines?  Ah well, easy to fix by going back to left-justified
numbers.  I was wondering about that!

Apologies for the hassle!

							Thanx, Paul

[PATCH] Documentation: RCU: use code blocks with autogenerated line (was: Re: linux-next: build warning after merge of the rcu tree)

[Bagas Sanjaya]

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

On Sun, Nov 06, 2022 at 09:02:12PM -0800, Paul E. McKenney wrote:
> On Mon, Nov 07, 2022 at 02:26:41PM +1100, Stephen Rothwell wrote:
> > Hi all,
> > 
> > After merging the rcu tree, today's linux-next build (htmldocs)
> > produced this warning:
> > 
> > Documentation/RCU/rcubarrier.rst:205: WARNING: Literal block ends without a blank line; unexpected unindent.
> > 
> > Introduced by commit
> > 
> >   21c2e3909721 ("doc: Update rcubarrier.rst")
> 
> Huh.  I guess that numbered code samples are not supposed to have more
> than nine lines?  Ah well, easy to fix by going back to left-justified
> numbers.  I was wondering about that!
> 

I think the proper fix is just let Sphinx generates line number:

---- >8 ----

From 5cea54b61b2dbf2ec5e00479c6c8f9e190b49091 Mon Sep 17 00:00:00 2001
From: Bagas Sanjaya <bagasdotme@gmail.com>
Date: Mon, 7 Nov 2022 15:41:31 +0700
Subject: [PATCH] Documentation: RCU: use code blocks with autogenerated line
 numbers in RCU barrier doc

Recently Stephen Rothwell reported htmldocs warning in
Documentation/RCU/rcubarrier.rst when merging rcu tree [1], which has been
fixed by left-justifying the culprit line numbers. However, similar
issues can occur when line numbers are manually added without caution.

Instead, use code-block syntax with :linenos: option, which automatically
generates line numbers in code snippets.

Link: https://lore.kernel.org/linux-next/20221107142641.527396ea@canb.auug.org.au/ [1]
Signed-off-by: Bagas Sanjaya <bagasdotme@gmail.com>
---
 Documentation/RCU/rcubarrier.rst | 213 +++++++++++++++++--------------
 1 file changed, 114 insertions(+), 99 deletions(-)

diff --git a/Documentation/RCU/rcubarrier.rst b/Documentation/RCU/rcubarrier.rst
index 5a643e5233d5f6..79adf39838653e 100644
--- a/Documentation/RCU/rcubarrier.rst
+++ b/Documentation/RCU/rcubarrier.rst
@@ -70,81 +70,87 @@ If your module uses multiple srcu_struct structures, then it must also
 use multiple invocations of srcu_barrier() when unloading that module.
 For example, if it uses call_rcu(), call_srcu() on srcu_struct_1, and
 call_srcu() on srcu_struct_2, then the following three lines of code
-will be required when unloading::
+will be required when unloading:
 
- 1 rcu_barrier();
- 2 srcu_barrier(&srcu_struct_1);
- 3 srcu_barrier(&srcu_struct_2);
+.. code-block:: c
+   :linenos:
+
+   rcu_barrier();
+   srcu_barrier(&srcu_struct_1);
+   srcu_barrier(&srcu_struct_2);
 
 If latency is of the essence, workqueues could be used to run these
 three functions concurrently.
 
 An ancient version of the rcutorture module makes use of rcu_barrier()
-in its exit function as follows::
+in its exit function as follows:
 
- 1  static void
- 2  rcu_torture_cleanup(void)
- 3  {
- 4    int i;
- 5
- 6    fullstop = 1;
- 7    if (shuffler_task != NULL) {
- 8     VERBOSE_PRINTK_STRING("Stopping rcu_torture_shuffle task");
- 9     kthread_stop(shuffler_task);
- 10   }
- 11   shuffler_task = NULL;
- 12
- 13   if (writer_task != NULL) {
- 14     VERBOSE_PRINTK_STRING("Stopping rcu_torture_writer task");
- 15     kthread_stop(writer_task);
- 16   }
- 17   writer_task = NULL;
- 18
- 19   if (reader_tasks != NULL) {
- 20     for (i = 0; i < nrealreaders; i++) {
- 21       if (reader_tasks[i] != NULL) {
- 22         VERBOSE_PRINTK_STRING(
- 23           "Stopping rcu_torture_reader task");
- 24         kthread_stop(reader_tasks[i]);
- 25       }
- 26       reader_tasks[i] = NULL;
- 27     }
- 28     kfree(reader_tasks);
- 29     reader_tasks = NULL;
- 30   }
- 31   rcu_torture_current = NULL;
- 32
- 33   if (fakewriter_tasks != NULL) {
- 34     for (i = 0; i < nfakewriters; i++) {
- 35       if (fakewriter_tasks[i] != NULL) {
- 36         VERBOSE_PRINTK_STRING(
- 37           "Stopping rcu_torture_fakewriter task");
- 38         kthread_stop(fakewriter_tasks[i]);
- 39       }
- 40       fakewriter_tasks[i] = NULL;
- 41     }
- 42     kfree(fakewriter_tasks);
- 43     fakewriter_tasks = NULL;
- 44   }
- 45
- 46   if (stats_task != NULL) {
- 47     VERBOSE_PRINTK_STRING("Stopping rcu_torture_stats task");
- 48     kthread_stop(stats_task);
- 49   }
- 50   stats_task = NULL;
- 51
- 52   /* Wait for all RCU callbacks to fire. */
- 53   rcu_barrier();
- 54
- 55   rcu_torture_stats_print(); /* -After- the stats thread is stopped! */
- 56
- 57   if (cur_ops->cleanup != NULL)
- 58     cur_ops->cleanup();
- 59   if (atomic_read(&n_rcu_torture_error))
- 60     rcu_torture_print_module_parms("End of test: FAILURE");
- 61   else
- 62     rcu_torture_print_module_parms("End of test: SUCCESS");
- 63 }
+.. code-block:: c
+   :linenos:
+
+   static void
+   rcu_torture_cleanup(void)
+   {
+     int i;
+   
+     fullstop = 1;
+     if (shuffler_task != NULL) {
+      VERBOSE_PRINTK_STRING("Stopping rcu_torture_shuffle task");
+      kthread_stop(shuffler_task);
+     }
+     shuffler_task = NULL;
+   
+     if (writer_task != NULL) {
+       VERBOSE_PRINTK_STRING("Stopping rcu_torture_writer task");
+       kthread_stop(writer_task);
+     }
+     writer_task = NULL;
+   
+     if (reader_tasks != NULL) {
+       for (i = 0; i < nrealreaders; i++) {
+         if (reader_tasks[i] != NULL) {
+           VERBOSE_PRINTK_STRING(
+             "Stopping rcu_torture_reader task");
+           kthread_stop(reader_tasks[i]);
+         }
+         reader_tasks[i] = NULL;
+       }
+       kfree(reader_tasks);
+       reader_tasks = NULL;
+     }
+     rcu_torture_current = NULL;
+   
+     if (fakewriter_tasks != NULL) {
+       for (i = 0; i < nfakewriters; i++) {
+         if (fakewriter_tasks[i] != NULL) {
+           VERBOSE_PRINTK_STRING(
+             "Stopping rcu_torture_fakewriter task");
+           kthread_stop(fakewriter_tasks[i]);
+         }
+         fakewriter_tasks[i] = NULL;
+       }
+       kfree(fakewriter_tasks);
+       fakewriter_tasks = NULL;
+     }
+   
+     if (stats_task != NULL) {
+       VERBOSE_PRINTK_STRING("Stopping rcu_torture_stats task");
+       kthread_stop(stats_task);
+     }
+     stats_task = NULL;
+   
+     /* Wait for all RCU callbacks to fire. */
+     rcu_barrier();
+   
+     rcu_torture_stats_print(); /* -After- the stats thread is stopped! */
+   
+     if (cur_ops->cleanup != NULL)
+       cur_ops->cleanup();
+     if (atomic_read(&n_rcu_torture_error))
+       rcu_torture_print_module_parms("End of test: FAILURE");
+     else
+       rcu_torture_print_module_parms("End of test: SUCCESS");
+   }
 
 Line 6 sets a global variable that prevents any RCU callbacks from
 re-posting themselves. This will not be necessary in most cases, since
@@ -191,21 +197,24 @@ queues. His implementation queues an RCU callback on each of the per-CPU
 callback queues, and then waits until they have all started executing, at
 which point, all earlier RCU callbacks are guaranteed to have completed.
 
-The original code for rcu_barrier() was roughly as follows::
+The original code for rcu_barrier() was roughly as follows:
 
- 1   void rcu_barrier(void)
- 2   {
- 3     BUG_ON(in_interrupt());
- 4     /* Take cpucontrol mutex to protect against CPU hotplug */
- 5     mutex_lock(&rcu_barrier_mutex);
- 6     init_completion(&rcu_barrier_completion);
- 7     atomic_set(&rcu_barrier_cpu_count, 1);
- 8     on_each_cpu(rcu_barrier_func, NULL, 0, 1);
- 9     if (atomic_dec_and_test(&rcu_barrier_cpu_count))
- 10       complete(&rcu_barrier_completion);
- 11    wait_for_completion(&rcu_barrier_completion);
- 12    mutex_unlock(&rcu_barrier_mutex);
- 13  }
+.. code-block:: c
+   :linenos:
+
+   void rcu_barrier(void)
+   {
+     BUG_ON(in_interrupt());
+     /* Take cpucontrol mutex to protect against CPU hotplug */
+     mutex_lock(&rcu_barrier_mutex);
+     init_completion(&rcu_barrier_completion);
+     atomic_set(&rcu_barrier_cpu_count, 1);
+     on_each_cpu(rcu_barrier_func, NULL, 0, 1);
+     if (atomic_dec_and_test(&rcu_barrier_cpu_count))
+        complete(&rcu_barrier_completion);
+     wait_for_completion(&rcu_barrier_completion);
+     mutex_unlock(&rcu_barrier_mutex);
+   }
 
 Line 3 verifies that the caller is in process context, and lines 5 and 12
 use rcu_barrier_mutex to ensure that only one rcu_barrier() is using the
@@ -230,18 +239,21 @@ This code was rewritten in 2008 and several times thereafter, but this
 still gives the general idea.
 
 The rcu_barrier_func() runs on each CPU, where it invokes call_rcu()
-to post an RCU callback, as follows::
+to post an RCU callback, as follows:
 
- 1  static void rcu_barrier_func(void *notused)
- 2  {
- 3    int cpu = smp_processor_id();
- 4    struct rcu_data *rdp = &per_cpu(rcu_data, cpu);
- 5    struct rcu_head *head;
- 6
- 7    head = &rdp->barrier;
- 8    atomic_inc(&rcu_barrier_cpu_count);
- 9    call_rcu(head, rcu_barrier_callback);
- 10 }
+.. code-block:: c
+   :linenos:
+
+   static void rcu_barrier_func(void *notused)
+   {
+     int cpu = smp_processor_id();
+     struct rcu_data *rdp = &per_cpu(rcu_data, cpu);
+     struct rcu_head *head;
+   
+     head = &rdp->barrier;
+     atomic_inc(&rcu_barrier_cpu_count);
+     call_rcu(head, rcu_barrier_callback);
+   }
 
 Lines 3 and 4 locate RCU's internal per-CPU rcu_data structure,
 which contains the struct rcu_head that needed for the later call to
@@ -252,13 +264,16 @@ the current CPU's queue.
 
 The rcu_barrier_callback() function simply atomically decrements the
 rcu_barrier_cpu_count variable and finalizes the completion when it
-reaches zero, as follows::
+reaches zero, as follows:
 
- 1 static void rcu_barrier_callback(struct rcu_head *notused)
- 2 {
- 3   if (atomic_dec_and_test(&rcu_barrier_cpu_count))
- 4     complete(&rcu_barrier_completion);
- 5 }
+.. code-block:: c
+   :linenos:
+
+   static void rcu_barrier_callback(struct rcu_head *notused)
+   {
+     if (atomic_dec_and_test(&rcu_barrier_cpu_count))
+       complete(&rcu_barrier_completion);
+   }
 
 .. _rcubarrier_quiz_3:
 

base-commit: 2c9ea2f2e0a56cbf6931992812bffe47506f23d0

Thanks.

-- 
An old man doll... just what I always wanted! - Clara

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

[PATCH] Documentation: RCU: use code blocks with autogenerated line (was: Re: linux-next: build warning after merge of the rcu tree)

[Akira Yokosawa]

Hi Bagas,

On Mon, 7 Nov 2022 16:55:13 +0700, Bagas Sanjaya wrote:
> On Sun, Nov 06, 2022 at 09:02:12PM -0800, Paul E. McKenney wrote:
>> On Mon, Nov 07, 2022 at 02:26:41PM +1100, Stephen Rothwell wrote:
>> > Hi all,
>> > 
>> > After merging the rcu tree, today's linux-next build (htmldocs)
>> > produced this warning:
>> > 
>> > Documentation/RCU/rcubarrier.rst:205: WARNING: Literal block ends without a blank line; unexpected unindent.
>> > 
>> > Introduced by commit
>> > 
>> >   21c2e3909721 ("doc: Update rcubarrier.rst")
>> 
>> Huh.  I guess that numbered code samples are not supposed to have more
>> than nine lines?  Ah well, easy to fix by going back to left-justified
>> numbers.  I was wondering about that!
>> 
> 
> I think the proper fix is just let Sphinx generates line number:

That might be true if all you care about were the generated documents,
but we need to pay attention to readers of .rst files as plain-text.

There are a bunch of references to line numbers in RCU documents.
If explicit line numbers are removed from snippets, such readers need
to count the lines by themselves, which doesn't sound reasonable to me.

If you can put labels to referenced lines within code snippets, auto
generation of line numbers might work, but as far as I know, Sphinx
doesn't provide such a nice feature.

Of course, you can prove me wrong.

          Thanks, Akira 

  

Re: [PATCH] Documentation: RCU: use code blocks with autogenerated line (was: Re: linux-next: build warning after merge of the rcu tree)

[Paul E. McKenney]

On Mon, Nov 07, 2022 at 08:48:23PM +0900, Akira Yokosawa wrote:
> Hi Bagas,
> 
> On Mon, 7 Nov 2022 16:55:13 +0700, Bagas Sanjaya wrote:
> > On Sun, Nov 06, 2022 at 09:02:12PM -0800, Paul E. McKenney wrote:
> >> On Mon, Nov 07, 2022 at 02:26:41PM +1100, Stephen Rothwell wrote:
> >> > Hi all,
> >> > 
> >> > After merging the rcu tree, today's linux-next build (htmldocs)
> >> > produced this warning:
> >> > 
> >> > Documentation/RCU/rcubarrier.rst:205: WARNING: Literal block ends without a blank line; unexpected unindent.
> >> > 
> >> > Introduced by commit
> >> > 
> >> >   21c2e3909721 ("doc: Update rcubarrier.rst")
> >> 
> >> Huh.  I guess that numbered code samples are not supposed to have more
> >> than nine lines?  Ah well, easy to fix by going back to left-justified
> >> numbers.  I was wondering about that!
> >> 
> > 
> > I think the proper fix is just let Sphinx generates line number:
> 
> That might be true if all you care about were the generated documents,
> but we need to pay attention to readers of .rst files as plain-text.
> 
> There are a bunch of references to line numbers in RCU documents.
> If explicit line numbers are removed from snippets, such readers need
> to count the lines by themselves, which doesn't sound reasonable to me.
> 
> If you can put labels to referenced lines within code snippets, auto
> generation of line numbers might work, but as far as I know, Sphinx
> doesn't provide such a nice feature.
> 
> Of course, you can prove me wrong.

I will give Bagas a few days to prove Akira wrong.  ;-)

Either way, thank you both for looking into this!

							Thanx, Paul

Re: [PATCH] Documentation: RCU: use code blocks with autogenerated line (was: Re: linux-next: build warning after merge of the rcu tree)

[Bagas Sanjaya]

On 11/7/22 18:48, Akira Yokosawa wrote:
> That might be true if all you care about were the generated documents,
> but we need to pay attention to readers of .rst files as plain-text.
> 
> There are a bunch of references to line numbers in RCU documents.
> If explicit line numbers are removed from snippets, such readers need
> to count the lines by themselves, which doesn't sound reasonable to me.
> 

I think only rcubarrier.rst have explicit references to line numbers.

Also, besides manual line counting, readers seeing rst sources can deduce
where actually the lines are from explanation of the snippet. Of course
they can make htmldocs and seeing the output if they want.

> If you can put labels to referenced lines within code snippets, auto
> generation of line numbers might work, but as far as I know, Sphinx
> doesn't provide such a nice feature.
> 

There's also :emphasize-lines: option to highlight selected line numbers.

Thanks.

-- 
An old man doll... just what I always wanted! - Clara

Re: [PATCH] Documentation: RCU: use code blocks with autogenerated line (was: Re: linux-next: build warning after merge of the rcu tree)

[Akira Yokosawa]

[Dropping most CCs]

On Tue, 8 Nov 2022 09:29:01 +0700, Bagas Sanjaya wrote:
> On 11/7/22 18:48, Akira Yokosawa wrote:
>> That might be true if all you care about were the generated documents,
>> but we need to pay attention to readers of .rst files as plain-text.
>>
>> There are a bunch of references to line numbers in RCU documents.
>> If explicit line numbers are removed from snippets, such readers need
>> to count the lines by themselves, which doesn't sound reasonable to me.
>>
> 
> I think only rcubarrier.rst have explicit references to line numbers.

Oh, I find such references in (not limited to):

  - Documentation/RCU/Design/Requirements/Requirements.rst
  - Documentation/RCU/Design/Data-Structures/Data-Structures.rst

> 
> Also, besides manual line counting, readers seeing rst sources can deduce
> where actually the lines are from explanation of the snippet.

Maybe, maybe not... Deducing may take time.

>                                                               Of course
> they can make htmldocs and seeing the output if they want.

There can be situations where you can't afford such luxuries.

Remember there is a note in Documentation/doc-guide/sphinx.rst
which reads:

  Please don't go overboard with reStructuredText markup. Keep it simple.
  For the most part the documentation should be plain text with just enough
  consistency in formatting that it can be converted to other formats.

My interpretation of above:

  .rst sources should be kept as close to plain-text which can be
  easily understood in dumb terminals, as far as possible.

> 
>> If you can put labels to referenced lines within code snippets, auto
>> generation of line numbers might work, but as far as I know, Sphinx
>> doesn't provide such a nice feature.
>>
> 
> There's also :emphasize-lines: option to highlight selected line numbers.

But that option doesn't do any highlighting while viewing .rst files
as plain-text. What am I missing?

        Thanks, Akira

> 
> Thanks.
> 

Re: [PATCH] Documentation: RCU: use code blocks with autogenerated line (was: Re: linux-next: build warning after merge of the rcu tree)

[Bagas Sanjaya]

On 11/8/22 21:53, Akira Yokosawa wrote:
>> I think only rcubarrier.rst have explicit references to line numbers.
> 
> Oh, I find such references in (not limited to):
> 
>   - Documentation/RCU/Design/Requirements/Requirements.rst
>   - Documentation/RCU/Design/Data-Structures/Data-Structures.rst
> 

Ah, I don't see these above!

> Remember there is a note in Documentation/doc-guide/sphinx.rst
> which reads:
> 
>   Please don't go overboard with reStructuredText markup. Keep it simple.
>   For the most part the documentation should be plain text with just enough
>   consistency in formatting that it can be converted to other formats.
> 
> My interpretation of above:
> 
>   .rst sources should be kept as close to plain-text which can be
>   easily understood in dumb terminals, as far as possible.
> 

Ah! I always forget that. I'm leaning towards *abusing* the markup,
though.

Thanks for the reminder.

-- 
An old man doll... just what I always wanted! - Clara

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

[Paul E. McKenney]

On Thu, Jan 25, 2024 at 02:33:32PM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the rcu tree, today's linux-next build (htmldocs) produced
> this warning:
> 
> include/linux/hrtimer.h:199: warning: Function parameter or struct member 'online' not described in 'hrtimer_cpu_base'
> 
> Introduced by commit
> 
>   7525a3cbb106 ("hrtimer: Report offline hrtimer enqueue")

Apologies for the hassle, and please see the proposed fixup patch below.
Unless I hear otherwise, I will merge this into the original before the
likely start of your day.

							Thanx, Paul

------------------------------------------------------------------------

diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h
index 4f2cf7309486..991c83e929b4 100644
--- a/include/linux/hrtimer.h
+++ b/include/linux/hrtimer.h
@@ -151,6 +151,7 @@ enum  hrtimer_base_type {
  * @hang_detected:	The last hrtimer interrupt detected a hang
  * @softirq_activated:	displays, if the softirq is raised - update of softirq
  *			related settings is not required then.
+ * @online:		CPU is online from an hrtimers viewpoint
  * @nr_events:		Total number of hrtimer interrupt events
  * @nr_retries:		Total number of hrtimer interrupt retries
  * @nr_hangs:		Total number of hrtimer interrupt hangs

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

[Stephen Rothwell]

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

Hi all,

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

include/linux/hrtimer.h:199: warning: Function parameter or struct member 'online' not described in 'hrtimer_cpu_base'

Introduced by commit

  7525a3cbb106 ("hrtimer: Report offline hrtimer enqueue")

-- 
Cheers,
Stephen Rothwell

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

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

[Stephen Rothwell]

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

Hi Paul,

On Tue, 25 Jul 2023 20:48:13 -0700 "Paul E. McKenney" <paulmck@kernel.org> wrote:
>
> Does the following incremental diff (to be squashed into the original) help?
> 
> 							Thanx, Paul
> 
> ------------------------------------------------------------------------
> 
> diff --git a/init/main.c b/init/main.c
> index c946ab87783a..981170da0b1c 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -135,7 +135,7 @@ EXPORT_SYMBOL(system_state);
>  void (*__initdata late_time_init)(void);
>  
>  /* Untouched command line saved by arch-specific code. */
> -char __initdata boot_command_line[COMMAND_LINE_SIZE];
> +char boot_command_line[COMMAND_LINE_SIZE] __ro_after_init;
>  /* Untouched saved command line (eg. for /proc) */
>  char *saved_command_line __ro_after_init;
>  unsigned int saved_command_line_len __ro_after_init;

I needed the following (only tested x86_64 allmodconfig build):

diff --git a/include/linux/init.h b/include/linux/init.h
index 9a5973324072..e3ce68988e1b 100644
--- a/include/linux/init.h
+++ b/include/linux/init.h
@@ -112,6 +112,9 @@
 #define __REFCONST       .section       ".ref.rodata", "a"
 
 #ifndef __ASSEMBLY__
+
+#include <linux/cache.h>
+
 /*
  * Used for initialization calls..
  */
@@ -143,7 +146,7 @@ struct file_system_type;
 
 /* Defined in init/main.c */
 extern int do_one_initcall(initcall_t fn);
-extern char __initdata boot_command_line[];
+extern char boot_command_line[] __ro_after_init;
 extern char *saved_command_line;
 extern unsigned int saved_command_line_len;
 extern unsigned int reset_devices;
diff --git a/init/main.c b/init/main.c
index c946ab87783a..981170da0b1c 100644
--- a/init/main.c
+++ b/init/main.c
@@ -135,7 +135,7 @@ EXPORT_SYMBOL(system_state);
 void (*__initdata late_time_init)(void);
 
 /* Untouched command line saved by arch-specific code. */
-char __initdata boot_command_line[COMMAND_LINE_SIZE];
+char boot_command_line[COMMAND_LINE_SIZE] __ro_after_init;
 /* Untouched saved command line (eg. for /proc) */
 char *saved_command_line __ro_after_init;
 unsigned int saved_command_line_len __ro_after_init;

-- 
Cheers,
Stephen Rothwell

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

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

[Paul E. McKenney]

On Tue, Jul 25, 2023 at 08:33:06PM -0700, Paul E. McKenney wrote:
> On Wed, Jul 26, 2023 at 12:32:30PM +1000, Stephen Rothwell wrote:
> > Hi all,
> > 
> > After merging the rcu tree, today's linux-next build (x86_64 allmodconfig)
> > produced this warning:
> > 
> > WARNING: modpost: vmlinux: section mismatch in reference: cmdline_load_proc_show+0x22 (section: .text) -> boot_command_line (section: .init.data)
> > 
> > Introduced by commit
> > 
> >   cf9eca90a339 ("fs/proc: Add /proc/cmdline_load for boot loader arguments")
> 
> That __initdata needs to be __ro_after_init, doesn't it?  Will fix,
> thank you!

Sigh.  Hit "send" too soon.  :-/

Does the following incremental diff (to be squashed into the original) help?

							Thanx, Paul

------------------------------------------------------------------------

diff --git a/init/main.c b/init/main.c
index c946ab87783a..981170da0b1c 100644
--- a/init/main.c
+++ b/init/main.c
@@ -135,7 +135,7 @@ EXPORT_SYMBOL(system_state);
 void (*__initdata late_time_init)(void);
 
 /* Untouched command line saved by arch-specific code. */
-char __initdata boot_command_line[COMMAND_LINE_SIZE];
+char boot_command_line[COMMAND_LINE_SIZE] __ro_after_init;
 /* Untouched saved command line (eg. for /proc) */
 char *saved_command_line __ro_after_init;
 unsigned int saved_command_line_len __ro_after_init;

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

[Paul E. McKenney]

On Wed, Jul 26, 2023 at 12:32:30PM +1000, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the rcu tree, today's linux-next build (x86_64 allmodconfig)
> produced this warning:
> 
> WARNING: modpost: vmlinux: section mismatch in reference: cmdline_load_proc_show+0x22 (section: .text) -> boot_command_line (section: .init.data)
> 
> Introduced by commit
> 
>   cf9eca90a339 ("fs/proc: Add /proc/cmdline_load for boot loader arguments")

That __initdata needs to be __ro_after_init, doesn't it?  Will fix,
thank you!

							Thanx, Paul

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

[Stephen Rothwell]

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

Hi all,

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

WARNING: modpost: vmlinux: section mismatch in reference: cmdline_load_proc_show+0x22 (section: .text) -> boot_command_line (section: .init.data)

Introduced by commit

  cf9eca90a339 ("fs/proc: Add /proc/cmdline_load for boot loader arguments")

-- 
Cheers,
Stephen Rothwell

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

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

[Paul E. McKenney]

On Thu, Apr 06, 2023 at 02:43:46PM +1000, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the rcu tree, today's linux-next build (htmldocs) produced
> this warning:
> 
> kernel/rcu/tree.c:2808: warning: Function parameter or member 'head_free_gp_snap' not described in 'kfree_rcu_cpu_work'
> 
> Introduced by commit
> 
>   07030d20c8ec ("rcu/kvfree: Add debug check for GP complete for kfree_rcu_cpu list")

Will fix, thank you!

							Thanx, Paul

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

[Stephen Rothwell]

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

Hi all,

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

kernel/rcu/tree.c:2808: warning: Function parameter or member 'head_free_gp_snap' not described in 'kfree_rcu_cpu_work'

Introduced by commit

  07030d20c8ec ("rcu/kvfree: Add debug check for GP complete for kfree_rcu_cpu list")

-- 
Cheers,
Stephen Rothwell

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

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

[Boqun Feng]

On Tue, Mar 21, 2023 at 01:50:47PM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the rcu tree, today's linux-next build (htmldocs) produced
> this warning:
> 
> include/linux/srcu.h:106: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst
>  * Annotations provide deadlock detection for SRCU.

Thanks for reporting. Fix it locally.

Regards,
Boqun

> 
> Introduced by commit
> 
>   57e98b73317b ("rcu: Annotate SRCU's update-side lockdep dependencies")
> 
> -- 
> Cheers,
> Stephen Rothwell

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

[Stephen Rothwell]

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

Hi all,

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

include/linux/srcu.h:106: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst
 * Annotations provide deadlock detection for SRCU.

Introduced by commit

  57e98b73317b ("rcu: Annotate SRCU's update-side lockdep dependencies")

-- 
Cheers,
Stephen Rothwell

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

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

[Paul E. McKenney]

On Wed, Jun 15, 2022 at 03:38:02PM +1000, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the rcu tree, today's linux-next build (htmldocs) produced
> this warning:
> 
> Documentation/RCU/Design/Requirements/Requirements.rst:2220: WARNING: Malformed table.
> 
> +-----------------------------------------------------------------------+
> | **Quick Quiz**:                                                       |
> +-----------------------------------------------------------------------+
> | But what if my driver has a hardware interrupt handler that can run   |
> | for many seconds? I cannot invoke schedule() from an hardware         |
> | interrupt handler, after all!                                         |
> +-----------------------------------------------------------------------+
> | **Answer**:                                                           |
> +-----------------------------------------------------------------------+
> | One approach is to do ``ct_irq_exit();ct_irq_enter();`` every so    |
> | often. But given that long-running interrupt handlers can cause other |
> | problems, not least for response time, shouldn't you work to keep     |
> | your interrupt handler's runtime within reasonable bounds?            |
> +-----------------------------------------------------------------------+
> 
> Introduced by commit
> 
>   6c5218715286 ("context_tracking: Take IRQ eqs entrypoints over RCU")

Apologies and thank you, will fix.

							Thanx, Paul

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

[Stephen Rothwell]

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

Hi all,

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

Documentation/RCU/Design/Requirements/Requirements.rst:2220: WARNING: Malformed table.

+-----------------------------------------------------------------------+
| **Quick Quiz**:                                                       |
+-----------------------------------------------------------------------+
| But what if my driver has a hardware interrupt handler that can run   |
| for many seconds? I cannot invoke schedule() from an hardware         |
| interrupt handler, after all!                                         |
+-----------------------------------------------------------------------+
| **Answer**:                                                           |
+-----------------------------------------------------------------------+
| One approach is to do ``ct_irq_exit();ct_irq_enter();`` every so    |
| often. But given that long-running interrupt handlers can cause other |
| problems, not least for response time, shouldn't you work to keep     |
| your interrupt handler's runtime within reasonable bounds?            |
+-----------------------------------------------------------------------+

Introduced by commit

  6c5218715286 ("context_tracking: Take IRQ eqs entrypoints over RCU")

-- 
Cheers,
Stephen Rothwell

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

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

[Paul E. McKenney]

On Thu, Mar 04, 2021 at 12:41:05PM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the rcu tree, today's linux-next build (htmldocs) produced
> this warning:
> 
> kernel/rcu/tree.c:3824: warning: expecting prototype for start_poll_state_synchronize_rcu(). Prototype was for start_poll_synchronize_rcu() instead
> 
> Introduced by commit
> 
>   7f9a26bbfff2 ("rcu: Provide polling interfaces for Tree RCU grace periods")

Apologies and thank you, will fix!

							Thanx, Paul

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

[Stephen Rothwell]

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

Hi all,

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

kernel/rcu/tree.c:3824: warning: expecting prototype for start_poll_state_synchronize_rcu(). Prototype was for start_poll_synchronize_rcu() instead

Introduced by commit

  7f9a26bbfff2 ("rcu: Provide polling interfaces for Tree RCU grace periods")

-- 
Cheers,
Stephen Rothwell

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

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

[Paul E. McKenney]

On Mon, Dec 07, 2020 at 10:48:51AM -0700, Jonathan Corbet wrote:
> On Mon, 7 Dec 2020 08:47:04 -0800
> "Paul E. McKenney" <paulmck@kernel.org> wrote:
> 
> > I freely confess that I have absolutely no idea what it doesn't like.
> > It is complaining about this header comment, correct?
> > 
> > /**
> >  * kmem_last_alloc_stack - Get return address and stack for last allocation
> >  * @object: object for which to find last-allocation return address.
> >  * @stackp: %NULL or pointer to location to place return-address stack.
> >  * @nstackp: maximum number of return addresses that may be stored.
> >  *
> >  * If the pointer references a slab-allocated object and if sufficient
> >  * debugging is enabled, return the return address for the corresponding
> >  * allocation.  If stackp is non-%NULL in %CONFIG_STACKTRACE kernels running
> >  * the slub allocator, also copy the return-address stack into @stackp,
> >  * limited by @nstackp.  Otherwise, return %NULL or an appropriate error
> >  * code using %ERR_PTR().
> >  *
> >  * Return: return address from last allocation, %NULL or negative error code.
> >  */
> 
> The problem is the %ERR_PTR().  I'm honestly not quite sure why, Sphinx is
> being a little weird there.  But in any case the % notation is supposed to
> mark a constant, which is not the case here.  I'd just take the % signs
> out.

Thank you, will do!

							Thanx, Paul

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

[Jonathan Corbet]

On Mon, 7 Dec 2020 08:47:04 -0800
"Paul E. McKenney" <paulmck@kernel.org> wrote:

> I freely confess that I have absolutely no idea what it doesn't like.
> It is complaining about this header comment, correct?
> 
> /**
>  * kmem_last_alloc_stack - Get return address and stack for last allocation
>  * @object: object for which to find last-allocation return address.
>  * @stackp: %NULL or pointer to location to place return-address stack.
>  * @nstackp: maximum number of return addresses that may be stored.
>  *
>  * If the pointer references a slab-allocated object and if sufficient
>  * debugging is enabled, return the return address for the corresponding
>  * allocation.  If stackp is non-%NULL in %CONFIG_STACKTRACE kernels running
>  * the slub allocator, also copy the return-address stack into @stackp,
>  * limited by @nstackp.  Otherwise, return %NULL or an appropriate error
>  * code using %ERR_PTR().
>  *
>  * Return: return address from last allocation, %NULL or negative error code.
>  */

The problem is the %ERR_PTR().  I'm honestly not quite sure why, Sphinx is
being a little weird there.  But in any case the % notation is supposed to
mark a constant, which is not the case here.  I'd just take the % signs
out.

jon

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

[Paul E. McKenney]

On Mon, Dec 07, 2020 at 07:20:28PM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the rcu tree, today's linux-next build (htmldocs) produced
> this warning:
> 
> Documentation/core-api/mm-api:49: mm/slab_common.c:569: WARNING: Inline literal start-string without end-string.
> Documentation/core-api/mm-api:49: mm/slab_common.c:595: WARNING: Inline literal start-string without end-string.
> 
> Maybe introduced by commit
> 
>   f7c3fb4fc476 ("mm: Add kmem_last_alloc() to return last allocation for memory block")
> 
> (or one of the following ones).

I freely confess that I have absolutely no idea what it doesn't like.
It is complaining about this header comment, correct?

/**
 * kmem_last_alloc_stack - Get return address and stack for last allocation
 * @object: object for which to find last-allocation return address.
 * @stackp: %NULL or pointer to location to place return-address stack.
 * @nstackp: maximum number of return addresses that may be stored.
 *
 * If the pointer references a slab-allocated object and if sufficient
 * debugging is enabled, return the return address for the corresponding
 * allocation.  If stackp is non-%NULL in %CONFIG_STACKTRACE kernels running
 * the slub allocator, also copy the return-address stack into @stackp,
 * limited by @nstackp.  Otherwise, return %NULL or an appropriate error
 * code using %ERR_PTR().
 *
 * Return: return address from last allocation, %NULL or negative error code.
 */

							Thanx, Paul

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

[Stephen Rothwell]

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

Hi all,

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

Documentation/core-api/mm-api:49: mm/slab_common.c:569: WARNING: Inline literal start-string without end-string.
Documentation/core-api/mm-api:49: mm/slab_common.c:595: WARNING: Inline literal start-string without end-string.

Maybe introduced by commit

  f7c3fb4fc476 ("mm: Add kmem_last_alloc() to return last allocation for memory block")

(or one of the following ones).

-- 
Cheers,
Stephen Rothwell

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

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

[Paul E. McKenney]

On Tue, Mar 10, 2020 at 01:27:09PM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the rcu tree, today's linux-next build (x86_64 allmodconfig)
> produced this warning:
> 
> WARNING: "rcu_tasks_rude_wait_gp" [vmlinux] is a static EXPORT_SYMBOL_GPL

Good catch, will fix, thank you!

> Introduced by commit
> 
>   cbd703932774 ("rcu-tasks: Add an RCU-tasks rude variant")
> 
> Why is all that code in a header file?  Do you intend to have those
> functions defined in each C file that includes kernel/rcu/tasks.h?

Same setup as for tree_plugin.h, tree_exp.h, tree_stall.h, and so in
in that same directory.  Plugins of various sorts.

							Thanx, Paul

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

[Stephen Rothwell]

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

Hi all,

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

WARNING: "rcu_tasks_rude_wait_gp" [vmlinux] is a static EXPORT_SYMBOL_GPL

Introduced by commit

  cbd703932774 ("rcu-tasks: Add an RCU-tasks rude variant")

Why is all that code in a header file?  Do you intend to have those
functions defined in each C file that includes kernel/rcu/tasks.h?

-- 
Cheers,
Stephen Rothwell

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

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

[Paul E. McKenney]

On Wed, Dec 11, 2019 at 10:57:24PM -0800, Eric Dumazet wrote:
> On Wed, Dec 11, 2019 at 10:38 PM Eric Dumazet <edumazet@google.com> wrote:
> > On Wed, Dec 11, 2019 at 10:02 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> > > On Thu, Dec 12, 2019 at 04:06:22PM +1100, Stephen Rothwell wrote:
> > > > Hi all,
> > > >
> > > > After merging the rcu (I think) tree, today's linux-next build (x86_64
> > > > allnoconfig) produced this warning:
> > > >
> > > > kernel/time/timer.c: In function 'schedule_timeout':
> > > > kernel/time/timer.c:969:20: warning: 'timer.expires' may be used uninitialized in this function [-Wmaybe-uninitialized]
> > > >   969 |   long diff = timer->expires - expires;
> > > >       |               ~~~~~^~~~~~~~~
> > > >
> > > > Introduced by (bisected to) commit
> > > >
> > > >   c4127fce1d02 ("timer: Use hlist_unhashed_lockless() in timer_pending()")
> > > >
> > > > x86_64-linux-gnu-gcc (Debian 9.2.1-21) 9.2.1 20191130
> > >
> > > Well, if the timer is pending, then ->expires has to have been
> > > initialized, but off where the compiler cannot see it, such as during a
> > > previous call to __mod_timer().  And the change may have made it harder
> > > for the compiler to see all of these relationships, but...
> > >
> > > I don't see this warning with gcc version 7.4.0.  Just out of curiosity,
> > > what are you running, Stephen?
> > >
> > > Eric, any thoughts for properly educating the compiler on this one?
> >
> > Ah... the READ_ONCE() apparently turns off the compiler ability to
> > infer that this branch should not be taken.
> >
> > Since __mod_timer() is inlined we could perhaps add a new option
> >
> > diff --git a/kernel/time/timer.c b/kernel/time/timer.c
> > index 4820823515e9..8bbce552568b 100644
> > --- a/kernel/time/timer.c
> > +++ b/kernel/time/timer.c
> > @@ -944,6 +944,7 @@ static struct timer_base *lock_timer_base(struct
> > timer_list *timer,
> >
> >  #define MOD_TIMER_PENDING_ONLY         0x01
> >  #define MOD_TIMER_REDUCE               0x02
> > +#define MOD_TIMER_NOTPENDING           0x04
> >
> >  static inline int
> >  __mod_timer(struct timer_list *timer, unsigned long expires, unsigned
> > int options)
> > @@ -960,7 +961,7 @@ __mod_timer(struct timer_list *timer, unsigned
> > long expires, unsigned int option
> >          * the timer is re-modified to have the same timeout or ends up in the
> >          * same array bucket then just return:
> >          */
> > -       if (timer_pending(timer)) {
> > +       if (!(options & MOD_TIMER_NOTPENDING) && timer_pending(timer)) {
> >                 /*
> >                  * The downside of this optimization is that it can result in
> >                  * larger granularity than you would get from adding a new
> > @@ -1891,7 +1892,7 @@ signed long __sched schedule_timeout(signed long timeout)
> >
> >         timer.task = current;
> >         timer_setup_on_stack(&timer.timer, process_timeout, 0);
> > -       __mod_timer(&timer.timer, expire, 0);
> > +       __mod_timer(&timer.timer, expire, MOD_TIMER_NOTPENDING);
> >         schedule();
> >         del_singleshot_timer_sync(&timer.timer);
> 
> 
> Also add_timer() can benefit from the same hint, since it seems inlined as well.
> 
> (untested patch)

Apologies for the delay, fat fingers and holidays...  :-/

I folded this into your earlier patch, resulting in the patch at the
end.  Could you please let me know whether this matches your intent?

							Thanx, Paul

> diff --git a/kernel/time/timer.c b/kernel/time/timer.c
> index 4820823515e9..568564ae3597 100644
> --- a/kernel/time/timer.c
> +++ b/kernel/time/timer.c
> @@ -944,6 +944,7 @@ static struct timer_base *lock_timer_base(struct
> timer_list *timer,
> 
>  #define MOD_TIMER_PENDING_ONLY         0x01
>  #define MOD_TIMER_REDUCE               0x02
> +#define MOD_TIMER_NOTPENDING           0x04
> 
>  static inline int
>  __mod_timer(struct timer_list *timer, unsigned long expires, unsigned
> int options)
> @@ -960,7 +961,7 @@ __mod_timer(struct timer_list *timer, unsigned
> long expires, unsigned int option
>          * the timer is re-modified to have the same timeout or ends up in the
>          * same array bucket then just return:
>          */
> -       if (timer_pending(timer)) {
> +       if (!(options & MOD_TIMER_NOTPENDING) && timer_pending(timer)) {
>                 /*
>                  * The downside of this optimization is that it can result in
>                  * larger granularity than you would get from adding a new
> @@ -1133,7 +1134,7 @@ EXPORT_SYMBOL(timer_reduce);
>  void add_timer(struct timer_list *timer)
>  {
>         BUG_ON(timer_pending(timer));
> -       mod_timer(timer, timer->expires);
> +       __mod_timer(timer, timer->expires, MOD_TIMER_NOTPENDING);
>  }
>  EXPORT_SYMBOL(add_timer);
> 
> @@ -1891,7 +1892,7 @@ signed long __sched schedule_timeout(signed long timeout)
> 
>         timer.task = current;
>         timer_setup_on_stack(&timer.timer, process_timeout, 0);
> -       __mod_timer(&timer.timer, expire, 0);
> +       __mod_timer(&timer.timer, expire, MOD_TIMER_NOTPENDING);
>         schedule();
>         del_singleshot_timer_sync(&timer.timer);
> 
------------------------------------------------------------------------

commit 704c46852c8f8c15cc0ecb45b19f8d3cd983faa6
Author: Eric Dumazet <edumazet@google.com>
Date:   Thu Nov 7 11:37:38 2019 -0800

    timer: Use hlist_unhashed_lockless() in timer_pending()
    
    The timer_pending() function is mostly used in lockless contexts, so
    Without proper annotations, KCSAN might detect a data-race [1].
    
    Using hlist_unhashed_lockless() instead of hand-coding it seems
    appropriate (as suggested by Paul E. McKenney).
    
    [1]
    
    BUG: KCSAN: data-race in del_timer / detach_if_pending
    
    write to 0xffff88808697d870 of 8 bytes by task 10 on cpu 0:
     __hlist_del include/linux/list.h:764 [inline]
     detach_timer kernel/time/timer.c:815 [inline]
     detach_if_pending+0xcd/0x2d0 kernel/time/timer.c:832
     try_to_del_timer_sync+0x60/0xb0 kernel/time/timer.c:1226
     del_timer_sync+0x6b/0xa0 kernel/time/timer.c:1365
     schedule_timeout+0x2d2/0x6e0 kernel/time/timer.c:1896
     rcu_gp_fqs_loop+0x37c/0x580 kernel/rcu/tree.c:1639
     rcu_gp_kthread+0x143/0x230 kernel/rcu/tree.c:1799
     kthread+0x1d4/0x200 drivers/block/aoe/aoecmd.c:1253
     ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:352
    
    read to 0xffff88808697d870 of 8 bytes by task 12060 on cpu 1:
     del_timer+0x3b/0xb0 kernel/time/timer.c:1198
     sk_stop_timer+0x25/0x60 net/core/sock.c:2845
     inet_csk_clear_xmit_timers+0x69/0xa0 net/ipv4/inet_connection_sock.c:523
     tcp_clear_xmit_timers include/net/tcp.h:606 [inline]
     tcp_v4_destroy_sock+0xa3/0x3f0 net/ipv4/tcp_ipv4.c:2096
     inet_csk_destroy_sock+0xf4/0x250 net/ipv4/inet_connection_sock.c:836
     tcp_close+0x6f3/0x970 net/ipv4/tcp.c:2497
     inet_release+0x86/0x100 net/ipv4/af_inet.c:427
     __sock_release+0x85/0x160 net/socket.c:590
     sock_close+0x24/0x30 net/socket.c:1268
     __fput+0x1e1/0x520 fs/file_table.c:280
     ____fput+0x1f/0x30 fs/file_table.c:313
     task_work_run+0xf6/0x130 kernel/task_work.c:113
     tracehook_notify_resume include/linux/tracehook.h:188 [inline]
     exit_to_usermode_loop+0x2b4/0x2c0 arch/x86/entry/common.c:163
    
    Reported by Kernel Concurrency Sanitizer on:
    CPU: 1 PID: 12060 Comm: syz-executor.5 Not tainted 5.4.0-rc3+ #0
    Hardware name: Google Google Compute Engine/Google Compute Engine,
    
    Signed-off-by: Eric Dumazet <edumazet@google.com>
    Cc: Thomas Gleixner <tglx@linutronix.de>
    [ paulmck: Pulled in Eric's later amendments. ]
    Signed-off-by: Paul E. McKenney <paulmck@kernel.org>

diff --git a/include/linux/timer.h b/include/linux/timer.h
index 1e6650e..0dc19a8 100644
--- a/include/linux/timer.h
+++ b/include/linux/timer.h
@@ -164,7 +164,7 @@ static inline void destroy_timer_on_stack(struct timer_list *timer) { }
  */
 static inline int timer_pending(const struct timer_list * timer)
 {
-	return timer->entry.pprev != NULL;
+	return !hlist_unhashed_lockless(&timer->entry);
 }
 
 extern void add_timer_on(struct timer_list *timer, int cpu);
diff --git a/kernel/time/timer.c b/kernel/time/timer.c
index 4820823..568564a 100644
--- a/kernel/time/timer.c
+++ b/kernel/time/timer.c
@@ -944,6 +944,7 @@ static struct timer_base *lock_timer_base(struct timer_list *timer,
 
 #define MOD_TIMER_PENDING_ONLY		0x01
 #define MOD_TIMER_REDUCE		0x02
+#define MOD_TIMER_NOTPENDING		0x04
 
 static inline int
 __mod_timer(struct timer_list *timer, unsigned long expires, unsigned int options)
@@ -960,7 +961,7 @@ __mod_timer(struct timer_list *timer, unsigned long expires, unsigned int option
 	 * the timer is re-modified to have the same timeout or ends up in the
 	 * same array bucket then just return:
 	 */
-	if (timer_pending(timer)) {
+	if (!(options & MOD_TIMER_NOTPENDING) && timer_pending(timer)) {
 		/*
 		 * The downside of this optimization is that it can result in
 		 * larger granularity than you would get from adding a new
@@ -1133,7 +1134,7 @@ EXPORT_SYMBOL(timer_reduce);
 void add_timer(struct timer_list *timer)
 {
 	BUG_ON(timer_pending(timer));
-	mod_timer(timer, timer->expires);
+	__mod_timer(timer, timer->expires, MOD_TIMER_NOTPENDING);
 }
 EXPORT_SYMBOL(add_timer);
 
@@ -1891,7 +1892,7 @@ signed long __sched schedule_timeout(signed long timeout)
 
 	timer.task = current;
 	timer_setup_on_stack(&timer.timer, process_timeout, 0);
-	__mod_timer(&timer.timer, expire, 0);
+	__mod_timer(&timer.timer, expire, MOD_TIMER_NOTPENDING);
 	schedule();
 	del_singleshot_timer_sync(&timer.timer);
 

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

[Paul E. McKenney]

Please accept my apologies for losing track of this one, and for
top-posting to any of you who might be sticklers for that sort of thing.
I must pull this commit out of my set for the next merge window, apply
it to the group for the next merge window, and try out Eric's suggested
changes.  Might still make the next merge window, but clearly not in
its current condition.

If it has taken some other path in the meantime, please do let me know!

							Thanx, Paul

On Wed, Dec 11, 2019 at 10:57:24PM -0800, Eric Dumazet wrote:
> On Wed, Dec 11, 2019 at 10:38 PM Eric Dumazet <edumazet@google.com> wrote:
> >
> > On Wed, Dec 11, 2019 at 10:02 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> > >
> > > On Thu, Dec 12, 2019 at 04:06:22PM +1100, Stephen Rothwell wrote:
> > > > Hi all,
> > > >
> > > > After merging the rcu (I think) tree, today's linux-next build (x86_64
> > > > allnoconfig) produced this warning:
> > > >
> > > > kernel/time/timer.c: In function 'schedule_timeout':
> > > > kernel/time/timer.c:969:20: warning: 'timer.expires' may be used uninitialized in this function [-Wmaybe-uninitialized]
> > > >   969 |   long diff = timer->expires - expires;
> > > >       |               ~~~~~^~~~~~~~~
> > > >
> > > > Introduced by (bisected to) commit
> > > >
> > > >   c4127fce1d02 ("timer: Use hlist_unhashed_lockless() in timer_pending()")
> > > >
> > > > x86_64-linux-gnu-gcc (Debian 9.2.1-21) 9.2.1 20191130
> > >
> > > Well, if the timer is pending, then ->expires has to have been
> > > initialized, but off where the compiler cannot see it, such as during a
> > > previous call to __mod_timer().  And the change may have made it harder
> > > for the compiler to see all of these relationships, but...
> > >
> > > I don't see this warning with gcc version 7.4.0.  Just out of curiosity,
> > > what are you running, Stephen?
> > >
> > > Eric, any thoughts for properly educating the compiler on this one?
> >
> > Ah... the READ_ONCE() apparently turns off the compiler ability to
> > infer that this branch should not be taken.
> >
> > Since __mod_timer() is inlined we could perhaps add a new option
> >
> > diff --git a/kernel/time/timer.c b/kernel/time/timer.c
> > index 4820823515e9..8bbce552568b 100644
> > --- a/kernel/time/timer.c
> > +++ b/kernel/time/timer.c
> > @@ -944,6 +944,7 @@ static struct timer_base *lock_timer_base(struct
> > timer_list *timer,
> >
> >  #define MOD_TIMER_PENDING_ONLY         0x01
> >  #define MOD_TIMER_REDUCE               0x02
> > +#define MOD_TIMER_NOTPENDING           0x04
> >
> >  static inline int
> >  __mod_timer(struct timer_list *timer, unsigned long expires, unsigned
> > int options)
> > @@ -960,7 +961,7 @@ __mod_timer(struct timer_list *timer, unsigned
> > long expires, unsigned int option
> >          * the timer is re-modified to have the same timeout or ends up in the
> >          * same array bucket then just return:
> >          */
> > -       if (timer_pending(timer)) {
> > +       if (!(options & MOD_TIMER_NOTPENDING) && timer_pending(timer)) {
> >                 /*
> >                  * The downside of this optimization is that it can result in
> >                  * larger granularity than you would get from adding a new
> > @@ -1891,7 +1892,7 @@ signed long __sched schedule_timeout(signed long timeout)
> >
> >         timer.task = current;
> >         timer_setup_on_stack(&timer.timer, process_timeout, 0);
> > -       __mod_timer(&timer.timer, expire, 0);
> > +       __mod_timer(&timer.timer, expire, MOD_TIMER_NOTPENDING);
> >         schedule();
> >         del_singleshot_timer_sync(&timer.timer);
> 
> 
> Also add_timer() can benefit from the same hint, since it seems inlined as well.
> 
> (untested patch)
> 
> diff --git a/kernel/time/timer.c b/kernel/time/timer.c
> index 4820823515e9..568564ae3597 100644
> --- a/kernel/time/timer.c
> +++ b/kernel/time/timer.c
> @@ -944,6 +944,7 @@ static struct timer_base *lock_timer_base(struct
> timer_list *timer,
> 
>  #define MOD_TIMER_PENDING_ONLY         0x01
>  #define MOD_TIMER_REDUCE               0x02
> +#define MOD_TIMER_NOTPENDING           0x04
> 
>  static inline int
>  __mod_timer(struct timer_list *timer, unsigned long expires, unsigned
> int options)
> @@ -960,7 +961,7 @@ __mod_timer(struct timer_list *timer, unsigned
> long expires, unsigned int option
>          * the timer is re-modified to have the same timeout or ends up in the
>          * same array bucket then just return:
>          */
> -       if (timer_pending(timer)) {
> +       if (!(options & MOD_TIMER_NOTPENDING) && timer_pending(timer)) {
>                 /*
>                  * The downside of this optimization is that it can result in
>                  * larger granularity than you would get from adding a new
> @@ -1133,7 +1134,7 @@ EXPORT_SYMBOL(timer_reduce);
>  void add_timer(struct timer_list *timer)
>  {
>         BUG_ON(timer_pending(timer));
> -       mod_timer(timer, timer->expires);
> +       __mod_timer(timer, timer->expires, MOD_TIMER_NOTPENDING);
>  }
>  EXPORT_SYMBOL(add_timer);
> 
> @@ -1891,7 +1892,7 @@ signed long __sched schedule_timeout(signed long timeout)
> 
>         timer.task = current;
>         timer_setup_on_stack(&timer.timer, process_timeout, 0);
> -       __mod_timer(&timer.timer, expire, 0);
> +       __mod_timer(&timer.timer, expire, MOD_TIMER_NOTPENDING);
>         schedule();
>         del_singleshot_timer_sync(&timer.timer);

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

[Paul E. McKenney]

On Mon, Jan 06, 2020 at 01:08:37PM -0800, Olof Johansson wrote:
> On Mon, Jan 6, 2020 at 10:10 AM Paul E. McKenney <paulmck@kernel.org> wrote:
> >
> > On Mon, Jan 06, 2020 at 09:51:47AM -0800, Olof Johansson wrote:
> > > Hi,
> > >
> > > On Wed, Dec 11, 2019 at 9:06 PM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> > > >
> > > > Hi all,
> > > >
> > > > After merging the rcu (I think) tree, today's linux-next build (x86_64
> > > > allnoconfig) produced this warning:
> > > >
> > > > kernel/time/timer.c: In function 'schedule_timeout':
> > > > kernel/time/timer.c:969:20: warning: 'timer.expires' may be used uninitialized in this function [-Wmaybe-uninitialized]
> > > >   969 |   long diff = timer->expires - expires;
> > > >       |               ~~~~~^~~~~~~~~
> > > >
> > > > Introduced by (bisected to) commit
> > > >
> > > >   c4127fce1d02 ("timer: Use hlist_unhashed_lockless() in timer_pending()")
> > > >
> > > > x86_64-linux-gnu-gcc (Debian 9.2.1-21) 9.2.1 20191130
> > >
> > > This is still there as of last night's -next. Any update on getting a
> > > fix queued together with the offending patch?
> >
> > Hello, Olof,
> >
> > Thank you, I had indeed lost track of this one.  :-/
> >
> > Does Eric's patch fix things for you?
> >
> > https://lore.kernel.org/lkml/CANn89i+xomdo4HFqewrfNf_Z4Q5ayXuW6A4SjSkE46JXP9KuFw@mail.gmail.com/
> 
> It does, but it's not a proper patch (whitespace damage, no S-o-b, etc).

Understood, and thank you for checking!

Eric, would you be willing to turn this into a something that can be
sent upstream?

							Thanx, Paul

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

[Olof Johansson]

On Mon, Jan 6, 2020 at 10:10 AM Paul E. McKenney <paulmck@kernel.org> wrote:
>
> On Mon, Jan 06, 2020 at 09:51:47AM -0800, Olof Johansson wrote:
> > Hi,
> >
> > On Wed, Dec 11, 2019 at 9:06 PM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> > >
> > > Hi all,
> > >
> > > After merging the rcu (I think) tree, today's linux-next build (x86_64
> > > allnoconfig) produced this warning:
> > >
> > > kernel/time/timer.c: In function 'schedule_timeout':
> > > kernel/time/timer.c:969:20: warning: 'timer.expires' may be used uninitialized in this function [-Wmaybe-uninitialized]
> > >   969 |   long diff = timer->expires - expires;
> > >       |               ~~~~~^~~~~~~~~
> > >
> > > Introduced by (bisected to) commit
> > >
> > >   c4127fce1d02 ("timer: Use hlist_unhashed_lockless() in timer_pending()")
> > >
> > > x86_64-linux-gnu-gcc (Debian 9.2.1-21) 9.2.1 20191130
> >
> > This is still there as of last night's -next. Any update on getting a
> > fix queued together with the offending patch?
>
> Hello, Olof,
>
> Thank you, I had indeed lost track of this one.  :-/
>
> Does Eric's patch fix things for you?
>
> https://lore.kernel.org/lkml/CANn89i+xomdo4HFqewrfNf_Z4Q5ayXuW6A4SjSkE46JXP9KuFw@mail.gmail.com/

It does, but it's not a proper patch (whitespace damage, no S-o-b, etc).


-Olof

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

[Paul E. McKenney]

On Mon, Jan 06, 2020 at 09:51:47AM -0800, Olof Johansson wrote:
> Hi,
> 
> On Wed, Dec 11, 2019 at 9:06 PM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> >
> > Hi all,
> >
> > After merging the rcu (I think) tree, today's linux-next build (x86_64
> > allnoconfig) produced this warning:
> >
> > kernel/time/timer.c: In function 'schedule_timeout':
> > kernel/time/timer.c:969:20: warning: 'timer.expires' may be used uninitialized in this function [-Wmaybe-uninitialized]
> >   969 |   long diff = timer->expires - expires;
> >       |               ~~~~~^~~~~~~~~
> >
> > Introduced by (bisected to) commit
> >
> >   c4127fce1d02 ("timer: Use hlist_unhashed_lockless() in timer_pending()")
> >
> > x86_64-linux-gnu-gcc (Debian 9.2.1-21) 9.2.1 20191130
> 
> This is still there as of last night's -next. Any update on getting a
> fix queued together with the offending patch?

Hello, Olof,

Thank you, I had indeed lost track of this one.  :-/

Does Eric's patch fix things for you?

https://lore.kernel.org/lkml/CANn89i+xomdo4HFqewrfNf_Z4Q5ayXuW6A4SjSkE46JXP9KuFw@mail.gmail.com/

							Thanx, Paul

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

[Olof Johansson]

Hi,

On Wed, Dec 11, 2019 at 9:06 PM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Hi all,
>
> After merging the rcu (I think) tree, today's linux-next build (x86_64
> allnoconfig) produced this warning:
>
> kernel/time/timer.c: In function 'schedule_timeout':
> kernel/time/timer.c:969:20: warning: 'timer.expires' may be used uninitialized in this function [-Wmaybe-uninitialized]
>   969 |   long diff = timer->expires - expires;
>       |               ~~~~~^~~~~~~~~
>
> Introduced by (bisected to) commit
>
>   c4127fce1d02 ("timer: Use hlist_unhashed_lockless() in timer_pending()")
>
> x86_64-linux-gnu-gcc (Debian 9.2.1-21) 9.2.1 20191130

This is still there as of last night's -next. Any update on getting a
fix queued together with the offending patch?


Thanks!

-Olof

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

[Paul E. McKenney]

On Thu, Dec 12, 2019 at 10:40:50PM +1100, Stephen Rothwell wrote:
> Hi Paul,
> 
> On Wed, 11 Dec 2019 22:02:00 -0800 "Paul E. McKenney" <paulmck@kernel.org> wrote:
> >
> > On Thu, Dec 12, 2019 at 04:06:22PM +1100, Stephen Rothwell wrote:
> > > 
> > > x86_64-linux-gnu-gcc (Debian 9.2.1-21) 9.2.1 20191130  
> > 
> > I don't see this warning with gcc version 7.4.0.  Just out of curiosity,
> > what are you running, Stephen?
> 
> See above (9.2.1). ;-)

Color me blind.  :-/

							Thanx, Paul

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

[Stephen Rothwell]

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

Hi Paul,

On Wed, 11 Dec 2019 22:02:00 -0800 "Paul E. McKenney" <paulmck@kernel.org> wrote:
>
> On Thu, Dec 12, 2019 at 04:06:22PM +1100, Stephen Rothwell wrote:
> > 
> > x86_64-linux-gnu-gcc (Debian 9.2.1-21) 9.2.1 20191130  
> 
> I don't see this warning with gcc version 7.4.0.  Just out of curiosity,
> what are you running, Stephen?

See above (9.2.1). ;-)

-- 
Cheers,
Stephen Rothwell

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

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

[Eric Dumazet]

On Wed, Dec 11, 2019 at 10:38 PM Eric Dumazet <edumazet@google.com> wrote:
>
> On Wed, Dec 11, 2019 at 10:02 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> >
> > On Thu, Dec 12, 2019 at 04:06:22PM +1100, Stephen Rothwell wrote:
> > > Hi all,
> > >
> > > After merging the rcu (I think) tree, today's linux-next build (x86_64
> > > allnoconfig) produced this warning:
> > >
> > > kernel/time/timer.c: In function 'schedule_timeout':
> > > kernel/time/timer.c:969:20: warning: 'timer.expires' may be used uninitialized in this function [-Wmaybe-uninitialized]
> > >   969 |   long diff = timer->expires - expires;
> > >       |               ~~~~~^~~~~~~~~
> > >
> > > Introduced by (bisected to) commit
> > >
> > >   c4127fce1d02 ("timer: Use hlist_unhashed_lockless() in timer_pending()")
> > >
> > > x86_64-linux-gnu-gcc (Debian 9.2.1-21) 9.2.1 20191130
> >
> > Well, if the timer is pending, then ->expires has to have been
> > initialized, but off where the compiler cannot see it, such as during a
> > previous call to __mod_timer().  And the change may have made it harder
> > for the compiler to see all of these relationships, but...
> >
> > I don't see this warning with gcc version 7.4.0.  Just out of curiosity,
> > what are you running, Stephen?
> >
> > Eric, any thoughts for properly educating the compiler on this one?
>
> Ah... the READ_ONCE() apparently turns off the compiler ability to
> infer that this branch should not be taken.
>
> Since __mod_timer() is inlined we could perhaps add a new option
>
> diff --git a/kernel/time/timer.c b/kernel/time/timer.c
> index 4820823515e9..8bbce552568b 100644
> --- a/kernel/time/timer.c
> +++ b/kernel/time/timer.c
> @@ -944,6 +944,7 @@ static struct timer_base *lock_timer_base(struct
> timer_list *timer,
>
>  #define MOD_TIMER_PENDING_ONLY         0x01
>  #define MOD_TIMER_REDUCE               0x02
> +#define MOD_TIMER_NOTPENDING           0x04
>
>  static inline int
>  __mod_timer(struct timer_list *timer, unsigned long expires, unsigned
> int options)
> @@ -960,7 +961,7 @@ __mod_timer(struct timer_list *timer, unsigned
> long expires, unsigned int option
>          * the timer is re-modified to have the same timeout or ends up in the
>          * same array bucket then just return:
>          */
> -       if (timer_pending(timer)) {
> +       if (!(options & MOD_TIMER_NOTPENDING) && timer_pending(timer)) {
>                 /*
>                  * The downside of this optimization is that it can result in
>                  * larger granularity than you would get from adding a new
> @@ -1891,7 +1892,7 @@ signed long __sched schedule_timeout(signed long timeout)
>
>         timer.task = current;
>         timer_setup_on_stack(&timer.timer, process_timeout, 0);
> -       __mod_timer(&timer.timer, expire, 0);
> +       __mod_timer(&timer.timer, expire, MOD_TIMER_NOTPENDING);
>         schedule();
>         del_singleshot_timer_sync(&timer.timer);


Also add_timer() can benefit from the same hint, since it seems inlined as well.

(untested patch)

diff --git a/kernel/time/timer.c b/kernel/time/timer.c
index 4820823515e9..568564ae3597 100644
--- a/kernel/time/timer.c
+++ b/kernel/time/timer.c
@@ -944,6 +944,7 @@ static struct timer_base *lock_timer_base(struct
timer_list *timer,

 #define MOD_TIMER_PENDING_ONLY         0x01
 #define MOD_TIMER_REDUCE               0x02
+#define MOD_TIMER_NOTPENDING           0x04

 static inline int
 __mod_timer(struct timer_list *timer, unsigned long expires, unsigned
int options)
@@ -960,7 +961,7 @@ __mod_timer(struct timer_list *timer, unsigned
long expires, unsigned int option
         * the timer is re-modified to have the same timeout or ends up in the
         * same array bucket then just return:
         */
-       if (timer_pending(timer)) {
+       if (!(options & MOD_TIMER_NOTPENDING) && timer_pending(timer)) {
                /*
                 * The downside of this optimization is that it can result in
                 * larger granularity than you would get from adding a new
@@ -1133,7 +1134,7 @@ EXPORT_SYMBOL(timer_reduce);
 void add_timer(struct timer_list *timer)
 {
        BUG_ON(timer_pending(timer));
-       mod_timer(timer, timer->expires);
+       __mod_timer(timer, timer->expires, MOD_TIMER_NOTPENDING);
 }
 EXPORT_SYMBOL(add_timer);

@@ -1891,7 +1892,7 @@ signed long __sched schedule_timeout(signed long timeout)

        timer.task = current;
        timer_setup_on_stack(&timer.timer, process_timeout, 0);
-       __mod_timer(&timer.timer, expire, 0);
+       __mod_timer(&timer.timer, expire, MOD_TIMER_NOTPENDING);
        schedule();
        del_singleshot_timer_sync(&timer.timer);

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

[Eric Dumazet]

On Wed, Dec 11, 2019 at 10:02 PM Paul E. McKenney <paulmck@kernel.org> wrote:
>
> On Thu, Dec 12, 2019 at 04:06:22PM +1100, Stephen Rothwell wrote:
> > Hi all,
> >
> > After merging the rcu (I think) tree, today's linux-next build (x86_64
> > allnoconfig) produced this warning:
> >
> > kernel/time/timer.c: In function 'schedule_timeout':
> > kernel/time/timer.c:969:20: warning: 'timer.expires' may be used uninitialized in this function [-Wmaybe-uninitialized]
> >   969 |   long diff = timer->expires - expires;
> >       |               ~~~~~^~~~~~~~~
> >
> > Introduced by (bisected to) commit
> >
> >   c4127fce1d02 ("timer: Use hlist_unhashed_lockless() in timer_pending()")
> >
> > x86_64-linux-gnu-gcc (Debian 9.2.1-21) 9.2.1 20191130
>
> Well, if the timer is pending, then ->expires has to have been
> initialized, but off where the compiler cannot see it, such as during a
> previous call to __mod_timer().  And the change may have made it harder
> for the compiler to see all of these relationships, but...
>
> I don't see this warning with gcc version 7.4.0.  Just out of curiosity,
> what are you running, Stephen?
>
> Eric, any thoughts for properly educating the compiler on this one?

Ah... the READ_ONCE() apparently turns off the compiler ability to
infer that this branch should not be taken.

Since __mod_timer() is inlined we could perhaps add a new option

diff --git a/kernel/time/timer.c b/kernel/time/timer.c
index 4820823515e9..8bbce552568b 100644
--- a/kernel/time/timer.c
+++ b/kernel/time/timer.c
@@ -944,6 +944,7 @@ static struct timer_base *lock_timer_base(struct
timer_list *timer,

 #define MOD_TIMER_PENDING_ONLY         0x01
 #define MOD_TIMER_REDUCE               0x02
+#define MOD_TIMER_NOTPENDING           0x04

 static inline int
 __mod_timer(struct timer_list *timer, unsigned long expires, unsigned
int options)
@@ -960,7 +961,7 @@ __mod_timer(struct timer_list *timer, unsigned
long expires, unsigned int option
         * the timer is re-modified to have the same timeout or ends up in the
         * same array bucket then just return:
         */
-       if (timer_pending(timer)) {
+       if (!(options & MOD_TIMER_NOTPENDING) && timer_pending(timer)) {
                /*
                 * The downside of this optimization is that it can result in
                 * larger granularity than you would get from adding a new
@@ -1891,7 +1892,7 @@ signed long __sched schedule_timeout(signed long timeout)

        timer.task = current;
        timer_setup_on_stack(&timer.timer, process_timeout, 0);
-       __mod_timer(&timer.timer, expire, 0);
+       __mod_timer(&timer.timer, expire, MOD_TIMER_NOTPENDING);
        schedule();
        del_singleshot_timer_sync(&timer.timer);

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

[Paul E. McKenney]

On Thu, Dec 12, 2019 at 04:06:22PM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the rcu (I think) tree, today's linux-next build (x86_64
> allnoconfig) produced this warning:
> 
> kernel/time/timer.c: In function 'schedule_timeout':
> kernel/time/timer.c:969:20: warning: 'timer.expires' may be used uninitialized in this function [-Wmaybe-uninitialized]
>   969 |   long diff = timer->expires - expires;
>       |               ~~~~~^~~~~~~~~
> 
> Introduced by (bisected to) commit
> 
>   c4127fce1d02 ("timer: Use hlist_unhashed_lockless() in timer_pending()")
> 
> x86_64-linux-gnu-gcc (Debian 9.2.1-21) 9.2.1 20191130

Well, if the timer is pending, then ->expires has to have been
initialized, but off where the compiler cannot see it, such as during a
previous call to __mod_timer().  And the change may have made it harder
for the compiler to see all of these relationships, but...

I don't see this warning with gcc version 7.4.0.  Just out of curiosity,
what are you running, Stephen?

Eric, any thoughts for properly educating the compiler on this one?

							Thanx, Paul

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

[Stephen Rothwell]

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

Hi all,

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

kernel/time/timer.c: In function 'schedule_timeout':
kernel/time/timer.c:969:20: warning: 'timer.expires' may be used uninitialized in this function [-Wmaybe-uninitialized]
  969 |   long diff = timer->expires - expires;
      |               ~~~~~^~~~~~~~~

Introduced by (bisected to) commit

  c4127fce1d02 ("timer: Use hlist_unhashed_lockless() in timer_pending()")

x86_64-linux-gnu-gcc (Debian 9.2.1-21) 9.2.1 20191130

-- 
Cheers,
Stephen Rothwell

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

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

[Paul E. McKenney]

On Wed, Jun 21, 2017 at 04:48:06PM +1000, Stephen Rothwell wrote:
> Hi Paul,
> 
> After merging the rcu tree, today's linux-next build (x86_64 allnoconfig)
> produced this warning:
> 
> kernel/cpu.c: In function 'boot_cpu_state_init':
> kernel/cpu.c:1778:6: warning: unused variable 'cpu' [-Wunused-variable]
>   int cpu;
>       ^
> 
> Introduced by commit
> 
>   faeb334286b7 ("rcu: Migrate callbacks earlier in the CPU-offline timeline")

Hello, Stephen,

Apologies, should be fixed now -- added a __maybe_unused.

But I didn't realize you did SMP=n builds!  ;-)

							Thanx, Paul

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

[Stephen Rothwell]

Hi Paul,

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

kernel/cpu.c: In function 'boot_cpu_state_init':
kernel/cpu.c:1778:6: warning: unused variable 'cpu' [-Wunused-variable]
  int cpu;
      ^

Introduced by commit

  faeb334286b7 ("rcu: Migrate callbacks earlier in the CPU-offline timeline")

-- 
Cheers,
Stephen Rothwell

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

[Paul E. McKenney]

On Wed, Aug 24, 2011 at 08:46:15PM -0400, Arnaud Lacombe wrote:
> Hi,
> 
> On Wed, Aug 24, 2011 at 8:27 AM, Paul E. McKenney
> <paulmck@linux.vnet.ibm.com> wrote:
> > On Wed, Aug 24, 2011 at 02:23:37PM +1000, Stephen Rothwell wrote:
> >> Hi Paul,
> >>
> >> After merging the rcu tree, today's linux-next build (powerpc
> >> ppc64_defconfig) produced this warning:
> >>
> >> kernel/rtmutex.c: In function '__rt_mutex_slowlock':
> >> kernel/rtmutex.c:605:3: warning: suggest parentheses around assignment used as truth value
> >
> > There actually already are parentheses around it, and the first pass
> > through the loop it is uninitialized at that point.  But hey, that
> > is gcc for you!  Does the patch below cure it?
> >
> Yes.

Thank you!

							Thanx, Paul

>  - Arnaud
> 
> >> Introduced by commit 83841f021d4b ("rcu: Permit rt_mutex_unlock() with
> >> irqs disabled").
> >
> > If it does, then I will fold into that commit.
> >
> >                                                        Thanx, Paul
> >
> > ------------------------------------------------------------------------
> >
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> >
> > diff --git a/kernel/rtmutex.c b/kernel/rtmutex.c
> > index 0222e34..2548f44 100644
> > --- a/kernel/rtmutex.c
> > +++ b/kernel/rtmutex.c
> > @@ -602,7 +602,8 @@ __rt_mutex_slowlock(struct rt_mutex *lock, int state,
> >
> >                raw_spin_unlock(&lock->wait_lock);
> >
> > -               if (was_disabled = irqs_disabled())
> > +               was_disabled = irqs_disabled();
> > +               if (was_disabled)
> >                        local_irq_enable();
> >
> >                debug_rt_mutex_print_deadlock(waiter);
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
> >

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

[Arnaud Lacombe]

Hi,

On Wed, Aug 24, 2011 at 8:27 AM, Paul E. McKenney
<paulmck@linux.vnet.ibm.com> wrote:
> On Wed, Aug 24, 2011 at 02:23:37PM +1000, Stephen Rothwell wrote:
>> Hi Paul,
>>
>> After merging the rcu tree, today's linux-next build (powerpc
>> ppc64_defconfig) produced this warning:
>>
>> kernel/rtmutex.c: In function '__rt_mutex_slowlock':
>> kernel/rtmutex.c:605:3: warning: suggest parentheses around assignment used as truth value
>
> There actually already are parentheses around it, and the first pass
> through the loop it is uninitialized at that point.  But hey, that
> is gcc for you!  Does the patch below cure it?
>
Yes.

 - Arnaud

>> Introduced by commit 83841f021d4b ("rcu: Permit rt_mutex_unlock() with
>> irqs disabled").
>
> If it does, then I will fold into that commit.
>
>                                                        Thanx, Paul
>
> ------------------------------------------------------------------------
>
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
>
> diff --git a/kernel/rtmutex.c b/kernel/rtmutex.c
> index 0222e34..2548f44 100644
> --- a/kernel/rtmutex.c
> +++ b/kernel/rtmutex.c
> @@ -602,7 +602,8 @@ __rt_mutex_slowlock(struct rt_mutex *lock, int state,
>
>                raw_spin_unlock(&lock->wait_lock);
>
> -               if (was_disabled = irqs_disabled())
> +               was_disabled = irqs_disabled();
> +               if (was_disabled)
>                        local_irq_enable();
>
>                debug_rt_mutex_print_deadlock(waiter);
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

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

[Paul E. McKenney]

On Wed, Aug 24, 2011 at 02:23:37PM +1000, Stephen Rothwell wrote:
> Hi Paul,
> 
> After merging the rcu tree, today's linux-next build (powerpc
> ppc64_defconfig) produced this warning:
> 
> kernel/rtmutex.c: In function '__rt_mutex_slowlock':
> kernel/rtmutex.c:605:3: warning: suggest parentheses around assignment used as truth value

There actually already are parentheses around it, and the first pass
through the loop it is uninitialized at that point.  But hey, that 
is gcc for you!  Does the patch below cure it?

> Introduced by commit 83841f021d4b ("rcu: Permit rt_mutex_unlock() with
> irqs disabled").

If it does, then I will fold into that commit.

							Thanx, Paul

------------------------------------------------------------------------

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

diff --git a/kernel/rtmutex.c b/kernel/rtmutex.c
index 0222e34..2548f44 100644
--- a/kernel/rtmutex.c
+++ b/kernel/rtmutex.c
@@ -602,7 +602,8 @@ __rt_mutex_slowlock(struct rt_mutex *lock, int state,
 
 		raw_spin_unlock(&lock->wait_lock);
 
-		if (was_disabled = irqs_disabled())
+		was_disabled = irqs_disabled();
+		if (was_disabled)
 			local_irq_enable();
 
 		debug_rt_mutex_print_deadlock(waiter);

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

[Stephen Rothwell]

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

Hi Paul,

After merging the rcu tree, today's linux-next build (powerpc
ppc64_defconfig) produced this warning:

kernel/rtmutex.c: In function '__rt_mutex_slowlock':
kernel/rtmutex.c:605:3: warning: suggest parentheses around assignment used as truth value

Introduced by commit 83841f021d4b ("rcu: Permit rt_mutex_unlock() with
irqs disabled").

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

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

[Paul E. McKenney]

On Fri, Mar 25, 2011 at 04:05:39PM +1100, Stephen Rothwell wrote:
> Hi Paul,
> 
> After merging the rcu tree, today's linux-next build (powerpc
> ppc64_defconfig) produced this warning:
> 
> kernel/softirq.c:62: warning: excess elements in array initializer
> kernel/softirq.c:62: warning: (near initialization for 'softirq_to_name')
> 
> Introduced by commit 0ac7e739ad17 ("rcu: move TREE_RCU from softirq to
> kthread").

I must confess that it didn't occur to me to check for more than one
array of softirq names, and I have no idea why I didn't see this warning
in my builds.  In any case, I have fixed this and updated -rcu.

I just now found the mention of RCU in the proc.txt documentation file,
and have a fix for that queued as well, and will export to -rcu on the
next round.

							Thanx, Paul

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

[Stephen Rothwell]

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

Hi Paul,

After merging the rcu tree, today's linux-next build (powerpc
ppc64_defconfig) produced this warning:

kernel/softirq.c:62: warning: excess elements in array initializer
kernel/softirq.c:62: warning: (near initialization for 'softirq_to_name')

Introduced by commit 0ac7e739ad17 ("rcu: move TREE_RCU from softirq to
kthread").
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

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

[Paul E. McKenney]

On Mon, Sep 06, 2010 at 12:32:55PM +1000, Neil Brown wrote:
> On Mon, 6 Sep 2010 12:14:08 +1000
> Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> 
> > Hi Paul,
> > 
> > After merging the rcu tree, today's linux-next build
> > (powerpc ppc64_defconfig) produced this warning:
> > 
> > drivers/md/raid1.c: In function 'read_balance':
> > drivers/md/raid1.c:445: warning: operation on 'new_disk' may be undefined
> > 
> > I am picking on the rcu tree because the line above has not changed since
> > 2005.  The line is:
> > 
> >                 for (rdev = rcu_dereference(conf->mirrors[new_disk].rdev);
> >                      r1_bio->bios[new_disk] == IO_BLOCKED ||
> >                      !rdev || !test_bit(In_sync, &rdev->flags)
> >                              || test_bit(WriteMostly, &rdev->flags);
> >                      rdev = rcu_dereference(conf->mirrors[++new_disk].rdev)) {
> > 
> > Where new_disk is being updated in the parameter to the last
> > rcu_dereference.
> > 
> 
> Hi Stephen,
> 
>  There is a patch in linux-kernel
>     "PATCH] md: do not use ++ in rcu_dereference() argument"
> 
>  that addresses this - it is a problem in md: I guess rcu as become more
>  helpful in finding these problems.

Indeed, the sparse-based checking is a bit more picky about things,
even when you are not using sparse...

							Thanx, Paul

>  I'll probably fix it a little differently, but I'll have something in my
>  for-next for tomorrow.
> 
> Thanks,
> NeilBrown

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

[Stephen Rothwell]

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

Hi Neil,


On Mon, 6 Sep 2010 12:32:55 +1000 Neil Brown <neilb@suse.de> wrote:
>
>  There is a patch in linux-kernel
>     "PATCH] md: do not use ++ in rcu_dereference() argument"
> 
>  that addresses this - it is a problem in md: I guess rcu as become more
>  helpful in finding these problems.
> 
>  I'll probably fix it a little differently, but I'll have something in my
>  for-next for tomorrow.

Ah, ok.  Thanks.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

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

[Neil Brown]

On Mon, 6 Sep 2010 12:14:08 +1000
Stephen Rothwell <sfr@canb.auug.org.au> wrote:

> Hi Paul,
> 
> After merging the rcu tree, today's linux-next build
> (powerpc ppc64_defconfig) produced this warning:
> 
> drivers/md/raid1.c: In function 'read_balance':
> drivers/md/raid1.c:445: warning: operation on 'new_disk' may be undefined
> 
> I am picking on the rcu tree because the line above has not changed since
> 2005.  The line is:
> 
>                 for (rdev = rcu_dereference(conf->mirrors[new_disk].rdev);
>                      r1_bio->bios[new_disk] == IO_BLOCKED ||
>                      !rdev || !test_bit(In_sync, &rdev->flags)
>                              || test_bit(WriteMostly, &rdev->flags);
>                      rdev = rcu_dereference(conf->mirrors[++new_disk].rdev)) {
> 
> Where new_disk is being updated in the parameter to the last
> rcu_dereference.
> 

Hi Stephen,

 There is a patch in linux-kernel
    "PATCH] md: do not use ++ in rcu_dereference() argument"

 that addresses this - it is a problem in md: I guess rcu as become more
 helpful in finding these problems.

 I'll probably fix it a little differently, but I'll have something in my
 for-next for tomorrow.

Thanks,
NeilBrown

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

[Stephen Rothwell]

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

Hi Paul,

After merging the rcu tree, today's linux-next build
(powerpc ppc64_defconfig) produced this warning:

drivers/md/raid1.c: In function 'read_balance':
drivers/md/raid1.c:445: warning: operation on 'new_disk' may be undefined

I am picking on the rcu tree because the line above has not changed since
2005.  The line is:

                for (rdev = rcu_dereference(conf->mirrors[new_disk].rdev);
                     r1_bio->bios[new_disk] == IO_BLOCKED ||
                     !rdev || !test_bit(In_sync, &rdev->flags)
                             || test_bit(WriteMostly, &rdev->flags);
                     rdev = rcu_dereference(conf->mirrors[++new_disk].rdev)) {

Where new_disk is being updated in the parameter to the last
rcu_dereference.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]