From: Souptick Joarder <jrdr.linux@gmail.com> To: boris.ostrovsky@oracle.com, jgross@suse.com, sstabellini@kernel.org Cc: xen-devel@lists.xenproject.org, linux-kernel@vger.kernel.org, Souptick Joarder <jrdr.linux@gmail.com>, John Hubbard <jhubbard@nvidia.com>, Paul Durrant <xadimgnik@gmail.com> Subject: [PATCH 1/2] xen/privcmd: Corrected error handling path and mark pages dirty Date: Thu, 25 Jun 2020 08:32:39 +0530 [thread overview] Message-ID: <1593054160-12628-1-git-send-email-jrdr.linux@gmail.com> (raw) Previously, if lock_pages() end up partially mapping pages, it used to return -ERRNO due to which unlock_pages() have to go through each pages[i] till *nr_pages* to validate them. This can be avoided by passing correct number of partially mapped pages & -ERRNO separately, while returning from lock_pages() due to error. With this fix unlock_pages() doesn't need to validate pages[i] till *nr_pages* for error scenario and few condition checks can be ignored. As discussed, pages need to be marked as dirty before unpinned it in unlock_pages() which was oversight. 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 | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c index a250d11..0da417c 100644 --- a/drivers/xen/privcmd.c +++ b/drivers/xen/privcmd.c @@ -580,43 +580,44 @@ static long privcmd_ioctl_mmap_batch( static int lock_pages( struct privcmd_dm_op_buf kbufs[], unsigned int num, - struct page *pages[], unsigned int nr_pages) + struct page *pages[], unsigned int nr_pages, int *pinned) { unsigned int i; + int errno = 0, page_count = 0; for (i = 0; i < num; i++) { unsigned int requested; - int pinned; + *pinned += page_count; requested = DIV_ROUND_UP( offset_in_page(kbufs[i].uptr) + kbufs[i].size, PAGE_SIZE); if (requested > nr_pages) return -ENOSPC; - pinned = get_user_pages_fast( + page_count = get_user_pages_fast( (unsigned long) kbufs[i].uptr, requested, FOLL_WRITE, pages); - if (pinned < 0) - return pinned; + if (page_count < 0) { + errno = page_count; + return errno; + } - nr_pages -= pinned; - pages += pinned; + nr_pages -= page_count; + pages += page_count; } - return 0; + return errno; } static void unlock_pages(struct page *pages[], unsigned int nr_pages) { unsigned int i; - if (!pages) - return; - for (i = 0; i < nr_pages; i++) { - if (pages[i]) - put_page(pages[i]); + if (!PageDirty(page)) + set_page_dirty_lock(page); + put_page(pages[i]); } } @@ -630,6 +631,7 @@ static long privcmd_ioctl_dm_op(struct file *file, void __user *udata) struct xen_dm_op_buf *xbufs = NULL; unsigned int i; long rc; + int pinned = 0; if (copy_from_user(&kdata, udata, sizeof(kdata))) return -EFAULT; @@ -683,9 +685,11 @@ static long privcmd_ioctl_dm_op(struct file *file, void __user *udata) goto out; } - rc = lock_pages(kbufs, kdata.num, pages, nr_pages); - if (rc) + rc = lock_pages(kbufs, kdata.num, pages, nr_pages, &pinned); + if (rc < 0) { + nr_pages = pinned; goto out; + } for (i = 0; i < kdata.num; i++) { set_xen_guest_handle(xbufs[i].h, kbufs[i].uptr); -- 1.9.1
WARNING: multiple messages have this Message-ID (diff)
From: Souptick Joarder <jrdr.linux@gmail.com> To: boris.ostrovsky@oracle.com, jgross@suse.com, sstabellini@kernel.org Cc: xen-devel@lists.xenproject.org, Paul Durrant <xadimgnik@gmail.com>, linux-kernel@vger.kernel.org, John Hubbard <jhubbard@nvidia.com>, Souptick Joarder <jrdr.linux@gmail.com> Subject: [PATCH 1/2] xen/privcmd: Corrected error handling path and mark pages dirty Date: Thu, 25 Jun 2020 08:32:39 +0530 [thread overview] Message-ID: <1593054160-12628-1-git-send-email-jrdr.linux@gmail.com> (raw) Previously, if lock_pages() end up partially mapping pages, it used to return -ERRNO due to which unlock_pages() have to go through each pages[i] till *nr_pages* to validate them. This can be avoided by passing correct number of partially mapped pages & -ERRNO separately, while returning from lock_pages() due to error. With this fix unlock_pages() doesn't need to validate pages[i] till *nr_pages* for error scenario and few condition checks can be ignored. As discussed, pages need to be marked as dirty before unpinned it in unlock_pages() which was oversight. 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 | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c index a250d11..0da417c 100644 --- a/drivers/xen/privcmd.c +++ b/drivers/xen/privcmd.c @@ -580,43 +580,44 @@ static long privcmd_ioctl_mmap_batch( static int lock_pages( struct privcmd_dm_op_buf kbufs[], unsigned int num, - struct page *pages[], unsigned int nr_pages) + struct page *pages[], unsigned int nr_pages, int *pinned) { unsigned int i; + int errno = 0, page_count = 0; for (i = 0; i < num; i++) { unsigned int requested; - int pinned; + *pinned += page_count; requested = DIV_ROUND_UP( offset_in_page(kbufs[i].uptr) + kbufs[i].size, PAGE_SIZE); if (requested > nr_pages) return -ENOSPC; - pinned = get_user_pages_fast( + page_count = get_user_pages_fast( (unsigned long) kbufs[i].uptr, requested, FOLL_WRITE, pages); - if (pinned < 0) - return pinned; + if (page_count < 0) { + errno = page_count; + return errno; + } - nr_pages -= pinned; - pages += pinned; + nr_pages -= page_count; + pages += page_count; } - return 0; + return errno; } static void unlock_pages(struct page *pages[], unsigned int nr_pages) { unsigned int i; - if (!pages) - return; - for (i = 0; i < nr_pages; i++) { - if (pages[i]) - put_page(pages[i]); + if (!PageDirty(page)) + set_page_dirty_lock(page); + put_page(pages[i]); } } @@ -630,6 +631,7 @@ static long privcmd_ioctl_dm_op(struct file *file, void __user *udata) struct xen_dm_op_buf *xbufs = NULL; unsigned int i; long rc; + int pinned = 0; if (copy_from_user(&kdata, udata, sizeof(kdata))) return -EFAULT; @@ -683,9 +685,11 @@ static long privcmd_ioctl_dm_op(struct file *file, void __user *udata) goto out; } - rc = lock_pages(kbufs, kdata.num, pages, nr_pages); - if (rc) + rc = lock_pages(kbufs, kdata.num, pages, nr_pages, &pinned); + if (rc < 0) { + nr_pages = pinned; goto out; + } for (i = 0; i < kdata.num; i++) { set_xen_guest_handle(xbufs[i].h, kbufs[i].uptr); -- 1.9.1
next reply other threads:[~2020-06-25 2:54 UTC|newest] Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-06-25 3:02 Souptick Joarder [this message] 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 ` [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 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=1593054160-12628-1-git-send-email-jrdr.linux@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.