On Mon, Sep 09, 2024 at 03:25:46PM -0700, Andrew Morton wrote:
> On Tue, 3 Sep 2024 17:23:38 -0400 Peter Xu <peterx@xxxxxxxxxx> wrote:
>
> > > > @@ -1686,8 +1706,11 @@ int copy_huge_pud(struct mm_struct *dst_mm, struct mm_struct *src_mm,
> > > > * TODO: once we support anonymous pages, use
> > > > * folio_try_dup_anon_rmap_*() and split if duplicating fails.
> > > > */
> > > > - pudp_set_wrprotect(src_mm, addr, src_pud);
> > > > - pud = pud_mkold(pud_wrprotect(pud));
> > > > + if (is_cow_mapping(vma->vm_flags) && pud_write(pud)) {
> > > > + pudp_set_wrprotect(src_mm, addr, src_pud);
> > > > + pud = pud_wrprotect(pud);
> > > > + }
> > > Do we need the logic to clear dirty bit in the child as that in
> > > __copy_present_ptes()? (and also for the pmd's case).
> > >
> > > e.g.
> > > if (vma->vm_flags & VM_SHARED)
> > > pud = pud_mkclean(pud);
> >
> > Yeah, good question. I remember I thought about that when initially
> > working on these lines, but I forgot the details, or maybe I simply tried
> > to stick with the current code base, as the dirty bit used to be kept even
> > in the child here.
> >
> > I'd expect there's only performance differences, but still sounds like I'd
> > better leave that to whoever knows the best on the implications, then draft
> > it as a separate patch but only when needed.
>
> Sorry, but this vaguensss simply leaves me with nowhere to go.
>
> I'll drop the series - let's revisit after -rc1 please.
Andrew, would you please explain why it needs to be dropped?
I meant in the reply that I think we should leave that as is, and I think
so far nobody in real life should care much on this bit, so I think it's
fine to leave the dirty bit as-is.
I still think whoever has a better use of the dirty bit and would like to
change the behavior should find the use case and work on top, but only if
necessary.
At least this whole fork() path is not useful at all for the use case we're
working on. Please still consider having this series as I think it's useful.
Thanks,
--
Peter Xu
