* Re: [PATCH 1/4] slab: do ClearSlabPfmemalloc() for all pages of slab
From: JoonSoo Kim @ 2012-09-06 17:57 UTC (permalink / raw)
To: Mel Gorman
Cc: Andrew Morton, Linux-MM, Linux-Netdev, LKML, David Miller,
Chuck Lever, Pekka, "Enberg <penberg", David Rientjes,
Christoph Lameter
In-Reply-To: <1346779479-1097-2-git-send-email-mgorman@suse.de>
Add "Cc" to "Christoph Lameter" <cl@linux.com>
2012/9/5 Mel Gorman <mgorman@suse.de>:
> Right now, we call ClearSlabPfmemalloc() for first page of slab when we
> clear SlabPfmemalloc flag. This is fine for most swap-over-network use
> cases as it is expected that order-0 pages are in use. Unfortunately it
> is possible that that __ac_put_obj() checks SlabPfmemalloc on a tail page
> and while this is harmless, it is sloppy. This patch ensures that the head
> page is always used.
>
> This problem was originally identified by Joonsoo Kim.
>
> [js1304@gmail.com: Original implementation and problem identification]
> Signed-off-by: Mel Gorman <mgorman@suse.de>
> ---
> mm/slab.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/mm/slab.c b/mm/slab.c
> index 811af03..d34a903 100644
> --- a/mm/slab.c
> +++ b/mm/slab.c
> @@ -1000,7 +1000,7 @@ static void *__ac_get_obj(struct kmem_cache *cachep, struct array_cache *ac,
> l3 = cachep->nodelists[numa_mem_id()];
> if (!list_empty(&l3->slabs_free) && force_refill) {
> struct slab *slabp = virt_to_slab(objp);
> - ClearPageSlabPfmemalloc(virt_to_page(slabp->s_mem));
> + ClearPageSlabPfmemalloc(virt_to_head_page(slabp->s_mem));
> clear_obj_pfmemalloc(&objp);
> recheck_pfmemalloc_active(cachep, ac);
> return objp;
We assume that slabp->s_mem's address is always in head page, so
"virt_to_head_page" is not needed.
> @@ -1032,7 +1032,7 @@ static void *__ac_put_obj(struct kmem_cache *cachep, struct array_cache *ac,
> {
> if (unlikely(pfmemalloc_active)) {
> /* Some pfmemalloc slabs exist, check if this is one */
> - struct page *page = virt_to_page(objp);
> + struct page *page = virt_to_head_page(objp);
> if (PageSlabPfmemalloc(page))
> set_obj_pfmemalloc(&objp);
> }
> --
> 1.7.9.2
>
If we always use head page, following suggestion is more good to me.
How about you?
diff --git a/mm/slab.c b/mm/slab.c
index f8b0d53..ce70989 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -1032,7 +1032,7 @@ static void *__ac_put_obj(struct kmem_cache
*cachep, struct array_cache *ac,
{
if (unlikely(pfmemalloc_active)) {
/* Some pfmemalloc slabs exist, check if this is one */
- struct page *page = virt_to_page(objp);
+ struct page *page = virt_to_head_page(objp);
if (PageSlabPfmemalloc(page))
set_obj_pfmemalloc(&objp);
}
@@ -1921,10 +1921,9 @@ static void *kmem_getpages(struct kmem_cache
*cachep, gfp_t flags, int nodeid)
NR_SLAB_UNRECLAIMABLE, nr_pages);
for (i = 0; i < nr_pages; i++) {
__SetPageSlab(page + i);
-
- if (page->pfmemalloc)
- SetPageSlabPfmemalloc(page + i);
}
+ if (page->pfmemalloc)
+ SetPageSlabPfmemalloc(page);
if (kmemcheck_enabled && !(cachep->flags & SLAB_NOTRACK)) {
kmemcheck_alloc_shadow(page, cachep->gfporder, flags, nodeid);
@@ -1943,26 +1942,26 @@ static void *kmem_getpages(struct kmem_cache
*cachep, gfp_t flags, int nodeid)
*/
static void kmem_freepages(struct kmem_cache *cachep, void *addr)
{
- unsigned long i = (1 << cachep->gfporder);
+ int nr_pages = (1 << cachep->gfporder);
+ int i;
struct page *page = virt_to_page(addr);
- const unsigned long nr_freed = i;
kmemcheck_free_shadow(page, cachep->gfporder);
if (cachep->flags & SLAB_RECLAIM_ACCOUNT)
sub_zone_page_state(page_zone(page),
- NR_SLAB_RECLAIMABLE, nr_freed);
+ NR_SLAB_RECLAIMABLE, nr_pages);
else
sub_zone_page_state(page_zone(page),
- NR_SLAB_UNRECLAIMABLE, nr_freed);
- while (i--) {
- BUG_ON(!PageSlab(page));
- __ClearPageSlabPfmemalloc(page);
- __ClearPageSlab(page);
- page++;
+ NR_SLAB_UNRECLAIMABLE, nr_pages);
+ for (i = 0; i < nr_pages; i++) {
+ BUG_ON(!PageSlab(page + i));
+ __ClearPageSlab(page + i);
}
+ __ClearPageSlabPfmemalloc(page);
+
if (current->reclaim_state)
- current->reclaim_state->reclaimed_slab += nr_freed;
+ current->reclaim_state->reclaimed_slab += nr_pages;
free_pages((unsigned long)addr, cachep->gfporder);
}
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* Re: [PATCH 0/5] dev_<level> and dynamic_debug cleanups
From: Jason Baron @ 2012-09-06 17:53 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Jim Cromie, Kay Sievers, Joe Perches, Andrew Morton, netdev,
David S. Miller, linux-kernel
In-Reply-To: <20120906161359.GC23641@kroah.com>
On Thu, Sep 06, 2012 at 09:13:59AM -0700, Greg Kroah-Hartman wrote:
> On Thu, Aug 30, 2012 at 09:48:12PM -0600, Jim Cromie wrote:
> > On Thu, Aug 30, 2012 at 11:43 AM, Jim Cromie <jim.cromie@gmail.com> wrote:
> > > On Sun, Aug 26, 2012 at 5:25 AM, Joe Perches <joe@perches.com> wrote:
> > >> The recent commit to fix dynamic_debug was a bit unclean.
> > >> Neaten the style for dynamic_debug.
> > >> Reduce the stack use of message logging that uses netdev_printk
> > >> Add utility functions dev_printk_emit and dev_vprintk_emit for /dev/kmsg.
> > >>
> > >> Joe Perches (5):
> > >> dev_dbg/dynamic_debug: Update to use printk_emit, optimize stack
> > >> netdev_printk/dynamic_netdev_dbg: Directly call printk_emit
> > >> netdev_printk/netif_printk: Remove a superfluous logging colon
> > >> dev: Add dev_vprintk_emit and dev_printk_emit
> > >> device and dynamic_debug: Use dev_vprintk_emit and dev_printk_emit
> > >>
> > >
> > > Ive tested this on 2 builds differing only by DYNAMIC_DEBUG
> > > It works for me on x86-64
> > >
> > > However, I just booted a non-dyndbg build on x86-32, and got this.
> > >
> >
> > Ok, transient error, went away with a clean build.
> >
> > tested-by: Jim Cromie <jim.cromie@gmail.com>
>
> Jason, any ACK on these, or any of the other random dynamic debug
> patches floating around? What am I supposed to be applying here?
>
Hi Greg,
I just posted some follow up comments to Joe, so let's see where that
discussion goes.
Thanks,
-Jason
^ permalink raw reply
* Re: [PATCH 0/5] dev_<level> and dynamic_debug cleanups
From: Jason Baron @ 2012-09-06 17:51 UTC (permalink / raw)
To: Joe Perches
Cc: Andrew Morton, netdev, Greg Kroah-Hartman, David S. Miller,
Jim Cromie, Kay Sievers, linux-kernel
In-Reply-To: <cover.1345978012.git.joe@perches.com>
On Sun, Aug 26, 2012 at 04:25:25AM -0700, Joe Perches wrote:
> The recent commit to fix dynamic_debug was a bit unclean.
> Neaten the style for dynamic_debug.
> Reduce the stack use of message logging that uses netdev_printk
> Add utility functions dev_printk_emit and dev_vprintk_emit for /dev/kmsg.
>
> Joe Perches (5):
> dev_dbg/dynamic_debug: Update to use printk_emit, optimize stack
> netdev_printk/dynamic_netdev_dbg: Directly call printk_emit
> netdev_printk/netif_printk: Remove a superfluous logging colon
> dev: Add dev_vprintk_emit and dev_printk_emit
> device and dynamic_debug: Use dev_vprintk_emit and dev_printk_emit
>
Looks Good.
The one thing that is bothering me though, is that for
__dynamic_dev_dbg(), __dynamic_netdev_dbg(), we are copying much of the core
logic of __dev_printk(), __netdev_printk(), respectively. I would prefer
have this in one place. Can we add a 'prefix' argument to __dev_printk(),
and __netdev_printk() that dynamic debug can use, but is simply empty
for dev_printk() and netdev_printk().
Thanks,
-Jason
^ permalink raw reply
* Re: [PATCH 11/12] qlcnic: 83xx adpater ethtool
From: Ben Hutchings @ 2012-09-06 17:49 UTC (permalink / raw)
To: Sony Chacko; +Cc: davem, netdev, Dept_NX_Linux_NIC_Driver
In-Reply-To: <1346912529-17406-12-git-send-email-sony.chacko@qlogic.com>
On Thu, 2012-09-06 at 02:22 -0400, Sony Chacko wrote:
> From: Sony Chacko <sony.chacko@qlogic.com>
>
> 83xx ethtool interface routines
>
> Signed-off-by: Anirban Chakraborty <anirban.chakraborty@qlogic.com>
> Signed-off-by: Sucheta Chakraborty <sucheta.chakraborty@qlogic.com>
> Signed-off-by: Jitendra Kalsaria <jitendra.kalsaria@qlogic.com>
> Signed-off-by: Rajesh Borundia <rajesh.borundia@qlogic.com>
> Signed-off-by: Sritej Velaga <sritej.velaga@qlogic.com>
> Signed-off-by: Sony Chacko <sony.chacko@qlogic.com>
> ---
> .../net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c | 583 +++++++++++++-------
> 1 files changed, 374 insertions(+), 209 deletions(-)
>
> diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
> index 4625253..b0d21ac 100644
> --- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
> +++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
[...]
> @@ -146,6 +188,12 @@ static const u32 diag_registers[] = {
> QLCNIC_PEG_ALIVE_COUNTER,
> QLCNIC_PEG_HALT_STATUS1,
> QLCNIC_PEG_HALT_STATUS2,
> + -1
> +};
> +
> +static const u32 ext_diag_registers[] = {
> + CRB_XG_STATE_P3P,
> + ISR_INT_STATE_REG,
> QLCNIC_CRB_PEG_NET_0+0x3c,
> QLCNIC_CRB_PEG_NET_1+0x3c,
> QLCNIC_CRB_PEG_NET_2+0x3c,
> @@ -154,12 +202,19 @@ static const u32 diag_registers[] = {
> };
>
> #define QLCNIC_MGMT_API_VERSION 2
> -#define QLCNIC_DEV_INFO_SIZE 1
> #define QLCNIC_ETHTOOL_REGS_VER 2
I think QLCNIC_ETHTOOL_REGS_VER needs to be changed, as you appear to be
dumping more registers from the existing hardware as well.
> +
> static int qlcnic_get_regs_len(struct net_device *dev)
> {
> - return sizeof(diag_registers) + QLCNIC_RING_REGS_LEN +
> - QLCNIC_DEV_INFO_SIZE + 1;
> + struct qlcnic_adapter *adapter = netdev_priv(dev);
> + u32 len;
> +
> + if (QLCNIC_IS_83XX(adapter))
> + len = qlcnic_83xx_get_regs_len(adapter);
> + else
> + len = sizeof(ext_diag_registers) + sizeof(diag_registers);
> +
> + return QLCNIC_RING_REGS_LEN + len + QLCNIC_DEV_INFO_SIZE + 1;
> }
>
> static int qlcnic_get_eeprom_len(struct net_device *dev)
[...]
> @@ -509,25 +613,6 @@ qlcnic_set_ringparam(struct net_device *dev,
> return qlcnic_reset_context(adapter);
> }
>
> -static void qlcnic_get_channels(struct net_device *dev,
> - struct ethtool_channels *channel)
> -{
> - struct qlcnic_adapter *adapter = netdev_priv(dev);
> -
> - channel->max_rx = rounddown_pow_of_two(min_t(int,
> - adapter->ahw->max_rx_ques, num_online_cpus()));
> - channel->max_tx = adapter->ahw->max_tx_ques;
> -
> - channel->rx_count = adapter->max_sds_rings;
> - channel->tx_count = adapter->ahw->max_tx_ques;
> -}
> -
> -static int qlcnic_set_channels(struct net_device *dev,
> - struct ethtool_channels *channel)
> -{
> - return 0;
> -}
> -
> static void
> qlcnic_get_pauseparam(struct net_device *netdev,
> struct ethtool_pauseparam *pause)
[...]
> +static void qlcnic_get_channels(struct net_device *dev,
> + struct ethtool_channels *channel)
> +{
> + struct qlcnic_adapter *adapter = netdev_priv(dev);
> +
> + channel->max_rx = rounddown_pow_of_two(min_t(int,
> + adapter->ahw->max_rx_ques,
> + num_online_cpus()));
> + channel->max_tx = adapter->ahw->max_tx_ques;
> +
> + channel->rx_count = adapter->max_sds_rings;
> + channel->tx_count = adapter->ahw->max_tx_ques;
> +}
> +
> +static int qlcnic_set_channels(struct net_device *dev,
> + struct ethtool_channels *channel)
> +{
> + struct qlcnic_adapter *adapter = netdev_priv(dev);
> + int err;
> +
> + if (channel->other_count || channel->combined_count ||
> + channel->tx_count != channel->max_tx)
> + return -EINVAL;
> +
> + err = qlcnic_validate_max_rss(channel->max_rx, channel->rx_count);
> + if (err)
> + return err;
> +
> + err = qlcnic_set_max_rss(adapter, channel->rx_count, 0);
> + netdev_info(dev, "allocated 0x%x sds rings\n",
> + adapter->max_sds_rings);
> + return err;
> +}
So you removed the body of qlcnic_set_channels() in an earlier patch,
and now you're moving it and putting the body back how it was... Please
clean up the patch series so it doesn't have noise like this in it.
[...]
> +static int
> +qlcnic_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
> +{
> + int err;
> + struct qlcnic_adapter *adapter = netdev_priv(dev);
> + u32 wol_cfg;
> +
> + if (QLCNIC_IS_83XX(adapter))
> + return -EOPNOTSUPP;
> + if (wol->wolopts & ~WAKE_MAGIC)
> + return -EOPNOTSUPP;
Should be -EINVAL in the second error case (the device supports the
operation, but not the mode requested).
> + wol_cfg = QLCRD32(adapter, QLCNIC_WOL_CONFIG_NV, &err);
> + if (!(wol_cfg & (1 << adapter->portnum)))
> + return -EOPNOTSUPP;
> +
> + wol_cfg = QLCRD32(adapter, QLCNIC_WOL_CONFIG, &err);
> + if (wol->wolopts & WAKE_MAGIC)
> + wol_cfg |= 1UL << adapter->portnum;
> + else
> + wol_cfg &= ~(1UL << adapter->portnum);
> +
> + QLCWR32(adapter, QLCNIC_WOL_CONFIG, wol_cfg);
> +
> + return 0;
> }
>
> static int qlcnic_set_led(struct net_device *dev,
[...]
> @@ -1081,50 +1297,6 @@ static int qlcnic_set_led(struct net_device *dev,
> return err;
> }
>
> -static void
> -qlcnic_get_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
> -{
> - int err;
> - struct qlcnic_adapter *adapter = netdev_priv(dev);
> - u32 wol_cfg;
> -
> - wol->supported = 0;
> - wol->wolopts = 0;
> -
> - wol_cfg = QLCRD32(adapter, QLCNIC_WOL_CONFIG_NV, &err);
> - if (wol_cfg & (1UL << adapter->portnum))
> - wol->supported |= WAKE_MAGIC;
> -
> - wol_cfg = QLCRD32(adapter, QLCNIC_WOL_CONFIG, &err);
> - if (wol_cfg & (1UL << adapter->portnum))
> - wol->wolopts |= WAKE_MAGIC;
> -}
> -
> -static int
> -qlcnic_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol)
> -{
> - int err;
> - struct qlcnic_adapter *adapter = netdev_priv(dev);
> - u32 wol_cfg;
> -
> - if (wol->wolopts & ~WAKE_MAGIC)
> - return -EOPNOTSUPP;
> -
> - wol_cfg = QLCRD32(adapter, QLCNIC_WOL_CONFIG_NV, &err);
> - if (!(wol_cfg & (1 << adapter->portnum)))
> - return -EOPNOTSUPP;
> -
> - wol_cfg = QLCRD32(adapter, QLCNIC_WOL_CONFIG, &err);
> - if (wol->wolopts & WAKE_MAGIC)
> - wol_cfg |= 1UL << adapter->portnum;
> - else
> - wol_cfg &= ~(1UL << adapter->portnum);
> -
> - QLCWR32(adapter, QLCNIC_WOL_CONFIG, wol_cfg);
> -
> - return 0;
> -}
[...]
Again you've moved functions around for no obvious reason.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* [PATCH] Fix "ip -6 route add ... nexthop"
From: Vincent Bernat @ 2012-09-06 17:30 UTC (permalink / raw)
To: netdev; +Cc: Vincent Bernat
In-Reply-To: <1346952649-5716-1-git-send-email-bernat@luffy.cx>
IPv6 multipath routes were not accepted by "ip route" because an IPv4
address was expected for each gateway. Use `get_addr()` instead of
`get_addr32()`.
Signed-off-by: Vincent Bernat <bernat@luffy.cx>
---
ip/iproute.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/ip/iproute.c b/ip/iproute.c
index 522dd28..c78d4f7 100644
--- a/ip/iproute.c
+++ b/ip/iproute.c
@@ -624,16 +624,20 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
}
-int parse_one_nh(struct rtattr *rta, struct rtnexthop *rtnh, int *argcp, char ***argvp)
+int parse_one_nh(struct rtmsg *r, struct rtattr *rta, struct rtnexthop *rtnh, int *argcp, char ***argvp)
{
int argc = *argcp;
char **argv = *argvp;
while (++argv, --argc > 0) {
if (strcmp(*argv, "via") == 0) {
+ inet_prefix addr;
NEXT_ARG();
- rta_addattr32(rta, 4096, RTA_GATEWAY, get_addr32(*argv));
- rtnh->rtnh_len += sizeof(struct rtattr) + 4;
+ get_addr(&addr, *argv, r->rtm_family);
+ if (r->rtm_family == AF_UNSPEC)
+ r->rtm_family = addr.family;
+ rta_addattr_l(rta, 4096, RTA_GATEWAY, &addr.data, addr.bytelen);
+ rtnh->rtnh_len += sizeof(struct rtattr) + addr.bytelen;
} else if (strcmp(*argv, "dev") == 0) {
NEXT_ARG();
if ((rtnh->rtnh_ifindex = ll_name_to_index(*argv)) == 0) {
@@ -685,7 +689,7 @@ int parse_nexthops(struct nlmsghdr *n, struct rtmsg *r, int argc, char **argv)
memset(rtnh, 0, sizeof(*rtnh));
rtnh->rtnh_len = sizeof(*rtnh);
rta->rta_len += rtnh->rtnh_len;
- parse_one_nh(rta, rtnh, &argc, &argv);
+ parse_one_nh(r, rta, rtnh, &argc, &argv);
rtnh = RTNH_NEXT(rtnh);
}
--
1.7.10.4
^ permalink raw reply related
* IPv6 multipath routes
From: Vincent Bernat @ 2012-09-06 17:30 UTC (permalink / raw)
To: netdev
Hi!
It appears that "ip -6 route add" expects IPv4 addresses with nexthop directives:
$ ip -6 route add to 2a01:c9c0:a1:982::/64 proto bird \
nexthop via fe80::ea39:35ff:febd:f9e dev bai2.2008 weight 1 \
nexthop via fe80::ea39:35ff:febd:fd6 dev bai1.2009 weight 1
Error: an IP address is expected rather than "fe80::ea39:35ff:febd:f9e"
The following patch fix this problem. However, it does not work. I now have:
RTNETLINK answers: No such device
I have little knowledge of netlink so it is likely that my patch is
buggy. However, I have found this problem by trying to debug what
appears to be a valid netlink message refused by the kernel with the
same error. Therefore, I suspect that there is also a bug in the kernel.
^ permalink raw reply
* Re: [PATCH net-next] ipv6: Export nd_tbl to allow modules to support IPv6
From: David Miller @ 2012-09-06 17:47 UTC (permalink / raw)
To: tgraf; +Cc: netdev
In-Reply-To: <20120906085102.GA14711@canuck.infradead.org>
From: Thomas Graf <tgraf@suug.ch>
Date: Thu, 6 Sep 2012 04:51:02 -0400
> On Wed, Sep 05, 2012 at 01:06:36PM -0400, David Miller wrote:
>> So if one of our goals is to move towards a situation where all neigh
>> accesses are refcount'less, having those external users makes that
>> nearly impossible.
>>
>> Instead, I'd rather see patches that mark arp_tbl as being exported
>> only for internal usage inside of the tree, so that we can reach that
>> goal.
>>
>> I'm not applying this, sorry.
>
> Fair enough
>
> Does that mean you dismiss neighbour lookups by external users in
> general in order to get rid of the refcnt?
>
> Assuming that lookups would still be ok, would an ipv6 version of
> __ipv4_neigh_lookup() be an acceptable API for external users?
> (Yes there is __ipv6_neigh_lookup() already but unlike the ipv4 version
> it takes the table as first argument)
Right now we're in a transition period where ipv4 is mostly refcount'less
and ipv6 is not.
This is part of the reason I don't want to expose these things, the
calling convention and locking requirements are going to be fluid for
any interface you might propose.
arp_tbl was exported only for in-tree users, and I therefore say that
we should only export nd_tbl for in-tree users as well, since that's
the only way we can audit and update all the referencing callers as
the semantics radically change.
^ permalink raw reply
* Re: [PATCH 08/10] net/macb: macb_get_drvinfo: add GEM/MACB suffix to differentiate revision
From: David Miller @ 2012-09-06 17:42 UTC (permalink / raw)
To: nicolas.ferre
Cc: bhutchings, netdev, linux-arm-kernel, havard, plagnioj, jamie,
linux-kernel, patrice.vilchez
In-Reply-To: <5048ACBE.2030205@atmel.com>
From: Nicolas Ferre <nicolas.ferre@atmel.com>
Date: Thu, 6 Sep 2012 16:01:34 +0200
> Absolutely, I will do this.
Please, when you receive feedback on your patches, you need to
resubmit the whole patch series for review not just the patches where
changes were asked for.
^ permalink raw reply
* Re: [PATCH 10/12] qlcnic: register dump utility
From: Ben Hutchings @ 2012-09-06 17:38 UTC (permalink / raw)
To: Sony Chacko; +Cc: davem, netdev, Dept_NX_Linux_NIC_Driver
In-Reply-To: <1346912529-17406-11-git-send-email-sony.chacko@qlogic.com>
On Thu, 2012-09-06 at 02:22 -0400, Sony Chacko wrote:
> From: Sony Chacko <sony.chacko@qlogic.com>
>
> Modify 82xx driver to support new adapter - Qlogic 83XX CNA
> Common register dump utility for 82xx and 83xx adapters
[...]
> --- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
> +++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_ethtool.c
> @@ -525,21 +525,7 @@ static void qlcnic_get_channels(struct net_device *dev,
> static int qlcnic_set_channels(struct net_device *dev,
> struct ethtool_channels *channel)
> {
> - struct qlcnic_adapter *adapter = netdev_priv(dev);
> - int err;
> -
> - if (channel->other_count || channel->combined_count ||
> - channel->tx_count != channel->max_tx)
> - return -EINVAL;
> -
> - err = qlcnic_validate_max_rss(dev, channel->max_rx, channel->rx_count);
> - if (err)
> - return err;
> -
> - err = qlcnic_set_max_rss(adapter, channel->rx_count);
> - netdev_info(dev, "allocated 0x%x sds rings\n",
> - adapter->max_sds_rings);
> - return err;
> + return 0;
> }
[...]
No, I don't think so. If you're going to remove support for this
operation then delete the function entirely. And don't put it in a
patch that's supposed to do something unrelated.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH 06/12] qlcnic: 83xx data path and HW interfaces routines
From: Ben Hutchings @ 2012-09-06 17:33 UTC (permalink / raw)
To: Sony Chacko; +Cc: davem, netdev, Dept_NX_Linux_NIC_Driver
In-Reply-To: <1346912529-17406-7-git-send-email-sony.chacko@qlogic.com>
On Thu, 2012-09-06 at 02:22 -0400, Sony Chacko wrote:
> From: Sony Chacko <sony.chacko@qlogic.com>
>
> Modify 82xx driver to support new adapter - Qlogic 83XX CNA
> Create 83xx adapter data path and hardware interface routines
[...]
> --- /dev/null
> +++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c
[...]
> +int qlcnic_83xx_set_settings(struct qlcnic_adapter *adapter,
> + struct ethtool_cmd *ecmd)
> +{
> + int status = 0;
> + u32 config = adapter->ahw->port_config;
> +
> + if (ecmd->autoneg)
> + adapter->ahw->port_config |= BIT_15;
> +
> + switch (ethtool_cmd_speed(ecmd)) {
> + case SPEED_10:
> + adapter->ahw->port_config |= BIT_8;
> + break;
> + case SPEED_100:
> + adapter->ahw->port_config |= BIT_9;
> + break;
> + case SPEED_1000:
> + adapter->ahw->port_config |= BIT_10;
> + break;
> + case SPEED_10000:
> + adapter->ahw->port_config |= BIT_11;
> + break;
> + default:
> + return -EIO;
Should be -EINVAL.
> + }
> +
> + status = qlcnic_83xx_set_port_config(adapter);
> + if (status) {
> + dev_info(&adapter->pdev->dev,
> + "Faild to Set Link Speed and autoneg.\n");
> + adapter->ahw->port_config = config;
> + }
> + return status;
> +}
[...]
> +static u64*
> +qlcnic_83xx_fill_stats(struct qlcnic_adapter *adapter,
> + struct qlcnic_cmd_args *cmd, u64 *data, int type,
> + int *ret)
> +{
> + int err, k, total_regs;
> +
> + *ret = 0;
> + err = qlcnic_issue_cmd(adapter, cmd);
> + if (err != QLCNIC_RCODE_SUCCESS) {
> + dev_info(&adapter->pdev->dev,
> + "Error in get statistics mailbox command\n");
> + *ret = -EIO;
> + return data;
> + }
> + total_regs = cmd->rsp.num;
> + switch (type) {
[...]
> + default:
> + dev_info(&adapter->pdev->dev, "Unknown get statistics mode\n");
It seems like this default case can only be reached in case of a driver
bug, so WARN would be more appropriate.
> + *ret = -EIO;
> + }
> + return data;
> +}
[...]
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH v3 01/17] hashtable: introduce a small and naive hashtable
From: Steven Rostedt @ 2012-09-06 17:15 UTC (permalink / raw)
To: Sasha Levin
Cc: Josh Triplett, Mathieu Desnoyers, Pedro Alves, Tejun Heo,
torvalds, akpm, linux-kernel, linux-mm, paul.gortmaker, davem,
mingo, ebiederm, aarcange, ericvh, netdev, eric.dumazet, axboe,
agk, dm-devel, neilb, ccaulfie, teigland, Trond.Myklebust,
bfields, fweisbec, jesse, venkat.x.venkatsubra, ejt, snitzer,
edumazet, linux-nfs, dev, rds-devel, lw
In-Reply-To: <5048CDA2.10300@gmail.com>
On Thu, 2012-09-06 at 18:21 +0200, Sasha Levin wrote:
> On 09/06/2012 06:00 PM, Steven Rostedt wrote:
> >> > I think that that code doesn't make sense. The users of hlist_for_each_* aren't
> >> > supposed to be changing the loop cursor.
> > I totally agree. Modifying the 'node' pointer is just asking for issues.
> > Yes that is error prone, but not due to the double loop. It's due to the
> > modifying of the node pointer that is used internally by the loop
> > counter. Don't do that :-)
>
> While we're on this subject, I haven't actually seen hlist_for_each_entry() code
> that even *touches* 'pos'.
>
> Will people yell at me loudly if I change the prototype of those macros to be:
>
> hlist_for_each_entry(tpos, head, member)
>
> (Dropping the 'pos' parameter), and updating anything that calls those macros to
> drop it as well?
If 'pos' is no longer used in the macro, I don't see any reason for
keeping it around.
-- Steve
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH] sctp: check dst validity after IPsec operations
From: Vlad Yasevich @ 2012-09-06 17:03 UTC (permalink / raw)
To: nicolas.dichtel; +Cc: sri, linux-sctp, netdev
In-Reply-To: <5048D219.4020001@6wind.com>
On 09/06/2012 12:40 PM, Nicolas Dichtel wrote:
> Le 06/09/2012 18:04, Vlad Yasevich a écrit :
>> On 09/06/2012 01:40 PM, Nicolas Dichtel wrote:
>>> dst stored in struct sctp_transport needs to be recalculated when
>>> ipsec policy
>>> are updated. We use flow_cache_genid for that.
>>>
>>> For example, if a SCTP connection is established and then an IPsec
>>> policy is
>>> set, the old SCTP flow will not be updated and thus will not use the new
>>> IPsec policy.
>>>
>>> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
>>
>> why doesn't this need to be done for TCP? What makes SCTP special in
>> this case?
> Tests prove that the pb does not exist with TCP. I made the patch some
> times ago, I will look again deeply to find the difference.
>
TCP appears to cache the flowi and uses that to re-route the packet.
However, re-route still seems predicated on dst_check()...
>>
>> ip_queue_xmit does an __sk_dst_check() which is essentially what
>> sctp_transport_dst_check() does. That should determine if the
>> currently cached
>> route is valid or not.
> The problem is that route will not be invalidated, because dst->check()
> has no xfrm path so xfrm_dst_check() will never be called.
>
Shouldn't the cache be invalidated in this case? If the cache is
invalidated, that should cause a new lookup. If the cache isn't
invalidated, then any established connections that may now be impacted
by the policy will not pick it up.
-vlad
>
> Regards,
> Nicolas
>
>>
>> Looks like sctp may need to change to using ip_route_output_ports() call
>> as ip_route_output_key may not do all that is necessary
>>
>> -vlad
>>
>>> ---
>>> include/net/sctp/sctp.h | 3 ++-
>>> include/net/sctp/structs.h | 3 +++
>>> net/sctp/ipv6.c | 1 +
>>> net/sctp/protocol.c | 1 +
>>> 4 files changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
>>> index 9c6414f..3267246 100644
>>> --- a/include/net/sctp/sctp.h
>>> +++ b/include/net/sctp/sctp.h
>>> @@ -712,7 +712,8 @@ static inline void sctp_v4_map_v6(union sctp_addr
>>> *addr)
>>> */
>>> static inline struct dst_entry *sctp_transport_dst_check(struct
>>> sctp_transport *t)
>>> {
>>> - if (t->dst && !dst_check(t->dst, 0)) {
>>> + if ((t->dst && !dst_check(t->dst, 0)) ||
>>> + (t->flow_cache_genid != atomic_read(&flow_cache_genid))) {
>>> dst_release(t->dst);
>>> t->dst = NULL;
>>> }
>>> diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
>>> index 0fef00f..9dab882 100644
>>> --- a/include/net/sctp/structs.h
>>> +++ b/include/net/sctp/structs.h
>>> @@ -948,6 +948,9 @@ struct sctp_transport {
>>>
>>> /* 64-bit random number sent with heartbeat. */
>>> __u64 hb_nonce;
>>> +
>>> + /* used to check validity of dst */
>>> + __u32 flow_cache_genid;
>>> };
>>>
>>> struct sctp_transport *sctp_transport_new(struct net *, const union
>>> sctp_addr *,
>>> diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
>>> index ea14cb4..2ed7410 100644
>>> --- a/net/sctp/ipv6.c
>>> +++ b/net/sctp/ipv6.c
>>> @@ -349,6 +349,7 @@ out:
>>> struct rt6_info *rt;
>>> rt = (struct rt6_info *)dst;
>>> t->dst = dst;
>>> + t->flow_cache_genid = atomic_read(&flow_cache_genid);
>>> SCTP_DEBUG_PRINTK("rt6_dst:%pI6 rt6_src:%pI6\n",
>>> &rt->rt6i_dst.addr, &fl6->saddr);
>>> } else {
>>> diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
>>> index 2d51842..4795a1a 100644
>>> --- a/net/sctp/protocol.c
>>> +++ b/net/sctp/protocol.c
>>> @@ -512,6 +512,7 @@ out_unlock:
>>> rcu_read_unlock();
>>> out:
>>> t->dst = dst;
>>> + t->flow_cache_genid = atomic_read(&flow_cache_genid);
>>> if (dst)
>>> SCTP_DEBUG_PRINTK("rt_dst:%pI4, rt_src:%pI4\n",
>>> &fl4->daddr, &fl4->saddr);
>>>
>>
^ permalink raw reply
* Re: [PATCH v3 01/17] hashtable: introduce a small and naive hashtable
From: Mathieu Desnoyers @ 2012-09-06 17:02 UTC (permalink / raw)
To: Sasha Levin
Cc: Steven Rostedt, Josh Triplett, Pedro Alves, Tejun Heo,
torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ,
davem-fT/PcQaiUtIeIZ0/mPfg9Q, mingo-X9Un+BFzKDI,
ebiederm-aS9lmoZGLiVWk0Htik3J/w, aarcange-H+wXaHxf7aLQT0dZR+AlfA,
ericvh-Re5JQEeQqe8AvxtiuMwx3w, netdev-u79uwXL29TY76Z2rM5mHXA,
eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w, axboe-tSWWG44O7X1aa/9Udqfwiw,
agk-H+wXaHxf7aLQT0dZR+AlfA, dm-devel-H+wXaHxf7aLQT0dZR+AlfA,
neilb-l3A5Bk7waGM, ccaulfie-H+wXaHxf7aLQT0dZR+AlfA,
teigland-H+wXaHxf7aLQT0dZR+AlfA,
Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA,
bfields-uC3wQj2KruNg9hUCZPvPmw, fweisbec-Re5JQEeQqe8AvxtiuMwx3w,
jesse-l0M0P4e3n4LQT0dZR+AlfA,
venkat.x.venkatsubra-QHcLZuEGTsvQT0dZR+AlfA,
ejt-H+wXaHxf7aLQT0dZR+AlfA, snitzer-H+wXaHxf7aLQT0dZR+AlfA,
edumazet-hpIqsD4AKlfQT0dZR+AlfA, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
dev-yBygre7rU0TnMu66kgdUjQ, rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
lw-BthXqXjhjHXQFUHtdCDX3A
In-Reply-To: <5048D6DE.8090805-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
* Sasha Levin (levinsasha928-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org) wrote:
> On 09/06/2012 06:50 PM, Mathieu Desnoyers wrote:
> > * Sasha Levin (levinsasha928-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org) wrote:
> >> On 09/06/2012 06:00 PM, Steven Rostedt wrote:
> >>>>> I think that that code doesn't make sense. The users of hlist_for_each_* aren't
> >>>>> supposed to be changing the loop cursor.
> >>> I totally agree. Modifying the 'node' pointer is just asking for issues.
> >>> Yes that is error prone, but not due to the double loop. It's due to the
> >>> modifying of the node pointer that is used internally by the loop
> >>> counter. Don't do that :-)
> >>
> >> While we're on this subject, I haven't actually seen hlist_for_each_entry() code
> >> that even *touches* 'pos'.
> >>
> >> Will people yell at me loudly if I change the prototype of those macros to be:
> >>
> >> hlist_for_each_entry(tpos, head, member)
> >>
> >> (Dropping the 'pos' parameter), and updating anything that calls those macros to
> >> drop it as well?
> >
> > I think the intent there is to keep hlist macros and list macros
> > slightly in sync. Given those are vastly used, I'm not sure you want to
> > touch them. But hey, that's just my 2 cents.
>
> Actually, the corresponding list macro looks like this:
>
> list_for_each_entry(pos, head, member)
>
> With 'pos' being the equivalent of 'tpos' in the hlist macros (the type *).
> Changing hlist macro will make them both look as follows:
>
> hlist_for_each_entry(pos, head, member)
> list_for_each_entry(pos, head, member)
>
> So following this suggesting will actually bring them back to sync...
>
> The only issue I can see is that as you've said, they're used almost everywhere,
> so doing something to change that will require some coordination.
if this brings hlist and list in sync, then it looks like an
improvement. It might be good to propose this change as a separate
patchset.
Thanks,
Mathieu
>
>
> Thanks,
> Sasha
--
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 01/17] hashtable: introduce a small and naive hashtable
From: Sasha Levin @ 2012-09-06 17:01 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: snitzer-H+wXaHxf7aLQT0dZR+AlfA, neilb-l3A5Bk7waGM,
fweisbec-Re5JQEeQqe8AvxtiuMwx3w,
Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA,
bfields-uC3wQj2KruNg9hUCZPvPmw,
paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ,
dm-devel-H+wXaHxf7aLQT0dZR+AlfA, agk-H+wXaHxf7aLQT0dZR+AlfA,
aarcange-H+wXaHxf7aLQT0dZR+AlfA, rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
venkat.x.venkatsubra-QHcLZuEGTsvQT0dZR+AlfA,
ccaulfie-H+wXaHxf7aLQT0dZR+AlfA, mingo-X9Un+BFzKDI,
dev-yBygre7rU0TnMu66kgdUjQ, ericvh-Re5JQEeQqe8AvxtiuMwx3w,
Josh Triplett, Steven Rostedt, lw-BthXqXjhjHXQFUHtdCDX3A,
teigland-H+wXaHxf7aLQT0dZR+AlfA, axboe-tSWWG44O7X1aa/9Udqfwiw,
linux-nfs-u79uwXL29TY76Z2rM5mHXA, edumazet-hpIqsD4AKlfQT0dZR+AlfA,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Pedro Alves,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, ejt-H+wXaHxf7aLQT0dZR+AlfA,
ebiederm-aS9lmoZGLiVWk0Htik3J/w, netdev-u79uwXL29TY76Z2rM5mHXA,
Tejun Heo, akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <20120906165055.GC7385@Krystal>
On 09/06/2012 06:50 PM, Mathieu Desnoyers wrote:
> * Sasha Levin (levinsasha928-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org) wrote:
>> On 09/06/2012 06:00 PM, Steven Rostedt wrote:
>>>>> I think that that code doesn't make sense. The users of hlist_for_each_* aren't
>>>>> supposed to be changing the loop cursor.
>>> I totally agree. Modifying the 'node' pointer is just asking for issues.
>>> Yes that is error prone, but not due to the double loop. It's due to the
>>> modifying of the node pointer that is used internally by the loop
>>> counter. Don't do that :-)
>>
>> While we're on this subject, I haven't actually seen hlist_for_each_entry() code
>> that even *touches* 'pos'.
>>
>> Will people yell at me loudly if I change the prototype of those macros to be:
>>
>> hlist_for_each_entry(tpos, head, member)
>>
>> (Dropping the 'pos' parameter), and updating anything that calls those macros to
>> drop it as well?
>
> I think the intent there is to keep hlist macros and list macros
> slightly in sync. Given those are vastly used, I'm not sure you want to
> touch them. But hey, that's just my 2 cents.
Actually, the corresponding list macro looks like this:
list_for_each_entry(pos, head, member)
With 'pos' being the equivalent of 'tpos' in the hlist macros (the type *).
Changing hlist macro will make them both look as follows:
hlist_for_each_entry(pos, head, member)
list_for_each_entry(pos, head, member)
So following this suggesting will actually bring them back to sync...
The only issue I can see is that as you've said, they're used almost everywhere,
so doing something to change that will require some coordination.
Thanks,
Sasha
^ permalink raw reply
* Re: [PATCH v3 01/17] hashtable: introduce a small and naive hashtable
From: Mathieu Desnoyers @ 2012-09-06 16:50 UTC (permalink / raw)
To: Sasha Levin
Cc: Steven Rostedt, Josh Triplett, Pedro Alves, Tejun Heo,
torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ,
davem-fT/PcQaiUtIeIZ0/mPfg9Q, mingo-X9Un+BFzKDI,
ebiederm-aS9lmoZGLiVWk0Htik3J/w, aarcange-H+wXaHxf7aLQT0dZR+AlfA,
ericvh-Re5JQEeQqe8AvxtiuMwx3w, netdev-u79uwXL29TY76Z2rM5mHXA,
eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w, axboe-tSWWG44O7X1aa/9Udqfwiw,
agk-H+wXaHxf7aLQT0dZR+AlfA, dm-devel-H+wXaHxf7aLQT0dZR+AlfA,
neilb-l3A5Bk7waGM, ccaulfie-H+wXaHxf7aLQT0dZR+AlfA,
teigland-H+wXaHxf7aLQT0dZR+AlfA,
Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA,
bfields-uC3wQj2KruNg9hUCZPvPmw, fweisbec-Re5JQEeQqe8AvxtiuMwx3w,
jesse-l0M0P4e3n4LQT0dZR+AlfA,
venkat.x.venkatsubra-QHcLZuEGTsvQT0dZR+AlfA,
ejt-H+wXaHxf7aLQT0dZR+AlfA, snitzer-H+wXaHxf7aLQT0dZR+AlfA,
edumazet-hpIqsD4AKlfQT0dZR+AlfA, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
dev-yBygre7rU0TnMu66kgdUjQ, rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
lw-BthXqXjhjHXQFUHtdCDX3A
In-Reply-To: <5048CDA2.10300-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
* Sasha Levin (levinsasha928-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org) wrote:
> On 09/06/2012 06:00 PM, Steven Rostedt wrote:
> >> > I think that that code doesn't make sense. The users of hlist_for_each_* aren't
> >> > supposed to be changing the loop cursor.
> > I totally agree. Modifying the 'node' pointer is just asking for issues.
> > Yes that is error prone, but not due to the double loop. It's due to the
> > modifying of the node pointer that is used internally by the loop
> > counter. Don't do that :-)
>
> While we're on this subject, I haven't actually seen hlist_for_each_entry() code
> that even *touches* 'pos'.
>
> Will people yell at me loudly if I change the prototype of those macros to be:
>
> hlist_for_each_entry(tpos, head, member)
>
> (Dropping the 'pos' parameter), and updating anything that calls those macros to
> drop it as well?
I think the intent there is to keep hlist macros and list macros
slightly in sync. Given those are vastly used, I'm not sure you want to
touch them. But hey, that's just my 2 cents.
Thanks,
Mathieu
>
>
> Thanks,
> Sasha
--
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 9/10] drivers/net/wireless/rtlwifi/rtl8192de/phy.c: removes unnecessary semicolon
From: Larry Finger @ 2012-09-06 16:49 UTC (permalink / raw)
To: Peter Senna Tschudin
Cc: kernel-janitors, Chaoming Li, John W. Linville, linux-wireless,
netdev, linux-kernel
In-Reply-To: <1346947757-10481-10-git-send-email-peter.senna@gmail.com>
On 09/06/2012 11:09 AM, Peter Senna Tschudin wrote:
> From: Peter Senna Tschudin <peter.senna@gmail.com>
>
> removes unnecessary semicolon
>
> Found by Coccinelle: http://coccinelle.lip6.fr/
>
> Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
>
> ---
> drivers/net/wireless/rtlwifi/rtl8192de/phy.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff -u -p a/drivers/net/wireless/rtlwifi/rtl8192de/phy.c b/drivers/net/wireless/rtlwifi/rtl8192de/phy.c
> --- a/drivers/net/wireless/rtlwifi/rtl8192de/phy.c
> +++ b/drivers/net/wireless/rtlwifi/rtl8192de/phy.c
> @@ -1314,7 +1314,7 @@ static void _rtl92d_phy_restore_rf_env(s
> struct bb_reg_def *pphyreg = &rtlphy->phyreg_def[rfpath];
>
> RT_TRACE(rtlpriv, COMP_RF, DBG_LOUD, "=====>\n");
> - /*----Restore RFENV control type----*/ ;
> + /*----Restore RFENV control type----*/
> switch (rfpath) {
> case RF90_PATH_A:
> case RF90_PATH_C:
>
>
Acked-by: Larry Finger <Larry.Finger@lwfinger.net>
Thanks,
Larry
^ permalink raw reply
* Re: [PATCHv3] virtio-spec: virtio network device multiqueue support
From: Paolo Bonzini @ 2012-09-06 16:43 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, kvm, virtualization
In-Reply-To: <20120906120828.GA1534@redhat.com>
Il 06/09/2012 14:08, Michael S. Tsirkin ha scritto:
> +#define VIRTIO_NET_CTRL_STEERING_HOST 1
You didn't change this occurrence.
Paolo
^ permalink raw reply
* Re: [PATCH] sctp: check dst validity after IPsec operations
From: Nicolas Dichtel @ 2012-09-06 16:40 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: sri, linux-sctp, netdev
In-Reply-To: <5048C984.3030306@gmail.com>
Le 06/09/2012 18:04, Vlad Yasevich a écrit :
> On 09/06/2012 01:40 PM, Nicolas Dichtel wrote:
>> dst stored in struct sctp_transport needs to be recalculated when ipsec policy
>> are updated. We use flow_cache_genid for that.
>>
>> For example, if a SCTP connection is established and then an IPsec policy is
>> set, the old SCTP flow will not be updated and thus will not use the new
>> IPsec policy.
>>
>> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
>
> why doesn't this need to be done for TCP? What makes SCTP special in this case?
Tests prove that the pb does not exist with TCP. I made the patch some times
ago, I will look again deeply to find the difference.
>
> ip_queue_xmit does an __sk_dst_check() which is essentially what
> sctp_transport_dst_check() does. That should determine if the currently cached
> route is valid or not.
The problem is that route will not be invalidated, because dst->check() has no
xfrm path so xfrm_dst_check() will never be called.
Regards,
Nicolas
>
> Looks like sctp may need to change to using ip_route_output_ports() call
> as ip_route_output_key may not do all that is necessary
>
> -vlad
>
>> ---
>> include/net/sctp/sctp.h | 3 ++-
>> include/net/sctp/structs.h | 3 +++
>> net/sctp/ipv6.c | 1 +
>> net/sctp/protocol.c | 1 +
>> 4 files changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
>> index 9c6414f..3267246 100644
>> --- a/include/net/sctp/sctp.h
>> +++ b/include/net/sctp/sctp.h
>> @@ -712,7 +712,8 @@ static inline void sctp_v4_map_v6(union sctp_addr *addr)
>> */
>> static inline struct dst_entry *sctp_transport_dst_check(struct
>> sctp_transport *t)
>> {
>> - if (t->dst && !dst_check(t->dst, 0)) {
>> + if ((t->dst && !dst_check(t->dst, 0)) ||
>> + (t->flow_cache_genid != atomic_read(&flow_cache_genid))) {
>> dst_release(t->dst);
>> t->dst = NULL;
>> }
>> diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
>> index 0fef00f..9dab882 100644
>> --- a/include/net/sctp/structs.h
>> +++ b/include/net/sctp/structs.h
>> @@ -948,6 +948,9 @@ struct sctp_transport {
>>
>> /* 64-bit random number sent with heartbeat. */
>> __u64 hb_nonce;
>> +
>> + /* used to check validity of dst */
>> + __u32 flow_cache_genid;
>> };
>>
>> struct sctp_transport *sctp_transport_new(struct net *, const union
>> sctp_addr *,
>> diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
>> index ea14cb4..2ed7410 100644
>> --- a/net/sctp/ipv6.c
>> +++ b/net/sctp/ipv6.c
>> @@ -349,6 +349,7 @@ out:
>> struct rt6_info *rt;
>> rt = (struct rt6_info *)dst;
>> t->dst = dst;
>> + t->flow_cache_genid = atomic_read(&flow_cache_genid);
>> SCTP_DEBUG_PRINTK("rt6_dst:%pI6 rt6_src:%pI6\n",
>> &rt->rt6i_dst.addr, &fl6->saddr);
>> } else {
>> diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
>> index 2d51842..4795a1a 100644
>> --- a/net/sctp/protocol.c
>> +++ b/net/sctp/protocol.c
>> @@ -512,6 +512,7 @@ out_unlock:
>> rcu_read_unlock();
>> out:
>> t->dst = dst;
>> + t->flow_cache_genid = atomic_read(&flow_cache_genid);
>> if (dst)
>> SCTP_DEBUG_PRINTK("rt_dst:%pI4, rt_src:%pI4\n",
>> &fl4->daddr, &fl4->saddr);
>>
>
^ permalink raw reply
* Re: [PATCH v3 01/17] hashtable: introduce a small and naive hashtable
From: Sasha Levin @ 2012-09-06 16:21 UTC (permalink / raw)
To: Steven Rostedt
Cc: Josh Triplett, Mathieu Desnoyers, Pedro Alves, Tejun Heo,
torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ,
davem-fT/PcQaiUtIeIZ0/mPfg9Q, mingo-X9Un+BFzKDI,
ebiederm-aS9lmoZGLiVWk0Htik3J/w, aarcange-H+wXaHxf7aLQT0dZR+AlfA,
ericvh-Re5JQEeQqe8AvxtiuMwx3w, netdev-u79uwXL29TY76Z2rM5mHXA,
eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w, axboe-tSWWG44O7X1aa/9Udqfwiw,
agk-H+wXaHxf7aLQT0dZR+AlfA, dm-devel-H+wXaHxf7aLQT0dZR+AlfA,
neilb-l3A5Bk7waGM, ccaulfie-H+wXaHxf7aLQT0dZR+AlfA,
teigland-H+wXaHxf7aLQT0dZR+AlfA,
Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA,
bfields-uC3wQj2KruNg9hUCZPvPmw, fweisbec-Re5JQEeQqe8AvxtiuMwx3w,
jesse-l0M0P4e3n4LQT0dZR+AlfA,
venkat.x.venkatsubra-QHcLZuEGTsvQT0dZR+AlfA,
ejt-H+wXaHxf7aLQT0dZR+AlfA, snitzer-H+wXaHxf7aLQT0dZR+AlfA,
edumazet-hpIqsD4AKlfQT0dZR+AlfA, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
dev-yBygre7rU0TnMu66kgdUjQ, rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
lw-BthXqXjhjHXQFUHtdCDX3A
In-Reply-To: <1346947206.1680.36.camel-f9ZlEuEWxVcJvu8Pb33WZ0EMvNT87kid@public.gmane.org>
On 09/06/2012 06:00 PM, Steven Rostedt wrote:
>> > I think that that code doesn't make sense. The users of hlist_for_each_* aren't
>> > supposed to be changing the loop cursor.
> I totally agree. Modifying the 'node' pointer is just asking for issues.
> Yes that is error prone, but not due to the double loop. It's due to the
> modifying of the node pointer that is used internally by the loop
> counter. Don't do that :-)
While we're on this subject, I haven't actually seen hlist_for_each_entry() code
that even *touches* 'pos'.
Will people yell at me loudly if I change the prototype of those macros to be:
hlist_for_each_entry(tpos, head, member)
(Dropping the 'pos' parameter), and updating anything that calls those macros to
drop it as well?
Thanks,
Sasha
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 0/5] dev_<level> and dynamic_debug cleanups
From: Greg Kroah-Hartman @ 2012-09-06 16:13 UTC (permalink / raw)
To: Jim Cromie, Jason Baron
Cc: Kay Sievers, Joe Perches, Andrew Morton, netdev, David S. Miller,
linux-kernel
In-Reply-To: <CAJfuBxyToA9-=c3H88GGba3GFX64kdbP4OCEC0f4trnBnjpU-w@mail.gmail.com>
On Thu, Aug 30, 2012 at 09:48:12PM -0600, Jim Cromie wrote:
> On Thu, Aug 30, 2012 at 11:43 AM, Jim Cromie <jim.cromie@gmail.com> wrote:
> > On Sun, Aug 26, 2012 at 5:25 AM, Joe Perches <joe@perches.com> wrote:
> >> The recent commit to fix dynamic_debug was a bit unclean.
> >> Neaten the style for dynamic_debug.
> >> Reduce the stack use of message logging that uses netdev_printk
> >> Add utility functions dev_printk_emit and dev_vprintk_emit for /dev/kmsg.
> >>
> >> Joe Perches (5):
> >> dev_dbg/dynamic_debug: Update to use printk_emit, optimize stack
> >> netdev_printk/dynamic_netdev_dbg: Directly call printk_emit
> >> netdev_printk/netif_printk: Remove a superfluous logging colon
> >> dev: Add dev_vprintk_emit and dev_printk_emit
> >> device and dynamic_debug: Use dev_vprintk_emit and dev_printk_emit
> >>
> >
> > Ive tested this on 2 builds differing only by DYNAMIC_DEBUG
> > It works for me on x86-64
> >
> > However, I just booted a non-dyndbg build on x86-32, and got this.
> >
>
> Ok, transient error, went away with a clean build.
>
> tested-by: Jim Cromie <jim.cromie@gmail.com>
Jason, any ACK on these, or any of the other random dynamic debug
patches floating around? What am I supposed to be applying here?
thanks,
greg k-h
^ permalink raw reply
* [PATCH 9/10] drivers/net/wireless/rtlwifi/rtl8192de/phy.c: removes unnecessary semicolon
From: Peter Senna Tschudin @ 2012-09-06 16:09 UTC (permalink / raw)
To: Larry Finger
Cc: kernel-janitors, Peter Senna Tschudin, Chaoming Li,
John W. Linville, linux-wireless, netdev, linux-kernel
In-Reply-To: <1346947757-10481-1-git-send-email-peter.senna@gmail.com>
From: Peter Senna Tschudin <peter.senna@gmail.com>
removes unnecessary semicolon
Found by Coccinelle: http://coccinelle.lip6.fr/
Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
---
drivers/net/wireless/rtlwifi/rtl8192de/phy.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff -u -p a/drivers/net/wireless/rtlwifi/rtl8192de/phy.c b/drivers/net/wireless/rtlwifi/rtl8192de/phy.c
--- a/drivers/net/wireless/rtlwifi/rtl8192de/phy.c
+++ b/drivers/net/wireless/rtlwifi/rtl8192de/phy.c
@@ -1314,7 +1314,7 @@ static void _rtl92d_phy_restore_rf_env(s
struct bb_reg_def *pphyreg = &rtlphy->phyreg_def[rfpath];
RT_TRACE(rtlpriv, COMP_RF, DBG_LOUD, "=====>\n");
- /*----Restore RFENV control type----*/ ;
+ /*----Restore RFENV control type----*/
switch (rfpath) {
case RF90_PATH_A:
case RF90_PATH_C:
^ permalink raw reply
* [PATCH 8/10] net/mac80211/scan.c: removes unnecessary semicolon
From: Peter Senna Tschudin @ 2012-09-06 16:09 UTC (permalink / raw)
To: John W. Linville
Cc: kernel-janitors, Peter Senna Tschudin, Johannes Berg,
David S. Miller, linux-wireless, netdev, linux-kernel
In-Reply-To: <1346947757-10481-1-git-send-email-peter.senna@gmail.com>
From: Peter Senna Tschudin <peter.senna@gmail.com>
removes unnecessary semicolon
Found by Coccinelle: http://coccinelle.lip6.fr/
Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
---
net/mac80211/scan.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff -u -p a/net/mac80211/scan.c b/net/mac80211/scan.c
--- a/net/mac80211/scan.c
+++ b/net/mac80211/scan.c
@@ -407,7 +407,7 @@ static void ieee80211_scan_state_send_pr
enum ieee80211_band band = local->hw.conf.channel->band;
sdata = rcu_dereference_protected(local->scan_sdata,
- lockdep_is_held(&local->mtx));;
+ lockdep_is_held(&local->mtx));
for (i = 0; i < local->scan_req->n_ssids; i++)
ieee80211_send_probe_req(
^ permalink raw reply
* Re: changing usbnet's API to better deal with cdc-ncm
From: Ming Lei @ 2012-09-06 16:09 UTC (permalink / raw)
To: Bjørn Mork
Cc: Oliver Neukum, alexey.orishko-0IS4wlFg1OjSUeElwK9/Pw,
netdev-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <87a9x33656.fsf-lbf33ChDnrE/G1V5fR+Y7Q@public.gmane.org>
On Thu, Sep 6, 2012 at 4:30 PM, Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org> wrote:
> Ming Lei <tom.leiming-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:
>> On Thu, Sep 6, 2012 at 4:12 AM, Oliver Neukum <oneukum-l3A5Bk7waGM@public.gmane.org> wrote:
>>> Hi,
>>>
>>> looking at cdc-ncm it seeems to me that cdc-ncm is forced to play
>>> very dirty games because usbnet doesn't have a notion about aggregating
>>> packets for a single transfer.
>>
>> The Ethernet API we are using does not support transmitting multiple Ethernet
>> frames in a single call, so the aggregation things should be done inside low
>> level driver, in fact it is just what some wlan(802.11n) drivers have
>> been doing.
>>
>> IMO, the current .tx_fixup is intelligent enough to handle aggregation:
>>
>> - return a skb_out for sending if low level drivers think it is
>> ready to send
>> the aggregated frames
>> - return NULL if the low level drivers think they need to wait
>> for more frames
>>
>> Of course, the low level drivers need to start a timer to trigger sending
>> remainder frames in case of timeout and no further frames come from
>> upper protocol stack.
>>
>> Looks the introduced .tx_bundle is not necessary since .tx_fixup is OK.
>
> The minidriver does not have any information about tx in progress. The
Inside .tx_fixup, the low level driver will get the tx progress information.
> strategy above, which is what cdc_ncm uses today, is fine as long as you
> always want to wait as long as you always want to fill as many frames as
> possible in each packet. But what if the queue is empty and the device
For cdc_ncm, the wait time is controlled by cdc_ncm driver, see
cdc_ncm_tx_timeout_start().
> is just sitting there doing nothing while you are waiting for more
> frames? Then you are just adding unnecessary latency.
As said above, cdc_ncm can control the latency by itself.
>
> There are also several usbnet minidrivers for protocols with frame
> aggregation support. Most of them do not currently implement tx
> aggregation, possibly due to the complexity. This makes a common
> aggregation framework interesting in any case. Reimplementing similar
> loginc in multiple minidrivers is meaningless.
If we can abstract some common things about aggregation, it should be
meaningful. As far as I know, most of aggregation protocol is very different,
so almost all aggregation work is only done by low level driver, such as
cdc_ncm.
If we want to implement some aggregation framework, maybe below is
one solution, correct me if it is wrong.
usbnet_start_xmit():
to_send = info->tx_bundle(in_skb, &out_skb)
if (!to_send) {
usbnet_tx_timeout_start(usbnet);
goto not_drop;
}
skb = info->tx_fixup(dev, out_skb, GFP_ATOMIC);
...
Looks the above is no much advantage than doing bundling in .tx_fixup
just like cdc_ncm.
>
> I believe Oliver is adding very useful functionality to usbnet here.
> Functionality which at least cdc_ncm and possibly other existing
> minidrivers can take advantage of immediately. There also seems to be a
Sorry, I don't know other minidrivers, and is there some common
things in building aggregation with cdc_ncm?
Considered that the patch doesn't implement wait_for_more, suggest
Oliver to post a new one with basic wait_for_more support for further
discussion.
Thanks,
--
Ming Lei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 1/10] drivers/net/usb/sierra_net.c: removes unnecessary semicolon
From: Peter Senna Tschudin @ 2012-09-06 16:09 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: kernel-janitors, Peter Senna Tschudin, linux-usb, netdev,
linux-kernel
From: Peter Senna Tschudin <peter.senna@gmail.com>
removes unnecessary semicolon
Found by Coccinelle: http://coccinelle.lip6.fr/
Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
---
drivers/net/usb/sierra_net.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff -u -p a/drivers/net/usb/sierra_net.c b/drivers/net/usb/sierra_net.c
--- a/drivers/net/usb/sierra_net.c
+++ b/drivers/net/usb/sierra_net.c
@@ -842,7 +842,7 @@ static int sierra_net_rx_fixup(struct us
netdev_err(dev->net, "HIP/ETH: Invalid pkt\n");
dev->net->stats.rx_frame_errors++;
- /* dev->net->stats.rx_errors incremented by caller */;
+ /* dev->net->stats.rx_errors incremented by caller */
return 0;
}
^ permalink raw reply
* Re: [PATCH] sctp: check dst validity after IPsec operations
From: Vlad Yasevich @ 2012-09-06 16:04 UTC (permalink / raw)
To: Nicolas Dichtel; +Cc: sri, linux-sctp, netdev
In-Reply-To: <1346953229-3825-1-git-send-email-nicolas.dichtel@6wind.com>
On 09/06/2012 01:40 PM, Nicolas Dichtel wrote:
> dst stored in struct sctp_transport needs to be recalculated when ipsec policy
> are updated. We use flow_cache_genid for that.
>
> For example, if a SCTP connection is established and then an IPsec policy is
> set, the old SCTP flow will not be updated and thus will not use the new
> IPsec policy.
>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
why doesn't this need to be done for TCP? What makes SCTP special in
this case?
ip_queue_xmit does an __sk_dst_check() which is essentially what
sctp_transport_dst_check() does. That should determine if the currently
cached route is valid or not.
Looks like sctp may need to change to using ip_route_output_ports() call
as ip_route_output_key may not do all that is necessary
-vlad
> ---
> include/net/sctp/sctp.h | 3 ++-
> include/net/sctp/structs.h | 3 +++
> net/sctp/ipv6.c | 1 +
> net/sctp/protocol.c | 1 +
> 4 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
> index 9c6414f..3267246 100644
> --- a/include/net/sctp/sctp.h
> +++ b/include/net/sctp/sctp.h
> @@ -712,7 +712,8 @@ static inline void sctp_v4_map_v6(union sctp_addr *addr)
> */
> static inline struct dst_entry *sctp_transport_dst_check(struct sctp_transport *t)
> {
> - if (t->dst && !dst_check(t->dst, 0)) {
> + if ((t->dst && !dst_check(t->dst, 0)) ||
> + (t->flow_cache_genid != atomic_read(&flow_cache_genid))) {
> dst_release(t->dst);
> t->dst = NULL;
> }
> diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
> index 0fef00f..9dab882 100644
> --- a/include/net/sctp/structs.h
> +++ b/include/net/sctp/structs.h
> @@ -948,6 +948,9 @@ struct sctp_transport {
>
> /* 64-bit random number sent with heartbeat. */
> __u64 hb_nonce;
> +
> + /* used to check validity of dst */
> + __u32 flow_cache_genid;
> };
>
> struct sctp_transport *sctp_transport_new(struct net *, const union sctp_addr *,
> diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
> index ea14cb4..2ed7410 100644
> --- a/net/sctp/ipv6.c
> +++ b/net/sctp/ipv6.c
> @@ -349,6 +349,7 @@ out:
> struct rt6_info *rt;
> rt = (struct rt6_info *)dst;
> t->dst = dst;
> + t->flow_cache_genid = atomic_read(&flow_cache_genid);
> SCTP_DEBUG_PRINTK("rt6_dst:%pI6 rt6_src:%pI6\n",
> &rt->rt6i_dst.addr, &fl6->saddr);
> } else {
> diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
> index 2d51842..4795a1a 100644
> --- a/net/sctp/protocol.c
> +++ b/net/sctp/protocol.c
> @@ -512,6 +512,7 @@ out_unlock:
> rcu_read_unlock();
> out:
> t->dst = dst;
> + t->flow_cache_genid = atomic_read(&flow_cache_genid);
> if (dst)
> SCTP_DEBUG_PRINTK("rt_dst:%pI4, rt_src:%pI4\n",
> &fl4->daddr, &fl4->saddr);
>
^ 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