Hi, On 03/28/2014 10:27 AM, oliver@xxxxxxxxxx wrote: > From: Oliver Neukum <oneukum@xxxxxxx> > > There is an allocation with GFP_NOIO with a spinlock > held that the checker didn't catch. ACK, my bad. When I wrote this commit: http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/drivers/usb/storage/uas.c?id=e36e64930cffd94e1c37fdb82f35989384aa946b I did not take spinlocks into account, only the general context in which the code-block was being called. My bad. So combined with your previous patch in essence this means that commit e36e64930cffd94e1c37fdb82f35989384aa946b should be reverted, it might be better to do a new patch doing just that (so that history clearly shows that commit was bad, and has been completely undone. > There are also two allocations with GFP_KERNEL in the pre-/post_reset > code paths. That is no good because that is a part of the SCSI error handler. Ah right, those should indeed be GFP_NOIO. Can you respin the 2 patches into a revert of e36e64930 + a separate patch fixing the pre / post reset handlers? I think that would be cleaner. Otherwise ACK. Thanks & Regards, Hans > > Signed-off-by: Oliver Neukum <oliver@xxxxxxxxxx> > --- > drivers/usb/storage/uas.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c > index befa0c0..b2bd8ea 100644 > --- a/drivers/usb/storage/uas.c > +++ b/drivers/usb/storage/uas.c > @@ -137,7 +137,7 @@ static void uas_do_work(struct work_struct *work) > if (!(cmdinfo->state & IS_IN_WORK_LIST)) > continue; > > - err = uas_submit_urbs(cmnd, cmnd->device->hostdata, GFP_NOIO); > + err = uas_submit_urbs(cmnd, cmnd->device->hostdata, GFP_ATOMIC); > if (!err) > cmdinfo->state &= ~IS_IN_WORK_LIST; > else > @@ -1030,7 +1030,7 @@ static int uas_configure_endpoints(struct uas_dev_info *devinfo) > devinfo->use_streams = 0; > } else { > devinfo->qdepth = usb_alloc_streams(devinfo->intf, eps + 1, > - 3, 256, GFP_KERNEL); > + 3, 256, GFP_NOIO); > if (devinfo->qdepth < 0) > return devinfo->qdepth; > devinfo->use_streams = 1; > @@ -1047,7 +1047,7 @@ static void uas_free_streams(struct uas_dev_info *devinfo) > eps[0] = usb_pipe_endpoint(udev, devinfo->status_pipe); > eps[1] = usb_pipe_endpoint(udev, devinfo->data_in_pipe); > eps[2] = usb_pipe_endpoint(udev, devinfo->data_out_pipe); > - usb_free_streams(devinfo->intf, eps, 3, GFP_KERNEL); > + usb_free_streams(devinfo->intf, eps, 3, GFP_NOIO); > } > > static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
