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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
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
> 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
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 <[email protected]>
---
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
> ---
> 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
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
> 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
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 <[email protected]>
---
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
> ---
> 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
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
> 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
The kfifo_alloc() failure generates enough information and doesn't need
to be accompanied by another error statement.
Signed-off-by: Keyur Patel <[email protected]>
---
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
> 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
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