So, can I remind this simple cleanup?
2010/1/31 Anton Maksimenkov <anton...@gmail.com>: > Here is some cleanup of uvm_map.c code. > > At first, in uvm_map_advice() function. > Let it looks as simple as uvm_map_inherit() - no need to lock/unlock map > just to realize that sanity check fail. Also no need to do it in every loop. > > Second, remove redundant "temp_entry" variable from both functions. > One "entry" variable is enough to do the job. > > $ cvs diff -Nup sys/uvm/uvm_map.c > Index: sys/uvm/uvm_map.c > =================================================================== > RCS file: /cvs/src/sys/uvm/uvm_map.c,v > retrieving revision 1.123 > diff -u -p -r1.123 uvm_map.c > --- sys/uvm/uvm_map.c 28 Aug 2009 00:40:03 -0000 1.123 > +++ sys/uvm/uvm_map.c 31 Jan 2010 11:35:56 -0000 > @@ -2473,7 +2473,7 @@ int > uvm_map_inherit(struct vm_map *map, vaddr_t start, vaddr_t end, > vm_inherit_t new_inheritance) > { > - struct vm_map_entry *entry, *temp_entry; > + struct vm_map_entry *entry; > UVMHIST_FUNC("uvm_map_inherit"); UVMHIST_CALLED(maphist); > UVMHIST_LOG(maphist,"(map=%p,start=0x%lx,end=0x%lx,new_inh=0x%lx)", > map, start, end, new_inheritance); > @@ -2492,11 +2492,10 @@ uvm_map_inherit(struct vm_map *map, vadd > > VM_MAP_RANGE_CHECK(map, start, end); > > - if (uvm_map_lookup_entry(map, start, &temp_entry)) { > - entry = temp_entry; > + if (uvm_map_lookup_entry(map, start, &entry)) { > UVM_MAP_CLIP_START(map, entry, start); > } else { > - entry = temp_entry->next; > + entry = entry->next; > } > > while ((entry != &map->header) && (entry->start < end)) { > @@ -2519,18 +2518,29 @@ uvm_map_inherit(struct vm_map *map, vadd > int > uvm_map_advice(struct vm_map *map, vaddr_t start, vaddr_t end, int new_advice) > { > - struct vm_map_entry *entry, *temp_entry; > + struct vm_map_entry *entry; > UVMHIST_FUNC("uvm_map_advice"); UVMHIST_CALLED(maphist); > UVMHIST_LOG(maphist,"(map=%p,start=0x%lx,end=0x%lx,new_adv=0x%lx)", > map, start, end, new_advice); > > + switch (new_advice) { > + case MADV_NORMAL: > + case MADV_RANDOM: > + case MADV_SEQUENTIAL: > + /* nothing special here */ > + break; > + > + default: > + UVMHIST_LOG(maphist,"<- done (INVALID ARG)",0,0,0,0); > + return (EINVAL); > + } > + > vm_map_lock(map); > VM_MAP_RANGE_CHECK(map, start, end); > - if (uvm_map_lookup_entry(map, start, &temp_entry)) { > - entry = temp_entry; > + if (uvm_map_lookup_entry(map, start, &entry)) { > UVM_MAP_CLIP_START(map, entry, start); > } else { > - entry = temp_entry->next; > + entry = entry->next; > } > > /* > @@ -2539,19 +2549,6 @@ uvm_map_advice(struct vm_map *map, vaddr > > while ((entry != &map->header) && (entry->start < end)) { > UVM_MAP_CLIP_END(map, entry, end); > - > - switch (new_advice) { > - case MADV_NORMAL: > - case MADV_RANDOM: > - case MADV_SEQUENTIAL: > - /* nothing special here */ > - break; > - > - default: > - vm_map_unlock(map); > - UVMHIST_LOG(maphist,"<- done (INVALID ARG)",0,0,0,0); > - return (EINVAL); > - } > entry->advice = new_advice; > entry = entry->next; > } > > -- > antonvm > -- antonvm