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 EA51D12FF94 for ; Thu, 25 Jan 2024 16:47:20 +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=1706201242; cv=none; b=lKJ4QRIvHqeF325CoE1Qaa35M7y31tjRalF3/dL7u7tST4Ptc8nNR3pS7k3ll++093dnTXz81pWxH2YgBxZ+lWzq58anShDGNZ/uVlpJY7EJGcUBfHsP/LQF1Wca37oBB4iPaafFb3DU4XoAx4v+Nn66ld72b2oAvh/sFDuF+34= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1706201242; c=relaxed/simple; bh=/FHwRI4r7JgDwNCxrf6OQAN4XCPE9f4qryDKOoKaftc=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=gDoCnEGTCQDQj4XNg5SUSuqpkQfXG2EBrlqA3shmsd8qGHiWo/yfUfVixbFb2CaduM64QoSajSJTsM3HJpUmmL16Fdd7nN1EsLqBo+S45hjH8W82UPlKR2dzYQzzx3xYmsWbD+gMAsK+AtlCEcYuchaYT3tJOUgcvpkvvVTlYvQ= 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 1E5B71763; Thu, 25 Jan 2024 08:48:05 -0800 (PST) Received: from [10.1.27.41] (e122027.cambridge.arm.com [10.1.27.41]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 454BA3F5A1; Thu, 25 Jan 2024 08:47:17 -0800 (PST) Message-ID: <08fd5c89-9fec-4fc8-b0a3-03e4ab9447d7@arm.com> Date: Thu, 25 Jan 2024 16:47:15 +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 v19 17/30] drm/panfrost: Fix the error path in panfrost_mmu_map_fault_addr() To: Dmitry Osipenko , David Airlie , Gerd Hoffmann , Gurchetan Singh , Chia-I Wu , Daniel Vetter , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , =?UTF-8?Q?Christian_K=C3=B6nig?= , Qiang Yu , Boris Brezillon , Emma Anholt , Melissa Wen Cc: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, kernel@collabora.com, virtualization@lists.linux-foundation.org References: <20240105184624.508603-1-dmitry.osipenko@collabora.com> <20240105184624.508603-18-dmitry.osipenko@collabora.com> Content-Language: en-GB From: Steven Price In-Reply-To: <20240105184624.508603-18-dmitry.osipenko@collabora.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On 05/01/2024 18:46, Dmitry Osipenko wrote: > From: Boris Brezillon > > If some the pages or sgt allocation failed, we shouldn't release the > pages ref we got earlier, otherwise we will end up with unbalanced > get/put_pages() calls. We should instead leave everything in place > and let the BO release function deal with extra cleanup when the object > is destroyed, or let the fault handler try again next time it's called. > > Fixes: 187d2929206e ("drm/panfrost: Add support for GPU heap allocations") > Cc: > Signed-off-by: Boris Brezillon > Co-developed-by: Dmitry Osipenko > Signed-off-by: Dmitry Osipenko Reviewed-by: Steven Price > --- > drivers/gpu/drm/panfrost/panfrost_mmu.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c > index bd5a0073009d..4a0b4bf03f1a 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c > +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c > @@ -502,11 +502,18 @@ static int panfrost_mmu_map_fault_addr(struct panfrost_device *pfdev, int as, > mapping_set_unevictable(mapping); > > for (i = page_offset; i < page_offset + NUM_FAULT_PAGES; i++) { > + /* Can happen if the last fault only partially filled this > + * section of the pages array before failing. In that case > + * we skip already filled pages. > + */ > + if (pages[i]) > + continue; > + > pages[i] = shmem_read_mapping_page(mapping, i); > if (IS_ERR(pages[i])) { > ret = PTR_ERR(pages[i]); > pages[i] = NULL; > - goto err_pages; > + goto err_unlock; > } > } > > @@ -514,7 +521,7 @@ static int panfrost_mmu_map_fault_addr(struct panfrost_device *pfdev, int as, > ret = sg_alloc_table_from_pages(sgt, pages + page_offset, > NUM_FAULT_PAGES, 0, SZ_2M, GFP_KERNEL); > if (ret) > - goto err_pages; > + goto err_unlock; > > ret = dma_map_sgtable(pfdev->dev, sgt, DMA_BIDIRECTIONAL, 0); > if (ret) > @@ -537,8 +544,6 @@ static int panfrost_mmu_map_fault_addr(struct panfrost_device *pfdev, int as, > > err_map: > sg_free_table(sgt); > -err_pages: > - drm_gem_shmem_put_pages_locked(&bo->base); > err_unlock: > dma_resv_unlock(obj->resv); > err_bo: