* [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 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
* 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
* 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
* 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: [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
* 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: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 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: 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] 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 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: Appana Durga Kedareswara Rao @ 2014-11-13 10:58 UTC (permalink / raw)
To: Marc Kleine-Budde, wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org,
Michal Simek, Soren Brinkmann,
grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
Cc: linux-can-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <54648BF4.60007-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Hi Marc,
-----Original Message-----
From: Marc Kleine-Budde [mailto:mkl@pengutronix.de]
Sent: Thursday, November 13, 2014 4:16 PM
To: Appana Durga Kedareswara Rao; wg@grandegger.com; Michal Simek; Soren Brinkmann; grant.likely@linaro.org; robh+dt@kernel.org
Cc: linux-can@vger.kernel.org; netdev@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; devicetree@vger.kernel.org; Appana Durga Kedareswara Rao
Subject: Re: [PATCH] can: Fix bug in suspend/resume
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
Thanks for the suggestion.
Will use this macro(to_platform_device) .
>
>> 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.
>
The clocks are getting disabled and un prepared at the end of the probe.
In the resume the driver is doing register write.
In order to do that register write I have to again enable and prepare the clocks.
Regards,
Kedar.
>> +
>> 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 |
This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
^ permalink raw reply
* Re: [PATCH v4 3/8] net: can: c_can: Add RAMINIT register information to driver data
From: Marc Kleine-Budde @ 2014-11-13 10:59 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-4-git-send-email-rogerq@ti.com>
[-- Attachment #1: Type: text/plain, Size: 1724 bytes --]
On 11/07/2014 03:49 PM, Roger Quadros wrote:
> Some platforms (e.g. TI) need special RAMINIT register handling.
> Provide a way to store RAMINIT register description in driver data.
>
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
> drivers/net/can/c_can/c_can.h | 6 ++++++
> drivers/net/can/c_can/c_can_platform.c | 1 +
> 2 files changed, 7 insertions(+)
>
> diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
> index 26c975d..3c305a1 100644
> --- a/drivers/net/can/c_can/c_can.h
> +++ b/drivers/net/can/c_can/c_can.h
> @@ -171,6 +171,12 @@ enum c_can_dev_id {
>
> struct c_can_driver_data {
> enum c_can_dev_id id;
> +
> + /* RAMINIT register description. Optional. */
> + u8 num_can; /* Number of CAN instances on the SoC */
> + u8 *raminit_start_bits; /* Array of START bit positions */
> + u8 *raminit_done_bits; /* Array of DONE bit positions */
> + bool raminit_pulse; /* If set, sets and clears START bit (pulse) */
> };
>
> /* c_can private data structure */
> diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
> index 1546c2b..20deb67 100644
> --- a/drivers/net/can/c_can/c_can_platform.c
> +++ b/drivers/net/can/c_can/c_can_platform.c
> @@ -250,6 +250,7 @@ static int c_can_plat_probe(struct platform_device *pdev)
> }
>
> priv = netdev_priv(dev);
> +
Dropped this hunk while applying.
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: Appana Durga Kedareswara Rao @ 2014-11-13 11:01 UTC (permalink / raw)
To: Marc Kleine-Budde, wg@grandegger.com, Michal Simek,
Soren Brinkmann, grant.likely@linaro.org, robh+dt@kernel.org
Cc: linux-can@vger.kernel.org, netdev@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
In-Reply-To: <54648BF4.60007@pengutronix.de>
Hi Marc,
-----Original Message-----
From: Appana Durga Kedareswara Rao
Sent: Thursday, November 13, 2014 4:28 PM
To: 'Marc Kleine-Budde'; wg@grandegger.com; Michal Simek; Soren Brinkmann; grant.likely@linaro.org; robh+dt@kernel.org
Cc: linux-can@vger.kernel.org; netdev@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; devicetree@vger.kernel.org
Subject: RE: [PATCH] can: Fix bug in suspend/resume
Hi Marc,
-----Original Message-----
From: Marc Kleine-Budde [mailto:mkl@pengutronix.de]
Sent: Thursday, November 13, 2014 4:16 PM
To: Appana Durga Kedareswara Rao; wg@grandegger.com; Michal Simek; Soren Brinkmann; grant.likely@linaro.org; robh+dt@kernel.org
Cc: linux-can@vger.kernel.org; netdev@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; devicetree@vger.kernel.org; Appana Durga Kedareswara Rao
Subject: Re: [PATCH] can: Fix bug in suspend/resume
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
Thanks for the suggestion.
Will use this macro(to_platform_device) .
>
>> 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.
>
The clocks are getting disabled and un prepared at the end of the probe.
In the resume the driver is doing register write.
Sorry spell mistake not in resume in the suspend I am doing a register write.
In order to do that register write I have to again enable and prepare the clocks.
Regards,
Kedar.
Regards,
Kedar.
>> +
>> 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 |
This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
^ permalink raw reply
* Re: [PATCH v4 4/8] net: can: c_can: Add syscon/regmap RAMINIT mechanism
From: Marc Kleine-Budde @ 2014-11-13 11:09 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-5-git-send-email-rogerq@ti.com>
[-- Attachment #1: Type: text/plain, Size: 940 bytes --]
On 11/07/2014 03:49 PM, Roger Quadros wrote:
> Some TI SoCs like DRA7 have a RAMINIT register specification
> different from the other AMxx SoCs and as expected by the
> existing driver.
>
> To add more insanity, this register is shared with other
> IPs like DSS, PCIe and PWM.
>
> Provides a more generic mechanism to specify the RAMINIT
> register location and START/DONE bit position and use the
> syscon/regmap framework to access the register.
What about the existing device trees that don't have the syscon-raminit
phandle? We can either keep the existing init routines or create regmap
in the platform driver an use the new ones.
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: [ethtool][PATCH] Fix build with musl by using more common typedefs
From: Paul Barker @ 2014-11-13 11:22 UTC (permalink / raw)
To: Ben Hutchings, netdev; +Cc: Paul Barker, John Spencer
In-Reply-To: <1414323849-5739-2-git-send-email-paul@paulbarker.me.uk>
On 26 October 2014 11:44, Paul Barker <paul@paulbarker.me.uk> wrote:
> When using musl as the standard C library, type names such as '__int32_t' are
> not defined. Instead we must use the more commonly defined type names such as
> 'int32_t', which are defined in <stdint.h>.
>
> Signed-off-by: John Spencer <maillist-linux@barfooze.de>
> Signed-off-by: Paul Barker <paul@paulbarker.me.uk>
> ---
> internal.h | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/internal.h b/internal.h
> index a9dfae0..262a39f 100644
> --- a/internal.h
> +++ b/internal.h
> @@ -7,6 +7,7 @@
> #include "ethtool-config.h"
> #endif
> #include <stdio.h>
> +#include <stdint.h>
> #include <stdlib.h>
> #include <string.h>
> #include <sys/types.h>
> @@ -17,16 +18,16 @@
>
> /* ethtool.h expects these to be defined by <linux/types.h> */
> #ifndef HAVE_BE_TYPES
> -typedef __uint16_t __be16;
> -typedef __uint32_t __be32;
> +typedef uint16_t __be16;
> +typedef uint32_t __be32;
> typedef unsigned long long __be64;
> #endif
>
> typedef unsigned long long u64;
> -typedef __uint32_t u32;
> -typedef __uint16_t u16;
> -typedef __uint8_t u8;
> -typedef __int32_t s32;
> +typedef uint32_t u32;
> +typedef uint16_t u16;
> +typedef uint8_t u8;
> +typedef int32_t s32;
>
> #include "ethtool-copy.h"
> #include "net_tstamp-copy.h"
> --
> 2.1.2
>
Ping. Could you let me know if this patch is acceptable or not?
Cheers,
--
Paul Barker
Email: paul@paulbarker.me.uk
http://www.paulbarker.me.uk
^ permalink raw reply
* Re: [PATCH] can: Fix bug in suspend/resume
From: Lothar Waßmann @ 2014-11-13 11:32 UTC (permalink / raw)
To: Appana Durga Kedareswara Rao
Cc: Marc Kleine-Budde, wg@grandegger.com, Michal Simek,
Soren Brinkmann, grant.likely@linaro.org, robh+dt@kernel.org,
netdev@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, linux-can@vger.kernel.org
In-Reply-To: <9455da2b86c74d5aafe9bf118edec7bb@BN1AFFO11FD045.protection.gbl>
Hi,
Appana Durga Kedareswara Rao wrote:
> Hi Marc,
>
> -----Original Message-----
> From: Marc Kleine-Budde [mailto:mkl@pengutronix.de]
> Sent: Thursday, November 13, 2014 4:16 PM
> To: Appana Durga Kedareswara Rao; wg@grandegger.com; Michal Simek; Soren Brinkmann; grant.likely@linaro.org; robh+dt@kernel.org
> Cc: linux-can@vger.kernel.org; netdev@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; devicetree@vger.kernel.org; Appana Durga Kedareswara Rao
> Subject: Re: [PATCH] can: Fix bug in suspend/resume
>
> 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
>
> Thanks for the suggestion.
> Will use this macro(to_platform_device) .
>
> >
> >> 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);
>
Why not simply:
struct net_device *ndev = dev_get_drvdata(dev);
There is no need for a struct platform_device* at all.
Lothar Waßmann
--
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
www.karo-electronics.de | info@karo-electronics.de
___________________________________________________________
^ permalink raw reply
* Re: arm64 allmodconfig failures in nft_reject_bridge.c
From: Mark Brown @ 2014-11-13 11:33 UTC (permalink / raw)
To: David S. Miller, Guenter Roeck, Pablo Neira Ayuso,
Patrick McHardy, Jozsef Kadlecsik, Stephen Hemminger
Cc: linaro-kernel, kernel-build-reports, netfilter-devel, coreteam,
bridge, netdev
In-Reply-To: <20141112233738.GB3815@sirena.org.uk>
[-- Attachment #1: Type: text/plain, Size: 541 bytes --]
Since about -rc3 we've been seeing build failures in Linus' tree on
arm64 allmodconfig due to:
> arm64-allmodconfig
> ../net/bridge/netfilter/nft_reject_bridge.c:240:3: error: implicit declaration of function 'csum_ipv6_magic' [-Werror=implicit-function-declaration]
By the time I looked into this it was fixed in -next by c1207c049b204b0
(netfilter: nft_reject_bridge: Fix powerpc build error) from Guenter but
that doesn't seem to have made it into -rc4 so I just wanted to check
that this fix was intended to go to Linus before v3.18?
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* Re: [GIT PULL nf] Second Round of IPVS Fixes for v3.18
From: Pablo Neira Ayuso @ 2014-11-13 11:38 UTC (permalink / raw)
To: Simon Horman
Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
Julian Anastasov
In-Reply-To: <1415758920-32578-1-git-send-email-horms@verge.net.au>
On Wed, Nov 12, 2014 at 11:21:59AM +0900, Simon Horman wrote:
> Hi Pablo,
>
> please consider this fix for v3.18.
>
> It fixes handling of skb->sk which may cause incorrect handling
> of connections from a local process.
>
> This problem was introduced in its current form by 8052ba292559f907e
> ("ipvs: support ipv4 in ipv6 and ipv6 in ipv4 tunnel forwarding") in
> v3.18-rc1.
Pulled, thanks Simon.
> I believe it also exists in a different form in older kernels.
> No fix for that is available at this time.
AFAIK -stable also accepts backports if there's a clear relation
between this original patch in mainstream and the backported version.
^ permalink raw reply
* Re: arm64 allmodconfig failures in nft_reject_bridge.c
From: Pablo Neira Ayuso @ 2014-11-13 11:43 UTC (permalink / raw)
To: Mark Brown
Cc: linaro-kernel, kernel-build-reports, netdev, bridge,
Patrick McHardy, Stephen Hemminger, coreteam, netfilter-devel,
Jozsef Kadlecsik, David S. Miller, Guenter Roeck
In-Reply-To: <20141113113320.GF3815@sirena.org.uk>
On Thu, Nov 13, 2014 at 11:33:20AM +0000, Mark Brown wrote:
> Since about -rc3 we've been seeing build failures in Linus' tree on
> arm64 allmodconfig due to:
>
> > arm64-allmodconfig
> > ../net/bridge/netfilter/nft_reject_bridge.c:240:3: error: implicit declaration of function 'csum_ipv6_magic' [-Werror=implicit-function-declaration]
>
> By the time I looked into this it was fixed in -next by c1207c049b204b0
> (netfilter: nft_reject_bridge: Fix powerpc build error) from Guenter but
> that doesn't seem to have made it into -rc4 so I just wanted to check
> that this fix was intended to go to Linus before v3.18?
I can see this in David's tree:
http://git.kernel.org/cgit/linux/kernel/git/davem/net.git/commit/?id=c1207c049b204b0a96535dc5416aee331b51e0e1
I think this was reported by when -rc4 was already out, so you'll see
this by -rc5.
^ permalink raw reply
* [PATCH] FOU: Fix no return statement warning for !CONFIG_NET_FOU_IP_TUNNELS
From: Thomas Graf @ 2014-11-13 11:48 UTC (permalink / raw)
To: netdev; +Cc: therbert
net/ipv4/fou.c: In function ‘ip_tunnel_encap_del_fou_ops’:
net/ipv4/fou.c:861:1: warning: no return statement in function returning non-void [-Wreturn-type]
Fixes: a8c5f90fb5 ("ip_tunnel: Ops registration for secondary encap (fou, gue)")
Signed-off-by: Thomas Graf <tgraf@suug.ch>
---
net/ipv4/fou.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv4/fou.c b/net/ipv4/fou.c
index fe09077..b0b436b 100644
--- a/net/ipv4/fou.c
+++ b/net/ipv4/fou.c
@@ -856,7 +856,7 @@ static int ip_tunnel_encap_add_fou_ops(void)
return 0;
}
-static int ip_tunnel_encap_del_fou_ops(void)
+static void ip_tunnel_encap_del_fou_ops(void)
{
}
--
1.9.3
^ permalink raw reply related
* Re: [PATCH] can: Fix bug in suspend/resume
From: Marc Kleine-Budde @ 2014-11-13 12:03 UTC (permalink / raw)
To: Lothar Waßmann, Appana Durga Kedareswara Rao
Cc: wg@grandegger.com, Michal Simek, Soren Brinkmann,
grant.likely@linaro.org, robh+dt@kernel.org,
netdev@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, linux-can@vger.kernel.org
In-Reply-To: <20141113123218.79850236@ipc1.ka-ro>
[-- Attachment #1: Type: text/plain, Size: 847 bytes --]
On 11/13/2014 12:32 PM, Lothar Waßmann wrote:
>>>> 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);
>>
> Why not simply:
> struct net_device *ndev = dev_get_drvdata(dev);
>
> There is no need for a struct platform_device* at all.
ACK
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 nf] netfilter: conntrack: fix race in __nf_conntrack_confirm against get_next_corpse
From: Pablo Neira Ayuso @ 2014-11-13 12:08 UTC (permalink / raw)
To: Jörg Marx
Cc: Jesper Dangaard Brouer, programme110, netfilter-devel,
Florian Westphal, netdev, Patrick McHardy
In-Reply-To: <54633D1B.5090404@secunet.com>
On Wed, Nov 12, 2014 at 11:57:31AM +0100, Jörg Marx wrote:
> On 12.11.2014 08:35, Jesper Dangaard Brouer wrote:
>
> Hi,
>
> I wrote the patch in 2010, so find some arguments below:
>
> >>> > > + nf_ct_del_from_dying_or_unconfirmed_list(ct);
> >>> > >
> >>> > > if (unlikely(nf_ct_is_dying(ct))) {
> >>> > > + nf_ct_add_to_dying_list(ct);
> >>> > > nf_conntrack_double_unlock(hash, reply_hash);
> >>> > > local_bh_enable();
> >>> > > return NF_ACCEPT;
> >> >
> >> > Not directly related to your patch, but I don't find a good reason why
> >> > we're accepting this packet.
> >> >
> >> > If the conntrack from the unconfirmed list is dying, then the object
> >> > will be released by when the packet leaves the stack to its
> >> > destination. With stateful filtering depending in place, the follow up
> >> > packet in the reply direction will likely be considered invalid (if
> >> > tcp tracking is on). Fortunately for us, the origin will likely
> >> > retransmit the syn again, so the ct will be setup accordingly.
> >> >
> >> > So, why should we allow this to go through?
> > True, it also seems strange to me that we accept this packet.
>
> The raise was triggered in a scenario when we tested high-load IPsec
> tunnels and flushed the conntrack hashs from userspace.
>
> For me there is little difference in choosing DROP or ACCEPT as verdict.
> The packet/skb belongs to a formerly allowed connection, most likely
> this connection is still allowed (but the conntrack hash entry is about
> to be removed due to userspace is flushing the conntrack table).
__nf_conntrack_confirm() is only called for the first packet that we
see in a flow. If you just invoked the flush command once (which
should be the common case), then this is likely to be the first packet
of the flow (unless you already called flush anytime soon in the
past).
> To minimize the impact (lost packets -> retransmit) I decided to allow
> the skb in flight, so were is no lost packet at this place.
I understand your original motivation was to be conservative.
> When the connection is not allowed anymore (but was allowed up to now,
> because the hash entry exists), the impact is one last packet 'slipping
> through'.
The general policy in conntrack is to not drop packets, but in this
case we'll leave things in inconsistent state (ie. we will likely
receive a reply packet in response to the original packet that has no
conntrack yet).
Thanks.
^ permalink raw reply
* Re: [PATCH net-next 1/1] ipvlan: Initial check-in of the IPVLAN driver.
From: Pavel Emelyanov @ 2014-11-13 11:07 UTC (permalink / raw)
To: Mahesh Bandewar
Cc: netdev, Eric Dumazet, Maciej Zenczykowski, Laurent Chavey,
Tim Hockin, David Miller, Brandon Philips
In-Reply-To: <CAF2d9jgRFZzjtEkwVqo5Jw1rzbAS_9NC8LiOtO6xHgWqkVM2Zg@mail.gmail.com>
>>> +static int ipvlan_link_new(struct net *src_net, struct net_device *dev,
>>> + struct nlattr *tb[], struct nlattr *data[])
>>> +{
>>> + struct ipvl_dev *ipvlan = netdev_priv(dev);
>>> + struct ipvl_port *port;
>>> + struct net_device *phy_dev;
>>> + int err;
>>> +
>>> + ipvlan_dbg(3, "%s[%d]: Entering...\n", __func__, __LINE__);
>>> + if (!tb[IFLA_LINK]) {
>>> + ipvlan_dbg(3, "%s[%d]: Returning -EINVAL...\n",
>>> + __func__, __LINE__);
>>> + return -EINVAL;
>>> + }
>>> +
>>> + phy_dev = __dev_get_by_index(src_net, nla_get_u32(tb[IFLA_LINK]));
>>> + if (phy_dev == NULL) {
>>> + ipvlan_dbg(3, "%s[%d]: Returning -ENODEV...\n",
>>> + __func__, __LINE__);
>>> + return -ENODEV;
>>> + }
>>> +
>>> + /* TODO will someone try creating ipvlan-dev on an ipvlan-virtual dev?*/
>>> + if (!ipvlan_dev_master(phy_dev)) {
>>> + err = ipvlan_port_create(phy_dev);
>>> + if (err < 0) {
>>> + ipvlan_dbg(3, "%s[%d]: Returning error (%d)...\n",
>>> + __func__, __LINE__, err);
>>> + return err;
>>> + }
>>> + }
>>> +
>>> + port = ipvlan_port_get_rtnl(phy_dev);
>>> + /* Get the mode if specified. */
>>> + if (data && data[IFLA_IPVLAN_MODE])
>>> + port->mode = nla_get_u16(data[IFLA_IPVLAN_MODE]);
>>
>> Should the invalid value be checked here? There are places
>> where we BUG() in mode being "unknown".
>>
> Assuming the calls come over netlink, the ".validate" will be called
> before ".newlink", so that would be unnecessary, isn't it?
Yes, you're right. I've missed the validate callback.
>>> + break;
>>> +
>>
>>> +static int ipvlan_addr4_event(struct notifier_block *unused,
>>> + unsigned long event, void *ptr)
>>> +{
>>> + struct in_ifaddr *if4 = (struct in_ifaddr *)ptr;
>>> + struct net_device *dev = (struct net_device *)if4->ifa_dev->dev;
>>> + struct ipvl_dev *ipvlan = netdev_priv(dev);
>>> + struct in_addr ip4_addr;
>>> +
>>> + ipvlan_dbg(3, "%s[%d]: Entering...\n", __func__, __LINE__);
>>> + if (!ipvlan_dev_slave(dev))
>>> + return NOTIFY_DONE;
>>> +
>>> + if (!ipvlan || !ipvlan->port)
>>> + return NOTIFY_DONE;
>>> +
>>> + switch (event) {
>>> + case NETDEV_UP:
>>
>> Can it be (in the future) somehow restricted so that net-namespace wouldn't
>> be able to assign arbitrary IP address here? One of the reasons for using
>> such devices is to enforce the container to use the IP address given from
>> the host.
>>
> Probably this could be a config (sysfs?) entry which would lockup the
> config coming from ns when set. So code could look like -
> case NETDEV_UP:
> if (!restrict_ns_config) {
> ...
> }
> break;
Maybe introduce some "lock" call for ipvlan device after which no new IP addresses
can be assigned? And the configuration would look like
1. create ipvlan
2. move to proper net namespace
3. add addresses
4. lock
?
Thanks,
Pavel
^ 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