On Mon, Jun 14, 2021 at 05:35:07PM +0200, Mark Kettenis wrote:
> > Date: Mon, 14 Jun 2021 11:50:24 +0200
> > From: Martin Pieuchot <[email protected]>
> >
> > Now that operations on amaps are serialized using a per-map rwlock
> > the KERNEL_LOCK() shouldn't be necessary to call amap_unref(). The
> > diff below allows the reaper to do this operation before grabbing it.
> >
> > I haven't seen any relevant contention on the reaper in my profilings,
> > so I don't expect any visible change related to this change. However
> > this reflects the current state of locking in UVM and helps me shrink
> > my diff.
> >
> > ok?
>
> This means we no longer call uvm_pause() for these, but I believe the
> main reason for calling uvm_pause() is to prevent us from holding the
> kernel lock for too long. So I think that's fine.
>
> ok kettenis@
And I guess to allow something else to run if we're on a single
processor system, which I don't think is a huge concern.
ok jmatthew@
>
>
> > Index: uvm/uvm_map.c
> > ===================================================================
> > RCS file: /cvs/src/sys/uvm/uvm_map.c,v
> > retrieving revision 1.275
> > diff -u -p -r1.275 uvm_map.c
> > --- uvm/uvm_map.c 22 May 2021 08:38:29 -0000 1.275
> > +++ uvm/uvm_map.c 14 Jun 2021 09:32:04 -0000
> > @@ -1571,10 +1571,16 @@ uvm_unmap_detach(struct uvm_map_deadq *d
> >
> > TAILQ_FOREACH_SAFE(entry, deadq, dfree.deadq, tmp) {
> > /* Skip entries for which we have to grab the kernel lock. */
> > - if (entry->aref.ar_amap || UVM_ET_ISSUBMAP(entry) ||
> > - UVM_ET_ISOBJ(entry))
> > + if (UVM_ET_ISSUBMAP(entry) || UVM_ET_ISOBJ(entry))
> > continue;
> >
> > + /* Drop reference to amap, if we've got one. */
> > + if (entry->aref.ar_amap)
> > + amap_unref(entry->aref.ar_amap,
> > + entry->aref.ar_pageoff,
> > + atop(entry->end - entry->start),
> > + flags & AMAP_REFALL);
> > +
> > TAILQ_REMOVE(deadq, entry, dfree.deadq);
> > uvm_mapent_free(entry);
> > }
> > @@ -1586,12 +1592,6 @@ uvm_unmap_detach(struct uvm_map_deadq *d
> > while ((entry = TAILQ_FIRST(deadq)) != NULL) {
> > if (waitok)
> > uvm_pause();
> > - /* Drop reference to amap, if we've got one. */
> > - if (entry->aref.ar_amap)
> > - amap_unref(entry->aref.ar_amap,
> > - entry->aref.ar_pageoff,
> > - atop(entry->end - entry->start),
> > - flags & AMAP_REFALL);
> >
> > /* Drop reference to our backing object, if we've got one. */
> > if (UVM_ET_ISSUBMAP(entry)) {
> >
> >
>
