lkml.org 
[lkml]   [2022]   [Jul]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2 bpf-next 3/5] ftrace: introduce FTRACE_OPS_FL_SHARE_IPMODIFY
    On Fri, 15 Jul 2022 17:42:55 +0000
    Song Liu <songliubraving@fb.com> wrote:


    > A quick update and ask for feedback/clarification.
    >
    > Based on my understanding, you recommended calling ops_func() from
    > __ftrace_hash_update_ipmodify() and in ops_func() the direct trampoline
    > may make changes to the trampoline. Did I get this right?
    >
    >
    > I am going towards this direction, but hit some issue. Specifically, in
    > __ftrace_hash_update_ipmodify(), ftrace_lock is already locked, so the
    > direct trampoline cannot easily make changes with
    > modify_ftrace_direct_multi(), which locks both direct_mutex and
    > ftrace_mutex.
    >
    > One solution would be have no-lock version of all the functions called
    > by modify_ftrace_direct_multi(), but that's a lot of functions and the
    > code will be pretty ugly. The alternative would be the logic in v2:
    > __ftrace_hash_update_ipmodify() returns -EAGAIN, and we make changes to
    > the direct trampoline in other places:
    >
    > 1) if DIRECT ops attached first, the trampoline is updated in
    > prepare_direct_functions_for_ipmodify(), see 3/5 of v2;
    >
    > 2) if IPMODIFY ops attached first, the trampoline is updated in
    > bpf_trampoline_update(), see "goto again" path in 5/5 of v2.
    >
    > Overall, I think this way is still cleaner. What do you think about this?

    What about if we release the lock when doing the callback?

    Then we just need to make sure things are the same after reacquiring the
    lock, and if they are different, we release the lock again and do the
    callback with the new update. Wash, rinse, repeat, until the state is the
    same before and after the callback with locks acquired?

    This is a common way to handle callbacks that need to do something that
    takes the lock held before doing a callback.

    The reason I say this, is because the more we can keep the accounting
    inside of ftrace the better.

    Wouldn't this need to be done anyway if BPF was first and live kernel
    patching needed the update? An -EAGAIN would not suffice.

    -- Steve

    \
     
     \ /
      Last update: 2022-07-15 21:14    [W:2.572 / U:0.044 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site