* [PATCH] staging: greybus: add logging statement when kfifo_alloc fails @ 2019-07-10 12:20 ` Keyur Patel 0 siblings, 0 replies; 8+ messages in thread From: Keyur Patel @ 2019-07-10 12:20 UTC (permalink / raw) Cc: iamkeyur96, David Lin, Johan Hovold, Alex Elder, Greg Kroah-Hartman, greybus-dev, devel, linux-kernel Added missing logging statement when kfifo_alloc fails, to improve debugging. Signed-off-by: Keyur Patel <iamkeyur96@gmail.com> --- drivers/staging/greybus/uart.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/staging/greybus/uart.c b/drivers/staging/greybus/uart.c index b3bffe91ae99..86a395ae177d 100644 --- a/drivers/staging/greybus/uart.c +++ b/drivers/staging/greybus/uart.c @@ -856,8 +856,10 @@ static int gb_uart_probe(struct gbphy_device *gbphy_dev, retval = kfifo_alloc(&gb_tty->write_fifo, GB_UART_WRITE_FIFO_SIZE, GFP_KERNEL); - if (retval) + if (retval) { + pr_err("kfifo_alloc failed\n"); goto exit_buf_free; + } gb_tty->credits = GB_UART_FIRMWARE_CREDITS; init_completion(&gb_tty->credits_complete); -- 2.22.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH] staging: greybus: add logging statement when kfifo_alloc fails @ 2019-07-10 12:20 ` Keyur Patel 0 siblings, 0 replies; 8+ messages in thread From: Keyur Patel @ 2019-07-10 12:20 UTC (permalink / raw) Cc: devel, Alex Elder, Greg Kroah-Hartman, Johan Hovold, David Lin, greybus-dev, linux-kernel, iamkeyur96 Added missing logging statement when kfifo_alloc fails, to improve debugging. Signed-off-by: Keyur Patel <iamkeyur96@gmail.com> --- drivers/staging/greybus/uart.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/staging/greybus/uart.c b/drivers/staging/greybus/uart.c index b3bffe91ae99..86a395ae177d 100644 --- a/drivers/staging/greybus/uart.c +++ b/drivers/staging/greybus/uart.c @@ -856,8 +856,10 @@ static int gb_uart_probe(struct gbphy_device *gbphy_dev, retval = kfifo_alloc(&gb_tty->write_fifo, GB_UART_WRITE_FIFO_SIZE, GFP_KERNEL); - if (retval) + if (retval) { + pr_err("kfifo_alloc failed\n"); goto exit_buf_free; + } gb_tty->credits = GB_UART_FIRMWARE_CREDITS; init_completion(&gb_tty->credits_complete); -- 2.22.0 _______________________________________________ devel mailing list devel@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] staging: greybus: add logging statement when kfifo_alloc fails 2019-07-10 12:20 ` Keyur Patel @ 2019-07-10 16:35 ` Greg Kroah-Hartman -1 siblings, 0 replies; 8+ messages in thread From: Greg Kroah-Hartman @ 2019-07-10 16:35 UTC (permalink / raw) To: Keyur Patel Cc: devel, Alex Elder, Johan Hovold, David Lin, greybus-dev, linux-kernel On Wed, Jul 10, 2019 at 08:20:17AM -0400, Keyur Patel wrote: > Added missing logging statement when kfifo_alloc fails, to improve > debugging. > > Signed-off-by: Keyur Patel <iamkeyur96@gmail.com> > --- > drivers/staging/greybus/uart.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/staging/greybus/uart.c b/drivers/staging/greybus/uart.c > index b3bffe91ae99..86a395ae177d 100644 > --- a/drivers/staging/greybus/uart.c > +++ b/drivers/staging/greybus/uart.c > @@ -856,8 +856,10 @@ static int gb_uart_probe(struct gbphy_device *gbphy_dev, > > retval = kfifo_alloc(&gb_tty->write_fifo, GB_UART_WRITE_FIFO_SIZE, > GFP_KERNEL); > - if (retval) > + if (retval) { > + pr_err("kfifo_alloc failed\n"); > goto exit_buf_free; > + } You should have already gotten an error message from the log if this fails, from the kmalloc_array() call failing, right? So why is this needed? We have been trying to remove these types of messages and keep them in the "root" place where the failure happens. thanks, greg k-h ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] staging: greybus: add logging statement when kfifo_alloc fails @ 2019-07-10 16:35 ` Greg Kroah-Hartman 0 siblings, 0 replies; 8+ messages in thread From: Greg Kroah-Hartman @ 2019-07-10 16:35 UTC (permalink / raw) To: Keyur Patel Cc: devel, Alex Elder, Johan Hovold, linux-kernel, greybus-dev, David Lin On Wed, Jul 10, 2019 at 08:20:17AM -0400, Keyur Patel wrote: > Added missing logging statement when kfifo_alloc fails, to improve > debugging. > > Signed-off-by: Keyur Patel <iamkeyur96@gmail.com> > --- > drivers/staging/greybus/uart.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/staging/greybus/uart.c b/drivers/staging/greybus/uart.c > index b3bffe91ae99..86a395ae177d 100644 > --- a/drivers/staging/greybus/uart.c > +++ b/drivers/staging/greybus/uart.c > @@ -856,8 +856,10 @@ static int gb_uart_probe(struct gbphy_device *gbphy_dev, > > retval = kfifo_alloc(&gb_tty->write_fifo, GB_UART_WRITE_FIFO_SIZE, > GFP_KERNEL); > - if (retval) > + if (retval) { > + pr_err("kfifo_alloc failed\n"); > goto exit_buf_free; > + } You should have already gotten an error message from the log if this fails, from the kmalloc_array() call failing, right? So why is this needed? We have been trying to remove these types of messages and keep them in the "root" place where the failure happens. thanks, greg k-h _______________________________________________ devel mailing list devel@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] staging: greybus: add logging statement when kfifo_alloc fails 2019-07-10 16:35 ` Greg Kroah-Hartman @ 2019-07-10 14:24 ` Keyur Patel -1 siblings, 0 replies; 8+ messages in thread From: Keyur Patel @ 2019-07-10 14:24 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: devel, Alex Elder, Johan Hovold, David Lin, greybus-dev, linux-kernel On Wed, Jul 10, 2019 at 06:35:38PM +0200, Greg Kroah-Hartman wrote: > On Wed, Jul 10, 2019 at 08:20:17AM -0400, Keyur Patel wrote: > > Added missing logging statement when kfifo_alloc fails, to improve > > debugging. > > > > Signed-off-by: Keyur Patel <iamkeyur96@gmail.com> > > --- > > drivers/staging/greybus/uart.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/staging/greybus/uart.c b/drivers/staging/greybus/uart.c > > index b3bffe91ae99..86a395ae177d 100644 > > --- a/drivers/staging/greybus/uart.c > > +++ b/drivers/staging/greybus/uart.c > > @@ -856,8 +856,10 @@ static int gb_uart_probe(struct gbphy_device *gbphy_dev, > > > > retval = kfifo_alloc(&gb_tty->write_fifo, GB_UART_WRITE_FIFO_SIZE, > > GFP_KERNEL); > > - if (retval) > > + if (retval) { > > + pr_err("kfifo_alloc failed\n"); > > goto exit_buf_free; > > + } > > You should have already gotten an error message from the log if this > fails, from the kmalloc_array() call failing, right? > > So why is this needed? We have been trying to remove these types of > messages and keep them in the "root" place where the failure happens. > > thanks, > > greg k-h Didn't notice that. I agree that this will result only into redundancy. Quick look over files reveal that there are multiple places where people are using print statements after memory allocation fails. Should I go ahead and send patches to remove those redundant print statements? Sorry, if you're receiving this message again. Thnaks. Keyur Patel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] staging: greybus: add logging statement when kfifo_alloc fails @ 2019-07-10 14:24 ` Keyur Patel 0 siblings, 0 replies; 8+ messages in thread From: Keyur Patel @ 2019-07-10 14:24 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: devel, Alex Elder, Johan Hovold, linux-kernel, greybus-dev, David Lin On Wed, Jul 10, 2019 at 06:35:38PM +0200, Greg Kroah-Hartman wrote: > On Wed, Jul 10, 2019 at 08:20:17AM -0400, Keyur Patel wrote: > > Added missing logging statement when kfifo_alloc fails, to improve > > debugging. > > > > Signed-off-by: Keyur Patel <iamkeyur96@gmail.com> > > --- > > drivers/staging/greybus/uart.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/staging/greybus/uart.c b/drivers/staging/greybus/uart.c > > index b3bffe91ae99..86a395ae177d 100644 > > --- a/drivers/staging/greybus/uart.c > > +++ b/drivers/staging/greybus/uart.c > > @@ -856,8 +856,10 @@ static int gb_uart_probe(struct gbphy_device *gbphy_dev, > > > > retval = kfifo_alloc(&gb_tty->write_fifo, GB_UART_WRITE_FIFO_SIZE, > > GFP_KERNEL); > > - if (retval) > > + if (retval) { > > + pr_err("kfifo_alloc failed\n"); > > goto exit_buf_free; > > + } > > You should have already gotten an error message from the log if this > fails, from the kmalloc_array() call failing, right? > > So why is this needed? We have been trying to remove these types of > messages and keep them in the "root" place where the failure happens. > > thanks, > > greg k-h Didn't notice that. I agree that this will result only into redundancy. Quick look over files reveal that there are multiple places where people are using print statements after memory allocation fails. Should I go ahead and send patches to remove those redundant print statements? Sorry, if you're receiving this message again. Thnaks. Keyur Patel _______________________________________________ devel mailing list devel@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] staging: greybus: add logging statement when kfifo_alloc fails 2019-07-10 14:24 ` Keyur Patel @ 2019-07-11 7:00 ` Greg Kroah-Hartman -1 siblings, 0 replies; 8+ messages in thread From: Greg Kroah-Hartman @ 2019-07-11 7:00 UTC (permalink / raw) To: Keyur Patel Cc: devel, Alex Elder, Johan Hovold, linux-kernel, greybus-dev, David Lin On Wed, Jul 10, 2019 at 10:24:06AM -0400, Keyur Patel wrote: > On Wed, Jul 10, 2019 at 06:35:38PM +0200, Greg Kroah-Hartman wrote: > > On Wed, Jul 10, 2019 at 08:20:17AM -0400, Keyur Patel wrote: > > > Added missing logging statement when kfifo_alloc fails, to improve > > > debugging. > > > > > > Signed-off-by: Keyur Patel <iamkeyur96@gmail.com> > > > --- > > > drivers/staging/greybus/uart.c | 4 +++- > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/staging/greybus/uart.c b/drivers/staging/greybus/uart.c > > > index b3bffe91ae99..86a395ae177d 100644 > > > --- a/drivers/staging/greybus/uart.c > > > +++ b/drivers/staging/greybus/uart.c > > > @@ -856,8 +856,10 @@ static int gb_uart_probe(struct gbphy_device *gbphy_dev, > > > > > > retval = kfifo_alloc(&gb_tty->write_fifo, GB_UART_WRITE_FIFO_SIZE, > > > GFP_KERNEL); > > > - if (retval) > > > + if (retval) { > > > + pr_err("kfifo_alloc failed\n"); > > > goto exit_buf_free; > > > + } > > > > You should have already gotten an error message from the log if this > > fails, from the kmalloc_array() call failing, right? > > > > So why is this needed? We have been trying to remove these types of > > messages and keep them in the "root" place where the failure happens. > > > > thanks, > > > > greg k-h > > Didn't notice that. I agree that this will result only into redundancy. > Quick look over files reveal that there are multiple places > where people are using print statements after memory allocation fails. > Should I go ahead and send patches to remove those > redundant print statements? It all depends on where in the kernel those are. Some subsystems want minor cleanups like this, but most do not. So unless the issue you find is in the drivers/staging/ directory, I would not worry about it. thanks, greg k-h ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] staging: greybus: add logging statement when kfifo_alloc fails @ 2019-07-11 7:00 ` Greg Kroah-Hartman 0 siblings, 0 replies; 8+ messages in thread From: Greg Kroah-Hartman @ 2019-07-11 7:00 UTC (permalink / raw) To: Keyur Patel Cc: devel, Alex Elder, linux-kernel, Johan Hovold, David Lin, greybus-dev On Wed, Jul 10, 2019 at 10:24:06AM -0400, Keyur Patel wrote: > On Wed, Jul 10, 2019 at 06:35:38PM +0200, Greg Kroah-Hartman wrote: > > On Wed, Jul 10, 2019 at 08:20:17AM -0400, Keyur Patel wrote: > > > Added missing logging statement when kfifo_alloc fails, to improve > > > debugging. > > > > > > Signed-off-by: Keyur Patel <iamkeyur96@gmail.com> > > > --- > > > drivers/staging/greybus/uart.c | 4 +++- > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/staging/greybus/uart.c b/drivers/staging/greybus/uart.c > > > index b3bffe91ae99..86a395ae177d 100644 > > > --- a/drivers/staging/greybus/uart.c > > > +++ b/drivers/staging/greybus/uart.c > > > @@ -856,8 +856,10 @@ static int gb_uart_probe(struct gbphy_device *gbphy_dev, > > > > > > retval = kfifo_alloc(&gb_tty->write_fifo, GB_UART_WRITE_FIFO_SIZE, > > > GFP_KERNEL); > > > - if (retval) > > > + if (retval) { > > > + pr_err("kfifo_alloc failed\n"); > > > goto exit_buf_free; > > > + } > > > > You should have already gotten an error message from the log if this > > fails, from the kmalloc_array() call failing, right? > > > > So why is this needed? We have been trying to remove these types of > > messages and keep them in the "root" place where the failure happens. > > > > thanks, > > > > greg k-h > > Didn't notice that. I agree that this will result only into redundancy. > Quick look over files reveal that there are multiple places > where people are using print statements after memory allocation fails. > Should I go ahead and send patches to remove those > redundant print statements? It all depends on where in the kernel those are. Some subsystems want minor cleanups like this, but most do not. So unless the issue you find is in the drivers/staging/ directory, I would not worry about it. thanks, greg k-h _______________________________________________ devel mailing list devel@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-07-11 7:00 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-07-10 12:20 [PATCH] staging: greybus: add logging statement when kfifo_alloc fails Keyur Patel 2019-07-10 12:20 ` Keyur Patel 2019-07-10 16:35 ` Greg Kroah-Hartman 2019-07-10 16:35 ` Greg Kroah-Hartman 2019-07-10 14:24 ` Keyur Patel 2019-07-10 14:24 ` Keyur Patel 2019-07-11 7:00 ` Greg Kroah-Hartman 2019-07-11 7:00 ` Greg Kroah-Hartman
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.