Re: [PATCH] rt2x00: fix spelling mistake in various macros, UKNOWN -> UNKNOWN
On Wed, Apr 18, 2018 at 12:47:50PM +0100, Colin King wrote: > From: Colin Ian King <colin.k...@canonical.com> > > Rename several macros that contain mispellings of UNKNOWN > > Signed-off-by: Colin Ian King <colin.k...@canonical.com> Acked-by: Stanislaw Gruszka <sgrus...@redhat.com>
Re: [PATCH] rt2x00: Delete an error message for a failed memory allocation in rt2x00queue_allocate()
On Fri, Dec 29, 2017 at 10:18:14PM +0100, SF Markus Elfring wrote: > From: Markus Elfring <elfr...@users.sourceforge.net> > Date: Fri, 29 Dec 2017 22:11:42 +0100 > > Omit an extra message for a memory allocation failure in this function. > > This issue was detected by using the Coccinelle software. > > Signed-off-by: Markus Elfring <elfr...@users.sourceforge.net> Acked-by: Stanislaw Gruszka <sgrus...@redhat.com>
Re: [PATCH] wireless: iwlegacy: Convert timers to use timer_setup()
On Mon, Oct 16, 2017 at 04:37:44PM -0700, Kees Cook wrote: > In preparation for unconditionally passing the struct timer_list pointer to > all timer callbacks, switch to using the new timer_setup() and from_timer() > to pass the timer pointer explicitly. > > Cc: Stanislaw Gruszka <sgrus...@redhat.com> > Cc: Kalle Valo <kv...@codeaurora.org> > Cc: linux-wirel...@vger.kernel.org > Cc: netdev@vger.kernel.org > Signed-off-by: Kees Cook <keesc...@chromium.org> Acked-by: Stanislaw Gruszka <sgrus...@redhat.com>
Re: usb/net/rt2x00: warning in rt2800_eeprom_word_index
Hi Dmitry On Sat, Oct 14, 2017 at 04:38:03PM +0200, Dmitry Vyukov wrote: > On Thu, Oct 12, 2017 at 9:25 AM, Stanislaw Gruszka <sgrus...@redhat.com> > wrote: > > Hi > > > > On Mon, Oct 09, 2017 at 07:50:53PM +0200, Andrey Konovalov wrote: > >> I've got the following report while fuzzing the kernel with syzkaller. > >> > >> On commit 8a5776a5f49812d29fe4b2d0a2d71675c3facf3f (4.14-rc4). > >> > >> I'm not sure whether this is a bug in the driver, or just a way to > >> report misbehaving device. In the latter case this shouldn't be a > >> WARN() call, since WARN() means bug in the kernel. > > > > This is about wrong EEPROM, which reported 3 tx streams on > > non 3 antenna device. I think WARN() is justified and thanks > > to the call trace I was actually able to to understand what > > happened. > > > > In general I do not think WARN() only means a kernel bug, it > > can be F/W or H/W bug too. > > Hi Stanislaw, > > Printing messages is fine. Printing stacks is fine. Just please make > them distinguishable from kernel bugs and don't kill the whole > possibility of automated Linux kernel testing. That's an important > capability. We do not distinguish between bugs and other problems when WARN() is used in (wireless) drivers, what I think is correct, taking comment from include/asm-generic/bug.h : /* * WARN(), WARN_ON(), WARN_ON_ONCE, and so on can be used to report * significant issues that need prompt attention if they should ever * appear at runtime. Use the versions with printk format strings * to provide better diagnostics. */ Historically we have BUG() to mark the bugs, but usage if it is not recommended as it can kill the system, so for anything that can be recovered in runtime - WARN() is recommended. Perhaps we can introduce another helper like PROBLEM() for marking situations when something is wrong, but it is not a bug. However I'm not even sure at what extent it can be used, since for many cases if not the most, driver author can not tell apriori if the problem is a bug in the driver or HW/FW misbehaviour (or maybe particular issue can happen because of both). Thanks Stanislaw
Re: usb/net/rt2x00: warning in rt2800_eeprom_word_index
Hi On Mon, Oct 09, 2017 at 07:50:53PM +0200, Andrey Konovalov wrote: > I've got the following report while fuzzing the kernel with syzkaller. > > On commit 8a5776a5f49812d29fe4b2d0a2d71675c3facf3f (4.14-rc4). > > I'm not sure whether this is a bug in the driver, or just a way to > report misbehaving device. In the latter case this shouldn't be a > WARN() call, since WARN() means bug in the kernel. This is about wrong EEPROM, which reported 3 tx streams on non 3 antenna device. I think WARN() is justified and thanks to the call trace I was actually able to to understand what happened. In general I do not think WARN() only means a kernel bug, it can be F/W or H/W bug too. Thanks Stanislaw
Re: [PATCH] wireless: iwlegacy: make const array static to shink object code size Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit
On Thu, Sep 21, 2017 at 11:56:30PM +0100, Colin King wrote: > From: Colin Ian King <colin.k...@canonical.com> > > Don't populate const array ac_to_fifo on the stack in an inlined > function, instead make it static. Makes the object code smaller > by over 800 bytes: > >text data bss dec hex filename > 159029 331541216 193399 2f377 4965-mac.o > >text data bss dec hex filename > 158122 332501216 192588 2f04c 4965-mac.o > > (gcc version 7.2.0 x86_64) > > Signed-off-by: Colin Ian King <colin.k...@canonical.com> Content type information was added at the end of the topic, but I think Kalle can fix that when he will be committing the patch. Acked-by: Stanislaw Gruszka <sgrus...@redhat.com>
Re: [PATCH] rt2x00: make const array glrt_table static
On Tue, Jul 11, 2017 at 12:47:33PM +0100, Colin King wrote: > From: Colin Ian King <colin.k...@canonical.com> > > Don't populate array glrt_table on the stack but make it static. > Makes the object code a smaller by over 670 bytes: > > Before: >text data bss dec hex filename > 131772 4733 0 136505 21539 rt2800lib.o > > After: >text data bss dec hex filename > 131043 4789 0 135832 21298 rt2800lib.o > > Signed-off-by: Colin Ian King <colin.k...@canonical.com> Acked-by: Stanislaw Gruszka <sgrus...@redhat.com> I wonder why compiler do not optimize by itself since array is const, but patch is ok. Stanislaw
Re: [PATCH] rt2x00: improve calling conventions for register accessors
On Tue, May 16, 2017 at 01:55:17PM +0200, Stanislaw Gruszka wrote:
> On Mon, May 15, 2017 at 10:39:51AM -0400, David Miller wrote:
> > From: Stanislaw Gruszka <sgrus...@redhat.com>
> > Date: Mon, 15 May 2017 16:28:01 +0200
> >
> > > On Mon, May 15, 2017 at 03:46:55PM +0200, Arnd Bergmann wrote:
> > >> With CONFIG_KASAN enabled and gcc-7, we get a warning about rather high
> > >> stack usage (with a private patch set I have to turn on this warning,
> > >> which I intend to get into the next kernel release):
> > >>
> > >> wireless/ralink/rt2x00/rt2800lib.c: In function
> > >> 'rt2800_bw_filter_calibration':
> > >> wireless/ralink/rt2x00/rt2800lib.c:7990:1: error: the frame size of 2144
> > >> bytes is larger than 1536 bytes [-Werror=frame-larger-than=]
> > >>
> > >> The problem is that KASAN inserts a redzone around each local variable
> > >> that
> > >> gets passed by reference, and the newly added function has a lot of them.
> > >> We can easily avoid that here by changing the calling convention to have
> > >> the output as the return value of the function. This should also results
> > >> in
> > >> smaller object code, saving around 4KB in .text with KASAN, or 2KB
> > >> without
> > >> KASAN.
> > >>
> > >> Fixes: 41977e86c984 ("rt2x00: add support for MT7620")
> > >> Signed-off-by: Arnd Bergmann <a...@arndb.de>
> > >> ---
> > >> drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 319
> > >> +
> > >> 1 file changed, 164 insertions(+), 155 deletions(-)
> > >
> > > We have read(, ) calling convention since forever in rt2x00 and that
> > > was never a problem. I dislike to change that now to make some tools
> > > happy, I think problem should be fixed in the tools instead.
> >
> > Passing return values by reference is and always has been a really
> > poor way to achieve what these functions are doing.
> >
> > And frankly, whilst the tool could see what's going on here better, we
> > should be making code easier rather than more difficult to audit.
> >
> > I am therefore very much in favor of Arnd's change.
> >
> > This isn't even a situation where there are multiple return values,
> > such as needing to signal an error and return an unsigned value at the
> > same time.
> >
> > These functions return _one_ value, and therefore they should be
> > returned as a true return value.
>
> In rt2x00 driver we use poor convention in other kind of registers
> accessors like bbp, mac, eeprom. I dislike to changing only rfcsr
> accessors and leaving others in the old way. And changing all accessors
> would be massive and error prone change, which I'm not prefer either.
>
> Arnd, could this be fixed by refactoring rt2800_bw_filter_calibration()
> function (which is enormous and definitely should be split into smaller
> subroutines) ? If not, I would accept this patch.
Does below patch make things better with KASAN compilation ?
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
index d11c7b2..4b85ef3 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
@@ -7740,6 +7740,39 @@ static char rt2800_lp_tx_filter_bw_cal(struct rt2x00_dev
*rt2x00dev)
return cal_val;
}
+#define RF_SAVE_NUM 24
+#define BBP_SAVE_NUM 3
+static const int rf_save_ids[RF_SAVE_NUM] = {0, 1, 3, 4, 5, 6, 7, 8,
+ 17, 18, 19, 20, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 58, 59};
+
+static void rt2800_phy_save(struct rt2x00_dev *rt2x00dev,
+ u8 *const bbp_regs, u8 *const rf_regs)
+{
+ int i;
+
+ rt2800_bbp_read(rt2x00dev, 23, _regs[0]);
+
+ rt2800_bbp_dcoc_read(rt2x00dev, 0, _regs[1]);
+ rt2800_bbp_dcoc_read(rt2x00dev, 2, _regs[2]);
+
+ for (i = 0; i < RF_SAVE_NUM; i++)
+ rt2800_rfcsr_read_bank(rt2x00dev, 5, rf_save_ids[i],
_regs[i]);
+}
+
+static void rt2800_phy_restore(struct rt2x00_dev *rt2x00dev,
+ const u8 *const bbp_regs, const u8 *const
rf_regs)
+{
+ int i;
+
+ for (i = 0; i < RF_SAVE_NUM; i++)
+ rt2800_rfcsr_write_bank(rt2x00dev, 5, rf_save_ids[i],
rf_regs[i]);
+
+ rt2800_bbp_write(rt2x00dev, 23, bbp_regs[0]);
+
+ rt2800_bbp_dcoc_write(rt2x00dev, 0, bbp_regs[1]);
+ rt2800_bbp_dcoc_write(rt2x00dev, 2, bbp_regs[2]);
+}
+
static void rt2800_bw_filter_calibration(struct rt2x00_dev *rt2x00dev,
Re: [PATCH] rt2x00: improve calling conventions for register accessors
On Tue, May 16, 2017 at 01:58:56PM +0200, Johannes Berg wrote: > On Tue, 2017-05-16 at 13:55 +0200, Stanislaw Gruszka wrote: > > > > In rt2x00 driver we use poor convention in other kind of registers > > accessors like bbp, mac, eeprom. I dislike to changing only rfcsr > > accessors and leaving others in the old way. And changing all > > accessors would be massive and error prone change, which I'm not > > prefer either. > > That's a stupid argument, but for the sake of it - the conversion can > easily be done with coccinelle/spatch without being "error prone". Sure, but still I think it would be preferable to fix newly added rt2800_bw_filter_calibration() function, instead of ancient rfcsr accessors. Stanislaw
Re: [PATCH] rt2x00: improve calling conventions for register accessors
On Mon, May 15, 2017 at 10:39:51AM -0400, David Miller wrote:
> From: Stanislaw Gruszka <sgrus...@redhat.com>
> Date: Mon, 15 May 2017 16:28:01 +0200
>
> > On Mon, May 15, 2017 at 03:46:55PM +0200, Arnd Bergmann wrote:
> >> With CONFIG_KASAN enabled and gcc-7, we get a warning about rather high
> >> stack usage (with a private patch set I have to turn on this warning,
> >> which I intend to get into the next kernel release):
> >>
> >> wireless/ralink/rt2x00/rt2800lib.c: In function
> >> 'rt2800_bw_filter_calibration':
> >> wireless/ralink/rt2x00/rt2800lib.c:7990:1: error: the frame size of 2144
> >> bytes is larger than 1536 bytes [-Werror=frame-larger-than=]
> >>
> >> The problem is that KASAN inserts a redzone around each local variable that
> >> gets passed by reference, and the newly added function has a lot of them.
> >> We can easily avoid that here by changing the calling convention to have
> >> the output as the return value of the function. This should also results in
> >> smaller object code, saving around 4KB in .text with KASAN, or 2KB without
> >> KASAN.
> >>
> >> Fixes: 41977e86c984 ("rt2x00: add support for MT7620")
> >> Signed-off-by: Arnd Bergmann <a...@arndb.de>
> >> ---
> >> drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 319
> >> +
> >> 1 file changed, 164 insertions(+), 155 deletions(-)
> >
> > We have read(, ) calling convention since forever in rt2x00 and that
> > was never a problem. I dislike to change that now to make some tools
> > happy, I think problem should be fixed in the tools instead.
>
> Passing return values by reference is and always has been a really
> poor way to achieve what these functions are doing.
>
> And frankly, whilst the tool could see what's going on here better, we
> should be making code easier rather than more difficult to audit.
>
> I am therefore very much in favor of Arnd's change.
>
> This isn't even a situation where there are multiple return values,
> such as needing to signal an error and return an unsigned value at the
> same time.
>
> These functions return _one_ value, and therefore they should be
> returned as a true return value.
In rt2x00 driver we use poor convention in other kind of registers
accessors like bbp, mac, eeprom. I dislike to changing only rfcsr
accessors and leaving others in the old way. And changing all accessors
would be massive and error prone change, which I'm not prefer either.
Arnd, could this be fixed by refactoring rt2800_bw_filter_calibration()
function (which is enormous and definitely should be split into smaller
subroutines) ? If not, I would accept this patch.
Thanks
Stanislaw
Re: [PATCH] rt2x00: improve calling conventions for register accessors
On Mon, May 15, 2017 at 03:46:55PM +0200, Arnd Bergmann wrote:
> With CONFIG_KASAN enabled and gcc-7, we get a warning about rather high
> stack usage (with a private patch set I have to turn on this warning,
> which I intend to get into the next kernel release):
>
> wireless/ralink/rt2x00/rt2800lib.c: In function
> 'rt2800_bw_filter_calibration':
> wireless/ralink/rt2x00/rt2800lib.c:7990:1: error: the frame size of 2144
> bytes is larger than 1536 bytes [-Werror=frame-larger-than=]
>
> The problem is that KASAN inserts a redzone around each local variable that
> gets passed by reference, and the newly added function has a lot of them.
> We can easily avoid that here by changing the calling convention to have
> the output as the return value of the function. This should also results in
> smaller object code, saving around 4KB in .text with KASAN, or 2KB without
> KASAN.
>
> Fixes: 41977e86c984 ("rt2x00: add support for MT7620")
> Signed-off-by: Arnd Bergmann
> ---
> drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 319
> +
> 1 file changed, 164 insertions(+), 155 deletions(-)
We have read(, ) calling convention since forever in rt2x00 and that
was never a problem. I dislike to change that now to make some tools
happy, I think problem should be fixed in the tools instead.
Stanislaw
[PATCH 4.11] genetlink: fix counting regression on ctrl_dumpfamily()
Commit 2ae0f17df1cd ("genetlink: use idr to track families") replaced
if (++n < fams_to_skip)
continue;
into:
if (n++ < fams_to_skip)
continue;
This subtle change cause that on retry ctrl_dumpfamily() call we omit
one family that failed to do ctrl_fill_info() on previous call, because
cb->args[0] = n number counts also family that failed to do
ctrl_fill_info().
Patch fixes the problem and avoid confusion in the future just decrease
n counter when ctrl_fill_info() fail.
User visible problem caused by this bug is failure to get access to
some genetlink family i.e. nl80211. However problem is reproducible
only if number of registered genetlink families is big enough to
cause second call of ctrl_dumpfamily().
Cc: Xose Vazquez Perez <xose.vazq...@gmail.com>
Cc: Larry Finger <larry.fin...@lwfinger.net>
Cc: Johannes Berg <johan...@sipsolutions.net>
Fixes: 2ae0f17df1cd ("genetlink: use idr to track families")
Signed-off-by: Stanislaw Gruszka <sgrus...@redhat.com>
---
Dave, please also target this for 4.10+ -stable.
net/netlink/genetlink.c |4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index fb6e10f..92e0981 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -783,8 +783,10 @@ static int ctrl_dumpfamily(struct sk_buff *skb, struct
netlink_callback *cb)
if (ctrl_fill_info(rt, NETLINK_CB(cb->skb).portid,
cb->nlh->nlmsg_seq, NLM_F_MULTI,
- skb, CTRL_CMD_NEWFAMILY) < 0)
+ skb, CTRL_CMD_NEWFAMILY) < 0) {
+ n--;
break;
+ }
}
cb->args[0] = n;
--
1.7.1
Re: [PATCH v2] ethtool: do not vzalloc(0) on registers dump
On Thu, Feb 02, 2017 at 09:27:18AM -0500, John W. Linville wrote: > > - regbuf = vzalloc(reglen); > > - if (reglen && !regbuf) > > - return -ENOMEM; > > + regbuf = NULL; > > Any reason to prefer this over changing the declaration to include > the assignment? > > void *regbuf = NULL; I've chosen this form to have initialization near the vzalloc() call, after sanity checks, however I don't think it's better or worse over declaration initialization. Stanislaw
[PATCH v2] ethtool: do not vzalloc(0) on registers dump
If ->get_regs_len() callback return 0, we allocate 0 bytes of memory,
what print ugly warning in dmesg, which can be found further below.
This happen on mac80211 devices where ieee80211_get_regs_len() just
return 0 and driver only fills ethtool_regs structure and actually
do not provide any dump. However I assume this can happen on other
drivers i.e. when for some devices driver provide regs dump and for
others do not. Hence preventing to to print warning in ethtool code
seems to be reasonable.
ethtool: vmalloc: allocation failure: 0 bytes,
mode:0x24080c2(GFP_KERNEL|__GFP_HIGHMEM|__GFP_ZERO)
Call Trace:
[] dump_stack+0x63/0x8c
[] warn_alloc+0x13f/0x170
[] __vmalloc_node_range+0x1e6/0x2c0
[] vzalloc+0x54/0x60
[] dev_ethtool+0xb4c/0x1b30
[] dev_ioctl+0x181/0x520
[] sock_do_ioctl+0x42/0x50
Mem-Info:
active_anon:435809 inactive_anon:173951 isolated_anon:0
active_file:835822 inactive_file:196932 isolated_file:0
unevictable:0 dirty:8 writeback:0 unstable:0
slab_reclaimable:157732 slab_unreclaimable:10022
mapped:83042 shmem:306356 pagetables:9507 bounce:0
free:130041 free_pcp:1080 free_cma:0
Node 0 active_anon:1743236kB inactive_anon:695804kB active_file:3343288kB
inactive_file:787728kB unevictable:0kB isolated(anon):0kB isolated(file):0kB
mapped:332168kB dirty:32kB writeback:0kB shmem:0kB shmem_thp: 0kB
shmem_pmdmapped: 0kB anon_thp: 1225424kB writeback_tmp:0kB unstable:0kB
pages_scanned:0 all_unreclaimable? no
Node 0 DMA free:15900kB min:136kB low:168kB high:200kB active_anon:0kB
inactive_anon:0kB active_file:0kB inactive_file:0kB unevictable:0kB
writepending:0kB present:15984kB managed:15900kB mlocked:0kB
slab_reclaimable:0kB slab_unreclaimable:0kB kernel_stack:0kB pagetables:0kB
bounce:0kB free_pcp:0kB local_pcp:0kB free_cma:0kB
lowmem_reserve[]: 0 3187 7643 7643
Node 0 DMA32 free:419732kB min:28124kB low:35152kB high:42180kB
active_anon:541180kB inactive_anon:248988kB active_file:1466388kB
inactive_file:389632kB unevictable:0kB writepending:0kB present:3370280kB
managed:3290932kB mlocked:0kB slab_reclaimable:217184kB
slab_unreclaimable:4180kB kernel_stack:160kB pagetables:984kB bounce:0kB
free_pcp:2236kB local_pcp:660kB free_cma:0kB
lowmem_reserve[]: 0 0 4456 4456
Signed-off-by: Stanislaw Gruszka <sgrus...@redhat.com>
---
v1 -> v2: nullify regbuf to avoid using uninitialized variable in line:
if (regbuf && copy_to_user(useraddr, regbuf, regs.len))
net/core/ethtool.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 6b3eee0..4b6dc9b 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -1405,9 +1405,12 @@ static int ethtool_get_regs(struct net_device *dev, char
__user *useraddr)
if (regs.len > reglen)
regs.len = reglen;
- regbuf = vzalloc(reglen);
- if (reglen && !regbuf)
- return -ENOMEM;
+ regbuf = NULL;
+ if (reglen) {
+ regbuf = vzalloc(reglen);
+ if (!regbuf)
+ return -ENOMEM;
+ }
ops->get_regs(dev, , regbuf);
--
1.8.3.1
[PATCH] ethtool: do not vzalloc(0) on registers dump
If ->get_regs_len() callback return 0, we allocate 0 bytes of memory,
what print ugly warning in dmesg, which can be found further below.
This happen on mac80211 devices where ieee80211_get_regs_len() just
return 0 and driver only fills ethtool_regs structure and actually
do not provide any dump. However I assume this can happen on other
drivers i.e. when for some devices driver provide regs dump and for
others do not. Hence preventing to to print warning in ethtool code
seems to be reasonable.
ethtool: vmalloc: allocation failure: 0 bytes,
mode:0x24080c2(GFP_KERNEL|__GFP_HIGHMEM|__GFP_ZERO)
Call Trace:
[] dump_stack+0x63/0x8c
[] warn_alloc+0x13f/0x170
[] __vmalloc_node_range+0x1e6/0x2c0
[] vzalloc+0x54/0x60
[] dev_ethtool+0xb4c/0x1b30
[] dev_ioctl+0x181/0x520
[] sock_do_ioctl+0x42/0x50
Mem-Info:
active_anon:435809 inactive_anon:173951 isolated_anon:0
active_file:835822 inactive_file:196932 isolated_file:0
unevictable:0 dirty:8 writeback:0 unstable:0
slab_reclaimable:157732 slab_unreclaimable:10022
mapped:83042 shmem:306356 pagetables:9507 bounce:0
free:130041 free_pcp:1080 free_cma:0
Node 0 active_anon:1743236kB inactive_anon:695804kB active_file:3343288kB
inactive_file:787728kB unevictable:0kB isolated(anon):0kB isolated(file):0kB
mapped:332168kB dirty:32kB writeback:0kB shmem:0kB shmem_thp: 0kB
shmem_pmdmapped: 0kB anon_thp: 1225424kB writeback_tmp:0kB unstable:0kB
pages_scanned:0 all_unreclaimable? no
Node 0 DMA free:15900kB min:136kB low:168kB high:200kB active_anon:0kB
inactive_anon:0kB active_file:0kB inactive_file:0kB unevictable:0kB
writepending:0kB present:15984kB managed:15900kB mlocked:0kB
slab_reclaimable:0kB slab_unreclaimable:0kB kernel_stack:0kB pagetables:0kB
bounce:0kB free_pcp:0kB local_pcp:0kB free_cma:0kB
lowmem_reserve[]: 0 3187 7643 7643
Node 0 DMA32 free:419732kB min:28124kB low:35152kB high:42180kB
active_anon:541180kB inactive_anon:248988kB active_file:1466388kB
inactive_file:389632kB unevictable:0kB writepending:0kB present:3370280kB
managed:3290932kB mlocked:0kB slab_reclaimable:217184kB
slab_unreclaimable:4180kB kernel_stack:160kB pagetables:984kB bounce:0kB
free_pcp:2236kB local_pcp:660kB free_cma:0kB
lowmem_reserve[]: 0 0 4456 4456
Signed-off-by: Stanislaw Gruszka <sgrus...@redhat.com>
---
net/core/ethtool.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 6b3eee0..3ec897f 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -1405,9 +1405,11 @@ static int ethtool_get_regs(struct net_device *dev, char
__user *useraddr)
if (regs.len > reglen)
regs.len = reglen;
- regbuf = vzalloc(reglen);
- if (reglen && !regbuf)
- return -ENOMEM;
+ if (reglen) {
+ regbuf = vzalloc(reglen);
+ if (!regbuf)
+ return -ENOMEM;
+ }
ops->get_regs(dev, , regbuf);
--
1.8.3.1
Re: [PATCH 08/26] iwlegacy: constify local structures
On Sun, Sep 11, 2016 at 03:05:50PM +0200, Julia Lawall wrote: > For structure types defined in the same file or local header files, find > top-level static structure declarations that have the following > properties: > 1. Never reassigned. > 2. Address never taken > 3. Not passed to a top-level macro call > 4. No pointer or array-typed field passed to a function or stored in a > variable. > Declare structures having all of these properties as const. > > Done using Coccinelle. > Based on a suggestion by Joe Perches <j...@perches.com>. > > Signed-off-by: Julia Lawall <julia.law...@lip6.fr> Acked-by: Stanislaw Gruszka <sgrus...@redhat.com>
Re: [PATCH RESEND] iwlwifi, Do not implement thermal zone unless ucode is loaded
On Thu, Jul 14, 2016 at 09:44:22AM +, Grumbach, Emmanuel wrote: > > If I understad correctly this error happen 100% of the time, not only during > > init. Hence seems there is an issue here, i.e. cur_ucode is not marked > > correctly as IWL_UCODE_REGULAR or iwl_mvm_get_temp() fail 100% of the > > time (iwl_mvm_is_tt_in_fw() incorrecly return true on Prarit device ? ). > > Cur_ucode will not be IWL_UCODE_REGULAR until you load the firmware which > will happen upon ifup. Then creating thermal_device on ifup looks more reasonable to me. Otherwise we can create device that can be non-functional virtually forever, i.e. when soft RFKILL is enabled. However I admit that creating thermal_device when HW is detected has some advantages too. Stanislaw
Re: [PATCH RESEND] iwlwifi, Do not implement thermal zone unless ucode is loaded
On Mon, Jul 11, 2016 at 06:27:30PM +, Grumbach, Emmanuel wrote: > I guess that works, but it seems wrong to me. Usually, registration > should happen only upon INIT, and yes, at that time the firmware is not > ready to provide the information yet. > > > > As can be seen in the current code base, iwl_mvm_tzone_get_temp() > > will return > > -EIO 100% of the time when the firmware doesn't support reading the If I understad correctly this error happen 100% of the time, not only during init. Hence seems there is an issue here, i.e. cur_ucode is not marked correctly as IWL_UCODE_REGULAR or iwl_mvm_get_temp() fail 100% of the time (iwl_mvm_is_tt_in_fw() incorrecly return true on Prarit device ? ). BTW, you implement thermal_zone device, but do you also need hwmon device? Perhaps using theramal_zone_params no_hwmon option would be proper here? Stanislaw
Re: [PATCH] iwlegacy: avoid warning about missing braces
On Thu, May 19, 2016 at 09:58:49AM +0200, Arnd Bergmann wrote: > gcc-6 warns about code in il3945_hw_txq_ctx_free() being > somewhat ambiguous: > > drivers/net/wireless/intel/iwlegacy/3945.c:1022:5: warning: suggest explicit > braces to avoid ambiguous 'else' [-Wparentheses] > > This adds a set of curly braces to avoid the warning. > > Signed-off-by: Arnd Bergmann <a...@arndb.de> Acked-by: Stanislaw Gruszka <sgrus...@redhat.com>
myri10ge: fix sleeping with bh disabled
napi_disable() can not be called with bh disabled, move locking just
around myri10ge_ss_lock_napi() .
Patches fixes following bug:
[ 114.278378] BUG: sleeping function called from invalid context at
net/core/dev.c:4383
[ 114.313712] Call Trace:
[ 114.314943] [] dump_stack+0x19/0x1b
[ 114.317673] [] __might_sleep+0x173/0x230
[ 114.320566] [] napi_disable+0x27/0x90
[ 114.323254] [] myri10ge_close+0xbf/0x3f0 [myri10ge]
Signed-off-by: Stanislaw Gruszka <sgrus...@redhat.com>
---
diff --git a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
index 270c9ee..6d1a956 100644
--- a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
+++ b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
@@ -2668,9 +2668,9 @@ static int myri10ge_close(struct net_device *dev)
del_timer_sync(>watchdog_timer);
mgp->running = MYRI10GE_ETH_STOPPING;
- local_bh_disable(); /* myri10ge_ss_lock_napi needs bh disabled */
for (i = 0; i < mgp->num_slices; i++) {
napi_disable(>ss[i].napi);
+ local_bh_disable(); /* myri10ge_ss_lock_napi needs this */
/* Lock the slice to prevent the busy_poll handler from
* accessing it. Later when we bring the NIC up, myri10ge_open
* resets the slice including this lock.
@@ -2679,8 +2679,8 @@ static int myri10ge_close(struct net_device *dev)
pr_info("Slice %d locked\n", i);
mdelay(1);
}
+ local_bh_enable();
}
- local_bh_enable();
netif_carrier_off(dev);
netif_tx_stop_all_queues(dev);
Re: [PATCH] rt2x00: add new rt2800usb device Buffalo WLI-UC-G450
On Tue, Feb 23, 2016 at 11:09:22PM +0800, Anthony Wong wrote: > Add USB ID 0411:01fd for Buffalo WLI-UC-G450 wireless adapter, > RT chipset 3593 > > Signed-off-by: Anthony Wong <anthony.w...@ubuntu.com> > Cc: sta...@vger.kernel.org Acked-by: Stanislaw Gruszka <sgrus...@redhat.com>
Re: [PATCH 1/3] net-iwlegacy: Refactoring for il_eeprom_init()
On Fri, Jan 01, 2016 at 09:30:10PM +0100, SF Markus Elfring wrote: > From: Markus Elfring <elfr...@users.sourceforge.net> > Date: Fri, 1 Jan 2016 20:54:25 +0100 > > Return directly if a memory allocation failed at the beginning. > > Signed-off-by: Markus Elfring <elfr...@users.sourceforge.net> Acked-by: Stanislaw Gruszka <sgrus...@redhat.com> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] iwlegacy: 4965-mac: constify il_sensitivity_ranges structure
On Wed, Dec 30, 2015 at 12:20:49PM +0100, Julia Lawall wrote: > The il_sensitivity_ranges is never modified, so declare it as const. > > Done with the help of Coccinelle. > > Signed-off-by: Julia Lawall <julia.law...@lip6.fr> Acked-by: Stanislaw Gruszka <sgrus...@redhat.com> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] iwlegacy: mark il_adjust_beacon_interval as noinline
On Wed, Dec 09, 2015 at 05:42:41PM +0100, Arnd Bergmann wrote: > With the new optimized do_div() code, some versions of gcc > produce obviously incorrect code that leads to a link error > in iwlegacy/common.o: > > drivers/built-in.o: In function `il_send_rxon_timing': > :(.text+0xa6b4d4): undefined reference to `ilog2_NaN' > :(.text+0xa6b4f0): undefined reference to `__aeabi_uldivmod' > > In a few thousand randconfig builds, I have seen this problem > a couple of times in this file, but never anywhere else in the > kernel, so we can try to work around this in the only file > that shows the behavior, by marking the il_adjust_beacon_interval > function as noinline, which convinces gcc to use the unoptimized > do_div() all the time. I don't think this is good way to "fix" the issue, but also have nothing against to this particular change. Acked-by: Stanislaw Gruszka <sgrus...@redhat.com> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rt2x00: adjust EEPROM_SIZE for rt2500usb
On Tue, Aug 11, 2015 at 12:25:53AM +0200, Adrien Schildknecht wrote: rt2500usb_validate_eeprom() read data up to 0x6e (EEPROM_CALIBRATE_OFFSET) but only 0x6a bytes has been allocated and read from the eeprom. This lead to out-of-bound accesses and invalid values for EEPROM_BBPTUNE_R17 and EEPROM_CALIBRATE_OFFSET. Change the EEPROM_SIZE to 0x6e in order to retrieve all the fields. Tested with a rt2570 device. Signed-off-by: Adrien Schildknecht adrien+...@schischi.me Acked-by: Stanislaw Gruszka sgrus...@redhat.com -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] iwl4965: Enable checking of format strings
On Tue, Apr 28, 2015 at 07:19:02PM +0300, Kalle Valo wrote: Rasmus Villemoes li...@rasmusvillemoes.dk writes: Since these fmt_* variables are just const char*, and not const char[], gcc (and smatch) doesn't to type checking of the arguments to the printf functions. Since the linker knows perfectly well to merge identical string constants, there's no point in having three static pointers waste memory and give an extra level of indirection. This removes over 100 non-constant format argument warnings from smatch, accounting for about 20% of all such warnings in an allmodconfig. Signed-off-by: Rasmus Villemoes li...@rasmusvillemoes.dk So what's the conclusion, should I commit this patch or not? Full discussion here: https://patchwork.kernel.org/patch/5814811/ I do not see the point of the patch. If compiler behave not optimally, fix the compiler. NACK. Stanislaw -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
