Messages in this thread | | | From | Namhyung Kim <> | Subject | Re: [PATCH 10/13] tracing/uprobes: Fetch args before reserving a ring buffer | Date | Wed, 04 Sep 2013 16:08:52 +0900 |
| |
On Tue, 03 Sep 2013 19:50:28 +0900, Masami Hiramatsu wrote: > (2013/09/03 14:44), Namhyung Kim wrote: >> From: Namhyung Kim <namhyung.kim@lge.com> >> >> Fetching from user space should be done in a non-atomic context. So >> use a per-cpu buffer and copy its content to the ring buffer >> atomically. Note that we can migrate during accessing user memory >> thus use a per-cpu mutex to protect concurrent accesses. >> >> This is needed since we'll be able to fetch args from an user memory >> which can be swapped out. Before that uprobes could fetch args from >> registers only which saved in a kernel space. >> >> While at it, use __get_data_size() and store_trace_args() to reduce >> code duplication.
[SNIP]
>> + size = esize + tu->p.size + dsize; >> event = trace_current_buffer_lock_reserve(&buffer, call->event.type, >> - size + tu->p.size, 0, 0); >> - if (!event) >> + size, 0, 0); >> + if (!event) { >> + mutex_unlock(mutex); >> return; > > Just for a maintenance reason, I personally like to use "goto" in this case > to fold up the mutex_unlock. :) > > Other parts are good for me. > > Reviewed-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Thank you for review!
I'll change it as you said and fix atomic_dec bug that Jovi found.
Thanks, Namhyung
| |