bpf(4): separate nonblocking status from read timeout
Hi,
I want to remove the use of ticks from bpf(4) and replace it
with the system uptime clock.
Before we can do that, though, we need to separate the bpf(4)
descriptor's nonblocking status and its read timeout. The
two states share the same struct member, bpf_d.bd_rtout.
Currently, if bpf_d.bd_rtout == -1 you have a nonblocking read. If it
is equal to 0, there is no read timeout. If it is greater than or
equal to 1, it is the number of ticks the read will wait for data
before giving up.
This is a potentially confusing mixture of states.
I propose moving nonblocking status to a seprate boolean,
bpf_d.bd_rnonblock.
This has one side-effect: the bpf(4) descriptor will no
longer "forget" its read timeout if the program toggles
nonblocking status on/off on the descriptor.
I would argue that this behavior is more intuitive.
Thoughts? ok?
Index: bpfdesc.h
===
RCS file: /cvs/src/sys/net/bpfdesc.h,v
retrieving revision 1.40
diff -u -p -r1.40 bpfdesc.h
--- bpfdesc.h 2 Jan 2020 16:23:01 - 1.40
+++ bpfdesc.h 25 Mar 2020 00:32:10 -
@@ -74,6 +74,7 @@ struct bpf_d {
struct bpf_if *bd_bif; /* interface descriptor */
u_long bd_rtout; /* Read timeout in 'ticks' */
u_long bd_rdStart; /* when the read started */
+ int bd_rnonblock; /* true if nonblocking reads are set */
struct bpf_program_smr
*bd_rfilter; /* read filter code */
struct bpf_program_smr
Index: bpf.c
===
RCS file: /cvs/src/sys/net/bpf.c,v
retrieving revision 1.188
diff -u -p -r1.188 bpf.c
--- bpf.c 20 Feb 2020 16:56:52 - 1.188
+++ bpf.c 25 Mar 2020 00:32:10 -
@@ -379,8 +379,8 @@ bpfopen(dev_t dev, int flag, int mode, s
smr_init(&bd->bd_smr);
sigio_init(&bd->bd_sigio);
- if (flag & FNONBLOCK)
- bd->bd_rtout = -1;
+ bd->bd_rtout = 0; /* no timeout by default */
+ bd->bd_rnonblock = ISSET(flag, FNONBLOCK);
bpf_get(bd);
LIST_INSERT_HEAD(&bpf_d_list, bd, bd_list);
@@ -453,7 +453,7 @@ bpfread(dev_t dev, struct uio *uio, int
* If there's a timeout, bd_rdStart is tagged when we start the read.
* we can then figure out when we're done reading.
*/
- if (d->bd_rtout != -1 && d->bd_rdStart == 0)
+ if (d->bd_rnonblock == 0 && d->bd_rdStart == 0)
d->bd_rdStart = ticks;
else
d->bd_rdStart = 0;
@@ -482,7 +482,7 @@ bpfread(dev_t dev, struct uio *uio, int
ROTATE_BUFFERS(d);
break;
}
- if (d->bd_rtout == -1) {
+ if (d->bd_rnonblock) {
/* User requested non-blocking I/O */
error = EWOULDBLOCK;
} else {
@@ -960,9 +960,9 @@ bpfioctl(dev_t dev, u_long cmd, caddr_t
case FIONBIO: /* Non-blocking I/O */
if (*(int *)addr)
- d->bd_rtout = -1;
+ d->bd_rnonblock = 1;
else
- d->bd_rtout = 0;
+ d->bd_rnonblock = 0;
break;
case FIOASYNC: /* Send signal on receive packets */
@@ -1153,7 +1153,7 @@ bpfpoll(dev_t dev, int events, struct pr
* if there's a timeout, mark the time we
* started waiting.
*/
- if (d->bd_rtout != -1 && d->bd_rdStart == 0)
+ if (d->bd_rnonblock == 0 && d->bd_rdStart == 0)
d->bd_rdStart = ticks;
selrecord(p, &d->bd_sel);
}
@@ -1193,7 +1193,7 @@ bpfkqfilter(dev_t dev, struct knote *kn)
SLIST_INSERT_HEAD(klist, kn, kn_selnext);
mtx_enter(&d->bd_mtx);
- if (d->bd_rtout != -1 && d->bd_rdStart == 0)
+ if (d->bd_rnonblock == 0 && d->bd_rdStart == 0)
d->bd_rdStart = ticks;
mtx_leave(&d->bd_mtx);
i2c driver for Elan Touchpad
A long while ago, Ben Pye did an i2c driver for the Elan Touchpad. I believe
it had been reviewed at the time, but it was never committed. Attached
is that patch against -current. OK?
Index: arch/amd64/conf/GENERIC
===
RCS file: /cvs/src/sys/arch/amd64/conf/GENERIC,v
retrieving revision 1.488
diff -u -p -u -r1.488 GENERIC
--- arch/amd64/conf/GENERIC 6 Mar 2020 08:39:34 - 1.488
+++ arch/amd64/conf/GENERIC 24 Mar 2020 23:44:54 -
@@ -181,6 +181,8 @@ imt*at ihidev? # HID-over-i2c multitou
wsmouse* at imt? mux 0
iatp* at iic? # Atmel maXTouch i2c touchpad/touchscreen
wsmouse* at iatp? mux 0
+ietp* at iic? # Elantech i2c touchpad
+wsmouse* at ietp? mux 0
skgpio0 at isa? port 0x680 # Soekris net6501 GPIO and LEDs
gpio* at skgpio?
Index: dev/acpi/dwiic_acpi.c
===
RCS file: /cvs/src/sys/dev/acpi/dwiic_acpi.c,v
retrieving revision 1.13
diff -u -p -u -r1.13 dwiic_acpi.c
--- dev/acpi/dwiic_acpi.c 18 Feb 2020 12:13:39 - 1.13
+++ dev/acpi/dwiic_acpi.c 24 Mar 2020 23:44:55 -
@@ -50,6 +50,8 @@ int dwiic_acpi_found_ihidev(struct dwii
struct aml_node *, char *, struct dwiic_crs);
intdwiic_acpi_found_iatp(struct dwiic_softc *, struct aml_node *,
char *, struct dwiic_crs);
+intdwiic_acpi_found_ietp(struct dwiic_softc *, struct aml_node *,
+ char *, struct dwiic_crs);
void dwiic_acpi_get_params(struct dwiic_softc *, char *, uint16_t *,
uint16_t *, uint32_t *);
void dwiic_acpi_power(struct dwiic_softc *, int);
@@ -87,6 +89,11 @@ const char *iatp_hids[] = {
NULL
};
+const char *ietp_hids[] = {
+ "ELAN",
+ NULL
+};
+
int
dwiic_acpi_match(struct device *parent, void *match, void *aux)
{
@@ -389,6 +396,8 @@ dwiic_acpi_found_hid(struct aml_node *no
return dwiic_acpi_found_ihidev(sc, node, dev, crs);
else if (dwiic_matchhids(dev, iatp_hids))
return dwiic_acpi_found_iatp(sc, node, dev, crs);
+ else if (dwiic_matchhids(dev, ietp_hids))
+ return dwiic_acpi_found_ietp(sc, node, dev, crs);
memset(&ia, 0, sizeof(ia));
ia.ia_tag = sc->sc_iba.iba_tag;
@@ -491,6 +500,34 @@ dwiic_acpi_found_iatp(struct dwiic_softc
ia.ia_tag = sc->sc_iba.iba_tag;
ia.ia_size = 1;
ia.ia_name = "iatp";
+ ia.ia_addr = crs.i2c_addr;
+ ia.ia_cookie = dev;
+
+ if (crs.irq_int <= 0 && crs.gpio_int_node == NULL) {
+ printf("%s: couldn't find irq for %s\n", sc->sc_dev.dv_xname,
+ aml_nodename(node->parent));
+ return 0;
+ }
+ ia.ia_intr = &crs;
+
+ if (config_found(sc->sc_iic, &ia, dwiic_i2c_print)) {
+ node->parent->attached = 1;
+ return 0;
+ }
+
+ return 1;
+}
+
+int
+dwiic_acpi_found_ietp(struct dwiic_softc *sc, struct aml_node *node, char *dev,
+struct dwiic_crs crs)
+{
+ struct i2c_attach_args ia;
+
+ memset(&ia, 0, sizeof(ia));
+ ia.ia_tag = sc->sc_iba.iba_tag;
+ ia.ia_size = 1;
+ ia.ia_name = "ietp";
ia.ia_addr = crs.i2c_addr;
ia.ia_cookie = dev;
Index: dev/i2c/files.i2c
===
RCS file: /cvs/src/sys/dev/i2c/files.i2c,v
retrieving revision 1.64
diff -u -p -u -r1.64 files.i2c
--- dev/i2c/files.i2c 6 Sep 2019 09:38:19 - 1.64
+++ dev/i2c/files.i2c 24 Mar 2020 23:44:55 -
@@ -215,6 +215,11 @@ device iatp: wsmousedev
attach iatp at i2c
file dev/i2c/iatp.c iatp
+# Elantech trackpad
+device ietp: wsmousedev
+attach ietp at i2c
+file dev/i2c/ietp.c ietp
+
# Bosch BMC150 6-axis eCompass
device bgw
attach bgw at i2c
Index: dev/i2c/ietp.c
===
RCS file: dev/i2c/ietp.c
diff -N dev/i2c/ietp.c
--- /dev/null 1 Jan 1970 00:00:00 -
+++ dev/i2c/ietp.c 24 Mar 2020 23:44:55 -
@@ -0,0 +1,529 @@
+/*
+ * Elantech i2c touchpad driver
+ *
+ * Copyright (c) 2018 Ben Pye
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TO
Re: Regarding the understanding of the malloc(3) code
On Wed, Mar 25, 2020 at 01:54:51AM +0530, Neeraj Pal wrote:
> Hi Otto,
>
> I am having two small issues or confusions:
>
> First Query:
>
> 885 /*
> 886 * Allocate a page of chunks
> 887 */
> 888 static struct chunk_info *
> 889 omalloc_make_chunks(struct dir_info *d, int bits, int listnum)
> 890 {
> 891 struct chunk_info *bp;
> 892 void *pp;
> 893
> 894 /* Allocate a new bucket */
> 895 pp = map(d, NULL, MALLOC_PAGESIZE, 0);
> 896 if (pp == MAP_FAILED)
> 897 return NULL;
> 898
> 899 /* memory protect the page allocated in the malloc(0) case */
> 900 if (bits == 0 && mprotect(pp, MALLOC_PAGESIZE, PROT_NONE) ==
> -1)
> 901 goto err;
> 902
> 903 bp = alloc_chunk_info(d, bits);
> 904 if (bp == NULL)
> 905 goto err;
> 906 bp->page = pp;
> 907
> 908 if (insert(d, (void *)((uintptr_t)pp | (bits + 1)),
> (uintptr_t)bp,
> 909 NULL))
> 910 goto err;
> 911 LIST_INSERT_HEAD(&d->chunk_dir[bits][listnum], bp, entries);
> 912 return bp;
> 913
> 914 err:
> 915 unmap(d, pp, MALLOC_PAGESIZE, 0, d->malloc_junk);
> 916 return NULL;
> 917 }
>
> So, actually, as per the comment on line no. 885 in the above code, it is
> mentioned that it will allocate a page of chunks. But, what I have observed
> as from the code that pp is the new bucket means pp is the page which is
> full of chunks or which has chunks. As we can see on line 905, it calls
> alloc_chunk_info() function then inside that it calls init_chunk_info(),
> so, in short, we can say that first, it will allocate some chunk and then
> initialized that allocated chunk and then returns the same.
pp points to a page of chunks
bp point to the associated meta info: a bitmap that says which chunks
in the page are free. The bitmap is an aray of shorts, so 16 bits per entry.
>
> So, if we compare from here then it means, bp is the allocated and
> initialized chunk and the bp->page = pp, means it stores the page pp to
> bp->page. Then, after the hash table, it returns bp, means it returns the
> allocated-initialized chunk. But at the same time, I was referring the
> https://junk.tintagel.pl/openbsd-daily-malloc-3.txt by @mulander where he
> mentioned that "so bp is a page of chunks". So, I became little confused
> because pp is the page of chunks, which is used in function malloc_bytes()
> where it calculates the page offset and adds it to page bp->page, which is
> used by the user to input or writes stuff, like it returns address bp->page
> + k addr returns by malloc(3).
again bp is the meta info. bp->page is the page of chunks itself
>
> Second Query:
>
> And, bp->page is the pp and k is the offset, so, is it possible to get the
> address of the specific chunk because (bp->page + k) belongs to some chunks
> or we can say k is the index of the chunk that is inside the bucket
> bp->page or pp?
in the code k is first is the chunk number, and then multiplied (by
shifting it by bp->shift) to get the byte offset of the chunk inside
the chunk page.
>
>
> And in the structure chunk_info, u_short bits[1] is the bit for tracking
> whether the chunk is free or not. So, it belongs to each and every chunk.
> For example, there are 10 chunks in a page and 5fth chunk is not free then
> it will set that bits[1] to 0 and other 9 will be 1.
>
> OR
>
> Is it like bits denote the bits, like in the function init_chunk_info, in
> the end, it copies 0xff bytes to p->bits with size 30. Then it calculates
> p->bits[15] = 65535, so it is like it makes the last bit to 1.
p->bits is a bit mask. Each short in it holds 16 bits, so the first 16
chunks end uo in the first short, the next in the 2nd short etc.
The *lp ^= 1 << k line actuall sets the bit.
> sometimes I also have things in my mind like if bits[1] then how it is
> possible to assignbits[15] it means it performs the operation on bits. that
> I have analyzed by debugging.
>
> I have referred the paper for the understanding of bits[1] value,
> http://www.ouah.org/BSD-heap-smashing.txt. Actually not have proper
> confidence of understanding on bits logic.
>
> Apart from the issues discussed above, I mostly understood most of the
> stuff on malloc(3) but for the above still not getting the convinsible
> understainding.
>
>
> Regards,
> Neeraj
Re: Regarding the understanding of the malloc(3) code
Hi Otto,
I am having two small issues or confusions:
First Query:
885 /*
886 * Allocate a page of chunks
887 */
888 static struct chunk_info *
889 omalloc_make_chunks(struct dir_info *d, int bits, int listnum)
890 {
891 struct chunk_info *bp;
892 void *pp;
893
894 /* Allocate a new bucket */
895 pp = map(d, NULL, MALLOC_PAGESIZE, 0);
896 if (pp == MAP_FAILED)
897 return NULL;
898
899 /* memory protect the page allocated in the malloc(0) case */
900 if (bits == 0 && mprotect(pp, MALLOC_PAGESIZE, PROT_NONE) ==
-1)
901 goto err;
902
903 bp = alloc_chunk_info(d, bits);
904 if (bp == NULL)
905 goto err;
906 bp->page = pp;
907
908 if (insert(d, (void *)((uintptr_t)pp | (bits + 1)),
(uintptr_t)bp,
909 NULL))
910 goto err;
911 LIST_INSERT_HEAD(&d->chunk_dir[bits][listnum], bp, entries);
912 return bp;
913
914 err:
915 unmap(d, pp, MALLOC_PAGESIZE, 0, d->malloc_junk);
916 return NULL;
917 }
So, actually, as per the comment on line no. 885 in the above code, it is
mentioned that it will allocate a page of chunks. But, what I have observed
as from the code that pp is the new bucket means pp is the page which is
full of chunks or which has chunks. As we can see on line 905, it calls
alloc_chunk_info() function then inside that it calls init_chunk_info(),
so, in short, we can say that first, it will allocate some chunk and then
initialized that allocated chunk and then returns the same.
So, if we compare from here then it means, bp is the allocated and
initialized chunk and the bp->page = pp, means it stores the page pp to
bp->page. Then, after the hash table, it returns bp, means it returns the
allocated-initialized chunk. But at the same time, I was referring the
https://junk.tintagel.pl/openbsd-daily-malloc-3.txt by @mulander where he
mentioned that "so bp is a page of chunks". So, I became little confused
because pp is the page of chunks, which is used in function malloc_bytes()
where it calculates the page offset and adds it to page bp->page, which is
used by the user to input or writes stuff, like it returns address bp->page
+ k addr returns by malloc(3).
Second Query:
And, bp->page is the pp and k is the offset, so, is it possible to get the
address of the specific chunk because (bp->page + k) belongs to some chunks
or we can say k is the index of the chunk that is inside the bucket
bp->page or pp?
And in the structure chunk_info, u_short bits[1] is the bit for tracking
whether the chunk is free or not. So, it belongs to each and every chunk.
For example, there are 10 chunks in a page and 5fth chunk is not free then
it will set that bits[1] to 0 and other 9 will be 1.
OR
Is it like bits denote the bits, like in the function init_chunk_info, in
the end, it copies 0xff bytes to p->bits with size 30. Then it calculates
p->bits[15] = 65535, so it is like it makes the last bit to 1.
sometimes I also have things in my mind like if bits[1] then how it is
possible to assignbits[15] it means it performs the operation on bits. that
I have analyzed by debugging.
I have referred the paper for the understanding of bits[1] value,
http://www.ouah.org/BSD-heap-smashing.txt. Actually not have proper
confidence of understanding on bits logic.
Apart from the issues discussed above, I mostly understood most of the
stuff on malloc(3) but for the above still not getting the convinsible
understainding.
Regards,
Neeraj
Re: [PLEASE TEST] acpi(4): acpi_sleep(): tsleep(9) -> tsleep_nsec(9)
> Date: Tue, 24 Mar 2020 12:52:55 -0500
> From: Scott Cheloha
>
> On Tue, Mar 24, 2020 at 09:17:34AM +0100, Mark Kettenis wrote:
> > > Date: Mon, 23 Mar 2020 21:57:46 -0500
> > > From: Scott Cheloha
> > >
> > > 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.
> >
> > AMLOP_SLEEP can occur anywhere when executing AML code.
>
> Oh, good to know.
>
> > The current code tries to protect against negative timeouts, but
> > your new code doesn't?
>
> What would a negative value here actually mean? A firmware bug?
> Should we log that? Or panic?
>
> The simplest thing would be to just MAX it up to 1. Which I have
> done in this patch.
>
> But it looks like there are other "what if we get a negative value
> from the firmware" problems in this file. I dunno if you want to
> address all of them.
>
> Related:
>
> I'm looking around at these functions accepting .v_integer as
> argument: acpi_stall, acpi_sleep(), acpi_event_wait(). They all take
> ints, but aml_value.v_integer is an int64_t. So there's implicit
> type-casting.
>
> Should we change these functions to handle 64-bit integers? Or should
> we clamp .v_integer to within [INT_MIN, INT_MAX]?
The ACPI code isn't the best code in our tree I fear. One issue here
is that ACPI 1.0 had 32-bit integers, which became 640-bit in ACPI
2.0. Treating all integers as 64-bit numbers seems to work fine
though. So acpi_sleep() should really accept a 64-bit integer as an
argument. And ACPI says that integers are unsigned, so the sign
problem should never occur and our codebase is just plain doing it
wrong...
Here is the description of the operation lifted form the ACPI 6.3 spec:
19.6.125 Sleep (Milliseconds Sleep)
Syntax
Sleep (MilliSeconds)
Arguments
The Sleep term is used to implement long-term timing
requirements. Execution is delayed for at least the required
number of milliseconds.
Description
The implementation of Sleep is to round the request up to the
closest sleep time supported by the OS and relinquish the
processor.
> 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.c16 Oct 2019 01:43:50 - 1.249
> +++ dsdt.c24 Mar 2020 17:50:05 -
> @@ -465,15 +465,13 @@ void
> acpi_sleep(int ms, char *reason)
> {
> static int acpinowait;
> - int to = ms * hz / 1000;
> +
> + ms = MAX(1, ms);
>
> if (cold)
> delay(ms * 1000);
> - else {
> - if (to <= 0)
> - to = 1;
> - tsleep(&acpinowait, PWAIT, reason, to);
> - }
> + else
> + tsleep_nsec(&acpinowait, PWAIT, reason, MSEC_TO_NSEC(ms));
> }
>
> void
>
Re: xorg packaging issue
On Tue, Mar 24, 2020 at 01:26:36PM +0100, Marc Espie wrote: > There's some inconsistency: > lists/xserv/mi:./usr/X11R6/share/aclocal/xorg-macros.m4 > lists/xshare/mi:./usr/X11R6/lib/pkgconfig/xorg-macros.pc > > both should be in xshare It was moved to xserv by fries@ some years ago for I don't know what reason. Fixed now. Thanks. > > This does explain the failures of tightvnc on some bulk machines. > By default, proot does NOT copy xserv in the chroot, and I believe > it's correct. -- Matthieu Herrb
Re: [PLEASE TEST] acpi(4): acpi_sleep(): tsleep(9) -> tsleep_nsec(9)
On Tue, Mar 24, 2020 at 09:17:34AM +0100, Mark Kettenis wrote:
> > Date: Mon, 23 Mar 2020 21:57:46 -0500
> > From: Scott Cheloha
> >
> > 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.
>
> AMLOP_SLEEP can occur anywhere when executing AML code.
Oh, good to know.
> The current code tries to protect against negative timeouts, but
> your new code doesn't?
What would a negative value here actually mean? A firmware bug?
Should we log that? Or panic?
The simplest thing would be to just MAX it up to 1. Which I have
done in this patch.
But it looks like there are other "what if we get a negative value
from the firmware" problems in this file. I dunno if you want to
address all of them.
Related:
I'm looking around at these functions accepting .v_integer as
argument: acpi_stall, acpi_sleep(), acpi_event_wait(). They all take
ints, but aml_value.v_integer is an int64_t. So there's implicit
type-casting.
Should we change these functions to handle 64-bit integers? Or should
we clamp .v_integer to within [INT_MIN, INT_MAX]?
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 24 Mar 2020 17:50:05 -
@@ -465,15 +465,13 @@ void
acpi_sleep(int ms, char *reason)
{
static int acpinowait;
- int to = ms * hz / 1000;
+
+ ms = MAX(1, ms);
if (cold)
delay(ms * 1000);
- else {
- if (to <= 0)
- to = 1;
- tsleep(&acpinowait, PWAIT, reason, to);
- }
+ else
+ tsleep_nsec(&acpinowait, PWAIT, reason, MSEC_TO_NSEC(ms));
}
void
Re: Dead code in uvm_pageout()
> Date: Tue, 24 Mar 2020 16:11:48 +0100 > From: Martin Pieuchot > > On 24/03/20(Tue) 15:55, Mark Kettenis wrote: > > > Date: Tue, 24 Mar 2020 15:18:13 +0100 > > > From: Martin Pieuchot > > > > > > As soon as an entry is found on `pmr_control.allocs' the boolean > > > `work_done' will be set to true. So it is impossible to reach the > > > case below that sets UVM_PMA_FAIL. > > > > > > CID 1453061 Logically dead code > > > > > > Ok? > > > > Almost certainly not. > > Could you elaborate? Are you implying this is the symptom of a deeper > bug? Yes.
Re: uvm_mapent_alloc() & NULL check
> Date: Tue, 24 Mar 2020 16:08:56 +0100
> From: Martin Pieuchot
>
> Variable `me' is never NULL before reaching RBT_POISON(). Diff has a
> lot of context to ease the review.
>
> CID 1453116 Dereference before null check
>
> ok?
ok kettenis@
> Index: uvm/uvm_map.c
> ===
> RCS file: /cvs/src/sys/uvm/uvm_map.c,v
> retrieving revision 1.263
> diff -u -p -u -2 -0 -r1.263 uvm_map.c
> --- uvm/uvm_map.c 4 Mar 2020 21:15:38 - 1.263
> +++ uvm/uvm_map.c 24 Mar 2020 15:06:26 -
> @@ -1736,44 +1736,41 @@ uvm_mapent_alloc(struct vm_map *map, int
> }
> me = SLIST_FIRST(&uvm.kentry_free);
> SLIST_REMOVE_HEAD(&uvm.kentry_free, daddrs.addr_kentry);
> uvmexp.kmapent++;
> mtx_leave(&uvm_kmapent_mtx);
> me->flags = UVM_MAP_STATIC;
> } else if (map == kernel_map) {
> splassert(IPL_NONE);
> me = pool_get(&uvm_map_entry_kmem_pool, pool_flags);
> if (me == NULL)
> goto out;
> me->flags = UVM_MAP_KMEM;
> } else {
> splassert(IPL_NONE);
> me = pool_get(&uvm_map_entry_pool, pool_flags);
> if (me == NULL)
> goto out;
> me->flags = 0;
> }
>
> - if (me != NULL) {
> - RBT_POISON(uvm_map_addr, me, UVMMAP_DEADBEEF);
> - }
> -
> + RBT_POISON(uvm_map_addr, me, UVMMAP_DEADBEEF);
> out:
> return(me);
> }
>
> /*
> * uvm_mapent_free: free map entry
> *
> * => XXX: static pool for kernel map?
> */
> void
> uvm_mapent_free(struct vm_map_entry *me)
> {
> if (me->flags & UVM_MAP_STATIC) {
> mtx_enter(&uvm_kmapent_mtx);
> SLIST_INSERT_HEAD(&uvm.kentry_free, me, daddrs.addr_kentry);
> uvmexp.kmapent--;
> mtx_leave(&uvm_kmapent_mtx);
> } else if (me->flags & UVM_MAP_KMEM) {
> splassert(IPL_NONE);
> pool_put(&uvm_map_entry_kmem_pool, me);
>
>
Re: Dead code in uvm_pageout()
On 24/03/20(Tue) 15:55, Mark Kettenis wrote: > > Date: Tue, 24 Mar 2020 15:18:13 +0100 > > From: Martin Pieuchot > > > > As soon as an entry is found on `pmr_control.allocs' the boolean > > `work_done' will be set to true. So it is impossible to reach the > > case below that sets UVM_PMA_FAIL. > > > > CID 1453061 Logically dead code > > > > Ok? > > Almost certainly not. Could you elaborate? Are you implying this is the symptom of a deeper bug?
uvm_mapent_alloc() & NULL check
Variable `me' is never NULL before reaching RBT_POISON(). Diff has a
lot of context to ease the review.
CID 1453116 Dereference before null check
ok?
Index: uvm/uvm_map.c
===
RCS file: /cvs/src/sys/uvm/uvm_map.c,v
retrieving revision 1.263
diff -u -p -u -2 -0 -r1.263 uvm_map.c
--- uvm/uvm_map.c 4 Mar 2020 21:15:38 - 1.263
+++ uvm/uvm_map.c 24 Mar 2020 15:06:26 -
@@ -1736,44 +1736,41 @@ uvm_mapent_alloc(struct vm_map *map, int
}
me = SLIST_FIRST(&uvm.kentry_free);
SLIST_REMOVE_HEAD(&uvm.kentry_free, daddrs.addr_kentry);
uvmexp.kmapent++;
mtx_leave(&uvm_kmapent_mtx);
me->flags = UVM_MAP_STATIC;
} else if (map == kernel_map) {
splassert(IPL_NONE);
me = pool_get(&uvm_map_entry_kmem_pool, pool_flags);
if (me == NULL)
goto out;
me->flags = UVM_MAP_KMEM;
} else {
splassert(IPL_NONE);
me = pool_get(&uvm_map_entry_pool, pool_flags);
if (me == NULL)
goto out;
me->flags = 0;
}
- if (me != NULL) {
- RBT_POISON(uvm_map_addr, me, UVMMAP_DEADBEEF);
- }
-
+ RBT_POISON(uvm_map_addr, me, UVMMAP_DEADBEEF);
out:
return(me);
}
/*
* uvm_mapent_free: free map entry
*
* => XXX: static pool for kernel map?
*/
void
uvm_mapent_free(struct vm_map_entry *me)
{
if (me->flags & UVM_MAP_STATIC) {
mtx_enter(&uvm_kmapent_mtx);
SLIST_INSERT_HEAD(&uvm.kentry_free, me, daddrs.addr_kentry);
uvmexp.kmapent--;
mtx_leave(&uvm_kmapent_mtx);
} else if (me->flags & UVM_MAP_KMEM) {
splassert(IPL_NONE);
pool_put(&uvm_map_entry_kmem_pool, me);
Re: Dead code in uvm_pageout()
> Date: Tue, 24 Mar 2020 15:18:13 +0100
> From: Martin Pieuchot
>
> As soon as an entry is found on `pmr_control.allocs' the boolean
> `work_done' will be set to true. So it is impossible to reach the
> case below that sets UVM_PMA_FAIL.
>
> CID 1453061 Logically dead code
>
> Ok?
Almost certainly not.
> Index: uvm/uvm_pdaemon.c
> ===
> RCS file: /cvs/src/sys/uvm/uvm_pdaemon.c,v
> retrieving revision 1.85
> diff -u -p -u -4 -r1.85 uvm_pdaemon.c
> --- uvm/uvm_pdaemon.c 30 Dec 2019 23:58:38 - 1.85
> +++ uvm/uvm_pdaemon.c 24 Mar 2020 14:12:22 -
> @@ -209,9 +209,8 @@ void
> uvm_pageout(void *arg)
> {
> struct uvm_constraint_range constraint;
> struct uvm_pmalloc *pma;
> - int work_done;
> int npages = 0;
>
> /* ensure correct priority and set paging parameters... */
> uvm.pagedaemon_proc = curproc;
> @@ -222,9 +221,8 @@ uvm_pageout(void *arg)
> uvm_unlock_pageq();
>
> for (;;) {
> long size;
> - work_done = 0; /* No work done this iteration. */
>
> uvm_lock_fpageq();
> if (!uvm_nowait_failed && TAILQ_EMPTY(&uvm.pmr_control.allocs))
> {
> msleep_nsec(&uvm.pagedaemon, &uvm.fpageqlock, PVM,
> @@ -281,9 +279,8 @@ uvm_pageout(void *arg)
> if (pma != NULL ||
> ((uvmexp.free - BUFPAGES_DEFICIT) < uvmexp.freetarg) ||
> ((uvmexp.inactive + BUFPAGES_INACT) < uvmexp.inactarg)) {
> uvmpd_scan();
> - work_done = 1; /* XXX we hope... */
> }
>
> /*
>* if there's any free memory to be had,
> @@ -296,10 +293,8 @@ uvm_pageout(void *arg)
> }
>
> if (pma != NULL) {
> pma->pm_flags &= ~UVM_PMA_BUSY;
> - if (!work_done)
> - pma->pm_flags |= UVM_PMA_FAIL;
> if (pma->pm_flags & (UVM_PMA_FAIL | UVM_PMA_FREED)) {
> pma->pm_flags &= ~UVM_PMA_LINKED;
> TAILQ_REMOVE(&uvm.pmr_control.allocs, pma,
> pmq);
>
>
Dead code in uvm_pageout()
As soon as an entry is found on `pmr_control.allocs' the boolean
`work_done' will be set to true. So it is impossible to reach the
case below that sets UVM_PMA_FAIL.
CID 1453061 Logically dead code
Ok?
Index: uvm/uvm_pdaemon.c
===
RCS file: /cvs/src/sys/uvm/uvm_pdaemon.c,v
retrieving revision 1.85
diff -u -p -u -4 -r1.85 uvm_pdaemon.c
--- uvm/uvm_pdaemon.c 30 Dec 2019 23:58:38 - 1.85
+++ uvm/uvm_pdaemon.c 24 Mar 2020 14:12:22 -
@@ -209,9 +209,8 @@ void
uvm_pageout(void *arg)
{
struct uvm_constraint_range constraint;
struct uvm_pmalloc *pma;
- int work_done;
int npages = 0;
/* ensure correct priority and set paging parameters... */
uvm.pagedaemon_proc = curproc;
@@ -222,9 +221,8 @@ uvm_pageout(void *arg)
uvm_unlock_pageq();
for (;;) {
long size;
- work_done = 0; /* No work done this iteration. */
uvm_lock_fpageq();
if (!uvm_nowait_failed && TAILQ_EMPTY(&uvm.pmr_control.allocs))
{
msleep_nsec(&uvm.pagedaemon, &uvm.fpageqlock, PVM,
@@ -281,9 +279,8 @@ uvm_pageout(void *arg)
if (pma != NULL ||
((uvmexp.free - BUFPAGES_DEFICIT) < uvmexp.freetarg) ||
((uvmexp.inactive + BUFPAGES_INACT) < uvmexp.inactarg)) {
uvmpd_scan();
- work_done = 1; /* XXX we hope... */
}
/*
* if there's any free memory to be had,
@@ -296,10 +293,8 @@ uvm_pageout(void *arg)
}
if (pma != NULL) {
pma->pm_flags &= ~UVM_PMA_BUSY;
- if (!work_done)
- pma->pm_flags |= UVM_PMA_FAIL;
if (pma->pm_flags & (UVM_PMA_FAIL | UVM_PMA_FREED)) {
pma->pm_flags &= ~UVM_PMA_LINKED;
TAILQ_REMOVE(&uvm.pmr_control.allocs, pma,
pmq);
Re: Regarding the understanding of the malloc(3) code
On Wed, Mar 18, 2020 at 4:01 PM Otto Moerbeek wrote: > Not all meta-data canaries live in r/o memory. > > A thing that also helps is to follow the cvs history of a file. The > first version of my malloc (form 2008) was more simple, and looking at > the diffs through the years gives you great hints at what features > were added over the years, plus a few bugfixes. > Thanks for the tip. I have gone through GitHub history and also read CVS history. It was very helpful. Also got some information from the pkhmalloc code and some old papers. And, I have read and tried to understand the malloc internals. So, I have shared whatever I have learned on the blog but I haven't published it yet. I have sent the link to you @drijf.net for review as I don't want to convey any wrong understanding or misguiding the people. So, requesting you to review it whenever you have time. Thanks and regards, Neeraj
Re: Saving stack frames in dt(4)
On 23/03/20(Mon) 15:18, Martin Pieuchot wrote:
> 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?
New diff, new approach suggested by visa@. Passes a number of frames to
skip to the unwinding function. This is compatible with the mips64
unwinder and makes sure that the __builtin_frame_address() isn't called
with an index other than 0.
Indexes have been adjusted because __builtin_frame_address(0) is now in
a deeper frame.
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 24 Mar 2020 13:03:35 -
@@ -256,14 +256,17 @@ db_stack_trace_print(db_expr_t addr, int
}
void
-stacktrace_save(struct stacktrace *st)
+stacktrace_save_at(struct stacktrace *st, unsigned int skip)
{
struct callframe *frame, *lastframe;
frame = __builtin_frame_address(0);
st->st_count = 0;
while (st->st_count < STACKTRACE_MAX) {
- st->st_pc[st->st_count++] = frame->f_retaddr;
+ if (skip == 0)
+ st->st_pc[st->st_count++] = frame->f_retaddr;
+ else
+ skip--;
lastframe = frame;
frame = frame->f_frame;
@@ -275,6 +278,12 @@ stacktrace_save(struct stacktrace *st)
if (!INKERNEL(frame->f_retaddr))
break;
}
+}
+
+void
+stacktrace_save(struct stacktrace *st)
+{
+ return stacktrace_save_at(st, 0);
}
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 24 Mar 2020 12:59:34 -
@@ -261,14 +261,17 @@ db_stack_trace_print(db_expr_t addr, int
}
void
-stacktrace_save(struct stacktrace *st)
+stacktrace_save_at(struct stacktrace *st, unsigned int skip)
{
struct callframe *frame, *lastframe;
frame = __builtin_frame_address(0);
st->st_count = 0;
while (st->st_count < STACKTRACE_MAX) {
- st->st_pc[st->st_count++] = frame->f_retaddr;
+ if (skip == 0)
+ st->st_pc[st->st_count++] = frame->f_retaddr;
+
Re: iked users database misses entries after ikectl reload
On Mon, Mar 23, 2020 at 05:53:00PM -0300, Bernardo Cunha Vieira wrote:
> Hi,
> This fixes the users' database corruption after an iked reload.
> The old code overwrites the pointers in the RB tree, losing users
> if a list of users is provided in config file.
> Regards,
> Bernardo
Good find, thanks! I committed your diff.
>
> Index: config.c
> ===
> RCS file: /cvs/src/sbin/iked/config.c,v
> retrieving revision 1.54
> diff -u -p -r1.54 config.c
> --- config.c 9 Mar 2020 11:50:43 - 1.54
> +++ config.c 23 Mar 2020 19:19:07 -
> @@ -434,7 +434,7 @@ config_new_user(struct iked *env, struct
>
> if ((old = RB_INSERT(iked_users, &env->sc_users, usr)) != NULL) {
> /* Update the password of an existing user*/
> - memcpy(old, new, sizeof(*old));
> + memcpy(old->usr_pass, new->usr_pass, IKED_PASSWORD_SIZE);
>
> log_debug("%s: updating user %s", __func__, usr->usr_name);
> free(usr);
>
bootblocks ffs2 support for sparc64
Hi,
As some of you know I have been working on the ability to boot from an
ffs2 root partition on as many platforms as possible. Many platforms
are done, but sparc64, landisk, octeon and luna88k remain.
sparc64 uses bootblock written in Forth that interpret the filesystem
on the boot disk.
This diff takes the bootblk changes netbsd did to support ffs2 and
merges it with the softraid changes stsp@ did a few years back. Result
is bootblocks that can load the 2nd stage bootloader from ffs1, ffs2
and softraid.
Please test it does not break your current setup with either ffs1 or
softraid, procedure is:
1. Make sure you have the very recent fgen changes
2. Build and install in sys/arch/sparc64/stand/bootblk
3. installboot (for softraid setups this is not the
physical disk, but the softraid).
4. Reboot and make sure you are using the right version of the
bootblocks: 2.0.
Note that this does not actually enable ffs2 for installs. That comes
later, first I need to make sure I did not break existing setups.
-Otto
Index: sys/arch/sparc64/stand/bootblk/Makefile
===
RCS file: /cvs/src/sys/arch/sparc64/stand/bootblk/Makefile,v
retrieving revision 1.14
diff -u -p -r1.14 Makefile
--- sys/arch/sparc64/stand/bootblk/Makefile 17 Oct 2017 19:31:56 -
1.14
+++ sys/arch/sparc64/stand/bootblk/Makefile 24 Mar 2020 07:33:45 -
@@ -8,8 +8,8 @@ S= ${CURDIR}/../../../..
# Override normal settings
#
-CLEANFILES=assym.fth.h assym.fth.h.tmp machine \
- bootblk bootblk.text bootblk.text.tmp
+CLEANFILES=machine ffs.fth.h \
+ bootblk bootblk.text bootblk.text.tmp -.d
NOMAN=
STRIPFLAG=
@@ -26,17 +26,17 @@ CPPFLAGS= ${INCLUDES} ${IDENT} ${PARAM}
all: bootblk.text bootblk
-assym.fth.h: ${.CURDIR}/genassym.sh genfth.cf
- sh ${.CURDIR}/genassym.sh ${CC} ${CFLAGS} \
- ${CPPFLAGS} ${PROF} <${.CURDIR}/genfth.cf >assym.fth.h.tmp && \
- mv -f assym.fth.h.tmp assym.fth.h
-
-bootblk.text: bootblk.fth assym.fth.h
- awk '/fload/ { file=$$2; while ((ret = getline "/dev/stderr"; next
}; !/fload/' \
- ${.CURDIR}/bootblk.fth >bootblk.text.tmp && \
+ffs.fth.h: ${.CURDIR}/genassym.sh genfth.cf
+ sh ${.CURDIR}/genassym.sh -f ${CC} ${CFLAGS} ${CPPFLAGS} ${PROF} \
+ < ${.CURDIR}/genfth.cf >ffs.fth.h.tmp && \
+ mv -f ffs.fth.h.tmp ffs.fth.h
+
+bootblk.text: bootblk.fth ffs.fth.h
+ awk '/fload/ { print "#include \"" $$2 "\"" }; !/fload/' \
+ ${.CURDIR}/bootblk.fth | /usr/bin/cpp -P > bootblk.text.tmp && \
mv -f bootblk.text.tmp bootblk.text
-bootblk: bootblk.fth assym.fth.h
+bootblk: bootblk.fth ffs.fth.h
fgen -o bootblk ${.CURDIR}/bootblk.fth
beforeinstall:
Index: sys/arch/sparc64/stand/bootblk/bootblk.fth
===
RCS file: /cvs/src/sys/arch/sparc64/stand/bootblk/bootblk.fth,v
retrieving revision 1.8
diff -u -p -r1.8 bootblk.fth
--- sys/arch/sparc64/stand/bootblk/bootblk.fth 26 Nov 2014 19:57:41 -
1.8
+++ sys/arch/sparc64/stand/bootblk/bootblk.fth 24 Mar 2020 07:33:45 -
@@ -1,12 +1,12 @@
-\ $OpenBSD: bootblk.fth,v 1.8 2014/11/26 19:57:41 stsp Exp $
-\ $NetBSD: bootblk.fth,v 1.3 2001/08/15 20:10:24 eeh Exp $
+\ $OpenBSD$
+\ $NetBSD: bootblk.fth,v 1.15 2015/08/20 05:40:08 dholland Exp $
\
\ IEEE 1275 Open Firmware Boot Block
\
\ Parses disklabel and UFS and loads the file called `ofwboot'
\
\
-\ Copyright (c) 1998 Eduardo Horvath.
+\ Copyright (c) 1998-2010 Eduardo Horvath.
\ All rights reserved.
\
\ Redistribution and use in source and binary forms, with or without
@@ -17,11 +17,6 @@
\ 2. Redistributions in binary form must reproduce the above copyright
\ notice, this list of conditions and the following disclaimer in the
\ documentation and/or other materials provided with the distribution.
-\ 3. All advertising materials mentioning features or use of this software
-\ must display the following acknowledgement:
-\ This product includes software developed by Eduardo Horvath.
-\ 4. The name of the author may not be used to endorse or promote products
-\ derived from this software without specific prior written permission
\
\ THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
\ IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
WARRANTIES
@@ -41,6 +36,8 @@ headers
false value boot-debug?
+: KB d# 1024 * ;
+
\
\ First some housekeeping: Open /chosen and set up vectors into
\ client-services
@@ -61,15 +58,15 @@ defer cif-seek ( low high ihandle -- -1|
\ defer cif-peer ( phandle -- phandle )
\ defer cif-getprop ( len adr cstr phandle -- )
-: find-cif-method ( method,len -- xf )
+: find-cif-method ( method len -- xf )
cif-phandle find-meth
xorg packaging issue
There's some inconsistency: lists/xserv/mi:./usr/X11R6/share/aclocal/xorg-macros.m4 lists/xshare/mi:./usr/X11R6/lib/pkgconfig/xorg-macros.pc both should be in xshare This does explain the failures of tightvnc on some bulk machines. By default, proot does NOT copy xserv in the chroot, and I believe it's correct.
iked users database misses entries after ikectl reload
Hi,
This fixes the users' database corruption after an iked reload.
The old code overwrites the pointers in the RB tree, losing users
if a list of users is provided in config file.
Regards,
Bernardo
Index: config.c
===
RCS file: /cvs/src/sbin/iked/config.c,v
retrieving revision 1.54
diff -u -p -r1.54 config.c
--- config.c 9 Mar 2020 11:50:43 - 1.54
+++ config.c 23 Mar 2020 19:19:07 -
@@ -434,7 +434,7 @@ config_new_user(struct iked *env, struct
if ((old = RB_INSERT(iked_users, &env->sc_users, usr)) != NULL) {
/* Update the password of an existing user*/
- memcpy(old, new, sizeof(*old));
+ memcpy(old->usr_pass, new->usr_pass, IKED_PASSWORD_SIZE);
log_debug("%s: updating user %s", __func__, usr->usr_name);
free(usr);
Re: Untangle proc * in VOP_*
On 23/03/20(Mon) 18:51, Ted Unangst wrote: > [...] > If this is a temporary measure, I think it should include some > annotation to that effect, so that it doesn't continue spreading. It isn't, it's to help review or upcoming diffs :o) It's not to make this nice it's to tedu it!
Re: [PLEASE TEST] acpi(4): acpi_sleep(): tsleep(9) -> tsleep_nsec(9)
> Date: Mon, 23 Mar 2020 21:57:46 -0500
> From: Scott Cheloha
>
> 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.
AMLOP_SLEEP can occur anywhere when executing AML code. The current
code tries to protect against negative timeouts, but your new code
doesn't?
> 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.c16 Oct 2019 01:43:50 - 1.249
> +++ dsdt.c16 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(&acpinowait, PWAIT, reason, to);
> - }
> + else
> + tsleep_nsec(&acpinowait, PWAIT, reason, MSEC_TO_NSEC(ms));
> }
>
> void
>
>
Re: rtsock: redundant NULL pointer check
On Mon, Mar 23, 2020 at 10:36:20PM +0100, Tobias Heider wrote:
> It seems that there is no way 'rtm' could actually be NULL here, which
> means we can get rid of the check.
>
> ok?
OK claudio@
> 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.c 24 Nov 2019 07:56:03 - 1.297
> +++ net/rtsock.c 23 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);
>
--
:wq Claudio
