Netdev List
 help / color / mirror / Atom feed
* You have been officially chosen by the Board  of Trustees of the Oxfam GB (NGO UK) as one of the final Recipients  of a Cash Grant/Donation of $850,000usd,provide Name,Age,Occuptaion,Nationality ,Country,Gender,Tel Number to redeem your donations sum      $850,000usd,
From: OXFAM GB UK @ 2009-08-02 15:28 UTC (permalink / raw)




^ permalink raw reply

* Re: [RFC] [PATCH] Avoid enqueuing skb for default qdiscs
From: Krishna Kumar2 @ 2009-08-02  8:51 UTC (permalink / raw)
  To: davem, herbert, jarkao2, kaber, netdev
In-Reply-To: <20090728155055.2266.41649.sendpatchset@localhost.localdomain>

Krishna Kumar2/India/IBM@IBMIN wrote on 07/28/2009 09:20:55 PM:

> Subject [RFC] [PATCH] Avoid enqueuing skb for default qdiscs
>
> From: Krishna Kumar <krkumar2@in.ibm.com>
>
> dev_queue_xmit enqueue's a skb and calls qdisc_run which
> dequeue's the skb and xmits it. In most cases (after
> instrumenting the code), the skb that is enqueue'd is the
> same one that is dequeue'd (unless the queue gets stopped
> or multiple cpu's write to the same queue and ends in a
> race with qdisc_run). For default qdiscs, we can remove
> this path and simply xmit the skb since this is a work
> conserving queue.

Any comments on this patch?

Thanks,

- KK

> The patch uses a new flag - TCQ_F_CAN_BYPASS to identify
> the default fast queue. I plan to use this flag for the
> previous patch also (rename if required).  The controversial
> part of the patch is incrementing qlen when a skb is
> requeued, this is to avoid checks like the second line below:
>
> +    } else if ((q->flags & TCQ_F_CAN_BYPASS) && !qdisc_qlen(q) &&
> >> THIS LINE:   !q->gso_skb &&
> +               !test_and_set_bit(__QDISC_STATE_RUNNING, &q->state)) {
>
> Results of a 4 hour testing for multiple netperf sessions
> (1, 2, 4, 8, 12 sessions on a 4 cpu system-X and 1, 2, 4,
> 8, 16, 32 sessions on a 16 cpu P6). Aggregate Mb/s across
> the iterations:
>
> -----------------------------------------------------------------
>      |         System-X          |               P6
> -----------------------------------------------------------------
> Size |  ORG BW          NEW BW   |      ORG BW          NEW BW
> -----|---------------------------|-------------------------------
> 16K  |  154264          156234   |      155350          157569
> 64K  |  154364          154825   |      155790          158845
> 128K |  154644          154803   |      153418          155572
> 256K |  153882          152007   |      154784          154596
> -----------------------------------------------------------------
>
> Netperf reported Service demand reduced by 15% on the P6 but
> no noticeable difference on the system-X box.
>
> Please review.
>
> Thanks,
>
> - KK


^ permalink raw reply

* [PATCH 12/15] drivers/atm: Use DIV_ROUND_CLOSEST
From: Julia Lawall @ 2009-08-02  8:48 UTC (permalink / raw)
  To: chas, linux-atm-general, netdev, linux-kernel, kernel-janitors

From: Julia Lawall <julia@diku.dk>

The kernel.h macro DIV_ROUND_CLOSEST performs the computation (x + d/2)/d
but is perhaps more readable.

The semantic patch that makes this change is as follows:
(http://www.emn.fr/x-info/coccinelle/)

// <smpl>
@haskernel@
@@

#include <linux/kernel.h>

@depends on haskernel@
expression x,__divisor;
@@

- (((x) + ((__divisor) / 2)) / (__divisor))
+ DIV_ROUND_CLOSEST(x,__divisor)
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 drivers/atm/horizon.c               |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/atm/horizon.c b/drivers/atm/horizon.c
index 6b969f8..01ce241 100644
--- a/drivers/atm/horizon.c
+++ b/drivers/atm/horizon.c
@@ -641,7 +641,7 @@ static int make_rate (const hrz_dev * dev, u32 c, rounding r,
 					pre = 1;
 				break;
 			case round_nearest:
-				pre = (br+(c<<div)/2)/(c<<div);
+				pre = DIV_ROUND_CLOSEST(br, c<<div);
 				// but p must be non-zero
 				if (!pre)
 					pre = 1;
@@ -671,7 +671,7 @@ static int make_rate (const hrz_dev * dev, u32 c, rounding r,
 					pre = DIV_ROUND_UP(br, c<<div);
 					break;
 				case round_nearest:
-					pre = (br+(c<<div)/2)/(c<<div);
+					pre = DIV_ROUND_CLOSEST(br, c<<div);
 					break;
 				default: /* round_up */
 					pre = br/(c<<div);

^ permalink raw reply related

* [PATCH] netxen: fix coherent dma mask setting
From: Dhananjay Phadke @ 2009-08-02  7:36 UTC (permalink / raw)
  To: davem; +Cc: netdev, Dhananjay Phadke

Change default dma mask for NX3031 to 39 bit with ability
to update it to 64-bit (if firmware indicates support). Old
code was restricting it under 4GB (32-bit), sometimes causing
failure to allocate descriptor rings on heavily populated
system. NX2031 based NICs will still get 32-bit coherent mask.

Signed-off-by: Dhananjay Phadke <dhananjay@netxen.com>
---
 drivers/net/netxen/netxen_nic_main.c |   37 +++++++++++++++++++++++-----------
 1 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/drivers/net/netxen/netxen_nic_main.c b/drivers/net/netxen/netxen_nic_main.c
index 637ac8b..3cd8cfc 100644
--- a/drivers/net/netxen/netxen_nic_main.c
+++ b/drivers/net/netxen/netxen_nic_main.c
@@ -221,7 +221,7 @@ netxen_napi_disable(struct netxen_adapter *adapter)
 	}
 }
 
-static int nx_set_dma_mask(struct netxen_adapter *adapter, uint8_t revision_id)
+static int nx_set_dma_mask(struct netxen_adapter *adapter)
 {
 	struct pci_dev *pdev = adapter->pdev;
 	uint64_t mask, cmask;
@@ -229,19 +229,17 @@ static int nx_set_dma_mask(struct netxen_adapter *adapter, uint8_t revision_id)
 	adapter->pci_using_dac = 0;
 
 	mask = DMA_BIT_MASK(32);
-	/*
-	 * Consistent DMA mask is set to 32 bit because it cannot be set to
-	 * 35 bits. For P3 also leave it at 32 bits for now. Only the rings
-	 * come off this pool.
-	 */
 	cmask = DMA_BIT_MASK(32);
 
+	if (NX_IS_REVISION_P2(adapter->ahw.revision_id)) {
 #ifndef CONFIG_IA64
-	if (revision_id >= NX_P3_B0)
-		mask = DMA_BIT_MASK(39);
-	else if (revision_id == NX_P2_C1)
 		mask = DMA_BIT_MASK(35);
 #endif
+	} else {
+		mask = DMA_BIT_MASK(39);
+		cmask = mask;
+	}
+
 	if (pci_set_dma_mask(pdev, mask) == 0 &&
 		pci_set_consistent_dma_mask(pdev, cmask) == 0) {
 		adapter->pci_using_dac = 1;
@@ -256,7 +254,7 @@ static int
 nx_update_dma_mask(struct netxen_adapter *adapter)
 {
 	int change, shift, err;
-	uint64_t mask, old_mask;
+	uint64_t mask, old_mask, old_cmask;
 	struct pci_dev *pdev = adapter->pdev;
 
 	change = 0;
@@ -272,14 +270,29 @@ nx_update_dma_mask(struct netxen_adapter *adapter)
 
 	if (change) {
 		old_mask = pdev->dma_mask;
+		old_cmask = pdev->dev.coherent_dma_mask;
+
 		mask = (1ULL<<(32+shift)) - 1;
 
 		err = pci_set_dma_mask(pdev, mask);
 		if (err)
-			return pci_set_dma_mask(pdev, old_mask);
+			goto err_out;
+
+		if (NX_IS_REVISION_P3(adapter->ahw.revision_id)) {
+
+			err = pci_set_consistent_dma_mask(pdev, mask);
+			if (err)
+				goto err_out;
+		}
+		dev_info(&pdev->dev, "using %d-bit dma mask\n", 32+shift);
 	}
 
 	return 0;
+
+err_out:
+	pci_set_dma_mask(pdev, old_mask);
+	pci_set_consistent_dma_mask(pdev, old_cmask);
+	return err;
 }
 
 static void netxen_check_options(struct netxen_adapter *adapter)
@@ -1006,7 +1019,7 @@ netxen_nic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	revision_id = pdev->revision;
 	adapter->ahw.revision_id = revision_id;
 
-	err = nx_set_dma_mask(adapter, revision_id);
+	err = nx_set_dma_mask(adapter);
 	if (err)
 		goto err_out_free_netdev;
 
-- 
1.6.0.2


^ permalink raw reply related

* [PATCH] tulip: Read buffer overflow
From: Roel Kluin @ 2009-08-02  6:26 UTC (permalink / raw)
  To: grundler, kyle, netdev, Andrew Morton

Check whether index is within bounds before testing the element.

Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
diff --git a/drivers/net/tulip/de4x5.c b/drivers/net/tulip/de4x5.c
index eb72d2e..acfdccd 100644
--- a/drivers/net/tulip/de4x5.c
+++ b/drivers/net/tulip/de4x5.c
@@ -5059,7 +5059,7 @@ mii_get_phy(struct net_device *dev)
 	if ((id == 0) || (id == 65535)) continue;  /* Valid ID? */
 	for (j=0; j<limit; j++) {                  /* Search PHY table */
 	    if (id != phy_info[j].id) continue;    /* ID match? */
-	    for (k=0; lp->phy[k].id && (k < DE4X5_MAX_PHY); k++);
+	    for (k=0; k < DE4X5_MAX_PHY && lp->phy[k].id; k++);
 	    if (k < DE4X5_MAX_PHY) {
 		memcpy((char *)&lp->phy[k],
 		       (char *)&phy_info[j], sizeof(struct phy_table));
@@ -5072,7 +5072,7 @@ mii_get_phy(struct net_device *dev)
 	    break;
 	}
 	if ((j == limit) && (i < DE4X5_MAX_MII)) {
-	    for (k=0; lp->phy[k].id && (k < DE4X5_MAX_PHY); k++);
+	    for (k=0; k < DE4X5_MAX_PHY && lp->phy[k].id; k++);
 	    lp->phy[k].addr = i;
 	    lp->phy[k].id = id;
 	    lp->phy[k].spd.reg = GENERIC_REG;      /* ANLPA register         */
@@ -5091,7 +5091,7 @@ mii_get_phy(struct net_device *dev)
   purgatory:
     lp->active = 0;
     if (lp->phy[0].id) {                           /* Reset the PHY devices */
-	for (k=0; lp->phy[k].id && (k < DE4X5_MAX_PHY); k++) { /*For each PHY*/
+	for (k=0; k < DE4X5_MAX_PHY && lp->phy[k].id; k++) { /*For each PHY*/
 	    mii_wr(MII_CR_RST, MII_CR, lp->phy[k].addr, DE4X5_MII);
 	    while (mii_rd(MII_CR, lp->phy[k].addr, DE4X5_MII) & MII_CR_RST);
 

^ permalink raw reply related

* [PATCH] s6gmac: Read buffer overflow
From: Roel Kluin @ 2009-08-02  6:20 UTC (permalink / raw)
  To: davem, netdev, Andrew Morton

Check whether index is within bounds before testing the element.
In the last iteration i is PHY_MAX_ADDR. the condition
`!(p = pd->mii.bus->phy_map[PHY_MAX_ADDR])' is undefined and may
evaluate to false, which leads to a dereference of this invalid
phy_map in the phy_connect() below.

Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
diff --git a/drivers/net/s6gmac.c b/drivers/net/s6gmac.c
index 5345e47..4525cbe 100644
--- a/drivers/net/s6gmac.c
+++ b/drivers/net/s6gmac.c
@@ -793,7 +793,7 @@ static inline int s6gmac_phy_start(struct net_device *dev)
 	struct s6gmac *pd = netdev_priv(dev);
 	int i = 0;
 	struct phy_device *p = NULL;
-	while ((!(p = pd->mii.bus->phy_map[i])) && (i < PHY_MAX_ADDR))
+	while ((i < PHY_MAX_ADDR) && (!(p = pd->mii.bus->phy_map[i])))
 		i++;
 	p = phy_connect(dev, dev_name(&p->dev), &s6gmac_adjust_link, 0,
 			PHY_INTERFACE_MODE_RGMII);

^ permalink raw reply related

* Re: [net-next-2.6 PATCH 0/9] return PCI_ERS_RESULT_DISCONNECT on permanent failure
From: David Miller @ 2009-08-02  6:13 UTC (permalink / raw)
  To: dnelson; +Cc: netdev
In-Reply-To: <20090731191254.5470.99642.send-patch@aqua>

From: Dean Nelson <dnelson@redhat.com>
Date: Fri, 31 Jul 2009 15:12:55 -0400

> PCI drivers that implement the struct pci_error_handlers' error_detected
> callback should return PCI_ERS_RESULT_DISCONNECT if the state passed in is
> pci_channel_io_perm_failure.
 ...
> This patchset is an attempt to address the remaining net drivers of this type.
> None of the patches in this patchset have been tested, since I don't have the
> hardware to test any of them. I'm simply making these patches available for
> whomever is interested and able to test them, to do so. Perhaps the individual
> patches can be committed as they get tested, reviewed and ACK'd.

I think these are safe to apply, they look so straightforward
to me.  I've thus added them all to net-next-2.6, thanks!

^ permalink raw reply

* Re: [net-next-2.6 PATCH 5/5] be2net: delete unnecessary code from be_cmd_POST()
From: David Miller @ 2009-08-02  6:08 UTC (permalink / raw)
  To: sathyap; +Cc: netdev
In-Reply-To: <20090728085330.GA5025@serverengines.com>

From: Sathya Perla <sathyap@serverengines.com>
Date: Tue, 28 Jul 2009 14:23:30 +0530

> When the driver loads the POST stage of the card is expected to be
> POST_STAGE_ARMFW_RDY.
> 
> Signed-off-by: Sathya Perla <sathyap@serverengines.com>

Applied.

^ permalink raw reply

* Re: [net-next-2.6 PATCH 4/5] be2net: some name changes for consistency
From: David Miller @ 2009-08-02  6:08 UTC (permalink / raw)
  To: sathyap; +Cc: netdev
In-Reply-To: <20090728085310.GA5009@serverengines.com>

From: Sathya Perla <sathyap@serverengines.com>
Date: Tue, 28 Jul 2009 14:23:10 +0530

> be_mbox_db_ring() has been changed to be_mbox_notify() (to be consistent with
> be_mcc_notify()) and struct be_mcc_cq_entry changed to be_mcc_compl
> to be consistent with rx/tx_compl.
> 
> Signed-off-by: Sathya Perla <sathyap@serverengines.com>

Applied.

^ permalink raw reply

* Re: [net-next-2.6 PATCH 3/5] be2net: replace some printks with dev_err()/dev_warn()
From: David Miller @ 2009-08-02  6:07 UTC (permalink / raw)
  To: sathyap; +Cc: netdev
In-Reply-To: <20090728085256.GA4993@serverengines.com>

From: Sathya Perla <sathyap@serverengines.com>
Date: Tue, 28 Jul 2009 14:22:56 +0530

> And get rid of some unnecessary printks.
> 
> Signed-off-by: Sathya Perla <sathyap@serverengines.com>

Applied.

^ permalink raw reply

* Re: [net-next-2.6 PATCH 2/5] be2net: remove pci_func field from be_adapter struct
From: David Miller @ 2009-08-02  6:07 UTC (permalink / raw)
  To: sathyap; +Cc: netdev
In-Reply-To: <20090728085223.GA4977@serverengines.com>

From: Sathya Perla <sathyap@serverengines.com>
Date: Tue, 28 Jul 2009 14:22:23 +0530

> pci function is available as PCI_FUNC(pdev->devfn); no need for a
> separate field.
> 
> Signed-off-by: Sathya Perla <sathyap@serverengines.com>

Applied.

^ permalink raw reply

* Re: [PATCH 1/5] be2net: get rid of be_ctrl_info struct/abstraction
From: David Miller @ 2009-08-02  6:07 UTC (permalink / raw)
  To: sathyap; +Cc: netdev
In-Reply-To: <20090728084408.GA4409@serverengines.com>

From: Sathya Perla <sathyap@serverengines.com>
Date: Tue, 28 Jul 2009 14:14:08 +0530

> Currently only the be_ctrl_info struct ptr (instead of adapter) is
>  passed to all the routines in be_cmds.c. Instead pass be_adapter
> ptr to allow access to all its fields. Merge the contents of struct be_ctrl_info
> into be_adapter. The resulting code is simpler.
> 
> Signed-off-by: Sathya Perla <sathyap@serverengines.com>

Applied.

^ permalink raw reply

* [PATCH v2 2.6.31-rc4] gigaset: really fix chars_in_buffer
From: Tilman Schmidt @ 2009-08-02  0:36 UTC (permalink / raw)
  To: davem, linux-kernel, netdev; +Cc: Alan Cox, Hansjoerg Lipp
In-Reply-To: <20090801-patch-01.tilman@imap.cc>

The tty_operation chars_in_buffer() is not allowed to return a
negative value to signal an error. Corrects the problem flagged
by commit 23198fda7182969b619613a555f8645fdc3dc334,
"tty: fix chars_in_buffers".

Impact: error handling bugfix
Signed-off-by: Tilman Schmidt <tilman@imap.cc>
---
 drivers/isdn/gigaset/interface.c |   19 +++++++------------
 1 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/isdn/gigaset/interface.c b/drivers/isdn/gigaset/interface.c
index 8ff7e35..f33ac27 100644
--- a/drivers/isdn/gigaset/interface.c
+++ b/drivers/isdn/gigaset/interface.c
@@ -408,33 +408,28 @@ static int if_write_room(struct tty_struct *tty)
 	return retval;
 }
 
-/* FIXME: This function does not have error returns */
-
 static int if_chars_in_buffer(struct tty_struct *tty)
 {
 	struct cardstate *cs;
-	int retval = -ENODEV;
+	int retval = 0;
 
 	cs = (struct cardstate *) tty->driver_data;
 	if (!cs) {
 		pr_err("%s: no cardstate\n", __func__);
-		return -ENODEV;
+		return 0;
 	}
 
 	gig_dbg(DEBUG_IF, "%u: %s()", cs->minor_index, __func__);
 
-	if (mutex_lock_interruptible(&cs->mutex))
-		return -ERESTARTSYS; // FIXME -EINTR?
+	mutex_lock(&cs->mutex);
 
-	if (!cs->connected) {
+	if (!cs->connected)
 		gig_dbg(DEBUG_IF, "not connected");
-		retval = -ENODEV;
-	} else if (!cs->open_count)
+	else if (!cs->open_count)
 		dev_warn(cs->dev, "%s: device not opened\n", __func__);
-	else if (cs->mstate != MS_LOCKED) {
+	else if (cs->mstate != MS_LOCKED)
 		dev_warn(cs->dev, "can't write to unlocked device\n");
-		retval = -EBUSY;
-	} else
+	else
 		retval = cs->ops->chars_in_buffer(cs);
 
 	mutex_unlock(&cs->mutex);
-- 
1.6.2.1.214.ge986c

^ permalink raw reply related

* Re: [PATCH 2.6.31-rc4] gigaset: really fix chars_in_buffer
From: Tilman Schmidt @ 2009-08-01 23:37 UTC (permalink / raw)
  To: Alan Cox; +Cc: davem, linux-kernel, netdev, Alan Cox, Hansjoerg Lipp
In-Reply-To: <20090801171141.7a463347@lxorguk.ukuu.org.uk>

[-- Attachment #1: Type: text/plain, Size: 1452 bytes --]

Hi Alan,

thanks a lot for getting in contact with me, and for your
explanation of the rationale behind your patch.

On Sat, 1 Aug 2009 17:11:41 +0100, Alan Cox wrote:
> Is that correct. If you take a signal so the mutex_lock_interruptible
> takes the error path we'll exit any waits for characters to be flushed
> from buffers and lose the bytes in some cases ?

You are right. I'll have to change it to use non-interruptible
mutex_lock. I'll prepare a new patch.

> Thats why I didn't
> convert it - I didn't understand why it was using _interruptible at all ?

No specific reason. just the general rule to prefer sleeping
interuptibly whenever possible, because unkillable processes are
so annoying. Seeing that chars_in_buffer was declared as returning
int, I assumed (wrongly as it now turns out) that it would be
allowed to return the customary negative values for errors, so
that it could use the _interruptible variant. When I saw your
patch I read into it that zero would be the appropriate return
value for all error cases, including a signal while waiting for
the mutex. I guess there was a bit of wishful thinking in that.

Well, thanks again for your help. I'll follow up with a revised
patch.

Regards,
Tilman

-- 
Tilman Schmidt                    E-Mail: tilman@imap.cc
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Ungeöffnet mindestens haltbar bis: (siehe Rückseite)


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 254 bytes --]

^ permalink raw reply

* (unknown)
From: Marc Fiuczynski @ 2009-08-01 21:21 UTC (permalink / raw)
  To: netdev@vger.kernel.org



^ permalink raw reply

* [PATCH 5/5] drivers/net: Use DIV_ROUND_CLOSEST
From: Julia Lawall @ 2009-08-01 19:51 UTC (permalink / raw)
  To: netdev, linux-kernel, kernel-janitors

From: Julia Lawall <julia@diku.dk>

The kernel.h macro DIV_ROUND_CLOSEST performs the computation (x + d/2)/d
but is perhaps more readable.

The semantic patch that makes this change is as follows:
(http://www.emn.fr/x-info/coccinelle/)

// <smpl>
@haskernel@
@@

#include <linux/kernel.h>

@depends on haskernel@
expression x,__divisor;
@@

- (((x) + ((__divisor) / 2)) / (__divisor))
+ DIV_ROUND_CLOSEST(x,__divisor)
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 drivers/net/b44.c                   |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/net/b44.c b/drivers/net/b44.c
index 1f7f015..07c92f3 100644
--- a/drivers/net/b44.c
+++ b/drivers/net/b44.c
@@ -1297,7 +1297,8 @@ static void b44_chip_reset(struct b44 *bp, int reset_kind)
 	switch (sdev->bus->bustype) {
 	case SSB_BUSTYPE_SSB:
 		bw32(bp, B44_MDIO_CTRL, (MDIO_CTRL_PREAMBLE |
-		     (((ssb_clockspeed(sdev->bus) + (B44_MDC_RATIO / 2)) / B44_MDC_RATIO)
+		     (DIV_ROUND_CLOSEST(ssb_clockspeed(sdev->bus),
+					B44_MDC_RATIO)
 		     & MDIO_CTRL_MAXF_MASK)));
 		break;
 	case SSB_BUSTYPE_PCI:

^ permalink raw reply related

* [PATCH 4/5] drivers/net/wireless: Use DIV_ROUND_CLOSEST
From: Julia Lawall @ 2009-08-01 19:50 UTC (permalink / raw)
  To: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA

From: Julia Lawall <julia-dAYI7NvHqcQ@public.gmane.org>

The kernel.h macro DIV_ROUND_CLOSEST performs the computation (x + d/2)/d
but is perhaps more readable.

The semantic patch that makes this change is as follows:
(http://www.emn.fr/x-info/coccinelle/)

// <smpl>
@haskernel@
@@

#include <linux/kernel.h>

@depends on haskernel@
expression x,__divisor;
@@

- (((x) + ((__divisor) / 2)) / (__divisor))
+ DIV_ROUND_CLOSEST(x,__divisor)
// </smpl>

Signed-off-by: Julia Lawall <julia-dAYI7NvHqcQ@public.gmane.org>

---
 drivers/net/wireless/strip.c        |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/strip.c b/drivers/net/wireless/strip.c
index 73300c2..8eabb56 100644
--- a/drivers/net/wireless/strip.c
+++ b/drivers/net/wireless/strip.c
@@ -1550,9 +1550,12 @@ static int strip_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	if (time_after(jiffies, strip_info->pps_timer + HZ)) {
 		unsigned long t = jiffies - strip_info->pps_timer;
-		unsigned long rx_pps_count = (strip_info->rx_pps_count * HZ * 8 + t / 2) / t;
-		unsigned long tx_pps_count = (strip_info->tx_pps_count * HZ * 8 + t / 2) / t;
-		unsigned long sx_pps_count = (strip_info->sx_pps_count * HZ * 8 + t / 2) / t;
+		unsigned long rx_pps_count =
+			DIV_ROUND_CLOSEST(strip_info->rx_pps_count*HZ*8, t);
+		unsigned long tx_pps_count =
+			DIV_ROUND_CLOSEST(strip_info->tx_pps_count*HZ*8, t);
+		unsigned long sx_pps_count =
+			DIV_ROUND_CLOSEST(strip_info->sx_pps_count*HZ*8, t);
 
 		strip_info->pps_timer = jiffies;
 		strip_info->rx_pps_count = 0;
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* Re: [PATCH 2.6.31-rc4] gigaset: really fix chars_in_buffer
From: Alan Cox @ 2009-08-01 16:11 UTC (permalink / raw)
  To: Tilman Schmidt; +Cc: davem, linux-kernel, netdev, Alan Cox, Hansjoerg Lipp
In-Reply-To: <20090801-patch-01.tilman@imap.cc>

On Sat,  1 Aug 2009 14:18:51 +0200 (CEST)
Tilman Schmidt <tilman@imap.cc> wrote:

> This patch corrects the problem noted but left unfixed by commit
> 23198fda7182969b619613a555f8645fdc3dc334 "tty: fix chars_in_buffers".
> The tty_operation chars_in_buffer() should return zero, not a
> negative value, in the error case.

Is that correct. If you take a signal so the mutex_lock_interruptible
takes the error path we'll exit any waits for characters to be flushed
from buffers and lose the bytes in some cases ? Thats why I didn't
convert it - I didn't understand why it was using _interruptible at all ?

Alan

^ permalink raw reply

* [PATCH 2.6.31-rc4] gigaset: really fix chars_in_buffer
From: Tilman Schmidt @ 2009-08-01 12:18 UTC (permalink / raw)
  To: davem, linux-kernel, netdev; +Cc: Alan Cox, Hansjoerg Lipp

This patch corrects the problem noted but left unfixed by commit
23198fda7182969b619613a555f8645fdc3dc334 "tty: fix chars_in_buffers".
The tty_operation chars_in_buffer() should return zero, not a
negative value, in the error case.

Impact: error handling bugfix
Signed-off-by: Tilman Schmidt <tilman@imap.cc>
---

I only just noticed the "tty: fix chars_in_buffers" patch going in.
Please merge this follow-up fix for 2.6.31, too. Thanks.

 drivers/isdn/gigaset/interface.c |   18 +++++++-----------
 1 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/isdn/gigaset/interface.c b/drivers/isdn/gigaset/interface.c
index 8b62df8..dfeec68 100644
--- a/drivers/isdn/gigaset/interface.c
+++ b/drivers/isdn/gigaset/interface.c
@@ -408,33 +408,29 @@ static int if_write_room(struct tty_struct *tty)
 	return retval;
 }
 
-/* FIXME: This function does not have error returns */
-
 static int if_chars_in_buffer(struct tty_struct *tty)
 {
 	struct cardstate *cs;
-	int retval = -ENODEV;
+	int retval = 0;
 
 	cs = (struct cardstate *) tty->driver_data;
 	if (!cs) {
 		pr_err("%s: no cardstate\n", __func__);
-		return -ENODEV;
+		return 0;
 	}
 
 	gig_dbg(DEBUG_IF, "%u: %s()", cs->minor_index, __func__);
 
 	if (mutex_lock_interruptible(&cs->mutex))
-		return -ERESTARTSYS; // FIXME -EINTR?
+		return 0;
 
-	if (!cs->connected) {
+	if (!cs->connected)
 		gig_dbg(DEBUG_IF, "not connected");
-		retval = -ENODEV;
-	} else if (!cs->open_count)
+	else if (!cs->open_count)
 		dev_warn(cs->dev, "%s: device not opened\n", __func__);
-	else if (cs->mstate != MS_LOCKED) {
+	else if (cs->mstate != MS_LOCKED)
 		dev_warn(cs->dev, "can't write to unlocked device\n");
-		retval = -EBUSY;
-	} else
+	else
 		retval = cs->ops->chars_in_buffer(cs);
 
 	mutex_unlock(&cs->mutex);
-- 
1.6.2.1.214.ge986c

^ permalink raw reply related

* Re: tty_register_device NULL pointer dereference in 2.6.31-rc4
From: Dave Young @ 2009-08-01  9:32 UTC (permalink / raw)
  To: Oliver Hartkopp
  Cc: Alan Cox, Marcel Holtmann, Linux Netdev List, linux-bluetooth
In-Reply-To: <4A740918.1040504@hartkopp.net>

On Sat, Aug 01, 2009 at 11:21:28AM +0200, Oliver Hartkopp wrote:
> 
> > --- linux-2.6.orig/include/net/bluetooth/rfcomm.h	2009-08-01 10:53:18.000000000 +0800
> > +++ linux-2.6/include/net/bluetooth/rfcomm.h	2009-08-01 10:55:29.000000000 +0800
> > @@ -355,7 +355,18 @@ struct rfcomm_dev_list_req {
> >  };
> >  
> >  int  rfcomm_dev_ioctl(struct sock *sk, unsigned int cmd, void __user *arg);
> > +
> > +#ifdef CONFIG_BT_RFCOMM_TTY
> >  int  rfcomm_init_ttys(void);
> >  void rfcomm_cleanup_ttys(void);
> > -
> > +#else
> > +static inline int rfcomm_init_ttys(void)
> > +{
> > +	return 0;
> > +}
> > +static inline int rfcomm_cleanup_ttys(void)
> > +{
> > +	return 0;
> > +}
> > +#endif
> 
> Just a minor thing:
> 
> Does rfcomm_cleanup_ttys() return 'int' or is it 'void' ?

Yes it should be void. Thanks for pointing out, here update the patch:
---

rfcomm tty may be used before rfcomm_tty_driver initilized,
The problem is that now socket layer init before tty layer, if userspace
program do socket callback right here then oops will happen.

reporting in:
http://marc.info/?l=linux-bluetooth&m=124404919324542&w=2

make 3 changes:
1. remove #ifdef in rfcomm/core.c,
make it blank function when rfcomm tty not selected in rfcomm.h

2. tune the rfcomm_init error patch to ensure
tty driver initilized before rfcomm socket usage.

3. remove __exit for rfcomm_cleanup_sockets
because above change need call it in a __init function. 


CC: Alan Cox <alan@lxorguk.ukuu.org.uk>
Reported-by: Oliver Hartkopp <oliver@hartkopp.net>
Signed-off-by: Dave Young <hidave.darkstar@gmail.com>
--
include/net/bluetooth/rfcomm.h |   12 +++++++++++-
net/bluetooth/rfcomm/core.c    |   29 ++++++++++++++++++++---------
net/bluetooth/rfcomm/sock.c    |    2 +-
3 files changed, 32 insertions(+), 11 deletions(-)

--- linux-2.6.orig/include/net/bluetooth/rfcomm.h	2009-08-01 13:56:53.000000000 +0800
+++ linux-2.6/include/net/bluetooth/rfcomm.h	2009-08-01 17:24:59.000000000 +0800
@@ -355,7 +355,17 @@ struct rfcomm_dev_list_req {
 };
 
 int  rfcomm_dev_ioctl(struct sock *sk, unsigned int cmd, void __user *arg);
+
+#ifdef CONFIG_BT_RFCOMM_TTY
 int  rfcomm_init_ttys(void);
 void rfcomm_cleanup_ttys(void);
-
+#else
+static inline int rfcomm_init_ttys(void)
+{
+	return 0;
+}
+static inline void rfcomm_cleanup_ttys(void)
+{
+}
+#endif
 #endif /* __RFCOMM_H */
--- linux-2.6.orig/net/bluetooth/rfcomm/core.c	2009-08-01 13:56:53.000000000 +0800
+++ linux-2.6/net/bluetooth/rfcomm/core.c	2009-08-01 13:57:18.000000000 +0800
@@ -2080,28 +2080,41 @@ static CLASS_ATTR(rfcomm_dlc, S_IRUGO, r
 /* ---- Initialization ---- */
 static int __init rfcomm_init(void)
 {
+	int ret;
+
 	l2cap_load();
 
 	hci_register_cb(&rfcomm_cb);
 
 	rfcomm_thread = kthread_run(rfcomm_run, NULL, "krfcommd");
 	if (IS_ERR(rfcomm_thread)) {
-		hci_unregister_cb(&rfcomm_cb);
-		return PTR_ERR(rfcomm_thread);
+		ret = PTR_ERR(rfcomm_thread);
+		goto out_thread;
 	}
 
 	if (class_create_file(bt_class, &class_attr_rfcomm_dlc) < 0)
 		BT_ERR("Failed to create RFCOMM info file");
 
-	rfcomm_init_sockets();
-
-#ifdef CONFIG_BT_RFCOMM_TTY
-	rfcomm_init_ttys();
-#endif
+	ret = rfcomm_init_ttys();
+	if (ret)
+		goto out_tty;
+
+	ret = rfcomm_init_sockets();
+	if (ret)
+		goto out_sock;
 
 	BT_INFO("RFCOMM ver %s", VERSION);
 
 	return 0;
+
+out_sock:
+	rfcomm_cleanup_ttys();
+out_tty:
+	kthread_stop(rfcomm_thread);
+out_thread:
+	hci_unregister_cb(&rfcomm_cb);
+
+	return ret;
 }
 
 static void __exit rfcomm_exit(void)
@@ -2112,9 +2125,7 @@ static void __exit rfcomm_exit(void)
 
 	kthread_stop(rfcomm_thread);
 
-#ifdef CONFIG_BT_RFCOMM_TTY
 	rfcomm_cleanup_ttys();
-#endif
 
 	rfcomm_cleanup_sockets();
 }
--- linux-2.6.orig/net/bluetooth/rfcomm/sock.c	2009-08-01 13:56:53.000000000 +0800
+++ linux-2.6/net/bluetooth/rfcomm/sock.c	2009-08-01 13:57:18.000000000 +0800
@@ -1132,7 +1132,7 @@ error:
 	return err;
 }
 
-void __exit rfcomm_cleanup_sockets(void)
+void rfcomm_cleanup_sockets(void)
 {
 	class_remove_file(bt_class, &class_attr_rfcomm);
 

^ permalink raw reply

* Re: tty_register_device NULL pointer dereference in 2.6.31-rc4
From: Oliver Hartkopp @ 2009-08-01  9:21 UTC (permalink / raw)
  To: Dave Young
  Cc: Alan Cox, Marcel Holtmann, Linux Netdev List,
	linux-bluetooth-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20090801031705.GA2817@darkstar>


> --- linux-2.6.orig/include/net/bluetooth/rfcomm.h	2009-08-01 10:53:18.000000000 +0800
> +++ linux-2.6/include/net/bluetooth/rfcomm.h	2009-08-01 10:55:29.000000000 +0800
> @@ -355,7 +355,18 @@ struct rfcomm_dev_list_req {
>  };
>  
>  int  rfcomm_dev_ioctl(struct sock *sk, unsigned int cmd, void __user *arg);
> +
> +#ifdef CONFIG_BT_RFCOMM_TTY
>  int  rfcomm_init_ttys(void);
>  void rfcomm_cleanup_ttys(void);
> -
> +#else
> +static inline int rfcomm_init_ttys(void)
> +{
> +	return 0;
> +}
> +static inline int rfcomm_cleanup_ttys(void)
> +{
> +	return 0;
> +}
> +#endif

Just a minor thing:

Does rfcomm_cleanup_ttys() return 'int' or is it 'void' ?

Regards,
Oliver

^ permalink raw reply

* Re: tty_register_device NULL pointer dereference in 2.6.31-rc4
From: Oliver Hartkopp @ 2009-08-01  9:15 UTC (permalink / raw)
  To: Dave Young; +Cc: Alan Cox, Marcel Holtmann, Linux Netdev List, linux-bluetooth
In-Reply-To: <20090801031705.GA2817@darkstar>

Dave Young wrote:
> On Fri, Jul 31, 2009 at 01:20:19PM +0200, Oliver Hartkopp wrote:

>> applied this patch and replaced mdelay(100) with msleep(100).
>>
>> I got two crashes and three proper boots.
>>
>> The crashes look like the formerly posted screenshots.
>> When it boots properly i can see the delay in the boot process.
>>
>> Does this help?
> 
> Yes, I think so.
> 
> Please unapply the before two patch, try the following v2 patch instead.
> Changes from v1: fixes 'goto' path again, make tty init before sock init.

Hi Dave,

i did a 'git reset --hard' to get back to vanilla 2.6.31-rc4 .

Applied the v2 patch and booted five times without any problems.

I'll keep you informed if anything changes ;-)

Thanks,
Oliver


> ---
> 
> rfcomm tty may be used before rfcomm_tty_driver initilized,
> The problem is that now socket layer init before tty layer, if userspace
> program do socket callback right here then oops will happen.
> 
> reporting in:
> http://marc.info/?l=linux-bluetooth&m=124404919324542&w=2
> 
> make 3 changes:
> 1. remove #ifdef in rfcomm/core.c,
> make it blank function when rfcomm tty not selected in rfcomm.h
> 
> 2. tune the rfcomm_init error patch to ensure
> tty driver initilized before rfcomm socket usage.
> 
> 3. remove __exit for rfcomm_cleanup_sockets
> because above change need call it in a __init function. 
> 
> 
> CC: Alan Cox <alan@lxorguk.ukuu.org.uk>
> Reported-by: Oliver Hartkopp <oliver@hartkopp.net>
> Signed-off-by: Dave Young <hidave.darkstar@gmail.com>
> --
> include/net/bluetooth/rfcomm.h |   13 ++++++++++++-
> net/bluetooth/rfcomm/core.c    |   29 ++++++++++++++++++++---------
> net/bluetooth/rfcomm/sock.c    |    2 +-
> 3 files changed, 33 insertions(+), 11 deletions(-)
> 
> --- linux-2.6.orig/include/net/bluetooth/rfcomm.h	2009-08-01 10:53:18.000000000 +0800
> +++ linux-2.6/include/net/bluetooth/rfcomm.h	2009-08-01 10:55:29.000000000 +0800
> @@ -355,7 +355,18 @@ struct rfcomm_dev_list_req {
>  };
>  
>  int  rfcomm_dev_ioctl(struct sock *sk, unsigned int cmd, void __user *arg);
> +
> +#ifdef CONFIG_BT_RFCOMM_TTY
>  int  rfcomm_init_ttys(void);
>  void rfcomm_cleanup_ttys(void);
> -
> +#else
> +static inline int rfcomm_init_ttys(void)
> +{
> +	return 0;
> +}
> +static inline int rfcomm_cleanup_ttys(void)
> +{
> +	return 0;
> +}
> +#endif
>  #endif /* __RFCOMM_H */
> --- linux-2.6.orig/net/bluetooth/rfcomm/core.c	2009-08-01 10:53:18.000000000 +0800
> +++ linux-2.6/net/bluetooth/rfcomm/core.c	2009-08-01 11:03:24.000000000 +0800
> @@ -2080,28 +2080,41 @@ static CLASS_ATTR(rfcomm_dlc, S_IRUGO, r
>  /* ---- Initialization ---- */
>  static int __init rfcomm_init(void)
>  {
> +	int ret;
> +
>  	l2cap_load();
>  
>  	hci_register_cb(&rfcomm_cb);
>  
>  	rfcomm_thread = kthread_run(rfcomm_run, NULL, "krfcommd");
>  	if (IS_ERR(rfcomm_thread)) {
> -		hci_unregister_cb(&rfcomm_cb);
> -		return PTR_ERR(rfcomm_thread);
> +		ret = PTR_ERR(rfcomm_thread);
> +		goto out_thread;
>  	}
>  
>  	if (class_create_file(bt_class, &class_attr_rfcomm_dlc) < 0)
>  		BT_ERR("Failed to create RFCOMM info file");
>  
> -	rfcomm_init_sockets();
> -
> -#ifdef CONFIG_BT_RFCOMM_TTY
> -	rfcomm_init_ttys();
> -#endif
> +	ret = rfcomm_init_ttys();
> +	if (ret)
> +		goto out_tty;
> +
> +	ret = rfcomm_init_sockets();
> +	if (ret)
> +		goto out_sock;
>  
>  	BT_INFO("RFCOMM ver %s", VERSION);
>  
>  	return 0;
> +
> +out_sock:
> +	rfcomm_cleanup_ttys();
> +out_tty:
> +	kthread_stop(rfcomm_thread);
> +out_thread:
> +	hci_unregister_cb(&rfcomm_cb);
> +
> +	return ret;
>  }
>  
>  static void __exit rfcomm_exit(void)
> @@ -2112,9 +2125,7 @@ static void __exit rfcomm_exit(void)
>  
>  	kthread_stop(rfcomm_thread);
>  
> -#ifdef CONFIG_BT_RFCOMM_TTY
>  	rfcomm_cleanup_ttys();
> -#endif
>  
>  	rfcomm_cleanup_sockets();
>  }
> --- linux-2.6.orig/net/bluetooth/rfcomm/sock.c	2009-08-01 10:53:18.000000000 +0800
> +++ linux-2.6/net/bluetooth/rfcomm/sock.c	2009-08-01 10:55:29.000000000 +0800
> @@ -1132,7 +1132,7 @@ error:
>  	return err;
>  }
>  
> -void __exit rfcomm_cleanup_sockets(void)
> +void rfcomm_cleanup_sockets(void)
>  {
>  	class_remove_file(bt_class, &class_attr_rfcomm);
>  


^ permalink raw reply

* [PATCH 10/10] drivers/net/wireless: introduce missing kfree
From: Julia Lawall @ 2009-08-01  8:55 UTC (permalink / raw)
  To: linville, linux-wireless, netdev, linux-kernel, kernel-janitors

From: Julia Lawall <julia@diku.dk>

Error handling code following a kzalloc should free the allocated data.

The semantic match that finds the problem is as follows:
(http://www.emn.fr/x-info/coccinelle/)

// <smpl>
@r exists@
local idexpression x;
statement S;
expression E;
identifier f,f1,l;
position p1,p2;
expression *ptr != NULL;
@@

x@p1 = \(kmalloc\|kzalloc\|kcalloc\)(...);
...
if (x == NULL) S
<... when != x
     when != if (...) { <+...x...+> }
(
x->f1 = E
|
 (x->f1 == NULL || ...)
|
 f(...,x->f1,...)
)
...>
(
 return \(0\|<+...x...+>\|ptr\);
|
 return@p2 ...;
)

@script:python@
p1 << r.p1;
p2 << r.p2;
@@

print "* file: %s kmalloc %s return %s" % (p1[0].file,p1[0].line,p2[0].line)
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>
---
 drivers/net/wireless/iwmc3200wifi/commands.c   |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/iwmc3200wifi/commands.c b/drivers/net/wireless/iwmc3200wifi/commands.c
index 0d66370..2bf9ef9 100644
--- a/drivers/net/wireless/iwmc3200wifi/commands.c
+++ b/drivers/net/wireless/iwmc3200wifi/commands.c
@@ -233,6 +233,7 @@ int iwm_store_rxiq_calib_result(struct iwm_priv *iwm)
 	eeprom_rxiq = iwm_eeprom_access(iwm, IWM_EEPROM_CALIB_RXIQ);
 	if (IS_ERR(eeprom_rxiq)) {
 		IWM_ERR(iwm, "Couldn't access EEPROM RX IQ entry\n");
+		kfree(rxiq);
 		return PTR_ERR(eeprom_rxiq);
 	}
 

^ permalink raw reply related

* [PATCH 6/10] drivers/net/wireless/iwlwifi: introduce missing kfree
From: Julia Lawall @ 2009-08-01  8:54 UTC (permalink / raw)
  To: yi.zhu-ral2JQCrhuEAvxtiuMwx3w,
	reinette.chatre-ral2JQCrhuEAvxtiuMwx3w,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	ipw3945-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, kernel-janitors

From: Julia Lawall <julia-dAYI7NvHqcQ@public.gmane.org>

Error handling code following a kzalloc should free the allocated data.

The semantic match that finds the problem is as follows:
(http://www.emn.fr/x-info/coccinelle/)

// <smpl>
@r exists@
local idexpression x;
statement S;
expression E;
identifier f,f1,l;
position p1,p2;
expression *ptr != NULL;
@@

x@p1 = \(kmalloc\|kzalloc\|kcalloc\)(...);
...
if (x == NULL) S
<... when != x
     when != if (...) { <+...x...+> }
(
x->f1 = E
|
 (x->f1 == NULL || ...)
|
 f(...,x->f1,...)
)
...>
(
 return \(0\|<+...x...+>\|ptr\);
|
 return@p2 ...;
)

@script:python@
p1 << r.p1;
p2 << r.p2;
@@

print "* file: %s kmalloc %s return %s" % (p1[0].file,p1[0].line,p2[0].line)
// </smpl>

Signed-off-by: Julia Lawall <julia-dAYI7NvHqcQ@public.gmane.org>
---
 drivers/net/wireless/iwlwifi/iwl-debugfs.c     |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-debugfs.c b/drivers/net/wireless/iwlwifi/iwl-debugfs.c
index 7707a26..c2106d0 100644
--- a/drivers/net/wireless/iwlwifi/iwl-debugfs.c
+++ b/drivers/net/wireless/iwlwifi/iwl-debugfs.c
@@ -320,6 +320,7 @@ static ssize_t iwl_dbgfs_nvm_read(struct file *file,
 	ptr = priv->eeprom;
 	if (!ptr) {
 		IWL_ERR(priv, "Invalid EEPROM/OTP memory\n");
+		kfree(buf);
 		return -ENOMEM;
 	}
 	pos += scnprintf(buf + pos, buf_size - pos, "NVM Type: %s\n",
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* Re: tty_register_device NULL pointer dereference in 2.6.31-rc4
From: Dave Young @ 2009-08-01  3:17 UTC (permalink / raw)
  To: Oliver Hartkopp
  Cc: Alan Cox, Marcel Holtmann, Linux Netdev List,
	linux-bluetooth-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4A72D373.7080802-fJ+pQTUTwRTk1uMJSBkQmQ@public.gmane.org>

On Fri, Jul 31, 2009 at 01:20:19PM +0200, Oliver Hartkopp wrote:
> Dave Young wrote:
> > On Fri, Jul 31, 2009 at 5:39 PM, Dave Young<hidave.darkstar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >> On Thu, Jul 30, 2009 at 12:05:55PM +0200, Oliver Hartkopp wrote:
> >>> Dave Young wrote:
> >>>> On Wed, Jul 29, 2009 at 10:00 PM, Oliver Hartkopp<oliver-fJ+pQTUTwRTk1uMJSBkQmQ@public.gmane.org> wrote:
> >>>>> Hi Dave,
> >>>>>
> >>>>> i got it again - even with your patch (that's why it's 2.6.31-rc4-dirty in the
> >>>>> attached screenshot).
> >>>> Weird, the oops occurs between sock init and tty init routines. Could
> >>>> you tell your bluez version and your configuration?
> >>>>
> >>> No problem:
> >> Thanks.
> >>
> >> It's still reasonable, after rfcomm sock layer initialized, userspace do sock ioctl callback but tty layer was not initilized yet at this time.
> >>
> >> Could you confirm it by applying following debug patch on top of my previous patch? if you get more oops with it then above reason will be right.
> >>
> >> --- linux-2.6.orig/net/bluetooth/rfcomm/core.c  2009-07-31 17:14:07.000000000 +0800
> >> +++ linux-2.6/net/bluetooth/rfcomm/core.c       2009-07-31 17:30:39.000000000 +0800
> >> @@ -36,6 +36,7 @@
> >>  #include <linux/net.h>
> >>  #include <linux/mutex.h>
> >>  #include <linux/kthread.h>
> >> +#include <linux/nmi.h>
> >>
> >>  #include <net/sock.h>
> >>  #include <asm/uaccess.h>
> >> @@ -2080,7 +2081,7 @@ static CLASS_ATTR(rfcomm_dlc, S_IRUGO, r
> >>  /* ---- Initialization ---- */
> >>  static int __init rfcomm_init(void)
> >>  {
> >> -       int ret;
> >> +       int ret, i;
> >>
> >>        l2cap_load();
> >>
> >> @@ -2088,6 +2089,12 @@ static int __init rfcomm_init(void)
> >>        if (ret)
> >>                goto out_sock;
> >>
> >> +       /* delay 5 seconds to trigger the tty bug */
> >> +       for (i = 0; i < 50; i++) {
> >> +               touch_nmi_watchdog();
> >> +               mdelay(100);
> > 
> > Hi, for this case, msleep is better, you can just replace the above
> > two lines with msleep(100)
> > 
> 
> Hi Dave,
> 
> applied this patch and replaced mdelay(100) with msleep(100).
> 
> I got two crashes and three proper boots.
> 
> The crashes look like the formerly posted screenshots.
> When it boots properly i can see the delay in the boot process.
> 
> Does this help?

Yes, I think so.

Please unapply the before two patch, try the following v2 patch instead.
Changes from v1: fixes 'goto' path again, make tty init before sock init.

Thanks.
---

rfcomm tty may be used before rfcomm_tty_driver initilized,
The problem is that now socket layer init before tty layer, if userspace
program do socket callback right here then oops will happen.

reporting in:
http://marc.info/?l=linux-bluetooth&m=124404919324542&w=2

make 3 changes:
1. remove #ifdef in rfcomm/core.c,
make it blank function when rfcomm tty not selected in rfcomm.h

2. tune the rfcomm_init error patch to ensure
tty driver initilized before rfcomm socket usage.

3. remove __exit for rfcomm_cleanup_sockets
because above change need call it in a __init function. 


CC: Alan Cox <alan-qBU/x9rampVanCEyBjwyrvXRex20P6io@public.gmane.org>
Reported-by: Oliver Hartkopp <oliver-fJ+pQTUTwRTk1uMJSBkQmQ@public.gmane.org>
Signed-off-by: Dave Young <hidave.darkstar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
--
include/net/bluetooth/rfcomm.h |   13 ++++++++++++-
net/bluetooth/rfcomm/core.c    |   29 ++++++++++++++++++++---------
net/bluetooth/rfcomm/sock.c    |    2 +-
3 files changed, 33 insertions(+), 11 deletions(-)

--- linux-2.6.orig/include/net/bluetooth/rfcomm.h	2009-08-01 10:53:18.000000000 +0800
+++ linux-2.6/include/net/bluetooth/rfcomm.h	2009-08-01 10:55:29.000000000 +0800
@@ -355,7 +355,18 @@ struct rfcomm_dev_list_req {
 };
 
 int  rfcomm_dev_ioctl(struct sock *sk, unsigned int cmd, void __user *arg);
+
+#ifdef CONFIG_BT_RFCOMM_TTY
 int  rfcomm_init_ttys(void);
 void rfcomm_cleanup_ttys(void);
-
+#else
+static inline int rfcomm_init_ttys(void)
+{
+	return 0;
+}
+static inline int rfcomm_cleanup_ttys(void)
+{
+	return 0;
+}
+#endif
 #endif /* __RFCOMM_H */
--- linux-2.6.orig/net/bluetooth/rfcomm/core.c	2009-08-01 10:53:18.000000000 +0800
+++ linux-2.6/net/bluetooth/rfcomm/core.c	2009-08-01 11:03:24.000000000 +0800
@@ -2080,28 +2080,41 @@ static CLASS_ATTR(rfcomm_dlc, S_IRUGO, r
 /* ---- Initialization ---- */
 static int __init rfcomm_init(void)
 {
+	int ret;
+
 	l2cap_load();
 
 	hci_register_cb(&rfcomm_cb);
 
 	rfcomm_thread = kthread_run(rfcomm_run, NULL, "krfcommd");
 	if (IS_ERR(rfcomm_thread)) {
-		hci_unregister_cb(&rfcomm_cb);
-		return PTR_ERR(rfcomm_thread);
+		ret = PTR_ERR(rfcomm_thread);
+		goto out_thread;
 	}
 
 	if (class_create_file(bt_class, &class_attr_rfcomm_dlc) < 0)
 		BT_ERR("Failed to create RFCOMM info file");
 
-	rfcomm_init_sockets();
-
-#ifdef CONFIG_BT_RFCOMM_TTY
-	rfcomm_init_ttys();
-#endif
+	ret = rfcomm_init_ttys();
+	if (ret)
+		goto out_tty;
+
+	ret = rfcomm_init_sockets();
+	if (ret)
+		goto out_sock;
 
 	BT_INFO("RFCOMM ver %s", VERSION);
 
 	return 0;
+
+out_sock:
+	rfcomm_cleanup_ttys();
+out_tty:
+	kthread_stop(rfcomm_thread);
+out_thread:
+	hci_unregister_cb(&rfcomm_cb);
+
+	return ret;
 }
 
 static void __exit rfcomm_exit(void)
@@ -2112,9 +2125,7 @@ static void __exit rfcomm_exit(void)
 
 	kthread_stop(rfcomm_thread);
 
-#ifdef CONFIG_BT_RFCOMM_TTY
 	rfcomm_cleanup_ttys();
-#endif
 
 	rfcomm_cleanup_sockets();
 }
--- linux-2.6.orig/net/bluetooth/rfcomm/sock.c	2009-08-01 10:53:18.000000000 +0800
+++ linux-2.6/net/bluetooth/rfcomm/sock.c	2009-08-01 10:55:29.000000000 +0800
@@ -1132,7 +1132,7 @@ error:
 	return err;
 }
 
-void __exit rfcomm_cleanup_sockets(void)
+void rfcomm_cleanup_sockets(void)
 {
 	class_remove_file(bt_class, &class_attr_rfcomm);
 

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox