From: Souptick Joarder <jrdr.linux@gmail.com> To: Dan Carpenter <dan.carpenter@oracle.com> Cc: mchehab@kernel.org, sakari.ailus@linux.intel.com, Greg KH <gregkh@linuxfoundation.org>, daniel.m.jordan@oracle.com, Andrew Morton <akpm@linux-foundation.org>, Michel Lespinasse <walken@google.com>, gustavoars@kernel.org, linux-media@vger.kernel.org, "open list:ANDROID DRIVERS" <devel@driverdev.osuosl.org>, linux-kernel@vger.kernel.org, John Hubbard <jhubbard@nvidia.com>, Ira Weiny <ira.weiny@intel.com> Subject: Re: [PATCH] media: atomisp: Fixed error handling path Date: Tue, 29 Sep 2020 07:34:39 +0530 [thread overview] Message-ID: <CAFqt6zZaf=+JcUuxKdoEj1vMs5MOsb1iYKStmwKiKLW8bbcnYQ@mail.gmail.com> (raw) In-Reply-To: <20200928083757.GA18329@kadam> Hi Dan, On Mon, Sep 28, 2020 at 2:08 PM Dan Carpenter <dan.carpenter@oracle.com> 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 <jrdr.linux@gmail.com> > > Cc: John Hubbard <jhubbard@nvidia.com> > > Cc: Ira Weiny <ira.weiny@intel.com> > > Cc: Dan Carpenter <dan.carpenter@oracle.com> > > --- > > 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); >
WARNING: multiple messages have this Message-ID (diff)
From: Souptick Joarder <jrdr.linux@gmail.com> To: Dan Carpenter <dan.carpenter@oracle.com> Cc: "open list:ANDROID DRIVERS" <devel@driverdev.osuosl.org>, Michel Lespinasse <walken@google.com>, Greg KH <gregkh@linuxfoundation.org>, gustavoars@kernel.org, daniel.m.jordan@oracle.com, sakari.ailus@linux.intel.com, John Hubbard <jhubbard@nvidia.com>, Andrew Morton <akpm@linux-foundation.org>, mchehab@kernel.org, linux-kernel@vger.kernel.org, linux-media@vger.kernel.org Subject: Re: [PATCH] media: atomisp: Fixed error handling path Date: Tue, 29 Sep 2020 07:34:39 +0530 [thread overview] Message-ID: <CAFqt6zZaf=+JcUuxKdoEj1vMs5MOsb1iYKStmwKiKLW8bbcnYQ@mail.gmail.com> (raw) In-Reply-To: <20200928083757.GA18329@kadam> Hi Dan, On Mon, Sep 28, 2020 at 2:08 PM Dan Carpenter <dan.carpenter@oracle.com> 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 <jrdr.linux@gmail.com> > > Cc: John Hubbard <jhubbard@nvidia.com> > > Cc: Ira Weiny <ira.weiny@intel.com> > > Cc: Dan Carpenter <dan.carpenter@oracle.com> > > --- > > 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
next prev parent reply other threads:[~2020-09-29 2:04 UTC|newest] Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-09-27 15:08 [PATCH] media: atomisp: Fixed error handling path Souptick Joarder 2020-09-27 15:08 ` Souptick Joarder 2020-09-28 8:37 ` Dan Carpenter 2020-09-28 8:37 ` Dan Carpenter 2020-09-29 2:04 ` Souptick Joarder [this message] 2020-09-29 2:04 ` Souptick Joarder 2020-09-29 6:14 ` Dan Carpenter 2020-09-29 6:14 ` Dan Carpenter 2020-09-27 17:49 Markus Elfring
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CAFqt6zZaf=+JcUuxKdoEj1vMs5MOsb1iYKStmwKiKLW8bbcnYQ@mail.gmail.com' \ --to=jrdr.linux@gmail.com \ --cc=akpm@linux-foundation.org \ --cc=dan.carpenter@oracle.com \ --cc=daniel.m.jordan@oracle.com \ --cc=devel@driverdev.osuosl.org \ --cc=gregkh@linuxfoundation.org \ --cc=gustavoars@kernel.org \ --cc=ira.weiny@intel.com \ --cc=jhubbard@nvidia.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-media@vger.kernel.org \ --cc=mchehab@kernel.org \ --cc=sakari.ailus@linux.intel.com \ --cc=walken@google.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.