MIME-Version: 1.0
In-Reply-To: <20171210204350.GA25013@amd>
References: <2809506.pL8kVbvXcY@aspire.rjw.lan> <1578405.51lzoSX1jh@aspire.rjw.lan>
 <20171209103325.GA13867@amd> <CA+55aFw8tuoJ2gcXx3K2sKFf2Y9hXX4naMVQNqGOUivnjwhjkg@mail.gmail.com>
 <20171209220110.GA11496@amd> <CA+55aFySAdiBZhZ0PSDjH5PuvPPcMsBRXbxCkObfm1eY7gHDbQ@mail.gmail.com>
 <20171210162305.GA10159@amd> <CA+55aFxc9e=CuwrAzhX7BgV4Or+unTQDXW1Xk-gXOZPiAtN3Bw@mail.gmail.com>
 <20171210185638.GA10363@amd> <CA+55aFy2-vNAXSnSBoiSfvhijKXzYVmpYr1VCOb87+tgrw0B1Q@mail.gmail.com>
 <20171210204350.GA25013@amd>
From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Sun, 10 Dec 2017 13:28:50 -0800
Message-ID: <CA+55aFwK96Eryb2PMoeaLxPSu4srGQakh=jO646Wqv9advLp0A@mail.gmail.com>
Subject: Re: Linux 4.15-rc2: Regression in resume from ACPI S3
To: Pavel Machek <pavel@ucw.cz>
Cc: Zhang Rui <rui.zhang@intel.com>, Andrew Lutomirski <luto@kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Jarkko Nikula <jarkko.nikula@linux.intel.com>,
        "Rafael J. Wysocki" <rjw@rjwysocki.net>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        "the arch/x86 maintainers" <x86@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1056
Lines: 28

On Sun, Dec 10, 2017 at 12:43 PM, Pavel Machek <pavel@ucw.cz> wrote:
>
> For the record... this should fix it. Tested on x60. More tests pending.

This can't be right.

At the very least, now the comment is wrong. And the comment does seem
relevant for 32-bit too:

> -       fix_processor_context();
> -
>         /*
>          * Restore segment registers.  This happens after restoring the GDT
>          * and LDT, which happen in fix_processor_context().

Notice? You've moved down the 32-bit fix_processor_context() call to
after the loadsegment() calls, which smells wrong.

That said, this *all* smells wrong. Why is there a special
fix_processor_context() function at all with different 32-bit and
64-bit behavior? This code is all written to be maximally confusing.

I think this could do with some re-org to make it more logical. That
"some random things done in fix_processor_context(), other random
things done directly in __restore_processor_state()" makes no sense at
all to me. There's no logic to what is done where.

                Linus