Netdev List
 help / color / mirror / Atom feed
* [PATCH] ath10k: Use ARRAY_SIZE instead of dividing sizeof array with sizeof an element
From: zhong jiang @ 2019-09-04  7:41 UTC (permalink / raw)
  To: kvalo, davem; +Cc: ath10k, linux-wireless, zhongjiang, netdev, linux-kernel

With the help of Coccinelle, ARRAY_SIZE can be replaced in ath10k_snoc_wlan_enable.

Signed-off-by: zhong jiang <zhongjiang@huawei.com>
---
 drivers/net/wireless/ath/ath10k/snoc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/snoc.c b/drivers/net/wireless/ath/ath10k/snoc.c
index b491361..49fc044 100644
--- a/drivers/net/wireless/ath/ath10k/snoc.c
+++ b/drivers/net/wireless/ath/ath10k/snoc.c
@@ -976,8 +976,7 @@ static int ath10k_snoc_wlan_enable(struct ath10k *ar,
 				  sizeof(struct ath10k_svc_pipe_cfg);
 	cfg.ce_svc_cfg = (struct ath10k_svc_pipe_cfg *)
 		&target_service_to_ce_map_wlan;
-	cfg.num_shadow_reg_cfg = sizeof(target_shadow_reg_cfg_map) /
-					sizeof(struct ath10k_shadow_reg_cfg);
+	cfg.num_shadow_reg_cfg = ARRAY_SIZE(target_shadow_reg_cfg_map);
 	cfg.shadow_reg_cfg = (struct ath10k_shadow_reg_cfg *)
 		&target_shadow_reg_cfg_map;
 
-- 
1.7.12.4


^ permalink raw reply related

* Re: [PATCH] net/skbuff: silence warnings under memory pressure
From: Sergey Senozhatsky @ 2019-09-04  7:43 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Qian Cai, Eric Dumazet, davem, netdev, linux-mm, linux-kernel,
	Petr Mladek, Sergey Senozhatsky, Steven Rostedt
In-Reply-To: <20190904071911.GB11968@jagdpanzerIV>

On (09/04/19 16:19), Sergey Senozhatsky wrote:
> Hmm. I need to look at this more... wake_up_klogd() queues work only once
> on particular CPU: irq_work_queue(this_cpu_ptr(&wake_up_klogd_work));
> 
> bool irq_work_queue()
> {
> 	/* Only queue if not already pending */
> 	if (!irq_work_claim(work))
> 		return false;
> 
> 	 __irq_work_queue_local(work);
> }

Plus one more check - waitqueue_active(&log_wait). printk() adds
pending irq_work only if there is a user-space process sleeping on
log_wait and irq_work is not already scheduled. If the syslog is
active or there is noone to wakeup then we don't queue irq_work.

	-ss

^ permalink raw reply

* [patch net-next] rocker: add missing init_net check in FIB notifier
From: Jiri Pirko @ 2019-09-04  7:40 UTC (permalink / raw)
  To: netdev; +Cc: davem, mlxsw

From: Jiri Pirko <jiri@mellanox.com>

Take only FIB events that are happening in init_net into account. No other
namespaces are supported.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/ethernet/rocker/rocker_main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/rocker/rocker_main.c b/drivers/net/ethernet/rocker/rocker_main.c
index 2c5d3f5b84dd..786b158bd305 100644
--- a/drivers/net/ethernet/rocker/rocker_main.c
+++ b/drivers/net/ethernet/rocker/rocker_main.c
@@ -2189,6 +2189,9 @@ static int rocker_router_fib_event(struct notifier_block *nb,
 	struct rocker_fib_event_work *fib_work;
 	struct fib_notifier_info *info = ptr;
 
+	if (!net_eq(info->net, &init_net))
+		return NOTIFY_DONE;
+
 	if (info->family != AF_INET)
 		return NOTIFY_DONE;
 
-- 
2.21.0


^ permalink raw reply related

* [PATCH net] ipv4: fix ifa_flags reuse problem in using ifconfig tool
From: Su Yanjun @ 2019-09-04  7:37 UTC (permalink / raw)
  To: davem, kuznet, yoshfuji; +Cc: netdev, linux-kernel, suyj.fnst

When NetworkManager has already set ipv4 address then uses
ifconfig set another ipv4 address. It will use previous ifa_flags
that will cause device route not be inserted.

As NetworkManager has already support IFA_F_NOPREFIXROUTE flag [1],
but ifconfig will reuse the ifa_flags. It's weird especially
some old scripts or program [2]  still  use ifconfig.

[1] https://gitlab.freedesktop.org/NetworkManager/NetworkManager/
commit/fec80e7473ad16979af75ed299d68103e7aa3fe9

[2] LTP or TAHI

Signed-off-by: Su Yanjun <suyj.fnst@cn.fujitsu.com>
---
 net/ipv4/devinet.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index a4b5bd4..56ca339 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -1159,6 +1159,7 @@ int devinet_ioctl(struct net *net, unsigned int cmd, struct ifreq *ifr)
 			inet_del_ifa(in_dev, ifap, 0);
 			ifa->ifa_broadcast = 0;
 			ifa->ifa_scope = 0;
+			ifa->ifa_flags = 0;
 		}
 
 		ifa->ifa_address = ifa->ifa_local = sin->sin_addr.s_addr;
-- 
2.7.4




^ permalink raw reply related

* Re: rtnl_lock() question
From: Eric Dumazet @ 2019-09-04  7:39 UTC (permalink / raw)
  To: Jonathan Lemon, Netdev
In-Reply-To: <29EC5179-D939-42CD-8577-682BE4B05916@gmail.com>



On 9/3/19 11:55 PM, Jonathan Lemon wrote:
> How appropriate is it to hold the rtnl_lock() across a sleepable
> memory allocation?  On one hand it's just a mutex, but it would
> seem like it could block quite a few things.
> 

Sure, all GFP_KERNEL allocations can sleep for quite a while.

On the other hand, we may want to delay stuff if memory is under pressure,
or complex operations like NEWLINK would fail.

RTNL is mostly taken for control path operations, we prefer them to be
mostly reliable, otherwise admins job would be a nightmare.

In some cases, it is relatively easy to pre-allocate memory before rtnl is taken,
but that will only take care of some selected paths.

^ permalink raw reply

* Re: [RFC PATCH v2 net-next 10/15] net: dsa: Pass ndo_setup_tc slave callback to drivers
From: Kurt Kanzenbach @ 2019-09-04  7:31 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: Florian Fainelli, Vivien Didelot, Andrew Lunn, David S. Miller,
	Vinicius Costa Gomes, vedang.patel, Richard Cochran, weifeng.voon,
	jiri, m-karicheri2, Jose.Abreu, Ilias Apalodimas,
	Jamal Hadi Salim, xiyou.wangcong, netdev
In-Reply-To: <CA+h21hoVv0SwFf8=MS_SZf85QsObrNKQf_w_p=j_97i16psjDQ@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 927 bytes --]

Hi Vladimir,

On Mon, Sep 02, 2019 at 11:49:30AM +0300, Vladimir Oltean wrote:
> I did something similar in v1 with a .port_setup_taprio in "[RFC PATCH
> net-next 3/6] net: dsa: Pass tc-taprio offload to drivers".

Okay, didn't see that one.

> Would this address Ilias's comment about DSA not really needing to
> have this level of awareness into the qdisc offload type? Rightfully I
> can agree that the added-value of making a .port_set_schedule and
> .port_del_schedule in DSA compared to simply passing the ndo_setup_tc
> is not that great.

I wanted to avoid that drivers have to the same kind of work, and it put
it therefore into the core part. However, I agree that the added-value
is not that high for TAPRIO.

>
> By the way, thanks for the iproute2 patch for parsing 64-bit base time
> on ARM 32, saved me a bit of debugging time :)

No problem :). That cost me a bit of time.

> Regards,
> -Vladimir

Thanks,
Kurt

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH v5] perf machine: arm/arm64: Improve completeness for kernel address space
From: Adrian Hunter @ 2019-09-04  7:26 UTC (permalink / raw)
  To: Leo Yan
  Cc: Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa,
	Namhyung Kim, Alexei Starovoitov, Daniel Borkmann,
	Martin KaFai Lau, Song Liu, Yonghong Song, linux-kernel, netdev,
	bpf, clang-built-linux, Mathieu Poirier, Peter Zijlstra,
	Suzuki Poulouse, coresight, linux-arm-kernel
In-Reply-To: <20190902141511.GF4931@leoy-ThinkPad-X240s>

On 2/09/19 5:15 PM, Leo Yan wrote:
> Hi Adrian,
> 
> On Mon, Aug 26, 2019 at 08:51:05PM +0800, Leo Yan wrote:
>> Hi Adrian,
>>
>> On Fri, Aug 16, 2019 at 04:00:02PM +0300, Adrian Hunter wrote:
>>> On 16/08/19 4:45 AM, Leo Yan wrote:
>>>> Hi Adrian,
>>>>
>>>> On Thu, Aug 15, 2019 at 02:45:57PM +0300, Adrian Hunter wrote:
>>>>
>>>> [...]
>>>>
>>>>>>> How come you cannot use kallsyms to get the information?
>>>>>>
>>>>>> Thanks for pointing out this.  Sorry I skipped your comment "I don't
>>>>>> know how you intend to calculate ARM_PRE_START_SIZE" when you reviewed
>>>>>> the patch v3, I should use that chance to elaborate the detailed idea
>>>>>> and so can get more feedback/guidance before procceed.
>>>>>>
>>>>>> Actually, I have considered to use kallsyms when worked on the previous
>>>>>> patch set.
>>>>>>
>>>>>> As mentioned in patch set v4's cover letter, I tried to implement
>>>>>> machine__create_extra_kernel_maps() for arm/arm64, the purpose is to
>>>>>> parse kallsyms so can find more kernel maps and thus also can fixup
>>>>>> the kernel start address.  But I found the 'perf script' tool directly
>>>>>> calls machine__get_kernel_start() instead of running into the flow for
>>>>>> machine__create_extra_kernel_maps();
>>>>>
>>>>> Doesn't it just need to loop through each kernel map to find the lowest
>>>>> start address?
>>>>
>>>> Based on your suggestion, I worked out below change and verified it
>>>> can work well on arm64 for fixing up start address; please let me know
>>>> if the change works for you?
>>>
>>> How does that work if take a perf.data file to a machine with a different
>>> architecture?
>>
>> Sorry I delayed so long to respond to your question; I didn't have
>> confidence to give out very reasonale answer and this is the main reason
>> for delaying.
> 
> Could you take chance to review my below replying?  I'd like to get
> your confirmation before I send out offical patch.

It is not necessary to do kallsyms__parse for x86_64, so it would be better
to check the arch before calling that.

However in general, having to copy and use kallsyms with perf.data if on a
different arch does not seem very user friendly.  But really that is up to
Arnaldo.

> 
> Thanks,
> Leo Yan
> 
>>
>> For your question for taking a perf.data file to a machine with a
>> different architecture, we can firstly use command 'perf buildid-list'
>> to print out the buildid for kallsyms, based on the dumped buildid we
>> can find out the location for the saved kallsyms file; then we can use
>> option '--kallsyms' to specify the offline kallsyms file and use the
>> offline kallsyms to fixup kernel start address.  The detailed commands
>> are listed as below:
>>
>> root@debian:~# perf buildid-list
>> 7b36dfca8317ef74974ebd7ee5ec0a8b35c97640 [kernel.kallsyms]
>> 56b84aa88a1bcfe222a97a53698b92723a3977ca /usr/lib/systemd/systemd
>> 0956b952e9cd673d48ff2cfeb1a9dbd0c853e686 /usr/lib/aarch64-linux-gnu/libm-2.28.so
>> [...]
>>
>> root@debian:~# perf script --kallsyms ~/.debug/\[kernel.kallsyms\]/7b36dfca8317ef74974ebd7ee5ec0a8b35c97640/kallsyms
>>
>> The amended patch is as below, please review and always welcome
>> any suggestions or comments!
>>
>> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
>> index 5734460fc89e..593f05cc453f 100644
>> --- a/tools/perf/util/machine.c
>> +++ b/tools/perf/util/machine.c
>> @@ -2672,9 +2672,26 @@ int machine__nr_cpus_avail(struct machine *machine)
>>  	return machine ? perf_env__nr_cpus_avail(machine->env) : 0;
>>  }
>>  
>> +static int machine__fixup_kernel_start(void *arg,
>> +				       const char *name __maybe_unused,
>> +				       char type,
>> +				       u64 start)
>> +{
>> +	struct machine *machine = arg;
>> +
>> +	type = toupper(type);
>> +
>> +	/* Fixup for text, weak, data and bss sections. */
>> +	if (type == 'T' || type == 'W' || type == 'D' || type == 'B')
>> +		machine->kernel_start = min(machine->kernel_start, start);
>> +
>> +	return 0;
>> +}
>> +
>>  int machine__get_kernel_start(struct machine *machine)
>>  {
>>  	struct map *map = machine__kernel_map(machine);
>> +	char filename[PATH_MAX];
>>  	int err = 0;
>>  
>>  	/*
>> @@ -2696,6 +2713,22 @@ int machine__get_kernel_start(struct machine *machine)
>>  		if (!err && !machine__is(machine, "x86_64"))
>>  			machine->kernel_start = map->start;
>>  	}
>> +
>> +	if (symbol_conf.kallsyms_name != NULL) {
>> +		strncpy(filename, symbol_conf.kallsyms_name, PATH_MAX);
>> +	} else {
>> +		machine__get_kallsyms_filename(machine, filename, PATH_MAX);
>> +
>> +		if (symbol__restricted_filename(filename, "/proc/kallsyms"))
>> +			goto out;
>> +	}
>> +
>> +	if (kallsyms__parse(filename, machine, machine__fixup_kernel_start))
>> +		pr_warning("Fail to fixup kernel start address. skipping...\n");
>> +
>> +out:
>>  	return err;
>>  }
>>  
>>
>> Thanks,
>> Leo Yan
> 


^ permalink raw reply

* Re: [PATCH] net/skbuff: silence warnings under memory pressure
From: Sergey Senozhatsky @ 2019-09-04  7:19 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Sergey Senozhatsky, Qian Cai, Eric Dumazet, davem, netdev,
	linux-mm, linux-kernel, Petr Mladek, Sergey Senozhatsky,
	Steven Rostedt
In-Reply-To: <20190904065455.GE3838@dhcp22.suse.cz>

On (09/04/19 08:54), Michal Hocko wrote:
> I am sorry, I could have been more explicit when CCing you.

Oh, sorry! My bad!

> Sure the ratelimit is part of the problem. But I was more interested
> in the potential livelock (infinite loop) mentioned by Qian Cai. It
> is not important whether we generate one or more lines of output from
> the softirq context as long as the printk generates more irq processing
> which might end up doing the same. Is this really possible?

Hmm. I need to look at this more... wake_up_klogd() queues work only once
on particular CPU: irq_work_queue(this_cpu_ptr(&wake_up_klogd_work));

bool irq_work_queue()
{
	/* Only queue if not already pending */
	if (!irq_work_claim(work))
		return false;

	 __irq_work_queue_local(work);
}

softirqs are processed in batches, right? The softirq batch can add XXXX
lines to printk logbuf, but there will be only one PRINTK_PENDING_WAKEUP
queued. Qian Cai mentioned that "net_rx_action softirqs again which are
plenty due to connected via ssh etc." so the proportion still seems to be
N:1 - we process N softirqs, add 1 printk irq_work.

But need to think more.
Interesting question.

	-ss

^ permalink raw reply

* Re: [Bridge] [PATCH v3 1/2] net: bridge: use mac_len in bridge forwarding
From: Toshiaki Makita @ 2019-09-04  7:14 UTC (permalink / raw)
  To: Zahari Doychev
  Cc: netdev, makita.toshiaki, jiri, nikolay, simon.horman, roopa,
	bridge, jhs, dsahern, xiyou.wangcong, johannes,
	alexei.starovoitov
In-Reply-To: <20190903133635.siw6xcaqwk7m5a5a@tycho>

On 2019/09/03 22:36, Zahari Doychev wrote:
> On Tue, Sep 03, 2019 at 08:37:36PM +0900, Toshiaki Makita wrote:
>> Hi Zahari,
>>
>> Sorry for reviewing this late.
>>
>> On 2019/09/03 3:09, Zahari Doychev wrote:
>> ...
>>> @@ -466,13 +466,14 @@ static bool __allowed_ingress(const struct net_bridge *br,
>>>    		/* Tagged frame */
>>>    		if (skb->vlan_proto != br->vlan_proto) {
>>>    			/* Protocol-mismatch, empty out vlan_tci for new tag */
>>> -			skb_push(skb, ETH_HLEN);
>>> +			skb_push(skb, skb->mac_len);
>>>    			skb = vlan_insert_tag_set_proto(skb, skb->vlan_proto,
>>>    							skb_vlan_tag_get(skb));
>>
>> I think we should insert vlan at skb->data, i.e. mac_header + mac_len, while this
>> function inserts the tag at mac_header + ETH_HLEN which is not always the correct
>> offset.
> 
> Maybe I am misunderstanding the concern here but this should make sure that
> the VLAN tag from the skb is move back in the payload as the outer most tag.
> So it should follow the ethernet header. It looks like this e.g.,:
> 
> VLAN1 in skb:
> +------+------+-------+
> | DMAC | SMAC | ETYPE |
> +------+------+-------+
> 
> VLAN1 moved to payload:
> +------+------+-------+-------+
> | DMAC | SMAC | VLAN1 | ETYPE |
> +------+------+-------+-------+
> 
> VLAN2 in skb:
> +------+------+-------+-------+
> | DMAC | SMAC | VLAN1 | ETYPE |
> +------+------+-------+-------+
> 
> VLAN2 moved to payload:
> 
> +------+------+-------+-------+
> | DMAC | SMAC | VLAN2 | VLAN1 | ....
> +------+------+-------+-------+
> 
> Doing the skb push with mac_len makes sure that VLAN tag is inserted in the
> correct offset. For mac_len == ETH_HLEN this does not change the current
> behaviour.

Reordering VLAN headers here does not look correct to me. If skb->data points to ETH+VLAN,
then we should insert the vlan at the offset.
Vlan devices with reorder_hdr disabled produce packets whose mac_len includes ETH+VLAN header,
and they expects vlan insertion after the outer vlan header.

Also I'm not sure there is standard ethernet header in mac_len, as mac_len is not ETH_HLEN.
E.g. tun devices can produce vlan packets without ehternet header.

> 
>>
>>>    			if (unlikely(!skb))
>>>    				return false;
>>>    			skb_pull(skb, ETH_HLEN);
>>
>> Now skb->data is mac_header + ETH_HLEN which would be broken when mac_len is not
>> ETH_HLEN?
> 
> I thought it would be better to point in this case to the outer tag as otherwise
> if mac_len is used the skb->data will point to the next tag which I find somehow
> inconsistent or do you see some case where this can cause problems?

Vlan devices with reorder_hdr off will break because it relies on skb->data offset
as I described in the previous discussion.

Toshiaki Makita

^ permalink raw reply

* Re: [PATCH net-next v3] net: openvswitch: Set OvS recirc_id from tc chain index
From: Paul Blakey @ 2019-09-04  7:13 UTC (permalink / raw)
  To: Edward Cree, Pravin B Shelar, netdev@vger.kernel.org,
	David S. Miller, Justin Pettit, Simon Horman,
	Marcelo Ricardo Leitner, Vlad Buslov
  Cc: Jiri Pirko, Roi Dayan, Yossi Kuperman, Rony Efraim, Oz Shlomo
In-Reply-To: <6d2e1ef7-f859-32f4-584f-1f0f772edadf@solarflare.com>


On 9/3/2019 5:56 PM, Edward Cree wrote:
> On 03/09/2019 14:23, Paul Blakey wrote:
>> Offloaded OvS datapath rules are translated one to one to tc rules,
>> for example the following simplified OvS rule:
>>
>> recirc_id(0),in_port(dev1),eth_type(0x0800),ct_state(-trk) actions:ct(),recirc(2)
>>
>> Will be translated to the following tc rule:
>>
>> $ tc filter add dev dev1 ingress \
>> 	    prio 1 chain 0 proto ip \
>> 		flower tcp ct_state -trk \
>> 		action ct pipe \
>> 		action goto chain 2
>>
>> Received packets will first travel though tc, and if they aren't stolen
>> by it, like in the above rule, they will continue to OvS datapath.
>> Since we already did some actions (action ct in this case) which might
>> modify the packets, and updated action stats, we would like to continue
>> the proccessing with the correct recirc_id in OvS (here recirc_id(2))
>> where we left off.
> IMHO each offload (OvS -> tc, and tc -> hw) ought only take place for a rule
>   if all sequelae of that rule are also offloaded, or if non-offloaded sequelae
>   can be guaranteed to provide an unmodified packet so that the exception path
>   can start from the beginning.  I don't like this idea of doing part of the
>   processing in one place and then resuming the rest later in an entirely
>   different piece of code.

We hope to replicate the entire software module to hardware, not just tc.

For tc offloading, we currently offload tc rules one by one, including 
tc chains rules (match on chain, and goto chain action),

and the offloaded rules might not catch all packets:

tc fiter add dev1 ..... chain 0 ... flower dst_mac aa:bb:cc:dd:ee:ff 
action pedit munge ex set dst src 12:34:56:78:aa:bb action goto chain 5

tc fiter add dev1 ..... chain 5 ... flower ip_proto UDP action mirred 
egress redirect dev dev2

If the packet isn't UDP we miss on chain5.


Besides this basic case, there can be actions that aren't available 
currently, and need software assistance, such as encapsulation.

if we had the following rule:

tc fiter add dev1 ..... chain 5 ... flower ip_proto UDP action 
tunnel_key set dst_port 4789 vni 98 dst_ip 1.1.1.1 mirred egress 
redirect dev vxlan_sys_4789

And neighbor for the dst_ip 1.1.1.1 is unreachable, we can't do the 
encapsulation in hardware currently, and wait for software to resolve

the neighbor via arp.

Another is the action ct we plan on offloading here, we send packets 
back to conntrack if needed (before the connection is established, for 
connection setup)

There can also be trap action, to explicitly continue in software, and 
the user might want the partial processing before it.


We currently support up to several millions of such rules, and any 
update (delete/add) of a continuation of a rule (e.g chain 5 rule),

might affect the processing of millions of other rules before it (goto 
chain 5 rules), with tc priorities, it's even worse and happens more often.



^ permalink raw reply

* Re: [PATCH 1/2] linux/kernel.h: add yesno(), onoff(), enableddisabled(), plural() helpers
From: Greg Kroah-Hartman @ 2019-09-04  7:05 UTC (permalink / raw)
  To: Jani Nikula
  Cc: linux-kernel, Joonas Lahtinen, Rodrigo Vivi, intel-gfx,
	Vishal Kulkarni, netdev, linux-usb, Andrew Morton, Julia Lawall
In-Reply-To: <20190903133731.2094-1-jani.nikula@intel.com>

On Tue, Sep 03, 2019 at 04:37:30PM +0300, Jani Nikula wrote:
> The kernel has plenty of ternary operators to choose between constant
> strings, such as condition ? "yes" : "no", as well as value == 1 ? "" :
> "s":
> 
> $ git grep '? "yes" : "no"' | wc -l
> 258
> $ git grep '? "on" : "off"' | wc -l
> 204
> $ git grep '? "enabled" : "disabled"' | wc -l
> 196
> $ git grep '? "" : "s"' | wc -l
> 25
> 
> Additionally, there are some occurences of the same in reverse order,
> split to multiple lines, or otherwise not caught by the simple grep.
> 
> Add helpers to return the constant strings. Remove existing equivalent
> and conflicting functions in i915, cxgb4, and USB core. Further
> conversion can be done incrementally.
> 
> While the main goal here is to abstract recurring patterns, and slightly
> clean up the code base by not open coding the ternary operators, there
> are also some space savings to be had via better string constant
> pooling.
> 
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Cc: Vishal Kulkarni <vishal@chelsio.com>
> Cc: netdev@vger.kernel.org
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-usb@vger.kernel.org
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: linux-kernel@vger.kernel.org
> Cc: Julia Lawall <julia.lawall@lip6.fr>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_utils.h             | 15 -------------
>  .../ethernet/chelsio/cxgb4/cxgb4_debugfs.c    | 11 ----------
>  drivers/usb/core/config.c                     |  5 -----
>  drivers/usb/core/generic.c                    |  5 -----
>  include/linux/kernel.h                        | 21 +++++++++++++++++++
>  5 files changed, 21 insertions(+), 36 deletions(-)

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

^ permalink raw reply

* Re: [PATCH] net/skbuff: silence warnings under memory pressure
From: Sergey Senozhatsky @ 2019-09-04  7:00 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Qian Cai, Eric Dumazet, davem, netdev, linux-mm, linux-kernel,
	Petr Mladek, Sergey Senozhatsky, Steven Rostedt,
	Sergey Senozhatsky
In-Reply-To: <20190904064144.GA5487@jagdpanzerIV>

On (09/04/19 15:41), Sergey Senozhatsky wrote:
> But the thing is different in case of dump_stack() + show_mem() +
> some other output. Because now we ratelimit not a single printk() line,
> but hundreds of them. The ratelimit becomes - 10 * $$$ lines in 5 seconds
> (IOW, now we talk about thousands of lines).

And on devices with slow serial consoles this can be somewhat close to
"no ratelimit". *Suppose* that warn_alloc() adds 700 lines each time.
Within 5 seconds we can call warn_alloc() 10 times, which will add 7000
lines to the logbuf. If printk() can evict only 6000 lines in 5 seconds
then we have a growing number of pending logbuf messages.

	-ss

^ permalink raw reply

* Re: [PATCH bpf 2/2] libbpf: remove dependency on barrier.h in xsk.h
From: Yauheni Kaliuta @ 2019-09-04  6:56 UTC (permalink / raw)
  To: Magnus Karlsson; +Cc: Magnus Karlsson, bpf, Network Development
In-Reply-To: <CAJ8uoz2LEun-bjUYQKZdx9NbLBOSRGsZZTWAp10=vhiP7Dms9g@mail.gmail.com>

Hi, Magnus!

>>>>> On Wed, 4 Sep 2019 08:39:24 +0200, Magnus Karlsson  wrote:

 > On Wed, Sep 4, 2019 at 7:32 AM Yauheni Kaliuta
 > <yauheni.kaliuta@redhat.com> wrote:
 >> 
 >> Hi, Magnus!
 >> 
 >> >>>>> On Tue,  9 Apr 2019 08:44:13 +0200, Magnus Karlsson  wrote:
 >> 
 >> > The use of smp_rmb() and smp_wmb() creates a Linux header dependency
 >> > on barrier.h that is uneccessary in most parts. This patch implements
 >> > the two small defines that are needed from barrier.h. As a bonus, the
 >> > new implementations are faster than the default ones as they default
 >> > to sfence and lfence for x86, while we only need a compiler barrier in
 >> > our case. Just as it is when the same ring access code is compiled in
 >> > the kernel.
 >> 
 >> > Fixes: 1cad07884239 ("libbpf: add support for using AF_XDP sockets")
 >> > Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
 >> > ---
 >> >  tools/lib/bpf/xsk.h | 19 +++++++++++++++++--
 >> >  1 file changed, 17 insertions(+), 2 deletions(-)
 >> 
 >> > diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
 >> > index 3638147..317b44f 100644
 >> > --- a/tools/lib/bpf/xsk.h
 >> > +++ b/tools/lib/bpf/xsk.h
 >> > @@ -39,6 +39,21 @@ DEFINE_XSK_RING(xsk_ring_cons);
 >> >  struct xsk_umem;
 >> >  struct xsk_socket;
 >> 
 >> > +#if !defined bpf_smp_rmb && !defined bpf_smp_wmb
 >> > +# if defined(__i386__) || defined(__x86_64__)
 >> > +#  define bpf_smp_rmb() asm volatile("" : : : "memory")
 >> > +#  define bpf_smp_wmb() asm volatile("" : : : "memory")
 >> > +# elif defined(__aarch64__)
 >> > +#  define bpf_smp_rmb() asm volatile("dmb ishld" : : : "memory")
 >> > +#  define bpf_smp_wmb() asm volatile("dmb ishst" : : : "memory")
 >> > +# elif defined(__arm__)
 >> > +#  define bpf_smp_rmb() asm volatile("dmb ish" : : : "memory")
 >> > +#  define bpf_smp_wmb() asm volatile("dmb ishst" : : : "memory")
 >> > +# else
 >> > +#  error Architecture not supported by the XDP socket code in libbpf.
 >> > +# endif
 >> > +#endif
 >> > +
 >> 
 >> What about other architectures then?

 > AF_XDP has not been tested on anything else, as far as I
 > know. But contributions that extend it to more archs are very
 > welcome.

Well, I'll may be try to fetch something from barrier.h's (since
I cannot consider myself as a specialist in the area), but at the
moment the patch breaks the build on that arches.

 > /Magnus

 >> 
 >> >  static inline __u64 *xsk_ring_prod__fill_addr(struct xsk_ring_prod *fill,
 >> >                                            __u32 idx)
 >> >  {
 >> > @@ -119,7 +134,7 @@ static inline void xsk_ring_prod__submit(struct xsk_ring_prod *prod, size_t nb)
 >> >      /* Make sure everything has been written to the ring before signalling
 >> >       * this to the kernel.
 >> >       */
 >> > -    smp_wmb();
 >> > +    bpf_smp_wmb();
 >> 
 >> >      *prod->producer += nb;
 >> >  }
 >> > @@ -133,7 +148,7 @@ static inline size_t xsk_ring_cons__peek(struct xsk_ring_cons *cons,
 >> >              /* Make sure we do not speculatively read the data before
 >> >               * we have received the packet buffers from the ring.
 >> >               */
 >> > -            smp_rmb();
 >> > +            bpf_smp_rmb();
 >> 
 >> >              *idx = cons->cached_cons;
 cons-> cached_cons += entries;
 >> > --
 >> > 2.7.4
 >> 
 >> 
 >> --
 >> WBR,
 >> Yauheni Kaliuta

-- 
WBR,
Yauheni Kaliuta

^ permalink raw reply

* Re: [PATCH] net/skbuff: silence warnings under memory pressure
From: Michal Hocko @ 2019-09-04  6:54 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: Qian Cai, Eric Dumazet, davem, netdev, linux-mm, linux-kernel,
	Petr Mladek, Sergey Senozhatsky, Steven Rostedt
In-Reply-To: <20190904064144.GA5487@jagdpanzerIV>

On Wed 04-09-19 15:41:44, Sergey Senozhatsky wrote:
> On (09/04/19 08:15), Michal Hocko wrote:
> > > If you look at the original report, the failed allocation dump_stack() is,
> > > 
> > >  <IRQ>
> > >  warn_alloc.cold.43+0x8a/0x148
> > >  __alloc_pages_nodemask+0x1a5c/0x1bb0
> > >  alloc_pages_current+0x9c/0x110
> > >  allocate_slab+0x34a/0x11f0
> > >  new_slab+0x46/0x70
> > >  ___slab_alloc+0x604/0x950
> > >  __slab_alloc+0x12/0x20
> > >  kmem_cache_alloc+0x32a/0x400
> > >  __build_skb+0x23/0x60
> > >  build_skb+0x1a/0xb0
> > >  igb_clean_rx_irq+0xafc/0x1010 [igb]
> > >  igb_poll+0x4bb/0xe30 [igb]
> > >  net_rx_action+0x244/0x7a0
> > >  __do_softirq+0x1a0/0x60a
> > >  irq_exit+0xb5/0xd0
> > >  do_IRQ+0x81/0x170
> > >  common_interrupt+0xf/0xf
> > >  </IRQ>
> > > 
> > > Since it has no __GFP_NOWARN to begin with, it will call,
> 
> I think that DEFAULT_RATELIMIT_INTERVAL and DEFAULT_RATELIMIT_BURST
> are good when we ratelimit just a single printk() call, so the ratelimit
> is "max 10 kernel log lines in 5 seconds".

I am sorry, I could have been more explicit when CCing you. Sure the
ratelimit is part of the problem. But I was more interested in the
potential livelock (infinite loop) mentioned by Qian Cai. It is not
important whether we generate one or more lines of output from the
softirq context as long as the printk generates more irq processing
which might end up doing the same. Is this really possible?
-- 
Michal Hocko
SUSE Labs

^ permalink raw reply

* [PATCH] ath9k: Remove unneeded variable to store return value
From: zhong jiang @ 2019-09-04  6:43 UTC (permalink / raw)
  To: kvalo; +Cc: davem, linux-wireless, zhongjiang, netdev, linux-kernel

ath9k_reg_rmw_single do not need return value to cope with different
cases. And change functon return type to void.

Signed-off-by: zhong jiang <zhongjiang@huawei.com>
---
 drivers/net/wireless/ath/ath9k/htc_drv_init.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_init.c b/drivers/net/wireless/ath/ath9k/htc_drv_init.c
index 214c682..d961095 100644
--- a/drivers/net/wireless/ath/ath9k/htc_drv_init.c
+++ b/drivers/net/wireless/ath/ath9k/htc_drv_init.c
@@ -463,7 +463,7 @@ static void ath9k_enable_rmw_buffer(void *hw_priv)
 	atomic_inc(&priv->wmi->m_rmw_cnt);
 }
 
-static u32 ath9k_reg_rmw_single(void *hw_priv,
+static void ath9k_reg_rmw_single(void *hw_priv,
 				 u32 reg_offset, u32 set, u32 clr)
 {
 	struct ath_hw *ah = hw_priv;
@@ -471,7 +471,6 @@ static u32 ath9k_reg_rmw_single(void *hw_priv,
 	struct ath9k_htc_priv *priv = (struct ath9k_htc_priv *) common->priv;
 	struct register_rmw buf, buf_ret;
 	int ret;
-	u32 val = 0;
 
 	buf.reg = cpu_to_be32(reg_offset);
 	buf.set = cpu_to_be32(set);
@@ -485,7 +484,6 @@ static u32 ath9k_reg_rmw_single(void *hw_priv,
 		ath_dbg(common, WMI, "REGISTER RMW FAILED:(0x%04x, %d)\n",
 			reg_offset, ret);
 	}
-	return val;
 }
 
 static u32 ath9k_reg_rmw(void *hw_priv, u32 reg_offset, u32 set, u32 clr)
-- 
1.7.12.4


^ permalink raw reply related

* Re: [PATCH] net/skbuff: silence warnings under memory pressure
From: Sergey Senozhatsky @ 2019-09-04  6:41 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Qian Cai, Eric Dumazet, davem, netdev, linux-mm, linux-kernel,
	Petr Mladek, Sergey Senozhatsky, Steven Rostedt
In-Reply-To: <20190904061501.GB3838@dhcp22.suse.cz>

On (09/04/19 08:15), Michal Hocko wrote:
> > If you look at the original report, the failed allocation dump_stack() is,
> > 
> >  <IRQ>
> >  warn_alloc.cold.43+0x8a/0x148
> >  __alloc_pages_nodemask+0x1a5c/0x1bb0
> >  alloc_pages_current+0x9c/0x110
> >  allocate_slab+0x34a/0x11f0
> >  new_slab+0x46/0x70
> >  ___slab_alloc+0x604/0x950
> >  __slab_alloc+0x12/0x20
> >  kmem_cache_alloc+0x32a/0x400
> >  __build_skb+0x23/0x60
> >  build_skb+0x1a/0xb0
> >  igb_clean_rx_irq+0xafc/0x1010 [igb]
> >  igb_poll+0x4bb/0xe30 [igb]
> >  net_rx_action+0x244/0x7a0
> >  __do_softirq+0x1a0/0x60a
> >  irq_exit+0xb5/0xd0
> >  do_IRQ+0x81/0x170
> >  common_interrupt+0xf/0xf
> >  </IRQ>
> > 
> > Since it has no __GFP_NOWARN to begin with, it will call,

I think that DEFAULT_RATELIMIT_INTERVAL and DEFAULT_RATELIMIT_BURST
are good when we ratelimit just a single printk() call, so the ratelimit
is "max 10 kernel log lines in 5 seconds".

But the thing is different in case of dump_stack() + show_mem() +
some other output. Because now we ratelimit not a single printk() line,
but hundreds of them. The ratelimit becomes - 10 * $$$ lines in 5 seconds
(IOW, now we talk about thousands of lines). Significantly more permissive
ratelimiting.

	-ss

^ permalink raw reply

* Re: [PATCH bpf 2/2] libbpf: remove dependency on barrier.h in xsk.h
From: Magnus Karlsson @ 2019-09-04  6:39 UTC (permalink / raw)
  To: Yauheni Kaliuta; +Cc: Magnus Karlsson, bpf, Network Development
In-Reply-To: <xunyo9007hk9.fsf@redhat.com>

On Wed, Sep 4, 2019 at 7:32 AM Yauheni Kaliuta
<yauheni.kaliuta@redhat.com> wrote:
>
> Hi, Magnus!
>
> >>>>> On Tue,  9 Apr 2019 08:44:13 +0200, Magnus Karlsson  wrote:
>
>  > The use of smp_rmb() and smp_wmb() creates a Linux header dependency
>  > on barrier.h that is uneccessary in most parts. This patch implements
>  > the two small defines that are needed from barrier.h. As a bonus, the
>  > new implementations are faster than the default ones as they default
>  > to sfence and lfence for x86, while we only need a compiler barrier in
>  > our case. Just as it is when the same ring access code is compiled in
>  > the kernel.
>
>  > Fixes: 1cad07884239 ("libbpf: add support for using AF_XDP sockets")
>  > Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
>  > ---
>  >  tools/lib/bpf/xsk.h | 19 +++++++++++++++++--
>  >  1 file changed, 17 insertions(+), 2 deletions(-)
>
>  > diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
>  > index 3638147..317b44f 100644
>  > --- a/tools/lib/bpf/xsk.h
>  > +++ b/tools/lib/bpf/xsk.h
>  > @@ -39,6 +39,21 @@ DEFINE_XSK_RING(xsk_ring_cons);
>  >  struct xsk_umem;
>  >  struct xsk_socket;
>
>  > +#if !defined bpf_smp_rmb && !defined bpf_smp_wmb
>  > +# if defined(__i386__) || defined(__x86_64__)
>  > +#  define bpf_smp_rmb() asm volatile("" : : : "memory")
>  > +#  define bpf_smp_wmb() asm volatile("" : : : "memory")
>  > +# elif defined(__aarch64__)
>  > +#  define bpf_smp_rmb() asm volatile("dmb ishld" : : : "memory")
>  > +#  define bpf_smp_wmb() asm volatile("dmb ishst" : : : "memory")
>  > +# elif defined(__arm__)
>  > +#  define bpf_smp_rmb() asm volatile("dmb ish" : : : "memory")
>  > +#  define bpf_smp_wmb() asm volatile("dmb ishst" : : : "memory")
>  > +# else
>  > +#  error Architecture not supported by the XDP socket code in libbpf.
>  > +# endif
>  > +#endif
>  > +
>
> What about other architectures then?

AF_XDP has not been tested on anything else, as far as I know. But
contributions that extend it to more archs are very welcome.

/Magnus

>
>  >  static inline __u64 *xsk_ring_prod__fill_addr(struct xsk_ring_prod *fill,
>  >                                            __u32 idx)
>  >  {
>  > @@ -119,7 +134,7 @@ static inline void xsk_ring_prod__submit(struct xsk_ring_prod *prod, size_t nb)
>  >      /* Make sure everything has been written to the ring before signalling
>  >       * this to the kernel.
>  >       */
>  > -    smp_wmb();
>  > +    bpf_smp_wmb();
>
>  >      *prod->producer += nb;
>  >  }
>  > @@ -133,7 +148,7 @@ static inline size_t xsk_ring_cons__peek(struct xsk_ring_cons *cons,
>  >              /* Make sure we do not speculatively read the data before
>  >               * we have received the packet buffers from the ring.
>  >               */
>  > -            smp_rmb();
>  > +            bpf_smp_rmb();
>
>  >              *idx = cons->cached_cons;
>  cons-> cached_cons += entries;
>  > --
>  > 2.7.4
>
>
> --
> WBR,
> Yauheni Kaliuta

^ permalink raw reply

* Re: [PATCH 1/2] Fix a NULL-ptr-deref bug in ath6kl_usb_alloc_urb_from_pipe
From: Kalle Valo @ 2019-09-04  6:23 UTC (permalink / raw)
  To: Hui Peng
  Cc: davem, Hui Peng, Mathias Payer, linux-wireless, netdev,
	linux-kernel
In-Reply-To: <20190804002905.11292-1-benquike@gmail.com>

Hui Peng <benquike@gmail.com> wrote:

> The `ar_usb` field of `ath6kl_usb_pipe_usb_pipe` objects
> are initialized to point to the containing `ath6kl_usb` object
> according to endpoint descriptors read from the device side, as shown
> below in `ath6kl_usb_setup_pipe_resources`:
> 
> for (i = 0; i < iface_desc->desc.bNumEndpoints; ++i) {
> 	endpoint = &iface_desc->endpoint[i].desc;
> 
> 	// get the address from endpoint descriptor
> 	pipe_num = ath6kl_usb_get_logical_pipe_num(ar_usb,
> 						endpoint->bEndpointAddress,
> 						&urbcount);
> 	......
> 	// select the pipe object
> 	pipe = &ar_usb->pipes[pipe_num];
> 
> 	// initialize the ar_usb field
> 	pipe->ar_usb = ar_usb;
> }
> 
> The driver assumes that the addresses reported in endpoint
> descriptors from device side  to be complete. If a device is
> malicious and does not report complete addresses, it may trigger
> NULL-ptr-deref `ath6kl_usb_alloc_urb_from_pipe` and
> `ath6kl_usb_free_urb_to_pipe`.
> 
> This patch fixes the bug by preventing potential NULL-ptr-deref
> (CVE-2019-15098).
> 
> Signed-off-by: Hui Peng <benquike@gmail.com>
> Reported-by: Hui Peng <benquike@gmail.com>
> Reported-by: Mathias Payer <mathias.payer@nebelwelt.net>
> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>

Patch applied to ath-next branch of ath.git, thanks.

39d170b3cb62 ath6kl: fix a NULL-ptr-deref bug in ath6kl_usb_alloc_urb_from_pipe()

-- 
https://patchwork.kernel.org/patch/11074655/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


^ permalink raw reply

* Re: [PATCH] wcn36xx: use dynamic allocation for large variables
From: Kalle Valo @ 2019-09-04  6:22 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Arnd Bergmann, Eugene Krasnikov, John W. Linville,
	David S. Miller, YueHaibing, wcn36xx, linux-wireless, netdev,
	linux-kernel, clang-built-linux
In-Reply-To: <20190722145910.1156473-1-arnd@arndb.de>

Arnd Bergmann <arnd@arndb.de> wrote:

> clang triggers a warning about oversized stack frames that gcc does not
> notice because of slightly different inlining decisions:
> 
> ath/wcn36xx/smd.c:1409:5: error: stack frame size of 1040 bytes in function 'wcn36xx_smd_config_bss' [-Werror,-Wframe-larger-than=]
> ath/wcn36xx/smd.c:640:5: error: stack frame size of 1032 bytes in function 'wcn36xx_smd_start_hw_scan' [-Werror,-Wframe-larger-than=]
> 
> Basically the wcn36xx_hal_start_scan_offload_req_msg,
> wcn36xx_hal_config_bss_req_msg_v1, and wcn36xx_hal_config_bss_req_msg
> structures are too large to be put on the kernel stack, but small
> enough that gcc does not warn about them.
> 
> Use kzalloc() to allocate them all. There are similar structures in other
> parts of this driver, but they are all smaller, with the next largest
> stack frame at 480 bytes for wcn36xx_smd_send_beacon.
> 
> Fixes: 8e84c2582169 ("wcn36xx: mac80211 driver for Qualcomm WCN3660/WCN3680 hardware")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>

Patch applied to ath-next branch of ath.git, thanks.

355cf3191201 wcn36xx: use dynamic allocation for large variables

-- 
https://patchwork.kernel.org/patch/11052589/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


^ permalink raw reply

* Re: [PATCH] ath6kl: Fix a possible null-pointer dereference in ath6kl_htc_mbox_create()
From: Kalle Valo @ 2019-09-04  6:21 UTC (permalink / raw)
  To: Jia-Ju Bai; +Cc: davem, linux-wireless, netdev, linux-kernel, Jia-Ju Bai
In-Reply-To: <20190729030305.18410-1-baijiaju1990@gmail.com>

Jia-Ju Bai <baijiaju1990@gmail.com> wrote:

> In ath6kl_htc_mbox_create(), when kzalloc() on line 2855 fails,
> target->dev is assigned to NULL, and ath6kl_htc_mbox_cleanup(target) is
> called on line 2885.
> 
> In ath6kl_htc_mbox_cleanup(), target->dev is used on line 2895:
>     ath6kl_hif_cleanup_scatter(target->dev->ar);
> 
> Thus, a null-pointer dereference may occur.
> 
> To fix this bug, kfree(target) is called and NULL is returned when
> kzalloc() on line 2855 fails.
> 
> This bug is found by a static analysis tool STCheck written by us.
> 
> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>

Patch applied to ath-next branch of ath.git, thanks.

0e7bf23e4967 ath6kl: Fix a possible null-pointer dereference in ath6kl_htc_mbox_create()

-- 
https://patchwork.kernel.org/patch/11063157/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


^ permalink raw reply

* Re: linux-next: build failure after merge of the net-next tree
From: Masahiro Yamada @ 2019-09-04  6:18 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: David Miller, Networking, Linux Next Mailing List,
	Linux Kernel Mailing List, Andrii Nakryiko, Daniel Borkmann
In-Reply-To: <20190904160021.72d104f1@canb.auug.org.au>

On Wed, Sep 4, 2019 at 3:00 PM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Hi all,
>
> After merging the net-next tree, today's linux-next build (arm
> multi_v7_defconfig) failed like this:
>
> scripts/link-vmlinux.sh: 74: Bad substitution
>
> Caused by commit
>
>   341dfcf8d78e ("btf: expose BTF info through sysfs")
>
> interacting with commit
>
>   1267f9d3047d ("kbuild: add $(BASH) to run scripts with bash-extension")
>
> from the kbuild tree.


I knew that they were using bash-extension
in the #!/bin/sh script.  :-D


In fact, I wrote my patch in order to break their code
and  make btf people realize that they were doing wrong.



> The change in the net-next tree turned link-vmlinux.sh into a bash script
> (I think).
>
> I have applied the following patch for today:


But, this is a temporary fix only for linux-next.

scripts/link-vmlinux.sh does not need to use the
bash-extension ${@:2} in the first place.

I hope btf people will write the correct code.

Thanks.




> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Wed, 4 Sep 2019 15:43:41 +1000
> Subject: [PATCH] link-vmlinux.sh is now a bash script
>
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
>  Makefile                | 4 ++--
>  scripts/link-vmlinux.sh | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index ac97fb282d99..523d12c5cebe 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1087,7 +1087,7 @@ ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(SRCARCH)/Makefile.postlink)
>
>  # Final link of vmlinux with optional arch pass after final link
>  cmd_link-vmlinux =                                                 \
> -       $(CONFIG_SHELL) $< $(LD) $(KBUILD_LDFLAGS) $(LDFLAGS_vmlinux) ;    \
> +       $(BASH) $< $(LD) $(KBUILD_LDFLAGS) $(LDFLAGS_vmlinux) ;    \
>         $(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)
>
>  vmlinux: scripts/link-vmlinux.sh autoksyms_recursive $(vmlinux-deps) FORCE
> @@ -1403,7 +1403,7 @@ clean: rm-files := $(CLEAN_FILES)
>  PHONY += archclean vmlinuxclean
>
>  vmlinuxclean:
> -       $(Q)$(CONFIG_SHELL) $(srctree)/scripts/link-vmlinux.sh clean
> +       $(Q)$(BASH) $(srctree)/scripts/link-vmlinux.sh clean
>         $(Q)$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) clean)
>
>  clean: archclean vmlinuxclean
> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> index f7edb75f9806..ea1f8673869d 100755
> --- a/scripts/link-vmlinux.sh
> +++ b/scripts/link-vmlinux.sh
> @@ -1,4 +1,4 @@
> -#!/bin/sh
> +#!/bin/bash
>  # SPDX-License-Identifier: GPL-2.0
>  #
>  # link vmlinux
> --
> 2.23.0.rc1
>
> --
> Cheers,
> Stephen Rothwell



--
Best Regards
Masahiro Yamada

^ permalink raw reply

* Re: [PATCH] net/skbuff: silence warnings under memory pressure
From: Michal Hocko @ 2019-09-04  6:15 UTC (permalink / raw)
  To: Qian Cai; +Cc: Eric Dumazet, davem, netdev, linux-mm, linux-kernel
In-Reply-To: <1567546948.5576.68.camel@lca.pw>

> On Tue, 2019-09-03 at 20:53 +0200, Michal Hocko wrote:
> > On Tue 03-09-19 11:42:22, Qian Cai wrote:
> > > On Tue, 2019-09-03 at 15:22 +0200, Michal Hocko wrote:
> > > > On Fri 30-08-19 18:15:22, Eric Dumazet wrote:
> > > > > If there is a risk of flooding the syslog, we should fix this
> > > > > generically
> > > > > in mm layer, not adding hundred of __GFP_NOWARN all over the places.
> > > > 
> > > > We do already ratelimit in warn_alloc. If it isn't sufficient then we
> > > > can think of a different parameters. Or maybe it is the ratelimiting
> > > > which doesn't work here. Hard to tell and something to explore.
> > > 
> > > The time-based ratelimit won't work for skb_build() as when a system under
> > > memory pressure, and the CPU is fast and IO is so slow, it could take a long
> > > time to swap and trigger OOM.
> > 
> > I really do not understand what does OOM and swapping have to do with
> > the ratelimiting here. The sole purpose of the ratelimit is to reduce
> > the amount of warnings to be printed. Slow IO might have an effect on
> > when the OOM killer is invoked but atomic allocations are not directly
> > dependent on IO.
> 
> When there is a heavy memory pressure, the system is trying hard to reclaim
> memory to fill up the watermark. However, the IO is slow to page out, but the
> memory pressure keep draining atomic reservoir, and some of those skb_build()
> will fail eventually.

Yes this is true but this has nothing to do with the ratelimitted
warn_alloc AFAICS. It is natural that atomic allocations are going
to fail more likely under extreme memory pressure but we are talking
about an excessive amount of debugging output that is generated and
that should be throttled. And that's why we have ratelimit there. If it
doesn't work well then we should look into why.

> Only if there is a fast IO, it will finish swapping sooner and then invoke the
> OOM to end the memory pressure.
-- 
Michal Hocko
SUSE Labs

^ permalink raw reply

* Re: [PATCH] net/skbuff: silence warnings under memory pressure
From: Michal Hocko @ 2019-09-04  6:15 UTC (permalink / raw)
  To: Qian Cai
  Cc: Eric Dumazet, davem, netdev, linux-mm, linux-kernel, Petr Mladek,
	Sergey Senozhatsky, Steven Rostedt
In-Reply-To: <1567546948.5576.68.camel@lca.pw>

Cc printk maintainers

On Tue 03-09-19 17:42:28, Qian Cai wrote:
> > > I suppose what happens is those skb_build() allocations are from softirq,
> > > and
> > > once one of them failed, it calls printk() which generates more interrupts.
> > > Hence, the infinite loop.
> > 
> > Please elaborate more.
> > 
> 
> If you look at the original report, the failed allocation dump_stack() is,
> 
>  <IRQ>
>  warn_alloc.cold.43+0x8a/0x148
>  __alloc_pages_nodemask+0x1a5c/0x1bb0
>  alloc_pages_current+0x9c/0x110
>  allocate_slab+0x34a/0x11f0
>  new_slab+0x46/0x70
>  ___slab_alloc+0x604/0x950
>  __slab_alloc+0x12/0x20
>  kmem_cache_alloc+0x32a/0x400
>  __build_skb+0x23/0x60
>  build_skb+0x1a/0xb0
>  igb_clean_rx_irq+0xafc/0x1010 [igb]
>  igb_poll+0x4bb/0xe30 [igb]
>  net_rx_action+0x244/0x7a0
>  __do_softirq+0x1a0/0x60a
>  irq_exit+0xb5/0xd0
>  do_IRQ+0x81/0x170
>  common_interrupt+0xf/0xf
>  </IRQ>
> 
> Since it has no __GFP_NOWARN to begin with, it will call,
> 
> printk
>   vprintk_default
>     vprintk_emit
>       wake_up_klogd
>         irq_work_queue
>           __irq_work_queue_local
>             arch_irq_work_raise
>               apic->send_IPI_self(IRQ_WORK_VECTOR)
>                 smp_irq_work_interrupt
>                   exiting_irq
>                     irq_exit
> 
> and end up processing pending net_rx_action softirqs again which are plenty due
> to connected via ssh etc.
-- 
Michal Hocko
SUSE Labs

^ permalink raw reply

* Re: [PATCH -next] carl9170: remove set but not used variable 'udev'
From: Kalle Valo @ 2019-09-04  6:10 UTC (permalink / raw)
  To: YueHaibing
  Cc: chunkeey, linux-kernel, netdev, linux-wireless, davem, YueHaibing
In-Reply-To: <20190702141207.47552-1-yuehaibing@huawei.com>

YueHaibing <yuehaibing@huawei.com> wrote:

> Fixes gcc '-Wunused-but-set-variable' warning:
> 
> drivers/net/wireless/ath/carl9170/usb.c: In function carl9170_usb_disconnect:
> drivers/net/wireless/ath/carl9170/usb.c:1110:21:
>  warning: variable udev set but not used [-Wunused-but-set-variable]
> 
> It is not use since commit feb09b293327 ("carl9170:
> fix misuse of device driver API")
> 
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> Acked-by: Christian Lamparter <chunkeey@gmail.com>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>

Patch applied to ath-next branch of ath.git, thanks.

68092f9cf932 carl9170: remove set but not used variable 'udev'

-- 
https://patchwork.kernel.org/patch/11027909/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


^ permalink raw reply

* Re: [PATCH] wil6210: Delete an unnecessary kfree() call in wil_tid_ampdu_rx_alloc()
From: Kalle Valo @ 2019-09-04  6:08 UTC (permalink / raw)
  To: Markus Elfring
  Cc: linux-wireless, netdev, wil6210, David S. Miller, Maya Erez, LKML,
	kernel-janitors
In-Reply-To: <b9620e49-618d-b392-6456-17de5807df75@web.de>

Markus Elfring <Markus.Elfring@web.de> wrote:

> A null pointer would be passed to a call of the function “kfree”
> directly after a call of the function “kcalloc” failed at one place.
> Remove this superfluous function call.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> Reviewed-by: Maya Erez <merez@codeaurora.org>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>

Patch applied to ath-next branch of ath.git, thanks.

d20b1e6c8307 wil6210: Delete an unnecessary kfree() call in wil_tid_ampdu_rx_alloc()

-- 
https://patchwork.kernel.org/patch/11117119/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox