Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751539Ab0B0FiN (ORCPT ); Sat, 27 Feb 2010 00:38:13 -0500 Received: from fg-out-1718.google.com ([72.14.220.159]:6749 "EHLO fg-out-1718.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751256Ab0B0FiL convert rfc822-to-8bit (ORCPT ); Sat, 27 Feb 2010 00:38:11 -0500 DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; b=wOYlt/jPPFqpG/cu1qzil/xLVckSZVbkFVl7U1tgbRCsRa4u8haklK6lkyAHPtUnSx OjstkXpWb6qvW8p3T62tNFuA9kBlt3gEC5kUJmGDslOQAxa4W0JdwIx6wLoKnv6GQA7Y msiXYLc3GnrbFuMCsRtsjR+UHopMV7Niv44ak= MIME-Version: 1.0 In-Reply-To: <20100227052648.GA31418@kroah.com> References: <20100227035639.GA11680@kroah.com> <20100227041815.GA12956@kroah.com> <20100227051737.GA14976@kroah.com> <20100227052648.GA31418@kroah.com> Date: Sat, 27 Feb 2010 06:38:09 +0100 Message-ID: Subject: Re: 2.6.33 bugs (USBFS, Intel graphic) From: Markus Rechberger To: Greg KH Cc: Linus Torvalds , linux-usb@vger.kernel.org, werner@guyane.dyn-o-saur.com, Marcus Meissner , linux-kernel@vger.kernel.org Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3978 Lines: 101 On Sat, Feb 27, 2010 at 6:26 AM, Greg KH wrote: > On Fri, Feb 26, 2010 at 09:17:37PM -0800, Greg KH wrote: >> On Sat, Feb 27, 2010 at 05:34:27AM +0100, Markus Rechberger wrote: >> > On Sat, Feb 27, 2010 at 5:29 AM, Linus Torvalds >> > wrote: >> > > >> > > >> > > On Fri, 26 Feb 2010, Greg KH wrote: >> > >> >> > >> Yes, and that patch didn't touch the iso frames. ?That happens later on >> > >> in the functions that were modified. ?The patch should not have had any >> > >> affect on iso transfers. ?Unless I'm missing something? >> > > >> > > Hmm. What seems to happen is that for an isochronous transfer, the buffer >> > > is split for each microframe. No? >> > > >> > >> > exactly. and each microframe has its own buffer length identifier. >> > >> > the current behaviour breaks VMware, QEMU and virtualbox .. probably >> > other things too. >> > >> > >> > > So the total length may be in 'urb->actual_length', but the actual data in >> > > the buffer may not be contiguous, because it's created from multiple >> > > smaller frames, some of which might not be full length? >> > > >> > >> > yes, it's only contiguous for BULK. >> > >> > > I dunno. That would explain the problem - actual_length is correct, but >> > > the 'copy_to_user()' still doesn't copy all the data, because it's >> > > fragmented. >> > > >> > >> > no you got it, but your patch does not work. The best way would be to >> > revert it if someone wants to speed up BULK it should go down another >> > path, leaving the old working implementation untouched. >> >> Hm, so it's back to the original idea of just doing a kzalloc of the >> initial buffer, that should solve the problem that Marcus found. >> >> I'll go dig that back up and if you could test it, that would be most >> appreciated. > > Here, can you try this on top of everything? > just tested it, everything's back to normal again now! thanks, Markus > thanks, > > greg k-h > > > diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c > index a678186..252d3b4 100644 > --- a/drivers/usb/core/devio.c > +++ b/drivers/usb/core/devio.c > @@ -1168,7 +1168,7 @@ static int proc_do_submiturb(struct dev_state *ps, struct usbdevfs_urb *uurb, > ? ? ? ? ? ? ? ?return -ENOMEM; > ? ? ? ?} > ? ? ? ?if (uurb->buffer_length > 0) { > - ? ? ? ? ? ? ? as->urb->transfer_buffer = kmalloc(uurb->buffer_length, > + ? ? ? ? ? ? ? as->urb->transfer_buffer = kzalloc(uurb->buffer_length, > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?GFP_KERNEL); > ? ? ? ? ? ? ? ?if (!as->urb->transfer_buffer) { > ? ? ? ? ? ? ? ? ? ? ? ?kfree(isopkt); > @@ -1312,9 +1312,9 @@ static int processcompl(struct async *as, void __user * __user *arg) > ? ? ? ?void __user *addr = as->userurb; > ? ? ? ?unsigned int i; > > - ? ? ? if (as->userbuffer && urb->actual_length) > + ? ? ? if (as->userbuffer) > ? ? ? ? ? ? ? ?if (copy_to_user(as->userbuffer, urb->transfer_buffer, > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?urb->actual_length)) > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?urb->transfer_buffer_length)) > ? ? ? ? ? ? ? ? ? ? ? ?goto err_out; > ? ? ? ?if (put_user(as->status, &userurb->status)) > ? ? ? ? ? ? ? ?goto err_out; > @@ -1480,9 +1480,9 @@ static int processcompl_compat(struct async *as, void __user * __user *arg) > ? ? ? ?void __user *addr = as->userurb; > ? ? ? ?unsigned int i; > > - ? ? ? if (as->userbuffer && urb->actual_length) > + ? ? ? if (as->userbuffer) > ? ? ? ? ? ? ? ?if (copy_to_user(as->userbuffer, urb->transfer_buffer, > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?urb->actual_length)) > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?urb->transfer_buffer_length)) > ? ? ? ? ? ? ? ? ? ? ? ?return -EFAULT; > ? ? ? ?if (put_user(as->status, &userurb->status)) > ? ? ? ? ? ? ? ?return -EFAULT; > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/