This series fixes some warnings edmited by the checkpatch in the file
vmbus_drv.c. I thought it would be good to split each fix into a commit to
help with the review.
Matheus Castello (6):
drivers: hv: Fix hyperv_record_panic_msg path on comment
drivers: hv: vmbus: Replace symbolic permissions by octal permissions
drivers: hv: vmbus: Fix checkpatch LINE_SPACING
drivers: hv: vmbus: Fix checkpatch SPLIT_STRING
drivers: hv: vmbus: Fix unnecessary OOM_MESSAGE
drivers: hv: vmbus: Fix call msleep using < 20ms
drivers/hv/vmbus_drv.c | 55 +++++++++++++++++++++++-------------------
1 file changed, 30 insertions(+), 25 deletions(-)
--
2.28.0
Fixed checkpatch warning: MSLEEP: msleep < 20ms can sleep for up to
20ms; see Documentation/timers/timers-howto.rst
Signed-off-by: Matheus Castello <[email protected]>
---
drivers/hv/vmbus_drv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 774b88dd0e15..cf49c0c01206 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -2382,7 +2382,7 @@ static int vmbus_bus_suspend(struct device *dev)
* We wait here until the completion of any channel
* offers that are currently in progress.
*/
- msleep(1);
+ usleep_range(1000, 2000);
}
mutex_lock(&vmbus_connection.channel_mutex);
--
2.28.0
Fix the kernel parameter path in the comment, in the documentation the
parameter is correct but if someone who is studying the code and see
this first can get confused and try to access the wrong path/parameter
Signed-off-by: Matheus Castello <[email protected]>
---
drivers/hv/vmbus_drv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 4fad3e6745e5..9ed7e3b1d654 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -55,7 +55,7 @@ int vmbus_interrupt;
/*
* Boolean to control whether to report panic messages over Hyper-V.
*
- * It can be set via /proc/sys/kernel/hyperv/record_panic_msg
+ * It can be set via /proc/sys/kernel/hyperv_record_panic_msg
*/
static int sysctl_record_panic_msg = 1;
--
2.28.0
Checkpatch emits WARNING: quoted string split across lines.
To keep the code clean and with the 80 column length indentation the
check and registration code for kmsg_dump_register has been transferred
to a new function hv_kmsg_dump_register.
Signed-off-by: Matheus Castello <[email protected]>
---
drivers/hv/vmbus_drv.c | 34 +++++++++++++++++++---------------
1 file changed, 19 insertions(+), 15 deletions(-)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 61d28c743263..09d8236a51cf 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1387,6 +1387,23 @@ static struct kmsg_dumper hv_kmsg_dumper = {
.dump = hv_kmsg_dump,
};
+static void hv_kmsg_dump_register(void)
+{
+ int ret;
+
+ hv_panic_page = (void *)hv_alloc_hyperv_zeroed_page();
+ if (hv_panic_page) {
+ ret = kmsg_dump_register(&hv_kmsg_dumper);
+ if (ret) {
+ pr_err("Hyper-V: kmsg dump register error 0x%x\n", ret);
+ hv_free_hyperv_page((unsigned long)hv_panic_page);
+ hv_panic_page = NULL;
+ }
+ } else {
+ pr_err("Hyper-V: panic message page memory allocation failed");
+ }
+}
+
static struct ctl_table_header *hv_ctl_table_hdr;
/*
@@ -1477,21 +1494,8 @@ static int vmbus_bus_init(void)
* capability is supported by the hypervisor.
*/
hv_get_crash_ctl(hyperv_crash_ctl);
- if (hyperv_crash_ctl & HV_CRASH_CTL_CRASH_NOTIFY_MSG) {
- hv_panic_page = (void *)hv_alloc_hyperv_zeroed_page();
- if (hv_panic_page) {
- ret = kmsg_dump_register(&hv_kmsg_dumper);
- if (ret) {
- pr_err("Hyper-V: kmsg dump register "
- "error 0x%x\n", ret);
- hv_free_hyperv_page(
- (unsigned long)hv_panic_page);
- hv_panic_page = NULL;
- }
- } else
- pr_err("Hyper-V: panic message page memory "
- "allocation failed");
- }
+ if (hyperv_crash_ctl & HV_CRASH_CTL_CRASH_NOTIFY_MSG)
+ hv_kmsg_dump_register();
register_die_notifier(&hyperv_die_block);
}
--
2.28.0
This fixed the below checkpatch issue:
WARNING: Symbolic permissions 'S_IRUGO' are not preferred.
Consider using octal permissions '0444'.
Signed-off-by: Matheus Castello <[email protected]>
---
drivers/hv/vmbus_drv.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 9ed7e3b1d654..52c1407c1849 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1812,7 +1812,7 @@ static ssize_t channel_pending_show(struct vmbus_channel *channel,
channel_pending(channel,
vmbus_connection.monitor_pages[1]));
}
-static VMBUS_CHAN_ATTR(pending, S_IRUGO, channel_pending_show, NULL);
+static VMBUS_CHAN_ATTR(pending, 0444, channel_pending_show, NULL);
static ssize_t channel_latency_show(struct vmbus_channel *channel,
char *buf)
@@ -1821,19 +1821,19 @@ static ssize_t channel_latency_show(struct vmbus_channel *channel,
channel_latency(channel,
vmbus_connection.monitor_pages[1]));
}
-static VMBUS_CHAN_ATTR(latency, S_IRUGO, channel_latency_show, NULL);
+static VMBUS_CHAN_ATTR(latency, 0444, channel_latency_show, NULL);
static ssize_t channel_interrupts_show(struct vmbus_channel *channel, char *buf)
{
return sprintf(buf, "%llu\n", channel->interrupts);
}
-static VMBUS_CHAN_ATTR(interrupts, S_IRUGO, channel_interrupts_show, NULL);
+static VMBUS_CHAN_ATTR(interrupts, 0444, channel_interrupts_show, NULL);
static ssize_t channel_events_show(struct vmbus_channel *channel, char *buf)
{
return sprintf(buf, "%llu\n", channel->sig_events);
}
-static VMBUS_CHAN_ATTR(events, S_IRUGO, channel_events_show, NULL);
+static VMBUS_CHAN_ATTR(events, 0444, channel_events_show, NULL);
static ssize_t channel_intr_in_full_show(struct vmbus_channel *channel,
char *buf)
@@ -1872,7 +1872,7 @@ static ssize_t subchannel_monitor_id_show(struct vmbus_channel *channel,
{
return sprintf(buf, "%u\n", channel->offermsg.monitorid);
}
-static VMBUS_CHAN_ATTR(monitor_id, S_IRUGO, subchannel_monitor_id_show, NULL);
+static VMBUS_CHAN_ATTR(monitor_id, 0444, subchannel_monitor_id_show, NULL);
static ssize_t subchannel_id_show(struct vmbus_channel *channel,
char *buf)
--
2.28.0
Fixed checkpatch warning: Possible unnecessary 'out of memory' message
checkpatch(OOM_MESSAGE)
Signed-off-by: Matheus Castello <[email protected]>
---
drivers/hv/vmbus_drv.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 09d8236a51cf..774b88dd0e15 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1989,10 +1989,8 @@ struct hv_device *vmbus_device_create(const guid_t *type,
struct hv_device *child_device_obj;
child_device_obj = kzalloc(sizeof(struct hv_device), GFP_KERNEL);
- if (!child_device_obj) {
- pr_err("Unable to allocate device object for child device\n");
+ if (!child_device_obj)
return NULL;
- }
child_device_obj->channel = channel;
guid_copy(&child_device_obj->dev_type, type);
--
2.28.0
From: Matheus Castello <[email protected]> Sent: Sunday, November 15, 2020 11:57 AM
>
> Fix the kernel parameter path in the comment, in the documentation the
> parameter is correct but if someone who is studying the code and see
> this first can get confused and try to access the wrong path/parameter
>
> Signed-off-by: Matheus Castello <[email protected]>
> ---
> drivers/hv/vmbus_drv.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index 4fad3e6745e5..9ed7e3b1d654 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -55,7 +55,7 @@ int vmbus_interrupt;
> /*
> * Boolean to control whether to report panic messages over Hyper-V.
> *
> - * It can be set via /proc/sys/kernel/hyperv/record_panic_msg
> + * It can be set via /proc/sys/kernel/hyperv_record_panic_msg
> */
> static int sysctl_record_panic_msg = 1;
>
> --
> 2.28.0
Reviewed-by: Michael Kelley <[email protected]>
From: Matheus Castello <[email protected]> Sent: Sunday, November 15, 2020 11:58 AM
>
> Fixed checkpatch warning: MSLEEP: msleep < 20ms can sleep for up to
> 20ms; see Documentation/timers/timers-howto.rst
>
> Signed-off-by: Matheus Castello <[email protected]>
> ---
> drivers/hv/vmbus_drv.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index 774b88dd0e15..cf49c0c01206 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -2382,7 +2382,7 @@ static int vmbus_bus_suspend(struct device *dev)
> * We wait here until the completion of any channel
> * offers that are currently in progress.
> */
> - msleep(1);
> + usleep_range(1000, 2000);
> }
>
> mutex_lock(&vmbus_connection.channel_mutex);
> --
> 2.28.0
Reviewed-by: Michael Kelley <[email protected]>
From: Matheus Castello <[email protected]> Sent: Sunday, November 15, 2020 11:58 AM
>
> This fixed the below checkpatch issue:
> WARNING: Symbolic permissions 'S_IRUGO' are not preferred.
> Consider using octal permissions '0444'.
>
> Signed-off-by: Matheus Castello <[email protected]>
> ---
> drivers/hv/vmbus_drv.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index 9ed7e3b1d654..52c1407c1849 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -1812,7 +1812,7 @@ static ssize_t channel_pending_show(struct vmbus_channel
> *channel,
> channel_pending(channel,
> vmbus_connection.monitor_pages[1]));
> }
> -static VMBUS_CHAN_ATTR(pending, S_IRUGO, channel_pending_show, NULL);
> +static VMBUS_CHAN_ATTR(pending, 0444, channel_pending_show, NULL);
>
> static ssize_t channel_latency_show(struct vmbus_channel *channel,
> char *buf)
> @@ -1821,19 +1821,19 @@ static ssize_t channel_latency_show(struct vmbus_channel
> *channel,
> channel_latency(channel,
> vmbus_connection.monitor_pages[1]));
> }
> -static VMBUS_CHAN_ATTR(latency, S_IRUGO, channel_latency_show, NULL);
> +static VMBUS_CHAN_ATTR(latency, 0444, channel_latency_show, NULL);
>
> static ssize_t channel_interrupts_show(struct vmbus_channel *channel, char *buf)
> {
> return sprintf(buf, "%llu\n", channel->interrupts);
> }
> -static VMBUS_CHAN_ATTR(interrupts, S_IRUGO, channel_interrupts_show, NULL);
> +static VMBUS_CHAN_ATTR(interrupts, 0444, channel_interrupts_show, NULL);
>
> static ssize_t channel_events_show(struct vmbus_channel *channel, char *buf)
> {
> return sprintf(buf, "%llu\n", channel->sig_events);
> }
> -static VMBUS_CHAN_ATTR(events, S_IRUGO, channel_events_show, NULL);
> +static VMBUS_CHAN_ATTR(events, 0444, channel_events_show, NULL);
>
> static ssize_t channel_intr_in_full_show(struct vmbus_channel *channel,
> char *buf)
> @@ -1872,7 +1872,7 @@ static ssize_t subchannel_monitor_id_show(struct
> vmbus_channel *channel,
> {
> return sprintf(buf, "%u\n", channel->offermsg.monitorid);
> }
> -static VMBUS_CHAN_ATTR(monitor_id, S_IRUGO, subchannel_monitor_id_show, NULL);
> +static VMBUS_CHAN_ATTR(monitor_id, 0444, subchannel_monitor_id_show, NULL);
>
> static ssize_t subchannel_id_show(struct vmbus_channel *channel,
> char *buf)
> --
> 2.28.0
Reviewed-by: Michael Kelley <[email protected]>
From: Matheus Castello <[email protected]> Sent: Sunday, November 15, 2020 11:58 AM
>
> Checkpatch emits WARNING: quoted string split across lines.
> To keep the code clean and with the 80 column length indentation the
> check and registration code for kmsg_dump_register has been transferred
> to a new function hv_kmsg_dump_register.
>
> Signed-off-by: Matheus Castello <[email protected]>
> ---
> drivers/hv/vmbus_drv.c | 34 +++++++++++++++++++---------------
> 1 file changed, 19 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index 61d28c743263..09d8236a51cf 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -1387,6 +1387,23 @@ static struct kmsg_dumper hv_kmsg_dumper = {
> .dump = hv_kmsg_dump,
> };
>
> +static void hv_kmsg_dump_register(void)
> +{
> + int ret;
> +
> + hv_panic_page = (void *)hv_alloc_hyperv_zeroed_page();
> + if (hv_panic_page) {
> + ret = kmsg_dump_register(&hv_kmsg_dumper);
> + if (ret) {
> + pr_err("Hyper-V: kmsg dump register error 0x%x\n", ret);
> + hv_free_hyperv_page((unsigned long)hv_panic_page);
> + hv_panic_page = NULL;
> + }
> + } else {
> + pr_err("Hyper-V: panic message page memory allocation failed");
> + }
> +}
> +
The above would be marginally better if organized as follows so that the
main execution path isn't in an "if" clause. Also reduces indentation.
hv_panic_page = (void *)hv_alloc_hyperv_zeroed_page();
if (!hv_panic_page) {
pr_err("Hyper-V: panic message page memory allocation failed");
return;
}
ret = kmsg_dump_register(&hv_kmsg_dumper);
if (ret) {
pr_err("Hyper-V: kmsg dump register error 0x%x\n", ret);
hv_free_hyperv_page((unsigned long)hv_panic_page);
hv_panic_page = NULL;
}
> static struct ctl_table_header *hv_ctl_table_hdr;
>
> /*
> @@ -1477,21 +1494,8 @@ static int vmbus_bus_init(void)
> * capability is supported by the hypervisor.
> */
> hv_get_crash_ctl(hyperv_crash_ctl);
> - if (hyperv_crash_ctl & HV_CRASH_CTL_CRASH_NOTIFY_MSG) {
> - hv_panic_page = (void *)hv_alloc_hyperv_zeroed_page();
> - if (hv_panic_page) {
> - ret = kmsg_dump_register(&hv_kmsg_dumper);
> - if (ret) {
> - pr_err("Hyper-V: kmsg dump register "
> - "error 0x%x\n", ret);
> - hv_free_hyperv_page(
> - (unsigned long)hv_panic_page);
> - hv_panic_page = NULL;
> - }
> - } else
> - pr_err("Hyper-V: panic message page memory "
> - "allocation failed");
> - }
> + if (hyperv_crash_ctl & HV_CRASH_CTL_CRASH_NOTIFY_MSG)
> + hv_kmsg_dump_register();
>
> register_die_notifier(&hyperv_die_block);
> }
> --
> 2.28.0
On Sun, Nov 15, 2020 at 04:57:33PM -0300, Matheus Castello wrote:
> Fixed checkpatch warning: Possible unnecessary 'out of memory' message
> checkpatch(OOM_MESSAGE)
>
> Signed-off-by: Matheus Castello <[email protected]>
> ---
> drivers/hv/vmbus_drv.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index 09d8236a51cf..774b88dd0e15 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -1989,10 +1989,8 @@ struct hv_device *vmbus_device_create(const guid_t *type,
> struct hv_device *child_device_obj;
>
> child_device_obj = kzalloc(sizeof(struct hv_device), GFP_KERNEL);
> - if (!child_device_obj) {
> - pr_err("Unable to allocate device object for child device\n");
> + if (!child_device_obj)
The generic OOM message would give you a stack dump but not as specific
/ clear as the message you deleted.
Also, the original intent of this check was to check for things like
printk("Out of memory");
which was clearly redundant. The message we print here is not that.
See https://lkml.org/lkml/2014/6/10/382 .
Wei.
> return NULL;
> - }
>
> child_device_obj->channel = channel;
> guid_copy(&child_device_obj->dev_type, type);
> --
> 2.28.0
>
On Mon, Nov 16, 2020 at 11:21:48AM +0000, Wei Liu wrote:
> On Sun, Nov 15, 2020 at 04:57:33PM -0300, Matheus Castello wrote:
> > Fixed checkpatch warning: Possible unnecessary 'out of memory' message
> > checkpatch(OOM_MESSAGE)
> >
> > Signed-off-by: Matheus Castello <[email protected]>
> > ---
> > drivers/hv/vmbus_drv.c | 4 +---
> > 1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> > index 09d8236a51cf..774b88dd0e15 100644
> > --- a/drivers/hv/vmbus_drv.c
> > +++ b/drivers/hv/vmbus_drv.c
> > @@ -1989,10 +1989,8 @@ struct hv_device *vmbus_device_create(const guid_t *type,
> > struct hv_device *child_device_obj;
> >
> > child_device_obj = kzalloc(sizeof(struct hv_device), GFP_KERNEL);
> > - if (!child_device_obj) {
> > - pr_err("Unable to allocate device object for child device\n");
> > + if (!child_device_obj)
>
> The generic OOM message would give you a stack dump but not as specific
> / clear as the message you deleted.
>
> Also, the original intent of this check was to check for things like
>
> printk("Out of memory");
>
> which was clearly redundant. The message we print here is not that.
>
> See https://lkml.org/lkml/2014/6/10/382 .
>
In case my message is not clear, I think this pr_err should stay. ;-)
Wei.
On Sun, Nov 15, 2020 at 04:57:28PM -0300, Matheus Castello wrote:
> This series fixes some warnings edmited by the checkpatch in the file
> vmbus_drv.c. I thought it would be good to split each fix into a commit to
> help with the review.
>
> Matheus Castello (6):
> drivers: hv: Fix hyperv_record_panic_msg path on comment
> drivers: hv: vmbus: Replace symbolic permissions by octal permissions
> drivers: hv: vmbus: Fix checkpatch LINE_SPACING
> drivers: hv: vmbus: Fix checkpatch SPLIT_STRING
> drivers: hv: vmbus: Fix unnecessary OOM_MESSAGE
> drivers: hv: vmbus: Fix call msleep using < 20ms
I've pushed patch 1-3 and 6 to hyperv-next.
Patch 4 has a pending comment from Michael. Patch 5 can be dropped.
Wei.
Hi Michael,
Em 11/15/2020 7:25 PM, Michael Kelley escreveu:
> From: Matheus Castello <[email protected]> Sent: Sunday, November 15, 2020 11:58 AM
>>
>> Checkpatch emits WARNING: quoted string split across lines.
>> To keep the code clean and with the 80 column length indentation the
>> check and registration code for kmsg_dump_register has been transferred
>> to a new function hv_kmsg_dump_register.
>>
>> Signed-off-by: Matheus Castello <[email protected]>
>> ---
>> drivers/hv/vmbus_drv.c | 34 +++++++++++++++++++---------------
>> 1 file changed, 19 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
>> index 61d28c743263..09d8236a51cf 100644
>> --- a/drivers/hv/vmbus_drv.c
>> +++ b/drivers/hv/vmbus_drv.c
>> @@ -1387,6 +1387,23 @@ static struct kmsg_dumper hv_kmsg_dumper = {
>> .dump = hv_kmsg_dump,
>> };
>>
>> +static void hv_kmsg_dump_register(void)
>> +{
>> + int ret;
>> +
>> + hv_panic_page = (void *)hv_alloc_hyperv_zeroed_page();
>> + if (hv_panic_page) {
>> + ret = kmsg_dump_register(&hv_kmsg_dumper);
>> + if (ret) {
>> + pr_err("Hyper-V: kmsg dump register error 0x%x\n", ret);
>> + hv_free_hyperv_page((unsigned long)hv_panic_page);
>> + hv_panic_page = NULL;
>> + }
>> + } else {
>> + pr_err("Hyper-V: panic message page memory allocation failed");
>> + }
>> +}
>> +
>
> The above would be marginally better if organized as follows so that the
> main execution path isn't in an "if" clause. Also reduces indentation.
>
> hv_panic_page = (void *)hv_alloc_hyperv_zeroed_page();
> if (!hv_panic_page) {
> pr_err("Hyper-V: panic message page memory allocation failed");
> return;
> }
> ret = kmsg_dump_register(&hv_kmsg_dumper);
> if (ret) {
> pr_err("Hyper-V: kmsg dump register error 0x%x\n", ret);
> hv_free_hyperv_page((unsigned long)hv_panic_page);
> hv_panic_page = NULL;
> }
>
>
Thanks for the review, great I will use it on the v2.
>> static struct ctl_table_header *hv_ctl_table_hdr;
>>
>> /*
>> @@ -1477,21 +1494,8 @@ static int vmbus_bus_init(void)
>> * capability is supported by the hypervisor.
>> */
>> hv_get_crash_ctl(hyperv_crash_ctl);
>> - if (hyperv_crash_ctl & HV_CRASH_CTL_CRASH_NOTIFY_MSG) {
>> - hv_panic_page = (void *)hv_alloc_hyperv_zeroed_page();
>> - if (hv_panic_page) {
>> - ret = kmsg_dump_register(&hv_kmsg_dumper);
>> - if (ret) {
>> - pr_err("Hyper-V: kmsg dump register "
>> - "error 0x%x\n", ret);
>> - hv_free_hyperv_page(
>> - (unsigned long)hv_panic_page);
>> - hv_panic_page = NULL;
>> - }
>> - } else
>> - pr_err("Hyper-V: panic message page memory "
>> - "allocation failed");
>> - }
>> + if (hyperv_crash_ctl & HV_CRASH_CTL_CRASH_NOTIFY_MSG)
>> + hv_kmsg_dump_register();
>>
>> register_die_notifier(&hyperv_die_block);
>> }
>> --
>> 2.28.0
>
Hi Wei,
Em 11/17/2020 7:58 AM, Wei Liu escreveu:
> On Sun, Nov 15, 2020 at 04:57:28PM -0300, Matheus Castello wrote:
>> This series fixes some warnings edmited by the checkpatch in the file
>> vmbus_drv.c. I thought it would be good to split each fix into a commit to
>> help with the review.
>>
>> Matheus Castello (6):
>> drivers: hv: Fix hyperv_record_panic_msg path on comment
>> drivers: hv: vmbus: Replace symbolic permissions by octal permissions
>> drivers: hv: vmbus: Fix checkpatch LINE_SPACING
>> drivers: hv: vmbus: Fix checkpatch SPLIT_STRING
>> drivers: hv: vmbus: Fix unnecessary OOM_MESSAGE
>> drivers: hv: vmbus: Fix call msleep using < 20ms
>
> I've pushed patch 1-3 and 6 to hyperv-next.
>
> Patch 4 has a pending comment from Michael. Patch 5 can be dropped.
>
Thanks for the review, got it and I agree with the drop of patch 5
thanks to point it. I will send the v2 for the patch 4.
> Wei.
>
On Tue, 2020-11-24 at 21:54 -0300, Matheus Castello wrote:
> > > diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
[]
> > The above would be marginally better if organized as follows so that the
> > main execution path isn't in an "if" clause. Also reduces indentation.
> >
> > hv_panic_page = (void *)hv_alloc_hyperv_zeroed_page();
> > if (!hv_panic_page) {
> > pr_err("Hyper-V: panic message page memory allocation failed");
And nicer to add a terminating newline to the format like the pr_err below.
> > return;
> > }
> > ret = kmsg_dump_register(&hv_kmsg_dumper);
> > if (ret) {
> > pr_err("Hyper-V: kmsg dump register error 0x%x\n", ret);
> > hv_free_hyperv_page((unsigned long)hv_panic_page);
> > hv_panic_page = NULL;
> > }
Em 11/24/2020 11:56 PM, Joe Perches escreveu:
> On Tue, 2020-11-24 at 21:54 -0300, Matheus Castello wrote:
>>>> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> []
>>> The above would be marginally better if organized as follows so that the
>>> main execution path isn't in an "if" clause. Also reduces indentation.
>>>
>>> hv_panic_page = (void *)hv_alloc_hyperv_zeroed_page();
>>> if (!hv_panic_page) {
>>> pr_err("Hyper-V: panic message page memory allocation failed");
>
> And nicer to add a terminating newline to the format like the pr_err below.
>
Oops, sending v3. Thanks Joe Perches!
>>> return;
>>> }
>>> ret = kmsg_dump_register(&hv_kmsg_dumper);
>>> if (ret) {
>>> pr_err("Hyper-V: kmsg dump register error 0x%x\n", ret);
>>> hv_free_hyperv_page((unsigned long)hv_panic_page);
>>> hv_panic_page = NULL;
>>> }
>
>