Hey Rob,
On Thu, 6 Apr 2023 at 22:59, Rob Clark <robdclark@xxxxxxxxx> wrote:
> +- drm-purgeable-memory: <uint> [KiB|MiB]
> +
> +The total size of buffers that are purgable.
s/purgable/purgeable/
> +static void print_size(struct drm_printer *p, const char *stat, size_t sz)
> +{
> + const char *units[] = {"B", "KiB", "MiB", "GiB"};
The documentation says:
> Default unit shall be bytes with optional unit specifiers of 'KiB' or 'MiB'
> indicating kibi- or mebi-bytes.
So I would drop the B and/or update the documentation to mention B && GiB.
> + unsigned u;
> +
> + for (u = 0; u < ARRAY_SIZE(units) - 1; u++) {
> + if (sz < SZ_1K)
> + break;
> + sz /= SZ_1K;
IIRC size_t can be 64bit, so we should probably use do_div() here.
> + }
> +
> + drm_printf(p, "%s:\t%lu %s\n", stat, sz, units[u]);
> +}
> +
> +/**
> + * drm_print_memory_stats - Helper to print standard fdinfo memory stats
> + * @file: the DRM file
> + * @p: the printer to print output to
> + * @status: callback to get driver tracked object status
> + *
> + * Helper to iterate over GEM objects with a handle allocated in the specified
> + * file. The optional status callback can return additional object state which
s/return additional/return an additional/
> + * determines which stats the object is counted against. The callback is called
> + * under table_lock. Racing against object status change is "harmless", and the
> + * callback can expect to not race against object destroy.
s/destroy/destruction/
> + */
> +void drm_print_memory_stats(struct drm_file *file, struct drm_printer *p,
> + enum drm_gem_object_status (*status)(struct drm_gem_object *))
> +{
> + if (s & DRM_GEM_OBJECT_RESIDENT) {
> + size.resident += obj->size;
> + s &= ~DRM_GEM_OBJECT_PURGEABLE;
Is MSM capable of marking the object as both purgeable and resident or
is this to catch other drivers? Should we add a note to the
documentation above - resident memory cannot be purgeable
> + }
> +
> + if (s & DRM_GEM_OBJECT_ACTIVE) {
> + size.active += obj->size;
> + s &= ~DRM_GEM_OBJECT_PURGEABLE;
Ditto.
With the above nits, the patch is:
Reviewed-by: Emil Velikov <emil.l.velikov@xxxxxxxxx>
HTH
Emil
