sort: don't do top level comparison when invoked with -c
Hi,
I found a bug in OpenBSD's sort utility, related to a previous bug I found.[1]
The fix I provided for that bug excluded the top level comparison when -k
was in use. Recently I discovered that there are other cases where OpenBSD's
sort does not produce the correct results. I've appended these test cases
below:
Given an input file "input.txt", containing:
-
-
+
+
=
=
for each of the following invocations sort should return 0, but OpenBSD's
sort currently reports disorder:
OpenBSD:
% sort -c -d -f input.txt
sort: input.txt:2: disorder: -
% sort -c -f -i input.txt
sort: input.txt:2: disorder: -
GNU:
% sort -c -d -f input.txt
% echo $?
0
% sort -c -f -i input.txt
% echo $?
0
After thinking about it for a while I don't see why the second top level
comparison is needed at all for -c, a top-level comparison is needed
in the general case to act as a "tiebreaker" for lines whose keys
compare equal, but for -c no such tiebreaker is needed since we're not
really sorting the input, just detecting disorder.
The following patch seems to fix the issue without causing any regressions
as far as I am able to determine.
Thanks,
Richard
[1]: https://marc.info/?l=openbsd-tech=157755445524793=2
diff --git usr.bin/sort/file.c usr.bin/sort/file.c
index d3b97f5b2df..a803fc71fec 100644
--- usr.bin/sort/file.c
+++ usr.bin/sort/file.c
@@ -384,17 +384,7 @@ check(const char *fn)
}
int cmp = key_coll(ka2, ka1, 0);
if (debug_sort)
- printf("; cmp1=%d", cmp);
-
- if (!cmp && sort_opts_vals.complex_sort &&
- !(sort_opts_vals.uflag) && !(sort_opts_vals.sflag) &&
- !(sort_opts_vals.kflag)) {
- cmp = top_level_str_coll(s2, s1);
- if (debug_sort)
- printf("; cmp2=%d", cmp);
- }
- if (debug_sort)
- printf("\n");
+ printf("; cmp1=%d\n", cmp);
if ((sort_opts_vals.uflag && (cmp <= 0)) || (cmp < 0)) {
if (!(sort_opts_vals.csilentflag)) {
slaacd: Reduce maximum IPv6 PIO lifetimes
Folks/Florian,
This reduces the maximum PIO lifetimes on the host-side, as discussed in
https://tools.ietf.org/html/draft-gont-6man-slaac-renum-05#section-4.1.2
This helps improve the reaction of IPv6 SLAAC to renumbering events, and
also helps limit the time-span of damage in the event of attacks or
misconfigurations.
--- cut here
diff --git engine.c engine.c
index be5d3fc827b..fbf53f83936 100644
--- engine.c
+++ engine.c
@@ -1266,8 +1266,10 @@ parse_ra(struct slaacd_iface *iface, struct
imsg_ra *ra)
ND_OPT_PI_FLAG_ONLINK;
prefix->autonomous = prf->nd_opt_pi_flags_reserved &
ND_OPT_PI_FLAG_AUTO;
- prefix->vltime = ntohl(prf->nd_opt_pi_valid_time);
- prefix->pltime = ntohl(prf->nd_opt_pi_preferred_time);
+ prefix->pltime = min(radv->router_lifetime,
+ntohl(prf->nd_opt_pi_preferred_time));
+ prefix->vltime = min(ntohl(prf->nd_opt_pi_valid_time),
+DFLT_VLTIME_MULT * prefix->pltime);
if (radv->min_lifetime > prefix->pltime)
radv->min_lifetime = prefix->pltime;
diff --git engine.h engine.h
index b0276e71406..0d44b251adb 100644
--- engine.h
+++ engine.h
@@ -34,3 +34,5 @@ struct imsg_configure_dfr {
voidengine(int, int);
int engine_imsg_compose_frontend(int, pid_t, void *, uint16_t);
+
+#define min(a,b) ((a < b)?a:b)
diff --git slaacd.h slaacd.h
index d8e15d00aad..ad399a5ff22 100644
--- slaacd.h
+++ slaacd.h
@@ -31,6 +31,8 @@
#defineIMSG_DATA_SIZE(imsg)((imsg).hdr.len - IMSG_HEADER_SIZE)
+#define DFLT_VLTIME_MULT 48
+
static const char * const log_procnames[] = {
"main",
"engine",
cut here
Also available at:
https://www.gont.com.ar/code/patch-fgont-slaacd-max-lifetimes.txt
Thanks,
--
Fernando Gont
e-mail: ferna...@gont.com.ar || fg...@si6networks.com
PGP Fingerprint: 7809 84F5 322E 45C7 F1C9 3945 96EE A9EF D076 FFF1
Untangle proc * in VOP_*
Most of the VOP_* methods include an argument of type "struct proc *" called `a_p'. This pointer is always set to `curproc' as confirmed by the diff below. The diff has been though base build with NFS on amd64 and sparc64 as well as a full port bulk on amd64 and is in the current octeon port bulk. I'd like to commit it in order to start removing the requirement of passing a "struct proc *" pointer which is always set to curproc. This is unnecessary and has spread in many other places in the kernel. That makes codes unnecessarily complicated: - a "struct proc *" is carried over just to satisfy already complex interfaces - it isn't obvious which "struct proc *" is not the curproc and asserts like KASSERT(p == curproc) are being made - in many places where curproc is used people put XXX as if it was wrong So this is just a starting diff, cleanups can start afterward. Ok? Index: kern/vfs_vops.c === RCS file: /cvs/src/sys/kern/vfs_vops.c,v retrieving revision 1.25 diff -u -p -r1.25 vfs_vops.c --- kern/vfs_vops.c 14 Feb 2020 11:57:56 - 1.25 +++ kern/vfs_vops.c 16 Mar 2020 14:16:42 - @@ -145,6 +145,8 @@ VOP_OPEN(struct vnode *vp, int mode, str a.a_cred = cred; a.a_p = p; + KASSERT(p == curproc); + if (vp->v_op->vop_open == NULL) return (EOPNOTSUPP); @@ -164,6 +166,7 @@ VOP_CLOSE(struct vnode *vp, int fflag, s a.a_cred = cred; a.a_p = p; + KASSERT(p == curproc); ASSERT_VP_ISLOCKED(vp); if (vp->v_op->vop_close == NULL) @@ -184,6 +187,7 @@ VOP_ACCESS(struct vnode *vp, int mode, s a.a_cred = cred; a.a_p = p; + KASSERT(p == curproc); ASSERT_VP_ISLOCKED(vp); if (vp->v_op->vop_access == NULL) @@ -202,6 +206,7 @@ VOP_GETATTR(struct vnode *vp, struct vat a.a_cred = cred; a.a_p = p; + KASSERT(p == curproc); if (vp->v_op->vop_getattr == NULL) return (EOPNOTSUPP); @@ -219,6 +224,7 @@ VOP_SETATTR(struct vnode *vp, struct vat a.a_cred = cred; a.a_p = p; + KASSERT(p == curproc); ASSERT_VP_ISLOCKED(vp); if (vp->v_op->vop_setattr == NULL) @@ -282,6 +288,7 @@ VOP_IOCTL(struct vnode *vp, u_long comma a.a_cred = cred; a.a_p = p; + KASSERT(p == curproc); if (vp->v_op->vop_ioctl == NULL) return (EOPNOTSUPP); @@ -300,6 +307,7 @@ VOP_POLL(struct vnode *vp, int fflag, in a.a_events = events; a.a_p = p; + KASSERT(p == curproc); if (vp->v_op->vop_poll == NULL) return (EOPNOTSUPP); @@ -343,6 +351,7 @@ VOP_FSYNC(struct vnode *vp, struct ucred a.a_waitfor = waitfor; a.a_p = p; + KASSERT(p == curproc); ASSERT_VP_ISLOCKED(vp); if (vp->v_op->vop_fsync == NULL) @@ -564,6 +573,7 @@ VOP_INACTIVE(struct vnode *vp, struct pr a.a_vp = vp; a.a_p = p; + KASSERT(p == curproc); ASSERT_VP_ISLOCKED(vp); if (vp->v_op->vop_inactive == NULL) @@ -580,6 +590,7 @@ VOP_RECLAIM(struct vnode *vp, struct pro a.a_vp = vp; a.a_p = p; + KASSERT(p == curproc); if (vp->v_op->vop_reclaim == NULL) return (EOPNOTSUPP);
Re: Saving stack frames in dt(4)
On 19/03/20(Thu) 15:37, Martin Pieuchot wrote:
> People are starting to capture kernel stack traces and produce
> Flamegraphs. Those traces currently include the frames used by
> dt(4) itself:
>
> dt_pcb_ring_get+0x11d
> dt_prov_profile_enter+0x6e
> hardclock+0x1a9
> lapic_clockintr+0x3f
> Xresume_lapic_ltimer+0x26
> acpicpu_idle+0x261<--- CPU was Idle
> sched_idle+0x225
> proc_trampoline+0x1c
> 1
>
> Saving the last 5 frames in the example above consumes CPU time and
> require some post-processing to cleanup gathered data. So the diff
> below extends the existing stacktrace_save() into and *_at() version
> that takes an arbitrary frame as argument.
>
> The fun start when we ask "How many frames do we skip?". There are
> currently two different contexts in which dt(4) can gather data:
> - in hardlock(9), which mean a specialized interrupt context
> - in any thread or interrupt context
>
> Those two contexts require a different number of frames to reach the
> recording function: dt_pcb_ring_get(). Now what's fun is that every
> architecture has its own way to reach hardclock(9) (possibly multiple
> ones), has its own stack unwinder (with specific bugs) and might use
> different compilers with different optimizations (yeah!).
>
> So the diff below introduce two per-arch defines that specify at which
> frame the unwinding should start. This has been tested on sparc64 and
> amd64 and here's the result:
>
> # btrace -e 'tracepoint:sched:sleep { printf("%s1\n", kstack); }'
> sleep_setup+0x16c
> msleep+0x160
> taskq_next_work+0x5c
> taskq_thread+0x38
> 0x1013074
> 1
>
> Note that the number of frames for hardclock(9) on amd64 assumes lapic
> is used. That's why on vmm(4) you'll still see an extra frame as below:
>
> # btrace -e 'profile:hz:99 { printf("%s1\n", kstack); }'
> Xresume_legacy0+0x1d3
> cpu_idle_cycle+0x1b
> proc_trampoline+0x1c
> 1
>
> Whereas on real hardware:
>
> # btrace -e 'profile:hz:99 { printf("%s1\n", kstack); }'
> acpicpu_idle+0x1d2
> sched_idle+0x225
> proc_trampoline+0x1c
> 1
>
> The diff below will break compiling dt(4) on the other archs because
> they don't yet provide stacktrace_save_at(), but it shouldn't be hard
> to fix ;)
Updated diff including stacktrace_save_at() for i386, any comment on the
approach or Ok?
Index: arch/amd64/amd64/db_trace.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/db_trace.c,v
retrieving revision 1.49
diff -u -p -r1.49 db_trace.c
--- arch/amd64/amd64/db_trace.c 20 Jan 2020 15:58:23 - 1.49
+++ arch/amd64/amd64/db_trace.c 19 Mar 2020 14:06:01 -
@@ -256,11 +256,13 @@ db_stack_trace_print(db_expr_t addr, int
}
void
-stacktrace_save(struct stacktrace *st)
+stacktrace_save_at(struct stacktrace *st, void *xframe)
{
- struct callframe *frame, *lastframe;
+ struct callframe *lastframe, *frame = xframe;
+
+ if (frame == NULL)
+ frame = __builtin_frame_address(0);
- frame = __builtin_frame_address(0);
st->st_count = 0;
while (st->st_count < STACKTRACE_MAX) {
st->st_pc[st->st_count++] = frame->f_retaddr;
@@ -275,6 +277,12 @@ stacktrace_save(struct stacktrace *st)
if (!INKERNEL(frame->f_retaddr))
break;
}
+}
+
+void
+stacktrace_save(struct stacktrace *st)
+{
+ return stacktrace_save_at(st, NULL);
}
vaddr_t
Index: arch/i386/i386/db_trace.c
===
RCS file: /cvs/src/sys/arch/i386/i386/db_trace.c,v
retrieving revision 1.38
diff -u -p -r1.38 db_trace.c
--- arch/i386/i386/db_trace.c 20 Jan 2020 15:58:23 - 1.38
+++ arch/i386/i386/db_trace.c 20 Mar 2020 10:50:28 -
@@ -263,9 +263,17 @@ db_stack_trace_print(db_expr_t addr, int
void
stacktrace_save(struct stacktrace *st)
{
- struct callframe *frame, *lastframe;
+ return stacktrace_save_at(st, NULL);
+}
+
+void
+stacktrace_save_at(struct stacktrace *st, void *xframe)
+{
+ struct callframe *lastframe, *frame = xframe;
+
+ if (frame == NULL)
+ frame = __builtin_frame_address(0);
- frame = __builtin_frame_address(0);
st->st_count = 0;
while (st->st_count < STACKTRACE_MAX) {
st->st_pc[st->st_count++] = frame->f_retaddr;
Index: arch/sparc64/sparc64/db_trace.c
===
RCS file: /cvs/src/sys/arch/sparc64/sparc64/db_trace.c,v
retrieving revision 1.20
diff -u -p -r1.20 db_trace.c
--- arch/sparc64/sparc64/db_trace.c 20 Jan 2020 15:58:23 - 1.20
+++ arch/sparc64/sparc64/db_trace.c 19 Mar 2020 14:06:01 -
@@ -155,7 +155,7 @@ db_stack_trace_print(db_expr_t addr, int
}
void
-stacktrace_save(struct stacktrace *st)
+stacktrace_save_at(struct stacktrace *st, void *xframe)
Re: sort: don't do top level comparison when invoked with -c
GNU sort on Linux behaves the same as the OpenBSD sort when run in the C locale. $ LANG=C sort -c -d -f input.txt sort: input.txt:2: disorder: - $ LANG=C sort -c -d -i input.txt sort: input.txt:2: disorder: - Since our C library doesn't really support other locales I think this is the expected behavior. - todd
Re: tar confused error messages
On Sun, 22 Mar 2020 11:41:55 +0100, Marc Espie wrote: > If tar can't create intermediate directories due to permission > issues, the resulting message is confusing: > > ./tar xf gcc.tar gcc-8.3.0/include/obstack.h > tar: Unable to create gcc-8.3.0/include/obstack.h: No such file or directory > > (here I have gcc-8.3.0 owned by root and no permissions) > > The following patch changes this to: > > ./tar xf gcc.tar gcc-8.3.0/include/obstack.h > tar: Unable to create gcc-8.3.0/include: Permission denied > tar: Unable to create gcc-8.3.0/include/obstack.h: No such file or directory > > okay ? or a better way to do this ? I think that is the best way to accomplish this. GNU tar does something similar except they are a bit more explicit. E.g. gtar: gcc-8.3.0/include: Cannot mkdir: Permission denied You might consider: syswarn(1, errno, "Unable to mkdir %s", name); instead so it is clear exactly what is failing. Either way, OK millert@ - todd
rtsock: redundant NULL pointer check
It seems that there is no way 'rtm' could actually be NULL here, which
means we can get rid of the check.
ok?
Index: net/rtsock.c
===
RCS file: /mount/openbsd/cvs/src/sys/net/rtsock.c,v
retrieving revision 1.297
diff -u -p -r1.297 rtsock.c
--- net/rtsock.c24 Nov 2019 07:56:03 - 1.297
+++ net/rtsock.c23 Mar 2020 21:15:51 -
@@ -858,14 +858,12 @@ fail:
return (error);
}
}
- if (rtm) {
- if (m_copyback(m, 0, len, rtm, M_NOWAIT)) {
- m_freem(m);
- m = NULL;
- } else if (m->m_pkthdr.len > len)
- m_adj(m, len - m->m_pkthdr.len);
- free(rtm, M_RTABLE, len);
- }
+ if (m_copyback(m, 0, len, rtm, M_NOWAIT)) {
+ m_freem(m);
+ m = NULL;
+ } else if (m->m_pkthdr.len > len)
+ m_adj(m, len - m->m_pkthdr.len);
+ free(rtm, M_RTABLE, len);
if (m)
route_input(m, so, info.rti_info[RTAX_DST] ?
info.rti_info[RTAX_DST]->sa_family : AF_UNSPEC);
umidi: missing NULL checks
In alloc_all_jacks() the variables 'sc_in_jacks' and 'sc_out_checks'
are set to NULL if 'sc_in_num_jacks' and 'sc_out_num_jacks' are 0.
Further down both are dereferenced unconditionally. I added explicit NULL
checks where I think they belong.
I think 'sc_in_ep' and 'sc_out_ep' can also be NULL, but I am not
entirely sure about those.
Objections or ok?
Index: dev/usb/umidi.c
===
RCS file: /mount/openbsd/cvs/src/sys/dev/usb/umidi.c,v
retrieving revision 1.53
diff -u -p -r1.53 umidi.c
--- dev/usb/umidi.c 16 Mar 2020 16:12:43 - 1.53
+++ dev/usb/umidi.c 23 Mar 2020 21:50:49 -
@@ -724,50 +724,58 @@ alloc_all_jacks(struct umidi_softc *sc)
sc->sc_in_jacks =
sc->sc_in_num_jacks ? sc->sc_jacks+sc->sc_out_num_jacks : NULL;
- jack = >sc_out_jacks[0];
- for (i=0; isc_out_num_jacks; i++) {
- jack->opened = 0;
- jack->binded = 0;
- jack->arg = NULL;
- jack->u.out.intr = NULL;
- jack->intr = 0;
- jack->cable_number = i;
- jack++;
+ if (sc->sc_out_jacks) {
+ jack = >sc_out_jacks[0];
+ for (i=0; isc_out_num_jacks; i++) {
+ jack->opened = 0;
+ jack->binded = 0;
+ jack->arg = NULL;
+ jack->u.out.intr = NULL;
+ jack->intr = 0;
+ jack->cable_number = i;
+ jack++;
+ }
}
- jack = >sc_in_jacks[0];
- for (i=0; isc_in_num_jacks; i++) {
- jack->opened = 0;
- jack->binded = 0;
- jack->arg = NULL;
- jack->u.in.intr = NULL;
- jack->cable_number = i;
- jack++;
+ if (sc->sc_in_jacks) {
+ jack = >sc_in_jacks[0];
+ for (i=0; isc_in_num_jacks; i++) {
+ jack->opened = 0;
+ jack->binded = 0;
+ jack->arg = NULL;
+ jack->u.in.intr = NULL;
+ jack->cable_number = i;
+ jack++;
+ }
}
/* assign each jacks to each endpoints */
- jack = >sc_out_jacks[0];
- ep = >sc_out_ep[0];
- for (i=0; isc_out_num_endpoints; i++) {
- rjack = >jacks[0];
- for (j=0; jnum_jacks; j++) {
- *rjack = jack;
- jack->endpoint = ep;
- jack++;
- rjack++;
+ if (sc->sc_out_jacks && sc->sc_out_ep) {
+ jack = >sc_out_jacks[0];
+ ep = >sc_out_ep[0];
+ for (i=0; isc_out_num_endpoints; i++) {
+ rjack = >jacks[0];
+ for (j=0; jnum_jacks; j++) {
+ *rjack = jack;
+ jack->endpoint = ep;
+ jack++;
+ rjack++;
+ }
+ ep++;
}
- ep++;
}
- jack = >sc_in_jacks[0];
- ep = >sc_in_ep[0];
- for (i=0; isc_in_num_endpoints; i++) {
- rjack = >jacks[0];
- for (j=0; jnum_jacks; j++) {
- *rjack = jack;
- jack->endpoint = ep;
- jack++;
- rjack++;
+ if (sc->sc_in_jacks && sc->sc_in_ep) {
+ jack = >sc_in_jacks[0];
+ ep = >sc_in_ep[0];
+ for (i=0; isc_in_num_endpoints; i++) {
+ rjack = >jacks[0];
+ for (j=0; jnum_jacks; j++) {
+ *rjack = jack;
+ jack->endpoint = ep;
+ jack++;
+ rjack++;
+ }
+ ep++;
}
- ep++;
}
return USBD_NORMAL_COMPLETION;
snmpd(8): Remove restricted socket
snmpd's normal socket is pretty much deprecated and the restricted
variant is even more useless. In other words lets pick it apart one
step at a time. This diff removes the restricted keyword and related
code.
While here I also removed the unimplemented IMSG_CTL_RELOAD logic.
For those wondering why I removed the CTL_CONN_LOCKED flag: It's only
checked in control_dispatch_imsg, so there's no point in setting it on
agentx sockets.
OK?
martijn@
Index: control.c
===
--- control.c (revision 1)
+++ control.c (working copy)
@@ -80,7 +80,7 @@
return (-1);
}
- if (cs->cs_restricted || cs->cs_agentx) {
+ if (cs->cs_agentx) {
old_umask = umask(S_IXUSR|S_IXGRP|S_IXOTH);
mode = S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH;
} else {
@@ -174,7 +174,6 @@
log_warn("%s: agentx", __func__);
return;
}
- c->flags |= CTL_CONN_LOCKED;
c->iev.handler = control_dispatch_agentx;
TAILQ_INIT(>oids);
} else
@@ -249,21 +248,6 @@
if (n == 0)
break;
- if (cs->cs_restricted || (c->flags & CTL_CONN_LOCKED)) {
- switch (imsg.hdr.type) {
- case IMSG_SNMP_AGENTX:
- case IMSG_SNMP_ELEMENT:
- case IMSG_SNMP_END:
- case IMSG_SNMP_LOCK:
- break;
- default:
- control_close(c,
- "client requested restricted command",
- );
- return;
- }
- }
-
control_imsg_forward();
switch (imsg.hdr.type) {
@@ -282,14 +266,6 @@
c->flags |= CTL_CONN_NOTIFY;
break;
- case IMSG_SNMP_LOCK:
- if (IMSG_DATA_SIZE())
- return control_close(c, "invalid size", );
-
- /* enable restricted control mode */
- c->flags |= CTL_CONN_LOCKED;
- break;
-
case IMSG_SNMP_AGENTX:
if (IMSG_DATA_SIZE())
return control_close(c, "invalid size", );
@@ -313,7 +289,6 @@
}
/* disable IMSG notifications */
c->flags &= ~CTL_CONN_NOTIFY;
- c->flags |= CTL_CONN_LOCKED;
c->iev.handler = control_dispatch_agentx;
break;
@@ -330,11 +305,7 @@
proc_forward_imsg(>sc_ps, , i, -1);
}
break;
- case IMSG_CTL_RELOAD:
- if (IMSG_DATA_SIZE())
- return control_close(c, "invalid size", );
- proc_forward_imsg(>sc_ps, , PROC_PARENT, -1);
- break;
+
default:
control_close(c, "invalid type", );
return;
Index: parse.y
===
--- parse.y (revision 1)
+++ parse.y (working copy)
@@ -51,11 +51,6 @@
#include "snmpd.h"
#include "mib.h"
-enum socktype {
- SOCK_TYPE_RESTRICTED = 1,
- SOCK_TYPE_AGENTX = 2
-};
-
TAILQ_HEAD(files, file) files = TAILQ_HEAD_INITIALIZER(files);
static struct file {
TAILQ_ENTRY(file)entry;
@@ -133,7 +128,7 @@
%token SYSTEM CONTACT DESCR LOCATION NAME OBJECTID SERVICES RTFILTER
%token READONLY READWRITE OCTETSTRING INTEGER COMMUNITY TRAP RECEIVER
%token SECLEVEL NONE AUTH ENC USER AUTHKEY ENCKEY ERROR DISABLED
-%token SOCKET RESTRICTED AGENTX HANDLE DEFAULT SRCADDR TCP UDP PFADDRFILTER
+%token SOCKET AGENTX HANDLE DEFAULT SRCADDR TCP UDP PFADDRFILTER
%token STRING
%token NUMBER
%typehostcmn
@@ -305,10 +300,7 @@
YYERROR;
}
rcsock->cs_name = $2;
- if ($3 == SOCK_TYPE_RESTRICTED)
- rcsock->cs_restricted = 1;
- else if ($3 == SOCK_TYPE_AGENTX)
- rcsock->cs_agentx = 1;
+ rcsock->cs_agentx = 1;
TAILQ_INSERT_TAIL(>sc_ps.ps_rcsocks,
rcsock, cs_entry);
} else {
@@ -541,8 +533,7 @@
}
;
-socktype : RESTRICTED{ $$ =
Re: [PLEASE TEST] acpi(4): acpi_sleep(): tsleep(9) -> tsleep_nsec(9)
On Sun, Mar 15, 2020 at 09:55:53PM -0500, Scott Cheloha wrote:
>
> [...]
>
> This is a straightforward ticks-to-milliseconds conversion, but IIRC
> pirofti@ wanted me to get some tests before committing it.
>
> The only users of acpi_sleep() are (a) acpitz(4) and (b) any AML code
> that uses AMLOP_SLEEP. AMLOP_SLEEP seems to trigger just before a
> suspend. I don't know when else it is used.
>
> If you have an acpi(4) laptop with suspend/resume support, please
> apply this patch and let me know if anything doesn't work,
> particularly with suspend/resume.
>
> [...]
1 week bump.
I have one test report. I'm hoping for a few more.
I think acpi(4) machines with suspend/resume support should be
somewhat common amongst tech@ readers.
Index: dsdt.c
===
RCS file: /cvs/src/sys/dev/acpi/dsdt.c,v
retrieving revision 1.249
diff -u -p -r1.249 dsdt.c
--- dsdt.c 16 Oct 2019 01:43:50 - 1.249
+++ dsdt.c 16 Mar 2020 02:26:25 -
@@ -465,15 +465,11 @@ void
acpi_sleep(int ms, char *reason)
{
static int acpinowait;
- int to = ms * hz / 1000;
if (cold)
delay(ms * 1000);
- else {
- if (to <= 0)
- to = 1;
- tsleep(, PWAIT, reason, to);
- }
+ else
+ tsleep_nsec(, PWAIT, reason, MSEC_TO_NSEC(ms));
}
void
Re: Untangle proc * in VOP_*
On 2020-03-23, Martin Pieuchot wrote: > Most of the VOP_* methods include an argument of type "struct proc *" > called `a_p'. This pointer is always set to `curproc' as confirmed by > the diff below. The diff has been though base build with NFS on amd64 > and sparc64 as well as a full port bulk on amd64 and is in the current > octeon port bulk. > > I'd like to commit it in order to start removing the requirement of > passing a "struct proc *" pointer which is always set to curproc. This > is unnecessary and has spread in many other places in the kernel. > > That makes codes unnecessarily complicated: > - a "struct proc *" is carried over just to satisfy already complex >interfaces > - it isn't obvious which "struct proc *" is not the curproc and >asserts like KASSERT(p == curproc) are being made > - in many places where curproc is used people put XXX as if it was >wrong > > So this is just a starting diff, cleanups can start afterward. If this is a temporary measure, I think it should include some annotation to that effect, so that it doesn't continue spreading.
