Re: [PATCH] mm: make munlock fast when mlock is canceled by sigkill

[Minchan Kim]

On Tue, 25 Aug 2009 13:46:19 +0900
Hiroaki Wakabayashi <primulaelatior@xxxxxxxxx> wrote:

> Thank you for reviews.
> 
> >>> > @@ -254,6 +254,7 @@ static inline void
> >>> > mminit_validate_memmodel_limits(unsigned long *start_pfn,
> >>> > Â#define GUP_FLAGS_FORCE Â Â Â Â Â Â Â Â Â0x2
> >>> > Â#define GUP_FLAGS_IGNORE_VMA_PERMISSIONS 0x4
> >>> > Â#define GUP_FLAGS_IGNORE_SIGKILL Â Â Â Â 0x8
> >>> > +#define GUP_FLAGS_ALLOW_NULL Â Â Â Â Â Â 0x10
> >>> >
> >>>
> >>> I am worried about adding new flag whenever we need it.
> >>> But I think this case makes sense to me.
> >>> In addition, I guess ZERO page can also use this flag.
> >>>
> >>> Kame. What do you think about it?
> >>>
> >> I do welcome this !
> >> Then, I don't have to take care of mlock/munlock in ZERO_PAGE patch.
> >>
> >> And without this patch, munlock() does copy-on-write just for unpinning memory.
> >> So, this patch shows some right direction, I think.
> >>
> >> One concern is flag name, ALLOW_NULL sounds not very good.
> >>
> >> ÂGUP_FLAGS_NOFAULT ?
> >>
> >> I wonder we can remove a hack of FOLL_ANON for core-dump by this flag, too.
> >
> > Yeah, GUP_FLAGS_NOFAULT is better.
> 
> Me too.
> I will change this flag name.
> 
> > Plus, this patch change __get_user_pages() return value meaning IOW.
> > after this patch, it can return following value,
> >
> > Âreturn value: 3
> > Âpages[0]: hoge-page
> > Âpages[1]: null
> > Âpages[2]: fuga-page
> >
> > but, it can be
> >
> > Âreturn value: 2
> > Âpages[0]: hoge-page
> > Âpages[1]: fuga-page
> >
> > no?
> 
> I did misunderstand mean of get_user_pages()'s return value.
> 
> When I try to change __get_user_pages(), I got problem.
> If remove NULLs from pages,
> __mlock_vma_pages_range() cannot know how long __get_user_pages() readed.
> So, I have to get the virtual address of the page from vma and page.
> Because __mlock_vma_pages_range() have to call
> __get_user_pages() many times with different `start' argument.
> 
> I try to use page_address_in_vma(), but it failed.
> (page_address_in_vma() returned -EFAULT)
> I cannot find way to solve this problem.
> Are there good ideas?
> Please give me some ideas.


Could you satisfy your needs with this ?

--- a/mm/mlock.c
+++ b/mm/mlock.c
@@ -217,6 +217,11 @@ static long __mlock_vma_pages_range(struct vm_area_struct *vma,
 
                lru_add_drain();        /* push cached pages to LRU */
 
+               /*
+                * here we assume that get_user_pages() has given us
+                * a list of virtually contiguous pages.
+                */
+               addr += PAGE_SIZE * ret; /* for next get_user_pages() */
                for (i = 0; i < ret; i++) {
                        struct page *page = pages[i];
 
@@ -234,12 +239,6 @@ static long __mlock_vma_pages_range(struct vm_area_struct *vma,
                        }
                        unlock_page(page);
                        put_page(page);         /* ref from get_user_pages() */
-
-                       /*
-                        * here we assume that get_user_pages() has given us
-                        * a list of virtually contiguous pages.
-                        */
-                       addr += PAGE_SIZE;      /* for next get_user_pages() */
                        nr_pages--;
                }
                ret = 0;

> 
> Thanks.
> --
> Hiroaki Wakabayashi


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