diff mbox series

usb: yurex: make waiting on yurex_write interruptible

Message ID 20240923141649.148563-1-oneukum@suse.com (mailing list archive)
State Superseded
Headers show
Series usb: yurex: make waiting on yurex_write interruptible | expand

Commit Message

Oliver Neukum Sept. 23, 2024, 2:16 p.m. UTC
The IO yurex_write() needs to wait for in order to have a device
ready for writing again can take a long time time.
Consequently the sleep is done in an interruptible state.
Therefore others waiting for yurex_write() itself to finish should
use mutex_lock_interruptible.

Signed-off-by: Oliver Neukum <oneukum@suse.com>
Fixes: 6bc235a2e24a5 ("USB: add driver for Meywa-Denki & Kayac YUREX")
---
 drivers/usb/misc/yurex.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

kernel test robot Sept. 23, 2024, 8:15 p.m. UTC | #1
Hi Oliver,

kernel test robot noticed the following build warnings:

[auto build test WARNING on usb/usb-testing]
[also build test WARNING on usb/usb-next usb/usb-linus westeri-thunderbolt/next linus/master v6.11 next-20240923]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Oliver-Neukum/usb-yurex-make-waiting-on-yurex_write-interruptible/20240923-221833
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link:    https://lore.kernel.org/r/20240923141649.148563-1-oneukum%40suse.com
patch subject: [PATCH] usb: yurex: make waiting on yurex_write interruptible
config: x86_64-buildonly-randconfig-004-20240924 (https://download.01.org/0day-ci/archive/20240924/202409240433.Bl9ay4Ua-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240924/202409240433.Bl9ay4Ua-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409240433.Bl9ay4Ua-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/usb/misc/yurex.c:444:6: warning: variable 'retval' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
     444 |         if (count == 0)
         |             ^~~~~~~~~~
   drivers/usb/misc/yurex.c:524:9: note: uninitialized use occurs here
     524 |         return retval;
         |                ^~~~~~
   drivers/usb/misc/yurex.c:444:2: note: remove the 'if' if its condition is always false
     444 |         if (count == 0)
         |         ^~~~~~~~~~~~~~~
     445 |                 goto error;
         |                 ~~~~~~~~~~
   drivers/usb/misc/yurex.c:433:24: note: initialize the variable 'retval' to silence this warning
     433 |         int i, set = 0, retval;
         |                               ^
         |                                = 0
   1 warning generated.


vim +444 drivers/usb/misc/yurex.c

6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  428  
1cc373c654acde Sudip Mukherjee     2014-10-10  429  static ssize_t yurex_write(struct file *file, const char __user *user_buffer,
1cc373c654acde Sudip Mukherjee     2014-10-10  430  			   size_t count, loff_t *ppos)
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  431  {
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  432  	struct usb_yurex *dev;
e9cac1c1ecfe84 Oliver Neukum       2024-09-23  433  	int i, set = 0, retval;
7e10f14ebface4 Ben Hutchings       2018-08-15  434  	char buffer[16 + 1];
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  435  	char *data = buffer;
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  436  	unsigned long long c, c2 = 0;
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  437  	signed long timeout = 0;
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  438  	DEFINE_WAIT(wait);
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  439  
7e10f14ebface4 Ben Hutchings       2018-08-15  440  	count = min(sizeof(buffer) - 1, count);
113ad911ad4a1c Arjun Sreedharan    2014-08-19  441  	dev = file->private_data;
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  442  
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  443  	/* verify that we actually have some data to write */
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29 @444  	if (count == 0)
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  445  		goto error;
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  446  
e9cac1c1ecfe84 Oliver Neukum       2024-09-23  447  	retval = mutex_lock_interruptible(&dev->io_mutex);
e9cac1c1ecfe84 Oliver Neukum       2024-09-23  448  	if (retval < 0)
e9cac1c1ecfe84 Oliver Neukum       2024-09-23  449  		return -EINTR;
e9cac1c1ecfe84 Oliver Neukum       2024-09-23  450  
aafb00a977cf7d Johan Hovold        2019-10-09  451  	if (dev->disconnected) {		/* already disconnected */
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  452  		mutex_unlock(&dev->io_mutex);
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  453  		retval = -ENODEV;
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  454  		goto error;
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  455  	}
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  456  
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  457  	if (copy_from_user(buffer, user_buffer, count)) {
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  458  		mutex_unlock(&dev->io_mutex);
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  459  		retval = -EFAULT;
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  460  		goto error;
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  461  	}
7e10f14ebface4 Ben Hutchings       2018-08-15  462  	buffer[count] = 0;
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  463  	memset(dev->cntl_buffer, CMD_PADDING, YUREX_BUF_SIZE);
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  464  
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  465  	switch (buffer[0]) {
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  466  	case CMD_ANIMATE:
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  467  	case CMD_LED:
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  468  		dev->cntl_buffer[0] = buffer[0];
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  469  		dev->cntl_buffer[1] = buffer[1];
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  470  		dev->cntl_buffer[2] = CMD_EOF;
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  471  		break;
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  472  	case CMD_READ:
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  473  	case CMD_VERSION:
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  474  		dev->cntl_buffer[0] = buffer[0];
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  475  		dev->cntl_buffer[1] = 0x00;
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  476  		dev->cntl_buffer[2] = CMD_EOF;
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  477  		break;
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  478  	case CMD_SET:
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  479  		data++;
0d9b6d49fe39bd Gustavo A. R. Silva 2020-07-07  480  		fallthrough;
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  481  	case '0' ... '9':
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  482  		set = 1;
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  483  		c = c2 = simple_strtoull(data, NULL, 0);
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  484  		dev->cntl_buffer[0] = CMD_SET;
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  485  		for (i = 1; i < 6; i++) {
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  486  			dev->cntl_buffer[i] = (c>>32) & 0xff;
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  487  			c <<= 8;
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  488  		}
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  489  		buffer[6] = CMD_EOF;
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  490  		break;
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  491  	default:
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  492  		mutex_unlock(&dev->io_mutex);
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  493  		return -EINVAL;
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  494  	}
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  495  
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  496  	/* send the data as the control msg */
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  497  	prepare_to_wait(&dev->waitq, &wait, TASK_INTERRUPTIBLE);
aadd6472d904c3 Greg Kroah-Hartman  2012-05-01  498  	dev_dbg(&dev->interface->dev, "%s - submit %c\n", __func__,
aadd6472d904c3 Greg Kroah-Hartman  2012-05-01  499  		dev->cntl_buffer[0]);
f176ede3a3bde5 Alan Stern          2020-08-10  500  	retval = usb_submit_urb(dev->cntl_urb, GFP_ATOMIC);
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  501  	if (retval >= 0)
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  502  		timeout = schedule_timeout(YUREX_WRITE_TIMEOUT);
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  503  	finish_wait(&dev->waitq, &wait);
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  504  
372c93131998c0 Johan Hovold        2020-12-14  505  	/* make sure URB is idle after timeout or (spurious) CMD_ACK */
372c93131998c0 Johan Hovold        2020-12-14  506  	usb_kill_urb(dev->cntl_urb);
372c93131998c0 Johan Hovold        2020-12-14  507  
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  508  	mutex_unlock(&dev->io_mutex);
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  509  
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  510  	if (retval < 0) {
45714104b9e85f Greg Kroah-Hartman  2012-04-20  511  		dev_err(&dev->interface->dev,
45714104b9e85f Greg Kroah-Hartman  2012-04-20  512  			"%s - failed to send bulk msg, error %d\n",
45714104b9e85f Greg Kroah-Hartman  2012-04-20  513  			__func__, retval);
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  514  		goto error;
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  515  	}
93907620b30860 Oliver Neukum       2024-09-12  516  	if (set && timeout) {
93907620b30860 Oliver Neukum       2024-09-12  517  		spin_lock_irq(&dev->lock);
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  518  		dev->bbu = c2;
93907620b30860 Oliver Neukum       2024-09-12  519  		spin_unlock_irq(&dev->lock);
93907620b30860 Oliver Neukum       2024-09-12  520  	}
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  521  	return timeout ? count : -EIO;
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  522  
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  523  error:
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  524  	return retval;
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  525  }
6bc235a2e24a5e Tomoki Sekiyama     2010-09-29  526
diff mbox series

Patch

diff --git a/drivers/usb/misc/yurex.c b/drivers/usb/misc/yurex.c
index 4a9859e03f6b..0deffdc8205f 100644
--- a/drivers/usb/misc/yurex.c
+++ b/drivers/usb/misc/yurex.c
@@ -430,7 +430,7 @@  static ssize_t yurex_write(struct file *file, const char __user *user_buffer,
 			   size_t count, loff_t *ppos)
 {
 	struct usb_yurex *dev;
-	int i, set = 0, retval = 0;
+	int i, set = 0, retval;
 	char buffer[16 + 1];
 	char *data = buffer;
 	unsigned long long c, c2 = 0;
@@ -444,7 +444,10 @@  static ssize_t yurex_write(struct file *file, const char __user *user_buffer,
 	if (count == 0)
 		goto error;
 
-	mutex_lock(&dev->io_mutex);
+	retval = mutex_lock_interruptible(&dev->io_mutex);
+	if (retval < 0)
+		return -EINTR;
+
 	if (dev->disconnected) {		/* already disconnected */
 		mutex_unlock(&dev->io_mutex);
 		retval = -ENODEV;