Re: kern/56764

["Robert Elz via gnats" <gnats-admin%NetBSD.org@localhost>]

The following reply was made to PR kern/56764; it has been noted by GNATS.

From: Robert Elz <kre%munnari.OZ.AU@localhost>
To: Thomas Klausner <wiz%netbsd.org@localhost>
Cc: gnats-bugs%netbsd.org@localhost
Subject: Re: kern/56764
Date: Sun, 22 Feb 2026 01:14:26 +0700

     Date:        Sat, 21 Feb 2026 18:30:55 +0100
     From:        Thomas Klausner <wiz%netbsd.org@localhost>
     Message-ID:  <aZnqBQcezBYRM5zw%exadelic.gatalith.at@localhost>
 
 
   | It only accesses uvmexp members and still grabs the lock.
 
 OK, like I said, I am guessing a lot, but if
 
   | If you look closely you'll find that swpginuse is only modified when
   | uvm_swap_data_lock is held.
 
 that's true, then good.
 
   | I've hijacked this PR a bit - it was about
   | kernel diagnostic assertion "uvmexp.swpgonly > 0" failed
 
 Yes.   I saw.
 
   | while I was hitting
   |
   | kernel diagnostic assertion "uvmexp.swpgonly < uvmexp.swpginuse" failed
 
 But that's an assertion you just added in Jan, right?
 
   | the swpgonly > 0 definitely sounds sane while the other one should
   | (according to Chuck) always be true too.
 
 Logically, I agree, but with new compilers and the current C rules,
 I'm not sure the code can guarantee that.
 
 Since the addr of uvmexp.swpgonly is passed to the atomic functions,
 the compiler must assume that it might be being modified, which of
 course it is.   But that means it needs to be read from the struct
 on any reference after one of those operations (there's just the one
 in uvmpd_scan_queue() which is the KASSERT() in question).
 
 But that's not true of uvmexp.swpginuse ... uvmpd_scan_queue() is just one
 big loop, runs until it has nothing left to do, over and over.   The compiler
 might read uvmexp.swpginuse, stick in a register, and as nothing in that
 func is touching it, nor passing its addr to any other funcs, and it isn't
 volatile, the compiler is entitled (by modern C rules) to assume that nothing
 is changing it.   Maybe that doesn't apply to extern structs, beats me.
 
 But I also see this code in uvmpd_scan_queue() (not that I expect this
 is related to your issue):
 
                 if ((p->flags & PG_SWAPBACKED) == 0) {
                         KASSERT(uobj != NULL);
                         (void) (uobj->pgops->pgo_put)(uobj, p->offset,
                             p->offset + PAGE_SIZE, PGO_CLEANIT|PGO_FREE);
                         continue;
                 }       
 
 
 That's going around the big loop again - but I see no attempt to
 
 			rw_exit(slock);
 
 slock is a local var in uvmpd_scan_queue() so whatever the function is
 (uobj->pgops->pgo_put) it cannot be exiting the locked region.
 
 Every other "continue;" in that section of code (before and after that one
 - from after the slock is obtained) has that rw_exit(slock) just before the
 continue, so there may well be lots of locked stuff hanging around, if
 that particular piece of code ever runs.   Very unlikely to be related to
 your issue(s) but it looks broken to me.
 
 kre