Re: [Nouveau] [BUG/PATCH] x86 mmiotrace: dynamically disable non-boot CPUs
On Wed, Apr 16, 2008 at 6:59 PM, Pekka Paalanen [EMAIL PROTECTED] wrote: On Wed, 16 Apr 2008 13:46:09 +0200 Ingo Molnar [EMAIL PROTECTED] wrote: * Pekka Paalanen [EMAIL PROTECTED] wrote: we should fix this restriction ASAP. Forcibly dropping to UP will cause mmiotrace to be much less useful for diagnostic purposes of Linux Ok, how do you propose we solve this? I have asked the question before, and then I had two ideas. Well, the first one was actually your idea (so I hear) to solve the same problem for kmemcheck. - per-cpu page tables - instead of single-stepping, emulate the faulting instruction and never disarm pages during tracing. (Use and modify code from KVM.) I don't believe either of these is easy or fast to implement. Given some months, I might be able to achieve emulation. Page tables are still magic to me. yeah - it looks complex. Not a showstopper for now :-) but given that Xorg is usually just a single task, do we _really_ need this? We're not tracing Xorg at all. Mmiotrace still cannot catch accesses originating in user space. It is tracing MMIO accesses from within the kernel, and this means that IRQ services and device syscalls may be accessing the hardware at the same time. Vblank interrupts happen quite often, some GPU commands are actually emulated in kernel via interrupts and whatnot. The nvidia proprietary kernel blob is many times bigger than my bzImage! (A simple X startup and quit creates in the order of 1-2 million MMIO events.) As do we really need this, I think it might save a lot of head scratching when someone is reverse engineering a feature and gets every time a different trace due to some events being missed. But this is theory. So far everyone has been tracing with UP, and this has not been a problem. I have no idea if it would make a real difference. [Recap for nouveau@ list: mmiotrace has a race on SMP, where during instruction single stepping other CPUs can run freely on the page which the faulted instruction accessed. This causes some of the simultaneous accesses to the same page of the same iomem-mapping to be missed.] It does sound very rare. Nouveau people, what do you think, can this be a problem? In the nvidia case, I don't think this would happen. The register ranges for different purposes are set apart by more than 1 page usually, so the risk of accessing a page that's been faulted in is probably extremely low. Not to mention that the design of the binary module doesn't use threads currently (only tasklets for interrupt handlers, this might be the corner case but again the interrupt handler doesn't touch the same reg families). Stephane ___ Nouveau mailing list Nouveau@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [BUG/PATCH] x86 mmiotrace: dynamically disable non-boot CPUs
On Wed, 16 Apr 2008, Ingo Molnar wrote: so lets fix those preemptability bugs. They show that the cpu-up/cpu-down ops are called from atomic context - it should normally be straightforward to sort out - there's no particular reason why the -open()/-close() methods of an ftrace plugin should run in atomic context. Steve, any ideas where the atomicity might come from? They shouldn't be called in an atomic section. The only thing I do to protect them is call mutex_lock/unlock. Those should allow preemption to take place. -- Steve ___ Nouveau mailing list Nouveau@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [BUG/PATCH] x86 mmiotrace: dynamically disable non-boot CPUs
On Wed, 16 Apr 2008 13:46:09 +0200 Ingo Molnar [EMAIL PROTECTED] wrote: * Pekka Paalanen [EMAIL PROTECTED] wrote: we should fix this restriction ASAP. Forcibly dropping to UP will cause mmiotrace to be much less useful for diagnostic purposes of Linux Ok, how do you propose we solve this? I have asked the question before, and then I had two ideas. Well, the first one was actually your idea (so I hear) to solve the same problem for kmemcheck. - per-cpu page tables - instead of single-stepping, emulate the faulting instruction and never disarm pages during tracing. (Use and modify code from KVM.) I don't believe either of these is easy or fast to implement. Given some months, I might be able to achieve emulation. Page tables are still magic to me. yeah - it looks complex. Not a showstopper for now :-) but given that Xorg is usually just a single task, do we _really_ need this? We're not tracing Xorg at all. Mmiotrace still cannot catch accesses originating in user space. It is tracing MMIO accesses from within the kernel, and this means that IRQ services and device syscalls may be accessing the hardware at the same time. Vblank interrupts happen quite often, some GPU commands are actually emulated in kernel via interrupts and whatnot. The nvidia proprietary kernel blob is many times bigger than my bzImage! (A simple X startup and quit creates in the order of 1-2 million MMIO events.) As do we really need this, I think it might save a lot of head scratching when someone is reverse engineering a feature and gets every time a different trace due to some events being missed. But this is theory. So far everyone has been tracing with UP, and this has not been a problem. I have no idea if it would make a real difference. [Recap for nouveau@ list: mmiotrace has a race on SMP, where during instruction single stepping other CPUs can run freely on the page which the faulted instruction accessed. This causes some of the simultaneous accesses to the same page of the same iomem-mapping to be missed.] It does sound very rare. Nouveau people, what do you think, can this be a problem? i suspect the bug is that you bring the CPU down from an atomic (spinlocked or irq disabled) context. Hmm, it should not be... I have to double-check, but all the other code, too, from where enter_uniprocessor() is called, may sleep. The first thing the caller does is to acquire a mutex, which I assume would complain loudly if spinlocked or irq-disabled. Ingo, thank you for fixing this patch, though I'd like to suggest to leave it out for now, since there clearly are worse problems with it than without it. And if we can solve the SMP issue, this is not needed. For the time being we can just instruct users to disable all but one CPU when try want to trace. i think we still need to make this as 'transparent' to users as possible. Disabling CPUs can be tedious. Compared to the out-of-tree mmiotrace, the in-kernel version is already a lot easier to use. Instructing people to drop to UP before tracing is simple compared to what it was. i'm leaving out this patch from the series for now. Thanks. -- Pekka Paalanen http://www.iki.fi/pq/ ___ Nouveau mailing list Nouveau@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [BUG/PATCH] x86 mmiotrace: dynamically disable non-boot CPUs
* Pekka Paalanen [EMAIL PROTECTED] wrote: yeah - it looks complex. Not a showstopper for now :-) but given that Xorg is usually just a single task, do we _really_ need this? We're not tracing Xorg at all. Mmiotrace still cannot catch accesses originating in user space. It is tracing MMIO accesses from within the kernel, and this means that IRQ services and device syscalls may be accessing the hardware at the same time. Vblank interrupts happen quite often, some GPU commands are actually emulated in kernel via interrupts and whatnot. [...] ok, understood - i forgot about IRQ generated GPU accesses. In fact UP probably generates a more readable trace because DRM accesses from one CPU are not mixed up with IRQ completion from another CPU. So i think we need to fix your automatic-cpudown/cpuup patch. I tried that and it worked very intuitively and the cpus were disabled/enabled without any trouble - with ftrace based mmiotrace we now basically have something that most distros could enable by default without thinking twice about it. But if it means an UP kernel has to be used then it will be turned off immediately and the barrier to users will be huge again. I really envision mmiotrace to be usable by default on _any_ generic distro, without rebooting and without any hassle on the user's part. the automatic drop-to-single-CPU-when-tracing solution from you is OK - it will also test our CPU hotplug primitives some more ;-) And it's not like users expect a mmiotraced X session to be particularly fast, right? so lets fix those preemptability bugs. They show that the cpu-up/cpu-down ops are called from atomic context - it should normally be straightforward to sort out - there's no particular reason why the -open()/-close() methods of an ftrace plugin should run in atomic context. Steve, any ideas where the atomicity might come from? Ingo ___ Nouveau mailing list Nouveau@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [BUG/PATCH] x86 mmiotrace: dynamically disable non-boot CPUs
On Wed, 16 Apr 2008 20:32:58 +0200 Ingo Molnar [EMAIL PROTECTED] wrote: * Pekka Paalanen [EMAIL PROTECTED] wrote: yeah - it looks complex. Not a showstopper for now :-) but given that Xorg is usually just a single task, do we _really_ need this? We're not tracing Xorg at all. Mmiotrace still cannot catch accesses originating in user space. It is tracing MMIO accesses from within the kernel, and this means that IRQ services and device syscalls may be accessing the hardware at the same time. Vblank interrupts happen quite often, some GPU commands are actually emulated in kernel via interrupts and whatnot. [...] ok, understood - i forgot about IRQ generated GPU accesses. In fact UP probably generates a more readable trace because DRM accesses from one CPU are not mixed up with IRQ completion from another CPU. In the future, when things get more stable feature-wise, I will revise the mmiotrace log format. One thing to add is cpu number, which will then easily separate interleaved operations. Maybe I should also think about if someone wants to trace things that are not in PCI bus address space. If kmemcheck and mmiotrace could be unified somehow, it would be a tool offering uninitialised memory access traps, MMIO tracing and basically for watching almost any page and recording accesses to that page. In a time far, far away... So i think we need to fix your automatic-cpudown/cpuup patch. I tried that and it worked very intuitively and the cpus were disabled/enabled without any trouble - with ftrace based mmiotrace we now basically have something that most distros could enable by default without thinking twice about it. Without any trouble - you didn't hit the bug I did? But if it means an UP kernel has to be used then it will be turned off immediately and the barrier to users will be huge again. I really envision mmiotrace to be usable by default on _any_ generic distro, without rebooting and without any hassle on the user's part. UP kernel is not mandatory anyway, we just need only one cpu running, which can be realised by maxcpus=1 kernel argument or hot-un-plugging it by hand via sysfs. the automatic drop-to-single-CPU-when-tracing solution from you is OK - it will also test our CPU hotplug primitives some more ;-) And it's not like users expect a mmiotraced X session to be particularly fast, right? They shouldn't, although in my experience X startup is slow but other things after that work with only a minor slowdown. Btw. when I did that SMP drop-to-UP tracing test, the resulting log was 63 MB and 'cat' process was accounted for 24 seconds of cpu time. I will do comparisons some day, but it sounds a lot. I guess optimising ftrace speed is not yet a priority :-) (I'm not even sure if it's the framework or mmiotrace.) so lets fix those preemptability bugs. They show that the cpu-up/cpu-down ops are called from atomic context - it should normally be straightforward to sort out - there's no particular reason why the -open()/-close() methods of an ftrace plugin should run in atomic context. Steve, any ideas where the atomicity might come from? Since Steve says it should not be an ftrace issue, I'll dig in it myself. Might be a weekend job, again. During the week I don't usually fancy doing anything else than relax and write emails ;-) Thanks! -- Pekka Paalanen http://www.iki.fi/pq/ ___ Nouveau mailing list Nouveau@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [BUG/PATCH] x86 mmiotrace: dynamically disable non-boot CPUs
* Pekka Paalanen [EMAIL PROTECTED] wrote: So i think we need to fix your automatic-cpudown/cpuup patch. I tried that and it worked very intuitively and the cpus were disabled/enabled without any trouble - with ftrace based mmiotrace we now basically have something that most distros could enable by default without thinking twice about it. Without any trouble - you didn't hit the bug I did? i did get the warning both on enable/disable but it still worked. Ingo ___ Nouveau mailing list Nouveau@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/nouveau
