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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7326CC433FE for ; Wed, 2 Nov 2022 19:39:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230161AbiKBTjs (ORCPT ); Wed, 2 Nov 2022 15:39:48 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53212 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229742AbiKBTjq (ORCPT ); Wed, 2 Nov 2022 15:39:46 -0400 Received: from mail-pj1-x1035.google.com (mail-pj1-x1035.google.com [IPv6:2607:f8b0:4864:20::1035]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 51A35112E for ; Wed, 2 Nov 2022 12:39:45 -0700 (PDT) Received: by mail-pj1-x1035.google.com with SMTP id gw22so4279195pjb.3 for ; Wed, 02 Nov 2022 12:39:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=Oy/+InM7thBp0kwixsoFXfh5sgFzh+UFZjFGkPyLYAQ=; b=AlcBvu+Y8a9n0rxdCd7zQ9e17/IgiXWiW1brOwB5ZbqKr0SLoquafVF2YULPC3vlet uSvOds2cnLBsHxmhHE7ndhPTYL5pauSfC7xzaQss2UczgTV3VYIG6/OP33iqNuAJKL+d LvWvNA2iJGJPbsOQpB/Dji8i51YZCf6Q/ISjFxFbxFbku9lnCGmjRd0Dgdx9WZGItFV/ 2AiTf+3QkbGxpIrmP/8lnU62kw9iNvAiIoGPBkKzwLwHrqSFwtZxTIHgIhpOED9s7cKB 3c4cmTHomEsxZCom4U2kcddeyipmi4giT/3jA5bvxdMV/McfpAXZhOh+lEB408GUCwPX AW1w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=Oy/+InM7thBp0kwixsoFXfh5sgFzh+UFZjFGkPyLYAQ=; b=pQ5RcICDHoq8zZIS200cCPU2NoGovmpxHNsiAZ4Gp6va6rmo/vOGpStwxtOc1JRMse XN9QQqJcj2WJsTx9AnqmQqaUztNU7Nxo9I8jVLmiaV9Ew5ON9lKqD91nlJWsrkXthsJP iW1tJaxK0ZeJWim9PWNwZrhbxF1eZv53g0iGzoVb4X+id0GiqLeAKk6fc2uM3vmrnnLg UG9vd2a4JAPtbDoTOUesjHfJfMNiuECt7ut6YrVC253FAVEJ9DD3XLqm5JrRo3SqqXoo mW74guuHxocjIffQTLZvBnH4nwCGfGVSCqqpenILeyklWu9AjJHHCKSSd3fOzs3DbfH/ BGvQ== X-Gm-Message-State: ACrzQf2iW8MCl0saGPXwx50ckZ8XsqqBW/EX5ela4WIFOOJa7ZJKm08j EWioap+dJof4SqNgP/Lh5EW7OPgS/5wQeA== X-Google-Smtp-Source: AMsMyM4nTTPc3fRpd+P93lNOuXEOQVEyD2E+BsPyWFnexxUNkM1wQSGyy2pTEo93wVPY4+XonbOyww== X-Received: by 2002:a17:90b:3144:b0:215:db2e:bc6e with SMTP id ip4-20020a17090b314400b00215db2ebc6emr1492273pjb.12.1667417984739; Wed, 02 Nov 2022 12:39:44 -0700 (PDT) Received: from google.com (7.104.168.34.bc.googleusercontent.com. [34.168.104.7]) by smtp.gmail.com with ESMTPSA id w2-20020aa79542000000b0054ee4b632dasm8813384pfq.169.2022.11.02.12.39.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 02 Nov 2022 12:39:44 -0700 (PDT) Date: Wed, 2 Nov 2022 19:39:40 +0000 From: Sean Christopherson To: Paolo Bonzini Cc: Bo Liu , kvm@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] KVM: replace DEFINE_SIMPLE_ATTRIBUTE with DEFINE_DEBUGFS_ATTRIBUTE Message-ID: References: <20221101072506.7307-1-liubo03@inspur.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Nov 02, 2022, Paolo Bonzini wrote: > On 11/1/22 08:25, Bo Liu wrote: > > Fix the following coccicheck warning: > > virt/kvm/kvm_main.c:3847:0-23: WARNING > > vcpu_get_pid_fops should be defined with DEFINE_DEBUGFS_ATTRIBUTE > > > > Signed-off-by: Bo Liu > > --- > > virt/kvm/kvm_main.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > > index f1df24c2bc84..3f383f27d3d7 100644 > > --- a/virt/kvm/kvm_main.c > > +++ b/virt/kvm/kvm_main.c > > @@ -3844,7 +3844,7 @@ static int vcpu_get_pid(void *data, u64 *val) > > return 0; > > } > > -DEFINE_SIMPLE_ATTRIBUTE(vcpu_get_pid_fops, vcpu_get_pid, NULL, "%llu\n"); > > +DEFINE_DEBUGFS_ATTRIBUTE(vcpu_get_pid_fops, vcpu_get_pid, NULL, "%llu\n"); > > static void kvm_create_vcpu_debugfs(struct kvm_vcpu *vcpu) > > { > > If you really wanted to do this, you would also have to replace > debugfs_create_file with debugfs_create_file_unsafe. > > However, this is not a good idea. The rationale in the .cocci file is that > "DEFINE_SIMPLE_ATTRIBUTE + debugfs_create_file() imposes some significant > overhead", but this should not really be relevant for a debugfs file. > > Such a patch would only make sense if there was a version of > debugfs_create_file_unsafe() with a less-terrible name (e.g. > debugfs_create_simple_attr?), which could _only_ be used with fops created > by DEFINE_DEBUGFS_ATTRIBUTE. Without such a type-safe trick, the .cocci > file is only adding confusion to perfectly fine code. Heh, some serious deja vu here[1]. This is the second case of identical, flawed patches being sent in response to misguided coccinelle warnings in a rather short amount of time, the "return min(r, 0)" horror being the other case[2][3]. The min() thing is supposed to be fixed by commit aeb300c1dbfc ("coccinelle: misc: minmax: suppress patch generation for err returns"). Is that patch broken, or are folks just running old scripts? As for the DEFINE_DEBUGFS_ATTRIBUTE check, can that warning be downgraded (is that even a thing?) or even deleted? As much as I enjoyed the opportunity to learn more about debugfs, the unnecessary confusion and wasted time was/is annoying. [1] https://lore.kernel.org/all/Yxoo1A2fmlAWruyV@google.com [2] https://lore.kernel.org/all/8881d7b4-0c31-cafd-1158-0d42c1c7f43a@redhat.com [3] https://lore.kernel.org/all/d8a518c4a4014307b30020b38022d633@AcuMS.aculab.com