lkml.org 
[lkml]   [2008]   [Oct]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] markers: remove 2 exported symbols
    Mathieu Desnoyers wrote:
    > * Lai Jiangshan (laijs@cn.fujitsu.com) wrote:
    >> __mark_empty_function() and marker_probe_cb_noarg()
    >> should not be seen by outer code. this patch remove them.
    >>
    >> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
    >> ---
    >> diff --git a/include/linux/marker.h b/include/linux/marker.h
    >> index 1290653..f4d4d28 100644
    >> --- a/include/linux/marker.h
    >> +++ b/include/linux/marker.h
    >> @@ -132,12 +132,8 @@ static inline void __printf(1, 2) ___mark_check_format(const char *fmt, ...)
    >> ___mark_check_format(format, ## args); \
    >> } while (0)
    >>
    >> -extern marker_probe_func __mark_empty_function;
    >> -
    >
    > Hi Lai,
    >
    > Hrm ? Have a good look at the macro __trace_mark() in
    > include/linux/marker.h, you'll see that __mark_empty_function is
    > referenced. Have you tested this against code with declared markers ?

    Sorry for this,
    I have markers in my kernel test code.
    I hasn't tested this patch, for I thought it's to simple.
    I used "grep" to find "__mark_empty_function",
    but I missed one line of the results.

    Other problems:
    1)
    why we need marker_probe_cb_noarg()?
    marker_probe_cb_noarg() has no performance optimization,
    and no additional format check, or other thing?

    if we remove marker_probe_cb_noarg, we can remove struct marker.call also.

    2)
    why we use va_list *?
    As I know, sizeof(va_list) = 4 or 8.


    please ignore this patch.

    Thanks, Lai.

    >
    >> extern void marker_probe_cb(const struct marker *mdata,
    >> void *call_private, ...);
    >> -extern void marker_probe_cb_noarg(const struct marker *mdata,
    >> - void *call_private, ...);
    >
    > This second change is correct. marker_probe_cb is referenced by
    > __trace_mark(), but not marker_probe_cb_noarg, which is only connected
    > when non-empty format string is found by the registration function in
    > marker.c.
    >
    >>
    >> /*
    >> * Connect a probe to a marker.
    >> diff --git a/kernel/marker.c b/kernel/marker.c
    >> index 7d1faec..4440a09 100644
    >> --- a/kernel/marker.c
    >> +++ b/kernel/marker.c
    >> @@ -81,11 +81,10 @@ static struct hlist_head marker_table[MARKER_TABLE_SIZE];
    >> * though the function pointer change and the marker enabling are two distinct
    >> * operations that modifies the execution flow of preemptible code.
    >> */
    >> -void __mark_empty_function(void *probe_private, void *call_private,
    >> +static void __mark_empty_function(void *probe_private, void *call_private,
    >> const char *fmt, va_list *args)
    >> {
    >> }
    >> -EXPORT_SYMBOL_GPL(__mark_empty_function);
    >>
    >
    > Same as comment above.
    >
    >> /*
    >> * marker_probe_cb Callback that prepares the variable argument list for probes.
    >> @@ -157,7 +156,7 @@ EXPORT_SYMBOL_GPL(marker_probe_cb);
    >> *
    >> * Should be connected to markers "MARK_NOARGS".
    >> */
    >> -void marker_probe_cb_noarg(const struct marker *mdata, void *call_private, ...)
    >> +static void marker_probe_cb_noarg(const struct marker *mdata, void *call_private, ...)
    >> {
    >> va_list args; /* not initialized */
    >> char ptype;
    >> @@ -197,7 +196,6 @@ void marker_probe_cb_noarg(const struct marker *mdata, void *call_private, ...)
    >> }
    >> preempt_enable();
    >> }
    >> -EXPORT_SYMBOL_GPL(marker_probe_cb_noarg);
    >>
    >
    > This one is ok.
    >
    > So overall, if you could check why you have not hit any problem when
    > removing __mark_empty_function, that would be great. The only reason I
    > see is that you had no markers in your kernel test code.
    >
    > Mathieu
    >
    >> static void free_old_closure(struct rcu_head *head)
    >> {
    >>
    >>
    >>
    >




    \
     
     \ /
      Last update: 2008-10-08 05:11    [W:0.029 / U:0.824 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site