[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <526D26AD.5040805@asianux.com>
Date: Sun, 27 Oct 2013 22:43:57 +0800
From: Chen Gang <gang.chen@...anux.com>
To: paulmck@...ux.vnet.ibm.com
CC: josh@...edesktop.org,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] kernel/rcutorture.c: be sure of enough memory for result
printing.
Hello Maintainers:
Is it patch OK or not? when you have time, please help check (if it is
necessary to send patch v2, please let me know).
Thanks.
On 10/21/2013 05:35 PM, Chen Gang wrote:
>
> BTW: I use kmalloc(), and still can pass build-in and let boot runnable
> under my laptop, but I think it is not quite precise, may still need
> more think of, please check (or give a confirmation whether it is OK).
>
> Hmm... maybe the pr_err("...") also need "srcu-torture" prefix just like
> another printing to let checkers easy search?
>
> Thanks.
>
> On 10/21/2013 02:18 PM, Chen Gang wrote:
>>
>> Oh, sorry, I forgot to let it pass "./scripts/checkpatch.pl", after
>> finish checking, it finds a style issue (which is pointed out below), if
>> necessary to send patch v2, please let me know.
>>
>> Thanks.
>>
>> On 10/21/2013 01:51 PM, Chen Gang wrote:
>>> If the contents is more than 4096 bytes (e.g. if have 1K cpus), current
>>> sprintf() will cause memory overflow. And this fix patch is to be sure
>>> of memory large enough.
>>>
>>> Benefit:
>>>
>>> - do not truncate printing contents.
>>> - extensible, it is large enough for printing various related contents.
>>> - simple and clear enough for both source code readers and writers.
>>>
>>> Shortcoming:
>>>
>>> - It will waste some memory:
>>> 1 cpu may waste 24KB,
>>> 10 cpus may waste 96KB,
>>> 100 cpus may waste 816KB,
>>> 1K cpus may waste 8MB
>>> ...
>>> after finish printing, it will free the related memory, quickly.
>>> it is a test module, so wast a little memory for extensible is OK.
>>>
>>> Related test (Fedora16 2 CPUs, 2GB RAM x86_64)
>>>
>>> - as module, with/without "torture_type=srcu".
>>> - build-in not boot runnable, with/without "torture_type=srcu".
>>> - build-in let boot runnable, with/without "torture_type=srcu".
>>>
>>>
>>> Signed-off-by: Chen Gang <gang.chen@...anux.com>
>>> ---
>>> kernel/rcutorture.c | 67 ++++++++++++++++++++++++++-------------------------
>>> 1 files changed, 34 insertions(+), 33 deletions(-)
>>>
>>> diff --git a/kernel/rcutorture.c b/kernel/rcutorture.c
>>> index be63101..3413bc1 100644
>>> --- a/kernel/rcutorture.c
>>> +++ b/kernel/rcutorture.c
>>> @@ -133,8 +133,6 @@ MODULE_PARM_DESC(verbose, "Enable verbose debugging printk()s");
>>> #define VERBOSE_PRINTK_ERRSTRING(s) \
>>> do { if (verbose) pr_alert("%s" TORTURE_FLAG "!!! " s "\n", torture_type); } while (0)
>>>
>>> -static char printk_buf[4096];
>>> -
>>> static int nrealreaders;
>>> static struct task_struct *writer_task;
>>> static struct task_struct **fakewriter_tasks;
>>> @@ -370,7 +368,7 @@ struct rcu_torture_ops {
>>> void (*call)(struct rcu_head *head, void (*func)(struct rcu_head *rcu));
>>> void (*cb_barrier)(void);
>>> void (*fqs)(void);
>>> - int (*stats)(char *page);
>>> + void (*stats)(char *page);
>>> int irq_capable;
>>> int can_boost;
>>> const char *name;
>>> @@ -572,21 +570,19 @@ static void srcu_torture_barrier(void)
>>> srcu_barrier(&srcu_ctl);
>>> }
>>>
>>> -static int srcu_torture_stats(char *page)
>>> +static void srcu_torture_stats(char *page)
>>> {
>>> - int cnt = 0;
>>> int cpu;
>>> int idx = srcu_ctl.completed & 0x1;
>>>
>>> - cnt += sprintf(&page[cnt], "%s%s per-CPU(idx=%d):",
>>> + page += sprintf(page, "%s%s per-CPU(idx=%d):",
>>> torture_type, TORTURE_FLAG, idx);
>>> for_each_possible_cpu(cpu) {
>>> - cnt += sprintf(&page[cnt], " %d(%lu,%lu)", cpu,
>>> + page += sprintf(page, " %d(%lu,%lu)", cpu,
>>> per_cpu_ptr(srcu_ctl.per_cpu_ref, cpu)->c[!idx],
>>> per_cpu_ptr(srcu_ctl.per_cpu_ref, cpu)->c[idx]);
>>> }
>>> - cnt += sprintf(&page[cnt], "\n");
>>> - return cnt;
>>> + sprintf(page, "\n");
>>> }
>>>
>>> static void srcu_torture_synchronize_expedited(void)
>>> @@ -1046,10 +1042,9 @@ rcu_torture_reader(void *arg)
>>> /*
>>> * Create an RCU-torture statistics message in the specified buffer.
>>> */
>>> -static int
>>> +static void
>>> rcu_torture_printk(char *page)
>>> {
>>> - int cnt = 0;
>>> int cpu;
>>> int i;
>>> long pipesummary[RCU_TORTURE_PIPE_LEN + 1] = { 0 };
>>> @@ -1065,8 +1060,8 @@ rcu_torture_printk(char *page)
>>> if (pipesummary[i] != 0)
>>> break;
>>> }
>>> - cnt += sprintf(&page[cnt], "%s%s ", torture_type, TORTURE_FLAG);
>>> - cnt += sprintf(&page[cnt],
>>> + page += sprintf(page, "%s%s ", torture_type, TORTURE_FLAG);
>>> + page += sprintf(page,
>>> "rtc: %p ver: %lu tfle: %d rta: %d rtaf: %d rtf: %d ",
>>> rcu_torture_current,
>>> rcu_torture_current_version,
>>> @@ -1074,53 +1069,52 @@ rcu_torture_printk(char *page)
>>> atomic_read(&n_rcu_torture_alloc),
>>> atomic_read(&n_rcu_torture_alloc_fail),
>>> atomic_read(&n_rcu_torture_free));
>>> - cnt += sprintf(&page[cnt], "rtmbe: %d rtbke: %ld rtbre: %ld ",
>>> + page += sprintf(page, "rtmbe: %d rtbke: %ld rtbre: %ld ",
>>> atomic_read(&n_rcu_torture_mberror),
>>> n_rcu_torture_boost_ktrerror,
>>> n_rcu_torture_boost_rterror);
>>> - cnt += sprintf(&page[cnt], "rtbf: %ld rtb: %ld nt: %ld ",
>>> + page += sprintf(page, "rtbf: %ld rtb: %ld nt: %ld ",
>>> n_rcu_torture_boost_failure,
>>> n_rcu_torture_boosts,
>>> n_rcu_torture_timers);
>>> - cnt += sprintf(&page[cnt],
>>> + page += sprintf(page,
>>> "onoff: %ld/%ld:%ld/%ld %d,%d:%d,%d %lu:%lu (HZ=%d) ",
>>> n_online_successes, n_online_attempts,
>>> n_offline_successes, n_offline_attempts,
>>> min_online, max_online,
>>> min_offline, max_offline,
>>> sum_online, sum_offline, HZ);
>>> - cnt += sprintf(&page[cnt], "barrier: %ld/%ld:%ld",
>>> + page += sprintf(page, "barrier: %ld/%ld:%ld",
>>> n_barrier_successes,
>>> n_barrier_attempts,
>>> n_rcu_torture_barrier_error);
>>> - cnt += sprintf(&page[cnt], "\n%s%s ", torture_type, TORTURE_FLAG);
>>> + page += sprintf(page, "\n%s%s ", torture_type, TORTURE_FLAG);
>>> if (atomic_read(&n_rcu_torture_mberror) != 0 ||
>>> n_rcu_torture_barrier_error != 0 ||
>>> n_rcu_torture_boost_ktrerror != 0 ||
>>> n_rcu_torture_boost_rterror != 0 ||
>>> n_rcu_torture_boost_failure != 0 ||
>>> i > 1) {
>>> - cnt += sprintf(&page[cnt], "!!! ");
>>> + page += sprintf(page, "!!! ");
>>> atomic_inc(&n_rcu_torture_error);
>>> WARN_ON_ONCE(1);
>>> }
>>> - cnt += sprintf(&page[cnt], "Reader Pipe: ");
>>> + page += sprintf(page, "Reader Pipe: ");
>>> for (i = 0; i < RCU_TORTURE_PIPE_LEN + 1; i++)
>>> - cnt += sprintf(&page[cnt], " %ld", pipesummary[i]);
>>> - cnt += sprintf(&page[cnt], "\n%s%s ", torture_type, TORTURE_FLAG);
>>> - cnt += sprintf(&page[cnt], "Reader Batch: ");
>>> + page += sprintf(page, " %ld", pipesummary[i]);
>>> + page += sprintf(page, "\n%s%s ", torture_type, TORTURE_FLAG);
>>> + page += sprintf(page, "Reader Batch: ");
>>> for (i = 0; i < RCU_TORTURE_PIPE_LEN + 1; i++)
>>> - cnt += sprintf(&page[cnt], " %ld", batchsummary[i]);
>>> - cnt += sprintf(&page[cnt], "\n%s%s ", torture_type, TORTURE_FLAG);
>>> - cnt += sprintf(&page[cnt], "Free-Block Circulation: ");
>>> + page += sprintf(page, " %ld", batchsummary[i]);
>>> + page += sprintf(page, "\n%s%s ", torture_type, TORTURE_FLAG);
>>> + page += sprintf(page, "Free-Block Circulation: ");
>>> for (i = 0; i < RCU_TORTURE_PIPE_LEN + 1; i++) {
>>> - cnt += sprintf(&page[cnt], " %d",
>>> + page += sprintf(page, " %d",
>>> atomic_read(&rcu_torture_wcount[i]));
>>> }
>>> - cnt += sprintf(&page[cnt], "\n");
>>> + page += sprintf(page, "\n");
>>> if (cur_ops->stats)
>>> - cnt += cur_ops->stats(&page[cnt]);
>>> - return cnt;
>>> + cur_ops->stats(page);
>>
>> Oh, sorry, this line has a waste white space.
>>
>>> }
>>>
>>> /*
>>> @@ -1134,10 +1128,17 @@ rcu_torture_printk(char *page)
>>> static void
>>> rcu_torture_stats_print(void)
>>> {
>>> - int cnt;
>>> + int size = (nr_cpu_ids + 2) * PAGE_SIZE; /* be sure of large enough */
>>> + char *buf;
>>>
>>> - cnt = rcu_torture_printk(printk_buf);
>>> - pr_alert("%s", printk_buf);
>>> + buf = kmalloc(size, GFP_KERNEL);
>>> + if (!buf) {
>>> + pr_err("no enough memory for printing, requre: %d", size);
>>> + return;
>>> + }
>>> + rcu_torture_printk(buf);
>>> + pr_alert("%s", buf);
>>> + kfree(buf);
>>> }
>>>
>>> /*
>>>
>>
>>
>
>
--
Chen Gang
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists