Re: Functions that require interrupts be enabled

[Mike Mason <mmlnx@us.ibm.com>]

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