This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
Home Products Support Community News


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

To: Isaku Yamahata <yamahata@xxxxxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH 2/3] User-space grant table device - main driver
From: Derek Murray <Derek.Murray@xxxxxxxxxxxx>
Date: Thu, 29 Mar 2007 10:23:05 +0100
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Thu, 29 Mar 2007 02:21:59 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <20070329062651.GB3737%yamahata@xxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <68F773A4-30BF-49BE-A3CB-8551D223AC0F@xxxxxxxxxxxx> <20070329062651.GB3737%yamahata@xxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
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