gmane.linux.kernel.wireless.general

Johannes Berg | 6 Oct 18:00

[PATCH] iwlwifi: fix DMA code and bugs

This patch cleans up the DMA code to be understandable and not
completely wrong. In particular:
 * there is no need to have a weird iwl_tfd_frame_data struct that is
   used 10 times, just use an address struct 20 times
 * therefore, all the is_odd junk goes away
 * fix a bug in iwl_hcmd_queue_reclaim where it would reclaim all the
   fragments of a descriptor rather than all descriptors (this may be
   the cause of the dma unmapping problem I reported)
 * some more cleanups

Signed-off-by: Johannes Berg <johannes@...>
---
Tested on 5000 hw, please apply.

 drivers/net/wireless/iwlwifi/iwl-4965-hw.h |   98 ++++++++++++++---------------
 drivers/net/wireless/iwlwifi/iwl-5000.c    |    2 
 drivers/net/wireless/iwlwifi/iwl-helpers.h |    5 -
 drivers/net/wireless/iwlwifi/iwl-tx.c      |   84 +++++-------------------
 4 files changed, 70 insertions(+), 119 deletions(-)

--- wireless-testing.orig/drivers/net/wireless/iwlwifi/iwl-4965-hw.h	2008-10-06
16:41:59.074744190 +0200
+++ wireless-testing/drivers/net/wireless/iwlwifi/iwl-4965-hw.h	2008-10-06
17:20:39.754106476 +0200
@@ -822,54 +822,11 @@ enum {
 #define IWL49_NUM_QUEUES	16
 #define IWL49_NUM_AMPDU_QUEUES	8

-/**
- * struct iwl_tfd_frame_data

(Continue reading)

Johannes Berg | 6 Oct 18:10

[PATCH v2] iwlwifi: fix DMA code and bugs

This patch cleans up the DMA code to be understandable and not
completely wrong. In particular:
 * there is no need to have a weird iwl_tfd_frame_data struct that is
   used 10 times, just use an address struct 20 times
 * therefore, all the is_odd junk goes away
 * fix a bug in iwl_hcmd_queue_reclaim where it would reclaim all the
   fragments of a descriptor rather than all descriptors (this may be
   the cause of the dma unmapping problem I reported)
 * some more cleanups

Signed-off-by: Johannes Berg <johannes@...>
---
Tested on 5000 hw, please apply.

v2: fixes small issue with getting rid of iwl_get_dma_hi_address.

Oh and who came up with struct iwl_tfd_frame_data? Were drugs involved?

 drivers/net/wireless/iwlwifi/iwl-4965-hw.h |   98 ++++++++++++++---------------
 drivers/net/wireless/iwlwifi/iwl-5000.c    |    3 
 drivers/net/wireless/iwlwifi/iwl-helpers.h |    5 -
 drivers/net/wireless/iwlwifi/iwl-tx.c      |   84 +++++-------------------
 4 files changed, 70 insertions(+), 120 deletions(-)

--- everything.orig/drivers/net/wireless/iwlwifi/iwl-4965-hw.h	2008-10-06 17:55:40.000000000 +0200
+++ everything/drivers/net/wireless/iwlwifi/iwl-4965-hw.h	2008-10-06 17:55:45.000000000 +0200
@@ -822,54 +822,11 @@ enum {
 #define IWL49_NUM_QUEUES	16
 #define IWL49_NUM_AMPDU_QUEUES	8

(Continue reading)

Tomas Winkler | 6 Oct 18:49

Re: [PATCH v2] iwlwifi: fix DMA code and bugs

On Mon, Oct 6, 2008 at 6:10 PM, Johannes Berg <johannes@...> wrote:
> This patch cleans up the DMA code to be understandable and not
> completely wrong. In particular:
>  * there is no need to have a weird iwl_tfd_frame_data struct that is
>   used 10 times, just use an address struct 20 times

In the hardware this is defined as couples

>  * therefore, all the is_odd junk goes away
>  * fix a bug in iwl_hcmd_queue_reclaim where it would reclaim all the
>   fragments of a descriptor rather than all descriptors (this may be
>   the cause of the dma unmapping problem I reported)
>  * some more cleanups

> Signed-off-by: Johannes Berg <johannes@...>
> --

> Tested on 5000 hw, please apply.

Great job,  however do not apply this before I review it I had strong
feeling this will not
work with aggregation flows.
Thanks
Tomas
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@...
More majordomo info at  http://vger.kernel.org/majordomo-info.html

(Continue reading)

Johannes Berg | 6 Oct 18:51

Re: [PATCH v2] iwlwifi: fix DMA code and bugs

On Mon, 2008-10-06 at 18:49 +0200, Tomas Winkler wrote:
> On Mon, Oct 6, 2008 at 6:10 PM, Johannes Berg <johannes@...> wrote:
> > This patch cleans up the DMA code to be understandable and not
> > completely wrong. In particular:
> >  * there is no need to have a weird iwl_tfd_frame_data struct that is
> >   used 10 times, just use an address struct 20 times
> 
> In the hardware this is defined as couples

So? That doesn't actually matter at all.

> >  * therefore, all the is_odd junk goes away
> >  * fix a bug in iwl_hcmd_queue_reclaim where it would reclaim all the
> >   fragments of a descriptor rather than all descriptors (this may be
> >   the cause of the dma unmapping problem I reported)
> >  * some more cleanups
> 
> > Signed-off-by: Johannes Berg <johannes@...>
> > --
> 
> 
> > Tested on 5000 hw, please apply.
> 
> Great job,  however do not apply this before I review it I had strong
> feeling this will not
> work with aggregation flows.

I cannot imagine why you think that, care to explain?

johannes

(Continue reading)

Johannes Berg | 6 Oct 18:54

Re: [PATCH v2] iwlwifi: fix DMA code and bugs

On Mon, 2008-10-06 at 18:51 +0200, Johannes Berg wrote:

> > >  * therefore, all the is_odd junk goes away
> > >  * fix a bug in iwl_hcmd_queue_reclaim where it would reclaim all the
> > >   fragments of a descriptor rather than all descriptors (this may be
> > >   the cause of the dma unmapping problem I reported)
> > >  * some more cleanups
> > 
> > > Signed-off-by: Johannes Berg <johannes@...>
> > > --
> > 
> > 
> > > Tested on 5000 hw, please apply.
> > 
> > Great job,  however do not apply this before I review it I had strong
> > feeling this will not
> > work with aggregation flows.
> 
> I cannot imagine why you think that, care to explain?

Of course, I would very much appreciate you review the actual bug fix in
iwl_hcmd_queue_reclaim, which consist of the addition of the line

+               bd = &txq->bd[index];

johannes

Tomas Winkler | 6 Oct 20:30

Re: [PATCH v2] iwlwifi: fix DMA code and bugs

On Mon, Oct 6, 2008 at 6:54 PM, Johannes Berg <johannes@...> wrote:
> On Mon, 2008-10-06 at 18:51 +0200, Johannes Berg wrote:
>
>> > >  * therefore, all the is_odd junk goes away
>> > >  * fix a bug in iwl_hcmd_queue_reclaim where it would reclaim all the
>> > >   fragments of a descriptor rather than all descriptors (this may be
>> > >   the cause of the dma unmapping problem I reported)
>> > >  * some more cleanups
>> >
>> > > Signed-off-by: Johannes Berg <johannes@...>
>> > > --
>> >
>> >
>> > > Tested on 5000 hw, please apply.
>> >
>> > Great job,  however do not apply this before I review it I had strong
>> > feeling this will not
>> > work with aggregation flows.
>>
>> I cannot imagine why you think that, care to explain?

Yep, no connection I thought at the first glance you've thatched more code.
>
> Of course, I would very much appreciate you review the actual bug fix in
> iwl_hcmd_queue_reclaim, which consist of the addition of the line
>
> +               bd = &txq->bd[index];

Okay I found the source of the evil, with your big pointer :)
The bug caused by this memory optimization juggling we've done and

(Continue reading)

Johannes Berg | 6 Oct 20:34

Re: [PATCH v2] iwlwifi: fix DMA code and bugs

On Mon, 2008-10-06 at 20:30 +0200, Tomas Winkler wrote:

> > Of course, I would very much appreciate you review the actual bug fix in
> > iwl_hcmd_queue_reclaim, which consist of the addition of the line
> >
> > +               bd = &txq->bd[index];
> 
> Okay I found the source of the evil, with your big pointer :)
> The bug caused by this memory optimization juggling we've done and
> very  bad cut and paste coding
> We moved for command buffers from pcI_alloc consitent to kmalloc which
> required pci mapping that that was coded wrongly. Now this kmalloc
> stuff is not good as well since we have this 36 bit memory limitation
> on 64 bit platforms.
> This is the issue I'm trying to address recently I'm not sure what yet
> what allocation schema would be best yet.

That can't be all though; it still crashes right away when I enable 64k
pages on my box.

johannes

John W. Linville | 6 Oct 21:23

Re: [PATCH v2] iwlwifi: fix DMA code and bugs

On Mon, Oct 06, 2008 at 08:30:29PM +0200, Tomas Winkler wrote:
> On Mon, Oct 6, 2008 at 6:54 PM, Johannes Berg <johannes@...> wrote:
> > On Mon, 2008-10-06 at 18:51 +0200, Johannes Berg wrote:
> >
> >> > >  * therefore, all the is_odd junk goes away
> >> > >  * fix a bug in iwl_hcmd_queue_reclaim where it would reclaim all the
> >> > >   fragments of a descriptor rather than all descriptors (this may be
> >> > >   the cause of the dma unmapping problem I reported)
> >> > >  * some more cleanups
> >> >
> >> > > Signed-off-by: Johannes Berg <johannes@...>
> >> > > --
> >> >
> >> >
> >> > > Tested on 5000 hw, please apply.
> >> >
> >> > Great job,  however do not apply this before I review it I had strong
> >> > feeling this will not
> >> > work with aggregation flows.
> >>
> >> I cannot imagine why you think that, care to explain?
> 
> Yep, no connection I thought at the first glance you've thatched more code.
> >
> > Of course, I would very much appreciate you review the actual bug fix in
> > iwl_hcmd_queue_reclaim, which consist of the addition of the line
> >
> > +               bd = &txq->bd[index];
> 
> Okay I found the source of the evil, with your big pointer :)

(Continue reading)

Johannes Berg | 6 Oct 21:52

Re: [PATCH v2] iwlwifi: fix DMA code and bugs

On Mon, 2008-10-06 at 15:23 -0400, John W. Linville wrote:
> On Mon, Oct 06, 2008 at 08:30:29PM +0200, Tomas Winkler wrote:
> > On Mon, Oct 6, 2008 at 6:54 PM, Johannes Berg <johannes@...t> wrote:
> > > On Mon, 2008-10-06 at 18:51 +0200, Johannes Berg wrote:
> > >
> > >> > >  * therefore, all the is_odd junk goes away
> > >> > >  * fix a bug in iwl_hcmd_queue_reclaim where it would reclaim all the
> > >> > >   fragments of a descriptor rather than all descriptors (this may be
> > >> > >   the cause of the dma unmapping problem I reported)
> > >> > >  * some more cleanups
> > >> >
> > >> > > Signed-off-by: Johannes Berg <johannes@...>
> > >> > > --
> > >> >
> > >> >
> > >> > > Tested on 5000 hw, please apply.
> > >> >
> > >> > Great job,  however do not apply this before I review it I had strong
> > >> > feeling this will not
> > >> > work with aggregation flows.
> > >>
> > >> I cannot imagine why you think that, care to explain?
> > 
> > Yep, no connection I thought at the first glance you've thatched more code.
> > >
> > > Of course, I would very much appreciate you review the actual bug fix in
> > > iwl_hcmd_queue_reclaim, which consist of the addition of the line
> > >
> > > +               bd = &txq->bd[index];
> >

(Continue reading)

Tomas Winkler | 6 Oct 23:37

Re: [PATCH v2] iwlwifi: fix DMA code and bugs

On Mon, Oct 6, 2008 at 9:23 PM, John W. Linville <linville@...> wrote:
> On Mon, Oct 06, 2008 at 08:30:29PM +0200, Tomas Winkler wrote:
>> On Mon, Oct 6, 2008 at 6:54 PM, Johannes Berg <johannes@...> wrote:
>> > On Mon, 2008-10-06 at 18:51 +0200, Johannes Berg wrote:
>> >
>> >> > >  * therefore, all the is_odd junk goes away
>> >> > >  * fix a bug in iwl_hcmd_queue_reclaim where it would reclaim all the
>> >> > >   fragments of a descriptor rather than all descriptors (this may be
>> >> > >   the cause of the dma unmapping problem I reported)
>> >> > >  * some more cleanups
>> >> >
>> >> > > Signed-off-by: Johannes Berg <johannes@...>
>> >> > > --
>> >> >
>> >> >
>> >> > > Tested on 5000 hw, please apply.
>> >> >
>> >> > Great job,  however do not apply this before I review it I had strong
>> >> > feeling this will not
>> >> > work with aggregation flows.
>> >>
>> >> I cannot imagine why you think that, care to explain?
>>
>> Yep, no connection I thought at the first glance you've thatched more code.
>> >
>> > Of course, I would very much appreciate you review the actual bug fix in
>> > iwl_hcmd_queue_reclaim, which consist of the addition of the line
>> >
>> > +               bd = &txq->bd[index];
>>

(Continue reading)

Marcel Holtmann | 6 Oct 18:42

Re: [PATCH v2] iwlwifi: fix DMA code and bugs

Hi John,

> This patch cleans up the DMA code to be understandable and not
> completely wrong. In particular:
>  * there is no need to have a weird iwl_tfd_frame_data struct that is
>    used 10 times, just use an address struct 20 times
>  * therefore, all the is_odd junk goes away
>  * fix a bug in iwl_hcmd_queue_reclaim where it would reclaim all the
>    fragments of a descriptor rather than all descriptors (this may be
>    the cause of the dma unmapping problem I reported)
>  * some more cleanups
> 
> Signed-off-by: Johannes Berg <johannes@...>
> ---
> Tested on 5000 hw, please apply.
> 
> v2: fixes small issue with getting rid of iwl_get_dma_hi_address.

if this fixes the crashes with my 4965 card in my X61, then this is a
candidate for 2.6.27-rc8. I will built a new kernel for my X61 as soon
as possible.

Regards

Marcel

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@...
More majordomo info at  http://vger.kernel.org/majordomo-info.html

(Continue reading)

Johannes Berg | 6 Oct 18:47

Re: [PATCH v2] iwlwifi: fix DMA code and bugs

On Mon, 2008-10-06 at 18:42 +0200, Marcel Holtmann wrote:
> Hi John,
> 
> > This patch cleans up the DMA code to be understandable and not
> > completely wrong. In particular:
> >  * there is no need to have a weird iwl_tfd_frame_data struct that is
> >    used 10 times, just use an address struct 20 times
> >  * therefore, all the is_odd junk goes away
> >  * fix a bug in iwl_hcmd_queue_reclaim where it would reclaim all the
> >    fragments of a descriptor rather than all descriptors (this may be
> >    the cause of the dma unmapping problem I reported)
> >  * some more cleanups
> > 
> > Signed-off-by: Johannes Berg <johannes@...>
> > ---
> > Tested on 5000 hw, please apply.
> > 
> > v2: fixes small issue with getting rid of iwl_get_dma_hi_address.
> 
> if this fixes the crashes with my 4965 card in my X61, then this is a
> candidate for 2.6.27-rc8. I will built a new kernel for my X61 as soon
> as possible.

I doubt it will, but who knows. Let me know. Right now I'm trying to
massage  the code into something readable.

johannes

Johannes Berg | 6 Oct 19:20

Re: [PATCH v2] iwlwifi: fix DMA code and bugs

On Mon, 2008-10-06 at 18:42 +0200, Marcel Holtmann wrote:
> Hi John,
> 
> > This patch cleans up the DMA code to be understandable and not
> > completely wrong. In particular:
> >  * there is no need to have a weird iwl_tfd_frame_data struct that is
> >    used 10 times, just use an address struct 20 times
> >  * therefore, all the is_odd junk goes away
> >  * fix a bug in iwl_hcmd_queue_reclaim where it would reclaim all the
> >    fragments of a descriptor rather than all descriptors (this may be
> >    the cause of the dma unmapping problem I reported)
> >  * some more cleanups
> > 
> > Signed-off-by: Johannes Berg <johannes@...>
> > ---
> > Tested on 5000 hw, please apply.
> > 
> > v2: fixes small issue with getting rid of iwl_get_dma_hi_address.
> 
> if this fixes the crashes with my 4965 card in my X61, then this is a
> candidate for 2.6.27-rc8. I will built a new kernel for my X61 as soon
> as possible.

I just tried, it certainly doesn't stop the thing from corrupting my
memory all over when I have 64k pages turned on.

johannes

Johannes Berg | 14 Oct 17:21

Re: [PATCH v2] iwlwifi: fix DMA code and bugs

On Mon, 2008-10-06 at 18:10 +0200, Johannes Berg wrote:
> This patch cleans up the DMA code to be understandable and not
> completely wrong. In particular:
>  * there is no need to have a weird iwl_tfd_frame_data struct that is
>    used 10 times, just use an address struct 20 times
>  * therefore, all the is_odd junk goes away
>  * fix a bug in iwl_hcmd_queue_reclaim where it would reclaim all the
>    fragments of a descriptor rather than all descriptors (this may be
>    the cause of the dma unmapping problem I reported)
>  * some more cleanups
> 
> Signed-off-by: Johannes Berg <johannes@...>
> ---
> Tested on 5000 hw, please apply.
> 
> v2: fixes small issue with getting rid of iwl_get_dma_hi_address.

John, can you apply this? I keep getting invalid frees, or double-frees
without this patch.

[ 1831.106142] iommu_free: invalid entry
[ 1831.106207] 	free_entry= 0xb0040
[ 1831.106213] 	npages    = 0x1
[ 1831.106219] 	entry     = 0xb0040
[ 1831.106225] 	dma_addr  = 0xb0040100
[ 1831.106231] 	Table     = 0xc00000000083f348
[ 1831.106238] 	bus#      = 0x0
[ 1831.106244] 	size      = 0x80000
[ 1831.106249] 	startOff  = 0x0
[ 1831.106255] 	index     = 0x0

(Continue reading)

reinette chatre | 14 Oct 17:42

Re: [PATCH v2] iwlwifi: fix DMA code and bugs

On Tue, 2008-10-14 at 17:21 +0200, Johannes Berg wrote:
> On Mon, 2008-10-06 at 18:10 +0200, Johannes Berg wrote:
> > This patch cleans up the DMA code to be understandable and not
> > completely wrong. In particular:
> >  * there is no need to have a weird iwl_tfd_frame_data struct that is
> >    used 10 times, just use an address struct 20 times
> >  * therefore, all the is_odd junk goes away
> >  * fix a bug in iwl_hcmd_queue_reclaim where it would reclaim all the
> >    fragments of a descriptor rather than all descriptors (this may be
> >    the cause of the dma unmapping problem I reported)
> >  * some more cleanups
> > 
> > Signed-off-by: Johannes Berg <johannes@...>
> > ---
> > Tested on 5000 hw, please apply.
> > 
> > v2: fixes small issue with getting rid of iwl_get_dma_hi_address.
> 
> John, can you apply this? I keep getting invalid frees, or double-frees
> without this patch.

Tomas reworked this patch. I will send the new patch upstream soon.

Reinette
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@...
More majordomo info at  http://vger.kernel.org/majordomo-info.html

(Continue reading)

Return

Return to gmane.linux.kernel.wireless.general.

Advertisement

Search Archive

Language

Change language

Options

Current view: Threads only / Showing only 20 lines / Not hiding cited text.
Change to All messages, whole messages, or hide cited text.

Post a message
NNTP Newsgroup
Classic Gmane web interface
XML

XML

RSS Feed
List Information

About Gmane