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=-9.6 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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 36CE5C2D0A8 for ; Tue, 29 Sep 2020 02:04:56 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id DAF4021531 for ; Tue, 29 Sep 2020 02:04:55 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="ElzTpIgl" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727301AbgI2CEw (ORCPT ); Mon, 28 Sep 2020 22:04:52 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34578 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726961AbgI2CEw (ORCPT ); Mon, 28 Sep 2020 22:04:52 -0400 Received: from mail-vs1-xe44.google.com (mail-vs1-xe44.google.com [IPv6:2607:f8b0:4864:20::e44]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 530EBC061755; Mon, 28 Sep 2020 19:04:52 -0700 (PDT) Received: by mail-vs1-xe44.google.com with SMTP id y190so2026978vsy.1; Mon, 28 Sep 2020 19:04:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=iDhv+ozgLfJpz4ReM/64bivezuXaXXhI3uaVkJqD6ls=; b=ElzTpIglC+vmQlt1o4Loci+G0xfVId1TvwEqqaq9K2WDKtIa9s/LqIro0Fr2c73mVn kmhkY9wYhFBx8SLb+/vdoOGgOLpEv8t6yt8jtquj71p3kzrlKl+Wl+REsmWjuuZ1XBUD TMLpUWCLSaojzcxUGI4TIWNmUb0MH8Krs0tUvv8CAjBtoak+oQQ21aC5RfyLBIpJ3C55 mPtRWvnb9PjSzLLQn0xBj8juntZIieZH7daRO0vmOv70RxDXTKXUMWsfo9rdlpUglp73 VjmwWGFYZJGsHB64AqjSAdayBHKQkf3SzoRzMHpuVWbwG2L9vT/AxdfHrO+o6iiVip3B LPYg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=iDhv+ozgLfJpz4ReM/64bivezuXaXXhI3uaVkJqD6ls=; b=UB7yyfgfPhLVD37HQWH1Eb8oyxG68J5gys1zBcgFBsIFXi77MaZc7TKsjTIEj8x6H6 iJUiKv2kQnSwn3kCq4pD8w60/eUQJ/7HS7ifMRPcZisUxg6bflYigB+MOg3GC9GXXIDg xiH3fUaoxYCOKUMtJicLORoSxBR9FqHFWSvXiUzZ4OdISXCJyuDSemRKPjzGD3SndLx9 Vdgihyrsc4TNtISe+VO9GlF3RqP9TzD6GXwxy5haCa67R38yP51wuW6wYq2BNof9bMYL LWrg7iN76uAq4G1BJ8rxsDiuiS0FJrtsAljGxswFJ9keUa7bU67T49dH8hTQuaDdJKTO wlcw== X-Gm-Message-State: AOAM531sQenUbFzKysHTPYihXZCxbL2IZZ6V3NPRvsXqfl23h0sd7CNk yhFVZk59CwY75QcAxJF1lF699QZoIhdF8TK8SQ8= X-Google-Smtp-Source: ABdhPJyr6ao6JKY3kh4wo/uPStmXsu/G2ImOGPcqB6umN+lBpwROmdYRQa76oJabfTEMI8k+KK7JQYiXMJJ5NntMMhM= X-Received: by 2002:a67:1bc4:: with SMTP id b187mr1540419vsb.59.1601345091353; Mon, 28 Sep 2020 19:04:51 -0700 (PDT) MIME-Version: 1.0 References: <1601219284-13275-1-git-send-email-jrdr.linux@gmail.com> <20200928083757.GA18329@kadam> In-Reply-To: <20200928083757.GA18329@kadam> From: Souptick Joarder Date: Tue, 29 Sep 2020 07:34:39 +0530 Message-ID: Subject: Re: [PATCH] media: atomisp: Fixed error handling path To: Dan Carpenter Cc: mchehab@kernel.org, sakari.ailus@linux.intel.com, Greg KH , daniel.m.jordan@oracle.com, Andrew Morton , Michel Lespinasse , gustavoars@kernel.org, linux-media@vger.kernel.org, "open list:ANDROID DRIVERS" , linux-kernel@vger.kernel.org, John Hubbard , Ira Weiny Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Dan, On Mon, Sep 28, 2020 at 2:08 PM Dan Carpenter wrote: > > On Sun, Sep 27, 2020 at 08:38:04PM +0530, Souptick Joarder wrote: > > Inside alloc_user_pages() based on flag value either pin_user_pages() > > or get_user_pages_fast() will be called. However, these API might fail. > > > > But free_user_pages() called in error handling path doesn't bother > > about return value and will try to unpin bo->pgnr pages, which is > > incorrect. > > > > Fix this by passing the page_nr to free_user_pages(). If page_nr > 0 > > pages will be unpinned based on bo->mem_type. This will also take care > > of non error handling path. > > > > Fixes: 14a638ab96c5 ("media: atomisp: use pin_user_pages() for memory > > allocation") > > Signed-off-by: Souptick Joarder > > Cc: John Hubbard > > Cc: Ira Weiny > > Cc: Dan Carpenter > > --- > > drivers/staging/media/atomisp/pci/hmm/hmm_bo.c | 13 ++++++++----- > > 1 file changed, 8 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/staging/media/atomisp/pci/hmm/hmm_bo.c b/drivers/staging/media/atomisp/pci/hmm/hmm_bo.c > > index f13af23..0168f98 100644 > > --- a/drivers/staging/media/atomisp/pci/hmm/hmm_bo.c > > +++ b/drivers/staging/media/atomisp/pci/hmm/hmm_bo.c > > @@ -857,16 +857,17 @@ static void free_private_pages(struct hmm_buffer_object *bo, > > kfree(bo->page_obj); > > } > > > > -static void free_user_pages(struct hmm_buffer_object *bo) > > +static void free_user_pages(struct hmm_buffer_object *bo, > > + unsigned int page_nr) > > { > > int i; > > > > hmm_mem_stat.usr_size -= bo->pgnr; > ^^^^^^^^^^^ > This is still a problem. It needs to be hmm_mem_stat.usr_size -= page_nr. In failure path, it is hmm_mem_stat.usr_size += bo->pgnr. I think, hmm_mem_stat.usr_size -= bo->pgnr is correct as per existing code. Do you think that needs to be changed ? > > regards, > dan carpenter > > > > > if (bo->mem_type == HMM_BO_MEM_TYPE_PFN) { > > - unpin_user_pages(bo->pages, bo->pgnr); > > + unpin_user_pages(bo->pages, page_nr); > > } else { > > - for (i = 0; i < bo->pgnr; i++) > > + for (i = 0; i < page_nr; i++) > > put_page(bo->pages[i]); > > } > > kfree(bo->pages); > 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=-9.3 required=3.0 tests=BAYES_00,DKIM_ADSP_CUSTOM_MED, DKIM_INVALID,DKIM_SIGNED,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable 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 00D42C2D0A8 for ; Tue, 29 Sep 2020 02:04:59 +0000 (UTC) Received: from whitealder.osuosl.org (smtp1.osuosl.org [140.211.166.138]) (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 8869820774 for ; Tue, 29 Sep 2020 02:04:58 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="ElzTpIgl" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 8869820774 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=driverdev-devel-bounces@linuxdriverproject.org Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 9861285044; Tue, 29 Sep 2020 02:04:57 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from whitealder.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id ydUzKiFdmfN0; Tue, 29 Sep 2020 02:04:55 +0000 (UTC) Received: from ash.osuosl.org (ash.osuosl.org [140.211.166.34]) by whitealder.osuosl.org (Postfix) with ESMTP id 68A4A86658; Tue, 29 Sep 2020 02:04:55 +0000 (UTC) Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136]) by ash.osuosl.org (Postfix) with ESMTP id 7804B1BF327 for ; Tue, 29 Sep 2020 02:04:54 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 55DE6204F5 for ; Tue, 29 Sep 2020 02:04:54 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id C+jDf75rbWhI for ; Tue, 29 Sep 2020 02:04:52 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from mail-vs1-f68.google.com (mail-vs1-f68.google.com [209.85.217.68]) by silver.osuosl.org (Postfix) with ESMTPS id 7656720336 for ; Tue, 29 Sep 2020 02:04:52 +0000 (UTC) Received: by mail-vs1-f68.google.com with SMTP id w11so22509vsw.13 for ; Mon, 28 Sep 2020 19:04:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=iDhv+ozgLfJpz4ReM/64bivezuXaXXhI3uaVkJqD6ls=; b=ElzTpIglC+vmQlt1o4Loci+G0xfVId1TvwEqqaq9K2WDKtIa9s/LqIro0Fr2c73mVn kmhkY9wYhFBx8SLb+/vdoOGgOLpEv8t6yt8jtquj71p3kzrlKl+Wl+REsmWjuuZ1XBUD TMLpUWCLSaojzcxUGI4TIWNmUb0MH8Krs0tUvv8CAjBtoak+oQQ21aC5RfyLBIpJ3C55 mPtRWvnb9PjSzLLQn0xBj8juntZIieZH7daRO0vmOv70RxDXTKXUMWsfo9rdlpUglp73 VjmwWGFYZJGsHB64AqjSAdayBHKQkf3SzoRzMHpuVWbwG2L9vT/AxdfHrO+o6iiVip3B LPYg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=iDhv+ozgLfJpz4ReM/64bivezuXaXXhI3uaVkJqD6ls=; b=OsR/HbD//fUaeIFrYWBglICXLCmudvnzyaj+pgYpPc8F0AM+8hYfp/3sThjwWabaOT ziIkHFs286qecsDF/kA2Dydag4WEisZhk9+se6WtpKu9vPovpHlAjRKVyzpfkmvIq0p8 wQSpdloAdbNgp54ODKHaHv+WHM+fwBhIczOeLjPp5hVcNm5smkVegPu8bHuNT51iley0 k/DTgMo/xnKJwAu9uDtwQtlR82431Ad1eaT5XHZDTNdhfPucnqI1WEbIXgIgq73qpLRA ZGN7QlVcme9JwtLDsHkloEQCrRdVxEC4cDMCAjJhm2BTw0tlrO3HI1ONDUhXO6IQK0Jw qr1g== X-Gm-Message-State: AOAM532u5IoeySMAG+IT186wmZrqiRINMNeQa5QmKo0X9bfaxTY7+y0x xnvvfri7etOCpKoGQ1e58FsPxJ8rCVnUO+DpoKo= X-Google-Smtp-Source: ABdhPJyr6ao6JKY3kh4wo/uPStmXsu/G2ImOGPcqB6umN+lBpwROmdYRQa76oJabfTEMI8k+KK7JQYiXMJJ5NntMMhM= X-Received: by 2002:a67:1bc4:: with SMTP id b187mr1540419vsb.59.1601345091353; Mon, 28 Sep 2020 19:04:51 -0700 (PDT) MIME-Version: 1.0 References: <1601219284-13275-1-git-send-email-jrdr.linux@gmail.com> <20200928083757.GA18329@kadam> In-Reply-To: <20200928083757.GA18329@kadam> From: Souptick Joarder Date: Tue, 29 Sep 2020 07:34:39 +0530 Message-ID: Subject: Re: [PATCH] media: atomisp: Fixed error handling path To: Dan Carpenter X-BeenThere: driverdev-devel@linuxdriverproject.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux Driver Project Developer List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "open list:ANDROID DRIVERS" , Michel Lespinasse , Greg KH , gustavoars@kernel.org, daniel.m.jordan@oracle.com, sakari.ailus@linux.intel.com, John Hubbard , Andrew Morton , mchehab@kernel.org, linux-kernel@vger.kernel.org, linux-media@vger.kernel.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: driverdev-devel-bounces@linuxdriverproject.org Sender: "devel" Hi Dan, On Mon, Sep 28, 2020 at 2:08 PM Dan Carpenter wrote: > > On Sun, Sep 27, 2020 at 08:38:04PM +0530, Souptick Joarder wrote: > > Inside alloc_user_pages() based on flag value either pin_user_pages() > > or get_user_pages_fast() will be called. However, these API might fail. > > > > But free_user_pages() called in error handling path doesn't bother > > about return value and will try to unpin bo->pgnr pages, which is > > incorrect. > > > > Fix this by passing the page_nr to free_user_pages(). If page_nr > 0 > > pages will be unpinned based on bo->mem_type. This will also take care > > of non error handling path. > > > > Fixes: 14a638ab96c5 ("media: atomisp: use pin_user_pages() for memory > > allocation") > > Signed-off-by: Souptick Joarder > > Cc: John Hubbard > > Cc: Ira Weiny > > Cc: Dan Carpenter > > --- > > drivers/staging/media/atomisp/pci/hmm/hmm_bo.c | 13 ++++++++----- > > 1 file changed, 8 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/staging/media/atomisp/pci/hmm/hmm_bo.c b/drivers/staging/media/atomisp/pci/hmm/hmm_bo.c > > index f13af23..0168f98 100644 > > --- a/drivers/staging/media/atomisp/pci/hmm/hmm_bo.c > > +++ b/drivers/staging/media/atomisp/pci/hmm/hmm_bo.c > > @@ -857,16 +857,17 @@ static void free_private_pages(struct hmm_buffer_object *bo, > > kfree(bo->page_obj); > > } > > > > -static void free_user_pages(struct hmm_buffer_object *bo) > > +static void free_user_pages(struct hmm_buffer_object *bo, > > + unsigned int page_nr) > > { > > int i; > > > > hmm_mem_stat.usr_size -= bo->pgnr; > ^^^^^^^^^^^ > This is still a problem. It needs to be hmm_mem_stat.usr_size -= page_nr. In failure path, it is hmm_mem_stat.usr_size += bo->pgnr. I think, hmm_mem_stat.usr_size -= bo->pgnr is correct as per existing code. Do you think that needs to be changed ? > > regards, > dan carpenter > > > > > if (bo->mem_type == HMM_BO_MEM_TYPE_PFN) { > > - unpin_user_pages(bo->pages, bo->pgnr); > > + unpin_user_pages(bo->pages, page_nr); > > } else { > > - for (i = 0; i < bo->pgnr; i++) > > + for (i = 0; i < page_nr; i++) > > put_page(bo->pages[i]); > > } > > kfree(bo->pages); > _______________________________________________ devel mailing list devel@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel