Re: [PATCH] USB: gadget/printer, fix sleep inside atomic
From: David Brownell
Date: Tue Jun 22 2010 - 01:33:16 EST
--- On Mon, 6/21/10, Jiri Slaby <jslaby@xxxxxxx> wrote:
> From: Jiri Slaby <jslaby@xxxxxxx>
> Subject: [PATCH] USB: gadget/printer, fix sleep inside atomic
> To: gregkh@xxxxxxx
> Cc: linux-usb@xxxxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx, jirislaby@xxxxxxxxx, "Craig W. Nadler" <craig@xxxxxxxxx>, "David Brownell" <dbrownell@xxxxxxxxxxxxxxxxxxxxx>
> Date: Monday, June 21, 2010, 8:02 AM
> Stanse found that sleep is called
> inside atomic context created by
> lock_printer_io spinlock in several functions. It's used in
> process
> context only and some functions sleep inside its critical
> section. As
> this is not allowed for spinlocks, switch it to mutex.
>
> Signed-off-by: Jiri Slaby <jslaby@xxxxxxx>
> Cc: Craig W. Nadler <craig@xxxxxxxxx>
> Cc: David Brownell <dbrownell@xxxxxxxxxxxxxxxxxxxxx>
ACK ... :)
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxx>
> ---
> drivers/usb/gadget/printer.c | 32
> ++++++++++++++++----------------
> 1 files changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/usb/gadget/printer.c
> b/drivers/usb/gadget/printer.c
> index 43abf55..4c3ac5c 100644
> --- a/drivers/usb/gadget/printer.c
> +++ b/drivers/usb/gadget/printer.c
> @@ -82,7 +82,7 @@ static struct class *usb_gadget_class;
> struct printer_dev {
> spinlock_t
> lock;
> /* lock this structure */
> /* lock buffer lists during read/write
> calls */
> - spinlock_t
> lock_printer_io;
> + struct mutex
> lock_printer_io;
> struct usb_gadget
> *gadget;
> struct usb_request
> *req; /* for control
> responses */
> u8
> config;
> @@ -567,7 +567,7 @@ printer_read(struct file *fd, char
> __user *buf, size_t len, loff_t *ptr)
>
> DBG(dev, "printer_read trying to read
> %d bytes\n", (int)len);
>
> -
> spin_lock(&dev->lock_printer_io);
> +
> mutex_lock(&dev->lock_printer_io);
> spin_lock_irqsave(&dev->lock,
> flags);
>
> /* We will use this flag later to check
> if a printer reset happened
> @@ -601,7 +601,7 @@ printer_read(struct file *fd, char
> __user *buf, size_t len, loff_t *ptr)
> * call or
> not.
> */
> if (fd->f_flags
> & (O_NONBLOCK|O_NDELAY)) {
> -
> spin_unlock(&dev->lock_printer_io);
> +
> mutex_unlock(&dev->lock_printer_io);
>
> return -EAGAIN;
> }
>
> @@ -648,7 +648,7 @@ printer_read(struct file *fd, char
> __user *buf, size_t len, loff_t *ptr)
> if
> (dev->reset_printer) {
>
> list_add(¤t_rx_req->list,
> &dev->rx_reqs);
>
> spin_unlock_irqrestore(&dev->lock, flags);
> -
> spin_unlock(&dev->lock_printer_io);
> +
> mutex_unlock(&dev->lock_printer_io);
>
> return -EAGAIN;
> }
>
> @@ -673,7 +673,7 @@ printer_read(struct file *fd, char
> __user *buf, size_t len, loff_t *ptr)
> dev->current_rx_buf =
> current_rx_buf;
>
>
> spin_unlock_irqrestore(&dev->lock, flags);
> -
> spin_unlock(&dev->lock_printer_io);
> +
> mutex_unlock(&dev->lock_printer_io);
>
> DBG(dev, "printer_read returned %d
> bytes\n", (int)bytes_copied);
>
> @@ -697,7 +697,7 @@ printer_write(struct file *fd, const
> char __user *buf, size_t len, loff_t *ptr)
> if (len == 0)
> return -EINVAL;
>
> -
> spin_lock(&dev->lock_printer_io);
> +
> mutex_lock(&dev->lock_printer_io);
> spin_lock_irqsave(&dev->lock,
> flags);
>
> /* Check if a printer reset happens
> while we have interrupts on */
> @@ -713,7 +713,7 @@ printer_write(struct file *fd, const
> char __user *buf, size_t len, loff_t *ptr)
> * a
> NON-Blocking call or not.
> */
> if (fd->f_flags
> & (O_NONBLOCK|O_NDELAY)) {
> -
> spin_unlock(&dev->lock_printer_io);
> +
> mutex_unlock(&dev->lock_printer_io);
>
> return -EAGAIN;
> }
>
> @@ -752,7 +752,7 @@ printer_write(struct file *fd, const
> char __user *buf, size_t len, loff_t *ptr)
>
> if
> (copy_from_user(req->buf, buf, size)) {
>
> list_add(&req->list, &dev->tx_reqs);
> -
> spin_unlock(&dev->lock_printer_io);
> +
> mutex_unlock(&dev->lock_printer_io);
>
> return bytes_copied;
> }
>
> @@ -766,14 +766,14 @@ printer_write(struct file *fd, const
> char __user *buf, size_t len, loff_t *ptr)
> if
> (dev->reset_printer) {
>
> list_add(&req->list, &dev->tx_reqs);
>
> spin_unlock_irqrestore(&dev->lock, flags);
> -
> spin_unlock(&dev->lock_printer_io);
> +
> mutex_unlock(&dev->lock_printer_io);
>
> return -EAGAIN;
> }
>
> if
> (usb_ep_queue(dev->in_ep, req, GFP_ATOMIC)) {
>
> list_add(&req->list, &dev->tx_reqs);
>
> spin_unlock_irqrestore(&dev->lock, flags);
> -
> spin_unlock(&dev->lock_printer_io);
> +
> mutex_unlock(&dev->lock_printer_io);
>
> return -EAGAIN;
> }
>
> @@ -782,7 +782,7 @@ printer_write(struct file *fd, const
> char __user *buf, size_t len, loff_t *ptr)
> }
>
>
> spin_unlock_irqrestore(&dev->lock, flags);
> -
> spin_unlock(&dev->lock_printer_io);
> +
> mutex_unlock(&dev->lock_printer_io);
>
> DBG(dev, "printer_write sent %d
> bytes\n", (int)bytes_copied);
>
> @@ -820,11 +820,11 @@ printer_poll(struct file *fd,
> poll_table *wait)
> unsigned long
> flags;
> int
> status = 0;
>
> -
> spin_lock(&dev->lock_printer_io);
> +
> mutex_lock(&dev->lock_printer_io);
> spin_lock_irqsave(&dev->lock,
> flags);
> setup_rx_reqs(dev);
>
> spin_unlock_irqrestore(&dev->lock, flags);
> -
> spin_unlock(&dev->lock_printer_io);
> +
> mutex_unlock(&dev->lock_printer_io);
>
> poll_wait(fd, &dev->rx_wait,
> wait);
> poll_wait(fd, &dev->tx_wait,
> wait);
> @@ -1461,7 +1461,7 @@ autoconf_fail:
> }
>
> spin_lock_init(&dev->lock);
> -
> spin_lock_init(&dev->lock_printer_io);
> +
> mutex_init(&dev->lock_printer_io);
> INIT_LIST_HEAD(&dev->tx_reqs);
>
> INIT_LIST_HEAD(&dev->tx_reqs_active);
> INIT_LIST_HEAD(&dev->rx_reqs);
> @@ -1594,7 +1594,7 @@ cleanup(void)
> {
> int status;
>
> -
> spin_lock(&usb_printer_gadget.lock_printer_io);
> +
> mutex_lock(&usb_printer_gadget.lock_printer_io);
> class_destroy(usb_gadget_class);
>
> unregister_chrdev_region(g_printer_devno, 2);
>
> @@ -1602,6 +1602,6 @@ cleanup(void)
> if (status)
> ERROR(dev,
> "usb_gadget_unregister_driver %x\n", status);
>
> -
> spin_unlock(&usb_printer_gadget.lock_printer_io);
> +
> mutex_unlock(&usb_printer_gadget.lock_printer_io);
> }
> module_exit(cleanup);
> --
> 1.7.1
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/