On Mon, Dec 15, 2014 at 03:03:43PM -0800, akpm@xxxxxxxxxxxxxxxxxxxx wrote:
> From: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> Subject: mm-introduce-do_shared_fault-and-drop-do_fault-fix-fix
>
> add comment which may not be true :(
>
> Cc: Andi Kleen <ak@xxxxxxxxxxxxxxx>
> Cc: Bob Liu <lliubbo@xxxxxxxxx>
> Cc: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>
> Cc: "Kirill A. Shutemov" <kirill@xxxxxxxxxxxxx>
> Cc: Matthew Wilcox <matthew.r.wilcox@xxxxxxxxx>
> Cc: Mel Gorman <mgorman@xxxxxxx>
> Cc: Naoya Horiguchi <n-horiguchi@xxxxxxxxxxxxx>
> Cc: Rik van Riel <riel@xxxxxxxxxx>
> Cc: Sasha Levin <sasha.levin@xxxxxxxxxx>
> Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> ---
>
> mm/memory.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff -puN mm/memory.c~mm-introduce-do_shared_fault-and-drop-do_fault-fix-fix mm/memory.c
> --- a/mm/memory.c~mm-introduce-do_shared_fault-and-drop-do_fault-fix-fix
> +++ a/mm/memory.c
> @@ -3009,6 +3009,12 @@ static int do_shared_fault(struct mm_str
>
> if (set_page_dirty(fault_page))
> dirtied = 1;
> + /*
> + * Take a local copy of the address_space - page.mapping may be zeroed
> + * by truncate after unlock_page(). The address_space itself remains
> + * pinned by vma->vm_file's reference. We rely on unlock_page()'s
> + * release semantics to prevent the compiler from undoing this copying.
> + */
Looks correct to me.
We need the same comment or reference to this one in do_wp_page().
> mapping = fault_page->mapping;
BTW, I noticed that fault_page here can be a tail page: sound subsytem
allocates its pages with GFP_COMP and maps them with ptes. The problem is
that we never set ->mapping for tail pages and the check below is always
false. It seems doesn't cause any problems right now (looks like ->mapping
is NULL also for head page sound case), but logic is somewhat broken.
I only triggered the problem when tried to reuse ->mapping in first tail
page for compound_mapcount in my thp refcounting rework.
If it sounds right, I will prepare patch to replace the line above and the
same case in do_wp_page() with
mapping = compound_head(fault_page)->mapping;
Ok?
> unlock_page(fault_page);
> if ((dirtied || vma->vm_ops->page_mkwrite) && mapping) {
> _
>
--
Kirill A. Shutemov
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>