Re: Small diff for 73.html
Oops! Thank you for the explanation. On Thu, Apr 27, 2023 at 3:37 PM Jonathan Gray wrote: > > On Thu, Apr 27, 2023 at 03:24:06PM -0900, Andras Farkas wrote: > > Small diff for 73.html: fixes incorrect link to amdgpu man page. > > amdgpu(4) is the xorg driver, it is intentionally drm.4 > > > > > SHA512 (73diff) = > > b3a2afb78744cddfaa148d1bfcd825af376dc4e168e12116e49f1f16614cdc108f001b8fd592eb884e7fcf6e342da527fb437dba991fc583fbc85ef9f98b85e3 > > > > Index: 73.html > > === > > RCS file: /cvs/www/73.html,v > > retrieving revision 1.61 > > diff -u -p -r1.61 73.html > > --- 73.html 20 Apr 2023 14:17:41 - 1.61 > > +++ 73.html 28 Apr 2023 00:21:04 - > > @@ -173,7 +173,7 @@ to 7.3. > > > >Updated https://man.openbsd.org/drm.4;>drm(4) > >to Linux 6.1.15 > > - https://man.openbsd.org/drm.4;>amdgpu(4): Added > > + https://man.openbsd.org/amdgpu.4;>amdgpu(4): Added > >support for Ryzen 7000 "Raphael", Ryzen 7020 series "Mendocino", > >Ryzen 7045 series "Dragon Range", > >Radeon RX 7900 XT/XTX "Navi 31", > > > > Also attached to avoid potential mangling. > >
Re: Small diff for 73.html
On Thu, Apr 27, 2023 at 03:24:06PM -0900, Andras Farkas wrote: > Small diff for 73.html: fixes incorrect link to amdgpu man page. amdgpu(4) is the xorg driver, it is intentionally drm.4 > > SHA512 (73diff) = > b3a2afb78744cddfaa148d1bfcd825af376dc4e168e12116e49f1f16614cdc108f001b8fd592eb884e7fcf6e342da527fb437dba991fc583fbc85ef9f98b85e3 > > Index: 73.html > === > RCS file: /cvs/www/73.html,v > retrieving revision 1.61 > diff -u -p -r1.61 73.html > --- 73.html 20 Apr 2023 14:17:41 - 1.61 > +++ 73.html 28 Apr 2023 00:21:04 - > @@ -173,7 +173,7 @@ to 7.3. > >Updated https://man.openbsd.org/drm.4;>drm(4) >to Linux 6.1.15 > - https://man.openbsd.org/drm.4;>amdgpu(4): Added > + https://man.openbsd.org/amdgpu.4;>amdgpu(4): Added >support for Ryzen 7000 "Raphael", Ryzen 7020 series "Mendocino", >Ryzen 7045 series "Dragon Range", >Radeon RX 7900 XT/XTX "Navi 31", > > Also attached to avoid potential mangling.
Small diff for 73.html
Small diff for 73.html: fixes incorrect link to amdgpu man page. SHA512 (73diff) = b3a2afb78744cddfaa148d1bfcd825af376dc4e168e12116e49f1f16614cdc108f001b8fd592eb884e7fcf6e342da527fb437dba991fc583fbc85ef9f98b85e3 Index: 73.html === RCS file: /cvs/www/73.html,v retrieving revision 1.61 diff -u -p -r1.61 73.html --- 73.html 20 Apr 2023 14:17:41 - 1.61 +++ 73.html 28 Apr 2023 00:21:04 - @@ -173,7 +173,7 @@ to 7.3. Updated https://man.openbsd.org/drm.4;>drm(4) to Linux 6.1.15 - https://man.openbsd.org/drm.4;>amdgpu(4): Added + https://man.openbsd.org/amdgpu.4;>amdgpu(4): Added support for Ryzen 7000 "Raphael", Ryzen 7020 series "Mendocino", Ryzen 7045 series "Dragon Range", Radeon RX 7900 XT/XTX "Navi 31", Also attached to avoid potential mangling. 73diff Description: Binary data
Re: OpenBSD 7.2 on Oracle Cloud
On Tue, Apr 25, 2023 at 10:53 PM Aaron Mason wrote:
>
> On Mon, Apr 24, 2023 at 3:47 PM Aaron Mason wrote:
> >
> > On Fri, Apr 21, 2023 at 2:50 PM Aaron Mason
> > wrote:
> > >
> > > On Fri, Apr 21, 2023 at 1:39 PM Aaron Mason
> > > wrote:
> > > >
> > > > On Fri, Apr 7, 2023 at 3:25 AM Antun Matanović
> > > > wrote:
> > > > >
> > > > > On Thu, 6 Apr 2023 at 12:55, Fabio Martins wrote:
> > > > > >
> > > > > > Try to add an entry in grub like in this article:
> > > > > >
> > > > > > https://raby.sh/installing-openbsd-on-ovhs-vps-2016-kvm-machines.html
> > > > >
> > > > > I have tried that, but it did not resolve the issue. Sorry I forgot to
> > > > > mention it originally.
> > > > >
> > > > > On Thu, 6 Apr 2023 at 14:24, Janne Johansson
> > > > > wrote:
> > > > > >
> > > > > > That is very much not the same issue. The arm64 instances on Oracle
> > > > > > finds the correct kernel and boots it, it just crashes at or after
> > > > > > the
> > > > > > scsi attachment.
> > > > >
> > > > > This has been my experience as well, except on the amd64 instance,
> > > > > haven't tried arm64.
> > > > >
> > > >
> > > > Yeah I'm getting the same thing. Trying a build in QEMU and
> > > > transferring in to see if that helps. Will report back.
> > > >
> > >
> > > Ok, good news, it still crashes at the same spot, but this time I've
> > > got more data. Copying in tech@ - if I've forgotten anything let me
> > > know and I'll fire up a fresh instance.
> > >
> > > [REDACTED]
> > > vioscsi_req_done(e,80024a00,fd803f81c338,e,80024a00,800
> > > d3228) at vioscsi_req_done+0x26
> > > [REDACTED]
> >
> > Ok, so based on the trace I got, I was able to trace the stop itself
> > back to line 299 of vioscsi.c (thank. you. random relink. And
> > anonymous CVS):
> >
> >293 vioscsi_req_done(struct vioscsi_softc *sc, struct virtio_softc *vsc,
> >294 struct vioscsi_req *vr)
> >295 {
> >296 struct scsi_xfer *xs = vr->vr_xs;
> >297 DPRINTF("vioscsi_req_done: enter vr: %p xs: %p\n", vr, xs);
> >298
> > -->299 int isread = !!(xs->flags & SCSI_DATA_IN);
> >300 bus_dmamap_sync(vsc->sc_dmat, vr->vr_control,
> >301 offsetof(struct vioscsi_req, vr_req),
> >302 sizeof(struct virtio_scsi_req_hdr),
> >303 BUS_DMASYNC_POSTWRITE);
> >
> > Maybe if I follow the rabbit hole enough, I might find out what's
> > going wrong between the driver and OCI. I've got a day off tomorrow
> > (yay for war I guess), I'll give it a bash and see where we end up.
> >
> > --
> > Aaron Mason - Programmer, open source addict
> > I've taken my software vows - for beta or for worse
>
> I enabled debugging on the vioscsi driver, rebuilt the RAMDISK kernel
> with those drivers enabled, and got this:
>
> vioscsi0 at virtio1: qsize 128
> scsibus0 at vioscsi0: 255 targets
> vioscsi_req_get: 0xfd803f80d338
> vioscsi_scsi_cmd: enter
> vioscsi_scsi_cmd: polling...
> vioscsi_scsi_cmd: polling timeout
> vioscsi_scsi_cmd: done (timeout=0)
> vioscsi_scsi_cmd: enter
> vioscsi_scsi_cmd: polling...
> vioscsi_vq_done: enter
> vioscsi_vq_done: slot=127
> vioscsi_req_done: enter vr: 0xfd803f80d338 xs: 0xfd803f8a5e58
> vioscsi_req_done: done 0, 2, 0
> vioscsi_vq_done: slot=127
> vioscsi_req_done: enter vr: 0xfd803f80d338 xs: 0x0
> uvm_fault(0x813ec2e0, 0x8, 0, 1) -> e
> fatal page fault in supervisor mode
> trap type 6 code 0 rip 810e6190 cs 8 rflags 10286 cr2 8 cpl e
> rsp 81606670
> gsbase 0x813dfff0 kgsbase 0x0
> panic: trap type 6, code=0, pc=810e6190
>
> That "xs: 0x0" bit feels like a clue. It should be trivial to pick up
> and handle, but what would be the correct way to handle that?
>
> If I have it return if "xs" is found to be NULL, it continues - the
> debugging suggests it goes through each possible target before
> finishing up. I don't know if that's correct, but it seems to continue
> booting after that even if my example didn't detect the drive with the
> kernel I built (I used the RAMDISK kernel and it was pretty stripped
> down).
>
> I'm about to attempt a -STABLE build (I've got 7.3 installed and thus
> can't yet build a snapshot, but I will do that if this test succeeds)
> - here's the patch that hopefully fixes the problem. (and hopefully
> gmail doesn't clobber the tabs)
>
> Index: sys/dev/pv/vioscsi.c
> ===
> RCS file: /cvs/src/sys/dev/pv/vioscsi.c,v
> retrieving revision 1.30
> diff -u -p -u -p -r1.30 vioscsi.c
> --- sys/dev/pv/vioscsi.c 16 Apr 2022 19:19:59 - 1.30
> +++ sys/dev/pv/vioscsi.c 25 Apr 2023 12:51:16 -
> @@ -296,6 +296,7 @@ vioscsi_req_done(struct vioscsi_softc *s
> struct scsi_xfer *xs = vr->vr_xs;
> DPRINTF("vioscsi_req_done: enter vr: %p xs: %p\n", vr, xs);
>
> + if (xs == NULL) return;
> int isread = !!(xs->flags & SCSI_DATA_IN);
> bus_dmamap_sync(vsc->sc_dmat,
Re: fill_file(): use solock_shared() to protect socket data
On Thu, Apr 27, 2023 at 02:54:38PM +0200, Claudio Jeker wrote:
> On Thu, Apr 27, 2023 at 01:55:33PM +0300, Vitaliy Makkoveev wrote:
> > Now only direct netlock used for inet sockets protection. The unlocked
> > access to all other sockets is safe, but we could lost consistency for a
> > little. Since the solock() used for sockets protection, make locking
> > path common and use it. Make it shared because this is read-only access
> > to sockets data. For same reason use shared netlock while performing
> > inpcb tables foreach walkthrough.
>
> This is still wrong. fill_file() is not allowed to sleep. So you can not
> use a rwlock in here. fill_file is called inside a allprocess
> LIST_FOREACH loop and sleeping there will blow up.
>
> Maybe it is enough to grab a shared NETLOCK around the LIST_FOREACH() to
> ensure that we don't sleep inside.
>
> Please fix this properly. Right now running fstat is like playing russian
> roulette.
>
This time fill_file() already has sleep point introduced by direct
netlock invocation. So, this diff doesn't make it worse.
The netlock around allprocess foreach loops is not the properly fix,
because we left non inet sockets unlocked.
However, these allprocess foreach loops are not the only place with
similar problem. Some times ago I did the diff to introduce the
`allprocess_lock' rwlock for `allprocess' list protection.
Unfortunately, this diff was abandoned after some discussions about
proc_stop_sweep() software interrupt handler. I reworked it to kernel
thread to use this new rwlock within. Since proc_stop_sweep() only
schedules parents of stopped processes to run I assume this extra delay
as non significant.
This is the last iteration of `allprocess' diff. I propose to commit it
first to fix sysctl_file() in the right way.
Index: sys/arch/sh/sh/pmap.c
===
RCS file: /cvs/src/sys/arch/sh/sh/pmap.c,v
retrieving revision 1.30
diff -u -p -r1.30 pmap.c
--- sys/arch/sh/sh/pmap.c 1 Jan 2023 19:49:17 - 1.30
+++ sys/arch/sh/sh/pmap.c 9 Jan 2023 09:06:42 -
@@ -1071,6 +1071,11 @@ __pmap_asid_alloc(void)
* so it's far from LRU but rather almost pessimal once you have
* too many processes.
*/
+ /*
+* XXX Unlocked access to `allprocess' list is safe. This is
+* uniprocessor machine, we don't modify the list and we have
+* no context switch within.
+*/
LIST_FOREACH(pr, , ps_list) {
pmap_t pmap = pr->ps_vmspace->vm_map.pmap;
Index: sys/kern/kern_exit.c
===
RCS file: /cvs/src/sys/kern/kern_exit.c,v
retrieving revision 1.210
diff -u -p -r1.210 kern_exit.c
--- sys/kern/kern_exit.c29 Dec 2022 01:36:36 - 1.210
+++ sys/kern/kern_exit.c9 Jan 2023 09:06:42 -
@@ -222,6 +222,8 @@ exit1(struct proc *p, int xexit, int xsi
p->p_fd = NULL; /* zap the thread's copy */
+ rw_enter_write(_lock);
+
/*
* Remove proc from pidhash chain and allproc so looking
* it up won't work. We will put the proc on the
@@ -294,6 +296,8 @@ exit1(struct proc *p, int xexit, int xsi
process_clear_orphan(qr);
}
}
+
+ rw_exit_write(_lock);
/* add thread's accumulated rusage into the process's total */
ruadd(rup, >p_ru);
Index: sys/kern/kern_fork.c
===
RCS file: /cvs/src/sys/kern/kern_fork.c,v
retrieving revision 1.245
diff -u -p -r1.245 kern_fork.c
--- sys/kern/kern_fork.c7 Jan 2023 05:24:58 - 1.245
+++ sys/kern/kern_fork.c9 Jan 2023 09:06:42 -
@@ -62,6 +62,11 @@
#include
#include
+/*
+ * Locks used to protect struct members in this file:
+ * P allprocess_lock
+ */
+
intnprocesses = 1; /* process 0 */
intnthreads = 1; /* proc 0 */
struct forkstat forkstat;
@@ -372,6 +377,8 @@ fork1(struct proc *curp, int flags, void
return (ENOMEM);
}
+ rw_enter_write(_lock);
+
/*
* From now on, we're committed to the fork and cannot fail.
*/
@@ -468,19 +475,28 @@ fork1(struct proc *curp, int flags, void
KNOTE(>ps_klist, NOTE_FORK | pr->ps_pid);
/*
-* Update stats now that we know the fork was successful.
+* Return child pid to parent process
*/
- uvmexp.forks++;
- if (flags & FORK_PPWAIT)
- uvmexp.forks_ppwait++;
- if (flags & FORK_SHAREVM)
- uvmexp.forks_sharevm++;
+ if (retval != NULL)
+ *retval = pr->ps_pid;
/*
* Pass a pointer to the new process to the caller.
+* XXX but we don't return `rnewprocp' to sys_fork()
+* or sys_vfork().
*/
if (rnewprocp != NULL)
*rnewprocp = p;
Re: arm64 install.md: fix softraid crypto installation on Mac
> Op 27-04-2023 13:52 CEST schreef Klemens Nanni :
>
>
> Another approach would be to make installboot(8) -p to retain existing
> EFI Sys partitions instead of always recreating them.
I don't think it is the job of installboot(8) to device whether the ESP
should be retained or not. If you want to retain the ESP, don't run
installboot -p!
Since that is what Caspar's diff does, I ave no objection to it.
However, I do think we shouldn't be creating an EFI system partition on the
softraid
volume in the first place; only on the underlying physical disk(s). But I
appreciate
that is a different diff.
> This way, it was nothing to do with softraid, but installing on machines
> like Apple arm64 depends on existing non-OpenBSD partitions and files on
> them.
And we will need this behaviour on other systems as well. The Apple systems
are not unqie.
> We hacked 'installboot -p' to do the 'try mount, fsck, retry mount, newfs'
> dance it does in its normal mode (no '-p'), but that its own design choice
> spilling over to all supported installboot architectures.
>
> For this apple case, it'd be nice to have a sort of idempotent
> 'installboot -p', but at this point it seems overkill and I prefer to fix
> it like caspar did down in the arm64 apple specific installer bits.
>
> This is nicely contained and has no behaviour changes for machines without
> APFS ISC.
>
> On Thu, Apr 27, 2023 at 12:31:09PM +0100, Caspar Schutijser wrote:
> > I was trying to install OpenBSD on the arm64 MacBook Air with
> > softraid crypto (that's the "Encrypt the root disk?" question in the
> > installer). Right now that does not work out of the box.
> >
> > In a regular install, md_prep_fdisk is careful and leaves the EFI Sys
> > partition alone with the "if disk_has $_disk gpt apfsisc" check.
> >
> > However, in the "encrypt the root disk" case, the installer goes through
> > md_prep_fdisk twice. The second time, it's called on the softraid crypto
> > disk, which does not have the special EFI Sys partition. So there we
> > don't hit the special "if disk_has $_disk gpt apfsisc" case. Instead,
> > we go to a more regular case where we end up running "installboot -p".
> >
> > Because it's a softraid disk, installboot also looks at the "chunks"
> > that the softraid volume resides on. I.e., it also looks at sd0 and
> > there "installboot -p" will newfs the EFI Sys partition, which is
> > something we don't want on the Mac.
> >
> > This diff addresses this problem. The first time we go through
> > md_prep_fdisk, we keep track of whether we're in this special Mac
> > case by setting KEEP_EFI_SYS. Then when we go through md_prep_fdisk
> > for the second time to prepare the softraid disk, we can check this
> > variable to see if we should avoid running "installboot -p".
> >
> > Debugged with help from and came up with a fix with kn@, thanks!
> >
> > Comments or OKs?
> >
> > Caspar
> >
> > ---
> >
> > arm64 install.md: fix softraid crypto installation on Mac
> >
> > Make sure we don't newfs the EFI Sys partition on systems that have an
> > "apfsisc" partition in the case we're installing with softraid crypto.
> >
> > Debugged with help from and came up with a fix with kn@
> >
> >
> > Index: install.md
> > ===
> > RCS file: /cvs/src/distrib/arm64/ramdisk/install.md,v
> > retrieving revision 1.46
> > diff -u -p -r1.46 install.md
> > --- install.md 27 Apr 2023 10:03:49 - 1.46
> > +++ install.md 27 Apr 2023 11:26:56 -
> > @@ -36,6 +36,7 @@ MDBOOTSR=y
> > NCPU=$(sysctl -n hw.ncpufound)
> > COMPATIBLE=$(sysctl -n machdep.compatible)
> > MOUNT_ARGS_msdos="-o-l"
> > +KEEP_EFI_SYS=false
> >
> > md_installboot() {
> > local _disk=$1 _chunks _bootdisk _mdec _plat
> > @@ -109,6 +110,11 @@ md_prep_fdisk() {
> > [wW]*)
> > echo -n "Creating a ${bootfstype} partition and an
> > OpenBSD partition for rest of $_disk..."
> > if disk_has $_disk gpt apfsisc; then
> > + # On Apple hardware, the existing EFI Sys
> > + # partition contains boot firmware and MUST NOT
> > + # be recreated.
> > + KEEP_EFI_SYS=true
> > +
> > # Is this a boot disk?
> > if [[ $_disk == @($ROOTDISK|$CRYPTOCHUNK) ]];
> > then
> > fdisk -Ay -b "${bootsectorsize}"
> > ${_disk} >/dev/null
> > @@ -119,13 +125,20 @@ md_prep_fdisk() {
> > # Is this a boot disk?
> > if [[ $_disk == @($ROOTDISK|$CRYPTOCHUNK) ]];
> > then
> > fdisk -gy -b "${bootsectorsize}"
> > ${_disk} >/dev/null
> > - installboot -p $_disk
> > +
> > + # With root on softraid,
> > +
Re: fill_file(): use solock_shared() to protect socket data
On Thu, Apr 27, 2023 at 02:54:38PM +0200, Claudio Jeker wrote:
> On Thu, Apr 27, 2023 at 01:55:33PM +0300, Vitaliy Makkoveev wrote:
> > Now only direct netlock used for inet sockets protection. The unlocked
> > access to all other sockets is safe, but we could lost consistency for a
> > little. Since the solock() used for sockets protection, make locking
> > path common and use it. Make it shared because this is read-only access
> > to sockets data. For same reason use shared netlock while performing
> > inpcb tables foreach walkthrough.
>
> This is still wrong. fill_file() is not allowed to sleep. So you can not
> use a rwlock in here. fill_file is called inside a allprocess
> LIST_FOREACH loop and sleeping there will blow up.
Yes.
> Maybe it is enough to grab a shared NETLOCK around the LIST_FOREACH() to
> ensure that we don't sleep inside.
I have tired to fix this for years. It is not easy. Taking the
net lock to early results in a lock ordering problem. Down in
fill_file() fdplock() is called:
#define fdplock(fdp) NET_ASSERT_UNLOCKED(); rw_enter_write(&(fdp)->fd_lock);
This guarantees lock ordering fdlock before netlock. You probably
run into NFS problems if you want to relax that.
> Please fix this properly. Right now running fstat is like playing russian
> roulette.
Shared netlock is possible russian roulette with less bullets
compared to exclusive netlock. For network sockets pcb mutex and
pru_lock the situation gets better. For non-network sockets this
diff does rw_enter_write(>so_lock). Do we need that? It means
more sleeping points and more opportunity to crash.
bluhm
> > Index: sys/kern/kern_sysctl.c
> > ===
> > RCS file: /cvs/src/sys/kern/kern_sysctl.c,v
> > retrieving revision 1.411
> > diff -u -p -r1.411 kern_sysctl.c
> > --- sys/kern/kern_sysctl.c 22 Jan 2023 12:05:44 - 1.411
> > +++ sys/kern/kern_sysctl.c 27 Apr 2023 10:18:01 -
> > @@ -1173,14 +1173,11 @@ fill_file(struct kinfo_file *kf, struct
> >
> > if (so == NULL) {
> > so = (struct socket *)fp->f_data;
> > + solock_shared(so);
> > + locked = 1;
> > + } else {
> > /* if so is passed as parameter it is already locked */
> > - switch (so->so_proto->pr_domain->dom_family) {
> > - case AF_INET:
> > - case AF_INET6:
> > - NET_LOCK();
> > - locked = 1;
> > - break;
> > - }
> > + soassertlocked(so);
> > }
> >
> > kf->so_type = so->so_type;
> > @@ -1203,14 +1200,13 @@ fill_file(struct kinfo_file *kf, struct
> > kf->so_splicelen = -1;
> > if (so->so_pcb == NULL) {
> > if (locked)
> > - NET_UNLOCK();
> > + sounlock_shared(so);
> > break;
> > }
> > switch (kf->so_family) {
> > case AF_INET: {
> > struct inpcb *inpcb = so->so_pcb;
> >
> > - NET_ASSERT_LOCKED();
> > if (show_pointers)
> > kf->inp_ppcb = PTRTOINT64(inpcb->inp_ppcb);
> > kf->inp_lport = inpcb->inp_lport;
> > @@ -1232,7 +1228,6 @@ fill_file(struct kinfo_file *kf, struct
> > case AF_INET6: {
> > struct inpcb *inpcb = so->so_pcb;
> >
> > - NET_ASSERT_LOCKED();
> > if (show_pointers)
> > kf->inp_ppcb = PTRTOINT64(inpcb->inp_ppcb);
> > kf->inp_lport = inpcb->inp_lport;
> > @@ -1279,7 +1274,7 @@ fill_file(struct kinfo_file *kf, struct
> > }
> > }
> > if (locked)
> > - NET_UNLOCK();
> > + sounlock_shared(so);
> > break;
> > }
> >
> > @@ -1375,7 +1370,7 @@ sysctl_file(int *name, u_int namelen, ch
> > if (arg == DTYPE_SOCKET) {
> > struct inpcb *inp;
> >
> > - NET_LOCK();
> > + NET_LOCK_SHARED();
> > mtx_enter(_mtx);
> > TAILQ_FOREACH(inp, _queue, inp_queue)
> > FILLSO(inp->inp_socket);
> > @@ -1395,7 +1390,7 @@ sysctl_file(int *name, u_int namelen, ch
> > FILLSO(inp->inp_socket);
> > mtx_leave(_mtx);
> > #endif
> > - NET_UNLOCK();
> > + NET_UNLOCK_SHARED();
> > }
> > fp = NULL;
> > while ((fp = fd_iterfile(fp, p)) != NULL) {
> >
>
> --
> :wq Claudio
Re: Remove kernel lock from rtfree(9)
On Thu, Apr 27, 2023 at 03:47:40PM +0300, Vitaliy Makkoveev wrote:
>
>
> > On 27 Apr 2023, at 15:40, Alexander Bluhm wrote:
> >
> > On Thu, Apr 27, 2023 at 03:22:10PM +0300, Vitaliy Makkoveev wrote:
> >>> On 27 Apr 2023, at 15:16, Alexander Bluhm wrote:
> >>>
> >>> On Wed, Apr 26, 2023 at 11:17:37PM +0300, Vitaliy Makkoveev wrote:
> Route timers and route labels protected by corresponding mutexes. `ifa'
> uses references counting for protection. No protection required for `rt'
> passed to rt_mpls_clear() because only current thread owns it.
>
> ok?
> >>>
> >>> I have tested your diff and it works for me. But I don't have a
> >>> MPLS setup. And there is a rt_mpls_clear(rt) which your diff does
> >>> not show.
> >>>
> >>> if (rt->rt_llinfo != NULL)
> >>> free(rt->rt_llinfo);
> >>> rt->rt_llinfo = NULL;
> >>>
> >>> and rt->rt_flags &= ~RTF_MPLS are not mpsafe.
> >>>
> >>> What about this? Compile tested only due to lacking MPLS setup.
> >>
> >> This is not required. We hold the last reference of this dying `rt???.
> >
> > You are right. OK bluhm@ for rtfree unlocking.
> >
> > But I am still not convinced with rt_mpls_clear().
> >
> > What about htis call path? soreceive -> pru_send -> route_send ->
> > route_output -> rtm_output -> rt_mpls_clear
> >
> > Since solock() takes rw_enter_write(>so_lock) for route sockets,
> > we don't have the kernel lock anymore. I would feel safer with
> > this kenrel lock hammer for mpls. Of course rt_mpls_set() would
> > need something simmilar.
>
> This path is netlock protected.
Ah yes. It is the exclusive netlock, so it is sufficient.
> 911 rtm_output(struct rt_msghdr *rtm, struct rtentry **prt,
> 912 struct rt_addrinfo *info, uint8_t prio, unsigned int tableid)
> 913 {
> /* skip */
> 1132 #ifdef MPLS
> 1133 if (rtm->rtm_flags & RTF_MPLS) {
> 1134 NET_LOCK();
> 1135 error = rt_mpls_set(rt,
> 1136 info->rti_info[RTAX_SRC], info->rti_mpls);
> 1137 NET_UNLOCK();
> 1138 if (error)
> 1139 break;
> 1140 } else if (newgate || (rtm->rtm_fmask & RTF_MPLS)) {
> 1141 NET_LOCK();
> 1142 /* if gateway changed remove MPLS information */
> 1143 rt_mpls_clear(rt);
> 1144 NET_UNLOCK();
> 1145 }
> 1146 #endif
>
Re: fill_file(): use solock_shared() to protect socket data
On Thu, Apr 27, 2023 at 01:55:33PM +0300, Vitaliy Makkoveev wrote:
> Now only direct netlock used for inet sockets protection. The unlocked
> access to all other sockets is safe, but we could lost consistency for a
> little. Since the solock() used for sockets protection, make locking
> path common and use it. Make it shared because this is read-only access
> to sockets data. For same reason use shared netlock while performing
> inpcb tables foreach walkthrough.
This is still wrong. fill_file() is not allowed to sleep. So you can not
use a rwlock in here. fill_file is called inside a allprocess
LIST_FOREACH loop and sleeping there will blow up.
Maybe it is enough to grab a shared NETLOCK around the LIST_FOREACH() to
ensure that we don't sleep inside.
Please fix this properly. Right now running fstat is like playing russian
roulette.
> Index: sys/kern/kern_sysctl.c
> ===
> RCS file: /cvs/src/sys/kern/kern_sysctl.c,v
> retrieving revision 1.411
> diff -u -p -r1.411 kern_sysctl.c
> --- sys/kern/kern_sysctl.c22 Jan 2023 12:05:44 - 1.411
> +++ sys/kern/kern_sysctl.c27 Apr 2023 10:18:01 -
> @@ -1173,14 +1173,11 @@ fill_file(struct kinfo_file *kf, struct
>
> if (so == NULL) {
> so = (struct socket *)fp->f_data;
> + solock_shared(so);
> + locked = 1;
> + } else {
> /* if so is passed as parameter it is already locked */
> - switch (so->so_proto->pr_domain->dom_family) {
> - case AF_INET:
> - case AF_INET6:
> - NET_LOCK();
> - locked = 1;
> - break;
> - }
> + soassertlocked(so);
> }
>
> kf->so_type = so->so_type;
> @@ -1203,14 +1200,13 @@ fill_file(struct kinfo_file *kf, struct
> kf->so_splicelen = -1;
> if (so->so_pcb == NULL) {
> if (locked)
> - NET_UNLOCK();
> + sounlock_shared(so);
> break;
> }
> switch (kf->so_family) {
> case AF_INET: {
> struct inpcb *inpcb = so->so_pcb;
>
> - NET_ASSERT_LOCKED();
> if (show_pointers)
> kf->inp_ppcb = PTRTOINT64(inpcb->inp_ppcb);
> kf->inp_lport = inpcb->inp_lport;
> @@ -1232,7 +1228,6 @@ fill_file(struct kinfo_file *kf, struct
> case AF_INET6: {
> struct inpcb *inpcb = so->so_pcb;
>
> - NET_ASSERT_LOCKED();
> if (show_pointers)
> kf->inp_ppcb = PTRTOINT64(inpcb->inp_ppcb);
> kf->inp_lport = inpcb->inp_lport;
> @@ -1279,7 +1274,7 @@ fill_file(struct kinfo_file *kf, struct
> }
> }
> if (locked)
> - NET_UNLOCK();
> + sounlock_shared(so);
> break;
> }
>
> @@ -1375,7 +1370,7 @@ sysctl_file(int *name, u_int namelen, ch
> if (arg == DTYPE_SOCKET) {
> struct inpcb *inp;
>
> - NET_LOCK();
> + NET_LOCK_SHARED();
> mtx_enter(_mtx);
> TAILQ_FOREACH(inp, _queue, inp_queue)
> FILLSO(inp->inp_socket);
> @@ -1395,7 +1390,7 @@ sysctl_file(int *name, u_int namelen, ch
> FILLSO(inp->inp_socket);
> mtx_leave(_mtx);
> #endif
> - NET_UNLOCK();
> + NET_UNLOCK_SHARED();
> }
> fp = NULL;
> while ((fp = fd_iterfile(fp, p)) != NULL) {
>
--
:wq Claudio
Re: Remove kernel lock from rtfree(9)
> On 27 Apr 2023, at 15:40, Alexander Bluhm wrote:
>
> On Thu, Apr 27, 2023 at 03:22:10PM +0300, Vitaliy Makkoveev wrote:
>>> On 27 Apr 2023, at 15:16, Alexander Bluhm wrote:
>>>
>>> On Wed, Apr 26, 2023 at 11:17:37PM +0300, Vitaliy Makkoveev wrote:
Route timers and route labels protected by corresponding mutexes. `ifa'
uses references counting for protection. No protection required for `rt'
passed to rt_mpls_clear() because only current thread owns it.
ok?
>>>
>>> I have tested your diff and it works for me. But I don't have a
>>> MPLS setup. And there is a rt_mpls_clear(rt) which your diff does
>>> not show.
>>>
>>> if (rt->rt_llinfo != NULL)
>>> free(rt->rt_llinfo);
>>> rt->rt_llinfo = NULL;
>>>
>>> and rt->rt_flags &= ~RTF_MPLS are not mpsafe.
>>>
>>> What about this? Compile tested only due to lacking MPLS setup.
>>
>> This is not required. We hold the last reference of this dying `rt???.
>
> You are right. OK bluhm@ for rtfree unlocking.
>
> But I am still not convinced with rt_mpls_clear().
>
> What about htis call path? soreceive -> pru_send -> route_send ->
> route_output -> rtm_output -> rt_mpls_clear
>
> Since solock() takes rw_enter_write(>so_lock) for route sockets,
> we don't have the kernel lock anymore. I would feel safer with
> this kenrel lock hammer for mpls. Of course rt_mpls_set() would
> need something simmilar.
This path is netlock protected.
911 rtm_output(struct rt_msghdr *rtm, struct rtentry **prt,
912 struct rt_addrinfo *info, uint8_t prio, unsigned int tableid)
913 {
/* skip */
1132 #ifdef MPLS
1133 if (rtm->rtm_flags & RTF_MPLS) {
1134 NET_LOCK();
1135 error = rt_mpls_set(rt,
1136 info->rti_info[RTAX_SRC], info->rti_mpls);
1137 NET_UNLOCK();
1138 if (error)
1139 break;
1140 } else if (newgate || (rtm->rtm_fmask & RTF_MPLS)) {
1141 NET_LOCK();
1142 /* if gateway changed remove MPLS information */
1143 rt_mpls_clear(rt);
1144 NET_UNLOCK();
1145 }
1146 #endif
Re: Remove kernel lock from rtfree(9)
On Thu, Apr 27, 2023 at 03:22:10PM +0300, Vitaliy Makkoveev wrote:
> > On 27 Apr 2023, at 15:16, Alexander Bluhm wrote:
> >
> > On Wed, Apr 26, 2023 at 11:17:37PM +0300, Vitaliy Makkoveev wrote:
> >> Route timers and route labels protected by corresponding mutexes. `ifa'
> >> uses references counting for protection. No protection required for `rt'
> >> passed to rt_mpls_clear() because only current thread owns it.
> >>
> >> ok?
> >
> > I have tested your diff and it works for me. But I don't have a
> > MPLS setup. And there is a rt_mpls_clear(rt) which your diff does
> > not show.
> >
> > if (rt->rt_llinfo != NULL)
> > free(rt->rt_llinfo);
> > rt->rt_llinfo = NULL;
> >
> > and rt->rt_flags &= ~RTF_MPLS are not mpsafe.
> >
> > What about this? Compile tested only due to lacking MPLS setup.
>
> This is not required. We hold the last reference of this dying `rt’.
Correct, this is burying a dead rtentry,
OK kn
>
> >
> > bluhm
> >
> > Index: net/route.c
> > ===
> > RCS file: /data/mirror/openbsd/cvs/src/sys/net/route.c,v
> > retrieving revision 1.419
> > diff -u -p -r1.419 route.c
> > --- net/route.c 26 Apr 2023 19:54:35 - 1.419
> > +++ net/route.c 27 Apr 2023 12:13:01 -
> > @@ -1593,11 +1593,16 @@ rt_mpls_set(struct rtentry *rt, struct s
> > void
> > rt_mpls_clear(struct rtentry *rt)
> > {
> > + if (!ISSET(rt->rt_flags, RTF_MPLS))
> > + return;
> > +
> > + KERNEL_LOCK();
> > if (rt->rt_llinfo != NULL && rt->rt_flags & RTF_MPLS) {
> > free(rt->rt_llinfo, M_TEMP, sizeof(struct rt_mpls));
> > rt->rt_llinfo = NULL;
> > }
> > rt->rt_flags &= ~RTF_MPLS;
> > + KERNEL_UNLOCK();
> > }
> > #endif
> >
> >
>
Re: Remove kernel lock from rtfree(9)
On Thu, Apr 27, 2023 at 03:22:10PM +0300, Vitaliy Makkoveev wrote:
> > On 27 Apr 2023, at 15:16, Alexander Bluhm wrote:
> >
> > On Wed, Apr 26, 2023 at 11:17:37PM +0300, Vitaliy Makkoveev wrote:
> >> Route timers and route labels protected by corresponding mutexes. `ifa'
> >> uses references counting for protection. No protection required for `rt'
> >> passed to rt_mpls_clear() because only current thread owns it.
> >>
> >> ok?
> >
> > I have tested your diff and it works for me. But I don't have a
> > MPLS setup. And there is a rt_mpls_clear(rt) which your diff does
> > not show.
> >
> > if (rt->rt_llinfo != NULL)
> > free(rt->rt_llinfo);
> > rt->rt_llinfo = NULL;
> >
> > and rt->rt_flags &= ~RTF_MPLS are not mpsafe.
> >
> > What about this? Compile tested only due to lacking MPLS setup.
>
> This is not required. We hold the last reference of this dying `rt???.
You are right. OK bluhm@ for rtfree unlocking.
But I am still not convinced with rt_mpls_clear().
What about htis call path? soreceive -> pru_send -> route_send ->
route_output -> rtm_output -> rt_mpls_clear
Since solock() takes rw_enter_write(>so_lock) for route sockets,
we don't have the kernel lock anymore. I would feel safer with
this kenrel lock hammer for mpls. Of course rt_mpls_set() would
need something simmilar.
bluhm
> > Index: net/route.c
> > ===
> > RCS file: /data/mirror/openbsd/cvs/src/sys/net/route.c,v
> > retrieving revision 1.419
> > diff -u -p -r1.419 route.c
> > --- net/route.c 26 Apr 2023 19:54:35 - 1.419
> > +++ net/route.c 27 Apr 2023 12:13:01 -
> > @@ -1593,11 +1593,16 @@ rt_mpls_set(struct rtentry *rt, struct s
> > void
> > rt_mpls_clear(struct rtentry *rt)
> > {
> > + if (!ISSET(rt->rt_flags, RTF_MPLS))
> > + return;
> > +
> > + KERNEL_LOCK();
> > if (rt->rt_llinfo != NULL && rt->rt_flags & RTF_MPLS) {
> > free(rt->rt_llinfo, M_TEMP, sizeof(struct rt_mpls));
> > rt->rt_llinfo = NULL;
> > }
> > rt->rt_flags &= ~RTF_MPLS;
> > + KERNEL_UNLOCK();
> > }
> > #endif
> >
> >
>
Re: urtwn: support new chip RTL8188FTV
On Thu, Apr 27, 2023 at 01:38:04PM +0800, Kevin Lo wrote: > Hi, > > The diff below adds initial support for RTL8188FTV adapters. > RTL8188FTV is an 802.11b/g/n, 1T1R chipset. > The firmware file comes from Linux's rtl8188fufw.bin [1]. > > Tested with Comfast CF-WU710N v4 on amd64. > > Test reports and OKs are welcome. OK stsp@ I don't have hardware to test this. I have verified that 'make release' is still passing on amd64 with this patch.
Re: Remove kernel lock from rtfree(9)
> On 27 Apr 2023, at 15:16, Alexander Bluhm wrote:
>
> On Wed, Apr 26, 2023 at 11:17:37PM +0300, Vitaliy Makkoveev wrote:
>> Route timers and route labels protected by corresponding mutexes. `ifa'
>> uses references counting for protection. No protection required for `rt'
>> passed to rt_mpls_clear() because only current thread owns it.
>>
>> ok?
>
> I have tested your diff and it works for me. But I don't have a
> MPLS setup. And there is a rt_mpls_clear(rt) which your diff does
> not show.
>
> if (rt->rt_llinfo != NULL)
> free(rt->rt_llinfo);
> rt->rt_llinfo = NULL;
>
> and rt->rt_flags &= ~RTF_MPLS are not mpsafe.
>
> What about this? Compile tested only due to lacking MPLS setup.
This is not required. We hold the last reference of this dying `rt’.
>
> bluhm
>
> Index: net/route.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/net/route.c,v
> retrieving revision 1.419
> diff -u -p -r1.419 route.c
> --- net/route.c 26 Apr 2023 19:54:35 - 1.419
> +++ net/route.c 27 Apr 2023 12:13:01 -
> @@ -1593,11 +1593,16 @@ rt_mpls_set(struct rtentry *rt, struct s
> void
> rt_mpls_clear(struct rtentry *rt)
> {
> + if (!ISSET(rt->rt_flags, RTF_MPLS))
> + return;
> +
> + KERNEL_LOCK();
> if (rt->rt_llinfo != NULL && rt->rt_flags & RTF_MPLS) {
> free(rt->rt_llinfo, M_TEMP, sizeof(struct rt_mpls));
> rt->rt_llinfo = NULL;
> }
> rt->rt_flags &= ~RTF_MPLS;
> + KERNEL_UNLOCK();
> }
> #endif
>
>
malloc: less unlock/lock dancing
This was introduced to not stall other threads while mmap is called by
a thread. But now that mmap is unlocked, I believe it is no longer
useful.
A full build is slighlty faster with this. But this also needs testing
with you favorite multithreaded program.
-Otto
Index: stdlib/malloc.c
===
RCS file: /home/cvs/src/lib/libc/stdlib/malloc.c,v
retrieving revision 1.282
diff -u -p -r1.282 malloc.c
--- stdlib/malloc.c 21 Apr 2023 06:19:40 - 1.282
+++ stdlib/malloc.c 27 Apr 2023 05:40:49 -
@@ -264,24 +264,6 @@ static void malloc_exit(void);
(sz) = (uintptr_t)(r)->p & MALLOC_PAGEMASK, \
(sz) = ((sz) == 0 ? (r)->size : B2SIZE((sz) - 1))
-static inline void
-_MALLOC_LEAVE(struct dir_info *d)
-{
- if (d->malloc_mt) {
- d->active--;
- _MALLOC_UNLOCK(d->mutex);
- }
-}
-
-static inline void
-_MALLOC_ENTER(struct dir_info *d)
-{
- if (d->malloc_mt) {
- _MALLOC_LOCK(d->mutex);
- d->active++;
- }
-}
-
static inline size_t
hash(void *p)
{
@@ -879,9 +861,7 @@ map(struct dir_info *d, size_t sz, int z
return p;
}
if (psz <= 1) {
- _MALLOC_LEAVE(d);
p = MMAP(cache->max * sz, d->mmap_flag);
- _MALLOC_ENTER(d);
if (p != MAP_FAILED) {
STATS_ADD(d->malloc_used, cache->max * sz);
cache->length = cache->max - 1;
@@ -901,9 +881,7 @@ map(struct dir_info *d, size_t sz, int z
}
}
- _MALLOC_LEAVE(d);
p = MMAP(sz, d->mmap_flag);
- _MALLOC_ENTER(d);
if (p != MAP_FAILED)
STATS_ADD(d->malloc_used, sz);
/* zero fill not needed */
Re: Remove kernel lock from rtfree(9)
On Wed, Apr 26, 2023 at 11:17:37PM +0300, Vitaliy Makkoveev wrote:
> Route timers and route labels protected by corresponding mutexes. `ifa'
> uses references counting for protection. No protection required for `rt'
> passed to rt_mpls_clear() because only current thread owns it.
>
> ok?
I have tested your diff and it works for me. But I don't have a
MPLS setup. And there is a rt_mpls_clear(rt) which your diff does
not show.
if (rt->rt_llinfo != NULL)
free(rt->rt_llinfo);
rt->rt_llinfo = NULL;
and rt->rt_flags &= ~RTF_MPLS are not mpsafe.
What about this? Compile tested only due to lacking MPLS setup.
bluhm
Index: net/route.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/net/route.c,v
retrieving revision 1.419
diff -u -p -r1.419 route.c
--- net/route.c 26 Apr 2023 19:54:35 - 1.419
+++ net/route.c 27 Apr 2023 12:13:01 -
@@ -1593,11 +1593,16 @@ rt_mpls_set(struct rtentry *rt, struct s
void
rt_mpls_clear(struct rtentry *rt)
{
+ if (!ISSET(rt->rt_flags, RTF_MPLS))
+ return;
+
+ KERNEL_LOCK();
if (rt->rt_llinfo != NULL && rt->rt_flags & RTF_MPLS) {
free(rt->rt_llinfo, M_TEMP, sizeof(struct rt_mpls));
rt->rt_llinfo = NULL;
}
rt->rt_flags &= ~RTF_MPLS;
+ KERNEL_UNLOCK();
}
#endif
Re: arm64 install.md: fix softraid crypto installation on Mac
Another approach would be to make installboot(8) -p to retain existing
EFI Sys partitions instead of always recreating them.
This way, it was nothing to do with softraid, but installing on machines
like Apple arm64 depends on existing non-OpenBSD partitions and files on
them.
We hacked 'installboot -p' to do the 'try mount, fsck, retry mount, newfs'
dance it does in its normal mode (no '-p'), but that its own design choice
spilling over to all supported installboot architectures.
For this apple case, it'd be nice to have a sort of idempotent
'installboot -p', but at this point it seems overkill and I prefer to fix
it like caspar did down in the arm64 apple specific installer bits.
This is nicely contained and has no behaviour changes for machines without
APFS ISC.
On Thu, Apr 27, 2023 at 12:31:09PM +0100, Caspar Schutijser wrote:
> I was trying to install OpenBSD on the arm64 MacBook Air with
> softraid crypto (that's the "Encrypt the root disk?" question in the
> installer). Right now that does not work out of the box.
>
> In a regular install, md_prep_fdisk is careful and leaves the EFI Sys
> partition alone with the "if disk_has $_disk gpt apfsisc" check.
>
> However, in the "encrypt the root disk" case, the installer goes through
> md_prep_fdisk twice. The second time, it's called on the softraid crypto
> disk, which does not have the special EFI Sys partition. So there we
> don't hit the special "if disk_has $_disk gpt apfsisc" case. Instead,
> we go to a more regular case where we end up running "installboot -p".
>
> Because it's a softraid disk, installboot also looks at the "chunks"
> that the softraid volume resides on. I.e., it also looks at sd0 and
> there "installboot -p" will newfs the EFI Sys partition, which is
> something we don't want on the Mac.
>
> This diff addresses this problem. The first time we go through
> md_prep_fdisk, we keep track of whether we're in this special Mac
> case by setting KEEP_EFI_SYS. Then when we go through md_prep_fdisk
> for the second time to prepare the softraid disk, we can check this
> variable to see if we should avoid running "installboot -p".
>
> Debugged with help from and came up with a fix with kn@, thanks!
>
> Comments or OKs?
>
> Caspar
>
> ---
>
> arm64 install.md: fix softraid crypto installation on Mac
>
> Make sure we don't newfs the EFI Sys partition on systems that have an
> "apfsisc" partition in the case we're installing with softraid crypto.
>
> Debugged with help from and came up with a fix with kn@
>
>
> Index: install.md
> ===
> RCS file: /cvs/src/distrib/arm64/ramdisk/install.md,v
> retrieving revision 1.46
> diff -u -p -r1.46 install.md
> --- install.md27 Apr 2023 10:03:49 - 1.46
> +++ install.md27 Apr 2023 11:26:56 -
> @@ -36,6 +36,7 @@ MDBOOTSR=y
> NCPU=$(sysctl -n hw.ncpufound)
> COMPATIBLE=$(sysctl -n machdep.compatible)
> MOUNT_ARGS_msdos="-o-l"
> +KEEP_EFI_SYS=false
>
> md_installboot() {
> local _disk=$1 _chunks _bootdisk _mdec _plat
> @@ -109,6 +110,11 @@ md_prep_fdisk() {
> [wW]*)
> echo -n "Creating a ${bootfstype} partition and an
> OpenBSD partition for rest of $_disk..."
> if disk_has $_disk gpt apfsisc; then
> + # On Apple hardware, the existing EFI Sys
> + # partition contains boot firmware and MUST NOT
> + # be recreated.
> + KEEP_EFI_SYS=true
> +
> # Is this a boot disk?
> if [[ $_disk == @($ROOTDISK|$CRYPTOCHUNK) ]];
> then
> fdisk -Ay -b "${bootsectorsize}"
> ${_disk} >/dev/null
> @@ -119,13 +125,20 @@ md_prep_fdisk() {
> # Is this a boot disk?
> if [[ $_disk == @($ROOTDISK|$CRYPTOCHUNK) ]];
> then
> fdisk -gy -b "${bootsectorsize}"
> ${_disk} >/dev/null
> - installboot -p $_disk
> +
> + # With root on softraid,
> + # 'installboot -p' on the root disk
> + # nukes EFI Sys on the chunks.
> + $KEEP_EFI_SYS || installboot -p $_disk
> else
> fdisk -gy ${_disk} >/dev/null
> fi
> else
> fdisk -iy -b
> "${bootsectorsize}@${bootsectorstart}:${bootparttype}" ${_disk} >/dev/null
> - installboot -p $_disk
> +
> + # With root on softraid, 'installboot -p' on
> + # the root disk nukes EFI Sys on the chunks.
> +
arm64 install.md: fix softraid crypto installation on Mac
I was trying to install OpenBSD on the arm64 MacBook Air with
softraid crypto (that's the "Encrypt the root disk?" question in the
installer). Right now that does not work out of the box.
In a regular install, md_prep_fdisk is careful and leaves the EFI Sys
partition alone with the "if disk_has $_disk gpt apfsisc" check.
However, in the "encrypt the root disk" case, the installer goes through
md_prep_fdisk twice. The second time, it's called on the softraid crypto
disk, which does not have the special EFI Sys partition. So there we
don't hit the special "if disk_has $_disk gpt apfsisc" case. Instead,
we go to a more regular case where we end up running "installboot -p".
Because it's a softraid disk, installboot also looks at the "chunks"
that the softraid volume resides on. I.e., it also looks at sd0 and
there "installboot -p" will newfs the EFI Sys partition, which is
something we don't want on the Mac.
This diff addresses this problem. The first time we go through
md_prep_fdisk, we keep track of whether we're in this special Mac
case by setting KEEP_EFI_SYS. Then when we go through md_prep_fdisk
for the second time to prepare the softraid disk, we can check this
variable to see if we should avoid running "installboot -p".
Debugged with help from and came up with a fix with kn@, thanks!
Comments or OKs?
Caspar
---
arm64 install.md: fix softraid crypto installation on Mac
Make sure we don't newfs the EFI Sys partition on systems that have an
"apfsisc" partition in the case we're installing with softraid crypto.
Debugged with help from and came up with a fix with kn@
Index: install.md
===
RCS file: /cvs/src/distrib/arm64/ramdisk/install.md,v
retrieving revision 1.46
diff -u -p -r1.46 install.md
--- install.md 27 Apr 2023 10:03:49 - 1.46
+++ install.md 27 Apr 2023 11:26:56 -
@@ -36,6 +36,7 @@ MDBOOTSR=y
NCPU=$(sysctl -n hw.ncpufound)
COMPATIBLE=$(sysctl -n machdep.compatible)
MOUNT_ARGS_msdos="-o-l"
+KEEP_EFI_SYS=false
md_installboot() {
local _disk=$1 _chunks _bootdisk _mdec _plat
@@ -109,6 +110,11 @@ md_prep_fdisk() {
[wW]*)
echo -n "Creating a ${bootfstype} partition and an
OpenBSD partition for rest of $_disk..."
if disk_has $_disk gpt apfsisc; then
+ # On Apple hardware, the existing EFI Sys
+ # partition contains boot firmware and MUST NOT
+ # be recreated.
+ KEEP_EFI_SYS=true
+
# Is this a boot disk?
if [[ $_disk == @($ROOTDISK|$CRYPTOCHUNK) ]];
then
fdisk -Ay -b "${bootsectorsize}"
${_disk} >/dev/null
@@ -119,13 +125,20 @@ md_prep_fdisk() {
# Is this a boot disk?
if [[ $_disk == @($ROOTDISK|$CRYPTOCHUNK) ]];
then
fdisk -gy -b "${bootsectorsize}"
${_disk} >/dev/null
- installboot -p $_disk
+
+ # With root on softraid,
+ # 'installboot -p' on the root disk
+ # nukes EFI Sys on the chunks.
+ $KEEP_EFI_SYS || installboot -p $_disk
else
fdisk -gy ${_disk} >/dev/null
fi
else
fdisk -iy -b
"${bootsectorsize}@${bootsectorstart}:${bootparttype}" ${_disk} >/dev/null
- installboot -p $_disk
+
+ # With root on softraid, 'installboot -p' on
+ # the root disk nukes EFI Sys on the chunks.
+ $KEEP_EFI_SYS || installboot -p $_disk
fi
echo "done."
return ;;
ws(4): swap saved x/y values if axes are swapped
If a touchscreen is rotated (Option "Rotate" "CW" in ws(4)), then
when a finger slides purely horizontally or purely vertically across
the screen, the cursor jumps to a *diagonal* line starting at the
corner, as if x and y were equal.
The problem manifests here, in wsReadInput():
if (priv->swap_axes) {
int tmp;
tmp = hw.ax;
hw.ax = hw.ay;
hw.ay = tmp;
}
if ((hw.ax != priv->old_ax) || (hw.ay != priv->old_ay)) {
...
When the first block is entered, the second block is entered more often
than it should, because the old values that ax and ay are being checked
against have not been swapped.
With the diff below, cursor motion from my Steam Deck touch screen is
smooth and unbroken as expected, whether rotated or unrotated.
ok?
Index: ws.c
===
RCS file: /cvs/xenocara/driver/xf86-input-ws/src/ws.c,v
retrieving revision 1.68
diff -u -p -r1.68 ws.c
--- ws.c25 Apr 2023 20:18:48 - 1.68
+++ ws.c27 Apr 2023 11:13:04 -
@@ -563,8 +563,13 @@ wsReadHwState(InputInfoPtr pInfo, wsHwSt
bzero(hw, sizeof(wsHwState));
hw->buttons = priv->lastButtons;
- hw->ax = priv->old_ax;
- hw->ay = priv->old_ay;
+ if (!priv->swap_axes) {
+ hw->ax = priv->old_ax;
+ hw->ay = priv->old_ay;
+ } else {
+ hw->ax = priv->old_ay;
+ hw->ay = priv->old_ax;
+ }
while ((event = wsGetEvent(pInfo)) != NULL) {
switch (event->type) {
Re: changlist: add apmd(8) hooks
On Thu, Apr 27, 2023 at 10:53:03AM +, Klemens Nanni wrote: > Would be nice to record changes to critical scripts run on state changes > and have modifications recorded through security(8). > > Feedback? Objection? OK? This gets ugly if you use binary files instead of scripts, so we'd either want their hashes or not handle them at all. > > Index: changelist > === > RCS file: /cvs/src/etc/changelist,v > retrieving revision 1.136 > diff -u -p -r1.136 changelist > --- changelist24 Apr 2023 16:36:54 - 1.136 > +++ changelist27 Apr 2023 10:39:20 - > @@ -11,6 +11,12 @@ > /etc/acme-client.conf > /etc/adduser.conf > /etc/adduser.message > +/etc/apm/hibernate > +/etc/apm/powerdown > +/etc/apm/powerup > +/etc/apm/resume > +/etc/apm/standby > +/etc/apm/suspend > /etc/bgpd.conf > /etc/boot.conf > /etc/bootparams >
fill_file(): use solock_shared() to protect socket data
Now only direct netlock used for inet sockets protection. The unlocked
access to all other sockets is safe, but we could lost consistency for a
little. Since the solock() used for sockets protection, make locking
path common and use it. Make it shared because this is read-only access
to sockets data. For same reason use shared netlock while performing
inpcb tables foreach walkthrough.
Index: sys/kern/kern_sysctl.c
===
RCS file: /cvs/src/sys/kern/kern_sysctl.c,v
retrieving revision 1.411
diff -u -p -r1.411 kern_sysctl.c
--- sys/kern/kern_sysctl.c 22 Jan 2023 12:05:44 - 1.411
+++ sys/kern/kern_sysctl.c 27 Apr 2023 10:18:01 -
@@ -1173,14 +1173,11 @@ fill_file(struct kinfo_file *kf, struct
if (so == NULL) {
so = (struct socket *)fp->f_data;
+ solock_shared(so);
+ locked = 1;
+ } else {
/* if so is passed as parameter it is already locked */
- switch (so->so_proto->pr_domain->dom_family) {
- case AF_INET:
- case AF_INET6:
- NET_LOCK();
- locked = 1;
- break;
- }
+ soassertlocked(so);
}
kf->so_type = so->so_type;
@@ -1203,14 +1200,13 @@ fill_file(struct kinfo_file *kf, struct
kf->so_splicelen = -1;
if (so->so_pcb == NULL) {
if (locked)
- NET_UNLOCK();
+ sounlock_shared(so);
break;
}
switch (kf->so_family) {
case AF_INET: {
struct inpcb *inpcb = so->so_pcb;
- NET_ASSERT_LOCKED();
if (show_pointers)
kf->inp_ppcb = PTRTOINT64(inpcb->inp_ppcb);
kf->inp_lport = inpcb->inp_lport;
@@ -1232,7 +1228,6 @@ fill_file(struct kinfo_file *kf, struct
case AF_INET6: {
struct inpcb *inpcb = so->so_pcb;
- NET_ASSERT_LOCKED();
if (show_pointers)
kf->inp_ppcb = PTRTOINT64(inpcb->inp_ppcb);
kf->inp_lport = inpcb->inp_lport;
@@ -1279,7 +1274,7 @@ fill_file(struct kinfo_file *kf, struct
}
}
if (locked)
- NET_UNLOCK();
+ sounlock_shared(so);
break;
}
@@ -1375,7 +1370,7 @@ sysctl_file(int *name, u_int namelen, ch
if (arg == DTYPE_SOCKET) {
struct inpcb *inp;
- NET_LOCK();
+ NET_LOCK_SHARED();
mtx_enter(_mtx);
TAILQ_FOREACH(inp, _queue, inp_queue)
FILLSO(inp->inp_socket);
@@ -1395,7 +1390,7 @@ sysctl_file(int *name, u_int namelen, ch
FILLSO(inp->inp_socket);
mtx_leave(_mtx);
#endif
- NET_UNLOCK();
+ NET_UNLOCK_SHARED();
}
fp = NULL;
while ((fp = fd_iterfile(fp, p)) != NULL) {
changlist: add apmd(8) hooks
Would be nice to record changes to critical scripts run on state changes and have modifications recorded through security(8). Feedback? Objection? OK? Index: changelist === RCS file: /cvs/src/etc/changelist,v retrieving revision 1.136 diff -u -p -r1.136 changelist --- changelist 24 Apr 2023 16:36:54 - 1.136 +++ changelist 27 Apr 2023 10:39:20 - @@ -11,6 +11,12 @@ /etc/acme-client.conf /etc/adduser.conf /etc/adduser.message +/etc/apm/hibernate +/etc/apm/powerdown +/etc/apm/powerup +/etc/apm/resume +/etc/apm/standby +/etc/apm/suspend /etc/bgpd.conf /etc/boot.conf /etc/bootparams
apmd: zap dead cancel code
#if 0 since import, APM_CANCEL does not appear anywhere else. OK? Index: apmd.c === RCS file: /cvs/src/usr.sbin/apmd/apmd.c,v retrieving revision 1.111 diff -u -p -r1.111 apmd.c --- apmd.c 8 Mar 2023 04:43:13 - 1.111 +++ apmd.c 27 Apr 2023 10:43:09 - @@ -584,11 +584,6 @@ main(int argc, char *argv[]) case APM_USER_HIBERNATE_REQ: hibernates++; break; -#if 0 - case APM_CANCEL: - suspends = standbys = 0; - break; -#endif case APM_NORMAL_RESUME: case APM_CRIT_RESUME: case APM_SYS_STANDBY_RESUME:
Re: DIOCGETRULE is slow for large rulesets (2/3)
Hello,
Hello,
On Thu, Apr 27, 2023 at 10:41:53AM +1000, David Gwynne wrote:
>
> t could be NULL here. just do the unit check inside the loop?
>
> >
> > if (t->pft_unit != unit)
> > return (NULL);
> >
> > return (t);
> > }
> >
> > just return NULL on unit mismatch. updated diff is below.
>
> ok once you fix the nit above.
>
well spotted. This is the change I made to get 2/3 patch
fixed:
diff --git a/sys/net/pf_ioctl.c b/sys/net/pf_ioctl.c
index ebe1b912766..b527610737b 100644
--- a/sys/net/pf_ioctl.c
+++ b/sys/net/pf_ioctl.c
@@ -3291,13 +3291,10 @@ pf_find_trans(uint32_t unit, uint64_t ticket)
rw_assert_anylock(_rw);
LIST_FOREACH(t, _ioctl_trans, pft_entry) {
- if (t->pft_ticket == ticket)
+ if (t->pft_ticket == ticket && t->pft_unit == unit)
break;
}
- if (t->pft_unit != unit)
- return (NULL);
-
return (t);
}
thanks and
regards
sashan
Re: mg: rework adjustname() and drop expandtilde()
Please disregard for now; this introduces an issue since in main()
paths are resolved from the working directory of the current buffer so
`mg foo/bar' works (i.e. opens $PWD/foo/bar) but `mg foo/bar foo/baz'
doesn't since it first opens $PWD/foo/bar and then foo/baz relatively
to it, i.e. $PWD/foo/foo/baz.
On 2023/04/23 18:08:18 +0200, Omar Polo wrote:
> adjustname() should canonicalize the given path, but it does not do it
> always, resulting in some bizzare situations.
>
> it calls realpath(3) but when it fails the given path is returned
> as-is. This in practice can happen quite often since it's not usual
> for an editor to visit new files.
>
> A quick way to replicate the issue is to
>
> $ mg foo# assuming `foo' does not exist
> type something then save
> C-x C-f foo RET
>
> surprise, now you're in "foo<2>", a different buffer for the same
> file.
>
> On the other hand, expandtilde() has its own fair share of issues too.
>
> We can instead syntactically clean the path. It loose the "follow
> symlink" part, but it always work and is generally less surprising.
>
> I've checked that all the callers of adjustname() either provide an
> absolute path or are happy with it being resolved via the current
> buffer working directory. getbufcwd() respects the global-wd-mode so
> that case is also handled.
>
> (Some of the calls can/should be bubbled up or dropped, but that's for
> another diff.)
>
> An annoying thing is that adjustname() returns a pointer to a static
> area and it's not uncommon for that path to be fed to adjustname()
> again, hence the temp buffer. One quick way to hit that case is "mg
> ." which will adjustname() in main and then later again in dired_().
>
> ok?
>
>
> Index: cscope.c
> ===
> RCS file: /home/cvs/src/usr.bin/mg/cscope.c,v
> retrieving revision 1.22
> diff -u -p -r1.22 cscope.c
> --- cscope.c 8 Mar 2023 04:43:11 - 1.22
> +++ cscope.c 23 Apr 2023 15:28:25 -
> @@ -338,7 +338,7 @@ jumptomatch(void)
>
> if (curmatch == NULL || currecord == NULL)
> return (FALSE);
> - adjf = adjustname(currecord->filename, TRUE);
> + adjf = adjustname(currecord->filename);
> if (adjf == NULL)
> return (FALSE);
> if ((bp = findbuffer(adjf)) == NULL)
> Index: def.h
> ===
> RCS file: /home/cvs/src/usr.bin/mg/def.h,v
> retrieving revision 1.180
> diff -u -p -r1.180 def.h
> --- def.h 21 Apr 2023 13:39:37 - 1.180
> +++ def.h 23 Apr 2023 15:30:55 -
> @@ -485,7 +485,7 @@ intffclose(FILE *, struct buffer *);
> int ffputbuf(FILE *, struct buffer *, int);
> int ffgetline(FILE *, char *, int, int *);
> int fbackupfile(const char *);
> -char *adjustname(const char *, int);
> +char *adjustname(const char *);
> FILE *startupfile(char *, char *, char *, size_t);
> int copy(char *, char *);
> struct list *make_file_list(char *);
> Index: dir.c
> ===
> RCS file: /home/cvs/src/usr.bin/mg/dir.c,v
> retrieving revision 1.33
> diff -u -p -r1.33 dir.c
> --- dir.c 8 Mar 2023 04:43:11 - 1.33
> +++ dir.c 23 Apr 2023 15:28:25 -
> @@ -117,7 +117,7 @@ do_makedir(char *path)
> mode_t dir_mode, f_mode, oumask;
> char*slash;
>
> - if ((path = adjustname(path, TRUE)) == NULL)
> + if ((path = adjustname(path)) == NULL)
> return (FALSE);
>
> /* Remove trailing slashes */
> Index: dired.c
> ===
> RCS file: /home/cvs/src/usr.bin/mg/dired.c,v
> retrieving revision 1.102
> diff -u -p -r1.102 dired.c
> --- dired.c 8 Mar 2023 04:43:11 - 1.102
> +++ dired.c 23 Apr 2023 15:28:25 -
> @@ -469,7 +469,7 @@ d_copy(int f, int n)
> else if (bufp[0] == '\0')
> return (FALSE);
>
> - topath = adjustname(toname, TRUE);
> + topath = adjustname(toname);
> if (stat(topath, ) == 0) {
> if (S_ISDIR(statbuf.st_mode)) {
> ret = snprintf(toname, sizeof(toname), "%s/%s",
> @@ -479,7 +479,7 @@ d_copy(int f, int n)
> ewprintf("Directory name too long");
> return (FALSE);
> }
> - topath = adjustname(toname, TRUE);
> + topath = adjustname(toname);
> }
> }
> if (topath == NULL)
> @@ -528,7 +528,7 @@ d_rename(int f, int n)
> else if (bufp[0] == '\0')
> return (FALSE);
>
> - topath = adjustname(toname, TRUE);
> + topath = adjustname(toname);
> if (stat(topath, ) == 0) {
> if (S_ISDIR(statbuf.st_mode)) {
> ret
Add another Lenovo NVMe device id
Found in my x1 extreme gen 1: nvme0 at pci4 dev 0 function 0 vendor "Lenovo", unknown product 0x0006 rev 0x00: msix, NVMe 1.2 ok? Index: sys/dev/pci/pcidevs === RCS file: /cvs/src/sys/dev/pci/pcidevs,v retrieving revision 1.2032 diff -u -p -u -p -r1.2032 pcidevs --- sys/dev/pci/pcidevs 25 Apr 2023 21:57:29 - 1.2032 +++ sys/dev/pci/pcidevs 27 Apr 2023 06:38:24 - @@ -7061,6 +7061,7 @@ product LEADTEK WINFAST_XP0x6609 Leadte /* Lenovo products */ product LENOVO NVME0x0003 NVMe +product LENOVO NVME_2 0x0006 NVMe /* Level 1 (Intel) */ product LEVEL1 LXT1001 0x0001 LXT1001
