Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 11 Jan 2023 05:02:02 -0500
From:   Rodrigo Vivi <rodrigo.vivi@intel.com>
To:     Deepak R Varma <drv@mailo.com>
CC:     Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>,
        Saurabh Singh Sengar <ssengar@microsoft.com>,
        <dri-devel@lists.freedesktop.org>,
        Praveen Kumar <kumarpraveen@linux.microsoft.com>,
        <intel-gvt-dev@lists.freedesktop.org>,
        <intel-gfx@lists.freedesktop.org>, <linux-kernel@vger.kernel.org>,
        Zhi Wang <zhi.a.wang@intel.com>
Subject: Re: [PATCH 2/2] drm/i915/gvt: Avoid full proxy f_ops for vgpu_status
 debug attributes
Message-ID: <Y76JGj0cJpYr6/rv@intel.com>
References: <cover.1673375066.git.drv@mailo.com>
 <188df08e0feba0cda2c92145f513dd4e57c6e6cf.1673375066.git.drv@mailo.com>
 <Y72zVXYLVHXuyK05@intel.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <Y72zVXYLVHXuyK05@intel.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?ta8uFXFzIdcIJ16A2wlH7kksYT0wyBntsjmJ2zv2oU6tgiIAZ7EXSCVHHWmU?=
 =?us-ascii?Q?ku9pEhCr43tPXblkc2DHrt3+8uMGeXNJYF4q4KnrLqc1KL2f+oeyIVXulqqB?=
 =?us-ascii?Q?DcHCJLdUd+Z/yyBWr7jDE9eCBrgsNce2qhpmGfOmNO22kOU2/4FEg7V0xj6S?=
 =?us-ascii?Q?24PXXmqhq+u0JbxdZR1kmjMo9DM4YbXJ2UH2iWeLSpWSWF3Np70zTqGN54Yr?=
 =?us-ascii?Q?/EjTR/XKPpVheHP2Ip51glWuZ0fXIePm5IQho2NnHddh+EiyGTDegur/cfeO?=
 =?us-ascii?Q?e9bdIWOcU/X9shn13RNkYD85svMUaEpxYyZy4QXbDKkaYEL3Eb0j46/V5Ofv?=
 =?us-ascii?Q?Z5F+Pul2lyz7yQBrvHUFFpQu5RqaHFWAYKTnM/gJ9vfUGd49izr+BH18/8vA?=
 =?us-ascii?Q?YATp18R/t+vAdr9uwNhknnUDE7HLewa4YUMZxBFrEjJy0CGnrWAXUokyIHT7?=
 =?us-ascii?Q?9OVeAAl1yGuWRvy3cI0lSx0GWN3O9OHuV9Ex/vFqBIBnovrhallcQ8LHmJmh?=
 =?us-ascii?Q?RUKIU2X0w3HUFXdpi2JhX6PApHzAa+9Smrat5o/OpPaSfZ8dkf6yy7pmfezM?=
 =?us-ascii?Q?sTc69YUvwX9iBNP35mMRDs/ztf2dAf+F3dQxcbi0J9r7dfodNI+fXji4bSm6?=
 =?us-ascii?Q?LUhZxIXhmnmAtMOU68QmQoHdMyVkaDCj/coQHzzcDwKeCRIZMcXea7Xm3Ne2?=
 =?us-ascii?Q?q+4uPdFajRPg0eKXvkEwe7+pUcbY9D5oba9EqDacLpEMcNLMis10Yd86Ig4F?=
 =?us-ascii?Q?sN5KwjSJq86nI0YNEZ2FzfxJfYzrHLZOzd9UV+IFUSNlJyk0hJ4UVTc6FbGp?=
 =?us-ascii?Q?s7b75vhQKvjldLjVA+1LtU5fEIp0rnNHPU2o/j57m0V71AaBMqa69av32frq?=
 =?us-ascii?Q?LEpF9L5Ic+PMm1xcLTLCnDu2Q2DrBHNOYLU3Mr6s0fE6o7qJB4w2znYC2Krq?=
 =?us-ascii?Q?nEF/8RDnfesiOyO/zlU1uWWXK2+7yqy2pk8ibzCQ2jtlvMItZ/qhkBxitj2Q?=
 =?us-ascii?Q?j6qVIC35nHyakVBWW+ODuTaFQkJ99Oav7+Jx30TH3RKOpVwMiim/DOIYDKzW?=
 =?us-ascii?Q?QoDODO9ZtYK8fr4gEtXqDFw1jrcuo8Dl1Q9EMk4y6B62TCXfc6DOXBC0oDr2?=
 =?us-ascii?Q?frXxAm3lpVb/KGIhp0ZSsLGrj8kaI7rZjfQOzBoLyBaguern3ccJ6BNpVqeL?=
 =?us-ascii?Q?WjWLi/29a0S8u0Mw2N9fwjB+5XUY2B1fyMgxeVM1f5XOSs9p2o2WQcxDFbFl?=
 =?us-ascii?Q?YCNDZi/ffZHprHeqn4Wq+3nD6tGjl/JEXLtcc+fnzGBES0y+N4HX34filDd5?=
 =?us-ascii?Q?CQPzLWWclDN/sYF/Upq/a9Gy2o7CcRsCI2tik3Y4l+Iu7g6qrP/q7zc1hA8g?=
 =?us-ascii?Q?qBv6Z4rLrHzBFQqjGL8gQKUt46ohzzA5WRTfIuoSPVZgZw1VojXQW3XlD6gf?=
 =?us-ascii?Q?E+OnXJsij0Bf1GIxL5DCVud+xkaTN6LhJii4ecnWEQ9bfOJhAoXeCbtNx/0c?=
 =?us-ascii?Q?F6/BrJK7wH8G6R21pcw5yA6IVncWF9Jr4kdL3qLDrlTCqzJ0gNNi0DFYlpOR?=
 =?us-ascii?Q?BN/pXFQirDHe3m4maDmeFx3D2w7u23jfg2qmVuiesGP8HBeCJo9xVvZXLtLk?=
 =?us-ascii?Q?Mw=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: bd5ff15b-3a01-491b-07e8-08daf3bae5e5
X-MS-Exchange-CrossTenant-AuthSource: MN0PR11MB6059.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Jan 2023 10:02:07.5161
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: AS+E7NNs0ZzaY9FANFoWa4/IS0xszIHgD0ksRFTNVI5sVuZvi1e4czsIVzGXH0naEoZxu1ZBSTwTKXEleKZucg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY8PR11MB7105
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        SPF_HELO_NONE,SPF_NONE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Tue, Jan 10, 2023 at 01:49:57PM -0500, Rodrigo Vivi wrote:
> On Wed, Jan 11, 2023 at 12:00:12AM +0530, Deepak R Varma wrote:
> > Using DEFINE_SIMPLE_ATTRIBUTE macro with the debugfs_create_file()
> > function adds the overhead of introducing a proxy file operation
> > functions to wrap the original read/write inside file removal protection
> > functions. This adds significant overhead in terms of introducing and
> > managing the proxy factory file operations structure and function
> > wrapping at runtime.
> > As a replacement, a combination of DEFINE_DEBUGFS_ATTRIBUTE macro paired
> > with debugfs_create_file_unsafe() is suggested to be used instead.  The
> > DEFINE_DEBUGFS_ATTRIBUTE utilises debugfs_file_get() and
> > debugfs_file_put() wrappers to protect the original read and write
> > function calls for the debug attributes. There is no need for any
> > runtime proxy file operations to be managed by the debugfs core.
> > Following coccicheck make command helped identify this change:
> > 
> > make coccicheck M=drivers/gpu/drm/i915/ MODE=patch COCCI=./scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci
> > 
> > Signed-off-by: Deepak R Varma <drv@mailo.com>
> 
> I believe these 2 gvt cases could be done in one patch.
> But anyways,
> 
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> for both patches... and will leave these 2 patches for gvt folks
> to apply. Unless they ack and I apply in the drm-intel along with the other ones.

Actually, could you please address the checkpatch issues before we can push?
Sorry about that, but just noticed now when I was going to push the other ones.

> 
> > ---
> >  drivers/gpu/drm/i915/gvt/debugfs.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gvt/debugfs.c b/drivers/gpu/drm/i915/gvt/debugfs.c
> > index 03f081c3d9a4..baccbf1761b7 100644
> > --- a/drivers/gpu/drm/i915/gvt/debugfs.c
> > +++ b/drivers/gpu/drm/i915/gvt/debugfs.c
> > @@ -165,7 +165,7 @@ static int vgpu_status_get(void *data, u64 *val)
> >  	return 0;
> >  }
> >  
> > -DEFINE_SIMPLE_ATTRIBUTE(vgpu_status_fops, vgpu_status_get, NULL, "0x%llx\n");
> > +DEFINE_DEBUGFS_ATTRIBUTE(vgpu_status_fops, vgpu_status_get, NULL, "0x%llx\n");
> >  
> >  /**
> >   * intel_gvt_debugfs_add_vgpu - register debugfs entries for a vGPU
> > @@ -182,8 +182,8 @@ void intel_gvt_debugfs_add_vgpu(struct intel_vgpu *vgpu)
> >  			    &vgpu_mmio_diff_fops);
> >  	debugfs_create_file_unsafe("scan_nonprivbb", 0644, vgpu->debugfs, vgpu,
> >  				   &vgpu_scan_nonprivbb_fops);
> > -	debugfs_create_file("status", 0644, vgpu->debugfs, vgpu,
> > -			    &vgpu_status_fops);
> > +	debugfs_create_file_unsafe("status", 0644, vgpu->debugfs, vgpu,
> > +				   &vgpu_status_fops);
> >  }
> >  
> >  /**
> > -- 
> > 2.34.1
> > 
> > 
> >