Re: CVS commit: src
wrote: > I guess something like following change is necessary so that when both > tv_nsec is set to UTIME_NOW, > > 1) to perform same permission check as when NULL is passed to 2nd > arg. > 2) to set same value for both atime and mtime. I think you are right. -- Emmanuel Dreyfus http://hcpnet.free.fr/pubz m...@netbsd.org
Re: CVS commit: src
Emmanuel Dreyfus writes:
> Module Name: src
> Committed By: manu
> Date: Wed Aug 17 07:22:35 UTC 2011
>
> Modified Files:
> src/distrib/sets/lists/comp: mi
> src/lib/libc/sys: Makefile.inc utimes.2
> src/sys/kern: syscalls.master vfs_syscalls.c
>
> Log Message:
> Add futimens(2) and part of utimnsat(2)
I guess something like following change is necessary so that when both
tv_nsec is set to UTIME_NOW,
1) to perform same permission check as when NULL is passed to 2nd
arg.
2) to set same value for both atime and mtime.
enami.
Index: vfs_syscalls.c
===
RCS file: /cvsroot/src/sys/kern/vfs_syscalls.c,v
retrieving revision 1.437
diff -c -r1.437 vfs_syscalls.c
*** vfs_syscalls.c 18 Aug 2011 19:34:47 - 1.437
--- vfs_syscalls.c 22 Aug 2011 03:50:05 -
***
*** 3137,3146
}
}
! if (ts[0].tv_nsec == UTIME_NOW)
nanotime(&ts[0]);
!
! if (ts[1].tv_nsec == UTIME_NOW)
nanotime(&ts[1]);
if (vp == NULL) {
--- 3137,3149
}
}
! if (ts[0].tv_nsec == UTIME_NOW) {
nanotime(&ts[0]);
! if (ts[1].tv_nsec == UTIME_NOW) {
! vanull = true;
! ts[1] = ts[0];
! }
! } else if (ts[1].tv_nsec == UTIME_NOW)
nanotime(&ts[1]);
if (vp == NULL) {
Re: CVS commit: src/common/lib/libc/string
On Mon, Aug 22, 2011 at 03:13:29AM +0200, Joerg Sonnenberger wrote: > On Sun, Aug 21, 2011 at 11:37:08PM +, David Holland wrote: > > On Mon, Aug 22, 2011 at 01:31:31AM +0200, Joerg Sonnenberger wrote: > > > > Modified Files: > > > > src/common/lib/libc/string: popcount32.c popcount64.c > > > > > > > > Log Message: > > > > Requires stdint.h. > > > > > > No? > > > > uh what? > > It doesn't. The prototypes in strings.h already ensure that > uint32_t/uint64_t are present and that's the only thing it could ever > need from stdint.h. Yes it does. strings.h is included by string.h and is therefore not allowed to include stdint.h itself. -- David A. Holland dholl...@netbsd.org
Re: CVS commit: src/common/lib/libc/string
On Sun, Aug 21, 2011 at 11:37:08PM +, David Holland wrote: > On Mon, Aug 22, 2011 at 01:31:31AM +0200, Joerg Sonnenberger wrote: > > > Modified Files: > > > src/common/lib/libc/string: popcount32.c popcount64.c > > > > > > Log Message: > > > Requires stdint.h. > > > > No? > > uh what? It doesn't. The prototypes in strings.h already ensure that uint32_t/uint64_t are present and that's the only thing it could ever need from stdint.h. Joerg
Re: CVS commit: src/common/lib/libc/string
On Mon, Aug 22, 2011 at 01:31:31AM +0200, Joerg Sonnenberger wrote: > > Modified Files: > >src/common/lib/libc/string: popcount32.c popcount64.c > > > > Log Message: > > Requires stdint.h. > > No? uh what? -- David A. Holland dholl...@netbsd.org
Re: CVS commit: src/common/lib/libc/string
On Sun, Aug 21, 2011 at 09:25:04PM +, David A. Holland wrote: > Module Name: src > Committed By: dholland > Date: Sun Aug 21 21:25:04 UTC 2011 > > Modified Files: > src/common/lib/libc/string: popcount32.c popcount64.c > > Log Message: > Requires stdint.h. > > > To generate a diff of this commit: > cvs rdiff -u -r1.3 -r1.4 src/common/lib/libc/string/popcount32.c > cvs rdiff -u -r1.5 -r1.6 src/common/lib/libc/string/popcount64.c No? Joerg
Re: CVS commit: src/sys/arch/xen
On Sun, Aug 21, 2011 at 12:38:06PM +0200, Jean-Yves Migeon wrote: > On 21.08.2011 12:26, Jean-Yves Migeon wrote: > > - second, the lock is not placed at the correct abstraction level IMHO, > > it is way too high in the caller/callee hierarchy. It should remain > > hidden from most consumers of the xpq_queue API, and should only be used > > to protect the xpq_queue array together with its counters (and > > everything that isn't safe for all memory operations happening in xpq). > > > > Reason behind this is that your lock protects calls to hypervisor MMU > > operations, which are hypercalls (hence, a "slow" operation with regard > > to kernel). You are serializing lots of memory operations, something > > that should not happen from a performance point of view (some may take a > > faire amount of cycles to complete, like TLB flushes). I'd expect all > > Xen MMU hypercalls to be reentrant. > > An alternative would be to have per-CPU xpq_queue[] also. This is not > completely stupid, xpq_queue is meant as a way to put multiple MMU > operations in a queue asynchronously before issuing only one hypercall > to handle them all. Sure, this is the way to go. -- Manuel Bouyer NetBSD: 26 ans d'experience feront toujours la difference --
Re: CVS commit: src/sys/arch/xen
On 21.08.2011 21:39, Cherry G. Mathew wrote: > JM> An alternative would be to have per-CPU xpq_queue[] also. This > JM> is not completely stupid, xpq_queue is meant as a way to put > JM> multiple MMU operations in a queue asynchronously before issuing > JM> only one hypercall to handle them all. > > This is slightly more complicated than it appears. Some of the "ops" in > a per-cpu queue may have ordering dependencies with other cpu queues, > and I think this would be hard to express trivially. (an example would > be a pte update on one queue, and reading the same pte read on another > queue - these cases are quite analogous (although completely unrelated) > to classic RAW and other ordering dependencies in out-of-order execution > scenarios due to pipelining, etc.). Yes; however this should be handled by the pmap layer. xpq_queue is fairly low level and has no access to all the knowledge/semantic required to ensure proper PT/PD synchronization. It is rather a way to handle multiple MMU operations into one hypercall, like readv/writev can do for multiple read/write(s). Mapping different pmaps in the same VA space requires to lock them, so you should not see multiple CPUs concurrently reading/writing shared entries. One thing remains with the old => new PTE updates that need CAS, but this is Xen's mission to do that correctly as it manages the MMU for us. All we have to ensure is that there's no stale operation in the queue before leaving pmap. > I'm thinking that it might be easier and more justifiable to nuke the > current queue scheme and implement shadow page tables, which would fit > more naturally and efficiently with CAS pte updates, etc. Beware with those; shadow page tables were mainly designed with Linux in mind, and the way it manages virtual memory is completely different to ours: it maps the entire physical memory in the kernel virtual space (with tricks when there's more than 1GiB involved), while we use recursive mappings. And Xen has problems validating these. -- Jean-Yves Migeon jeanyves.mig...@free.fr
Re: CVS commit: src/sys/arch/xen
> "JM" == Jean-Yves Migeon writes: JM> On 21.08.2011 12:26, Jean-Yves Migeon wrote: >> - second, the lock is not placed at the correct abstraction level >> IMHO, it is way too high in the caller/callee hierarchy. It >> should remain hidden from most consumers of the xpq_queue API, >> and should only be used to protect the xpq_queue array together >> with its counters (and everything that isn't safe for all memory >> operations happening in xpq). >> I agree - part of the reason I did this was to make sure I didn't mess with the current queueing scheme before having a working implementation/PoC. >> Reason behind this is that your lock protects calls to hypervisor >> MMU operations, which are hypercalls (hence, a "slow" operation >> with regard to kernel). You are serializing lots of memory >> operations, something that should not happen from a performance >> point of view (some may take a faire amount of cycles to >> complete, like TLB flushes). I'd expect all Xen MMU hypercalls to >> be reentrant. I agree - it's not meant to be an efficient implementation - just a correct one. JM> An alternative would be to have per-CPU xpq_queue[] also. This JM> is not completely stupid, xpq_queue is meant as a way to put JM> multiple MMU operations in a queue asynchronously before issuing JM> only one hypercall to handle them all. This is slightly more complicated than it appears. Some of the "ops" in a per-cpu queue may have ordering dependencies with other cpu queues, and I think this would be hard to express trivially. (an example would be a pte update on one queue, and reading the same pte read on another queue - these cases are quite analogous (although completely unrelated) to classic RAW and other ordering dependencies in out-of-order execution scenarios due to pipelining, etc.). I'm thinking that it might be easier and more justifiable to nuke the current queue scheme and implement shadow page tables, which would fit more naturally and efficiently with CAS pte updates, etc. Otherwise, we'll have to re-invent scoreboarding or other dynamic scheduling tricks - I think that's a bit overkill :-) Cheers, -- Cherry
Re: CVS commit: src
NAKAJIMA Yoshihiro wrote: > Because new function calls were added to libc. > I think new APIs bring minor version bumping, under a NetBSD policy. Um, what should be done in distribs/sets/lists? Would s/libc.so.12.178/libc.so.12.179/ eveywhere be enough? -- Emmanuel Dreyfus http://hcpnet.free.fr/pubz m...@netbsd.org
Re: CVS commit: src
On Sat, 20 Aug 2011 22:10:13 +0200, m...@netbsd.org (Emmanuel Dreyfus) wrote: > NAKAJIMA Yoshihiro wrote: > >> > Add futimens(2) and part of utimnsat(2) >> Isn't shlib_version bumped? > > Perhaps, but I must confess I do not know the rules for that. What are > they? Because new function calls were added to libc. I think new APIs bring minor version bumping, under a NetBSD policy. -- nakayosh
Re: CVS commit: src/sys/arch/xen
On 21.08.2011 12:26, Jean-Yves Migeon wrote: > - second, the lock is not placed at the correct abstraction level IMHO, > it is way too high in the caller/callee hierarchy. It should remain > hidden from most consumers of the xpq_queue API, and should only be used > to protect the xpq_queue array together with its counters (and > everything that isn't safe for all memory operations happening in xpq). > > Reason behind this is that your lock protects calls to hypervisor MMU > operations, which are hypercalls (hence, a "slow" operation with regard > to kernel). You are serializing lots of memory operations, something > that should not happen from a performance point of view (some may take a > faire amount of cycles to complete, like TLB flushes). I'd expect all > Xen MMU hypercalls to be reentrant. An alternative would be to have per-CPU xpq_queue[] also. This is not completely stupid, xpq_queue is meant as a way to put multiple MMU operations in a queue asynchronously before issuing only one hypercall to handle them all. -- Jean-Yves Migeon jeanyves.mig...@free.fr
Re: CVS commit: src/sys/arch/xen
On 10.08.2011 11:50, Cherry G. Mathew wrote: > Module Name: src > Committed By: cherry > Date: Wed Aug 10 09:50:37 UTC 2011 > > Modified Files: > src/sys/arch/xen/include: xenpmap.h > src/sys/arch/xen/x86: x86_xpmap.c > > Log Message: > Introduce locking primitives for Xen pte operations, and xen helper calls for > MP related MMU ops Hi Cherry, Sorry for this very late comment, as I was AFK most of the time for the last three weeks. I have two concerns with your patch: - first, you introduce locking primitives based on simple_lock(9). This mechanism ought to go in the not-so-distant future, so I would replace all these with a properly chosen mutex, likely at IPL_VM (depending on the priority of all consumers of the xpq_*() functions). This must be carefully reviewed first. - second, the lock is not placed at the correct abstraction level IMHO, it is way too high in the caller/callee hierarchy. It should remain hidden from most consumers of the xpq_queue API, and should only be used to protect the xpq_queue array together with its counters (and everything that isn't safe for all memory operations happening in xpq). Reason behind this is that your lock protects calls to hypervisor MMU operations, which are hypercalls (hence, a "slow" operation with regard to kernel). You are serializing lots of memory operations, something that should not happen from a performance point of view (some may take a faire amount of cycles to complete, like TLB flushes). I'd expect all Xen MMU hypercalls to be reentrant. -- Jean-Yves Migeon jeanyves.mig...@free.fr
