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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id BF380C433EF for ; Fri, 24 Sep 2021 15:13:13 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 95CED61076 for ; Fri, 24 Sep 2021 15:13:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1346999AbhIXPOp (ORCPT ); Fri, 24 Sep 2021 11:14:45 -0400 Received: from out01.mta.xmission.com ([166.70.13.231]:44548 "EHLO out01.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1346962AbhIXPOp (ORCPT ); Fri, 24 Sep 2021 11:14:45 -0400 Received: from in02.mta.xmission.com ([166.70.13.52]:58196) by out01.mta.xmission.com with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.93) (envelope-from ) id 1mTmsw-006RNF-7i; Fri, 24 Sep 2021 09:13:10 -0600 Received: from ip68-227-160-95.om.om.cox.net ([68.227.160.95]:44116 helo=email.xmission.com) by in02.mta.xmission.com with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.93) (envelope-from ) id 1mTmsu-009Kdu-3q; Fri, 24 Sep 2021 09:13:09 -0600 From: ebiederm@xmission.com (Eric W. Biederman) To: Linus Torvalds Cc: Paolo Bonzini , Thomas Gleixner , Oleg Nesterov , Al Viro , Linux Kernel Mailing List , KVM list References: <20210923181252.44385-1-pbonzini@redhat.com> Date: Fri, 24 Sep 2021 10:13:00 -0500 In-Reply-To: (Linus Torvalds's message of "Thu, 23 Sep 2021 11:35:06 -0700") Message-ID: <87r1deqa6b.fsf@disp2133> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-XM-SPF: eid=1mTmsu-009Kdu-3q;;;mid=<87r1deqa6b.fsf@disp2133>;;;hst=in02.mta.xmission.com;;;ip=68.227.160.95;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX1+1c3ufRhzrTpnNjJCv5wkwlRCouBizLRc= X-SA-Exim-Connect-IP: 68.227.160.95 X-SA-Exim-Mail-From: ebiederm@xmission.com Subject: Re: [GIT PULL] KVM/rseq changes for Linux 5.15-rc3 X-SA-Exim-Version: 4.2.1 (built Sat, 08 Feb 2020 21:53:50 +0000) X-SA-Exim-Scanned: Yes (on in02.mta.xmission.com) Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org Linus Torvalds writes: > On Thu, Sep 23, 2021 at 11:13 AM Paolo Bonzini wrote: >> >> A fix for a bug with restartable sequences and KVM. KVM's handling >> of TIF_NOTIFY_RESUME, e.g. for task migration, clears the flag without >> informing rseq and leads to stale data in userspace's rseq struct. > > Ok, patches look reasonable. > >> I'm sending this as a separate pull request since it's not code >> that I usually touch. In particular, patch 2 ("entry: rseq: Call >> rseq_handle_notify_resume() in tracehook_notify_resume()") is just a >> cleanup to try and make future bugs less likely. If you prefer this to >> be sent via Thomas and only in 5.16, please speak up. > > So I took the pull request this way, thanks for separating it like this. > > But I'm adding a few people to the cc for a completely different > reason: the cleanup to move all the notify_resume stuff to > tracehook_notify_resume() is good, but it does make me go - once again > - "Hmm, that naming is really really bad". > > The code was literally meant for tracing. It's > where the name comes from, and it's the original intent: having a > place that you can hook into for tracing that doesn't depend on how > the core kernel code ends up changing. > > But that's not how it actually acts right now. That header file is now > some very core functionality, and little of it is actually related to > tracing any more. It's more core process state handling for the user > space return path. Yes. The tracehook header was a precursor to merging utrace which ultimately was replaced by uprobes. Quite a few of the tracehooks hooks have become regular ptrace hooks over the years, and left tracehook.h It looks like that is the path that should happen with the rest of the hooks as well. It looks like: tracehook_report_syscall_entry, and tracehook_report_syscall_exit should just become ptrace_report_syscall_entry and ptrace_report_syscall_exit. That tracehook_signal_handler should just be inlined into it's one caller. That leaves set_notify_resume, tracehook_notify_resume, tracehook_notify_signal, and set_notify_signal. I am still waiting to hear if we can just remove tracehook_notify_signal now that io_uring has become an ordinary process thing. It looks like tracehook_notify_resume should be renamed and put somewhere I just don't know where. The config option HAVE_ARCH_TRACEHOOK appears to have nothing to do with the header tracehook.h any more. It looks to be just about regsets, and task_current_syscall. It looks like only alpha, h8300, m68k, and microblaze need an implementation and then we can make all of the code that depends upon HAVE_ARCH_TRACEHOOK unconditional. > So I don't object to the patches, and they are merged, but I'm cc'ing people to > > (a) let them know about this (see commit a68de80f61f6: "entry: rseq: > Call rseq_handle_notify_resume() in tracehook_notify_resume()" in the > current -git tree) > > (b) possibly prod some people into perhaps moving/renaming some of > that code to actual core kernel C files, instead of a misnamed header > file.. > > Hmm? It is on my radar. Does anyone have any idea what to call tracehook_notify_resume so that it describes it's current usage? Eric