From: John Hubbard <jhubbard@nvidia.com>
To: Souptick Joarder <jrdr.linux@gmail.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: Thu, 25 Jun 2020 23:54:29 -0700 [thread overview]
Message-ID: <a750e5e5-fd5d-663b-c5fd-261d7c939ba7@nvidia.com> (raw)
In-Reply-To: <CAFqt6zZdq_OMZ3EBDGC+Bn4uPBEhDGOYF=jB4B16z7rY6hpZ7g@mail.gmail.com>
On 2020-06-25 22:26, Souptick Joarder wrote:
> On Thu, Jun 25, 2020 at 11:19 AM John Hubbard <jhubbard@nvidia.com> wrote:
>> On 2020-06-24 20:02, Souptick Joarder wrote:
...
>>> @@ -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
Correction, I forgot that I put that same if(!PageDirty(page)) check into
unpin_user_pages_dirty_lock(). So it doesn't change behavior. That's good.
>> 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 ?
>
Just leave it as you have it, but of course use "true" instead of 1, please.
thanks,
--
John Hubbard
NVIDIA
WARNING: multiple messages have this Message-ID (diff)
From: John Hubbard <jhubbard@nvidia.com>
To: Souptick Joarder <jrdr.linux@gmail.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: Thu, 25 Jun 2020 23:54:29 -0700 [thread overview]
Message-ID: <a750e5e5-fd5d-663b-c5fd-261d7c939ba7@nvidia.com> (raw)
In-Reply-To: <CAFqt6zZdq_OMZ3EBDGC+Bn4uPBEhDGOYF=jB4B16z7rY6hpZ7g@mail.gmail.com>
On 2020-06-25 22:26, Souptick Joarder wrote:
> On Thu, Jun 25, 2020 at 11:19 AM John Hubbard <jhubbard@nvidia.com> wrote:
>> On 2020-06-24 20:02, Souptick Joarder wrote:
...
>>> @@ -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
Correction, I forgot that I put that same if(!PageDirty(page)) check into
unpin_user_pages_dirty_lock(). So it doesn't change behavior. That's good.
>> 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 ?
>
Just leave it as you have it, but of course use "true" instead of 1, please.
thanks,
--
John Hubbard
NVIDIA
next prev parent reply other threads:[~2020-06-26 6:54 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
2020-06-26 5:26 ` Souptick Joarder
2020-06-26 6:54 ` John Hubbard [this message]
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=a750e5e5-fd5d-663b-c5fd-261d7c939ba7@nvidia.com \
--to=jhubbard@nvidia.com \
--cc=boris.ostrovsky@oracle.com \
--cc=jgross@suse.com \
--cc=jrdr.linux@gmail.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: link
Be 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.