2008-11-19 00:46:33

by John W. Linville

[permalink] [raw]
Subject: pull request: wireless-2.6 2008-11-18

Dave,

Three fixes from Johannes, intended for 2.6.28... One fixes a 32/64
issue in libertas_tf, another removes a mac80211 callback only used
by iwlwifi which is not obviously needed and which is causing locking
issues, and a final one fixes some mysterious DMA alignment problems
that have been plaguing iwlwifi for some time.

Please let me know if there are problems!

Thanks,

John

P.S. The libertas_tf is actually already in net-next-2.6. Also,
the other fixes cause merge conflicts with net-next-2.6 -- I'll
included a merge-test branch in wireless-next-2.6 to indicates how
those conflicts should be resolved.

---

Individual patches are available here:

http://www.kernel.org/pub/linux/kernel/people/linville/wireless-2.6/

---

The following changes since commit 5f9021cfdc3524a4c5e3d7ae2d049eb7adcd6776:
Johannes Berg (1):
rtnetlink: propagate error from dev_change_flags in do_setlink()

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless-2.6.git master

Johannes Berg (3):
libertas_tf: fix skb tail pointer
mac80211: remove ieee80211_notify_mac
iwlagn: fix RX skb alignment

drivers/net/wireless/iwlwifi/iwl-agn.c | 7 +++----
drivers/net/wireless/iwlwifi/iwl-dev.h | 3 ++-
drivers/net/wireless/iwlwifi/iwl-rx.c | 26 +++++++++++++++++---------
drivers/net/wireless/iwlwifi/iwl3945-base.c | 1 -
drivers/net/wireless/libertas_tf/if_usb.c | 2 +-
include/net/mac80211.h | 20 --------------------
net/mac80211/mlme.c | 22 ----------------------
7 files changed, 23 insertions(+), 58 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-agn.c b/drivers/net/wireless/iwlwifi/iwl-agn.c
index 8d690a0..444c5cc 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn.c
@@ -1384,7 +1384,7 @@ void iwl_rx_handle(struct iwl_priv *priv)

rxq->queue[i] = NULL;

- pci_dma_sync_single_for_cpu(priv->pci_dev, rxb->dma_addr,
+ pci_dma_sync_single_for_cpu(priv->pci_dev, rxb->aligned_dma_addr,
priv->hw_params.rx_buf_size,
PCI_DMA_FROMDEVICE);
pkt = (struct iwl_rx_packet *)rxb->skb->data;
@@ -1436,8 +1436,8 @@ void iwl_rx_handle(struct iwl_priv *priv)
rxb->skb = NULL;
}

- pci_unmap_single(priv->pci_dev, rxb->dma_addr,
- priv->hw_params.rx_buf_size,
+ pci_unmap_single(priv->pci_dev, rxb->real_dma_addr,
+ priv->hw_params.rx_buf_size + 256,
PCI_DMA_FROMDEVICE);
spin_lock_irqsave(&rxq->lock, flags);
list_add_tail(&rxb->list, &priv->rxq.rx_used);
@@ -2341,7 +2341,6 @@ static void iwl_bg_alive_start(struct work_struct *data)
mutex_lock(&priv->mutex);
iwl_alive_start(priv);
mutex_unlock(&priv->mutex);
- ieee80211_notify_mac(priv->hw, IEEE80211_NOTIFY_RE_ASSOC);
}

static void iwl4965_bg_rf_kill(struct work_struct *work)
diff --git a/drivers/net/wireless/iwlwifi/iwl-dev.h b/drivers/net/wireless/iwlwifi/iwl-dev.h
index c018121..9966d4e 100644
--- a/drivers/net/wireless/iwlwifi/iwl-dev.h
+++ b/drivers/net/wireless/iwlwifi/iwl-dev.h
@@ -89,7 +89,8 @@ extern struct iwl_cfg iwl5100_abg_cfg;
#define DEFAULT_LONG_RETRY_LIMIT 4U

struct iwl_rx_mem_buffer {
- dma_addr_t dma_addr;
+ dma_addr_t real_dma_addr;
+ dma_addr_t aligned_dma_addr;
struct sk_buff *skb;
struct list_head list;
};
diff --git a/drivers/net/wireless/iwlwifi/iwl-rx.c b/drivers/net/wireless/iwlwifi/iwl-rx.c
index 7cde9d7..0509c16 100644
--- a/drivers/net/wireless/iwlwifi/iwl-rx.c
+++ b/drivers/net/wireless/iwlwifi/iwl-rx.c
@@ -204,7 +204,7 @@ int iwl_rx_queue_restock(struct iwl_priv *priv)
list_del(element);

/* Point to Rx buffer via next RBD in circular buffer */
- rxq->bd[rxq->write] = iwl_dma_addr2rbd_ptr(priv, rxb->dma_addr);
+ rxq->bd[rxq->write] = iwl_dma_addr2rbd_ptr(priv, rxb->aligned_dma_addr);
rxq->queue[rxq->write] = rxb;
rxq->write = (rxq->write + 1) & RX_QUEUE_MASK;
rxq->free_count--;
@@ -251,7 +251,7 @@ void iwl_rx_allocate(struct iwl_priv *priv)
rxb = list_entry(element, struct iwl_rx_mem_buffer, list);

/* Alloc a new receive buffer */
- rxb->skb = alloc_skb(priv->hw_params.rx_buf_size,
+ rxb->skb = alloc_skb(priv->hw_params.rx_buf_size + 256,
__GFP_NOWARN | GFP_ATOMIC);
if (!rxb->skb) {
if (net_ratelimit())
@@ -266,9 +266,17 @@ void iwl_rx_allocate(struct iwl_priv *priv)
list_del(element);

/* Get physical address of RB/SKB */
- rxb->dma_addr =
- pci_map_single(priv->pci_dev, rxb->skb->data,
- priv->hw_params.rx_buf_size, PCI_DMA_FROMDEVICE);
+ rxb->real_dma_addr = pci_map_single(
+ priv->pci_dev,
+ rxb->skb->data,
+ priv->hw_params.rx_buf_size + 256,
+ PCI_DMA_FROMDEVICE);
+ /* dma address must be no more than 36 bits */
+ BUG_ON(rxb->real_dma_addr & ~DMA_BIT_MASK(36));
+ /* and also 256 byte aligned! */
+ rxb->aligned_dma_addr = ALIGN(rxb->real_dma_addr, 256);
+ skb_reserve(rxb->skb, rxb->aligned_dma_addr - rxb->real_dma_addr);
+
list_add_tail(&rxb->list, &rxq->rx_free);
rxq->free_count++;
}
@@ -300,8 +308,8 @@ void iwl_rx_queue_free(struct iwl_priv *priv, struct iwl_rx_queue *rxq)
for (i = 0; i < RX_QUEUE_SIZE + RX_FREE_BUFFERS; i++) {
if (rxq->pool[i].skb != NULL) {
pci_unmap_single(priv->pci_dev,
- rxq->pool[i].dma_addr,
- priv->hw_params.rx_buf_size,
+ rxq->pool[i].real_dma_addr,
+ priv->hw_params.rx_buf_size + 256,
PCI_DMA_FROMDEVICE);
dev_kfree_skb(rxq->pool[i].skb);
}
@@ -354,8 +362,8 @@ void iwl_rx_queue_reset(struct iwl_priv *priv, struct iwl_rx_queue *rxq)
* to an SKB, so we need to unmap and free potential storage */
if (rxq->pool[i].skb != NULL) {
pci_unmap_single(priv->pci_dev,
- rxq->pool[i].dma_addr,
- priv->hw_params.rx_buf_size,
+ rxq->pool[i].real_dma_addr,
+ priv->hw_params.rx_buf_size + 256,
PCI_DMA_FROMDEVICE);
priv->alloc_rxb_skb--;
dev_kfree_skb(rxq->pool[i].skb);
diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c b/drivers/net/wireless/iwlwifi/iwl3945-base.c
index 285b53e..45a6b0c 100644
--- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
+++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
@@ -6012,7 +6012,6 @@ static void iwl3945_bg_alive_start(struct work_struct *data)
mutex_lock(&priv->mutex);
iwl3945_alive_start(priv);
mutex_unlock(&priv->mutex);
- ieee80211_notify_mac(priv->hw, IEEE80211_NOTIFY_RE_ASSOC);
}

static void iwl3945_bg_rf_kill(struct work_struct *work)
diff --git a/drivers/net/wireless/libertas_tf/if_usb.c b/drivers/net/wireless/libertas_tf/if_usb.c
index 1cc03a8..59634c3 100644
--- a/drivers/net/wireless/libertas_tf/if_usb.c
+++ b/drivers/net/wireless/libertas_tf/if_usb.c
@@ -331,7 +331,7 @@ static int __if_usb_submit_rx_urb(struct if_usb_card *cardp,
/* Fill the receive configuration URB and initialise the Rx call back */
usb_fill_bulk_urb(cardp->rx_urb, cardp->udev,
usb_rcvbulkpipe(cardp->udev, cardp->ep_in),
- (void *) (skb->tail),
+ skb_tail_pointer(skb),
MRVDRV_ETH_RX_PACKET_BUFFER_SIZE, callbackfn, cardp);

cardp->rx_urb->transfer_flags |= URB_ZERO_PACKET;
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 8856e2d..73d81bc 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -74,14 +74,6 @@
*/

/**
- * enum ieee80211_notification_type - Low level driver notification
- * @IEEE80211_NOTIFY_RE_ASSOC: start the re-association sequence
- */
-enum ieee80211_notification_types {
- IEEE80211_NOTIFY_RE_ASSOC,
-};
-
-/**
* struct ieee80211_ht_bss_info - describing BSS's HT characteristics
*
* This structure describes most essential parameters needed
@@ -1798,18 +1790,6 @@ void ieee80211_stop_tx_ba_cb_irqsafe(struct ieee80211_hw *hw, const u8 *ra,
u16 tid);

/**
- * ieee80211_notify_mac - low level driver notification
- * @hw: pointer as obtained from ieee80211_alloc_hw().
- * @notif_type: enum ieee80211_notification_types
- *
- * This function must be called by low level driver to inform mac80211 of
- * low level driver status change or force mac80211 to re-assoc for low
- * level driver internal error that require re-assoc.
- */
-void ieee80211_notify_mac(struct ieee80211_hw *hw,
- enum ieee80211_notification_types notif_type);
-
-/**
* ieee80211_find_sta - find a station
*
* @hw: pointer as obtained from ieee80211_alloc_hw()
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 14d165f..409bb77 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -2560,25 +2560,3 @@ void ieee80211_mlme_notify_scan_completed(struct ieee80211_local *local)
ieee80211_restart_sta_timer(sdata);
rcu_read_unlock();
}
-
-/* driver notification call */
-void ieee80211_notify_mac(struct ieee80211_hw *hw,
- enum ieee80211_notification_types notif_type)
-{
- struct ieee80211_local *local = hw_to_local(hw);
- struct ieee80211_sub_if_data *sdata;
-
- switch (notif_type) {
- case IEEE80211_NOTIFY_RE_ASSOC:
- rtnl_lock();
- list_for_each_entry(sdata, &local->interfaces, list) {
- if (sdata->vif.type != NL80211_IFTYPE_STATION)
- continue;
-
- ieee80211_sta_req_auth(sdata, &sdata->u.sta);
- }
- rtnl_unlock();
- break;
- }
-}
-EXPORT_SYMBOL(ieee80211_notify_mac);
--
John W. Linville Linux should be at the core
[email protected] of your literate lifestyle.


2008-11-19 01:34:39

by Johannes Berg

[permalink] [raw]
Subject: Re: pull request: wireless-2.6 2008-11-18

> and a final one fixes some mysterious DMA alignment problems
> that have been plaguing iwlwifi for some time.


> The following changes since commit 5f9021cfdc3524a4c5e3d7ae2d049eb7adcd6776:
> Johannes Berg (1):
> rtnetlink: propagate error from dev_change_flags in do_setlink()

heh.

> iwlagn: fix RX skb alignment

So Luis pointed out a bug in that, it might be useful to squish in this
patch: http://marc.info/?l=linux-wireless&m=122705425425763&w=2

(also below for reference)

But we can also just add it on top, whichever way you prefer.

johannes
---
drivers/net/wireless/iwlwifi/iwl-agn.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)

--- everything.orig/drivers/net/wireless/iwlwifi/iwl-agn.c 2008-11-19 01:16:12.000000000 +0100
+++ everything/drivers/net/wireless/iwlwifi/iwl-agn.c 2008-11-19 01:19:13.000000000 +0100
@@ -1229,9 +1229,11 @@ void iwl_rx_handle(struct iwl_priv *priv

rxq->queue[i] = NULL;

- pci_dma_sync_single_for_cpu(priv->pci_dev, rxb->aligned_dma_addr,
- priv->hw_params.rx_buf_size,
- PCI_DMA_FROMDEVICE);
+ dma_sync_single_range_for_cpu(
+ &priv->pci_dev->dev, rxb->real_dma_addr,
+ rxb->aligned_dma_addr - rxb->real_dma_addr,
+ priv->hw_params.rx_buf_size,
+ PCI_DMA_FROMDEVICE);
pkt = (struct iwl_rx_packet *)rxb->skb->data;

/* Reclaim a command buffer only if this packet is a response

2008-11-19 02:01:24

by John W. Linville

[permalink] [raw]
Subject: Re: pull request: wireless-2.6 2008-11-18

On Wed, Nov 19, 2008 at 02:33:38AM +0100, Johannes Berg wrote:
> > and a final one fixes some mysterious DMA alignment problems
> > that have been plaguing iwlwifi for some time.
>
>
> > The following changes since commit 5f9021cfdc3524a4c5e3d7ae2d049eb7adcd6776:
> > Johannes Berg (1):
> > rtnetlink: propagate error from dev_change_flags in do_setlink()
>
> heh.
>
> > iwlagn: fix RX skb alignment
>
> So Luis pointed out a bug in that, it might be useful to squish in this
> patch: http://marc.info/?l=linux-wireless&m=122705425425763&w=2
>
> (also below for reference)
>
> But we can also just add it on top, whichever way you prefer.

I'll get it in the next run, soon.

John
--
John W. Linville Linux should be at the core
[email protected] of your literate lifestyle.

2008-11-19 06:46:43

by Tomas Winkler

[permalink] [raw]
Subject: Re: pull request: wireless-2.6 2008-11-18

On Wed, Nov 19, 2008 at 2:07 AM, John W. Linville
<[email protected]> wrote:
> Dave,
>
> Three fixes from Johannes, intended for 2.6.28... One fixes a 32/64
> issue in libertas_tf, another removes a mac80211 callback only used
> by iwlwifi which is not obviously needed and which is causing locking
> issues, and a final one fixes some mysterious DMA alignment problems
> that have been plaguing iwlwifi for some time.
>
> Please let me know if there are problems!
>
> Thanks,
>
> John
>
> P.S. The libertas_tf is actually already in net-next-2.6. Also,
> the other fixes cause merge conflicts with net-next-2.6 -- I'll
> included a merge-test branch in wireless-next-2.6 to indicates how
> those conflicts should be resolved.
>
> ---
>
> Individual patches are available here:
>
> http://www.kernel.org/pub/linux/kernel/people/linville/wireless-2.6/
>
> ---
>
> The following changes since commit 5f9021cfdc3524a4c5e3d7ae2d049eb7adcd6776:
> Johannes Berg (1):
> rtnetlink: propagate error from dev_change_flags in do_setlink()
>
> are available in the git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless-2.6.git master
>
> Johannes Berg (3):
> libertas_tf: fix skb tail pointer
> mac80211: remove ieee80211_notify_mac
> iwlagn: fix RX skb alignment

IMHO its premature pushing ttese 2 patches up, they came in yestrday
and nobody here has run tthe code
Tomas

>
> drivers/net/wireless/iwlwifi/iwl-agn.c | 7 +++----
> drivers/net/wireless/iwlwifi/iwl-dev.h | 3 ++-
> drivers/net/wireless/iwlwifi/iwl-rx.c | 26 +++++++++++++++++---------
> drivers/net/wireless/iwlwifi/iwl3945-base.c | 1 -
> drivers/net/wireless/libertas_tf/if_usb.c | 2 +-
> include/net/mac80211.h | 20 --------------------
> net/mac80211/mlme.c | 22 ----------------------
> 7 files changed, 23 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/net/wireless/iwlwifi/iwl-agn.c b/drivers/net/wireless/iwlwifi/iwl-agn.c
> index 8d690a0..444c5cc 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-agn.c
> +++ b/drivers/net/wireless/iwlwifi/iwl-agn.c
> @@ -1384,7 +1384,7 @@ void iwl_rx_handle(struct iwl_priv *priv)
>
> rxq->queue[i] = NULL;
>
> - pci_dma_sync_single_for_cpu(priv->pci_dev, rxb->dma_addr,
> + pci_dma_sync_single_for_cpu(priv->pci_dev, rxb->aligned_dma_addr,
> priv->hw_params.rx_buf_size,
> PCI_DMA_FROMDEVICE);
> pkt = (struct iwl_rx_packet *)rxb->skb->data;
> @@ -1436,8 +1436,8 @@ void iwl_rx_handle(struct iwl_priv *priv)
> rxb->skb = NULL;
> }
>
> - pci_unmap_single(priv->pci_dev, rxb->dma_addr,
> - priv->hw_params.rx_buf_size,
> + pci_unmap_single(priv->pci_dev, rxb->real_dma_addr,
> + priv->hw_params.rx_buf_size + 256,
> PCI_DMA_FROMDEVICE);
> spin_lock_irqsave(&rxq->lock, flags);
> list_add_tail(&rxb->list, &priv->rxq.rx_used);
> @@ -2341,7 +2341,6 @@ static void iwl_bg_alive_start(struct work_struct *data)
> mutex_lock(&priv->mutex);
> iwl_alive_start(priv);
> mutex_unlock(&priv->mutex);
> - ieee80211_notify_mac(priv->hw, IEEE80211_NOTIFY_RE_ASSOC);
> }
>
> static void iwl4965_bg_rf_kill(struct work_struct *work)
> diff --git a/drivers/net/wireless/iwlwifi/iwl-dev.h b/drivers/net/wireless/iwlwifi/iwl-dev.h
> index c018121..9966d4e 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-dev.h
> +++ b/drivers/net/wireless/iwlwifi/iwl-dev.h
> @@ -89,7 +89,8 @@ extern struct iwl_cfg iwl5100_abg_cfg;
> #define DEFAULT_LONG_RETRY_LIMIT 4U
>
> struct iwl_rx_mem_buffer {
> - dma_addr_t dma_addr;
> + dma_addr_t real_dma_addr;
> + dma_addr_t aligned_dma_addr;
> struct sk_buff *skb;
> struct list_head list;
> };
> diff --git a/drivers/net/wireless/iwlwifi/iwl-rx.c b/drivers/net/wireless/iwlwifi/iwl-rx.c
> index 7cde9d7..0509c16 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-rx.c
> +++ b/drivers/net/wireless/iwlwifi/iwl-rx.c
> @@ -204,7 +204,7 @@ int iwl_rx_queue_restock(struct iwl_priv *priv)
> list_del(element);
>
> /* Point to Rx buffer via next RBD in circular buffer */
> - rxq->bd[rxq->write] = iwl_dma_addr2rbd_ptr(priv, rxb->dma_addr);
> + rxq->bd[rxq->write] = iwl_dma_addr2rbd_ptr(priv, rxb->aligned_dma_addr);
> rxq->queue[rxq->write] = rxb;
> rxq->write = (rxq->write + 1) & RX_QUEUE_MASK;
> rxq->free_count--;
> @@ -251,7 +251,7 @@ void iwl_rx_allocate(struct iwl_priv *priv)
> rxb = list_entry(element, struct iwl_rx_mem_buffer, list);
>
> /* Alloc a new receive buffer */
> - rxb->skb = alloc_skb(priv->hw_params.rx_buf_size,
> + rxb->skb = alloc_skb(priv->hw_params.rx_buf_size + 256,
> __GFP_NOWARN | GFP_ATOMIC);
> if (!rxb->skb) {
> if (net_ratelimit())
> @@ -266,9 +266,17 @@ void iwl_rx_allocate(struct iwl_priv *priv)
> list_del(element);
>
> /* Get physical address of RB/SKB */
> - rxb->dma_addr =
> - pci_map_single(priv->pci_dev, rxb->skb->data,
> - priv->hw_params.rx_buf_size, PCI_DMA_FROMDEVICE);
> + rxb->real_dma_addr = pci_map_single(
> + priv->pci_dev,
> + rxb->skb->data,
> + priv->hw_params.rx_buf_size + 256,
> + PCI_DMA_FROMDEVICE);
> + /* dma address must be no more than 36 bits */
> + BUG_ON(rxb->real_dma_addr & ~DMA_BIT_MASK(36));
> + /* and also 256 byte aligned! */
> + rxb->aligned_dma_addr = ALIGN(rxb->real_dma_addr, 256);
> + skb_reserve(rxb->skb, rxb->aligned_dma_addr - rxb->real_dma_addr);
> +
> list_add_tail(&rxb->list, &rxq->rx_free);
> rxq->free_count++;
> }
> @@ -300,8 +308,8 @@ void iwl_rx_queue_free(struct iwl_priv *priv, struct iwl_rx_queue *rxq)
> for (i = 0; i < RX_QUEUE_SIZE + RX_FREE_BUFFERS; i++) {
> if (rxq->pool[i].skb != NULL) {
> pci_unmap_single(priv->pci_dev,
> - rxq->pool[i].dma_addr,
> - priv->hw_params.rx_buf_size,
> + rxq->pool[i].real_dma_addr,
> + priv->hw_params.rx_buf_size + 256,
> PCI_DMA_FROMDEVICE);
> dev_kfree_skb(rxq->pool[i].skb);
> }
> @@ -354,8 +362,8 @@ void iwl_rx_queue_reset(struct iwl_priv *priv, struct iwl_rx_queue *rxq)
> * to an SKB, so we need to unmap and free potential storage */
> if (rxq->pool[i].skb != NULL) {
> pci_unmap_single(priv->pci_dev,
> - rxq->pool[i].dma_addr,
> - priv->hw_params.rx_buf_size,
> + rxq->pool[i].real_dma_addr,
> + priv->hw_params.rx_buf_size + 256,
> PCI_DMA_FROMDEVICE);
> priv->alloc_rxb_skb--;
> dev_kfree_skb(rxq->pool[i].skb);
> diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c b/drivers/net/wireless/iwlwifi/iwl3945-base.c
> index 285b53e..45a6b0c 100644
> --- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
> +++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
> @@ -6012,7 +6012,6 @@ static void iwl3945_bg_alive_start(struct work_struct *data)
> mutex_lock(&priv->mutex);
> iwl3945_alive_start(priv);
> mutex_unlock(&priv->mutex);
> - ieee80211_notify_mac(priv->hw, IEEE80211_NOTIFY_RE_ASSOC);
> }
>
> static void iwl3945_bg_rf_kill(struct work_struct *work)
> diff --git a/drivers/net/wireless/libertas_tf/if_usb.c b/drivers/net/wireless/libertas_tf/if_usb.c
> index 1cc03a8..59634c3 100644
> --- a/drivers/net/wireless/libertas_tf/if_usb.c
> +++ b/drivers/net/wireless/libertas_tf/if_usb.c
> @@ -331,7 +331,7 @@ static int __if_usb_submit_rx_urb(struct if_usb_card *cardp,
> /* Fill the receive configuration URB and initialise the Rx call back */
> usb_fill_bulk_urb(cardp->rx_urb, cardp->udev,
> usb_rcvbulkpipe(cardp->udev, cardp->ep_in),
> - (void *) (skb->tail),
> + skb_tail_pointer(skb),
> MRVDRV_ETH_RX_PACKET_BUFFER_SIZE, callbackfn, cardp);
>
> cardp->rx_urb->transfer_flags |= URB_ZERO_PACKET;
> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> index 8856e2d..73d81bc 100644
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -74,14 +74,6 @@
> */
>
> /**
> - * enum ieee80211_notification_type - Low level driver notification
> - * @IEEE80211_NOTIFY_RE_ASSOC: start the re-association sequence
> - */
> -enum ieee80211_notification_types {
> - IEEE80211_NOTIFY_RE_ASSOC,
> -};
> -
> -/**
> * struct ieee80211_ht_bss_info - describing BSS's HT characteristics
> *
> * This structure describes most essential parameters needed
> @@ -1798,18 +1790,6 @@ void ieee80211_stop_tx_ba_cb_irqsafe(struct ieee80211_hw *hw, const u8 *ra,
> u16 tid);
>
> /**
> - * ieee80211_notify_mac - low level driver notification
> - * @hw: pointer as obtained from ieee80211_alloc_hw().
> - * @notif_type: enum ieee80211_notification_types
> - *
> - * This function must be called by low level driver to inform mac80211 of
> - * low level driver status change or force mac80211 to re-assoc for low
> - * level driver internal error that require re-assoc.
> - */
> -void ieee80211_notify_mac(struct ieee80211_hw *hw,
> - enum ieee80211_notification_types notif_type);
> -
> -/**
> * ieee80211_find_sta - find a station
> *
> * @hw: pointer as obtained from ieee80211_alloc_hw()
> diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
> index 14d165f..409bb77 100644
> --- a/net/mac80211/mlme.c
> +++ b/net/mac80211/mlme.c
> @@ -2560,25 +2560,3 @@ void ieee80211_mlme_notify_scan_completed(struct ieee80211_local *local)
> ieee80211_restart_sta_timer(sdata);
> rcu_read_unlock();
> }
> -
> -/* driver notification call */
> -void ieee80211_notify_mac(struct ieee80211_hw *hw,
> - enum ieee80211_notification_types notif_type)
> -{
> - struct ieee80211_local *local = hw_to_local(hw);
> - struct ieee80211_sub_if_data *sdata;
> -
> - switch (notif_type) {
> - case IEEE80211_NOTIFY_RE_ASSOC:
> - rtnl_lock();
> - list_for_each_entry(sdata, &local->interfaces, list) {
> - if (sdata->vif.type != NL80211_IFTYPE_STATION)
> - continue;
> -
> - ieee80211_sta_req_auth(sdata, &sdata->u.sta);
> - }
> - rtnl_unlock();
> - break;
> - }
> -}
> -EXPORT_SYMBOL(ieee80211_notify_mac);
> --
> John W. Linville Linux should be at the core
> [email protected] of your literate lifestyle.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>

2008-11-19 09:34:54

by Tomas Winkler

[permalink] [raw]
Subject: Re: pull request: wireless-2.6 2008-11-18

On Wed, Nov 19, 2008 at 8:46 AM, Tomas Winkler <[email protected]> wrote:
> On Wed, Nov 19, 2008 at 2:07 AM, John W. Linville
> <[email protected]> wrote:
>> Dave,
>>
>> Three fixes from Johannes, intended for 2.6.28... One fixes a 32/64
>> issue in libertas_tf, another removes a mac80211 callback only used
>> by iwlwifi which is not obviously needed and which is causing locking
>> issues, and a final one fixes some mysterious DMA alignment problems
>> that have been plaguing iwlwifi for some time.
>>
>> Please let me know if there are problems!
>>
>> Thanks,
>>
>> John
>>
>> P.S. The libertas_tf is actually already in net-next-2.6. Also,
>> the other fixes cause merge conflicts with net-next-2.6 -- I'll
>> included a merge-test branch in wireless-next-2.6 to indicates how
>> those conflicts should be resolved.
>>
>> ---
>>
>> Individual patches are available here:
>>
>> http://www.kernel.org/pub/linux/kernel/people/linville/wireless-2.6/
>>
>> ---
>>
>> The following changes since commit 5f9021cfdc3524a4c5e3d7ae2d049eb7adcd6776:
>> Johannes Berg (1):
>> rtnetlink: propagate error from dev_change_flags in do_setlink()
>>
>> are available in the git repository at:
>>
>> git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless-2.6.git master
>>
>> Johannes Berg (3):
>> libertas_tf: fix skb tail pointer
>> mac80211: remove ieee80211_notify_mac
>> iwlagn: fix RX skb alignment
>
> IMHO its premature pushing ttese 2 patches up, they came in yestrday
> and nobody here has run tthe code

Just checked our bug database, the ieee80211_notify_mac was
introduced mainly to overcome HW bug when
receiver become deaf in heavy traffic such as ftp in noisy
environment. Otherwise reconnection was too slow to keep ftp going. We
need to check whether we are still hitting this before applying
removal this function
Thanks
Tomas

>>
>> drivers/net/wireless/iwlwifi/iwl-agn.c | 7 +++----
>> drivers/net/wireless/iwlwifi/iwl-dev.h | 3 ++-
>> drivers/net/wireless/iwlwifi/iwl-rx.c | 26 +++++++++++++++++---------
>> drivers/net/wireless/iwlwifi/iwl3945-base.c | 1 -
>> drivers/net/wireless/libertas_tf/if_usb.c | 2 +-
>> include/net/mac80211.h | 20 --------------------
>> net/mac80211/mlme.c | 22 ----------------------
>> 7 files changed, 23 insertions(+), 58 deletions(-)
>>
>> diff --git a/drivers/net/wireless/iwlwifi/iwl-agn.c b/drivers/net/wireless/iwlwifi/iwl-agn.c
>> index 8d690a0..444c5cc 100644
>> --- a/drivers/net/wireless/iwlwifi/iwl-agn.c
>> +++ b/drivers/net/wireless/iwlwifi/iwl-agn.c
>> @@ -1384,7 +1384,7 @@ void iwl_rx_handle(struct iwl_priv *priv)
>>
>> rxq->queue[i] = NULL;
>>
>> - pci_dma_sync_single_for_cpu(priv->pci_dev, rxb->dma_addr,
>> + pci_dma_sync_single_for_cpu(priv->pci_dev, rxb->aligned_dma_addr,
>> priv->hw_params.rx_buf_size,
>> PCI_DMA_FROMDEVICE);
>> pkt = (struct iwl_rx_packet *)rxb->skb->data;
>> @@ -1436,8 +1436,8 @@ void iwl_rx_handle(struct iwl_priv *priv)
>> rxb->skb = NULL;
>> }
>>
>> - pci_unmap_single(priv->pci_dev, rxb->dma_addr,
>> - priv->hw_params.rx_buf_size,
>> + pci_unmap_single(priv->pci_dev, rxb->real_dma_addr,
>> + priv->hw_params.rx_buf_size + 256,
>> PCI_DMA_FROMDEVICE);
>> spin_lock_irqsave(&rxq->lock, flags);
>> list_add_tail(&rxb->list, &priv->rxq.rx_used);
>> @@ -2341,7 +2341,6 @@ static void iwl_bg_alive_start(struct work_struct *data)
>> mutex_lock(&priv->mutex);
>> iwl_alive_start(priv);
>> mutex_unlock(&priv->mutex);
>> - ieee80211_notify_mac(priv->hw, IEEE80211_NOTIFY_RE_ASSOC);
>> }
>>
>> static void iwl4965_bg_rf_kill(struct work_struct *work)
>> diff --git a/drivers/net/wireless/iwlwifi/iwl-dev.h b/drivers/net/wireless/iwlwifi/iwl-dev.h
>> index c018121..9966d4e 100644
>> --- a/drivers/net/wireless/iwlwifi/iwl-dev.h
>> +++ b/drivers/net/wireless/iwlwifi/iwl-dev.h
>> @@ -89,7 +89,8 @@ extern struct iwl_cfg iwl5100_abg_cfg;
>> #define DEFAULT_LONG_RETRY_LIMIT 4U
>>
>> struct iwl_rx_mem_buffer {
>> - dma_addr_t dma_addr;
>> + dma_addr_t real_dma_addr;
>> + dma_addr_t aligned_dma_addr;
>> struct sk_buff *skb;
>> struct list_head list;
>> };
>> diff --git a/drivers/net/wireless/iwlwifi/iwl-rx.c b/drivers/net/wireless/iwlwifi/iwl-rx.c
>> index 7cde9d7..0509c16 100644
>> --- a/drivers/net/wireless/iwlwifi/iwl-rx.c
>> +++ b/drivers/net/wireless/iwlwifi/iwl-rx.c
>> @@ -204,7 +204,7 @@ int iwl_rx_queue_restock(struct iwl_priv *priv)
>> list_del(element);
>>
>> /* Point to Rx buffer via next RBD in circular buffer */
>> - rxq->bd[rxq->write] = iwl_dma_addr2rbd_ptr(priv, rxb->dma_addr);
>> + rxq->bd[rxq->write] = iwl_dma_addr2rbd_ptr(priv, rxb->aligned_dma_addr);
>> rxq->queue[rxq->write] = rxb;
>> rxq->write = (rxq->write + 1) & RX_QUEUE_MASK;
>> rxq->free_count--;
>> @@ -251,7 +251,7 @@ void iwl_rx_allocate(struct iwl_priv *priv)
>> rxb = list_entry(element, struct iwl_rx_mem_buffer, list);
>>
>> /* Alloc a new receive buffer */
>> - rxb->skb = alloc_skb(priv->hw_params.rx_buf_size,
>> + rxb->skb = alloc_skb(priv->hw_params.rx_buf_size + 256,
>> __GFP_NOWARN | GFP_ATOMIC);
>> if (!rxb->skb) {
>> if (net_ratelimit())
>> @@ -266,9 +266,17 @@ void iwl_rx_allocate(struct iwl_priv *priv)
>> list_del(element);
>>
>> /* Get physical address of RB/SKB */
>> - rxb->dma_addr =
>> - pci_map_single(priv->pci_dev, rxb->skb->data,
>> - priv->hw_params.rx_buf_size, PCI_DMA_FROMDEVICE);
>> + rxb->real_dma_addr = pci_map_single(
>> + priv->pci_dev,
>> + rxb->skb->data,
>> + priv->hw_params.rx_buf_size + 256,
>> + PCI_DMA_FROMDEVICE);
>> + /* dma address must be no more than 36 bits */
>> + BUG_ON(rxb->real_dma_addr & ~DMA_BIT_MASK(36));
>> + /* and also 256 byte aligned! */
>> + rxb->aligned_dma_addr = ALIGN(rxb->real_dma_addr, 256);
>> + skb_reserve(rxb->skb, rxb->aligned_dma_addr - rxb->real_dma_addr);
>> +
>> list_add_tail(&rxb->list, &rxq->rx_free);
>> rxq->free_count++;
>> }
>> @@ -300,8 +308,8 @@ void iwl_rx_queue_free(struct iwl_priv *priv, struct iwl_rx_queue *rxq)
>> for (i = 0; i < RX_QUEUE_SIZE + RX_FREE_BUFFERS; i++) {
>> if (rxq->pool[i].skb != NULL) {
>> pci_unmap_single(priv->pci_dev,
>> - rxq->pool[i].dma_addr,
>> - priv->hw_params.rx_buf_size,
>> + rxq->pool[i].real_dma_addr,
>> + priv->hw_params.rx_buf_size + 256,
>> PCI_DMA_FROMDEVICE);
>> dev_kfree_skb(rxq->pool[i].skb);
>> }
>> @@ -354,8 +362,8 @@ void iwl_rx_queue_reset(struct iwl_priv *priv, struct iwl_rx_queue *rxq)
>> * to an SKB, so we need to unmap and free potential storage */
>> if (rxq->pool[i].skb != NULL) {
>> pci_unmap_single(priv->pci_dev,
>> - rxq->pool[i].dma_addr,
>> - priv->hw_params.rx_buf_size,
>> + rxq->pool[i].real_dma_addr,
>> + priv->hw_params.rx_buf_size + 256,
>> PCI_DMA_FROMDEVICE);
>> priv->alloc_rxb_skb--;
>> dev_kfree_skb(rxq->pool[i].skb);
>> diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c b/drivers/net/wireless/iwlwifi/iwl3945-base.c
>> index 285b53e..45a6b0c 100644
>> --- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
>> +++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
>> @@ -6012,7 +6012,6 @@ static void iwl3945_bg_alive_start(struct work_struct *data)
>> mutex_lock(&priv->mutex);
>> iwl3945_alive_start(priv);
>> mutex_unlock(&priv->mutex);
>> - ieee80211_notify_mac(priv->hw, IEEE80211_NOTIFY_RE_ASSOC);
>> }
>>
>> static void iwl3945_bg_rf_kill(struct work_struct *work)
>> diff --git a/drivers/net/wireless/libertas_tf/if_usb.c b/drivers/net/wireless/libertas_tf/if_usb.c
>> index 1cc03a8..59634c3 100644
>> --- a/drivers/net/wireless/libertas_tf/if_usb.c
>> +++ b/drivers/net/wireless/libertas_tf/if_usb.c
>> @@ -331,7 +331,7 @@ static int __if_usb_submit_rx_urb(struct if_usb_card *cardp,
>> /* Fill the receive configuration URB and initialise the Rx call back */
>> usb_fill_bulk_urb(cardp->rx_urb, cardp->udev,
>> usb_rcvbulkpipe(cardp->udev, cardp->ep_in),
>> - (void *) (skb->tail),
>> + skb_tail_pointer(skb),
>> MRVDRV_ETH_RX_PACKET_BUFFER_SIZE, callbackfn, cardp);
>>
>> cardp->rx_urb->transfer_flags |= URB_ZERO_PACKET;
>> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
>> index 8856e2d..73d81bc 100644
>> --- a/include/net/mac80211.h
>> +++ b/include/net/mac80211.h
>> @@ -74,14 +74,6 @@
>> */
>>
>> /**
>> - * enum ieee80211_notification_type - Low level driver notification
>> - * @IEEE80211_NOTIFY_RE_ASSOC: start the re-association sequence
>> - */
>> -enum ieee80211_notification_types {
>> - IEEE80211_NOTIFY_RE_ASSOC,
>> -};
>> -
>> -/**
>> * struct ieee80211_ht_bss_info - describing BSS's HT characteristics
>> *
>> * This structure describes most essential parameters needed
>> @@ -1798,18 +1790,6 @@ void ieee80211_stop_tx_ba_cb_irqsafe(struct ieee80211_hw *hw, const u8 *ra,
>> u16 tid);
>>
>> /**
>> - * ieee80211_notify_mac - low level driver notification
>> - * @hw: pointer as obtained from ieee80211_alloc_hw().
>> - * @notif_type: enum ieee80211_notification_types
>> - *
>> - * This function must be called by low level driver to inform mac80211 of
>> - * low level driver status change or force mac80211 to re-assoc for low
>> - * level driver internal error that require re-assoc.
>> - */
>> -void ieee80211_notify_mac(struct ieee80211_hw *hw,
>> - enum ieee80211_notification_types notif_type);
>> -
>> -/**
>> * ieee80211_find_sta - find a station
>> *
>> * @hw: pointer as obtained from ieee80211_alloc_hw()
>> diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
>> index 14d165f..409bb77 100644
>> --- a/net/mac80211/mlme.c
>> +++ b/net/mac80211/mlme.c
>> @@ -2560,25 +2560,3 @@ void ieee80211_mlme_notify_scan_completed(struct ieee80211_local *local)
>> ieee80211_restart_sta_timer(sdata);
>> rcu_read_unlock();
>> }
>> -
>> -/* driver notification call */
>> -void ieee80211_notify_mac(struct ieee80211_hw *hw,
>> - enum ieee80211_notification_types notif_type)
>> -{
>> - struct ieee80211_local *local = hw_to_local(hw);
>> - struct ieee80211_sub_if_data *sdata;
>> -
>> - switch (notif_type) {
>> - case IEEE80211_NOTIFY_RE_ASSOC:
>> - rtnl_lock();
>> - list_for_each_entry(sdata, &local->interfaces, list) {
>> - if (sdata->vif.type != NL80211_IFTYPE_STATION)
>> - continue;
>> -
>> - ieee80211_sta_req_auth(sdata, &sdata->u.sta);
>> - }
>> - rtnl_unlock();
>> - break;
>> - }
>> -}
>> -EXPORT_SYMBOL(ieee80211_notify_mac);
>> --
>> John W. Linville Linux should be at the core
>> [email protected] of your literate lifestyle.
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/
>>
>

2008-11-19 14:46:26

by John W. Linville

[permalink] [raw]
Subject: Re: pull request: wireless-2.6 2008-11-18

On Wed, Nov 19, 2008 at 11:34:37AM +0200, Tomas Winkler wrote:
> On Wed, Nov 19, 2008 at 8:46 AM, Tomas Winkler <[email protected]> wrote:
> > On Wed, Nov 19, 2008 at 2:07 AM, John W. Linville
> > <[email protected]> wrote:

> >> mac80211: remove ieee80211_notify_mac
> >> iwlagn: fix RX skb alignment
> >
> > IMHO its premature pushing ttese 2 patches up, they came in yestrday
> > and nobody here has run tthe code

Well, thanks for your opinion. Here is mine:

The iwlwifi drivers have been hitting these problems for months,
and I have seen no effort by your team to address the problems.
It is possible that your team is working on them behind closed doors
and will eventually throw something over the wall to us. I'm tired
of waiting for that, and I imagine that hordes of iwlagn users are
tired of waiting as well.

Johannes has presented us with plausible fixes, and people are
reporting that the patches work for them. I can merge these patches,
or wait/hope/pray for some to come from your team. Experience suggests
that if fixes do come from your team that they will either be buried
in some monster patch that largely addresses something else, or
that the patches will arrive with a changelog that is terse and/or
unintelligble. Subsequently, I am not optimistic about waiting.

I think Johannes cited four different bugzilla.kernel.org entries
between those two patches. What is your team doing to address
those bugs?

Obviously, I still think that those patches should be merged.

> Just checked our bug database, the ieee80211_notify_mac was
> introduced mainly to overcome HW bug when
> receiver become deaf in heavy traffic such as ftp in noisy
> environment. Otherwise reconnection was too slow to keep ftp going. We
> need to check whether we are still hitting this before applying
> removal this function

You are welcome to submit patches that address your hardware issue
and which do not introduce locking problems.

Thanks,

John
--
John W. Linville Linux should be at the core
[email protected] of your literate lifestyle.

2008-11-19 16:52:11

by Tomas Winkler

[permalink] [raw]
Subject: Re: pull request: wireless-2.6 2008-11-18

On Wed, Nov 19, 2008 at 4:39 PM, John W. Linville
<[email protected]> wrote:
> On Wed, Nov 19, 2008 at 11:34:37AM +0200, Tomas Winkler wrote:
>> On Wed, Nov 19, 2008 at 8:46 AM, Tomas Winkler <[email protected]> wrote:
>> > On Wed, Nov 19, 2008 at 2:07 AM, John W. Linville
>> > <[email protected]> wrote:
>
>> >> mac80211: remove ieee80211_notify_mac
>> >> iwlagn: fix RX skb alignment
>> >
>> > IMHO its premature pushing ttese 2 patches up, they came in yestrday
>> > and nobody here has run tthe code
>
> Well, thanks for your opinion. Here is mine:
>
> The iwlwifi drivers have been hitting these problems for months,
> and I have seen no effort by your team to address the problems.

There is a lot of effort to solve this I'm not sure what mailing list
are u reading.
We even presented test patches with alignment issues as Johannes few
month ago but
we didn't get the feedback that it hit anything. So you are totally off here.

> It is possible that your team is working on them behind closed doors
> and will eventually throw something over the wall to us. I'm tired
> of waiting for that, and I imagine that hordes of iwlagn users are
> tired of waiting as well.

This is embarrassing also for us that we cannot locate this specific
problem but it not like we are not doing anything.Not sure there are
other examples that we are not addressing bugs promptly.

> Johannes has presented us with plausible fixes, and people are
> reporting that the patches work for them.

I'm not against merging these patches to wireless-testing but pushing
them upstream before testing them
is just plainly wrong and all I asked for to test them. You are
presenting here double standards here.

I can merge these patches,
> or wait/hope/pray for some to come from your team. Experience suggests
> that if fixes do come from your team that they will either be buried
> in some monster patch that largely addresses something else, or
> that the patches will arrive with a changelog that is terse and/or
> unintelligble. Subsequently, I am not optimistic about waiting.

I've believe that we are addressing all the complains above and
currently if you are not seeing this I'm really sorry.

> I think Johannes cited four different bugzilla.kernel.org entries
> between those two patches. What is your team doing to address
> those bugs?

Again you are not reading the mailing list and not the bugs logs.

>
> Obviously, I still think that those patches should be merged.

I'm not against just give it day or two to run it. We have for
example nightly regression builds with publicly available results why
you cannot wait till we can run? I think you've hurried to push it up
just as a statement forgetting of possible regressions.

>
>> Just checked our bug database, the ieee80211_notify_mac was
>> introduced mainly to overcome HW bug when
>> receiver become deaf in heavy traffic such as ftp in noisy
>> environment. Otherwise reconnection was too slow to keep ftp going. We
>> need to check whether we are still hitting this before applying
>> removal this function
>
> You are welcome to submit patches that address your hardware issue
> and which do not introduce locking problems.

That was my suggestion that we fix it and I believe we have normal
discussion about it till you took your eager pushing steps. Replacing
one regression with another is just not sane.

Thanks
Tomas

2008-11-19 19:01:28

by John W. Linville

[permalink] [raw]
Subject: Re: pull request: wireless-2.6 2008-11-18

On Wed, Nov 19, 2008 at 06:51:49PM +0200, Tomas Winkler wrote:
> On Wed, Nov 19, 2008 at 4:39 PM, John W. Linville
> <[email protected]> wrote:
> > On Wed, Nov 19, 2008 at 11:34:37AM +0200, Tomas Winkler wrote:
> >> On Wed, Nov 19, 2008 at 8:46 AM, Tomas Winkler <[email protected]> wrote:
> >> > On Wed, Nov 19, 2008 at 2:07 AM, John W. Linville
> >> > <[email protected]> wrote:
> >
> >> >> mac80211: remove ieee80211_notify_mac
> >> >> iwlagn: fix RX skb alignment
> >> >
> >> > IMHO its premature pushing ttese 2 patches up, they came in yestrday
> >> > and nobody here has run tthe code
> >
> > Well, thanks for your opinion. Here is mine:
> >
> > The iwlwifi drivers have been hitting these problems for months,
> > and I have seen no effort by your team to address the problems.
>
> There is a lot of effort to solve this I'm not sure what mailing list
> are u reading.
> We even presented test patches with alignment issues as Johannes few
> month ago but
> we didn't get the feedback that it hit anything. So you are totally off here.

Citation needed. All I see is Johannes' patch followed by your FUD.

> > It is possible that your team is working on them behind closed doors
> > and will eventually throw something over the wall to us. I'm tired
> > of waiting for that, and I imagine that hordes of iwlagn users are
> > tired of waiting as well.
>
> This is embarrassing also for us that we cannot locate this specific
> problem but it not like we are not doing anything.Not sure there are
> other examples that we are not addressing bugs promptly.
>
> > Johannes has presented us with plausible fixes, and people are
> > reporting that the patches work for them.
>
> I'm not against merging these patches to wireless-testing but pushing
> them upstream before testing them
> is just plainly wrong and all I asked for to test them. You are
> presenting here double standards here.

Others have tested it already.

> I can merge these patches,
> > or wait/hope/pray for some to come from your team. Experience suggests
> > that if fixes do come from your team that they will either be buried
> > in some monster patch that largely addresses something else, or
> > that the patches will arrive with a changelog that is terse and/or
> > unintelligble. Subsequently, I am not optimistic about waiting.
>
> I've believe that we are addressing all the complains above and
> currently if you are not seeing this I'm really sorry.

I'm not seeing it.

> > I think Johannes cited four different bugzilla.kernel.org entries
> > between those two patches. What is your team doing to address
> > those bugs?
>
> Again you are not reading the mailing list and not the bugs logs.

http://bugzilla.kernel.org/show_bug.cgi?id=11731
http://bugzilla.kernel.org/show_bug.cgi?id=11818
http://bugzilla.kernel.org/show_bug.cgi?id=11923
http://bugzilla.kernel.org/show_bug.cgi?id=11983
http://bugzilla.kernel.org/show_bug.cgi?id=12017
http://bugzilla.kernel.org/show_bug.cgi?id=12044
http://bugzilla.kernel.org/show_bug.cgi?id=12046

Those are only the ones not assigned to a specific person. Yet, each
of them has a member of your team CC'ed. Also, I made no effort to
track-down the ones in vendor bugzillas.

> > Obviously, I still think that those patches should be merged.
>
> I'm not against just give it day or two to run it. We have for
> example nightly regression builds with publicly available results why
> you cannot wait till we can run? I think you've hurried to push it up
> just as a statement forgetting of possible regressions.

I merely did not wait for your blessing, mostly because I have no
idea when it might come.

> >> Just checked our bug database, the ieee80211_notify_mac was
> >> introduced mainly to overcome HW bug when
> >> receiver become deaf in heavy traffic such as ftp in noisy
> >> environment. Otherwise reconnection was too slow to keep ftp going. We
> >> need to check whether we are still hitting this before applying
> >> removal this function
> >
> > You are welcome to submit patches that address your hardware issue
> > and which do not introduce locking problems.
>
> That was my suggestion that we fix it and I believe we have normal
> discussion about it till you took your eager pushing steps. Replacing
> one regression with another is just not sane.

ieee80211_notify_mac is creating locking problems. I'd rather hit
a few disconnects than endure locking problems.

Now, if you could put as much energy into fixing these issues as you
are putting into arguing with me or anyone else contradicting you...

John
--
John W. Linville Linux should be at the core
[email protected] of your literate lifestyle.

2008-11-19 21:55:33

by Tomas Winkler

[permalink] [raw]
Subject: Re: pull request: wireless-2.6 2008-11-18

On Wed, Nov 19, 2008 at 8:53 PM, John W. Linville
<[email protected]> wrote:
> On Wed, Nov 19, 2008 at 06:51:49PM +0200, Tomas Winkler wrote:
>> On Wed, Nov 19, 2008 at 4:39 PM, John W. Linville
>> <[email protected]> wrote:
>> > On Wed, Nov 19, 2008 at 11:34:37AM +0200, Tomas Winkler wrote:
>> >> On Wed, Nov 19, 2008 at 8:46 AM, Tomas Winkler <[email protected]> wrote:
>> >> > On Wed, Nov 19, 2008 at 2:07 AM, John W. Linville
>> >> > <[email protected]> wrote:
>> >
>> >> >> mac80211: remove ieee80211_notify_mac
>> >> >> iwlagn: fix RX skb alignment
>> >> >
>> >> > IMHO its premature pushing ttese 2 patches up, they came in yestrday
>> >> > and nobody here has run tthe code
>> >
>> > Well, thanks for your opinion. Here is mine:
>> >
>> > The iwlwifi drivers have been hitting these problems for months,
>> > and I have seen no effort by your team to address the problems.
>>
>> There is a lot of effort to solve this I'm not sure what mailing list
>> are u reading.
>> We even presented test patches with alignment issues as Johannes few
>> month ago but
>> we didn't get the feedback that it hit anything. So you are totally off here.
>
> Citation needed. All I see is Johannes' patch followed by your FUD.
>

http://marc.info/?l=linux-wireless&m=122549613209468&w=2,
there is Yi's patch also mentioned int in the Johannes commit.
unfortunately there was a programming typo in it.

There is named 'iwlwifi: get some more information about command
failure' in your pull request you've just issued!!!

>> > It is possible that your team is working on them behind closed doors
>> > and will eventually throw something over the wall to us. I'm tired
>> > of waiting for that, and I imagine that hordes of iwlagn users are
>> > tired of waiting as well.
>>
>> This is embarrassing also for us that we cannot locate this specific
>> problem but it not like we are not doing anything.Not sure there are
>> other examples that we are not addressing bugs promptly.
>>
>> > Johannes has presented us with plausible fixes, and people are
>> > reporting that the patches work for them.
>>
>> I'm not against merging these patches to wireless-testing but pushing
>> them upstream before testing them
>> is just plainly wrong and all I asked for to test them. You are
>> presenting here double standards here.
>
> Others have tested it already.

You cannot be serious about 5 minute testing. We already have reports
that it doesn't solve the issue completely.

>
>> I can merge these patches,
>> > or wait/hope/pray for some to come from your team. Experience suggests
>> > that if fixes do come from your team that they will either be buried
>> > in some monster patch that largely addresses something else, or
>> > that the patches will arrive with a changelog that is terse and/or
>> > unintelligble. Subsequently, I am not optimistic about waiting.
>>
>> I've believe that we are addressing all the complains above and
>> currently if you are not seeing this I'm really sorry.
>
> I'm not seeing i

I've resubmitted every patch you've had an issue with readability of
the commit message so you should be first to see it.

>
>> > I think Johannes cited four different bugzilla.kernel.org entries
>> > between those two patches. What is your team doing to address
>> > those bugs?
>>
>> Again you are not reading the mailing list and not the bugs logs.
>
> http://bugzilla.kernel.org/show_bug.cgi?id=11731
> http://bugzilla.kernel.org/show_bug.cgi?id=11818
> http://bugzilla.kernel.org/show_bug.cgi?id=11923
> http://bugzilla.kernel.org/show_bug.cgi?id=11983
> http://bugzilla.kernel.org/show_bug.cgi?id=12017
> http://bugzilla.kernel.org/show_bug.cgi?id=12044
> http://bugzilla.kernel.org/show_bug.cgi?id=12046
>
All of those are actively tracked by Yi as far as I can say.

> Those are only the ones not assigned to a specific person. Yet, each
> of them has a member of your team CC'ed. Also, I made no effort to
> track-down the ones in vendor bugzillas.



>> > Obviously, I still think that those patches should be merged.
>>
>> I'm not against just give it day or two to run it. We have for
>> example nightly regression builds with publicly available results why
>> you cannot wait till we can run? I think you've hurried to push it up
>> just as a statement forgetting of possible regressions.
>
> I merely did not wait for your blessing, mostly because I have no
> idea when it might come.

This just prove you are doing statements.

>
>> >> Just checked our bug database, the ieee80211_notify_mac was
>> >> introduced mainly to overcome HW bug when
>> >> receiver become deaf in heavy traffic such as ftp in noisy
>> >> environment. Otherwise reconnection was too slow to keep ftp going. We
>> >> need to check whether we are still hitting this before applying
>> >> removal this function
>> >
>> > You are welcome to submit patches that address your hardware issue
>> > and which do not introduce locking problems.
>>
>> That was my suggestion that we fix it and I believe we have normal
>> discussion about it till you took your eager pushing steps. Replacing
>> one regression with another is just not sane.
>
> ieee80211_notify_mac is creating locking problems. I'd rather hit
> a few disconnects than endure locking problems.
>
> Now, if you could put as much energy into fixing these issues as you
> are putting into arguing with me or anyone else contradicting you...

I'm not a yes man and if I think differently I say so. It's legitimate
to have another opinion and I'm really trying to keep it technical and
I'm not ashamed to say if I'm wrong, but you people are so hostile
from some unknown reason I just hope somebody in anthropology is
making his doctoral thesis of it to explain it to me.

Tomas

2008-11-20 00:46:33

by John W. Linville

[permalink] [raw]
Subject: Re: pull request: wireless-2.6 2008-11-18

On Wed, Nov 19, 2008 at 11:55:15PM +0200, Tomas Winkler wrote:
> On Wed, Nov 19, 2008 at 8:53 PM, John W. Linville
> <[email protected]> wrote:

> > Now, if you could put as much energy into fixing these issues as you
> > are putting into arguing with me or anyone else contradicting you...
>
> I'm not a yes man and if I think differently I say so. It's legitimate
> to have another opinion and I'm really trying to keep it technical and
> I'm not ashamed to say if I'm wrong, but you people are so hostile
> from some unknown reason I just hope somebody in anthropology is
> making his doctoral thesis of it to explain it to me.

Of course! You are right and all the rest of us are wrong. How could
I have been so stupid? Thank you for being so easy to work with
while the rest of us are being so difficult.

Are you done?

John
--
John W. Linville Linux should be at the core
[email protected] of your literate lifestyle.

2008-11-20 09:27:24

by Tomas Winkler

[permalink] [raw]
Subject: Re: pull request: wireless-2.6 2008-11-18

On Thu, Nov 20, 2008 at 2:37 AM, John W. Linville
<[email protected]> wrote:
> On Wed, Nov 19, 2008 at 11:55:15PM +0200, Tomas Winkler wrote:
>> On Wed, Nov 19, 2008 at 8:53 PM, John W. Linville
>> <[email protected]> wrote:
>
>> > Now, if you could put as much energy into fixing these issues as you
>> > are putting into arguing with me or anyone else contradicting you...
>>
>> I'm not a yes man and if I think differently I say so. It's legitimate
>> to have another opinion and I'm really trying to keep it technical and
>> I'm not ashamed to say if I'm wrong, but you people are so hostile
>> from some unknown reason I just hope somebody in anthropology is
>> making his doctoral thesis of it to explain it to me.
>
> Of course! You are right and all the rest of us are wrong. How could
> I have been so stupid? Thank you for being so easy to work with
> while the rest of us are being so difficult.
>

Citing my self again ' I'm not ashamed to say if I'm wrong' I'm not
always right and you be careful with majority
rightness it put whole nations into misery.

> Are you done?

Common John you've started that. Compare your IMHO with mine, it's not
full of empty accusations. I'm not less frustrated with development
process of mac80211 then you are with our, and if you see the history
this is where we always clash. I think we also have valid points if
you would care to listen.

Thanks
Tomas

2008-11-20 12:05:24

by David Miller

[permalink] [raw]
Subject: Re: pull request: wireless-2.6 2008-11-18

From: "John W. Linville" <[email protected]>
Date: Tue, 18 Nov 2008 19:07:19 -0500

> Three fixes from Johannes, intended for 2.6.28... One fixes a 32/64
> issue in libertas_tf, another removes a mac80211 callback only used
> by iwlwifi which is not obviously needed and which is causing locking
> issues, and a final one fixes some mysterious DMA alignment problems
> that have been plaguing iwlwifi for some time.
>
> Please let me know if there are problems!

Pulled, thanks John.

2008-11-24 03:34:17

by Zhu Yi

[permalink] [raw]
Subject: Re: pull request: wireless-2.6 2008-11-18

On Wed, 2008-11-19 at 22:39 +0800, John W. Linville wrote:
> Well, thanks for your opinion. Here is mine:

I have to say something here. Although not related to the original
ieee80211_notify_mac change.

> The iwlwifi drivers have been hitting these problems for months,
> and I have seen no effort by your team to address the problems.
> It is possible that your team is working on them behind closed doors
> and will eventually throw something over the wall to us. I'm tired
> of waiting for that, and I imagine that hordes of iwlagn users are
> tired of waiting as well.

This is not correct. We are tracing this problem in our bugzilla. And it
is the open place. Much more people have used it for a much longer time
even before the kernel bugzilla existed. If you take a look at the bug,
you know how much effort we put on it:
http://www.intellinuxwireless.org/bugzilla/show_bug.cgi?id=1703

> Johannes has presented us with plausible fixes, and people are
> reporting that the patches work for them. I can merge these patches,
> or wait/hope/pray for some to come from your team. Experience
> suggests
> that if fixes do come from your team that they will either be buried
> in some monster patch that largely addresses something else, or
> that the patches will arrive with a changelog that is terse and/or
> unintelligble. Subsequently, I am not optimistic about waiting.
>
> I think Johannes cited four different bugzilla.kernel.org entries
> between those two patches. What is your team doing to address
> those bugs?

Johannes' effort is great and should certainly be praised. But looking
down upon Intel's effort is nonsense. We suspected this alignment issue
in the very beginning. But the set of users helped us to debug with this
problem happened to have a different root cause than Johannes. Thus came
with the alignment BUG_ON assert put into the code in case other people
experience with it (unfortunately, it was a wrong one. As I had
appologized for this already.)

The problem should at least have two causes. Johannes had resolved one
of them. The other one (AFAICS) relates to be in an 11n AP environment
(no need to be associated with it) and takes longer time to reproduce.
We will keep tracking it whatever we you think us about it.

Thanks,
-yi

2008-11-24 03:51:09

by Marcel Holtmann

[permalink] [raw]
Subject: Re: pull request: wireless-2.6 2008-11-18

Hi Yi,

>> The iwlwifi drivers have been hitting these problems for months,
>> and I have seen no effort by your team to address the problems.
>> It is possible that your team is working on them behind closed doors
>> and will eventually throw something over the wall to us. I'm tired
>> of waiting for that, and I imagine that hordes of iwlagn users are
>> tired of waiting as well.
>
> This is not correct. We are tracing this problem in our bugzilla.
> And it
> is the open place. Much more people have used it for a much longer
> time
> even before the kernel bugzilla existed. If you take a look at the
> bug,
> you know how much effort we put on it:
> http://www.intellinuxwireless.org/bugzilla/show_bug.cgi?id=1703

and just relying on Bugzilla is not gonna be good enough. The Linux
kernel development has always been done via mailing lists. And while
there is a kernel and other Bugzillas, the main authority is still the
mailing list. So if you wanna get massive feedback or have testing
patches for debugging, then these need to be posted to linux-wireless
and not "hidden" into a Bugzilla.

Using Bugzilla for tracking is perfectly fine, but you have to treat
it as second citizen when it comes to upstream development. Reinette
is trying to bridge the gap between the Bugzilla by pointing people to
the patches in the Bugzilla when new reports come to the mailing list.
And this has to be done more actively. I prefer if we remove any
Bugzilla discussion for complicated issues to the mailing list.

If we really need it, then someone has to start a iwlwifi-debugging
kernel tree that derives from wireless-testing where all patches are
available via GIT and easy to review. Or ask John to create a iwlwifi-
debugging branch in wireless-testing. I am happy to run such tree. And
then use WARN() so I can just call kerneloops to push any stack traces.

>> Johannes has presented us with plausible fixes, and people are
>> reporting that the patches work for them. I can merge these patches,
>> or wait/hope/pray for some to come from your team. Experience
>> suggests
>> that if fixes do come from your team that they will either be buried
>> in some monster patch that largely addresses something else, or
>> that the patches will arrive with a changelog that is terse and/or
>> unintelligble. Subsequently, I am not optimistic about waiting.
>>
>> I think Johannes cited four different bugzilla.kernel.org entries
>> between those two patches. What is your team doing to address
>> those bugs?
>
> Johannes' effort is great and should certainly be praised. But looking
> down upon Intel's effort is nonsense. We suspected this alignment
> issue
> in the very beginning. But the set of users helped us to debug with
> this
> problem happened to have a different root cause than Johannes. Thus
> came
> with the alignment BUG_ON assert put into the code in case other
> people
> experience with it (unfortunately, it was a wrong one. As I had
> appologized for this already.)
>
> The problem should at least have two causes. Johannes had resolved one
> of them. The other one (AFAICS) relates to be in an 11n AP environment
> (no need to be associated with it) and takes longer time to reproduce.
> We will keep tracking it whatever we you think us about it.

I do have a perfect environment in Vancouver that allows me to trigger
this behavior within a few minutes. So if you really wanna debug this,
then just send debug patches to the mailing list and I am happy to run
such kernels. Only minor details is that you have to wait until the
end of December before I am back there ;)

Regards

Marcel

2008-11-24 06:33:16

by Zhu Yi

[permalink] [raw]
Subject: Re: pull request: wireless-2.6 2008-11-18

On Mon, 2008-11-24 at 11:50 +0800, Marcel Holtmann wrote:
> http://www.intellinuxwireless.org/bugzilla/show_bug.cgi?id=1703
>
> and just relying on Bugzilla is not gonna be good enough. The Linux
> kernel development has always been done via mailing lists. And while
> there is a kernel and other Bugzillas, the main authority is still the
> mailing list. So if you wanna get massive feedback or have testing
> patches for debugging, then these need to be posted to linux-wireless
> and not "hidden" into a Bugzilla.

This is generally true for bugs easy to debug with. But in this example,
the first reporter opened this bug 4 months ago. People usually don't
follow thread for such a long time. There are also some reporters never
show up in this ML.

> I do have a perfect environment in Vancouver that allows me to trigger
> this behavior within a few minutes. So if you really wanna debug this,
> then just send debug patches to the mailing list and I am happy to run
> such kernels.

Can you still reproduce this bug with Johannes' patch?

Thanks,
-yi

2008-11-24 08:34:03

by Marcel Holtmann

[permalink] [raw]
Subject: Re: pull request: wireless-2.6 2008-11-18

Hi Yi,

>> http://www.intellinuxwireless.org/bugzilla/show_bug.cgi?id=1703
>>
>> and just relying on Bugzilla is not gonna be good enough. The Linux
>> kernel development has always been done via mailing lists. And while
>> there is a kernel and other Bugzillas, the main authority is still
>> the
>> mailing list. So if you wanna get massive feedback or have testing
>> patches for debugging, then these need to be posted to linux-wireless
>> and not "hidden" into a Bugzilla.
>
> This is generally true for bugs easy to debug with. But in this
> example,
> the first reporter opened this bug 4 months ago. People usually don't
> follow thread for such a long time. There are also some reporters
> never
> show up in this ML.

true to some degree, but searching mailing list archives is a lot
simpler then bug reports.

>> I do have a perfect environment in Vancouver that allows me to
>> trigger
>> this behavior within a few minutes. So if you really wanna debug
>> this,
>> then just send debug patches to the mailing list and I am happy to
>> run
>> such kernels.
>
> Can you still reproduce this bug with Johannes' patch?

we will see once I am back in Vancouver. Will be some time in December
though.

Regards

Marcel