From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752526AbcHLVLL (ORCPT ); Fri, 12 Aug 2016 17:11:11 -0400 Received: from mail-sn1nam02on0098.outbound.protection.outlook.com ([104.47.36.98]:56634 "EHLO NAM02-SN1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751010AbcHLVLJ (ORCPT ); Fri, 12 Aug 2016 17:11:09 -0400 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=waiman.long@hpe.com; Message-ID: <57AE3B61.5010202@hpe.com> Date: Fri, 12 Aug 2016 17:10:57 -0400 From: Waiman Long User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:10.0.12) Gecko/20130109 Thunderbird/10.0.12 MIME-Version: 1.0 To: Andy Lutomirski CC: Thomas Gleixner , John Stultz , Ingo Molnar , "linux-kernel@vger.kernel.org" , Borislav Petkov , Jiang Liu , Randy Wright , Scott J Norton , Douglas Hatch , Dave Hansen , Prarit Bhargava , X86 ML , "H. Peter Anvin" Subject: Re: [RESEND PATCH v4] x86/hpet: Reduce HPET counter read contention References: <1470853770-37625-1-git-send-email-Waiman.Long@hpe.com> <57ACD2DE.6080306@intel.com> <57AD0898.7030506@hpe.com> <57AD18D1.1050107@intel.com> <57AE00EE.8070904@hpe.com> <57AE0469.10503@intel.com> <57AE1606.2010304@hpe.com> In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [72.71.243.96] X-ClientProxiedBy: BLUPR0301CA0014.namprd03.prod.outlook.com (10.162.113.152) To TU4PR84MB0319.NAMPRD84.PROD.OUTLOOK.COM (10.162.186.29) X-MS-Office365-Filtering-Correlation-Id: 6e484117-443a-4c97-6610-08d3c2f52cf2 X-Microsoft-Exchange-Diagnostics: 1;TU4PR84MB0319;2:f3KSXxbuP1p2HCgJRnC8cfsK2ar/a0DIWhmfjLkMxaD94dESlhWKm5ZaTZ4I/fd2ilKG6HG/vkmnVyo4+DB62ZbW7szbSgrarRCOUdfPFtOdxGI3OBjv5ZoBgzgyJvVIfuGv7ZJCPH17Bfu20gYcP0rh9p2HxDwLYyIerZLQcKBuHMlQbckRcxGbMxKKzlD5;3:IUp+gmcguwcGRe13JJBJ2tH8RKvosL4rdngfqYyFuk+DUBz2UNzxeH0/g5gk+uqUcqqtDF8TMEB+JRiFBEQo6W3slVM3tcy6I1R43XS38hohWR8D7PaQTNVRr0IrbWcV X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:TU4PR84MB0319; X-Microsoft-Exchange-Diagnostics: 1;TU4PR84MB0319;25:ETbJH/PMzcHqp6H9YykLin1Iu5dPl/kXmVqWNWkzrpdBqVTdTctgEynUiGIwcb+DXyvLlR0lD1J88nE7/bPiWEfwXApv9LwMBXdng87ryp1MepWnYeO0dF9OgAgqg7+E1niKXQ/OaJBQT0F9cNV1Xd/He2VYX8IO1uqQc5ISJkptazX2+x9vYtQl5ZhqQmLPzUqNZIn3beTiRSKSl+i8AUOTwufCAH6f6K0FtxW2wZvEpNf34JWrJeohLicbXQqmIWlxTx3x1YHIdq0wfswGYn8Nl79EtmAzrlIZj41hV06xPAArxkZw2WLaOvogudFYAnCMDkHWtbCtHY9oBgP2RzUGF3Cfq4UHxUgwDxqi0Y1LtKBQpzO9ajK/hEZQH3F7ouTA6coeqUZXjQP+Jo/NqZHt/mGIMuiePfY0RRlc7pw4oTByRsxX+kMuwmYI4//H59VZkWCsZjU9fE2YjfTsp/puoOpkLDVIOkjZnGzUJBQ1qLlz2sb+hx0BEmhCVox/an3TI0nzn1Oyh2sxSHWYibtKRPplHirK07zFoyb2UE4f+OQrptq//V3xIW/MCG48ymVnFZSyOtQSEvHZF0VdrLWYWL6O4Sqcvun8ktnXR51bXSXyyIifAvjbEsjmYGf5Wc+kcut7T4HoVI75NdrCTBa/Q/XSWw8N0qsGbWuGElRAz3obWSwITuYIa1uHZuGCJxOvrMHOxKxf1X8sxaGWBw==;31:OLFwFsK4CiZOzMFmjheKQu7EhtfXgKD0fhTIX9fWbzMJzUoSTwRUXRl617NIInDbFEp/XR5D5rr0NnurIp15FSvO/T7AtQbRFE0dB436ncwf5Jo/efHy71AqiOEO0zipGhP/5Ot/XkXb4LtLoyI3qgoDsLcgzPUvUWtNdxqDjD60BXYYqa7G61QJ38nlLADFvBYh+xhMjtVOJ/Zj/8R0519kS+2+Ej75slQnLtcFoNU= X-Microsoft-Exchange-Diagnostics: 1;TU4PR84MB0319;20:Nj+GSF8Ysm4/UnP70du/yAZO3S4Zu/O9DxI2FFHKbd9iEJwpAaw4LTgZori9f4ZE9eYrSokVrfbX1DUdystpKiXINgmOt6yeOHuaQyrp8tD2z+LQVsCRShugtJTtfTDSPFaXRd/57744i6rrPvYBWCKMOXm/V2FrEzUTra1Kb4DzjdqjLrUfqXNAGEZK0skukxTqUIvbLu1NWILxPIV335ciM0tybxMXrvyzGT+Yg0xlfyHsXPRXEiREf4CpMtxnsMSCAGvn5QtuG3XacJY5Pd0T1vw3O2Bg3n32WGZXcJq/eLIZCSAd+j64AslQvIreu91F40lT4bifl2SZZ7raPk75pra0036ox5h8wYxDq1vaRyis5lvpOR6I2Lqegk17BW2gSJ9/6k6g7GibZ8RcyHkfSY+6fOWLaldZs1LIqVm8mRWCTZD09OYQfwCUfvgFLuk7H5mhLiQ0cbRVqHL3GgXF5SpW/CoTZf18VlPXvUj7vA+UR5aGLbuDekbmIM3S;4:JvO+Ks7C4Msvm9QNh8FJwi7Z1wMw7ZA9fJO+W60fEu4Wnm7tMd4LhR9THZqDRH1ZIfa2a9/tfLwQv/cogWtYYLm7k6rzNSzNbNIQDps93jxYIkO5QSj3o5CAZgFsE4JaLSypjUzYa2wQRLOBFukqqNmlbFieRQwtncVFJZeXz2lmMTqytmEmZpnUHj8ugSJQjz3Xdl/Dnimp7AW+nBSPWzUi76sifI3/ay+NnQI7IGZxCtMNaLuCKBYAYN0OGB7vH6wy9TGu3oQJBNn7ObiHvYQyGHyffWgeRl0p+Y/Gwkn4pijWhiVOAGwV9lrlr77AM/RPA93UcNc/OynKhqkhbFUucxYcCrb7XuSiG4ODIIBb6hD4GLyAO73mRyErJ+3FCfSoM/jERx+WQtUE2u6LRxmhHBoMdmi+x4ySgnna+fylSV12BriMB0RNpPAuNNZI X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(227479698468861); X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(6040176)(601004)(2401047)(8121501046)(5005006)(3002001)(10201501046)(6055026);SRVR:TU4PR84MB0319;BCL:0;PCL:0;RULEID:;SRVR:TU4PR84MB0319; X-Forefront-PRVS: 003245E729 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10019020)(4630300001)(6009001)(6049001)(7916002)(51914003)(24454002)(189002)(199003)(57704003)(51444003)(377454003)(7846002)(66066001)(77096005)(23676002)(42186005)(2950100001)(65956001)(83506001)(7416002)(33656002)(305945005)(7736002)(19580405001)(80316001)(101416001)(19580395003)(50466002)(110136002)(65816999)(65806001)(47776003)(50986999)(87266999)(54356999)(105586002)(76176999)(97736004)(106356001)(68736007)(81166006)(81156014)(6116002)(3846002)(189998001)(4001350100001)(2906002)(59896002)(93886004)(64126003)(586003)(92566002)(36756003)(86362001)(4326007)(8676002)(230700001)(117156001)(217873001);DIR:OUT;SFP:1102;SCL:1;SRVR:TU4PR84MB0319;H:[192.168.142.185];FPR:;SPF:None;PTR:InfoNoRecords;MX:1;A:1;LANG:en; X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtUVTRQUjg0TUIwMzE5OzIzOk5kVUFIajlLQk02WTFzL2poRGczN0JGRWw2?= =?utf-8?B?Ukt3bC8rLzFPZFcvOCtxOTZ0bWtNOUJtUWx4R2owWnB5aGxPYkVkcDQ4S2Ro?= =?utf-8?B?ZUc1MHZzZ3Z4bGxpS09iU1VHV2ZpTzArVWVzNlA3WnBFdytzQ2YwRHBlWVJq?= =?utf-8?B?S2QrZGxnZmR6UjNjN2hkdnUxSFpWSGtZZDhCNWVBYWY5UW9PQjFNRmd6Vjlv?= =?utf-8?B?dERuUHBUeUR5WThvMkt2S21meUNrejJtSVp4UndDckUzNUdoWkVFaUl6QVBD?= =?utf-8?B?S0FodVNITE8zSWRNOEtnQzFjMDBEY1pYc0pPTWRHV2JUZlAzMlAzN2FPNEJD?= =?utf-8?B?WVdvWE9iWkt6OUt6VTAvamNVdVlianlFVlVJS2NyYUJjZFlzZHdmVWtEd0dB?= =?utf-8?B?cDcwUjd3S0pyNzVBNkNlYys4bDFhNTNnQ2wxVWhheGw4VzNXVkVCTGRFcmNE?= =?utf-8?B?TGxac1pOdjV4bzd2aGRGaUpZN3cyaTJhQndNT3JGZDY2Z3pnSUdBaWYxMWlV?= =?utf-8?B?Y3pUb21jTW5vNGpWWnJLUzM4REJ1QkUzcnpFRThDc3BoQkgwRUhTdnl5Qkl2?= =?utf-8?B?M0VLdUJxd1M1R201VVhwbGFFazRhZFhWWmh1ajg1bFJDcW83MFk2VVBhWkE5?= =?utf-8?B?dGNMdEY5M3FtcVFFOUZoVFBON3ZjV0JKZVkyYm1ZTmNmVXRTdTVCZTgxOHZH?= =?utf-8?B?Z1pRN2ZlWjMzcGNPbUJsSHpDVStZemEvUlJuTC9xTXBqZlFrclkrVVgreHVt?= =?utf-8?B?Vzl5a3lHMC9HeTRFYnBweE9TTEhJUUFvTUFhdGVRMmRPQmE2VHc2dXd4ektr?= =?utf-8?B?VUpCRno5cmRZSVhSaHdkUmE2cWJWZER3WG9zZ1Y2dUd5dGdSeUpOSmoxK1Ru?= =?utf-8?B?cmY1TGJTMm9MelJVZEdzZ1YyS29ET1prOVdUWnJyNmw1UlozODBIZ20wbW5s?= =?utf-8?B?SnNJRkpzYnN5Y3NZSEFHc3prSDk2WXBMUEdGQ3JES2ZCbVYwSkNNcFRoZDRU?= =?utf-8?B?akptNGRjT0Vvb2VkY0E4K05HSnR0a3NUMDgzOUxSaUYyc1gzZEEvS3dGSXls?= =?utf-8?B?ZGJlSzZQSUFQekJqVk9uSUFxcUhWclNCZlQvQ2tpK3RkY0tXM1BJZ3ZWMnR1?= =?utf-8?B?cFV1UmVaazdCYnpNQW43N3cyOGpKcWtqYkg1UG1pRDNNd0FuNzBrU1FHeU5K?= =?utf-8?B?REZNSHh6RzVMYkYzVnJ3b1hzdWsyeXN1Uk1iTG9OemVoV1RrSm15ZmF2WFhB?= =?utf-8?B?Y0FkZkZUUzUweHNTQVhyTUpFTitITFBqVmpmdTBOcE9pbXdwUGlseVpTTjI0?= =?utf-8?B?N2E5OUI4eldrTi93dUNpaWVhOGVjRHVVbGxWTWNPa3hkQ1ZCTlk0VVcyRkxS?= =?utf-8?B?ZHE3WGoxOHlyZ01lZDBzZ1IxK2RzeTlMTE5UWkI0Y1lwYVJxSGRZa2V5RXhW?= =?utf-8?B?U01iMyt3eGFIK1Jha0JKQWE2Q2NGajhFWG1QakZVTy9WM3c4RDRmVDQ5YURz?= =?utf-8?B?OTdPZVpuakkveGJsankzWGhkSDlvRXJId2tOOFI3dnpTUDl3Y1BnN09wckJl?= =?utf-8?B?WWV5aS9Fa1BtNGxjeXdJZDBLeko5Q3NGV3NvSDZOOTN5QmI5d2czaHFKZDNn?= =?utf-8?B?eEk1dktkMkVBeVE0K3BiMXNlWHBjb3FheDdPeGFMSnJMYmhKSWlXUXljZTJZ?= =?utf-8?B?V1Nxc29NeW9jNWxsSGdLMkRaZ09Ib09tWnBtdmJpa1VTNUJadXZoSWJ4My9P?= =?utf-8?B?MTcxL09CL1JhU3gyZjBBMmJBa2VRaFlONkI4UFBxUDBZTDJQTkt1N041Nm12?= =?utf-8?B?aVk0eUR3Z2Nwc0V6L21jS3haR1hWRXhwT0RGVGZ5dVBkTXp5VXNDR1EzMnc4?= =?utf-8?B?L0daMnpEQUo3RVZwdEQ3LzJneGNvYkFrZjdzUk5oNmZrTytDSk1rYzdyZFBY?= =?utf-8?Q?rjUWk5z3QNTr1/BM9aqdiJbWvxqvJg=3D?= X-Microsoft-Exchange-Diagnostics: 1;TU4PR84MB0319;6:JMM6hyu+tbeYAkDcjq3DMI4QflsuaSieSV1DUwxROSUIXyaHF//i0HW62lvAplSbFfNHcLD1ihThkfv26+ydPteYt5/FXd5Cgoc7sufXbfGK3QjCP/qogXEG01w5VQ/K243qip34ZsFtJBZDt60wjrD8piGg+anBFYMZh5mlGIapluLGn7RPuawbKhKlmGINJyDEgghKPy7STN/+rJYJNeH6p3cv7HtjGHqd9mjYb/XqsmUguvLhKEtFs2b5LainCQJHlNmng/NiVuKL+ELqjOKF1DFjkmSs9sCvjNdmgo5zjh0a6Ktwk6nFOzIp99oiu7hoGjkrsfI962Zu7qj1JQ==;5:wW+UxDQKkRxhVhHJQLwF/ngUcZfVSc2fLB1blgRtu/D9GE3MTouYe/UzE6P/wGSzXzG0LIcqpJYgVijyYK9dM6tQCWZclC25ljRqRwgQf2X5KrOH8BNqu0ViqRiEwLRSjix3FRB0oakHVUqW6gq3ZQ==;24:PxwqjXXIFeg6DP6sUFxWhIhmhuaPrPOs42rSlbsTdD70Mbubv7z0VeECZbwOktk1DsOGu6ViLooZFxt+TCZEMpFF1Lr3kMO7/Qsm/4qlV6s=;7:yfUwHPEtxjq+64S+Wl/ekDc7KEX5ymfJE8ffLHbk2dJanomCkwEru6lDW1Ybb+ERFmwnwBnzkTVtX0LceLsHUK7qEPnRwkCaYYyWU1WMTWDER4Oi8AHZDngHHe6yN+NQFyx4IMVLVx3SfF4kAuwGUMG7XrjK4LCUjgkJGT4D8yrInLWcL4TEriJujN1qUidAFdCmz78LOibeXOE7CzNJYQ7Ag4gVkFrx6xYNgbUYwmd7nji5oqtzTr7rH0YBVGky SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: hpe.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 Aug 2016 21:11:04.1589 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: TU4PR84MB0319 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 08/12/2016 04:18 PM, Andy Lutomirski wrote: > On Aug 12, 2016 9:31 PM, "Waiman Long" wrote: >> On 08/12/2016 01:16 PM, Dave Hansen wrote: >>> On 08/12/2016 10:01 AM, Waiman Long wrote: >>>> The reason for using a special lock is that I want both sequence number >>>> update and locking to be done together atomically. They can be made >>>> separate as is in the seqlock. However, that will make the code more >>>> complex to make sure that all the threads see a consistent set of lock >>>> state and sequence number. >>> Why do we need a sequence number? The "cached" HPET itself could be used. >>> >>> I'm thinking something like below could use a spinlock instead of the >>> doing a custom cmpxchg sequence. The spin_is_locked() should allow the >>> contended "readers" to avoid using atomics. >>> >>> spinlock_t hpet_lock; >>> u32 hpet_value; >>> ... >>> { >>> u32 old_hpet = READ_ONCE(hpet_value); >>> u32 new_hpet; >>> >>> // need to ensure that the spin_is_locked() is ordered after >>> // the READ_ONCE(). >>> smp_rmb(); >>> // spin_is_locked() doesn't do atomics >>> if (!spin_is_locked(&hpet_lock)&& spin_trylock(&hpet_lock)) { >>> >>> WRITE_ONCE(hpet_value, real_read_hpet()); >>> spin_unlock(&hpet_lock); >>> return hpet_value; >>> } >>> // Contended case. We spin here waiting for the guy who holds >>> // the lock to write a new value to 'hpet_value'. >>> // >>> // We know that our old_hpet is older than our check for the >>> // spinlock being locked. So, someone must either have already >>> // updated it or be updating it. >>> do { >>> cpu_relax(); >>> // We do not do a rmb() here. We don't need a guarantee >>> // that this read is up-to-date, just that it will >>> // _eventually_ see an up-to-date value. >>> new_hpet = READ_ONCE(hpet_value); >>> } while (old_hpet == new_hpet); >>> return new_hpet; >>> } >> >> Yes, I think that work too. I will update my patch accordingly. Thanks for the input. > Why is Dave more convincing than I was a couple months ago when I > asked a similar question? :) I thought I made some changes in accordance with your comments previously. Maybe I missed something:-[ > I don't think this is right. If the HPET ever returns the same value > twice in a row (unlikely because it's generally too slow to read, but > it's plausible that someone will make a fast HPET some day), then this > could deadlock. What is the deadlock scenario you are talking about? > > Also, does this code need to be NMI-safe? This implementation is > deadlocky if it's called from an NMI. The code isn't NMI-safe as it is not maskable. So is the original code. We can check the interrupt state and read HPET directly if in NMI. > > The original code was wait-free, right? That was a nice property, too. > > --Andy The v4 patch isn't wait-free as need to make sure that we read the new HPET value instead of the stale one. Cheers, Longman