On 24/01/22(Mon) 12:06, Klemens Nanni wrote: > On Sun, Jan 16, 2022 at 09:22:50AM -0300, Martin Pieuchot wrote: > > IMHO this approach of let's try if it works now and revert if it isn't > > doesn't help us make progress. I'd be more confident seeing diffs that > > assert for the right lock in the functions called by uvm_mapanon() and > > documentation about which lock is protecting which field of the data > > structures. > > I picked `vm_map's `size' as first underdocumented member. > All accesses to it are protected by vm_map_lock(), either through the > function itself or implicitly by already requiring the calling function > to lock the map.
Could we use a vm_map_assert_locked() or something similar that reflect the exclusive or shared (read) lock comments? I don't trust comments. It's too easy to miss a lock in a code path. > So annotate functions using `size' wrt. the required lock. > > Index: uvm_map.c > =================================================================== > RCS file: /cvs/src/sys/uvm/uvm_map.c,v > retrieving revision 1.282 > diff -u -p -r1.282 uvm_map.c > --- uvm_map.c 21 Dec 2021 22:21:32 -0000 1.282 > +++ uvm_map.c 21 Jan 2022 13:03:05 -0000 > @@ -805,6 +805,8 @@ uvm_map_sel_limits(vaddr_t *min, vaddr_t > * Fills in *start_ptr and *end_ptr to be the first and last entry describing > * the space. > * If called with prefilled *start_ptr and *end_ptr, they are to be correct. > + * > + * map must be at least read-locked. > */ > int > uvm_map_isavail(struct vm_map *map, struct uvm_addr_state *uaddr, > @@ -2206,6 +2208,8 @@ uvm_unmap_kill_entry(struct vm_map *map, > * If remove_holes, then remove ET_HOLE entries as well. > * If markfree, entry will be properly marked free, otherwise, no replacement > * entry will be put in the tree (corrupting the tree). > + * > + * map must be locked. > */ > void > uvm_unmap_remove(struct vm_map *map, vaddr_t start, vaddr_t end, > @@ -2976,6 +2980,9 @@ uvm_tree_sanity(struct vm_map *map, char > UVM_ASSERT(map, addr == vm_map_max(map), file, line); > } > > +/* > + * map must be at least read-locked. > + */ > void > uvm_tree_size_chk(struct vm_map *map, char *file, int line) > { > Index: uvm_map.h > =================================================================== > RCS file: /cvs/src/sys/uvm/uvm_map.h,v > retrieving revision 1.71 > diff -u -p -r1.71 uvm_map.h > --- uvm_map.h 15 Dec 2021 12:53:53 -0000 1.71 > +++ uvm_map.h 21 Jan 2022 12:51:26 -0000 > @@ -272,7 +272,7 @@ struct vm_map { > > struct uvm_map_addr addr; /* [v] Entry tree, by addr */ > > - vsize_t size; /* virtual size */ > + vsize_t size; /* [v] virtual size */ > int ref_count; /* [a] Reference count */ > int flags; /* flags */ > struct mutex flags_lock; /* flags lock */ >