Re: [Linux-kernel-mentees][PATCH v4] staging: kpc2000: Convert put_page to put_user_page*()

[Bharath Vedartham]

On Tue, Jul 30, 2019 at 11:36:06AM +0200, Greg KH wrote:
> On Tue, Jul 30, 2019 at 02:58:44PM +0530, Bharath Vedartham wrote:
> > put_page() to put_user_page*()
> 
> What does this mean?

That must have been a mistake! I just wanted to forward this patch to
the Linux-kernel-mentees mailing list. THis patch has already been taken
by for staging-testing. I ll forward another patch just cc'ing the
mentees mailing lists and won't disturb the other devs.

Thank you
Bharath
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [Linux-kernel-mentees][PATCH v4] staging: kpc2000: Convert put_page to put_user_page*()

[Bharath Vedartham]

put_page() to put_user_page*()
Reply-To: 
In-Reply-To: <1564058658-3551-1-git-send-email-linux.b...@gmail.com>

On Thu, Jul 25, 2019 at 06:14:18PM +0530, Bharath Vedartham wrote:
[Forwarding patch to linux-kernel-mentees mailing list]
> For pages that were retained via get_user_pages*(), release those pages
> via the new put_user_page*() routines, instead of via put_page().
> 
> This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
> ("mm: introduce put_user_page*(), placeholder versions").
> 
> Cc: Ira Weiny 
> Cc: John Hubbard 
> Cc: Jérôme Glisse 
> Cc: Greg Kroah-Hartman 
> Cc: Matt Sickler 
> Cc: de...@driverdev.osuosl.org
> Cc: linux-ker...@vger.kernel.org
> Cc: linux...@kvack.org
> Reviewed-by: John Hubbard 
> Signed-off-by: Bharath Vedartham 
> ---
> Changes since v1
> - Improved changelog by John's suggestion.
> - Moved logic to dirty pages below sg_dma_unmap
>  and removed PageReserved check.
> Changes since v2
> - Added back PageResevered check as
> suggested by John Hubbard.
> Changes since v3
> - Changed the changelog as suggested by John.
> - Added John's Reviewed-By tag.
> Changes since v4
> - Rebased the patch on the staging tree.
> - Improved commit log by fixing a line wrap.
> ---
>  drivers/staging/kpc2000/kpc_dma/fileops.c | 17 ++---
>  1 file changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c 
> b/drivers/staging/kpc2000/kpc_dma/fileops.c
> index 48ca88b..f15e292 100644
> --- a/drivers/staging/kpc2000/kpc_dma/fileops.c
> +++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
> @@ -190,9 +190,7 @@ static int kpc_dma_transfer(struct dev_private_data *priv,
>   sg_free_table(>sgt);
>   err_dma_map_sg:
>   err_alloc_sg_table:
> - for (i = 0 ; i < acd->page_count ; i++) {
> - put_page(acd->user_pages[i]);
> - }
> + put_user_pages(acd->user_pages, acd->page_count);
>   err_get_user_pages:
>   kfree(acd->user_pages);
>   err_alloc_userpages:
> @@ -211,16 +209,13 @@ void  transfer_complete_cb(struct aio_cb_data *acd, 
> size_t xfr_count, u32 flags)
>   BUG_ON(acd->ldev == NULL);
>   BUG_ON(acd->ldev->pldev == NULL);
>  
> - for (i = 0 ; i < acd->page_count ; i++) {
> - if (!PageReserved(acd->user_pages[i])) {
> - set_page_dirty(acd->user_pages[i]);
> - }
> - }
> -
>   dma_unmap_sg(>ldev->pldev->dev, acd->sgt.sgl, acd->sgt.nents, 
> acd->ldev->dir);
>  
> - for (i = 0 ; i < acd->page_count ; i++) {
> - put_page(acd->user_pages[i]);
> + for (i = 0; i < acd->page_count; i++) {
> + if (!PageReserved(acd->user_pages[i]))
> + put_user_pages_dirty(>user_pages[i], 1);
> + else
> + put_user_page(acd->user_pages[i]);
>   }
>  
>   sg_free_table(>sgt);
> -- 
> 2.7.4
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4] staging: kpc2000: Convert put_page() to put_user_page*()

[Bharath Vedartham]

For pages that were retained via get_user_pages*(), release those pages
via the new put_user_page*() routines, instead of via put_page().

This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
("mm: introduce put_user_page*(), placeholder versions").

Cc: Ira Weiny 
Cc: John Hubbard 
Cc: Jérôme Glisse 
Cc: Greg Kroah-Hartman 
Cc: Matt Sickler 
Cc: de...@driverdev.osuosl.org
Cc: linux-ker...@vger.kernel.org
Cc: linux...@kvack.org
Reviewed-by: John Hubbard 
Signed-off-by: Bharath Vedartham 
---
Changes since v1
- Improved changelog by John's suggestion.
- Moved logic to dirty pages below sg_dma_unmap
 and removed PageReserved check.
Changes since v2
- Added back PageResevered check as
suggested by John Hubbard.
Changes since v3
- Changed the changelog as suggested by John.
- Added John's Reviewed-By tag.
Changes since v4
- Rebased the patch on the staging tree.
- Improved commit log by fixing a line wrap.
---
 drivers/staging/kpc2000/kpc_dma/fileops.c | 17 ++---
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c 
b/drivers/staging/kpc2000/kpc_dma/fileops.c
index 48ca88b..f15e292 100644
--- a/drivers/staging/kpc2000/kpc_dma/fileops.c
+++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
@@ -190,9 +190,7 @@ static int kpc_dma_transfer(struct dev_private_data *priv,
sg_free_table(>sgt);
  err_dma_map_sg:
  err_alloc_sg_table:
-   for (i = 0 ; i < acd->page_count ; i++) {
-   put_page(acd->user_pages[i]);
-   }
+   put_user_pages(acd->user_pages, acd->page_count);
  err_get_user_pages:
kfree(acd->user_pages);
  err_alloc_userpages:
@@ -211,16 +209,13 @@ void  transfer_complete_cb(struct aio_cb_data *acd, 
size_t xfr_count, u32 flags)
BUG_ON(acd->ldev == NULL);
BUG_ON(acd->ldev->pldev == NULL);
 
-   for (i = 0 ; i < acd->page_count ; i++) {
-   if (!PageReserved(acd->user_pages[i])) {
-   set_page_dirty(acd->user_pages[i]);
-   }
-   }
-
dma_unmap_sg(>ldev->pldev->dev, acd->sgt.sgl, acd->sgt.nents, 
acd->ldev->dir);
 
-   for (i = 0 ; i < acd->page_count ; i++) {
-   put_page(acd->user_pages[i]);
+   for (i = 0; i < acd->page_count; i++) {
+   if (!PageReserved(acd->user_pages[i]))
+   put_user_pages_dirty(>user_pages[i], 1);
+   else
+   put_user_page(acd->user_pages[i]);
}
 
sg_free_table(>sgt);
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v4] staging: kpc2000: Convert put_page to put_user_page*()

[Bharath Vedartham]

On Thu, Jul 25, 2019 at 09:46:34AM +0200, Greg KH wrote:
> On Sat, Jul 20, 2019 at 11:02:14PM +0530, Bharath Vedartham wrote:
> > For pages that were retained via get_user_pages*(), release those pages
> > via the new put_user_page*() routines, instead of via put_page().
> > 
> > This is part a tree-wide conversion, as described in commit fc1d8e7cca2d 
> > ("mm: introduce put_user_page*(), placeholder versions").
> 
> Please line-wrap this line.
> 
> > 
> > Cc: Ira Weiny 
> > Cc: John Hubbard 
> > Cc: Jérôme Glisse 
> > Cc: Greg Kroah-Hartman 
> > Cc: Matt Sickler 
> > Cc: de...@driverdev.osuosl.org
> > Cc: linux-ker...@vger.kernel.org
> > Cc: linux...@kvack.org
> > Reviewed-by: John Hubbard 
> > Signed-off-by: Bharath Vedartham 
> > ---
> > Changes since v1
> >- Improved changelog by John's suggestion.
> >- Moved logic to dirty pages below sg_dma_unmap
> >and removed PageReserved check.
> > Changes since v2
> >- Added back PageResevered check as suggested by John Hubbard.
> > Changes since v3
> >- Changed the commit log as suggested by John.
> >- Added John's Reviewed-By tag
> > 
> > ---
> >  drivers/staging/kpc2000/kpc_dma/fileops.c | 17 ++---
> >  1 file changed, 6 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c 
> > b/drivers/staging/kpc2000/kpc_dma/fileops.c
> > index 6166587..75ad263 100644
> > --- a/drivers/staging/kpc2000/kpc_dma/fileops.c
> > +++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
> > @@ -198,9 +198,7 @@ int  kpc_dma_transfer(struct dev_private_data *priv, 
> > struct kiocb *kcb, unsigned
> > sg_free_table(>sgt);
> >   err_dma_map_sg:
> >   err_alloc_sg_table:
> > -   for (i = 0 ; i < acd->page_count ; i++){
> > -   put_page(acd->user_pages[i]);
> > -   }
> > +   put_user_pages(acd->user_pages, acd->page_count);
> >   err_get_user_pages:
> > kfree(acd->user_pages);
> >   err_alloc_userpages:
> > @@ -221,16 +219,13 @@ void  transfer_complete_cb(struct aio_cb_data *acd, 
> > size_t xfr_count, u32 flags)
> > 
> > dev_dbg(>ldev->pldev->dev, "transfer_complete_cb(acd = [%p])\n", 
> > acd);
> > 
> > -   for (i = 0 ; i < acd->page_count ; i++){
> > -   if (!PageReserved(acd->user_pages[i])){
> > -   set_page_dirty(acd->user_pages[i]);
> > -   }
> > -   }
> > -   
> > dma_unmap_sg(>ldev->pldev->dev, acd->sgt.sgl, acd->sgt.nents, 
> > acd->ldev->dir);
> > 
> > -   for (i = 0 ; i < acd->page_count ; i++){
> > -   put_page(acd->user_pages[i]);
> > +   for (i = 0; i < acd->page_count; i++) {
> > +   if (!PageReserved(acd->user_pages[i]))
> > +   put_user_pages_dirty(>user_pages[i], 1);
> > +   else
> > +   put_user_page(acd->user_pages[i]);
> > }
> > 
> > sg_free_table(>sgt);
> > -- 
> > 2.7.4
> 
> This patch can not be applied at all :(
> 
> Can you redo it against the latest staging-next branch and resend?
> 
> thanks,
Yup. Will do that!
> greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3] staging: kpc2000: Convert put_page to put_user_page*()

[Bharath Vedartham]

On Fri, Jul 19, 2019 at 08:59:02PM +, Matt Sickler wrote:
> >From: Bharath Vedartham 
> >Changes since v2
> >- Added back PageResevered check as suggested by John Hubbard.
> >
> >The PageReserved check needs a closer look and is not worth messing
> >around with for now.
> >
> >Matt, Could you give any suggestions for testing this patch?
> 
> Myself or someone else from Daktronics would have to do the testing since the
> hardware isn't really commercially available.  I've been toying with the idea
> of asking for a volunteer from the mailing list to help me out with this - I'd
> send them some hardware and they'd do all the development and testing. :)
> I still have to run that idea by Management though.
> 
> >If in-case, you are willing to pick this up to test. Could you
> >apply this patch to this tree and test it with your devices?
> 
> I've been meaning to get to testing the changes to the drivers since 
> upstreaming
> them, but I've been swamped with other development.  I'm keeping an eye on the
> mailing lists, so I'm at least aware of what is coming down the pipe.
> I'm not too worried about this specific change, even though I don't really 
> know
> if the reserved check and the dirtying are even necessary.
> It sounded like John's suggestion was to not do the PageReserved() check and 
> just
> use put_user_pges_dirty() all the time.  John, is that incorrect?
The change is fairly trivial in the upstream kernel. It requires no
testing in the upstream kernel. It would be great if you could test it
on John's git tree with the implemented gup tracking subsystem and check
if gup tracking is working alright with your dma driver. I think this
patch will easily apply to John's git tree.

Thanks!
Bharath
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4] staging: kpc2000: Convert put_page to put_user_page*()

[Bharath Vedartham]

For pages that were retained via get_user_pages*(), release those pages
via the new put_user_page*() routines, instead of via put_page().

This is part a tree-wide conversion, as described in commit fc1d8e7cca2d ("mm: 
introduce put_user_page*(), placeholder versions").

Cc: Ira Weiny 
Cc: John Hubbard 
Cc: Jérôme Glisse 
Cc: Greg Kroah-Hartman 
Cc: Matt Sickler 
Cc: de...@driverdev.osuosl.org
Cc: linux-ker...@vger.kernel.org
Cc: linux...@kvack.org
Reviewed-by: John Hubbard 
Signed-off-by: Bharath Vedartham 
---
Changes since v1
   - Improved changelog by John's suggestion.
   - Moved logic to dirty pages below sg_dma_unmap
   and removed PageReserved check.
Changes since v2
   - Added back PageResevered check as suggested by John Hubbard.
Changes since v3
   - Changed the commit log as suggested by John.
   - Added John's Reviewed-By tag

---
 drivers/staging/kpc2000/kpc_dma/fileops.c | 17 ++---
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c 
b/drivers/staging/kpc2000/kpc_dma/fileops.c
index 6166587..75ad263 100644
--- a/drivers/staging/kpc2000/kpc_dma/fileops.c
+++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
@@ -198,9 +198,7 @@ int  kpc_dma_transfer(struct dev_private_data *priv, struct 
kiocb *kcb, unsigned
sg_free_table(>sgt);
  err_dma_map_sg:
  err_alloc_sg_table:
-   for (i = 0 ; i < acd->page_count ; i++){
-   put_page(acd->user_pages[i]);
-   }
+   put_user_pages(acd->user_pages, acd->page_count);
  err_get_user_pages:
kfree(acd->user_pages);
  err_alloc_userpages:
@@ -221,16 +219,13 @@ void  transfer_complete_cb(struct aio_cb_data *acd, 
size_t xfr_count, u32 flags)

dev_dbg(>ldev->pldev->dev, "transfer_complete_cb(acd = [%p])\n", 
acd);

-   for (i = 0 ; i < acd->page_count ; i++){
-   if (!PageReserved(acd->user_pages[i])){
-   set_page_dirty(acd->user_pages[i]);
-   }
-   }
-   
dma_unmap_sg(>ldev->pldev->dev, acd->sgt.sgl, acd->sgt.nents, 
acd->ldev->dir);

-   for (i = 0 ; i < acd->page_count ; i++){
-   put_page(acd->user_pages[i]);
+   for (i = 0; i < acd->page_count; i++) {
+   if (!PageReserved(acd->user_pages[i]))
+   put_user_pages_dirty(>user_pages[i], 1);
+   else
+   put_user_page(acd->user_pages[i]);
}

sg_free_table(>sgt);
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3] staging: kpc2000: Convert put_page to put_user_page*()

[Bharath Vedartham]

On Fri, Jul 19, 2019 at 02:28:39PM -0700, John Hubbard wrote:
> On 7/19/19 1:02 PM, Bharath Vedartham wrote:
> > There have been issues with coordination of various subsystems using
> > get_user_pages. These issues are better described in [1].
> > 
> > An implementation of tracking get_user_pages is currently underway
> > The implementation requires the use put_user_page*() variants to release
> > a reference rather than put_page(). The commit that introduced
> > put_user_pages, Commit fc1d8e7cca2daa18d2fe56b94874848adf89d7f5 ("mm: 
> > introduce
> > put_user_page*(), placeholder version").
> > 
> > The implementation currently simply calls put_page() within
> > put_user_page(). But in the future, it is to change to add a mechanism
> > to keep track of get_user_pages. Once a tracking mechanism is
> > implemented, we can make attempts to work on improving on coordination
> > between various subsystems using get_user_pages.
> > 
> > [1] https://lwn.net/Articles/753027/
> 
> Optional: I've been fussing about how to keep the change log reasonable,
> and finally came up with the following recommended template for these 
> conversion patches. This would replace the text you have above, because the 
> put_user_page placeholder commit has all the documentation (and then some) 
> that we need:
> 
> 
> For pages that were retained via get_user_pages*(), release those pages
> via the new put_user_page*() routines, instead of via put_page().
> 
> This is part a tree-wide conversion, as described in commit fc1d8e7cca2d
> ("mm: introduce put_user_page*(), placeholder versions").
Great then, I ll send another patch with the updated changelog.
> 
> For the change itself, you will need to rebase it onto the latest 
> linux.git, as it doesn't quite apply there. 
> 
> Testing is good if we can get it, but as far as I can tell this is
> correct, so you can also add:
> 
> Reviewed-by: John Hubbard 
Thanks! 
> thanks,
> -- 
> John Hubbard
> NVIDIA
>
> > 
> > Cc: Ira Weiny 
> > Cc: John Hubbard 
> > Cc: Jérôme Glisse 
> > Cc: Greg Kroah-Hartman 
> > Cc: Matt Sickler 
> > Cc: de...@driverdev.osuosl.org 
> > Cc: linux-ker...@vger.kernel.org
> > Cc: linux...@kvack.org
> > Signed-off-by: Bharath Vedartham 
> > ---
> > Changes since v1
> > - Improved changelog by John's suggestion.
> > - Moved logic to dirty pages below sg_dma_unmap
> > and removed PageReserved check.
> > Changes since v2
> > - Added back PageResevered check as suggested by John Hubbard.
> > 
> > The PageReserved check needs a closer look and is not worth messing
> > around with for now.
> > 
> > Matt, Could you give any suggestions for testing this patch?
> > 
> > If in-case, you are willing to pick this up to test. Could you
> > apply this patch to this tree
> > https://github.com/johnhubbard/linux/tree/gup_dma_core
> > and test it with your devices?
> > 
> > ---
> >  drivers/staging/kpc2000/kpc_dma/fileops.c | 17 ++---
> >  1 file changed, 6 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c 
> > b/drivers/staging/kpc2000/kpc_dma/fileops.c
> > index 6166587..75ad263 100644
> > --- a/drivers/staging/kpc2000/kpc_dma/fileops.c
> > +++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
> > @@ -198,9 +198,7 @@ int  kpc_dma_transfer(struct dev_private_data *priv, 
> > struct kiocb *kcb, unsigned
> > sg_free_table(>sgt);
> >   err_dma_map_sg:
> >   err_alloc_sg_table:
> > -   for (i = 0 ; i < acd->page_count ; i++){
> > -   put_page(acd->user_pages[i]);
> > -   }
> > +   put_user_pages(acd->user_pages, acd->page_count);
> >   err_get_user_pages:
> > kfree(acd->user_pages);
> >   err_alloc_userpages:
> > @@ -221,16 +219,13 @@ void  transfer_complete_cb(struct aio_cb_data *acd, 
> > size_t xfr_count, u32 flags)
> > 
> > dev_dbg(>ldev->pldev->dev, "transfer_complete_cb(acd = [%p])\n", 
> > acd);
> > 
> > -   for (i = 0 ; i < acd->page_count ; i++){
> > -   if (!PageReserved(acd->user_pages[i])){
> > -   set_page_dirty(acd->user_pages[i]);
> > -   }
> > -   }
> > -   
> > dma_unmap_sg(>ldev->pldev->dev, acd->sgt.sgl, acd->sgt.nents, 
> > acd->ldev->dir);
> > 
> > -   for (i = 0 ; i < acd->page_count ; i++){
> > -   put_page(acd->user_pages[i]);
> > +   for (i = 0; i < acd->page_count; i++) {
> > +   if (!PageReserved(acd->user_pages[i]))
> > +   put_user_pages_dirty(>user_pages[i], 1);
> > +   else
> > +   put_user_page(acd->user_pages[i]);
> > }
> > 
> > sg_free_table(>sgt);
> > 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3] staging: kpc2000: Convert put_page to put_user_page*()

[Bharath Vedartham]

There have been issues with coordination of various subsystems using
get_user_pages. These issues are better described in [1].

An implementation of tracking get_user_pages is currently underway
The implementation requires the use put_user_page*() variants to release
a reference rather than put_page(). The commit that introduced
put_user_pages, Commit fc1d8e7cca2daa18d2fe56b94874848adf89d7f5 ("mm: introduce
put_user_page*(), placeholder version").

The implementation currently simply calls put_page() within
put_user_page(). But in the future, it is to change to add a mechanism
to keep track of get_user_pages. Once a tracking mechanism is
implemented, we can make attempts to work on improving on coordination
between various subsystems using get_user_pages.

[1] https://lwn.net/Articles/753027/

Cc: Ira Weiny 
Cc: John Hubbard 
Cc: Jérôme Glisse 
Cc: Greg Kroah-Hartman 
Cc: Matt Sickler 
Cc: de...@driverdev.osuosl.org 
Cc: linux-ker...@vger.kernel.org
Cc: linux...@kvack.org
Signed-off-by: Bharath Vedartham 
---
Changes since v1
- Improved changelog by John's suggestion.
- Moved logic to dirty pages below sg_dma_unmap
and removed PageReserved check.
Changes since v2
- Added back PageResevered check as suggested by John Hubbard.

The PageReserved check needs a closer look and is not worth messing
around with for now.

Matt, Could you give any suggestions for testing this patch?

If in-case, you are willing to pick this up to test. Could you
apply this patch to this tree
https://github.com/johnhubbard/linux/tree/gup_dma_core
and test it with your devices?

---
 drivers/staging/kpc2000/kpc_dma/fileops.c | 17 ++---
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c 
b/drivers/staging/kpc2000/kpc_dma/fileops.c
index 6166587..75ad263 100644
--- a/drivers/staging/kpc2000/kpc_dma/fileops.c
+++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
@@ -198,9 +198,7 @@ int  kpc_dma_transfer(struct dev_private_data *priv, struct 
kiocb *kcb, unsigned
sg_free_table(>sgt);
  err_dma_map_sg:
  err_alloc_sg_table:
-   for (i = 0 ; i < acd->page_count ; i++){
-   put_page(acd->user_pages[i]);
-   }
+   put_user_pages(acd->user_pages, acd->page_count);
  err_get_user_pages:
kfree(acd->user_pages);
  err_alloc_userpages:
@@ -221,16 +219,13 @@ void  transfer_complete_cb(struct aio_cb_data *acd, 
size_t xfr_count, u32 flags)

dev_dbg(>ldev->pldev->dev, "transfer_complete_cb(acd = [%p])\n", 
acd);

-   for (i = 0 ; i < acd->page_count ; i++){
-   if (!PageReserved(acd->user_pages[i])){
-   set_page_dirty(acd->user_pages[i]);
-   }
-   }
-   
dma_unmap_sg(>ldev->pldev->dev, acd->sgt.sgl, acd->sgt.nents, 
acd->ldev->dir);

-   for (i = 0 ; i < acd->page_count ; i++){
-   put_page(acd->user_pages[i]);
+   for (i = 0; i < acd->page_count; i++) {
+   if (!PageReserved(acd->user_pages[i]))
+   put_user_pages_dirty(>user_pages[i], 1);
+   else
+   put_user_page(acd->user_pages[i]);
}

sg_free_table(>sgt);
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: kpc2000: Convert put_page() to put_user_page*()

[Bharath Vedartham]

On Mon, Jul 15, 2019 at 03:01:43PM -0700, John Hubbard wrote:
> On 7/15/19 2:47 PM, Matt Sickler wrote:
> > It looks like Outlook is going to absolutely trash this email.  Hopefully 
> > it comes through okay.
> > 
> ...
> >>
> >> Because this is a common pattern, and because the code here doesn't likely
> >> need to set page dirty before the dma_unmap_sg call, I think the following
> >> would be better (it's untested), instead of the above diff hunk:
> >>
> >> diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c
> >> b/drivers/staging/kpc2000/kpc_dma/fileops.c
> >> index 48ca88bc6b0b..d486f9866449 100644
> >> --- a/drivers/staging/kpc2000/kpc_dma/fileops.c
> >> +++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
> >> @@ -211,16 +211,13 @@ void  transfer_complete_cb(struct aio_cb_data
> >> *acd, size_t xfr_count, u32 flags)
> >>BUG_ON(acd->ldev == NULL);
> >>BUG_ON(acd->ldev->pldev == NULL);
> >>
> >> -   for (i = 0 ; i < acd->page_count ; i++) {
> >> -   if (!PageReserved(acd->user_pages[i])) {
> >> -   set_page_dirty(acd->user_pages[i]);
> >> -   }
> >> -   }
> >> -
> >>dma_unmap_sg(>ldev->pldev->dev, acd->sgt.sgl, acd->sgt.nents, 
> >> acd->ldev->dir);
> >>
> >>for (i = 0 ; i < acd->page_count ; i++) {
> >> -   put_page(acd->user_pages[i]);
> >> +   if (!PageReserved(acd->user_pages[i])) {
> >> +   put_user_pages_dirty(>user_pages[i], 1);
> >> +   else
> >> +   put_user_page(acd->user_pages[i]);
> >>}
> >>
> >>sg_free_table(>sgt);
> > 
> > I don't think I ever really knew the right way to do this. 
> > 
> > The changes Bharath suggested look okay to me.  I'm not sure about the 
> > check for PageReserved(), though.  At first glance it appears to be 
> > equivalent to what was there before, but maybe I should learn what that 
> > Reserved page flag really means.
> > From [1], the only comment that seems applicable is
> > * - MMIO/DMA pages. Some architectures don't allow to ioremap pages that are
> >  *   not marked PG_reserved (as they might be in use by somebody else who 
> > does
> >  *   not respect the caching strategy).
> > 
> > These pages should be coming from anonymous (RAM, not file backed) memory 
> > in userspace.  Sometimes it comes from hugepage backed memory, though I 
> > don't think that makes a difference.  I should note that 
> > transfer_complete_cb handles both RAM to device and device to RAM DMAs, if 
> > that matters.
Yes. file_operations->read passes a userspace buffer which AFAIK is
anonymous memory.
> > [1] 
> > https://elixir.bootlin.com/linux/v5.2/source/include/linux/page-flags.h#L17
> > 
> 
> I agree: the PageReserved check looks unnecessary here, from my 
> outside-the-kpc_2000-team
> perspective, anyway. Assuming that your analysis above is correct, you could 
> collapse that
> whole think into just:
Since the file_operations->read passes a userspace buffer, I doubt that
the pages of the userspace buffer will be reserved.
> @@ -211,17 +209,8 @@ void  transfer_complete_cb(struct aio_cb_data *acd, 
> size_t xfr_count, u32 flags)
> BUG_ON(acd->ldev == NULL);
> BUG_ON(acd->ldev->pldev == NULL);
>  
> -   for (i = 0 ; i < acd->page_count ; i++) {
> -   if (!PageReserved(acd->user_pages[i])) {
> -   set_page_dirty(acd->user_pages[i]);
> -   }
> -   }
> -
> dma_unmap_sg(>ldev->pldev->dev, acd->sgt.sgl, acd->sgt.nents, 
> acd->ldev->dir);
> -
> -   for (i = 0 ; i < acd->page_count ; i++) {
> -   put_page(acd->user_pages[i]);
> -   }
> +   put_user_pages_dirty(>user_pages[i], acd->page_count);
>  
> sg_free_table(>sgt);
>  
> (Also, Matt, I failed to Cc: you on a semi-related cleanup that I just sent 
> out for this
> driver, as long as I have your attention:
> 
>https://lore.kernel.org/r/20190715212123.432-1-jhubb...@nvidia.com
> )
Matt will you be willing to pick this up for testing or do you want a
different patch?
> thanks,
> -- 
> John Hubbard
> NVIDIA
Thank you
Bharath
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2] staging: kpc2000: Convert put_page() to put_user_page*()

[Bharath Vedartham]

There have been issues with get_user_pages and filesystem writeback.
The issues are better described in [1].

The solution being proposed wants to keep track of gup_pinned pages
which will allow to take furthur steps to coordinate between subsystems
using gup.

put_user_page() simply calls put_page inside for now. But the
implementation will change once all call sites of put_page() are
converted.

[1] https://lwn.net/Articles/753027/

Cc: Matt Sickler 
Cc: Greg Kroah-Hartman 
Cc: Jérôme Glisse 
Cc: Ira Weiny 
Cc: John Hubbard 
Cc: linux...@kvack.org
Cc: de...@driverdev.osuosl.org

Reviewed-by: John Hubbard 
Signed-off-by: Bharath Vedartham 
---
Changes since v1
- Added John's reviewed-by tag
- Moved para talking about testing below
the '---'
- Moved logic of set_page_diry below dma_unmap_sg
as per John's suggestion

I currently do not have the driver to test. Could I have some
suggestions to test this code? The solution is currently implemented
in https://github.com/johnhubbard/linux/tree/gup_dma_core and it would be great 
if we could apply the patch on top of the repo and run some 
tests to check if any regressions occur.
---
 drivers/staging/kpc2000/kpc_dma/fileops.c | 17 ++---
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c 
b/drivers/staging/kpc2000/kpc_dma/fileops.c
index 48ca88b..3d1a00a 100644
--- a/drivers/staging/kpc2000/kpc_dma/fileops.c
+++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
@@ -190,9 +190,7 @@ static int kpc_dma_transfer(struct dev_private_data *priv,
sg_free_table(>sgt);
  err_dma_map_sg:
  err_alloc_sg_table:
-   for (i = 0 ; i < acd->page_count ; i++) {
-   put_page(acd->user_pages[i]);
-   }
+   put_user_pages(acd->user_pages, acd->page_count);
  err_get_user_pages:
kfree(acd->user_pages);
  err_alloc_userpages:
@@ -211,16 +209,13 @@ void  transfer_complete_cb(struct aio_cb_data *acd, 
size_t xfr_count, u32 flags)
BUG_ON(acd->ldev == NULL);
BUG_ON(acd->ldev->pldev == NULL);
 
-   for (i = 0 ; i < acd->page_count ; i++) {
-   if (!PageReserved(acd->user_pages[i])) {
-   set_page_dirty(acd->user_pages[i]);
-   }
-   }
-
dma_unmap_sg(>ldev->pldev->dev, acd->sgt.sgl, acd->sgt.nents, 
acd->ldev->dir);
 
-   for (i = 0 ; i < acd->page_count ; i++) {
-   put_page(acd->user_pages[i]);
+   for (i = 0; i < acd->page_count; i++) {
+   if (!PageReserved(acd->user_pages[i])) 
+   put_user_pages_dirty(>user_pages[i], 1);
+   else
+   put_user_page(acd->user_pages[i]);
}
 
sg_free_table(>sgt);
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: kpc2000: Convert put_page() to put_user_page*()

[Bharath Vedartham]

On Mon, Jul 15, 2019 at 01:14:13PM -0700, John Hubbard wrote:
> On 7/15/19 12:52 PM, Bharath Vedartham wrote:
> > There have been issues with get_user_pages and filesystem writeback.
> > The issues are better described in [1].
> > 
> > The solution being proposed wants to keep track of gup_pinned pages which 
> > will allow to take furthur steps to coordinate between subsystems using gup.
> > 
> > put_user_page() simply calls put_page inside for now. But the 
> > implementation will change once all call sites of put_page() are converted.
> > 
> > I currently do not have the driver to test. Could I have some suggestions 
> > to test this code? The solution is currently implemented in [2] and
> > it would be great if we could apply the patch on top of [2] and run some 
> > tests to check if any regressions occur.
> 
> Hi Bharath,
> 
> Process point: the above paragraph, and other meta-questions (about the 
> patch, rather than part of the patch) should be placed either after the 
> "---", or in a cover letter (git-send-email --cover-letter). That way, the 
> patch itself is in a commit-able state.
> 
> One more below:
Will fix that in the next version. 
> > 
> > [1] https://lwn.net/Articles/753027/
> > [2] https://github.com/johnhubbard/linux/tree/gup_dma_core
> > 
> > Cc: Matt Sickler 
> > Cc: Greg Kroah-Hartman 
> > Cc: Jérôme Glisse 
> > Cc: Ira Weiny 
> > Cc: John Hubbard 
> > Cc: linux...@kvack.org
> > Cc: de...@driverdev.osuosl.org
> > 
> > Signed-off-by: Bharath Vedartham 
> > ---
> >  drivers/staging/kpc2000/kpc_dma/fileops.c | 8 ++--
> >  1 file changed, 2 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c 
> > b/drivers/staging/kpc2000/kpc_dma/fileops.c
> > index 6166587..82c70e6 100644
> > --- a/drivers/staging/kpc2000/kpc_dma/fileops.c
> > +++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
> > @@ -198,9 +198,7 @@ int  kpc_dma_transfer(struct dev_private_data *priv, 
> > struct kiocb *kcb, unsigned
> > sg_free_table(>sgt);
> >   err_dma_map_sg:
> >   err_alloc_sg_table:
> > -   for (i = 0 ; i < acd->page_count ; i++){
> > -   put_page(acd->user_pages[i]);
> > -   }
> > +   put_user_pages(acd->user_pages, acd->page_count);
> >   err_get_user_pages:
> > kfree(acd->user_pages);
> >   err_alloc_userpages:
> > @@ -229,9 +227,7 @@ void  transfer_complete_cb(struct aio_cb_data *acd, 
> > size_t xfr_count, u32 flags)
> > 
> > dma_unmap_sg(>ldev->pldev->dev, acd->sgt.sgl, acd->sgt.nents, 
> > acd->ldev->dir);
> > 
> > -   for (i = 0 ; i < acd->page_count ; i++){
> > -   put_page(acd->user_pages[i]);
> > -   }
> > +   put_user_pages(acd->user_pages, acd->page_count);
> > 
> > sg_free_table(>sgt);
> > 
> > 
> 
> Because this is a common pattern, and because the code here doesn't likely 
> need to set page dirty before the dma_unmap_sg call, I think the following 
> would be better (it's untested), instead of the above diff hunk:
>
> diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c 
> b/drivers/staging/kpc2000/kpc_dma/fileops.c
> index 48ca88bc6b0b..d486f9866449 100644
> --- a/drivers/staging/kpc2000/kpc_dma/fileops.c
> +++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
> @@ -211,16 +211,13 @@ void  transfer_complete_cb(struct aio_cb_data *acd, 
> size_t xfr_count, u32 flags)
> BUG_ON(acd->ldev == NULL);
> BUG_ON(acd->ldev->pldev == NULL);
>  
> -   for (i = 0 ; i < acd->page_count ; i++) {
> -   if (!PageReserved(acd->user_pages[i])) {
> -   set_page_dirty(acd->user_pages[i]);
> -   }
> -   }
> -
> dma_unmap_sg(>ldev->pldev->dev, acd->sgt.sgl, acd->sgt.nents, 
> acd->ldev->dir);
>  
> for (i = 0 ; i < acd->page_count ; i++) {
> -   put_page(acd->user_pages[i]);
> +   if (!PageReserved(acd->user_pages[i])) {
> +   put_user_pages_dirty(>user_pages[i], 1);
> +   else
> +   put_user_page(acd->user_pages[i]);
> }
>  
> sg_free_table(>sgt);
I had my doubts on this. This definitley needs to be looked at by the
driver author. 
> Assuming that you make those two changes, you can add:
> 
> Reviewed-by: John Hubbard 
Great!
> 
> thanks,
> -- 
> John Hubbard
> NVIDIA
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: kpc2000: Convert put_page() to put_user_page*()

[Bharath Vedartham]

There have been issues with get_user_pages and filesystem writeback.
The issues are better described in [1].

The solution being proposed wants to keep track of gup_pinned pages which will 
allow to take furthur steps to coordinate between subsystems using gup.

put_user_page() simply calls put_page inside for now. But the implementation 
will change once all call sites of put_page() are converted.

I currently do not have the driver to test. Could I have some suggestions to 
test this code? The solution is currently implemented in [2] and
it would be great if we could apply the patch on top of [2] and run some tests 
to check if any regressions occur.

[1] https://lwn.net/Articles/753027/
[2] https://github.com/johnhubbard/linux/tree/gup_dma_core

Cc: Matt Sickler 
Cc: Greg Kroah-Hartman 
Cc: Jérôme Glisse 
Cc: Ira Weiny 
Cc: John Hubbard 
Cc: linux...@kvack.org
Cc: de...@driverdev.osuosl.org

Signed-off-by: Bharath Vedartham 
---
 drivers/staging/kpc2000/kpc_dma/fileops.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c 
b/drivers/staging/kpc2000/kpc_dma/fileops.c
index 6166587..82c70e6 100644
--- a/drivers/staging/kpc2000/kpc_dma/fileops.c
+++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
@@ -198,9 +198,7 @@ int  kpc_dma_transfer(struct dev_private_data *priv, struct 
kiocb *kcb, unsigned
sg_free_table(>sgt);
  err_dma_map_sg:
  err_alloc_sg_table:
-   for (i = 0 ; i < acd->page_count ; i++){
-   put_page(acd->user_pages[i]);
-   }
+   put_user_pages(acd->user_pages, acd->page_count);
  err_get_user_pages:
kfree(acd->user_pages);
  err_alloc_userpages:
@@ -229,9 +227,7 @@ void  transfer_complete_cb(struct aio_cb_data *acd, size_t 
xfr_count, u32 flags)

dma_unmap_sg(>ldev->pldev->dev, acd->sgt.sgl, acd->sgt.nents, 
acd->ldev->dir);

-   for (i = 0 ; i < acd->page_count ; i++){
-   put_page(acd->user_pages[i]);
-   }
+   put_user_pages(acd->user_pages, acd->page_count);

sg_free_table(>sgt);

-- 
1.8.3.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] mm/gup: Use put_user_page*() instead of put_page*()

[Bharath Vedartham]

On Mon, Jul 15, 2019 at 11:10:20AM -0700, John Hubbard wrote:
> On 7/14/19 11:56 PM, Bharath Vedartham wrote:
> > On Sun, Jul 14, 2019 at 04:33:42PM -0700, John Hubbard wrote:
> >> On 7/14/19 12:08 PM, Bharath Vedartham wrote:
> [...]
> >> 1. Pull down https://github.com/johnhubbard/linux/commits/gup_dma_core
> >> and find missing conversions: look for any additional missing 
> >> get_user_pages/put_page conversions. You've already found a couple missing 
> >> ones. I haven't re-run a search in a long time, so there's probably even 
> >> more.
> >>a) And find more, after I rebase to 5.3-rc1: people probably are adding
> >>get_user_pages() calls as we speak. :)
> > Shouldn't this be documented then? I don't see any docs for using
> > put_user_page*() in v5.2.1 in the memory management API section?
> 
> Yes, it needs documentation. My first try (which is still in the above git
> repo) was reviewed and found badly wanting, so I'm going to rewrite it. 
> Meanwhile,
> I agree that an interim note would be helpful, let me put something together.
> 
> [...]
> >> https://github.com/johnhubbard/linux/commits/gup_dma_core
> >>
> >> a) gets rebased often, and
> >>
> >> b) has a bunch of commits (iov_iter and related) that conflict
> >>with the latest linux.git,
> >>
> >> c) has some bugs in the bio area, that I'm fixing, so I don't trust
> >>that's it's safely runnable, for a few more days.
> > I assume your repo contains only work related to fixing gup issues and
> > not the main repo for gup development? i.e where gup changes are merged?
> 
> Correct, this is just a private tree, not a maintainer tree. But I'll try to
> keep the gup_dma_core branch something that is usable by others, during the
> transition over to put_user_page(), because the page-tracking patches are the
> main way to test any put_user_page() conversions.
> 
> As Ira said, we're using linux-mm as the real (maintainer) tree.
Thanks for the info! 
> 
> thanks,
> -- 
> John Hubbard
> NVIDIA
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] mm/gup: Use put_user_page*() instead of put_page*()

[Bharath Vedartham]

On Mon, Jul 15, 2019 at 09:29:53AM -0700, Ira Weiny wrote:
> On Mon, Jul 15, 2019 at 12:26:54PM +0530, Bharath Vedartham wrote:
> > On Sun, Jul 14, 2019 at 04:33:42PM -0700, John Hubbard wrote:
> > > On 7/14/19 12:08 PM, Bharath Vedartham wrote:
> > > > This patch converts all call sites of get_user_pages
> > > > to use put_user_page*() instead of put_page*() functions to
> > > > release reference to gup pinned pages.
> > Hi John, 
> > > Hi Bharath,
> > > 
> > > Thanks for jumping in to help, and welcome to the party!
> > > 
> > > You've caught everyone in the middle of a merge window, btw.  As a
> > > result, I'm busy rebasing and reworking the get_user_pages call sites, 
> > > and gup tracking, in the wake of some semi-traumatic changes to bio 
> > > and gup and such. I plan to re-post right after 5.3-rc1 shows up, from 
> > > here:
> > > 
> > > https://github.com/johnhubbard/linux/commits/gup_dma_core
> > > 
> > > ...which you'll find already covers the changes you've posted, except for:
> > > 
> > > drivers/misc/sgi-gru/grufault.c
> > > drivers/staging/kpc2000/kpc_dma/fileops.c
> > > 
> > > ...and this one, which is undergoing to larger local changes, due to
> > > bvec, so let's leave it out of the choices:
> > > 
> > > fs/io_uring.c
> > > 
> > > Therefore, until -rc1, if you'd like to help, I'd recommend one or more
> > > of the following ideas:
> > > 
> > > 1. Pull down https://github.com/johnhubbard/linux/commits/gup_dma_core
> > > and find missing conversions: look for any additional missing 
> > > get_user_pages/put_page conversions. You've already found a couple 
> > > missing 
> > > ones. I haven't re-run a search in a long time, so there's probably even 
> > > more.
> > >   a) And find more, after I rebase to 5.3-rc1: people probably are adding
> > >   get_user_pages() calls as we speak. :)
> > Shouldn't this be documented then? I don't see any docs for using
> > put_user_page*() in v5.2.1 in the memory management API section?
> > > 2. Patches: Focus on just one subsystem at a time, and perfect the patch 
> > > for
> > > it. For example, I think this the staging driver would be perfect to 
> > > start with:
> > > 
> > > drivers/staging/kpc2000/kpc_dma/fileops.c
> > > 
> > >   a) verify that you've really, corrected converted the whole
> > >   driver. (Hint: I think you might be overlooking a put_page call.)
> > Yup. I did see that! Will fix it!
> > >   b) Attempt to test it if you can (I'm being hypocritical in
> > >   the extreme here, but one of my problems is that testing
> > >   has been light, so any help is very valuable). qemu...?
> > >   OTOH, maybe even qemu cannot easily test a kpc2000, but
> > >   perhaps `git blame` and talking to the authors would help
> > >   figure out a way to validate the changes.
> > Great! I ll do that, I ll mail the patch authors and ask them for help
> > in testing. 
> > >   Thinking about whether you can run a test that would prove or
> > >   disprove my claim in (a), above, could be useful in coming up
> > >   with tests to run.
> > 
> > > In other words, a few very high quality conversions (even just one) that
> > > we can really put our faith in, is what I value most here. Tested patches
> > > are awesome.
> > I understand that! 
> > > 3. Once I re-post, turn on the new CONFIG_DEBUG_GET_USER_PAGES_REFERENCES
> > > and run things such as xfstest/fstest. (Again, doing so would be going
> > > further than I have yet--very helpful). Help clarify what conversions have
> > > actually been tested and work, and which ones remain unvalidated.
> > > Other: Please note that this:
> > Yup will do that.
> > > https://github.com/johnhubbard/linux/commits/gup_dma_core
> > > 
> > > a) gets rebased often, and
> > > 
> > > b) has a bunch of commits (iov_iter and related) that conflict
> > >with the latest linux.git,
> > > 
> > > c) has some bugs in the bio area, that I'm fixing, so I don't trust
> > >that's it's safely runnable, for a few more days.
> > I assume your repo contains only work related to fixing gup issues and
> > not the main repo for gup development? i.e where gup changes are merged?
> 
> We have been using Andrews tree for merging.
> 
> > Also are release_pages a

Re: [PATCH] mm/gup: Use put_user_page*() instead of put_page*()

[Bharath Vedartham]

On Sun, Jul 14, 2019 at 08:33:57PM -0600, Jens Axboe wrote:
> On 7/14/19 1:08 PM, Bharath Vedartham wrote:
> > diff --git a/fs/io_uring.c b/fs/io_uring.c
> > index 4ef62a4..b4a4549 100644
> > --- a/fs/io_uring.c
> > +++ b/fs/io_uring.c
> > @@ -2694,10 +2694,9 @@ static int io_sqe_buffer_register(struct io_ring_ctx 
> > *ctx, void __user *arg,
> >  * if we did partial map, or found file backed vmas,
> >  * release any pages we did get
> >  */
> > -   if (pret > 0) {
> > -   for (j = 0; j < pret; j++)
> > -   put_page(pages[j]);
> > -   }
> > +   if (pret > 0)
> > +   put_user_pages(pages, pret);
> > +
> > if (ctx->account_mem)
> > io_unaccount_mem(ctx->user, nr_pages);
> > kvfree(imu->bvec);
> 
> You handled just the failure case of the buffer registration, but not
> the actual free in io_sqe_buffer_unregister().
> 
> -- 
> Jens Axboe
Yup got it! Thanks! I won't be sending a patch again as fs/io_uring.c
may have larger local changes for put_user_pages.

Thanks
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] mm/gup: Use put_user_page*() instead of put_page*()

[Bharath Vedartham]

On Sun, Jul 14, 2019 at 04:33:42PM -0700, John Hubbard wrote:
> On 7/14/19 12:08 PM, Bharath Vedartham wrote:
> > This patch converts all call sites of get_user_pages
> > to use put_user_page*() instead of put_page*() functions to
> > release reference to gup pinned pages.
Hi John, 
> Hi Bharath,
> 
> Thanks for jumping in to help, and welcome to the party!
> 
> You've caught everyone in the middle of a merge window, btw.  As a
> result, I'm busy rebasing and reworking the get_user_pages call sites, 
> and gup tracking, in the wake of some semi-traumatic changes to bio 
> and gup and such. I plan to re-post right after 5.3-rc1 shows up, from 
> here:
> 
> https://github.com/johnhubbard/linux/commits/gup_dma_core
> 
> ...which you'll find already covers the changes you've posted, except for:
> 
> drivers/misc/sgi-gru/grufault.c
> drivers/staging/kpc2000/kpc_dma/fileops.c
> 
> ...and this one, which is undergoing to larger local changes, due to
> bvec, so let's leave it out of the choices:
> 
> fs/io_uring.c
> 
> Therefore, until -rc1, if you'd like to help, I'd recommend one or more
> of the following ideas:
> 
> 1. Pull down https://github.com/johnhubbard/linux/commits/gup_dma_core
> and find missing conversions: look for any additional missing 
> get_user_pages/put_page conversions. You've already found a couple missing 
> ones. I haven't re-run a search in a long time, so there's probably even more.
>   a) And find more, after I rebase to 5.3-rc1: people probably are adding
>   get_user_pages() calls as we speak. :)
Shouldn't this be documented then? I don't see any docs for using
put_user_page*() in v5.2.1 in the memory management API section?
> 2. Patches: Focus on just one subsystem at a time, and perfect the patch for
> it. For example, I think this the staging driver would be perfect to start 
> with:
> 
> drivers/staging/kpc2000/kpc_dma/fileops.c
> 
>   a) verify that you've really, corrected converted the whole
>   driver. (Hint: I think you might be overlooking a put_page call.)
Yup. I did see that! Will fix it!
>   b) Attempt to test it if you can (I'm being hypocritical in
>   the extreme here, but one of my problems is that testing
>   has been light, so any help is very valuable). qemu...?
>   OTOH, maybe even qemu cannot easily test a kpc2000, but
>   perhaps `git blame` and talking to the authors would help
>   figure out a way to validate the changes.
Great! I ll do that, I ll mail the patch authors and ask them for help
in testing. 
>   Thinking about whether you can run a test that would prove or
>   disprove my claim in (a), above, could be useful in coming up
>   with tests to run.

> In other words, a few very high quality conversions (even just one) that
> we can really put our faith in, is what I value most here. Tested patches
> are awesome.
I understand that! 
> 3. Once I re-post, turn on the new CONFIG_DEBUG_GET_USER_PAGES_REFERENCES
> and run things such as xfstest/fstest. (Again, doing so would be going
> further than I have yet--very helpful). Help clarify what conversions have
> actually been tested and work, and which ones remain unvalidated.
> Other: Please note that this:
Yup will do that.
> https://github.com/johnhubbard/linux/commits/gup_dma_core
> 
> a) gets rebased often, and
> 
> b) has a bunch of commits (iov_iter and related) that conflict
>with the latest linux.git,
> 
> c) has some bugs in the bio area, that I'm fixing, so I don't trust
>that's it's safely runnable, for a few more days.
I assume your repo contains only work related to fixing gup issues and
not the main repo for gup development? i.e where gup changes are merged?
Also are release_pages and put_user_pages interchangable? 
> One note below, for the future:
> 
> > 
> > This is a bunch of trivial conversions which is a part of an effort
> > by John Hubbard to solve issues with gup pinned pages and 
> > filesystem writeback.
> > 
> > The issue is more clearly described in John Hubbard's patch[1] where
> > put_user_page*() functions are introduced.
> > 
> > Currently put_user_page*() simply does put_page but future implementations
> > look to change that once treewide change of put_page callsites to 
> > put_user_page*() is finished.
> > 
> > The lwn article describing the issue with gup pinned pages and filesystem 
> > writeback [2].
> > 
> > This patch has been tested by building and booting the kernel as I don't
> > have the required hardware to test the device drivers.
> > 
> > I did not modify gpu/drm drivers which use release_pages instead of
> > put_page() to

[PATCH] mm/gup: Use put_user_page*() instead of put_page*()

[Bharath Vedartham]

This patch converts all call sites of get_user_pages
to use put_user_page*() instead of put_page*() functions to
release reference to gup pinned pages.

This is a bunch of trivial conversions which is a part of an effort
by John Hubbard to solve issues with gup pinned pages and 
filesystem writeback.

The issue is more clearly described in John Hubbard's patch[1] where
put_user_page*() functions are introduced.

Currently put_user_page*() simply does put_page but future implementations
look to change that once treewide change of put_page callsites to 
put_user_page*() is finished.

The lwn article describing the issue with gup pinned pages and filesystem 
writeback [2].

This patch has been tested by building and booting the kernel as I don't
have the required hardware to test the device drivers.

I did not modify gpu/drm drivers which use release_pages instead of
put_page() to release reference of gup pinned pages as I am not clear
whether release_pages and put_page are interchangable. 

[1] https://lkml.org/lkml/2019/3/26/1396

[2] https://lwn.net/Articles/784574/

Signed-off-by: Bharath Vedartham 
---
 drivers/media/v4l2-core/videobuf-dma-sg.c | 3 +--
 drivers/misc/sgi-gru/grufault.c   | 2 +-
 drivers/staging/kpc2000/kpc_dma/fileops.c | 4 +---
 drivers/vfio/vfio_iommu_type1.c   | 2 +-
 fs/io_uring.c | 7 +++
 mm/gup_benchmark.c| 6 +-
 net/xdp/xdp_umem.c| 7 +--
 7 files changed, 9 insertions(+), 22 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf-dma-sg.c 
b/drivers/media/v4l2-core/videobuf-dma-sg.c
index 66a6c6c..d6eeb43 100644
--- a/drivers/media/v4l2-core/videobuf-dma-sg.c
+++ b/drivers/media/v4l2-core/videobuf-dma-sg.c
@@ -349,8 +349,7 @@ int videobuf_dma_free(struct videobuf_dmabuf *dma)
BUG_ON(dma->sglen);
 
if (dma->pages) {
-   for (i = 0; i < dma->nr_pages; i++)
-   put_page(dma->pages[i]);
+   put_user_pages(dma->pages, dma->nr_pages);
kfree(dma->pages);
dma->pages = NULL;
}
diff --git a/drivers/misc/sgi-gru/grufault.c b/drivers/misc/sgi-gru/grufault.c
index 4b713a8..61b3447 100644
--- a/drivers/misc/sgi-gru/grufault.c
+++ b/drivers/misc/sgi-gru/grufault.c
@@ -188,7 +188,7 @@ static int non_atomic_pte_lookup(struct vm_area_struct *vma,
if (get_user_pages(vaddr, 1, write ? FOLL_WRITE : 0, , NULL) <= 0)
return -EFAULT;
*paddr = page_to_phys(page);
-   put_page(page);
+   put_user_page(page);
return 0;
 }
 
diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c 
b/drivers/staging/kpc2000/kpc_dma/fileops.c
index 6166587..26dceed 100644
--- a/drivers/staging/kpc2000/kpc_dma/fileops.c
+++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
@@ -198,9 +198,7 @@ int  kpc_dma_transfer(struct dev_private_data *priv, struct 
kiocb *kcb, unsigned
sg_free_table(>sgt);
  err_dma_map_sg:
  err_alloc_sg_table:
-   for (i = 0 ; i < acd->page_count ; i++){
-   put_page(acd->user_pages[i]);
-   }
+   put_user_pages(acd->user_pages, acd->page_count);
  err_get_user_pages:
kfree(acd->user_pages);
  err_alloc_userpages:
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index add34ad..c491524 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -369,7 +369,7 @@ static int vaddr_get_pfn(struct mm_struct *mm, unsigned 
long vaddr,
 */
if (ret > 0 && vma_is_fsdax(vmas[0])) {
ret = -EOPNOTSUPP;
-   put_page(page[0]);
+   put_user_page(page[0]);
}
}
up_read(>mmap_sem);
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 4ef62a4..b4a4549 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -2694,10 +2694,9 @@ static int io_sqe_buffer_register(struct io_ring_ctx 
*ctx, void __user *arg,
 * if we did partial map, or found file backed vmas,
 * release any pages we did get
 */
-   if (pret > 0) {
-   for (j = 0; j < pret; j++)
-   put_page(pages[j]);
-   }
+   if (pret > 0)
+   put_user_pages(pages, pret);
+
if (ctx->account_mem)
io_unaccount_mem(ctx->user, nr_pages);
kvfree(imu->bvec);
diff --git a/mm/gup_benchmark.c b/mm/gup_benchmark.c
index 7dd602d..15fc7a2 100644
--- a/mm/gup_benchmark.c
+++ b/mm/gup_benchmark.c
@@ -76,11 +76,7 @@ static int __gup_benchmark_ioctl(unsigned int cmd,
gup->size = addr - gup->addr;
 
start_time = ktime_get();
-   for 

[RFC/PATCH] staging: olpc_dcon: Use WARN_ON instead of BUG_ON

[Bharath Vedartham]

This is with respect to a checkpatch.pl CHECK: "Avoid crashing the
kernel. Use WARN_ON instead of BUG_ON". But I maybe wrong here. Is a
kernel crash desired if olpc_board_at_least(olpc_board(0xc2)) fails,
will there be inconsistent results if execution continues?

Signed-off-by: Bharath Vedartham 
---
 drivers/staging/olpc_dcon/olpc_dcon.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/olpc_dcon/olpc_dcon.c 
b/drivers/staging/olpc_dcon/olpc_dcon.c
index 6b714f7..900baab7 100644
--- a/drivers/staging/olpc_dcon/olpc_dcon.c
+++ b/drivers/staging/olpc_dcon/olpc_dcon.c
@@ -138,7 +138,7 @@ static int dcon_bus_stabilize(struct dcon_priv *dcon, int 
is_powered_down)
}
if (x < 0) {
pr_err("unable to stabilize dcon's smbus, reasserting power and 
praying.\n");
-   BUG_ON(olpc_board_at_least(olpc_board(0xc2)));
+   WARN_ON(olpc_board_at_least(olpc_board(0xc2)));
pm = 0;
olpc_ec_cmd(EC_DCON_POWER_MODE, , 1, NULL, 0);
msleep(100);
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: ralink-gdma: Remove space after cast

[Bharath Vedartham]

This fixes the checkpatch.pl check: "No space is necessary after the
cast".

Signed-off-by: Bharath Vedartham 
---
 drivers/staging/ralink-gdma/ralink-gdma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/ralink-gdma/ralink-gdma.c 
b/drivers/staging/ralink-gdma/ralink-gdma.c
index dfdc425..04c551a 100644
--- a/drivers/staging/ralink-gdma/ralink-gdma.c
+++ b/drivers/staging/ralink-gdma/ralink-gdma.c
@@ -818,7 +818,7 @@ static int gdma_dma_probe(struct platform_device *pdev)
match = of_match_device(gdma_of_match_table, >dev);
if (!match)
return -EINVAL;
-   data = (struct gdma_data *) match->data;
+   data = (struct gdma_data *)match->data;
 
dma_dev = devm_kzalloc(>dev,
   struct_size(dma_dev, chan, data->chancnt),
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: ralink-gdma: Convert unsigned to unsigned int

[Bharath Vedartham]

On Sun, Mar 24, 2019 at 09:58:42AM +0100, Greg KH wrote:
> On Sat, Mar 23, 2019 at 05:04:44PM +0530, Bharath Vedartham wrote:
> > Fix the checkpatch.pl warning: "Use unsigned int instead of unsigned".
> > 
> > Signed-off-by: Bharath Vedartham 
> > ---
> >  drivers/staging/ralink-gdma/ralink-gdma.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> This patch also does not apply at all :(
I have sent version 2 based of the latest staging branch.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2] staging: ralink-gdma: Change unsigned to unsigned int

[Bharath Vedartham]

This fixes the checkpatch.pl warning: "Prefer unsigned to unsigned int"

Signed-off-by: Bharath Vedartham 
---
Changes since v1
- Based this patch of the staging branch and not linus's branch
  unlike the last patch.
---
 drivers/staging/ralink-gdma/ralink-gdma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/ralink-gdma/ralink-gdma.c 
b/drivers/staging/ralink-gdma/ralink-gdma.c
index 04c551a..680ce02 100644
--- a/drivers/staging/ralink-gdma/ralink-gdma.c
+++ b/drivers/staging/ralink-gdma/ralink-gdma.c
@@ -164,7 +164,7 @@ static inline uint32_t gdma_dma_read(struct gdma_dma_dev 
*dma_dev,
 }
 
 static inline void gdma_dma_write(struct gdma_dma_dev *dma_dev,
- unsigned reg, uint32_t val)
+ unsigned int reg, uint32_t val)
 {
writel(val, dma_dev->base + reg);
 }
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v4] staging: ralink-gdma: Use u32 over uint32_t

[Bharath Vedartham]

On Sun, Mar 24, 2019 at 09:58:33AM +0100, Greg KH wrote:
> On Sat, Mar 23, 2019 at 09:16:48PM +0530, Bharath Vedartham wrote:
> > Fixes the checkpatch.pl warning: "Prefer u32 over uint32_t"
> > 
> > Signed-off-by: Bharath Vedartham 
> > ---
> > Changes since v3
> > - Fixed allignment issues.
> > - Submitted a seperate patch to change unsigned
> > to unsigned int.
> > - One of the allignments crosses 80 characters in a line
> > but I feel that we may loose readability if we try to reduce
> > the characters in the line.
> > ---
> >  drivers/staging/ralink-gdma/ralink-gdma.c | 14 ++
> >  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> This patch does not apply to my staging-testing branch at all :(
Same case here :(. I have used linus's branch rather than the staging
branch. I have also noticed that this patch does not apply to the
staging branch as this checkpatch warning has already been  fixed in the
staging branch.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: ralink-gdma: Convert unsigned to unsigned int

[Bharath Vedartham]

On Sun, Mar 24, 2019 at 09:58:42AM +0100, Greg KH wrote:
> On Sat, Mar 23, 2019 at 05:04:44PM +0530, Bharath Vedartham wrote:
> > Fix the checkpatch.pl warning: "Use unsigned int instead of unsigned".
> > 
> > Signed-off-by: Bharath Vedartham 
> > ---
> >  drivers/staging/ralink-gdma/ralink-gdma.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> This patch also does not apply at all :(

I have been working on linus's branch and not the staging branch. I
thought that linus's branch would have the latest updates and would be
the latest branch :( . Is it a problem if I use linus's branch for
staging drivers?
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4] staging: ralink-gdma: Use u32 over uint32_t

[Bharath Vedartham]

Fixes the checkpatch.pl warning: "Prefer u32 over uint32_t"

Signed-off-by: Bharath Vedartham 
---
Changes since v3
- Fixed allignment issues.
- Submitted a seperate patch to change unsigned
to unsigned int.
- One of the allignments crosses 80 characters in a line
but I feel that we may loose readability if we try to reduce
the characters in the line.
---
 drivers/staging/ralink-gdma/ralink-gdma.c | 14 ++
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/ralink-gdma/ralink-gdma.c 
b/drivers/staging/ralink-gdma/ralink-gdma.c
index d78042e..0b831f8 100644
--- a/drivers/staging/ralink-gdma/ralink-gdma.c
+++ b/drivers/staging/ralink-gdma/ralink-gdma.c
@@ -157,14 +157,12 @@ static struct gdma_dma_desc *to_gdma_dma_desc(struct 
virt_dma_desc *vdesc)
return container_of(vdesc, struct gdma_dma_desc, vdesc);
 }
 
-static inline uint32_t gdma_dma_read(struct gdma_dma_dev *dma_dev,
-unsigned int reg)
+static inline u32 gdma_dma_read(struct gdma_dma_dev *dma_dev, unsigned int reg)
 {
return readl(dma_dev->base + reg);
 }
 
-static inline void gdma_dma_write(struct gdma_dma_dev *dma_dev,
- unsigned reg, uint32_t val)
+static inline void gdma_dma_write(struct gdma_dma_dev *dma_dev, unsigned reg, 
u32 val)
 {
writel(val, dma_dev->base + reg);
 }
@@ -283,7 +281,7 @@ static int rt305x_gdma_start_transfer(struct 
gdma_dmaengine_chan *chan)
struct gdma_dma_dev *dma_dev = gdma_dma_chan_get_dev(chan);
dma_addr_t src_addr, dst_addr;
struct gdma_dma_sg *sg;
-   uint32_t ctrl0, ctrl1;
+   u32 ctrl0, ctrl1;
 
/* verify chan is already stopped */
ctrl0 = gdma_dma_read(dma_dev, GDMA_REG_CTRL0(chan->id));
@@ -361,7 +359,7 @@ static int rt3883_gdma_start_transfer(struct 
gdma_dmaengine_chan *chan)
struct gdma_dma_dev *dma_dev = gdma_dma_chan_get_dev(chan);
dma_addr_t src_addr, dst_addr;
struct gdma_dma_sg *sg;
-   uint32_t ctrl0, ctrl1;
+   u32 ctrl0, ctrl1;
 
/* verify chan is already stopped */
ctrl0 = gdma_dma_read(dma_dev, GDMA_REG_CTRL0(chan->id));
@@ -753,7 +751,7 @@ static void gdma_dma_tasklet(unsigned long arg)
 
 static void rt305x_gdma_init(struct gdma_dma_dev *dma_dev)
 {
-   uint32_t gct;
+   u32 gct;
 
/* all chans round robin */
gdma_dma_write(dma_dev, GDMA_RT305X_GCT, GDMA_REG_GCT_ARBIT_RR);
@@ -767,7 +765,7 @@ static void rt305x_gdma_init(struct gdma_dma_dev *dma_dev)
 
 static void rt3883_gdma_init(struct gdma_dma_dev *dma_dev)
 {
-   uint32_t gct;
+   u32 gct;
 
/* all chans round robin */
gdma_dma_write(dma_dev, GDMA_REG_GCT, GDMA_REG_GCT_ARBIT_RR);
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: ralink-gdma: Convert unsigned to unsigned int

[Bharath Vedartham]

Fix the checkpatch.pl warning: "Use unsigned int instead of unsigned".

Signed-off-by: Bharath Vedartham 
---
 drivers/staging/ralink-gdma/ralink-gdma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/ralink-gdma/ralink-gdma.c 
b/drivers/staging/ralink-gdma/ralink-gdma.c
index 0b831f8..523c39e 100644
--- a/drivers/staging/ralink-gdma/ralink-gdma.c
+++ b/drivers/staging/ralink-gdma/ralink-gdma.c
@@ -162,7 +162,7 @@ static inline u32 gdma_dma_read(struct gdma_dma_dev 
*dma_dev, unsigned int reg)
return readl(dma_dev->base + reg);
 }
 
-static inline void gdma_dma_write(struct gdma_dma_dev *dma_dev, unsigned reg, 
u32 val)
+static inline void gdma_dma_write(struct gdma_dma_dev *dma_dev, unsigned int 
reg, u32 val)
 {
writel(val, dma_dev->base + reg);
 }
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3] staging: ralink-gdma: Use u32 over uint32_t

[Bharath Vedartham]

This fixes the checkpatch.pl warning: "Prefer u32 over uint32_t"

Signed-off-by: Bharath Vedartham 
---
Changes since v2
- Improved changelog
- Thanks for the good feedback. I am a beginner. I will learn
  and grow. :)
---
 drivers/staging/ralink-gdma/ralink-gdma.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/ralink-gdma/ralink-gdma.c 
b/drivers/staging/ralink-gdma/ralink-gdma.c
index d78042e..7e824d3 100644
--- a/drivers/staging/ralink-gdma/ralink-gdma.c
+++ b/drivers/staging/ralink-gdma/ralink-gdma.c
@@ -157,14 +157,14 @@ static struct gdma_dma_desc *to_gdma_dma_desc(struct 
virt_dma_desc *vdesc)
return container_of(vdesc, struct gdma_dma_desc, vdesc);
 }
 
-static inline uint32_t gdma_dma_read(struct gdma_dma_dev *dma_dev,
+static inline u32 gdma_dma_read(struct gdma_dma_dev *dma_dev,
 unsigned int reg)
 {
return readl(dma_dev->base + reg);
 }
 
 static inline void gdma_dma_write(struct gdma_dma_dev *dma_dev,
- unsigned reg, uint32_t val)
+ unsigned reg, u32 val)
 {
writel(val, dma_dev->base + reg);
 }
@@ -283,7 +283,7 @@ static int rt305x_gdma_start_transfer(struct 
gdma_dmaengine_chan *chan)
struct gdma_dma_dev *dma_dev = gdma_dma_chan_get_dev(chan);
dma_addr_t src_addr, dst_addr;
struct gdma_dma_sg *sg;
-   uint32_t ctrl0, ctrl1;
+   u32 ctrl0, ctrl1;
 
/* verify chan is already stopped */
ctrl0 = gdma_dma_read(dma_dev, GDMA_REG_CTRL0(chan->id));
@@ -361,7 +361,7 @@ static int rt3883_gdma_start_transfer(struct 
gdma_dmaengine_chan *chan)
struct gdma_dma_dev *dma_dev = gdma_dma_chan_get_dev(chan);
dma_addr_t src_addr, dst_addr;
struct gdma_dma_sg *sg;
-   uint32_t ctrl0, ctrl1;
+   u32 ctrl0, ctrl1;
 
/* verify chan is already stopped */
ctrl0 = gdma_dma_read(dma_dev, GDMA_REG_CTRL0(chan->id));
@@ -753,7 +753,7 @@ static void gdma_dma_tasklet(unsigned long arg)
 
 static void rt305x_gdma_init(struct gdma_dma_dev *dma_dev)
 {
-   uint32_t gct;
+   u32 gct;
 
/* all chans round robin */
gdma_dma_write(dma_dev, GDMA_RT305X_GCT, GDMA_REG_GCT_ARBIT_RR);
@@ -767,7 +767,7 @@ static void rt305x_gdma_init(struct gdma_dma_dev *dma_dev)
 
 static void rt3883_gdma_init(struct gdma_dma_dev *dma_dev)
 {
-   uint32_t gct;
+   u32 gct;
 
/* all chans round robin */
gdma_dma_write(dma_dev, GDMA_REG_GCT, GDMA_REG_GCT_ARBIT_RR);
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2] staging: ralink-gdma: Change uint32_t to u32

[Bharath Vedartham]

Change occurences of uint32_t to u32 to fix checkpatch.pl warnings.

Signed-off-by: Bharath Vedartham 
---
Changes since v1:
- Improved changelog.
---
---
 drivers/staging/ralink-gdma/ralink-gdma.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/ralink-gdma/ralink-gdma.c 
b/drivers/staging/ralink-gdma/ralink-gdma.c
index 2c19287..7e824d3 100644
--- a/drivers/staging/ralink-gdma/ralink-gdma.c
+++ b/drivers/staging/ralink-gdma/ralink-gdma.c
@@ -157,14 +157,14 @@ static struct gdma_dma_desc *to_gdma_dma_desc(struct 
virt_dma_desc *vdesc)
return container_of(vdesc, struct gdma_dma_desc, vdesc);
 }
 
-static inline uint32_t gdma_dma_read(struct gdma_dma_dev *dma_dev,
+static inline u32 gdma_dma_read(struct gdma_dma_dev *dma_dev,
 unsigned int reg)
 {
return readl(dma_dev->base + reg);
 }
 
 static inline void gdma_dma_write(struct gdma_dma_dev *dma_dev,
- unsigned reg, uint32_t val)
+ unsigned reg, u32 val)
 {
writel(val, dma_dev->base + reg);
 }
@@ -283,7 +283,7 @@ static int rt305x_gdma_start_transfer(struct 
gdma_dmaengine_chan *chan)
struct gdma_dma_dev *dma_dev = gdma_dma_chan_get_dev(chan);
dma_addr_t src_addr, dst_addr;
struct gdma_dma_sg *sg;
-   uint32_t ctrl0, ctrl1;
+   u32 ctrl0, ctrl1;
 
/* verify chan is already stopped */
ctrl0 = gdma_dma_read(dma_dev, GDMA_REG_CTRL0(chan->id));
@@ -753,7 +753,7 @@ static void gdma_dma_tasklet(unsigned long arg)
 
 static void rt305x_gdma_init(struct gdma_dma_dev *dma_dev)
 {
-   uint32_t gct;
+   u32 gct;
 
/* all chans round robin */
gdma_dma_write(dma_dev, GDMA_RT305X_GCT, GDMA_REG_GCT_ARBIT_RR);
@@ -767,7 +767,7 @@ static void rt305x_gdma_init(struct gdma_dma_dev *dma_dev)
 
 static void rt3883_gdma_init(struct gdma_dma_dev *dma_dev)
 {
-   uint32_t gct;
+   u32 gct;
 
/* all chans round robin */
gdma_dma_write(dma_dev, GDMA_REG_GCT, GDMA_REG_GCT_ARBIT_RR);
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2] staging: ralink-gdma: Change uint32_t to u32

[Bharath Vedartham]

Change occurences of uint32_t to u32 to fix checkpatch.pl warning

Signed-off-by: Bharath Vedartham 
---
Changes since v1:
-Improved the changelog.
---
 drivers/staging/ralink-gdma/ralink-gdma.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/ralink-gdma/ralink-gdma.c 
b/drivers/staging/ralink-gdma/ralink-gdma.c
index 2c19287..7e824d3 100644
--- a/drivers/staging/ralink-gdma/ralink-gdma.c
+++ b/drivers/staging/ralink-gdma/ralink-gdma.c
@@ -157,14 +157,14 @@ static struct gdma_dma_desc *to_gdma_dma_desc(struct 
virt_dma_desc *vdesc)
return container_of(vdesc, struct gdma_dma_desc, vdesc);
 }
 
-static inline uint32_t gdma_dma_read(struct gdma_dma_dev *dma_dev,
+static inline u32 gdma_dma_read(struct gdma_dma_dev *dma_dev,
 unsigned int reg)
 {
return readl(dma_dev->base + reg);
 }
 
 static inline void gdma_dma_write(struct gdma_dma_dev *dma_dev,
- unsigned reg, uint32_t val)
+ unsigned reg, u32 val)
 {
writel(val, dma_dev->base + reg);
 }
@@ -283,7 +283,7 @@ static int rt305x_gdma_start_transfer(struct 
gdma_dmaengine_chan *chan)
struct gdma_dma_dev *dma_dev = gdma_dma_chan_get_dev(chan);
dma_addr_t src_addr, dst_addr;
struct gdma_dma_sg *sg;
-   uint32_t ctrl0, ctrl1;
+   u32 ctrl0, ctrl1;
 
/* verify chan is already stopped */
ctrl0 = gdma_dma_read(dma_dev, GDMA_REG_CTRL0(chan->id));
@@ -753,7 +753,7 @@ static void gdma_dma_tasklet(unsigned long arg)
 
 static void rt305x_gdma_init(struct gdma_dma_dev *dma_dev)
 {
-   uint32_t gct;
+   u32 gct;
 
/* all chans round robin */
gdma_dma_write(dma_dev, GDMA_RT305X_GCT, GDMA_REG_GCT_ARBIT_RR);
@@ -767,7 +767,7 @@ static void rt305x_gdma_init(struct gdma_dma_dev *dma_dev)
 
 static void rt3883_gdma_init(struct gdma_dma_dev *dma_dev)
 {
-   uint32_t gct;
+   u32 gct;
 
/* all chans round robin */
gdma_dma_write(dma_dev, GDMA_REG_GCT, GDMA_REG_GCT_ARBIT_RR);
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: ralink-gdma: Change uint32_t to u32

[Bharath Vedartham]

On Fri, Mar 22, 2019 at 01:01:40PM +0300, Dan Carpenter wrote:
> On Fri, Mar 22, 2019 at 12:55:02AM +0530, Bharath Vedartham wrote:
> > This is a follow up on my previous patch. Change occurences of the
>   ^
I did not send a patch set. I should have worded my changelog better.
Thanks for the feedback. This was just one patch.
> Put this sort of information under the --- cut off below the Sign off.
> 
> > stdint type uint32_t to its shortened type u32. This fixed the 
> > checkpatch.pl warning: "Prefer u32 over uint32_t".
> 
> This line is too long.  Limit it to 72 characters.
> 
Yes will do that.
> > 
> > Signed-off-by: Bharath Vedartham 
> > ---
>   ^^^
Got it. Thanks for the feedback.
> 
> regards,
> dan carpenter
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: ralink-gdma: Change uint32_t to u32

[Bharath Vedartham]

This is a follow up on my previous patch. Change occurences of the
stdint type uint32_t to its shortened type u32. This fixed the checkpatch.pl 
warning: "Prefer u32 over uint32_t".

Signed-off-by: Bharath Vedartham 
---
 drivers/staging/ralink-gdma/ralink-gdma.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/ralink-gdma/ralink-gdma.c 
b/drivers/staging/ralink-gdma/ralink-gdma.c
index 2c19287..7e824d3 100644
--- a/drivers/staging/ralink-gdma/ralink-gdma.c
+++ b/drivers/staging/ralink-gdma/ralink-gdma.c
@@ -157,14 +157,14 @@ static struct gdma_dma_desc *to_gdma_dma_desc(struct 
virt_dma_desc *vdesc)
return container_of(vdesc, struct gdma_dma_desc, vdesc);
 }
 
-static inline uint32_t gdma_dma_read(struct gdma_dma_dev *dma_dev,
+static inline u32 gdma_dma_read(struct gdma_dma_dev *dma_dev,
 unsigned int reg)
 {
return readl(dma_dev->base + reg);
 }
 
 static inline void gdma_dma_write(struct gdma_dma_dev *dma_dev,
- unsigned reg, uint32_t val)
+ unsigned reg, u32 val)
 {
writel(val, dma_dev->base + reg);
 }
@@ -283,7 +283,7 @@ static int rt305x_gdma_start_transfer(struct 
gdma_dmaengine_chan *chan)
struct gdma_dma_dev *dma_dev = gdma_dma_chan_get_dev(chan);
dma_addr_t src_addr, dst_addr;
struct gdma_dma_sg *sg;
-   uint32_t ctrl0, ctrl1;
+   u32 ctrl0, ctrl1;
 
/* verify chan is already stopped */
ctrl0 = gdma_dma_read(dma_dev, GDMA_REG_CTRL0(chan->id));
@@ -753,7 +753,7 @@ static void gdma_dma_tasklet(unsigned long arg)
 
 static void rt305x_gdma_init(struct gdma_dma_dev *dma_dev)
 {
-   uint32_t gct;
+   u32 gct;
 
/* all chans round robin */
gdma_dma_write(dma_dev, GDMA_RT305X_GCT, GDMA_REG_GCT_ARBIT_RR);
@@ -767,7 +767,7 @@ static void rt305x_gdma_init(struct gdma_dma_dev *dma_dev)
 
 static void rt3883_gdma_init(struct gdma_dma_dev *dma_dev)
 {
-   uint32_t gct;
+   u32 gct;
 
/* all chans round robin */
gdma_dma_write(dma_dev, GDMA_REG_GCT, GDMA_REG_GCT_ARBIT_RR);
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2] staging: ralink-gdma: Prefer u32 over uint32_t

[Bharath Vedartham]

Fix the checkpatch.pl warning: "Prefer u32 over uint32_t".

Signed-off-by: Bharath Vedartham 
---
 drivers/staging/ralink-gdma/ralink-gdma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/ralink-gdma/ralink-gdma.c 
b/drivers/staging/ralink-gdma/ralink-gdma.c
index d78042e..2c19287 100644
--- a/drivers/staging/ralink-gdma/ralink-gdma.c
+++ b/drivers/staging/ralink-gdma/ralink-gdma.c
@@ -361,7 +361,7 @@ static int rt3883_gdma_start_transfer(struct 
gdma_dmaengine_chan *chan)
struct gdma_dma_dev *dma_dev = gdma_dma_chan_get_dev(chan);
dma_addr_t src_addr, dst_addr;
struct gdma_dma_sg *sg;
-   uint32_t ctrl0, ctrl1;
+   u32 ctrl0, ctrl1;
 
/* verify chan is already stopped */
ctrl0 = gdma_dma_read(dma_dev, GDMA_REG_CTRL0(chan->id));
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: ralink-gdma: Prefer u32 over uint32_t

[Bharath Vedartham]

On Thu, Mar 21, 2019 at 07:46:09PM +0100, Greg KH wrote:
> On Fri, Mar 22, 2019 at 12:07:10AM +0530, Bharath Vedartham wrote:
> > Change uint32_t to u32
> 
> That says _what_ you did, but _why_ are you doing this?  That's the main
> content a changelog text should have in it.
> 
> thanks,
> 
> greg k-h
That was a checkpatch.pl warning! I forgot to mention that! It fixed a
checkpatch.pl warning. I ll write better changelogs from my next commit
on. Thanks for the feedback!


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: ralink-gdma: Prefer u32 over uint32_t

[Bharath Vedartham]

Change uint32_t to u32

Signed-off-by: Bharath Vedartham 
---
 drivers/staging/ralink-gdma/ralink-gdma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/ralink-gdma/ralink-gdma.c 
b/drivers/staging/ralink-gdma/ralink-gdma.c
index d78042e..2c19287 100644
--- a/drivers/staging/ralink-gdma/ralink-gdma.c
+++ b/drivers/staging/ralink-gdma/ralink-gdma.c
@@ -361,7 +361,7 @@ static int rt3883_gdma_start_transfer(struct 
gdma_dmaengine_chan *chan)
struct gdma_dma_dev *dma_dev = gdma_dma_chan_get_dev(chan);
dma_addr_t src_addr, dst_addr;
struct gdma_dma_sg *sg;
-   uint32_t ctrl0, ctrl1;
+   u32 ctrl0, ctrl1;
 
/* verify chan is already stopped */
ctrl0 = gdma_dma_read(dma_dev, GDMA_REG_CTRL0(chan->id));
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3] staging: erofs: Add whitespace after declaration

[Bharath Vedartham]

On Mon, Mar 18, 2019 at 11:40:35PM +0800, Gao Xiang wrote:
> 
> 
> On 2019/3/18 23:38, Greg KH wrote:
> > On Mon, Mar 18, 2019 at 09:02:24PM +0530, Bharath Vedartham wrote:
> >> Add whitespace after declaration. Fixed the checkpatch.pl warning.
> >>
> >> ---
> > 
> > No signed-off-by?
> > 
> >> changes since v2
> >>- mutt gave me a lot of problems. Hence 3 patches for a really
> >>  small problem.
> >>- Fixed the extra missing line.
> >>
> >> Signed-off-by: Bharath Vedartham 
> > 
> > It needs to be above the --- line, otherwise it will be stripped off
> > when applied.
> > 
> > v4 please?
Missed that! Really sorry!
>
> Actually a duplicate commit, see:
> https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/commit/?h=staging-next=8af36478f9516d51fcfedf9b9ed8905a1c00d278
> 
> several people submit patches on this line in erofs_iget()
> 
Oh alright then! I ll stop this patch here! Thank for your time! I have
noticed that there are a few functions and structs local to a file in erofs 
without static
declaration. I ll send a patch soon.
> Thanks,
> Gao Xiang
> 
> > 
> > thanks,
> > 
> > greg k-h
> > 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v3] staging: erofs: Add whitespace after declaration

[Bharath Vedartham]

Add whitespace after declaration. Fixed the checkpatch.pl warning.

---
changes since v2
- mutt gave me a lot of problems. Hence 3 patches for a really
  small problem.
- Fixed the extra missing line.

Signed-off-by: Bharath Vedartham 
---
 drivers/staging/erofs/inode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/erofs/inode.c b/drivers/staging/erofs/inode.c
index 924b8df..e09d29f 100644
--- a/drivers/staging/erofs/inode.c
+++ b/drivers/staging/erofs/inode.c
@@ -270,8 +270,8 @@ struct inode *erofs_iget(struct super_block *sb,
if (inode->i_state & I_NEW) {
int err;
struct erofs_vnode *vi = EROFS_V(inode);
-   vi->nid = nid;
 
+   vi->nid = nid;
err = fill_inode(inode, isdir);
if (likely(!err))
unlock_new_inode(inode);
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2] staging: erofs: Add whitespace after declaration

[Bharath Vedartham]

Fix the checkpatch.pl warning to add a whitespace after declaration
statements.

Signed-off-by: Bharath Vedartham 
---
Changes since v2
- Mutt was giving me a few problems. I apologise for the
  previous poor patches.
---
 drivers/staging/erofs/inode.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/erofs/inode.c b/drivers/staging/erofs/inode.c
index 924b8df..9a36a1f 100644
--- a/drivers/staging/erofs/inode.c
+++ b/drivers/staging/erofs/inode.c
@@ -270,8 +270,8 @@ struct inode *erofs_iget(struct super_block *sb,
if (inode->i_state & I_NEW) {
int err;
struct erofs_vnode *vi = EROFS_V(inode);
-   vi->nid = nid;
 
+   vi->nid = nid;
err = fill_inode(inode, isdir);
if (likely(!err))
unlock_new_inode(inode);
@@ -305,4 +305,3 @@ const struct inode_operations erofs_fast_symlink_iops = {
 #endif
.get_acl = erofs_get_acl,
 };
-
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: erofs: Add whitespace after declaration

[Bharath Vedartham]

linux-ker...@vger.kernel.org 
Bcc: 
Subject: [PATCH] staging: erofs: Add whitespace after declaration
Reply-To: 

Fix the checkpatch.pl warning to add a whitespace after declaration
statements.

Signed-off-by: Bharath Vedartham 
---
 drivers/staging/erofs/inode.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/erofs/inode.c b/drivers/staging/erofs/inode.c
index 924b8df..9a36a1f 100644
--- a/drivers/staging/erofs/inode.c
+++ b/drivers/staging/erofs/inode.c
@@ -270,8 +270,8 @@ struct inode *erofs_iget(struct super_block *sb,
if (inode->i_state & I_NEW) {
int err;
struct erofs_vnode *vi = EROFS_V(inode);
-   vi->nid = nid;
 
+   vi->nid = nid;
err = fill_inode(inode, isdir);
if (likely(!err))
unlock_new_inode(inode);
@@ -305,4 +305,3 @@ const struct inode_operations erofs_fast_symlink_iops = {
 #endif
.get_acl = erofs_get_acl,
 };
-
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[no subject]

[Bharath Vedartham]

linux-ker...@vger.kernel.org 
Bcc: 
Subject: [PATCH] staging: erofs: Add whitespace after declaration
Reply-To: 

Fix the checkpatch.pl warning to add a whitespace after declaration
statements.

Signed-off-by: Bharath Vedartham 
---
 drivers/staging/erofs/inode.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/erofs/inode.c b/drivers/staging/erofs/inode.c
index 924b8df..9a36a1f 100644
--- a/drivers/staging/erofs/inode.c
+++ b/drivers/staging/erofs/inode.c
@@ -270,8 +270,8 @@ struct inode *erofs_iget(struct super_block *sb,
if (inode->i_state & I_NEW) {
int err;
struct erofs_vnode *vi = EROFS_V(inode);
-   vi->nid = nid;
 
+   vi->nid = nid;
err = fill_inode(inode, isdir);
if (likely(!err))
unlock_new_inode(inode);
@@ -305,4 +305,3 @@ const struct inode_operations erofs_fast_symlink_iops = {
 #endif
.get_acl = erofs_get_acl,
 };
-
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2] staging : speakup: Fix format issues in Kconfig

[Bharath Vedartham]

Fix the checkpatch.pl warning to replace ---help--- with help in
Kconfig.

Signed-off-by: Bharath Vedartham 
---
Changes since v1:
- Allign help and removed redundant spaces.
---
 drivers/staging/speakup/Kconfig | 42 -
 1 file changed, 16 insertions(+), 26 deletions(-)

diff --git a/drivers/staging/speakup/Kconfig b/drivers/staging/speakup/Kconfig
index efd6f45..4a78ab1 100644
--- a/drivers/staging/speakup/Kconfig
+++ b/drivers/staging/speakup/Kconfig
@@ -3,7 +3,7 @@ menu "Speakup console speech"
 config SPEAKUP
depends on VT
tristate "Speakup core"
-   ---help---
+   help
This is the Speakup screen reader.  Think of it as a
video console for blind people.  If built in to the
kernel, it can speak everything on the text console from
@@ -43,7 +43,7 @@ config SPEAKUP
 if SPEAKUP
 config SPEAKUP_SYNTH_ACNTSA
tristate "Accent SA synthesizer support"
-   ---help---
+   help
This is the Speakup driver for the Accent SA
synthesizer.  You can say y to build it into the kernel,
or m to build it as a module.  See the configuration
@@ -52,7 +52,7 @@ config SPEAKUP_SYNTH_ACNTSA
 config SPEAKUP_SYNTH_ACNTPC
tristate "Accent PC synthesizer support"
depends on ISA || COMPILE_TEST
-   ---help---
+   help
This is the Speakup driver for the accent pc
synthesizer.  You can say y to build it into the kernel,
or m to build it as a module.  See the configuration
@@ -60,7 +60,7 @@ config SPEAKUP_SYNTH_ACNTPC
 
 config SPEAKUP_SYNTH_APOLLO
tristate "Apollo II synthesizer support"
-   ---help---
+   help
This is the Speakup driver for the Apollo II
synthesizer.  You can say y to build it into the kernel,
or m to build it as a module.  See the configuration
@@ -68,7 +68,7 @@ config SPEAKUP_SYNTH_APOLLO
 
 config SPEAKUP_SYNTH_AUDPTR
tristate "Audapter synthesizer support"
-   ---help---
+   help
This is the Speakup driver for the Audapter synthesizer.
 You can say y to build it into the kernel, or m to
build it as a module.  See the configuration help on the
@@ -76,7 +76,7 @@ config SPEAKUP_SYNTH_AUDPTR
 
 config SPEAKUP_SYNTH_BNS
tristate "Braille 'n' Speak synthesizer support"
-   ---help---
+   help
This is the Speakup driver for the Braille 'n' Speak
synthesizer.  You can say y to build it into the kernel,
or m to build it as a module.  See the configuration
@@ -84,8 +84,7 @@ config SPEAKUP_SYNTH_BNS
 
 config SPEAKUP_SYNTH_DECTLK
tristate "DECtalk Express synthesizer support"
-   ---help---
-
+   help
This is the Speakup driver for the DecTalk Express
synthesizer.  You can say y to build it into the kernel,
or m to build it as a module.  See the configuration
@@ -93,8 +92,7 @@ config SPEAKUP_SYNTH_DECTLK
 
 config SPEAKUP_SYNTH_DECEXT
tristate "DECtalk External (old) synthesizer support"
-   ---help---
-
+   help
This is the Speakup driver for the DecTalk External
(old) synthesizer.  You can say y to build it into the
kernel, or m to build it as a module.  See the
@@ -105,8 +103,7 @@ config SPEAKUP_SYNTH_DECPC
depends on m
depends on ISA || COMPILE_TEST
tristate "DECtalk PC (big ISA card) synthesizer support"
-   ---help---
-
+   help
This is the Speakup driver for the DecTalk PC (full
length ISA) synthesizer.  You can say m to build it as
a module.  See the configuration help on the Speakup
@@ -127,8 +124,7 @@ config SPEAKUP_SYNTH_DECPC
 config SPEAKUP_SYNTH_DTLK
tristate "DoubleTalk PC synthesizer support"
depends on ISA || COMPILE_TEST
-   ---help---
-
+   help
This is the Speakup driver for the internal DoubleTalk
PC synthesizer.  You can say y to build it into the
kernel, or m to build it as a module.  See the
@@ -138,8 +134,7 @@ config SPEAKUP_SYNTH_DTLK
 config SPEAKUP_SYNTH_KEYPC
tristate "Keynote Gold PC synthesizer support"
depends on ISA || COMPILE_TEST
-   ---help---
-
+   help
This is the Speakup driver for the Keynote Gold
PC synthesizer.  You can say y to build it into the
kernel, or m to build it as a module.  See the
@@ -148,8 +143,7 @@ config SPEAKUP_SYNTH_KEYPC
 
 config SPEAKUP_SYNTH_LTLK
tristate "DoubleTalk LT/LiteTalk synthesizer support"
help---
-
+   h

[PATCH] staging : speakup: replace ---help--- with help in Kconfig

[Bharath Vedartham]

Fix the checkpatch.pl warning to replace ---help--- with help in
Kconfig.

Signed-off-by: Bharath Vedartham 
---
 drivers/staging/speakup/Kconfig | 32 
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/speakup/Kconfig b/drivers/staging/speakup/Kconfig
index efd6f45..d8ec780 100644
--- a/drivers/staging/speakup/Kconfig
+++ b/drivers/staging/speakup/Kconfig
@@ -3,7 +3,7 @@ menu "Speakup console speech"
 config SPEAKUP
depends on VT
tristate "Speakup core"
-   ---help---
+   help
This is the Speakup screen reader.  Think of it as a
video console for blind people.  If built in to the
kernel, it can speak everything on the text console from
@@ -43,7 +43,7 @@ config SPEAKUP
 if SPEAKUP
 config SPEAKUP_SYNTH_ACNTSA
tristate "Accent SA synthesizer support"
-   ---help---
+   help
This is the Speakup driver for the Accent SA
synthesizer.  You can say y to build it into the kernel,
or m to build it as a module.  See the configuration
@@ -52,7 +52,7 @@ config SPEAKUP_SYNTH_ACNTSA
 config SPEAKUP_SYNTH_ACNTPC
tristate "Accent PC synthesizer support"
depends on ISA || COMPILE_TEST
-   ---help---
+   help
This is the Speakup driver for the accent pc
synthesizer.  You can say y to build it into the kernel,
or m to build it as a module.  See the configuration
@@ -60,7 +60,7 @@ config SPEAKUP_SYNTH_ACNTPC
 
 config SPEAKUP_SYNTH_APOLLO
tristate "Apollo II synthesizer support"
-   ---help---
+   help
This is the Speakup driver for the Apollo II
synthesizer.  You can say y to build it into the kernel,
or m to build it as a module.  See the configuration
@@ -68,7 +68,7 @@ config SPEAKUP_SYNTH_APOLLO
 
 config SPEAKUP_SYNTH_AUDPTR
tristate "Audapter synthesizer support"
-   ---help---
+   help
This is the Speakup driver for the Audapter synthesizer.
 You can say y to build it into the kernel, or m to
build it as a module.  See the configuration help on the
@@ -76,7 +76,7 @@ config SPEAKUP_SYNTH_AUDPTR
 
 config SPEAKUP_SYNTH_BNS
tristate "Braille 'n' Speak synthesizer support"
-   ---help---
+   help
This is the Speakup driver for the Braille 'n' Speak
synthesizer.  You can say y to build it into the kernel,
or m to build it as a module.  See the configuration
@@ -84,7 +84,7 @@ config SPEAKUP_SYNTH_BNS
 
 config SPEAKUP_SYNTH_DECTLK
tristate "DECtalk Express synthesizer support"
-   ---help---
+   help
 
This is the Speakup driver for the DecTalk Express
synthesizer.  You can say y to build it into the kernel,
@@ -93,7 +93,7 @@ config SPEAKUP_SYNTH_DECTLK
 
 config SPEAKUP_SYNTH_DECEXT
tristate "DECtalk External (old) synthesizer support"
-   ---help---
+   help
 
This is the Speakup driver for the DecTalk External
(old) synthesizer.  You can say y to build it into the
@@ -105,7 +105,7 @@ config SPEAKUP_SYNTH_DECPC
depends on m
depends on ISA || COMPILE_TEST
tristate "DECtalk PC (big ISA card) synthesizer support"
-   ---help---
+   help
 
This is the Speakup driver for the DecTalk PC (full
length ISA) synthesizer.  You can say m to build it as
@@ -127,7 +127,7 @@ config SPEAKUP_SYNTH_DECPC
 config SPEAKUP_SYNTH_DTLK
tristate "DoubleTalk PC synthesizer support"
depends on ISA || COMPILE_TEST
-   ---help---
+   help
 
This is the Speakup driver for the internal DoubleTalk
PC synthesizer.  You can say y to build it into the
@@ -138,7 +138,7 @@ config SPEAKUP_SYNTH_DTLK
 config SPEAKUP_SYNTH_KEYPC
tristate "Keynote Gold PC synthesizer support"
depends on ISA || COMPILE_TEST
-   ---help---
+   help
 
This is the Speakup driver for the Keynote Gold
PC synthesizer.  You can say y to build it into the
@@ -148,7 +148,7 @@ config SPEAKUP_SYNTH_KEYPC
 
 config SPEAKUP_SYNTH_LTLK
tristate "DoubleTalk LT/LiteTalk synthesizer support"
help---
+help
 
This is the Speakup driver for the LiteTalk/DoubleTalk
LT synthesizer.  You can say y to build it into the
@@ -158,7 +158,7 @@ config SPEAKUP_SYNTH_LTLK
 
 config SPEAKUP_SYNTH_SOFT
tristate "Userspace software synthesizer support"
-   ---help---
+   help
 
This is the software synthesizer device node.  It will
register a device /dev/softsynth

Re: [PATCH] staging: speakup: Replace simple_strtoul with kstrtoul

[Bharath Vedartham]

On Sat, Feb 23, 2019 at 08:37:50PM +0100, Samuel Thibault wrote:
> Bharath Vedartham, le dim. 24 févr. 2019 01:01:21 +0530, a ecrit:
> > simple_strtoul is obsolete. Change it to kstrtoul. Kernel is building
> > and booting successfully.
> 
> Please recheck your patch, temp is used after the simple_strtoul call.
> 
> Samuel

Sorry about that. After some thought, I feel this is a special case in
replacing simple_strtoul to kstrtoul. simple_strtoul can give back a
pointer to the end of the parsed string, whereas kstrtoul does not
give any thing of that kind. In our case, temp is the end of the
parsed cp string. 

To replace simple_strtoul to kstrtoul, we need to know the length of the
unsigned long int returned and shift the cp pointer by that length to
get temp. This might involve a bit of code refactoring.

Bharath
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: speakup: Replace simple_strtoul with kstrtoul

[Bharath Vedartham]

simple_strtoul is obsolete. Change it to kstrtoul. Kernel is building
and booting successfully.

Signed-off-by: Bharath Vedartham 
---
 drivers/staging/speakup/kobjects.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/speakup/kobjects.c 
b/drivers/staging/speakup/kobjects.c
index 2e36d87..ad5a2b2 100644
--- a/drivers/staging/speakup/kobjects.c
+++ b/drivers/staging/speakup/kobjects.c
@@ -787,7 +787,9 @@ static ssize_t message_store_helper(const char *buf, size_t 
count,
continue;
}
 
-   index = simple_strtoul(cp, , 10);
+   retval = kstrtoul(cp, 0, );
+   if (retval)
+   return retval;
 
while ((temp < linefeed) && (*temp == ' ' || *temp == '\t'))
temp++;
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: wlan-ng: replace ---help--- with help in Kconfig

[Bharath Vedartham]

This patch fixes the checkpatch.pl warning:

WARNING: prefer 'help' over '---help---' for new help texts

Signed-off-by: Bharath Vedartham 
---
 drivers/staging/wlan-ng/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/wlan-ng/Kconfig b/drivers/staging/wlan-ng/Kconfig
index 426d4ef..9723801 100644
--- a/drivers/staging/wlan-ng/Kconfig
+++ b/drivers/staging/wlan-ng/Kconfig
@@ -4,7 +4,7 @@ config PRISM2_USB
select WIRELESS_EXT
select WEXT_PRIV
default n
-   ---help---
+   help
  This is the wlan-ng prism 2.5/3 USB driver for a wide range of
  old USB wireless devices.
 
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: netlogic: replace ---help--- with help in Kconfig

[Bharath Vedartham]

This patch fixes the checkpatch.pl warning:

WARNING: prefer 'help' over '---help---' for new help texts

Signed-off-by: Bharath Vedartham 
---
 drivers/staging/netlogic/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/netlogic/Kconfig b/drivers/staging/netlogic/Kconfig
index d660de5..c25a00d 100644
--- a/drivers/staging/netlogic/Kconfig
+++ b/drivers/staging/netlogic/Kconfig
@@ -2,6 +2,6 @@ config NETLOGIC_XLR_NET
tristate "Netlogic XLR/XLS network device"
depends on CPU_XLR
select PHYLIB
-   ---help---
+   help
This driver support Netlogic XLR/XLS on chip gigabit
Ethernet.
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: netlogic: replace ---help--- with help in Kconfig

[Bharath Vedartham]

This patch fixes the checkpatch.pl warning:

WARNING: prefer 'help' over '---help---' for new help texts

Signed-off-by: Bharath Vedartham 
---
 drivers/staging/netlogic/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/netlogic/Kconfig b/drivers/staging/netlogic/Kconfig
index d660de5..c25a00d 100644
--- a/drivers/staging/netlogic/Kconfig
+++ b/drivers/staging/netlogic/Kconfig
@@ -2,6 +2,6 @@ config NETLOGIC_XLR_NET
tristate "Netlogic XLR/XLS network device"
depends on CPU_XLR
select PHYLIB
-   ---help---
+   help
This driver support Netlogic XLR/XLS on chip gigabit
Ethernet.
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel