Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 29 Dec 2016 13:48:06 +0200
From:      Konstantin Belousov <kostikbel@gmail.com>
To:        Adrian Chadd <adrian.chadd@gmail.com>
Cc:        "src-committers@freebsd.org" <src-committers@freebsd.org>, "svn-src-all@freebsd.org" <svn-src-all@freebsd.org>, "svn-src-head@freebsd.org" <svn-src-head@freebsd.org>
Subject:   Re: svn commit: r310234 - head/sys/vm
Message-ID:  <20161229114806.GH1923@kib.kiev.ua>
In-Reply-To: <CAJ-Vmok_9ZN5OzUpK00uOTHUKxHgs8mhC5btr70UfV4%2BnwBvDA@mail.gmail.com>
References:  <201612182056.uBIKuElT070792@repo.freebsd.org> <CAJ-Vmok_9ZN5OzUpK00uOTHUKxHgs8mhC5btr70UfV4%2BnwBvDA@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Dec 28, 2016 at 07:03:52PM -0800, Adrian Chadd wrote:
> hi,
> 
> I've bisected a boot failure on mips24k (carambola2) to this commit.
> Nothing useful is printed - it hangs at startup time.
What do you mean exactly ?  Is copyright notice printed ?
Show verbose dmesg of the failing boot.

> 
> Any ideas?
No. The patch you picked changes code that is only executed for the
shadowed anonymous memory. AFAIK, including mips, there is no shadow
chains in the kernel map. Such constructs are only instantiated when
user space is operating and forking. Even if such data structure appears
in the kernel mappings, vm object code is not initialized until much
later in the boot, at vm_init stage.

> 
> 
> -adrian
> 
> 
> On 18 December 2016 at 12:56, Konstantin Belousov <kib@freebsd.org> wrote:
> > Author: kib
> > Date: Sun Dec 18 20:56:14 2016
> > New Revision: 310234
> > URL: https://svnweb.freebsd.org/changeset/base/310234
> >
> > Log:
> >   Improve vm_object_scan_all_shadowed() to also check swap backing objects.
> >
> >   As noted in the removed comment, it is possible and not prohibitively
> >   costly to look up the swap blocks for the given page index.  Implement
> >   a swap_pager_find_least() function to do that, and use it to iterate
> >   simultaneously over both backing object page queue and swap
> >   allocations when looking for shadowed pages.
> >
> >   Testing shows that number of new succesful scans, enabled by this
> >   addition, is small but non-zero.  When worked out, the change both
> >   further reduces the depth of the shadow object chain, and frees unused
> >   but allocated swap and memory.
> >
> >   Suggested and reviewed by:    alc
> >   Tested by:    pho (previous version)
> >   Sponsored by: The FreeBSD Foundation
> >   MFC after:    2 weeks
> >
> > Modified:
> >   head/sys/vm/swap_pager.c
> >   head/sys/vm/swap_pager.h
> >   head/sys/vm/vm_object.c
> >
> > Modified: head/sys/vm/swap_pager.c
> > ==============================================================================
> > --- head/sys/vm/swap_pager.c    Sun Dec 18 20:40:22 2016        (r310233)
> > +++ head/sys/vm/swap_pager.c    Sun Dec 18 20:56:14 2016        (r310234)
> > @@ -2013,6 +2013,44 @@ swp_pager_meta_ctl(vm_object_t object, v
> >  }
> >
> >  /*
> > + * Returns the least page index which is greater than or equal to the
> > + * parameter pindex and for which there is a swap block allocated.
> > + * Returns object's size if the object's type is not swap or if there
> > + * are no allocated swap blocks for the object after the requested
> > + * pindex.
> > + */
> > +vm_pindex_t
> > +swap_pager_find_least(vm_object_t object, vm_pindex_t pindex)
> > +{
> > +       struct swblock **pswap, *swap;
> > +       vm_pindex_t i, j, lim;
> > +       int idx;
> > +
> > +       VM_OBJECT_ASSERT_LOCKED(object);
> > +       if (object->type != OBJT_SWAP || object->un_pager.swp.swp_bcount == 0)
> > +               return (object->size);
> > +
> > +       mtx_lock(&swhash_mtx);
> > +       for (j = pindex; j < object->size; j = lim) {
> > +               pswap = swp_pager_hash(object, j);
> > +               lim = rounddown2(j + SWAP_META_PAGES, SWAP_META_PAGES);
> > +               if (lim > object->size)
> > +                       lim = object->size;
> > +               if ((swap = *pswap) != NULL) {
> > +                       for (idx = j & SWAP_META_MASK, i = j; i < lim;
> > +                           i++, idx++) {
> > +                               if (swap->swb_pages[idx] != SWAPBLK_NONE)
> > +                                       goto found;
> > +                       }
> > +               }
> > +       }
> > +       i = object->size;
> > +found:
> > +       mtx_unlock(&swhash_mtx);
> > +       return (i);
> > +}
> > +
> > +/*
> >   * System call swapon(name) enables swapping on device name,
> >   * which must be in the swdevsw.  Return EBUSY
> >   * if already swapping on this device.
> >
> > Modified: head/sys/vm/swap_pager.h
> > ==============================================================================
> > --- head/sys/vm/swap_pager.h    Sun Dec 18 20:40:22 2016        (r310233)
> > +++ head/sys/vm/swap_pager.h    Sun Dec 18 20:56:14 2016        (r310234)
> > @@ -79,6 +79,7 @@ extern int swap_pager_avail;
> >  struct xswdev;
> >  int swap_dev_info(int name, struct xswdev *xs, char *devname, size_t len);
> >  void swap_pager_copy(vm_object_t, vm_object_t, vm_pindex_t, int);
> > +vm_pindex_t swap_pager_find_least(vm_object_t object, vm_pindex_t pindex);
> >  void swap_pager_freespace(vm_object_t, vm_pindex_t, vm_size_t);
> >  void swap_pager_swap_init(void);
> >  int swap_pager_isswapped(vm_object_t, struct swdevt *);
> >
> > Modified: head/sys/vm/vm_object.c
> > ==============================================================================
> > --- head/sys/vm/vm_object.c     Sun Dec 18 20:40:22 2016        (r310233)
> > +++ head/sys/vm/vm_object.c     Sun Dec 18 20:56:14 2016        (r310234)
> > @@ -1436,36 +1436,40 @@ vm_object_scan_all_shadowed(vm_object_t
> >  {
> >         vm_object_t backing_object;
> >         vm_page_t p, pp;
> > -       vm_pindex_t backing_offset_index, new_pindex;
> > +       vm_pindex_t backing_offset_index, new_pindex, pi, ps;
> >
> >         VM_OBJECT_ASSERT_WLOCKED(object);
> >         VM_OBJECT_ASSERT_WLOCKED(object->backing_object);
> >
> >         backing_object = object->backing_object;
> >
> > -       /*
> > -        * Initial conditions:
> > -        *
> > -        * We do not want to have to test for the existence of swap
> > -        * pages in the backing object.  XXX but with the new swapper this
> > -        * would be pretty easy to do.
> > -        */
> > -       if (backing_object->type != OBJT_DEFAULT)
> > +       if (backing_object->type != OBJT_DEFAULT &&
> > +           backing_object->type != OBJT_SWAP)
> >                 return (false);
> >
> > -       backing_offset_index = OFF_TO_IDX(object->backing_object_offset);
> > +       pi = backing_offset_index = OFF_TO_IDX(object->backing_object_offset);
> > +       p = vm_page_find_least(backing_object, pi);
> > +       ps = swap_pager_find_least(backing_object, pi);
> >
> > -       for (p = TAILQ_FIRST(&backing_object->memq); p != NULL;
> > -           p = TAILQ_NEXT(p, listq)) {
> > -               new_pindex = p->pindex - backing_offset_index;
> > +       /*
> > +        * Only check pages inside the parent object's range and
> > +        * inside the parent object's mapping of the backing object.
> > +        */
> > +       for (;; pi++) {
> > +               if (p != NULL && p->pindex < pi)
> > +                       p = TAILQ_NEXT(p, listq);
> > +               if (ps < pi)
> > +                       ps = swap_pager_find_least(backing_object, pi);
> > +               if (p == NULL && ps >= backing_object->size)
> > +                       break;
> > +               else if (p == NULL)
> > +                       pi = ps;
> > +               else
> > +                       pi = MIN(p->pindex, ps);
> >
> > -               /*
> > -                * Ignore pages outside the parent object's range and outside
> > -                * the parent object's mapping of the backing object.
> > -                */
> > -               if (p->pindex < backing_offset_index ||
> > -                   new_pindex >= object->size)
> > -                       continue;
> > +               new_pindex = pi - backing_offset_index;
> > +               if (new_pindex >= object->size)
> > +                       break;
> >
> >                 /*
> >                  * See if the parent has the page or if the parent's object
> >



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20161229114806.GH1923>