lkml.org 
[lkml]   [2008]   [Jun]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: "fb-defio: fix page list with concurrent processes"
    On Mon, Jun 16, 2008 at 3:05 PM, Jeremy Fitzhardinge <jeremy@goop.org> wrote:
    > 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...

    It isn't currently queued. I had intended to improve its performance
    by taking advantage of Andrew's suggestion of using !list_empty on the
    page->lru to avoid walking the page list to find the duplicate page,
    but I ran into trouble since the page starts off being on the lru
    list. I'll try to take a look at doing this next weekend.

    Thanks,
    jaya


    >
    > 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 */
    >
    >
    >


    \
     
     \ /
      Last update: 2008-06-17 03:13    [W:2.453 / U:0.256 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site