Subject: Re: lock bug in getnewvnode, or uvm_km_kmemalloc/uvm_map ?
To: Manuel Bouyer <bouyer@antioche.eu.org>
From: enami tsugutomo <enami@but-b.or.jp>
List: tech-kern
Date: 11/29/2002 07:52:28
> > also you're reversing the direction in:
> > 
> > @@ -807,7 +822,9 @@
> >                          */
> >                         if (prev_entry->next->aref.ar_amap) {
> >                                 error = amap_extend(prev_entry->next, size,
> > -                                   AMAP_EXTEND_BACKWARDS);
> > +                                   (flags & UVM_KMF_NOWAIT) ?
> > +                                   (AMAP_EXTEND_FORWARDS|AMAP_EXTEND_NOWAIT) :
> > +                                   AMAP_EXTEND_FORWARDS);
> >                                 if (error) {
> >                                         vm_map_unlock(map);
> >                                         if (new_entry) {
> 
> Ops, thanks !

There is another one?  See the next.

> > it'd be clearer if you just did
> > 
> > 	amapwaitflag = (flags & UVM_KMF_NOWAIT) ? AMAP_EXTEND_NOWAIT : 0;

I prefer this rather than ...

> @@ -537,6 +543,8 @@
>  	uvm_flag_t flags;
>  {
>  	struct vm_map_entry *prev_entry, *new_entry;
> +	int aflags = (flags & UVM_KMF_NOWAIT) ?
> +	    (AMAP_EXTEND_FORWARDS|AMAP_EXTEND_NOWAIT) : AMAP_EXTEND_FORWARDS;
>  	vm_prot_t prot = UVM_PROTECTION(flags), maxprot =
>  	    UVM_MAXPROTECTION(flags);
>  	vm_inherit_t inherit = UVM_INHERIT(flags);
		     :
> @@ -779,7 +788,7 @@
>  				if (amap_extend(prev_entry,
>  				    prev_entry->next->end -
>  				    prev_entry->next->start,
> -				    AMAP_EXTEND_FORWARDS))
> +				    aflags))
>  				goto nomerge;
>  			}
>  
> @@ -797,7 +806,7 @@
>  				if (amap_extend(prev_entry->next,
>  				    prev_entry->end -
>  				    prev_entry->start + size,
> -				    AMAP_EXTEND_BACKWARDS))
> +				    aflags))
>  				goto nomerge;
>  			}
>  		} else {

... this.  It makes code hard to read since one can't imagine `aflags'
means (waitflags | FORWARD).  And actually, you're using for both
forward and backward case.  Since AMAP_EXTEND_FORWARD and _BACKWARD
are mandatory flags and the direction is important here, i'd like to
see in each call, i.e.

enami.