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

[Alan Stern]

On Wed, 6 Dec 2017, Joe Perches wrote:

> On Wed, 2017-12-06 at 11:45 -0500, Alan Stern wrote:
> > On Wed, 6 Dec 2017, SF Markus Elfring wrote:
> > > Omit an extra message for a memory allocation failure in this function.
> 
> Markus' typical terrible commit message.
> 
> > > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> []
> > > @@ -829,7 +829,6 @@ int usb_hub_clear_tt_buffer(struct urb *urb)
> > >*/
> > >   clear = kmalloc(sizeof *clear, GFP_ATOMIC);
> > >   if (clear == NULL) {
> > > - dev_err(>dev, "can't save CLEAR_TT_BUFFER state\n");
> > >   /* FIXME recover somehow ... RESET_TT? */
> > >   return -ENOMEM;
> > >   }
> > 
> > Does the existing memory allocation error message include the 
> > >dev device name and driver name?  If it doesn't, there will be 
> > no way for the user to tell that the error message is related to the 
> > device failure.
> 
> No, but the effect is similar.
> 
> OOM does a dump_stack() so this function's call tree is shown.

A call stack doesn't tell you which device was being handled.

Alan Stern

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

[Alan Stern]

On Wed, 6 Dec 2017, Joe Perches wrote:

> On Wed, 2017-12-06 at 11:45 -0500, Alan Stern wrote:
> > On Wed, 6 Dec 2017, SF Markus Elfring wrote:
> > > Omit an extra message for a memory allocation failure in this function.
> 
> Markus' typical terrible commit message.
> 
> > > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> []
> > > @@ -829,7 +829,6 @@ int usb_hub_clear_tt_buffer(struct urb *urb)
> > >*/
> > >   clear = kmalloc(sizeof *clear, GFP_ATOMIC);
> > >   if (clear == NULL) {
> > > - dev_err(>dev, "can't save CLEAR_TT_BUFFER state\n");
> > >   /* FIXME recover somehow ... RESET_TT? */
> > >   return -ENOMEM;
> > >   }
> > 
> > Does the existing memory allocation error message include the 
> > >dev device name and driver name?  If it doesn't, there will be 
> > no way for the user to tell that the error message is related to the 
> > device failure.
> 
> No, but the effect is similar.
> 
> OOM does a dump_stack() so this function's call tree is shown.

A call stack doesn't tell you which device was being handled.

Alan Stern

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

[Joe Perches]

On Wed, 2017-12-06 at 11:45 -0500, Alan Stern wrote:
> On Wed, 6 Dec 2017, SF Markus Elfring wrote:
> > Omit an extra message for a memory allocation failure in this function.

Markus' typical terrible commit message.

> > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
[]
> > @@ -829,7 +829,6 @@ int usb_hub_clear_tt_buffer(struct urb *urb)
> >*/
> >   clear = kmalloc(sizeof *clear, GFP_ATOMIC);
> >   if (clear == NULL) {
> > - dev_err(>dev, "can't save CLEAR_TT_BUFFER state\n");
> >   /* FIXME recover somehow ... RESET_TT? */
> >   return -ENOMEM;
> >   }
> 
> Does the existing memory allocation error message include the 
> >dev device name and driver name?  If it doesn't, there will be 
> no way for the user to tell that the error message is related to the 
> device failure.

No, but the effect is similar.

OOM does a dump_stack() so this function's call tree is shown.

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

[Joe Perches]

On Wed, 2017-12-06 at 11:45 -0500, Alan Stern wrote:
> On Wed, 6 Dec 2017, SF Markus Elfring wrote:
> > Omit an extra message for a memory allocation failure in this function.

Markus' typical terrible commit message.

> > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
[]
> > @@ -829,7 +829,6 @@ int usb_hub_clear_tt_buffer(struct urb *urb)
> >*/
> >   clear = kmalloc(sizeof *clear, GFP_ATOMIC);
> >   if (clear == NULL) {
> > - dev_err(>dev, "can't save CLEAR_TT_BUFFER state\n");
> >   /* FIXME recover somehow ... RESET_TT? */
> >   return -ENOMEM;
> >   }
> 
> Does the existing memory allocation error message include the 
> >dev device name and driver name?  If it doesn't, there will be 
> no way for the user to tell that the error message is related to the 
> device failure.

No, but the effect is similar.

OOM does a dump_stack() so this function's call tree is shown.

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

[Geert Uytterhoeven]

Hi Alan,

On Wed, Dec 6, 2017 at 5:45 PM, Alan Stern  wrote:
> On Wed, 6 Dec 2017, SF Markus Elfring wrote:
>
>> From: Markus Elfring 
>> Date: Wed, 6 Dec 2017 17:00:18 +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 
>> ---
>>  drivers/usb/core/hub.c | 1 -
>>  1 file changed, 1 deletion(-)
>>
>> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
>> index 7ccdd3d4db84..9fbb908e9552 100644
>> --- a/drivers/usb/core/hub.c
>> +++ b/drivers/usb/core/hub.c
>> @@ -829,7 +829,6 @@ int usb_hub_clear_tt_buffer(struct urb *urb)
>>*/
>>   clear = kmalloc(sizeof *clear, GFP_ATOMIC);
>>   if (clear == NULL) {
>> - dev_err(>dev, "can't save CLEAR_TT_BUFFER state\n");
>>   /* FIXME recover somehow ... RESET_TT? */
>>   return -ENOMEM;
>>   }
>
> Does the existing memory allocation error message include the
> >dev device name and driver name?  If it doesn't, there will be
> no way for the user to tell that the error message is related to the
> device failure.

The system will come to a grinding halt anyway if it can't allocate 24 or 40
bytes. Which is BTW more or less the amount of memory saved by killing
the useless (error) message.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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

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

[Geert Uytterhoeven]

Hi Alan,

On Wed, Dec 6, 2017 at 5:45 PM, Alan Stern  wrote:
> On Wed, 6 Dec 2017, SF Markus Elfring wrote:
>
>> From: Markus Elfring 
>> Date: Wed, 6 Dec 2017 17:00:18 +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 
>> ---
>>  drivers/usb/core/hub.c | 1 -
>>  1 file changed, 1 deletion(-)
>>
>> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
>> index 7ccdd3d4db84..9fbb908e9552 100644
>> --- a/drivers/usb/core/hub.c
>> +++ b/drivers/usb/core/hub.c
>> @@ -829,7 +829,6 @@ int usb_hub_clear_tt_buffer(struct urb *urb)
>>*/
>>   clear = kmalloc(sizeof *clear, GFP_ATOMIC);
>>   if (clear == NULL) {
>> - dev_err(>dev, "can't save CLEAR_TT_BUFFER state\n");
>>   /* FIXME recover somehow ... RESET_TT? */
>>   return -ENOMEM;
>>   }
>
> Does the existing memory allocation error message include the
> >dev device name and driver name?  If it doesn't, there will be
> no way for the user to tell that the error message is related to the
> device failure.

The system will come to a grinding halt anyway if it can't allocate 24 or 40
bytes. Which is BTW more or less the amount of memory saved by killing
the useless (error) message.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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

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

[Greg Kroah-Hartman]

On Wed, Dec 06, 2017 at 11:45:31AM -0500, Alan Stern wrote:
> On Wed, 6 Dec 2017, SF Markus Elfring wrote:
> 
> > From: Markus Elfring 
> > Date: Wed, 6 Dec 2017 17:00:18 +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 
> > ---
> >  drivers/usb/core/hub.c | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> > index 7ccdd3d4db84..9fbb908e9552 100644
> > --- a/drivers/usb/core/hub.c
> > +++ b/drivers/usb/core/hub.c
> > @@ -829,7 +829,6 @@ int usb_hub_clear_tt_buffer(struct urb *urb)
> >  */
> > clear = kmalloc(sizeof *clear, GFP_ATOMIC);
> > if (clear == NULL) {
> > -   dev_err(>dev, "can't save CLEAR_TT_BUFFER state\n");
> > /* FIXME recover somehow ... RESET_TT? */
> > return -ENOMEM;
> > }
> 
> Does the existing memory allocation error message include the 
> >dev device name and driver name?  If it doesn't, there will be 
> no way for the user to tell that the error message is related to the 
> device failure.

Don't worry, I have a filter for this person's emails and do not see
their patches so they will not get applied.

thanks,

greg k-h

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

[Greg Kroah-Hartman]

On Wed, Dec 06, 2017 at 11:45:31AM -0500, Alan Stern wrote:
> On Wed, 6 Dec 2017, SF Markus Elfring wrote:
> 
> > From: Markus Elfring 
> > Date: Wed, 6 Dec 2017 17:00:18 +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 
> > ---
> >  drivers/usb/core/hub.c | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> > index 7ccdd3d4db84..9fbb908e9552 100644
> > --- a/drivers/usb/core/hub.c
> > +++ b/drivers/usb/core/hub.c
> > @@ -829,7 +829,6 @@ int usb_hub_clear_tt_buffer(struct urb *urb)
> >  */
> > clear = kmalloc(sizeof *clear, GFP_ATOMIC);
> > if (clear == NULL) {
> > -   dev_err(>dev, "can't save CLEAR_TT_BUFFER state\n");
> > /* FIXME recover somehow ... RESET_TT? */
> > return -ENOMEM;
> > }
> 
> Does the existing memory allocation error message include the 
> >dev device name and driver name?  If it doesn't, there will be 
> no way for the user to tell that the error message is related to the 
> device failure.

Don't worry, I have a filter for this person's emails and do not see
their patches so they will not get applied.

thanks,

greg k-h

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

[Alan Stern]

On Wed, 6 Dec 2017, SF Markus Elfring wrote:

> From: Markus Elfring 
> Date: Wed, 6 Dec 2017 17:00:18 +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 
> ---
>  drivers/usb/core/hub.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 7ccdd3d4db84..9fbb908e9552 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -829,7 +829,6 @@ int usb_hub_clear_tt_buffer(struct urb *urb)
>*/
>   clear = kmalloc(sizeof *clear, GFP_ATOMIC);
>   if (clear == NULL) {
> - dev_err(>dev, "can't save CLEAR_TT_BUFFER state\n");
>   /* FIXME recover somehow ... RESET_TT? */
>   return -ENOMEM;
>   }

Does the existing memory allocation error message include the 
>dev device name and driver name?  If it doesn't, there will be 
no way for the user to tell that the error message is related to the 
device failure.

Alan Stern

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

[Alan Stern]

On Wed, 6 Dec 2017, SF Markus Elfring wrote:

> From: Markus Elfring 
> Date: Wed, 6 Dec 2017 17:00:18 +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 
> ---
>  drivers/usb/core/hub.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 7ccdd3d4db84..9fbb908e9552 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -829,7 +829,6 @@ int usb_hub_clear_tt_buffer(struct urb *urb)
>*/
>   clear = kmalloc(sizeof *clear, GFP_ATOMIC);
>   if (clear == NULL) {
> - dev_err(>dev, "can't save CLEAR_TT_BUFFER state\n");
>   /* FIXME recover somehow ... RESET_TT? */
>   return -ENOMEM;
>   }

Does the existing memory allocation error message include the 
>dev device name and driver name?  If it doesn't, there will be 
no way for the user to tell that the error message is related to the 
device failure.

Alan Stern