[PATCH 2/2] staging: most: remove redundant print statement when kfifo_alloc fails

[PATCH 2/2] staging: most: remove redundant print statement when kfifo_alloc fails

[Keyur Patel]

This print statement is redundant as kfifo_alloc just calls kmalloc_array
and without the __GFP_NOWARN flag, already does a dump_stack().

Signed-off-by: Keyur Patel <iamkeyur96@gmail.com>
---
 drivers/staging/most/cdev/cdev.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/most/cdev/cdev.c b/drivers/staging/most/cdev/cdev.c
index d0cc0b746107..bc0219ceac50 100644
--- a/drivers/staging/most/cdev/cdev.c
+++ b/drivers/staging/most/cdev/cdev.c
@@ -463,10 +463,9 @@ static int comp_probe(struct most_interface *iface, int channel_id,
 	spin_lock_init(&c->unlink);
 	INIT_KFIFO(c->fifo);
 	retval = kfifo_alloc(&c->fifo, cfg->num_buffers, GFP_KERNEL);
-	if (retval) {
-		pr_info("failed to alloc channel kfifo");
+	if (retval)
 		goto err_del_cdev_and_free_channel;
-	}
+
 	init_waitqueue_head(&c->wq);
 	mutex_init(&c->io_mutex);
 	spin_lock_irqsave(&ch_list_lock, cl_flags);
-- 
2.22.0

[PATCH v2] staging: most: remove redundant print statement when

[Keyur Patel]

This print statement is redundant as kfifo_alloc just calls kmalloc_array
and without the __GFP_NOWARN flag, already does a dump_stack().

Signed-off-by: Keyur Patel <iamkeyur96@gmail.com>
---
Changes in v2:
- Edit subject line.
---
 drivers/staging/most/cdev/cdev.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/most/cdev/cdev.c b/drivers/staging/most/cdev/cdev.c
index d0cc0b746107..bc0219ceac50 100644
--- a/drivers/staging/most/cdev/cdev.c
+++ b/drivers/staging/most/cdev/cdev.c
@@ -463,10 +463,9 @@ static int comp_probe(struct most_interface *iface, int channel_id,
 	spin_lock_init(&c->unlink);
 	INIT_KFIFO(c->fifo);
 	retval = kfifo_alloc(&c->fifo, cfg->num_buffers, GFP_KERNEL);
-	if (retval) {
-		pr_info("failed to alloc channel kfifo");
+	if (retval)
 		goto err_del_cdev_and_free_channel;
-	}
+
 	init_waitqueue_head(&c->wq);
 	mutex_init(&c->io_mutex);
 	spin_lock_irqsave(&ch_list_lock, cl_flags);
-- 
2.22.0

[PATCH v3] staging: most: remove redundant print statement when kfifo_alloc fails

[Keyur Patel]

This print statement is redundant as kfifo_alloc just calls kmalloc_array
and without the __GFP_NOWARN flag, already does a dump_stack().

Signed-off-by: Keyur Patel <iamkeyur96@gmail.com>
Changes in v3:
- fix checkpatch warrning
---
 drivers/staging/most/cdev/cdev.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/staging/most/cdev/cdev.c b/drivers/staging/most/cdev/cdev.c
index d0cc0b746107..724d098aeef0 100644
--- a/drivers/staging/most/cdev/cdev.c
+++ b/drivers/staging/most/cdev/cdev.c
@@ -463,10 +463,8 @@ static int comp_probe(struct most_interface *iface, int channel_id,
 	spin_lock_init(&c->unlink);
 	INIT_KFIFO(c->fifo);
 	retval = kfifo_alloc(&c->fifo, cfg->num_buffers, GFP_KERNEL);
-	if (retval) {
-		pr_info("failed to alloc channel kfifo");
+	if (retval)
 		goto err_del_cdev_and_free_channel;
-	}
 	init_waitqueue_head(&c->wq);
 	mutex_init(&c->io_mutex);
 	spin_lock_irqsave(&ch_list_lock, cl_flags);
-- 
2.22.0

Re: [v3] staging: most: remove redundant print statement when kfifo_alloc fails

[Markus Elfring]

> This print statement is redundant as kfifo_alloc just calls kmalloc_array
> and without the __GFP_NOWARN flag, already does a dump_stack().

I suggest to omit the word “and” from this sentence.
Will any further wording adjustments become helpful for commit descriptions?


> Changes in v3:
> - fix checkpatch warrning
> ---

Please move such version information below the triple dashes without a typo.

Regards,
Markus

[PATCH v4] staging: most: remove redundant print statement when kfifo_alloc fails

[Keyur Patel]

This print statement is redundant as kfifo_alloc just calls kmalloc_array
without the __GFP_NOWARN flag, already does a dump_stack().

Signed-off-by: Keyur Patel <iamkeyur96@gmail.com>
---
Changes in v3:
- fix checkpatch warning

 drivers/staging/most/cdev/cdev.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/staging/most/cdev/cdev.c b/drivers/staging/most/cdev/cdev.c
index d0cc0b746107..724d098aeef0 100644
--- a/drivers/staging/most/cdev/cdev.c
+++ b/drivers/staging/most/cdev/cdev.c
@@ -463,10 +463,8 @@ static int comp_probe(struct most_interface *iface, int channel_id,
 	spin_lock_init(&c->unlink);
 	INIT_KFIFO(c->fifo);
 	retval = kfifo_alloc(&c->fifo, cfg->num_buffers, GFP_KERNEL);
-	if (retval) {
-		pr_info("failed to alloc channel kfifo");
+	if (retval)
 		goto err_del_cdev_and_free_channel;
-	}
 	init_waitqueue_head(&c->wq);
 	mutex_init(&c->io_mutex);
 	spin_lock_irqsave(&ch_list_lock, cl_flags);
-- 
2.22.0

Re: [v4] staging: most: remove redundant print statement when kfifo_alloc fails

[Markus Elfring]

> ---
> Changes in v3:

Thanks for your quick response.

I find the change log incomplete (even if corresponding information
can be determined also from public message archives).

Regards,
Markus

Re: [v4] staging: most: remove redundant print statement when kfifo_alloc fails

[Keyur Patel]

Re: [v4] staging: most: remove redundant print statement when kfifo_alloc fails

[Keyur Patel]

I didn't get you. I stiil need to update changelog and send more 
version or not. If you say so, I can send one more.

Thnaks.
On Sun, Jul 14, 2019 at 05:23:34PM +0200, Markus Elfring wrote:
> > ---
> > Changes in v3:
> 
> Thanks for your quick response.
> 
> I find the change log incomplete (even if corresponding information
> can be determined also from public message archives).
> 
> Regards,
> Markus

Re: [v4] staging: most: remove redundant print statement when kfifo_alloc fails

[Markus Elfring]

> I didn't get you. I stiil need to update changelog

I would appreciate the completion of the listing for V2 till V4.
I guess that a message resend could be sufficient for these adjustments.


> and send more version

This could be another opportunity if you would like to improve
the commit description considerably.
How do you think about previous clarification attempts on a topic like
“Delete an error message for a failed memory allocation”?

Regards,
Markus

Re: [v4] staging: most: remove redundant print statement when kfifo_alloc fails

[Greg Kroah-Hartman]

On Sun, Jul 14, 2019 at 11:47:37AM -0400, Keyur Patel wrote:
> I didn't get you. I stiil need to update changelog and send more 
> version or not. If you say so, I can send one more.

Please note that the person/bot you are responding to is on in my email
blacklist, and many others, so I wouldn't worry too much about the
responses.

I'll review this patch once 5.3-rc1 is out as I can't do anything with
it during the merge window.

thanks,

greg k-h

[PATCH v4] staging: most: Delete an error message for a failed memory allocation

[Keyur Patel]

This error message for a failed memory allocation is redundant as 
kfifo_alloc just calls kmalloc_array without the __GFP_NOWARN flag,
already does a dump_stack().

Signed-off-by: Keyur Patel <iamkeyur96@gmail.com>
---
Changes in v4:
- change subject line
- improve commit description
- fix checkpatch warning

 drivers/staging/most/cdev/cdev.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/staging/most/cdev/cdev.c b/drivers/staging/most/cdev/cdev.c
index d0cc0b746107..724d098aeef0 100644
--- a/drivers/staging/most/cdev/cdev.c
+++ b/drivers/staging/most/cdev/cdev.c
@@ -463,10 +463,8 @@ static int comp_probe(struct most_interface *iface, int channel_id,
 	spin_lock_init(&c->unlink);
 	INIT_KFIFO(c->fifo);
 	retval = kfifo_alloc(&c->fifo, cfg->num_buffers, GFP_KERNEL);
-	if (retval) {
-		pr_info("failed to alloc channel kfifo");
+	if (retval)
 		goto err_del_cdev_and_free_channel;
-	}
 	init_waitqueue_head(&c->wq);
 	mutex_init(&c->io_mutex);
 	spin_lock_irqsave(&ch_list_lock, cl_flags);
-- 
2.22.0

Re: [v4] staging: most: Delete an error message for a failed memory allocation

[Markus Elfring]

> ---
> Changes in v4:

I find this change log still incomplete.

You have chosen to adjust the commit message once more.
(Some contributors might be also not satisfied with this variant.)

Such a change requires to increase the corresponding patch version number,
doesn't it?

Regards,
Markus

Re: [v4] staging: most: Delete an error message for a failed memory allocation

[Keyur Patel]

I think commit message is clear enough to understand why this is needed.
You can send me what should I include in commit description and I will
add and send it again. Otherwise, Greg can comment on this.

Thanks.


On Sun, Jul 14, 2019 at 06:55:30PM +0200, Markus Elfring wrote:
> > ---
> > Changes in v4:
> 
> I find this change log still incomplete.
> 
> You have chosen to adjust the commit message once more.
> (Some contributors might be also not satisfied with this variant.)
> 
> Such a change requires to increase the corresponding patch version number,
> doesn't it?
> 
> Regards,
> Markus

Re: [v4] staging: most: Delete an error message for a failed memory allocation

[Markus Elfring]

> I think commit message is clear enough to understand why this is needed.

There are differences to consider between the involved software developers.


> You can send me what should I include in commit description

The clarification should be continued with the number “v5”
in the message subject.


> and I will add and send it again. Otherwise, Greg can comment on this.

Would you like to recheck information sources for the safe description
of the Linux allocation failure report?

Regards,
Markus

[PATCH v5] staging: most: Delete an error message for a failed memory allocation

[Keyur Patel]

The kfifo_alloc() failure generates enough information and doesn't need 
to be accompanied by another error statement.

Signed-off-by: Keyur Patel <iamkeyur96@gmail.com>
---
Changes in v5:
- change subject line
- simplify commit description
- fix checkpatch warning
- removed braces around if

 drivers/staging/most/cdev/cdev.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/staging/most/cdev/cdev.c b/drivers/staging/most/cdev/cdev.c
index d0cc0b746107..724d098aeef0 100644
--- a/drivers/staging/most/cdev/cdev.c
+++ b/drivers/staging/most/cdev/cdev.c
@@ -463,10 +463,8 @@ static int comp_probe(struct most_interface *iface, int channel_id,
 	spin_lock_init(&c->unlink);
 	INIT_KFIFO(c->fifo);
 	retval = kfifo_alloc(&c->fifo, cfg->num_buffers, GFP_KERNEL);
-	if (retval) {
-		pr_info("failed to alloc channel kfifo");
+	if (retval)
 		goto err_del_cdev_and_free_channel;
-	}
 	init_waitqueue_head(&c->wq);
 	mutex_init(&c->io_mutex);
 	spin_lock_irqsave(&ch_list_lock, cl_flags);
-- 
2.22.0

Re: [v5] staging: most: Delete an error message for a failed memory allocation

[Markus Elfring]

> The kfifo_alloc() failure generates enough information and doesn't need
> to be accompanied by another error statement.

I am curious how the acceptance will evolve for this variant of
another different commit description according to a known software
transformation pattern.


> ---
> Changes in v5:

The subsequent listing should usually be split between V2 till V5
for such a patch change log.

Regards,
Markus