David Hill wrote:
> Hello -
>
> The following diff adds free sizes to free() calls in uvm/. Only one
> remaining in uvm/.
ok stefan@
> Index: uvm/uvm_amap.c
> ===
> RCS file: /cvs/src/sys/uvm/uvm_amap.c,v
> retrieving revision
Jonathan Gray wrote:
> Base gcc4 changes the defaults to set -Wno-pointer-sign.
> Base clang does not, I'm not sure where in the llvm code to do so.
> Base gcc3 does not handle -Wno-pointer-sign.
I think this should turn off -Wpointer-sign off by default.
Passing -Wpointer-sign on the command
When uvm pivots are enabled, allocations
with an address hint provided by the user program are
handled by the uaddr_hint selector.
I'd like to remove the uaddr_hint selector. The uaddr_rnd selector
already deals with hinted allocations correctly, so why not use
it for hinted allocations in the
Theo Buehler wrote:
> On Sat, Dec 24, 2016 at 10:45:16PM +0100, Theo Buehler wrote:
> > On Sat, Dec 24, 2016 at 05:44:07PM +0100, Jérôme FRGACIC wrote:
> > > Hi @tech,
> > >
> > > I remark that ed(1) do not support adress ranges which begin with
> > > comma or semicolon, for example ",10p" which
Otto Moerbeek wrote:
> Hi,
>
> Pages in the malloc cache are either reused quickly or unmapped
> quickly. In both cases it does not make sense to set hints on them.
> So remove that option, which is just a remainder of old times when
> mallco used to hold on to pages. OK?
Makes sense. ok stefan@
Philip Guenther wrote:
> On Mon, Aug 1, 2016 at 11:45 AM, Mark Kettenis <mark.kette...@xs4all.nl>
> wrote:
> >> From: j...@wxcvbn.org (Jeremie Courreges-Anglas)
> >> Date: Mon, 01 Aug 2016 20:30:33 +0200
> >>
> >> Stefan Kempf <sisn
Philip Guenther wrote:
> On Mon, Aug 1, 2016 at 11:45 AM, Mark Kettenis <mark.kette...@xs4all.nl>
> wrote:
> >> From: j...@wxcvbn.org (Jeremie Courreges-Anglas)
> >> Date: Mon, 01 Aug 2016 20:30:33 +0200
> >>
> >> Stefan Kempf <sisn
The constructor and destructor tables are declared as arrays with one
non-NULL element. Walking those until a NULL element is reached looks
like out-of-bound accesses to newer compilers, and they turn the code
into infinite loops (e.g. clang 3.8), because it is undefined behavior.
Use
clang errors out about use of undefined variables when building
binutils-2.17.
In elf.c, do not increment `s' before it is initialized. At the time
of the increment, `s' is otherwise unused anyway.
In elflink.c, initialize sec_contents and l_sec_contents to make
sure that the free(sec_contents)
Ingo Schwarze wrote:
> Hi,
>
> when p->p_rlimit[RLIMIT_DATA].rlim_cur < ptoa(p->p_vmspace->vm_dused),
> the following comparison fails because the subtraction wraps around
> to huge positive values.
>
> I noticed this because i tried to do systematic testing of specific
> malloc failure
This re-introduces the vnode-to-filename mapping code with adaptions to
the new namecache layout.
For example, running procmap -a 1 now prints the .text, .data, and
.rodata segments of init(1) as below, if /sbin/init is in the
name cache:
104ccb70-104ccb73dfff 248k 0 r-xp+ (rwx) 1/0/0
Aaron Miller wrote:
> Hi All,
>
> I was experiencing ~8 minute linking times for a large C++ application
> I have been working on when running -current on amd64. It turns out
> that the decade-old version of ld in the OpenBSD source tree has a bug
> that causes quadratic complexity for some
Martin Natano wrote:
> There seem to be a number of issues with statfs related code in the
> kernel. The first issue is inside of the copy_statfs_info() function
> which is designed to be used by the filesystem's .vfs_statfs
> implementations to copy data from mp->mnt_stat to the target stat
>
Stefan Kempf wrote:
> Next try. A problem with the last version was that amaps for shared
> mappings still would grow pretty large: establishing a shared mapping
> causes uvm to immediately allocate an amap for the whole range.
I messed up the last diff.
tb@ found and fixed bug in t
Next try. A problem with the last version was that amaps for shared
mappings still would grow pretty large: establishing a shared mapping
causes uvm to immediately allocate an amap for the whole range.
That's a problem when changing vmm to use shared memory to allow vmd(8)
to access the memory of
_kvm_uread maps a userspace page to a slot in the amap, but
only checks whether the slot is indeed within the amap.
It must also use the slot to extract the correct vm_anon in order
to find the physical address to read from.
The attached program shows that kvm_uread otherwise attempts reads
from
am_maxslot represents the total number of slots an amap can be extended
to. Since we do not extend amaps, this field as well as rounding the
number of slots to the next malloc bucket is not useful.
This also removes the corresponding output from procmap(1).
ok?
Index: sys/uvm/uvm_amap.c
I'd like to commit this soon unless there are objections.
Stefan Kempf wrote:
> The recent uvm commits fixed hangs because machines went out of memory
> because of using too much space for amap slots.
>
> It's possible to shrink memory requirements for amaps even more,
> but t
Stefan Kempf wrote:
> Hi,
>
> here comes a diff for vmm, and I'd like to ask people that are
> interested in our hypervisor to test this. If you are experimenting
> with vmm already, just do what you always do with vmm when running
> with this diff :-)
>
> [...]
&g
Hi,
here comes a diff for vmm, and I'd like to ask people that are
interested in our hypervisor to test this. If you are experimenting
with vmm already, just do what you always do with vmm when running
with this diff :-)
How to apply the diff:
$ cd /usr/src
$ patch < vmm.diff
$ doas make
This flag caused amaps to be allocated with additional spare slots, to
make extending them cheaper. However, the kernel never extends amaps,
so allocating spare slots is pointless. Also UVM_FLAG_AMAPPAD only
has an effect in combination with UVM_FLAG_OVERLAY. The only function
that used both flags
Miod Vallat wrote:
>
> > It seems per-page reference counting is used since forever. I think
> > there's no reason to ever turn it off (and track referenced pages
> > with less accuracy, causing leaks).
>
> Actually, assuming the #undef code path works, it might work keeping
> this and only
It seems per-page reference counting is used since forever. I think
there's no reason to ever turn it off (and track referenced pages
with less accuracy, causing leaks).
So remove those #ifdefs.
ok?
Index: uvm/uvm_amap.c
===
RCS
Martin Pieuchot wrote:
> On 26/03/16(Sat) 19:19, Stefan Kempf wrote:
> > Stefan Kempf wrote:
> > > amap_extend is called when merging two adjacent areas of virtual address
> > > space. However, merging is done only for kernel
> > > virtual address
Stefan Kempf wrote:
> amap_extend is called when merging two adjacent areas of virtual address
> space. However, merging is done only for kernel
> virtual address space. It's not done for user space:
>
> uvm/uvm_map.c:1359
> /*
>* Try to merge entry.
>
amap_extend is called when merging two adjacent areas of virtual address
space. However, merging is done only for kernel
virtual address space. It's not done for user space:
uvm/uvm_map.c:1359
/*
* Try to merge entry.
*
* Userland allocations are kept separated
Mark Kettenis wrote:
> > Date: Mon, 21 Mar 2016 20:02:28 +0100
> > From: Stefan Kempf <sisnk...@gmail.com>
> >
> > Recently we found that amaps consume a good deal of kernel address space.
> > See this thread: https://marc.info/?l=openbsd-tech=145752756005014
Recently we found that amaps consume a good deal of kernel address space.
See this thread: https://marc.info/?l=openbsd-tech=145752756005014=2.
And we found a way to reduce kernel mem pressure for some architectures
at least. See the diffs in that thread.
Besides that, it's possible to shrink the
The recent uvm commits fixed hangs because machines went out of memory
because of using too much space for amap slots.
It's possible to shrink memory requirements for amaps even more,
but the current code needs some simplifications first. So here's another
cleanup diff. There'll be one or two
The compiler is also smart enough to recognize that this is redundant.
The resulting code on amd64 is basically equivalent (slightly different
register allocation and instruction scheduling).
ok?
Index: uvm/uvm_amap.c
===
RCS file:
Tobias Ulmer wrote:
> Just wanted to note this diff in combination with your other uvm diff
> does really well on sparc, building ports. Cuts down amap "INUSE" by
> about a factor of 20.
> Will report if anything bad happens.
Cool, thanks for testing as well!
So I plan to commit both diffs if
When sbrk() allocates a range of virtual memory, it immediately allocates
a vm_amap and an am_slots array inside the amap There's one slot per page
allocated, and a slot is 16 bytes in size (on 64 bit CPUs, 12 on 32 bit
CPUs).
Preallocating slots makes sense mostly when we know that the memory
Stuart Henderson wrote:
> On 2016/03/10 19:18, Stefan Kempf wrote:
> > There's still at least one issue with the diff. Again in amap_extend().
> > The slotalloc computation was still off :-(
>
> It's not perfect but this is very significantly better. I've put
> it un
Stefan Kempf wrote:
> Stuart Henderson wrote:
> > On 2016/03/09 20:49, Stefan Kempf wrote:
> > > Here's a diff that allocates the most commonly used amap slot sizes
> > > (between 1 and 16) using pool_get(9) instead of malloc(9). That should
> > > reduce th
Stuart Henderson wrote:
> On 2016/03/09 20:49, Stefan Kempf wrote:
> > Here's a diff that allocates the most commonly used amap slot sizes
> > (between 1 and 16) using pool_get(9) instead of malloc(9). That should
> > reduce the pressure on kernel virtual address spa
Stuart Henderson wrote:
> While running some fairly memory-hungry jobs I hit a state where wchan
> in top was showing "fltamap" and the machine locked up (couldn't enter
> DDB).
>
> Which must be this:
>
> /* didn't work? must be out of RAM. sleep. */
> if
See subject.
ok?
diff --git a/uvm/uvm_amap.c b/uvm/uvm_amap.c
index ef8e505..3cc7ed1 100644
--- a/uvm/uvm_amap.c
+++ b/uvm/uvm_amap.c
@@ -403,49 +403,6 @@ amap_extend(struct vm_map_entry *entry, vsize_t addsize)
}
/*
- * amap_share_protect: change protection of anons in a shared amap
- *
- *
Martin Natano wrote:
> The VOP_UNLOCK() function has a flags parameter, which is documented to
> should be zero in most cases. It turns out that the flags argument is
> zero for all ~200 callers in the tree. On a closer look it is revealed
> that VOP_UNLOCK() uses lockmgr() for all filesystems
Martin Natano wrote:
> On Sun, Feb 28, 2016 at 04:40:21PM +0100, Stefan Kempf wrote:
> > Stefan Kempf wrote:
> > > Martin Pieuchot wrote:
> > > > I'm also wondering when you say "an offset that's at or paste the
> > > > EOF" does tha
This changes uiomovei calls to uiomove in usb. It fixes
a few integer truncations due to use of min, and uses
unsigned types for count variables where it makes sense.
This also allows us to get rid of a couple of 'if (len < 0)' checks
that just cannot happen.
udbd_get_cdesc() returns a length
Stefan Kempf wrote:
> Martin Pieuchot wrote:
> > On 28/02/16(Sun) 13:14, Martin Natano wrote:
> > > The ext2fs_read() and ffs_read() functions return EFBIG when uio_offset
> > > if smaller than zero or larger than the maxium file size. However this
> > &
Martin Pieuchot wrote:
> On 28/02/16(Sun) 13:14, Martin Natano wrote:
> > The ext2fs_read() and ffs_read() functions return EFBIG when uio_offset
> > if smaller than zero or larger than the maxium file size. However this
> > doesn't seem to be in accordance with the POSIX read(2) specification,
>
Martin Pieuchot wrote:
> On 17/02/16(Wed) 20:38, Stefan Kempf wrote:
> > Martin Pieuchot wrote:
> > It looks like NetBSD removed the SIOCSIFALIFETIME_IN6 ioctl a long time
> > ago, along with the overflow checks, saying that this ioctl could never
> > have worked:
&
Todd C. Miller wrote:
> On Sun, 21 Feb 2016 11:49:55 +0100, Martin Natano wrote:
>
> > The diff below addresses the issues you mentioned. It converts
> > mnt_maxsymlinklen to unsigned and adds a check to ffs_validate() that
> > makes sure, that fs_maxsymlinklen is >= 0. That function is called
>
Martin Natano wrote:
> When diff encounters a line that consists of a single dot, it emits two
> dots instead, stops the current command and emits a substitute command
> to replace the double dot with a single one. Then it restarts the
> (original) command if necessary and inserts further lines.
Martin Natano wrote:
> The fusefs_fhtovp() function makes use of the ROOTINO ((ufsino_t)2)
> define instead of using FUSE_ROOTINO ((ino_t)1), which is used
> everywhere else in the fuse filesystem. This causes a file handle for
> the filesystem root to be falsely rejected with ESTALE.
>
>
Martin Natano wrote:
> The fusefs_checkexp() function returns 0, indicating that export via NFS
> is allowed, while in fact fusefs doesn't support NFS at all. (There is
> a vfs_export() call missing in fusefs_mount() and maybe other stuff too.)
>
> Furthermore, it does so without setting
Martin Natano wrote:
> Hi,
>
> The add_entropy_words() function performs a right shift by
> (32 - entropy_input_rotate) bits, with entropy_input_rotate being an
> integer between [0..31]. This can lead to a shift of 32 on a 32 bit
> value, which is undefined behaviour in C. The standard says
Todd C. Miller wrote:
> On Wed, 17 Feb 2016 10:22:04 +0100, Martin Natano wrote:
>
> > Casting the result of ext2fs_size() and DIP(ip, size) to int potentially
> > truncates the result. Issue found by Stefan Kempf, see
> > https://marc.info/?l=openbsd-tech=145495905416536
Martin Pieuchot wrote:
> On 13/02/16(Sat) 18:51, Stefan Kempf wrote:
> > Some thoughts about this:
> >
> > If this particular type of undefined behavior is really a concern: maybe
> > looking for bounds/overflow checks that are incorrect besides undefined
> > be
Some thoughts about this:
If this particular type of undefined behavior is really a concern: maybe
looking for bounds/overflow checks that are incorrect besides undefined
behavior first is a better approach. A good way of fixing those will
be found, which could then be applied to the "just
Martin Natano wrote:
> Below the conversion to uiomovei() for ufs. While there I changed all
> instances of 'blkoffset', 'size' and 'xfersize' that where declared as
> long integers to be plain integers instead for consistency with the
> surrounding code. These variables are limited by fs_bsize
Martin Natano wrote:
> Below the conversion from uiomovei() to uiomove() for bktr. The patch
> also replaces two occurrences of uio->uio_iov->iov_len with
> uio->uio_resid. I don't see a reason why bktr should inspect iov_len
> directly.
Looks good. As for computing count, we have 0 <= bktr->rows
Martin Natano wrote:
> Below the uiomove() conversion for bpf. bd_hlen is a signed integer, but
> can't be negative, because it contains the size of a buffer. Thus, the
> conversion to size_t is ok.
Looks good. bd_hlen is assigned from bd_slen which is in turn computed
in bpf_catchpacket(). This
Martin Natano wrote:
> Below the uiomove() conversion for nfs. I didn't change the type of 'n'
> to be size_t, because it never exceeds the maximum rpc size (nm_rsize),
> which is an integer too. (Also, to avoid unnecessary code churn.)
Looks good. It's easy to see in the code that (0 < on <
Mike Belopuhov wrote:
> Any OKs or objections to this diff? This looks solid to me, FWIW.
Looks good to me as well.
ok stefan@
> On Wed, Jan 27, 2016 at 09:52 +0100, Martin Natano wrote:
> > In ufs, the calculation of i_modrev can produce signed overflow on 32
> > bit architectures (found on
Martin Natano wrote:
> Stefan Kempf wrote:
> > I'm a bit uneasy though with passing signed values as-is to uiomove().
> > Can we somehow make it explicit that we know that the uiomove() argument is
> > >= 0?
> >
> > Changing types all over the place would be
Martin Natano wrote:
> On Tue, Feb 02, 2016 at 07:30:08PM +0100, Stefan Kempf wrote:
> > Looks good. I agree with changing left to size_t. One small remark
> > though: size_t is defined as unsigned long. Do the DPRINTFs that print
> > the value of left have to be changed to
Martin Natano wrote:
> Below the conversion to uiomove() for ntfs. In the first three hunks the
> size passed to uiomove(i) already was of type size_t. I also converted
> the 'left' variable in ntfs_readattr() to size_t, because it tracks the
> remainder of 'rsize', which also is size_t.
Looks
Martin Natano wrote:
> Below the conversion to uiomove() for isofs/udf/. Note that converting
> size to size_t is not possible in udf_read(), as udf_readatoffset()
> requires a pointer to an integer variable. Changing that would cause a
> lot of code churn, so i chose to truncate uio_resid to
Martin Natano wrote:
> Below the conversion from uiomovei() to uiomove() for kern/tty.c and
> kern/tty_pty.c. 'cc' consistently contains small, non-negative integer
> values, so leaving the type as int should be ok. It could as well be
> changed to size_t, but I don't see a benefit in doing so for
Martin Natano wrote:
> Below the uiomove() conversion for msdosfs. This diff prevents
> truncation of uio_resid in both msdosfs_read() and msdosfs_write().
Yes. That's similar to the cd9660 diff.
> Index: msdosfs/msdosfs_vnops.c
>
The following diff prevents integer truncation of uio_resid
by using ulmin() instead of min() and calls uiomove() instead
of the legacy uiomove().
That's straightforward because the m_len mbuf field is unsigned.
mlen can be turned to a size_t because it's set to MLEN or MCLBYTES,
which is > 0.
I
Martin Natano wrote:
> Below is a diff to convert videoread() to use uiomove() instead of
> uiomovei(). Note, that passing sc_fsize to ulmin() is not problematic,
> even though it is an int, because the value is always positive.
Yes, sc_fsize *should* always be positive. I had to convince myself
Martin Natano wrote:
> 0 is the same as UIO_USERSPACE, but more readable. No binary change.
Makes sense. The entire tree uses the symbolic enum values.
These are the only places that use 0.
> Index: ./arch/octeon/dev/amdcf.c
> ===
Diff reads good. ok?
Some thoughts (mostly for myself) inline.
Martin Natano wrote:
> Below the uiomove conversion for isofs/cd9660/cd9660_vnops.c.
>
> cheers,
> natano
>
> Index: isofs/cd9660/cd9660_vnops.c
> ===
> RCS file:
Martin Natano wrote:
> Below the uiomove() conversion for net/ppp_tty.c. M_TRAILINGSPACE()
> returns int, but the result can't be negative, so using u_int for the
> return value should be fine.
Looks good. m->m_len is unsigned and yes, given that the mbuf fields
aren't corrupted M_TRAILINGSPACE
pipe_write() has an orig_resid = uio->uio_resid(). So orig_resid better
be a size_t also. Looks good otherwise.
ok with the updated diff below?
Index: kern/sys_pipe.c
===
RCS file: /cvs/src/sys/kern/sys_pipe.c,v
retrieving revision
Martin Natano wrote:
> Below the uiomove() conversion for kern/subr_log.c. msg_buf[rsx] are all
> of type long, but are always positive. This diff prevents truncation of
> uio_resid (and l) due to min() usage.
Makes sense.
ok?
> Index: kern/subr_log.c
>
Stefan Sperling wrote:
> I've run into an issue where iwn(4) fails to init the hardware.
>
> Running 'ifconfig iwn0 scan' resulted in:
>
> setting configuration
> iwn0: fatal firmware error
> firmware error log:
> error type = "SYSASSERT" (0x0005)
> program counter = 0x00022088
>
Martin Natano wrote:
> Below the uiomove() conversion for dev/wscons/wsevent.c. 'cnt' could
> as well be a size_t, but using u_int makes clear, that it will never
> exceed UINT_MAX, and that 'ev->get = cnt;' won't overflow.
Makes sense to me.
ok?
> Index: dev/wscons/wsevent.c
>
Martin Natano wrote:
> Below the uiomove() conversion for dev/ksyms.c. 'len' is a size_t.
And len is computed from values >= 0 and from expressions that do not
wrap around.
ok?
> Index: dev/ksyms.c
> ===
> RCS file:
looks good to me.
ok?
Martin Natano wrote:
> Below the conversion from uiovmovei() to uiomove() for miscfs/fusefs/.
> All size parameters already are size_t, so the diff is straightforward.
>
> Index: miscfs/fuse/fuse_device.c
>
Thanks. A similar diff was discussed privately with a few
developers during the last few days and is about to be
committed soon.
Martin Natano wrote:
> Another integer overflow: A recv() call with a size of 2^32 bytes causes
> soreceive() to spin in an endless loop, resulting in a system freeze.
Index: uvm/uvm_map.c
===
RCS file: /cvs/src/sys/uvm/uvm_map.c,v
retrieving revision 1.204
diff -u -p -r1.204 uvm_map.c
--- uvm/uvm_map.c 14 Nov 2015 14:53:14 - 1.204
+++ uvm/uvm_map.c 11 Dec 2015 18:47:51 -
@@
Tati Chevron wrote:
> This assembled and linked without problems on 5.7-release, but now when
> I try it on 5.8-release, I get an error:
>
> $ as -o charset.o charset.S
> $ ld -Bstatic charset.o
> ld: charset.o: relocation R_X86_64_32S against `a local symbol' can not
> be used when making a
Hi,
this diff makes guests runnable without holding the biglock.
If the guest exits because of an interrupt, the interrupt is
handled first before re-grabbing the lock.
This is needed because the TLB shootdown code runs under the biglock
and issues an IPI to other CPUs to invalidate the TLB.
77 matches
Mail list logo
