On 16/06/21(Wed) 11:26, Martin Pieuchot wrote:
> Diff below does two things:
> 
> - Use atomic operations for incrementing/decrementing references of
>   anonymous objects.  This allows us to manipulate them without holding
>   the KERNEL_LOCK().
> 
> - Rewrite the loop from uao_swap_off() to only keep a reference to the
>   next item in the list.  This is imported from NetBSD and is necessary
>   to introduce locking around uao_pagein().
> 
> ok?

Anyone?

> 
> Index: uvm/uvm_aobj.c
> ===================================================================
> RCS file: /cvs/src/sys/uvm/uvm_aobj.c,v
> retrieving revision 1.98
> diff -u -p -r1.98 uvm_aobj.c
> --- uvm/uvm_aobj.c    16 Jun 2021 09:02:21 -0000      1.98
> +++ uvm/uvm_aobj.c    16 Jun 2021 09:20:26 -0000
> @@ -779,19 +779,11 @@ uao_init(void)
>  void
>  uao_reference(struct uvm_object *uobj)
>  {
> -     KERNEL_ASSERT_LOCKED();
> -     uao_reference_locked(uobj);
> -}
> -
> -void
> -uao_reference_locked(struct uvm_object *uobj)
> -{
> -
>       /* Kernel object is persistent. */
>       if (UVM_OBJ_IS_KERN_OBJECT(uobj))
>               return;
>  
> -     uobj->uo_refs++;
> +     atomic_inc_int(&uobj->uo_refs);
>  }
>  
>  
> @@ -801,34 +793,19 @@ uao_reference_locked(struct uvm_object *
>  void
>  uao_detach(struct uvm_object *uobj)
>  {
> -     KERNEL_ASSERT_LOCKED();
> -     uao_detach_locked(uobj);
> -}
> -
> -
> -/*
> - * uao_detach_locked: drop a reference to an aobj
> - *
> - * => aobj may freed upon return.
> - */
> -void
> -uao_detach_locked(struct uvm_object *uobj)
> -{
>       struct uvm_aobj *aobj = (struct uvm_aobj *)uobj;
>       struct vm_page *pg;
>  
>       /*
>        * Detaching from kernel_object is a NOP.
>        */
> -     if (UVM_OBJ_IS_KERN_OBJECT(uobj)) {
> +     if (UVM_OBJ_IS_KERN_OBJECT(uobj))
>               return;
> -     }
>  
>       /*
>        * Drop the reference.  If it was the last one, destroy the object.
>        */
> -     uobj->uo_refs--;
> -     if (uobj->uo_refs) {
> +     if (atomic_dec_int_nv(&uobj->uo_refs) > 0) {
>               return;
>       }
>  
> @@ -1265,68 +1242,54 @@ uao_dropswap(struct uvm_object *uobj, in
>  boolean_t
>  uao_swap_off(int startslot, int endslot)
>  {
> -     struct uvm_aobj *aobj, *nextaobj, *prevaobj = NULL;
> +     struct uvm_aobj *aobj;
>  
>       /*
> -      * Walk the list of all anonymous UVM objects.
> +      * Walk the list of all anonymous UVM objects.  Grab the first.
>        */
>       mtx_enter(&uao_list_lock);
> +     if ((aobj = LIST_FIRST(&uao_list)) == NULL) {
> +             mtx_leave(&uao_list_lock);
> +             return FALSE;
> +     }
> +     uao_reference(&aobj->u_obj);
>  
> -     for (aobj = LIST_FIRST(&uao_list);
> -          aobj != NULL;
> -          aobj = nextaobj) {
> +     do {
> +             struct uvm_aobj *nextaobj;
>               boolean_t rv;
>  
>               /*
> -              * add a ref to the aobj so it doesn't disappear
> -              * while we're working.
> -              */
> -             uao_reference_locked(&aobj->u_obj);
> -
> -             /*
> -              * now it's safe to unlock the uao list.
> -              * note that lock interleaving is alright with IPL_NONE mutexes.
> +              * Prefetch the next object and immediately hold a reference
> +              * on it, so neither the current nor the next entry could
> +              * disappear while we are iterating.
>                */
> -             mtx_leave(&uao_list_lock);
> -
> -             if (prevaobj) {
> -                     uao_detach_locked(&prevaobj->u_obj);
> -                     prevaobj = NULL;
> +             if ((nextaobj = LIST_NEXT(aobj, u_list)) != NULL) {
> +                     uao_reference(&nextaobj->u_obj);
>               }
> +             mtx_leave(&uao_list_lock);
>  
>               /*
> -              * page in any pages in the swslot range.
> -              * if there's an error, abort and return the error.
> +              * Page in all pages in the swap slot range.
>                */
>               rv = uao_pagein(aobj, startslot, endslot);
> +
> +             /* Drop the reference of the current object. */
> +             uao_detach(&aobj->u_obj);
>               if (rv) {
> -                     uao_detach_locked(&aobj->u_obj);
> +                     if (nextaobj) {
> +                             uao_detach(&nextaobj->u_obj);
> +                     }
>                       return rv;
>               }
>  
> -             /*
> -              * we're done with this aobj.
> -              * relock the list and drop our ref on the aobj.
> -              */
> +             aobj = nextaobj;
>               mtx_enter(&uao_list_lock);
> -             nextaobj = LIST_NEXT(aobj, u_list);
> -             /*
> -              * prevaobj means that we have an object that we need
> -              * to drop a reference for. We can't just drop it now with
> -              * the list locked since that could cause lock recursion in
> -              * the case where we reduce the refcount to 0. It will be
> -              * released the next time we drop the list lock.
> -              */
> -             prevaobj = aobj;
> -     }
> +     } while (aobj);
>  
>       /*
>        * done with traversal, unlock the list
>        */
>       mtx_leave(&uao_list_lock);
> -     if (prevaobj) {
> -             uao_detach_locked(&prevaobj->u_obj);
> -     }
>       return FALSE;
>  }
>  
> 

Reply via email to