Re: request for testing: malloc and large allocations
On Sun, Jan 09, 2022 at 02:54:43PM +0100, Otto Moerbeek wrote: > Hi, > > currently malloc does cache a number of free'ed regions up to 128k in > size. This cache is indexed by size (in # of pages), so it is very > quick to check. > > Some programs allocate and deallocate larger allocations in a frantic > way. Accodomate those programs by also keeping a cache of regions > betwen 128k and 2M, in a cache of variable sized regions. > > My test case speeds up about twice. A make build gets a small speedup. > > This has been tested by myself on amd64 quite intensively. I am asking > for more tests, especialy on more "exotic" platforms. I wil do arm64 > myself soon. Test can be running your favorite programs, doing make > builds or running tests in regress/lib/libc/malloc. > > Thanks in advance! I received several success reports and one failure on macppc report from Miod. I'm investiging that report to see if this diff can be blamed. In the meantime: keep on testing! -Otto
usr.sbin/eigrpd: fix -Wunused-but-set-variable warning
usr.sbin/eigrpd: fix -Wunused-but-set-variable warning
M usr.sbin/eigrpd/rde_dual.c
diff 41bbcfa017d9537de08312789d0087c674ce4732
77e83947795bf5b53aef72070d7630a825b52c1f
blob - f1ed306e1371896676cf45b26e102af5e4d77b36
blob + 9465250934a6b1a5f6d565a13d32c4c450786670
--- usr.sbin/eigrpd/rde_dual.c
+++ usr.sbin/eigrpd/rde_dual.c
@@ -1048,11 +1048,9 @@ rde_last_reply(struct rt_node *rn)
struct eigrp*eigrp = rn->eigrp;
struct eigrp_route *successor;
struct rde_nbr *old_successor;
- uint32_t old_fdistance;
struct rinfo ri;
old_successor = rn->successor.nbr;
- old_fdistance = rn->successor.fdistance;
switch (rn->state) {
case DUAL_STA_ACTIVE0:
--
Christian "naddy" Weisgerber na...@mips.inka.de
usr.sbin/dvmrpctl: fix -Wunused-but-set-variable warning
usr.sbin/dvmrpctl: fix -Wunused-but-set-variable warning
This looks like /* not yet implemented */, but the companion functions
show_rib_detail_msg() and show_mfc_detail_msg() are equally empty.
M usr.sbin/dvmrpctl/dvmrpctl.c
diff 4bd575d8630c92f404211d2c625b200ac28213b9
41bbcfa017d9537de08312789d0087c674ce4732
blob - 1b06d2bd134200716094881258b254877a1bf5d1
blob + ab02e0bb29b038ddaad26a49f43ba567d43770bc
--- usr.sbin/dvmrpctl/dvmrpctl.c
+++ usr.sbin/dvmrpctl/dvmrpctl.c
@@ -515,11 +515,8 @@ print_dvmrp_options(u_int8_t opts)
int
show_nbr_detail_msg(struct imsg *imsg)
{
- struct ctl_nbr *nbr;
-
switch (imsg->hdr.type) {
case IMSG_CTL_SHOW_NBR:
- nbr = imsg->data;
break;
case IMSG_CTL_END:
printf("\n");
--
Christian "naddy" Weisgerber na...@mips.inka.de
usr.sbin/dhcpd: fix -Wunused-but-set-variable warning
usr.sbin/dhcpd: fix -Wunused-but-set-variable warning
I think this should be fine as strtonum() will catch any errors.
M usr.sbin/dhcpd/parse.c
diff 5ce17aab12dafb0a17452c4e0e86b29d89d83d13
4bd575d8630c92f404211d2c625b200ac28213b9
blob - 5b719219d00046e02e932796a78dc14ceddaa109
blob + 28334a443547ac7de98e80b4ca493429bfd60b09
--- usr.sbin/dhcpd/parse.c
+++ usr.sbin/dhcpd/parse.c
@@ -293,9 +293,8 @@ parse_lease_time(FILE *cfile, time_t *timep)
const char *errstr;
char *val;
uint32_t value;
- int token;
- token = next_token(, cfile);
+ next_token(, cfile);
value = strtonum(val, 0, UINT32_MAX, );
if (errstr) {
--
Christian "naddy" Weisgerber na...@mips.inka.de
Re: fix active scan on iwm and iwx
> Date: Sun, 16 Jan 2022 19:28:06 +0100 > From: Stefan Sperling > > On Sun, Jan 16, 2022 at 03:50:55PM +0100, Mark Kettenis wrote: > > However, running this diff I had a problem after resuming my laptop > > twice. After resume the interface didn't work and I found the > > following in dmesg: > > > > iwm0: could not initialize hardware > > > > I tried to reset the interface by bringing it down and up again, which > > crashed the machine. It must have been in ddb since typing "bo re" > > made it reset. Unfortunately I don't have further information since I > > was in X. > > Did you try reproducing this problem without the patch in place? > It would be good to know whether this problem is being introduced by > this patch. I don't believe this is likely, my bet would be that this > is an existing problem. But it would be good to know for sure. Yes. I switched back to regular snapshots. Will keep you posted.
Re: uvm_swap: introduce uvm_swap_data_lock
Nice!
On 30/12/21(Thu) 23:38, Theo Buehler wrote:
> The diff below does two things: it adds a uvm_swap_data_lock mutex and
> trades it for the KERNEL_LOCK in uvm_swapisfull() and uvm_swap_markbad()
Why is it enough? Which fields is the lock protecting in these
function? Is it `uvmexp.swpages', could that be documented?
What about `nswapdev'? Why is the rwlock grabbed before reading it in
sys_swapctl()?i
What about `swpginuse'?
If the mutex/rwlock are used to protect the global `swap_priority' could
that be also documented? Once this is documented it should be trivial to
see that some places are missing some locking. Is it intentional?
> The uvm_swap_data_lock protects all swap data structures, so needs to be
> grabbed a few times, many of them already documented in the comments.
>
> For review, I suggest comparing to what NetBSD did and also going
> through the consumers (swaplist_insert, swaplist_find, swaplist_trim)
> and check that they are properly locked when called, or that there is
> the KERNEL_LOCK() in place when swap data structures are manipulated.
I'd suggest using the KASSERT(rw_write_held()) idiom to further reduce
the differences with NetBSD.
> In swapmount() I introduced locking since that's needed to be able to
> assert that the proper locks are held in swaplist_{insert,find,trim}.
Could the KERNEL_LOCK() in uvm_swap_get() be pushed a bit further down?
What about `uvmexp.nswget' and `uvmexp.swpgonly' in there?
> Index: uvm/uvm_swap.c
> ===
> RCS file: /cvs/src/sys/uvm/uvm_swap.c,v
> retrieving revision 1.152
> diff -u -p -r1.152 uvm_swap.c
> --- uvm/uvm_swap.c12 Dec 2021 09:14:59 - 1.152
> +++ uvm/uvm_swap.c30 Dec 2021 15:47:20 -
> @@ -44,6 +44,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -91,6 +92,9 @@
> * - swap_syscall_lock (sleep lock): this lock serializes the swapctl
> *system call and prevents the swap priority list from changing
> *while we are in the middle of a system call (e.g. SWAP_STATS).
> + * - uvm_swap_data_lock (mutex): this lock protects all swap data
> + *structures including the priority list, the swapdev structures,
> + *and the swapmap arena.
> *
> * each swap device has the following info:
> * - swap device in use (could be disabled, preventing future use)
> @@ -212,6 +216,7 @@ LIST_HEAD(swap_priority, swappri);
> struct swap_priority swap_priority;
>
> /* locks */
> +struct mutex uvm_swap_data_lock = MUTEX_INITIALIZER(IPL_NONE);
> struct rwlock swap_syscall_lock = RWLOCK_INITIALIZER("swplk");
>
> /*
> @@ -442,7 +447,7 @@ uvm_swap_finicrypt_all(void)
> /*
> * swaplist_insert: insert swap device "sdp" into the global list
> *
> - * => caller must hold both swap_syscall_lock and uvm.swap_data_lock
> + * => caller must hold both swap_syscall_lock and uvm_swap_data_lock
> * => caller must provide a newly malloc'd swappri structure (we will
> * FREE it if we don't need it... this it to prevent malloc blocking
> * here while adding swap)
> @@ -452,6 +457,9 @@ swaplist_insert(struct swapdev *sdp, str
> {
> struct swappri *spp, *pspp;
>
> + rw_assert_wrlock(_syscall_lock);
> + MUTEX_ASSERT_LOCKED(_swap_data_lock);
> +
> /*
>* find entry at or after which to insert the new device.
>*/
> @@ -493,7 +501,7 @@ swaplist_insert(struct swapdev *sdp, str
> * swaplist_find: find and optionally remove a swap device from the
> * global list.
> *
> - * => caller must hold both swap_syscall_lock and uvm.swap_data_lock
> + * => caller must hold both swap_syscall_lock and uvm_swap_data_lock
> * => we return the swapdev we found (and removed)
> */
> struct swapdev *
> @@ -502,6 +510,9 @@ swaplist_find(struct vnode *vp, boolean_
> struct swapdev *sdp;
> struct swappri *spp;
>
> + rw_assert_wrlock(_syscall_lock);
> + MUTEX_ASSERT_LOCKED(_swap_data_lock);
> +
> /*
>* search the lists for the requested vp
>*/
> @@ -524,13 +535,16 @@ swaplist_find(struct vnode *vp, boolean_
> * swaplist_trim: scan priority list for empty priority entries and kill
> * them.
> *
> - * => caller must hold both swap_syscall_lock and uvm.swap_data_lock
> + * => caller must hold both swap_syscall_lock and uvm_swap_data_lock
> */
> void
> swaplist_trim(void)
> {
> struct swappri *spp, *nextspp;
>
> + rw_assert_wrlock(_syscall_lock);
> + MUTEX_ASSERT_LOCKED(_swap_data_lock);
> +
> LIST_FOREACH_SAFE(spp, _priority, spi_swappri, nextspp) {
> if (!TAILQ_EMPTY(>spi_swapdev))
> continue;
> @@ -543,7 +557,7 @@ swaplist_trim(void)
> * swapdrum_add: add a "swapdev"'s blocks into /dev/drum's area.
> *
> * => caller must hold swap_syscall_lock
> - * => uvm.swap_data_lock should be unlocked (we may sleep)
> + * => uvm_swap_data_lock should be unlocked
Re: fix active scan on iwm and iwx
On Sun, Jan 16, 2022 at 03:50:55PM +0100, Mark Kettenis wrote: > However, running this diff I had a problem after resuming my laptop > twice. After resume the interface didn't work and I found the > following in dmesg: > > iwm0: could not initialize hardware > > I tried to reset the interface by bringing it down and up again, which > crashed the machine. It must have been in ddb since typing "bo re" > made it reset. Unfortunately I don't have further information since I > was in X. Did you try reproducing this problem without the patch in place? It would be good to know whether this problem is being introduced by this patch. I don't believe this is likely, my bet would be that this is an existing problem. But it would be good to know for sure.
Re: AX210 wifi
On Sun, Jan 16, 2022 at 05:51:03PM +0300, Alex Beakes wrote: > FreeBSD has tested iwlwifi with Intel(R) Wi-Fi 6 AX210 160MHz, REV=0x420. > Wifi 6E, ty-a0-gf-a0-63.ucode. > > Is there a way of implementing this and making the wifi module work. > > https://wiki.freebsd.org/WiFi/Iwlwifi I will likely work on AX210 suport sometimebut this year. If anyone else has already done work on this and can provide working diffs against the OpenBSD drivers I would be happy to review them. Since FreeBSD has decided to no longer help with developing iwm or iwx drivers, but implement a completely different approach instead, there is no chance of cross-polination between the projects to make this any easier.
Re: fw_update(8) improve verbose output
On Wed, Jan 05, 2022 at 05:36:42PM -0800, Andrew Hewus Fresh wrote:
> After getting fw_update(8) into a state where it could get some testing,
> I found that the man page indicated that -v should indicate different
> levels of verbosity and I currently only had one.
>
> This was useful as I didn't really like the output anyway.
>
> Now one -v prints out an additional line when it's doing something to
> the firmware. Two add a progress bar and mentions "detecting". Three
> provide a bit more debugging mostly from ftp(1).
If you want multiple levels, I'd treat VERBOSE as an integer. Basically
use `((VERBOSE))' as "non-zero?", `((VERBOSE > 1))', etc.
I went through your patch (does not apply anymore after the other diffs
were committed) and did the mechanical conversion whilst introducing
your levels and messages.
`doas ./fw_update -n -v[[v]v]' shows different output for me, but I have
not tested it thoroughly.
> Also a couple extra small improvements, hiding errors from killing the
> ftp subprocess which could happen if it exits before we do, just
> using `firmware_devicename "$_d"` instead of `echo "${...}"`, and using
> a normal [=] comparison instead of [[=]] because we don't want pattern
> matching there.
I'd do that separately, the verbosity changes are big and noisy already.
Feel free to take over and commit any part of this diff, I just put it
here to demonstrate the shell bits.
Index: fw_update.sh
===
RCS file: /cvs/src/usr.sbin/fw_update/fw_update.sh,v
retrieving revision 1.30
diff -u -p -r1.30 fw_update.sh
--- fw_update.sh12 Jan 2022 02:21:15 - 1.30
+++ fw_update.sh16 Jan 2022 16:27:09 -
@@ -35,7 +35,7 @@ FWURL=http://firmware.openbsd.org/firmwa
FWPUB_KEY=${DESTDIR}/etc/signify/openbsd-${VERSION}-fw.pub
DRYRUN=false
-VERBOSE=false
+integer VERBOSE=0
DELETE=false
DOWNLOAD=true
INSTALL=true
@@ -69,18 +69,19 @@ tmpdir() {
fetch() {
local _src="${FWURL}/${1##*/}" _dst=$1 _user=_file _exit _error=''
+ local _flags=-VM
+
+ ((VERBOSE > 1)) && _flags=-vm
# The installer uses a limited doas(1) as a tiny su(1)
set -o monitor # make sure ftp gets its own process group
(
- flags=-VM
- "$VERBOSE" && flags=-vm
if [ -x /usr/bin/su ]; then
exec /usr/bin/su -s /bin/ksh "$_user" -c \
- "/usr/bin/ftp -N '${0##/}' -D 'Get/Verify' $flags -o-
'$_src'" > "$_dst"
+ "/usr/bin/ftp -N '${0##/}' -D 'Get/Verify' $_flags -o-
'$_src'" > "$_dst"
else
exec /usr/bin/doas -u "$_user" \
- /usr/bin/ftp -N "${0##/}" -D 'Get/Verify' $flags -o-
"$_src" > "$_dst"
+ /usr/bin/ftp -N "${0##/}" -D 'Get/Verify' $_flags -o-
"$_src" > "$_dst"
fi
) & FTPPID=$!
set +o monitor
@@ -216,9 +217,9 @@ detect_firmware() {
add_firmware () {
local _f="${1##*/}" _pkgname
FWPKGTMP="$( tmpdir "${DESTDIR}/var/db/pkg/.firmware" )"
- local flags=-VM
- "$VERBOSE" && flags=-vm
- ftp -N "${0##/}" -D "Install" "$flags" -o- "file:${1}" |
+ local _flags=-VM
+ ((VERBOSE > 1)) && _flags=-Vm
+ ftp -N "${0##/}" -D "Install" "$_flags" -o- "file:${1}" |
tar -s ",^\+,${FWPKGTMP}/+," \
-s ",^firmware,${DESTDIR}/etc/firmware," \
-C / -zxphf - "+*" "firmware/*"
@@ -249,7 +250,7 @@ delete_firmware() {
local _cwd _pkg="$1" _pkgdir="${DESTDIR}/var/db/pkg"
# TODO: Check hash for files before deleting
- "$VERBOSE" && echo "Uninstalling $_pkg"
+ ((VERBOSE > 2)) && echo "Uninstall $_pkg ..."
_cwd="${_pkgdir}/$_pkg"
if [ ! -e "$_cwd/+CONTENTS" ] ||
@@ -283,6 +284,8 @@ delete_firmware() {
rm -f "$_r"
fi
done
+
+ ((VERBOSE > 2)) && echo " done."
}
usage() {
@@ -300,7 +303,7 @@ do
F) OPT_F=true ;;
n) DRYRUN=true ;;
p) LOCALSRC="$OPTARG" ;;
- v) VERBOSE=true ;;
+ v) ((++VERBOSE)) ;;
:)
echo "${0##*/}: option requires an argument -- -$OPTARG" >&2
usage 2
@@ -354,6 +357,9 @@ set -sA devices -- "$@"
if "$DELETE"; then
[ "$OPT_F" ] && echo "Cannot use -F and -d" >&2 && usage 22
+ # Show the "Uninstall" message when just deleting not upgrading
+ ((VERBOSE > 1)) && VERBOSE=3
+
set -A installed
if [ "${devices[*]:-}" ]; then
"$ALL" && echo "Cannot use -a and devices/files" >&2 && usage 22
@@ -381,7 +387,7 @@ if "$DELETE"; then
if [ "${installed:-}" ]; then
for fw in "${installed[@]}"; do
if "$DRYRUN"; then
- echo "Delete $fw"
+ ((VERBOSE)) && echo "Delete $fw"
else
delete_firmware "$fw"
usr.bin/mg: fix -Wunused-but-set-variable warnings
usr.bin/mg: fix -Wunused-but-set-variable warnings
* strtonum() is only called to verify that a string is numerical, the
return value is unused.
* inlist is no longer used after the code was refactored.
OK?
M usr.bin/mg/interpreter.c
diff 6e5c342a53c05496c18849837c67b7dc05ce3792
5ce17aab12dafb0a17452c4e0e86b29d89d83d13
blob - 320b4089849fb28a0b1ee4ff0db2542da902d56e
blob + f3a4310dfe01138c77e624877eefa92d29feb039
--- usr.bin/mg/interpreter.c
+++ usr.bin/mg/interpreter.c
@@ -492,9 +492,8 @@ multiarg(char *cmdp, char *argbuf, int numparams)
if (!doregex(regs, argp)) {
const char *errstr;
- int iters;
- iters = strtonum(argp, 0, INT_MAX, );
+ strtonum(argp, 0, INT_MAX, );
if (errstr != NULL)
return (dobeep_msgs("Var not found:",
argp));
@@ -628,7 +627,7 @@ expandvals(char *cmdp, char *valp, char *bp)
char*argp, *endp, *p, *v, *s = " ";
char*regs;
int spc, cnt;
- int inlist, sizof, fin, inquote;
+ int sizof, fin, inquote;
/* now find the first argument */
p = skipwhite(valp);
@@ -637,7 +636,7 @@ expandvals(char *cmdp, char *valp, char *bp)
return (dobeep_msg("strlcpy error"));
argp = argbuf;
spc = 1; /* initially fake a space so we find first argument */
- inlist = fin = inquote = cnt = spc = 0;
+ fin = inquote = cnt = spc = 0;
for (p = argbuf; *p != '\0'; p++) {
if (*(p + 1) == '\0')
@@ -693,9 +692,8 @@ expandvals(char *cmdp, char *valp, char *bp)
continue;
} else {
const char *errstr;
- int iters;
- iters = strtonum(argp, 0, INT_MAX, );
+ strtonum(argp, 0, INT_MAX, );
if (errstr != NULL)
return (dobeep_msgs("Var not found:",
argp));
--
Christian "naddy" Weisgerber na...@mips.inka.de
Re: fix active scan on iwm and iwx
> Date: Thu, 13 Jan 2022 17:45:03 +0100
> From: Stefan Sperling
>
> At present active scans (which send probe requests, as opposed to
> just listening for beacons) are disabled on iwm 9k and iwx. This
> was done because firmware misbehaved after association.
>
> zxystd from the OpenIntelWireless project has debugged the issue
> and has sent me a patch against OpenBSD which fixes this problem.
> The patch is below, with some small tweaks by me which have already
> been reviewed by zxystd.
>
> It seems that firmware misbehaves if the driver sets the DTIM period
> to zero. This value is read from TIM information elements (IE) in beacons.
> Passive scans worked because we picked up the DTIM period from a beacon,
> while probe responses received during active scans lack the TIM IE, which
> resulted in a zero DTIM period being configured in firmware. We then never
> updated TIM information when a beacon was recieved, letting firmware run
> with a zero DTIM period until it eventually stopped working.
>
> I have tested this patch on iwm 8265 and iwx ax200.
> And jmc@ has been testing this on ax200 for some time.
> More tests are welcome, on any supported device.
This seems to be an improvement on iwm 9260. At least connecting to
the WiFi at my parents' worked better than during christmas.
However, running this diff I had a problem after resuming my laptop
twice. After resume the interface didn't work and I found the
following in dmesg:
iwm0: could not initialize hardware
I tried to reset the interface by bringing it down and up again, which
crashed the machine. It must have been in ddb since typing "bo re"
made it reset. Unfortunately I don't have further information since I
was in X.
> diff refs/heads/remove-find-node-for-beacon refs/heads/iwm-iwx-active-scan
> blob - 607a45b71f7ea71773809136cadf122c72375558
> blob + 937f2cc28f6c85502031e4c9efa0a02c75fd1a6d
> --- sys/dev/pci/if_iwm.c
> +++ sys/dev/pci/if_iwm.c
> @@ -340,6 +340,7 @@ void iwm_updateprot(struct ieee80211com *);
> void iwm_updateslot(struct ieee80211com *);
> void iwm_updateedca(struct ieee80211com *);
> void iwm_updatechan(struct ieee80211com *);
> +void iwm_updatedtim(struct ieee80211com *);
> void iwm_init_reorder_buffer(struct iwm_reorder_buffer *, uint16_t,
> uint16_t);
> void iwm_clear_reorder_buffer(struct iwm_softc *, struct iwm_rxba_data *);
> @@ -3374,6 +3375,8 @@ iwm_mac_ctxt_task(void *arg)
> err = iwm_mac_ctxt_cmd(sc, in, IWM_FW_CTXT_ACTION_MODIFY, 1);
> if (err)
> printf("%s: failed to update MAC\n", DEVNAME(sc));
> +
> + iwm_unprotect_session(sc, in);
>
> refcnt_rele_wake(>task_refs);
> splx(s);
> @@ -3454,6 +3457,16 @@ iwm_updatechan(struct ieee80211com *ic)
> iwm_add_task(sc, systq, >phy_ctxt_task);
> }
>
> +void
> +iwm_updatedtim(struct ieee80211com *ic)
> +{
> + struct iwm_softc *sc = ic->ic_softc;
> +
> + if (ic->ic_state == IEEE80211_S_RUN &&
> + !task_pending(>newstate_task))
> + iwm_add_task(sc, systq, >mac_ctxt_task);
> +}
> +
> int
> iwm_sta_tx_agg(struct iwm_softc *sc, struct ieee80211_node *ni, uint8_t tid,
> uint16_t ssn, uint16_t winsize, int start)
> @@ -7275,12 +7288,7 @@ iwm_lmac_scan_fill_channels(struct iwm_softc *sc,
> chan->iter_count = htole16(1);
> chan->iter_interval = 0;
> chan->flags = htole32(IWM_UNIFIED_SCAN_CHANNEL_PARTIAL);
> - /*
> - * Firmware may become unresponsive when asked to send
> - * a directed probe request on a passive channel.
> - */
> - if (n_ssids != 0 && !bgscan &&
> - (c->ic_flags & IEEE80211_CHAN_PASSIVE) == 0)
> + if (n_ssids != 0 && !bgscan)
> chan->flags |= htole32(1 << 1); /* select SSID 0 */
> chan++;
> nchan++;
> @@ -7307,12 +7315,7 @@ iwm_umac_scan_fill_channels(struct iwm_softc *sc,
> chan->channel_num = ieee80211_mhz2ieee(c->ic_freq, 0);
> chan->iter_count = 1;
> chan->iter_interval = htole16(0);
> - /*
> - * Firmware may become unresponsive when asked to send
> - * a directed probe request on a passive channel.
> - */
> - if (n_ssids != 0 && !bgscan &&
> - (c->ic_flags & IEEE80211_CHAN_PASSIVE) == 0)
> + if (n_ssids != 0 && !bgscan)
> chan->flags = htole32(1 << 0); /* select SSID 0 */
> chan++;
> nchan++;
> @@ -7782,13 +7785,7 @@ iwm_umac_scan(struct iwm_softc *sc, int bgscan)
> IWM_UMAC_SCAN_GEN_FLAGS2_ALLOW_CHNL_REORDER;
> }
>
> - /*
> - * Check if we're doing an active directed scan.
> - * 9k devices may randomly lock up (no interrupts) after association
> - * following active scans. Use passive scan only for now on 9k.
> - */
>
Re: unlock mmap(2) for anonymous mappings
On 14/01/22(Fri) 23:01, Mark Kettenis wrote: > > Date: Tue, 11 Jan 2022 23:13:20 + > > From: Klemens Nanni > > > > On Tue, Jan 11, 2022 at 09:54:44AM -0700, Theo de Raadt wrote: > > > > Now this is clearly a "slow" path. I don't think there is any reason > > > > not to put all the code in that if (uvw_wxabort) block under the > > > > kernel lock. For now I think making access to ps_wxcounter atomic is > > > > just too fine grained. > > > > > > Right. Lock the whole block. > > > > Thanks everyone, here's the combined diff for that. > > I think mpi@ should be involved in the actual unlocking of mmap(2), > munmap(2) and mprotect(2). But the changes to uvm_mmap.c are ok > kettenis@ and can be committed now. It isn't clear to me what changed since the last time this has been tried. Why is it safe now? What are the locking assumptions? IMHO this approach of let's try if it works now and revert if it isn't doesn't help us make progress. I'd be more confident seeing diffs that assert for the right lock in the functions called by uvm_mapanon() and documentation about which lock is protecting which field of the data structures. NetBSD has done much of this and the code bases do not diverge much so it can be useful to look there as well.
