Re: [linux-next:master] BUILD REGRESSION 6dc544b66971c7f9909ff038b62149105272d26a

2024-05-28 Thread Jakub Kicinski
On Wed, 29 May 2024 02:19:47 +0800 kernel test robot wrote:
> |   `-- 
> net-ipv6-route.c-rt6_fill_node()-error:we-previously-assumed-dst-could-be-null-(see-line-)

Is there a way for us to mark this as false positive?


Re: [PATCH v1 10/12] sfc: falcon: Make I2C terminology more inclusive

2024-05-03 Thread Jakub Kicinski
On Tue, 30 Apr 2024 17:38:09 + Easwar Hariharan wrote:
> I2C v7, SMBus 3.2, and I3C 1.1.1 specifications have replaced "master/slave"
> with more appropriate terms. Inspired by and following on to Wolfram's
> series to fix drivers/i2c/[1], fix the terminology for users of
> I2C_ALGOBIT bitbanging interface, now that the approved verbiage exists
> in the specification.
> 
> Compile tested, no functionality changes intended

FWIW we're assuming someone (Wolfram?) will take all of these,
instead of area maintainers picking them individually.
Please let us know if that's incorrect.


Re: [linux-next:master] BUILD SUCCESS WITH WARNING 76cf65d1377f733af1e2a55233e3353ffa577f54

2022-10-25 Thread Jakub Kicinski
On Tue, 25 Oct 2022 09:21:07 +0100 Russell King (Oracle) wrote:
> Not me, Sean. My original implementation of phylink_validate_mask_caps()
> doesn't know anything about rate matching, so my version didn't have
> this issue.
> 
> Sean's version of my patch (which is what was submitted) added the
> dereference that causes this, so, it's up to Sean to figure out a fix -
> but he reading his follow up to the build bot's message, he seems to
> be passing it over to me to fix!
> 
> I've got other issues to be worked on right now, and have no time to
> spare to fix other people's mistakes. Sorry.
> 
> You can't always rely on the apparent author mentioned in the commit to
> be the actual person responsible for the changes in a patch.

Eh, confusing authorship trail, sorry.

I'll send a patch to drop the if (), if it's really needed we'll hear
about it sooner or later.


Re: [linux-next:master] BUILD SUCCESS WITH WARNING 76cf65d1377f733af1e2a55233e3353ffa577f54

2022-10-24 Thread Jakub Kicinski
On Tue, 25 Oct 2022 00:58:57 +0800 kernel test robot wrote:
> drivers/net/phy/phylink.c:588 phylink_validate_mask_caps() warn: variable 
> dereferenced before check 'state' (see line 583)

Hi Russell, I think the warning is semi-legit. Your commit f392a1846489
("net: phylink: provide phylink_validate_mask_caps() helper") added an 
if (state) before defer'ing state but it's already deref'ed higher up
so can't be null.


Re: [linux-next:master] BUILD REGRESSION 1e1b28b936aed946122b4e0991e7144fdbbfd77e

2022-05-16 Thread Jakub Kicinski
On Sun, 15 May 2022 21:00:21 +0800 kernel test robot wrote:
> tree/branch: 
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
> branch HEAD: 1e1b28b936aed946122b4e0991e7144fdbbfd77e  Add linux-next 
> specific files for 20220513
> 
> Error/Warning reports:
> 
> https://lore.kernel.org/linux-mm/202204181931.klac6fwo-...@intel.com
> https://lore.kernel.org/linux-mm/202204291924.vtgzmeri-...@intel.com
> https://lore.kernel.org/linux-mm/202205031017.4twman3l-...@intel.com
> https://lore.kernel.org/linux-mm/202205041248.wgcwpcev-...@intel.com
> https://lore.kernel.org/linux-mm/202205122113.ulkzd3sz-...@intel.com
> https://lore.kernel.org/linux-mm/202205150051.3rzuooag-...@intel.com
> https://lore.kernel.org/linux-mm/202205150117.sd6hzbvm-...@intel.com
> https://lore.kernel.org/lkml/202205100617.5uum3uet-...@intel.com
> https://lore.kernel.org/llvm/202204210555.dnvfhvib-...@intel.com
> https://lore.kernel.org/llvm/202205060132.uhqyux1l-...@intel.com
> https://lore.kernel.org/llvm/202205120010.zwbednzm-...@intel.com
> https://lore.kernel.org/llvm/202205141122.qihfguem-...@intel.com
> 
> Error/Warning: (recently discovered and may have been fixed)
> 
> : fatal error: ./include/generated/utsrelease.h: No such file 
> or directory
> arch/arm/mach-versatile/versatile.c:56:14: warning: no previous prototype for 
> function 'mmc_status' [-Wmissing-prototypes]
> arch/x86/kvm/pmu.h:20:32: warning: 'vmx_icl_pebs_cpu' defined but not used 
> [-Wunused-const-variable=]
> drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dp.c:5102:7: warning: 
> variable 'allow_lttpr_non_transparent_mode' set but not used 
> [-Wunused-but-set-variable]
> drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dp.c:5147:6: warning: 
> no previous prototype for function 'dp_parse_lttpr_mode' 
> [-Wmissing-prototypes]
> drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c:1364:5: warning: no previous 
> prototype for 'amdgpu_discovery_get_mall_info' [-Wmissing-prototypes]
> drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c:1983:6: warning: no previous prototype 
> for function 'gfx_v11_0_rlc_stop' [-Wmissing-prototypes]
> drivers/gpu/drm/amd/amdgpu/soc21.c:171:6: warning: no previous prototype for 
> 'soc21_grbm_select' [-Wmissing-prototypes]
> drivers/gpu/drm/solomon/ssd130x-spi.c:154:35: warning: 'ssd130x_spi_table' 
> defined but not used [-Wunused-const-variable=]
> drivers/hwmon/nct6775-platform.c:199:9: sparse:unsigned char
> drivers/hwmon/nct6775-platform.c:199:9: sparse:void
> drivers/video/fbdev/omap/hwa742.c:492:5: warning: no previous prototype for 
> 'hwa742_update_window_async' [-Wmissing-prototypes]
> fs/buffer.c:2254:5: warning: stack frame size (2144) exceeds limit (1024) in 
> 'block_read_full_folio' [-Wframe-larger-than]
> fs/ntfs/aops.c:378:12: warning: stack frame size (2224) exceeds limit (1024) 
> in 'ntfs_read_folio' [-Wframe-larger-than]
> kernel/trace/fgraph.c:37:12: warning: no previous prototype for 
> 'ftrace_enable_ftrace_graph_caller' [-Wmissing-prototypes]
> kernel/trace/fgraph.c:46:12: warning: no previous prototype for 
> 'ftrace_disable_ftrace_graph_caller' [-Wmissing-prototypes]

Is this report CCed everywhere or there's a reason why netdev@ is CCed?
I'm trying to figure out we need to care and it's not obvious..


Re: [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr

2022-02-28 Thread Jakub Kicinski
On Mon, 28 Feb 2022 16:41:04 -0800 Linus Torvalds wrote:
> So yes, initially my idea had been to just move the iterator entirely
> inside the macro. But specifying the type got so ugly that I think
> that
> 
> typeof (pos) pos
> 
> trick inside the macro really ends up giving us the best of all worlds:
> 
>  (a) let's us keep the existing syntax and code for all the nice cases
> that did everything inside the loop anyway
> 
>  (b) gives us a nice warning for any normal use-after-loop case
> (unless you explicitly initialized it like that
> sgx_mmu_notifier_release() function did for no good reason
> 
>  (c) also guarantees that even if you don't get a warning,
> non-converted (or newly written) bad code won't actually _work_
> 
> so you end up getting the new rules without any ambiguity or mistaken

I presume the goal is that we can do this without changing existing
code? Otherwise actually moving the iterator into the loop body would
be an option, by creating a different hidden variable:

#define list_iter(head) \
for (struct list head *_l = (head)->next; _l != (head); _l = _l->next)

#define list_iter_entry(var, member)\
list_entry(_l, typeof(*var), member)


list_iter(>a_head) {
struct entry *e = list_iter_entry(e, a_member);

/* use e->... */
}


Or we can slide into soft insanity and exploit one of Kees'es tricks
to encode the type of the entries "next to" the head:

#define LIST_HEAD_MEM(name, type)   \
union { \
struct list_head name;  \
type *name ## _entry;   \
}

struct entry {
struct list_head a_member;
};

struct parent {
LIST_HEAD_MEM(a_head, struct entry);
};

#define list_for_each_magic(pos, head, member)  \
for (typeof(**(head ## _entry)) *pos = list_first_entry(head, 
typeof(**(head ## _entry)), member); \
 >member != (head);\
 pos = list_next_entry(pos, member))


list_for_each_magic(e, >a_head, a_member) {
/* use e->... */
}


I'll show myself out...


Re: Build regressions/improvements in v5.17-rc1

2022-01-24 Thread Jakub Kicinski
On Mon, 24 Jan 2022 09:04:33 -0800 Jakub Kicinski wrote:
> On Mon, 24 Jan 2022 08:55:40 +0100 (CET) Geert Uytterhoeven wrote:
> > >  + /kisskb/src/drivers/net/ethernet/freescale/fec_mpc52xx.c: error: 
> > > passing argument 2 of 'mpc52xx_fec_set_paddr' discards 'const' qualifier 
> > > from pointer target type [-Werror=discarded-qualifiers]:  => 659:29
> > 
> > powerpc-gcc5/ppc32_allmodconfig

Sent:
https://lore.kernel.org/r/20220124172249.2827138-1-k...@kernel.org/

> > >  + /kisskb/src/drivers/pinctrl/pinctrl-thunderbay.c: error: assignment 
> > > discards 'const' qualifier from pointer target type 
> > > [-Werror=discarded-qualifiers]:  => 815:8, 815:29
> > 
> > arm64-gcc5.4/arm64-allmodconfig
> > arm64-gcc8/arm64-allmodconfig  

I take this one back, that's not me.


Re: Build regressions/improvements in v5.17-rc1

2022-01-24 Thread Jakub Kicinski
On Mon, 24 Jan 2022 08:55:40 +0100 (CET) Geert Uytterhoeven wrote:
> >  + /kisskb/src/drivers/net/ethernet/freescale/fec_mpc52xx.c: error: passing 
> > argument 2 of 'mpc52xx_fec_set_paddr' discards 'const' qualifier from 
> > pointer target type [-Werror=discarded-qualifiers]:  => 659:29  
> 
> powerpc-gcc5/ppc32_allmodconfig
> 
> >  + /kisskb/src/drivers/pinctrl/pinctrl-thunderbay.c: error: assignment 
> > discards 'const' qualifier from pointer target type 
> > [-Werror=discarded-qualifiers]:  => 815:8, 815:29  
> 
> arm64-gcc5.4/arm64-allmodconfig
> arm64-gcc8/arm64-allmodconfig

Let me take care of these in net.


Re: [Intel-wired-lan] [PATCH 000/141] Fix fall-through warnings for Clang

2020-11-25 Thread Jakub Kicinski
On Wed, 25 Nov 2020 04:24:27 -0800 Nick Desaulniers wrote:
> I even agree that most of the churn comes from
> 
> case 0:
>   ++x;
> default:
>   break;

And just to spell it out,

case ENUM_VALUE1:
bla();
break;
case ENUM_VALUE2:
bla();
default:
break;

is a fairly idiomatic way of indicating that not all values of the enum
are expected to be handled by the switch statement. 

I really hope the Clang folks are reasonable and merge your patch.

> If trivial patches are adding too much to your workload, consider
> training a co-maintainer or asking for help from one of your reviewers
> whom you trust.  I don't doubt it's hard to find maintainers, but
> existing maintainers should go out of their way to entrust
> co-maintainers especially when they find their workload becomes too
> high.  And reviewing/picking up trivial patches is probably a great
> way to get started.  If we allow too much knowledge of any one
> subsystem to collect with one maintainer, what happens when that
> maintainer leaves the community (which, given a finite lifespan, is an
> inevitability)?

The burn out point is about enjoying your work and feeling that it
matters. It really doesn't make much difference if you're doing
something you don't like for 12 hours every day or only in shifts with
another maintainer. You'll dislike it either way.

Applying a real patch set and then getting a few follow ups the next day
for trivial coding things like fallthrough missing or static missing,
just because I didn't have the full range of compilers to check with
before applying makes me feel pretty shitty, like I'm not doing a good
job. YMMV.
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH 000/141] Fix fall-through warnings for Clang

2020-11-23 Thread Jakub Kicinski
On Mon, 23 Nov 2020 17:32:51 -0800 Nick Desaulniers wrote:
> On Sun, Nov 22, 2020 at 8:17 AM Kees Cook  wrote:
> > On Fri, Nov 20, 2020 at 11:51:42AM -0800, Jakub Kicinski wrote:  
> > > If none of the 140 patches here fix a real bug, and there is no change
> > > to machine code then it sounds to me like a W=2 kind of a warning.  
> >
> > FWIW, this series has found at least one bug so far:
> > https://lore.kernel.org/lkml/CAFCwf11izHF=g1mGry1fE5kvFFFrxzhPSM6qKAO8gxSp=kr...@mail.gmail.com/
> >   
> 
> So looks like the bulk of these are:
> switch (x) {
>   case 0:
> ++x;
>   default:
> break;
> }
> 
> I have a patch that fixes those up for clang:
> https://reviews.llvm.org/D91895


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH 000/141] Fix fall-through warnings for Clang

2020-11-20 Thread Jakub Kicinski
On Fri, 20 Nov 2020 11:30:40 -0800 Kees Cook wrote:
> On Fri, Nov 20, 2020 at 10:53:44AM -0800, Jakub Kicinski wrote:
> > On Fri, 20 Nov 2020 12:21:39 -0600 Gustavo A. R. Silva wrote:  
> > > This series aims to fix almost all remaining fall-through warnings in
> > > order to enable -Wimplicit-fallthrough for Clang.
> > > 
> > > In preparation to enable -Wimplicit-fallthrough for Clang, explicitly
> > > add multiple break/goto/return/fallthrough statements instead of just
> > > letting the code fall through to the next case.
> > > 
> > > Notice that in order to enable -Wimplicit-fallthrough for Clang, this
> > > change[1] is meant to be reverted at some point. So, this patch helps
> > > to move in that direction.
> > > 
> > > Something important to mention is that there is currently a discrepancy
> > > between GCC and Clang when dealing with switch fall-through to empty case
> > > statements or to cases that only contain a break/continue/return
> > > statement[2][3][4].  
> > 
> > Are we sure we want to make this change? Was it discussed before?
> > 
> > Are there any bugs Clangs puritanical definition of fallthrough helped
> > find?
> > 
> > IMVHO compiler warnings are supposed to warn about issues that could
> > be bugs. Falling through to default: break; can hardly be a bug?!  
> 
> It's certainly a place where the intent is not always clear. I think
> this makes all the cases unambiguous, and doesn't impact the machine
> code, since the compiler will happily optimize away any behavioral
> redundancy.

If none of the 140 patches here fix a real bug, and there is no change
to machine code then it sounds to me like a W=2 kind of a warning.

I think clang is just being annoying here, but if I'm the only one who
feels this way chances are I'm wrong :)
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH 000/141] Fix fall-through warnings for Clang

2020-11-20 Thread Jakub Kicinski
On Fri, 20 Nov 2020 12:21:39 -0600 Gustavo A. R. Silva wrote:
> This series aims to fix almost all remaining fall-through warnings in
> order to enable -Wimplicit-fallthrough for Clang.
> 
> In preparation to enable -Wimplicit-fallthrough for Clang, explicitly
> add multiple break/goto/return/fallthrough statements instead of just
> letting the code fall through to the next case.
> 
> Notice that in order to enable -Wimplicit-fallthrough for Clang, this
> change[1] is meant to be reverted at some point. So, this patch helps
> to move in that direction.
> 
> Something important to mention is that there is currently a discrepancy
> between GCC and Clang when dealing with switch fall-through to empty case
> statements or to cases that only contain a break/continue/return
> statement[2][3][4].

Are we sure we want to make this change? Was it discussed before?

Are there any bugs Clangs puritanical definition of fallthrough helped
find?

IMVHO compiler warnings are supposed to warn about issues that could
be bugs. Falling through to default: break; can hardly be a bug?!
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx