On Wed, 2024-02-21 at 11:26 +0100, Christian König wrote:
> Am 21.02.24 um 08:33 schrieb Thomas Hellström:
> > If caching mode change fails due to, for example, OOM we
> > free the allocated pages in a two-step process. First the pages
> > for which the caching change has already succeeded. Secondly
> > the pages for which a caching change did not succeed.
> >
> > However the second step was incorrectly freeing the pages already
> > freed in the first step.
> >
> > Fix.
> >
> > Signed-off-by: Thomas Hellström <thomas.hellstrom@xxxxxxxxxxxxxxx>
> > Fixes: 379989e7cbdc ("drm/ttm/pool: Fix ttm_pool_alloc error path")
> > Cc: Christian König <christian.koenig@xxxxxxx>
> > Cc: Dave Airlie <airlied@xxxxxxxxxx>
> > Cc: Christian Koenig <christian.koenig@xxxxxxx>
> > Cc: Huang Rui <ray.huang@xxxxxxx>
> > Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx
> > Cc: <stable@xxxxxxxxxxxxxxx> # v6.4+
>
> You don't know how much time I've spend staring at this line to find
> the
> bug in it and haven't seen it. Got bug reports about that for month
> as well.
Yeah, sorry about that. We should probably have Kunit tests exercising
OOM in the pool code involving WC pages.
I'll push this to drm-misc-next.
/Thomas
>
> Reviewed-by: Christian König <christian.koenig@xxxxxxx>
>
> > ---
> > drivers/gpu/drm/ttm/ttm_pool.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/ttm/ttm_pool.c
> > b/drivers/gpu/drm/ttm/ttm_pool.c
> > index b62f420a9f96..112438d965ff 100644
> > --- a/drivers/gpu/drm/ttm/ttm_pool.c
> > +++ b/drivers/gpu/drm/ttm/ttm_pool.c
> > @@ -387,7 +387,7 @@ static void ttm_pool_free_range(struct ttm_pool
> > *pool, struct ttm_tt *tt,
> > enum ttm_caching caching,
> > pgoff_t start_page, pgoff_t
> > end_page)
> > {
> > - struct page **pages = tt->pages;
> > + struct page **pages = &tt->pages[start_page];
> > unsigned int order;
> > pgoff_t i, nr;
> >
>
