From: Souptick Joarder <jrdr.linux@gmail.com> To: John Hubbard <jhubbard@nvidia.com> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>, Juergen Gross <jgross@suse.com>, sstabellini@kernel.org, xen-devel@lists.xenproject.org, linux-kernel@vger.kernel.org, Paul Durrant <xadimgnik@gmail.com> Subject: Re: [PATCH 2/2] xen/privcmd: Convert get_user_pages*() to pin_user_pages*() Date: Fri, 26 Jun 2020 10:56:53 +0530 [thread overview] Message-ID: <CAFqt6zZdq_OMZ3EBDGC+Bn4uPBEhDGOYF=jB4B16z7rY6hpZ7g@mail.gmail.com> (raw) In-Reply-To: <59afe2fe-3718-85aa-f3b5-83ca0b9df577@nvidia.com> On Thu, Jun 25, 2020 at 11:19 AM John Hubbard <jhubbard@nvidia.com> wrote: > > On 2020-06-24 20:02, Souptick Joarder wrote: > > In 2019, we introduced pin_user_pages*() and now we are converting > > get_user_pages*() to the new API as appropriate. [1] & [2] could > > be referred for more information. This is case 5 as per document [1]. > > > > [1] Documentation/core-api/pin_user_pages.rst > > > > [2] "Explicit pinning of user-space pages": > > https://lwn.net/Articles/807108/ > > > > Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com> > > Cc: John Hubbard <jhubbard@nvidia.com> > > Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com> > > Cc: Paul Durrant <xadimgnik@gmail.com> > > --- > > Hi, > > > > I'm compile tested this, but unable to run-time test, so any testing > > help is much appriciated. > > > > drivers/xen/privcmd.c | 10 ++-------- > > 1 file changed, 2 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c > > index 0da417c..eb05254 100644 > > --- a/drivers/xen/privcmd.c > > +++ b/drivers/xen/privcmd.c > > @@ -595,7 +595,7 @@ static int lock_pages( > > if (requested > nr_pages) > > return -ENOSPC; > > > > - page_count = get_user_pages_fast( > > + page_count = pin_user_pages_fast( > > (unsigned long) kbufs[i].uptr, > > requested, FOLL_WRITE, pages); > > if (page_count < 0) { > > @@ -612,13 +612,7 @@ static int lock_pages( > > > > static void unlock_pages(struct page *pages[], unsigned int nr_pages) > > { > > - unsigned int i; > > - > > - for (i = 0; i < nr_pages; i++) { > > - if (!PageDirty(page)) > > - set_page_dirty_lock(page); > > - put_page(pages[i]); > > - } > > + unpin_user_pages_dirty_lock(pages, nr_pages, 1); > > "true", not "1", is the correct way to call that function. Ok. > > Also, this approach changes the behavior slightly, but I think it's > reasonable to just set_page_dirty_lock() on the whole range--hard to > see much benefit in checking PageDirty first. unpin_user_pages_dirty_lock() internally will do the same check after patch [2/2] So I thought to keep old and new code in sync. Shall we avoid this check ? > > > > } > > > > static long privcmd_ioctl_dm_op(struct file *file, void __user *udata) > > > > thanks, > -- > John Hubbard > NVIDIA
WARNING: multiple messages have this Message-ID (diff)
From: Souptick Joarder <jrdr.linux@gmail.com> To: John Hubbard <jhubbard@nvidia.com> Cc: Juergen Gross <jgross@suse.com>, sstabellini@kernel.org, linux-kernel@vger.kernel.org, Paul Durrant <xadimgnik@gmail.com>, xen-devel@lists.xenproject.org, Boris Ostrovsky <boris.ostrovsky@oracle.com> Subject: Re: [PATCH 2/2] xen/privcmd: Convert get_user_pages*() to pin_user_pages*() Date: Fri, 26 Jun 2020 10:56:53 +0530 [thread overview] Message-ID: <CAFqt6zZdq_OMZ3EBDGC+Bn4uPBEhDGOYF=jB4B16z7rY6hpZ7g@mail.gmail.com> (raw) In-Reply-To: <59afe2fe-3718-85aa-f3b5-83ca0b9df577@nvidia.com> On Thu, Jun 25, 2020 at 11:19 AM John Hubbard <jhubbard@nvidia.com> wrote: > > On 2020-06-24 20:02, Souptick Joarder wrote: > > In 2019, we introduced pin_user_pages*() and now we are converting > > get_user_pages*() to the new API as appropriate. [1] & [2] could > > be referred for more information. This is case 5 as per document [1]. > > > > [1] Documentation/core-api/pin_user_pages.rst > > > > [2] "Explicit pinning of user-space pages": > > https://lwn.net/Articles/807108/ > > > > Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com> > > Cc: John Hubbard <jhubbard@nvidia.com> > > Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com> > > Cc: Paul Durrant <xadimgnik@gmail.com> > > --- > > Hi, > > > > I'm compile tested this, but unable to run-time test, so any testing > > help is much appriciated. > > > > drivers/xen/privcmd.c | 10 ++-------- > > 1 file changed, 2 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c > > index 0da417c..eb05254 100644 > > --- a/drivers/xen/privcmd.c > > +++ b/drivers/xen/privcmd.c > > @@ -595,7 +595,7 @@ static int lock_pages( > > if (requested > nr_pages) > > return -ENOSPC; > > > > - page_count = get_user_pages_fast( > > + page_count = pin_user_pages_fast( > > (unsigned long) kbufs[i].uptr, > > requested, FOLL_WRITE, pages); > > if (page_count < 0) { > > @@ -612,13 +612,7 @@ static int lock_pages( > > > > static void unlock_pages(struct page *pages[], unsigned int nr_pages) > > { > > - unsigned int i; > > - > > - for (i = 0; i < nr_pages; i++) { > > - if (!PageDirty(page)) > > - set_page_dirty_lock(page); > > - put_page(pages[i]); > > - } > > + unpin_user_pages_dirty_lock(pages, nr_pages, 1); > > "true", not "1", is the correct way to call that function. Ok. > > Also, this approach changes the behavior slightly, but I think it's > reasonable to just set_page_dirty_lock() on the whole range--hard to > see much benefit in checking PageDirty first. unpin_user_pages_dirty_lock() internally will do the same check after patch [2/2] So I thought to keep old and new code in sync. Shall we avoid this check ? > > > > } > > > > static long privcmd_ioctl_dm_op(struct file *file, void __user *udata) > > > > thanks, > -- > John Hubbard > NVIDIA
next prev parent reply other threads:[~2020-06-26 5:27 UTC|newest] Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-06-25 3:02 [PATCH 1/2] xen/privcmd: Corrected error handling path and mark pages dirty Souptick Joarder 2020-06-25 3:02 ` Souptick Joarder 2020-06-25 3:02 ` [PATCH 2/2] xen/privcmd: Convert get_user_pages*() to pin_user_pages*() Souptick Joarder 2020-06-25 3:02 ` Souptick Joarder 2020-06-25 5:49 ` John Hubbard 2020-06-25 5:49 ` John Hubbard 2020-06-26 5:26 ` Souptick Joarder [this message] 2020-06-26 5:26 ` Souptick Joarder 2020-06-26 6:54 ` John Hubbard 2020-06-26 6:54 ` John Hubbard 2020-06-25 23:31 ` [PATCH 1/2] xen/privcmd: Corrected error handling path and mark pages dirty Boris Ostrovsky 2020-06-25 23:31 ` Boris Ostrovsky 2020-06-26 5:36 ` Souptick Joarder 2020-06-26 5:36 ` Souptick Joarder 2020-06-26 5:52 ` Jürgen Groß 2020-06-26 5:52 ` Jürgen Groß 2020-06-26 6:48 ` Souptick Joarder 2020-06-26 6:48 ` Souptick Joarder
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='CAFqt6zZdq_OMZ3EBDGC+Bn4uPBEhDGOYF=jB4B16z7rY6hpZ7g@mail.gmail.com' \ --to=jrdr.linux@gmail.com \ --cc=boris.ostrovsky@oracle.com \ --cc=jgross@suse.com \ --cc=jhubbard@nvidia.com \ --cc=linux-kernel@vger.kernel.org \ --cc=sstabellini@kernel.org \ --cc=xadimgnik@gmail.com \ --cc=xen-devel@lists.xenproject.org \ /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.