All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeremy Fitzhardinge <jeremy@goop.org>
To: Jaya Kumar <jayakumar.lkml@gmail.com>
Cc: Markus Armbruster <armbru@redhat.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: "fb-defio: fix page list with concurrent processes"
Date: Mon, 16 Jun 2008 15:05:02 -0700	[thread overview]
Message-ID: <4856E38E.1040203@goop.org> (raw)

Your patch "fb-defio: fix page list with concurrent processes" 
definitely seems to help with the suspend/resume problem I had with the 
Xen pvfb device.  Is it queued up anywhere?  It seems to be a real 
bugfix, and should probably be queued for 2.6.26...

    J

fb-defio: fix page list with concurrent processes

From: Jaya Kumar <jayakumar.lkml@gmail.com>

Hi Tony, Geert, Andrew, fbdev,

This patch is a bugfix for how defio handles multiple processes manipulating
the same framebuffer. Thanks to Bernard Blackham for identifying this bug.
It occurs when two applications mmap the same framebuffer and concurrently
write to the same page. Normally, this doesn't occur since only a single
process mmaps the framebuffer. The symptom of the bug is that the mapping
applications will hang. The cause is that defio incorrectly tries to add the
same page twice to the pagelist. The solution I have is to walk the pagelist
and check for a duplicate before adding. Since I needed to walk the
pagelist, I now also keep the pagelist in sorted order.

Thanks,
jaya

Signed-off-by: Jaya Kumar <jayakumar.lkml@gmail.com>

---
 drivers/video/fb_defio.c |   20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

===================================================================
--- a/drivers/video/fb_defio.c
+++ b/drivers/video/fb_defio.c
@@ -74,6 +74,7 @@ static int fb_deferred_io_mkwrite(struct vm_area_struct *vma,
 {
 	struct fb_info *info = vma->vm_private_data;
 	struct fb_deferred_io *fbdefio = info->fbdefio;
+	struct page *cur;
 
 	/* this is a callback we get when userspace first tries to
 	write to the page. we schedule a workqueue. that workqueue
@@ -83,7 +84,24 @@ static int fb_deferred_io_mkwrite(struct vm_area_struct *vma,
 
 	/* protect against the workqueue changing the page list */
 	mutex_lock(&fbdefio->lock);
-	list_add(&page->lru, &fbdefio->pagelist);
+
+	/* we loop through the pagelist before adding in order
+	to keep the pagelist sorted */
+	list_for_each_entry(cur, &fbdefio->pagelist, lru) {
+		/* this check is to catch the case where a new
+		process could start writing to the same page
+		through a new pte. this new access can cause the
+		mkwrite even when the original ps's pte is marked
+		writable */
+		if (unlikely(cur == page))
+			goto page_already_added;
+		else if (cur->index > page->index)
+			break;
+	}
+
+	list_add_tail(&page->lru, &cur->lru);
+
+page_already_added:
 	mutex_unlock(&fbdefio->lock);
 
 	/* come back after delay to process the deferred IO */



             reply	other threads:[~2008-06-16 22:05 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-16 22:05 Jeremy Fitzhardinge [this message]
2008-06-17  1:11 ` "fb-defio: fix page list with concurrent processes" Jaya Kumar
2008-06-17  7:34   ` Markus Armbruster
2008-06-17  8:21     ` Jaya Kumar
2008-06-17  9:31       ` Markus Armbruster
2008-07-03 21:10   ` Markus Armbruster
2008-07-03 23:44     ` Jaya Kumar
2008-07-07 20:43       ` Markus Armbruster
2008-07-08  0:54         ` Jaya Kumar

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=4856E38E.1040203@goop.org \
    --to=jeremy@goop.org \
    --cc=armbru@redhat.com \
    --cc=jayakumar.lkml@gmail.com \
    --cc=linux-kernel@vger.kernel.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.