Messages in this thread | | | Date | Fri, 16 Jun 2017 23:53:33 +0900 | From | Minchan Kim <> | Subject | Re: [PATCHv2 3/3] mm: Use updated pmdp_invalidate() inteface to track dirty/accessed bits |
| |
Hi Andrea,
On Fri, Jun 16, 2017 at 04:27:20PM +0200, Andrea Arcangeli wrote: > Hello Minchan, > > On Fri, Jun 16, 2017 at 10:52:09PM +0900, Minchan Kim wrote: > > > > > @@ -1995,8 +1984,6 @@ static void __split_huge_pmd_locked(struct vm_area_struct *vma, pmd_t *pmd, > > > > > if (soft_dirty) > > > > > entry = pte_mksoft_dirty(entry); > > > > > } > > > > > - if (dirty) > > > > > - SetPageDirty(page + i); > > > > > pte = pte_offset_map(&_pmd, addr); > [..] > > > > split_huge_page set PG_dirty to all subpages unconditionally? > > If it's true, yes, it doesn't break MADV_FREE. However, I didn't spot > > that piece of code. What I found one is just __split_huge_page_tail > > which set PG_dirty to subpage if head page is dirty. IOW, if the head > > page is not dirty, tail page will be clean, too. > > Could you point out what routine set PG_dirty to all subpages unconditionally? > > On a side note the snippet deleted above was useless, as long as > there's one left hugepmd to split, the physical page has to be still > compound and huge and as long as that's the case the tail pages > PG_dirty bit is meaningless (even if set, it's going to be clobbered > during the physical split).
I got it during reviewing this patch. That's why I didn't argue this patch would break MADV_FREE by deleting routine which propagate dirty to pte of subpages. However, although it's useless, I prefer not removing the transfer of dirty bit. Because it would help MADV_FREE users who want to use smaps to know how many of pages are not freeable (i.e, dirtied) since MADV_FREE although it is not 100% correct.
> > In short PG_dirty is only meaningful in the head as long as it's > compound. The physical split in __split_huge_page_tail transfer the > head value to the tails like you mentioned, that's all as far as I can > tell.
Thanks for the comment. Then, this patch is to fix MADV_FREE's bug which has lost dirty bit by transferring dirty bit too early.
Thanks.
| |