Re: CVS commit: src/bin/cp
On Sun, Oct 24, 2010 at 10:56:40PM +, David Holland wrote: Anyway, ISTM that writing from the mmap buffer in say 64K chunks would retain nearly all the advantages and get rid of the latency problem. The way the code is currently written it only uses mmap(2) for files smaller than 8MB anyway. Your suggested change would require more work than reducing the size of the mapped memory. There is also the problem that the overhead per call to mmap(2) or munmap(2) is high on some platforms, IIRC alpha is one of them. Changing the code as you suggested above might therefore impact performance on some ports. Kind regards -- Matthias Scheler http://zhadum.org.uk/
Re: CVS commit: src/bin/cp
On Mon, Oct 25, 2010 at 08:52:43AM +0100, Matthias Scheler wrote: On Sun, Oct 24, 2010 at 10:56:40PM +, David Holland wrote: Anyway, ISTM that writing from the mmap buffer in say 64K chunks would retain nearly all the advantages and get rid of the latency problem. The way the code is currently written it only uses mmap(2) for files smaller than 8MB anyway. Your suggested change would require more work than reducing the size of the mapped memory. Forget that, I misunderstood what you said. We can still use one mmap(2) but should write out that memory with multile write(2) system calls. Kind regards -- Matthias Scheler http://zhadum.org.uk/
Re: CVS commit: src/bin/cp
On Mon, Oct 25, 2010 at 11:17:22AM +0200, Juergen Hannken-Illjes wrote: On Sun, Oct 24, 2010 at 05:21:06AM +, David Holland wrote: On Fri, Oct 22, 2010 at 05:56:06PM +, Antti Kantee wrote: Disable mmap path. With the current vnode locking scheme it has a very annoying property: if the source media is slow (like a slow network), the target file will be locked for the duration of the entire max 8MB write and cause processes attempting to e.g. stat() it to tstile (for several minutes in the worst case). Revisit this if/when vnode locking gets a little smarter. Wouldn't it be better to just ratchet back the block size to something like 64K that happens faster? Or first fault the mapped region instead of madvise() like: Do we implement MADV_WILLNEED? Joerg
Re: CVS commit: src/bin/cp
On Mon, Oct 25, 2010 at 06:30:54PM +0200, Joerg Sonnenberger wrote: On Mon, Oct 25, 2010 at 11:17:22AM +0200, Juergen Hannken-Illjes wrote: On Sun, Oct 24, 2010 at 05:21:06AM +, David Holland wrote: On Fri, Oct 22, 2010 at 05:56:06PM +, Antti Kantee wrote: Disable mmap path. With the current vnode locking scheme it has a very annoying property: if the source media is slow (like a slow network), the target file will be locked for the duration of the entire max 8MB write and cause processes attempting to e.g. stat() it to tstile (for several minutes in the worst case). Revisit this if/when vnode locking gets a little smarter. Wouldn't it be better to just ratchet back the block size to something like 64K that happens faster? Or first fault the mapped region instead of madvise() like: Do we implement MADV_WILLNEED? According to the man page This WILL NOT fault pages in from backing store. -- Juergen Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/bin/cp
On Mon, Oct 25, 2010 at 11:17:22AM +0200, Juergen Hannken-Illjes wrote: On Sun, Oct 24, 2010 at 05:21:06AM +, David Holland wrote: On Fri, Oct 22, 2010 at 05:56:06PM +, Antti Kantee wrote: Disable mmap path. With the current vnode locking scheme it has a very annoying property: if the source media is slow (like a slow network), the target file will be locked for the duration of the entire max 8MB write and cause processes attempting to e.g. stat() it to tstile (for several minutes in the worst case). Revisit this if/when vnode locking gets a little smarter. Wouldn't it be better to just ratchet back the block size to something like 64K that happens faster? Or first fault the mapped region instead of madvise() like: int pgsz = getpagesize(); char *q; volatile char c; for (q = p; q p+fs-st_size; q += pgsz) c = *q; That won't really help - the pages could easily be discarded almost immediately - eg in order to fault in a later page. David -- David Laight: da...@l8s.co.uk
Re: CVS commit: src/bin/cp
On Sun, Oct 24, 2010 at 10:56:40PM +, David Holland wrote: I think write() only needs to lock the the file enough to ensure that the file offset is correct. No, since in general the file is also being extended (certainly in this case it is) it also has to lock the file size, and that's going to deny stat() until it's done. A stat request during a write can safely return the old size. David -- David Laight: da...@l8s.co.uk
Re: CVS commit: src/bin/cp
On Mon, Oct 25, 2010 at 06:41:11PM +0200, Juergen Hannken-Illjes wrote: Do we implement MADV_WILLNEED? According to the man page This WILL NOT fault pages in from backing store. The version of the man page I have says It might or might not fault pages in from backing store. Joerg
re: CVS commit: src/bin/cp
On Mon, Oct 25, 2010 at 08:52:43AM +0100, Matthias Scheler wrote: On Sun, Oct 24, 2010 at 10:56:40PM +, David Holland wrote: Anyway, ISTM that writing from the mmap buffer in say 64K chunks would retain nearly all the advantages and get rid of the latency problem. The way the code is currently written it only uses mmap(2) for files smaller than 8MB anyway. Your suggested change would require more work than reducing the size of the mapped memory. Forget that, I misunderstood what you said. We can still use one mmap(2) but should write out that memory with multile write(2) system calls. FWIW, bozohttpd defaults to mapping upto 64MB regions and writing out up to 64KB at a time. .mrg.
Re: CVS commit: src/bin/cp
On Mon, Oct 25, 2010 at 06:46:36PM +0200, Joerg Sonnenberger wrote: On Mon, Oct 25, 2010 at 06:41:11PM +0200, Juergen Hannken-Illjes wrote: Do we implement MADV_WILLNEED? According to the man page This WILL NOT fault pages in from backing store. The version of the man page I have says It might or might not fault pages in from backing store. Mine was 5.0.2, yours looks more -current :-) -- Juergen Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/bin/cp
On Tue, Oct 26, 2010 at 05:05:28AM +1100, matthew green wrote: On Mon, Oct 25, 2010 at 08:52:43AM +0100, Matthias Scheler wrote: On Sun, Oct 24, 2010 at 10:56:40PM +, David Holland wrote: Anyway, ISTM that writing from the mmap buffer in say 64K chunks would retain nearly all the advantages and get rid of the latency problem. The way the code is currently written it only uses mmap(2) for files smaller than 8MB anyway. Your suggested change would require more work than reducing the size of the mapped memory. Forget that, I misunderstood what you said. We can still use one mmap(2) but should write out that memory with multile write(2) system calls. FWIW, bozohttpd defaults to mapping upto 64MB regions and writing out up to 64KB at a time. cp(1) now maps a region upto 8MB and writes it out in chunks of at most 64KB. Kind regards -- Matthias Scheler http://zhadum.org.uk/
