Messages in this thread | | | Date | Wed, 1 Nov 2017 10:56:15 +0800 | From | "Du, Changbin" <> | Subject | Re: [PATCH v2] tracing: Allocate mask_str buffer dynamically |
| |
Hi Rostedt, On Tue, Oct 31, 2017 at 12:19:58PM -0400, Steven Rostedt wrote: > On Thu, 26 Oct 2017 00:20:28 +0800 > changbin.du@intel.com wrote: > > > From: Changbin Du <changbin.du@intel.com> > > > > The default NR_CPUS can be very large, but actual possible nr_cpu_ids > > usually is very small. For my x86 distribution, the NR_CPUS is 8192 and > > nr_cpu_ids is 4. About 2 pages are wasted. > > > > Most machines don't have so many CPUs, so define a array with NR_CPUS > > just wastes memory. So let's allocate the buffer dynamically when need. > > > > The exact buffer size should be: > > DIV_ROUND_UP(nr_cpu_ids, 4) + nr_cpu_ids/32 + 2; > > > > Example output: > > ff,ffffffff > > > > Signed-off-by: Changbin Du <changbin.du@intel.com> > > > > --- > > v2: > > - remove 'static' declaration. > > - fix buffer size. > > --- > > kernel/trace/trace.c | 18 ++++++++++-------- > > 1 file changed, 10 insertions(+), 8 deletions(-) > > > > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c > > index 752e5da..6b70648 100644 > > --- a/kernel/trace/trace.c > > +++ b/kernel/trace/trace.c > > @@ -4184,31 +4184,33 @@ static const struct file_operations show_traces_fops = { > > */ > > static DEFINE_MUTEX(tracing_cpumask_update_lock); > > The above mutex was used to protect mask_str. > > > > > -/* > > - * Temporary storage for the character representation of the > > - * CPU bitmask (and one more byte for the newline): > > - */ > > -static char mask_str[NR_CPUS + 1]; > > - > > static ssize_t > > tracing_cpumask_read(struct file *filp, char __user *ubuf, > > size_t count, loff_t *ppos) > > { > > struct trace_array *tr = file_inode(filp)->i_private; > > + char *mask_str; > > int len; > > > > + /* Bitmap, ',' and two more bytes for the newline and '\0'. */ > > + len = DIV_ROUND_UP(nr_cpu_ids, 4) + nr_cpu_ids/32 + 2; > > + mask_str = kmalloc(len, GFP_KERNEL); > > + if (!mask_str) > > + return -ENOMEM; > > + > > mutex_lock(&tracing_cpumask_update_lock); > > This patch can remove the mutex as well, since there's no sharing of > the mask anymore. > > -- Steve > ok, let me remove it in v3.
> > > > - len = snprintf(mask_str, count, "%*pb\n", > > + len = snprintf(mask_str, len, "%*pb\n", > > cpumask_pr_args(tr->tracing_cpumask)); > > if (len >= count) { > > count = -EINVAL; > > goto out_err; > > } > > - count = simple_read_from_buffer(ubuf, count, ppos, mask_str, NR_CPUS+1); > > + count = simple_read_from_buffer(ubuf, count, ppos, mask_str, len); > > > > out_err: > > mutex_unlock(&tracing_cpumask_update_lock); > > + kfree(mask_str); > > > > return count; > > } >
-- Thanks, Changbin Du [unhandled content-type:application/pgp-signature] | |