Re: port-xen/47057: Xen NetBSD DomU file system trash under Linux Dom0

[Manuel Bouyer <bouyer%antioche.eu.org@localhost>]

On Tue, Oct 23, 2012 at 04:01:06PM +0200, Roger Pau Monné wrote:
> Found the problem, grants from 0 to 8 (both included), shouldn't be
> used, they are reserved for the tools. I guess thats xenstore,
> xenconsole and friends, so that's where the corruption came from, and
> that's why the problem seemed to be related to xengnt_more_entries,
> because it gets called when those low grants are used. The attached
> patch solves the problem for me.

I guess it's new behavior of the tools ? Otherwise I guess we should have hit
this sooner. I see messages saying the kernel grows the grant entries
pool on a regular basis.

Anyway, good catch. one comment about the patch below.


> >From b80f10a3c3d0b95d3cd2a60a4669a2118fdbb9ef Mon Sep 17 00:00:00 2001
> From: Roger Pau Monne <roger.pau%citrix.com@localhost>
> Date: Tue, 23 Oct 2012 15:21:18 +0200
> Subject: [PATCH] xen: don't use grants 0-9
> 
> Not all grants from the first frame can be used, grants from 0 to 8
> (both included) are reserved for external tools. Using this grants
> caused system crashes and fs corruption.
> ---
>  sys/arch/xen/xen/xengnt.c |   15 +++++++++++----
>  1 files changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/sys/arch/xen/xen/xengnt.c b/sys/arch/xen/xen/xengnt.c
> index 621d2dc..2de4fd3 100644
> --- a/sys/arch/xen/xen/xengnt.c
> +++ b/sys/arch/xen/xen/xengnt.c
> @@ -51,6 +51,9 @@ __KERNEL_RCSID(0, "$NetBSD: xengnt.c,v 1.24 2012/06/30 
> 23:36:20 jym Exp $");
>  
>  #define NR_GRANT_ENTRIES_PER_PAGE (PAGE_SIZE / sizeof(grant_entry_t))
>  
> +/* External tools reserve first few grant table entries. */
> +#define NR_RESERVED_ENTRIES 8
> +
>  /* Current number of frames making up the grant table */
>  int gnt_nr_grant_frames;
>  /* Maximum number of frames that can make up the grant table */
> @@ -161,7 +164,7 @@ xengnt_more_entries(void)
>       gnttab_setup_table_t setup;
>       u_long *pages;
>       int nframes_new = gnt_nr_grant_frames + 1;
> -     int i;
> +     int i, start_gnt;
>       KASSERT(mutex_owned(&grant_lock));
>  
>       if (gnt_nr_grant_frames == gnt_max_grant_frames)
> @@ -204,9 +207,13 @@ xengnt_more_entries(void)
>  
>       /*
>        * add the grant entries associated to the last grant table frame
> -      * and mark them as free
> +      * and mark them as free. Prevent using the first grants (from 0 to 8)
> +      * since they are used by the tools.
>        */
> -     for (i = gnt_nr_grant_frames * NR_GRANT_ENTRIES_PER_PAGE;
> +     start_gnt = (gnt_nr_grant_frames * NR_GRANT_ENTRIES_PER_PAGE) <
> +                             NR_RESERVED_ENTRIES + 1 ? NR_RESERVED_ENTRIES + 
> 1 :
> +                             (gnt_nr_grant_frames * 
> NR_GRANT_ENTRIES_PER_PAGE);

please rewrite with parenthesis:
+       start_gnt = (gnt_nr_grant_frames * NR_GRANT_ENTRIES_PER_PAGE) <
+                               (NR_RESERVED_ENTRIES + 1) ?
                                (NR_RESERVED_ENTRIES + 1) :
+                               (gnt_nr_grant_frames * 
NR_GRANT_ENTRIES_PER_PAGE);

then please commit and request pullups for netbsd-5 and -6.

-- 
Manuel Bouyer <bouyer%antioche.eu.org@localhost>
     NetBSD: 26 ans d'experience feront toujours la difference
--