Subject: Re: lock bug in getnewvnode, or uvm_km_kmemalloc/uvm_map ?
To: Manuel Bouyer <bouyer@antioche.eu.org>
From: Chuck Silvers <chuq@chuq.com>
List: tech-kern
Date: 11/29/2002 08:53:36
On Thu, Nov 28, 2002 at 09:39:10PM +0100, Manuel Bouyer wrote:
> > it'd be clearer if you just did
> >
> > amapwaitflag = (flags & UVM_KMF_NOWAIT) ? AMAP_EXTEND_NOWAIT : 0;
> >
> > and then just combined that with the direction in each call.
>
> but the code may be less efficient; I guess the compiler will have to do a
> | operation, when it would be expanded to a constant otherwise.
in this case, I'll argue that code clarity is worth a few extra instructions,
since the runtime difference won't be detectable. if you really care about
this, you might want to think about enhancing gcc to detect this pattern and
do some smarter constant folding. maybe it does it already, I haven't looked
at the generated code.
you're still reversing the direction in the third case below.
> @@ -680,8 +690,7 @@
> }
>
> if (prev_entry->aref.ar_amap) {
> - error = amap_extend(prev_entry, size,
> - AMAP_EXTEND_FORWARDS);
> + error = amap_extend(prev_entry, size, aflags);
> if (error) {
> vm_map_unlock(map);
> if (new_entry) {
> @@ -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 {
> @@ -807,6 +816,8 @@
> */
> if (prev_entry->next->aref.ar_amap) {
> error = amap_extend(prev_entry->next, size,
> + (flags & UVM_KMF_NOWAIT) ?
> + (AMAP_EXTEND_BACKWARDS|AMAP_EXTEND_NOWAIT) :
> AMAP_EXTEND_BACKWARDS);
> if (error) {
> vm_map_unlock(map);
-Chuck