staging: most: remove redundant print statement when kfifo_alloc fails

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

---
 drivers/staging/most/cdev/cdev.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

index d0cc0b746107..bc0219ceac50 100644
@@ -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

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

---
Changes in v2:
- Edit subject line.

---
 drivers/staging/most/cdev/cdev.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

index d0cc0b746107..bc0219ceac50 100644
@@ -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

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

Changes in v3:
- fix checkpatch warrning

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

index d0cc0b746107..724d098aeef0 100644
@@ -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

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

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

Regards,
Markus

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

---
Changes in v3:
- fix checkpatch warning

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

index d0cc0b746107..724d098aeef0 100644
@@ -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

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

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.

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

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

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

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().

---
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(-)

index d0cc0b746107..724d098aeef0 100644
@@ -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

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

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.

There are differences to consider between the involved software developers.

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

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

Regards,
Markus

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

---
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(-)

index d0cc0b746107..724d098aeef0 100644
@@ -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

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

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

Regards,
Markus