lkml.org 
[lkml]   [2006]   [May]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH -mm] swsusp: support creating bigger images
    On Tue, 9 May 2006, Rafael J. Wysocki wrote:
    > On Tuesday 09 May 2006 09:33, Andrew Morton wrote:
    > > "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
    > >
    > > I have a host of minor problems with this patch.
    > >
    > > > --- linux-2.6.17-rc3-mm1.orig/mm/rmap.c
    > > > +++ linux-2.6.17-rc3-mm1/mm/rmap.c
    > > >
    > > > +#ifdef CONFIG_SOFTWARE_SUSPEND
    > > > +static int page_mapped_by_task(struct page *page, struct task_struct *task)
    > > > +{
    > > > + struct vm_area_struct *vma;
    > > > + int ret = 0;
    > > > +
    > > > + spin_lock(&task->mm->page_table_lock);
    > > > +
    > > > + for (vma = task->mm->mmap; vma; vma = vma->vm_next)
    > > > + if (page_address_in_vma(page, vma) != -EFAULT) {
    > > > + ret = 1;
    > > > + break;
    > > > + }
    > > > +
    > > > + spin_unlock(&task->mm->page_table_lock);
    > > > +
    > > > + return ret;
    > > > +}
    > >
    > > task_struct.mm can sometimes be NULL. This function assumes that it will
    > > never be NULL. That makes it a somewhat risky interface. Are we sure it
    > > can never be NULL?
    >
    > Well, now it's only called for task == current, but I can add a check.

    Better fold it into the (renamed and recommented) page_to_copy,
    applying only to current.

    The "use" of page_table_lock there is totally bogus. Normally you
    need down_read(&current->mm->mmap_sem) to walk that vma chain; but
    I'm guessing you have everything sufficiently frozen here that you
    don't need that.

    But if it is sufficiently frozen, I'm puzzled as to why pages mapped
    into the current process are (potentially) unsafe, while those mapped
    into others are safe. If the current process can get back to messing
    with its mapped pages, what if it maps a page you earlier judged safe?

    Hugh
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2006-05-10 04:01    [W:0.032 / U:0.648 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site