Re: [PATCH] NOMMU: Pages allocated to a ramfs inode's pagecache may get wrongly discarded

[Minchan Kim]

In the middle of writing the email, I seneded it by mistake.
Sorry for that.
Please, understand wrong patch title and changelog.
I think although i don't modify that, you can understand it, well.

So, I can't resend this until finising discussion. :)

On Thu, Mar 12, 2009 at 10:52 AM, Minchan Kim <minchan.kim@xxxxxxxxx> wrote:
> Hi, Kosaki-san.
>
> I think ramfs pages's unevictablility should not depend on CONFIG_UNEVICTABLE_LRU.
> It would be better to remove dependency of CONFIG_UNEVICTABLE_LRU ?
>
>
> How about this ?
> It's just RFC. It's not tested.
>
> That's because we can't reclaim that pages regardless of whether there is unevictable list or not
>
> From 487ce9577ea9c43b04ff340a1ba8c4030873e875 Mon Sep 17 00:00:00 2001
> From: MinChan Kim <minchan.kim@xxxxxxxxx>
> Date: Thu, 12 Mar 2009 10:35:37 +0900
> Subject: [PATCH] test
> ÂSigned-off-by: MinChan Kim <minchan.kim@xxxxxxxxx>
>
> ---
> Âinclude/linux/pagemap.h | Â Â9 ---------
> Âinclude/linux/swap.h  Â|  Â9 ++-------
> Â2 files changed, 2 insertions(+), 16 deletions(-)
>
> diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
> index 4d27bf8..0cf024c 100644
> --- a/include/linux/pagemap.h
> +++ b/include/linux/pagemap.h
> @@ -32,7 +32,6 @@ static inline void mapping_set_error(struct address_space *mapping, int error)
> Â Â Â Â}
> Â}
>
> -#ifdef CONFIG_UNEVICTABLE_LRU
> Â#define AS_UNEVICTABLE (__GFP_BITS_SHIFT + 2) Â/* e.g., ramdisk, SHM_LOCK */
>
> Âstatic inline void mapping_set_unevictable(struct address_space *mapping)
> @@ -51,14 +50,6 @@ static inline int mapping_unevictable(struct address_space *mapping)
> Â Â Â Â Â Â Â Âreturn test_bit(AS_UNEVICTABLE, &mapping->flags);
> Â Â Â Âreturn !!mapping;
> Â}
> -#else
> -static inline void mapping_set_unevictable(struct address_space *mapping) { }
> -static inline void mapping_clear_unevictable(struct address_space *mapping) { }
> -static inline int mapping_unevictable(struct address_space *mapping)
> -{
> - Â Â Â return 0;
> -}
> -#endif
>
> Âstatic inline gfp_t mapping_gfp_mask(struct address_space * mapping)
> Â{
> diff --git a/include/linux/swap.h b/include/linux/swap.h
> index a3af95b..18c639b 100644
> --- a/include/linux/swap.h
> +++ b/include/linux/swap.h
> @@ -233,8 +233,9 @@ static inline int zone_reclaim(struct zone *z, gfp_t mask, unsigned int order)
> Â}
> Â#endif
>
> -#ifdef CONFIG_UNEVICTABLE_LRU
> Âextern int page_evictable(struct page *page, struct vm_area_struct *vma);
> +
> +#ifdef CONFIG_UNEVICTABLE_LRU
> Âextern void scan_mapping_unevictable_pages(struct address_space *);
>
> Âextern unsigned long scan_unevictable_pages;
> @@ -243,12 +244,6 @@ extern int scan_unevictable_handler(struct ctl_table *, int, struct file *,
> Âextern int scan_unevictable_register_node(struct node *node);
> Âextern void scan_unevictable_unregister_node(struct node *node);
> Â#else
> -static inline int page_evictable(struct page *page,
> - Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â struct vm_area_struct *vma)
> -{
> - Â Â Â return 1;
> -}
> -
> Âstatic inline void scan_mapping_unevictable_pages(struct address_space *mapping)
> Â{
> Â}
> --
> 1.5.4.3
>
>
>
>> On Thu, 12 Mar 2009 10:04:41 +0900 (JST)
>> KOSAKI Motohiro <kosaki.motohiro@xxxxxxxxxxxxxx> wrote:
>>
>> Hi
>>
>> > >> Page reclaim shouldn't be even attempting to reclaim or write back
>> > >> ramfs pagecache pages - reclaim can't possibly do anything with these
>> > >> pages!
>> > >>
>> > >> Arguably those pages shouldn't be on the LRU at all, but we haven't
>> > >> done that yet.
>> > >>
>> > >> Now, my problem is that I can't 100% be sure that we _ever_ implemented
>> > >> this properly. ?I _think_ we did, in which case we later broke it. ?If
>> > >> we've always been (stupidly) trying to pageout these pages then OK, I
>> > >> guess your patch is a suitable 2.6.29 stopgap.
>> > >
>> > > OK, I can't find any code anywhere in which we excluded ramfs pages
>> > > from consideration by page reclaim. ?How dumb.
>> >
>> > The ramfs Âconsiders it in just CONFIG_UNEVICTABLE_LRU case
>> > It that case, ramfs_get_inode calls mapping_set_unevictable.
>> > So, Âpage reclaim can exclude ramfs pages by page_evictable.
>> > It's problem .
>>
>> Currently, CONFIG_UNEVICTABLE_LRU can't use on nommu machine
>> because nobody of vmscan folk havbe nommu machine.
>>
>> Yes, it is very stupid reason. _very_ welcome to tester! :)
>>
>>
>>
>> David, Could you please try following patch if you have NOMMU machine?
>> it is straightforward porting to nommu.
>>
>>
>> ==
>> Subject: [PATCH] remove to depend on MMU from CONFIG_UNEVICTABLE_LRU
>>
>> logically, CONFIG_UNEVICTABLE_LRU doesn't depend on MMU.
>> but current code does by mistake. fix it.
>>
>>
>> Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@xxxxxxxxxxxxxx>
>> ---
>> Âmm/Kconfig | Â Â1 -
>> Âmm/nommu.c | Â 24 ++++++++++++++++++++++++
>> Â2 files changed, 24 insertions(+), 1 deletion(-)
>>
>> Index: b/mm/Kconfig
>> ===================================================================
>> --- a/mm/Kconfig   Â2008-12-28 20:55:23.000000000 +0900
>> +++ b/mm/Kconfig   Â2008-12-28 21:24:08.000000000 +0900
>> @@ -212,7 +212,6 @@ config VIRT_TO_BUS
>> Âconfig UNEVICTABLE_LRU
>> Â Â Â bool "Add LRU list to track non-evictable pages"
>> Â Â Â default y
>> - Â Â depends on MMU
>> Â Â Â help
>> Â Â Â Â Keeps unevictable pages off of the active and inactive pageout
>> Â Â Â Â lists, so kswapd will not waste CPU time or have its balancing
>> Index: b/mm/nommu.c
>> ===================================================================
>> --- a/mm/nommu.c   Â2008-12-25 08:26:37.000000000 +0900
>> +++ b/mm/nommu.c   Â2008-12-28 21:29:36.000000000 +0900
>> @@ -1521,3 +1521,27 @@ int access_process_vm(struct task_struct
>> Â Â Â mmput(mm);
>> Â Â Â return len;
>> Â}
>> +
>> +/*
>> + * ÂLRU accounting for clear_page_mlock()
>> + */
>> +void __clear_page_mlock(struct page *page)
>> +{
>> + Â Â VM_BUG_ON(!PageLocked(page));
>> +
>> + Â Â if (!page->mapping) { Â /* truncated ? */
>> + Â Â Â Â Â Â return;
>> + Â Â }
>> +
>> + Â Â dec_zone_page_state(page, NR_MLOCK);
>> + Â Â count_vm_event(UNEVICTABLE_PGCLEARED);
>> + Â Â if (!isolate_lru_page(page)) {
>> + Â Â Â Â Â Â putback_lru_page(page);
>> + Â Â } else {
>> + Â Â Â Â Â Â /*
>> + Â Â Â Â Â Â Â* We lost the race. the page already moved to evictable list.
>> + Â Â Â Â Â Â Â*/
>> + Â Â Â Â Â Â if (PageUnevictable(page))
>> + Â Â Â Â Â Â Â Â Â Â count_vm_event(UNEVICTABLE_PGSTRANDED);
>> + Â Â }
>> +}
>>
>>
>>
>>
>
>
> --
> Kinds Regards
> Minchan Kim
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@xxxxxxxxxx ÂFor more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>
>



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