Re: git: dd0e5c02ab13 - main - swap_pager: small improvement to find_least

From: Peter Holm <peter_at_holm.cc>
Date: Sun, 16 Jun 2024 17:14:10 UTC
On Sun, Jun 16, 2024 at 11:55:48AM -0500, Doug Moore wrote:
> Thank you, Peter.  That was a stupid mistake and I have now undone it.
> 
> Doug
> 

Happy to help. It was a KASAN build that found the issue.

- Peter

> On 6/16/24 07:13, Peter Holm wrote:
> > On Tue, Jun 11, 2024 at 04:38:49PM +0000, Doug Moore wrote:
> > > The branch main has been updated by dougm:
> > > 
> > > URL: https://cgit.FreeBSD.org/src/commit/?id=dd0e5c02ab13b9eb240d42a71a8f41a8b036bd33
> > > 
> > > commit dd0e5c02ab13b9eb240d42a71a8f41a8b036bd33
> > > Author:     Doug Moore <dougm@FreeBSD.org>
> > > AuthorDate: 2024-06-11 16:36:23 +0000
> > > Commit:     Doug Moore <dougm@FreeBSD.org>
> > > CommitDate: 2024-06-11 16:36:23 +0000
> > > 
> > >      swap_pager: small improvement to find_least
> > >      Drop an unneeded test, a branch and a needless computation to save a
> > >      few instructions.
> > >      Reviewed by:    kib
> > >      Differential Revision:  https://reviews.freebsd.org/D45558
> > > ---
> > >   sys/vm/swap_pager.c | 19 +++++++------------
> > >   1 file changed, 7 insertions(+), 12 deletions(-)
> > > 
> > > diff --git a/sys/vm/swap_pager.c b/sys/vm/swap_pager.c
> > > index a07a38481490..15f227457bba 100644
> > > --- a/sys/vm/swap_pager.c
> > Doug,
> > 
> > Here's an easy way to reproduce the issue:
> > 
> > 12:43 ~ $ sort /dev/zero &
> > [1] 3650
> > 12:43 ~ $ sort /dev/zero &
> > [2] 3651
> > 12:43 ~ $ sort /dev/zero &
> > [3] 3652
> > 12:43 ~ $ panic: Negative index: -16
> > cpuid = 8
> > time = 1718534645
> > KDB: stack backtrace:
> > db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame 0xfffffe010855fa50
> > vpanic() at vpanic+0x13f/frame 0xfffffe010855fb80
> > panic() at panic+0x43/frame 0xfffffe010855fbe0
> > swap_pager_find_least() at swap_pager_find_least+0x105/frame 0xfffffe010855fc20
> > vm_object_collapse() at vm_object_collapse+0x68b/frame 0xfffffe010855fca0
> > vmspace_fork() at vmspace_fork+0xb68/frame 0xfffffe010855fd30
> > fork1() at fork1+0x4f8/frame 0xfffffe010855fda0
> > sys_fork() at sys_fork+0x54/frame 0xfffffe010855fe00
> > amd64_syscall() at amd64_syscall+0x158/frame 0xfffffe010855ff30
> > fast_syscall_common() at fast_syscall_common+0xf8/frame 0xfffffe010855ff30
> > --- syscall (2, FreeBSD ELF64, fork), rip = 0xaba1c31565a, rsp = 0xaba1b0cecb8, rbp = 0xaba1b0cf220 ---
> > KDB: enter: panic
> > [ thread pid 3653 tid 100349 ]
> > Stopped at         $0,0x104d5c2(%rip)
> > db> x/s version
> > version:        FreeBSD 15.0-CURRENT #2 main-n270778-d26c565597b8-dirty: Sun Jun 16 12:32:39 CEST 2024\012    pho@mercat1.netperf.freebsd.org:/usr/src/sys/amd64/compile/PHO\012
> > db>
> > 
> > --- a/sys/vm/swap_pager.c
> > +++ b/sys/vm/swap_pager.c
> > @@ -2303,6 +2303,7 @@ swap_pager_find_least(vm_object_t object, vm_pindex_t pindex)
> >          if (sb == NULL)
> >                  return (object->size);
> >          for (i = pindex - sb->p; i < SWAP_META_PAGES; i++) {
> > +               KASSERT((i >= 0), ("Negative index: %d", i));
> >                  if (sb->d[i] != SWAPBLK_NONE)
> >                          return (sb->p + i);
> >          }
> > 
> > - Peter