From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 72155 invoked by alias); 17 Feb 2016 13:42:59 -0000 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 Received: (qmail 71811 invoked by uid 89); 17 Feb 2016 13:42:56 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=Ones X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 17 Feb 2016 13:42:46 +0000 Received: from svr-orw-fem-06.mgc.mentorg.com ([147.34.97.120]) by relay1.mentorg.com with esmtp id 1aW2NO-00062I-PQ from Luis_Gustavo@mentor.com ; Wed, 17 Feb 2016 05:42:42 -0800 Received: from [172.30.1.77] (147.34.91.1) by SVR-ORW-FEM-06.mgc.mentorg.com (147.34.97.120) with Microsoft SMTP Server id 14.3.224.2; Wed, 17 Feb 2016 05:42:42 -0800 Subject: Re: [PATCH 5/5] gdb: Coalesce/aggregate (async) vCont packets/actions References: <1455677091-13683-1-git-send-email-palves@redhat.com> <1455677091-13683-6-git-send-email-palves@redhat.com> To: Pedro Alves , Reply-To: Luis Machado From: Luis Machado Message-ID: <56C478CD.6040900@codesourcery.com> Date: Wed, 17 Feb 2016 13:42:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <1455677091-13683-6-git-send-email-palves@redhat.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2016-02/txt/msg00508.txt.bz2 I missed 5/5... Mostly nits, overall it looks sane to me. On 02/17/2016 12:44 AM, Pedro Alves wrote: > diff --git a/gdb/infrun.c b/gdb/infrun.c > index 15210c9..f72937c 100644 > --- a/gdb/infrun.c > +++ b/gdb/infrun.c > @@ -2357,6 +2357,8 @@ do_target_resume (ptid_t resume_ptid, int step, enum gdb_signal sig) > target_pass_signals ((int) GDB_SIGNAL_LAST, signal_pass); > > target_resume (resume_ptid, step, sig); > + > + target_do_resume (); > } > I'm looking at the sequence of function names and they don't look too clear. do_target_resume/target_resume/target_do_resume. Should we have better names for these functions? Ones that make it more explicit what each function is doing and the fact that we are potentially defering resumptions? Like "queue_resume_actions" for target_resume or "commit_resumption_actions" for target_do_resume? > @@ -6200,6 +6555,56 @@ remove_new_fork_children (struct threads_listing_context *context) > remove_child_of_pending_fork, ¶m); > } > > +/* Check whether EVENT would prevent a global or process wildcard > + vCont action. */ > + > +static int > +check_pending_event_prevents_wildcard_vcont_callback > + (QUEUE (stop_reply_p) *q, > + QUEUE_ITER (stop_reply_p) *iter, > + stop_reply_p event, > + void *data) > +{ > + struct inferior *inf; > + int *may_global_wildcard_vcont = (int *) data; > + > + if (event->ws.kind == TARGET_WAITKIND_NO_RESUMED > + || event->ws.kind == TARGET_WAITKIND_NO_HISTORY) > + return 1; > + > + if (event->ws.kind == TARGET_WAITKIND_FORKED > + || event->ws.kind == TARGET_WAITKIND_VFORKED) > + *may_global_wildcard_vcont = 0; > + > + inf = find_inferior_ptid (event->ptid); > + > + /* This may be the first time we heard about this process. > + Regardless, we must not do a global wildcard resume, otherwise > + we'd resume this process too. */ > + *may_global_wildcard_vcont = 0; > + if (inf != NULL) > + inf->priv->may_wildcard_vcont = 0; > + > + return 1; > +} > + > +/* Check whether any event pending in the vStopped queue would prevent > + a global or process wildcard vCont action. Clear > + *may_global_wildcard if we can't do a global wildcard (vCont;c), > + and clear the event inferior's may_wildcard_vcont flag if we can do > + a process-wide wildcard resume (vCont;c:pPID.-1). */ > + ... inferior's may_wildcard_vcont flag if we can do ... Can do or can't do? > diff --git a/gdb/target.h b/gdb/target.h > index 7c8513a..c40d1c7 100644 > --- a/gdb/target.h > +++ b/gdb/target.h > @@ -461,6 +461,8 @@ struct target_ops > int TARGET_DEBUG_PRINTER (target_debug_print_step), > enum gdb_signal) > TARGET_DEFAULT_NORETURN (noprocess ()); > + void (*to_do_resume) (struct target_ops *) > + TARGET_DEFAULT_IGNORE (); > ptid_t (*to_wait) (struct target_ops *, > ptid_t, struct target_waitstatus *, > int TARGET_DEBUG_PRINTER (target_debug_print_options)) > @@ -1317,19 +1319,41 @@ extern void target_detach (const char *, int); > > extern void target_disconnect (const char *, int); > > -/* Resume execution of the target process PTID (or a group of > - threads). STEP says whether to hardware single-step or to run free; > - SIGGNAL is the signal to be given to the target, or GDB_SIGNAL_0 for no > - signal. The caller may not pass GDB_SIGNAL_DEFAULT. A specific > - PTID means `step/resume only this process id'. A wildcard PTID > - (all threads, or all threads of process) means `step/resume > - INFERIOR_PTID, and let other threads (for which the wildcard PTID > - matches) resume with their 'thread->suspend.stop_signal' signal > - (usually GDB_SIGNAL_0) if it is in "pass" state, or with no signal > - if in "no pass" state. */ > - > +/* Resume execution (or prepare for execution) of a target thread, > + process or all processes. STEP says whether to hardware > + single-step or to run free; SIGGNAL is the signal to be given to > + the target, or GDB_SIGNAL_0 for no signal. The caller may not pass > + GDB_SIGNAL_DEFAULT. A specific PTID means `step/resume only this > + process id'. A wildcard PTID (all threads, or all threads of > + process) means `step/resume INFERIOR_PTID, and let other threads > + (for which the wildcard PTID matches) resume with their > + 'thread->suspend.stop_signal' signal (usually GDB_SIGNAL_0) if it > + is in "pass" state, or with no signal if in "no pass" state. > + > + In order to efficiently handle batches of resumption requests, > + targets may implement this method such that it records the > + resumption request, but defers the actual resumption to the > + target_do_resume method implementation. See target_do_resume > + below. */ > extern void target_resume (ptid_t ptid, int step, enum gdb_signal signal); > > +/* Commit a series of resumption requests previously prepared with > + target_resume calls. > + > + GDB always call target_do_resume after calling target_resume or > + more times. A target may thus use this method in coordination with "GDB always calls ... target_resume one or more times."?