Re: USB: hub: Delete an error message for a failed memory allocation in usb_hub_clear_tt_buffer()
On Fri, 8 Dec 2017, Geert Uytterhoeven wrote: > Hi Alan, > > On Thu, Dec 7, 2017 at 10:26 PM, Alan Stern 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? I can't see how to make that work. We don't know beforehand how many structures will be needed at any time. Alan Stern
Re: USB: hub: Delete an error message for a failed memory allocation in usb_hub_clear_tt_buffer()
On Wed, Dec 06, 2017 at 09:20:05PM +0100, Geert Uytterhoeven wrote: > Hi Markus, > > On Wed, Dec 6, 2017 at 6:51 PM, SF Markus Elfring > wrote: > >> The system will come to a grinding halt anyway if it can't allocate 24 or > >> 40 bytes. > > > > Maybe. > > Since you've been sending zillions of patches for this, perhaps the time > is ripe to actually try to trigger this, and see what happens? > > >> Which is BTW more or less the amount of memory saved by killing > >> the useless (error) message. > > > > Would you dare to resend this update suggestion after such a view? > > Of course. That was implied. > No. Greg maintains USB and he's has blocked Markus, because he never listens to feedback but instead just repsonds that he has a different opinion. regards, dan carpenter
Re: USB: hub: Delete an error message for a failed memory allocation in usb_hub_clear_tt_buffer()
Hi Alan, On Thu, Dec 7, 2017 at 10:26 PM, Alan Stern 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 -- 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: USB: hub: Delete an error message for a failed memory allocation in usb_hub_clear_tt_buffer()
On Thu, 7 Dec 2017, Dan Carpenter wrote: > On Thu, Dec 07, 2017 at 10:12:27AM -0500, Alan Stern wrote: > > The real problem is that the kernel development community doesn't have > > a fixed policy on how to handle memory allocation errors. There are > > several possibilities: > > > > Ignore them on the grounds that they will never happen. > > (Really? And what is the size limit above which they > > might happen?) > > > > It's pretty rare to ignore allocation failures these days. It causes > static checker warnings. > > Sometimes it's accepted for people to ignore errors during boot but > I hate that because how am I supposed to filter out those static checker > warnings? It's better to pretend that the kernel will still boot > without essential hardware instead of wasting everyone's time who looks > at checker output. > > > Ignore them on the grounds that the machine will hang or > > crash in the near future. (Is this guaranteed?) > > On boot sometimes yes. > > > > > Treat them like other errors: try to press forward (perhaps > > in a degraded mode). > > > > Treat them like other errors: log an error message and try > > to press forward. > > > > 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? Alan Stern > > I also have a special static checker warning for when people do: > > foo = alloc(); > BUG_ON(!foo); > > People do that occasionally but fortunately it's pretty rare. 10 years > ago that's how btrfs did error handling, but now there are only 4 of > these still remaining in btrfs. > > regards, > dan carpenter > > >
Re: USB: hub: Delete an error message for a failed memory allocation in usb_hub_clear_tt_buffer()
On Thu, Dec 07, 2017 at 10:12:27AM -0500, Alan Stern wrote: > The real problem is that the kernel development community doesn't have > a fixed policy on how to handle memory allocation errors. There are > several possibilities: > > Ignore them on the grounds that they will never happen. > (Really? And what is the size limit above which they > might happen?) > It's pretty rare to ignore allocation failures these days. It causes static checker warnings. Sometimes it's accepted for people to ignore errors during boot but I hate that because how am I supposed to filter out those static checker warnings? It's better to pretend that the kernel will still boot without essential hardware instead of wasting everyone's time who looks at checker output. > Ignore them on the grounds that the machine will hang or > crash in the near future. (Is this guaranteed?) On boot sometimes yes. > > Treat them like other errors: try to press forward (perhaps > in a degraded mode). > > Treat them like other errors: log an error message and try > to press forward. > 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. I also have a special static checker warning for when people do: foo = alloc(); BUG_ON(!foo); People do that occasionally but fortunately it's pretty rare. 10 years ago that's how btrfs did error handling, but now there are only 4 of these still remaining in btrfs. regards, dan carpenter
Re: USB: hub: Delete an error message for a failed memory allocation in usb_hub_clear_tt_buffer()
On Thu, 2017-12-07 at 10:12 -0500, Alan Stern wrote: > The real problem is that the kernel development community doesn't have > a fixed policy on how to handle memory allocation errors. [] > If there was one agreed-upon policy, then we could definitively point > to old code and say "That's wrong, and here is how it should be fixed." > But currently this is not possible, and we end up with repetitive > discussions like this one that aren't of general use. Well stated. My preferred policy would be to remove all the individual allocation failure messages and only use the generic warn_alloc()/dump_stack() mechanism.
Re: USB: hub: Delete an error message for a failed memory allocation in usb_hub_clear_tt_buffer()
On Thu, 7 Dec 2017, Geert Uytterhoeven wrote: > Hi Alan, > > On Wed, Dec 6, 2017 at 11:02 PM, Alan Stern wrote: > > On Wed, 6 Dec 2017, SF Markus Elfring wrote: > >> >>> Does the existing memory allocation error message include the > >> >>> &udev->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. > >> > >> Do you find a default Linux allocation failure report insufficient then? > >> > >> Would you like to to achieve that the requested information can be > >> determined > >> from a backtrace? > > > > It is not practical to do this. The memory allocation routines do not > > for what purpose the memory is being allocated; hence when a failure > > occurs they cannot tell what device (or other part of the system) will > > be affected. > > If even allocation of 24 bytes fails, lots of other devices and other parts of > the system will start failing really soon... In fact, one wonders if the allocation routine's own error message and stack trace would actually appear anywhere... > > That's why we have a secondary error message. > > ... and the secondary error message would still be useless. Well, there is still a difference between GFP_ATOMIC and GFP_KERNEL allocations. Failure of the first doesn't necessarily imply failure of the second, so perhaps the system could recover. The real problem is that the kernel development community doesn't have a fixed policy on how to handle memory allocation errors. There are several possibilities: Ignore them on the grounds that they will never happen. (Really? And what is the size limit above which they might happen?) Ignore them on the grounds that the machine will hang or crash in the near future. (Is this guaranteed?) Treat them like other errors: try to press forward (perhaps in a degraded mode). Treat them like other errors: log an error message and try to press forward. And probably a few more that haven't occurred to me. No doubt there are examples of each at various places in the kernel. Nobody seems able to agree on a single course of action. Maybe not even Linus. If there was one agreed-upon policy, then we could definitively point to old code and say "That's wrong, and here is how it should be fixed." But currently this is not possible, and we end up with repetitive discussions like this one that aren't of general use. Alan Stern
Re: USB: hub: Delete an error message for a failed memory allocation in usb_hub_clear_tt_buffer()
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. >> >> Do you find a default Linux allocation failure report insufficient then? >> >> Would you like to to achieve that the requested information can be determined >> from a backtrace? > > It is not practical to do this. I imagine that this depends on details if a backtrace could eventually be configured for your specific needs. > The memory allocation routines do not for what purpose > the memory is being allocated; Do you want an improved accounting for these purposes? > hence when a failure occurs they cannot tell what device > (or other part of the system) will be affected. I know that other programs can provide dumps for function call stacks where the parameters which were passed in previous calls could be decoded to some degree. > That's why we have a secondary error message. I am curious on how the relevance of such messages will be interpreted by other developers in this software area. Regards, Markus
Re: USB: hub: Delete an error message for a failed memory allocation in usb_hub_clear_tt_buffer()
On Thu, Dec 07, 2017 at 09:45:38AM +0100, Geert Uytterhoeven wrote: > > > > Small allocations never fail in the current kernel. > > A few comments (this is in response to a patch from Markus, so there have > to be lots of questions and uncertainties ;-) > 1. In the current kernel. What about the future? Right. No one can predict. And the small allocations don't fail rule causes some problems. > 2. If a small allocation cannot fail, what happens if the small memory slab >is exhausted? A new page must be allocated, which will trigger an OOM, >and some other part of the system will be killed and fail. Right. > 3. This driver uses GFP_ATOMIC, is that guaranteed to succeed? I think not. > Right again. I was missing the first email in the thread because of my email filters so I didn't see this was atomic. regards, dan carpenter
Re: USB: hub: Delete an error message for a failed memory allocation in usb_hub_clear_tt_buffer()
Am Donnerstag, den 07.12.2017, 09:56 +0100 schrieb Greg Kroah-Hartman: > On Thu, Dec 07, 2017 at 09:45:38AM +0100, Geert Uytterhoeven wrote: > > > > A few comments (this is in response to a patch from Markus, so there have > > to be lots of questions and uncertainties ;-) > > Note, because of the patch author, I'm not applying it no matter what, > so this discussion is really going nowhere. We really need to come to a consensus on that question. If those additional messages really are redundant, we could save memory deleting them. Or alternatively, if they are not and we need to know which device is hit, we should centralize that reporting and tell the VM subsystem not the dump a stack trace. Regards Oliver
Re: USB: hub: Delete an error message for a failed memory allocation in usb_hub_clear_tt_buffer()
On Thu, Dec 07, 2017 at 09:45:38AM +0100, Geert Uytterhoeven wrote: > A few comments (this is in response to a patch from Markus, so there have > to be lots of questions and uncertainties ;-) Note, because of the patch author, I'm not applying it no matter what, so this discussion is really going nowhere. greg k-h
Re: USB: hub: Delete an error message for a failed memory allocation in usb_hub_clear_tt_buffer()
Hi Dan, On Thu, Dec 7, 2017 at 9:35 AM, Dan Carpenter wrote: > On Thu, Dec 07, 2017 at 08:40:07AM +0100, Geert Uytterhoeven wrote: >> On Wed, Dec 6, 2017 at 11:02 PM, Alan Stern >> wrote: >> > On Wed, 6 Dec 2017, SF Markus Elfring wrote: >> >> >>> Does the existing memory allocation error message include the >> >> >>> &udev->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. >> >> >> >> Do you find a default Linux allocation failure report insufficient then? >> >> >> >> Would you like to to achieve that the requested information can be >> >> determined >> >> from a backtrace? >> > >> > It is not practical to do this. The memory allocation routines do not >> > for what purpose the memory is being allocated; hence when a failure >> > occurs they cannot tell what device (or other part of the system) will >> > be affected. >> >> If even allocation of 24 bytes fails, lots of other devices and other parts >> of >> the system will start failing really soon... > > Small allocations never fail in the current kernel. A few comments (this is in response to a patch from Markus, so there have to be lots of questions and uncertainties ;-) 1. In the current kernel. What about the future? 2. If a small allocation cannot fail, what happens if the small memory slab is exhausted? A new page must be allocated, which will trigger an OOM, and some other part of the system will be killed and fail. 3. This driver uses GFP_ATOMIC, is that guaranteed to succeed? I think not. 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: USB: hub: Delete an error message for a failed memory allocation in usb_hub_clear_tt_buffer()
On Thu, Dec 07, 2017 at 08:40:07AM +0100, Geert Uytterhoeven wrote: > Hi Alan, > > On Wed, Dec 6, 2017 at 11:02 PM, Alan Stern wrote: > > On Wed, 6 Dec 2017, SF Markus Elfring wrote: > >> >>> Does the existing memory allocation error message include the > >> >>> &udev->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. > >> > >> Do you find a default Linux allocation failure report insufficient then? > >> > >> Would you like to to achieve that the requested information can be > >> determined > >> from a backtrace? > > > > It is not practical to do this. The memory allocation routines do not > > for what purpose the memory is being allocated; hence when a failure > > occurs they cannot tell what device (or other part of the system) will > > be affected. > > If even allocation of 24 bytes fails, lots of other devices and other parts of > the system will start failing really soon... > Small allocations never fail in the current kernel. regards, dan carpenter
Re: USB: hub: Delete an error message for a failed memory allocation in usb_hub_clear_tt_buffer()
Hi Alan, On Wed, Dec 6, 2017 at 11:02 PM, Alan Stern wrote: > On Wed, 6 Dec 2017, SF Markus Elfring wrote: >> >>> Does the existing memory allocation error message include the >> >>> &udev->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. >> >> Do you find a default Linux allocation failure report insufficient then? >> >> Would you like to to achieve that the requested information can be determined >> from a backtrace? > > It is not practical to do this. The memory allocation routines do not > for what purpose the memory is being allocated; hence when a failure > occurs they cannot tell what device (or other part of the system) will > be affected. If even allocation of 24 bytes fails, lots of other devices and other parts of the system will start failing really soon... > That's why we have a secondary error message. ... and the secondary error message would still be useless. 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: USB: hub: Delete an error message for a failed memory allocation in usb_hub_clear_tt_buffer()
On Wed, 6 Dec 2017, SF Markus Elfring wrote: > >>> Does the existing memory allocation error message include the > >>> &udev->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. > > Do you find a default Linux allocation failure report insufficient then? > > Would you like to to achieve that the requested information can be determined > from a backtrace? It is not practical to do this. The memory allocation routines do not for what purpose the memory is being allocated; hence when a failure occurs they cannot tell what device (or other part of the system) will be affected. That's why we have a secondary error message. Alan Stern
Re: USB: hub: Delete an error message for a failed memory allocation in usb_hub_clear_tt_buffer()
>>> Does the existing memory allocation error message include the >>> &udev->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. Do you find a default Linux allocation failure report insufficient then? Would you like to to achieve that the requested information can be determined from a backtrace? Regards, Markus
Re: [PATCH] USB: hub: Delete an error message for a failed memory allocation in usb_hub_clear_tt_buffer()
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(&udev->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 > > &udev->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: USB: hub: Delete an error message for a failed memory allocation in usb_hub_clear_tt_buffer()
Hi Markus, On Wed, Dec 6, 2017 at 6:51 PM, SF Markus Elfring wrote: >> The system will come to a grinding halt anyway if it can't allocate 24 or 40 >> bytes. > > Maybe. Since you've been sending zillions of patches for this, perhaps the time is ripe to actually try to trigger this, and see what happens? >> Which is BTW more or less the amount of memory saved by killing >> the useless (error) message. > > Would you dare to resend this update suggestion after such a view? Of course. That was implied. 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()
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(&udev->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 > &udev->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: USB: hub: Delete an error message for a failed memory allocation in usb_hub_clear_tt_buffer()
> The system will come to a grinding halt anyway if it can't allocate 24 or 40 > bytes. Maybe. > Which is BTW more or less the amount of memory saved by killing > the useless (error) message. Would you dare to resend this update suggestion after such a view? Regards, Markus
Re: [PATCH] USB: hub: Delete an error message for a failed memory allocation in usb_hub_clear_tt_buffer()
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(&udev->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 > &udev->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()
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(&udev->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 > &udev->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()
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(&udev->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 &udev->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
[PATCH] USB: hub: Delete an error message for a failed memory allocation in usb_hub_clear_tt_buffer()
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(&udev->dev, "can't save CLEAR_TT_BUFFER state\n"); /* FIXME recover somehow ... RESET_TT? */ return -ENOMEM; } -- 2.15.1