From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 1D803C433EF for ; Mon, 17 Jan 2022 16:23:35 +0000 (UTC) Received: from localhost ([::1]:55818 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1n9Un7-000774-Um for qemu-devel@archiver.kernel.org; Mon, 17 Jan 2022 11:23:33 -0500 Received: from eggs.gnu.org ([209.51.188.92]:59304) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n9UmC-00060M-GK for qemu-devel@nongnu.org; Mon, 17 Jan 2022 11:22:36 -0500 Received: from [2607:f8b0:4864:20::931] (port=46915 helo=mail-ua1-x931.google.com) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1n9UmA-0000DG-Ce for qemu-devel@nongnu.org; Mon, 17 Jan 2022 11:22:36 -0500 Received: by mail-ua1-x931.google.com with SMTP id c36so31314011uae.13 for ; Mon, 17 Jan 2022 08:22:33 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20210112.gappssmtp.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=lVl2uz5rc8KYOGNuGY1BDy5ZfB91bfpbYiHaxU29TPE=; b=Gd7lJw4nqDDQY/LiROxtgQ5EdkBPiE0fMnr4XuCU6kiyH5fJMXR47iMYyAazm7Yz6/ usRdprgLDTzQxBNNN6QiSJuCTARPvK2t/lIU68/XqxlL0k8efmrECTL1GwpEa0I16sW9 Zlgl4VCjfgokrDibHcQ87M820ys0OAVIaIeXtsY7U+iRHIw8tZnOG9UjwnTRvM2tgyyK m/6LluP14oYa/TK6SGXPZ+apJi8AXMkBnqIpsIZORhGV1E2HK/4PTWU9hm/oL4H0FNMz UBM4Q8i2pyESTeETOOpobslt+LxSjfAK7nXik9HcS6i3clAmD/IeMQYTbvqp1bmbx8bF 8gkg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=lVl2uz5rc8KYOGNuGY1BDy5ZfB91bfpbYiHaxU29TPE=; b=2tNUmOQZnIUhMTcrMIRgnhc+ZDAs65gqiV4gnrrPpHGUGldbPVRCGueb2/qU4Mdy9A /Yw2aaXM5hO31uGSzmkPzcBbXHUs1tKvdHOMibAhQ9/28CCWQ0kIYwnwuWhyo2sTtq4Q IlV7gO1IbnGkunTB9X9hqMzPVooRPyQAMJo/chYrVsQhgvRCCxKYyFhUOkYWOetesJ19 J6a+UdpLwp8L0etdpsL3aFiQV8aXdv6ufscZiamEhjCLqQwnI/duCbsxZZYW/vfFiP5Q 66gujRpTRdm8C8pb44idkvCjzWOVt7YlUvkLOONJGbr/l4O9M3OwL2wuQAn8L9fnA8TW o0dA== X-Gm-Message-State: AOAM530bH+2liJ9FKU2QepIumXZ4Kw+yTea7nQY94O0NIlaXzGagg9e8 cbiZNr0ZsMQVXcv3OjUy3jU/qHAGzlbZ5yWONLRsVw== X-Google-Smtp-Source: ABdhPJyi7V22p4bMUB89aIJbxlc+DrX9tmmhuT1BlAIcIJm7W4D2zmReIh9Eerte2PAP7Rw6cSK8dgSmEPXahkqcSQI= X-Received: by 2002:a05:6102:ecf:: with SMTP id m15mr7116663vst.68.1642436552950; Mon, 17 Jan 2022 08:22:32 -0800 (PST) MIME-Version: 1.0 References: <20220109161923.85683-1-imp@bsdimp.com> <20220109161923.85683-23-imp@bsdimp.com> In-Reply-To: From: Warner Losh Date: Mon, 17 Jan 2022 09:22:22 -0700 Message-ID: Subject: Re: [PATCH 22/30] bsd-user/signal.c: Fill in queue_signal To: Peter Maydell Content-Type: multipart/alternative; boundary="0000000000004e2c9705d5c98f90" X-Host-Lookup-Failed: Reverse DNS lookup failed for 2607:f8b0:4864:20::931 (failed) Received-SPF: none client-ip=2607:f8b0:4864:20::931; envelope-from=wlosh@bsdimp.com; helo=mail-ua1-x931.google.com X-Spam_score_int: -10 X-Spam_score: -1.1 X-Spam_bar: - X-Spam_report: (-1.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, HTML_MESSAGE=0.001, PDS_HP_HELO_NORDNS=0.001, RCVD_IN_DNSWL_NONE=-0.0001, RDNS_NONE=0.793, SPF_HELO_NONE=0.001, SPF_NONE=0.001 autolearn=no autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kyle Evans , Stacey Son , QEMU Developers Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" --0000000000004e2c9705d5c98f90 Content-Type: text/plain; charset="UTF-8" On Thu, Jan 13, 2022 at 1:37 PM Peter Maydell wrote: > On Sun, 9 Jan 2022 at 16:51, Warner Losh wrote: > > > > Fill in queue signal implementation, as well as routines allocate and > > delete elements of the signal queue. > > See reply to patch 18 for why you probably don't want to do this. > I've kept the former bits (implementing queue_signal() function), but removed the rest. > > Signed-off-by: Stacey Son > > Signed-off-by: Kyle Evans > > Signed-off-by: Warner Losh > > --- > > > > + /* > > + * FreeBSD signals are always queued. Linux only queues real time > signals. > > + * XXX this code is not thread safe. "What lock protects > ts->sigtab?" > > + */ > > ts->sigtab shouldn't need a lock, because it is per-thread, > like all of TaskState. (The TaskState structure is pointed > to by the CPUState 'opaque' field. CPUStates are per-thread; > the TaskState for a new thread's new CPUState is allocated > and initialized as part of the emulating of whatever the > "create new thread" syscall is. For Linux this is in > do_fork() for the CLONE_VM case. The TaskState for the > initial thread is allocated in main.c.) We do need to deal > with the fact that ts->sigtab can be updated by a signal > handler (which always runs in the thread corresponding to > that guest CPU): the linux-user process_pending_signals() > has been written with that in mind. > Gotcha. That makes sense. Any reason that atomics aren't used for this between the different routines? Warner --0000000000004e2c9705d5c98f90 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Thu, Jan 13, 2022 at 1:37 PM Peter= Maydell <peter.maydell@lina= ro.org> wrote:
On Sun, 9 Jan 2022 at 16:51, Warner Losh <imp@bsdimp.com> wrote:
>
> Fill in queue signal implementation, as well as routines allocate and<= br> > delete elements of the signal queue.

See reply to patch 18 for why you probably don't want to do this.

I've kept the former bits (implementing = queue_signal() function), but removed
the rest.
=C2=A0<= /div>
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Kyle Evans <kevans@freebsd.org>
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---


> +=C2=A0 =C2=A0 /*
> +=C2=A0 =C2=A0 =C2=A0* FreeBSD signals are always queued.=C2=A0 Linux = only queues real time signals.
> +=C2=A0 =C2=A0 =C2=A0* XXX this code is not thread safe.=C2=A0 "W= hat lock protects ts->sigtab?"
> +=C2=A0 =C2=A0 =C2=A0*/

ts->sigtab shouldn't need a lock, because it is per-thread,
like all of TaskState. (The TaskState structure is pointed
to by the CPUState 'opaque' field. CPUStates are per-thread;
the TaskState for a new thread's new CPUState is allocated
and initialized as part of the emulating of whatever the
"create new thread" syscall is. For Linux this is in
do_fork() for the CLONE_VM case. The TaskState for the
initial thread is allocated in main.c.) We do need to deal
with the fact that ts->sigtab can be updated by a signal
handler (which always runs in the thread corresponding to
that guest CPU): the linux-user process_pending_signals()
has been written with that in mind.

Got= cha. That makes sense. Any reason that atomics aren't used
fo= r this between the different routines?

Warner=C2= =A0
--0000000000004e2c9705d5c98f90--