From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 8B8981C07CF for ; Fri, 20 Dec 2024 11:08:49 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.140.110.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1734692931; cv=none; b=Lw50p6BtFE3pQtfonnNRdOwH4x7rRHU+mPkx5k6zLIuPbJTSww3dMfK4gbXooA5la14i8QEFTnmtrOBz7YL6QekkYYiNSfL3DLEGqlK0UQKfZ6ZPos9vCmPUVnsfgeuUw4y10y41fvse5UBEu1lGO9czddR/ha1Lu+S91Nrmhgs= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1734692931; c=relaxed/simple; bh=jerpklV12wZ1ubci0yQdTMtY4xsbbQVemF92Kq6Q5Ow=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=l2RQ2auw+hKDDbTnPtbceyWbVJTQL0qvPCjK4BFu1Zt187JiIyCiIohsf4xyUJQfcf4czxHnxNp+kTfY8F4iieSNP+Xjtz0uqAMw2hTYN1Yw1UUVaS06jqdo838/MPHfjUEXuNdwDy12ZNO6pw4BsZK86xfjpi+k0peXDya/Bho= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com; spf=pass smtp.mailfrom=arm.com; arc=none smtp.client-ip=217.140.110.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id CA0461480; Fri, 20 Dec 2024 03:09:16 -0800 (PST) Received: from [10.1.31.19] (e122027.cambridge.arm.com [10.1.31.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 13C863F58B; Fri, 20 Dec 2024 03:08:45 -0800 (PST) Message-ID: Date: Fri, 20 Dec 2024 11:08:44 +0000 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v5 1/2] drm/panthor: Expose size of driver internal BO's over fdinfo To: Mihail Atanassov , =?UTF-8?Q?Adri=C3=A1n_Mart=C3=ADnez_Larumbe?= , Boris Brezillon , Liviu Dudau , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Simona Vetter Cc: nd@arm.com, kernel@collabora.com, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org References: <20241218181844.886043-1-adrian.larumbe@collabora.com> <20241218181844.886043-2-adrian.larumbe@collabora.com> <2a7c5a0b-af3f-4f1c-8c77-ab6233afcc76@arm.com> From: Steven Price Content-Language: en-GB In-Reply-To: <2a7c5a0b-af3f-4f1c-8c77-ab6233afcc76@arm.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On 19/12/2024 16:30, Mihail Atanassov wrote: > > > On 18/12/2024 18:18, Adrián Martínez Larumbe wrote: >> From: Adrián Larumbe >> >> This will display the sizes of kenrel BO's bound to an open file, >> which are >> otherwise not exposed to UM through a handle. >> >> The sizes recorded are as follows: >>   - Per group: suspend buffer, protm-suspend buffer, syncobjcs >>   - Per queue: ringbuffer, profiling slots, firmware interface >>   - For all heaps in all heap pools across all VM's bound to an open >> file, >>   record size of all heap chuks, and for each pool the gpu_context BO >> too. >> >> This does not record the size of FW regions, as these aren't bound to a >> specific open file and remain active through the whole life of the >> driver. >> >> Signed-off-by: Adrián Larumbe >> Reviewed-by: Liviu Dudau >> --- [...] >> diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/ >> panthor/panthor_mmu.c >> index c39e3eb1c15d..51f6e66df3f5 100644 >> --- a/drivers/gpu/drm/panthor/panthor_mmu.c >> +++ b/drivers/gpu/drm/panthor/panthor_mmu.c >> @@ -1941,6 +1941,41 @@ struct panthor_heap_pool >> *panthor_vm_get_heap_pool(struct panthor_vm *vm, bool c >>       return pool; >>   } >>   +/** >> + * panthor_vm_heaps_size() - Calculate size of all heap chunks across >> all >> + * heaps over all the heap pools in a VM >> + * @pfile: File. >> + * @status: Memory status to be updated. >> + * >> + * Calculate all heap chunk sizes in all heap pools bound to a VM. If >> the VM >> + * is active, record the size as active as well. >> + */ >> +void panthor_vm_heaps_sizes(struct panthor_file *pfile, struct >> drm_memory_stats *status) >> +{ >> +    struct panthor_vm *vm; >> +    unsigned long i; >> + >> +    if (!pfile->vms) >> +        return; >> + >> +    xa_for_each(&pfile->vms->xa, i, vm) { >> +        size_t size; >> + >> +        mutex_lock(&vm->heaps.lock); > > Use `scoped_guard` instead? > > #include > > /* ... */ > >     xa_for_each(...) { >         size_t size; > >         scoped_guard(mutex, &vm->heaps.lock) { >             if (!vm->heaps.pool) >                 continue; > >             size = panthor_heap_pool_size(vm->heaps.pool); >         } >         /* ... */ I don't believe this actually works. The implementation of scoped_guard uses a for() loop. So the "continue" will be applied to this (hidden) internal loop rather than the xa_for_each() loop intended. An alternative would be: xa_for_each(&pfile->vms->xa, i, vm) { size_t size = 0; mutex_lock(&vm->heaps.lock); if (vm->heaps.pool) size = panthor_heap_pool_size(vm->heaps.pool); mutex_unlock(&vm->heaps.lock); status->resident += size; status->private += size; if (vm->as.id >= 0) status->active += size; } (relying on size=0 being a no-op for the additions). Although I was personally also happy with the original - but perhaps that's just because I'm old and still feel anxious when I see scoped_guard() ;) Steve