From: Christoph Hellwig on
On Mon, Jun 14, 2010 at 01:58:32PM +0530, Srikar Dronamraju wrote:
> User Space Breakpoint Assistance Layer (USER_BKPT)
> Changelog from V3: (reimplement background page replacement)
> * Replemented background page replacement based on inputs
> from Peter Zijlstra.
>
> Changelog from v2: (addressing comments from Oleg)

FYI the changelog should be under the --- marker so that it doesn't
get included into the final commit message.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Christoph Hellwig on
On Mon, Jun 14, 2010 at 01:58:32PM +0530, Srikar Dronamraju wrote:
> TODO: Merge user_bkpt layer with uprobes.

Yes, I don't think there's any point in that split. What's the
timeframe for you to finish that merge?

> + depends on MODULES

This doesn't make too much sense - the only real user of this is not
modular at all.

> + depends on HAVE_USER_BKPT

Should this be ARCH_HAVE_USER_BKPT or similar?

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Srikar Dronamraju on
* Christoph Hellwig <hch(a)infradead.org> [2010-06-14 13:50:48]:

> On Mon, Jun 14, 2010 at 01:58:32PM +0530, Srikar Dronamraju wrote:
> > TODO: Merge user_bkpt layer with uprobes.
>
> Yes, I don't think there's any point in that split. What's the
> timeframe for you to finish that merge?

I will take this up in v6.

>
> > + depends on MODULES
>
> This doesn't make too much sense - the only real user of this is not
> modular at all.

Okay.

>
> > + depends on HAVE_USER_BKPT
>
> Should this be ARCH_HAVE_USER_BKPT or similar?

Okay I shall change from HAVE_USER_BKPT to ARCH_HAVE_USER_BKPT.

--
Thanks and Regards
Srikar
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Steven Rostedt on
On Mon, 2010-06-14 at 13:58 +0530, Srikar Dronamraju wrote:
> user_bkpt_core.patch
>
> From: Srikar Dronamraju <srikar(a)linux.vnet.ibm.com>
>

> +/**
> + * user_bkpt_write_data - Write @nbytes from @kbuf at @vaddr in @tsk.
> + * Can be used to write to stack or data VM areas, but not instructions.
> + * Not exported, but available for use by arch-specific user_bkpt code.
> + * @tsk: The probed task
> + * @vaddr: Destination address, in user space.
> + * @kbuf: Source address, in kernel space to be read.

I'm curious. Anything prevent this from being called on instructions, or
is this just "Don't do that?".

> + *
> + * Context: This function may sleep.
> + *
> + * Return number of bytes written.
> + */
> +unsigned long user_bkpt_write_data(struct task_struct *tsk,
> + void __user *vaddr, const void *kbuf,
> + unsigned long nbytes)
> +{
> + unsigned long nleft;
> +
> + if (tsk == current) {
> + nleft = copy_to_user(vaddr, kbuf, nbytes);
> + return nbytes - nleft;
> + } else
> + return access_process_vm(tsk, (unsigned long) vaddr,
> + (void *) kbuf, nbytes, 1);
> +}
> +
> +static int write_opcode(struct task_struct *tsk, unsigned long vaddr,
> + user_bkpt_opcode_t opcode)
> +{
> + struct mm_struct *mm;
> + struct vm_area_struct *vma;
> + struct page *old_page, *new_page;
> + void *vaddr_old, *vaddr_new;
> + pte_t orig_pte;
> + int ret = -EINVAL;
> +
> + if (!tsk)
> + return ret;
> +
> + mm = get_task_mm(tsk);
> + if (!mm)
> + return ret;
> +
> + down_read(&mm->mmap_sem);
> +
> + /* Read the page with vaddr into memory */
> + ret = get_user_pages(tsk, mm, vaddr, 1, 1, 1, &old_page, &vma);
> + if (ret <= 0)
> + goto mmput_out;
> +
> + /*
> + * check if the page we are interested is read-only mapped
> + * Since we are interested in text pages, Our pages of interest
> + * should be mapped read-only.
> + */
> + if ((vma->vm_flags && (VM_READ|VM_WRITE)) != VM_READ) {
> + ret = -EINVAL;
> + goto put_out;
> + }
> +
> + /* If its VM_SHARED vma, lets not write to such vma's. */
> + if (vma->vm_flags & VM_SHARED) {
> + ret = -EINVAL;
> + goto put_out;
> + }
> +
> + /* Allocate a page */
> + new_page = alloc_page_vma(GFP_HIGHUSER_MOVABLE, vma, vaddr);
> + if (!new_page) {
> + ret = -ENOMEM;
> + goto put_out;
> + }
> +
> + /*
> + * lock page will serialize against do_wp_page()'s
> + * PageAnon() handling
> + */
> + lock_page(old_page);
> + /* mark page RO so any concurrent access will end up in do_wp_page() */
> + if (write_protect_page(vma, old_page, &orig_pte))
> + goto unlock_out;
> +
> + /* copy the page now that we've got it stable */
> + vaddr_old = kmap_atomic(old_page, KM_USER0);
> + vaddr_new = kmap_atomic(new_page, KM_USER1);
> +
> + memcpy(vaddr_new, vaddr_old, PAGE_SIZE);
> + /* poke the new insn in, ASSUMES we don't cross page boundary */
> + vaddr = vaddr & (PAGE_SIZE - 1);

This should probably be vaddr = vaddr & ~PAGE_MASK;


-- Steve

> + memcpy(vaddr_new + vaddr, &opcode, user_bkpt_opcode_sz);
> +
> + kunmap_atomic(vaddr_new, KM_USER1);
> + kunmap_atomic(vaddr_old, KM_USER0);
> +
> + lock_page(new_page);
> + /* flip pages, do_wp_page() will fail pte_same() and bail */
> + ret = replace_page(vma, old_page, new_page, orig_pte);
> +
> +unlock_out:
> + unlock_page(new_page);
> + unlock_page(old_page);
> + if (ret != 0)
> + page_cache_release(new_page);
> +
> +put_out:
> + put_page(old_page); /* we did a get_page in the beginning */
> +
> +mmput_out:
> + up_read(&mm->mmap_sem);
> + mmput(mm);
> + return ret;
> +}
> +

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Steven Rostedt on
On Mon, 2010-06-14 at 13:40 -0400, Christoph Hellwig wrote:
> On Mon, Jun 14, 2010 at 01:58:32PM +0530, Srikar Dronamraju wrote:
> > User Space Breakpoint Assistance Layer (USER_BKPT)
> > Changelog from V3: (reimplement background page replacement)
> > * Replemented background page replacement based on inputs
> > from Peter Zijlstra.
> >
> > Changelog from v2: (addressing comments from Oleg)
>
> FYI the changelog should be under the --- marker so that it doesn't
> get included into the final commit message.
>

Actually, I find the version'ing of a patch in the changelog useful. It
gives a bit of history to how the patch was created.

-- Steve


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/