2012-05-03 05:26:41

by David Miller

[permalink] [raw]
Subject: Re: [PATCH 2/2] tcp: cleanup tcp_try_coalesce

From: Eric Dumazet <[email protected]>
Date: Thu, 03 May 2012 07:19:33 +0200

> My last patch against iwlwifi is still waiting to make its way into
> official tree.
>
> http://www.spinics.net/lists/netdev/msg192629.html

John, please rectify this situation.

The Intel Wireless folks said they would test it, but that was more
than a month ago.

It's not acceptable to let bug fixes rot for that long, I don't care
what their special internal testing procedure is.

If they give you further pushback, please just ignore them and apply
Eric's fix directly.

Thank you.



2012-05-03 15:30:51

by Wey-Yi Guy

[permalink] [raw]
Subject: Re: [PATCH 2/2] tcp: cleanup tcp_try_coalesce

Hi John,

On Thu, 2012-05-03 at 11:14 -0400, John W. Linville wrote:
> On Thu, May 03, 2012 at 01:25:02AM -0400, David Miller wrote:
> > From: Eric Dumazet <[email protected]>
> > Date: Thu, 03 May 2012 07:19:33 +0200
> >
> > > My last patch against iwlwifi is still waiting to make its way into
> > > official tree.
> > >
> > > http://www.spinics.net/lists/netdev/msg192629.html
> >
> > John, please rectify this situation.
> >
> > The Intel Wireless folks said they would test it, but that was more
> > than a month ago.
> >
> > It's not acceptable to let bug fixes rot for that long, I don't care
> > what their special internal testing procedure is.
> >
> > If they give you further pushback, please just ignore them and apply
> > Eric's fix directly.
> >
> > Thank you.
>
> I imagine that this somehow got lost in the shuffle during the
> merge window. That doesn't excuse it, of course.
>
> It has waited long enough already, so I'll just go ahead and take it.
>
it is my mistake to lost this patch during the iwlwifi re-factor work,
the patch is no longer apply and I ask Eric to rebase the patch.

Sorry again for the mistake

Thanks
Wey



2012-05-03 15:17:54

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH 2/2] tcp: cleanup tcp_try_coalesce

On Thu, May 03, 2012 at 01:25:02AM -0400, David Miller wrote:
> From: Eric Dumazet <[email protected]>
> Date: Thu, 03 May 2012 07:19:33 +0200
>
> > My last patch against iwlwifi is still waiting to make its way into
> > official tree.
> >
> > http://www.spinics.net/lists/netdev/msg192629.html
>
> John, please rectify this situation.
>
> The Intel Wireless folks said they would test it, but that was more
> than a month ago.
>
> It's not acceptable to let bug fixes rot for that long, I don't care
> what their special internal testing procedure is.
>
> If they give you further pushback, please just ignore them and apply
> Eric's fix directly.
>
> Thank you.

I imagine that this somehow got lost in the shuffle during the
merge window. That doesn't excuse it, of course.

It has waited long enough already, so I'll just go ahead and take it.

John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.

2012-05-03 20:27:47

by Wey-Yi Guy

[permalink] [raw]
Subject: Re: [PATCH 2/2] tcp: cleanup tcp_try_coalesce

Hi John,

On Thu, 2012-05-03 at 13:07 -0400, John W. Linville wrote:
> On Thu, May 03, 2012 at 08:24:19AM -0700, Guy, Wey-Yi wrote:
> > Hi John,
> >
> > On Thu, 2012-05-03 at 11:14 -0400, John W. Linville wrote:
> > > On Thu, May 03, 2012 at 01:25:02AM -0400, David Miller wrote:
> > > > From: Eric Dumazet <[email protected]>
> > > > Date: Thu, 03 May 2012 07:19:33 +0200
> > > >
> > > > > My last patch against iwlwifi is still waiting to make its way into
> > > > > official tree.
> > > > >
> > > > > http://www.spinics.net/lists/netdev/msg192629.html
> > > >
> > > > John, please rectify this situation.
> > > >
> > > > The Intel Wireless folks said they would test it, but that was more
> > > > than a month ago.
> > > >
> > > > It's not acceptable to let bug fixes rot for that long, I don't care
> > > > what their special internal testing procedure is.
> > > >
> > > > If they give you further pushback, please just ignore them and apply
> > > > Eric's fix directly.
> > > >
> > > > Thank you.
> > >
> > > I imagine that this somehow got lost in the shuffle during the
> > > merge window. That doesn't excuse it, of course.
> > >
> > > It has waited long enough already, so I'll just go ahead and take it.
> > >
> > it is my mistake to lost this patch during the iwlwifi re-factor work,
> > the patch is no longer apply and I ask Eric to rebase the patch.
> >
> > Sorry again for the mistake
>
> Well, it seems like a fix needed for 3.4. And, the patch applies there.
>
> It does cause some merge breakage in wireless-testing (and presumably
> in linux-next). I'll attach the commit diff for the wireless-testing
> merge fixup I did, for review and/or as a peace offering to sfr... :-)
>
> Please take a look at the result in wireless-testing and let me know
> if it is broken...thanks!
>
Looks good to me, thanks very much for helping this.

Wey

>



2012-05-03 17:17:54

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH 2/2] tcp: cleanup tcp_try_coalesce

On Thu, May 03, 2012 at 08:24:19AM -0700, Guy, Wey-Yi wrote:
> Hi John,
>
> On Thu, 2012-05-03 at 11:14 -0400, John W. Linville wrote:
> > On Thu, May 03, 2012 at 01:25:02AM -0400, David Miller wrote:
> > > From: Eric Dumazet <[email protected]>
> > > Date: Thu, 03 May 2012 07:19:33 +0200
> > >
> > > > My last patch against iwlwifi is still waiting to make its way into
> > > > official tree.
> > > >
> > > > http://www.spinics.net/lists/netdev/msg192629.html
> > >
> > > John, please rectify this situation.
> > >
> > > The Intel Wireless folks said they would test it, but that was more
> > > than a month ago.
> > >
> > > It's not acceptable to let bug fixes rot for that long, I don't care
> > > what their special internal testing procedure is.
> > >
> > > If they give you further pushback, please just ignore them and apply
> > > Eric's fix directly.
> > >
> > > Thank you.
> >
> > I imagine that this somehow got lost in the shuffle during the
> > merge window. That doesn't excuse it, of course.
> >
> > It has waited long enough already, so I'll just go ahead and take it.
> >
> it is my mistake to lost this patch during the iwlwifi re-factor work,
> the patch is no longer apply and I ask Eric to rebase the patch.
>
> Sorry again for the mistake

Well, it seems like a fix needed for 3.4. And, the patch applies there.

It does cause some merge breakage in wireless-testing (and presumably
in linux-next). I'll attach the commit diff for the wireless-testing
merge fixup I did, for review and/or as a peace offering to sfr... :-)

Please take a look at the result in wireless-testing and let me know
if it is broken...thanks!

John

---

commit 22a101d22ad3296b55d87e92c4a94548aaf6f595
Merge: 20ef730 ed90542
Author: John W. Linville <[email protected]>
Date: Thu May 3 12:58:38 2012 -0400

Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless

Conflicts:
drivers/net/wireless/iwlwifi/iwl-agn-rx.c
drivers/net/wireless/iwlwifi/iwl-trans-pcie-rx.c
drivers/net/wireless/iwlwifi/iwl-trans.h

diff --cc drivers/net/wireless/iwlwifi/iwl-agn-rx.c
index f941223,2247460..ff758fe
--- a/drivers/net/wireless/iwlwifi/iwl-agn-rx.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn-rx.c
@@@ -752,15 -787,25 +751,25 @@@ static void iwlagn_pass_packet_to_mac80
iwlagn_set_decrypted_flag(priv, hdr, ampdu_status, stats))
return;

- skb = dev_alloc_skb(128);
+ /* Dont use dev_alloc_skb(), we'll have enough headroom once
+ * ieee80211_hdr pulled.
+ */
+ skb = alloc_skb(128, GFP_ATOMIC);
if (!skb) {
- IWL_ERR(priv, "dev_alloc_skb failed\n");
+ IWL_ERR(priv, "alloc_skb failed\n");
return;
}
+ hdrlen = min_t(unsigned int, len, skb_tailroom(skb));
+ memcpy(skb_put(skb, hdrlen), hdr, hdrlen);
+ fraglen = len - hdrlen;
+
+ if (fraglen) {
- int offset = (void *)hdr + hdrlen - rxb_addr(rxb);
++ int offset = (void *)hdr + hdrlen - rxb_addr(rxb)
++ + rxb_offset(rxb);

- offset = (void *)hdr - rxb_addr(rxb) + rxb_offset(rxb);
- p = rxb_steal_page(rxb);
- skb_add_rx_frag(skb, 0, p, offset, len, len);
+ skb_add_rx_frag(skb, 0, rxb_steal_page(rxb), offset,
+ fraglen, rxb->truesize);
+ }
- iwl_update_stats(priv, false, fc, len);

/*
* Wake any queues that were stopped due to a passive channel tx
diff --cc drivers/net/wireless/iwlwifi/iwl-trans-pcie-rx.c
index d2239aa,aa7aea1..08517d3
--- a/drivers/net/wireless/iwlwifi/iwl-trans-pcie-rx.c
+++ b/drivers/net/wireless/iwlwifi/iwl-trans-pcie-rx.c
@@@ -373,89 -374,72 +373,90 @@@ static void iwl_rx_handle_rxbuf(struct
if (WARN_ON(!rxb))
return;

- rxcb.truesize = PAGE_SIZE << hw_params(trans).rx_page_order;
- dma_unmap_page(trans->dev, rxb->page_dma,
- rxcb.truesize,
- DMA_FROM_DEVICE);
-
- rxcb._page = rxb->page;
- pkt = rxb_addr(&rxcb);
-
- IWL_DEBUG_RX(trans, "%s, 0x%02x\n",
- get_cmd_string(pkt->hdr.cmd), pkt->hdr.cmd);
+ dma_unmap_page(trans->dev, rxb->page_dma, max_len, DMA_FROM_DEVICE);

+ while (offset + sizeof(u32) + sizeof(struct iwl_cmd_header) < max_len) {
+ struct iwl_rx_packet *pkt;
+ struct iwl_device_cmd *cmd;
+ u16 sequence;
+ bool reclaim;
+ int index, cmd_index, err, len;
+ struct iwl_rx_cmd_buffer rxcb = {
+ ._offset = offset,
+ ._page = rxb->page,
+ ._page_stolen = false,
++ .truesize = max_len,
+ };

- len = le32_to_cpu(pkt->len_n_flags) & FH_RSCSR_FRAME_SIZE_MSK;
- len += sizeof(u32); /* account for status word */
- trace_iwlwifi_dev_rx(trans->dev, pkt, len);
+ pkt = rxb_addr(&rxcb);

- /* Reclaim a command buffer only if this packet is a response
- * to a (driver-originated) command.
- * If the packet (e.g. Rx frame) originated from uCode,
- * there is no command buffer to reclaim.
- * Ucode should set SEQ_RX_FRAME bit if ucode-originated,
- * but apparently a few don't get set; catch them here. */
- reclaim = !(pkt->hdr.sequence & SEQ_RX_FRAME);
- if (reclaim) {
- int i;
+ if (pkt->len_n_flags == cpu_to_le32(FH_RSCSR_FRAME_INVALID))
+ break;

- for (i = 0; i < trans_pcie->n_no_reclaim_cmds; i++) {
- if (trans_pcie->no_reclaim_cmds[i] == pkt->hdr.cmd) {
- reclaim = false;
- break;
+ IWL_DEBUG_RX(trans, "cmd at offset %d: %s (0x%.2x)\n",
+ rxcb._offset,
+ trans_pcie_get_cmd_string(trans_pcie, pkt->hdr.cmd),
+ pkt->hdr.cmd);
+
+ len = le32_to_cpu(pkt->len_n_flags) & FH_RSCSR_FRAME_SIZE_MSK;
+ len += sizeof(u32); /* account for status word */
+ trace_iwlwifi_dev_rx(trans->dev, pkt, len);
+
+ /* Reclaim a command buffer only if this packet is a response
+ * to a (driver-originated) command.
+ * If the packet (e.g. Rx frame) originated from uCode,
+ * there is no command buffer to reclaim.
+ * Ucode should set SEQ_RX_FRAME bit if ucode-originated,
+ * but apparently a few don't get set; catch them here. */
+ reclaim = !(pkt->hdr.sequence & SEQ_RX_FRAME);
+ if (reclaim) {
+ int i;
+
+ for (i = 0; i < trans_pcie->n_no_reclaim_cmds; i++) {
+ if (trans_pcie->no_reclaim_cmds[i] ==
+ pkt->hdr.cmd) {
+ reclaim = false;
+ break;
+ }
}
}
- }

- sequence = le16_to_cpu(pkt->hdr.sequence);
- index = SEQ_TO_INDEX(sequence);
- cmd_index = get_cmd_index(&txq->q, index);
+ sequence = le16_to_cpu(pkt->hdr.sequence);
+ index = SEQ_TO_INDEX(sequence);
+ cmd_index = get_cmd_index(&txq->q, index);

- if (reclaim)
- cmd = txq->cmd[cmd_index];
- else
- cmd = NULL;
+ if (reclaim)
+ cmd = txq->entries[cmd_index].cmd;
+ else
+ cmd = NULL;

- err = iwl_op_mode_rx(trans->op_mode, &rxcb, cmd);
+ err = iwl_op_mode_rx(trans->op_mode, &rxcb, cmd);

- /*
- * XXX: After here, we should always check rxcb._page
- * against NULL before touching it or its virtual
- * memory (pkt). Because some rx_handler might have
- * already taken or freed the pages.
- */
+ /*
+ * After here, we should always check rxcb._page_stolen,
+ * if it is true then one of the handlers took the page.
+ */

- if (reclaim) {
- /* Invoke any callbacks, transfer the buffer to caller,
- * and fire off the (possibly) blocking
- * iwl_trans_send_cmd()
- * as we reclaim the driver command queue */
- if (rxcb._page)
- iwl_tx_cmd_complete(trans, &rxcb, err);
- else
- IWL_WARN(trans, "Claim null rxb?\n");
+ if (reclaim) {
+ /* Invoke any callbacks, transfer the buffer to caller,
+ * and fire off the (possibly) blocking
+ * iwl_trans_send_cmd()
+ * as we reclaim the driver command queue */
+ if (!rxcb._page_stolen)
+ iwl_tx_cmd_complete(trans, &rxcb, err);
+ else
+ IWL_WARN(trans, "Claim null rxb?\n");
+ }
+
+ page_stolen |= rxcb._page_stolen;
+ offset += ALIGN(len, FH_RSCSR_FRAME_ALIGN);
}

- /* page was stolen from us */
- if (rxcb._page == NULL)
+ /* page was stolen from us -- free our reference */
+ if (page_stolen) {
+ __free_pages(rxb->page, trans_pcie->rx_page_order);
rxb->page = NULL;
+ }

/* Reuse the page if possible. For notification packets and
* SKBs that fail to Rx correctly, add them back into the
diff --cc drivers/net/wireless/iwlwifi/iwl-trans.h
index 7018d31,fdf9788..79a1e7a
--- a/drivers/net/wireless/iwlwifi/iwl-trans.h
+++ b/drivers/net/wireless/iwlwifi/iwl-trans.h
@@@ -256,8 -260,7 +256,9 @@@ static inline void iwl_free_resp(struc

struct iwl_rx_cmd_buffer {
struct page *_page;
+ int _offset;
+ bool _page_stolen;
+ unsigned int truesize;
};

static inline void *rxb_addr(struct iwl_rx_cmd_buffer *r)
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.