From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24209 invoked by alias); 14 Aug 2008 21:20:58 -0000 Received: (qmail 24177 invoked by uid 22791); 14 Aug 2008 21:20:55 -0000 X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.31) with ESMTP; Thu, 14 Aug 2008 21:20:19 +0000 Received: (qmail 21160 invoked from network); 14 Aug 2008 21:20:17 -0000 Received: from unknown (HELO orlando.local) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 14 Aug 2008 21:20:17 -0000 From: Pedro Alves To: gdb-patches@sourceware.org Subject: Re: [rfc] Preferred thread event reporting: Linux native target Date: Thu, 14 Aug 2008 21:20:00 -0000 User-Agent: KMail/1.9.9 Cc: "Ulrich Weigand" References: <200808142039.m7EKdxmf021349@d12av02.megacenter.de.ibm.com> In-Reply-To: <200808142039.m7EKdxmf021349@d12av02.megacenter.de.ibm.com> MIME-Version: 1.0 Content-Type: Multipart/Mixed; boundary="Boundary-00=_iGKpIO3xme7eLm0" Message-Id: <200808142220.34755.pedro@codesourcery.com> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-08/txt/msg00390.txt.bz2 --Boundary-00=_iGKpIO3xme7eLm0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Content-length: 2348 Hi Ulrich, On Thursday 14 August 2008 21:39:59, Ulrich Weigand wrote: > It seems to me that this implementation detail should not determine > user-visible behaviour in the way described above. Therefore, I'd suggest > to try to *always* prefer reporting events in the thread the user currently > "cares about" Agreed in principle, although that may create contention. But as you say, if the user is stepping a thread, your proposal seems less surprising. > -- this is actually simply the currently selected thread > (i.e. the current value of inferior_ptid). Disagreed. inferior_ptid will change if an event happens in another thread while you're stepping, but the core decides the event was not a good reason to stop. E.g., thread hopping. So, resetting the prefered thread here: > (linux_nat_resume): Set lp->preferred. .. is fallible. > The patch below implements this by adding a new member "preferred" to > "struct lwp_info", setting it according to the value of inferior_ptid > in linux_nat_resume, and using it (instead of the single-step flag) to > decide whether to prefer reporting events in this thread. I'd prefer to check if an lwp is stepping due to user request, by checking struct thread_info's data directly, intead of your "prefered" flag. We already have all the needed data in struct thread_info, although, because of context-switching, if the lwp matches inferior_ptid, you have to check the stepping state in the global vars; if it doesn't match inferior_ptid (which again, is not garanteed to be the last thread the user had selected), you get the stepping data directly from the thread_info list. I happen to be just preparing to submit the series that gets rid of context-switching, which gets rid of that special case. Unfortunatelly, currently, GDB doesn't always correctly clear the stepping state of all threads when proceeding (clear_proceed_status only clears the current thread), but I'm addressing that too in the series, see attached. Maybe the attached could be rebased against pristine head, and you could use it, although I would prefer to put the whole series in. For my series to go in, every target much register at least the main thread in GDB's thread tables, and as it happens, I think AIX is the only target I don't have covered, or that I know of no one covering. -- Pedro Alves --Boundary-00=_iGKpIO3xme7eLm0 Content-Type: text/x-diff; charset="iso-8859-1"; name="clear_proceed_other_threads.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="clear_proceed_other_threads.diff" Content-length: 2774 2008-07-31 Pedro Alves Clear proceed status of all threads that are going to be resumed. * infrun.c (clear_proceed_status_thread_callback): New. (clear_proceed_status): Use it. (proceed): Clear the proceed status off all threads that are going to be resumed. --- gdb/infrun.c | 49 ++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 38 insertions(+), 11 deletions(-) Index: src/gdb/infrun.c =================================================================== --- src.orig/gdb/infrun.c 2008-08-14 04:05:15.000000000 +0100 +++ src/gdb/infrun.c 2008-08-14 04:05:17.000000000 +0100 @@ -1081,6 +1081,29 @@ a command like `return' or `jump' to con /* Proceeding. */ +static int +clear_proceed_status_thread_callback (struct thread_info *tp, void *data) +{ + struct thread_info *ignore = data; + + if (ignore == tp) + return 0; + + if (is_exited (tp->ptid)) + return 0; + + tp->step_range_start = 0; + tp->step_range_end = 0; + tp->step_frame_id = null_frame_id; + tp->step_over_calls = STEP_OVER_UNDEBUGGABLE; + tp->proceed_to_finish = 0; + + /* Discard any remaining commands or status from previous stop. */ + bpstat_clear (&tp->stop_bpstat); + + return 0; +} + /* Clear out all variables saying what to do when inferior is continued. First do this, then set the ones you want, then call `proceed'. */ @@ -1091,19 +1114,10 @@ clear_proceed_status (void) { struct thread_info *tp = inferior_thread (); - tp->trap_expected = 0; - tp->step_range_start = 0; - tp->step_range_end = 0; - tp->step_frame_id = null_frame_id; - tp->step_over_calls = STEP_OVER_UNDEBUGGABLE; + clear_proceed_status_thread_callback (tp, NULL); + tp->trap_expected = 0; tp->stop_step = 0; - - tp->proceed_to_finish = 0; - - /* Discard any remaining commands or status from previous - stop. */ - bpstat_clear (&tp->stop_bpstat); } stop_after_trap = 0; @@ -1267,6 +1281,19 @@ proceed (CORE_ADDR addr, enum target_sig inferior. */ gdb_flush (gdb_stdout); + if (!non_stop + && (scheduler_mode == schedlock_off + || (scheduler_mode == schedlock_step && !step))) + { + /* We're going to let all threads run. Clear the user stepping + state of all other threads but the selected thread. The + selected thread is already taken care of and may be starting + a step request, so leave it be. */ + + iterate_over_threads (clear_proceed_status_thread_callback, + inferior_thread ()); + } + /* Refresh prev_pc value just prior to resuming. This used to be done in stop_stepping, however, setting prev_pc there did not handle scenarios such as inferior function calls or returning from --Boundary-00=_iGKpIO3xme7eLm0--