[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 2/3] User-space grant table device - main driver

On 29 Mar 2007, at 07:26, Isaku Yamahata wrote:
On Wed, Mar 28, 2007 at 01:56:16PM +0100, Derek Murray wrote:

+static int gntdev_mmap (struct file *flip, struct vm_area_struct *vma)

This doesn't undo vm_insert_page() when auto trasnalted physmap mode.
Probably just calling zap_page_range() would be OK instead
of undo_count_kmap loop.

Hmm, in fact, this whole section is redundant, because a failing gntdev_mmap() will be cleaned up in do_mmap_pgoff(), and, subsequently, the gntdev_clear_pte() hook will be called. This does a pte_clear_full() on the user-space PTE in auto translated physmap mode, so would this be enough?

+static pte_t gntdev_clear_pte(struct vm_area_struct *vma, unsigned long addr,
+                             pte_t *ptep, int is_fullmm)

Please clear the entry before grant table unmap.
If the virtual address is accessed between grant tabel unmap and
pte_clear_full(), something bad would happen.
The higher level exclusion is done and I might be somewhat paranoia, though.

Better safe than sorry! I'll fix this.

+static int gntdev_ioctl(struct inode *inode, struct file *flip,
+                       unsigned int cmd, unsigned long arg)

copy_from/to_user() may result in page fault and page fault handler
may try to obtain it causing dead lock.

Ah, good point. I'll move these copies out of the critical section.

Thanks for your comments!


Derek Murray.

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.