On Fri, Oct 30, 2020 at 08:46:20PM +0100, Martin Pieuchot wrote:
> On 23/10/20(Fri) 10:31, Martin Pieuchot wrote:
> > More refactoring. This time let's introduce a helper to manipulate
> > references. The goal is to reduce the upcoming diff adding locking.
> >
> > This is extracted from a bigger diff from guenther@ as well as some
> > bits from NetBSD.
>
> Now with the correct diff, ok?
This looks good to me (and survived a couple of full builds on amd64),
ok jmatthew@
>
> Index: uvm/uvm_amap.c
> ===================================================================
> RCS file: /cvs/src/sys/uvm/uvm_amap.c,v
> retrieving revision 1.85
> diff -u -p -r1.85 uvm_amap.c
> --- uvm/uvm_amap.c 12 Oct 2020 08:44:45 -0000 1.85
> +++ uvm/uvm_amap.c 23 Oct 2020 08:23:59 -0000
> @@ -68,7 +68,23 @@ static inline void amap_list_remove(stru
>
> struct vm_amap_chunk *amap_chunk_get(struct vm_amap *, int, int, int);
> void amap_chunk_free(struct vm_amap *, struct vm_amap_chunk *);
> -void amap_wiperange_chunk(struct vm_amap *, struct vm_amap_chunk *, int,
> int);
> +
> +/*
> + * if we enable PPREF, then we have a couple of extra functions that
> + * we need to prototype here...
> + */
> +
> +#ifdef UVM_AMAP_PPREF
> +
> +#define PPREF_NONE ((int *) -1) /* not using ppref */
> +
> +void amap_pp_adjref(struct vm_amap *, int, vsize_t, int);
> +void amap_pp_establish(struct vm_amap *);
> +void amap_wiperange_chunk(struct vm_amap *, struct vm_amap_chunk *, int,
> + int);
> +void amap_wiperange(struct vm_amap *, int, int);
> +
> +#endif /* UVM_AMAP_PPREF */
>
> static inline void
> amap_list_insert(struct vm_amap *amap)
> @@ -1153,6 +1169,32 @@ amap_unadd(struct vm_aref *aref, vaddr_t
> }
>
> /*
> + * amap_adjref_anons: adjust the reference count(s) on amap and its anons.
> + */
> +static void
> +amap_adjref_anons(struct vm_amap *amap, vaddr_t offset, vsize_t len,
> + int refv, boolean_t all)
> +{
> +#ifdef UVM_AMAP_PPREF
> + if (amap->am_ppref == NULL && !all && len != amap->am_nslot) {
> + amap_pp_establish(amap);
> + }
> +#endif
> +
> + amap->am_ref += refv;
> +
> +#ifdef UVM_AMAP_PPREF
> + if (amap->am_ppref && amap->am_ppref != PPREF_NONE) {
> + if (all) {
> + amap_pp_adjref(amap, 0, amap->am_nslot, refv);
> + } else {
> + amap_pp_adjref(amap, offset, len, refv);
> + }
> + }
> +#endif
> +}
> +
> +/*
> * amap_ref: gain a reference to an amap
> *
> * => "offset" and "len" are in units of pages
> @@ -1162,51 +1204,36 @@ void
> amap_ref(struct vm_amap *amap, vaddr_t offset, vsize_t len, int flags)
> {
>
> - amap->am_ref++;
> if (flags & AMAP_SHARED)
> amap->am_flags |= AMAP_SHARED;
> -#ifdef UVM_AMAP_PPREF
> - if (amap->am_ppref == NULL && (flags & AMAP_REFALL) == 0 &&
> - len != amap->am_nslot)
> - amap_pp_establish(amap);
> - if (amap->am_ppref && amap->am_ppref != PPREF_NONE) {
> - if (flags & AMAP_REFALL)
> - amap_pp_adjref(amap, 0, amap->am_nslot, 1);
> - else
> - amap_pp_adjref(amap, offset, len, 1);
> - }
> -#endif
> + amap_adjref_anons(amap, offset, len, 1, (flags & AMAP_REFALL) != 0);
> }
>
> /*
> * amap_unref: remove a reference to an amap
> *
> - * => caller must remove all pmap-level references to this amap before
> - * dropping the reference
> - * => called from uvm_unmap_detach [only] ... note that entry is no
> - * longer part of a map
> + * => All pmap-level references to this amap must be already removed.
> + * => Called from uvm_unmap_detach(); entry is already removed from the map.
> */
> void
> amap_unref(struct vm_amap *amap, vaddr_t offset, vsize_t len, boolean_t all)
> {
> + KASSERT(amap->am_ref > 0);
>
> - /* if we are the last reference, free the amap and return. */
> - if (amap->am_ref-- == 1) {
> - amap_wipeout(amap); /* drops final ref and frees */
> + if (amap->am_ref == 1) {
> + /*
> + * If the last reference - wipeout and destroy the amap.
> + */
> + amap->am_ref--;
> + amap_wipeout(amap);
> return;
> }
>
> - /* otherwise just drop the reference count(s) */
> - if (amap->am_ref == 1 && (amap->am_flags & AMAP_SHARED) != 0)
> - amap->am_flags &= ~AMAP_SHARED; /* clear shared flag */
> -#ifdef UVM_AMAP_PPREF
> - if (amap->am_ppref == NULL && all == 0 && len != amap->am_nslot)
> - amap_pp_establish(amap);
> - if (amap->am_ppref && amap->am_ppref != PPREF_NONE) {
> - if (all)
> - amap_pp_adjref(amap, 0, amap->am_nslot, -1);
> - else
> - amap_pp_adjref(amap, offset, len, -1);
> + /*
> + * Otherwise, drop the reference count(s) on anons.
> + */
> + if (amap->am_ref == 2 && (amap->am_flags & AMAP_SHARED) != 0) {
> + amap->am_flags &= ~AMAP_SHARED;
> }
> -#endif
> + amap_adjref_anons(amap, offset, len, -1, all);
> }
> Index: uvm/uvm_amap.h
> ===================================================================
> RCS file: /cvs/src/sys/uvm/uvm_amap.h,v
> retrieving revision 1.31
> diff -u -p -r1.31 uvm_amap.h
> --- uvm/uvm_amap.h 15 May 2019 06:12:19 -0000 1.31
> +++ uvm/uvm_amap.h 23 Oct 2020 08:19:43 -0000
> @@ -261,23 +261,6 @@ struct vm_amap {
> #define amap_flags(AMAP) ((AMAP)->am_flags)
> #define amap_refs(AMAP) ((AMAP)->am_ref)
>
> -/*
> - * if we enable PPREF, then we have a couple of extra functions that
> - * we need to prototype here...
> - */
> -
> -#ifdef UVM_AMAP_PPREF
> -
> -#define PPREF_NONE ((int *) -1) /* not using ppref */
> -
> - /* adjust references */
> -void amap_pp_adjref(struct vm_amap *, int, vsize_t, int);
> - /* establish ppref */
> -void amap_pp_establish(struct vm_amap *);
> - /* wipe part of an amap */
> -void amap_wiperange(struct vm_amap *, int, int);
> -#endif /* UVM_AMAP_PPREF */
> -
> #endif /* _KERNEL */
>
> #endif /* _UVM_UVM_AMAP_H_ */
>
