Re: USB: hub: Delete an error message for a failed memory allocation in usb_hub_clear_tt_buffer()

Geert Uytterhoeven
2017-12-08T07:43:40Z

Hi Alan,

On Thu, Dec 7, 2017 at 10:26 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Thu, 7 Dec 2017, Dan Carpenter wrote:
>> The standard is to treat them like errors and try press forward in a
>> degraded mode but don't print a message. Checkpatch.pl complains if you
>> print a warning for allocation failures. A lot of low level functions
>> handle their own messages pretty well but especially kmalloc() does.
>
> Which brings us back to my original objection. If an allocation
> failure has localized effects, but the low-level warning is unable to
> specify what will be affected, how is the user supposed to connect the
> effect to the cause?

The backtrace would include usb_hub_clear_tt_buffer, so the user will
know USB is affected.
Note that the cause of the memory exhaustion is usually not the caller.
Unless it requests a real big block of memory, in which case that will be
mentioned in the backtrace, too.

In this particular case, the driver uses GFP_ATOMIC, so allocation failures
aren't that rare, and I think the driver should be prepared for that, and try
to recover gracefully.

The comment

/* FIXME recover somehow ... RESET_TT? */

makes me think it isn't.

As this is a pretty small allocation, perhaps it can be done beforehand, without
GFP_ATOMIC?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds