Re: include/asm-generic/qspinlock.h:94:9: sparse: sparse: context imbalance in '__msm_console_write' - unexpected unlock

[Leo Yan]

Hi,

On Sat, Aug 29, 2020 at 08:27:03AM +0800, kernel test robot wrote:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head:   005c53447a63cbce10de37406975a34d7bdc8704
> commit: 0e4f7f920a5c6bfe5e851e989f27b35a0cc7fb7e tty: serial: msm_serial: Fix lockup for sysrq and oops
> date:   9 months ago
> config: arm64-randconfig-s031-20200829 (attached as .config)
> compiler: aarch64-linux-gcc (GCC) 9.3.0
> reproduce:
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # apt-get install sparse
>         # sparse version: v0.6.2-191-g10164920-dirty
>         git checkout 0e4f7f920a5c6bfe5e851e989f27b35a0cc7fb7e
>         # save the attached .config to linux build tree
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=arm64 
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@xxxxxxxxx>
> 
> 
> sparse warnings: (new ones prefixed by >>)
> 
>    drivers/tty/serial/msm_serial.c:736:25: sparse: sparse: context imbalance in 'msm_handle_rx_dm' - unexpected unlock
>    drivers/tty/serial/msm_serial.c:802:28: sparse: sparse: context imbalance in 'msm_handle_rx' - unexpected unlock
>    drivers/tty/serial/msm_serial.c:1100:12: sparse: sparse: context imbalance in 'msm_set_baud_rate' - unexpected unlock
>    drivers/tty/serial/msm_serial.c: note: in included file (through arch/arm64/include/generated/asm/qspinlock.h, arch/arm64/include/asm/spinlock.h, include/linux/spinlock.h, ...):
> >> include/asm-generic/qspinlock.h:94:9: sparse: sparse: context imbalance in '__msm_console_write' - unexpected unlock

I think this is a false positive report.  Let's see the locking
relevant code in __msm_console_write():

  __msm_console_write() {

      int locked = 1;

      [...]

      if (port->sysrq)
          locked = 0;
      else if (oops_in_progress)
          locked = spin_trylock(&port->lock);
      else
          spin_lock(&port->lock);

      [...]

      if (locked)
          spin_unlock(&port->lock);
  }

Since we need to handle oops and sysrq as special cases, it uses
'locked' to indicate the spinlock has been acquired and later will
release spinlock if 'locked' is true.  So locking and unlocking are
paired and I don't see what's the reason the robot reports the context
imbalance.

BTW, this report is not only for msm_serial driver, it also might be
applied to many other serial drivers, e.g.

  drivers/tty/serial/amba-pl011.c
  drivers/tty/serial/xilinx_uartps.c
  drivers/tty/serial/stm32-usart.c

Maybe I miss something at here, if so please let me know.

Thanks,
Leo

> # https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0e4f7f920a5c6bfe5e851e989f27b35a0cc7fb7e
> git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> git fetch --no-tags linus master
> git checkout 0e4f7f920a5c6bfe5e851e989f27b35a0cc7fb7e
> vim +/__msm_console_write +94 include/asm-generic/qspinlock.h
> 
> a33fda35e3a765 Waiman Long 2015-04-24  83  
> a33fda35e3a765 Waiman Long 2015-04-24  84  #ifndef queued_spin_unlock
> a33fda35e3a765 Waiman Long 2015-04-24  85  /**
> a33fda35e3a765 Waiman Long 2015-04-24  86   * queued_spin_unlock - release a queued spinlock
> a33fda35e3a765 Waiman Long 2015-04-24  87   * @lock : Pointer to queued spinlock structure
> a33fda35e3a765 Waiman Long 2015-04-24  88   */
> a33fda35e3a765 Waiman Long 2015-04-24  89  static __always_inline void queued_spin_unlock(struct qspinlock *lock)
> a33fda35e3a765 Waiman Long 2015-04-24  90  {
> a33fda35e3a765 Waiman Long 2015-04-24  91  	/*
> ca50e426f96c90 Pan Xinhui  2016-06-03  92  	 * unlock() needs release semantics:
> a33fda35e3a765 Waiman Long 2015-04-24  93  	 */
> 626e5fbc143589 Will Deacon 2018-04-26 @94  	smp_store_release(&lock->locked, 0);
> a33fda35e3a765 Waiman Long 2015-04-24  95  }
> a33fda35e3a765 Waiman Long 2015-04-24  96  #endif
> a33fda35e3a765 Waiman Long 2015-04-24  97  
> 
> :::::: The code at line 94 was first introduced by commit
> :::::: 626e5fbc14358901ddaa90ce510e0fbeab310432 locking/qspinlock: Use smp_store_release() in queued_spin_unlock()
> 
> :::::: TO: Will Deacon <will.deacon@xxxxxxx>
> :::::: CC: Ingo Molnar <mingo@xxxxxxxxxx>
> 
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@xxxxxxxxxxxx