Functions that require interrupts be enabled

Functions that require interrupts be enabled

[Mike Mason]

I want to add a function to the task.stp tapset file that grabs a process' arguments from its user address space.  We couldn't do this before because all probes ran with interrupts disabled and couldn't sleep.  Now that begin/end probes no longer require that interrupts be disabled, this function can be used in begin/end probes at least.  

How do I prevent the function from being used in other probes?  Is there a way to detect if interrupts are disabled or detect that the function was called from a begin/end probe?  Do we even want to provide functions with this type of limitation?

Mike

Re: Functions that require interrupts be enabled

[Frank Ch. Eigler]

Mike Mason <mmlnx@us.ibm.com> writes:

> I want to add a function to the task.stp tapset file that grabs a
> process' arguments from its user address space.  

OK.

> We couldn't do this before because all probes ran with interrupts
> disabled and couldn't sleep.

Why must this new routine be permitted to sleep?  We can tolerate
paged-out data via soft errors (=> blank strings).

> Now that begin/end probes no longer require that interrupts be
> disabled, this function can be used in begin/end probes at least. 

AFAIK, interrupts being enabled is not exactly the same thing as being
able to sleep.


Not that this is a good idea, but:

> How do I prevent the function from being used in other probes?  Is
> there a way to detect if interrupts are disabled

in_interrupt()

> or detect that the function was called from a begin/end probe?

CONTEXT->probe_point

> Do we even want to provide functions with this type of limitation?

Not really.

- FChE

Re: Functions that require interrupts be enabled

[Mike Mason]

Frank Ch. Eigler wrote:
> Mike Mason <mmlnx@us.ibm.com> writes:
> 
>> I want to add a function to the task.stp tapset file that grabs a
>> process' arguments from its user address space.  
> 
> OK.
> 
>> We couldn't do this before because all probes ran with interrupts
>> disabled and couldn't sleep.
> 
> Why must this new routine be permitted to sleep?  We can tolerate
> paged-out data via soft errors (=> blank strings).

My script filters based on the arguments, so having the routine randomly fail to return the actual arguments isn't good.  Plus I don't think whether the page is paged in or not is an issue.  I think the routine I'm using handles that (see below).

> 
>> Now that begin/end probes no longer require that interrupts be
>> disabled, this function can be used in begin/end probes at least. 
> 
> AFAIK, interrupts being enabled is not exactly the same thing as being
> able to sleep.

Here's the routine I'm using to grab the arguments from user space.  It's a modified version of access_process_vm(), which isn't callable from a module.  It can potentially sleep in two places: down_read() and kmap().  These functions do a might_sleep() check and fail if interrupts are disabled.  I considered using down_read_trylock() and kmap_atomic() (which won't sleep) but I don't clearly understand the side-effects of doing so.  Any suggestions would be appreciated.

int _read_process_args(struct task_struct *tsk, void *buf)
{
    struct mm_struct *mm;
    struct vm_area_struct *vma;
    struct page *page;
    void *old_buf = buf;
    int len;
    unsigned long addr;

    mm = get_task_mm(tsk);
    if (!mm)
        return 0;

    if (!mm->arg_end)
        goto out_mm;

    len = mm->arg_end - mm->arg_start;
    if (len > MAXSTRINGLEN)
        len = MAXSTRINGLEN;

    addr = mm->arg_start;

    down_read(&mm->mmap_sem); /* might_sleep */

    /* ignore errors, just check how much was successfully transfered */
    while (len) {
        int bytes, ret, offset;
        void *maddr;

        ret = get_user_pages(tsk, mm, addr, 1, 0, 1, &page, &vma);
        if (ret <= 0)
            break;

        bytes = len;
        offset = addr & (PAGE_SIZE - 1);
        if (bytes > PAGE_SIZE - offset)
            bytes = PAGE_SIZE - offset;

        maddr = kmap(page); /* might_sleep */
        copy_from_user_page(vma, page, addr, buf, maddr + offset, bytes);
        kunmap(page);
        page_cache_release(page);

        len -= bytes;
        buf += bytes;
        addr += bytes;
    }

    up_read(&mm->mmap_sem);

out_mm:
    mmput(mm);
    return buf - old_buf;
}

> 
> 
> Not that this is a good idea, but:
> 
>> How do I prevent the function from being used in other probes?  Is
>> there a way to detect if interrupts are disabled
> 
> in_interrupt()
> 
>> or detect that the function was called from a begin/end probe?
> 
> CONTEXT->probe_point
> 
>> Do we even want to provide functions with this type of limitation?
> 
> Not really.
> 
> - FChE

Re: Functions that require interrupts be enabled

[Frank Ch. Eigler]

Hi -

On Wed, May 16, 2007 at 12:00:38PM -0700, Mike Mason wrote:
> [...]
> >Why must this new routine be permitted to sleep?  We can tolerate
> >paged-out data via soft errors (=> blank strings).
> 
> My script filters based on the arguments, so having the routine randomly 
> fail to return the actual arguments isn't good.  

Yeah, though I wonder how frequently that can happens, considering
that running tools like "top" would keep those pages around.

> Plus I don't think whether the page is paged in or not is an issue.
> I think the routine I'm using handles that (see below).

Yes, via possible sleeps during the other routines.



> >
> >>Now that begin/end probes no longer require that interrupts be
> >>disabled, this function can be used in begin/end probes at least. 
> >
> >AFAIK, interrupts being enabled is not exactly the same thing as being
> >able to sleep.
> 
> Here's the routine I'm using to grab the arguments from user space.  It's a 
> modified version of access_process_vm(), which isn't callable from a 
> module.  It can potentially sleep in two places: down_read() and kmap().  
> These functions do a might_sleep() check and fail if interrupts are 
> disabled.

> I considered using down_read_trylock() and kmap_atomic() (which
> won't sleep) but I don't clearly understand the side-effects of
> doing so.  Any suggestions would be appreciated.

I too only have limited understanding of this.  For the _trylock, that
is not a problem, as one can detect contention and return early rather
than blocking.

For kmap_atomic(), it seems trickier.  You'd need to use one of the
KM_USER[01] slots.  But we're in trouble if we are running this code
within a probe in a kernel routine that is already using that slot.

Then we still have this stuff that might sleep indirectly via a page
fault (unless I'm mistaken):

>        ret = get_user_pages(tsk, mm, addr, 1, 0, 1, &page, &vma);
>        copy_from_user_page(vma, page, addr, buf, maddr + offset, bytes);


So it seems like our options are:
(a) write such a tapset function, permit it to only be called from
    begin/end-like probe contexts, and let it sleep and whatnot
(b) write a related tapset function, which uses very conservative
    atomic routines everywhere, and may return blanks
(c) accept an approximation, such as a deferred result.  It might
    look like this:

# ------------------------------------------------------------------------
# tapset/proc-args.stp
%{
// helper C code
%}

void deferred_lookup(...) %{
     // enqueue a possibly-sleepy lookup of process args using
     //    auxiliary thread or defer_work type callback
     // arrange to put results into _process_args[] when available
     // return right away
%}

global _process_args

function process_args:string (pid) {
  if (pid in _process_args) return _process_args [pid]
  else { deferred_lookup (pid)
         return "" }
}

# ------------------------------------------------------------------------

Then, ordinary user scripts would just call process_args(NNN), and
would have to accept & ignore empty results.  Eventually valid strings
should come by.

- FChE