resume from another engine results in loss of singlestep req.

Oleg Nesterov oleg at redhat.com
Wed Aug 12 13:46:40 UTC 2009


On 08/12, Oleg Nesterov wrote:
>
> On 08/11, Roland McGrath wrote:
> >
> > @@ -1298,14 +1305,14 @@ static void finish_report(struct utrace_
> >  {
> >  	bool clean = (report->takers && !report->detaches);
> >
> > -	if (report->action <= UTRACE_REPORT && !utrace->report) {
> > -		spin_lock(&utrace->lock);
> > -		utrace->report = 1;
> > -		set_tsk_thread_flag(task, TIF_NOTIFY_RESUME);
> > -	} else if (report->action == UTRACE_INTERRUPT && !utrace->interrupt) {
> > +	if (report->action == UTRACE_INTERRUPT && !utrace->interrupt) {
> >  		spin_lock(&utrace->lock);
> >  		utrace->interrupt = 1;
> >  		set_tsk_thread_flag(task, TIF_SIGPENDING);
> > +	} else if (report->action <= UTRACE_REPORT && !utrace->report) {
> > +		spin_lock(&utrace->lock);
> > +		utrace->report = 1;
>
> Hmm. I think this is not exacly right. Suppose that
> report->action == UTRACE_INTERRUPT but utrace->interrupt is already set.
> In that case, since UTRACE_INTERRUPT <= UTRACE_REPORT, we set ->report.
>
> Not really bad, but not right anyway.

I'd suggest:

	static void finish_report(struct utrace_report *report,
				  struct task_struct *task, struct utrace *utrace)
	{
		if (report->action <= UTRACE_REPORT) {
			bool intr = (report->action == UTRACE_INTERRUPT);
			if (!(intr ? utrace->interrupt : utrace->report)) {
				spin_lock(&utrace->lock);
				if (intr) {
					utrace->interrupt = true;
					set_tsk_thread_flag(task, TIF_SIGPENDING);
				} else {
					utrace->report = true;
					set_tsk_thread_flag(task, TIF_NOTIFY_RESUME);
				}
				spin_unlock(&utrace->lock);
			}
		}

		if (likely(report->takers && !report->detaches))
			return;

		spin_lock(&utrace->lock);
		utrace_reset(task, utrace, &report->action);
	}

personally, I don't think it makes sense to try to save one spin_lock()
like the current code does. utrace_reset() is unlikely case.

But we can do

	static void finish_report(struct utrace_report *report,
				  struct task_struct *task, struct utrace *utrace)
	{
		bool clean = (report->takers && !report->detaches);

		if (report->action <= UTRACE_REPORT) {
			bool intr = (report->action == UTRACE_INTERRUPT);
			if (!(intr ? utrace->interrupt : utrace->report)) {
				spin_lock(&utrace->lock);
				if (intr) {
					utrace->interrupt = true;
					set_tsk_thread_flag(task, TIF_SIGPENDING);
				} else {
					utrace->report = true;
					set_tsk_thread_flag(task, TIF_NOTIFY_RESUME);
				}
				if (unlikely(!clean))
					goto reset;
				spin_unlock(&utrace->lock);
			}
		}

		if (likely(clean))
			return;

		spin_lock(&utrace->lock);
	reset:
		utrace_reset(task, utrace, &report->action);
	}

Oleg.




More information about the utrace-devel mailing list