Re: [PATCH 2/2] tcp: cleanup tcp_try_coalesce

John W. Linville
2012-05-03T17:17:54Z

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 <eric.dumazet@gmail.com>
> > > 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 <linville@tuxdriver.com>
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
linville@tuxdriver.com might be all we have. Be ready.