"Kirill A. Shutemov" <kirill@shutemov.name> writes:

> On Tue, Feb 11, 2020 at 12:55:50PM -0500, Jeff Moyer wrote:
>> "Kirill A. Shutemov" <kirill@shutemov.name> writes:
>> 
>> > On Tue, Feb 11, 2020 at 11:44:06AM -0500, Jeff Moyer wrote:
>> >> Hi, Justin,
>> >> 
>> >> Justin He <Justin.He@arm.com> writes:
>> >> >> Thanks for the report. But this commit 83d116c53058 doesn't add the
>> >> >> new clear_page code path. Besides the pte_mkyoung part, It just refines
>> >> >> the codes(no functional change) and add a WARN_ON_ONCE to indicate
>> >> >> there is any obscure case before.
>> >> >
>> >> > I can't reproduce it with your provided test file on my arm64 qemu with
>> >> > a pmem device.
>> >> > Could you do me a favor that just revert 83d116c53058 but keep that
>> >> > WARN_ON_ONCE after clear_page()? Is there any difference?
>> >> > Thanks for your help
>> >> 
>> >> Below is the patch I used to put the WARN_ON_ONCE after the clear_page,
>> >> just to be sure that's what you intended.  So with 83d116c53058
>> >> reverted, and the below patch applied, the WARN_ON_ONCE does not
>> >> trigger.
>> >
>> > I cannot explain this. There is no locking to prevent the same scenario
>> > before. It might be an timing difference.
>> >
>> > Could try to put a delay before the copy to make race window larger?
>> 
>> I reverted my change to the reproducer, and now it triggers the warning.
>
> I'm not sure I follow.
>
> My understanding is that you failed to reproduce the issue with
> 83d116c53058 reverted and WARN_ON_ONCE() placed.

I failed to reproduce the issue with the test code I provided in this
email thread.  However, if I simply use the original t_mmap_cow_race
from xfstests, I can trigger the WARN_ON_ONCE.  There is no need to
insert a delay in the kernel.

Does that make sense?

> My ask was to try to put some mdelay() just before
> __copy_from_user_inatomic(). The mdelay() may help with reproducing the
> issue on the old code.
>
> If the bug still fails to reproduce I may misunderstand the source of the
> bug and need to look further.

I understand your request.  Inserting a udelay(10) with the code I
provided does not trigger the warning.  However, see above.

I'm including the unmodified t_mmap_cow_race.c code here for your
convenience.  This is the code that triggers the warning with
83d116c53058 reverted, and the WARN_ON_ONCE added.

Let me know if I'm stilling confusing you.  :)

Cheers,
Jeff