Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <alpine.LSU.2.11.2012231905300.5723@eggly.anvils>
 <20201225113157.e7hmluffh56fszfc@box> <CAHk-=wiT50aEErZgZOrbFQ=GhFuM3MnBmHoSBSScN9rmXMnOKQ@mail.gmail.com>
 <20201226204335.dikqkrkezqet6oqf@box> <alpine.LSU.2.11.2012261246450.1629@eggly.anvils>
 <CAHk-=wjesveWEQZ4tqRssSSQvuxx46LqYfME+uxKfghxAe6U_w@mail.gmail.com>
 <20201226224016.dxjmordcfj75xgte@box> <alpine.LSU.2.11.2012261623140.1022@eggly.anvils>
 <alpine.LSU.2.11.2012261816520.1071@eggly.anvils> <CAHk-=wjHvipz5DqWUFP5zuPK-kWM4QD-eokf75V8NGNATnn+BQ@mail.gmail.com>
 <X+jvZchuTptrxkCH@fedora.tometzki.de> <alpine.LSU.2.11.2012271418460.1091@eggly.anvils>
 <CAHk-=whykpz1RfBZ8Cqk=80_d6BWsppGYBJXoWuWB9n+uUKxVA@mail.gmail.com>
In-Reply-To: <CAHk-=whykpz1RfBZ8Cqk=80_d6BWsppGYBJXoWuWB9n+uUKxVA@mail.gmail.com>
From:   Linus Torvalds <torvalds@linux-foundation.org>
Date:   Sun, 27 Dec 2020 15:40:36 -0800
Message-ID: <CAHk-=wjvHxQDet0=DTkqmYni_fgDsG=2_-idECFxnd+ECKdg_g@mail.gmail.com>
Subject: Re: [PATCH 1/2] mm: Allow architectures to request 'old' entries when prefaulting
To:     Hugh Dickins <hughd@google.com>,
        Andrew Morton <akpm@linux-foundation.org>
Cc:     Damian Tometzki <linux@tometzki.de>,
        "Kirill A. Shutemov" <kirill@shutemov.name>,
        Matthew Wilcox <willy@infradead.org>,
        "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
        Will Deacon <will@kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Linux-MM <linux-mm@kvack.org>,
        Linux ARM <linux-arm-kernel@lists.infradead.org>,
        Catalin Marinas <catalin.marinas@arm.com>,
        Jan Kara <jack@suse.cz>, Minchan Kim <minchan@kernel.org>,
        Vinayak Menon <vinmenon@codeaurora.org>,
        Android Kernel Team <kernel-team@android.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Sun, Dec 27, 2020 at 3:12 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Ok, your fix for that folded in, and here's yet another version.

Still not good.

I don't know what happened, but the change of

-       vm_fault_t ret = 0;
+       vm_fault_t ret;

is very very wrong. The next user is

+       if (!(vma->vm_flags & VM_SHARED))
+               ret = check_stable_address_space(vma->vm_mm);
+       if (ret)
+               return ret;

so now 'ret' will potentially be used uninitialized (although this is
the kind of thing that a compiler might almost accidentally end up
fixing - with a single dominating assignment, I could imagine the
compiler moving the test to that assignment and thus "fixing" the code
without really even meaning to).

I think Kirill was intending to move the "if (ret)" up into the path
that sets it, IOW something like

+       if (!(vma->vm_flags & VM_SHARED)) {
+               ret = check_stable_address_space(vma->vm_mm);
+               if (ret)
+                       return ret;
+       }

instead. But that patch as-is is broken.

Kirill?

                 Linus