On Tue, 16 Dec 2014 01:55:32 +0200 "Kirill A. Shutemov" <kirill@xxxxxxxxxxxxx> wrote: > 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(). Can you please send a patch some time? > > 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. hm, why does it use __GFP_COMP? It could just use plain old alloc_pages(GFP_KERNEL) then set up a pte per 4k page? > 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? Generally I don't think we should encourage (or even permit) random driver code to use somewhat-internal-to-MM features unless they really need to. But I note that a lot of drivers are allocating with __GFP_COMP. Why is this? -- 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>