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 A5EBCC3DA7F for ; Thu, 15 Aug 2024 21:16:07 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 68E8910E0E8; Thu, 15 Aug 2024 21:16:07 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="YH7ZFFTk"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.17]) by gabe.freedesktop.org (Postfix) with ESMTPS id 2A12610E0E8 for ; Thu, 15 Aug 2024 21:16:06 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1723756566; x=1755292566; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version:content-transfer-encoding; bh=xffuDmV5zTqYwQlj6qKgx90CL+zcOAtClUnU5RdwOTc=; b=YH7ZFFTk8tBHf7bbS9MGRqiuQZAd2MV6lwPkHF5RfQdDIbnBs9PDEG8g zg15wbZBQBKwsR+7c4UKxsGaOrdZ0W8mB2IeVua2or6+FLMG5kEhPDdtY VGokveZ3esFlZql9n5ksz4rsJHmv/ogn43mqbqeCHf+BzBrvKExDCj35r j9psC25Q74bZ9ShVDUJKMCBQLb+eUQyLV61Ess8Xt/3OvfI0dOQ0/8mRZ L2XeSn/iY2B19Co0PbcQmQCzmZi+YKe/XZl+V9Kihhs2gnEVFCaQTE0Xi sH1d+/HIBk5Lt/ruuADHvN4jfDkDUkk/mmvyB26VeUJYZCbAwjlTvjuyv A==; X-CSE-ConnectionGUID: 2N3nC93aSBOTGYlIyKhjPw== X-CSE-MsgGUID: 3z1t1SE5TIekNpSSuinNHg== X-IronPort-AV: E=McAfee;i="6700,10204,11165"; a="21914824" X-IronPort-AV: E=Sophos;i="6.10,150,1719903600"; d="scan'208";a="21914824" Received: from fmviesa001.fm.intel.com ([10.60.135.141]) by fmvoesa111.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 15 Aug 2024 14:16:05 -0700 X-CSE-ConnectionGUID: Ndr54CDoRBCmZkGoCNlR5w== X-CSE-MsgGUID: AjkCwAOWQoqiX90I0+Y3HA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.10,150,1719903600"; d="scan'208";a="90255174" Received: from ticede-or-134.amr.corp.intel.com (HELO adixit-arch.intel.com) ([10.124.51.80]) by smtpauth.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 15 Aug 2024 14:16:05 -0700 Date: Thu, 15 Aug 2024 14:11:50 -0700 Message-ID: <874j7lebjd.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: =?ISO-8859-1?Q?Jos=E9?= Roberto de Souza Cc: intel-xe@lists.freedesktop.org Subject: Re: [PATCH 1/3] drm/xe/oa: Replace per GT oa mutex per a global one In-Reply-To: References: <20240815162758.36495-1-jose.souza@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/29.4 (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=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-BeenThere: intel-xe@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel Xe graphics driver List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-xe-bounces@lists.freedesktop.org Sender: "Intel-xe" On Thu, 15 Aug 2024 09:42:57 -0700, Matthew Brost wrote: > > On Thu, Aug 15, 2024 at 09:27:56AM -0700, Jos=E9 Roberto de Souza wrote: > > > This mutex is not frequently used so there is no reason to have one > > per GT, True, but still why bother? The mutex serializes concurrent stream create and destroy's so it cannot be stream level. So it is next higher level, which is gt. All OA units are organized per gt, so is the lock. > > also this reduce complexity. How? > > > > Also it was missed the code to destroy the mutex. > > > > drmm_mutex_init provides the destroy. Correct. > > Will stay out of this as Ashutosh owns this code but in general less > locks is better per guidelines from Sima. To me it is basically meh, can be per device or per gt. Don't seem much reason to change it. Thanks. -- Ashutosh > > Cc: Ashutosh Dixit > > Signed-off-by: Jos=E9 Roberto de Souza > > --- > > drivers/gpu/drm/xe/xe_oa.c | 13 +++++++------ > > drivers/gpu/drm/xe/xe_oa_types.h | 5 ++--- > > 2 files changed, 9 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/gpu/drm/xe/xe_oa.c b/drivers/gpu/drm/xe/xe_oa.c > > index 3ef92eb8fbb1e..d6eec8caf4c51 100644 > > --- a/drivers/gpu/drm/xe/xe_oa.c > > +++ b/drivers/gpu/drm/xe/xe_oa.c > > @@ -1205,9 +1205,9 @@ static int xe_oa_release(struct inode *inode, str= uct file *file) > > struct xe_oa_stream *stream =3D file->private_data; > > struct xe_gt *gt =3D stream->gt; > > > > - mutex_lock(>->oa.gt_lock); > > + mutex_lock(>->tile->xe->oa.streams_lock); > > xe_oa_destroy_locked(stream); > > - mutex_unlock(>->oa.gt_lock); > > + mutex_unlock(>->tile->xe->oa.streams_lock); > > > > /* Release the reference the OA stream kept on the driver */ > > drm_dev_put(>_to_xe(gt)->drm); > > @@ -1875,9 +1875,9 @@ int xe_oa_stream_open_ioctl(struct drm_device *de= v, u64 data, struct drm_file *f > > drm_dbg(&oa->xe->drm, "Using periodic sampling freq %lld Hz\n", oa_fre= q_hz); > > } > > > > - mutex_lock(¶m.hwe->gt->oa.gt_lock); > > + mutex_lock(&xe->oa.streams_lock); > > ret =3D xe_oa_stream_open_ioctl_locked(oa, ¶m); > > - mutex_unlock(¶m.hwe->gt->oa.gt_lock); > > + mutex_unlock(&xe->oa.streams_lock); > > err_exec_q: > > if (ret < 0 && param.exec_q) > > xe_exec_queue_put(param.exec_q); > > @@ -2388,8 +2388,6 @@ static int xe_oa_init_gt(struct xe_gt *gt) > > > > __xe_oa_init_oa_units(gt); > > > > - drmm_mutex_init(>_to_xe(gt)->drm, >->oa.gt_lock); > > - > > return 0; > > } > > > > @@ -2472,6 +2470,8 @@ int xe_oa_init(struct xe_device *xe) > > oa->xe =3D xe; > > oa->oa_formats =3D oa_formats; > > > > + mutex_init(&oa->streams_lock); > > + > > drmm_mutex_init(&oa->xe->drm, &oa->metrics_lock); > > idr_init_base(&oa->metrics_idr, 1); > > > > @@ -2508,5 +2508,6 @@ void xe_oa_fini(struct xe_device *xe) > > idr_for_each(&oa->metrics_idr, destroy_config, oa); > > idr_destroy(&oa->metrics_idr); > > > > + mutex_destroy(&oa->streams_lock); > > oa->xe =3D NULL; > > } > > diff --git a/drivers/gpu/drm/xe/xe_oa_types.h b/drivers/gpu/drm/xe/xe_o= a_types.h > > index 540c3ec53a6d7..17e17b5b93640 100644 > > --- a/drivers/gpu/drm/xe/xe_oa_types.h > > +++ b/drivers/gpu/drm/xe/xe_oa_types.h > > @@ -112,9 +112,6 @@ struct xe_oa_unit { > > * struct xe_oa_gt - OA per-gt information > > */ > > struct xe_oa_gt { > > - /** @gt_lock: lock protecting create/destroy OA streams */ > > - struct mutex gt_lock; > > - > > /** @num_oa_units: number of oa units for each gt */ > > u32 num_oa_units; > > > > @@ -149,6 +146,8 @@ struct xe_oa { > > > > /** @oa_unit_ids: tracks oa unit ids assigned across gt's */ > > u16 oa_unit_ids; > > + > > + struct mutex streams_lock; > > }; > > > > /** @xe_oa_buffer: State of the stream OA buffer */ > > -- > > 2.46.0 > >