On Wed, 2008-12-17 at 01:21 -0800, Roland McGrath wrote: > > The current implementation is that if I create a new engine in response > > to an exec (when called from some other engine's report_exec callback), > > and set that engine's flags to be notified of execs, the new engine gets > > notified of the exec that's already underway. This turns out to be > > rather inconvenient for uprobes, but is it counterintuitive? > > To clarify, this is not specific to exec. Every kind of event callback > constitutes what I call a "reporting pass", and they all behave the same. > A normal reporting pass is the loop across all engines, in which interested > ones get first a report_quiesce(eventbit) and then a report_event(). > A resume reporting pass is the similar loop where engines get either just > report_quiesce(0) or just report_signal(). > > The question is what happens in the current reporting pass when a callback > attaches a new engine to current and sets its event mask to include the > event that elicited this reporting pass. > > The current behavior is that the new engine goes immediately on the end of > the list of engines to get callbacks, so the reporting pass already in > progress will later get to all the new engines before it's done. > > The alternative behavior would be that any new engines attached after a > reporting pass has begun will not be included in that pass. They will be > included in the next reporting pass of any kind. A side effect is that if > there was not going to be any other report before returning to user mode, > there will be a resume reporting pass (that the new engine will see). > That is the same effect of utrace_control(UTRACE_REPORT) being done when > the utrace_attach_task() is done. > > Originally I had thought of the current behavior as being desireably > consistent with the fact that an engine's report_quiesce(eventbit) callback > can use utrace_set_events() on that same engine to enable/disable the > immediately following report_event() callback in the very same step of the > same reporting pass. > > But another way to look at it is that any utrace_attach_task() call from > any other task behaves this (alternative) way. That is, if some reporting > pass has already begun, the new engine is not included, but a UTRACE_REPORT > is done instead to get the new engine fully signed on "soon". So it would > be simply consistent for any attach made during a reporting pass > (synchronously or asynchronously) not to take effect during that same pass. > > I was musing about adding a UTRACE_ATTACH_* flag bit to let you select the > behavior. But that seems overly fiddly for no good reason. > > So I don't mind changing this as Jim prefers. The actual change is simple, > just remove the "splice_attaching" case from utrace_attach_task.
Yes, I'd prefer that you make the requested change, if you haven't already. Just before I went on vacation (about when you posted this), I coded a tentative fix to uprobes to work with the existing utrace behavior. It's about a 250-line patch, and I haven't tested it yet. It'd be nice if I could drop that. > > Jim, can you look through the kerneldoc comments and the Documentation/ > files and cite any places where the description of this behavior now needs > to be corrected or explained more clearly and explicitly? 1. On the "Events and Callbacks" page, paragraph 3 says: "When a thread has an event, each engine gets a callback if it has set the event flag for that event type." Either here or at the end of that page, you could add something like: [If you implement the requested behavior...] In response to an event, one engine's callback may create a new engine for the same task. This new engine will not be notified of the event already in progress, even if you immediately set its event flag for that type of event. [If not...] In response to an event of type UTRACE_EVENT(x), one engine's callback may create a new engine for the same task. This new engine will be appended to that task's list of engines; and if you set its event flag for UTRACE_EVENT(x), it will be notified in turn of the event already in progress. 2. Something similar could be added to the description of utrace_set_events(). > > > Thanks, > Roland Thanks. Jim
