Received-SPF: pass (google.com: domain of linux-kernel+bounces-26299-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Mon, 15 Jan 2024 18:11:57 +0100
From: =?utf-8?Q?Micha=C5=82?= Winiarski <michal.winiarski@intel.com>
To: Maxime Ripard <mripard@kernel.org>
CC: <intel-xe@lists.freedesktop.org>, <dri-devel@lists.freedesktop.org>,
	<linux-kernel@vger.kernel.org>, Rodrigo Vivi <rodrigo.vivi@intel.com>, Michal
 Wajdeczko <michal.wajdeczko@intel.com>, Maarten Lankhorst
	<maarten.lankhorst@linux.intel.com>, Thomas Zimmermann <tzimmermann@suse.de>,
	David Airlie <airlied@gmail.com>, Daniel Vetter <daniel@ffwll.ch>, "Javier
 Martinez Canillas" <javierm@redhat.com>, =?utf-8?B?TWHDrXJh?= Canal
	<mcanal@igalia.com>
Subject: Re: Re: [PATCH v4 6/6] drm/tests: managed: Add a simple test for
 drmm_managed_release
Message-ID: <fcwozeyyg5didfpiy42kn5vscrktt6exi55hio2lba5s6btitm@n7slxall6y63>
References: <20240105101324.26811-1-michal.winiarski@intel.com>
 <20240105101324.26811-7-michal.winiarski@intel.com>
 <2mgrb5lrm5oskkcauhdbegdb5jwhqx6unzlaltz6e2bzaqrzyr@dnkbsr3ykj7k>
Content-Type: text/plain; charset="utf-8"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <2mgrb5lrm5oskkcauhdbegdb5jwhqx6unzlaltz6e2bzaqrzyr@dnkbsr3ykj7k>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?VWhMVW1vYWljT2s3cFZsTGx1bHdIOGs4QmhPZmhaQmR2VlorS2dLNUhJTzJ2?=
 =?utf-8?B?OW00V3lCZy9iSGpBMEY2YjNCRko2dnovR1h4Wm1HZDRMdXlrUFhUbnErMUd5?=
 =?utf-8?B?dXBHMHN4TGJUaEoyVktTYjdmRm1hcjIyWUtVYkNYNmlWaFRsWDNaMXlnVlpi?=
 =?utf-8?B?dDZ2Zy9FYVlCR2lXb2lHMkxvaVdic0pmVGRWdHRROExmakdFK1dXd1Nta2NW?=
 =?utf-8?B?a09HQTVKYkxUMzkvWWY5UVdmbEc2MHlhUVFkcmF2M0ZYWFZGVHkzb3J0VU1Z?=
 =?utf-8?B?Q2l1QmNObXlzTjc0RkVZcmY1eW5lSWhnL0hjalhuOWNVODFjNDZyTlBGVHVa?=
 =?utf-8?B?OHplZmJwUHRuejhUNkpJbXBBS2lsMW1MZktYTzBYbkg2aGtRWElwSzBTU1VO?=
 =?utf-8?B?b0Z5Q0k3ZEV6SWFNOXI3ZzRTam1RUU8wTjNaUVBHOWc2QWJsTzRsVUVYdVFX?=
 =?utf-8?B?ek5YOUVieVZ1Y29QQjhXaXczcE9mTmRIajNvZ2RCVDZxdmtUTVZuaEUzaWxz?=
 =?utf-8?B?ZklIdmlKaUFUTDJtbkVMRU4rdjhRbitEcHo0VTdXckpJTXBDV1JlVDhLN0dy?=
 =?utf-8?B?bGsybjN0alpCdmhuMWNVUENFZTIvVlJYbVhiL2VvVzFHTzVmTkxnUWtUeURT?=
 =?utf-8?B?Wk84eE5Sa1VzWFk1UXVlWjRTM2pvYXRnamhQSzdzaC9ZY3Z6TUV2YWdQeVpC?=
 =?utf-8?B?NmZYVGEzeVZkOTJ3eDJuNzg0VTZ3MHF3MnlZVXJaZE5JbjF5V2hhQmN2R0l2?=
 =?utf-8?B?YjZ0SlIrR1RkWWRwenR3NEVKbjd6b2JCVmpPVjc5SURtaGpReDlkTWMvNGlM?=
 =?utf-8?B?S3Vqc2MrUytJQ0FNajBzUW9HTHYrOE9YRzE1VSswelJDaXVhT1BzT1NYc3Na?=
 =?utf-8?B?aUZseXhGSDJxR3BkcUpjdnBnYmVvZER0NnJaMFZSY3h3TjF5Nkd1NWF4by85?=
 =?utf-8?B?ZzdUSGlENzlWcXBqWWUwbXJFYUV1VGhaNmxMdzB5bHd4RG9zL3QycjlQRGpC?=
 =?utf-8?B?OTJBUHRTaXN6VmprVzRhRnErN2UzNXdVNDFQM2h3RlFIQ2VSeHVIbjJad1BM?=
 =?utf-8?B?WGlhc1hLbHRxS1J2QitJQVZpcERuNnowdUJqbDZNYmVCU1hteWZNdEZnK1JS?=
 =?utf-8?B?MTJGbUloSHZzLzVGUU16T0VVUUJnalNXYW5Bc2dBMFpZMWpzdlpBR1QzcnMr?=
 =?utf-8?B?bW0wNzZIem5ZdjRqRVNPaHlEeTMyeDdzSDQwaFdpaW9jcmhHL1lwbVdsandw?=
 =?utf-8?B?L0JUYlprTm1PejFkS3V1UGxPU2NzTExnZk5SWUw2OUorckVuRW5sSnpNd2Rl?=
 =?utf-8?B?dnJqZTlmM0dFQTNzWVlxUFhzSDNKdE1IVnZJVlBoTEtpUS9TRVF5dlJab2lC?=
 =?utf-8?B?SWZqdlRzTXEzZUZINWh3NDlLdkgxdmJkTzFORzhRbzVXenN3L0x0TlFuRllM?=
 =?utf-8?B?aE1EUlBrODBweC9LRHhJUzFFSTE0U3RGRk5iNjgvVzhXc2ljUHd1QS9JYUJ0?=
 =?utf-8?B?eVBSM091MVo1WXZqaGxpZFJMS21VQUoyaWhZVis5OXR6TkpmK3o4bDJtbklF?=
 =?utf-8?B?TjJRL1JCNHBxZTR3V1IzWTFBVExoRVhPMDdVYnhiellqYTRkVk9GQldOdDlY?=
 =?utf-8?B?MlRNMXNWSWdLMXo0U240Ky9xUzZTTUZjMm1QS3k1RWg5bUxNQjZWRnhvMWc0?=
 =?utf-8?B?ZXpxcjBkTWcrV0xZTzg2RmFtMXROcVA3c2lpM1dYenhsUTU5czgxSDB3dUZH?=
 =?utf-8?B?NXNqN2xWUC8xM3IxOGh3b3psQUdIRmhPTG5UWEltK09JSVJpZ21oNW9VMTB5?=
 =?utf-8?B?bEZsbDc1RnpSa1lIRXFaYmdCY2FvdWt0c0YwTTVwbDdRRys1SURFMGRndmRY?=
 =?utf-8?B?Z2x6L25PTnpTbWJzeDkvV1RRVE9YT3dEd283dWl3dW1lc3ZPbXV0MXNDaFlN?=
 =?utf-8?B?WWQrMXZ6c0cwdWxtMUVibTZtaENYRlRTNVlxMW9aVnBLSmNDVmdva0NPdXI3?=
 =?utf-8?B?ZmMrOVRBdTAvdzlQaFlxS1VXV2VBb1kxZisybFcwbmJsSWRxYWUzL2Z0SkIz?=
 =?utf-8?B?MzdnL2gvRjNPMDFDcXU2bVo5Z1hZOWV2dW53aGpUQ25yT01EWmNZR3JaMTNT?=
 =?utf-8?B?UXM0aEd0VmN3K0VHU2tYbkdNbklrZFdHbjFTdSttL0Y3eXM0R2REbXRvLzE0?=
 =?utf-8?B?QXc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: aeee82f4-7f7e-44cb-cbee-08dc15ed16c0
X-MS-Exchange-CrossTenant-AuthSource: DM4PR11MB5373.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Jan 2024 17:12:01.5128
 (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: lQ30jyfiyG4mLQP5jpV1bvCt7pEyrwauor9xMaYNRUSgWTbIZW30nLzMT5zEsDA0Lqo7dYoLNjBBqbKmXvQfYH0V1rWGK7f0mbgJookxU30=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA2PR11MB5178
X-OriginatorOrg: intel.com

On Wed, Jan 10, 2024 at 04:56:27PM +0100, Maxime Ripard wrote:
> On Fri, Jan 05, 2024 at 11:13:24AM +0100, Michał Winiarski wrote:
> > Add a simple test that checks whether the action is indeed called right
> > away and that it is not called on the final drm_dev_put().
> > 
> > Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> > ---
> >  drivers/gpu/drm/tests/drm_managed_test.c | 28 ++++++++++++++++++++++++
> >  1 file changed, 28 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/tests/drm_managed_test.c b/drivers/gpu/drm/tests/drm_managed_test.c
> > index c1fc1f0aac9b2..91863642efc13 100644
> > --- a/drivers/gpu/drm/tests/drm_managed_test.c
> > +++ b/drivers/gpu/drm/tests/drm_managed_test.c
> > @@ -41,6 +41,33 @@ static void drm_test_managed_run_action(struct kunit *test)
> >  	KUNIT_EXPECT_TRUE_MSG(test, priv->action_done, "Release action was not called");
> >  }
> >  
> > +/*
> > + * The test verifies that the release action is called immediately when
> > + * drmm_release_action is called and that it is not called for a second time
> > + * when the device is released.
> > + */
> > +static void drm_test_managed_release_action(struct kunit *test)
> > +{
> > +	struct managed_test_priv *priv = test->priv;
> > +	int ret;
> > +
> > +	ret = drmm_add_action_or_reset(priv->drm, drm_action, priv);
> > +	KUNIT_EXPECT_EQ(test, ret, 0);
> > +
> > +	ret = drm_dev_register(priv->drm, 0);
> > +	KUNIT_ASSERT_EQ(test, ret, 0);
> > +
> > +	drmm_release_action(priv->drm, drm_action, priv);
> > +	KUNIT_EXPECT_TRUE_MSG(test, priv->action_done, "Release action was not called");
> > +	priv->action_done = false;
> > +
> > +	drm_dev_unregister(priv->drm);
> > +	drm_kunit_helper_free_device(test, priv->drm->dev);
> > +
> > +	KUNIT_EXPECT_FALSE_MSG(test, priv->action_done,
> > +			       "Unexpected release action call during cleanup");
> > +}
> > +
> 
> I guess we can have something simpler if we switch action_done to a
> counter and just check that the counter didn't increase.
> 
> And I think the custom messages should be removed there too.
> 
> Maxime

I'll drop the custom messages here and in the previous patch.

I'll also simplify this test in the way you suggested in previous
revision, to not check for release action call on cleanup.

Thanks,
-Michał