KDB improvements for IA64
|[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]|
My latest mail seems not to have been dispatched in the mailing-lists. Let's try again ! About 70% of my mails are not received on vger.kernel.org, does someone have the same problem ? _______________________ Keith, first of all, I want to thank you for taking time on my proposition. Please find my answer inserted in your mail : > I have compared your versions against the standard kdb v4.4 2.6.12-rc2 > patches and gone through your changes. Unfortunately you lumped them > all together which made it very difficult to work out what you were > changing. AFAICT, you have these changes :- > > * Lots of whitespace changes, including changing indentation, > reformatting function calls, reformatting function declarations, > converting tabs to spaces and even adding whitespace to the end of > lines. Never change whitespace at the same time as making other > changes, it makes it extremely difficult to see what has really > changed. Sorry, I didn't take time to configure xemacs correctly. I will try to remove the lines where it has been changed. It may impact one or two functions. > * Saving and restoring itc around kdb. That only works for > architectures where you can overwrite the source of get_cycles(). On > i386, TSC is read only so this approach does not work there, and I do > not like workarounds that only work on some architectures. If there > are time critical bits of code that cannot tolerate long delays then > the solution is to provide a mechanism for all debug and dump code > (not just kdb) to reset the timers at the OS level, not at the > hardware level. Please, do read my answer to David Mosberger about this point. > * Variables and functions got renamed. Why? Because I estimated that in my code their role have changed (due to a state diagram modification). Then, for example : - kdb_initial_cpu to kdb_monarch_cpu, because its role is to be the CPU which runs kdb_local. - kdb_setstep has been split into kdb_setsinglestep and kdb_settakenbranch (to manage "ss" and "ssb" features) > * The entire kdb state diagram was redone. That may or may not be > useful, but without any discussion on what you are trying to achieve, > I cannot tell if the new state diagram is right. I do know that you > deleted some states that are still required Which ones ? In fact, I have done a deep modification in your code because I estimated that there were to many flags describing some useless KDB states and overall which are read and modified without any spinlock protection (which is dangerous on SMP and NUMA machines). > * Every registered command get a new int * parameter, which is unused > in all but one command. I cannot even tell what you are trying to do > with that extra option, it seems to be tied into the undocumented > state diagram changes. That change will break every other piece of > code that adds its own commands to kdb. A lot of subsystems take > advantage of kdb to provide the debugging fraemwork, breaking the ABI > like this is not acceptable. This new parameter is here to allow to pass KDB parameters instead of using global variables. For example, in your code, you have created a global variable which role is only to pass the CPU number chsoen by kdb_cpu function. I think that this way is not elegant at all (but that might just be a different school point of view) and I prefer to pass arguments into a function. > * The meaning of KDB_ENTER() was changed. The new version will cause > problems when KDB_ENTER() is used on paths that kdb also uses, you > will get recursive entry and deadlock You touched another point of my modifications : my KDB version is not reentrant! This is the main reason why your code deadlocked when setting a breakpoint on a really frequently called piece of code (if you do not believe me, just set a breakpoint in default_idle function and try to "go" again). > * The test for WARN_CONSOLE_UNLOCKED() was changed. I expect that to > generate lots of spurious warning messages on VGA consoles. No, kdb_event flag has been completely removed and this version does not encounter this kind of behaviour. > * kdb_notifier_list was removed. That is used by some subsystems to > get notification of entry to and exit from kdb. It is required. OK, I will add it again. It has only been removed because no module used this feature. > * Support for the ppc64 and sparc64 consoles was removed from > kdb_printf(). That support is required. More generally, the modifications that I have done are completely IA-64 oriented. I have no time and no machine to develop and test it on different platforms. This is what I have told you in my first mail. > * You removed all the "pause output after LINES lines" code and the > associated CMD_INTERRUPT flag. That code is the only thing that > catches output from runaway commands, it is required. Yes, I removed this because on the very first versions, I encountered some deadlock conditions. I can set it again. > * Bits of the catastrophic error handling were deleted, it is now > incomplete. Where ? I do not see where I removed CATASTROPHIC bit in my code ? > * XScale support has been removed from the main kdb control loop. Sorry, but I do not know XScale, what is is exactly ? > * Breakpoint handling has been redone. Alas with all the other > changes, I cannt tell which changes are for breakpoint handling so I > cannot tell if they work or not. The changes required for breakpoints were so deep that I had to rework the entire state diagram, KDB entry and exit conditions and some flags that were redundant or useless for me. > There are far too many changes in the Bull patches; many of these > changes are spurious or simply wrong. Maybe, but they work well. > I am rejecting these patches. > If you want to redo the breakpoint handling then that is fine, but make > that change on its own and discuss the change on the list first. > > Keith Owens. Keith, I tried to modify just a few SGI version to let breakpoints work correctly on SMP machines. But the existing state diagram was so complex and uncorrectly spinlock protected that I decided to rework it. Whatever, if you have some time (I know that you are really busy by other developments), I would prefer to discuss on that point first with you and the people would are interested by increasing the robustness of KDB. Best regards, Francois --------------------------- Use http://oss.sgi.com/ecartis to modify your settings or to unsubscribe.