mbox series

[00/11] Lock warning cleanup

Message ID cover.1581282103.git.jbi.octave@gmail.com (mailing list archive)
Headers show
Series Lock warning cleanup | expand

Message

Jules Irenge Feb. 9, 2020, 10:24 p.m. UTC 
This patch series adds missing annotations to functions that register warnings of context imbalance when built with Sparse tool.
The adds fix the warnings and give insight on what the functions are actually doing.

1. Within the futex subsystem, a __releases(&pi_state->.pi_mutex.wait_lock) is added because wake_futex_pi() only releases the lock at exit,
must_hold(q->lock_ptr) have been added to fixup_pi_state_owner() because the lock is held at entry and exit;
a __releases(&hb->lock) added to futex_wait_queue_me() as it only releases the lock.

2. Within fs_pin, a __releases(RCU) is added because the function exit RCU critical section at exit.

3. In kasan, an __acquires(&report_lock) has been added to start_report() and   __releases(&report_lock) to end_report() 

4. Within ring_buffer subsystem, a __releases(RCU) has been added perf_output_end() 

5. schedule subsystem recorded an addition of the __releases(rq->lock) annotation and a __must_hold(this_rq->lock)

6. At hrtimer subsystem, __acquires(timer) is added  to lock_hrtimer_base() as the function acquire the lock but never releases it.
Jules Irenge (11):
  hrtimer: Add missing annotation to lock_hrtimer_base()
  futex: Add missing annotation for wake_futex_pi()
  futex: Add missing annotation for fixup_pi_state_owner()
  perf/ring_buffer: Add missing annotation to perf_output_end()
  sched/fair: Add missing annotation for nohz_newidle_balance()
  sched/deadline: Add missing annotation for dl_task_offline_migration()
  fs_pin: Add missing annotation for pin_kill() declaration
  fs_pin: Add missing annotation for pin_kill() definition
  kasan: add missing annotation for start_report()
  kasan: add missing annotation for end_report()
  futex: Add missing annotation for futex_wait_queue_me()

 fs/fs_pin.c                 | 2 +-
 include/linux/fs_pin.h      | 2 +-
 kernel/events/ring_buffer.c | 2 +-
 kernel/futex.c              | 3 +++
 kernel/sched/deadline.c     | 1 +
 kernel/sched/fair.c         | 2 +-
 kernel/time/hrtimer.c       | 1 +
 mm/kasan/report.c           | 4 ++--
 8 files changed, 11 insertions(+), 6 deletions(-)

Comments

Boqun Feng Feb. 10, 2020, 5:06 a.m. UTC | #1 
Hi Jules,

On Sun, Feb 09, 2020 at 10:24:42PM +0000, Jules Irenge wrote:
> This patch series adds missing annotations to functions that register warnings of context imbalance when built with Sparse tool.
> The adds fix the warnings and give insight on what the functions are actually doing.
> 
> 1. Within the futex subsystem, a __releases(&pi_state->.pi_mutex.wait_lock) is added because wake_futex_pi() only releases the lock at exit,
> must_hold(q->lock_ptr) have been added to fixup_pi_state_owner() because the lock is held at entry and exit;
> a __releases(&hb->lock) added to futex_wait_queue_me() as it only releases the lock.
> 
> 2. Within fs_pin, a __releases(RCU) is added because the function exit RCU critical section at exit.
> 
> 3. In kasan, an __acquires(&report_lock) has been added to start_report() and   __releases(&report_lock) to end_report() 
> 
> 4. Within ring_buffer subsystem, a __releases(RCU) has been added perf_output_end() 
> 
> 5. schedule subsystem recorded an addition of the __releases(rq->lock) annotation and a __must_hold(this_rq->lock)
> 
> 6. At hrtimer subsystem, __acquires(timer) is added  to lock_hrtimer_base() as the function acquire the lock but never releases it.
> Jules Irenge (11):
>   hrtimer: Add missing annotation to lock_hrtimer_base()
>   futex: Add missing annotation for wake_futex_pi()
>   futex: Add missing annotation for fixup_pi_state_owner()

Given that those three patches have been sent and reviewed, please do
increase the version number (this time, for example, using v2) when
sending the updated ones. Also please add a few sentences after the
commit log describing what you have changed between versions.

Here is an example:

	https://lore.kernel.org/lkml/20200124231834.63628-4-pmalani@chromium.org/

Regards,
Boqun

>   perf/ring_buffer: Add missing annotation to perf_output_end()
>   sched/fair: Add missing annotation for nohz_newidle_balance()
>   sched/deadline: Add missing annotation for dl_task_offline_migration()
>   fs_pin: Add missing annotation for pin_kill() declaration
>   fs_pin: Add missing annotation for pin_kill() definition
>   kasan: add missing annotation for start_report()
>   kasan: add missing annotation for end_report()
>   futex: Add missing annotation for futex_wait_queue_me()
> 
>  fs/fs_pin.c                 | 2 +-
>  include/linux/fs_pin.h      | 2 +-
>  kernel/events/ring_buffer.c | 2 +-
>  kernel/futex.c              | 3 +++
>  kernel/sched/deadline.c     | 1 +
>  kernel/sched/fair.c         | 2 +-
>  kernel/time/hrtimer.c       | 1 +
>  mm/kasan/report.c           | 4 ++--
>  8 files changed, 11 insertions(+), 6 deletions(-)
> 
> -- 
> 2.24.1
>
Jules Irenge Feb. 10, 2020, 11:09 p.m. UTC | #2 
On Mon, 10 Feb 2020, Boqun Feng wrote:

> Hi Jules,
> 
> On Sun, Feb 09, 2020 at 10:24:42PM +0000, Jules Irenge wrote:
> > This patch series adds missing annotations to functions that register warnings of context imbalance when built with Sparse tool.
> > The adds fix the warnings and give insight on what the functions are actually doing.
> > 
> > 1. Within the futex subsystem, a __releases(&pi_state->.pi_mutex.wait_lock) is added because wake_futex_pi() only releases the lock at exit,
> > must_hold(q->lock_ptr) have been added to fixup_pi_state_owner() because the lock is held at entry and exit;
> > a __releases(&hb->lock) added to futex_wait_queue_me() as it only releases the lock.
> > 
> > 2. Within fs_pin, a __releases(RCU) is added because the function exit RCU critical section at exit.
> > 
> > 3. In kasan, an __acquires(&report_lock) has been added to start_report() and   __releases(&report_lock) to end_report() 
> > 
> > 4. Within ring_buffer subsystem, a __releases(RCU) has been added perf_output_end() 
> > 
> > 5. schedule subsystem recorded an addition of the __releases(rq->lock) annotation and a __must_hold(this_rq->lock)
> > 
> > 6. At hrtimer subsystem, __acquires(timer) is added  to lock_hrtimer_base() as the function acquire the lock but never releases it.
> > Jules Irenge (11):
> >   hrtimer: Add missing annotation to lock_hrtimer_base()
> >   futex: Add missing annotation for wake_futex_pi()
> >   futex: Add missing annotation for fixup_pi_state_owner()
> 
> Given that those three patches have been sent and reviewed, please do
> increase the version number (this time, for example, using v2) when
> sending the updated ones. Also please add a few sentences after the
> commit log describing what you have changed between versions.
> 
> Here is an example:
> 
> 	https://lore.kernel.org/lkml/20200124231834.63628-4-pmalani@chromium.org/
> 
> Regards,
> Boqun
> 
> >   perf/ring_buffer: Add missing annotation to perf_output_end()
> >   sched/fair: Add missing annotation for nohz_newidle_balance()
> >   sched/deadline: Add missing annotation for dl_task_offline_migration()
> >   fs_pin: Add missing annotation for pin_kill() declaration
> >   fs_pin: Add missing annotation for pin_kill() definition
> >   kasan: add missing annotation for start_report()
> >   kasan: add missing annotation for end_report()
> >   futex: Add missing annotation for futex_wait_queue_me()
> > 
> >  fs/fs_pin.c                 | 2 +-
> >  include/linux/fs_pin.h      | 2 +-
> >  kernel/events/ring_buffer.c | 2 +-
> >  kernel/futex.c              | 3 +++
> >  kernel/sched/deadline.c     | 1 +
> >  kernel/sched/fair.c         | 2 +-
> >  kernel/time/hrtimer.c       | 1 +
> >  mm/kasan/report.c           | 4 ++--
> >  8 files changed, 11 insertions(+), 6 deletions(-)
> > 
> > -- 
> > 2.24.1
> > 
> 

Thanks for the feedback, I take good notes. I am working on the 
second version.

Kind regards,
Jules