Messages in this thread | | | From | Roland McGrath <> | Subject | Re: [patch 04/11] Introduce virtual debug register in thread_struct and wrapper-routines around process related functions | Date | Wed, 11 Mar 2009 19:26:50 -0700 (PDT) |
| |
> detached from thread_struct? There's a lot of complications > (alloc/free, locking, etc.) from this for no good reason - the > hardware-breakpoints info structure is alway per thread and is > quite small, so there's no reason not to embedd it directly > inside thread_struct.
I do certainly think it's worthwhile to use a coherent struct type here rather than many fields in thread_struct, independent of the allocation for it being direct or indirect (not that you objected to that). That makes the code read cleaner, and should make it a minor change to most of the code later if the allocation plan changes.
I think in the original effort another motivating factor was concern about bloating the size of thread_struct. The struct thread_hw_breakpoint is at least a few times the size of the set old fields it replaces. We were concerned that inflating every task's thread_struct for the benefit of these rarely-used fancy new features might meet resistance from arch maintainers like you. If that issue doesn't hold you back from taking the new code, then I think we are more than happy to start with thread_struct directly containing a struct thread_hw_breakpoint member.
I do think we'll want to make it a pointer with later incremental changes. (But those may not need to come very soon.) Firstly, the size reduction to task_struct is fairly compelling since it's for the vast majority of tasks which never need to allocate it. The hair potential is really not very much at least to begin with, if you just make it allocate on first setup and never free (no locking et al, just if (thread->hwbkpt) ...). Second, eventually we'd like to have the possibility of sharing the struct among threads. This will come later on when we have higher-level things that would like to set common watchpoints on a whole group of threads (what debuggers usually really do). Such APIs are far improved and optimized by updating many threads together, and when a big app has thousands of threads to which all the same watchpoints apply, sharing at low level makes many of those intraprocess context switches quicker. As I said, all in the future. But it's far from being entirely baseless to think a pointer makes good sense.
Thanks, Roland
| |