Netdev List
 help / color / mirror / Atom feed
* Re: net/bluetooth: workqueue destruction WARNING in hci_unregister_dev
From: Jiri Slaby @ 2016-03-17 12:00 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Dmitry Vyukov, Marcel Holtmann, Gustavo Padovan, Johan Hedberg,
	David S. Miller, linux-bluetooth-u79uwXL29TY76Z2rM5mHXA, netdev,
	LKML, syzkaller, Kostya Serebryany, Alexander Potapenko,
	Sasha Levin, Eric Dumazet, Takashi Iwai
In-Reply-To: <20160311171205.GB24046-piEFEHQLUPpN0TnZuCh8vA@public.gmane.org>

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

Hello,

On 03/11/2016, 06:12 PM, Tejun Heo wrote:
> On Thu, Mar 03, 2016 at 10:12:01AM +0100, Jiri Slaby wrote:
>> On 03/02/2016, 04:45 PM, Tejun Heo wrote:
>>> On Fri, Feb 19, 2016 at 01:10:00PM +0100, Jiri Slaby wrote:
>>>>> 1. didn't help, the problem persists. So I haven't applied the patch from 2.
>>>>
>>>> FWIW I dumped more info about the wq:
>>>> wq->name='hci0' pwq=ffff8800390d7600 wq->dfl_pwq=ffff8800390d5200
>>>> pwq->refcnt=2 pwq->nr_active=0 delayed_works: <nothing>
>>>
>>> Can you please print out the same info for all pwq's during shutdown?
>>> It looks like we're leaking pwq refcnt but I can't spot a place where
>>> that could happen on an empty pwq.
>>
>> I have not done that yet, but today, I see:
>> destroy_workqueue: name='req_hci0' pwq=ffff88002f590300
>> wq->dfl_pwq=ffff88002f591e00 pwq->refcnt=2 pwq->nr_active=0 delayed_works:
>>    pwq 12: cpus=0-1 node=0 flags=0x4 nice=-20 active=0/1
>>      in-flight: 18568:wq_barrier_func
> 
> So, this means that there's flush_work() racing against workqueue
> destruction, which can't be safe. :(

But I cannot trigger the WARN_ONs in the attached patch, so I am
confused how this can happen :(. (While I am still seeing the destroy
WARNINGs.)

BTW. what did you mean by dumping the states at shutdown? Is it still
relevant?

thanks,
-- 
js
suse labs

[-- Attachment #2: vhci_debug.patch --]
[-- Type: text/x-patch, Size: 7549 bytes --]

---
 include/linux/workqueue.h        |    1 +
 include/net/bluetooth/hci_core.h |    5 +++++
 kernel/reboot.c                  |    1 +
 kernel/workqueue.c               |   34 +++++++++++++++++++++++++++++++---
 net/bluetooth/hci_core.c         |   39 +++++++++++++++++++++++++++++++++++----
 5 files changed, 73 insertions(+), 7 deletions(-)

--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -312,6 +312,7 @@ enum {
 	__WQ_DRAINING		= 1 << 16, /* internal: workqueue is draining */
 	__WQ_ORDERED		= 1 << 17, /* internal: workqueue is ordered */
 	__WQ_LEGACY		= 1 << 18, /* internal: create*_workqueue() */
+	__WQ_DESTROYING		= 1 << 19,
 
 	WQ_MAX_ACTIVE		= 512,	  /* I like 512, better ideas? */
 	WQ_MAX_UNBOUND_PER_CPU	= 4,	  /* 4 * #cpus for unbound wq */
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -312,6 +312,11 @@ struct hci_dev {
 
 	struct workqueue_struct	*workqueue;
 	struct workqueue_struct	*req_workqueue;
+#define HCI_WQ_A	0
+#define HCI_WQ_D	1
+#define HCI_WQR_A	8
+#define HCI_WQR_D	9
+	unsigned long		wq_status;
 
 	struct work_struct	power_on;
 	struct delayed_work	power_off;
--- a/kernel/reboot.c
+++ b/kernel/reboot.c
@@ -231,6 +231,7 @@ static void kernel_shutdown_prepare(enum
 		(state == SYSTEM_HALT) ? SYS_HALT : SYS_POWER_OFF, NULL);
 	system_state = state;
 	usermodehelper_disable();
+	show_workqueue_state();
 	device_shutdown();
 }
 /**
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -1366,6 +1366,9 @@ static void __queue_work(int cpu, struct
 	unsigned int work_flags;
 	unsigned int req_cpu = cpu;
 
+	if (WARN_ON(wq->flags & __WQ_DESTROYING))
+		return;
+
 	/*
 	 * While a work item is PENDING && off queue, a task trying to
 	 * steal the PENDING will busy-loop waiting for it to either get
@@ -2804,6 +2807,9 @@ static bool start_flush_work(struct work
 		pwq = worker->current_pwq;
 	}
 
+	if (WARN_ON(pwq->wq->flags & __WQ_DESTROYING))
+		return false;
+
 	check_flush_dependency(pwq->wq, work);
 
 	insert_wq_barrier(pwq, barr, work, worker);
@@ -2821,6 +2827,8 @@ static bool start_flush_work(struct work
 		lock_map_acquire_read(&pwq->wq->lockdep_map);
 	lock_map_release(&pwq->wq->lockdep_map);
 
+	WARN_ON(pwq->wq->flags & __WQ_DESTROYING);
+
 	return true;
 already_gone:
 	spin_unlock_irq(&pool->lock);
@@ -3998,6 +4006,8 @@ err_destroy:
 }
 EXPORT_SYMBOL_GPL(__alloc_workqueue_key);
 
+static void show_pwq(struct pool_workqueue *pwq);
+
 /**
  * destroy_workqueue - safely terminate a workqueue
  * @wq: target workqueue
@@ -4010,6 +4020,7 @@ void destroy_workqueue(struct workqueue_
 	int node;
 
 	/* drain it before proceeding with destruction */
+	wq->flags |= __WQ_DESTROYING;
 	drain_workqueue(wq);
 
 	/* sanity checks */
@@ -4024,9 +4035,26 @@ void destroy_workqueue(struct workqueue_
 			}
 		}
 
-		if (WARN_ON((pwq != wq->dfl_pwq) && (pwq->refcnt > 1)) ||
-		    WARN_ON(pwq->nr_active) ||
-		    WARN_ON(!list_empty(&pwq->delayed_works))) {
+		if ((pwq != wq->dfl_pwq) && (pwq->refcnt > 1)) {
+			pr_info("%s: name='%s' pwq=%p wq->dfl_pwq=%p pwq->refcnt=%d pwq->nr_active=%d delayed_works:",
+					__func__, wq->name, pwq, wq->dfl_pwq,
+					pwq->refcnt, pwq->nr_active);
+
+			show_pwq(pwq);
+
+			mutex_unlock(&wq->mutex);
+			WARN_ON(1);
+			return;
+		}
+
+		if (WARN_ON(pwq->nr_active)) {
+			pr_info("%s: %ps\n", __func__, wq);
+			mutex_unlock(&wq->mutex);
+			return;
+		}
+
+		if (WARN_ON(!list_empty(&pwq->delayed_works))) {
+			pr_info("%s: %ps\n", __func__, wq);
 			mutex_unlock(&wq->mutex);
 			return;
 		}
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1471,7 +1471,11 @@ int hci_dev_open(__u16 dev)
 	 * has finished. This means that error conditions like RFKILL
 	 * or no valid public or static random address apply.
 	 */
+	WARN_ON(test_bit(HCI_WQR_D, &hdev->wq_status));
+	WARN_ON(!test_bit(HCI_WQR_A, &hdev->wq_status));
 	flush_workqueue(hdev->req_workqueue);
+	WARN_ON(test_bit(HCI_WQR_D, &hdev->wq_status));
+	WARN_ON(!test_bit(HCI_WQR_A, &hdev->wq_status));
 
 	/* For controllers not using the management interface and that
 	 * are brought up using legacy ioctl, set the HCI_BONDABLE bit
@@ -2467,6 +2471,8 @@ static void hci_cmd_timeout(struct work_
 	struct hci_dev *hdev = container_of(work, struct hci_dev,
 					    cmd_timer.work);
 
+	if (WARN_ON(hci_dev_test_flag(hdev, HCI_UNREGISTER)))
+		return;
 	if (hdev->sent_cmd) {
 		struct hci_command_hdr *sent = (void *) hdev->sent_cmd->data;
 		u16 opcode = __le16_to_cpu(sent->opcode);
@@ -3043,15 +3049,17 @@ int hci_register_dev(struct hci_dev *hde
 
 	BT_DBG("%p name %s bus %d", hdev, hdev->name, hdev->bus);
 
-	hdev->workqueue = alloc_workqueue("%s", WQ_HIGHPRI | WQ_UNBOUND |
-					  WQ_MEM_RECLAIM, 1, hdev->name);
+	hdev->workqueue = alloc_workqueue("%s", WQ_HIGHPRI | WQ_UNBOUND
+					  , 1, hdev->name);
+	set_bit(HCI_WQ_A, &hdev->wq_status);
 	if (!hdev->workqueue) {
 		error = -ENOMEM;
 		goto err;
 	}
 
-	hdev->req_workqueue = alloc_workqueue("%s", WQ_HIGHPRI | WQ_UNBOUND |
-					      WQ_MEM_RECLAIM, 1, hdev->name);
+	hdev->req_workqueue = alloc_workqueue("req_%s", WQ_HIGHPRI | WQ_UNBOUND
+					      , 1, hdev->name);
+	set_bit(HCI_WQR_A, &hdev->wq_status);
 	if (!hdev->req_workqueue) {
 		destroy_workqueue(hdev->workqueue);
 		error = -ENOMEM;
@@ -3108,8 +3116,12 @@ int hci_register_dev(struct hci_dev *hde
 	return id;
 
 err_wqueue:
+	set_bit(HCI_WQ_D, &hdev->wq_status);
 	destroy_workqueue(hdev->workqueue);
+	clear_bit(HCI_WQ_A, &hdev->wq_status);
+	set_bit(HCI_WQR_D, &hdev->wq_status);
 	destroy_workqueue(hdev->req_workqueue);
+	clear_bit(HCI_WQR_A, &hdev->wq_status);
 err:
 	ida_simple_remove(&hci_index_ida, hdev->id);
 
@@ -3160,7 +3172,9 @@ void hci_unregister_dev(struct hci_dev *
 	debugfs_remove_recursive(hdev->debugfs);
 
 	destroy_workqueue(hdev->workqueue);
+	set_bit(HCI_WQR_D, &hdev->wq_status);
 	destroy_workqueue(hdev->req_workqueue);
+	clear_bit(HCI_WQR_A, &hdev->wq_status);
 
 	hci_dev_lock(hdev);
 	hci_bdaddr_list_clear(&hdev->blacklist);
@@ -3225,6 +3239,11 @@ int hci_recv_frame(struct hci_dev *hdev,
 		return -ENXIO;
 	}
 
+	if (WARN_ON(hci_dev_test_flag(hdev, HCI_UNREGISTER))) {
+		kfree_skb(skb);
+		return -ENXIO;
+	}
+
 	if (hci_skb_pkt_type(skb) != HCI_EVENT_PKT &&
 	    hci_skb_pkt_type(skb) != HCI_ACLDATA_PKT &&
 	    hci_skb_pkt_type(skb) != HCI_SCODATA_PKT) {
@@ -3248,6 +3267,9 @@ EXPORT_SYMBOL(hci_recv_frame);
 /* Receive diagnostic message from HCI drivers */
 int hci_recv_diag(struct hci_dev *hdev, struct sk_buff *skb)
 {
+	if (WARN_ON(hci_dev_test_flag(hdev, HCI_UNREGISTER)))
+		return -ENXIO;
+
 	/* Mark as diagnostic packet */
 	hci_skb_pkt_type(skb) = HCI_DIAG_PKT;
 
@@ -3326,6 +3348,9 @@ int hci_send_cmd(struct hci_dev *hdev, _
 {
 	struct sk_buff *skb;
 
+	if (WARN_ON(hci_dev_test_flag(hdev, HCI_UNREGISTER)))
+		return -ENXIO;
+
 	BT_DBG("%s opcode 0x%4.4x plen %d", hdev->name, opcode, plen);
 
 	skb = hci_prepare_cmd(hdev, opcode, plen, param);
@@ -3461,6 +3486,9 @@ void hci_send_acl(struct hci_chan *chan,
 {
 	struct hci_dev *hdev = chan->conn->hdev;
 
+	if (WARN_ON(hci_dev_test_flag(hdev, HCI_UNREGISTER)))
+		return;
+
 	BT_DBG("%s chan %p flags 0x%4.4x", hdev->name, chan, flags);
 
 	hci_queue_acl(chan, &chan->data_q, skb, flags);
@@ -3474,6 +3502,9 @@ void hci_send_sco(struct hci_conn *conn,
 	struct hci_dev *hdev = conn->hdev;
 	struct hci_sco_hdr hdr;
 
+	if (WARN_ON(hci_dev_test_flag(hdev, HCI_UNREGISTER)))
+		return;
+
 	BT_DBG("%s len %d", hdev->name, skb->len);
 
 	hdr.handle = cpu_to_le16(conn->handle);

^ permalink raw reply

* Re: [PATCH v2] ARC: axs10x - add Ethernet PHY description in .dts
From: Alexey Brodkin @ 2016-03-17 12:10 UTC (permalink / raw)
  To: sergei.shtylyov@cogentembedded.com
  Cc: linux-kernel@vger.kernel.org, Vineet.Gupta1@synopsys.com,
	linux-snps-arc@lists.infradead.org, stable@vger.kernel.org,
	robh@kernel.org, preid@electromag.com.au, netdev@vger.kernel.org,
	davem@davemloft.net
In-Reply-To: <56EA9C0D.1090000@cogentembedded.com>

Hi Sergei,

On Thu, 2016-03-17 at 14:59 +0300, Sergei Shtylyov wrote:
> Hello.
> 
> On 3/17/2016 2:41 PM, Vineet Gupta wrote:
> 
> > 
> > > 
> > > > 
> > > > > 
> > > > > Following commit broke DW GMAC functionality on AXS10x boards:
> > > > > http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=e34d65696d2ef13dc32f2a162556c86c461e
> > > > > d763
> > > >      Note that scripts/checkpatch.pl now enforces certain format for citing
> > > > commits: commit <12-digit SHA1> ("<commit summary>").
> > 
> > > 
> > > Frankly I haven't run that patch through checkpatch due to patch
> > > simplicity.
> > > 
> > > But I'll try to not do any assumptions from now on and will try to
> > > use checkpatch for each and every thing I send :)
> > > 
> > > Thanks for spotting all his!
> > > 
>     Sorry for not reporting everything on the 1st review.
> 
> > 
> > > 
> > > -Alexey
> > Sergei, do you mind providing a Ack/Reviewed-by on the patch below
>  >
> 
> Reviewed-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
>     The patch here is white space damaged however: tabs were converted to 
> spaces. :-(

Well, I'm not really sure why that substitution happened because my local patch
is indeed with tabs.

That's an output of checkpatch:
------------------------>8------------------------
./scripts/checkpatch.pl 0001-ARC-axs10x-add-Ethernet-PHY-description-in-.dts.patch 
[1]+  Done                    gedit 0001-ARC-axs10x-add-Ethernet-PHY-description-in-.dts.patch
WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#7: 
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=e34d65696d2ef13dc32f2a162556c86c461ed763

ERROR: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit e34d65696d2e
("stmmac: create of compatible mdio bus for stmmac driver")'
#7: 
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=e34d65696d2ef13dc32f2a162556c86c461ed763

total: 1 errors, 1 warnings, 14 lines checked

0001-ARC-axs10x-add-Ethernet-PHY-description-in-.dts.patch has style problems, please review.
------------------------>8------------------------

If there were spaces we would see tons of:
------------------------>8------------------------
ERROR: code indent should use tabs where possible
#43: FILE: arch/arc/boot/dts/axs10x_mb.dtsi:50:
+                        mdio0 {$
------------------------>8------------------------
which I didn't see.

-Alexey

^ permalink raw reply

* Re: [PATCH v2] ARC: axs10x - add Ethernet PHY description in .dts
From: Vineet Gupta @ 2016-03-17 12:16 UTC (permalink / raw)
  To: Sergei Shtylyov, Alexey Brodkin
  Cc: robh@kernel.org, preid@electromag.com.au, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, stable@vger.kernel.org,
	linux-snps-arc@lists.infradead.org, davem@davemloft.net
In-Reply-To: <56EA9C0D.1090000@cogentembedded.com>

On Thursday 17 March 2016 05:29 PM, Sergei Shtylyov wrote:
> Hello.
>
> On 3/17/2016 2:41 PM, Vineet Gupta wrote:
>
>>>>> >>>> Following commit broke DW GMAC functionality on AXS10x boards:
>>>>> >>>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=e34d65696d2ef13dc32f2a162556c86c461ed763
>>>> >>>      Note that scripts/checkpatch.pl now enforces certain format for citing
>>>> >>> commits: commit <12-digit SHA1> ("<commit summary>").
>>> >> Frankly I haven't run that patch through checkpatch due to patch
>>> >> simplicity.
>>> >>
>>> >> But I'll try to not do any assumptions from now on and will try to
>>> >> use checkpatch for each and every thing I send :)
>>> >>
>>> >> Thanks for spotting all his!
>>> >>
>     Sorry for not reporting everything on the 1st review.
>
>>> >> -Alexey
>> >
>> > Sergei, do you mind providing a Ack/Reviewed-by on the patch below
>  >
>
> Reviewed-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

Thx.

>     The patch here is white space damaged however: tabs were converted to 
> spaces. :-(
>
> MBR, Sergei


That was just because of copy-paste into mailer before switching to plain-text.
Don't ask - corporate email !
I added it just to give u an idea of what fixup i did.

Tx,
-Vineet

^ permalink raw reply

* info, view the attach and get bark to me...
From: mr. david ibe @ 2016-03-17  9:08 UTC (permalink / raw)

In-Reply-To: <1424089410.18657.1458205633930.JavaMail.zimbra@rspp.co.id>

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



[-- Attachment #2: Untitled.png --]
[-- Type: image/png, Size: 91489 bytes --]

^ permalink raw reply

* [PATCH] iwlwifi: dvm: convert create_singlethread_workqueue() to alloc_workqueue()
From: Eva Rachel Retuya @ 2016-03-17 12:37 UTC (permalink / raw)
  To: outreachy-kernel
  Cc: johannes.berg, emmanuel.grumbach, linuxwifi, kvalo,
	linux-wireless, netdev, linux-kernel, tj, Eva Rachel Retuya

Use alloc_workqueue() to allocate the workqueue instead of
create_singlethread_workqueue() since the latter is deprecated and is scheduled
for removal.

There are work items doing related operations that shouldn't be swapped when
queued in a certain order hence preserve the strict execution ordering of a
single threaded (ST) workqueue by setting max_active to 1 and adding the
WQ_UNBOUND flag.

In addition, there are work items dealing with temperature throttling (tt_work,
ct_enter, ct_exit). Adding the WQ_HIGHPRI flag would place the work items in a
high priority workqueue.

Lastly, guarantee forward progress for work items depended upon during memory
reclaim by the addition of the WQ_MEM_RECLAIM flag.

Signed-off-by: Eva Rachel Retuya <eraretuya@gmail.com>
---
To the maintainers:
Just to confirm, are work items depended upon during memory reclaim? If not, the WQ_MEM_RECLAIM flag will be dropped. I added it just in case since create_singlethread_workqueue() also sets this flag.

 drivers/net/wireless/intel/iwlwifi/dvm/main.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/dvm/main.c b/drivers/net/wireless/intel/iwlwifi/dvm/main.c
index f62c2d7..37fa11c 100644
--- a/drivers/net/wireless/intel/iwlwifi/dvm/main.c
+++ b/drivers/net/wireless/intel/iwlwifi/dvm/main.c
@@ -1071,7 +1071,8 @@ static void iwl_bg_restart(struct work_struct *data)
 
 static void iwl_setup_deferred_work(struct iwl_priv *priv)
 {
-	priv->workqueue = create_singlethread_workqueue(DRV_NAME);
+	priv->workqueue = alloc_workqueue(DRV_NAME, WQ_HIGHPRI | WQ_UNBOUND |
+					  WQ_MEM_RECLAIM, 1);
 
 	INIT_WORK(&priv->restart, iwl_bg_restart);
 	INIT_WORK(&priv->beacon_update, iwl_bg_beacon_update);
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH] iwlwifi: dvm: convert create_singlethread_workqueue() to alloc_workqueue()
From: Johannes Berg @ 2016-03-17 12:43 UTC (permalink / raw)
  To: Eva Rachel Retuya, outreachy-kernel
  Cc: emmanuel.grumbach, linuxwifi, kvalo, linux-wireless, netdev,
	linux-kernel, tj
In-Reply-To: <1458218246-18807-1-git-send-email-eraretuya@gmail.com>

On Thu, 2016-03-17 at 20:37 +0800, Eva Rachel Retuya wrote:
> Use alloc_workqueue() to allocate the workqueue instead of
> create_singlethread_workqueue() since the latter is deprecated and is
> scheduled for removal.

Scheduled where?

>  static void iwl_setup_deferred_work(struct iwl_priv *priv)
>  {
> -	priv->workqueue = create_singlethread_workqueue(DRV_NAME);
> +	priv->workqueue = alloc_workqueue(DRV_NAME, WQ_HIGHPRI |
> WQ_UNBOUND |
> +					  WQ_MEM_RECLAIM, 1);

Seems like you should use alloc_ordered_workqueue() though? That also
gets you UNBOUND immediately, and the "1".

I'm not really sure HIGHPRI is needed either.

johannes

^ permalink raw reply

* RE: [PATCH] iwlwifi: dvm: convert create_singlethread_workqueue() to alloc_workqueue()
From: Grumbach, Emmanuel @ 2016-03-17 12:43 UTC (permalink / raw)
  To: Eva Rachel Retuya, outreachy-kernel@googlegroups.com
  Cc: Berg, Johannes, linuxwifi, kvalo@codeaurora.org,
	linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, tj@kernel.org
In-Reply-To: <1458218246-18807-1-git-send-email-eraretuya@gmail.com>

> 
> Use alloc_workqueue() to allocate the workqueue instead of
> create_singlethread_workqueue() since the latter is deprecated and is
> scheduled
> for removal.

I can't see any indication of that in the code of workqueue.
Can you share details about that?


> 
> There are work items doing related operations that shouldn't be swapped
> when
> queued in a certain order hence preserve the strict execution ordering of a
> single threaded (ST) workqueue by setting max_active to 1 and adding the
> WQ_UNBOUND flag.
> 
> In addition, there are work items dealing with temperature throttling
> (tt_work,
> ct_enter, ct_exit). Adding the WQ_HIGHPRI flag would place the work items
> in a
> high priority workqueue.
> 
> Lastly, guarantee forward progress for work items depended upon during
> memory
> reclaim by the addition of the WQ_MEM_RECLAIM flag.
> 
> Signed-off-by: Eva Rachel Retuya <eraretuya@gmail.com>
> ---
> To the maintainers:
> Just to confirm, are work items depended upon during memory reclaim? If
> not, the WQ_MEM_RECLAIM flag will be dropped. I added it just in case since
> create_singlethread_workqueue() also sets this flag.
> 
>  drivers/net/wireless/intel/iwlwifi/dvm/main.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/intel/iwlwifi/dvm/main.c
> b/drivers/net/wireless/intel/iwlwifi/dvm/main.c
> index f62c2d7..37fa11c 100644
> --- a/drivers/net/wireless/intel/iwlwifi/dvm/main.c
> +++ b/drivers/net/wireless/intel/iwlwifi/dvm/main.c
> @@ -1071,7 +1071,8 @@ static void iwl_bg_restart(struct work_struct *data)
> 
>  static void iwl_setup_deferred_work(struct iwl_priv *priv)
>  {
> -	priv->workqueue = create_singlethread_workqueue(DRV_NAME);
> +	priv->workqueue = alloc_workqueue(DRV_NAME, WQ_HIGHPRI |
> WQ_UNBOUND |
> +					  WQ_MEM_RECLAIM, 1);
> 
>  	INIT_WORK(&priv->restart, iwl_bg_restart);
>  	INIT_WORK(&priv->beacon_update, iwl_bg_beacon_update);
> --
> 1.9.1

^ permalink raw reply

* Re: [PATCH (net-next.git) 0/2] STMMAC: MDIO settings
From: Giuseppe CAVALLARO @ 2016-03-17 13:10 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, gabriel.fernandez, afaerber, fschaefer.oss, dinh.linux,
	preid
In-Reply-To: <20160316.180905.1210684680763742658.davem@davemloft.net>

On 3/16/2016 11:09 PM, David Miller wrote:
>
> Don't _ever_ send the same two sets of patches targetting net and
> net-next at the same time.

cover-letter reports the difference between the two patches for
net and next.

> Especially right now, as 'net' is basically frozen, and 'net-next' is
> closed for new features and is only for last minute bug fixes before I
> send a merge window pull request to Linus.

I do think that:

     [PATCH (net-next.git) 0/2] STMMAC: MDIO settings
     [PATCH (net-next.git)] stmmac: fix TX normal DESC

are last minute bug fixes for net-next.

Peppe

>
> I'm tossing both of these series, figure it how to do this properly.

^ permalink raw reply

* Re: [PATCH] iwlwifi: dvm: convert create_singlethread_workqueue() to alloc_workqueue()
From: tj-DgEjT+Ai2ygdnm+yROfE0A @ 2016-03-17 13:18 UTC (permalink / raw)
  To: Grumbach, Emmanuel
  Cc: Eva Rachel Retuya,
	outreachy-kernel-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org,
	Berg, Johannes, linuxwifi,
	kvalo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <0BA3FCBA62E2DC44AF3030971E174FB32EA7C435-Jy8z56yoSI9wl47ZQwxUxrfspsVTdybXVpNB7YpNyf8@public.gmane.org>

Hello,

On Thu, Mar 17, 2016 at 12:43:35PM +0000, Grumbach, Emmanuel wrote:
> > 
> > Use alloc_workqueue() to allocate the workqueue instead of
> > create_singlethread_workqueue() since the latter is deprecated and is
> > scheduled
> > for removal.
> 
> I can't see any indication of that in the code of workqueue.
> Can you share details about that?

create*_workqueue()'s have been deprecated and replaced by
alloc*_workqueue()'s for some years now.  I probably should note that
in the header file.

Thanks.

-- 
tejun
--
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

* Re: [PATCH] iwlwifi: dvm: convert create_singlethread_workqueue() to alloc_workqueue()
From: Tejun Heo @ 2016-03-17 13:21 UTC (permalink / raw)
  To: Johannes Berg
  Cc: Eva Rachel Retuya, outreachy-kernel-/JYPxA39Uh5TLH3MbocFFw,
	emmanuel.grumbach-ral2JQCrhuEAvxtiuMwx3w,
	linuxwifi-ral2JQCrhuEAvxtiuMwx3w, kvalo-sgV2jX0FEOL9JmXXK+q4OQ,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1458218602.2158.17.camel-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>

Hello,

On Thu, Mar 17, 2016 at 01:43:22PM +0100, Johannes Berg wrote:
> On Thu, 2016-03-17 at 20:37 +0800, Eva Rachel Retuya wrote:
> > Use alloc_workqueue() to allocate the workqueue instead of
> > create_singlethread_workqueue() since the latter is deprecated and is
> > scheduled for removal.
> 
> Scheduled where?

They've been deprecated for years now.  I should note that in the
header.

> >  static void iwl_setup_deferred_work(struct iwl_priv *priv)
> >  {
> > -	priv->workqueue = create_singlethread_workqueue(DRV_NAME);
> > +	priv->workqueue = alloc_workqueue(DRV_NAME, WQ_HIGHPRI |
> > WQ_UNBOUND |
> > +					  WQ_MEM_RECLAIM, 1);
> 
> Seems like you should use alloc_ordered_workqueue() though? That also
> gets you UNBOUND immediately, and the "1".

Right, this one should have been alloc_ordered_workqueue().

> I'm not really sure HIGHPRI is needed either.

So, no WQ_MEM_RECLAIM either then, I suppose?  What are the latency
requirements here - what happens if a thermal management work gets
delayed?

Thanks.

-- 
tejun
--
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

* Re: [PATCH] net: phy: fix PHY_RUNNING in phy_state_machine
From: Andrew Lunn @ 2016-03-17 13:41 UTC (permalink / raw)
  To: Yegor Yefremov
  Cc: shaohui ???, netdev, David Miller, Shaohui Xie, Florian Fainelli,
	N, Mugunthan V, drivshin
In-Reply-To: <CAGm1_kv7eia4LPWT5TxUJp1NV+eFZj6gBQc-pHwxbygoF03EYg@mail.gmail.com>

On Thu, Mar 17, 2016 at 09:14:17AM +0100, Yegor Yefremov wrote:
> On Thu, Mar 17, 2016 at 12:05 AM, Andrew Lunn <andrew@lunn.ch> wrote:
> > On Wed, Mar 16, 2016 at 11:23:59PM +0100, Yegor Yefremov wrote:
> >> Hi Andrew,
> >>
> >> On Wed, Mar 16, 2016 at 5:18 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> >> > On Wed, Mar 16, 2016 at 04:59:23PM +0100, Yegor Yefremov wrote:
> >> >
> >> >> This patch breaks my am335x based board, where one of the CPSW slaves
> >> >> is connected to IP175D switch chip via RMII interface. Since this
> >> >> patch packet reception is not working.
> >> >
> >> > Hi Yegor
> >> >
> >> > Which phy is causing the problem? A PHY inside the switch?
> >> >
> >> > Do you have two back to back PHYs between the MAC and the switch, or
> >> > is the CPSW RMII connected directly to the switch?
> >>
> >> CPSW RMII is connected directly to the switch.
> >
> > So which PHY is causing you problems?

Hi Yegor

Thanks for the details. This helps explain what is going on.

I'm looking at:

http://www.icplus.com.tw/pp-IP175D.html

> First of all this is the system in question [1]. am335x CPSW has two
> slaves and in this particular configuration CPSW is working in Dual
> EMAC mode, so that both slaves are independent interfaces eth0 and
> eth1.
> 
> eth1 is connected to Atheros 8035 PHY via RGMII channel and is working
> as expected. eth0 is connected to ICPlus IP175D via RMII interface, so
> from CPSW point of view ICPlus IP175D is just an ordinary PHY.  Both
> Atheros 8035 and ICPlus IP175D are connected via MDIO, so that both of
> them will be detected at runtime:
> 
> davinci_mdio 4a101000.mdio: davinci mdio revision 1.6
> davinci_mdio 4a101000.mdio: detected phy mask f00fff00
> Atheros 8035 ethernet 4a101000.mdio:07: GPIO lookup for consumer reset
> Atheros 8035 ethernet 4a101000.mdio:07: using lookup tables for GPIO lookup
> Atheros 8035 ethernet 4a101000.mdio:07: lookup for GPIO reset failed
> libphy: 4a101000.mdio: probed
> davinci_mdio 4a101000.mdio: phy[0]: device 4a101000.mdio:00, driver
> ICPlus IP175C
> davinci_mdio 4a101000.mdio: phy[1]: device 4a101000.mdio:01, driver
> ICPlus IP175C
> davinci_mdio 4a101000.mdio: phy[2]: device 4a101000.mdio:02, driver
> ICPlus IP175C
> davinci_mdio 4a101000.mdio: phy[3]: device 4a101000.mdio:03, driver
> ICPlus IP175C
> davinci_mdio 4a101000.mdio: phy[4]: device 4a101000.mdio:04, driver
> ICPlus IP175C

So here we see the 5 PHYs connected to the switch. What we don't see
is what phy it connected to eth0. Since there is no PHY connected to
eth0, you should have a fixed_link node in your device tree.

I assume you are using am335x-baltos-ir5221.dts?

&cpsw_emac0 {
        phy_id = <&davinci_mdio>, <0>;
        phy-mode = "rmii";
        dual_emac_res_vlan = <1>;
};

I'm assuming this means use the PHY at address 0 on the MDIO bus. This
is your problem. You have logically connected PHY0 to the eth0. So if
PHY0 is down, the MAC logically has no carrier. Hence you don't see
any packets. You should be able to quickly prove this. See what
happens when you connect a peer to port0 so the link comes up.

In reality, your hardware does not have a PHY connected to the MAC. It
goes straight to the switch. So you should be using a fixed-link here.

Try something like:

&cpsw_emac0 {
	ixed-link {
        	speed = <100>;
        	full-duplex;
        };

        phy-mode = "rmii";
        dual_emac_res_vlan = <1>;
};

	Andrew

^ permalink raw reply

* RE: [PATCH] iwlwifi: dvm: convert create_singlethread_workqueue() to alloc_workqueue()
From: Grumbach, Emmanuel @ 2016-03-17 13:48 UTC (permalink / raw)
  To: Tejun Heo, Johannes Berg
  Cc: Eva Rachel Retuya, outreachy-kernel@googlegroups.com, linuxwifi,
	kvalo@codeaurora.org, linux-wireless@vger.kernel.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20160317132110.GP21104@mtj.duckdns.org>

> Hello,
> 
> On Thu, Mar 17, 2016 at 01:43:22PM +0100, Johannes Berg wrote:
> > On Thu, 2016-03-17 at 20:37 +0800, Eva Rachel Retuya wrote:
> > > Use alloc_workqueue() to allocate the workqueue instead of
> > > create_singlethread_workqueue() since the latter is deprecated and
> > > is scheduled for removal.
> >
> > Scheduled where?
> 
> They've been deprecated for years now.  I should note that in the header.
> 
> > >  static void iwl_setup_deferred_work(struct iwl_priv *priv)
> > >  {
> > > -	priv->workqueue = create_singlethread_workqueue(DRV_NAME);
> > > +	priv->workqueue = alloc_workqueue(DRV_NAME, WQ_HIGHPRI |
> > > WQ_UNBOUND |
> > > +					  WQ_MEM_RECLAIM, 1);
> >
> > Seems like you should use alloc_ordered_workqueue() though? That also
> > gets you UNBOUND immediately, and the "1".
> 
> Right, this one should have been alloc_ordered_workqueue().
> 
> > I'm not really sure HIGHPRI is needed either.
> 
> So, no WQ_MEM_RECLAIM either then, I suppose?  What are the latency
> requirements here - what happens if a thermal management work gets
> delayed?
> 

This worker is not supposed to free memory, so no WQ_MEM_RECLAIM needed. The latency is not critical.

^ permalink raw reply

* [PATCH] Add support to control PHY LEDs
From: Vishal Thanki @ 2016-03-17 13:59 UTC (permalink / raw)
  To: ujhelyi.m, f.fainelli, netdev; +Cc: Vishal Thanki

Hi all,

We had a requirement to disable the PHY link LEDs during system
standby mode without turning off the ethernet module. To achieve that,
I have prepared the sysfs interface turn on/off the PHY link LEDs.

Please suggest if there is any better way to achieve this.

Thanks,
Vishal

Vishal Thanki (1):
  net: phy: at803x: Add support to control PHY LEDs

 drivers/net/phy/Kconfig  |  7 +++++
 drivers/net/phy/at803x.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 78 insertions(+)

-- 
2.4.3

^ permalink raw reply

* [PATCH] net: phy: at803x: Add support to control PHY LEDs
From: Vishal Thanki @ 2016-03-17 13:59 UTC (permalink / raw)
  To: ujhelyi.m, f.fainelli, netdev; +Cc: Vishal Thanki
In-Reply-To: <1458223147-16142-1-git-send-email-vishalthanki@gmail.com>

The LEDs can be turned on and off by a sysfs interface
now. Write 0 to "led_enable" file to turn off the LEDs
and write 1 to turn on. The support is experimental
and can be enabled by kernel configuration option.

Signed-off-by: Vishal Thanki <vishalthanki@gmail.com>
---
 drivers/net/phy/Kconfig  |  7 +++++
 drivers/net/phy/at803x.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 78 insertions(+)

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 075a4cc..16f78cf 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -24,6 +24,13 @@ config AT803X_PHY
 	---help---
 	  Currently supports the AT8030 and AT8035 model
 
+config AT803X_PHY_LED_CONTROL
+	depends on AT803X_PHY
+	bool "Support for PHY LED control (Experimental)"
+	---help---
+	  This option enables a sysfs interface to turn on and off the link
+	  LEDs attached to phy.
+
 config AMD_PHY
 	tristate "Drivers for the AMD PHYs"
 	---help---
diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index 1e901c7..a606d52 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -59,12 +59,19 @@
 #define ATH8031_PHY_ID 0x004dd074
 #define ATH8035_PHY_ID 0x004dd072
 
+#ifdef CONFIG_AT803X_PHY_LED_CONTROL
+#define LED_OFF		0xF71F
+#endif
+
 MODULE_DESCRIPTION("Atheros 803x PHY driver");
 MODULE_AUTHOR("Matus Ujhelyi");
 MODULE_LICENSE("GPL");
 
 struct at803x_priv {
 	bool phy_reset:1;
+#ifdef CONFIG_AT803X_PHY_LED_CONTROL
+	int led_status;
+#endif
 	struct gpio_desc *gpiod_reset;
 };
 
@@ -267,6 +274,42 @@ done:
 	return 0;
 }
 
+#ifdef CONFIG_AT803X_PHY_LED_CONTROL
+static ssize_t at803x_led_get(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct phy_device *phydev = dev_get_drvdata(dev);
+	int read = phy_read(phydev, AT803X_LED_CONTROL);
+
+	read = (read == LED_OFF) ? 0 : 1;
+	return sprintf(buf, "%x\n", read);
+}
+
+static ssize_t at803x_led_set(struct device *dev,
+			       struct device_attribute *attr,
+			       const char *buf, size_t count)
+{
+	struct phy_device *phydev = dev_get_drvdata(dev);
+	struct at803x_priv *priv = phydev->priv;
+	int val;
+
+	if (!strncmp(buf, "0", 1)) {
+		priv->led_status = phy_read(phydev, AT803X_LED_CONTROL);
+		val = LED_OFF;
+	} else if (!strncmp(buf, "1", 1)) {
+		val = priv->led_status;
+	} else {
+		return -EINVAL;
+	}
+
+	phy_write(phydev, AT803X_LED_CONTROL, val);
+
+	return count;
+}
+
+static DEVICE_ATTR(led_enable, 0600, at803x_led_get, at803x_led_set);
+#endif
+
 static int at803x_probe(struct phy_device *phydev)
 {
 	struct device *dev = &phydev->mdio.dev;
@@ -285,12 +328,31 @@ static int at803x_probe(struct phy_device *phydev)
 
 	phydev->priv = priv;
 
+#ifdef CONFIG_AT803X_PHY_LED_CONTROL
+	dev_set_drvdata(dev, phydev);
+	return device_create_file(dev, &dev_attr_led_enable);
+#else
 	return 0;
+#endif
+}
+
+#ifdef CONFIG_AT803X_PHY_LED_CONTROL
+static void at803x_remove(struct phy_device *phydev)
+{
+	struct device *dev = &phydev->mdio.dev;
+	device_remove_file(dev, &dev_attr_led_enable);
 }
+#endif
 
 static int at803x_config_init(struct phy_device *phydev)
 {
 	int ret;
+#ifdef CONFIG_AT803X_PHY_LED_CONTROL
+	struct at803x_priv *priv;
+
+	priv = phydev->priv;
+	priv->led_status = phy_read(phydev, AT803X_LED_CONTROL);
+#endif
 
 	ret = genphy_config_init(phydev);
 	if (ret < 0)
@@ -396,6 +458,9 @@ static struct phy_driver at803x_driver[] = {
 	.flags			= PHY_HAS_INTERRUPT,
 	.config_aneg		= genphy_config_aneg,
 	.read_status		= genphy_read_status,
+#ifdef CONFIG_AT803X_PHY_LED_CONTROL
+	.remove			= at803x_remove,
+#endif
 	.ack_interrupt		= at803x_ack_interrupt,
 	.config_intr		= at803x_config_intr,
 }, {
@@ -414,6 +479,9 @@ static struct phy_driver at803x_driver[] = {
 	.flags			= PHY_HAS_INTERRUPT,
 	.config_aneg		= genphy_config_aneg,
 	.read_status		= genphy_read_status,
+#ifdef CONFIG_AT803X_PHY_LED_CONTROL
+	.remove			= at803x_remove,
+#endif
 	.ack_interrupt		= at803x_ack_interrupt,
 	.config_intr		= at803x_config_intr,
 }, {
@@ -432,6 +500,9 @@ static struct phy_driver at803x_driver[] = {
 	.flags			= PHY_HAS_INTERRUPT,
 	.config_aneg		= genphy_config_aneg,
 	.read_status		= genphy_read_status,
+#ifdef CONFIG_AT803X_PHY_LED_CONTROL
+	.remove			= at803x_remove,
+#endif
 	.ack_interrupt		= &at803x_ack_interrupt,
 	.config_intr		= &at803x_config_intr,
 } };
-- 
2.4.3

^ permalink raw reply related

* Extreme slowness in IPIP tunnel when routing through kernel 3.18 and later
From: Patrick Boutilier @ 2016-03-17 14:02 UTC (permalink / raw)
  To: netdev

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

I have an IPIP tunnel setup between two hosts in different buildings. 
The Linux router they route through causes extreme slowness in the 
tunnel when running kernels from 3.18 on . tcpdump shows many cksum 
errors which don't show up in the 3.17 and earlier kernels. Speed goes 
back to normal when rx-checksumming and tx-checksumming are turned off 
using ethtool on the Linux router . Would this be an effect of bulk 
network packet transmission that was introduced in 3.18 or some other issue?


eth2
driver: igb
version: 5.2.15-k
firmware-version: 1.5.1
bus-info: 0000:06:00.0
supports-statistics: yes
supports-test: yes
supports-eeprom-access: yes
supports-register-dump: yes
supports-priv-flags: no


eth6
driver: ixgbe
version: 3.19.1-k
firmware-version: 0x80000389
bus-info: 0000:41:00.0
supports-statistics: yes
supports-test: yes
supports-eeprom-access: yes
supports-register-dump: yes
supports-priv-flags: no





Example of cksum from tcpdump:

Flags [.], cksum 0x2237 (incorrect -> 0xbb02), seq 424119200:424120648, 
ack 1, win 115, options [nop,nop,TS val 303173784 ecr 3971495454], 
length 1448





Results of nuttcp test with rx-checksumming and tx-checksumming on:


Testing upload inside tunnel



     0.0625 MB /   5.00 sec =    0.1048 Mbps    79 retrans
     0.0625 MB /   5.00 sec =    0.1048 Mbps    59 retrans
     0.0625 MB /   5.00 sec =    0.1049 Mbps    27 retrans

     0.2096 MB /  16.11 sec =    0.1092 Mbps 0 %TX 0 %RX 186 retrans 
0.47 msRTT



Testing download inside tunnel



     0.5000 MB /   5.00 sec =    0.8389 Mbps   112 retrans
     2.6250 MB /   5.00 sec =    4.4040 Mbps   412 retrans
     0.8750 MB /   5.00 sec =    1.4680 Mbps   156 retrans

     4.0243 MB /  15.48 sec =    2.1809 Mbps 0 %TX 0 %RX 705 retrans 
0.47 msRTT


Testing upload outside tunnel



   423.0000 MB /   5.00 sec =  709.4949 Mbps   275 retrans
   442.0625 MB /   5.00 sec =  741.7008 Mbps    53 retrans
   418.5625 MB /   5.00 sec =  702.1329 Mbps   133 retrans

  1287.0080 MB /  15.08 sec =  715.9252 Mbps 1 %TX 30 %RX 461 retrans 
0.43 msRTT



Testing download outside tunnel



   552.4375 MB /   5.00 sec =  926.8347 Mbps    26 retrans
   555.1875 MB /   5.00 sec =  931.4499 Mbps    17 retrans
   553.0000 MB /   5.00 sec =  927.7802 Mbps    16 retrans

  1664.2149 MB /  15.03 sec =  928.6737 Mbps 9 %TX 5 %RX 60 retrans 0.46 
msRTT






Results of nuttcp test with rx-checksumming and tx-checksumming off:



Testing upload inside tunnel



   440.0000 MB /   5.00 sec =  738.0112 Mbps   117 retrans
   433.0000 MB /   5.00 sec =  726.4892 Mbps   135 retrans
   447.9375 MB /   5.00 sec =  751.5714 Mbps   112 retrans

  1321.0069 MB /  15.04 sec =  736.7213 Mbps 3 %TX 21 %RX 364 retrans 
0.51 msRTT



Testing download inside tunnel



   494.0625 MB /   5.00 sec =  828.8973 Mbps   335 retrans
   471.0000 MB /   5.00 sec =  790.2050 Mbps   219 retrans
   469.8750 MB /   5.00 sec =  788.3174 Mbps   220 retrans

  1435.1875 MB /  15.00 sec =  802.4203 Mbps 89 %TX 12 %RX 774 retrans 
0.46 msRTT


Testing upload outside tunnel



   431.2500 MB /   5.00 sec =  723.2808 Mbps    33 retrans
   446.3750 MB /   5.00 sec =  748.9268 Mbps    10 retrans
   475.4375 MB /   5.00 sec =  797.7044 Mbps     0 retrans

  1355.0834 MB /  15.06 sec =  754.6438 Mbps 1 %TX 33 %RX 43 retrans 
0.53 msRTT



Testing download outside tunnel



   424.8125 MB /   5.00 sec =  712.7061 Mbps   102 retrans
   456.5625 MB /   5.00 sec =  765.9872 Mbps    80 retrans
   454.0000 MB /   5.00 sec =  761.6935 Mbps   116 retrans

  1335.7823 MB /  15.01 sec =  746.6959 Mbps 4 %TX 5 %RX 298 retrans 
0.55 msRTT

[-- Attachment #2: boutilpj.vcf --]
[-- Type: text/x-vcard, Size: 286 bytes --]

begin:vcard
fn:Patrick Boutilier
n:Boutilier;Patrick
org:;Nova Scotia Department of Education
adr:;;2021 Brunswick Street;Halifax;NS;B3K 2Y5;Canada
email;internet:boutilpj@ednet.ns.ca
title:WAN Communications Specialist
tel;work:902-424-6800
tel;fax:902-424-0874
version:2.1
end:vcard


^ permalink raw reply

* [PATCH net-next] virtio_net: replace netdev_alloc_skb_ip_align() with napi_alloc_skb()
From: Paolo Abeni @ 2016-03-17 14:44 UTC (permalink / raw)
  To: netdev; +Cc: virtualization, Hannes Frederic Sowa, Michael S. Tsirkin

This gives small but noticeable rx performance improvement (2-3%)
and will allow exploiting future napi improvement.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 drivers/net/virtio_net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index fb0eae4..49d84e5 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -260,7 +260,7 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
 	p = page_address(page) + offset;
 
 	/* copy small packet so we can reuse these pages for small data */
-	skb = netdev_alloc_skb_ip_align(vi->dev, GOOD_COPY_LEN);
+	skb = napi_alloc_skb(&rq->napi, GOOD_COPY_LEN);
 	if (unlikely(!skb))
 		return NULL;
 
-- 
1.8.3.1

^ permalink raw reply related

* Re: [PATCH] net: phy: at803x: Add support to control PHY LEDs
From: Andrew Lunn @ 2016-03-17 14:50 UTC (permalink / raw)
  To: Vishal Thanki; +Cc: ujhelyi.m, f.fainelli, netdev
In-Reply-To: <1458223147-16142-2-git-send-email-vishalthanki@gmail.com>

On Thu, Mar 17, 2016 at 02:59:07PM +0100, Vishal Thanki wrote:
> The LEDs can be turned on and off by a sysfs interface
> now. Write 0 to "led_enable" file to turn off the LEDs
> and write 1 to turn on. The support is experimental
> and can be enabled by kernel configuration option.
> 
> Signed-off-by: Vishal Thanki <vishalthanki@gmail.com>
> ---
>  drivers/net/phy/Kconfig  |  7 +++++
>  drivers/net/phy/at803x.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++

Hi Vishal

This solution seems specific to the at803. You should be thinking of a
generic solution that all PHYs can use. eg add a new callback function
to phy_driver, and put the sysfs handling code in phylib.

   Andrew

^ permalink raw reply

* Re: [PATCH] net: phy: fix PHY_RUNNING in phy_state_machine
From: Yegor Yefremov @ 2016-03-17 14:50 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: shaohui ???, netdev, David Miller, Shaohui Xie, Florian Fainelli,
	N, Mugunthan V, drivshin
In-Reply-To: <20160317134134.GA24913@lunn.ch>

On Thu, Mar 17, 2016 at 2:41 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> On Thu, Mar 17, 2016 at 09:14:17AM +0100, Yegor Yefremov wrote:
>> On Thu, Mar 17, 2016 at 12:05 AM, Andrew Lunn <andrew@lunn.ch> wrote:
>> > On Wed, Mar 16, 2016 at 11:23:59PM +0100, Yegor Yefremov wrote:
>> >> Hi Andrew,
>> >>
>> >> On Wed, Mar 16, 2016 at 5:18 PM, Andrew Lunn <andrew@lunn.ch> wrote:
>> >> > On Wed, Mar 16, 2016 at 04:59:23PM +0100, Yegor Yefremov wrote:
>> >> >
>> >> >> This patch breaks my am335x based board, where one of the CPSW slaves
>> >> >> is connected to IP175D switch chip via RMII interface. Since this
>> >> >> patch packet reception is not working.
>> >> >
>> >> > Hi Yegor
>> >> >
>> >> > Which phy is causing the problem? A PHY inside the switch?
>> >> >
>> >> > Do you have two back to back PHYs between the MAC and the switch, or
>> >> > is the CPSW RMII connected directly to the switch?
>> >>
>> >> CPSW RMII is connected directly to the switch.
>> >
>> > So which PHY is causing you problems?
>
> Hi Yegor
>
> Thanks for the details. This helps explain what is going on.
>
> I'm looking at:
>
> http://www.icplus.com.tw/pp-IP175D.html
>
>> First of all this is the system in question [1]. am335x CPSW has two
>> slaves and in this particular configuration CPSW is working in Dual
>> EMAC mode, so that both slaves are independent interfaces eth0 and
>> eth1.
>>
>> eth1 is connected to Atheros 8035 PHY via RGMII channel and is working
>> as expected. eth0 is connected to ICPlus IP175D via RMII interface, so
>> from CPSW point of view ICPlus IP175D is just an ordinary PHY.  Both
>> Atheros 8035 and ICPlus IP175D are connected via MDIO, so that both of
>> them will be detected at runtime:
>>
>> davinci_mdio 4a101000.mdio: davinci mdio revision 1.6
>> davinci_mdio 4a101000.mdio: detected phy mask f00fff00
>> Atheros 8035 ethernet 4a101000.mdio:07: GPIO lookup for consumer reset
>> Atheros 8035 ethernet 4a101000.mdio:07: using lookup tables for GPIO lookup
>> Atheros 8035 ethernet 4a101000.mdio:07: lookup for GPIO reset failed
>> libphy: 4a101000.mdio: probed
>> davinci_mdio 4a101000.mdio: phy[0]: device 4a101000.mdio:00, driver
>> ICPlus IP175C
>> davinci_mdio 4a101000.mdio: phy[1]: device 4a101000.mdio:01, driver
>> ICPlus IP175C
>> davinci_mdio 4a101000.mdio: phy[2]: device 4a101000.mdio:02, driver
>> ICPlus IP175C
>> davinci_mdio 4a101000.mdio: phy[3]: device 4a101000.mdio:03, driver
>> ICPlus IP175C
>> davinci_mdio 4a101000.mdio: phy[4]: device 4a101000.mdio:04, driver
>> ICPlus IP175C
>
> So here we see the 5 PHYs connected to the switch. What we don't see
> is what phy it connected to eth0. Since there is no PHY connected to
> eth0, you should have a fixed_link node in your device tree.
>
> I assume you are using am335x-baltos-ir5221.dts?
>
> &cpsw_emac0 {
>         phy_id = <&davinci_mdio>, <0>;
>         phy-mode = "rmii";
>         dual_emac_res_vlan = <1>;
> };
>
> I'm assuming this means use the PHY at address 0 on the MDIO bus. This
> is your problem. You have logically connected PHY0 to the eth0. So if
> PHY0 is down, the MAC logically has no carrier. Hence you don't see
> any packets. You should be able to quickly prove this. See what
> happens when you connect a peer to port0 so the link comes up.
>
> In reality, your hardware does not have a PHY connected to the MAC. It
> goes straight to the switch. So you should be using a fixed-link here.
>
> Try something like:
>
> &cpsw_emac0 {
>         ixed-link {
>                 speed = <100>;
>                 full-duplex;
>         };
>
>         phy-mode = "rmii";
>         dual_emac_res_vlan = <1>;
> };

After changing cpsw_emac0 entry to:

&cpsw_emac0 {
        phy-mode = "rmii";
        dual_emac_res_vlan = <1>;
        fixed-link {
                speed = <100>;
                full-duplex;
        };
};

I've got packets running in both directions.

Now I have another problem: I cannot disable ICPlus IP175D ports via
SIOCSMIIREG as I could do previously. I get not ioctl errors [1], but
the ports are always on.

[1] https://github.com/visionsystemsgmbh/libonrisc/blob/master/src/onrisc.c#L83

^ permalink raw reply

* Re: [PATCH] Add support to control PHY LEDs
From: Andrew Lunn @ 2016-03-17 14:54 UTC (permalink / raw)
  To: Vishal Thanki; +Cc: ujhelyi.m, f.fainelli, netdev
In-Reply-To: <1458223147-16142-1-git-send-email-vishalthanki@gmail.com>

On Thu, Mar 17, 2016 at 02:59:06PM +0100, Vishal Thanki wrote:
> Hi all,
> 
> We had a requirement to disable the PHY link LEDs during system
> standby mode without turning off the ethernet module. To achieve that,
> I have prepared the sysfs interface turn on/off the PHY link LEDs.

Hi Vishal

Could you implement this in at803x_suspend()?

      Andrew

^ permalink raw reply

* Re: [PATCH] net: phy: fix PHY_RUNNING in phy_state_machine
From: Andrew Lunn @ 2016-03-17 15:12 UTC (permalink / raw)
  To: Yegor Yefremov
  Cc: shaohui ???, netdev, David Miller, Shaohui Xie, Florian Fainelli,
	N, Mugunthan V, drivshin
In-Reply-To: <CAGm1_kvJn0Pq38iqOmOcyNpe62EEUmq_s5G2_HXps4Gx+xB_=Q@mail.gmail.com>

> After changing cpsw_emac0 entry to:
> 
> &cpsw_emac0 {
>         phy-mode = "rmii";
>         dual_emac_res_vlan = <1>;
>         fixed-link {
>                 speed = <100>;
>                 full-duplex;
>         };
> };
> 
> I've got packets running in both directions.

Great.
 
> Now I have another problem: I cannot disable ICPlus IP175D ports via
> SIOCSMIIREG as I could do previously. I get not ioctl errors [1], but
> the ports are always on.
> 
> [1] https://github.com/visionsystemsgmbh/libonrisc/blob/master/src/onrisc.c#L83

The MDIO bus is now logically not connected to eth0. Instead you have
the fixed-link mdio device connected to eth0. So SIOCSMIIREG calls on
eth0 now try to manipulate the fixed link phys.

However, i think you can still access the MDIO bus, just use eth1 in
your ioctl call.

You should however consider writing a DSA driver for the switch.

    Andrew

^ permalink raw reply

* Re: [PATCH] net: phy: fix PHY_RUNNING in phy_state_machine
From: Yegor Yefremov @ 2016-03-17 15:21 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: shaohui ???, netdev, David Miller, Shaohui Xie, Florian Fainelli,
	N, Mugunthan V, drivshin
In-Reply-To: <20160317151240.GD26019@lunn.ch>

On Thu, Mar 17, 2016 at 4:12 PM, Andrew Lunn <andrew@lunn.ch> wrote:
>> After changing cpsw_emac0 entry to:
>>
>> &cpsw_emac0 {
>>         phy-mode = "rmii";
>>         dual_emac_res_vlan = <1>;
>>         fixed-link {
>>                 speed = <100>;
>>                 full-duplex;
>>         };
>> };
>>
>> I've got packets running in both directions.
>
> Great.
>
>> Now I have another problem: I cannot disable ICPlus IP175D ports via
>> SIOCSMIIREG as I could do previously. I get not ioctl errors [1], but
>> the ports are always on.
>>
>> [1] https://github.com/visionsystemsgmbh/libonrisc/blob/master/src/onrisc.c#L83
>
> The MDIO bus is now logically not connected to eth0. Instead you have
> the fixed-link mdio device connected to eth0. So SIOCSMIIREG calls on
> eth0 now try to manipulate the fixed link phys.
>
> However, i think you can still access the MDIO bus, just use eth1 in
> your ioctl call.

Good trick :-)

> You should however consider writing a DSA driver for the switch.

Do you mean SWITCHDEV or is this more or less the same? From time to
time I'm looking at DSA/switchdev patches in the mailing list, but
there seems to be not so many example in kernel. What are the latest
slides, papers aside from Documentation/networking/switchdev.txt?

Yegor

^ permalink raw reply

* Re: [PATCH net] vlan: propagate gso_max_segs
From: Eric Dumazet @ 2016-03-17 15:26 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <1458190789.7353.33.camel@edumazet-glaptop3.roam.corp.google.com>

On Wed, 2016-03-16 at 21:59 -0700, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
> 
> vlan drivers lack proper propagation of gso_max_segs from
> lower device.
> 

BTW, I suspect bridge code needs an update too.

This might be the time to add IFLA_MAX_GSO_SEGS and IFLA_MAX_GSO_SIZE to
help debugging these issues with "ip link show"

^ permalink raw reply

* Re: [PATCH] net: phy: fix PHY_RUNNING in phy_state_machine
From: Andrew Lunn @ 2016-03-17 15:28 UTC (permalink / raw)
  To: Yegor Yefremov
  Cc: shaohui ???, netdev, David Miller, Shaohui Xie, Florian Fainelli,
	N, Mugunthan V, drivshin
In-Reply-To: <CAGm1_kt+d7HooYi5JgHvkVYEKO0JLn=1nBgdUd1bSG08+ZLiRg@mail.gmail.com>

> > You should however consider writing a DSA driver for the switch.
> 
> Do you mean SWITCHDEV or is this more or less the same? From time to
> time I'm looking at DSA/switchdev patches in the mailing list, but
> there seems to be not so many example in kernel. What are the latest
> slides, papers aside from Documentation/networking/switchdev.txt?

I don't have access to the datasheet for this device. So i've no idea
how easy/hard it would be.

Documentation/networking/switchdev.txt and
Documentation/networking/dsa/dsa.txt would be a good place to start.

The mv88e6060.c is the simplest driver and gives you the minimum you
need to start with. Looking at the marketing brief, it looks like the
device can do more. But it is best to start simple, get the minimal
accepted, and then incrementally add more.

	  Andrew

^ permalink raw reply

* Re: [PATCH] net: phy: fix PHY_RUNNING in phy_state_machine
From: Yegor Yefremov @ 2016-03-17 15:33 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: shaohui ???, netdev, David Miller, Shaohui Xie, Florian Fainelli,
	N, Mugunthan V, drivshin
In-Reply-To: <20160317152836.GE26019@lunn.ch>

On Thu, Mar 17, 2016 at 4:28 PM, Andrew Lunn <andrew@lunn.ch> wrote:
>> > You should however consider writing a DSA driver for the switch.
>>
>> Do you mean SWITCHDEV or is this more or less the same? From time to
>> time I'm looking at DSA/switchdev patches in the mailing list, but
>> there seems to be not so many example in kernel. What are the latest
>> slides, papers aside from Documentation/networking/switchdev.txt?
>
> I don't have access to the datasheet for this device. So i've no idea
> how easy/hard it would be.
>
> Documentation/networking/switchdev.txt and
> Documentation/networking/dsa/dsa.txt would be a good place to start.
>
> The mv88e6060.c is the simplest driver and gives you the minimum you
> need to start with. Looking at the marketing brief, it looks like the
> device can do more. But it is best to start simple, get the minimal
> accepted, and then incrementally add more.

Will do. Thanks.

Yegor

^ permalink raw reply

* Re: [PATCH] rtlwifi: fix gcc-6 indentation warning
From: Larry Finger @ 2016-03-17 16:09 UTC (permalink / raw)
  To: Arnd Bergmann, Chaoming Li, Kalle Valo
  Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <1458209066-3979295-1-git-send-email-arnd@arndb.de>

On 03/17/2016 05:03 AM, Arnd Bergmann wrote:
> The rtl8821ae_dm_txpower_tracking_callback_thermalmeter function
> contains a call to RT_TRACE() that is indented in a misleading
> way, as pointed out by a gcc-6 warning:
>
> drivers/net/wireless/realtek/rtlwifi/rtl8821ae/dm.c: In function 'rtl8821ae_dm_txpower_tracking_callback_thermalmeter':
> drivers/net/wireless/realtek/rtlwifi/rtl8821ae/dm.c:2491:4: error: statement is indented as if it were guarded by...
>      RT_TRACE(rtlpriv, COMP_POWER_TRACKING, DBG_LOUD,
>      ^~~~~~~~
> drivers/net/wireless/realtek/rtlwifi/rtl8821ae/dm.c:2488:3: note: ...this 'for' clause, but it is not
>     for (p = RF90_PATH_A; p < MAX_PATH_NUM_8821A; p++)
>     ^~~
>
> It is clear from the context that the call was not meant to be
> part of the loop and only the indentation is wrong, so this
> removes the extra tabs.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>   drivers/net/wireless/realtek/rtlwifi/rtl8821ae/dm.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/dm.c b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/dm.c
> index 95dcbff4673b..6a8245c4ea48 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/dm.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/dm.c
> @@ -2488,9 +2488,9 @@ void rtl8821ae_dm_txpower_tracking_callback_thermalmeter(
>   		for (p = RF90_PATH_A; p < MAX_PATH_NUM_8821A; p++)
>   			rtldm->swing_idx_ofdm_base[p] = rtldm->swing_idx_ofdm[p];
>
> -			RT_TRACE(rtlpriv, COMP_POWER_TRACKING, DBG_LOUD,
> -				 "pDM_Odm->RFCalibrateInfo.ThermalValue = %d ThermalValue= %d\n",
> -				 rtldm->thermalvalue, thermal_value);
> +		RT_TRACE(rtlpriv, COMP_POWER_TRACKING, DBG_LOUD,
> +			 "pDM_Odm->RFCalibrateInfo.ThermalValue = %d ThermalValue= %d\n",
> +			 rtldm->thermalvalue, thermal_value);
>   		/*Record last Power Tracking Thermal Value*/
>   		rtldm->thermalvalue = thermal_value;
>   	}
>

This change looks good to me.

Acked-by: Larry Finger <Larry.Finger@lwfinger.net>

Thanks,

Larry

^ 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