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 gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (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 0C94BC433EF for ; Wed, 20 Apr 2022 16:30:21 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 5DBCA10E222; Wed, 20 Apr 2022 16:30:21 +0000 (UTC) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by gabe.freedesktop.org (Postfix) with ESMTPS id 48CC010E222 for ; Wed, 20 Apr 2022 16:30:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1650472220; x=1682008220; h=date:message-id:from:to:subject:in-reply-to:references: mime-version; bh=OfeM/9xlISXwsIw8tU3+f6CQukYzyxZHRsFlNF5lO0w=; b=nIaunVZRVP3pcQL2yefb3HzqcH2xk65ZRPcdm8JXCdo6aP17zh4a0i+8 UbnCsKpcsImcRh/9BDrciElQ4Gpdhllassa6JR71J9fpmkZtM46iK1PUd SxPZ5MM3ExFsVN+OMI9ZiKuJhnz8YaJXgfEo4PEH1wqJ7Emx+J0vYNOK6 Alv+E9quU9g5URAXCOANQRWJx2TO/dhD6X2h6AeoTg3RSgQabsGzlwYHq +6n/WNf95mJkd15FKR/KOXVj/gFWmoc8eb74ZHfqvp+e6u0bP0yhdVpbA MGDFKc2c2e2F5djO79d/MLhHtxBEQCWNB7oo4RtmkAKPQfwGgpyjjud7U A==; X-IronPort-AV: E=McAfee;i="6400,9594,10323"; a="350532252" X-IronPort-AV: E=Sophos;i="5.90,276,1643702400"; d="scan'208";a="350532252" Received: from orsmga007.jf.intel.com ([10.7.209.58]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Apr 2022 09:23:43 -0700 X-IronPort-AV: E=Sophos;i="5.90,276,1643702400"; d="scan'208";a="555278755" Received: from adixit-mobl1.amr.corp.intel.com (HELO adixit-arch.intel.com) ([10.212.215.49]) by orsmga007-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Apr 2022 09:23:43 -0700 Date: Wed, 20 Apr 2022 09:23:42 -0700 Message-ID: <87k0bjafld.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: intel-gfx@lists.freedesktop.org In-Reply-To: <9ed5af1177ad08c7c2d9c5d9b32ab0154dbd950f.1650435571.git.ashutosh.dixit@intel.com> References: <9ed5af1177ad08c7c2d9c5d9b32ab0154dbd950f.1650435571.git.ashutosh.dixit@intel.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?ISO-8859-4?Q?Goj=F2?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.2 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII Subject: Re: [Intel-gfx] [PATCH 7/9] drm/i915/gt: Fix memory leaks in per-gt sysfs X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On Wed, 20 Apr 2022 05:17:57 -0700, Andrzej Hajda wrote: > > Hi Ashutosh, Hi Andrzej, > On 20.04.2022 07:21, Ashutosh Dixit wrote: > > All kmalloc'd kobjects need a kobject_put() to free memory. For example in > > previous code, kobj_gt_release() never gets called. The requirement of > > kobject_put() now results in a slightly different code organization. > > > > Cc: Andi Shyti > > Cc: Andrzej Hajda > > Cc: Rodrigo Vivi > > Fixes: b770bcfae9ad ("drm/i915/gt: create per-tile sysfs interface") /snip/ > > +void intel_gt_sysfs_unregister(struct intel_gt *gt) > > +{ > > + kobject_put(>->sysfs_gtn); > > +} > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_sysfs.h b/drivers/gpu/drm/i915/gt/intel_gt_sysfs.h > > index 9471b26752cf..a99aa7e8b01a 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_gt_sysfs.h > > +++ b/drivers/gpu/drm/i915/gt/intel_gt_sysfs.h > > @@ -13,11 +13,6 @@ > > struct intel_gt; > > -struct kobj_gt { > > - struct kobject base; > > - struct intel_gt *gt; > > -}; > > - > > bool is_object_gt(struct kobject *kobj); > > struct drm_i915_private *kobj_to_i915(struct kobject *kobj); > > @@ -28,6 +23,7 @@ intel_gt_create_kobj(struct intel_gt *gt, > > const char *name); > > void intel_gt_sysfs_register(struct intel_gt *gt); > > +void intel_gt_sysfs_unregister(struct intel_gt *gt); > > struct intel_gt *intel_gt_sysfs_get_drvdata(struct device *dev, > > const char *name); > > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h > > b/drivers/gpu/drm/i915/gt/intel_gt_types.h > > index 937b2e1a305e..4c72b4f983a6 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h > > +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h > > @@ -222,6 +222,9 @@ struct intel_gt { > > } mocs; > > struct intel_pxp pxp; > > + > > + /* gt/gtN sysfs */ > > + struct kobject sysfs_gtn; > > If you put kobject as a part of intel_gt what assures you that lifetime of > kobject is shorter than intel_gt? Ie its refcounter is 0 on removal of > intel_gt? Because we are explicitly doing a kobject_put() in intel_gt_sysfs_unregister(). Which is exactly what we are *not* doing in the previous code. Let me explain a bit about the previous code (but feel free to skip since the patch should speak for itself): * Previously we kzalloc a 'struct kobj_gt' * But we don't save a pointer to the 'struct kobj_gt' so we don't have the pointer to the kobject to be able to do a kobject_put() on it later * Therefore we need to store the pointer in 'struct intel_gt' * But if we have to put the pointer in 'struct intel_gt' we might as well put the kobject as part of 'struct intel_gt' and that also removes the need to have a 'struct kobj_gt' (kobj_to_gt() can just use container_of() to get gt from kobj). * So I think this patch simpler/cleaner than the original code if you take the requirement for kobject_put() into account. Thanks. -- Ashutosh