2021-10-25 03:32:46

by Li Zhijian

[permalink] [raw]
Subject: [PATCH 3/3] refscale: always log the error message

Generally, error message should be logged anyhow.

Signed-off-by: Li Zhijian <[email protected]>
---
kernel/rcu/refscale.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/rcu/refscale.c b/kernel/rcu/refscale.c
index a4479f00dcdc..f055d168365a 100644
--- a/kernel/rcu/refscale.c
+++ b/kernel/rcu/refscale.c
@@ -58,8 +58,8 @@ do { \
} \
} while (0)

-#define VERBOSE_SCALEOUT_ERRSTRING(s, x...) \
- do { if (verbose) pr_alert("%s" SCALE_FLAG "!!! " s, scale_type, ## x); } while (0)
+#define SCALEOUT_ERRSTRING(s, x...) \
+ do { pr_alert("%s" SCALE_FLAG "!!! " s, scale_type, ## x); } while (0)

MODULE_LICENSE("GPL");
MODULE_AUTHOR("Joel Fernandes (Google) <[email protected]>");
@@ -651,7 +651,7 @@ static int main_func(void *arg)
result_avg = kzalloc(nruns * sizeof(*result_avg), GFP_KERNEL);
buf = kzalloc(800 + 64, GFP_KERNEL);
if (!result_avg || !buf) {
- VERBOSE_SCALEOUT_ERRSTRING("out of memory");
+ SCALEOUT_ERRSTRING("out of memory");
goto oom_exit;
}
if (holdoff)
@@ -837,7 +837,7 @@ ref_scale_init(void)
reader_tasks = kcalloc(nreaders, sizeof(reader_tasks[0]),
GFP_KERNEL);
if (!reader_tasks) {
- VERBOSE_SCALEOUT_ERRSTRING("out of memory");
+ SCALEOUT_ERRSTRING("out of memory");
firsterr = -ENOMEM;
goto unwind;
}
--
2.33.0




2021-10-25 07:00:13

by Zhijian Li (Fujitsu)

[permalink] [raw]
Subject: Re: [PATCH 3/3] refscale: always log the error message



On 25/10/2021 11:26, Li Zhijian wrote:
> Generally, error message should be logged anyhow.
>
> Signed-off-by: Li Zhijian <[email protected]>
> ---
> kernel/rcu/refscale.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/rcu/refscale.c b/kernel/rcu/refscale.c
> index a4479f00dcdc..f055d168365a 100644
> --- a/kernel/rcu/refscale.c
> +++ b/kernel/rcu/refscale.c
> @@ -58,8 +58,8 @@ do { \
> } \
> } while (0)
>
> -#define VERBOSE_SCALEOUT_ERRSTRING(s, x...) \
> - do { if (verbose) pr_alert("%s" SCALE_FLAG "!!! " s, scale_type, ## x); } while (0)
> +#define SCALEOUT_ERRSTRING(s, x...) \
> + do { pr_alert("%s" SCALE_FLAG "!!! " s, scale_type, ## x); } while (0)
>
> MODULE_LICENSE("GPL");
> MODULE_AUTHOR("Joel Fernandes (Google) <[email protected]>");
> @@ -651,7 +651,7 @@ static int main_func(void *arg)
> result_avg = kzalloc(nruns * sizeof(*result_avg), GFP_KERNEL);
> buf = kzalloc(800 + 64, GFP_KERNEL);
> if (!result_avg || !buf) {
> - VERBOSE_SCALEOUT_ERRSTRING("out of memory");
> + SCALEOUT_ERRSTRING("out of memory");

'\n' should be added to the last to flush it.



> goto oom_exit;
> }
> if (holdoff)
> @@ -837,7 +837,7 @@ ref_scale_init(void)
> reader_tasks = kcalloc(nreaders, sizeof(reader_tasks[0]),
> GFP_KERNEL);
> if (!reader_tasks) {
> - VERBOSE_SCALEOUT_ERRSTRING("out of memory");
> + SCALEOUT_ERRSTRING("out of memory");
ditto

Thanks
Zhijian
> firsterr = -ENOMEM;
> goto unwind;
> }

2021-10-26 17:29:16

by Paul E. McKenney

[permalink] [raw]
Subject: Re: [PATCH 3/3] refscale: always log the error message

On Mon, Oct 25, 2021 at 06:12:40AM +0000, [email protected] wrote:
>
>
> On 25/10/2021 11:26, Li Zhijian wrote:
> > Generally, error message should be logged anyhow.
> >
> > Signed-off-by: Li Zhijian <[email protected]>
> > ---
> > kernel/rcu/refscale.c | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/kernel/rcu/refscale.c b/kernel/rcu/refscale.c
> > index a4479f00dcdc..f055d168365a 100644
> > --- a/kernel/rcu/refscale.c
> > +++ b/kernel/rcu/refscale.c
> > @@ -58,8 +58,8 @@ do { \
> > } \
> > } while (0)
> >
> > -#define VERBOSE_SCALEOUT_ERRSTRING(s, x...) \
> > - do { if (verbose) pr_alert("%s" SCALE_FLAG "!!! " s, scale_type, ## x); } while (0)
> > +#define SCALEOUT_ERRSTRING(s, x...) \
> > + do { pr_alert("%s" SCALE_FLAG "!!! " s, scale_type, ## x); } while (0)
> >
> > MODULE_LICENSE("GPL");
> > MODULE_AUTHOR("Joel Fernandes (Google) <[email protected]>");
> > @@ -651,7 +651,7 @@ static int main_func(void *arg)
> > result_avg = kzalloc(nruns * sizeof(*result_avg), GFP_KERNEL);
> > buf = kzalloc(800 + 64, GFP_KERNEL);
> > if (!result_avg || !buf) {
> > - VERBOSE_SCALEOUT_ERRSTRING("out of memory");
> > + SCALEOUT_ERRSTRING("out of memory");
>
> '\n' should be added to the last to flush it.

And there might well be other missing "\n" instances in similar messages
in rcuscale.c, rcutorture.c, scftorture.c, locktorture.c, and torture.c.
Please feel free to send a patch for each file needing this help.

I queued your other three patches for v5.17 (not this coming merge window,
but the one after that), thank you! I did wordsmith the commit logs,
so please check to see if I messed anything up.

Thanx, Paul

> > goto oom_exit;
> > }
> > if (holdoff)
> > @@ -837,7 +837,7 @@ ref_scale_init(void)
> > reader_tasks = kcalloc(nreaders, sizeof(reader_tasks[0]),
> > GFP_KERNEL);
> > if (!reader_tasks) {
> > - VERBOSE_SCALEOUT_ERRSTRING("out of memory");
> > + SCALEOUT_ERRSTRING("out of memory");
> ditto
>
> Thanks
> Zhijian
> > firsterr = -ENOMEM;
> > goto unwind;
> > }

2021-10-27 17:32:44

by Zhijian Li (Fujitsu)

[permalink] [raw]
Subject: Re: [PATCH 3/3] refscale: always log the error message



On 26/10/2021 22:06, Paul E. McKenney wrote:
> On Mon, Oct 25, 2021 at 06:12:40AM +0000, [email protected] wrote:
>>
>> On 25/10/2021 11:26, Li Zhijian wrote:
>>> Generally, error message should be logged anyhow.
>>>
>>> Signed-off-by: Li Zhijian <[email protected]>
>>> ---
>>> kernel/rcu/refscale.c | 8 ++++----
>>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/kernel/rcu/refscale.c b/kernel/rcu/refscale.c
>>> index a4479f00dcdc..f055d168365a 100644
>>> --- a/kernel/rcu/refscale.c
>>> +++ b/kernel/rcu/refscale.c
>>> @@ -58,8 +58,8 @@ do { \
>>> } \
>>> } while (0)
>>>
>>> -#define VERBOSE_SCALEOUT_ERRSTRING(s, x...) \
>>> - do { if (verbose) pr_alert("%s" SCALE_FLAG "!!! " s, scale_type, ## x); } while (0)
>>> +#define SCALEOUT_ERRSTRING(s, x...) \
>>> + do { pr_alert("%s" SCALE_FLAG "!!! " s, scale_type, ## x); } while (0)
>>>
>>> MODULE_LICENSE("GPL");
>>> MODULE_AUTHOR("Joel Fernandes (Google) <[email protected]>");
>>> @@ -651,7 +651,7 @@ static int main_func(void *arg)
>>> result_avg = kzalloc(nruns * sizeof(*result_avg), GFP_KERNEL);
>>> buf = kzalloc(800 + 64, GFP_KERNEL);
>>> if (!result_avg || !buf) {
>>> - VERBOSE_SCALEOUT_ERRSTRING("out of memory");
>>> + SCALEOUT_ERRSTRING("out of memory");
>> '\n' should be added to the last to flush it.
> And there might well be other missing "\n" instances in similar messages
> in rcuscale.c, rcutorture.c, scftorture.c, locktorture.c, and torture.c.
> Please feel free to send a patch for each file needing this help.

Sure, i will do that.

Thanks
Zhijian

> I queued your other three patches for v5.17 (not this coming merge window,
> but the one after that), thank you! I did wordsmith the commit logs,
> so please check to see if I messed anything up.
>
> Thanx, Paul
>
>>> goto oom_exit;
>>> }
>>> if (holdoff)
>>> @@ -837,7 +837,7 @@ ref_scale_init(void)
>>> reader_tasks = kcalloc(nreaders, sizeof(reader_tasks[0]),
>>> GFP_KERNEL);
>>> if (!reader_tasks) {
>>> - VERBOSE_SCALEOUT_ERRSTRING("out of memory");
>>> + SCALEOUT_ERRSTRING("out of memory");
>> ditto
>>
>> Thanks
>> Zhijian
>>> firsterr = -ENOMEM;
>>> goto unwind;
>>> }

2021-10-27 20:37:52

by Zhijian Li (Fujitsu)

[permalink] [raw]
Subject: Re: [PATCH 3/3] refscale: always log the error message

Hi Paul

During adding the missing '\n', i also found there are some verbose control error message
Should we convert it like [PATCH 3/3] refscale: always log the error message

lizj@FNSTPC: linux$ git grep VERBOSE.*_ERRSTRING
include/linux/torture.h:#define VERBOSE_TOROUT_ERRSTRING(s) \
kernel/locking/locktorture.c: VERBOSE_TOROUT_ERRSTRING("writer_tasks: Out of memory");
kernel/locking/locktorture.c: VERBOSE_TOROUT_ERRSTRING("reader_tasks: Out of memory");
kernel/rcu/rcuscale.c:#define VERBOSE_SCALEOUT_ERRSTRING(s) \
kernel/rcu/rcuscale.c:          VERBOSE_SCALEOUT_ERRSTRING("All grace periods expedited, no normal ones to measure!");
kernel/rcu/rcuscale.c:          VERBOSE_SCALEOUT_ERRSTRING("All grace periods normal, no expedited ones to measure!");
kernel/rcu/rcuscale.c:          VERBOSE_SCALEOUT_ERRSTRING("No expedited async GPs, so went with async!");
kernel/rcu/rcuscale.c:          VERBOSE_SCALEOUT_ERRSTRING("out of memory");
kernel/rcu/rcuscale.c:          VERBOSE_SCALEOUT_ERRSTRING("out of memory");
kernel/rcu/rcutorture.c: VERBOSE_TOROUT_ERRSTRING("out of memory");
kernel/rcu/rcutorture.c: VERBOSE_TOROUT_ERRSTRING("out of memory");
kernel/rcu/rcutorture.c: VERBOSE_TOROUT_ERRSTRING("out of memory");
kernel/rcu/rcutorture.c: VERBOSE_TOROUT_ERRSTRING("out of memory");
kernel/scftorture.c:#define VERBOSE_SCFTORTOUT_ERRSTRING(s, x...) \
kernel/scftorture.c:            VERBOSE_SCFTORTOUT_ERRSTRING("all zero weights makes no sense");
kernel/scftorture.c:            VERBOSE_SCFTORTOUT_ERRSTRING("built as module, weight_resched ignored");
kernel/scftorture.c:            VERBOSE_SCFTORTOUT_ERRSTRING("out of memory");
kernel/torture.c:               VERBOSE_TOROUT_ERRSTRING("Failed to alloc mask");
kernel/torture.c:               VERBOSE_TOROUT_ERRSTRING(f);



Thanks



On 27/10/2021 13:17, Li Zhijian wrote:
>
>
> On 26/10/2021 22:06, Paul E. McKenney wrote:
>> On Mon, Oct 25, 2021 at 06:12:40AM +0000, [email protected] wrote:
>>>
>>> On 25/10/2021 11:26, Li Zhijian wrote:
>>>> Generally, error message should be logged anyhow.
>>>>
>>>> Signed-off-by: Li Zhijian <[email protected]>
>>>> ---
>>>>    kernel/rcu/refscale.c | 8 ++++----
>>>>    1 file changed, 4 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/kernel/rcu/refscale.c b/kernel/rcu/refscale.c
>>>> index a4479f00dcdc..f055d168365a 100644
>>>> --- a/kernel/rcu/refscale.c
>>>> +++ b/kernel/rcu/refscale.c
>>>> @@ -58,8 +58,8 @@ do {                                            \
>>>>        }                                        \
>>>>    } while (0)
>>>>    -#define VERBOSE_SCALEOUT_ERRSTRING(s, x...) \
>>>> -    do { if (verbose) pr_alert("%s" SCALE_FLAG "!!! " s, scale_type, ## x); } while (0)
>>>> +#define SCALEOUT_ERRSTRING(s, x...) \
>>>> +    do { pr_alert("%s" SCALE_FLAG "!!! " s, scale_type, ## x); } while (0)
>>>>       MODULE_LICENSE("GPL");
>>>>    MODULE_AUTHOR("Joel Fernandes (Google) <[email protected]>");
>>>> @@ -651,7 +651,7 @@ static int main_func(void *arg)
>>>>        result_avg = kzalloc(nruns * sizeof(*result_avg), GFP_KERNEL);
>>>>        buf = kzalloc(800 + 64, GFP_KERNEL);
>>>>        if (!result_avg || !buf) {
>>>> -        VERBOSE_SCALEOUT_ERRSTRING("out of memory");
>>>> +        SCALEOUT_ERRSTRING("out of memory");
>>> '\n' should be added to the last to flush it.
>> And there might well be other missing "\n" instances in similar messages
>> in rcuscale.c, rcutorture.c, scftorture.c, locktorture.c, and torture.c.
>> Please feel free to send a patch for each file needing this help.
>
> Sure, i will do that.
>
> Thanks
> Zhijian
>
>> I queued your other three patches for v5.17 (not this coming merge window,
>> but the one after that), thank you!  I did wordsmith the commit logs,
>> so please check to see if I messed anything up.
>>
>>                             Thanx, Paul
>>
>>>>            goto oom_exit;
>>>>        }
>>>>        if (holdoff)
>>>> @@ -837,7 +837,7 @@ ref_scale_init(void)
>>>>        reader_tasks = kcalloc(nreaders, sizeof(reader_tasks[0]),
>>>>                       GFP_KERNEL);
>>>>        if (!reader_tasks) {
>>>> -        VERBOSE_SCALEOUT_ERRSTRING("out of memory");
>>>> +        SCALEOUT_ERRSTRING("out of memory");
>>> ditto
>>>
>>> Thanks
>>> Zhijian
>>>>            firsterr = -ENOMEM;
>>>>            goto unwind;
>>>>        }
>

2021-10-28 13:33:37

by Paul E. McKenney

[permalink] [raw]
Subject: Re: [PATCH 3/3] refscale: always log the error message

On Wed, Oct 27, 2021 at 07:21:34AM +0000, [email protected] wrote:
> Hi Paul
>
> During adding the missing '\n', i also found there are some verbose control error message
> Should we convert it like [PATCH 3/3] refscale: always log the error message

These do indeed look like they should always be logged. Good catches!

Thanx, Paul

> lizj@FNSTPC: linux$ git grep VERBOSE.*_ERRSTRING
> include/linux/torture.h:#define VERBOSE_TOROUT_ERRSTRING(s) \
> kernel/locking/locktorture.c: VERBOSE_TOROUT_ERRSTRING("writer_tasks: Out of memory");
> kernel/locking/locktorture.c: VERBOSE_TOROUT_ERRSTRING("reader_tasks: Out of memory");
> kernel/rcu/rcuscale.c:#define VERBOSE_SCALEOUT_ERRSTRING(s) \
> kernel/rcu/rcuscale.c:????????? VERBOSE_SCALEOUT_ERRSTRING("All grace periods expedited, no normal ones to measure!");
> kernel/rcu/rcuscale.c:????????? VERBOSE_SCALEOUT_ERRSTRING("All grace periods normal, no expedited ones to measure!");
> kernel/rcu/rcuscale.c:????????? VERBOSE_SCALEOUT_ERRSTRING("No expedited async GPs, so went with async!");
> kernel/rcu/rcuscale.c:????????? VERBOSE_SCALEOUT_ERRSTRING("out of memory");
> kernel/rcu/rcuscale.c:????????? VERBOSE_SCALEOUT_ERRSTRING("out of memory");
> kernel/rcu/rcutorture.c: VERBOSE_TOROUT_ERRSTRING("out of memory");
> kernel/rcu/rcutorture.c: VERBOSE_TOROUT_ERRSTRING("out of memory");
> kernel/rcu/rcutorture.c: VERBOSE_TOROUT_ERRSTRING("out of memory");
> kernel/rcu/rcutorture.c: VERBOSE_TOROUT_ERRSTRING("out of memory");
> kernel/scftorture.c:#define VERBOSE_SCFTORTOUT_ERRSTRING(s, x...) \
> kernel/scftorture.c:??????????? VERBOSE_SCFTORTOUT_ERRSTRING("all zero weights makes no sense");
> kernel/scftorture.c:??????????? VERBOSE_SCFTORTOUT_ERRSTRING("built as module, weight_resched ignored");
> kernel/scftorture.c:??????????? VERBOSE_SCFTORTOUT_ERRSTRING("out of memory");
> kernel/torture.c:?????????????? VERBOSE_TOROUT_ERRSTRING("Failed to alloc mask");
> kernel/torture.c:?????????????? VERBOSE_TOROUT_ERRSTRING(f);
>
>
>
> Thanks
>
>
>
> On 27/10/2021 13:17, Li Zhijian wrote:
> >
> >
> > On 26/10/2021 22:06, Paul E. McKenney wrote:
> >> On Mon, Oct 25, 2021 at 06:12:40AM +0000, [email protected] wrote:
> >>>
> >>> On 25/10/2021 11:26, Li Zhijian wrote:
> >>>> Generally, error message should be logged anyhow.
> >>>>
> >>>> Signed-off-by: Li Zhijian <[email protected]>
> >>>> ---
> >>>> ?? kernel/rcu/refscale.c | 8 ++++----
> >>>> ?? 1 file changed, 4 insertions(+), 4 deletions(-)
> >>>>
> >>>> diff --git a/kernel/rcu/refscale.c b/kernel/rcu/refscale.c
> >>>> index a4479f00dcdc..f055d168365a 100644
> >>>> --- a/kernel/rcu/refscale.c
> >>>> +++ b/kernel/rcu/refscale.c
> >>>> @@ -58,8 +58,8 @@ do {??????????????????????????????????????????? \
> >>>> ?????? }??????????????????????????????????????? \
> >>>> ?? } while (0)
> >>>> ?? -#define VERBOSE_SCALEOUT_ERRSTRING(s, x...) \
> >>>> -??? do { if (verbose) pr_alert("%s" SCALE_FLAG "!!! " s, scale_type, ## x); } while (0)
> >>>> +#define SCALEOUT_ERRSTRING(s, x...) \
> >>>> +??? do { pr_alert("%s" SCALE_FLAG "!!! " s, scale_type, ## x); } while (0)
> >>>> ?? ?? MODULE_LICENSE("GPL");
> >>>> ?? MODULE_AUTHOR("Joel Fernandes (Google) <[email protected]>");
> >>>> @@ -651,7 +651,7 @@ static int main_func(void *arg)
> >>>> ?????? result_avg = kzalloc(nruns * sizeof(*result_avg), GFP_KERNEL);
> >>>> ?????? buf = kzalloc(800 + 64, GFP_KERNEL);
> >>>> ?????? if (!result_avg || !buf) {
> >>>> -??????? VERBOSE_SCALEOUT_ERRSTRING("out of memory");
> >>>> +??????? SCALEOUT_ERRSTRING("out of memory");
> >>> '\n' should be added to the last to flush it.
> >> And there might well be other missing "\n" instances in similar messages
> >> in rcuscale.c, rcutorture.c, scftorture.c, locktorture.c, and torture.c.
> >> Please feel free to send a patch for each file needing this help.
> >
> > Sure, i will do that.
> >
> > Thanks
> > Zhijian
> >
> >> I queued your other three patches for v5.17 (not this coming merge window,
> >> but the one after that), thank you!? I did wordsmith the commit logs,
> >> so please check to see if I messed anything up.
> >>
> >> ??????????????????????????? Thanx, Paul
> >>
> >>>> ?????????? goto oom_exit;
> >>>> ?????? }
> >>>> ?????? if (holdoff)
> >>>> @@ -837,7 +837,7 @@ ref_scale_init(void)
> >>>> ?????? reader_tasks = kcalloc(nreaders, sizeof(reader_tasks[0]),
> >>>> ????????????????????? GFP_KERNEL);
> >>>> ?????? if (!reader_tasks) {
> >>>> -??????? VERBOSE_SCALEOUT_ERRSTRING("out of memory");
> >>>> +??????? SCALEOUT_ERRSTRING("out of memory");
> >>> ditto
> >>>
> >>> Thanks
> >>> Zhijian
> >>>> ?????????? firsterr = -ENOMEM;
> >>>> ?????????? goto unwind;
> >>>> ?????? }
> >