RE: [kbuild-all] Re: drivers/greybus/es2.c:439 message_send() error: double unlocked 'es2->cport_out_urb_lock' (orig line 417)

[Xia, Hui]

>-----Original Message-----
>From: Johan Hovold <johan@xxxxxxxxxx>
>Sent: 2020年8月24日 17:54
>To: kernel test robot <lkp@xxxxxxxxx>
>Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>; kbuild-all@xxxxxxxxxxxx;
>linux-kernel@xxxxxxxxxxxxxxx
>Subject: [kbuild-all] Re: drivers/greybus/es2.c:439 message_send() error: double
>unlocked 'es2->cport_out_urb_lock' (orig line 417)
>
>On Fri, Aug 14, 2020 at 03:26:27AM +0800, kernel test robot wrote:
>> Hi Greg,
>>
>> First bad commit (maybe != root cause):
>>
>> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
>> head:   dc06fe51d26efc100ac74121607c01a454867c91
>> commit: b81beec9cb2d586412c7166c893894930f19965e staging: greybus:
>move es2 to drivers/greybus/
>> date:   12 months ago
>> config: x86_64-randconfig-m001-20200813 (attached as .config)
>> compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
>>
>> If you fix the issue, kindly add following tag as appropriate
>> Reported-by: kernel test robot <lkp@xxxxxxxxx>
>>
>> New smatch warnings:
>> drivers/greybus/es2.c:439 message_send() error: double unlocked
>> 'es2->cport_out_urb_lock' (orig line 417)
>> drivers/greybus/es2.c:879 cport_out_callback() error: double unlocked
>> 'es2->cport_out_urb_lock' (orig line 871)
>> drivers/greybus/es2.c:1018 arpc_sync() error: double unlocked
>> 'es2->arpc_lock' (orig line 992)
>>
>> Old smatch warnings:
>> drivers/greybus/es2.c:441 message_send() error: double unlocked
>> 'es2->cport_out_urb_lock' (orig line 439)
>
>None of these warnings make any sense. Is the robot drunk?
Hi Johan,

It is a false positive. Please kindly ignore.
There was a bug in robot that sent unconfident reports out directly and has been fixed. Sorry for inconvenient.

Best Regards,
Hui
>
>> vim +439 drivers/greybus/es2.c
>>
>> d29b3d631e57240 drivers/staging/greybus/es2.c    Alex Elder         2015-06-13
>385
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-
>01-21  386  /*
>> 3e136cc9e05e1a3 drivers/staging/greybus/es2.c    Johan Hovold       2015-07-01
>387   * Returns zero if the message was successfully queued, or a negative errno
>> 3e136cc9e05e1a3 drivers/staging/greybus/es2.c    Johan Hovold       2015-07-01
>388   * otherwise.
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-
>01-21  389   */
>> 2537636abae5b81 drivers/staging/greybus/es2.c    Johan Hovold       2015-11-
>03  390  static int message_send(struct gb_host_device *hd, u16 cport_id,
>> 7cf7bca9ec5659e drivers/staging/greybus/es2.c    Johan Hovold       2015-04-07
>391  			struct gb_message *message, gfp_t gfp_mask)
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-
>01-21  392  {
>> 4b1d82047ebbb10 drivers/staging/greybus/es2.c    Alex Elder         2015-10-27
>393  	struct es2_ap_dev *es2 = hd_to_es2(hd);
>> 4b1d82047ebbb10 drivers/staging/greybus/es2.c    Alex Elder         2015-10-27
>394  	struct usb_device *udev = es2->usb_dev;
>> 7cf7bca9ec5659e drivers/staging/greybus/es2.c    Johan Hovold       2015-04-07
>395  	size_t buffer_size;
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-
>01-21  396  	int retval;
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-
>01-21  397  	struct urb *urb;
>> 3e136cc9e05e1a3 drivers/staging/greybus/es2.c    Johan Hovold       2015-07-01
>398  	unsigned long flags;
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-
>01-21  399
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-
>01-21  400  	/*
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-
>01-21  401  	 * The data actually transferred will include an indication
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-
>01-21  402  	 * of where the data should be sent.  Do one last check of
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-
>01-21  403  	 * the target CPort id before filling it in.
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-
>01-21  404  	 */
>> 144670c2ae3f9b4 drivers/staging/greybus/es2.c    Fabien Parent      2015-09-02
>405  	if (!cport_id_valid(hd, cport_id)) {
>> 100e90000840741 drivers/staging/greybus/es2.c    Johan Hovold       2015-12-
>07  406  		dev_err(&udev->dev, "invalid cport %u\n", cport_id);
>> 3e136cc9e05e1a3 drivers/staging/greybus/es2.c    Johan Hovold       2015-07-01
>407  		return -EINVAL;
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-
>01-21  408  	}
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-
>01-21  409
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-
>01-21  410  	/* Find a free urb */
>> 4b1d82047ebbb10 drivers/staging/greybus/es2.c    Alex Elder         2015-10-27
>411  	urb = next_free_urb(es2, gfp_mask);
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-
>01-21  412  	if (!urb)
>> 3e136cc9e05e1a3 drivers/staging/greybus/es2.c    Johan Hovold       2015-07-01
>413  		return -ENOMEM;
>> 3e136cc9e05e1a3 drivers/staging/greybus/es2.c    Johan Hovold       2015-07-01
>414
>> 4b1d82047ebbb10 drivers/staging/greybus/es2.c    Alex Elder         2015-10-27
>415  	spin_lock_irqsave(&es2->cport_out_urb_lock, flags);
>> 3e136cc9e05e1a3 drivers/staging/greybus/es2.c    Johan Hovold       2015-07-01
>416  	message->hcpriv = urb;
>> 4b1d82047ebbb10 drivers/staging/greybus/es2.c    Alex Elder         2015-10-27
>@417  	spin_unlock_irqrestore(&es2->cport_out_urb_lock, flags);
>
>How is this a double unlock?
>
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-
>01-21  418
>> d29b3d631e57240 drivers/staging/greybus/es2.c    Alex Elder         2015-06-13
>419  	/* Pack the cport id into the message header */
>> d29b3d631e57240 drivers/staging/greybus/es2.c    Alex Elder         2015-06-13
>420  	gb_message_cport_pack(message->header, cport_id);
>> 491e60d63fde0a9 drivers/staging/greybus/es2.c    Johan Hovold       2015-04-07
>421
>> 821c620afa1ad29 drivers/staging/greybus/es2.c    Alex Elder         2015-06-13
>422  	buffer_size = sizeof(*message->header) + message->payload_size;
>> 491e60d63fde0a9 drivers/staging/greybus/es2.c    Johan Hovold       2015-04-07
>423
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-
>01-21  424  	usb_fill_bulk_urb(urb, udev,
>> 606addd2847ccc5 drivers/staging/greybus/es2.c    Alexandre Bailon   2015-06-
>15  425  			  usb_sndbulkpipe(udev,
>> 403074b50b66f1a drivers/staging/greybus/es2.c    Greg Kroah-Hartman 2016-
>08-17  426  					  es2->cport_out_endpoint),
>> 821c620afa1ad29 drivers/staging/greybus/es2.c    Alex Elder         2015-06-13
>427  			  message->buffer, buffer_size,
>> 7cf7bca9ec5659e drivers/staging/greybus/es2.c    Johan Hovold       2015-04-07
>428  			  cport_out_callback, message);
>> 977e209ab41073d drivers/staging/greybus/es2.c    Alexandre Bailon   2015-08-
>31  429  	urb->transfer_flags |= URB_ZERO_PACKET;
>> 495787a792ac498 drivers/staging/greybus/es2.c    Alex Elder         2016-06-03
>430
>> 495787a792ac498 drivers/staging/greybus/es2.c    Alex Elder         2016-06-03
>431  	trace_gb_message_submit(message);
>> 495787a792ac498 drivers/staging/greybus/es2.c    Alex Elder         2016-06-03
>432
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-
>01-21  433  	retval = usb_submit_urb(urb, gfp_mask);
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-
>01-21  434  	if (retval) {
>> 05e3095563670ab drivers/staging/greybus/es2.c    Viresh Kumar       2016-06-23
>435  		dev_err(&udev->dev, "failed to submit out-urb: %d\n", retval);
>> 3e136cc9e05e1a3 drivers/staging/greybus/es2.c    Johan Hovold       2015-07-01
>436
>> 4b1d82047ebbb10 drivers/staging/greybus/es2.c    Alex Elder         2015-10-27
>437  		spin_lock_irqsave(&es2->cport_out_urb_lock, flags);
>> 3e136cc9e05e1a3 drivers/staging/greybus/es2.c    Johan Hovold       2015-07-01
>438  		message->hcpriv = NULL;
>> 4b1d82047ebbb10 drivers/staging/greybus/es2.c    Alex Elder         2015-10-27
>@439  		spin_unlock_irqrestore(&es2->cport_out_urb_lock, flags);
>> 3e136cc9e05e1a3 drivers/staging/greybus/es2.c    Johan Hovold       2015-07-01
>440
>> 4b1d82047ebbb10 drivers/staging/greybus/es2.c    Alex Elder         2015-10-27
>441  		free_urb(es2, urb);
>> d29b3d631e57240 drivers/staging/greybus/es2.c    Alex Elder         2015-06-13
>442  		gb_message_cport_clear(message->header);
>> 3e136cc9e05e1a3 drivers/staging/greybus/es2.c    Johan Hovold       2015-07-01
>443
>> 3e136cc9e05e1a3 drivers/staging/greybus/es2.c    Johan Hovold       2015-07-01
>444  		return retval;
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-
>01-21  445  	}
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman 2015-
>01-21  446
>> 3e136cc9e05e1a3 drivers/staging/greybus/es2.c    Johan Hovold       2015-07-01
>447  	return 0;
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman
>> 2015-01-21  448  }
>> f587027e793cf89 drivers/staging/greybus/gb-es2.c Greg Kroah-Hartman
>> 2015-01-21  449
>>
>> :::::: The code at line 439 was first introduced by commit
>> :::::: 4b1d82047ebbb108e9961fffd51f8ccd54459221 greybus: es2: rename
>> es2 data structures
>>
>> :::::: TO: Alex Elder <elder@xxxxxxxxxx>
>> :::::: CC: Greg Kroah-Hartman <gregkh@xxxxxxxxxx>
>>
>> ---
>> 0-DAY CI Kernel Test Service, Intel Corporation
>> https://lists.01.org/hyperkitty/list/kbuild-all@xxxxxxxxxxxx
>
>Johan
>_______________________________________________
>kbuild-all mailing list -- kbuild-all@xxxxxxxxxxxx To unsubscribe send an email to
>kbuild-all-leave@xxxxxxxxxxxx