Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1710950 > unrolled thread

Re: [PATCH 2/2] mm, oom: fix potential data corruption when oom_reaper races with writer

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 2/2] mm, oom: fix potential data corruption when  oom_reaper races with writer Michal Hocko <mhocko@kernel.org> - 2017-08-14 16:00 +0200
    Re: Re: [PATCH 2/2] mm, oom: fix potential data corruption when  oom_reaper races with writer Michal Hocko <mhocko@kernel.org> - 2017-08-15 09:00 +0200
    Re: Re: [PATCH 2/2] mm, oom: fix potential data corruption when  oom_reaper races with writer Michal Hocko <mhocko@kernel.org> - 2017-08-15 10:50 +0200
      Re: Re: Re: [PATCH 2/2] mm, oom: fix potential data corruption when  oom_reaper races with writer Michal Hocko <mhocko@kernel.org> - 2017-08-15 14:30 +0200
        Re: Re: Re: Re: [PATCH 2/2] mm, oom: fix potential data corruption  when oom_reaper races with writer Michal Hocko <mhocko@kernel.org> - 2017-08-17 16:00 +0200

#1710950 — Re: [PATCH 2/2] mm, oom: fix potential data corruption when oom_reaper races with writer

On Sat 12-08-17 00:46:18, Tetsuo Handa wrote:
> Michal Hocko wrote:
> > On Fri 11-08-17 16:54:36, Tetsuo Handa wrote:
> > > Michal Hocko wrote:
> > > > On Fri 11-08-17 11:28:52, Tetsuo Handa wrote:
> > > > > Will you explain the mechanism why random values are written instead of zeros
> > > > > so that this patch can actually fix the race problem?
> > > > 
> > > > I am not sure what you mean here. Were you able to see a write with an
> > > > unexpected content?
> > > 
> > > Yes. See http://lkml.kernel.org/r/201708072228.FAJ09347.tOOVOFFQJSHMFL@I-love.SAKURA.ne.jp .
> > 
> > Ahh, I've missed that random part of your output. That is really strange
> > because AFAICS the oom reaper shouldn't really interact here. We are
> > only unmapping anonymous memory and even if a refault slips through we
> > should always get zeros.
> > 
> > Your test case doesn't mmap MAP_PRIVATE of a file so we shouldn't even
> > get any uninitialized data from a file by missing CoWed content. The
> > only possible explanations would be that a page fault returned a
> > non-zero data which would be a bug on its own or that a file write
> > extend the file without actually writing to it which smells like a fs
> > bug to me.
> 
> As I wrote at http://lkml.kernel.org/r/201708112053.FIG52141.tHJSOQFLOFMFOV@I-love.SAKURA.ne.jp ,
> I don't think it is a fs bug.

Were you able to reproduce with other filesystems? I wonder what is
different in my testing because I cannot reproduce this at all. Well, I
had to reduce the number of competing writer threads to 128 because I
quickly hit the trashing behavior with more of them (and 4 CPUs). I will
try on a larger machine.
-- 
Michal Hocko
SUSE Labs

[toc] | [next] | [standalone]

#1711912

On Tue 15-08-17 07:51:02, Tetsuo Handa wrote:
> Michal Hocko wrote:
[...]
> > Were you able to reproduce with other filesystems?
> 
> Yes, I can reproduce this problem using both xfs and ext4 on 4.11.11-200.fc25.x86_64
> on Oracle VM VirtualBox on Windows.
> 
> I believe that this is not old data from disk, for I can reproduce this problem
> using newly attached /dev/sdb which has never written any data (other than data
> written by mkfs.xfs and mkfs.ext4).
> 
>   /dev/sdb /tmp ext4 rw,seclabel,relatime,data=ordered 0 0
>   
> The garbage pattern (the last 4096 bytes) is identical for both xfs and ext4.

Thanks a lot for retesting. It is now obvious that FS doesn't have
anything to do with this issue which is in line with my investigation
from yesterday and Friday. I simply cannot see any way the file position
would be updated with a zero length write. So this must be something
else. I have double checked the MM side of the page fault I couldn't
find anything there either so this smells like a stray pte while the
underlying page got reused or something TLB related.
 
> >                                                    I wonder what is
> > different in my testing because I cannot reproduce this at all. Well, I
> > had to reduce the number of competing writer threads to 128 because I
> > quickly hit the trashing behavior with more of them (and 4 CPUs). I will
> > try on a larger machine.
> 
> I don't think a larger machine is necessary.
> I can reproduce this problem with 8 competing writer threads on 4 CPUs.

OK, I will try with fewer writers which should make it easier to have it
run for long time without any trashing.
 
> I don't have native Linux environment. Maybe that is the difference.
> Can you try VMware Workstation Player or Oracle VM VirtualBox environment?

Hmm, I do not have any of those handy for use, unfortunately. I will
keep focusing on the native HW and KVM for today.

Thanks!
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1711962

On Tue 15-08-17 07:51:02, Tetsuo Handa wrote:
> Michal Hocko wrote:
[...]
> > Were you able to reproduce with other filesystems?
> 
> Yes, I can reproduce this problem using both xfs and ext4 on 4.11.11-200.fc25.x86_64
> on Oracle VM VirtualBox on Windows.

Just a quick question.
http://lkml.kernel.org/r/201708112053.FIG52141.tHJSOQFLOFMFOV@I-love.SAKURA.ne.jp
mentioned next-20170811 kernel and this one 4.11. Your original report
as a reply to this thread
http://lkml.kernel.org/r/201708072228.FAJ09347.tOOVOFFQJSHMFL@I-love.SAKURA.ne.jp
mentioned next-20170728. None of them seem to have this fix
http://lkml.kernel.org/r/20170807113839.16695-3-mhocko@kernel.org so let
me ask again. Have you seen an unexpected content written with that
patch applied?
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1712136

On Tue 15-08-17 19:06:28, Tetsuo Handa wrote:
> Michal Hocko wrote:
> > On Tue 15-08-17 07:51:02, Tetsuo Handa wrote:
> > > Michal Hocko wrote:
> > [...]
> > > > Were you able to reproduce with other filesystems?
> > > 
> > > Yes, I can reproduce this problem using both xfs and ext4 on 4.11.11-200.fc25.x86_64
> > > on Oracle VM VirtualBox on Windows.
> > 
> > Just a quick question.
> > http://lkml.kernel.org/r/201708112053.FIG52141.tHJSOQFLOFMFOV@I-love.SAKURA.ne.jp
> > mentioned next-20170811 kernel and this one 4.11. Your original report
> > as a reply to this thread
> > http://lkml.kernel.org/r/201708072228.FAJ09347.tOOVOFFQJSHMFL@I-love.SAKURA.ne.jp
> > mentioned next-20170728. None of them seem to have this fix
> > http://lkml.kernel.org/r/20170807113839.16695-3-mhocko@kernel.org so let
> > me ask again. Have you seen an unexpected content written with that
> > patch applied?
> 
> No. All non-zero non-0xFF values are without that patch applied.
> I want to confirm that that patch actually fixes non-zero non-0xFF values
> (so that we can have better patch description for that patch).

OK, so I have clearly misunderstood you. I thought that you can still
see corrupted content with the patch _applied_. Now I see why I couldn't
reproduce this...

Now I also understand what you meant when asking for an explanation. I
can only speculate how we could end up with the non-zero page previously
but the closest match would be that the page got unmapped and reused by
a different path and a stalled tlb entry would leak the content. Such a
thing would happen if we freed the page _before_ we flushed the tlb
during unmap.

Considering that oom_reaper is relying on unmap_page_range which seems
to be doing the right thing wrt. flushing vs. freeing ordering (enforced
by the tlb_gather) I am wondering what else could go wrong but I vaguely
remember there were some races between THP and MADV_DONTNEED in the
past. Maybe we have hit an incarnation of something like that. Anyway
the oom_reaper doesn't try to be clever and it only calls to
unmap_page_range which should be safe from that context.

The primary bug here was that we allowed to refault an unmmaped memory
and that should be fixed by the patch AFAICS. If there are more issues
we should definitely track those down but those should be oom_reaper
independent because we really do not do anything special here.

-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1714054 — Re: Re: Re: Re: [PATCH 2/2] mm, oom: fix potential data corruption when oom_reaper races with writer

On Tue 15-08-17 21:58:29, Tetsuo Handa wrote:
[...]
> If I apply this patch, I can no longer reproduce this problem even with btrfs.
> 
> -+ * and could cause a memory corruption (zero pages instead of the
> -+ * original content).
> ++ * and could cause a memory corruption (random content instead of the
> ++ * original content).

If anything then I would word it this way

and could cause a memory corruption (zero pages for refaults but even a
random content has been observed but never explained properly)

> Tested-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>

Thanks
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web