On Sat, Dec 05, 2015 at 05:12:03PM -0500, Alan Stern wrote:
> To tell the truth, I'm not sure whether it would be a problem or not.
> That's why I said it "may" not be a good idea.
Fair enough.
>>> You don't really need to do it earlier. An unnecessary calculation of
>>> num_sgs won't hurt.
>> Is it unnecessary? I thought the test was really to force num_sgs==0 for the
>> DMA case, not to save a little arithmetic.
> Well, if you calculate num_sgs and then force it to 0 anyway, the
> calculation was unnecessary, right?
But the logic never calculates num_sgs if usbm == NULL? The if terminates
early and num_sgs stays 0.
Granted, the last version of the patch botched this and inverted the
condition. I'll fix that.
> BTW, in the future, could you put your patches in the body of the
> emails instead of making them attachments? That's how we normally do
> things on this mailing list; it makes replying and commenting on
> patches easier.
Will do.
>> - num_sgs = DIV_ROUND_UP(uurb->buffer_length, USB_SG_SIZE);
>> - if (num_sgs == 1 || num_sgs > ps->dev->bus->sg_tablesize)
>> - num_sgs = 0;
>> + /* do not use SG buffers when memory mapped segments
>> + * are in use
>> + */
>> + if (as->usbm) {
>> + num_sgs = DIV_ROUND_UP(uurb->buffer_length,
>> + USB_SG_SIZE);
>> + if (num_sgs == 1 ||
>> + num_sgs > ps->dev->bus->sg_tablesize) {
>> + num_sgs = 0;
>> + }
>> + }
> No, no. Leave this part of the code unchanged. as hasn't even been
> allocated yet.
Good point. I did warn you about the lack of testing... :-)
By “unchanged”, do you mean unchanged from previous patch or from the
original? Because this was here all along from Markus' version of the patch.
> As pointed out repeatedly by the kbuild test robot, this should be
> IS_ERR, not IS_ERR_VALUE. Also, you need to set as->usbm back to NULL
> before jumping.
>
> At this point, set num_sgs to 0 if as->usbm is non-NULL.
Didn't you just point out that this would be redundant calculation?
I'll try to update your patch with the other suggestions tomorrow. Thanks!
/* Steinar */
--
Homepage: https://www.sesse.net/
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
