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.