Re: [PATCH v2] usb: core: buffer: smallest buffer should start at ARCH_DMA_MINALIGN
On 11/27/2014 04:50 AM, Greg Kroah-Hartman wrote:
>> @@ -171,8 +172,11 @@ struct usb_hcd {
>> struct usb_hcd *shared_hcd;
>> struct usb_hcd *primary_hcd;
>>
>> -
>> -#define HCD_BUFFER_POOLS4
>> +#if ARCH_KMALLOC_MINALIGN <= 64
>> +#define HCD_BUFFER_POOLS4
>> +#else
>> +#define HCD_BUFFER_POOLS3
>> +#endif
>> struct dma_pool *pool[HCD_BUFFER_POOLS];
>>
>> int state;
>
> This breaks the build so badly, it's obvious you never tested it out.
I'm deeply sorry for this. It compiles fine on ARM for instance. But on
x86 it uses "__alignof__(unsigned long long)" and gcc's preprocessor
can't handle this.
> never do that again.
>
> greg k-h
>
Sebastian
--
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
Re: [PATCH v2] usb: core: buffer: smallest buffer should start at ARCH_DMA_MINALIGN
On Wed, Nov 26, 2014 at 09:39:36PM +0100, Sebastian Andrzej Siewior wrote:
> the following error pops up during "testusb -a -t 10"
> | musb-hdrc musb-hdrc.1.auto: dma_pool_free buffer-128,
> f134e000/be842000 (bad dma)
> hcd_buffer_create() creates a few buffers, the smallest has 32 bytes of
> size. ARCH_KMALLOC_MINALIGN is set to 64 bytes. This combo results in
> hcd_buffer_alloc() returning memory which is 32bytes aligned and it
> might by identified by buffer_offset() as another buffer. This means the
> buffer which is on a 32byte boundary will not get freed, instead it
> tries to free another buffer with the error message.
>
> This patch fixes the issue by creating the smallest DMA buffer with the
> size of ARCH_DMA_MINALIGN. This will be either 32 or 64 bytes. If the
> ARCH_DMA_MINALIGN is 128 (currently powerpc64, mips ip32, x86 pentium 4) then
> it will create the first buffer with 128 bytes and will have only 3
> buffers. There is a BUILD_BUG_ON() now in case someone has a minalign of
> more than 128 bytes.
>
> Cc: sta...@vger.kernel.org
> Signed-off-by: Sebastian Andrzej Siewior
> Acked-by: Alan Stern
> ---
> v1…v2: rewrite pool_max so it is less confusing.
>
> drivers/usb/core/buffer.c | 12
> include/linux/usb/hcd.h | 8 ++--
> 2 files changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/core/buffer.c b/drivers/usb/core/buffer.c
> index 684ef70dc09d..5ad5f71d6358 100644
> --- a/drivers/usb/core/buffer.c
> +++ b/drivers/usb/core/buffer.c
> @@ -26,10 +26,13 @@ static const size_t pool_max[HCD_BUFFER_POOLS] = {
> /* platforms without dma-friendly caches might need to
>* prevent cacheline sharing...
>*/
> - 32,
> - 128,
> - 512,
> - PAGE_SIZE / 2
> +#if ARCH_KMALLOC_MINALIGN <= 32
> + 32, 128, 512, PAGE_SIZE / 2,
> +#elif ARCH_KMALLOC_MINALIGN <= 64
> + 64, 128, 512, PAGE_SIZE / 2,
> +#else
> + 128, 512, PAGE_SIZE / 2
> +#endif
G> /* bigger --> allocate pages */
> };
>
> @@ -58,6 +61,7 @@ int hcd_buffer_create(struct usb_hcd *hcd)
> !(hcd->driver->flags & HCD_LOCAL_MEM))
> return 0;
>
> + BUILD_BUG_ON(ARCH_KMALLOC_MINALIGN > 128);
> for (i = 0; i < HCD_BUFFER_POOLS; i++) {
> size = pool_max[i];
> if (!size)
> diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
> index cd96a2bc3388..1e2234ca448d 100644
> --- a/include/linux/usb/hcd.h
> +++ b/include/linux/usb/hcd.h
> @@ -23,6 +23,7 @@
>
> #include
> #include
> +#include
>
> #define MAX_TOPO_LEVEL 6
>
> @@ -171,8 +172,11 @@ struct usb_hcd {
> struct usb_hcd *shared_hcd;
> struct usb_hcd *primary_hcd;
>
> -
> -#define HCD_BUFFER_POOLS 4
> +#if ARCH_KMALLOC_MINALIGN <= 64
> + #define HCD_BUFFER_POOLS4
> +#else
> + #define HCD_BUFFER_POOLS3
> +#endif
> struct dma_pool *pool[HCD_BUFFER_POOLS];
>
> int state;
This breaks the build so badly, it's obvious you never tested it out.
never do that again.
greg k-h
--
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
Re: [PATCH v2] usb: core: buffer: smallest buffer should start at ARCH_DMA_MINALIGN
On Wed, Nov 26, 2014 at 04:27:27PM -0500, Alan Stern wrote: > On Wed, 26 Nov 2014, Sebastian Andrzej Siewior wrote: > > > the following error pops up during "testusb -a -t 10" > > | musb-hdrc musb-hdrc.1.auto: dma_pool_free buffer-128, > > f134e000/be842000 (bad dma) > > hcd_buffer_create() creates a few buffers, the smallest has 32 bytes of > > size. ARCH_KMALLOC_MINALIGN is set to 64 bytes. This combo results in > > hcd_buffer_alloc() returning memory which is 32bytes aligned and it > > might by identified by buffer_offset() as another buffer. This means the > > buffer which is on a 32byte boundary will not get freed, instead it > > tries to free another buffer with the error message. > > > > This patch fixes the issue by creating the smallest DMA buffer with the > > size of ARCH_DMA_MINALIGN. This will be either 32 or 64 bytes. If the > > ARCH_DMA_MINALIGN is 128 (currently powerpc64, mips ip32, x86 pentium 4) > > then > > it will create the first buffer with 128 bytes and will have only 3 > > buffers. There is a BUILD_BUG_ON() now in case someone has a minalign of > > 128 bytes or more. > > More than 128 bytes. Not "128 bytes or more". edited by hand, thanks. greg k-h -- 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
Re: [PATCH v2] usb: core: buffer: smallest buffer should start at ARCH_DMA_MINALIGN
On Wed, 26 Nov 2014, Sebastian Andrzej Siewior wrote: > the following error pops up during "testusb -a -t 10" > | musb-hdrc musb-hdrc.1.auto: dma_pool_free buffer-128, > f134e000/be842000 (bad dma) > hcd_buffer_create() creates a few buffers, the smallest has 32 bytes of > size. ARCH_KMALLOC_MINALIGN is set to 64 bytes. This combo results in > hcd_buffer_alloc() returning memory which is 32bytes aligned and it > might by identified by buffer_offset() as another buffer. This means the > buffer which is on a 32byte boundary will not get freed, instead it > tries to free another buffer with the error message. > > This patch fixes the issue by creating the smallest DMA buffer with the > size of ARCH_DMA_MINALIGN. This will be either 32 or 64 bytes. If the > ARCH_DMA_MINALIGN is 128 (currently powerpc64, mips ip32, x86 pentium 4) then > it will create the first buffer with 128 bytes and will have only 3 > buffers. There is a BUILD_BUG_ON() now in case someone has a minalign of > 128 bytes or more. More than 128 bytes. Not "128 bytes or more". > > Cc: sta...@vger.kernel.org > Signed-off-by: Sebastian Andrzej Siewior > --- > v1…v2: rewrite pool_max so it is less confusing. Acked-by: Alan Stern -- 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
[PATCH v2] usb: core: buffer: smallest buffer should start at ARCH_DMA_MINALIGN
the following error pops up during "testusb -a -t 10"
| musb-hdrc musb-hdrc.1.auto: dma_pool_free buffer-128, f134e000/be842000 (bad
dma)
hcd_buffer_create() creates a few buffers, the smallest has 32 bytes of
size. ARCH_KMALLOC_MINALIGN is set to 64 bytes. This combo results in
hcd_buffer_alloc() returning memory which is 32bytes aligned and it
might by identified by buffer_offset() as another buffer. This means the
buffer which is on a 32byte boundary will not get freed, instead it
tries to free another buffer with the error message.
This patch fixes the issue by creating the smallest DMA buffer with the
size of ARCH_DMA_MINALIGN. This will be either 32 or 64 bytes. If the
ARCH_DMA_MINALIGN is 128 (currently powerpc64, mips ip32, x86 pentium 4) then
it will create the first buffer with 128 bytes and will have only 3
buffers. There is a BUILD_BUG_ON() now in case someone has a minalign of
128 bytes or more.
Cc: sta...@vger.kernel.org
Signed-off-by: Sebastian Andrzej Siewior
---
v1…v2: rewrite pool_max so it is less confusing.
drivers/usb/core/buffer.c | 12
include/linux/usb/hcd.h | 8 ++--
2 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/core/buffer.c b/drivers/usb/core/buffer.c
index 684ef70dc09d..5ad5f71d6358 100644
--- a/drivers/usb/core/buffer.c
+++ b/drivers/usb/core/buffer.c
@@ -26,10 +26,13 @@ static const size_t pool_max[HCD_BUFFER_POOLS] = {
/* platforms without dma-friendly caches might need to
* prevent cacheline sharing...
*/
- 32,
- 128,
- 512,
- PAGE_SIZE / 2
+#if ARCH_KMALLOC_MINALIGN <= 32
+ 32, 128, 512, PAGE_SIZE / 2,
+#elif ARCH_KMALLOC_MINALIGN <= 64
+ 64, 128, 512, PAGE_SIZE / 2,
+#else
+ 128, 512, PAGE_SIZE / 2
+#endif
/* bigger --> allocate pages */
};
@@ -58,6 +61,7 @@ int hcd_buffer_create(struct usb_hcd *hcd)
!(hcd->driver->flags & HCD_LOCAL_MEM))
return 0;
+ BUILD_BUG_ON(ARCH_KMALLOC_MINALIGN > 128);
for (i = 0; i < HCD_BUFFER_POOLS; i++) {
size = pool_max[i];
if (!size)
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index cd96a2bc3388..1e2234ca448d 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -23,6 +23,7 @@
#include
#include
+#include
#define MAX_TOPO_LEVEL 6
@@ -171,8 +172,11 @@ struct usb_hcd {
struct usb_hcd *shared_hcd;
struct usb_hcd *primary_hcd;
-
-#define HCD_BUFFER_POOLS 4
+#if ARCH_KMALLOC_MINALIGN <= 64
+ #define HCD_BUFFER_POOLS4
+#else
+ #define HCD_BUFFER_POOLS3
+#endif
struct dma_pool *pool[HCD_BUFFER_POOLS];
int state;
--
2.1.3
--
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
