On Tue, Sep 2, 2025 at 11:22 AM David Airlie <airl...@redhat.com> wrote: > > On Mon, Sep 1, 2025 at 6:02 PM Thomas Hellström > <thomas.hellst...@linux.intel.com> wrote: > > > > Hi Dave, > > > > On Mon, 2025-09-01 at 14:56 +1000, Dave Airlie wrote: > > > From: Dave Airlie <airl...@redhat.com> > > > > > > While discussing cgroups we noticed a problem where you could export > > > a BO to a dma-buf without having it ever being backed or accounted > > > for. > > > > > > This meant in low memory situations or eventually with cgroups, a > > > lower privledged process might cause the compositor to try and > > > allocate > > > a lot of memory on it's behalf and this could fail. At least make > > > sure the exporter has managed to allocate the RAM at least once > > > before exporting the object. > > > > With a shmem analogy, let's say process A creates a shmem object and > > doesn't populate it. It's then shared with process B which faults in > > the pages or otherwise causes it to be populated. Will this patch > > ensure we behave the same WRT memory usage accounting? > > > > I wandered down a few holes, but with cgroup v2, owner pays seems to > be how it works. > > They use the inode which records the cgroup owner who created it. > > cgroups v1 did it the other way, but I think we should ignore that. >
Also it does the allocations at fault time, however since we share our buffers with hw and higher privilege apps like compositors, allowing one process to ENOMEM those seems like a bad plan. This doesn't fully stop it, but at least makes sure the originating process can allocate the backing store. Dave. > > > Also some concerns below. > > > > I think we'd want to have the caller (driver) provide the > > ttm_operation_ctx. The driver may want to subclass or call > > interruptible. > > Indeed, might make more sense, though I think we should only be > calling this after creating an object in most cases, > > > > > > > + > > > + int ret; > > > + > > > + if (!bo->ttm) > > > + return 0; > > > + > > > + if (bo->ttm && ttm_tt_is_populated(bo->ttm)) > > > + return 0; > > > + > > > > bo->ttm is not safe to reference without the bo lock. Nor is the > > ttm_tt_is_populated stable without the bo lock since you may race with > > a swapout / shrink. > > Indeed, I was probably being a bit hacky here, I'll always reserve first. > > Dave. > > > > Thanks, > > Thomas > > > > > > > + ret = ttm_bo_reserve(bo, false, false, NULL); > > > + if (ret != 0) > > > + return ret; > > > + > > > + ret = ttm_bo_populate(bo, bo->resource->placement & > > > TTM_PL_FLAG_MEMCG, &ctx); > > > + ttm_bo_unreserve(bo); > > > + return ret; > > > +} > > > +EXPORT_SYMBOL(ttm_bo_setup_export); > > > diff --git a/include/drm/ttm/ttm_bo.h b/include/drm/ttm/ttm_bo.h > > > index c33b3667ae76..5cdd89da9ef5 100644 > > > --- a/include/drm/ttm/ttm_bo.h > > > +++ b/include/drm/ttm/ttm_bo.h > > > @@ -473,6 +473,7 @@ void ttm_bo_tt_destroy(struct ttm_buffer_object > > > *bo); > > > int ttm_bo_populate(struct ttm_buffer_object *bo, > > > bool memcg_account, > > > struct ttm_operation_ctx *ctx); > > > +int ttm_bo_setup_export(struct ttm_buffer_object *bo); > > > > > > /* Driver LRU walk helpers initially targeted for shrinking. */ > > > > >
