[PATCH -mm] split_lru: fix pagevec_move_tail() doesn't treat unevictable page - Kernel

This is a discussion on [PATCH -mm] split_lru: fix pagevec_move_tail() doesn't treat unevictable page - Kernel ; even under writebacking, page can move to unevictable list. so shouldn't pagevec_move_tail() check unevictable? Signed-off-by: KOSAKI Motohiro --- mm/swap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Index: b/mm/swap.c ================================================== ================= --- a/mm/swap.c +++ b/mm/swap.c @@ -116,7 +116,7 ...

+ Reply to Thread
Results 1 to 11 of 11

Thread: [PATCH -mm] split_lru: fix pagevec_move_tail() doesn't treat unevictable page

  1. [PATCH -mm] split_lru: fix pagevec_move_tail() doesn't treat unevictable page

    even under writebacking, page can move to unevictable list.
    so shouldn't pagevec_move_tail() check unevictable?


    Signed-off-by: KOSAKI Motohiro

    ---
    mm/swap.c | 2 +-
    1 file changed, 1 insertion(+), 1 deletion(-)

    Index: b/mm/swap.c
    ================================================== =================
    --- a/mm/swap.c
    +++ b/mm/swap.c
    @@ -116,7 +116,7 @@ static void pagevec_move_tail(struct pag
    zone = pagezone;
    spin_lock(&zone->lru_lock);
    }
    - if (PageLRU(page) && !PageActive(page)) {
    + if (PageLRU(page) && !PageActive(page) && !PageUnevictable(page)) {
    int lru = page_is_file_cache(page);
    list_move_tail(&page->lru, &zone->lru[lru].list);
    pgmoved++;


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

  2. Re: [PATCH -mm] split_lru: fix pagevec_move_tail() doesn't treat unevictable page

    On Tue, Jul 1, 2008 at 4:01 PM, KOSAKI Motohiro
    wrote:
    > even under writebacking, page can move to unevictable list.
    > so shouldn't pagevec_move_tail() check unevictable?
    >

    Hi, Kosaki-san.

    I can't understand this race situation.
    How the page can move to unevictable list while it is under writeback?

    Could you explain for me ?

    > Signed-off-by: KOSAKI Motohiro
    >
    > ---
    > mm/swap.c | 2 +-
    > 1 file changed, 1 insertion(+), 1 deletion(-)
    >
    > Index: b/mm/swap.c
    > ================================================== =================
    > --- a/mm/swap.c
    > +++ b/mm/swap.c
    > @@ -116,7 +116,7 @@ static void pagevec_move_tail(struct pag
    > zone = pagezone;
    > spin_lock(&zone->lru_lock);
    > }
    > - if (PageLRU(page) && !PageActive(page)) {
    > + if (PageLRU(page) && !PageActive(page) && !PageUnevictable(page)) {
    > int lru = page_is_file_cache(page);
    > list_move_tail(&page->lru, &zone->lru[lru].list);
    > pgmoved++;
    >
    >
    > --
    > 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/
    >




    --
    Kinds regards,
    MinChan Kim
    --
    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/

  3. Re: [PATCH -mm] split_lru: fix pagevec_move_tail() doesn't treat unevictable page

    Hi Kim-san,

    Thank you for good question.

    > > even under writebacking, page can move to unevictable list.
    > > so shouldn't pagevec_move_tail() check unevictable?
    > >

    > Hi, Kosaki-san.
    >
    > I can't understand this race situation.
    > How the page can move to unevictable list while it is under writeback?
    >
    > Could you explain for me ?


    Actually, I added below assertion and tested on stress workload.
    then system crashed after 4H runnings.

    ----------------------------------------------
    static void pagevec_move_tail(struct pagevec *pvec)
    {
    (snip)
    if (PageLRU(page) && !PageActive(page)) {
    int lru = page_is_file_cache(page);
    list_move_tail(&page->lru, &zone->lru[lru].list);
    BUG_ON(page_lru(page) != lru); // !!here
    pgmoved++;
    }
    }
    ----------------------------------------------------

    So, I guess below race exist (but I hope Rik's review)


    CPU1 CPU2
    ================================================== ================
    1. rotate_reclaimable_page()
    2. PageUnevictable(page) return 0
    3. local_irq_save()
    4. pagevec_move_tail()
    SetPageUnevictable() //mlock?
    move to unevictable list
    5. spin_lock(&zone->lru_lock);
    6. list_move_tail(); (move to inactive list)

    then page have PageUnevictable() and is chained inactive lru.
    Or, I misunderstand it?


    abstraction of related function
    ------------------------------------------------------------
    void rotate_reclaimable_page(struct page *page)
    {
    if (!PageLocked(page) && !PageDirty(page) && !PageActive(page) &&
    !PageUnevictable(page) && PageLRU(page)) {
    local_irq_save(flags);
    pagevec_move_tail(pvec);
    local_irq_restore(flags);
    }
    }

    pagevec_move_tail(){
    spin_lock(&zone->lru_lock);
    if (PageLRU(page) && !PageActive(page)) {
    list_move_tail(&page->lru, &zone->lru[lru].list);
    }
    spin_unlock(&zone->lru_lock);
    }




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

  4. Re: [PATCH -mm] split_lru: fix pagevec_move_tail() doesn't treat unevictable page

    On Tue, Jul 1, 2008 at 4:56 PM, KOSAKI Motohiro
    wrote:
    > Hi Kim-san,
    >
    > Thank you for good question.


    Thanks for good explaining.
    I guess your scenario have a possibility.

    If I don't have a test HPC, I will dig in source.

    >> > even under writebacking, page can move to unevictable list.
    >> > so shouldn't pagevec_move_tail() check unevictable?
    >> >

    >> Hi, Kosaki-san.
    >>
    >> I can't understand this race situation.
    >> How the page can move to unevictable list while it is under writeback?
    >>
    >> Could you explain for me ?

    >
    > Actually, I added below assertion and tested on stress workload.
    > then system crashed after 4H runnings.
    >
    > ----------------------------------------------
    > static void pagevec_move_tail(struct pagevec *pvec)
    > {
    > (snip)
    > if (PageLRU(page) && !PageActive(page)) {
    > int lru = page_is_file_cache(page);
    > list_move_tail(&page->lru, &zone->lru[lru].list);
    > BUG_ON(page_lru(page) != lru); // !!here
    > pgmoved++;
    > }
    > }
    > ----------------------------------------------------
    >
    > So, I guess below race exist (but I hope Rik's review)
    >
    >
    > CPU1 CPU2
    > ================================================== ================
    > 1. rotate_reclaimable_page()
    > 2. PageUnevictable(page) return 0
    > 3. local_irq_save()
    > 4. pagevec_move_tail()
    > SetPageUnevictable() //mlock?
    > move to unevictable list
    > 5. spin_lock(&zone->lru_lock);
    > 6. list_move_tail(); (move to inactive list)
    >
    > then page have PageUnevictable() and is chained inactive lru.
    > Or, I misunderstand it?
    >
    >
    > abstraction of related function
    > ------------------------------------------------------------
    > void rotate_reclaimable_page(struct page *page)
    > {
    > if (!PageLocked(page) && !PageDirty(page) && !PageActive(page) &&
    > !PageUnevictable(page) && PageLRU(page)) {
    > local_irq_save(flags);
    > pagevec_move_tail(pvec);
    > local_irq_restore(flags);
    > }
    > }
    >
    > pagevec_move_tail(){
    > spin_lock(&zone->lru_lock);
    > if (PageLRU(page) && !PageActive(page)) {
    > list_move_tail(&page->lru, &zone->lru[lru].list);
    > }
    > spin_unlock(&zone->lru_lock);
    > }
    >
    >
    >
    >
    >




    --
    Kinds regards,
    MinChan Kim
    --
    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/

  5. [resend][PATCH -mm] split_lru: fix pagevec_move_tail() doesn't treat unevictable page


    Agghh!
    I am really stupid. I forgot CCed Andrew ;-)

    So, I resend this.


    --------------------------------------
    even under writebacking, page can move to unevictable list.
    so shouldn't pagevec_move_tail() check unevictable?

    if pagevec_move_tail() doesn't PageUnevictable(),
    below race can occur.


    CPU1 CPU2
    ================================================== ================
    1. rotate_reclaimable_page()
    2. PageUnevictable(page) return 0
    3. local_irq_save()
    4. pagevec_move_tail()
    SetPageUnevictable() //mlock?
    move to unevictable list
    5. spin_lock(&zone->lru_lock);
    6. list_move_tail(); (move to inactive list)



    Signed-off-by: KOSAKI Motohiro

    ---
    mm/swap.c | 2 +-
    1 file changed, 1 insertion(+), 1 deletion(-)

    Index: b/mm/swap.c
    ================================================== =================
    --- a/mm/swap.c
    +++ b/mm/swap.c
    @@ -116,7 +116,7 @@ static void pagevec_move_tail(struct pag
    zone = pagezone;
    spin_lock(&zone->lru_lock);
    }
    - if (PageLRU(page) && !PageActive(page)) {
    + if (PageLRU(page) && !PageActive(page) && !PageUnevictable(page)) {
    int lru = page_is_file_cache(page);
    list_move_tail(&page->lru, &zone->lru[lru].list);
    pgmoved++;




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

  6. Re: [resend][PATCH -mm] split_lru: fix pagevec_move_tail() doesn't treat unevictable page

    On Tue, 01 Jul 2008 17:26:51 +0900
    KOSAKI Motohiro wrote:

    > Signed-off-by: KOSAKI Motohiro


    Acked-by: Rik van Riel

    Good catch!

    > @@ -116,7 +116,7 @@ static void pagevec_move_tail(struct pag
    > zone = pagezone;
    > spin_lock(&zone->lru_lock);
    > }
    > - if (PageLRU(page) && !PageActive(page)) {
    > + if (PageLRU(page) && !PageActive(page) && !PageUnevictable(page)) {
    > int lru = page_is_file_cache(page);
    > list_move_tail(&page->lru, &zone->lru[lru].list);
    > pgmoved++;


    --
    All rights reversed.
    --
    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/

  7. Re: [resend][PATCH -mm] split_lru: fix pagevec_move_tail() doesn't treat unevictable page

    Hi, Rik and Kosaki-san

    I want to know exact race situation for remaining git log.
    As you know, git log is important for me who is newbie to understand source

    There are many possibility in this race problem.

    Did you use hugepage in this test ?
    I think that If you used hugepage, it seems to happen following race.

    --------------

    CPU1 CPU2

    shm_unlock
    scan_mapping_unevictable_pages
    check_move_unevictable_page
    ClearPageUnevictable rotate_reclaimable_page

    PageUnevictable(page) return 0
    SetPageUnevictable
    list_move(LRU_UNEVICTABLE)

    local_irq_save

    pagevec_move_tail

    Do you think it is possible ?

    On Tue, Jul 1, 2008 at 10:38 PM, Rik van Riel wrote:
    > On Tue, 01 Jul 2008 17:26:51 +0900
    > KOSAKI Motohiro wrote:
    >
    >> Signed-off-by: KOSAKI Motohiro

    >
    > Acked-by: Rik van Riel
    >
    > Good catch!
    >
    >> @@ -116,7 +116,7 @@ static void pagevec_move_tail(struct pag
    >> zone = pagezone;
    >> spin_lock(&zone->lru_lock);
    >> }
    >> - if (PageLRU(page) && !PageActive(page)) {
    >> + if (PageLRU(page) && !PageActive(page) && !PageUnevictable(page)) {
    >> int lru = page_is_file_cache(page);
    >> list_move_tail(&page->lru, &zone->lru[lru].list);
    >> pgmoved++;

    >
    > --
    > All rights reversed.
    >




    --
    Kinds regards,
    MinChan Kim
    --
    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/

  8. Re: [resend][PATCH -mm] split_lru: fix pagevec_move_tail() doesn't treat unevictable page

    So sorry to make a noisy . :-<

    Gmail is mangling my mail.
    I hope this is last.

    CPU1 CPU2

    shm_unlock
    scan_mapping_unevictable_pages
    check_move_unevictable_page
    ClearPageUnevictable
    rotate_reclaimable_page
    PageUnevictable(page) return 0

    SetPageUnevictable
    list_move(LRU_UNEVICTABLE)

    local_irq_save
    pagevec_move_tail



    On Wed, Jul 2, 2008 at 9:49 AM, MinChan Kim wrote:
    > Sorry, it seems mail sender problem.
    > Resend
    >
    > CPU1 CPU2
    >
    > shm_unlock
    > scan_mapping_unevictable_pages
    > check_move_unevictable_page
    > ClearPageUnevictable
    > rotate_reclaimable_page
    >
    > PageUnevictable(page) return 0
    > SetPageUnevictable
    > list_move(LRU_UNEVICTABLE)
    >
    > local_irq_save
    > pagevec_move_tail
    >
    >
    > On Wed, Jul 2, 2008 at 9:39 AM, MinChan Kim wrote:
    >> Hi, Rik and Kosaki-san
    >>
    >> I want to know exact race situation for remaining git log.
    >> As you know, git log is important for me who is newbie to understand source
    >>
    >> There are many possibility in this race problem.
    >>
    >> Did you use hugepage in this test ?
    >> I think that If you used hugepage, it seems to happen following race.
    >>
    >> --------------
    >>
    >> CPU1 CPU2
    >>
    >> shm_unlock
    >> scan_mapping_unevictable_pages
    >> check_move_unevictable_page
    >> ClearPageUnevictable rotate_reclaimable_page
    >>
    >> PageUnevictable(page) return 0
    >> SetPageUnevictable
    >> list_move(LRU_UNEVICTABLE)
    >>
    >> local_irq_save
    >>
    >> pagevec_move_tail
    >>
    >> Do you think it is possible ?
    >>
    >> On Tue, Jul 1, 2008 at 10:38 PM, Rik van Riel wrote:
    >>> On Tue, 01 Jul 2008 17:26:51 +0900
    >>> KOSAKI Motohiro wrote:
    >>>
    >>>> Signed-off-by: KOSAKI Motohiro
    >>>
    >>> Acked-by: Rik van Riel
    >>>
    >>> Good catch!
    >>>
    >>>> @@ -116,7 +116,7 @@ static void pagevec_move_tail(struct pag
    >>>> zone = pagezone;
    >>>> spin_lock(&zone->lru_lock);
    >>>> }
    >>>> - if (PageLRU(page) && !PageActive(page)) {
    >>>> + if (PageLRU(page) && !PageActive(page) && !PageUnevictable(page)) {
    >>>> int lru = page_is_file_cache(page);
    >>>> list_move_tail(&page->lru, &zone->lru[lru].list);
    >>>> pgmoved++;
    >>>
    >>> --
    >>> All rights reversed.
    >>>

    >>
    >>
    >>
    >> --
    >> Kinds regards,
    >> MinChan Kim
    >>

    >
    >
    >
    > --
    > Kinds regards,
    > MinChan Kim
    >




    --
    Kinds regards,
    MinChan Kim
    --
    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/

  9. Re: [resend][PATCH -mm] split_lru: fix pagevec_move_tail() doesn't treat unevictable page

    Sorry, it seems mail sender problem.
    Resend

    CPU1 CPU2

    shm_unlock
    scan_mapping_unevictable_pages
    check_move_unevictable_page
    ClearPageUnevictable
    rotate_reclaimable_page

    PageUnevictable(page) return 0
    SetPageUnevictable
    list_move(LRU_UNEVICTABLE)

    local_irq_save
    pagevec_move_tail


    On Wed, Jul 2, 2008 at 9:39 AM, MinChan Kim wrote:
    > Hi, Rik and Kosaki-san
    >
    > I want to know exact race situation for remaining git log.
    > As you know, git log is important for me who is newbie to understand source
    >
    > There are many possibility in this race problem.
    >
    > Did you use hugepage in this test ?
    > I think that If you used hugepage, it seems to happen following race.
    >
    > --------------
    >
    > CPU1 CPU2
    >
    > shm_unlock
    > scan_mapping_unevictable_pages
    > check_move_unevictable_page
    > ClearPageUnevictable rotate_reclaimable_page
    >
    > PageUnevictable(page) return 0
    > SetPageUnevictable
    > list_move(LRU_UNEVICTABLE)
    >
    > local_irq_save
    >
    > pagevec_move_tail
    >
    > Do you think it is possible ?
    >
    > On Tue, Jul 1, 2008 at 10:38 PM, Rik van Riel wrote:
    >> On Tue, 01 Jul 2008 17:26:51 +0900
    >> KOSAKI Motohiro wrote:
    >>
    >>> Signed-off-by: KOSAKI Motohiro

    >>
    >> Acked-by: Rik van Riel
    >>
    >> Good catch!
    >>
    >>> @@ -116,7 +116,7 @@ static void pagevec_move_tail(struct pag
    >>> zone = pagezone;
    >>> spin_lock(&zone->lru_lock);
    >>> }
    >>> - if (PageLRU(page) && !PageActive(page)) {
    >>> + if (PageLRU(page) && !PageActive(page) && !PageUnevictable(page)) {
    >>> int lru = page_is_file_cache(page);
    >>> list_move_tail(&page->lru, &zone->lru[lru].list);
    >>> pgmoved++;

    >>
    >> --
    >> All rights reversed.
    >>

    >
    >
    >
    > --
    > Kinds regards,
    > MinChan Kim
    >




    --
    Kinds regards,
    MinChan Kim
    --
    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/

  10. Re: [resend][PATCH -mm] split_lru: fix pagevec_move_tail() doesn't treat unevictable page

    Hi Kim-san,

    > Hi, Rik and Kosaki-san
    >
    > I want to know exact race situation for remaining git log.
    > As you know, git log is important for me who is newbie to understand source
    >
    > There are many possibility in this race problem.
    >
    > Did you use hugepage in this test ?
    > I think that If you used hugepage, it seems to happen following race.


    I don't use hugepage. but use SYSV-shmem.
    so following scenario is very reasonable.

    OK.
    I resend my patch with following description.


    >
    > --------------
    >
    > CPU1 CPU2
    >
    > shm_unlock
    > scan_mapping_unevictable_pages
    > check_move_unevictable_page
    > ClearPageUnevictable rotate_reclaimable_page
    >
    > PageUnevictable(page) return 0
    > SetPageUnevictable
    > list_move(LRU_UNEVICTABLE)
    >
    > local_irq_save
    >
    > pagevec_move_tail
    >
    > Do you think it is possible ?



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

  11. Re: [resend][PATCH -mm] split_lru: fix pagevec_move_tail() doesn't treat unevictable page

    On Wed, Jul 2, 2008 at 12:30 PM, KOSAKI Motohiro
    wrote:
    > Hi Kim-san,
    >
    >> Hi, Rik and Kosaki-san
    >>
    >> I want to know exact race situation for remaining git log.
    >> As you know, git log is important for me who is newbie to understand source
    >>
    >> There are many possibility in this race problem.
    >>
    >> Did you use hugepage in this test ?
    >> I think that If you used hugepage, it seems to happen following race.

    >
    > I don't use hugepage. but use SYSV-shmem.
    > so following scenario is very reasonable.


    It is not reasonable if you don't use hugepage.
    That's because file's address_space is still unevictable.
    Am I missing your point?

    I think following case is more reasonable rather than it,
    Please, Let you review this scenario.
    ---

    CPU1 CPU2

    shrink_[in]active_list
    cull_unevictable_page
    putback_lru_page
    TestClearPageUnevicetable
    rotate_reclaimable_page
    !PageUnevictable(page)
    add_page_to_unevictable_list
    pagevec_move_tail



    > OK.
    > I resend my patch with following description.
    >
    >
    >>
    >> --------------
    >>
    >> CPU1 CPU2
    >>
    >> shm_unlock
    >> scan_mapping_unevictable_pages
    >> check_move_unevictable_page
    >> ClearPageUnevictable rotate_reclaimable_page
    >>
    >> PageUnevictable(page) return 0
    >> SetPageUnevictable
    >> list_move(LRU_UNEVICTABLE)
    >>
    >> local_irq_save
    >>
    >> pagevec_move_tail
    >>
    >> Do you think it is possible ?

    >
    >
    >




    --
    Kinds regards,
    MinChan Kim
    --
    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/

+ Reply to Thread