Re: [REVERT] be6b38bcb175613f239e0b302607db346472c6b6. v2.6.34-rc3-406 oops with 4965AGN wireless

Re: [REVERT] be6b38bcb175613f239e0b302607db346472c6b6. v2.6.34-rc3-406 oops with 4965AGN wireless

[Jeff Chua]

On Fri, Apr 9, 2010 at 1:19 AM, Linus Torvalds 
<torvalds@linux-foundation.org> wrote:

> I don't mind reverting patches, but I hate doing it blind and without 
> _any_ chance for the guilty party to try to fix it first.

> Can you post the oops that your subject implies happened? Maybe the 
> driver authors can find the proper fix without a revert?

The screen is so fast and never stops so I don't know how to capture that. 
I just tried the patch from Wey and had to modify it slightly to make it 
work.



On Fri, Apr 9, 2010 at 2:50 AM, Guy, Wey-Yi <wey-yi.w.guy@intel.com> 
wrote:

> Sorry for the problem caused by this patch, it is my mistake using the
similar approach for 4965 like the newer devices. could you try the
attach patch to see if this fix your system freeze problem.

Wey, the patch doesn't apply cleanly. I believe you meant "iwl_" instead 
of "iwlagn_".

Also, the checking for IWL_INVALID_STATION should be done after the "} 
else {" as in the original code prior to your patch. I just verified this 
with the patch below and it no longer oops.



On Fri, Apr 9, 2010 at 2:13 AM, Al Viro <viro@zeniv.linux.org.uk> wrote:

> So what happens if we hit sta_id == IWL_INVALID_STATION and
> !txq->sched_retry?

> I'm not familiar with that code and I don't have the hardware, so this 
> is just from RTFS, but... might make sense to replace that call of 
> iwl_free_tfds_in_queue with
>       if (sta_id == IWL_INVALID_STATION)
>              printk(KERN_ERR "buggered");
>       else
>              iwl_free_tfds_in_queue(priv, sta_id, tid, freed);


I just tried that, but not seeing any invalid station messages. The 
KERN_ERR has been added below.




On Fri, Apr 9, 2010 at 4:09 AM, Guy, Wey-Yi <wey-yi.w.guy@intel.com> 
wrote:

> We can check for IWL_INVALID_STATION and print log, but need to check
> for qc to make sure it is valid; maybe something like


Ok, added below.



Thanks,
Jeff

--- drivers/net/wireless/iwlwifi/iwl-4965.c.org	2010-04-09 02:11:45.000000000 +0800
+++ drivers/net/wireless/iwlwifi/iwl-4965.c	2010-04-09 03:21:57.000000000 +0800
@@ -2012,10 +2012,18 @@

  		if (txq->q.read_ptr != (scd_ssn & 0xff)) {
  			index = iwl_queue_dec_wrap(scd_ssn & 0xff, txq->q.n_bd);
-			IWL_DEBUG_TX_REPLY(priv, "Retry scheduler reclaim scd_ssn "
-					   "%d index %d\n", scd_ssn , index);
+			IWL_DEBUG_TX_REPLY(priv,
+					"Retry scheduler reclaim scd_ssn "
+					"%d index %d\n", scd_ssn , index);
  			freed = iwl_tx_queue_reclaim(priv, txq_id, index);
  			iwl_free_tfds_in_queue(priv, sta_id, tid, freed);
+			if (qc && likely(sta_id != IWL_INVALID_STATION))
+				iwl_free_tfds_in_queue(priv, sta_id, tid,
+					freed);
+			else {
+				if (sta_id == IWL_INVALID_STATION)
+				       IWL_ERR(priv, "invalid station");
+			}

  			if (priv->mac80211_registered &&
  			    (iwl_queue_space(&txq->q) > txq->q.low_mark) &&
@@ -2041,14 +2049,20 @@
  				   tx_resp->failure_frame);

  		freed = iwl_tx_queue_reclaim(priv, txq_id, index);
-		iwl_free_tfds_in_queue(priv, sta_id, tid, freed);
+		if (qc && likely(sta_id != IWL_INVALID_STATION))
+			iwl_free_tfds_in_queue(priv, sta_id, tid, freed);
+		else {
+			if (sta_id == IWL_INVALID_STATION)
+			       IWL_ERR(priv, "invalid station");
+		}

  		if (priv->mac80211_registered &&
  		    (iwl_queue_space(&txq->q) > txq->q.low_mark))
  			iwl_wake_queue(priv, txq_id);
  	}

-	iwl_txq_check_empty(priv, sta_id, tid, txq_id);
+	if (qc && likely(sta_id != IWL_INVALID_STATION))
+		iwl_txq_check_empty(priv, sta_id, tid, txq_id);

  	if (iwl_check_bits(status, TX_ABORT_REQUIRED_MSK))
  		IWL_ERR(priv, "TODO:  Implement Tx ABORT REQUIRED!!!\n");

Re: [REVERT] be6b38bcb175613f239e0b302607db346472c6b6. v2.6.34-rc3-406 oops with 4965AGN wireless

[Jeff Chua]

On Fri, Apr 9, 2010 at 4:24 AM, Guy, Wey-Yi <wey-yi.w.guy@intel.com> 
wrote:

> Maybe this patch looks better, if sched_rety and sta_id ==

Wey,

I've updated your patch and tested it. Wireless seems ok now.

Please verify it and inform Linus once you sign-off on it.


Thanks,
Jeff

--- a/drivers/net/wireless/iwlwifi/iwl-4965.c.org	2010-04-09 02:11:45.000000000 +0800
+++ a/drivers/net/wireless/iwlwifi/iwl-4965.c	2010-04-09 03:33:43.000000000 +0800
@@ -2012,10 +2012,15 @@

  		if (txq->q.read_ptr != (scd_ssn & 0xff)) {
  			index = iwl_queue_dec_wrap(scd_ssn & 0xff, txq->q.n_bd);
-			IWL_DEBUG_TX_REPLY(priv, "Retry scheduler reclaim scd_ssn "
-					   "%d index %d\n", scd_ssn , index);
+			IWL_DEBUG_TX_REPLY(priv,
+					"Retry scheduler reclaim scd_ssn "
+					"%d index %d\n", scd_ssn , index);
  			freed = iwl_tx_queue_reclaim(priv, txq_id, index);
-			iwl_free_tfds_in_queue(priv, sta_id, tid, freed);
+			if(qc)
+				iwl_free_tfds_in_queue(priv, sta_id, tid,
+					freed);
+			else if (sta_id == IWL_INVALID_STATION)
+				       IWL_ERR(priv, "invalid station");

  			if (priv->mac80211_registered &&
  			    (iwl_queue_space(&txq->q) > txq->q.low_mark) &&
@@ -2041,14 +2046,18 @@
  				   tx_resp->failure_frame);

  		freed = iwl_tx_queue_reclaim(priv, txq_id, index);
-		iwl_free_tfds_in_queue(priv, sta_id, tid, freed);
+		if (qc && likely(sta_id != IWL_INVALID_STATION))
+			iwl_free_tfds_in_queue(priv, sta_id, tid, freed);
+		else if (sta_id == IWL_INVALID_STATION)
+		       IWL_ERR(priv, "invalid station");

  		if (priv->mac80211_registered &&
  		    (iwl_queue_space(&txq->q) > txq->q.low_mark))
  			iwl_wake_queue(priv, txq_id);
  	}

-	iwl_txq_check_empty(priv, sta_id, tid, txq_id);
+	if (qc && likely(sta_id != IWL_INVALID_STATION))
+		iwl_txq_check_empty(priv, sta_id, tid, txq_id);

  	if (iwl_check_bits(status, TX_ABORT_REQUIRED_MSK))
  		IWL_ERR(priv, "TODO:  Implement Tx ABORT REQUIRED!!!\n");

Re: [REVERT] be6b38bcb175613f239e0b302607db346472c6b6. v2.6.34-rc3-406 oops with 4965AGN wireless

[John W. Linville]

On Thu, Apr 08, 2010 at 01:50:00PM -0700, Guy, Wey-Yi wrote:

> I attach here again in case you did not get it.
> 
> Thanks
> Wey
> 
> 

> From f3a5f691e32c0edfbc42a1e8687392dc3efe396e Mon Sep 17 00:00:00 2001
> From: Wey-Yi Guy <wey-yi.w.guy@intel.com>
> Date: Thu, 8 Apr 2010 13:17:37 -0700
> Subject: [PATCH 1/1] iwlwifi: need check for valid qos packet before free
> 
> For 4965, need to check it is valid qos frame before free, only valid
> QoS frame has the tid used to free the packets.
> 
> Signed-off-by: Wey-Yi Guy <wey-yi.w.guy@intel.com>

I would prefer to take this through the normal wireless-2.6 route if
you don't mind.  Are you satisifed with this version?

Thanks,

John

P.S.  In the future, I'm also quite open to revert requests as needed.
It makes my life easier for things (even reverts) under wireless to
come through my tree rather than trying to figure-out how something
happened later...
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

Re: [REVERT] be6b38bcb175613f239e0b302607db346472c6b6. v2.6.34-rc3-406 oops with 4965AGN wireless

[John W. Linville]

On Fri, Apr 09, 2010 at 03:27:53AM +0800, Jeff Chua wrote:

> Wey, the patch doesn't apply cleanly. I believe you meant "iwl_" instead  
> of "iwlagn_".

He based his patch on wireless-testing (or something similar), where
iwlagn_ is the proper prefix for the functions in question.

John

P.S. Cc'ing linux-wireless...

P.P.S.  You might try this version of his later patch...

>From ece6444c2fe80dab679beb5f0d58b091f1933b00 Mon Sep 17 00:00:00 2001
From: Wey-Yi Guy <wey-yi.w.guy@intel.com>
Date: Thu, 8 Apr 2010 13:17:37 -0700
Subject: [PATCH] iwlwifi: need check for valid qos packet before free

For 4965, need to check it is valid qos frame before free, only valid
QoS frame has the tid used to free the packets.

Signed-off-by: Wey-Yi Guy <wey-yi.w.guy@intel.com>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
 drivers/net/wireless/iwlwifi/iwl-4965.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-4965.c b/drivers/net/wireless/iwlwifi/iwl-4965.c
index 83c52a6..8972166 100644
--- a/drivers/net/wireless/iwlwifi/iwl-4965.c
+++ b/drivers/net/wireless/iwlwifi/iwl-4965.c
@@ -2015,7 +2015,9 @@ static void iwl4965_rx_reply_tx(struct iwl_priv *priv,
 			IWL_DEBUG_TX_REPLY(priv, "Retry scheduler reclaim scd_ssn "
 					   "%d index %d\n", scd_ssn , index);
 			freed = iwl_tx_queue_reclaim(priv, txq_id, index);
-			iwl_free_tfds_in_queue(priv, sta_id, tid, freed);
+			if (qc)
+				iwl_free_tfds_in_queue(priv, sta_id,
+						       tid, freed);
 
 			if (priv->mac80211_registered &&
 			    (iwl_queue_space(&txq->q) > txq->q.low_mark) &&
@@ -2041,14 +2043,17 @@ static void iwl4965_rx_reply_tx(struct iwl_priv *priv,
 				   tx_resp->failure_frame);
 
 		freed = iwl_tx_queue_reclaim(priv, txq_id, index);
-		iwl_free_tfds_in_queue(priv, sta_id, tid, freed);
+		if (qc && likely(sta_id != IWL_INVALID_STATION))
+			iwl_free_tfds_in_queue(priv, sta_id, tid, freed);
+		else if (sta_id == IWL_INVALID_STATION)
+			IWL_DEBUG_TX_REPLY(priv, "Station not known\n");
 
 		if (priv->mac80211_registered &&
 		    (iwl_queue_space(&txq->q) > txq->q.low_mark))
 			iwl_wake_queue(priv, txq_id);
 	}
-
-	iwl_txq_check_empty(priv, sta_id, tid, txq_id);
+	if (qc && likely(sta_id != IWL_INVALID_STATION))
+		iwl_txq_check_empty(priv, sta_id, tid, txq_id);
 
 	if (iwl_check_bits(status, TX_ABORT_REQUIRED_MSK))
 		IWL_ERR(priv, "TODO:  Implement Tx ABORT REQUIRED!!!\n");
-- 
1.6.2.5

-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

Re: [REVERT] be6b38bcb175613f239e0b302607db346472c6b6. v2.6.34-rc3-406 oops with 4965AGN wireless

[John W. Linville]

On Thu, Apr 08, 2010 at 04:19:53PM -0400, John W. Linville wrote:
> On Fri, Apr 09, 2010 at 03:27:53AM +0800, Jeff Chua wrote:
> 
> > Wey, the patch doesn't apply cleanly. I believe you meant "iwl_" instead  
> > of "iwlagn_".
> 
> He based his patch on wireless-testing (or something similar), where
> iwlagn_ is the proper prefix for the functions in question.

"She based her patch..." -- my apologies!

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

Re: [REVERT] be6b38bcb175613f239e0b302607db346472c6b6. v2.6.34-rc3-406 oops with 4965AGN wireless

[Guy, Wey-Yi]

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

Hi Jeff,


On Thu, 2010-04-08 at 12:42 -0700, Jeff Chua wrote:
> 
> On Fri, Apr 9, 2010 at 4:24 AM, Guy, Wey-Yi <wey-yi.w.guy@intel.com> 
> wrote:
> 
> > Maybe this patch looks better, if sched_rety and sta_id ==
> 
> Wey,
> 
> I've updated your patch and tested it. Wireless seems ok now.
> 
> Please verify it and inform Linus once you sign-off on it.
> 
> 
> Thanks,
> Jeff
> 
> --- a/drivers/net/wireless/iwlwifi/iwl-4965.c.org	2010-04-09 02:11:45.000000000 +0800
> +++ a/drivers/net/wireless/iwlwifi/iwl-4965.c	2010-04-09 03:33:43.000000000 +0800
> @@ -2012,10 +2012,15 @@
> 
>   		if (txq->q.read_ptr != (scd_ssn & 0xff)) {
>   			index = iwl_queue_dec_wrap(scd_ssn & 0xff, txq->q.n_bd);
> -			IWL_DEBUG_TX_REPLY(priv, "Retry scheduler reclaim scd_ssn "
> -					   "%d index %d\n", scd_ssn , index);
> +			IWL_DEBUG_TX_REPLY(priv,
> +					"Retry scheduler reclaim scd_ssn "
> +					"%d index %d\n", scd_ssn , index);
>   			freed = iwl_tx_queue_reclaim(priv, txq_id, index);
> -			iwl_free_tfds_in_queue(priv, sta_id, tid, freed);
> +			if(qc)
> +				iwl_free_tfds_in_queue(priv, sta_id, tid,
> +					freed);
> +			else if (sta_id == IWL_INVALID_STATION)
> +				       IWL_ERR(priv, "invalid station");
> 
do not need to check sta_id == IWL_INVALID_STATION here since already
check before.
could you try my revised version of patch I send after my first attempt.
to make sure it works. 

I attach here again in case you did not get it.

Thanks
Wey



[-- Attachment #2: 0001-iwlwifi-need-check-for-valid-qos-packet-before-free.patch --]
[-- Type: text/x-patch, Size: 1998 bytes --]

>From f3a5f691e32c0edfbc42a1e8687392dc3efe396e Mon Sep 17 00:00:00 2001
From: Wey-Yi Guy <wey-yi.w.guy@intel.com>
Date: Thu, 8 Apr 2010 13:17:37 -0700
Subject: [PATCH 1/1] iwlwifi: need check for valid qos packet before free

For 4965, need to check it is valid qos frame before free, only valid
QoS frame has the tid used to free the packets.

Signed-off-by: Wey-Yi Guy <wey-yi.w.guy@intel.com>
---
 drivers/net/wireless/iwlwifi/iwl-4965.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-4965.c b/drivers/net/wireless/iwlwifi/iwl-4965.c
index c710b26..2e3cda7 100644
--- a/drivers/net/wireless/iwlwifi/iwl-4965.c
+++ b/drivers/net/wireless/iwlwifi/iwl-4965.c
@@ -2014,7 +2014,9 @@ static void iwl4965_rx_reply_tx(struct iwl_priv *priv,
 			IWL_DEBUG_TX_REPLY(priv, "Retry scheduler reclaim scd_ssn "
 					   "%d index %d\n", scd_ssn , index);
 			freed = iwlagn_tx_queue_reclaim(priv, txq_id, index);
-			iwl_free_tfds_in_queue(priv, sta_id, tid, freed);
+			if (qc)
+				iwl_free_tfds_in_queue(priv, sta_id,
+						       tid, freed);
 
 			if (priv->mac80211_registered &&
 			    (iwl_queue_space(&txq->q) > txq->q.low_mark) &&
@@ -2040,14 +2042,17 @@ static void iwl4965_rx_reply_tx(struct iwl_priv *priv,
 				   tx_resp->failure_frame);
 
 		freed = iwlagn_tx_queue_reclaim(priv, txq_id, index);
-		iwl_free_tfds_in_queue(priv, sta_id, tid, freed);
+		if (qc && likely(sta_id != IWL_INVALID_STATION))
+			iwl_free_tfds_in_queue(priv, sta_id, tid, freed);
+		else if (sta_id == IWL_INVALID_STATION)
+			IWL_DEBUG_TX_REPLY(priv, "Station not known\n");
 
 		if (priv->mac80211_registered &&
 		    (iwl_queue_space(&txq->q) > txq->q.low_mark))
 			iwl_wake_queue(priv, txq_id);
 	}
-
-	iwlagn_txq_check_empty(priv, sta_id, tid, txq_id);
+	if (qc && likely(sta_id != IWL_INVALID_STATION))
+		iwlagn_txq_check_empty(priv, sta_id, tid, txq_id);
 
 	iwl_check_abort_status(priv, tx_resp->frame_count, status);
 }
-- 
1.5.6.3

Re: [REVERT] be6b38bcb175613f239e0b302607db346472c6b6. v2.6.34-rc3-406 oops with 4965AGN wireless

[Guy, Wey-Yi]

Hi John,

On Thu, 2010-04-08 at 13:02 -0700, John W. Linville wrote:
> On Thu, Apr 08, 2010 at 01:50:00PM -0700, Guy, Wey-Yi wrote:
> 
> > I attach here again in case you did not get it.
> > 
> > Thanks
> > Wey
> > 
> > 
> 
> > From f3a5f691e32c0edfbc42a1e8687392dc3efe396e Mon Sep 17 00:00:00 2001
> > From: Wey-Yi Guy <wey-yi.w.guy@intel.com>
> > Date: Thu, 8 Apr 2010 13:17:37 -0700
> > Subject: [PATCH 1/1] iwlwifi: need check for valid qos packet before free
> > 
> > For 4965, need to check it is valid qos frame before free, only valid
> > QoS frame has the tid used to free the packets.
> > 
> > Signed-off-by: Wey-Yi Guy <wey-yi.w.guy@intel.com>
> 
> I would prefer to take this through the normal wireless-2.6 route if
> you don't mind.  Are you satisifed with this version?
> 

Thanks for the help, the finial version should take care of the problem
and ready for upstream .

Regards
Wey
> 
> P.S.  In the future, I'm also quite open to revert requests as needed.
> It makes my life easier for things (even reverts) under wireless to
> come through my tree rather than trying to figure-out how something
> happened later...

Re: [REVERT] be6b38bcb175613f239e0b302607db346472c6b6. v2.6.34-rc3-406 oops with 4965AGN wireless

[Guy, Wey-Yi]

Hi Jeff,

On Thu, 2010-04-08 at 17:36 -0700, Jeff Chua wrote:
> 
> 
> On Fri, Apr 9, 2010 at 4:50 AM, Guy, Wey-Yi <wey-yi.w.guy@intel.com>
> wrote:
>         do not need to check sta_id == IWL_INVALID_STATION here since
>         already
>         check before.
>         could you try my revised version of patch I send after my
>         first attempt.
>         to make sure it works.
>         
>         I attach here again in case you did not get it.
> 
> Wey,
>  
> One small touch-up. Again, I've to rename "iwlagn_" to "iwl_" so that
> it can apply to Linus's base code.
> 
> Tested and working fine. Here's the updated patch.
> 
Thanks for verify the patch, you are correct, since I am using the
wireless-testing version of the kernel, you will need to make the
changes in order to get the patch apply. Sorry for the trouble.

Regards
Wey

> 
> 
> 

pull request: wireless-2.6 2010-04-09

[John W. Linville]

Dave,

This fix is intended for 2.6.34.  It resolves an issue involving an
Oops on boxes w/ iwl4965 hardware, apparently introduced by another
recent patch.  The thread describing the issue and the resolution
is here:

	http://marc.info/?l=linux-kernel&m=127074721531649&w=2

In order to avoid reverting that patch, please accept this fix instead.
As usual, please let me know if there are problems!

Thanks,

John

---

The following changes since commit 2626419ad5be1a054d350786b684b41d23de1538:
  David S. Miller (1):
        tcp: Set CHECKSUM_UNNECESSARY in tcp_init_nondata_skb

are available in the git repository at:

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

Wey-Yi Guy (1):
      iwlwifi: need check for valid qos packet before free

 drivers/net/wireless/iwlwifi/iwl-4965.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-4965.c b/drivers/net/wireless/iwlwifi/iwl-4965.c
index 83c52a6..8972166 100644
--- a/drivers/net/wireless/iwlwifi/iwl-4965.c
+++ b/drivers/net/wireless/iwlwifi/iwl-4965.c
@@ -2015,7 +2015,9 @@ static void iwl4965_rx_reply_tx(struct iwl_priv *priv,
 			IWL_DEBUG_TX_REPLY(priv, "Retry scheduler reclaim scd_ssn "
 					   "%d index %d\n", scd_ssn , index);
 			freed = iwl_tx_queue_reclaim(priv, txq_id, index);
-			iwl_free_tfds_in_queue(priv, sta_id, tid, freed);
+			if (qc)
+				iwl_free_tfds_in_queue(priv, sta_id,
+						       tid, freed);
 
 			if (priv->mac80211_registered &&
 			    (iwl_queue_space(&txq->q) > txq->q.low_mark) &&
@@ -2041,14 +2043,17 @@ static void iwl4965_rx_reply_tx(struct iwl_priv *priv,
 				   tx_resp->failure_frame);
 
 		freed = iwl_tx_queue_reclaim(priv, txq_id, index);
-		iwl_free_tfds_in_queue(priv, sta_id, tid, freed);
+		if (qc && likely(sta_id != IWL_INVALID_STATION))
+			iwl_free_tfds_in_queue(priv, sta_id, tid, freed);
+		else if (sta_id == IWL_INVALID_STATION)
+			IWL_DEBUG_TX_REPLY(priv, "Station not known\n");
 
 		if (priv->mac80211_registered &&
 		    (iwl_queue_space(&txq->q) > txq->q.low_mark))
 			iwl_wake_queue(priv, txq_id);
 	}
-
-	iwl_txq_check_empty(priv, sta_id, tid, txq_id);
+	if (qc && likely(sta_id != IWL_INVALID_STATION))
+		iwl_txq_check_empty(priv, sta_id, tid, txq_id);
 
 	if (iwl_check_bits(status, TX_ABORT_REQUIRED_MSK))
 		IWL_ERR(priv, "TODO:  Implement Tx ABORT REQUIRED!!!\n");
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

Re: pull request: wireless-2.6 2010-04-09

[David Miller]

From: "John W. Linville" <linville@tuxdriver.com>
Date: Fri, 9 Apr 2010 11:38:07 -0400

> This fix is intended for 2.6.34.  It resolves an issue involving an
> Oops on boxes w/ iwl4965 hardware, apparently introduced by another
> recent patch.  The thread describing the issue and the resolution
> is here:
> 
> 	http://marc.info/?l=linux-kernel&m=127074721531649&w=2
> 
> In order to avoid reverting that patch, please accept this fix instead.
> As usual, please let me know if there are problems!

Pulled, thanks John.