From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.1 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8C261C47082 for ; Mon, 7 Jun 2021 10:44:52 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 30DC7610A2 for ; Mon, 7 Jun 2021 10:44:52 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 30DC7610A2 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=shipmail.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id A8EE96E0F5; Mon, 7 Jun 2021 10:44:51 +0000 (UTC) Received: from ste-pvt-msa1.bahnhof.se (ste-pvt-msa1.bahnhof.se [213.80.101.70]) by gabe.freedesktop.org (Postfix) with ESMTPS id BAA596E0F5 for ; Mon, 7 Jun 2021 10:44:50 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by ste-pvt-msa1.bahnhof.se (Postfix) with ESMTP id E77503F5BA; Mon, 7 Jun 2021 12:44:48 +0200 (CEST) Authentication-Results: ste-pvt-msa1.bahnhof.se; dkim=pass (1024-bit key; unprotected) header.d=shipmail.org header.i=@shipmail.org header.b="J9WmkBMt"; dkim-atps=neutral X-Virus-Scanned: Debian amavisd-new at bahnhof.se Received: from ste-pvt-msa1.bahnhof.se ([127.0.0.1]) by localhost (ste-pvt-msa1.bahnhof.se [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id dYNdn6tKdmXC; Mon, 7 Jun 2021 12:44:47 +0200 (CEST) Received: by ste-pvt-msa1.bahnhof.se (Postfix) with ESMTPA id 5064E3F46C; Mon, 7 Jun 2021 12:44:44 +0200 (CEST) Received: from [192.168.0.209] (unknown [192.55.55.41]) by mail1.shipmail.org (Postfix) with ESMTPSA id 6E9913601A1; Mon, 7 Jun 2021 12:44:42 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=shipmail.org; s=mail; t=1623062683; bh=ZyDciiRdTAOqnBL3cyuZyCIusFBlSu1dmQv+UdvNwFo=; h=Subject:To:References:From:Date:In-Reply-To:From; b=J9WmkBMt71ViarKNMzZa/V0vgz/IGAbSQtC5ieZ1tFjjJPKD7TbH6FROiMQ53Yb7k J279D34UV9LEhjQp2dJXkZuZegLaQUEQAWas2Efvltw5q5b4VsHJYYnKwds6Rd36TP 5ZDtTKPeLpflEfevgpCmsjF+Mrh8wMh3GwBedKpA= Subject: Re: [PATCH 10/10] drm/ttm: flip the switch for driver allocated resources v2 To: =?UTF-8?Q?Christian_K=c3=b6nig?= , matthew.auld@intel.com, dri-devel@lists.freedesktop.org References: <20210602100914.46246-1-christian.koenig@amd.com> <20210602100914.46246-10-christian.koenig@amd.com> <6c2ccd57-73bb-fcc5-a22c-0e6b5be12566@gmail.com> From: =?UTF-8?Q?Thomas_Hellstr=c3=b6m_=28Intel=29?= Message-ID: <47c88617-2686-6df3-ac60-e8fd6e352963@shipmail.org> Date: Mon, 7 Jun 2021 12:44:37 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.1 MIME-Version: 1.0 In-Reply-To: <6c2ccd57-73bb-fcc5-a22c-0e6b5be12566@gmail.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On 6/7/21 12:37 PM, Christian König wrote: > Am 07.06.21 um 12:15 schrieb Thomas Hellström (Intel): >> >> On 6/2/21 12:09 PM, Christian König wrote: >>> Instead of both driver and TTM allocating memory finalize embedding the >>> ttm_resource object as base into the driver backends. >>> >>> v2: fix typo in vmwgfx grid mgr and double init in amdgpu_vram_mgr.c >>> >>> Signed-off-by: Christian König >>> Reviewed-by: Matthew Auld >>> --- >>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c   | 44 ++++++-------- >>>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c    |  2 +- >>>   .../gpu/drm/amd/amdgpu/amdgpu_res_cursor.h    |  5 +- >>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c  | 60 >>> +++++++++---------- >>>   drivers/gpu/drm/drm_gem_vram_helper.c         |  3 +- >>>   drivers/gpu/drm/nouveau/nouveau_bo.c          |  8 +-- >>>   drivers/gpu/drm/nouveau/nouveau_mem.c         | 11 ++-- >>>   drivers/gpu/drm/nouveau/nouveau_mem.h         | 14 ++--- >>>   drivers/gpu/drm/nouveau/nouveau_ttm.c         | 32 +++++----- >>>   drivers/gpu/drm/ttm/ttm_range_manager.c       | 23 +++---- >>>   drivers/gpu/drm/ttm/ttm_resource.c            | 18 +----- >>>   drivers/gpu/drm/ttm/ttm_sys_manager.c         | 12 ++-- >>>   drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c | 24 ++++---- >>>   drivers/gpu/drm/vmwgfx/vmwgfx_thp.c           | 27 ++++----- >>>   include/drm/ttm/ttm_range_manager.h           |  3 +- >>>   include/drm/ttm/ttm_resource.h                | 43 ++++++------- >>>   16 files changed, 140 insertions(+), 189 deletions(-) >> ... >>>   diff --git a/drivers/gpu/drm/ttm/ttm_range_manager.c >>> b/drivers/gpu/drm/ttm/ttm_range_manager.c >>> index ce5d07ca384c..c32e1aee2481 100644 >>> --- a/drivers/gpu/drm/ttm/ttm_range_manager.c >>> +++ b/drivers/gpu/drm/ttm/ttm_range_manager.c >>> @@ -58,7 +58,7 @@ to_range_manager(struct ttm_resource_manager *man) >>>   static int ttm_range_man_alloc(struct ttm_resource_manager *man, >>>                      struct ttm_buffer_object *bo, >>>                      const struct ttm_place *place, >>> -                   struct ttm_resource *mem) >>> +                   struct ttm_resource **res) >>>   { >>>       struct ttm_range_manager *rman = to_range_manager(man); >>>       struct ttm_range_mgr_node *node; >>> @@ -83,37 +83,30 @@ static int ttm_range_man_alloc(struct >>> ttm_resource_manager *man, >>>         spin_lock(&rman->lock); >>>       ret = drm_mm_insert_node_in_range(mm, &node->mm_nodes[0], >>> -                      mem->num_pages, bo->page_alignment, 0, >>> +                      node->base.num_pages, >>> +                      bo->page_alignment, 0, >>>                         place->fpfn, lpfn, mode); >>>       spin_unlock(&rman->lock); >>>   -    if (unlikely(ret)) { >>> +    if (unlikely(ret)) >>>           kfree(node); >>> -    } else { >>> -        mem->mm_node = &node->mm_nodes[0]; >>> -        mem->start = node->mm_nodes[0].start; >>> -    } >>> +    else >>> +        node->base.start = node->mm_nodes[0].start; >>>         return ret; >>>   } >> >> Looks like this patch forgets to assign *@res. Null pointer derefs >> when testing i915. > > I should really CC the Intel list for TTM patches as well. The CI > system should have spotted that. Unfortunately, the dg1 system is not participating in CI yet AFAICT, but moving forward I think it's a good idea. /Thomas