diff mbox series

[RFT,2/4] chaoskey: introduce asynchronous reads

Message ID 20200921113732.11524-3-oneukum@suse.com
State New
Headers show
Series [RFT,1/4] chaoskey: O_NONBLOCK in concurrent reads | expand

Commit Message

Oliver Neukum Sept. 21, 2020, 11:37 a.m. UTC 
This divides requesting IO and waiting for IO from each other.

Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
 drivers/usb/misc/chaoskey.c | 39 +++++++++++++++++++++++++++++++--------
 1 file changed, 31 insertions(+), 8 deletions(-)

Comments

kernel test robot Sept. 21, 2020, 3:25 p.m. UTC | #1 
Hi Oliver,

I love your patch! Perhaps something to improve:

[auto build test WARNING on usb/usb-testing]
[also build test WARNING on v5.9-rc6 next-20200921]
[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]

url:    https://github.com/0day-ci/linux/commits/Oliver-Neukum/chaoskey-O_NONBLOCK-in-concurrent-reads/20200921-203804
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=sh 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/usb/misc/chaoskey.c: In function 'chaoskey_request_fill':
>> drivers/usb/misc/chaoskey.c:340:7: warning: variable 'started' set but not used [-Wunused-but-set-variable]

     340 |  bool started;
         |       ^~~~~~~

# https://github.com/0day-ci/linux/commit/4393931662d77cd7c609d4cfd82ad59900c9b13b
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Oliver-Neukum/chaoskey-O_NONBLOCK-in-concurrent-reads/20200921-203804
git checkout 4393931662d77cd7c609d4cfd82ad59900c9b13b
vim +/started +340 drivers/usb/misc/chaoskey.c

0ca10122ca08d2 Oliver Neukum 2016-02-17  335  
4393931662d77c Oliver Neukum 2020-09-21  336  static int chaoskey_request_fill(struct chaoskey *dev)
66e3e591891da9 Keith Packard 2015-03-19  337  {
66e3e591891da9 Keith Packard 2015-03-19  338  	DEFINE_WAIT(wait);
66e3e591891da9 Keith Packard 2015-03-19  339  	int result;
e4a886e811cd07 Bob Ham       2016-06-03 @340  	bool started;
66e3e591891da9 Keith Packard 2015-03-19  341  
4393931662d77c Oliver Neukum 2020-09-21  342  	usb_dbg(dev->interface, "request fill");
66e3e591891da9 Keith Packard 2015-03-19  343  
66e3e591891da9 Keith Packard 2015-03-19  344  	/* Return immediately if someone called before the buffer was
66e3e591891da9 Keith Packard 2015-03-19  345  	 * empty */
66e3e591891da9 Keith Packard 2015-03-19  346  	if (dev->valid != dev->used) {
66e3e591891da9 Keith Packard 2015-03-19  347  		usb_dbg(dev->interface, "not empty yet (valid %d used %d)",
66e3e591891da9 Keith Packard 2015-03-19  348  			dev->valid, dev->used);
66e3e591891da9 Keith Packard 2015-03-19  349  		return 0;
66e3e591891da9 Keith Packard 2015-03-19  350  	}
66e3e591891da9 Keith Packard 2015-03-19  351  
66e3e591891da9 Keith Packard 2015-03-19  352  	/* Bail if the device has been removed */
66e3e591891da9 Keith Packard 2015-03-19  353  	if (!dev->present) {
66e3e591891da9 Keith Packard 2015-03-19  354  		usb_dbg(dev->interface, "device not present");
66e3e591891da9 Keith Packard 2015-03-19  355  		return -ENODEV;
66e3e591891da9 Keith Packard 2015-03-19  356  	}
66e3e591891da9 Keith Packard 2015-03-19  357  
66e3e591891da9 Keith Packard 2015-03-19  358  	/* Make sure the device is awake */
66e3e591891da9 Keith Packard 2015-03-19  359  	result = usb_autopm_get_interface(dev->interface);
66e3e591891da9 Keith Packard 2015-03-19  360  	if (result) {
66e3e591891da9 Keith Packard 2015-03-19  361  		usb_dbg(dev->interface, "wakeup failed (result %d)", result);
66e3e591891da9 Keith Packard 2015-03-19  362  		return result;
66e3e591891da9 Keith Packard 2015-03-19  363  	}
66e3e591891da9 Keith Packard 2015-03-19  364  
0ca10122ca08d2 Oliver Neukum 2016-02-17  365  	dev->reading = true;
0ca10122ca08d2 Oliver Neukum 2016-02-17  366  	result = usb_submit_urb(dev->urb, GFP_KERNEL);
0ca10122ca08d2 Oliver Neukum 2016-02-17  367  	if (result < 0) {
0ca10122ca08d2 Oliver Neukum 2016-02-17  368  		result = usb_translate_errors(result);
0ca10122ca08d2 Oliver Neukum 2016-02-17  369  		dev->reading = false;
0ca10122ca08d2 Oliver Neukum 2016-02-17  370  		goto out;
0ca10122ca08d2 Oliver Neukum 2016-02-17  371  	}
0ca10122ca08d2 Oliver Neukum 2016-02-17  372  
e4a886e811cd07 Bob Ham       2016-06-03  373  	/* The first read on the Alea takes a little under 2 seconds.
e4a886e811cd07 Bob Ham       2016-06-03  374  	 * Reads after the first read take only a few microseconds
e4a886e811cd07 Bob Ham       2016-06-03  375  	 * though.  Presumably the entropy-generating circuit needs
e4a886e811cd07 Bob Ham       2016-06-03  376  	 * time to ramp up.  So, we wait longer on the first read.
e4a886e811cd07 Bob Ham       2016-06-03  377  	 */
e4a886e811cd07 Bob Ham       2016-06-03  378  	started = dev->reads_started;
e4a886e811cd07 Bob Ham       2016-06-03  379  	dev->reads_started = true;
4393931662d77c Oliver Neukum 2020-09-21  380  	/*
4393931662d77c Oliver Neukum 2020-09-21  381  	 * powering down while a read is under way
4393931662d77c Oliver Neukum 2020-09-21  382  	 * is blocked in suspend()
4393931662d77c Oliver Neukum 2020-09-21  383  	 */
4393931662d77c Oliver Neukum 2020-09-21  384  	usb_autopm_put_interface(dev->interface);
4393931662d77c Oliver Neukum 2020-09-21  385  	return 0;
4393931662d77c Oliver Neukum 2020-09-21  386  out:
4393931662d77c Oliver Neukum 2020-09-21  387  	usb_autopm_put_interface(dev->interface);
4393931662d77c Oliver Neukum 2020-09-21  388  	return result;
4393931662d77c Oliver Neukum 2020-09-21  389  }
4393931662d77c Oliver Neukum 2020-09-21  390  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff mbox series

Patch

diff --git a/drivers/usb/misc/chaoskey.c b/drivers/usb/misc/chaoskey.c
index ad4c0b6d02cf..d47c2cc65269 100644
--- a/drivers/usb/misc/chaoskey.c
+++ b/drivers/usb/misc/chaoskey.c
@@ -336,15 +336,13 @@  static void chaos_read_callback(struct urb *urb)
 	wake_up(&dev->wait_q);
 }
 
-/* Fill the buffer. Called with dev->lock held
- */
-static int _chaoskey_fill(struct chaoskey *dev)
+static int chaoskey_request_fill(struct chaoskey *dev)
 {
 	DEFINE_WAIT(wait);
 	int result;
 	bool started;
 
-	usb_dbg(dev->interface, "fill");
+	usb_dbg(dev->interface, "request fill");
 
 	/* Return immediately if someone called before the buffer was
 	 * empty */
@@ -382,10 +380,26 @@  static int _chaoskey_fill(struct chaoskey *dev)
 	 */
 	started = dev->reads_started;
 	dev->reads_started = true;
+	/*
+	 * powering down while a read is under way
+	 * is blocked in suspend()
+	 */
+	usb_autopm_put_interface(dev->interface);
+	return 0;
+out:
+	usb_autopm_put_interface(dev->interface);
+	return result;
+}
+
+static int chaoskey_wait_fill(struct chaoskey *dev)
+{
+	DEFINE_WAIT(wait);
+	int result;
+
 	result = wait_event_interruptible_timeout(
 		dev->wait_q,
 		!dev->reading,
-		(started ? NAK_TIMEOUT : ALEA_FIRST_TIMEOUT) );
+		(dev->reads_started ? NAK_TIMEOUT : ALEA_FIRST_TIMEOUT) );
 
 	if (result < 0) {
 		usb_kill_urb(dev->urb);
@@ -400,7 +414,6 @@  static int _chaoskey_fill(struct chaoskey *dev)
 	}
 out:
 	/* Let the device go back to sleep eventually */
-	usb_autopm_put_interface(dev->interface);
 
 	usb_dbg(dev->interface, "read %d bytes", dev->valid);
 
@@ -449,7 +462,12 @@  static ssize_t chaoskey_read(struct file *file,
 				goto bail;
 		}
 		if (dev->valid == dev->used) {
-			result = _chaoskey_fill(dev);
+			result = chaoskey_request_fill(dev);
+			if (result < 0) {
+				mutex_unlock(&dev->lock);
+				goto bail;
+			}
+			result = chaoskey_wait_fill(dev);
 			if (result < 0) {
 				mutex_unlock(&dev->lock);
 				goto bail;
@@ -517,7 +535,7 @@  static int chaoskey_rng_read(struct hwrng *rng, void *data,
 	 * the buffer will still be empty
 	 */
 	if (dev->valid == dev->used)
-		(void) _chaoskey_fill(dev);
+		(void) chaoskey_request_fill(dev);
 
 	this_time = dev->valid - dev->used;
 	if (this_time > max)
@@ -537,6 +555,11 @@  static int chaoskey_rng_read(struct hwrng *rng, void *data,
 static int chaoskey_suspend(struct usb_interface *interface,
 			    pm_message_t message)
 {
+	struct chaoskey *dev = usb_get_intfdata(interface);
+
+	if (dev->reading && PMSG_IS_AUTO(message))
+		return -EBUSY;
+
 	usb_dbg(interface, "suspend");
 	return 0;
 }