Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Yao Qi <qiyaoltc@gmail.com>
To: Pedro Alves <palves@redhat.com>
Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
Subject: Re: [PATCH 7/9] Enqueue signal even when resuming threads
Date: Fri, 01 Jul 2016 16:45:00 -0000	[thread overview]
Message-ID: <CAH=s-PM38QRW2ofxiL3cBfb7UB_h1e=Lca82VLz-i3XUKZR3DA@mail.gmail.com> (raw)
In-Reply-To: <4a3c91d8-85bb-31f2-7f9e-bc0fe0de0ff6@redhat.com>

On Fri, Jul 1, 2016 at 4:06 PM, Pedro Alves <palves@redhat.com> wrote:
> On 06/30/2016 03:09 PM, Yao Qi wrote:
>> Nowadays, we only enqueue signal when we leave thread pending in
>> linux_resume_one_thread.  If lwp->resume->sig isn't zero (GDB wants
>> to resume with signal), we pass lwp->resume->sig to
>> linux_resume_one_lwp.
>>
>> In order to reduce the difference between resuming thread with signal
>> and proceeding thread with signal, when we resume thread, we can
>> enqueue signal too, and proceed thread.  The signal will be consumed in
>> linux_resume_one_lwp_throw from lwp->pending_signals.
>
> This makes one subtle change.  If the thread already
> had a pending signal, and we're getting a resume request with
> a signal that is a different signal from the one already queued,
> then before the patch, we'd tell the kernel to deliver the new
> signal first, and then only deliver the pending signal the next time
> the thread is resumed.  While after the patch, we'll enqueue the
> new signal, and deliver the one that was already pending first.
>
> Can't really say whether the old behavior was necessary.  At least
> the new behavior seems to make order or signal delivery consistent
> with how the order the signals were made pending in the
> first place, so seems better.
>
> This made me notice that this scenario of having more than one
> pending signal doesn't seem to be handled perfectly.  We deliver
> the first signal, but nothing makes sure to get back control
> of the thread immediately in order to deliver the other pending
> signal, so it seems the thread may execute a bunch of arbitrary code
> until it next stops and is re-resumed, at which point we'll deliver
> the other pending signal.  Maybe the simplest would be to
> force the thread to immediately stop again, by calling
> send_sigstop before resuming it, whenever we have more pending
> signals.  Subject for a separate patch, in any case.

You meant "after resuming it" rather than "before resuming it", right?  We have
two pending signals, so we resume the lwp and deliver the first signal.  After
resuming, we need to immediately deliver the second signal, so we call
send_sigstop.

IIUC, this patch is OK as-is, right?

-- 
Yao (齐尧)


  reply	other threads:[~2016-07-01 16:45 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-30 14:09 [PATCH 0/9 V3] Use reinsert breakpoint for vCont;s Yao Qi
2016-06-30 14:09 ` [PATCH 3/9] Refactor clone_all_breakpoints Yao Qi
2016-06-30 14:09 ` [PATCH 1/9] Pass breakpoint type in set_breakpoint_at Yao Qi
2016-06-30 14:09 ` [PATCH 9/9] Support vCont s and S actions with software single step Yao Qi
2016-06-30 14:09 ` [PATCH 7/9] Enqueue signal even when resuming threads Yao Qi
2016-07-01 15:06   ` Pedro Alves
2016-07-01 16:45     ` Yao Qi [this message]
2016-07-01 16:55       ` Pedro Alves
2016-07-01 17:01         ` Pedro Alves
2016-06-30 14:09 ` [PATCH 5/9] Switch current_thread to lwp's thread in install_software_single_step_breakpoints Yao Qi
2016-06-30 14:09 ` [PATCH 6/9] Use enqueue_pending_signal in linux_resume_one_thread Yao Qi
2016-06-30 14:09 ` [PATCH 8/9] Use reinsert_breakpoint for vCont;s Yao Qi
2016-07-01 15:07   ` Pedro Alves
2016-07-05  8:15     ` Yao Qi
2016-07-21  8:38       ` Yao Qi
2016-07-21 10:02       ` Pedro Alves
2016-06-30 14:09 ` [PATCH 4/9] Make reinsert_breakpoint thread specific Yao Qi
2016-06-30 14:09 ` [PATCH 2/9] Create sub classes of 'struct breakpoint' Yao Qi
2016-07-21 11:18 ` [PATCH 0/9 V3] Use reinsert breakpoint for vCont;s Yao Qi
2016-11-14 19:14 ` Antoine Tremblay
2016-11-21 12:08   ` Yao Qi
     [not found]     ` <wwok37ikrgmq.fsf@ericsson.com>
2016-11-23 19:03       ` Antoine Tremblay
2016-11-24 21:55       ` Yao Qi
2016-11-25 12:22         ` Antoine Tremblay
2016-11-25 13:13           ` Antoine Tremblay
2016-11-25 13:35             ` Antoine Tremblay
2016-11-25 13:44             ` Pedro Alves
2016-11-25 13:57               ` Antoine Tremblay
2016-11-25 14:28                 ` Antoine Tremblay

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAH=s-PM38QRW2ofxiL3cBfb7UB_h1e=Lca82VLz-i3XUKZR3DA@mail.gmail.com' \
    --to=qiyaoltc@gmail.com \
    --cc=gdb-patches@sourceware.org \
    --cc=palves@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox