lkml.org 
[lkml]   [2018]   [Jul]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v5 06/10] Uprobes: Support SDT markers having reference count (semaphore)
    On 07/11, Ravi Bangoria wrote:
    >
    > > However, I still think it would be better to avoid uprobe exporting and modifying
    > > set_swbp/set_orig_insn. May be we can simply kill both set_swbp() and set_orig_insn(),
    > > I'll re-check...
    >
    > Good that you bring this up. Actually, we can implement same logic
    > without exporting uprobe. We can do "uprobe = container_of(arch_uprobe)"
    > in uprobe_write_opcode(). No need to export struct uprobe outside,
    > no need to change set_swbp() / set_orig_insn() syntax. Just that we
    > need to pass arch_uprobe object to uprobe_write_opcode().

    Yes, but you still need to modify set_swbp/set_orig_insn to pass the new
    arg to uprobe_write_opcode(). OK, this is fine.


    > But, I wanted to discuss about making ref_ctr_offset a uprobe property
    > or a consumer property, before posting v6:
    >
    > If we make it a consumer property, the design becomes flexible for
    > user. User will have an option to either depend on kernel to handle
    > reference counter or he can create normal uprobe and manipulate
    > reference counter on his own. This will not require any changes to
    > existing tools. With this approach we need to increment / decrement
    > reference counter for each consumer. But, because of the fact that our
    > install_breakpoint() / remove_breakpoint() are not balanced, we have
    > to keep track of which reference counter have been updated in which
    > mm, for which uprobe and for which consumer. I.e. Maintain a list of
    > {uprobe, consumer, mm}.

    Did you explore the UPROBE_KERN_CTR hack I tried to suggest?

    If it can work then, again, *ctr_ptr |= UPROBE_KERN_CTR from install_breakpoint()
    paths is always fine, the nontrivial part is remove_breakpoint() case, perhaps
    you can do something like

    for (each uprobe in inode)
    for (each consumer)
    if (consumer_filter(consumer))
    goto keep_ctr;

    for (each vma which maps this counter)
    *ctr_ptr &= ~UPROBE_KERN_CTR;

    keep_ctr:
    set_orig_insn(...);

    but again, I didn't even try to think about details, not sure this
    can really work.

    And in any case:

    > This will make kernel implementation quite
    > complex

    Yes. So I personally won't insist on this feature.

    > Third options: How about allowing 0 as a special value for reference
    > counter? I mean allow uprobe_register() and uprobe_register_refctr()
    > in parallel but do not allow two uprobe_register_refctr() with two
    > different reference counter.

    I am not sure I understand how you can do this, and how much complications
    this needs, so I have no opinion.


    Cough, just noticed the final part below...

    > PS: We can't abuse MSB with first approach because any userspace tool
    > can also abuse MSB in parallel.

    For what?

    > Probably, we can abuse MSB in second
    > and third approach, though, there is no need to.

    Confused... If userspace can change it, how we can use it in 2nd approach?

    Oleg.

    \
     
     \ /
      Last update: 2018-07-15 22:06    [W:3.616 / U:0.052 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site