netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH net-next 2/2] net: exit busy loop when another process is runnable
From: Jason Wang @ 2014-09-02  3:35 UTC (permalink / raw)
  To: Eliezer Tamir, Eric Dumazet
  Cc: Ingo Molnar, Mike Galbraith, davem, netdev, linux-kernel, mst,
	Peter Zijlstra, Ingo Molnar jacob.e.keller@intel.com
In-Reply-To: <5404186E.4090409@linux.intel.com>

On 09/01/2014 02:55 PM, Eliezer Tamir wrote:
> On 26/08/2014 10:16, Jason Wang wrote:
>> On 08/25/2014 09:16 PM, Eliezer Tamir wrote:
>>> Here are my 2 cents:
>>> I think Ingo's suggestion of only yielding to tasks with same or higher
>>> priority makes sense.
>> I'm not sure I get your meaning. Do you mean calling yield_to() directly
>> in sk_busy_loop?
> Think about the case where two processes are busy polling on the
> same CPU and the same device queue. Since busy polling processes
> incoming packets on the queue from any process, this scenario works
> well currently,

I see, but looks like we can simply do this by exiting the busy loop
when ndo_busy_poll() finds something but not for current socket?
>  and will not work at all when polling yields to other
> processes that are of the same priority that are running on the same
> CPU.

So yielding has its limitation, we need let scheduler to do the choice
instead.
>
> As a side note, there is a lot of room for improvement when two
> processes on the same CPU want to busy poll on different device
> queues.
> The RFC code I published for epoll support showed one possible
> way of solving this, but I'm sure that there are other possibilities.
>
> Maybe the networking subsystem should maintain a list of device
> queues that need busypolling and have a thread that would poll
> all of them when there's nothing better to do.

Not sure whether this method will scale considering thousands of sockets
and processes.
>
> I'm aware of similar work on busy polling on NVMe devices, so
> maybe there should be a global busypoll thread for all devices
> that support it.
>
> BTW, I have someone inside Intel that wants to test future patches. Feel
> free to send me patches for testing, even if they are not ready for
> publishing yet.
>
> Cheers,
> Eliezer

Ok, will do it, thanks a lot.

^ permalink raw reply

* Re: [PATCH net-next v1 0/3] amd-xgbe: AMD XGBE driver updates 2014-08-29
From: David Miller @ 2014-09-02  3:31 UTC (permalink / raw)
  To: thomas.lendacky; +Cc: netdev
In-Reply-To: <20140829152148.13783.23789.stgit@tlendack-t1.amdoffice.net>

From: Tom Lendacky <thomas.lendacky@amd.com>
Date: Fri, 29 Aug 2014 10:21:48 -0500

> The amd-xgbe-phy phylib driver support is integral to the amd-xgbe
> driver and isn't meant to be used by other devices.  For this reason
> it is being removed from the driver/net/phy directory and integrated
> into the amd-xgbe driver.

I see no real reason to do this.

Keeping it in the phylib layer forces you to deal with the abstractions
and therefore keeps you from bypassing the phylib API and performing
layer violations.

Please just keep the PHY driver where it is and add your new features
there.

Thanks.

^ permalink raw reply

* Re: [PATCH net-next 2/2] net: exit busy loop when another process is runnable
From: Jason Wang @ 2014-09-02  3:29 UTC (permalink / raw)
  To: Eliezer Tamir, Ingo Molnar
  Cc: Mike Galbraith, davem, netdev, linux-kernel, mst, Peter Zijlstra,
	Ingo Molnar
In-Reply-To: <540414AB.9000004@linux.intel.com>

On 09/01/2014 02:39 PM, Eliezer Tamir wrote:
> On 29/08/2014 06:08, Jason Wang wrote:
>> > Yes, but rx busy polling only works in process context and does not
>> > disable bh, so it may be not an issue.
> sk_busy_loop() uses rcu_read_lock_bh(), so it does run with bh disabled.
>
> -Eliezer

True, so we need probably also exit the loop when there are pending bhs.

^ permalink raw reply

* Re: [PATCH 1/5] net: mvberlin_eth: add an Ethernet driver for Marvell Berlin
From: David Miller @ 2014-09-02  3:26 UTC (permalink / raw)
  To: antoine.tenart-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8
  Cc: sebastian.hesselbarth-Re5JQEeQqe8AvxtiuMwx3w,
	thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
	alexandre.belloni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
	zmxu-eYqpPyKDWXRBDgjK7y7TUQ, jszhang-eYqpPyKDWXRBDgjK7y7TUQ,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1409320263-10295-2-git-send-email-antoine.tenart-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

From: Antoine Tenart <antoine.tenart-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Date: Fri, 29 Aug 2014 15:50:59 +0200

> +struct rx_desc {
> +	u16 buf_size;		/* Buffer size				*/
> +	u16 byte_cnt;		/* Descriptor buffer byte count		*/
 ...
> +	u16 byte_cnt;		/* buffer byte count			*/
> +	u16 l4i_chk;		/* CPU provided TCP checksum		*/

I'm really suspcious that you're ordering these two members differently
than that used by the mv643xx_eth driver.

Looking at a diff of the two drivers, most of the transmit and recieve paths
are largely the same.

WRT. register differences, adding indirection to handle that is a no-brainer
because the cost of the register access itself will absolutely dwarf whatever
it costs to do an indirect call or whatever to implement the register access.

I really think you should look into sharing code more seriously.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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: calxedaxgmac: fix driver dependencies
From: Rob Herring @ 2014-09-02  2:36 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz
  Cc: David S. Miller, Kyungmin Park, netdev,
	linux-kernel@vger.kernel.org
In-Reply-To: <7268934.nBXHizFkl4@amdc1032>

On Mon, Sep 1, 2014 at 10:39 AM, Bartlomiej Zolnierkiewicz
<b.zolnierkie@samsung.com> wrote:
> Calxeda 1G/10G XGMAC Ethernet support should be available only on
> Calxeda ECX-1000/2000 (Highbank/Midway) platforms.
>
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: Rob Herring <robh@kernel.org>
> ---
>  drivers/net/ethernet/calxeda/Kconfig |    1 +
>  1 file changed, 1 insertion(+)
>
> Index: b/drivers/net/ethernet/calxeda/Kconfig
> ===================================================================
> --- a/drivers/net/ethernet/calxeda/Kconfig      2014-07-30 14:31:12.159522474 +0200
> +++ b/drivers/net/ethernet/calxeda/Kconfig      2014-09-01 17:33:49.232810483 +0200
> @@ -1,6 +1,7 @@
>  config NET_CALXEDA_XGMAC
>         tristate "Calxeda 1G/10G XGMAC Ethernet driver"
>         depends on HAS_IOMEM && HAS_DMA
> +       depends on ARCH_HIGHBANK || COMPILE_TEST

Opinions differ on whether drivers should be restricted in this way.
There is no actual dependency requiring ARCH_HIGHBANK. So I leave it
to David.

Rob

^ permalink raw reply

* [PATCH net-next] r8152: rename rx_buf_sz
From: Hayes Wang @ 2014-09-02  2:27 UTC (permalink / raw)
  To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb

The variable "rx_buf_sz" is used by both tx and rx buffers. Replace
it with "agg_buf_sz".

Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
 drivers/net/usb/r8152.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index cc64dc0..80b0179 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -607,9 +607,9 @@ enum tx_csum_stat {
  * The RTL chips use a 64 element hash table based on the Ethernet CRC.
  */
 static const int multicast_filter_limit = 32;
-static unsigned int rx_buf_sz = 16384;
+static unsigned int agg_buf_sz = 16384;
 
-#define RTL_LIMITED_TSO_SIZE	(rx_buf_sz - sizeof(struct tx_desc) - \
+#define RTL_LIMITED_TSO_SIZE	(agg_buf_sz - sizeof(struct tx_desc) - \
 				 VLAN_ETH_HLEN - VLAN_HLEN)
 
 static
@@ -1248,13 +1248,13 @@ static int alloc_all_mem(struct r8152 *tp)
 	skb_queue_head_init(&tp->tx_queue);
 
 	for (i = 0; i < RTL8152_MAX_RX; i++) {
-		buf = kmalloc_node(rx_buf_sz, GFP_KERNEL, node);
+		buf = kmalloc_node(agg_buf_sz, GFP_KERNEL, node);
 		if (!buf)
 			goto err1;
 
 		if (buf != rx_agg_align(buf)) {
 			kfree(buf);
-			buf = kmalloc_node(rx_buf_sz + RX_ALIGN, GFP_KERNEL,
+			buf = kmalloc_node(agg_buf_sz + RX_ALIGN, GFP_KERNEL,
 					   node);
 			if (!buf)
 				goto err1;
@@ -1274,13 +1274,13 @@ static int alloc_all_mem(struct r8152 *tp)
 	}
 
 	for (i = 0; i < RTL8152_MAX_TX; i++) {
-		buf = kmalloc_node(rx_buf_sz, GFP_KERNEL, node);
+		buf = kmalloc_node(agg_buf_sz, GFP_KERNEL, node);
 		if (!buf)
 			goto err1;
 
 		if (buf != tx_agg_align(buf)) {
 			kfree(buf);
-			buf = kmalloc_node(rx_buf_sz + TX_ALIGN, GFP_KERNEL,
+			buf = kmalloc_node(agg_buf_sz + TX_ALIGN, GFP_KERNEL,
 					   node);
 			if (!buf)
 				goto err1;
@@ -1518,7 +1518,7 @@ static int r8152_tx_agg_fill(struct r8152 *tp, struct tx_agg *agg)
 	tx_data = agg->head;
 	agg->skb_num = 0;
 	agg->skb_len = 0;
-	remain = rx_buf_sz;
+	remain = agg_buf_sz;
 
 	while (remain >= ETH_ZLEN + sizeof(struct tx_desc)) {
 		struct tx_desc *tx_desc;
@@ -1565,7 +1565,7 @@ static int r8152_tx_agg_fill(struct r8152 *tp, struct tx_agg *agg)
 
 		dev_kfree_skb_any(skb);
 
-		remain = rx_buf_sz - (int)(tx_agg_align(tx_data) - agg->head);
+		remain = agg_buf_sz - (int)(tx_agg_align(tx_data) - agg->head);
 	}
 
 	if (!skb_queue_empty(&skb_head)) {
@@ -1771,7 +1771,7 @@ static
 int r8152_submit_rx(struct r8152 *tp, struct rx_agg *agg, gfp_t mem_flags)
 {
 	usb_fill_bulk_urb(agg->urb, tp->udev, usb_rcvbulkpipe(tp->udev, 1),
-			  agg->head, rx_buf_sz,
+			  agg->head, agg_buf_sz,
 			  (usb_complete_t)read_bulk_callback, agg);
 
 	return usb_submit_urb(agg->urb, mem_flags);
@@ -1915,8 +1915,8 @@ static void set_tx_qlen(struct r8152 *tp)
 {
 	struct net_device *netdev = tp->netdev;
 
-	tp->tx_qlen = rx_buf_sz / (netdev->mtu + VLAN_ETH_HLEN + VLAN_HLEN +
-				   sizeof(struct tx_desc));
+	tp->tx_qlen = agg_buf_sz / (netdev->mtu + VLAN_ETH_HLEN + VLAN_HLEN +
+				    sizeof(struct tx_desc));
 }
 
 static inline u8 rtl8152_get_speed(struct r8152 *tp)
-- 
1.9.3

^ permalink raw reply related

* [PATCH] ethernet: arc: remove unused dev
From: Jingoo Han @ 2014-09-02  1:48 UTC (permalink / raw)
  To: 'David S. Miller'
  Cc: netdev, 'Romain Perier', 'Jingoo Han'

Remove unused 'dev' variable from arc_emac_remove(), since it's
not being used any more.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/net/ethernet/arc/emac_main.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/arc/emac_main.c b/drivers/net/ethernet/arc/emac_main.c
index b35c69e1b147..a7773923a7a0 100644
--- a/drivers/net/ethernet/arc/emac_main.c
+++ b/drivers/net/ethernet/arc/emac_main.c
@@ -829,7 +829,6 @@ EXPORT_SYMBOL_GPL(arc_emac_probe);
 
 int arc_emac_remove(struct net_device *ndev)
 {
-	struct device *dev = ndev->dev.parent;
 	struct arc_emac_priv *priv = netdev_priv(ndev);
 
 	phy_disconnect(priv->phy_dev);
-- 
2.0.0

^ permalink raw reply related

* Re: [PATCH 8/9] net: Don't keep around original SKB when we software segment GSO frames.
From: David Miller @ 2014-09-02  1:42 UTC (permalink / raw)
  To: therbert; +Cc: netdev
In-Reply-To: <CA+mtBx-1r-vJYDx0xJ+p=Tjr=_v559XdrSu1SZ4UXWE_4fLbZg@mail.gmail.com>

From: Tom Herbert <therbert@google.com>
Date: Mon, 1 Sep 2014 18:28:18 -0700

> On Mon, Sep 1, 2014 at 3:25 PM, David Miller <davem@davemloft.net> wrote:
>>  struct sk_buff *validate_xmit_skb(struct sk_buff *skb, struct net_device *dev);
>> -int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
>> -                       struct netdev_queue *txq);
>> +struct sk_buff *dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
>> +                                   struct netdev_queue *txq, int *ret);
> 
> Might be slightly better to still return int but pass struct sk_buff
> **skb. One less argument and doesn't change return type.

Sure we could do that, I don't have any preference either way and
besides there is only one call site.

^ permalink raw reply

* Re: [PATCH 4/9] net: Pass a "more" indication down into netdev_start_xmit() code paths.
From: David Miller @ 2014-09-02  1:41 UTC (permalink / raw)
  To: therbert; +Cc: netdev
In-Reply-To: <CA+mtBx-jOW1CrasHkivcoECCzAnBJ7h6Vui4j+2SrUaVCvAc-w@mail.gmail.com>

From: Tom Herbert <therbert@google.com>
Date: Mon, 1 Sep 2014 18:21:57 -0700

> On Mon, Sep 1, 2014 at 3:24 PM, David Miller <davem@davemloft.net> wrote:
>> +                                             bool more)
>>  {
>> -       skb->xmit_more = 0;
>> +       skb->xmit_more = more ? 1 : 0;
> 
> Maybe just
> 
> skb->xmit_more = more;

I'm just being overly cautious because we're storing a boolean into
an integer bitfield.

^ permalink raw reply

* Re: [net-next PATCH v2 02/15] net: rcu-ify tcf_proto
From: John Fastabend @ 2014-09-02  1:39 UTC (permalink / raw)
  To: David Miller; +Cc: xiyou.wangcong, jhs, eric.dumazet, netdev, paulmck, brouer
In-Reply-To: <20140824.223133.170922940598584254.davem@davemloft.net>

On 08/24/2014 10:31 PM, David Miller wrote:
> From: John Fastabend <john.fastabend@gmail.com>
> Date: Sun, 24 Aug 2014 17:48:31 -0700
>
>> @@ -722,8 +724,9 @@ static void sfq_free(void *addr)
>>   static void sfq_destroy(struct Qdisc *sch)
>>   {
>>   	struct sfq_sched_data *q = qdisc_priv(sch);
>> +	struct tcf_proto *fl = rtnl_dereference(q->filter_list);
>>
>> -	tcf_destroy_chain(&q->filter_list);
>> +	tcf_destroy_chain(&fl);

Sorry for the delayed reply...

>
> This will cause tcf_destroy_chain() to set the local variable
> 'fl' to NULL rather than q->filter_list.
>
> I don't see how this can be correct at all.

Right now (without these patches) nothing sets q->filter_list
to NULL and we only call this when the qdisc is being destroyed.
In all cases there is a rcu_assign_pointer() to detach the qdisc
from the netdev_queue followed by a synchronize_net().

	dev_deactivate_many
	   [...]
	   dev_deactivate_queue
		rcu_assign_pointer(dev_queue->qdisc, qdisc_default)

  	   [...]
	   synchronize_net()

After the synchronize_net there should be no other references
to q->filter_list other then in the destroy path so I think it
works and the rtnl_dereference in my patch is there for annotation
to make sparse happy but doesn't change the logic.

Does that make sense?

>
> You need to make tcf_destroy_chain() take a pointer to an __rcu
> pointer, and do the proper dereferencing and RCU assignments in
> that chain destroy loop.
>
> This might be why you're getting annotation warnings.
>

The annotation warnings comes from the find_tcf blocks for example
here is the block in the sfq scheduler,

     struct sfq_sched_data *q = qdisc_priv(sch);

     if (cl)
         return NULL;
     return &q->filter_list;

This is called inside the rtnl lock but filter list is rcu protected
fixing the function types like this seems to resolve it,

     static struct tcf_proto * __rcu *sfq_find_tcf(struct Qdisc *sch, 
unsigned long cl)

and similarly the ops struct,

     struct tcf_proto * __rcu * (*tcf_chain)(struct Qdisc *, unsigned long);

This fixes the sparse warnings and looks correct to me. I'll send
an update with these fixes.

Thanks!
John



-- 
John Fastabend         Intel Corporation

^ permalink raw reply

* RE: [PATCH net-next 0/3] fec: Do not use NULL pointer to indicate error
From: fugang.duan @ 2014-09-02  1:37 UTC (permalink / raw)
  To: Fabio Estevam, davem@davemloft.net
  Cc: linux@arm.linux.org.uk, netdev@vger.kernel.org,
	Fabio.Estevam@freescale.com
In-Reply-To: <1409620375-20284-1-git-send-email-festevam@gmail.com>

From: Fabio Estevam <festevam@gmail.com> Sent: Tuesday, September 02, 2014 9:13 AM
>To: davem@davemloft.net
>Cc: linux@arm.linux.org.uk; Duan Fugang-B38611; netdev@vger.kernel.org;
>Estevam Fabio-R49496
>Subject: [PATCH net-next 0/3] fec: Do not use NULL pointer to indicate
>error
>
>From: Fabio Estevam <fabio.estevam@freescale.com>
>
>Using IS_ERR() to indicate error can simplify the code a bit, so let's use
>it when appropriate.
>
>Fabio Estevam (3):
>  fec: Do not set fep->clk_ptp to NULL on error
>  fec: Do not set fep->clk_enet_out to NULL on error
>  fec: Do not set fep->reg_phy to NULL on error
>
> drivers/net/ethernet/freescale/fec_main.c | 28 +++++++++++---------------
>--
> 1 file changed, 11 insertions(+), 17 deletions(-)
>
>--
>1.9.1

The patch serial are fine.

Acked-by: Fugang Duan <B38611@freescale.com>

Thanks,
Andy

^ permalink raw reply

* Re: [PATCH net-next] bonding: add slave_changelink support and use it for queue_id
From: David Miller @ 2014-09-02  1:32 UTC (permalink / raw)
  To: nikolay; +Cc: netdev, j.vosburgh, vfalico, andy
In-Reply-To: <5400638C.2030502@redhat.com>

From: Nikolay Aleksandrov <nikolay@redhat.com>
Date: Fri, 29 Aug 2014 13:27:08 +0200

> On 08/27/2014 04:06 PM, Nikolay Aleksandrov wrote:
>> This patch adds support for slave_changelink to the bonding and uses it
>> to give the ability to change the queue_id of the enslaved devices via
>> netlink. It sets slave_maxtype and uses bond_changelink as a prototype for
>> bond_slave_changelink.
>> Example/test command after the iproute2 patch:
>>  ip link set eth0 type bond_slave queue_id 10
>> 
>> CC: David S. Miller <davem@davemloft.net>
>> CC: Jay Vosburgh <j.vosburgh@gmail.com>
>> CC: Veaceslav Falico <vfalico@gmail.com>
>> CC: Andy Gospodarek <andy@greyhouse.net>
>> 
>> Suggested-by: Jiri Pirko <jiri@resnulli.us>
>> Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
>> ---
> 
> Hi Dave,
> Would you mind telling me why did this patch get rejected so I can fix it up ?
> I didn't see any complains and Jiri acked it.

Sorry, I misread the feedback, this patch is fine, and is now applied.

^ permalink raw reply

* Re: [net PATCH 1/1] drivers: net: cpsw: dual_emac: fix reducing of rx descriptor during ifdown
From: David Miller @ 2014-09-02  1:31 UTC (permalink / raw)
  To: mugunthanvnm; +Cc: netdev
In-Reply-To: <1409304145-26179-1-git-send-email-mugunthanvnm@ti.com>

From: Mugunthan V N <mugunthanvnm@ti.com>
Date: Fri, 29 Aug 2014 14:52:25 +0530

> In Dual EMAC, when both interface are up and while doing ifdown with heavy
> traffic then skbs already processed by DMA from that slave emac has to be
> requeued as still the other interface is up and running.
> 
> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>

I don't see why this is important.

If the packet arrived via the down interface, let it be dropped and
the sender will simply resend if necessary.

Also you are putting this new multi-slave logic under the "status < 0"
condition as well as the interface being down, that's not right either.

^ permalink raw reply

* [PATCH net-next v4] lib/rhashtable: allow user to set the minimum shifts of shrinking
From: Ying Xue @ 2014-09-02  1:28 UTC (permalink / raw)
  To: tgraf; +Cc: davem, eric.dumazet, netdev

Although rhashtable library allows user to specify a quiet big size
for user's created hash table, the table may be shrunk to a
very small size - HASH_MIN_SIZE(4) after object is removed from
the table at the first time. Subsequently, even if the total amount
of objects saved in the table is quite lower than user's initial
setting in a long time, the hash table size is still dynamically
adjusted by rhashtable_shrink() or rhashtable_expand() each time
object is inserted or removed from the table. However, as
synchronize_rcu() has to be called when table is shrunk or
expanded by the two functions, we should permit user to set the
minimum table size through configuring the minimum number of shifts
according to user specific requirement, avoiding these expensive
actions of shrinking or expanding because of calling synchronize_rcu().

Signed-off-by: Ying Xue <ying.xue@windriver.com>
---
In the version, no any compiling warning appears, and all rhashtable
self test cases are passed.

 include/linux/rhashtable.h |    2 ++
 lib/rhashtable.c           |   12 ++++++++----
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/include/linux/rhashtable.h b/include/linux/rhashtable.h
index 36826c0..fb298e9d 100644
--- a/include/linux/rhashtable.h
+++ b/include/linux/rhashtable.h
@@ -44,6 +44,7 @@ struct rhashtable;
  * @head_offset: Offset of rhash_head in struct to be hashed
  * @hash_rnd: Seed to use while hashing
  * @max_shift: Maximum number of shifts while expanding
+ * @min_shift: Minimum number of shifts while shrinking
  * @hashfn: Function to hash key
  * @obj_hashfn: Function to hash object
  * @grow_decision: If defined, may return true if table should expand
@@ -57,6 +58,7 @@ struct rhashtable_params {
 	size_t			head_offset;
 	u32			hash_rnd;
 	size_t			max_shift;
+	size_t			min_shift;
 	rht_hashfn_t		hashfn;
 	rht_obj_hashfn_t	obj_hashfn;
 	bool			(*grow_decision)(const struct rhashtable *ht,
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index a2c7881..5b52100 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -298,7 +298,7 @@ int rhashtable_shrink(struct rhashtable *ht, gfp_t flags)
 
 	ASSERT_RHT_MUTEX(ht);
 
-	if (tbl->size <= HASH_MIN_SIZE)
+	if (ht->shift <= ht->p.min_shift)
 		return 0;
 
 	ntbl = bucket_table_alloc(tbl->size / 2, flags);
@@ -506,9 +506,10 @@ void *rhashtable_lookup_compare(const struct rhashtable *ht, u32 hash,
 }
 EXPORT_SYMBOL_GPL(rhashtable_lookup_compare);
 
-static size_t rounded_hashtable_size(unsigned int nelem)
+static size_t rounded_hashtable_size(struct rhashtable_params *params)
 {
-	return max(roundup_pow_of_two(nelem * 4 / 3), HASH_MIN_SIZE);
+	return max(roundup_pow_of_two(params->nelem_hint * 4 / 3),
+		   1UL << params->min_shift);
 }
 
 /**
@@ -566,8 +567,11 @@ int rhashtable_init(struct rhashtable *ht, struct rhashtable_params *params)
 	    (!params->key_len && !params->obj_hashfn))
 		return -EINVAL;
 
+	params->min_shift = max_t(size_t, params->min_shift,
+				  (size_t)ilog2(HASH_MIN_SIZE));
+
 	if (params->nelem_hint)
-		size = rounded_hashtable_size(params->nelem_hint);
+		size = rounded_hashtable_size(params);
 
 	tbl = bucket_table_alloc(size, GFP_KERNEL);
 	if (tbl == NULL)
-- 
1.7.9.5

^ permalink raw reply related

* Re: [PATCH 8/9] net: Don't keep around original SKB when we software segment GSO frames.
From: Tom Herbert @ 2014-09-02  1:28 UTC (permalink / raw)
  To: David Miller; +Cc: Linux Netdev List
In-Reply-To: <20140901.152515.1944476256109254437.davem@davemloft.net>

On Mon, Sep 1, 2014 at 3:25 PM, David Miller <davem@davemloft.net> wrote:
>
> Just maintain the list properly by returning the head of the remaining
> SKB list from dev_hard_start_xmit().
>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> ---
>  include/linux/netdevice.h |  4 +--
>  net/core/dev.c            | 79 +++++++++--------------------------------------
>  net/sched/sch_generic.c   |  2 +-
>  3 files changed, 17 insertions(+), 68 deletions(-)
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 47c49ba..202c25a 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -2828,8 +2828,8 @@ int dev_change_carrier(struct net_device *, bool new_carrier);
>  int dev_get_phys_port_id(struct net_device *dev,
>                          struct netdev_phys_port_id *ppid);
>  struct sk_buff *validate_xmit_skb(struct sk_buff *skb, struct net_device *dev);
> -int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
> -                       struct netdev_queue *txq);
> +struct sk_buff *dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
> +                                   struct netdev_queue *txq, int *ret);

Might be slightly better to still return int but pass struct sk_buff
**skb. One less argument and doesn't change return type.

>  int __dev_forward_skb(struct net_device *dev, struct sk_buff *skb);
>  int dev_forward_skb(struct net_device *dev, struct sk_buff *skb);
>  bool is_skb_forwardable(struct net_device *dev, struct sk_buff *skb);
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 75bc5b0..c89da4f 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -2485,52 +2485,6 @@ static int illegal_highdma(struct net_device *dev, struct sk_buff *skb)
>         return 0;
>  }
>
> -struct dev_gso_cb {
> -       void (*destructor)(struct sk_buff *skb);
> -};
> -
> -#define DEV_GSO_CB(skb) ((struct dev_gso_cb *)(skb)->cb)
> -
> -static void dev_gso_skb_destructor(struct sk_buff *skb)
> -{
> -       struct dev_gso_cb *cb;
> -
> -       kfree_skb_list(skb->next);
> -       skb->next = NULL;
> -
> -       cb = DEV_GSO_CB(skb);
> -       if (cb->destructor)
> -               cb->destructor(skb);
> -}
> -
> -/**
> - *     dev_gso_segment - Perform emulated hardware segmentation on skb.
> - *     @skb: buffer to segment
> - *     @features: device features as applicable to this skb
> - *
> - *     This function segments the given skb and stores the list of segments
> - *     in skb->next.
> - */
> -static int dev_gso_segment(struct sk_buff *skb, netdev_features_t features)
> -{
> -       struct sk_buff *segs;
> -
> -       segs = skb_gso_segment(skb, features);
> -
> -       /* Verifying header integrity only. */
> -       if (!segs)
> -               return 0;
> -
> -       if (IS_ERR(segs))
> -               return PTR_ERR(segs);
> -
> -       skb->next = segs;
> -       DEV_GSO_CB(skb)->destructor = skb->destructor;
> -       skb->destructor = dev_gso_skb_destructor;
> -
> -       return 0;
> -}
> -
>  /* If MPLS offload request, verify we are testing hardware MPLS features
>   * instead of standard features for the netdev.
>   */
> @@ -2682,8 +2636,13 @@ struct sk_buff *validate_xmit_skb(struct sk_buff *skb, struct net_device *dev)
>                 features &= dev->hw_enc_features;
>
>         if (netif_needs_gso(skb, features)) {
> -               if (unlikely(dev_gso_segment(skb, features)))
> -                       goto out_kfree_skb;
> +               struct sk_buff *segs;
> +
> +               segs = skb_gso_segment(skb, features);
> +               kfree_skb(skb);
> +               if (IS_ERR(segs))
> +                       segs = NULL;
> +               skb = segs;
>         } else {
>                 if (skb_needs_linearize(skb, features) &&
>                     __skb_linearize(skb))
> @@ -2714,26 +2673,16 @@ out_null:
>         return NULL;
>  }
>
> -int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
> -                       struct netdev_queue *txq)
> +struct sk_buff *dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
> +                                   struct netdev_queue *txq, int *ret)
>  {
> -       int rc = NETDEV_TX_OK;
> -
> -       if (likely(!skb->next))
> -               return xmit_one(skb, dev, txq, false);
> -
> -       skb->next = xmit_list(skb->next, dev, txq, &rc);
> -       if (likely(skb->next == NULL)) {
> -               skb->destructor = DEV_GSO_CB(skb)->destructor;
> -               consume_skb(skb);
> -               return rc;
> +       if (likely(!skb->next)) {
> +               *ret = xmit_one(skb, dev, txq, false);
> +               return skb;
>         }
>
> -       kfree_skb(skb);
> -
> -       return rc;
> +       return xmit_list(skb, dev, txq, ret);
>  }
> -EXPORT_SYMBOL_GPL(dev_hard_start_xmit);
>
>  static void qdisc_pkt_len_init(struct sk_buff *skb)
>  {
> @@ -2945,7 +2894,7 @@ static int __dev_queue_xmit(struct sk_buff *skb, void *accel_priv)
>
>                         if (!netif_xmit_stopped(txq)) {
>                                 __this_cpu_inc(xmit_recursion);
> -                               rc = dev_hard_start_xmit(skb, dev, txq);
> +                               skb = dev_hard_start_xmit(skb, dev, txq, &rc);
>                                 __this_cpu_dec(xmit_recursion);
>                                 if (dev_xmit_complete(rc)) {
>                                         HARD_TX_UNLOCK(dev, txq);
> diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
> index f178798..a8bf9f9 100644
> --- a/net/sched/sch_generic.c
> +++ b/net/sched/sch_generic.c
> @@ -129,7 +129,7 @@ int sch_direct_xmit(struct sk_buff *skb, struct Qdisc *q,
>
>         HARD_TX_LOCK(dev, txq, smp_processor_id());
>         if (!netif_xmit_frozen_or_stopped(txq))
> -               ret = dev_hard_start_xmit(skb, dev, txq);
> +               skb = dev_hard_start_xmit(skb, dev, txq, &ret);
>
>         HARD_TX_UNLOCK(dev, txq);
>
> --
> 1.7.11.7
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 4/9] net: Pass a "more" indication down into netdev_start_xmit() code paths.
From: Tom Herbert @ 2014-09-02  1:21 UTC (permalink / raw)
  To: David Miller; +Cc: Linux Netdev List
In-Reply-To: <20140901.152454.408917069371597628.davem@davemloft.net>

On Mon, Sep 1, 2014 at 3:24 PM, David Miller <davem@davemloft.net> wrote:
>
>
> For now it will always be false.
>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> ---
>  drivers/net/wan/dlci.c    | 2 +-
>  include/linux/netdevice.h | 9 +++++----
>  net/atm/mpc.c             | 2 +-
>  net/core/dev.c            | 2 +-
>  net/core/netpoll.c        | 2 +-
>  net/core/pktgen.c         | 2 +-
>  net/packet/af_packet.c    | 2 +-
>  net/sched/sch_teql.c      | 3 ++-
>  8 files changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/wan/dlci.c b/drivers/net/wan/dlci.c
> index 6427e82..ae6ecf4 100644
> --- a/drivers/net/wan/dlci.c
> +++ b/drivers/net/wan/dlci.c
> @@ -194,7 +194,7 @@ static netdev_tx_t dlci_transmit(struct sk_buff *skb, struct net_device *dev)
>
>         if (skb) {
>                 struct netdev_queue *txq = skb_get_tx_queue(dev, skb);
> -               netdev_start_xmit(skb, dlp->slave, txq);
> +               netdev_start_xmit(skb, dlp->slave, txq, false);
>         }
>         return NETDEV_TX_OK;
>  }
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 1617180..5050218 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -3431,19 +3431,20 @@ int __init dev_proc_init(void);
>  #endif
>
>  static inline netdev_tx_t __netdev_start_xmit(const struct net_device_ops *ops,
> -                                             struct sk_buff *skb, struct net_device *dev)
> +                                             struct sk_buff *skb, struct net_device *dev,
> +                                             bool more)
>  {
> -       skb->xmit_more = 0;
> +       skb->xmit_more = more ? 1 : 0;

Maybe just

skb->xmit_more = more;

>
>         return ops->ndo_start_xmit(skb, dev);
>  }
>
>  static inline netdev_tx_t netdev_start_xmit(struct sk_buff *skb, struct net_device *dev,
> -                                           struct netdev_queue *txq)
> +                                           struct netdev_queue *txq, bool more)
>  {
>         const struct net_device_ops *ops = dev->netdev_ops;
>         int rc;
>
> -       rc = __netdev_start_xmit(ops, skb, dev);
> +       rc = __netdev_start_xmit(ops, skb, dev, more);
>         if (rc == NETDEV_TX_OK)
>                 txq_trans_update(txq);
>
> diff --git a/net/atm/mpc.c b/net/atm/mpc.c
> index d662da1..0e98222 100644
> --- a/net/atm/mpc.c
> +++ b/net/atm/mpc.c
> @@ -599,7 +599,7 @@ static netdev_tx_t mpc_send_packet(struct sk_buff *skb,
>         }
>
>  non_ip:
> -       return __netdev_start_xmit(mpc->old_ops, skb, dev);
> +       return __netdev_start_xmit(mpc->old_ops, skb, dev, false);
>  }
>
>  static int atm_mpoa_vcc_attach(struct atm_vcc *vcc, void __user *arg)
> diff --git a/net/core/dev.c b/net/core/dev.c
> index ab7bb80..f0ed5a6 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -2610,7 +2610,7 @@ static int xmit_one(struct sk_buff *skb, struct net_device *dev,
>
>         len = skb->len;
>         trace_net_dev_start_xmit(skb, dev);
> -       rc = netdev_start_xmit(skb, dev, txq);
> +       rc = netdev_start_xmit(skb, dev, txq, false);
>         trace_net_dev_xmit(skb, rc, dev, len);
>
>         return rc;
> diff --git a/net/core/netpoll.c b/net/core/netpoll.c
> index 05bc57e..e6645b4 100644
> --- a/net/core/netpoll.c
> +++ b/net/core/netpoll.c
> @@ -91,7 +91,7 @@ static int netpoll_start_xmit(struct sk_buff *skb, struct net_device *dev,
>                 skb->vlan_tci = 0;
>         }
>
> -       status = netdev_start_xmit(skb, dev, txq);
> +       status = netdev_start_xmit(skb, dev, txq, false);
>
>  out:
>         return status;
> diff --git a/net/core/pktgen.c b/net/core/pktgen.c
> index 34bd2ff..5b36a94 100644
> --- a/net/core/pktgen.c
> +++ b/net/core/pktgen.c
> @@ -3335,7 +3335,7 @@ static void pktgen_xmit(struct pktgen_dev *pkt_dev)
>                 goto unlock;
>         }
>         atomic_inc(&(pkt_dev->skb->users));
> -       ret = netdev_start_xmit(pkt_dev->skb, odev, txq);
> +       ret = netdev_start_xmit(pkt_dev->skb, odev, txq, false);
>
>         switch (ret) {
>         case NETDEV_TX_OK:
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index fe305a0..87d20f4 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -259,7 +259,7 @@ static int packet_direct_xmit(struct sk_buff *skb)
>
>         HARD_TX_LOCK(dev, txq, smp_processor_id());
>         if (!netif_xmit_frozen_or_drv_stopped(txq))
> -               ret = netdev_start_xmit(skb, dev, txq);
> +               ret = netdev_start_xmit(skb, dev, txq, false);
>         HARD_TX_UNLOCK(dev, txq);
>
>         local_bh_enable();
> diff --git a/net/sched/sch_teql.c b/net/sched/sch_teql.c
> index 193dc2c..aaa8d03 100644
> --- a/net/sched/sch_teql.c
> +++ b/net/sched/sch_teql.c
> @@ -316,7 +316,8 @@ restart:
>                                 unsigned int length = qdisc_pkt_len(skb);
>
>                                 if (!netif_xmit_frozen_or_stopped(slave_txq) &&
> -                                   netdev_start_xmit(skb, slave, slave_txq) == NETDEV_TX_OK) {
> +                                   netdev_start_xmit(skb, slave, slave_txq, false) ==
> +                                   NETDEV_TX_OK) {
>                                         __netif_tx_unlock(slave_txq);
>                                         master->slaves = NEXT_SLAVE(q);
>                                         netif_wake_queue(dev);
> --
> 1.7.11.7
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH net-next 5/5] sched: replace bare printks
From: David Miller @ 2014-09-02  1:16 UTC (permalink / raw)
  To: stephen; +Cc: netdev
In-Reply-To: <20140829071035.547171617@networkplumber.org>

From: Stephen Hemminger <stephen@networkplumber.org>
Date: Fri, 29 Aug 2014 00:09:23 -0700

> @@ -140,7 +140,7 @@ static int tcf_mirred(struct sk_buff *sk
>  
>  	dev = m->tcfm_dev;
>  	if (!dev) {
> -		printk_once(KERN_NOTICE "tc mirred: target device is gone\n");
> +		netdev_notice(dev, "tc mirred: target device is gone\n");
>  		goto out;

As pointed out by others, doing netdev_*() in a path where we know
'dev' is NULL doesn't make much sense.

Please respin this series with this adjusted.

Thanks.

^ permalink raw reply

* [PATCH net-next 3/3] fec: Do not set fep->reg_phy to NULL on error
From: Fabio Estevam @ 2014-09-02  1:12 UTC (permalink / raw)
  To: davem; +Cc: linux, B38611, netdev, Fabio Estevam
In-Reply-To: <1409620375-20284-1-git-send-email-festevam@gmail.com>

From: Fabio Estevam <fabio.estevam@freescale.com>

There is no need to set fep->reg_phy to NULL when devm_regulator_get()
returns an error. We can simply use IS_ERR() instead, which makes the code
simpler.

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
 drivers/net/ethernet/freescale/fec_main.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index e87cc66..39c28d1 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2653,8 +2653,6 @@ fec_probe(struct platform_device *pdev)
 				"Failed to enable phy regulator: %d\n", ret);
 			goto failed_regulator;
 		}
-	} else {
-		fep->reg_phy = NULL;
 	}
 
 	fec_reset_phy(pdev);
@@ -2704,7 +2702,7 @@ failed_register:
 failed_mii_init:
 failed_irq:
 failed_init:
-	if (fep->reg_phy)
+	if (!IS_ERR(fep->reg_phy))
 		regulator_disable(fep->reg_phy);
 failed_regulator:
 	fec_enet_clk_enable(ndev, false);
@@ -2727,7 +2725,7 @@ fec_drv_remove(struct platform_device *pdev)
 	cancel_work_sync(&fep->tx_timeout_work);
 	unregister_netdev(ndev);
 	fec_enet_mii_remove(fep);
-	if (fep->reg_phy)
+	if (!IS_ERR(fep->reg_phy))
 		regulator_disable(fep->reg_phy);
 	if (fep->ptp_clock)
 		ptp_clock_unregister(fep->ptp_clock);
@@ -2757,7 +2755,7 @@ static int __maybe_unused fec_suspend(struct device *dev)
 	fec_enet_clk_enable(ndev, false);
 	pinctrl_pm_select_sleep_state(&fep->pdev->dev);
 
-	if (fep->reg_phy)
+	if (!IS_ERR(fep->reg_phy))
 		regulator_disable(fep->reg_phy);
 
 	return 0;
@@ -2769,7 +2767,7 @@ static int __maybe_unused fec_resume(struct device *dev)
 	struct fec_enet_private *fep = netdev_priv(ndev);
 	int ret;
 
-	if (fep->reg_phy) {
+	if (!IS_ERR(fep->reg_phy)) {
 		ret = regulator_enable(fep->reg_phy);
 		if (ret)
 			return ret;
@@ -2794,7 +2792,7 @@ static int __maybe_unused fec_resume(struct device *dev)
 	return 0;
 
 failed_clk:
-	if (fep->reg_phy)
+	if (!IS_ERR(fep->reg_phy))
 		regulator_disable(fep->reg_phy);
 	return ret;
 }
-- 
1.9.1

^ permalink raw reply related

* [PATCH net-next 2/3] fec: Do not set fep->clk_enet_out to NULL on error
From: Fabio Estevam @ 2014-09-02  1:12 UTC (permalink / raw)
  To: davem; +Cc: linux, B38611, netdev, Fabio Estevam
In-Reply-To: <1409620375-20284-1-git-send-email-festevam@gmail.com>

From: Fabio Estevam <fabio.estevam@freescale.com>

There is no need to set fep->clk_enet_out to NULL when devm_clk_get() returns an
error. We can simply use IS_ERR() instead, which makes the code simpler.

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
 drivers/net/ethernet/freescale/fec_main.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 76e4f56..e87cc66 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1605,7 +1605,7 @@ static int fec_enet_clk_enable(struct net_device *ndev, bool enable)
 		ret = clk_prepare_enable(fep->clk_ipg);
 		if (ret)
 			goto failed_clk_ipg;
-		if (fep->clk_enet_out) {
+		if (!IS_ERR(fep->clk_enet_out)) {
 			ret = clk_prepare_enable(fep->clk_enet_out);
 			if (ret)
 				goto failed_clk_enet_out;
@@ -1624,7 +1624,7 @@ static int fec_enet_clk_enable(struct net_device *ndev, bool enable)
 	} else {
 		clk_disable_unprepare(fep->clk_ahb);
 		clk_disable_unprepare(fep->clk_ipg);
-		if (fep->clk_enet_out)
+		if (!IS_ERR(fep->clk_enet_out))
 			clk_disable_unprepare(fep->clk_enet_out);
 		if (!IS_ERR(fep->clk_ptp)) {
 			mutex_lock(&fep->ptp_clk_mutex);
@@ -1636,7 +1636,7 @@ static int fec_enet_clk_enable(struct net_device *ndev, bool enable)
 
 	return 0;
 failed_clk_ptp:
-	if (fep->clk_enet_out)
+	if (!IS_ERR(fep->clk_enet_out))
 		clk_disable_unprepare(fep->clk_enet_out);
 failed_clk_enet_out:
 		clk_disable_unprepare(fep->clk_ipg);
@@ -2632,8 +2632,6 @@ fec_probe(struct platform_device *pdev)
 
 	/* enet_out is optional, depends on board */
 	fep->clk_enet_out = devm_clk_get(&pdev->dev, "enet_out");
-	if (IS_ERR(fep->clk_enet_out))
-		fep->clk_enet_out = NULL;
 
 	fep->ptp_clk_on = false;
 	mutex_init(&fep->ptp_clk_mutex);
-- 
1.9.1

^ permalink raw reply related

* [PATCH net-next 1/3] fec: Do not set fep->clk_ptp to NULL on error
From: Fabio Estevam @ 2014-09-02  1:12 UTC (permalink / raw)
  To: davem; +Cc: linux, B38611, netdev, Fabio Estevam
In-Reply-To: <1409620375-20284-1-git-send-email-festevam@gmail.com>

From: Fabio Estevam <fabio.estevam@freescale.com>

There is no need to set fep->clk_ptp to NULL when devm_clk_get() returns an
error. We can simply use IS_ERR() instead, which makes the code simpler.

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
 drivers/net/ethernet/freescale/fec_main.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 89355a7..76e4f56 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -1610,7 +1610,7 @@ static int fec_enet_clk_enable(struct net_device *ndev, bool enable)
 			if (ret)
 				goto failed_clk_enet_out;
 		}
-		if (fep->clk_ptp) {
+		if (!IS_ERR(fep->clk_ptp)) {
 			mutex_lock(&fep->ptp_clk_mutex);
 			ret = clk_prepare_enable(fep->clk_ptp);
 			if (ret) {
@@ -1626,7 +1626,7 @@ static int fec_enet_clk_enable(struct net_device *ndev, bool enable)
 		clk_disable_unprepare(fep->clk_ipg);
 		if (fep->clk_enet_out)
 			clk_disable_unprepare(fep->clk_enet_out);
-		if (fep->clk_ptp) {
+		if (!IS_ERR(fep->clk_ptp)) {
 			mutex_lock(&fep->ptp_clk_mutex);
 			clk_disable_unprepare(fep->clk_ptp);
 			fep->ptp_clk_on = false;
@@ -2640,10 +2640,8 @@ fec_probe(struct platform_device *pdev)
 	fep->clk_ptp = devm_clk_get(&pdev->dev, "ptp");
 	fep->bufdesc_ex =
 		pdev->id_entry->driver_data & FEC_QUIRK_HAS_BUFDESC_EX;
-	if (IS_ERR(fep->clk_ptp)) {
-		fep->clk_ptp = NULL;
+	if (IS_ERR(fep->clk_ptp))
 		fep->bufdesc_ex = 0;
-	}
 
 	ret = fec_enet_clk_enable(ndev, true);
 	if (ret)
-- 
1.9.1

^ permalink raw reply related

* [PATCH net-next 0/3] fec: Do not use NULL pointer to indicate error
From: Fabio Estevam @ 2014-09-02  1:12 UTC (permalink / raw)
  To: davem; +Cc: linux, B38611, netdev, Fabio Estevam

From: Fabio Estevam <fabio.estevam@freescale.com>

Using IS_ERR() to indicate error can simplify the code a bit, so let's use
it when appropriate.

Fabio Estevam (3):
  fec: Do not set fep->clk_ptp to NULL on error
  fec: Do not set fep->clk_enet_out to NULL on error
  fec: Do not set fep->reg_phy to NULL on error

 drivers/net/ethernet/freescale/fec_main.c | 28 +++++++++++-----------------
 1 file changed, 11 insertions(+), 17 deletions(-)

-- 
1.9.1

^ permalink raw reply

* Re: [PATCH net-next v2] tcp: whitespace fixes
From: David Miller @ 2014-09-02  1:13 UTC (permalink / raw)
  To: stephen; +Cc: netdev
In-Reply-To: <20140829233205.2bfa469c@urahara>

From: Stephen Hemminger <stephen@networkplumber.org>
Date: Fri, 29 Aug 2014 23:32:05 -0700

> Fix places where there is space before tab, long lines, and
> awkward if(){, double spacing etc. Add blank line after declaration/initialization.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> 
> ---
> v2 get rid of unnecessary double spacing

Applied, thanks Stephen.

^ permalink raw reply

* Re: [PATCH net-next] net: systemport: tell RXCHK if we are using Broadcom tags
From: David Miller @ 2014-09-02  1:12 UTC (permalink / raw)
  To: f.fainelli; +Cc: netdev
In-Reply-To: <1409263863-5364-1-git-send-email-f.fainelli@gmail.com>

From: Florian Fainelli <f.fainelli@gmail.com>
Date: Thu, 28 Aug 2014 15:11:03 -0700

> When Broadcom tags are enabled, e.g: when interfaced to an Ethernet
> switch, make sure that we tell the RXCHK engine that it should be
> expecting a 4-bytes Broadcom tag after the Ethernet MAC Source Address.
> 
> Use netdev_uses_dsa() to check for that condition since that will tell
> us if a switch is attached to our network interface.
> 
> Fixes: 80105befdb4b ("net: systemport: add Broadcom SYSTEMPORT Ethernet MAC driver")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Applied, thanks Florian.

^ permalink raw reply

* Re: [PATCH]     netlink: Safer deletion of sk_bind_node
From: Eric W. Biederman @ 2014-09-02  1:12 UTC (permalink / raw)
  To: Harish Jenny K N
  Cc: davem, dborkman, tgraf, darkjames-ws, rgb, eric.dumazet, stephen,
	netdev, linux-kernel
In-Reply-To: <1409555309-5893-1-git-send-email-harish_kandiga@mentor.com>

Harish Jenny K N writes:

> From: Harish Jenny K N <harish_kandiga@mentor.com>
>
>     Unable to handle kernel NULL pointer dereference at virtual address 00000000
>         (netlink_release+0x0/0x2a0) from [<8034e78c>] (sock_release+0x28/0xa4)
>         (sock_release+0x0/0xa4) from [<8034e830>] (sock_close+0x28/0x34)
>         (sock_close+0x0/0x34) from [<800f3490>] (__fput+0xf0/0x1ec)
>         (__fput+0x0/0x1ec) from [<800f3634>] (____fput+0x10/0x14)
>         (____fput+0x0/0x14) from [<80040a64>] (task_work_run+0xb8/0xd8)
>         (task_work_run+0x0/0xd8) from [<800113a0>] (do_work_pending+0xb0/0xc4)
>         (do_work_pending+0x0/0xc4) from [<8000d960>] (work_pending+0xc/0x20)
>     Call flow of the inline and static functions
>         netlink_release
>         -----netlink_remove
>         ---------__sk_del_bind_node
>         --------------__hlist_del

Is there any reason __sk_del_bind_node should not be changed instead?

If not there should be a description of what makes netlink's use of
__sk_del_bind_node special....

Eric

p.s. Your name was in your from line, but not your email address making
it hard to reply to you.

> Signed-off-by: Harish Jenny K N <harish_kandiga@mentor.com>
> ---
>  net/netlink/af_netlink.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
> index c416725..21a6b32 100644
> --- a/net/netlink/af_netlink.c
> +++ b/net/netlink/af_netlink.c
> @@ -1103,7 +1103,7 @@ static void netlink_remove(struct sock *sk)
>  
>  	netlink_table_grab();
>  	if (nlk_sk(sk)->subscriptions)
> -		__sk_del_bind_node(sk);
> +		hlist_del_init(&sk->sk_bind_node);
>  	netlink_table_ungrab();
>  }
>  
> @@ -1382,7 +1382,7 @@ netlink_update_subscriptions(struct sock *sk, unsigned int subscriptions)
>  	struct netlink_sock *nlk = nlk_sk(sk);
>  
>  	if (nlk->subscriptions && !subscriptions)
> -		__sk_del_bind_node(sk);
> +		hlist_del_init(&sk->sk_bind_node);
>  	else if (!nlk->subscriptions && subscriptions)
>  		sk_add_bind_node(sk, &nl_table[sk->sk_protocol].mc_list);
>  	nlk->subscriptions = subscriptions;

^ permalink raw reply

* Re: question about drivers/net/ethernet/ti/cpsw.c
From: David Miller @ 2014-09-02  1:11 UTC (permalink / raw)
  To: julia.lawall; +Cc: zonque, netdev
In-Reply-To: <alpine.DEB.2.02.1408282118560.2226@localhost6.localdomain6>

From: Julia Lawall <julia.lawall@lip6.fr>
Date: Thu, 28 Aug 2014 21:26:55 +0200 (CEST)

> I wonder if the following patch:
> 
> commit aa1a15e2d9199711cdcc9399fdb22544ab835a83
> Author: Daniel Mack <zonque@gmail.com>
> Date:   Sat Sep 21 00:50:38 2013 +0530
> 
> introduced a race condition in drivers/net/ethernet/ti/cpsw.c.  I was 
> looking at an old version of the file (Linux 3.10), and it has
> 
>  clean_irq_ret:
>          for (i = 0; i < priv->num_irqs; i++)
>                  free_irq(priv->irqs_table[i], priv);
> 
> at the beginning of the cleanup code of the probe function (cpsw_probe).  
> The above patch replaces request_irq by devm_request_irq and gets rid of 
> the above cleanup code.  But that moves the stopping of the interrupts 
> after the following code at the end of the function:
> 
> free_netdev(priv->ndev);
> 
> The interrupt handler (cpsw_interrupt) does reference priv->ndev:
> 
> 	if (netif_running(priv->ndev)) {
>                 napi_schedule(&priv->napi);
>                 return IRQ_HANDLED;
>         }
> 
> so perhaps this could be a problem.  The same happens in the remove 
> function.

It could definitely be a problem.

Probably it would be better for this device to request IRQs in open
and release them in close like so many other networking drivers do.

^ 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;
as well as URLs for NNTP newsgroup(s).