On Sun, Apr 14, 2019 at 06:26:02PM +0100, Russell King - ARM Linux admin wrote:
> On Sun, Apr 14, 2019 at 06:49:49AM +0200, Nicholas Mc Guire wrote:
> > Although it is very unlikely that the allocation during init would
> > fail any such failure should point to the original cause rather
> > than waiting for a null-pointer dereference to splat.
> >
> > Signed-off-by: Nicholas Mc Guire <hof...@osadl.org>
> > ---
> >
> > Problem located with experimental coccinelle script
> >
> > While this will not really help much - but kzalloc failures should not
> > go unhandled.
>
> Sorry, no, not like this.
>
ok - well I wsa not sure about it either - it just seems wrong
to leave a possible allocation failure without any response.
The issue of generating excessive outout make sense - so will fix
it up to a pr_err() and resend.
thx!
hofrat
> With this patch, rather than getting an oops and a stacktrace which
> people can capture and email, we instead end up getting a warning
> line, a stack trace, followed by an oops containing another stack
> trace.
>
> We _already_ have problems getting people to send us kernel message
> debug information without editing out what they deem to be "unnecessary
> verbage", like all those numbers and function names that comprise a
> stack trace. We don't need yet more of that stuff, especially when it
> is redundant.
>
> So, I think throwing WARN_ON() at this case is way too excessive, and
> will only have a detrimental effect on the reports we receive - and
> that is extremely important.
>
> IMHO, A better solution would be to just print a warning, rather than
> causing the kernel to print several kB of needless messages.
>
> if (!new_compat)
> pr_err("new_compat allocation failure in %s()\n",
> __func__);
>
> >
> > Patch was compile-tested: mvebu_v7_defconfig (implies MACH_MVEBU_ANY=y)
> > (with some unrelated sparse warnings about missing syscalls)
> >
> > Patch is against 5.1-rc4 (localversion-next is 20190412)
> >
> > arch/arm/mach-mvebu/board-v7.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/arm/mach-mvebu/board-v7.c b/arch/arm/mach-mvebu/board-v7.c
> > index 0b10acd..37f8cb6 100644
> > --- a/arch/arm/mach-mvebu/board-v7.c
> > +++ b/arch/arm/mach-mvebu/board-v7.c
> > @@ -128,6 +128,7 @@ static void __init i2c_quirk(void)
> > struct property *new_compat;
> >
> > new_compat = kzalloc(sizeof(*new_compat), GFP_KERNEL);
> > + WARN_ON(!new_compat);
> >
> > new_compat->name = kstrdup("compatible", GFP_KERNEL);
> > new_compat->length = sizeof("marvell,mv78230-a0-i2c");
> > --
> > 2.1.4
> >
> >
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
> According to speedtest.net: 11.9Mbps down 500kbps up
