Re: [PATCH v2] MIPS: Expand MIPS32 ASIDs to 64 bits

From: Paul Burton <paul.burton@mips.com>
To: Paul Burton <pburton@wavecomp.com>
Cc: "linux-mips@vger.kernel.org" <linux-mips@vger.kernel.org>,
	James Hogan <jhogan@kernel.org>,
	Paul Burton <pburton@wavecomp.com>,
	Jiwei Sun <jiwei.sun@windriver.com>,
	Yu Huabing <yhb@ruijie.com.cn>,
	"stable@vger.kernel.org" <stable@vger.kernel.org>,
	"linux-mips@vger.kernel.org" <linux-mips@vger.kernel.org>
Subject: Re: [PATCH v2] MIPS: Expand MIPS32 ASIDs to 64 bits
Date: Fri, 14 Dec 2018 19:03:16 +0000	[thread overview]
Message-ID: <MWHPR2201MB1277A6B7C058DC7A55CE3F23C1A10@MWHPR2201MB1277.namprd22.prod.outlook.com> (raw)
In-Reply-To: <20181204234333.21243-1-paul.burton@mips.com>

Hello,

Paul Burton wrote:
> ASIDs have always been stored as unsigned longs, ie. 32 bits on MIPS32
> kernels. This is problematic because it is feasible for the ASID version
> to overflow & wrap around to zero.
> 
> We currently attempt to handle this overflow by simply setting the ASID
> version to 1, using asid_first_version(), but we make no attempt to
> account for the fact that there may be mm_structs with stale ASIDs that
> have versions which we now reuse due to the overflow & wrap around.
> 
> Encountering this requires that:
> 
> 1) A struct mm_struct X is active on CPU A using ASID (V,n).
> 
> 2) That mm is not used on CPU A for the length of time that it takes
> for CPU A's asid_cache to overflow & wrap around to the same
> version V that the mm had in step 1. During this time tasks using
> the mm could either be sleeping or only scheduled on other CPUs.
> 
> 3) Some other mm Y becomes active on CPU A and is allocated the same
> ASID (V,n).
> 
> 4) mm X now becomes active on CPU A again, and now incorrectly has the
> same ASID as mm Y.
> 
> Where struct mm_struct ASIDs are represented above in the format
> (version, EntryHi.ASID), and on a typical MIPS32 system version will be
> 24 bits wide & EntryHi.ASID will be 8 bits wide.
> 
> The length of time required in step 2 is highly dependent upon the CPU &
> workload, but for a hypothetical 2GHz CPU running a workload which
> generates a new ASID every 10000 cycles this period is around 248 days.
> Due to this long period of time & the fact that tasks need to be
> scheduled in just the right (or wrong, depending upon your inclination)
> way, this is obviously a difficult bug to encounter but it's entirely
> possible as evidenced by reports.
> 
> In order to fix this, simply extend ASIDs to 64 bits even on MIPS32
> builds. This will extend the period of time required for the
> hypothetical system above to encounter the problem from 28 days to
> around 3 trillion years, which feels safely outside of the realms of
> possibility.
> 
> The cost of this is slightly more generated code in some commonly
> executed paths, but this is pretty minimal:
> 
> | Code Size Gain | Percentage
> -----------------------|----------------|-------------
> decstation_defconfig |           +270 | +0.00%
> 32r2el_defconfig |           +652 | +0.01%
> 32r6el_defconfig |          +1000 | +0.01%
> 
> I have been unable to measure any change in performance of the LMbench
> lat_ctx or lat_proc tests resulting from the 64b ASIDs on either
> 32r2el_defconfig+interAptiv or 32r6el_defconfig+I6500 systems.
> 
> Signed-off-by: Paul Burton <paul.burton@mips.com>
> Suggested-by: James Hogan <jhogan@kernel.org>
> References: https://lore.kernel.org/linux-mips/80B78A8B8FEE6145A87579E8435D78C30205D5F3@fzex.ruijie.com.cn/
> References: https://lore.kernel.org/linux-mips/1488684260-18867-1-git-send-email-jiwei.sun@windriver.com/
> Cc: Jiwei Sun <jiwei.sun@windriver.com>
> Cc: Yu Huabing <yhb@ruijie.com.cn>
> Cc: stable@vger.kernel.org # 2.6.12+

Applied to mips-next.

Thanks,
    Paul

[ This message was auto-generated; if you believe anything is incorrect
  then please email paul.burton@mips.com to report it. ]