Andrew Morton wrote:
>> ...
>>
>> -static inline int simple_check_buf(struct usbtest_dev *tdev, struct urb *urb)
>> +static inline unsigned buffer_offset(void *buf)
>> +{
>> + return (unsigned)buf & (ARCH_KMALLOC_MINALIGN - 1);
>> +}
>>
>
> drivers/usb/misc/usbtest.c: In function 'buffer_offset':
> drivers/usb/misc/usbtest.c:273: warning: cast from pointer to integer of different size
>
>
Oops,
I don't see this warning, presumably because the platforms
I have compiled on have sizeof(unsigned) == sizeof(void *)
> That's easily enough fixed, but this is the only site in the kernel
> outside the core slab implementation which uses ARCH_KMALLOC_MINALIGN.
> This is a good hint that the code is doing something wrong.
>
> I can't advise as to what the code _should_ be doing because you didn't
> bother documenting it, and I can't be bothered reverse-engineering it.
>
The purpose of this patch is to check that HCDs behave correctly
when given a transfer buffer that is not aligned in any particular way.
This involves submitting an URB with the transfer buffer being
K + n where K is an address obtained by kmalloc().
When the URB completes we only have access to this transfer buffer
pointer but need to find the offset (n) that was added to ensure that
the memory between K and K + n - 1 has not been modified
(typical behaviour when the hardware has alignment requirements
for DMA and the HCD has not worked around that somehow)
The function above obtains n from the transfer buffer so we
can do the checks without having to store n somewhere.
It uses the fact that kmalloc will align to ARCH_KMALLOC_MINALIGN
to do so.
Earlier versions of this patch actually performed the test for multiple
values of n however the current version only checks the case n=1
(worst case scenario). This means that ARCH_KMALLOC_MINALIGN - 1
could actually be replaced by 1 however I left it in because:
a) I thought it made things slightly clearer...
b) It won't break if somone later modifies the code
to call usbtest_alloc_urb() with values of offset other than 0 or 1
Would this be better?:
+/*
+ * Obtain the offset of a transfer buffer within the containing
+ * kmalloc'd buffer.
+ * Will normally be zero except during the alignment tests.
+ */
static inline unsigned buffer_offset(void *buf)
{
- return (unsigned)buf & (ARCH_KMALLOC_MINALIGN - 1);
+ return (unsigned)((ptrdiff_t)buf & (ARCH_KMALLOC_MINALIGN - 1));
}
For discussion, will post as a proper follow up patch if OK
Regards,
Martin
--
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
