* Re: [PATCH v4 2/8] net: can: c_can: Introduce c_can_driver_data structure
From: Marc Kleine-Budde @ 2014-11-13 10:57 UTC (permalink / raw)
To: Roger Quadros, wg
Cc: wsa, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm,
sergei.shtylyov, linux-omap, linux-can, netdev
In-Reply-To: <1415371762-29885-3-git-send-email-rogerq@ti.com>
[-- Attachment #1: Type: text/plain, Size: 1593 bytes --]
On 11/07/2014 03:49 PM, Roger Quadros wrote:
> We want to have more data than just can_dev_id to be present
> in the driver data e.g. TI platforms need RAMINIT register
> description. Introduce the c_can_driver_data structure and move
> the can_dev_id into it.
>
> Tidy up the way it is used on probe().
>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
[...]
> @@ -198,21 +206,19 @@ static int c_can_plat_probe(struct platform_device *pdev)
> struct net_device *dev;
> struct c_can_priv *priv;
> const struct of_device_id *match;
> - const struct platform_device_id *id;
> struct resource *mem, *res;
> int irq;
> struct clk *clk;
> -
> - if (pdev->dev.of_node) {
> - match = of_match_device(c_can_of_table, &pdev->dev);
> - if (!match) {
> - dev_err(&pdev->dev, "Failed to find matching dt id\n");
> - ret = -EINVAL;
> - goto exit;
> - }
> - id = match->data;
> + const struct c_can_driver_data *drvdata;
> +
> + match = of_match_device(c_can_of_table, &pdev->dev);
> + if (match) {
> + drvdata = match->data;
> + } else if (pdev->id_entry->driver_data) {
> + drvdata = (struct c_can_driver_data *)
> + pdev->id_entry->driver_data;
^^^^^^^^^^^^^^
I've changes this to platform_get_device_id() while aplying.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] can: Fix bug in suspend/resume
From: Marc Kleine-Budde @ 2014-11-13 10:46 UTC (permalink / raw)
To: Kedareswara rao Appana, wg, michal.simek, soren.brinkmann,
grant.likely, robh+dt
Cc: linux-can, netdev, linux-arm-kernel, linux-kernel, devicetree,
Kedareswara rao Appana
In-Reply-To: <546484D9.4030005@pengutronix.de>
[-- Attachment #1: Type: text/plain, Size: 2840 bytes --]
On 11/13/2014 11:15 AM, Marc Kleine-Budde wrote:
> On 11/13/2014 07:58 AM, Kedareswara rao Appana wrote:
>> When accessing the priv structure use container_of instead of dev_get_drvdata.
>
> Why?
The drvdata here is the struct net_device, not the platform device.
Please state this in the commit message.
If I understand the code correct, you can make use of the existing
helper function to_platform_device():
http://lxr.free-electrons.com/source/include/linux/platform_device.h#L42
>
>> Enable the clocks in the suspend before accessing the registers of the CAN.
>>
>> Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com>
>> ---
>> drivers/net/can/xilinx_can.c | 20 ++++++++++++++++++--
>> 1 files changed, 18 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
>> index 5e8b560..63ef645 100644
>> --- a/drivers/net/can/xilinx_can.c
>> +++ b/drivers/net/can/xilinx_can.c
>> @@ -972,15 +972,30 @@ static const struct net_device_ops xcan_netdev_ops = {
>> */
>> static int __maybe_unused xcan_suspend(struct device *dev)
>> {
>> - struct platform_device *pdev = dev_get_drvdata(dev);
>> + struct platform_device *pdev = container_of(dev,
>> + struct platform_device, dev);
>> struct net_device *ndev = platform_get_drvdata(pdev);
>> struct xcan_priv *priv = netdev_priv(ndev);
>> + int ret;
>>
>> if (netif_running(ndev)) {
>> netif_stop_queue(ndev);
>> netif_device_detach(ndev);
>> }
>>
>> + ret = clk_prepare_enable(priv->can_clk);
>> + if (ret) {
>> + dev_err(dev, "unable to enable device clock\n");
>> + return ret;
>> + }
>> +
>> + ret = clk_prepare_enable(priv->bus_clk);
>> + if (ret) {
>> + dev_err(dev, "unable to enable bus clock\n");
>> + clk_disable_unprepare(priv->can_clk);
>> + return ret;
>> + }
>
> Now you have clock imbalance. Per suspend/resume cycle the clocks are
> enabled twice, but disabled only once.
>
>> +
>> priv->write_reg(priv, XCAN_MSR_OFFSET, XCAN_MSR_SLEEP_MASK);
>> priv->can.state = CAN_STATE_SLEEPING;
>>
>> @@ -999,7 +1014,8 @@ static int __maybe_unused xcan_suspend(struct device *dev)
>> */
>> static int __maybe_unused xcan_resume(struct device *dev)
>> {
>> - struct platform_device *pdev = dev_get_drvdata(dev);
>> + struct platform_device *pdev = container_of(dev,
>> + struct platform_device, dev);
>> struct net_device *ndev = platform_get_drvdata(pdev);
>> struct xcan_priv *priv = netdev_priv(ndev);
>> int ret;
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 4/4] rhashtable: Add parent argument to mutex_is_held
From: Herbert Xu @ 2014-11-13 10:43 UTC (permalink / raw)
To: Thomas Graf; +Cc: netdev
In-Reply-To: <20141113104124.GA24379@casper.infradead.org>
On Thu, Nov 13, 2014 at 10:41:24AM +0000, Thomas Graf wrote:
>
> Never mind. You did fix it. I looked at the wrong patch.
OK. Now comes the fun part of shoehorning the xfrm_policy bydst
hash into rhashtable :)
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH 0/4] rhashtable: Allow local locks to be used and tested
From: Thomas Graf @ 2014-11-13 10:41 UTC (permalink / raw)
To: Herbert Xu; +Cc: netdev
In-Reply-To: <20141113101025.GA3728@gondor.apana.org.au>
On 11/13/14 at 06:10pm, Herbert Xu wrote:
> Hi:
>
> This series moves mutex_is_held entirely under PROVE_LOCKING so
> there is zero foot print when we're not debugging. More importantly
> it adds a parrent argument to mutex_is_held so that we can test
> local locks rather than global ones (e.g., per-namespace locks).
LGTM
Acked-by: Thomas Graf <tgraf@suug.ch>
^ permalink raw reply
* Re: [PATCH 4/4] rhashtable: Add parent argument to mutex_is_held
From: Thomas Graf @ 2014-11-13 10:41 UTC (permalink / raw)
To: Herbert Xu; +Cc: netdev
In-Reply-To: <20141113103834.GA4024@gondor.apana.org.au>
On 11/13/14 at 06:38pm, Herbert Xu wrote:
> On Thu, Nov 13, 2014 at 10:37:23AM +0000, Thomas Graf wrote:
> > On 11/13/14 at 06:11pm, Herbert Xu wrote:
> > > Currently mutex_is_held can only test locks in the that are global
> > > since it takes no arguments. This prevents rhashtable from being
> > > used in places where locks are lock, e.g., per-namespace locks.
> > >
> > > This patch adds a parent field to mutex_is_held and rhashtable_params
> > > so that local locks can be used (and tested).
> > >
> > > Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
> >
> > Could you fix the documentation of rhashtable_init() as well?
> >
> > [...]
> > * struct rhashtable_params params = {
> > * .head_offset = offsetof(struct test_obj, node),
> > * .key_offset = offsetof(struct test_obj, key),
> > * .key_len = sizeof(int),
> > * .hashfn = arch_fast_hash,
> > * .mutex_is_held = &my_mutex_is_held,
> > * };
> > [...]
>
> Sorry I missed that. Will do.
Never mind. You did fix it. I looked at the wrong patch.
^ permalink raw reply
* Re: [PATCH 4/4] rhashtable: Add parent argument to mutex_is_held
From: Herbert Xu @ 2014-11-13 10:38 UTC (permalink / raw)
To: Thomas Graf; +Cc: netdev
In-Reply-To: <20141113103723.GO19157@casper.infradead.org>
On Thu, Nov 13, 2014 at 10:37:23AM +0000, Thomas Graf wrote:
> On 11/13/14 at 06:11pm, Herbert Xu wrote:
> > Currently mutex_is_held can only test locks in the that are global
> > since it takes no arguments. This prevents rhashtable from being
> > used in places where locks are lock, e.g., per-namespace locks.
> >
> > This patch adds a parent field to mutex_is_held and rhashtable_params
> > so that local locks can be used (and tested).
> >
> > Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>
> Could you fix the documentation of rhashtable_init() as well?
>
> [...]
> * struct rhashtable_params params = {
> * .head_offset = offsetof(struct test_obj, node),
> * .key_offset = offsetof(struct test_obj, key),
> * .key_len = sizeof(int),
> * .hashfn = arch_fast_hash,
> * .mutex_is_held = &my_mutex_is_held,
> * };
> [...]
Sorry I missed that. Will do.
Thanks,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH 4/4] rhashtable: Add parent argument to mutex_is_held
From: Thomas Graf @ 2014-11-13 10:37 UTC (permalink / raw)
To: Herbert Xu; +Cc: netdev
In-Reply-To: <E1XorN4-0000zn-0H@gondolin.me.apana.org.au>
On 11/13/14 at 06:11pm, Herbert Xu wrote:
> Currently mutex_is_held can only test locks in the that are global
> since it takes no arguments. This prevents rhashtable from being
> used in places where locks are lock, e.g., per-namespace locks.
>
> This patch adds a parent field to mutex_is_held and rhashtable_params
> so that local locks can be used (and tested).
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Could you fix the documentation of rhashtable_init() as well?
[...]
* struct rhashtable_params params = {
* .head_offset = offsetof(struct test_obj, node),
* .key_offset = offsetof(struct test_obj, key),
* .key_len = sizeof(int),
* .hashfn = arch_fast_hash,
* .mutex_is_held = &my_mutex_is_held,
* };
[...]
^ permalink raw reply
* Hello my Beloved
From: Rosemary Zandile @ 2014-11-12 10:52 UTC (permalink / raw)
In-Reply-To: <1589298833.578642.1415735936528.JavaMail.root@ninhbinh.gov.vn>
[-- Attachment #1: Type: text/plain, Size: 83 bytes --]
My name is Rosemary Zandile
Please open my attached file and
get back to me.
[-- Attachment #2: MRS_ROSEMARY_ZANDILE.pdf --]
[-- Type: application/pdf, Size: 89693 bytes --]
^ permalink raw reply
* Re: randconfig build error with next-20141112, in net/sched
From: Hannes Frederic Sowa @ 2014-11-13 10:22 UTC (permalink / raw)
To: Jim Davis
Cc: Stephen Rothwell, linux-next, linux-kernel, jhs, David S. Miller,
netdev
In-Reply-To: <CA+r1ZhjMGJVSdp71pQ5aHjAvDDxDYBsgzmwcnt5oNyZTmdZDiw@mail.gmail.com>
On Mi, 2014-11-12 at 15:33 -0700, Jim Davis wrote:
> Building with the attached random configuration file,
>
> ERROR: "reciprocal_value" [net/sched/sch_sfq.ko] undefined!
> ERROR: "reciprocal_value" [net/sched/sch_netem.ko] undefined!
Thanks for the report. I think moving reciproval_div.o from lib-y to
obj-y should resolve the problem. On it...
The problem with lib-y is, if vmlinux itself doesn't use the symbol,
even if it is EXPORT_SYMBOLED, it won't be linked into the kernel. You
seem to hit a configuration where reciproval_divide wasn't used in the
kernel at all but only in modules, as such it got purged from vmlinux
during linking.
Thanks,
Hannes
^ permalink raw reply
* Re: [PATCH] can: Fix bug in suspend/resume
From: Marc Kleine-Budde @ 2014-11-13 10:15 UTC (permalink / raw)
To: Kedareswara rao Appana, wg, michal.simek, soren.brinkmann,
grant.likely, robh+dt
Cc: linux-can, netdev, linux-arm-kernel, linux-kernel, devicetree,
Kedareswara rao Appana
In-Reply-To: <a44efc1fb0144cca9d0c4851cc51db25@BL2FFO11FD024.protection.gbl>
[-- Attachment #1: Type: text/plain, Size: 2424 bytes --]
On 11/13/2014 07:58 AM, Kedareswara rao Appana wrote:
> When accessing the priv structure use container_of instead of dev_get_drvdata.
Why?
> Enable the clocks in the suspend before accessing the registers of the CAN.
>
> Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com>
> ---
> drivers/net/can/xilinx_can.c | 20 ++++++++++++++++++--
> 1 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
> index 5e8b560..63ef645 100644
> --- a/drivers/net/can/xilinx_can.c
> +++ b/drivers/net/can/xilinx_can.c
> @@ -972,15 +972,30 @@ static const struct net_device_ops xcan_netdev_ops = {
> */
> static int __maybe_unused xcan_suspend(struct device *dev)
> {
> - struct platform_device *pdev = dev_get_drvdata(dev);
> + struct platform_device *pdev = container_of(dev,
> + struct platform_device, dev);
> struct net_device *ndev = platform_get_drvdata(pdev);
> struct xcan_priv *priv = netdev_priv(ndev);
> + int ret;
>
> if (netif_running(ndev)) {
> netif_stop_queue(ndev);
> netif_device_detach(ndev);
> }
>
> + ret = clk_prepare_enable(priv->can_clk);
> + if (ret) {
> + dev_err(dev, "unable to enable device clock\n");
> + return ret;
> + }
> +
> + ret = clk_prepare_enable(priv->bus_clk);
> + if (ret) {
> + dev_err(dev, "unable to enable bus clock\n");
> + clk_disable_unprepare(priv->can_clk);
> + return ret;
> + }
Now you have clock imbalance. Per suspend/resume cycle the clocks are
enabled twice, but disabled only once.
> +
> priv->write_reg(priv, XCAN_MSR_OFFSET, XCAN_MSR_SLEEP_MASK);
> priv->can.state = CAN_STATE_SLEEPING;
>
> @@ -999,7 +1014,8 @@ static int __maybe_unused xcan_suspend(struct device *dev)
> */
> static int __maybe_unused xcan_resume(struct device *dev)
> {
> - struct platform_device *pdev = dev_get_drvdata(dev);
> + struct platform_device *pdev = container_of(dev,
> + struct platform_device, dev);
> struct net_device *ndev = platform_get_drvdata(pdev);
> struct xcan_priv *priv = netdev_priv(ndev);
> int ret;
>
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [PATCH 4/4] rhashtable: Add parent argument to mutex_is_held
From: Herbert Xu @ 2014-11-13 10:11 UTC (permalink / raw)
To: netdev, Thomas Graf
In-Reply-To: <20141113101025.GA3728@gondor.apana.org.au>
Currently mutex_is_held can only test locks in the that are global
since it takes no arguments. This prevents rhashtable from being
used in places where locks are lock, e.g., per-namespace locks.
This patch adds a parent field to mutex_is_held and rhashtable_params
so that local locks can be used (and tested).
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---
include/linux/rhashtable.h | 3 ++-
lib/rhashtable.c | 4 ++--
net/netfilter/nft_hash.c | 2 +-
net/netlink/af_netlink.c | 2 +-
4 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/include/linux/rhashtable.h b/include/linux/rhashtable.h
index 96ce8ce..473e26b 100644
--- a/include/linux/rhashtable.h
+++ b/include/linux/rhashtable.h
@@ -66,7 +66,8 @@ struct rhashtable_params {
bool (*shrink_decision)(const struct rhashtable *ht,
size_t new_size);
#ifdef CONFIG_PROVE_LOCKING
- int (*mutex_is_held)(void);
+ int (*mutex_is_held)(void *parent);
+ void *parent;
#endif
};
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index 548bb29..6acc4ce 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -32,7 +32,7 @@
#ifdef CONFIG_PROVE_LOCKING
int lockdep_rht_mutex_is_held(const struct rhashtable *ht)
{
- return ht->p.mutex_is_held();
+ return ht->p.mutex_is_held(ht->p.parent);
}
EXPORT_SYMBOL_GPL(lockdep_rht_mutex_is_held);
#endif
@@ -618,7 +618,7 @@ EXPORT_SYMBOL_GPL(rhashtable_destroy);
#define TEST_NEXPANDS 4
#ifdef CONFIG_PROVE_LOCKING
-static int test_mutex_is_held(void)
+static int test_mutex_is_held(void *parent)
{
return 1;
}
diff --git a/net/netfilter/nft_hash.c b/net/netfilter/nft_hash.c
index b86305c..3f75aaa 100644
--- a/net/netfilter/nft_hash.c
+++ b/net/netfilter/nft_hash.c
@@ -154,7 +154,7 @@ static unsigned int nft_hash_privsize(const struct nlattr * const nla[])
}
#ifdef CONFIG_PROVE_LOCKING
-static int lockdep_nfnl_lock_is_held(void)
+static int lockdep_nfnl_lock_is_held(void *parent)
{
return lockdep_nfnl_is_held(NFNL_SUBSYS_NFTABLES);
}
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 53b8ea7..9e0628c 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -115,7 +115,7 @@ DEFINE_MUTEX(nl_sk_hash_lock);
EXPORT_SYMBOL_GPL(nl_sk_hash_lock);
#ifdef CONFIG_PROVE_LOCKING
-static int lockdep_nl_sk_hash_is_held(void)
+static int lockdep_nl_sk_hash_is_held(void *parent)
{
if (debug_locks)
return lockdep_is_held(&nl_sk_hash_lock) || lockdep_is_held(&nl_table_lock);
^ permalink raw reply related
* [PATCH 3/4] rhashtable: Move mutex_is_held under PROVE_LOCKING
From: Herbert Xu @ 2014-11-13 10:11 UTC (permalink / raw)
To: netdev, Thomas Graf
In-Reply-To: <20141113101025.GA3728@gondor.apana.org.au>
The rhashtable function mutex_is_held is only used when PROVE_LOCKING
is enabled. This patch makes the mutex_is_held field in rhashtable
optional depending on PROVE_LOCKING.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---
include/linux/rhashtable.h | 2 ++
lib/rhashtable.c | 8 ++++++++
2 files changed, 10 insertions(+)
diff --git a/include/linux/rhashtable.h b/include/linux/rhashtable.h
index fb298e9d..96ce8ce 100644
--- a/include/linux/rhashtable.h
+++ b/include/linux/rhashtable.h
@@ -65,7 +65,9 @@ struct rhashtable_params {
size_t new_size);
bool (*shrink_decision)(const struct rhashtable *ht,
size_t new_size);
+#ifdef CONFIG_PROVE_LOCKING
int (*mutex_is_held)(void);
+#endif
};
/**
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index 624a0b7..548bb29 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -532,7 +532,9 @@ static size_t rounded_hashtable_size(struct rhashtable_params *params)
* .key_offset = offsetof(struct test_obj, key),
* .key_len = sizeof(int),
* .hashfn = arch_fast_hash,
+ * #ifdef CONFIG_PROVE_LOCKING
* .mutex_is_held = &my_mutex_is_held,
+ * #endif
* };
*
* Configuration Example 2: Variable length keys
@@ -552,7 +554,9 @@ static size_t rounded_hashtable_size(struct rhashtable_params *params)
* .head_offset = offsetof(struct test_obj, node),
* .hashfn = arch_fast_hash,
* .obj_hashfn = my_hash_fn,
+ * #ifdef CONFIG_PROVE_LOCKING
* .mutex_is_held = &my_mutex_is_held,
+ * #endif
* };
*/
int rhashtable_init(struct rhashtable *ht, struct rhashtable_params *params)
@@ -613,10 +617,12 @@ EXPORT_SYMBOL_GPL(rhashtable_destroy);
#define TEST_PTR ((void *) 0xdeadbeef)
#define TEST_NEXPANDS 4
+#ifdef CONFIG_PROVE_LOCKING
static int test_mutex_is_held(void)
{
return 1;
}
+#endif
struct test_obj {
void *ptr;
@@ -767,7 +773,9 @@ static int __init test_rht_init(void)
.key_offset = offsetof(struct test_obj, value),
.key_len = sizeof(int),
.hashfn = arch_fast_hash,
+#ifdef CONFIG_PROVE_LOCKING
.mutex_is_held = &test_mutex_is_held,
+#endif
.grow_decision = rht_grow_above_75,
.shrink_decision = rht_shrink_below_30,
};
^ permalink raw reply related
* [PATCH 2/4] netfilter: Move mutex_is_held under PROVE_LOCKING
From: Herbert Xu @ 2014-11-13 10:11 UTC (permalink / raw)
To: netdev, Thomas Graf
In-Reply-To: <20141113101025.GA3728@gondor.apana.org.au>
The rhashtable function mutex_is_held is only used when PROVE_LOCKING
is enabled. This patch modifies netfilter so that we can rhashtable.h
itself can later make mutex_is_held optional depending on PROVE_LOCKING.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---
net/netfilter/nft_hash.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/netfilter/nft_hash.c b/net/netfilter/nft_hash.c
index 8892b7b..b86305c 100644
--- a/net/netfilter/nft_hash.c
+++ b/net/netfilter/nft_hash.c
@@ -153,10 +153,12 @@ static unsigned int nft_hash_privsize(const struct nlattr * const nla[])
return sizeof(struct rhashtable);
}
+#ifdef CONFIG_PROVE_LOCKING
static int lockdep_nfnl_lock_is_held(void)
{
return lockdep_nfnl_is_held(NFNL_SUBSYS_NFTABLES);
}
+#endif
static int nft_hash_init(const struct nft_set *set,
const struct nft_set_desc *desc,
@@ -171,7 +173,9 @@ static int nft_hash_init(const struct nft_set *set,
.hashfn = jhash,
.grow_decision = rht_grow_above_75,
.shrink_decision = rht_shrink_below_30,
+#ifdef CONFIG_PROVE_LOCKING
.mutex_is_held = lockdep_nfnl_lock_is_held,
+#endif
};
return rhashtable_init(priv, ¶ms);
^ permalink raw reply related
* [PATCH 1/4] netlink: Move mutex_is_held under PROVE_LOCKING
From: Herbert Xu @ 2014-11-13 10:11 UTC (permalink / raw)
To: netdev, Thomas Graf
In-Reply-To: <20141113101025.GA3728@gondor.apana.org.au>
The rhashtable function mutex_is_held is only used when PROVE_LOCKING
is enabled. This patch modifies netlink so that we can rhashtable.h
itself can later make mutex_is_held optional depending on PROVE_LOCKING.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---
net/netlink/af_netlink.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 580b794..53b8ea7 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -114,14 +114,14 @@ static atomic_t nl_table_users = ATOMIC_INIT(0);
DEFINE_MUTEX(nl_sk_hash_lock);
EXPORT_SYMBOL_GPL(nl_sk_hash_lock);
+#ifdef CONFIG_PROVE_LOCKING
static int lockdep_nl_sk_hash_is_held(void)
{
-#ifdef CONFIG_LOCKDEP
if (debug_locks)
return lockdep_is_held(&nl_sk_hash_lock) || lockdep_is_held(&nl_table_lock);
-#endif
return 1;
}
+#endif
static ATOMIC_NOTIFIER_HEAD(netlink_chain);
@@ -3133,7 +3133,9 @@ static int __init netlink_proto_init(void)
.max_shift = 16, /* 64K */
.grow_decision = rht_grow_above_75,
.shrink_decision = rht_shrink_below_30,
+#ifdef CONFIG_PROVE_LOCKING
.mutex_is_held = lockdep_nl_sk_hash_is_held,
+#endif
};
if (err != 0)
^ permalink raw reply related
* [PATCH 0/4] rhashtable: Allow local locks to be used and tested
From: Herbert Xu @ 2014-11-13 10:10 UTC (permalink / raw)
To: netdev; +Cc: Thomas Graf
Hi:
This series moves mutex_is_held entirely under PROVE_LOCKING so
there is zero foot print when we're not debugging. More importantly
it adds a parrent argument to mutex_is_held so that we can test
local locks rather than global ones (e.g., per-namespace locks).
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH V4 2/3] can: m_can: update to support CAN FD features
From: Marc Kleine-Budde @ 2014-11-13 10:10 UTC (permalink / raw)
To: Dong Aisheng, linux-can
Cc: wg, varkabhadram, netdev, socketcan, linux-arm-kernel
In-Reply-To: <1415349914-9145-2-git-send-email-b29396@freescale.com>
[-- Attachment #1: Type: text/plain, Size: 3112 bytes --]
On 11/07/2014 09:45 AM, Dong Aisheng wrote:
> Bosch M_CAN is CAN FD capable device. This patch implements the CAN
> FD features include up to 64 bytes payload and bitrate switch function.
> 1) Change the Rx FIFO and Tx Buffer to 64 bytes for support CAN FD
> up to 64 bytes payload. It's backward compatible with old 8 bytes
> normal CAN frame.
> 2) Allocate can frame or canfd frame based on EDL bit
> 3) Bitrate Switch function is disabled by default and will be enabled
> according to CANFD_BRS bit in cf->flags.
>
> Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>
> Signed-off-by: Dong Aisheng <b29396@freescale.com>
[...]
> -static void m_can_read_fifo(const struct net_device *dev, struct can_frame *cf,
> - u32 rxfs)
> +static void m_can_read_fifo(struct net_device *dev, u32 rxfs)
> {
> + struct net_device_stats *stats = &dev->stats;
> struct m_can_priv *priv = netdev_priv(dev);
> - u32 id, fgi;
> + struct canfd_frame *cf;
> + struct sk_buff *skb;
> + u32 id, fgi, dlc;
> + int i;
>
> /* calculate the fifo get index for where to read data */
> fgi = (rxfs & RXFS_FGI_MASK) >> RXFS_FGI_OFF;
> + dlc = m_can_fifo_read(priv, fgi, M_CAN_FIFO_DLC);
> + if (dlc & RX_BUF_EDL)
> + skb = alloc_canfd_skb(dev, &cf);
> + else
> + skb = alloc_can_skb(dev, (struct can_frame **)&cf);
> + if (!skb) {
> + stats->rx_dropped++;
> + return;
> + }
> +
> id = m_can_fifo_read(priv, fgi, M_CAN_FIFO_ID);
> if (id & RX_BUF_XTD)
> cf->can_id = (id & CAN_EFF_MASK) | CAN_EFF_FLAG;
> else
> cf->can_id = (id >> 18) & CAN_SFF_MASK;
>
> - if (id & RX_BUF_RTR) {
> + if (id & RX_BUF_ESI) {
> + cf->flags |= CANFD_ESI;
> + netdev_dbg(dev, "ESI Error\n");
> + }
> +
> + if (!(dlc & RX_BUF_EDL) && (id & RX_BUF_RTR)) {
> cf->can_id |= CAN_RTR_FLAG;
I just noticed, that you don't set the cf->dlc (or cf->len) in the RTR
case. Please create a separate patch that fixes this problem.
> } else {
> id = m_can_fifo_read(priv, fgi, M_CAN_FIFO_DLC);
> - cf->can_dlc = get_can_dlc((id >> 16) & 0x0F);
> - *(u32 *)(cf->data + 0) = m_can_fifo_read(priv, fgi,
> - M_CAN_FIFO_DATA(0));
> - *(u32 *)(cf->data + 4) = m_can_fifo_read(priv, fgi,
> - M_CAN_FIFO_DATA(1));
> + if (dlc & RX_BUF_EDL)
> + cf->len = can_dlc2len((id >> 16) & 0x0F);
> + else
> + cf->len = get_can_dlc((id >> 16) & 0x0F);
> +
> + if (id & RX_BUF_BRS)
> + cf->flags |= CANFD_BRS;
> +
> + for (i = 0; i < cf->len; i += 4)
> + *(u32 *)(cf->data + i) =
> + m_can_fifo_read(priv, fgi,
> + M_CAN_FIFO_DATA(i / 4));
> }
>
> /* acknowledge rx fifo 0 */
> m_can_write(priv, M_CAN_RXF0A, fgi);
> +
> + stats->rx_packets++;
> + stats->rx_bytes += cf->len;
> +
> + netif_receive_skb(skb);
> }
Regards,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH V4 1/3] can: add can_is_canfd_skb() API
From: Marc Kleine-Budde @ 2014-11-13 10:04 UTC (permalink / raw)
To: Dong Aisheng, linux-can
Cc: wg, varkabhadram, netdev, socketcan, linux-arm-kernel
In-Reply-To: <1415349914-9145-1-git-send-email-b29396@freescale.com>
[-- Attachment #1: Type: text/plain, Size: 466 bytes --]
On 11/07/2014 09:45 AM, Dong Aisheng wrote:
> The CAN device drivers can use it to check if the frame to send is on
> CAN FD mode or normal CAN mode.
Applied patches 1 and 3.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Resume S4 - r8169 BROKEN
From: poma @ 2014-11-13 9:50 UTC (permalink / raw)
To: netdev
Resume S4 - r8169 RealTek RTL-8169 Gigabit Ethernet driver - BROKEN
Tested with:
kernel-3.18.0-0.rc4.git0.2.fc22
http://koji.fedoraproject.org/koji/buildinfo?buildID=592269
Nov 11 2014
&
kernel-3.18.0-rc2.git-9dfa9a2-net-next+
https://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commit/?id=9dfa9a2
2014-11-13
# lspci -knn -s 03:00.0
03:00.0 Ethernet controller [0200]: Realtek Semiconductor Co., Ltd. RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller [10ec:8168] (rev 02)
Subsystem: Gigabyte Technology Co., Ltd Motherboard [1458:e000]
Kernel driver in use: r8169
Kernel modules: r8169
# modprobe -v r8169
insmod /lib/modules/3.18.0-rc2.git-9dfa9a2-net-next+/kernel/drivers/net/mii.ko
insmod /lib/modules/3.18.0-rc2.git-9dfa9a2-net-next+/kernel/drivers/net/ethernet/realtek/r8169.ko
# dmesg
[ 142.344223] r8169 Gigabit Ethernet driver 2.3LK-NAPI loaded
[ 142.344229] r8169 0000:03:00.0: can't disable ASPM; OS doesn't have ASPM control
[ 142.344456] r8169 0000:03:00.0: irq 26 for MSI/MSI-X
[ 142.344600] r8169 0000:03:00.0 eth1: RTL8168c/8111c at 0xffffc9000291a000, 00:12:34:56:78:30, XID 1c4000c0 IRQ 26
[ 142.344602] r8169 0000:03:00.0 eth1: jumbo features [frames: 6128 bytes, tx checksumming: ko]
[ 142.388933] r8169 0000:03:00.0 enp3s0: renamed from eth1
[ 142.417502] r8169 0000:03:00.0 enp3s0: link down
[ 142.417512] r8169 0000:03:00.0 enp3s0: link down
[ 144.029918] r8169 0000:03:00.0 enp3s0: link up
# ifconfig enp3s0
enp3s0: flags=4163<UP,BROADCAST,RUNNING,MULTICAST> mtu 1500
inet 192.168.2.2 netmask 255.255.255.0 broadcast 192.168.2.255
inet6 fe80::212:34ff:fe56:7830 prefixlen 64 scopeid 0x20<link>
ether 00:12:34:56:78:30 txqueuelen 1000 (Ethernet)
RX packets 642 bytes 382970 (373.9 KiB)
RX errors 0 dropped 0 overruns 0 frame 0
TX packets 709 bytes 72634 (70.9 KiB)
TX errors 0 dropped 0 overruns 0 carrier 0 collisions 0
# ping kernel.org -c10
PING kernel.org (199.204.44.194) 56(84) bytes of data.
64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=1 ttl=49 time=134 ms
64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=2 ttl=49 time=133 ms
64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=3 ttl=49 time=134 ms
64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=4 ttl=49 time=133 ms
64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=5 ttl=49 time=134 ms
64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=6 ttl=49 time=134 ms
64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=7 ttl=49 time=134 ms
64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=8 ttl=49 time=133 ms
64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=9 ttl=49 time=133 ms
64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=10 ttl=49 time=134 ms
--- kernel.org ping statistics ---
10 packets transmitted, 10 received, 0% packet loss, time 9011ms
rtt min/avg/max/mdev = 133.008/134.163/134.987/0.683 ms
~~~~~~~~~~~~~~~~~~~
# systemctl suspend
& RESUME
~~~~~~~~
# dmesg
[ 673.601499] PM: Syncing filesystems ... done.
[ 673.793930] PM: Preparing system for mem sleep
[ 673.961198] Freezing user space processes ... (elapsed 0.002 seconds) done.
[ 673.963420] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done.
[ 673.964664] PM: Entering mem sleep
[ 676.457146] PM: suspend of devices complete after 2492.103 msecs
[ 676.457826] PM: late suspend of devices complete after 0.673 msecs
[ 676.469303] PM: noirq suspend of devices complete after 11.469 msecs
[ 676.469889] ACPI: Preparing to enter system sleep state S3
[ 676.470890] PM: Saving platform NVS memory
[ 676.479540] ACPI: Low-level resume complete
[ 676.479613] PM: Restoring platform NVS memory
[ 676.525167] ACPI: Waking up from system sleep state S3
[ 676.590780] r8169 0000:03:00.0 enp3s0: link down
[ 678.060741] r8169 0000:03:00.0 enp3s0: link up
[ 679.604068] PM: resume of devices complete after 3066.534 msecs
[ 679.604657] PM: Finishing wakeup.
[ 679.604662] Restarting tasks ... done.
# ifconfig enp3s0
enp3s0: flags=4163<UP,BROADCAST,RUNNING,MULTICAST> mtu 1500
inet 192.168.2.2 netmask 255.255.255.0 broadcast 192.168.2.255
inet6 fe80::212:34ff:fe56:7830 prefixlen 64 scopeid 0x20<link>
ether 00:12:34:56:78:30 txqueuelen 1000 (Ethernet)
RX packets 782 bytes 464566 (453.6 KiB)
RX errors 0 dropped 0 overruns 0 frame 0
TX packets 860 bytes 86524 (84.4 KiB)
TX errors 0 dropped 0 overruns 0 carrier 0 collisions 0
# ping kernel.org -c10
PING kernel.org (199.204.44.194) 56(84) bytes of data.
64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=1 ttl=49 time=134 ms
64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=2 ttl=49 time=134 ms
64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=3 ttl=49 time=133 ms
64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=4 ttl=49 time=132 ms
64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=5 ttl=49 time=134 ms
64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=6 ttl=49 time=133 ms
64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=7 ttl=49 time=133 ms
64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=8 ttl=49 time=133 ms
64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=9 ttl=49 time=133 ms
64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=10 ttl=49 time=134 ms
--- kernel.org ping statistics ---
10 packets transmitted, 10 received, 0% packet loss, time 9011ms
rtt min/avg/max/mdev = 132.798/133.770/134.919/0.682 ms
~~~~~~~~~~~~~~~~~~~~~
# systemctl hibernate
& THAW
~~~~~~
# dmesg
[ 1187.902839] PM: Hibernation mode set to 'platform'
[ 1188.098296] PM: Syncing filesystems ... done.
[ 1188.545125] Freezing user space processes ... (elapsed 0.002 seconds) done.
[ 1188.548022] PM: Marking nosave pages: [mem 0x00000000-0x00000fff]
[ 1188.548031] PM: Marking nosave pages: [mem 0x0009e000-0x000fffff]
[ 1188.548042] PM: Marking nosave pages: [mem 0xcfef0000-0xffffffff]
[ 1188.549729] PM: Basic memory bitmaps created
[ 1188.549755] PM: Preallocating image memory... done (allocated 641929 pages)
[ 1189.133132] PM: Allocated 2567716 kbytes in 0.58 seconds (4427.09 MB/s)
[ 1189.133241] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done.
[ 1191.867738] PM: freeze of devices complete after 2732.078 msecs
[ 1191.868597] PM: late freeze of devices complete after 0.855 msecs
[ 1191.869951] PM: noirq freeze of devices complete after 1.344 msecs
[ 1191.871294] ACPI: Preparing to enter system sleep state S4
[ 1191.871956] PM: Saving platform NVS memory
[ 1191.879106] PM: Creating hibernation image:
[ 1192.100544] PM: Need to copy 359761 pages
[ 1192.100558] PM: Normal pages needed: 359761 + 1024, available pages: 688358
[ 1191.880196] PM: Restoring platform NVS memory
[ 1191.925748] ACPI: Waking up from system sleep state S4
[ 1191.936325] PM: noirq restore of devices complete after 10.428 msecs
[ 1191.936796] PM: early restore of devices complete after 0.423 msecs
[ 1192.043225] r8169 0000:03:00.0 enp3s0: link down
[ 1192.689507] PM: restore of devices complete after 699.641 msecs
[ 1192.689835] PM: Image restored successfully.
[ 1192.689856] PM: Basic memory bitmaps freed
[ 1192.689860] Restarting tasks ... done.
[ 1193.672620] r8169 0000:03:00.0 enp3s0: link up
# ifconfig enp3s0
enp3s0: flags=4163<UP,BROADCAST,RUNNING,MULTICAST> mtu 1500
inet 192.168.2.2 netmask 255.255.255.0 broadcast 192.168.2.255
inet6 fe80::212:34ff:fe56:7830 prefixlen 64 scopeid 0x20<link>
ether 00:12:34:56:78:30 txqueuelen 1000 (Ethernet)
RX packets 794 bytes 465496 (454.5 KiB)
RX errors 0 dropped 0 overruns 0 frame 0
TX packets 939 bytes 90700 (88.5 KiB)
TX errors 0 dropped 0 overruns 0 carrier 0 collisions 0
# ping kernel.org -c10
ping: unknown host kernel.org
# ping 199.204.44.194 -c10
PING 199.204.44.194 (199.204.44.194) 56(84) bytes of data.
>From 192.168.2.2 icmp_seq=1 Destination Host Unreachable
>From 192.168.2.2 icmp_seq=2 Destination Host Unreachable
>From 192.168.2.2 icmp_seq=3 Destination Host Unreachable
>From 192.168.2.2 icmp_seq=4 Destination Host Unreachable
>From 192.168.2.2 icmp_seq=5 Destination Host Unreachable
>From 192.168.2.2 icmp_seq=6 Destination Host Unreachable
>From 192.168.2.2 icmp_seq=7 Destination Host Unreachable
>From 192.168.2.2 icmp_seq=8 Destination Host Unreachable
>From 192.168.2.2 icmp_seq=9 Destination Host Unreachable
>From 192.168.2.2 icmp_seq=10 Destination Host Unreachable
--- 199.204.44.194 ping statistics ---
10 packets transmitted, 0 received, +10 errors, 100% packet loss, time 9001ms
pipe 4
~~~~~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~
# modprobe -rv r8169 && modprobe -v r8169
rmmod r8169
rmmod mii
insmod /lib/modules/3.18.0-rc2.git-9dfa9a2-net-next+/kernel/drivers/net/mii.ko
insmod /lib/modules/3.18.0-rc2.git-9dfa9a2-net-next+/kernel/drivers/net/ethernet/realtek/r8169.ko
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
# dmesg
[ 1566.163904] r8169 Gigabit Ethernet driver 2.3LK-NAPI loaded
[ 1566.163932] r8169 0000:03:00.0: can't disable ASPM; OS doesn't have ASPM control
[ 1566.164443] r8169 0000:03:00.0: irq 26 for MSI/MSI-X
[ 1566.164997] r8169 0000:03:00.0 eth1: RTL8168c/8111c at 0xffffc900027d0000, 00:12:34:56:78:30, XID 1c4000c0 IRQ 26
[ 1566.165007] r8169 0000:03:00.0 eth1: jumbo features [frames: 6128 bytes, tx checksumming: ko]
[ 1566.383002] r8169 0000:03:00.0 enp3s0: renamed from eth1
[ 1566.401190] r8169 0000:03:00.0 enp3s0: link down
[ 1567.909282] r8169 0000:03:00.0 enp3s0: link up
# ifconfig enp3s0
enp3s0: flags=4163<UP,BROADCAST,RUNNING,MULTICAST> mtu 1500
inet 192.168.2.2 netmask 255.255.255.0 broadcast 192.168.2.255
inet6 fe80::212:34ff:fe56:7830 prefixlen 64 scopeid 0x20<link>
ether 00:12:34:56:78:30 txqueuelen 1000 (Ethernet)
RX packets 59 bytes 14200 (13.8 KiB)
RX errors 0 dropped 0 overruns 0 frame 0
TX packets 77 bytes 7441 (7.2 KiB)
TX errors 0 dropped 0 overruns 0 carrier 0 collisions 0
# ping kernel.org -c10
PING kernel.org (199.204.44.194) 56(84) bytes of data.
64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=1 ttl=49 time=133 ms
64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=2 ttl=49 time=134 ms
64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=3 ttl=49 time=134 ms
64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=4 ttl=49 time=133 ms
64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=5 ttl=49 time=133 ms
64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=6 ttl=49 time=132 ms
64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=7 ttl=49 time=134 ms
64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=8 ttl=49 time=134 ms
64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=9 ttl=49 time=133 ms
64 bytes from yul-korg-pub.kernel.org (199.204.44.194): icmp_seq=10 ttl=49 time=133 ms
--- kernel.org ping statistics ---
10 packets transmitted, 10 received, 0% packet loss, time 9012ms
rtt min/avg/max/mdev = 132.657/133.636/134.708/0.728 ms
poma
^ permalink raw reply
* Re: [PATCH 1/2] virito: introduce methods of fixing device features
From: Jason Wang @ 2014-11-13 9:49 UTC (permalink / raw)
To: Cornelia Huck; +Cc: netdev, virtualization, linux-kernel, mst
In-Reply-To: <20141113101456.43acfdb7.cornelia.huck@de.ibm.com>
On 11/13/2014 05:14 PM, Cornelia Huck wrote:
> On Thu, 13 Nov 2014 17:11:30 +0800
> Jason Wang <jasowang@redhat.com> wrote:
>
>> On 11/13/2014 04:46 PM, Cornelia Huck wrote:
>>> On Thu, 13 Nov 2014 13:52:53 +0800
>>> Jason Wang <jasowang@redhat.com> wrote:
>>>> +static inline void virtio_disable_feature(struct virtio_device *vdev,
>>>> + unsigned int fbit)
>>>> +{
>>>> + BUG_ON(fbit >= VIRTIO_TRANSPORT_F_START);
>>>> + BUG_ON(vdev->config->get_status(vdev) &
>>>> + ~(VIRTIO_CONFIG_S_ACKNOWLEDGE | VIRTIO_CONFIG_S_DRIVER));
>>> When we add virtio-1 support, we can add a check for FEATURES_OK here,
>>> so we're really on the safe side.
>>>
>> If I read the spec correctly, FEATURES_OK was set only after writing the
>> features bits to device. But we want to sanitize the them before.
> I meant doing a BUG when FEATURES_OK is set - sorry for not being clear.
>
I get it, thanks for the clarification.
^ permalink raw reply
* Re: [PATCH 1/2] virito: introduce methods of fixing device features
From: Cornelia Huck @ 2014-11-13 9:14 UTC (permalink / raw)
To: Jason Wang; +Cc: netdev, virtualization, linux-kernel, mst
In-Reply-To: <546475C2.50606@redhat.com>
On Thu, 13 Nov 2014 17:11:30 +0800
Jason Wang <jasowang@redhat.com> wrote:
> On 11/13/2014 04:46 PM, Cornelia Huck wrote:
> > On Thu, 13 Nov 2014 13:52:53 +0800
> > Jason Wang <jasowang@redhat.com> wrote:
> >> +static inline void virtio_disable_feature(struct virtio_device *vdev,
> >> + unsigned int fbit)
> >> +{
> >> + BUG_ON(fbit >= VIRTIO_TRANSPORT_F_START);
> >> + BUG_ON(vdev->config->get_status(vdev) &
> >> + ~(VIRTIO_CONFIG_S_ACKNOWLEDGE | VIRTIO_CONFIG_S_DRIVER));
> > When we add virtio-1 support, we can add a check for FEATURES_OK here,
> > so we're really on the safe side.
> >
>
> If I read the spec correctly, FEATURES_OK was set only after writing the
> features bits to device. But we want to sanitize the them before.
I meant doing a BUG when FEATURES_OK is set - sorry for not being clear.
^ permalink raw reply
* Re: [PATCH 2/2] virtio-net: fix buggy features advertised by host
From: Jason Wang @ 2014-11-13 9:12 UTC (permalink / raw)
To: Cornelia Huck; +Cc: netdev, virtualization, linux-kernel, mst
In-Reply-To: <20141113095326.3e296c77.cornelia.huck@de.ibm.com>
On 11/13/2014 04:53 PM, Cornelia Huck wrote:
> On Thu, 13 Nov 2014 13:52:54 +0800
> Jason Wang <jasowang@redhat.com> wrote:
>
>> This patch tries to detect the possible buggy features advertised by host
>> and fix them. One example is booting virtio-net with only ctrl_vq disabled,
>> qemu may still advertise many features which depends on it. This will
>> trigger several BUG()s in virtnet_send_command().
>>
>> This patch utilizes the fix_features() method, and disables all features that
>> depends on ctrl_vq if it was not advertised.
>>
>> This fixes the crash when booting with ctrl_vq=off.
> That's a qemu device property, right? Might want to mention that, as
> this line sounds like it is a kernel parameter.
Right, ok.
>> Cc: Rusty Russell <rusty@rustcorp.com.au>
>> Cc: Michael S. Tsirkin <mst@redhat.com>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
>> Changes from V1:
>> - fix the cut-and-paste error
>> ---
>> drivers/net/virtio_net.c | 35 +++++++++++++++++++++++++++++++++++
>> 1 file changed, 35 insertions(+)
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index ec2a8b4..6ce125e 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -1948,6 +1948,40 @@ static int virtnet_restore(struct virtio_device *vdev)
>> }
>> #endif
>>
>> +static void virtnet_fix_features(struct virtio_device *dev)
>> +{
>> + if (!virtio_has_feature(dev, VIRTIO_NET_F_CTRL_VQ)) {
>> + if (virtio_has_feature(dev, VIRTIO_NET_F_CTRL_RX)) {
>> + pr_warning("Disable VIRTIO_NET_F_CTRL_RX since host "
>> + "does not advertise VIRTIO_NET_F_CTRL_VQ");
>> + virtio_disable_feature(dev, VIRTIO_NET_F_CTRL_RX);
>> + }
> You should probably use dev_warn() or so, so that the user can figure
> out which device the message is for. And perhaps add "buggy hypervisor"
> to the message to make clear that it's not a guest problem.
Ok.
> I also like the suggestion to use a dependency array.
>
Yes, will do it in next version.
^ permalink raw reply
* Re: [PATCH 1/2] virito: introduce methods of fixing device features
From: Jason Wang @ 2014-11-13 9:11 UTC (permalink / raw)
To: Cornelia Huck; +Cc: netdev, virtualization, linux-kernel, mst
In-Reply-To: <20141113094604.1f248c3d.cornelia.huck@de.ibm.com>
On 11/13/2014 04:46 PM, Cornelia Huck wrote:
> On Thu, 13 Nov 2014 13:52:53 +0800
> Jason Wang <jasowang@redhat.com> wrote:
>
> typo in subject-prefix: s/virito/virtio/
>
Will correct this.
>> Buggy host may advertised buggy host features (a usual case is that host
>> advertise a feature whose dependencies were missed). In this case, driver
>> should detect and disable the buggy features by itself.
>>
>> This patch introduces driver specific fix_features() method which is called
>> just before features finalizing to detect and disable buggy features
>> advertised by host.
> So the basic problem this patch fixes is that an individual driver may
> only specify a static set of features but cannot specify any
> dependencies, right?
Right, and what even worse is qemu could not handle dependencies as
well. So we need fix both sides.
> Adding a sanitizer step makes sense, I guess.
>
>> Virtio-net will be the first user.
>>
>> Cc: Rusty Russell <rusty@rustcorp.com.au>
>> Cc: Michael S. Tsirkin <mst@redhat.com>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
>> drivers/virtio/virtio.c | 4 ++++
>> include/linux/virtio.h | 1 +
>> include/linux/virtio_config.h | 12 ++++++++++++
>> 3 files changed, 17 insertions(+)
>>
>> diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
>> index df598dd..7001d6e 100644
>> --- a/drivers/virtio/virtio.c
>> +++ b/drivers/virtio/virtio.c
>> @@ -181,6 +181,10 @@ static int virtio_dev_probe(struct device *_d)
>> if (device_features & (1 << i))
>> set_bit(i, dev->features);
>>
>> + /* Fix buggy features advertised by host */
>> + if (drv->fix_features)
>> + drv->fix_features(dev);
> I'd probably call this "sanitize_features" instead.
Ok.
>> +
>> dev->config->finalize_features(dev);
>>
>> err = drv->probe(dev);
>> diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
>> index 7f4ef66..7bd89ea 100644
>> --- a/include/linux/virtio_config.h
>> +++ b/include/linux/virtio_config.h
>> @@ -96,6 +96,18 @@ static inline bool virtio_has_feature(const struct virtio_device *vdev,
>> return test_bit(fbit, vdev->features);
>> }
>>
>> +static inline void virtio_disable_feature(struct virtio_device *vdev,
>> + unsigned int fbit)
>> +{
>> + BUG_ON(fbit >= VIRTIO_TRANSPORT_F_START);
>> + BUG_ON(vdev->config->get_status(vdev) &
>> + ~(VIRTIO_CONFIG_S_ACKNOWLEDGE | VIRTIO_CONFIG_S_DRIVER));
> When we add virtio-1 support, we can add a check for FEATURES_OK here,
> so we're really on the safe side.
>
If I read the spec correctly, FEATURES_OK was set only after writing the
features bits to device. But we want to sanitize the them before.
>> +
>> + virtio_check_driver_offered_feature(vdev, fbit);
>> +
>> + clear_bit(fbit, vdev->features);
>> +}
>> +
>> static inline
>> struct virtqueue *virtio_find_single_vq(struct virtio_device *vdev,
>> vq_callback_t *c, const char *n)
> The approach looks good to me.
>
Thanks for the review.
^ permalink raw reply
* [PATCH 2/2] xfrm: Use __xfrm_policy_link in xfrm_policy_insert
From: Herbert Xu @ 2014-11-13 9:09 UTC (permalink / raw)
To: Steffen Klassert, David S. Miller, netdev
In-Reply-To: <20141113090811.GA3280@gondor.apana.org.au>
For a long time we couldn't actually use __xfrm_policy_link in
xfrm_policy_insert because the latter wanted to do hashing at
a specific position.
Now that __xfrm_policy_link no longer does hashing it can now
be safely used in xfrm_policy_insert to kill some duplicate code,
finally reuniting general policies with socket policies.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---
net/xfrm/xfrm_policy.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 3e015ac..2205278 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -55,6 +55,7 @@ static int stale_bundle(struct dst_entry *dst);
static int xfrm_bundle_ok(struct xfrm_dst *xdst);
static void xfrm_policy_queue_process(unsigned long arg);
+static void __xfrm_policy_link(struct xfrm_policy *pol, int dir);
static struct xfrm_policy *__xfrm_policy_unlink(struct xfrm_policy *pol,
int dir);
@@ -779,8 +780,7 @@ int xfrm_policy_insert(int dir, struct xfrm_policy *policy, int excl)
hlist_add_behind(&policy->bydst, newpos);
else
hlist_add_head(&policy->bydst, chain);
- xfrm_pol_hold(policy);
- net->xfrm.policy_count[dir]++;
+ __xfrm_policy_link(policy, dir);
atomic_inc(&net->xfrm.flow_cache_genid);
/* After previous checking, family can either be AF_INET or AF_INET6 */
@@ -799,7 +799,6 @@ int xfrm_policy_insert(int dir, struct xfrm_policy *policy, int excl)
policy->curlft.use_time = 0;
if (!mod_timer(&policy->timer, jiffies + HZ))
xfrm_pol_hold(policy);
- list_add(&policy->walk.all, &net->xfrm.policy_all);
write_unlock_bh(&net->xfrm.xfrm_policy_lock);
if (delpol)
^ permalink raw reply related
* [PATCH 1/2] xfrm: Do not hash socket policies
From: Herbert Xu @ 2014-11-13 9:09 UTC (permalink / raw)
To: Steffen Klassert, David S. Miller, netdev
In-Reply-To: <20141113090811.GA3280@gondor.apana.org.au>
Back in 2003 when I added policy expiration, I half-heartedly
did a clean-up and renamed xfrm_sk_policy_link/xfrm_sk_policy_unlink
to __xfrm_policy_link/__xfrm_policy_unlink, because the latter
could be reused for all policies. I never actually got around
to using __xfrm_policy_link for non-socket policies.
Later on hashing was added to all xfrm policies, including socket
policies. In fact, we don't need hashing on socket policies at
all since they're always looked up via a linked list.
This patch restores xfrm_sk_policy_link/xfrm_sk_policy_unlink
as wrappers around __xfrm_policy_link/__xfrm_policy_unlink so
that it's obvious we're dealing with socket policies.
This patch also removes hashing from __xfrm_policy_link as for
now it's only used by socket policies which do not need to be
hashed. Ironically this will in fact allow us to use this helper
for non-socket policies which I shall do later.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---
include/net/netns/xfrm.h | 4 ++--
net/xfrm/xfrm_policy.c | 44 ++++++++++++++++++++++++++------------------
2 files changed, 28 insertions(+), 20 deletions(-)
diff --git a/include/net/netns/xfrm.h b/include/net/netns/xfrm.h
index 9da7982..730d82a 100644
--- a/include/net/netns/xfrm.h
+++ b/include/net/netns/xfrm.h
@@ -50,8 +50,8 @@ struct netns_xfrm {
struct list_head policy_all;
struct hlist_head *policy_byidx;
unsigned int policy_idx_hmask;
- struct hlist_head policy_inexact[XFRM_POLICY_MAX * 2];
- struct xfrm_policy_hash policy_bydst[XFRM_POLICY_MAX * 2];
+ struct hlist_head policy_inexact[XFRM_POLICY_MAX];
+ struct xfrm_policy_hash policy_bydst[XFRM_POLICY_MAX];
unsigned int policy_count[XFRM_POLICY_MAX * 2];
struct work_struct policy_hash_work;
struct xfrm_policy_hthresh policy_hthresh;
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 88bf289..3e015ac 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -561,7 +561,7 @@ static void xfrm_hash_resize(struct work_struct *work)
mutex_lock(&hash_resize_mutex);
total = 0;
- for (dir = 0; dir < XFRM_POLICY_MAX * 2; dir++) {
+ for (dir = 0; dir < XFRM_POLICY_MAX; dir++) {
if (xfrm_bydst_should_resize(net, dir, &total))
xfrm_bydst_resize(net, dir);
}
@@ -601,7 +601,7 @@ static void xfrm_hash_rebuild(struct work_struct *work)
write_lock_bh(&net->xfrm.xfrm_policy_lock);
/* reset the bydst and inexact table in all directions */
- for (dir = 0; dir < XFRM_POLICY_MAX * 2; dir++) {
+ for (dir = 0; dir < XFRM_POLICY_MAX; dir++) {
INIT_HLIST_HEAD(&net->xfrm.policy_inexact[dir]);
hmask = net->xfrm.policy_bydst[dir].hmask;
odst = net->xfrm.policy_bydst[dir].table;
@@ -1247,17 +1247,10 @@ out:
static void __xfrm_policy_link(struct xfrm_policy *pol, int dir)
{
struct net *net = xp_net(pol);
- struct hlist_head *chain = policy_hash_bysel(net, &pol->selector,
- pol->family, dir);
list_add(&pol->walk.all, &net->xfrm.policy_all);
- hlist_add_head(&pol->bydst, chain);
- hlist_add_head(&pol->byidx, net->xfrm.policy_byidx+idx_hash(net, pol->index));
net->xfrm.policy_count[dir]++;
xfrm_pol_hold(pol);
-
- if (xfrm_bydst_should_resize(net, dir, NULL))
- schedule_work(&net->xfrm.policy_hash_work);
}
static struct xfrm_policy *__xfrm_policy_unlink(struct xfrm_policy *pol,
@@ -1265,17 +1258,31 @@ static struct xfrm_policy *__xfrm_policy_unlink(struct xfrm_policy *pol,
{
struct net *net = xp_net(pol);
- if (hlist_unhashed(&pol->bydst))
+ if (list_empty(&pol->walk.all))
return NULL;
- hlist_del_init(&pol->bydst);
- hlist_del(&pol->byidx);
- list_del(&pol->walk.all);
+ /* Socket policies are not hashed. */
+ if (!hlist_unhashed(&pol->bydst)) {
+ hlist_del(&pol->bydst);
+ hlist_del(&pol->byidx);
+ }
+
+ list_del_init(&pol->walk.all);
net->xfrm.policy_count[dir]--;
return pol;
}
+static void xfrm_sk_policy_link(struct xfrm_policy *pol, int dir)
+{
+ __xfrm_policy_link(pol, XFRM_POLICY_MAX + dir);
+}
+
+static void xfrm_sk_policy_unlink(struct xfrm_policy *pol, int dir)
+{
+ __xfrm_policy_unlink(pol, XFRM_POLICY_MAX + dir);
+}
+
int xfrm_policy_delete(struct xfrm_policy *pol, int dir)
{
struct net *net = xp_net(pol);
@@ -1307,7 +1314,7 @@ int xfrm_sk_policy_insert(struct sock *sk, int dir, struct xfrm_policy *pol)
if (pol) {
pol->curlft.add_time = get_seconds();
pol->index = xfrm_gen_index(net, XFRM_POLICY_MAX+dir, 0);
- __xfrm_policy_link(pol, XFRM_POLICY_MAX+dir);
+ xfrm_sk_policy_link(pol, dir);
}
if (old_pol) {
if (pol)
@@ -1316,7 +1323,7 @@ int xfrm_sk_policy_insert(struct sock *sk, int dir, struct xfrm_policy *pol)
/* Unlinking succeeds always. This is the only function
* allowed to delete or replace socket policy.
*/
- __xfrm_policy_unlink(old_pol, XFRM_POLICY_MAX+dir);
+ xfrm_sk_policy_unlink(old_pol, dir);
}
write_unlock_bh(&net->xfrm.xfrm_policy_lock);
@@ -1349,7 +1356,7 @@ static struct xfrm_policy *clone_policy(const struct xfrm_policy *old, int dir)
memcpy(newp->xfrm_vec, old->xfrm_vec,
newp->xfrm_nr*sizeof(struct xfrm_tmpl));
write_lock_bh(&net->xfrm.xfrm_policy_lock);
- __xfrm_policy_link(newp, XFRM_POLICY_MAX+dir);
+ xfrm_sk_policy_link(newp, dir);
write_unlock_bh(&net->xfrm.xfrm_policy_lock);
xfrm_pol_put(newp);
}
@@ -2966,10 +2973,11 @@ static int __net_init xfrm_policy_init(struct net *net)
goto out_byidx;
net->xfrm.policy_idx_hmask = hmask;
- for (dir = 0; dir < XFRM_POLICY_MAX * 2; dir++) {
+ for (dir = 0; dir < XFRM_POLICY_MAX; dir++) {
struct xfrm_policy_hash *htab;
net->xfrm.policy_count[dir] = 0;
+ net->xfrm.policy_count[XFRM_POLICY_MAX + dir] = 0;
INIT_HLIST_HEAD(&net->xfrm.policy_inexact[dir]);
htab = &net->xfrm.policy_bydst[dir];
@@ -3021,7 +3029,7 @@ static void xfrm_policy_fini(struct net *net)
WARN_ON(!list_empty(&net->xfrm.policy_all));
- for (dir = 0; dir < XFRM_POLICY_MAX * 2; dir++) {
+ for (dir = 0; dir < XFRM_POLICY_MAX; dir++) {
struct xfrm_policy_hash *htab;
WARN_ON(!hlist_empty(&net->xfrm.policy_inexact[dir]));
^ permalink raw reply related
* Re: [PATCH 0/2] xfrm: Do not hash socket policies
From: Herbert Xu @ 2014-11-13 9:08 UTC (permalink / raw)
To: Steffen Klassert; +Cc: David S. Miller, netdev
In-Reply-To: <20141113090048.GA3120@gondor.apana.org.au>
On Thu, Nov 13, 2014 at 05:00:48PM +0800, Herbert Xu wrote:
> Hi Steffen:
>
> I'm working on converting the xfrm policy hashing over to RCU
> due to performance complaints. While doing this I noticed that
> we're needlessly hashing socket policies.
>
> Here are two patches to get rid of that and slightly clean things
> up.
Oops that didn't work very well as I left out some rather important
bits :)
Here is a second revision.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ 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