* Re: [PATCH net] hv_netvsc: Fix a warning of suspicious RCU usage
From: Jakub Kicinski @ 2019-08-06 19:12 UTC (permalink / raw)
To: Dexuan Cui
Cc: netdev@vger.kernel.org, David S. Miller, Haiyang Zhang,
Stephen Hemminger, sashal@kernel.org, KY Srinivasan,
Michael Kelley, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, olaf@aepfle.de, apw@canonical.com,
jasowang@redhat.com, vkuznets, marcelo.cerri@canonical.com
In-Reply-To: <PU1P153MB0169AECABF6094A3E7BEE381BFD50@PU1P153MB0169.APCP153.PROD.OUTLOOK.COM>
On Tue, 6 Aug 2019 05:17:44 +0000, Dexuan Cui wrote:
> This fixes a warning of "suspicious rcu_dereference_check() usage"
> when nload runs.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
Minor change in behaviour would perhaps be worth acknowledging in the
commit message (since you check ndev for NULL later now), and a Fixes
tag would be good.
But the looks pretty straightforward and correct!
^ permalink raw reply
* [PATCH net] netdevsim: Restore per-network namespace accounting for fib entries
From: David Ahern @ 2019-08-06 19:15 UTC (permalink / raw)
To: davem; +Cc: netdev, jiri, David Ahern
From: David Ahern <dsahern@gmail.com>
Prior to the commit in the fixes tag, the resource controller in netdevsim
tracked fib entries and rules per network namespace. Restore that behavior.
Fixes: 5fc494225c1e ("netdevsim: create devlink instance per netdevsim instance")
Signed-off-by: David Ahern <dsahern@gmail.com>
---
drivers/net/netdevsim/dev.c | 63 ++++++++++-------------
drivers/net/netdevsim/fib.c | 102 +++++++++++++++++++++++---------------
drivers/net/netdevsim/netdev.c | 9 +++-
drivers/net/netdevsim/netdevsim.h | 10 ++--
4 files changed, 98 insertions(+), 86 deletions(-)
diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index c5c417a3c0ce..bcc40a236624 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -73,46 +73,47 @@ static void nsim_dev_port_debugfs_exit(struct nsim_dev_port *nsim_dev_port)
debugfs_remove_recursive(nsim_dev_port->ddir);
}
+static struct net *nsim_devlink_net(struct devlink *devlink)
+{
+ return &init_net;
+}
+
static u64 nsim_dev_ipv4_fib_resource_occ_get(void *priv)
{
- struct nsim_dev *nsim_dev = priv;
+ struct net *net = priv;
- return nsim_fib_get_val(nsim_dev->fib_data,
- NSIM_RESOURCE_IPV4_FIB, false);
+ return nsim_fib_get_val(net, NSIM_RESOURCE_IPV4_FIB, false);
}
static u64 nsim_dev_ipv4_fib_rules_res_occ_get(void *priv)
{
- struct nsim_dev *nsim_dev = priv;
+ struct net *net = priv;
- return nsim_fib_get_val(nsim_dev->fib_data,
- NSIM_RESOURCE_IPV4_FIB_RULES, false);
+ return nsim_fib_get_val(net, NSIM_RESOURCE_IPV4_FIB_RULES, false);
}
static u64 nsim_dev_ipv6_fib_resource_occ_get(void *priv)
{
- struct nsim_dev *nsim_dev = priv;
+ struct net *net = priv;
- return nsim_fib_get_val(nsim_dev->fib_data,
- NSIM_RESOURCE_IPV6_FIB, false);
+ return nsim_fib_get_val(net, NSIM_RESOURCE_IPV6_FIB, false);
}
static u64 nsim_dev_ipv6_fib_rules_res_occ_get(void *priv)
{
- struct nsim_dev *nsim_dev = priv;
+ struct net *net = priv;
- return nsim_fib_get_val(nsim_dev->fib_data,
- NSIM_RESOURCE_IPV6_FIB_RULES, false);
+ return nsim_fib_get_val(net, NSIM_RESOURCE_IPV6_FIB_RULES, false);
}
static int nsim_dev_resources_register(struct devlink *devlink)
{
- struct nsim_dev *nsim_dev = devlink_priv(devlink);
struct devlink_resource_size_params params = {
.size_max = (u64)-1,
.size_granularity = 1,
.unit = DEVLINK_RESOURCE_UNIT_ENTRY
};
+ struct net *net = nsim_devlink_net(devlink);
int err;
u64 n;
@@ -126,8 +127,7 @@ static int nsim_dev_resources_register(struct devlink *devlink)
goto out;
}
- n = nsim_fib_get_val(nsim_dev->fib_data,
- NSIM_RESOURCE_IPV4_FIB, true);
+ n = nsim_fib_get_val(net, NSIM_RESOURCE_IPV4_FIB, true);
err = devlink_resource_register(devlink, "fib", n,
NSIM_RESOURCE_IPV4_FIB,
NSIM_RESOURCE_IPV4, ¶ms);
@@ -136,8 +136,7 @@ static int nsim_dev_resources_register(struct devlink *devlink)
return err;
}
- n = nsim_fib_get_val(nsim_dev->fib_data,
- NSIM_RESOURCE_IPV4_FIB_RULES, true);
+ n = nsim_fib_get_val(net, NSIM_RESOURCE_IPV4_FIB_RULES, true);
err = devlink_resource_register(devlink, "fib-rules", n,
NSIM_RESOURCE_IPV4_FIB_RULES,
NSIM_RESOURCE_IPV4, ¶ms);
@@ -156,8 +155,7 @@ static int nsim_dev_resources_register(struct devlink *devlink)
goto out;
}
- n = nsim_fib_get_val(nsim_dev->fib_data,
- NSIM_RESOURCE_IPV6_FIB, true);
+ n = nsim_fib_get_val(net, NSIM_RESOURCE_IPV6_FIB, true);
err = devlink_resource_register(devlink, "fib", n,
NSIM_RESOURCE_IPV6_FIB,
NSIM_RESOURCE_IPV6, ¶ms);
@@ -166,8 +164,7 @@ static int nsim_dev_resources_register(struct devlink *devlink)
return err;
}
- n = nsim_fib_get_val(nsim_dev->fib_data,
- NSIM_RESOURCE_IPV6_FIB_RULES, true);
+ n = nsim_fib_get_val(net, NSIM_RESOURCE_IPV6_FIB_RULES, true);
err = devlink_resource_register(devlink, "fib-rules", n,
NSIM_RESOURCE_IPV6_FIB_RULES,
NSIM_RESOURCE_IPV6, ¶ms);
@@ -179,19 +176,19 @@ static int nsim_dev_resources_register(struct devlink *devlink)
devlink_resource_occ_get_register(devlink,
NSIM_RESOURCE_IPV4_FIB,
nsim_dev_ipv4_fib_resource_occ_get,
- nsim_dev);
+ net);
devlink_resource_occ_get_register(devlink,
NSIM_RESOURCE_IPV4_FIB_RULES,
nsim_dev_ipv4_fib_rules_res_occ_get,
- nsim_dev);
+ net);
devlink_resource_occ_get_register(devlink,
NSIM_RESOURCE_IPV6_FIB,
nsim_dev_ipv6_fib_resource_occ_get,
- nsim_dev);
+ net);
devlink_resource_occ_get_register(devlink,
NSIM_RESOURCE_IPV6_FIB_RULES,
nsim_dev_ipv6_fib_rules_res_occ_get,
- nsim_dev);
+ net);
out:
return err;
}
@@ -199,11 +196,11 @@ static int nsim_dev_resources_register(struct devlink *devlink)
static int nsim_dev_reload(struct devlink *devlink,
struct netlink_ext_ack *extack)
{
- struct nsim_dev *nsim_dev = devlink_priv(devlink);
enum nsim_resource_id res_ids[] = {
NSIM_RESOURCE_IPV4_FIB, NSIM_RESOURCE_IPV4_FIB_RULES,
NSIM_RESOURCE_IPV6_FIB, NSIM_RESOURCE_IPV6_FIB_RULES
};
+ struct net *net = nsim_devlink_net(devlink);
int i;
for (i = 0; i < ARRAY_SIZE(res_ids); ++i) {
@@ -212,8 +209,7 @@ static int nsim_dev_reload(struct devlink *devlink,
err = devlink_resource_size_get(devlink, res_ids[i], &val);
if (!err) {
- err = nsim_fib_set_max(nsim_dev->fib_data,
- res_ids[i], val, extack);
+ err = nsim_fib_set_max(net, res_ids[i], val, extack);
if (err)
return err;
}
@@ -285,15 +281,9 @@ nsim_dev_create(struct nsim_bus_dev *nsim_bus_dev, unsigned int port_count)
mutex_init(&nsim_dev->port_list_lock);
nsim_dev->fw_update_status = true;
- nsim_dev->fib_data = nsim_fib_create();
- if (IS_ERR(nsim_dev->fib_data)) {
- err = PTR_ERR(nsim_dev->fib_data);
- goto err_devlink_free;
- }
-
err = nsim_dev_resources_register(devlink);
if (err)
- goto err_fib_destroy;
+ goto err_devlink_free;
err = devlink_register(devlink, &nsim_bus_dev->dev);
if (err)
@@ -315,8 +305,6 @@ nsim_dev_create(struct nsim_bus_dev *nsim_bus_dev, unsigned int port_count)
devlink_unregister(devlink);
err_resources_unregister:
devlink_resources_unregister(devlink, NULL);
-err_fib_destroy:
- nsim_fib_destroy(nsim_dev->fib_data);
err_devlink_free:
devlink_free(devlink);
return ERR_PTR(err);
@@ -330,7 +318,6 @@ static void nsim_dev_destroy(struct nsim_dev *nsim_dev)
nsim_dev_debugfs_exit(nsim_dev);
devlink_unregister(devlink);
devlink_resources_unregister(devlink, NULL);
- nsim_fib_destroy(nsim_dev->fib_data);
mutex_destroy(&nsim_dev->port_list_lock);
devlink_free(devlink);
}
diff --git a/drivers/net/netdevsim/fib.c b/drivers/net/netdevsim/fib.c
index 8c57ba747772..f61d094746c0 100644
--- a/drivers/net/netdevsim/fib.c
+++ b/drivers/net/netdevsim/fib.c
@@ -18,6 +18,7 @@
#include <net/ip_fib.h>
#include <net/ip6_fib.h>
#include <net/fib_rules.h>
+#include <net/netns/generic.h>
#include "netdevsim.h"
@@ -32,14 +33,15 @@ struct nsim_per_fib_data {
};
struct nsim_fib_data {
- struct notifier_block fib_nb;
struct nsim_per_fib_data ipv4;
struct nsim_per_fib_data ipv6;
};
-u64 nsim_fib_get_val(struct nsim_fib_data *fib_data,
- enum nsim_resource_id res_id, bool max)
+static unsigned int nsim_fib_net_id;
+
+u64 nsim_fib_get_val(struct net *net, enum nsim_resource_id res_id, bool max)
{
+ struct nsim_fib_data *fib_data = net_generic(net, nsim_fib_net_id);
struct nsim_fib_entry *entry;
switch (res_id) {
@@ -62,10 +64,10 @@ u64 nsim_fib_get_val(struct nsim_fib_data *fib_data,
return max ? entry->max : entry->num;
}
-int nsim_fib_set_max(struct nsim_fib_data *fib_data,
- enum nsim_resource_id res_id, u64 val,
+int nsim_fib_set_max(struct net *net, enum nsim_resource_id res_id, u64 val,
struct netlink_ext_ack *extack)
{
+ struct nsim_fib_data *fib_data = net_generic(net, nsim_fib_net_id);
struct nsim_fib_entry *entry;
int err = 0;
@@ -118,9 +120,9 @@ static int nsim_fib_rule_account(struct nsim_fib_entry *entry, bool add,
return err;
}
-static int nsim_fib_rule_event(struct nsim_fib_data *data,
- struct fib_notifier_info *info, bool add)
+static int nsim_fib_rule_event(struct fib_notifier_info *info, bool add)
{
+ struct nsim_fib_data *data = net_generic(info->net, nsim_fib_net_id);
struct netlink_ext_ack *extack = info->extack;
int err = 0;
@@ -155,9 +157,9 @@ static int nsim_fib_account(struct nsim_fib_entry *entry, bool add,
return err;
}
-static int nsim_fib_event(struct nsim_fib_data *data,
- struct fib_notifier_info *info, bool add)
+static int nsim_fib_event(struct fib_notifier_info *info, bool add)
{
+ struct nsim_fib_data *data = net_generic(info->net, nsim_fib_net_id);
struct netlink_ext_ack *extack = info->extack;
int err = 0;
@@ -176,22 +178,18 @@ static int nsim_fib_event(struct nsim_fib_data *data,
static int nsim_fib_event_nb(struct notifier_block *nb, unsigned long event,
void *ptr)
{
- struct nsim_fib_data *data = container_of(nb, struct nsim_fib_data,
- fib_nb);
struct fib_notifier_info *info = ptr;
int err = 0;
switch (event) {
case FIB_EVENT_RULE_ADD: /* fall through */
case FIB_EVENT_RULE_DEL:
- err = nsim_fib_rule_event(data, info,
- event == FIB_EVENT_RULE_ADD);
+ err = nsim_fib_rule_event(info, event == FIB_EVENT_RULE_ADD);
break;
case FIB_EVENT_ENTRY_ADD: /* fall through */
case FIB_EVENT_ENTRY_DEL:
- err = nsim_fib_event(data, info,
- event == FIB_EVENT_ENTRY_ADD);
+ err = nsim_fib_event(info, event == FIB_EVENT_ENTRY_ADD);
break;
}
@@ -201,23 +199,30 @@ static int nsim_fib_event_nb(struct notifier_block *nb, unsigned long event,
/* inconsistent dump, trying again */
static void nsim_fib_dump_inconsistent(struct notifier_block *nb)
{
- struct nsim_fib_data *data = container_of(nb, struct nsim_fib_data,
- fib_nb);
+ struct nsim_fib_data *data;
+ struct net *net;
+
+ rcu_read_lock();
+ for_each_net_rcu(net) {
+ data = net_generic(net, nsim_fib_net_id);
+
+ data->ipv4.fib.num = 0ULL;
+ data->ipv4.rules.num = 0ULL;
- data->ipv4.fib.num = 0ULL;
- data->ipv4.rules.num = 0ULL;
- data->ipv6.fib.num = 0ULL;
- data->ipv6.rules.num = 0ULL;
+ data->ipv6.fib.num = 0ULL;
+ data->ipv6.rules.num = 0ULL;
+ }
+ rcu_read_unlock();
}
-struct nsim_fib_data *nsim_fib_create(void)
-{
- struct nsim_fib_data *data;
- int err;
+static struct notifier_block nsim_fib_nb = {
+ .notifier_call = nsim_fib_event_nb,
+};
- data = kzalloc(sizeof(*data), GFP_KERNEL);
- if (!data)
- return ERR_PTR(-ENOMEM);
+/* Initialize per network namespace state */
+static int __net_init nsim_fib_netns_init(struct net *net)
+{
+ struct nsim_fib_data *data = net_generic(net, nsim_fib_net_id);
data->ipv4.fib.max = (u64)-1;
data->ipv4.rules.max = (u64)-1;
@@ -225,22 +230,37 @@ struct nsim_fib_data *nsim_fib_create(void)
data->ipv6.fib.max = (u64)-1;
data->ipv6.rules.max = (u64)-1;
- data->fib_nb.notifier_call = nsim_fib_event_nb;
- err = register_fib_notifier(&data->fib_nb, nsim_fib_dump_inconsistent);
- if (err) {
- pr_err("Failed to register fib notifier\n");
- goto err_out;
- }
+ return 0;
+}
- return data;
+static struct pernet_operations nsim_fib_net_ops = {
+ .init = nsim_fib_netns_init,
+ .id = &nsim_fib_net_id,
+ .size = sizeof(struct nsim_fib_data),
+};
-err_out:
- kfree(data);
- return ERR_PTR(err);
+void nsim_fib_exit(void)
+{
+ unregister_pernet_subsys(&nsim_fib_net_ops);
+ unregister_fib_notifier(&nsim_fib_nb);
}
-void nsim_fib_destroy(struct nsim_fib_data *data)
+int nsim_fib_init(void)
{
- unregister_fib_notifier(&data->fib_nb);
- kfree(data);
+ int err;
+
+ err = register_pernet_subsys(&nsim_fib_net_ops);
+ if (err < 0) {
+ pr_err("Failed to register pernet subsystem\n");
+ goto err_out;
+ }
+
+ err = register_fib_notifier(&nsim_fib_nb, nsim_fib_dump_inconsistent);
+ if (err < 0) {
+ pr_err("Failed to register fib notifier\n");
+ goto err_out;
+ }
+
+err_out:
+ return err;
}
diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index 0740940f41b1..55f57f76d01b 100644
--- a/drivers/net/netdevsim/netdev.c
+++ b/drivers/net/netdevsim/netdev.c
@@ -357,12 +357,18 @@ static int __init nsim_module_init(void)
if (err)
goto err_dev_exit;
- err = rtnl_link_register(&nsim_link_ops);
+ err = nsim_fib_init();
if (err)
goto err_bus_exit;
+ err = rtnl_link_register(&nsim_link_ops);
+ if (err)
+ goto err_fib_exit;
+
return 0;
+err_fib_exit:
+ nsim_fib_exit();
err_bus_exit:
nsim_bus_exit();
err_dev_exit:
@@ -373,6 +379,7 @@ static int __init nsim_module_init(void)
static void __exit nsim_module_exit(void)
{
rtnl_link_unregister(&nsim_link_ops);
+ nsim_fib_exit();
nsim_bus_exit();
nsim_dev_exit();
}
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index 79c05af2a7c0..9404637d34b7 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -169,12 +169,10 @@ int nsim_dev_port_add(struct nsim_bus_dev *nsim_bus_dev,
int nsim_dev_port_del(struct nsim_bus_dev *nsim_bus_dev,
unsigned int port_index);
-struct nsim_fib_data *nsim_fib_create(void);
-void nsim_fib_destroy(struct nsim_fib_data *fib_data);
-u64 nsim_fib_get_val(struct nsim_fib_data *fib_data,
- enum nsim_resource_id res_id, bool max);
-int nsim_fib_set_max(struct nsim_fib_data *fib_data,
- enum nsim_resource_id res_id, u64 val,
+int nsim_fib_init(void);
+void nsim_fib_exit(void);
+u64 nsim_fib_get_val(struct net *net, enum nsim_resource_id res_id, bool max);
+int nsim_fib_set_max(struct net *net, enum nsim_resource_id res_id, u64 val,
struct netlink_ext_ack *extack);
#if IS_ENABLED(CONFIG_XFRM_OFFLOAD)
--
2.11.0
^ permalink raw reply related
* Re: [PATCH net-next v2 1/1] qed: Add new ethtool supported port types based on media.
From: Jakub Kicinski @ 2019-08-06 19:14 UTC (permalink / raw)
To: Rahul Verma; +Cc: davem, netdev, aelior, mkalderon
In-Reply-To: <20190806065950.19073-1-rahulv@marvell.com>
On Mon, 5 Aug 2019 23:59:50 -0700, Rahul Verma wrote:
> Supported ports in ethtool <eth1> are displayed based on media type.
> For media type fibre and twinaxial, port type is "FIBRE". Media type
> Base-T is "TP" and media KR is "Backplane".
>
> V1->V2:
> Corrected the subject.
>
> Signed-off-by: Rahul Verma <rahulv@marvell.com>
> Signed-off-by: Michal Kalderon <michal.kalderon@marvell.com>
LGTM
^ permalink raw reply
* Re: [PATCH net 0/3] net: stmmac: Fixes for -net
From: David Miller @ 2019-08-06 19:26 UTC (permalink / raw)
To: Jose.Abreu
Cc: netdev, Joao.Pinto, peppe.cavallaro, alexandre.torgue,
mcoquelin.stm32, linux-stm32, linux-arm-kernel, linux-kernel
In-Reply-To: <cover.1565097294.git.joabreu@synopsys.com>
From: Jose Abreu <Jose.Abreu@synopsys.com>
Date: Tue, 6 Aug 2019 15:16:15 +0200
> Couple of fixes for -net. More info in commit log.
Series applied, thank you.
^ permalink raw reply
* Re: [PATCH 17/17] ieee802154: no need to check return value of debugfs_create functions
From: Stefan Schmidt @ 2019-08-06 19:22 UTC (permalink / raw)
To: Greg Kroah-Hartman, netdev
Cc: Michael Hennerich, Alexander Aring, David S. Miller, Harry Morris,
linux-wpan
In-Reply-To: <20190806161128.31232-18-gregkh@linuxfoundation.org>
Hello.
On 06.08.19 18:11, Greg Kroah-Hartman wrote:
> When calling debugfs functions, there is no need to ever check the
> return value. The function can work or not, but the code logic should
> never do something different based on this.
>
> Cc: Michael Hennerich <michael.hennerich@analog.com>
> Cc: Alexander Aring <alex.aring@gmail.com>
> Cc: Stefan Schmidt <stefan@datenfreihafen.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Harry Morris <h.morris@cascoda.com>
> Cc: linux-wpan@vger.kernel.org
> Cc: netdev@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> drivers/net/ieee802154/adf7242.c | 12 +++---------
> drivers/net/ieee802154/at86rf230.c | 20 +++++---------------
> drivers/net/ieee802154/ca8210.c | 9 +--------
> 3 files changed, 9 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/net/ieee802154/adf7242.c b/drivers/net/ieee802154/adf7242.c
> index c9392d70e639..7b95add2235a 100644
> --- a/drivers/net/ieee802154/adf7242.c
> +++ b/drivers/net/ieee802154/adf7242.c
> @@ -1158,7 +1158,7 @@ static int adf7242_stats_show(struct seq_file *file, void *offset)
> return 0;
> }
>
> -static int adf7242_debugfs_init(struct adf7242_local *lp)
> +static void adf7242_debugfs_init(struct adf7242_local *lp)
> {
> char debugfs_dir_name[DNAME_INLINE_LEN + 1] = "adf7242-";
> struct dentry *stats;
A quick look over the code indicates that the stats variable can go as
well now as it is only used in the now removed code.
> @@ -1166,15 +1166,9 @@ static int adf7242_debugfs_init(struct adf7242_local *lp)
> strncat(debugfs_dir_name, dev_name(&lp->spi->dev), DNAME_INLINE_LEN);
>
> lp->debugfs_root = debugfs_create_dir(debugfs_dir_name, NULL);
> - if (IS_ERR_OR_NULL(lp->debugfs_root))
> - return PTR_ERR_OR_ZERO(lp->debugfs_root);
>
> - stats = debugfs_create_devm_seqfile(&lp->spi->dev, "status",
> - lp->debugfs_root,
> - adf7242_stats_show);
> - return PTR_ERR_OR_ZERO(stats);
> -
> - return 0;
> + debugfs_create_devm_seqfile(&lp->spi->dev, "status", lp->debugfs_root,
> + adf7242_stats_show);
> }
>
> static const s32 adf7242_powers[] = {
> diff --git a/drivers/net/ieee802154/at86rf230.c b/drivers/net/ieee802154/at86rf230.c
> index 595cf7e2a651..7d67f41387f5 100644
> --- a/drivers/net/ieee802154/at86rf230.c
> +++ b/drivers/net/ieee802154/at86rf230.c
> @@ -1626,24 +1626,16 @@ static int at86rf230_stats_show(struct seq_file *file, void *offset)
> }
> DEFINE_SHOW_ATTRIBUTE(at86rf230_stats);
>
> -static int at86rf230_debugfs_init(struct at86rf230_local *lp)
> +static void at86rf230_debugfs_init(struct at86rf230_local *lp)
> {
> char debugfs_dir_name[DNAME_INLINE_LEN + 1] = "at86rf230-";
> - struct dentry *stats;
>
> strncat(debugfs_dir_name, dev_name(&lp->spi->dev), DNAME_INLINE_LEN);
>
> at86rf230_debugfs_root = debugfs_create_dir(debugfs_dir_name, NULL);
> - if (!at86rf230_debugfs_root)
> - return -ENOMEM;
> -
> - stats = debugfs_create_file("trac_stats", 0444,
> - at86rf230_debugfs_root, lp,
> - &at86rf230_stats_fops);
> - if (!stats)
> - return -ENOMEM;
>
> - return 0;
> + debugfs_create_file("trac_stats", 0444, at86rf230_debugfs_root, lp,
> + &at86rf230_stats_fops);
> }
>
> static void at86rf230_debugfs_remove(void)
> @@ -1651,7 +1643,7 @@ static void at86rf230_debugfs_remove(void)
> debugfs_remove_recursive(at86rf230_debugfs_root);
> }
> #else
> -static int at86rf230_debugfs_init(struct at86rf230_local *lp) { return 0; }
> +static void at86rf230_debugfs_init(struct at86rf230_local *lp) { }
> static void at86rf230_debugfs_remove(void) { }
> #endif
>
> @@ -1751,9 +1743,7 @@ static int at86rf230_probe(struct spi_device *spi)
> /* going into sleep by default */
> at86rf230_sleep(lp);
>
> - rc = at86rf230_debugfs_init(lp);
> - if (rc)
> - goto free_dev;
> + at86rf230_debugfs_init(lp);
>
> rc = ieee802154_register_hw(lp->hw);
> if (rc)
> diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
> index b188fce3f641..11402dc347db 100644
> --- a/drivers/net/ieee802154/ca8210.c
> +++ b/drivers/net/ieee802154/ca8210.c
> @@ -3019,14 +3019,7 @@ static int ca8210_test_interface_init(struct ca8210_priv *priv)
> priv,
> &test_int_fops
> );
> - if (IS_ERR(test->ca8210_dfs_spi_int)) {
> - dev_err(
> - &priv->spi->dev,
> - "Error %ld when creating debugfs node\n",
> - PTR_ERR(test->ca8210_dfs_spi_int)
> - );
> - return PTR_ERR(test->ca8210_dfs_spi_int);
> - }
> +
> debugfs_create_symlink("ca8210", NULL, node_name);
> init_waitqueue_head(&test->readq);
> return kfifo_alloc(
>
With a fix for the above included you can have my
Acked-by: Stefan Schmidt <stefan@datenfreihafen.org>
for version 2.
regards
Stefan Schmidt
^ permalink raw reply
* Re: [PATCH net-next 0/6] drop_monitor: Various improvements and cleanups
From: David Miller @ 2019-08-06 19:38 UTC (permalink / raw)
To: idosch; +Cc: netdev, nhorman, toke, jiri, dsahern, mlxsw, idosch
In-Reply-To: <20190806131956.26168-1-idosch@idosch.org>
From: Ido Schimmel <idosch@idosch.org>
Date: Tue, 6 Aug 2019 16:19:50 +0300
> From: Ido Schimmel <idosch@mellanox.com>
>
> This patchset performs various improvements and cleanups in drop monitor
> with no functional changes intended. There are no changes in these
> patches relative to the RFC I sent two weeks ago [1].
>
> A followup patchset will extend drop monitor with a packet alert mode in
> which the dropped packet is notified to user space instead of just a
> summary of recent drops. Subsequent patchsets will add the ability to
> monitor hardware originated drops via drop monitor.
>
> [1] https://patchwork.ozlabs.org/cover/1135226/
These all look fine to me, series applied.
Thanks.
^ permalink raw reply
* Re: [PATCH net-next v2 00/10] net: stmmac: Improvements for -next
From: Jakub Kicinski @ 2019-08-06 19:40 UTC (permalink / raw)
To: Jose Abreu
Cc: netdev, Joao Pinto, Giuseppe Cavallaro, Alexandre Torgue,
David S. Miller, Maxime Coquelin, linux-stm32, linux-arm-kernel,
linux-kernel
In-Reply-To: <cover.1565098881.git.joabreu@synopsys.com>
On Tue, 6 Aug 2019 15:42:41 +0200, Jose Abreu wrote:
> Couple of improvements for -next tree. More info in commit logs.
Code looks good to me now, thanks!
^ permalink raw reply
* Re: [PATCH 03/17] mlx5: no need to check return value of debugfs_create functions
From: Saeed Mahameed @ 2019-08-06 19:41 UTC (permalink / raw)
To: gregkh@linuxfoundation.org, netdev@vger.kernel.org; +Cc: leon@kernel.org
In-Reply-To: <20190806161128.31232-4-gregkh@linuxfoundation.org>
On Tue, 2019-08-06 at 18:11 +0200, Greg Kroah-Hartman wrote:
> When calling debugfs functions, there is no need to ever check the
> return value. The function can work or not, but the code logic
> should
> never do something different based on this.
>
> This cleans up a lot of unneeded code and logic around the debugfs
> files, making all of this much simpler and easier to understand as we
> don't need to keep the dentries saved anymore.
>
Hi Greg,
Basically i am ok with this patch and i like it very much.., but i am
concerned about some of the driver internal flows that are dependent on
these debug fs entries being valid.
for example mlx5_debug_eq_add if failed, it will fail the whole flow. I
know it is wrong even before your patch.. but maybe we should deal with
it now ? or let me know if you want me to follow up with my own patch.
All we need to improve in this patch is to void out add_res_tree()
implemented in
drivers/net/ethernet/mellanox/mlx5/core/debugfs.c
as i will comment below.
> Cc: Saeed Mahameed <saeedm@mellanox.com>
> Cc: Leon Romanovsky <leon@kernel.org>
> Cc: netdev@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> drivers/net/ethernet/mellanox/mlx5/core/cmd.c | 51 ++-------
> .../net/ethernet/mellanox/mlx5/core/debugfs.c | 102 ++------------
> ----
> drivers/net/ethernet/mellanox/mlx5/core/eq.c | 11 +-
> .../net/ethernet/mellanox/mlx5/core/lib/eq.h | 2 +-
> .../net/ethernet/mellanox/mlx5/core/main.c | 7 +-
> .../ethernet/mellanox/mlx5/core/mlx5_core.h | 2 +-
> include/linux/mlx5/driver.h | 12 +--
> 7 files changed, 24 insertions(+), 163 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
> b/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
> index 8cdd7e66f8df..973f90888b1f 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
> @@ -1368,49 +1368,19 @@ static void clean_debug_files(struct
> mlx5_core_dev *dev)
> debugfs_remove_recursive(dbg->dbg_root);
> }
>
[...]
> void mlx5_cq_debugfs_cleanup(struct mlx5_core_dev *dev)
> {
> - if (!mlx5_debugfs_root)
> - return;
> -
> debugfs_remove_recursive(dev->priv.cq_debugfs);
> }
>
> @@ -484,7 +418,6 @@ static int add_res_tree(struct mlx5_core_dev
Basically this function is a debugfs wrapper that should behave the
same as debug_fs_*, we should fix it to return void as well, and
improve all the its callers to ignore the return value.
callers are:
mlx5_debug_qp_add()
mlx5_debug_eq_add()
mlx5_debug_cq_add()
> *dev, enum dbg_rsc_type type,
> {
> struct mlx5_rsc_debug *d;
> char resn[32];
> - int err;
> int i;
>
> d = kzalloc(struct_size(d, fields, nfile), GFP_KERNEL);
> @@ -496,30 +429,15 @@ static int add_res_tree(struct mlx5_core_dev
> *dev, enum dbg_rsc_type type,
> d->type = type;
> sprintf(resn, "0x%x", rsn);
> d->root = debugfs_create_dir(resn, root);
> - if (!d->root) {
> - err = -ENOMEM;
> - goto out_free;
> - }
>
> for (i = 0; i < nfile; i++) {
> d->fields[i].i = i;
> - d->fields[i].dent = debugfs_create_file(field[i], 0400,
> - d->root, &d-
> >fields[i],
> - &fops);
> - if (!d->fields[i].dent) {
> - err = -ENOMEM;
> - goto out_rem;
> - }
> + debugfs_create_file(field[i], 0400, d->root, &d-
> >fields[i],
> + &fops);
> }
> *dbg = d;
>
> return 0;
> -out_rem:
> - debugfs_remove_recursive(d->root);
> -
> -out_free:
> - kfree(d);
> - return err;
> }
^ permalink raw reply
* Re: [PATCH 03/17] mlx5: no need to check return value of debugfs_create functions
From: gregkh @ 2019-08-06 19:47 UTC (permalink / raw)
To: Saeed Mahameed; +Cc: netdev@vger.kernel.org, leon@kernel.org
In-Reply-To: <d681be03ea2c1997004c8144c3a6062f895817a4.camel@mellanox.com>
On Tue, Aug 06, 2019 at 07:41:57PM +0000, Saeed Mahameed wrote:
> On Tue, 2019-08-06 at 18:11 +0200, Greg Kroah-Hartman wrote:
> > When calling debugfs functions, there is no need to ever check the
> > return value. The function can work or not, but the code logic
> > should
> > never do something different based on this.
> >
> > This cleans up a lot of unneeded code and logic around the debugfs
> > files, making all of this much simpler and easier to understand as we
> > don't need to keep the dentries saved anymore.
> >
>
> Hi Greg,
>
> Basically i am ok with this patch and i like it very much.., but i am
> concerned about some of the driver internal flows that are dependent on
> these debug fs entries being valid.
That's never good, that's a bug in the driver :)
> for example mlx5_debug_eq_add if failed, it will fail the whole flow. I
> know it is wrong even before your patch.. but maybe we should deal with
> it now ? or let me know if you want me to follow up with my own patch.
Your own patch would be good as I do not know this part of the codebase
at all, thanks.
> All we need to improve in this patch is to void out add_res_tree()
> implemented in
> drivers/net/ethernet/mellanox/mlx5/core/debugfs.c
> as i will comment below.
>
>
> > Cc: Saeed Mahameed <saeedm@mellanox.com>
> > Cc: Leon Romanovsky <leon@kernel.org>
> > Cc: netdev@vger.kernel.org
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> > drivers/net/ethernet/mellanox/mlx5/core/cmd.c | 51 ++-------
> > .../net/ethernet/mellanox/mlx5/core/debugfs.c | 102 ++------------
> > ----
> > drivers/net/ethernet/mellanox/mlx5/core/eq.c | 11 +-
> > .../net/ethernet/mellanox/mlx5/core/lib/eq.h | 2 +-
> > .../net/ethernet/mellanox/mlx5/core/main.c | 7 +-
> > .../ethernet/mellanox/mlx5/core/mlx5_core.h | 2 +-
> > include/linux/mlx5/driver.h | 12 +--
> > 7 files changed, 24 insertions(+), 163 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
> > b/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
> > index 8cdd7e66f8df..973f90888b1f 100644
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
> > @@ -1368,49 +1368,19 @@ static void clean_debug_files(struct
> > mlx5_core_dev *dev)
> > debugfs_remove_recursive(dbg->dbg_root);
> > }
> >
>
> [...]
>
> > void mlx5_cq_debugfs_cleanup(struct mlx5_core_dev *dev)
> > {
> > - if (!mlx5_debugfs_root)
> > - return;
> > -
> > debugfs_remove_recursive(dev->priv.cq_debugfs);
> > }
> >
> > @@ -484,7 +418,6 @@ static int add_res_tree(struct mlx5_core_dev
>
> Basically this function is a debugfs wrapper that should behave the
> same as debug_fs_*, we should fix it to return void as well, and
> improve all the its callers to ignore the return value.
But mlx5_cq_debugfs_cleanup() is a void function.
> callers are:
> mlx5_debug_qp_add()
> mlx5_debug_eq_add()
> mlx5_debug_cq_add()
Ah, you mean add_res_tree(). Yes, make that void as well.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 17/17] ieee802154: no need to check return value of debugfs_create functions
From: Greg Kroah-Hartman @ 2019-08-06 19:48 UTC (permalink / raw)
To: Stefan Schmidt
Cc: netdev, Michael Hennerich, Alexander Aring, David S. Miller,
Harry Morris, linux-wpan
In-Reply-To: <a16654ca-078a-8ce4-91a5-055a3ad6e838@datenfreihafen.org>
On Tue, Aug 06, 2019 at 09:22:43PM +0200, Stefan Schmidt wrote:
> Hello.
>
> On 06.08.19 18:11, Greg Kroah-Hartman wrote:
> > When calling debugfs functions, there is no need to ever check the
> > return value. The function can work or not, but the code logic should
> > never do something different based on this.
> >
> > Cc: Michael Hennerich <michael.hennerich@analog.com>
> > Cc: Alexander Aring <alex.aring@gmail.com>
> > Cc: Stefan Schmidt <stefan@datenfreihafen.org>
> > Cc: "David S. Miller" <davem@davemloft.net>
> > Cc: Harry Morris <h.morris@cascoda.com>
> > Cc: linux-wpan@vger.kernel.org
> > Cc: netdev@vger.kernel.org
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> > drivers/net/ieee802154/adf7242.c | 12 +++---------
> > drivers/net/ieee802154/at86rf230.c | 20 +++++---------------
> > drivers/net/ieee802154/ca8210.c | 9 +--------
> > 3 files changed, 9 insertions(+), 32 deletions(-)
> >
> > diff --git a/drivers/net/ieee802154/adf7242.c b/drivers/net/ieee802154/adf7242.c
> > index c9392d70e639..7b95add2235a 100644
> > --- a/drivers/net/ieee802154/adf7242.c
> > +++ b/drivers/net/ieee802154/adf7242.c
> > @@ -1158,7 +1158,7 @@ static int adf7242_stats_show(struct seq_file *file, void *offset)
> > return 0;
> > }
> >
> > -static int adf7242_debugfs_init(struct adf7242_local *lp)
> > +static void adf7242_debugfs_init(struct adf7242_local *lp)
> > {
> > char debugfs_dir_name[DNAME_INLINE_LEN + 1] = "adf7242-";
> > struct dentry *stats;
>
> A quick look over the code indicates that the stats variable can go as
> well now as it is only used in the now removed code.
Odd 0-day never gave me a build warning for it, sorry about that.
>
> > @@ -1166,15 +1166,9 @@ static int adf7242_debugfs_init(struct adf7242_local *lp)
> > strncat(debugfs_dir_name, dev_name(&lp->spi->dev), DNAME_INLINE_LEN);
> >
> > lp->debugfs_root = debugfs_create_dir(debugfs_dir_name, NULL);
> > - if (IS_ERR_OR_NULL(lp->debugfs_root))
> > - return PTR_ERR_OR_ZERO(lp->debugfs_root);
> >
> > - stats = debugfs_create_devm_seqfile(&lp->spi->dev, "status",
> > - lp->debugfs_root,
> > - adf7242_stats_show);
> > - return PTR_ERR_OR_ZERO(stats);
> > -
> > - return 0;
> > + debugfs_create_devm_seqfile(&lp->spi->dev, "status", lp->debugfs_root,
> > + adf7242_stats_show);
> > }
> >
> > static const s32 adf7242_powers[] = {
> > diff --git a/drivers/net/ieee802154/at86rf230.c b/drivers/net/ieee802154/at86rf230.c
> > index 595cf7e2a651..7d67f41387f5 100644
> > --- a/drivers/net/ieee802154/at86rf230.c
> > +++ b/drivers/net/ieee802154/at86rf230.c
> > @@ -1626,24 +1626,16 @@ static int at86rf230_stats_show(struct seq_file *file, void *offset)
> > }
> > DEFINE_SHOW_ATTRIBUTE(at86rf230_stats);
> >
> > -static int at86rf230_debugfs_init(struct at86rf230_local *lp)
> > +static void at86rf230_debugfs_init(struct at86rf230_local *lp)
> > {
> > char debugfs_dir_name[DNAME_INLINE_LEN + 1] = "at86rf230-";
> > - struct dentry *stats;
> >
> > strncat(debugfs_dir_name, dev_name(&lp->spi->dev), DNAME_INLINE_LEN);
> >
> > at86rf230_debugfs_root = debugfs_create_dir(debugfs_dir_name, NULL);
> > - if (!at86rf230_debugfs_root)
> > - return -ENOMEM;
> > -
> > - stats = debugfs_create_file("trac_stats", 0444,
> > - at86rf230_debugfs_root, lp,
> > - &at86rf230_stats_fops);
> > - if (!stats)
> > - return -ENOMEM;
> >
> > - return 0;
> > + debugfs_create_file("trac_stats", 0444, at86rf230_debugfs_root, lp,
> > + &at86rf230_stats_fops);
> > }
> >
> > static void at86rf230_debugfs_remove(void)
> > @@ -1651,7 +1643,7 @@ static void at86rf230_debugfs_remove(void)
> > debugfs_remove_recursive(at86rf230_debugfs_root);
> > }
> > #else
> > -static int at86rf230_debugfs_init(struct at86rf230_local *lp) { return 0; }
> > +static void at86rf230_debugfs_init(struct at86rf230_local *lp) { }
> > static void at86rf230_debugfs_remove(void) { }
> > #endif
> >
> > @@ -1751,9 +1743,7 @@ static int at86rf230_probe(struct spi_device *spi)
> > /* going into sleep by default */
> > at86rf230_sleep(lp);
> >
> > - rc = at86rf230_debugfs_init(lp);
> > - if (rc)
> > - goto free_dev;
> > + at86rf230_debugfs_init(lp);
> >
> > rc = ieee802154_register_hw(lp->hw);
> > if (rc)
> > diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
> > index b188fce3f641..11402dc347db 100644
> > --- a/drivers/net/ieee802154/ca8210.c
> > +++ b/drivers/net/ieee802154/ca8210.c
> > @@ -3019,14 +3019,7 @@ static int ca8210_test_interface_init(struct ca8210_priv *priv)
> > priv,
> > &test_int_fops
> > );
> > - if (IS_ERR(test->ca8210_dfs_spi_int)) {
> > - dev_err(
> > - &priv->spi->dev,
> > - "Error %ld when creating debugfs node\n",
> > - PTR_ERR(test->ca8210_dfs_spi_int)
> > - );
> > - return PTR_ERR(test->ca8210_dfs_spi_int);
> > - }
> > +
> > debugfs_create_symlink("ca8210", NULL, node_name);
> > init_waitqueue_head(&test->readq);
> > return kfifo_alloc(
> >
>
> With a fix for the above included you can have my
>
>
> Acked-by: Stefan Schmidt <stefan@datenfreihafen.org>
>
> for version 2.
I'll fix it up and resend tomorrow, thanks!
greg k-h
^ permalink raw reply
* Re: [Intel-wired-lan] [PATCH 1/1] ixgbe: sync the first fragment unconditionally
From: Alexander Duyck @ 2019-08-06 19:52 UTC (permalink / raw)
To: David Miller; +Cc: firo.yang, Netdev, intel-wired-lan, LKML
In-Reply-To: <20190806.113640.171591509807004446.davem@davemloft.net>
On Tue, Aug 6, 2019 at 11:36 AM David Miller <davem@davemloft.net> wrote:
>
> From: Firo Yang <firo.yang@suse.com>
> Date: Tue, 6 Aug 2019 09:29:51 +0000
>
> > In Xen environment, if Xen-swiotlb is enabled, ixgbe driver
> > could possibly allocate a page, DMA memory buffer, for the first
> > fragment which is not suitable for Xen-swiotlb to do DMA operations.
> > Xen-swiotlb will internally allocate another page for doing DMA
> > operations. It requires syncing between those two pages. Otherwise,
> > we may get an incomplete skb. To fix this problem, sync the first
> > fragment no matter the first fargment is makred as "page_released"
> > or not.
> >
> > Signed-off-by: Firo Yang <firo.yang@suse.com>
>
> I don't understand, an unmap operation implies a sync operation.
Actually it doesn't because ixgbe is mapping and unmapping with
DMA_ATTR_SKIP_CPU_SYNC.
The patch description isn't very good. The issue is that the sync in
this case is being skipped in ixgbe_get_rx_buffer for a frame where
the buffer spans more then a single page. As such we need to do both
the sync and the unmap call on the last frame when we encounter the
End Of Packet.
^ permalink raw reply
* Re: [PATCH net-next 2/5] r8152: replace array with linking list for rx information
From: Jakub Kicinski @ 2019-08-06 19:53 UTC (permalink / raw)
To: Hayes Wang; +Cc: netdev, nic_swsd, linux-kernel, linux-usb
In-Reply-To: <1394712342-15778-291-albertk@realtek.com>
On Tue, 6 Aug 2019 19:18:01 +0800, Hayes Wang wrote:
> The original method uses an array to store the rx information. The
> new one uses a list to link each rx structure. Then, it is possible
> to increase/decrease the number of rx structure dynamically.
>
> Signed-off-by: Hayes Wang <hayeswang@realtek.com>
> ---
> drivers/net/usb/r8152.c | 187 ++++++++++++++++++++++++++++------------
> 1 file changed, 132 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index 0f07ed05ab34..44d832ceb516 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -22,6 +22,7 @@
> #include <linux/mdio.h>
> #include <linux/usb/cdc.h>
> #include <linux/suspend.h>
> +#include <linux/atomic.h>
> #include <linux/acpi.h>
>
> /* Information for net-next */
> @@ -694,7 +695,7 @@ struct tx_desc {
> struct r8152;
>
> struct rx_agg {
> - struct list_head list;
> + struct list_head list, info_list;
> struct urb *urb;
> struct r8152 *context;
> void *buffer;
> @@ -719,7 +720,7 @@ struct r8152 {
> struct net_device *netdev;
> struct urb *intr_urb;
> struct tx_agg tx_info[RTL8152_MAX_TX];
> - struct rx_agg rx_info[RTL8152_MAX_RX];
> + struct list_head rx_info;
> struct list_head rx_done, tx_free;
> struct sk_buff_head tx_queue, rx_queue;
> spinlock_t rx_lock, tx_lock;
> @@ -744,6 +745,8 @@ struct r8152 {
> void (*autosuspend_en)(struct r8152 *tp, bool enable);
> } rtl_ops;
>
> + atomic_t rx_count;
I wonder what the advantage of rx_count being atomic is, perhpas it
could be protected by the same lock as the list head?
> int intr_interval;
> u32 saved_wolopts;
> u32 msg_enable;
> @@ -1468,19 +1471,86 @@ static inline void *tx_agg_align(void *data)
> return (void *)ALIGN((uintptr_t)data, TX_ALIGN);
> }
>
> +static void free_rx_agg(struct r8152 *tp, struct rx_agg *agg)
> +{
> + list_del(&agg->info_list);
> +
> + usb_free_urb(agg->urb);
> + kfree(agg->buffer);
> + kfree(agg);
> +
> + atomic_dec(&tp->rx_count);
> +}
> +
> +static struct rx_agg *alloc_rx_agg(struct r8152 *tp, gfp_t mflags)
> +{
> + struct net_device *netdev = tp->netdev;
> + int node = netdev->dev.parent ? dev_to_node(netdev->dev.parent) : -1;
> + struct rx_agg *rx_agg;
> +
> + rx_agg = kmalloc_node(sizeof(*rx_agg), mflags, node);
> + if (rx_agg) {
nit: you could possibly save the indentation by returning early here
> + unsigned long flags;
> + u8 *buf;
> +
> + buf = kmalloc_node(tp->rx_buf_sz, mflags, node);
> + if (!buf)
> + goto free_rx;
> +
> + if (buf != rx_agg_align(buf)) {
> + kfree(buf);
> + buf = kmalloc_node(tp->rx_buf_sz + RX_ALIGN, mflags,
> + node);
> + if (!buf)
> + goto free_rx;
> + }
> +
> + rx_agg->buffer = buf;
> + rx_agg->head = rx_agg_align(buf);
> +
> + rx_agg->urb = usb_alloc_urb(0, mflags);
> + if (!rx_agg->urb)
> + goto free_buf;
> +
> + rx_agg->context = tp;
> +
> + INIT_LIST_HEAD(&rx_agg->list);
> + INIT_LIST_HEAD(&rx_agg->info_list);
> + spin_lock_irqsave(&tp->rx_lock, flags);
> + list_add_tail(&rx_agg->info_list, &tp->rx_info);
> + spin_unlock_irqrestore(&tp->rx_lock, flags);
> +
> + atomic_inc(&tp->rx_count);
> + }
> +
> + return rx_agg;
> +
> +free_buf:
> + kfree(rx_agg->buffer);
> +free_rx:
> + kfree(rx_agg);
> + return NULL;
> +}
> +
> static void free_all_mem(struct r8152 *tp)
> {
> + struct list_head *cursor, *next;
> + unsigned long flags;
> int i;
>
> - for (i = 0; i < RTL8152_MAX_RX; i++) {
> - usb_free_urb(tp->rx_info[i].urb);
> - tp->rx_info[i].urb = NULL;
> + spin_lock_irqsave(&tp->rx_lock, flags);
>
> - kfree(tp->rx_info[i].buffer);
> - tp->rx_info[i].buffer = NULL;
> - tp->rx_info[i].head = NULL;
> + list_for_each_safe(cursor, next, &tp->rx_info) {
nit: list_for_each_entry_safe, perhaps?
> + struct rx_agg *agg;
> +
> + agg = list_entry(cursor, struct rx_agg, info_list);
> + free_rx_agg(tp, agg);
> }
>
> + spin_unlock_irqrestore(&tp->rx_lock, flags);
> +
> + WARN_ON(unlikely(atomic_read(&tp->rx_count)));
nit: WARN_ON() has an unlikely() already built in
> for (i = 0; i < RTL8152_MAX_TX; i++) {
> usb_free_urb(tp->tx_info[i].urb);
> tp->tx_info[i].urb = NULL;
> @@ -1503,46 +1573,28 @@ static int alloc_all_mem(struct r8152 *tp)
> struct usb_interface *intf = tp->intf;
> struct usb_host_interface *alt = intf->cur_altsetting;
> struct usb_host_endpoint *ep_intr = alt->endpoint + 2;
> - struct urb *urb;
> int node, i;
> - u8 *buf;
>
> node = netdev->dev.parent ? dev_to_node(netdev->dev.parent) : -1;
>
> spin_lock_init(&tp->rx_lock);
> spin_lock_init(&tp->tx_lock);
> + INIT_LIST_HEAD(&tp->rx_info);
> INIT_LIST_HEAD(&tp->tx_free);
> INIT_LIST_HEAD(&tp->rx_done);
> skb_queue_head_init(&tp->tx_queue);
> skb_queue_head_init(&tp->rx_queue);
> + atomic_set(&tp->rx_count, 0);
>
> for (i = 0; i < RTL8152_MAX_RX; i++) {
> - buf = kmalloc_node(tp->rx_buf_sz, GFP_KERNEL, node);
> - if (!buf)
> - goto err1;
> -
> - if (buf != rx_agg_align(buf)) {
> - kfree(buf);
> - buf = kmalloc_node(tp->rx_buf_sz + RX_ALIGN, GFP_KERNEL,
> - node);
> - if (!buf)
> - goto err1;
> - }
> -
> - urb = usb_alloc_urb(0, GFP_KERNEL);
> - if (!urb) {
> - kfree(buf);
> + if (!alloc_rx_agg(tp, GFP_KERNEL))
> goto err1;
> - }
> -
> - INIT_LIST_HEAD(&tp->rx_info[i].list);
> - tp->rx_info[i].context = tp;
> - tp->rx_info[i].urb = urb;
> - tp->rx_info[i].buffer = buf;
> - tp->rx_info[i].head = rx_agg_align(buf);
> }
>
> for (i = 0; i < RTL8152_MAX_TX; i++) {
> + struct urb *urb;
> + u8 *buf;
> +
> buf = kmalloc_node(agg_buf_sz, GFP_KERNEL, node);
> if (!buf)
> goto err1;
> @@ -2331,44 +2383,69 @@ static void rxdy_gated_en(struct r8152 *tp, bool enable)
>
> static int rtl_start_rx(struct r8152 *tp)
> {
> - int i, ret = 0;
> + struct list_head *cursor, *next, tmp_list;
> + unsigned long flags;
> + int ret = 0;
> +
> + INIT_LIST_HEAD(&tmp_list);
> +
> + spin_lock_irqsave(&tp->rx_lock, flags);
>
> INIT_LIST_HEAD(&tp->rx_done);
> - for (i = 0; i < RTL8152_MAX_RX; i++) {
> - INIT_LIST_HEAD(&tp->rx_info[i].list);
> - ret = r8152_submit_rx(tp, &tp->rx_info[i], GFP_KERNEL);
> - if (ret)
> - break;
> - }
>
> - if (ret && ++i < RTL8152_MAX_RX) {
> - struct list_head rx_queue;
> - unsigned long flags;
> + list_splice_init(&tp->rx_info, &tmp_list);
>
> - INIT_LIST_HEAD(&rx_queue);
> + spin_unlock_irqrestore(&tp->rx_lock, flags);
>
> - do {
> - struct rx_agg *agg = &tp->rx_info[i++];
> - struct urb *urb = agg->urb;
> + list_for_each_safe(cursor, next, &tmp_list) {
> + struct rx_agg *agg;
>
> - urb->actual_length = 0;
> - list_add_tail(&agg->list, &rx_queue);
> - } while (i < RTL8152_MAX_RX);
> + agg = list_entry(cursor, struct rx_agg, info_list);
> + INIT_LIST_HEAD(&agg->list);
>
> - spin_lock_irqsave(&tp->rx_lock, flags);
> - list_splice_tail(&rx_queue, &tp->rx_done);
> - spin_unlock_irqrestore(&tp->rx_lock, flags);
> + if (ret < 0)
> + list_add_tail(&agg->list, &tp->rx_done);
> + else
> + ret = r8152_submit_rx(tp, agg, GFP_KERNEL);
> }
>
> + spin_lock_irqsave(&tp->rx_lock, flags);
> + WARN_ON(unlikely(!list_empty(&tp->rx_info)));
> + list_splice(&tmp_list, &tp->rx_info);
> + spin_unlock_irqrestore(&tp->rx_lock, flags);
> +
> return ret;
> }
>
> static int rtl_stop_rx(struct r8152 *tp)
> {
> - int i;
> + struct list_head *cursor, *next, tmp_list;
> + unsigned long flags;
> +
> + INIT_LIST_HEAD(&tmp_list);
>
> - for (i = 0; i < RTL8152_MAX_RX; i++)
> - usb_kill_urb(tp->rx_info[i].urb);
> + /* The usb_kill_urb() couldn't be used in atomic.
> + * Therefore, move the list of rx_info to a tmp one.
> + * Then, list_for_each_safe could be used without
> + * spin lock.
> + */
Would you mind explaining in a little more detail why taking the
entries from the list for a brief period of time is safe?
> + spin_lock_irqsave(&tp->rx_lock, flags);
> + list_splice_init(&tp->rx_info, &tmp_list);
> + spin_unlock_irqrestore(&tp->rx_lock, flags);
> +
> + list_for_each_safe(cursor, next, &tmp_list) {
> + struct rx_agg *agg;
> +
> + agg = list_entry(cursor, struct rx_agg, info_list);
> + usb_kill_urb(agg->urb);
> + }
> +
> + /* Move back the list of temp to the rx_info */
> + spin_lock_irqsave(&tp->rx_lock, flags);
> + WARN_ON(unlikely(!list_empty(&tp->rx_info)));
> + list_splice(&tmp_list, &tp->rx_info);
> + spin_unlock_irqrestore(&tp->rx_lock, flags);
>
> while (!skb_queue_empty(&tp->rx_queue))
> dev_kfree_skb(__skb_dequeue(&tp->rx_queue));
^ permalink raw reply
* Re: [PATCH 1/1] ixgbe: sync the first fragment unconditionally
From: Alexander Duyck @ 2019-08-06 20:00 UTC (permalink / raw)
To: Firo Yang
Cc: netdev@vger.kernel.org, jeffrey.t.kirsher@intel.com,
davem@davemloft.net, intel-wired-lan@lists.osuosl.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20190806092919.13211-1-firo.yang@suse.com>
On Tue, Aug 6, 2019 at 2:38 AM Firo Yang <firo.yang@suse.com> wrote:
>
> In Xen environment, if Xen-swiotlb is enabled, ixgbe driver
> could possibly allocate a page, DMA memory buffer, for the first
> fragment which is not suitable for Xen-swiotlb to do DMA operations.
> Xen-swiotlb will internally allocate another page for doing DMA
> operations. It requires syncing between those two pages. Otherwise,
> we may get an incomplete skb. To fix this problem, sync the first
> fragment no matter the first fargment is makred as "page_released"
> or not.
>
> Signed-off-by: Firo Yang <firo.yang@suse.com>
From what I can tell the code below is fine. However the patch
description isn't very clear about the issue.
Specifically since we are mapping the frame with
DMA_ATTR_SKIP_CPU_SYNC we have to unmap with that as well. As a result
a sync is not performed on an unmap and must be done manually as we
skipped it for the first frag. As such we need to always sync before
possibly performing a page unmap operation.
Also you can probably add the following to your patch description"
Fixes: f3213d932173 ("ixgbe: Update driver to make use of DMA
attributes in Rx path")
Reviewed-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> ---
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index cbaf712d6529..200de9838096 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -1825,13 +1825,7 @@ static void ixgbe_pull_tail(struct ixgbe_ring *rx_ring,
> static void ixgbe_dma_sync_frag(struct ixgbe_ring *rx_ring,
> struct sk_buff *skb)
> {
> - /* if the page was released unmap it, else just sync our portion */
> - if (unlikely(IXGBE_CB(skb)->page_released)) {
> - dma_unmap_page_attrs(rx_ring->dev, IXGBE_CB(skb)->dma,
> - ixgbe_rx_pg_size(rx_ring),
> - DMA_FROM_DEVICE,
> - IXGBE_RX_DMA_ATTR);
> - } else if (ring_uses_build_skb(rx_ring)) {
> + if (ring_uses_build_skb(rx_ring)) {
> unsigned long offset = (unsigned long)(skb->data) & ~PAGE_MASK;
>
> dma_sync_single_range_for_cpu(rx_ring->dev,
> @@ -1848,6 +1842,14 @@ static void ixgbe_dma_sync_frag(struct ixgbe_ring *rx_ring,
> skb_frag_size(frag),
> DMA_FROM_DEVICE);
> }
> +
> + /* If the page was released, just unmap it. */
> + if (unlikely(IXGBE_CB(skb)->page_released)) {
> + dma_unmap_page_attrs(rx_ring->dev, IXGBE_CB(skb)->dma,
> + ixgbe_rx_pg_size(rx_ring),
> + DMA_FROM_DEVICE,
> + IXGBE_RX_DMA_ATTR);
> + }
> }
>
> /**
> --
> 2.16.4
>
^ permalink raw reply
* Re: [PATCH 05/14] gpio: lpc32xx: allow building on non-lpc32xx targets
From: Sylvain Lemieux @ 2019-08-06 20:02 UTC (permalink / raw)
To: Arnd Bergmann
Cc: soc, moderated list:ARM PORT, Vladimir Zapolskiy, Russell King,
Gregory Clement, Linus Walleij, Bartosz Golaszewski, Jason Cooper,
Andrew Lunn, Sebastian Hesselbarth, David S. Miller,
Greg Kroah-Hartman, Alan Stern, Guenter Roeck,
open list:GPIO SUBSYSTEM, Networking, linux-serial, USB list,
LINUXWATCHDOG, Lee Jones, Linux Kernel Mailing List
In-Reply-To: <20190731195713.3150463-6-arnd@arndb.de>
Hi Arnd,
On Wed, Jul 31, 2019 at 4:00 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> The driver uses hardwire MMIO addresses instead of the data
> that is passed in device tree. Change it over to only
> hardcode the register offset values and allow compile-testing.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/gpio/Kconfig | 8 +++++
> drivers/gpio/Makefile | 2 +-
> drivers/gpio/gpio-lpc32xx.c | 63 ++++++++++++++++++++++++-------------
> 3 files changed, 50 insertions(+), 23 deletions(-)
>
[...]
> diff --git a/drivers/gpio/gpio-lpc32xx.c b/drivers/gpio/gpio-lpc32xx.c
> index 24885b3db3d5..548f7cb69386 100644
> --- a/drivers/gpio/gpio-lpc32xx.c
> +++ b/drivers/gpio/gpio-lpc32xx.c
[...]
> @@ -498,6 +509,10 @@ static int lpc32xx_gpio_probe(struct platform_device *pdev)
> {
> int i;
>
> + gpio_reg_base = devm_platform_ioremap_resource(pdev, 0);
> + if (gpio_reg_base)
> + return -ENXIO;
The probe function will always return an error.
Please replace the previous 2 lines with:
if (IS_ERR(gpio_reg_base))
return PTR_ERR(gpio_reg_base);
You can add my acked-by and tested-by in the v2 patch.
Acked-by: Sylvain Lemieux <slemieux.tyco@gmail.com>
Tested-by: Sylvain Lemieux <slemieux.tyco@gmail.com>
> +
> for (i = 0; i < ARRAY_SIZE(lpc32xx_gpiochip); i++) {
> if (pdev->dev.of_node) {
> lpc32xx_gpiochip[i].chip.of_xlate = lpc32xx_of_xlate;
> @@ -527,3 +542,7 @@ static struct platform_driver lpc32xx_gpio_driver = {
> };
>
> module_platform_driver(lpc32xx_gpio_driver);
> +
> +MODULE_AUTHOR("Kevin Wells <kevin.wells@nxp.com>");
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("GPIO driver for LPC32xx SoC");
> --
> 2.20.0
>
Sylvain
^ permalink raw reply
* Re: [PATCH 07/14] net: lpc-enet: move phy setup into platform code
From: Sylvain Lemieux @ 2019-08-06 20:11 UTC (permalink / raw)
To: Arnd Bergmann
Cc: soc, moderated list:ARM PORT, Vladimir Zapolskiy, Russell King,
Gregory Clement, Linus Walleij, Jason Cooper, Andrew Lunn,
Sebastian Hesselbarth, David S. Miller, Greg Kroah-Hartman,
Alan Stern, Guenter Roeck, open list:GPIO SUBSYSTEM, Networking,
linux-serial, USB list, LINUXWATCHDOG, Linux Kernel Mailing List
In-Reply-To: <20190731195713.3150463-8-arnd@arndb.de>
On Wed, Jul 31, 2019 at 4:01 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> Setting the phy mode requires touching a platform specific
> register, which prevents us from building the driver without
> its header files.
>
> Move it into a separate function in arch/arm/mach/lpc32xx
> to hide the core registers from the network driver.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> arch/arm/mach-lpc32xx/common.c | 12 ++++++++++++
> drivers/net/ethernet/nxp/lpc_eth.c | 12 +-----------
> include/linux/soc/nxp/lpc32xx-misc.h | 5 +++++
> 3 files changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arm/mach-lpc32xx/common.c b/arch/arm/mach-lpc32xx/common.c
> index f648324d5fb4..a475339333c1 100644
> --- a/arch/arm/mach-lpc32xx/common.c
> +++ b/arch/arm/mach-lpc32xx/common.c
> @@ -63,6 +63,18 @@ u32 lpc32xx_return_iram(void __iomem **mapbase, dma_addr_t *dmaaddr)
> }
> EXPORT_SYMBOL_GPL(lpc32xx_return_iram);
>
> +void lpc32xx_set_phy_interface_mode(phy_interface_t mode)
> +{
> + u32 tmp = __raw_readl(LPC32XX_CLKPWR_MACCLK_CTRL);
> + tmp &= ~LPC32XX_CLKPWR_MACCTRL_PINS_MSK;
> + if (mode == PHY_INTERFACE_MODE_MII)
> + tmp |= LPC32XX_CLKPWR_MACCTRL_USE_MII_PINS;
> + else
> + tmp |= LPC32XX_CLKPWR_MACCTRL_USE_RMII_PINS;
> + __raw_writel(tmp, LPC32XX_CLKPWR_MACCLK_CTRL);
> +}
> +EXPORT_SYMBOL_GPL(lpc32xx_set_phy_interface_mode);
> +
> static struct map_desc lpc32xx_io_desc[] __initdata = {
> {
> .virtual = (unsigned long)IO_ADDRESS(LPC32XX_AHB0_START),
> diff --git a/drivers/net/ethernet/nxp/lpc_eth.c b/drivers/net/ethernet/nxp/lpc_eth.c
> index bcdd0adcfb0c..0893b77c385d 100644
> --- a/drivers/net/ethernet/nxp/lpc_eth.c
> +++ b/drivers/net/ethernet/nxp/lpc_eth.c
> @@ -20,9 +20,6 @@
> #include <linux/spinlock.h>
> #include <linux/soc/nxp/lpc32xx-misc.h>
>
> -#include <mach/hardware.h>
> -#include <mach/platform.h>
> -
> #define MODNAME "lpc-eth"
> #define DRV_VERSION "1.00"
>
> @@ -1237,16 +1234,9 @@ static int lpc_eth_drv_probe(struct platform_device *pdev)
> dma_addr_t dma_handle;
> struct resource *res;
> int irq, ret;
> - u32 tmp;
>
> /* Setup network interface for RMII or MII mode */
> - tmp = __raw_readl(LPC32XX_CLKPWR_MACCLK_CTRL);
> - tmp &= ~LPC32XX_CLKPWR_MACCTRL_PINS_MSK;
> - if (lpc_phy_interface_mode(dev) == PHY_INTERFACE_MODE_MII)
> - tmp |= LPC32XX_CLKPWR_MACCTRL_USE_MII_PINS;
> - else
> - tmp |= LPC32XX_CLKPWR_MACCTRL_USE_RMII_PINS;
> - __raw_writel(tmp, LPC32XX_CLKPWR_MACCLK_CTRL);
> + lpc32xx_set_phy_interface_mode(lpc_phy_interface_mode(dev));
>
> /* Get platform resources */
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> diff --git a/include/linux/soc/nxp/lpc32xx-misc.h b/include/linux/soc/nxp/lpc32xx-misc.h
> index f232e1a1bcdc..af4f82f6cf3b 100644
> --- a/include/linux/soc/nxp/lpc32xx-misc.h
> +++ b/include/linux/soc/nxp/lpc32xx-misc.h
> @@ -9,9 +9,11 @@
> #define __SOC_LPC32XX_MISC_H
>
> #include <linux/types.h>
> +#include <linux/phy.h>
>
> #ifdef CONFIG_ARCH_LPC32XX
> extern u32 lpc32xx_return_iram(void __iomem **mapbase, dma_addr_t *dmaaddr);
> +extern void lpc32xx_set_phy_interface_mode(phy_interface_t mode);
> #else
> static inline u32 lpc32xx_return_iram(void __iomem **mapbase, dma_addr_t *dmaaddr)
> {
> @@ -19,6 +21,9 @@ static inline u32 lpc32xx_return_iram(void __iomem **mapbase, dma_addr_t *dmaadd
> *dmaaddr = 0;
> return 0;
> }
> +static inline void lpc32xx_set_phy_interface_mode(phy_interface_t mode)
> +{
> +}
> #endif
>
> #endif /* __SOC_LPC32XX_MISC_H */
> --
> 2.20.0
>
^ permalink raw reply
* Re: [PATCH 07/14] net: lpc-enet: move phy setup into platform code
From: Sylvain Lemieux @ 2019-08-06 20:12 UTC (permalink / raw)
To: Arnd Bergmann
Cc: soc, moderated list:ARM PORT, Vladimir Zapolskiy, Russell King,
Gregory Clement, Linus Walleij, Jason Cooper, Andrew Lunn,
Sebastian Hesselbarth, David S. Miller, Greg Kroah-Hartman,
Alan Stern, Guenter Roeck, open list:GPIO SUBSYSTEM, Networking,
linux-serial, USB list, LINUXWATCHDOG, Linux Kernel Mailing List
In-Reply-To: <20190731195713.3150463-8-arnd@arndb.de>
Acked-by: Sylvain Lemieux <slemieux.tyco@gmail.com>
On Wed, Jul 31, 2019 at 4:01 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> Setting the phy mode requires touching a platform specific
> register, which prevents us from building the driver without
> its header files.
>
> Move it into a separate function in arch/arm/mach/lpc32xx
> to hide the core registers from the network driver.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> arch/arm/mach-lpc32xx/common.c | 12 ++++++++++++
> drivers/net/ethernet/nxp/lpc_eth.c | 12 +-----------
> include/linux/soc/nxp/lpc32xx-misc.h | 5 +++++
> 3 files changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arm/mach-lpc32xx/common.c b/arch/arm/mach-lpc32xx/common.c
> index f648324d5fb4..a475339333c1 100644
> --- a/arch/arm/mach-lpc32xx/common.c
> +++ b/arch/arm/mach-lpc32xx/common.c
> @@ -63,6 +63,18 @@ u32 lpc32xx_return_iram(void __iomem **mapbase, dma_addr_t *dmaaddr)
> }
> EXPORT_SYMBOL_GPL(lpc32xx_return_iram);
>
> +void lpc32xx_set_phy_interface_mode(phy_interface_t mode)
> +{
> + u32 tmp = __raw_readl(LPC32XX_CLKPWR_MACCLK_CTRL);
> + tmp &= ~LPC32XX_CLKPWR_MACCTRL_PINS_MSK;
> + if (mode == PHY_INTERFACE_MODE_MII)
> + tmp |= LPC32XX_CLKPWR_MACCTRL_USE_MII_PINS;
> + else
> + tmp |= LPC32XX_CLKPWR_MACCTRL_USE_RMII_PINS;
> + __raw_writel(tmp, LPC32XX_CLKPWR_MACCLK_CTRL);
> +}
> +EXPORT_SYMBOL_GPL(lpc32xx_set_phy_interface_mode);
> +
> static struct map_desc lpc32xx_io_desc[] __initdata = {
> {
> .virtual = (unsigned long)IO_ADDRESS(LPC32XX_AHB0_START),
> diff --git a/drivers/net/ethernet/nxp/lpc_eth.c b/drivers/net/ethernet/nxp/lpc_eth.c
> index bcdd0adcfb0c..0893b77c385d 100644
> --- a/drivers/net/ethernet/nxp/lpc_eth.c
> +++ b/drivers/net/ethernet/nxp/lpc_eth.c
> @@ -20,9 +20,6 @@
> #include <linux/spinlock.h>
> #include <linux/soc/nxp/lpc32xx-misc.h>
>
> -#include <mach/hardware.h>
> -#include <mach/platform.h>
> -
> #define MODNAME "lpc-eth"
> #define DRV_VERSION "1.00"
>
> @@ -1237,16 +1234,9 @@ static int lpc_eth_drv_probe(struct platform_device *pdev)
> dma_addr_t dma_handle;
> struct resource *res;
> int irq, ret;
> - u32 tmp;
>
> /* Setup network interface for RMII or MII mode */
> - tmp = __raw_readl(LPC32XX_CLKPWR_MACCLK_CTRL);
> - tmp &= ~LPC32XX_CLKPWR_MACCTRL_PINS_MSK;
> - if (lpc_phy_interface_mode(dev) == PHY_INTERFACE_MODE_MII)
> - tmp |= LPC32XX_CLKPWR_MACCTRL_USE_MII_PINS;
> - else
> - tmp |= LPC32XX_CLKPWR_MACCTRL_USE_RMII_PINS;
> - __raw_writel(tmp, LPC32XX_CLKPWR_MACCLK_CTRL);
> + lpc32xx_set_phy_interface_mode(lpc_phy_interface_mode(dev));
>
> /* Get platform resources */
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> diff --git a/include/linux/soc/nxp/lpc32xx-misc.h b/include/linux/soc/nxp/lpc32xx-misc.h
> index f232e1a1bcdc..af4f82f6cf3b 100644
> --- a/include/linux/soc/nxp/lpc32xx-misc.h
> +++ b/include/linux/soc/nxp/lpc32xx-misc.h
> @@ -9,9 +9,11 @@
> #define __SOC_LPC32XX_MISC_H
>
> #include <linux/types.h>
> +#include <linux/phy.h>
>
> #ifdef CONFIG_ARCH_LPC32XX
> extern u32 lpc32xx_return_iram(void __iomem **mapbase, dma_addr_t *dmaaddr);
> +extern void lpc32xx_set_phy_interface_mode(phy_interface_t mode);
> #else
> static inline u32 lpc32xx_return_iram(void __iomem **mapbase, dma_addr_t *dmaaddr)
> {
> @@ -19,6 +21,9 @@ static inline u32 lpc32xx_return_iram(void __iomem **mapbase, dma_addr_t *dmaadd
> *dmaaddr = 0;
> return 0;
> }
> +static inline void lpc32xx_set_phy_interface_mode(phy_interface_t mode)
> +{
> +}
> #endif
>
> #endif /* __SOC_LPC32XX_MISC_H */
> --
> 2.20.0
>
^ permalink raw reply
* Re: [PATCH 08/14] net: lpc-enet: allow compile testing
From: Sylvain Lemieux @ 2019-08-06 20:13 UTC (permalink / raw)
To: Arnd Bergmann
Cc: soc, moderated list:ARM PORT, Vladimir Zapolskiy, Russell King,
Gregory Clement, Linus Walleij, Jason Cooper, Andrew Lunn,
Sebastian Hesselbarth, David S. Miller, Greg Kroah-Hartman,
Alan Stern, Guenter Roeck, open list:GPIO SUBSYSTEM, Networking,
linux-serial, USB list, LINUXWATCHDOG, Linux Kernel Mailing List
In-Reply-To: <20190731195713.3150463-9-arnd@arndb.de>
Acked-by: Sylvain Lemieux <slemieux.tyco@gmail.com>
On Wed, Jul 31, 2019 at 4:01 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> The lpc-enet driver can now be built on all platforms, so
> allow compile testing as well.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/net/ethernet/nxp/Kconfig | 2 +-
> drivers/net/ethernet/nxp/lpc_eth.c | 1 +
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/nxp/Kconfig b/drivers/net/ethernet/nxp/Kconfig
> index 261f107e2be0..418afb84c84b 100644
> --- a/drivers/net/ethernet/nxp/Kconfig
> +++ b/drivers/net/ethernet/nxp/Kconfig
> @@ -1,7 +1,7 @@
> # SPDX-License-Identifier: GPL-2.0-only
> config LPC_ENET
> tristate "NXP ethernet MAC on LPC devices"
> - depends on ARCH_LPC32XX
> + depends on ARCH_LPC32XX || COMPILE_TEST
> select PHYLIB
> help
> Say Y or M here if you want to use the NXP ethernet MAC included on
> diff --git a/drivers/net/ethernet/nxp/lpc_eth.c b/drivers/net/ethernet/nxp/lpc_eth.c
> index 0893b77c385d..34fdf2100772 100644
> --- a/drivers/net/ethernet/nxp/lpc_eth.c
> +++ b/drivers/net/ethernet/nxp/lpc_eth.c
> @@ -14,6 +14,7 @@
> #include <linux/crc32.h>
> #include <linux/etherdevice.h>
> #include <linux/module.h>
> +#include <linux/of.h>
> #include <linux/of_net.h>
> #include <linux/phy.h>
> #include <linux/platform_device.h>
> --
> 2.20.0
>
^ permalink raw reply
* Re: [PATCH 10/14] ARM: lpc32xx: clean up header files
From: Sylvain Lemieux @ 2019-08-06 20:16 UTC (permalink / raw)
To: Arnd Bergmann
Cc: soc, moderated list:ARM PORT, Vladimir Zapolskiy, Russell King,
Gregory Clement, Linus Walleij, Jason Cooper, Andrew Lunn,
Sebastian Hesselbarth, David S. Miller, Greg Kroah-Hartman,
Alan Stern, Guenter Roeck, open list:GPIO SUBSYSTEM, Networking,
linux-serial, USB list, LINUXWATCHDOG, Linux Kernel Mailing List
In-Reply-To: <20190731195713.3150463-11-arnd@arndb.de>
Acked-by: Sylvain Lemieux <slemieux.tyco@gmail.com>
On Wed, Jul 31, 2019 at 4:03 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> All device drivers have stopped relying on mach/*.h headers,
> so move the remaining headers into arch/arm/mach-lpc32xx/lpc32xx.h
> to prepare for multiplatform builds.
>
> The mach/entry-macro.S file has been unused for a long time now
> and can simply get removed.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> arch/arm/mach-lpc32xx/common.c | 3 +-
> .../mach-lpc32xx/include/mach/entry-macro.S | 28 -------------------
> arch/arm/mach-lpc32xx/include/mach/hardware.h | 25 -----------------
> .../mach-lpc32xx/include/mach/uncompress.h | 4 +--
> .../{include/mach/platform.h => lpc32xx.h} | 18 ++++++++++--
> arch/arm/mach-lpc32xx/pm.c | 3 +-
> arch/arm/mach-lpc32xx/serial.c | 3 +-
> arch/arm/mach-lpc32xx/suspend.S | 3 +-
> 8 files changed, 21 insertions(+), 66 deletions(-)
> delete mode 100644 arch/arm/mach-lpc32xx/include/mach/entry-macro.S
> delete mode 100644 arch/arm/mach-lpc32xx/include/mach/hardware.h
> rename arch/arm/mach-lpc32xx/{include/mach/platform.h => lpc32xx.h} (98%)
>
> diff --git a/arch/arm/mach-lpc32xx/common.c b/arch/arm/mach-lpc32xx/common.c
> index a475339333c1..304ea61a0716 100644
> --- a/arch/arm/mach-lpc32xx/common.c
> +++ b/arch/arm/mach-lpc32xx/common.c
> @@ -13,8 +13,7 @@
> #include <asm/mach/map.h>
> #include <asm/system_info.h>
>
> -#include <mach/hardware.h>
> -#include <mach/platform.h>
> +#include "lpc32xx.h"
> #include "common.h"
>
> /*
> diff --git a/arch/arm/mach-lpc32xx/include/mach/entry-macro.S b/arch/arm/mach-lpc32xx/include/mach/entry-macro.S
> deleted file mode 100644
> index eec0f5f7e722..000000000000
> --- a/arch/arm/mach-lpc32xx/include/mach/entry-macro.S
> +++ /dev/null
> @@ -1,28 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-or-later */
> -/*
> - * arch/arm/mach-lpc32xx/include/mach/entry-macro.S
> - *
> - * Author: Kevin Wells <kevin.wells@nxp.com>
> - *
> - * Copyright (C) 2010 NXP Semiconductors
> - */
> -
> -#include <mach/hardware.h>
> -#include <mach/platform.h>
> -
> -#define LPC32XX_INTC_MASKED_STATUS_OFS 0x8
> -
> - .macro get_irqnr_preamble, base, tmp
> - ldr \base, =IO_ADDRESS(LPC32XX_MIC_BASE)
> - .endm
> -
> -/*
> - * Return IRQ number in irqnr. Also return processor Z flag status in CPSR
> - * as set if an interrupt is pending.
> - */
> - .macro get_irqnr_and_base, irqnr, irqstat, base, tmp
> - ldr \irqstat, [\base, #LPC32XX_INTC_MASKED_STATUS_OFS]
> - clz \irqnr, \irqstat
> - rsb \irqnr, \irqnr, #31
> - teq \irqstat, #0
> - .endm
> diff --git a/arch/arm/mach-lpc32xx/include/mach/hardware.h b/arch/arm/mach-lpc32xx/include/mach/hardware.h
> deleted file mode 100644
> index 4866f096ffce..000000000000
> --- a/arch/arm/mach-lpc32xx/include/mach/hardware.h
> +++ /dev/null
> @@ -1,25 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-or-later */
> -/*
> - * arch/arm/mach-lpc32xx/include/mach/hardware.h
> - *
> - * Copyright (c) 2005 MontaVista Software, Inc. <source@mvista.com>
> - */
> -
> -#ifndef __ASM_ARCH_HARDWARE_H
> -#define __ASM_ARCH_HARDWARE_H
> -
> -/*
> - * Start of virtual addresses for IO devices
> - */
> -#define IO_BASE 0xF0000000
> -
> -/*
> - * This macro relies on fact that for all HW i/o addresses bits 20-23 are 0
> - */
> -#define IO_ADDRESS(x) IOMEM(((((x) & 0xff000000) >> 4) | ((x) & 0xfffff)) |\
> - IO_BASE)
> -
> -#define io_p2v(x) ((void __iomem *) (unsigned long) IO_ADDRESS(x))
> -#define io_v2p(x) ((((x) & 0x0ff00000) << 4) | ((x) & 0x000fffff))
> -
> -#endif
> diff --git a/arch/arm/mach-lpc32xx/include/mach/uncompress.h b/arch/arm/mach-lpc32xx/include/mach/uncompress.h
> index a568812a0b91..74b7aa0da0e4 100644
> --- a/arch/arm/mach-lpc32xx/include/mach/uncompress.h
> +++ b/arch/arm/mach-lpc32xx/include/mach/uncompress.h
> @@ -12,15 +12,13 @@
>
> #include <linux/io.h>
>
> -#include <mach/hardware.h>
> -#include <mach/platform.h>
> -
> /*
> * Uncompress output is hardcoded to standard UART 5
> */
>
> #define UART_FIFO_CTL_TX_RESET (1 << 2)
> #define UART_STATUS_TX_MT (1 << 6)
> +#define LPC32XX_UART5_BASE 0x40090000
>
> #define _UARTREG(x) (void __iomem *)(LPC32XX_UART5_BASE + (x))
>
> diff --git a/arch/arm/mach-lpc32xx/include/mach/platform.h b/arch/arm/mach-lpc32xx/lpc32xx.h
> similarity index 98%
> rename from arch/arm/mach-lpc32xx/include/mach/platform.h
> rename to arch/arm/mach-lpc32xx/lpc32xx.h
> index 1c53790444fc..5eeb884a1993 100644
> --- a/arch/arm/mach-lpc32xx/include/mach/platform.h
> +++ b/arch/arm/mach-lpc32xx/lpc32xx.h
> @@ -7,8 +7,8 @@
> * Copyright (C) 2010 NXP Semiconductors
> */
>
> -#ifndef __ASM_ARCH_PLATFORM_H
> -#define __ASM_ARCH_PLATFORM_H
> +#ifndef __ARM_LPC32XX_H
> +#define __ARM_LPC32XX_H
>
> #define _SBF(f, v) ((v) << (f))
> #define _BIT(n) _SBF(n, 1)
> @@ -700,4 +700,18 @@
> #define LPC32XX_USB_OTG_DEV_CLOCK_ON _BIT(1)
> #define LPC32XX_USB_OTG_HOST_CLOCK_ON _BIT(0)
>
> +/*
> + * Start of virtual addresses for IO devices
> + */
> +#define IO_BASE 0xF0000000
> +
> +/*
> + * This macro relies on fact that for all HW i/o addresses bits 20-23 are 0
> + */
> +#define IO_ADDRESS(x) IOMEM(((((x) & 0xff000000) >> 4) | ((x) & 0xfffff)) |\
> + IO_BASE)
> +
> +#define io_p2v(x) ((void __iomem *) (unsigned long) IO_ADDRESS(x))
> +#define io_v2p(x) ((((x) & 0x0ff00000) << 4) | ((x) & 0x000fffff))
> +
> #endif
> diff --git a/arch/arm/mach-lpc32xx/pm.c b/arch/arm/mach-lpc32xx/pm.c
> index 32bca351a73b..b27fa1b9f56c 100644
> --- a/arch/arm/mach-lpc32xx/pm.c
> +++ b/arch/arm/mach-lpc32xx/pm.c
> @@ -70,8 +70,7 @@
>
> #include <asm/cacheflush.h>
>
> -#include <mach/hardware.h>
> -#include <mach/platform.h>
> +#include "lpc32xx.h"
> #include "common.h"
>
> #define TEMP_IRAM_AREA IO_ADDRESS(LPC32XX_IRAM_BASE)
> diff --git a/arch/arm/mach-lpc32xx/serial.c b/arch/arm/mach-lpc32xx/serial.c
> index cfb35e5691cd..3e765c4bf986 100644
> --- a/arch/arm/mach-lpc32xx/serial.c
> +++ b/arch/arm/mach-lpc32xx/serial.c
> @@ -16,8 +16,7 @@
> #include <linux/clk.h>
> #include <linux/io.h>
>
> -#include <mach/hardware.h>
> -#include <mach/platform.h>
> +#include "lpc32xx.h"
> #include "common.h"
>
> #define LPC32XX_SUART_FIFO_SIZE 64
> diff --git a/arch/arm/mach-lpc32xx/suspend.S b/arch/arm/mach-lpc32xx/suspend.S
> index 374f9f07fe48..3f0a8282ef6f 100644
> --- a/arch/arm/mach-lpc32xx/suspend.S
> +++ b/arch/arm/mach-lpc32xx/suspend.S
> @@ -11,8 +11,7 @@
> */
> #include <linux/linkage.h>
> #include <asm/assembler.h>
> -#include <mach/platform.h>
> -#include <mach/hardware.h>
> +#include "lpc32xx.h"
>
> /* Using named register defines makes the code easier to follow */
> #define WORK1_REG r0
> --
> 2.20.0
>
^ permalink raw reply
* Re: [PATCH net-next] mlx5: use correct counter
From: Jonathan Lemon @ 2019-08-06 20:16 UTC (permalink / raw)
To: Saeed Mahameed; +Cc: kernel-team, netdev
In-Reply-To: <128f1f62141faee7620fbe56202f35ec8f6b42b6.camel@mellanox.com>
On 6 Aug 2019, at 12:03, Saeed Mahameed wrote:
> On Tue, 2019-08-06 at 11:28 -0700, Jonathan Lemon wrote:
>> mlx5e_grp_q_update_stats seems to be using the wrong counter
>> for if_down_packets.
>>
>> Signed-off-by: Jonathan Lemon <jonathan.lemon@gmail.com>
>> ---
>> drivers/net/ethernet/mellanox/mlx5/core/en_stats.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
>> b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
>> index 6eee3c7d4b06..1d16e03a987d 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
>> @@ -363,7 +363,7 @@ static void mlx5e_grp_q_update_stats(struct
>> mlx5e_priv *priv)
>> !mlx5_core_query_q_counter(priv->mdev, priv-
>>> drop_rq_q_counter, 0,
>> out, sizeof(out)))
>> qcnt->rx_if_down_packets =
>> MLX5_GET(query_q_counter_out, out,
>> - out_of_buffer);
>> + rx_if_down_packets)
>> ;
>
> Hi Jonathan,
>
> This patch in not applicable (won't compile and there is no issue with
> current code).
>
> Although it is confusing but the code is correct as is.
>
> 1) your patch won't compile since there is no rx_if_down_packets field
> in query_q_counter_out hw definition struct: please check
> include/linux/mlx5/mlx5_ifc.h
> mlx5_ifc_query_q_counter_out_bits
> 2) the code works as is since when interface is down and port is up,
> technically from hw perspective there is "no buffer available" so the
> out_of_buffer counter of the drop_rq_q_counter will count packets
> dropped due to interface down..
Hmm, confusing, but okay. I'll send mail on an issue we're having
separately.
--
Jonathan
^ permalink raw reply
* [PATCH] staging: isdn: hysdn_procconf_init() remove parantheses from return value
From: Giridhar Prasath R @ 2019-08-07 2:03 UTC (permalink / raw)
To: isdn; +Cc: gregkh, arnd, cristianoprasath, netdev, devel, linux-kernel
ERROR: return is not a function, parentheses are not required
FILE: git/kernels/staging/drivers/staging/isdn/hysdn/hysdn_procconf.c:385
+ return (0);
Signed-off-by: Giridhar Prasath R <cristianoprasath@gmail.com>
---
drivers/staging/isdn/hysdn/hysdn_procconf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/isdn/hysdn/hysdn_procconf.c b/drivers/staging/isdn/hysdn/hysdn_procconf.c
index 73079213ec94..48afd9f5316e 100644
--- a/drivers/staging/isdn/hysdn/hysdn_procconf.c
+++ b/drivers/staging/isdn/hysdn/hysdn_procconf.c
@@ -382,7 +382,7 @@ hysdn_procconf_init(void)
}
printk(KERN_NOTICE "HYSDN: procfs initialised\n");
- return (0);
+ return 0;
} /* hysdn_procconf_init */
/*************************************************************************************/
--
2.22.0
^ permalink raw reply related
* Re: [PATCH net v2] net: dsa: Check existence of .port_mdb_add callback before calling it
From: Vivien Didelot @ 2019-08-06 20:34 UTC (permalink / raw)
To: Chen-Yu Tsai
Cc: Chen-Yu Tsai, Andrew Lunn, Florian Fainelli, David S. Miller,
netdev, linux-kernel
In-Reply-To: <CAGb2v67cZb_JKDHSb-9Tm1KnTxw5FOG3faZoQSGef_FzkdSszA@mail.gmail.com>
Hi Chen-Yu,
On Wed, 7 Aug 2019 01:49:37 +0800, Chen-Yu Tsai <wens@kernel.org> wrote:
> On Wed, Aug 7, 2019 at 1:15 AM Vivien Didelot <vivien.didelot@gmail.com> wrote:
> >
> > Hi Chen-Yu,
> >
> > On Tue, 6 Aug 2019 15:53:25 +0800, Chen-Yu Tsai <wens@kernel.org> wrote:
> > > From: Chen-Yu Tsai <wens@csie.org>
> > >
> > > With the recent addition of commit 75dad2520fc3 ("net: dsa: b53: Disable
> > > all ports on setup"), users of b53 (BCM53125 on Lamobo R1 in my case)
> > > are forced to use the dsa subsystem to enable the switch, instead of
> > > having it in the default transparent "forward-to-all" mode.
> > >
> > > The b53 driver does not support mdb bitmap functions. However the dsa
> > > layer does not check for the existence of the .port_mdb_add callback
> > > before actually using it. This results in a NULL pointer dereference,
> > > as shown in the kernel oops below.
> > >
> > > The other functions seem to be properly guarded. Do the same for
> > > .port_mdb_add in dsa_switch_mdb_add_bitmap() as well.
> > >
> > > b53 is not the only driver that doesn't support mdb bitmap functions.
> > > Others include bcm_sf2, dsa_loop, lantiq_gswip, mt7530, mv88e6060,
> > > qca8k, realtek-smi, and vitesse-vsc73xx.
> >
> > I don't know what you mean by that, there's no "mdb bitmap function"
> > support for drivers, only the port_mdb_{prepare,add,del} callbacks...
>
> The term was coined from commit e6db98db8a95 ("net: dsa: add switch mdb
> bitmap functions"). But yeah, .port_mdb_* ops/callbacks would be more
> appropriate.
>
> > > 8<--- cut here ---
> > > Unable to handle kernel NULL pointer dereference at virtual address 00000000
> > > pgd = (ptrval)
> > > [00000000] *pgd=00000000
> > > Internal error: Oops: 80000005 [#1] SMP ARM
> > > Modules linked in: rtl8xxxu rtl8192cu rtl_usb rtl8192c_common rtlwifi mac80211 cfg80211
> > > CPU: 1 PID: 134 Comm: kworker/1:2 Not tainted 5.3.0-rc1-00247-gd3519030752a #1
> > > Hardware name: Allwinner sun7i (A20) Family
> > > Workqueue: events switchdev_deferred_process_work
> > > PC is at 0x0
> > > LR is at dsa_switch_event+0x570/0x620
> > > pc : [<00000000>] lr : [<c08533ec>] psr: 80070013
> > > sp : ee871db8 ip : 00000000 fp : ee98d0a4
> > > r10: 0000000c r9 : 00000008 r8 : ee89f710
> > > r7 : ee98d040 r6 : ee98d088 r5 : c0f04c48 r4 : ee98d04c
> > > r3 : 00000000 r2 : ee89f710 r1 : 00000008 r0 : ee98d040
> > > Flags: Nzcv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
> > > Control: 10c5387d Table: 6deb406a DAC: 00000051
> > > Process kworker/1:2 (pid: 134, stack limit = 0x(ptrval))
> > > Stack: (0xee871db8 to 0xee872000)
> > > 1da0: ee871e14 103ace2d
> > > 1dc0: 00000000 ffffffff 00000000 ee871e14 00000005 00000000 c08524a0 00000000
> > > 1de0: ffffe000 c014bdfc c0f04c48 ee871e98 c0f04c48 ee9e5000 c0851120 c014bef0
> > > 1e00: 00000000 b643aea2 ee9b4068 c08509a8 ee2bf940 ee89f710 ee871ecb 00000000
> > > 1e20: 00000008 103ace2d 00000000 c087e248 ee29c868 103ace2d 00000001 ffffffff
> > > 1e40: 00000000 ee871e98 00000006 00000000 c0fb2a50 c087e2d0 ffffffff c08523c4
> > > 1e60: ffffffff c014bdfc 00000006 c0fad2d0 ee871e98 ee89f710 00000000 c014c500
> > > 1e80: 00000000 ee89f3c0 c0f04c48 00000000 ee9e5000 c087dfb4 ee9e5000 00000000
> > > 1ea0: ee89f710 ee871ecb 00000001 103ace2d 00000000 c0f04c48 00000000 c087e0a8
> > > 1ec0: 00000000 efd9a3e0 0089f3c0 103ace2d ee89f700 ee89f710 ee9e5000 00000122
> > > 1ee0: 00000100 c087e130 ee89f700 c0fad2c8 c1003ef0 c087de4c 2e928000 c0fad2ec
> > > 1f00: c0fad2ec ee839580 ef7a62c0 ef7a9400 00000000 c087def8 c0fad2ec c01447dc
> > > 1f20: ef315640 ef7a62c0 00000008 ee839580 ee839594 ef7a62c0 00000008 c0f03d00
> > > 1f40: ef7a62d8 ef7a62c0 ffffe000 c0145b84 ffffe000 c0fb2420 c0bfaa8c 00000000
> > > 1f60: ffffe000 ee84b600 ee84b5c0 00000000 ee870000 ee839580 c0145b40 ef0e5ea4
> > > 1f80: ee84b61c c014a6f8 00000001 ee84b5c0 c014a5b0 00000000 00000000 00000000
> > > 1fa0: 00000000 00000000 00000000 c01010e8 00000000 00000000 00000000 00000000
> > > 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> > > 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 00000000
> > > [<c08533ec>] (dsa_switch_event) from [<c014bdfc>] (notifier_call_chain+0x48/0x84)
> > > [<c014bdfc>] (notifier_call_chain) from [<c014bef0>] (raw_notifier_call_chain+0x18/0x20)
> > > [<c014bef0>] (raw_notifier_call_chain) from [<c08509a8>] (dsa_port_mdb_add+0x48/0x74)
> > > [<c08509a8>] (dsa_port_mdb_add) from [<c087e248>] (__switchdev_handle_port_obj_add+0x54/0xd4)
> > > [<c087e248>] (__switchdev_handle_port_obj_add) from [<c087e2d0>] (switchdev_handle_port_obj_add+0x8/0x14)
> > > [<c087e2d0>] (switchdev_handle_port_obj_add) from [<c08523c4>] (dsa_slave_switchdev_blocking_event+0x94/0xa4)
> > > [<c08523c4>] (dsa_slave_switchdev_blocking_event) from [<c014bdfc>] (notifier_call_chain+0x48/0x84)
> > > [<c014bdfc>] (notifier_call_chain) from [<c014c500>] (blocking_notifier_call_chain+0x50/0x68)
> > > [<c014c500>] (blocking_notifier_call_chain) from [<c087dfb4>] (switchdev_port_obj_notify+0x44/0xa8)
> > > [<c087dfb4>] (switchdev_port_obj_notify) from [<c087e0a8>] (switchdev_port_obj_add_now+0x90/0x104)
> > > [<c087e0a8>] (switchdev_port_obj_add_now) from [<c087e130>] (switchdev_port_obj_add_deferred+0x14/0x5c)
> > > [<c087e130>] (switchdev_port_obj_add_deferred) from [<c087de4c>] (switchdev_deferred_process+0x64/0x104)
> > > [<c087de4c>] (switchdev_deferred_process) from [<c087def8>] (switchdev_deferred_process_work+0xc/0x14)
> > > [<c087def8>] (switchdev_deferred_process_work) from [<c01447dc>] (process_one_work+0x218/0x50c)
> > > [<c01447dc>] (process_one_work) from [<c0145b84>] (worker_thread+0x44/0x5bc)
> > > [<c0145b84>] (worker_thread) from [<c014a6f8>] (kthread+0x148/0x150)
> > > [<c014a6f8>] (kthread) from [<c01010e8>] (ret_from_fork+0x14/0x2c)
> > > Exception stack(0xee871fb0 to 0xee871ff8)
> > > 1fa0: 00000000 00000000 00000000 00000000
> > > 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> > > 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000
> > > Code: bad PC value
> > > ---[ end trace 1292c61abd17b130 ]---
> > >
> > > [<c08533ec>] (dsa_switch_event) from [<c014bdfc>] (notifier_call_chain+0x48/0x84)
> > > corresponds to
> > >
> > > $ arm-linux-gnueabihf-addr2line -C -i -e vmlinux c08533ec
> > >
> > > linux/net/dsa/switch.c:156
> > > linux/net/dsa/switch.c:178
> > > linux/net/dsa/switch.c:328
> > >
> > > Fixes: e6db98db8a95 ("net: dsa: add switch mdb bitmap functions")
> > > Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> > > ---
> > > Changes since v1:
> > >
> > > - Moved the check to the beginning of dsa_switch_mdb_add()
> > >
> > > Looks like we could also move the ops check out of
> > > dsa_switch_mdb_prepare_bitmap(), though I suppose keeping the code the
> > > way it is now is clearer.
> > >
> > > ---
> > > net/dsa/switch.c | 3 +++
> > > 1 file changed, 3 insertions(+)
> > >
> > > diff --git a/net/dsa/switch.c b/net/dsa/switch.c
> > > index 4ec5b7f85d51..231af5268656 100644
> > > --- a/net/dsa/switch.c
> > > +++ b/net/dsa/switch.c
> > > @@ -164,6 +164,9 @@ static int dsa_switch_mdb_add(struct dsa_switch *ds,
> > > struct switchdev_trans *trans = info->trans;
> > > int port;
> > >
> > > + if (!ds->ops->port_mdb_add)
> > > + return -EOPNOTSUPP;
> > > +
> > > /* Build a mask of Multicast group members */
> > > bitmap_zero(ds->bitmap, ds->num_ports);
> > > if (ds->index == info->sw_index)
> > > --
> > > 2.20.1
> > >
> >
> > I don't understand the crash here, nor the fix. dsa_switch_mdb_add()
> > is supposed to be called through switchdev with a prepare phase,
> > which checks for ds->ops->port_mdb_add. Do you mean that a switchdev
> > MDB object is added somewhere without a prepare phase? If that's
> > the case, this is what the commit message must say. Then the
>
> I had pretty much zero understanding of how switchdev and dsa work.
> The symptom is a NULL pointer reference, resulting from an unsupported
> callback that was not checked before being called, as described above.
> And that is what I mean. A NULL pointer reference happened when it
> should not have.
>
> Based on what you just mentioned, yes it does look like an object was
> added without a prepare phase. Randomly looking through the net/dsa
> code, it seems only dsa_port_vid_add() does a prepare phase, judging
> by .ph_prepare being set. dsa_port_{vlan,mbd,fdb}_add directly call
> the add phase, without the prepare phase. So I'm guessing "supposed
> to be called with a prepare phase" is not quite accurate. This also
> exceeds the scope of the simple fix I had in mind.
>
> > ds->ops->port_mdb_add check must go where it is used, that is to say
> > at the beginning of dsa_switch_mdb_add_bitmap() (similarly to what
> > dsa_switch_mdb_prepare_bitmap() does), not in dsa_switch_mdb_add.
>
> Andrew asked me to move it to where it is now. Please take a look at
> v1 [2] if it's what you would like.
>
> I'm ok either way.
I still cannot find in the code where a SWITCHDEV_OBJ_ID_PORT_MDB object
gets added without a prepare phase or a trans object, but it wouldn't hurt to
double check the presence of ds->ops->port_mdb_add before calling it anyway,
since a patch may actually bypass this prepare phase.
Your v1 patch was a bit confusing and changed the signature of the function
at the same time. Please check the callback where it is used, like this:
diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index 4ec5b7f85d51..09d9286b27cc 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -153,6 +153,9 @@ static void dsa_switch_mdb_add_bitmap(struct dsa_switch *ds,
{
int port;
+ if (!ds->ops->port_mdb_add)
+ return;
+
for_each_set_bit(port, bitmap, ds->num_ports)
ds->ops->port_mdb_add(ds, port, mdb);
}
This will be easier to maintain. Please provide a simpler commit message,
this one is not relevant. Are you actually able to reproduce this stack
trace? If not, that is not necessary to add it to the commit message...
Thank you for your patience,
Vivien
^ permalink raw reply
* Re: [PATCH 03/17] mlx5: no need to check return value of debugfs_create functions
From: Saeed Mahameed @ 2019-08-06 20:37 UTC (permalink / raw)
To: gregkh@linuxfoundation.org, netdev@vger.kernel.org; +Cc: leon@kernel.org
In-Reply-To: <20190806161128.31232-4-gregkh@linuxfoundation.org>
On Tue, 2019-08-06 at 18:11 +0200, Greg Kroah-Hartman wrote:
> When calling debugfs functions, there is no need to ever check the
> return value. The function can work or not, but the code logic
> should
> never do something different based on this.
>
> This cleans up a lot of unneeded code and logic around the debugfs
> files, making all of this much simpler and easier to understand as we
> don't need to keep the dentries saved anymore.
>
> Cc: Saeed Mahameed <saeedm@mellanox.com>
> Cc: Leon Romanovsky <leon@kernel.org>
> Cc: netdev@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Acked-by: Saeed Mahameed <saeedm@mellanox.com>
Some unexpected/undefined driver behavior might occur if some of the
debug_fs_* calls should fail in this driver, I will follow up with a
patch to address this.
Thanks,
Saeed.
^ permalink raw reply
* Re: [PATCH v2 01/34] mm/gup: add make_dirty arg to put_user_pages_dirty_lock()
From: John Hubbard @ 2019-08-06 20:39 UTC (permalink / raw)
To: Ira Weiny, john.hubbard
Cc: Andrew Morton, Christoph Hellwig, Dan Williams, Dave Chinner,
Dave Hansen, Jan Kara, Jason Gunthorpe, Jérôme Glisse,
LKML, amd-gfx, ceph-devel, devel, devel, dri-devel, intel-gfx,
kvm, linux-arm-kernel, linux-block, linux-crypto, linux-fbdev,
linux-fsdevel, linux-media, linux-mm, linux-nfs, linux-rdma,
linux-rpi-kernel, linux-xfs, netdev, rds-devel, sparclinux, x86,
xen-devel, Christoph Hellwig, Matthew Wilcox
In-Reply-To: <20190806173945.GA4748@iweiny-DESK2.sc.intel.com>
On 8/6/19 10:39 AM, Ira Weiny wrote:
> On Sun, Aug 04, 2019 at 03:48:42PM -0700, john.hubbard@gmail.com wrote:
>> From: John Hubbard <jhubbard@nvidia.com>
...
>> -
>> /**
>> - * put_user_pages_dirty() - release and dirty an array of gup-pinned pages
>> - * @pages: array of pages to be marked dirty and released.
>> + * put_user_pages_dirty_lock() - release and optionally dirty gup-pinned pages
>> + * @pages: array of pages to be maybe marked dirty, and definitely released.
>
> Better would be.
>
> @pages: array of pages to be put
OK, I'll change to that wording.
>
>> * @npages: number of pages in the @pages array.
>> + * @make_dirty: whether to mark the pages dirty
>> *
>> * "gup-pinned page" refers to a page that has had one of the get_user_pages()
>> * variants called on that page.
>> *
>> * For each page in the @pages array, make that page (or its head page, if a
>> - * compound page) dirty, if it was previously listed as clean. Then, release
>> - * the page using put_user_page().
>> + * compound page) dirty, if @make_dirty is true, and if the page was previously
>> + * listed as clean. In any case, releases all pages using put_user_page(),
>> + * possibly via put_user_pages(), for the non-dirty case.
>
> I don't think users of this interface need this level of detail. I think
> something like.
>
> * For each page in the @pages array, release the page. If @make_dirty is
> * true, mark the page dirty prior to release.
Yes, it is too wordy, I'll change to that.
>
...
>> -void put_user_pages_dirty_lock(struct page **pages, unsigned long npages)
>> -{
>> - __put_user_pages_dirty(pages, npages, set_page_dirty_lock);
>> + /*
>> + * TODO: this can be optimized for huge pages: if a series of pages is
>> + * physically contiguous and part of the same compound page, then a
>> + * single operation to the head page should suffice.
>> + */
>
> I think this comment belongs to the for loop below... or just something about
> how to make this and put_user_pages() more efficient. It is odd, that this is
> the same comment as in put_user_pages()...
Actually I think I'll just delete the comment entirely, it's just noise really.
>
> The code is good. So... Other than the comments.
>
> Reviewed-by: Ira Weiny <ira.weiny@intel.com>
Thanks for the review!
thanks,
--
John Hubbard
NVIDIA
^ permalink raw reply
* Re: [PATCH v3] mlx5: Use refcount_t for refcount
From: Saeed Mahameed @ 2019-08-06 20:40 UTC (permalink / raw)
To: hslester96@gmail.com
Cc: linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org, davem@davemloft.net, jgg@ziepe.ca,
leon@kernel.org, dledford@redhat.com
In-Reply-To: <20190806015950.18167-1-hslester96@gmail.com>
On Tue, 2019-08-06 at 09:59 +0800, Chuhong Yuan wrote:
> Reference counters are preferred to use refcount_t instead of
> atomic_t.
> This is because the implementation of refcount_t can prevent
> overflows and detect possible use-after-free.
> So convert atomic_t ref counters to refcount_t.
>
> Signed-off-by: Chuhong Yuan <hslester96@gmail.com>
> ---
> Changes in v3:
> - Merge v2 patches together.
>
> drivers/infiniband/hw/mlx5/srq_cmd.c | 6 +++---
> drivers/net/ethernet/mellanox/mlx5/core/qp.c | 6 +++---
> include/linux/mlx5/driver.h | 3 ++-
> 3 files changed, 8 insertions(+), 7 deletions(-)
>
LGTM, Leon, let me know if you are happy with this version,
this should go to mlx5-next.
^ permalink raw reply
* Re: [PATCH v2] net/mlx5e: Use refcount_t for refcount
From: Saeed Mahameed @ 2019-08-06 20:42 UTC (permalink / raw)
To: hslester96@gmail.com
Cc: linux-rdma@vger.kernel.org, davem@davemloft.net,
netdev@vger.kernel.org, leon@kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20190802164828.20243-1-hslester96@gmail.com>
On Sat, 2019-08-03 at 00:48 +0800, Chuhong Yuan wrote:
> refcount_t is better for reference counters since its
> implementation can prevent overflows.
> So convert atomic_t ref counters to refcount_t.
>
> Signed-off-by: Chuhong Yuan <hslester96@gmail.com>
> ---
> Changes in v2:
> - Add #include.
>
Acked-by: Saeed Mahameed <saeedm@mellanox.com>
Dave, up to you take it, or leave it to me :).
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox