From patchwork Wed Nov 28 22:18:35 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Woodhouse X-Patchwork-Id: 202568 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 82B652C0085 for ; Thu, 29 Nov 2012 09:19:12 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932533Ab2K1WSy (ORCPT ); Wed, 28 Nov 2012 17:18:54 -0500 Received: from merlin.infradead.org ([205.233.59.134]:52816 "EHLO merlin.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755683Ab2K1WSw (ORCPT ); Wed, 28 Nov 2012 17:18:52 -0500 Received: from shinybook.infradead.org ([2001:8b0:10b:1:e6ce:8fff:fe1f:f2c0]) by merlin.infradead.org with esmtpsa (Exim 4.76 #1 (Red Hat Linux)) id 1Tdpxq-0005r2-AX; Wed, 28 Nov 2012 22:18:43 +0000 Message-ID: <1354141115.21562.101.camel@shinybook.infradead.org> Subject: Re: [PATCH v2 3/3] pppoatm: protect against freeing of vcc From: David Woodhouse To: David Laight Cc: chas williams - CONTRACTOR , Krzysztof Mazur , davem@davemloft.net, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, nathan@traverse.com.au Date: Wed, 28 Nov 2012 22:18:35 +0000 In-Reply-To: References: <1350926091-12642-1-git-send-email-krzysiek@podlesie.net> <1350926091-12642-3-git-send-email-krzysiek@podlesie.net> <1354036592.2534.6.camel@shinybook.infradead.org> <20121127173906.GA11390@shrek.podlesie.net> <1354039349.2534.11.camel@shinybook.infradead.org> <20121127135434.0728cd4f@thirdoffive.cmf.nrl.navy.mil> X-Mailer: Evolution 3.6.1 (3.6.1-1.fc18) Mime-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Wed, 2012-11-28 at 09:21 +0000, David Laight wrote: > Even when it might make sense to sleep in close until tx drains > there needs to be a finite timeout before it become abortive. You are, of course, right. We should never wait for hardware for ever. And just to serve me right, I seem to have hit a bug in the latest Solos firmware (1.11) which makes it sometimes lock up when I reboot. So it never responds to the PKT_PCLOSE packet... and thus it deadlocks when I try to kill pppd and unload the module to reset it :) New version... From 53dd01c08fec5b26006a009b25e4210127fdb27a Mon Sep 17 00:00:00 2001 From: David Woodhouse Date: Tue, 27 Nov 2012 23:49:24 +0000 Subject: [PATCH] solos-pci: Wait for pending TX to complete when releasing vcc We should no longer be calling the old pop routine for the vcc, after vcc_release() has completed. Make sure we wait for any pending TX skbs to complete, by waiting for our own PKT_PCLOSE control skb to be sent. Signed-off-by: David Woodhouse --- drivers/atm/solos-pci.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/drivers/atm/solos-pci.c b/drivers/atm/solos-pci.c index 9851093..3720670 100644 --- a/drivers/atm/solos-pci.c +++ b/drivers/atm/solos-pci.c @@ -92,6 +92,7 @@ struct pkt_hdr { }; struct solos_skb_cb { + struct completion c; struct atm_vcc *vcc; uint32_t dma_addr; }; @@ -881,11 +882,18 @@ static void pclose(struct atm_vcc *vcc) header->vci = cpu_to_le16(vcc->vci); header->type = cpu_to_le16(PKT_PCLOSE); + init_completion(&SKB_CB(skb)->c); + fpga_queue(card, SOLOS_CHAN(vcc->dev), skb, NULL); clear_bit(ATM_VF_ADDR, &vcc->flags); clear_bit(ATM_VF_READY, &vcc->flags); + if (!wait_for_completion_timeout(&SKB_CB(skb)->c, + jiffies + msecs_to_jiffies(5000))) + dev_warn(&card->dev->dev, "Timeout waiting for VCC close on port %d\n", + SOLOS_CHAN(vcc->dev)); + /* Hold up vcc_destroy_socket() (our caller) until solos_bh() in the tasklet has finished processing any incoming packets (and, more to the point, using the vcc pointer). */ @@ -1011,9 +1019,12 @@ static uint32_t fpga_tx(struct solos_card *card) if (vcc) { atomic_inc(&vcc->stats->tx); solos_pop(vcc, oldskb); - } else + } else { + struct pkt_hdr *header = (void *)oldskb->data; + if (le16_to_cpu(header->type) == PKT_PCLOSE) + complete(&SKB_CB(oldskb)->c); dev_kfree_skb_irq(oldskb); - + } } } /* For non-DMA TX, write the 'TX start' bit for all four ports simultaneously */ @@ -1345,6 +1356,8 @@ static struct pci_driver fpga_driver = { static int __init solos_pci_init(void) { + BUILD_BUG_ON(sizeof(struct solos_skb_cb) > sizeof(((struct sk_buff *)0)->cb)); + printk(KERN_INFO "Solos PCI Driver Version %s\n", VERSION); return pci_register_driver(&fpga_driver); }