Skip to contentSkip to conversation

usb/gadget: fix gadgetfs aio support

PATCH In review Mathieu Laurendeau 3 participants 6 messages Jul 8, 2016

Subsystems (2)
Stable branch USB
lore
Share & tools
lore

Mathieu Laurendeau

author
#0 lore raw

PATCH 1/1 usb/gadget: fix gadgetfs aio support

Hi,

Since commit 7fe3976e0f3ab26f8ffd9430d3d2a19a70f2c8d2 ("gadget: switch ep_io_operations to ->read_iter/->write_iter") io submissions fail with ENODEV.

Fixes 7fe3976e0f3a ("gadget: switch ep_io_operations to

->read_iter/->write_iter")

Signed-off-by Mathieu Laurendeau <[email protected]>
---
 drivers/usb/gadget/legacy/inode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/usb/gadget/legacy/inode.c +1 -1
index aa3707b..120a03a 100644
@@ -542,7 +542,7 @@ static ssize_t ep_aio(struct kiocb *iocb,
 	 */
 	spin_lock_irq(&epdata->dev->lock);
 	value = -ENODEV;
-	if (unlikely(epdata->ep))
+	if (unlikely(epdata->ep) == NULL)
 		goto fail;
 
 	req = usb_ep_alloc_request(epdata->ep, GFP_ATOMIC);
Signature · 1 line
1.9.1
build bot
#1 lore raw
reproduce:

Hi,

[auto build test WARNING on balbi-usb/next]
[also build test WARNING on v4.7-rc6 next-20160707]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Mathieu-Laurendeau/usb-gadget-fix-gadgetfs-aio-support/20160708-164431
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
config: i386-randconfig-r0-07081252 (attached as .config)
compiler: gcc-6 (Debian 6.1.1-1) 6.1.1 20160430
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386

All warnings (new ones prefixed by >>):

drivers/usb/gadget/legacy/inode.c: In function 'ep_aio':

>> drivers/usb/gadget/legacy/inode.c:545:27: warning: comparison between pointer and integer
     if (unlikely(epdata->ep) == NULL)
                              ^~

vim +545 drivers/usb/gadget/legacy/inode.c

529 ssize_t value;
530
531 iocb->private = priv;
532 priv->iocb = iocb;
533
534 kiocb_set_cancel_fn(iocb, ep_aio_cancel);
535 get_ep(epdata);
536 priv->epdata = epdata;
537 priv->actual = 0;
538 priv->mm = current->mm; /* mm teardown waits for iocbs in exit_aio() */
539
540 /* each kiocb is coupled to one usb_request, but we can't
541 * allocate or submit those if the host disconnected.
542 */
543 spin_lock_irq(&epdata->dev->lock);
544 value = -ENODEV;

 > 545		if (unlikely(epdata->ep) == NULL)

546 goto fail;
547
548 req = usb_ep_alloc_request(epdata->ep, GFP_ATOMIC);
549 value = -ENOMEM;
550 if (unlikely(!req))
551 goto fail;
552
553 priv->req = req;

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

build bot
#2 lore raw
In reply to Mathieu Laurendeau
reproduce:

Hi,

[auto build test WARNING on balbi-usb/next]
[also build test WARNING on v4.7-rc6 next-20160707]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Mathieu-Laurendeau/usb-gadget-fix-gadgetfs-aio-support/20160708-164431
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
config: x86_64-randconfig-s5-07081546 (attached as .config)
compiler: gcc-6 (Debian 6.1.1-1) 6.1.1 20160430
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64

All warnings (new ones prefixed by >>):

In file included from include/linux/init.h:4:0,
from drivers/usb/gadget/legacy/inode.c:16:
drivers/usb/gadget/legacy/inode.c: In function 'ep_aio':
drivers/usb/gadget/legacy/inode.c:545:27: warning: comparison between pointer and integer

     if (unlikely(epdata->ep) == NULL)
                              ^
   include/linux/compiler.h:151:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^~~~
>> drivers/usb/gadget/legacy/inode.c:545:2: note: in expansion of macro 'if'
     if (unlikely(epdata->ep) == NULL)
     ^~
   drivers/usb/gadget/legacy/inode.c:545:27: warning: comparison between pointer and integer
     if (unlikely(epdata->ep) == NULL)
                              ^
   include/linux/compiler.h:151:42: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                             ^~~~
>> drivers/usb/gadget/legacy/inode.c:545:2: note: in expansion of macro 'if'
     if (unlikely(epdata->ep) == NULL)
     ^~
   drivers/usb/gadget/legacy/inode.c:545:27: warning: comparison between pointer and integer
     if (unlikely(epdata->ep) == NULL)
                              ^
   include/linux/compiler.h:162:16: note: in definition of macro '__trace_if'
      ______r = !!(cond);     \
                   ^~~~
>> drivers/usb/gadget/legacy/inode.c:545:2: note: in expansion of macro 'if'
     if (unlikely(epdata->ep) == NULL)
     ^~

--
In file included from /kbuild/src/stress/include/linux/init.h:4:0,
from /kbuild/src/stress/drivers/usb/gadget/legacy/inode.c:16:
/kbuild/src/stress/drivers/usb/gadget/legacy/inode.c: In function 'ep_aio':
/kbuild/src/stress/drivers/usb/gadget/legacy/inode.c:545:27: warning: comparison between pointer and integer

     if (unlikely(epdata->ep) == NULL)
                              ^
   /kbuild/src/stress/include/linux/compiler.h:151:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^~~~
>> /kbuild/src/stress/drivers/usb/gadget/legacy/inode.c:545:2: note: in expansion of macro 'if'
     if (unlikely(epdata->ep) == NULL)
     ^~
   /kbuild/src/stress/drivers/usb/gadget/legacy/inode.c:545:27: warning: comparison between pointer and integer
     if (unlikely(epdata->ep) == NULL)
                              ^
   /kbuild/src/stress/include/linux/compiler.h:151:42: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                             ^~~~
>> /kbuild/src/stress/drivers/usb/gadget/legacy/inode.c:545:2: note: in expansion of macro 'if'
     if (unlikely(epdata->ep) == NULL)
     ^~
   /kbuild/src/stress/drivers/usb/gadget/legacy/inode.c:545:27: warning: comparison between pointer and integer
     if (unlikely(epdata->ep) == NULL)
                              ^
   /kbuild/src/stress/include/linux/compiler.h:162:16: note: in definition of macro '__trace_if'
      ______r = !!(cond);     \
                   ^~~~
>> /kbuild/src/stress/drivers/usb/gadget/legacy/inode.c:545:2: note: in expansion of macro 'if'
     if (unlikely(epdata->ep) == NULL)
     ^~

vim +/if +545 drivers/usb/gadget/legacy/inode.c

529 ssize_t value;
530
531 iocb->private = priv;
532 priv->iocb = iocb;
533
534 kiocb_set_cancel_fn(iocb, ep_aio_cancel);
535 get_ep(epdata);
536 priv->epdata = epdata;
537 priv->actual = 0;
538 priv->mm = current->mm; /* mm teardown waits for iocbs in exit_aio() */
539
540 /* each kiocb is coupled to one usb_request, but we can't
541 * allocate or submit those if the host disconnected.
542 */
543 spin_lock_irq(&epdata->dev->lock);
544 value = -ENODEV;

 > 545		if (unlikely(epdata->ep) == NULL)

546 goto fail;
547
548 req = usb_ep_alloc_request(epdata->ep, GFP_ATOMIC);
549 value = -ENOMEM;
550 if (unlikely(!req))
551 goto fail;
552
553 priv->req = req;

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

build bot
#3 lore raw
In reply to Mathieu Laurendeau
reproduce:

Hi,

[auto build test WARNING on balbi-usb/next]
[also build test WARNING on v4.7-rc6 next-20160707]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Mathieu-Laurendeau/usb-gadget-fix-gadgetfs-aio-support/20160708-164431
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__

sparse warnings: (new ones prefixed by >>)

include/linux/compiler.h:232:8: sparse: attribute 'no_sanitize_address': unknown attribute

>> drivers/usb/gadget/legacy/inode.c:545:34: sparse: incompatible types for operation (==)
   drivers/usb/gadget/legacy/inode.c:545:34:    left side has type int
   drivers/usb/gadget/legacy/inode.c:545:34:    right side has type void *
   drivers/usb/gadget/legacy/inode.c: In function 'ep_aio':
   drivers/usb/gadget/legacy/inode.c:545:27: warning: comparison between pointer and integer
     if (unlikely(epdata->ep) == NULL)
                              ^~

vim +545 drivers/usb/gadget/legacy/inode.c

529 ssize_t value;
530
531 iocb->private = priv;
532 priv->iocb = iocb;
533
534 kiocb_set_cancel_fn(iocb, ep_aio_cancel);
535 get_ep(epdata);
536 priv->epdata = epdata;
537 priv->actual = 0;
538 priv->mm = current->mm; /* mm teardown waits for iocbs in exit_aio() */
539
540 /* each kiocb is coupled to one usb_request, but we can't
541 * allocate or submit those if the host disconnected.
542 */
543 spin_lock_irq(&epdata->dev->lock);
544 value = -ENODEV;

 > 545		if (unlikely(epdata->ep) == NULL)

546 goto fail;
547
548 req = usb_ep_alloc_request(epdata->ep, GFP_ATOMIC);
549 value = -ENOMEM;
550 if (unlikely(!req))
551 goto fail;
552
553 priv->req = req;

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

#4 lore raw
In reply to Mathieu Laurendeau

Hello.

Quoted 1 line · Mathieu Laurendeau

Hi,

Greetings should not be a part of the patch description.

Quoted 16 lines

Since commit 7fe3976e0f3ab26f8ffd9430d3d2a19a70f2c8d2 ("gadget: switch ep_io_operations to ->read_iter/->write_iter") io submissions fail with ENODEV.

Fixes 7fe3976e0f3a ("gadget: switch ep_io_operations to

->read_iter/->write_iter")

Signed-off-by Mathieu Laurendeau <[email protected]>
---
 drivers/usb/gadget/legacy/inode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/usb/gadget/legacy/inode.c +0 -0
index aa3707b..120a03a 100644

drivers/usb/gadget/legacy/inode.c static ssize_t ep_aio(struct kiocb *iocb, · line 545

Quoted 5 lines
drivers/usb/gadget/legacy/inode.c +1 -1
@@ -542,7 +542,7 @@ static ssize_t ep_aio(struct kiocb *iocb,
 	 */
 	spin_lock_irq(&epdata->dev->lock);
 	value = -ENODEV;
-	if (unlikely(epdata->ep))
+	if (unlikely(epdata->ep) == NULL)

This placement of the inner parens doesn't make sense, you should enclose
the whole comparison.

[...]

MBR, Sergei

author
#5 lore raw

I'll resend the patch. Sorry for the noise.

Quoted 36 lines · Sergei Shtylyov

Hello.

Quoted 1 line · Mathieu Laurendeau

Hi,

Greetings should not be a part of the patch description.

Quoted 16 lines

Since commit 7fe3976e0f3ab26f8ffd9430d3d2a19a70f2c8d2 ("gadget: switch ep_io_operations to ->read_iter/->write_iter") io submissions fail with ENODEV.

Fixes 7fe3976e0f3a ("gadget: switch ep_io_operations to

->read_iter/->write_iter")

Signed-off-by Mathieu Laurendeau <[email protected]>
---
 drivers/usb/gadget/legacy/inode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/usb/gadget/legacy/inode.c +0 -0
index aa3707b..120a03a 100644

drivers/usb/gadget/legacy/inode.c static ssize_t ep_aio(struct kiocb *iocb, · line 545

Quoted 5 lines
drivers/usb/gadget/legacy/inode.c +1 -1
@@ -542,7 +542,7 @@ static ssize_t ep_aio(struct kiocb *iocb,
      */
     spin_lock_irq(&epdata->dev->lock);
     value = -ENODEV;
-    if (unlikely(epdata->ep))
+    if (unlikely(epdata->ep) == NULL)

This placement of the inner parens doesn't make sense, you should enclose the whole comparison.

[...]

MBR, Sergei