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 2C573C27C4F for ; Sun, 30 Jun 2024 22:59:18 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id CDA8E10E080; Sun, 30 Jun 2024 22:59:17 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=intel.com header.i=@intel.com header.b="nZl3nho0"; dkim-atps=neutral Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.16]) by gabe.freedesktop.org (Postfix) with ESMTPS id BDD2C10E080 for ; Sun, 30 Jun 2024 22:59:16 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1719788357; x=1751324357; h=date:message-id:from:to:cc:subject:in-reply-to: references:mime-version; bh=1+EtyYqUjG/Co2jdRWU/9NmpQYWzzHsla24Lodxs1wY=; b=nZl3nho0+wDa0xSnJR6in2CEdBE8jAwjNk0fwNjMkTmu+Ko80O2IGq9g WX5yaOZs3xAQffGrDiJJ/Qao8r3fwT2M5iD5Df1qa3sAczr26BiRbCDD6 /hcl4VutcJTYc/eMWx6jZzu1J5m1hjE0boLa0PZV6nDxEsIF2R3bkZgPZ TzCEfWDp1bXJUaXs5VGrMlz2NNBW5M4sJbBkBwzdNcinX+KOFPKxkj+A3 vd679TxxVfDXr5guUfPPzb0PJJ17xrNBbmzYS9bwjUyJEF3wyr4Z/h+1K A/qDo5b/VfWzn4iiFLAgUy9GB2xRan1uV0GWkpqfO5kzBDkfqOlzvq7r7 w==; X-CSE-ConnectionGUID: /diUh7q0TDKRyPFbRZnBJg== X-CSE-MsgGUID: h7sg7sUMT0mZ1JGAU64ggA== X-IronPort-AV: E=McAfee;i="6700,10204,11119"; a="17026589" X-IronPort-AV: E=Sophos;i="6.09,175,1716274800"; d="scan'208";a="17026589" Received: from orviesa006.jf.intel.com ([10.64.159.146]) by orvoesa108.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 30 Jun 2024 15:59:17 -0700 X-CSE-ConnectionGUID: Yp1DJyyTQcaqXOXSg1fzbg== X-CSE-MsgGUID: mxhLAoLrTBee1Cz/04Cv7Q== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.09,175,1716274800"; d="scan'208";a="45733635" Received: from lmeganx-mobl1.amr.corp.intel.com (HELO adixit-arch.intel.com) ([10.125.32.7]) by orviesa006-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 30 Jun 2024 15:59:16 -0700 Date: Sun, 30 Jun 2024 15:42:48 -0700 Message-ID: <877ce6aw7r.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" To: Umesh Nerlige Ramappa Cc: igt-dev@lists.freedesktop.org Subject: Re: [PATCH i-g-t 10/28] tests/intel/xe_oa: Add first tests In-Reply-To: References: <20240620200054.3550653-1-ashutosh.dixit@intel.com> <20240620200054.3550653-11-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/29.3 (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 X-BeenThere: igt-dev@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Development mailing list for IGT GPU Tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" On Thu, 20 Jun 2024 16:11:21 -0700, Umesh Nerlige Ramappa wrote: > Hi Umesh, > > +/** > > + * SUBTEST: xe-ref-count > > + * Description: Check that an open oa stream holds a reference on the xe module > > + */ > > +static void > > +test_xe_ref_count(void) > > +{ > > + uint64_t properties[] = { > > + DRM_XE_OA_PROPERTY_OA_UNIT_ID, 0, > > + > > + /* Include OA reports in samples */ > > + DRM_XE_OA_PROPERTY_SAMPLE_OA, true, > > + > > + /* OA unit configuration */ > > + DRM_XE_OA_PROPERTY_OA_METRIC_SET, 0 /* updated below */, > > + DRM_XE_OA_PROPERTY_OA_FORMAT, __ff(0), /* update below */ > > + DRM_XE_OA_PROPERTY_OA_PERIOD_EXPONENT, 0, /* update below */ > > + }; > > + struct intel_xe_oa_open_prop param = { > > + .num_properties = ARRAY_SIZE(properties) / 2, > > + .properties_ptr = to_user_pointer(properties), > > + }; > > + unsigned baseline, ref_count0, ref_count1; > > + uint32_t oa_report0[64]; > > + uint32_t oa_report1[64]; > > + > > + /* This should be the first test before the first fixture so no drm_fd > > + * should have been opened so far... > > + */ > > + igt_assert_eq(drm_fd, -1); > > + > > + baseline = read_xe_module_ref(); > > + igt_debug("baseline ref count (drm fd closed) = %u\n", baseline); > > + > > + drm_fd = __drm_open_driver(DRIVER_XE); > > + if (is_xe_device(drm_fd)) > > + xe_device_get(drm_fd); > > + devid = intel_get_drm_devid(drm_fd); > > + sysfs = igt_sysfs_open(drm_fd); > > + > > + /* Note: these global variables are only initialized after calling > > + * init_sys_info()... > > + */ > > + igt_require(init_sys_info()); > > + properties[5] = default_test_set->perf_oa_metrics_set; > > + properties[7] = __ff(default_test_set->perf_oa_format); > > + properties[9] = oa_exp_1_millisec; > > + > > + ref_count0 = read_xe_module_ref(); > > + igt_debug("initial ref count with drm_fd open = %u\n", ref_count0); > > + > > + stream_fd = __perf_open(drm_fd, ¶m, false); > > + set_fd_flags(stream_fd, O_CLOEXEC); > > + ref_count1 = read_xe_module_ref(); > > + igt_debug("ref count after opening oa stream = %u\n", ref_count1); > > + > > + drm_close_driver(drm_fd); > > + close(sysfs); > > + drm_fd = -1; > > + sysfs = -1; > > + ref_count0 = read_xe_module_ref(); > > + igt_debug("ref count after closing drm fd = %u\n", ref_count0); > > + > > + read_2_oa_reports(default_test_set->perf_oa_format, > > + oa_exp_1_millisec, > > + oa_report0, > > + oa_report1, > > + false); /* not just timer reports */ > > + > > + __perf_close(stream_fd); > > + ref_count0 = read_xe_module_ref(); > > + igt_debug("ref count after closing oa stream fd = %u\n", ref_count0); > > Looks like the asserts are removed, so we will never know if this failed > :). I think we should drop the test and create an issue to make it more > robust. Unfortunately I don't want to drop the test in this patch, since it will mean moving several other functions called by this code to other patches etc. (to make sure this patch continues to compile). It is a non trivial amount of work, not worth it IMO. So instead, I have added a patch at the end deleting the "xe-ref-count" subtest. I have also created an issue to re-add the test back after making it robust. I have already merged the IGT library and tools. So it is just the Xe OA IGT test patches left to merge. So now only the first and the second-last patches don't have a R-b. You can see these here: https://patchwork.freedesktop.org/series/130033/#rev5 Thanks. -- Ashutosh > > Ideally we should take a snapshot of the entire /proc/modules file and > ensure that no new driver has added a reference to xe while we are running > this test. That way, we know that an assert may fail due to a new driver > taking up a drm ref or dropping one. If we can attribute the failure to > such an event, we can re-run the test.