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 X-Spam-Level: X-Spam-Status: No, score=-13.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D9BCDC433E2 for ; Tue, 23 Mar 2021 10:42:24 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id A6C75619BD for ; Tue, 23 Mar 2021 10:42:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230260AbhCWKlr (ORCPT ); Tue, 23 Mar 2021 06:41:47 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55710 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230269AbhCWKlf (ORCPT ); Tue, 23 Mar 2021 06:41:35 -0400 Received: from mail-oo1-xc35.google.com (mail-oo1-xc35.google.com [IPv6:2607:f8b0:4864:20::c35]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0085EC061764 for ; Tue, 23 Mar 2021 03:41:34 -0700 (PDT) Received: by mail-oo1-xc35.google.com with SMTP id r17-20020a4acb110000b02901b657f28cdcso4820158ooq.6 for ; Tue, 23 Mar 2021 03:41:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=qaMgAdM2KItPrKUMCZHabW9a3SL1RYlGywDdCoOttJE=; b=HaI/Ls3Xxp4PiIqn7YsRY6f0Wr7X/xAaryHq0Ns2IEfD0GdSCR+4ITbGkbJT9UXXsT kY1r5QTmgtILT6A0Ne1jGw3AadSpClsCy4ta4xdHyx2yexqhxsoCMXKNWtx9NgShVWAc ogQ4n8oSLpEG3tsagpYf3fWuTd8Pe7N0n404eqU0dqGgqUyHjWK3hr+cPkggOYqfoRXn A2vbwFHHLtsODjLZuiJ86pZ4uGIMzZUmInN6SWqsIKZQZCPlxb69hD4CP4aZg27rtAgK YccAtquOslR1+Z6W9VmOh5EyQ6Km8qzQean7EpYrWGCBLstwapCRvahHJkFGzTcoPFM7 ZEpQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=qaMgAdM2KItPrKUMCZHabW9a3SL1RYlGywDdCoOttJE=; b=BBYNG2nU9koDEtc36CFhL/eb8zBwd1E4vMK9iBkgMfAYItKGr04OG+ybxswZg8ZO29 GM2FJMZLaMA20egrNpFXaHBkhCdyjeHOp9MIbfnYQc42e8WlhHClQskq9qgbsvQAJuaP qrfdrl4i/7PqVXUDvuVIRItGpkg+RNbhq/OhQW6nMUiY2zQtxC6asjU1lcz5pUtvPa8q cLXO0Ukpe0E4dFw2t5Wes9xNzsrc0AOTTbOki0Mjv1joCXpDE3kfhgM3V2xSVy5HRvHm d9U0vhdPEQiHlMQAu1fvbtQRq6PRJyAsHByxEy6RFR5TptK89KroY3zMLBjj1y7vK7Ok PBvA== X-Gm-Message-State: AOAM532NpXy8WhwHv2UuEMhVsUKkZlMXSdxrWgasMp6fVrG3W7V9iitg ShfhtWgS+w1fCQoBUp5MHn6LxpGjY1aWuiUUv7hpoA== X-Google-Smtp-Source: ABdhPJx9cOfUC85pOdYrj8gQKReOta+wufUAU0wiHMXFo141kFatu3HmQkubYd5iSEuvXln2c4xjLNLCd7IZj2+2Fec= X-Received: by 2002:a05:6820:273:: with SMTP id c19mr3310170ooe.54.1616496093820; Tue, 23 Mar 2021 03:41:33 -0700 (PDT) MIME-Version: 1.0 References: <20210310104139.679618-1-elver@google.com> <20210310104139.679618-9-elver@google.com> In-Reply-To: From: Marco Elver Date: Tue, 23 Mar 2021 11:41:22 +0100 Message-ID: Subject: Re: [PATCH RFC v2 8/8] selftests/perf: Add kselftest for remove_on_exec To: Peter Zijlstra Cc: Alexander Shishkin , Arnaldo Carvalho de Melo , Ingo Molnar , Jiri Olsa , Mark Rutland , Namhyung Kim , Thomas Gleixner , Alexander Potapenko , Al Viro , Arnd Bergmann , Christian Brauner , Dmitry Vyukov , Jann Horn , Jens Axboe , Matt Morehouse , Peter Collingbourne , Ian Rogers , kasan-dev , linux-arch , linux-fsdevel , LKML , "the arch/x86 maintainers" , "open list:KERNEL SELFTEST FRAMEWORK" Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 23 Mar 2021 at 11:32, Peter Zijlstra wrote: > > On Tue, Mar 23, 2021 at 10:52:41AM +0100, Marco Elver wrote: > > > with efs->func==__perf_event_enable. I believe it's sufficient to add > > > > mutex_lock(&parent_event->child_mutex); > > list_del_init(&event->child_list); > > mutex_unlock(&parent_event->child_mutex); > > > > right before removing from context. With the version I have now (below > > for completeness), extended torture with the above test results in no > > more warnings and the test also passes. > > > > > + list_for_each_entry_safe(event, next, &ctx->event_list, event_entry) { > > + struct perf_event *parent_event = event->parent; > > + > > + if (!event->attr.remove_on_exec) > > continue; > > > > + if (!is_kernel_event(event)) > > + perf_remove_from_owner(event); > > > > + modified = true; > > + > > + if (parent_event) { > > /* > > + * Remove event from parent, to avoid race where the > > + * parent concurrently iterates through its children to > > + * enable, disable, or otherwise modify an event. > > */ > > + mutex_lock(&parent_event->child_mutex); > > + list_del_init(&event->child_list); > > + mutex_unlock(&parent_event->child_mutex); > > } > > ^^^ this, right? > > But that's something perf_event_exit_event() alread does. So then you're > worried about the order of things. Correct. We somehow need to prohibit the parent from doing an event_function_call() while we potentially deactivate the context with perf_remove_from_context(). > > + > > + perf_remove_from_context(event, !!event->parent * DETACH_GROUP); > > + perf_event_exit_event(event, ctx, current, true); > > } > > perf_event_release_kernel() first does perf_remove_from_context() and > then clears the child_list, and that makes sense because if we're there, > there's no external access anymore, the filedesc is gone and nobody will > be iterating child_list anymore. > > perf_event_exit_task_context() and perf_event_exit_event() OTOH seem to > rely on ctx->task == TOMBSTONE to sabotage event_function_call() such > that if anybody is iterating the child_list, it'll NOP out. > > But here we don't have neither, and thus need to worry about the order > vs child_list iteration. > > I suppose we should stick sync_child_event() in there as well. > > And at that point there's very little value in still using > perf_event_exit_event()... let me see if there's something to be done > about that. I don't mind dropping use of perf_event_exit_event() and open coding all of this. That would also avoid modifying perf_event_exit_event(). But I leave it to you what you think is nicest. Thanks, -- Marco