From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751587AbdISO0w (ORCPT ); Tue, 19 Sep 2017 10:26:52 -0400 Received: from mail-db5eur01on0126.outbound.protection.outlook.com ([104.47.2.126]:3328 "EHLO EUR01-DB5-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751227AbdISO0u (ORCPT ); Tue, 19 Sep 2017 10:26:50 -0400 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=aryabinin@virtuozzo.com; Subject: Re: [PATCH 3/3] kcov: remove useless barrier()s To: Dmitry Vyukov Cc: Andrew Morton , Andrey Konovalov , Victor Chibotaru , syzkaller , Mark Rutland , LKML References: <20170919124648.28963-1-aryabinin@virtuozzo.com> <20170919124648.28963-3-aryabinin@virtuozzo.com> From: Andrey Ryabinin Message-ID: <62a9c122-1527-3466-bbb9-4e2035a160e4@virtuozzo.com> Date: Tue, 19 Sep 2017 17:29:40 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [195.214.232.6] X-ClientProxiedBy: AM5PR06CA0015.eurprd06.prod.outlook.com (2603:10a6:206:2::28) To DB6PR08MB2824.eurprd08.prod.outlook.com (2603:10a6:6:1d::27) X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 42f5fcaf-aab1-48ac-04be-08d4ff6a7600 X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:(300000500095)(300135000095)(300000501095)(300135300095)(22001)(300000502095)(300135100095)(2017030254152)(300000503095)(300135400095)(201703131423075)(201703031133081)(201702281549075)(300000504095)(300135200095)(300000505095)(300135600095)(300000506095)(300135500095);SRVR:DB6PR08MB2824; X-Microsoft-Exchange-Diagnostics: 1;DB6PR08MB2824;3:D6tyZGwiPMnJdlv1Bv24kEEga1a1R/I/zDhU+oxFDqwrkEghlRSrzrsX92msWxa/imBMYUCJ+c3CkSX4mUTVHhQNXDwHmLydwEYur67a2o+q6tHgXoGujM2g9cC/mQML7B5we01v9VF9P85j24xa1rUfhnpsqIcQtlB7y73adaN2CdZ/9dw0M5AAJP8sDVi1KuYXFXBCheon7cJqWMKTA2BQf3C7vABawVcLYZxVY+XFyMwR0qFvRLWPo3BZpBZg;25:3OiK9DJ7gpfC3iBgIAjZJVS7WjsCEriQraYHg3b3ngxmMbDK/ndieiDaG4UkJrVmVosBx4B4g5BNRn/5qnXWJF21dwi+RL8qvtbGfxsok7GZVoluesi/KpvEpuovAjSkG0A6biJsapb8wirMWfGs3O/EN22bp9Ge4XBq+ydE6q5E4o5w5MszG5meA0b+wFvaaL7FPrUuJ5w1AWc4zPp9B1yL/w3Olfz7VteVRsQBMleQxLrab+yrgtC3B43pt0e8s/wJfte8f9aLFJW9F34wacI7ohlItoCxvS1hoeu07a8GSMhRjFn9t7Ue/xVTTYCchmyXko5olWAKDHfBSw06jw==;31:Y7Rzho/ay8nRdFEM6pGbZeDHDjfZrxeXAILCTh8NLAT3LSwesBObX3/qGT9TKMS0h51r1GVQhJuMPuVIOPQMeEKp48tFu2b/dDUNVY15diAkBHefqYFRvW7oUfQ4i2szOOs0kezLzmrbtmBOZiQBo7ANV6R3QXs7yrPW2oOEFv9e5DSjYLHm0cwTVMPAud9/+SinbazSEhFfFdxuE7xOPnJ8nEvnNDZfLQV0VZJWYIw= X-MS-TrafficTypeDiagnostic: DB6PR08MB2824: X-Microsoft-Exchange-Diagnostics: 1;DB6PR08MB2824;20:rOZR5gw5tCDZcg8D1QdTDF1aJ1k5RmTsphw/XbbQtqLrZ0VLDFhGoOpnSODcy8iKi2Ocuu16wtNMOXQCN+zYlLVIgJ6MW8oPK8SKF0d+wT2RvIRGnixyqqbr5aq79C8C0z5nTQe9Jos57C9tvtOb45Tiy8unBBkMNAWNRsfYM+HXR/c81Lq+nszes9LVU+NunvQptK6bbyvDmpzvIUn4q42zRPiisMBKl6kOtUO9RFzSq1FzSDIbzN710A+VQcmUGZIn4Ql4pgmvm4Zy4xUoTPP0R0zd/2nU+oHFbwIDXIGRITe4ZV11O7DTmTIgzT1s676+NZKqEda8hdF9ezi8ua30st3MpGIixOzJkCRdWLGO3+83lTuCcA9CbxeqXqilCOx77H4rv8Fps2Ir0SbhuecZF8TP52PSn+8RqLSqm7w=;4:GFScjyupaMpdpra6xhM91yuJsUeGFS00AkzOiwE+8Gk9LTPK0oFADyuCuUGoT94hKSxdb0rvNOqn5+trlnVbnW9jSzusosJiXJ0pPqgEYH9i0180R2UbmwFMkSkrI9kplwc3GPOARhsNEf2ZSQmczN0WXEWLK+lqWrz2UPtsdomw7qP9SRI6JUtqalyihI+CLDJ9qCXRECH9DnN0Zwu7PjJLJ948IHrH/uHdizgJ5U6zC7Cs2IoAiam3YJUn7U2F X-Exchange-Antispam-Report-Test: UriScan:; X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(6040450)(2401047)(5005006)(8121501046)(3002001)(10201501046)(93006095)(93001095)(100000703101)(100105400095)(6041248)(20161123560025)(20161123564025)(20161123555025)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(20161123558100)(20161123562025)(6072148)(201708071742011)(100000704101)(100105200095)(100000705101)(100105500095);SRVR:DB6PR08MB2824;BCL:0;PCL:0;RULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095);SRVR:DB6PR08MB2824; X-Forefront-PRVS: 04359FAD81 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10019020)(7370300001)(4630300001)(6049001)(6009001)(346002)(376002)(51444003)(199003)(377454003)(189002)(24454002)(66066001)(4326008)(53936002)(65956001)(93886005)(16526017)(106356001)(6486002)(65806001)(6246003)(23676002)(33646002)(478600001)(105586002)(25786009)(77096006)(53546010)(86362001)(47776003)(2906002)(3846002)(83506001)(6116002)(6666003)(316002)(16576012)(36756003)(229853002)(68736007)(31696002)(64126003)(230700001)(2950100002)(6916009)(5660300001)(81166006)(81156014)(65826007)(54356999)(50986999)(76176999)(54906002)(8676002)(97736004)(101416001)(7736002)(7350300001)(58126008)(31686004)(575784001)(50466002)(305945005)(189998001);DIR:OUT;SFP:1102;SCL:1;SRVR:DB6PR08MB2824;H:[172.16.25.12];FPR:;SPF:None;PTR:InfoNoRecords;A:1;MX:1;LANG:en; X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtEQjZQUjA4TUIyODI0OzIzOnhTcVBMQ0NhSGhuNUJxbUJyNlZRYVdWb2Qv?= =?utf-8?B?QlMyT0xVN3NWV2lyeTIrTGl5b25seTdJN29OV1hXaHJQM1c3V3ViYUZkQWhx?= =?utf-8?B?SEVCeHVlZmRPMmFSRlNxakpLR1dlM3Z5ZkpGSGJGK2dlUWNsSThZblJoem4v?= =?utf-8?B?MFN0UGtpNXg0WDlDOFcxSTBjOHRqbjZFanA2S1A4aytXbnVucU96RkZWVGV4?= =?utf-8?B?eXVXakdsRkpud2dMOUZ3OHFOMm1abFZjTlhwcjB3WGR4UmxaUncxRHlzd2gw?= =?utf-8?B?RjVUWTVtUkxkWTlXWWhVRXJ1RGExMWtQd1FkWXlzb2FpV2tqd1hpcGsvZERr?= =?utf-8?B?dXVJNWJFaldOQ1dIbzErUjJrUWpRdzVOekpFbFIwamhlMG05bVpZMUtFWGd1?= =?utf-8?B?RUxISzhSeW1aMnVKek9zdEcxYlpNelFWR0FBOUIxbEs5Q3dqZ1ZPb3hwaXFF?= =?utf-8?B?aDJJbGI2TExGeFhOSk0rRVlZL0xHUy9nRTFpQkI4OVd3S0svQVRGcmRQQzVy?= =?utf-8?B?ODhzVFV5Mkd3bk5lVnNNbFl3azdzaWE4T1JUbG9YWkRFS2VudG8wcmNHOWwx?= =?utf-8?B?cXJlOWg2MEY1aHU5MnBFMEZGQlMzUm5ZVzJUSG1iRHVGcWlvYXVUbzlxb2Iv?= =?utf-8?B?bHBrb250NEZicGdaYWpBRFN5TE5wanM0bTRla1YrL1BEMlNUQTNUQlRjSXB2?= =?utf-8?B?MHRENHAwNHpLcjc1L0daUzNYczAzR3J5YWdrK0lVNVFEUS9JSXdKTW9odkhP?= =?utf-8?B?WWt6Z012MFRXOUZydXN2eVFQR3JONWg2OU43MDRqRXlMTkZlWnYyZDNmcmx4?= =?utf-8?B?L2V1ZzlWTG4wSmFla1Q1MFljWnBWNFoyejN0U0NJSE5WU2hUbVc5NlAxdUFU?= =?utf-8?B?bFc1a09ibHh0bXRwMEFyL2h2QkxmOGVJNjA3d1Zlc3MzZ0dxVXpoWjRZV3hU?= =?utf-8?B?WUFRTTEwb05tWUEvb2RucUpsNlJCaXRBYWg2ZlB3TSt1YjJVbEd3bUllR1Rp?= =?utf-8?B?VHE0ZVBVQ3lubHgyRnREekFxRnQxOXN2dTlpV3I4RjM3K3FGa1l1czJzNTND?= =?utf-8?B?T1pJaUpHY05Ub0gxZnljcVN3M0QxSnJrdW9MTnhIVWk2azc1cmFQbXMyM3pN?= =?utf-8?B?NDQ2RGlmWE1FRmNRQ0JhWmlRT0RYSHlCOTdkRVAzK05xclRxQU15aWRBVlcw?= =?utf-8?B?bkZGYTNObXY5by9XU285V2I1NWdyZE1La0kvd1F6WWt0TG1RODZCZGFlRjYv?= =?utf-8?B?d3R2YzcwNHgxWUxFWmh5bW90WjBrUC9uSGp0MWRMQXl5SkIvaEJVaTZQV0wv?= =?utf-8?B?YjA4YmlTWjhndDZzdXptQTU5NEtRckQvaHIwNm1sdGxwRUxjTDRyTitvNFFQ?= =?utf-8?B?dWtHcVVuMFVQUXcvV2pCeEw0QzJpTUtSYWhTQ0VHL0VuM1cyR293K09za1Az?= =?utf-8?B?Q3ZUTFFXQzZGSVRLaE9QMTE0Z3lvaTc3bldKQ25Wem1NUWhwWVN4R2xlS1dm?= =?utf-8?B?OGQyQkQ1emVnV0VhUzdxbDExMllhaW9wTWFsVjVUc00zOXp4bWgrVW9hMWVl?= =?utf-8?B?NmY0YXk1Vk0vaEhZZjNCQXcrVG9QTGFvL2V3bGFkQXYwcHVKSmhTK3NDNmdp?= =?utf-8?B?V1QyY0VzOFpmMUJkYWVudUZHaFFRWko4NU9ZWWJ1UG5nWlhYL1BhOVRVNXRU?= =?utf-8?B?eFRKbjdzZWRNOTNicmQvaVhFbmhiNXpqdkloVm4rdzlUY2M4T0E5SklpUkRi?= =?utf-8?B?MW1MQkpTVXBSa3N2dGdPWTZsTWNxYVZvc0lGQzV2U0taRWloMGV5OFc2bERG?= =?utf-8?B?cWl4SkdvT1VNRlZHRHYyODF2ZWZhbUxnMjVRb0p2QytaZ0E2MmxYd2NEK1N2?= =?utf-8?B?Mjh6cDhrVXUvTU01Wk4waXVzbDQ1NVRTYUk5WDlLaDNrWFJMdjJkRDVUZ1dk?= =?utf-8?B?bkdGT2VUYXc2eklKT1FYc1VIcHVaajVSRTZiREhZNnl4U1NGK21NUkxtTVJn?= =?utf-8?B?b0xYOVBOY3FvM0pINXRDYzBSY1ZkcmkrRjRNZz09?= X-Microsoft-Exchange-Diagnostics: 1;DB6PR08MB2824;6:poXVZuU+AJavmCutht/tkl/GVfWjryw5a6V23Yu/wC8LUZML0Ox3vswBWSTazJ0Ej7X8VRLJx/i0eoM7fE2ddT0/Eb7iJczx0mnNpZoOAOYBXwa9erkOXSafkZbMWk6sb4rbm0DiFvyPDkmDbIl9IvoBnUUyflnLTCRl0B7vkZHcEv3GSgh6vLzdNM/+N7KKKsN7bb7d5jv5JpsM4ti6aev2EoHZdNoW3A1RGyqgXDHJKWjtdKnIBXUl0/Ib1YwqHgpYZMs4oB6hTzpc4Jg9qat+0Hr0UC2V+V6vTCpt613u+2ku6JtW54I99IFwtTSoJD6zjUWTHT/VygDuDEDtJQ==;5:Nl3vSByPwwy+vNhsTadXgxO3xEswmMwCn5CmVwY6ZfXKe4DArL5QOzj4zZALIE6UN6mDL+5VE2i5NaX63JoCNuS7V7ZKi75N3fT5KTPsMjo60dYT6jKUDj2BKpF2AI81kxHy3JyRRokQluqyNdd1Yw==;24:PTolDhdf9apEWnFyQtTQyYPU325LzL5TuhlcxBydgRblmWx8IWo8Gzb7T9FbSXGtpVsK1hYsQ/y5eCwlu5q7O0PLKBKz92uNZSyKfVzasZs=;7:9ynvTOZRSiZF2Dzjtee6z6cduMd1qPM/k+skE+IT0FAvGmT9VLenS6a91Dk/UKSbVBfj0w5oN9vAsV8mgEUPG/PjP7G91zl0fznZ4kYxDzpMwti4lAX6NB9G/btCL7QOppUC5gvodKTeSgcR6I+W35NeAnv4MoNbkY2d5g7SHCJF0bzWGiUSbPDXxKOWi9eLBf2nVznOFaOiNsf2wbthozLDG9ISGb3gIsxiQwV2hxg= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-Microsoft-Exchange-Diagnostics: 1;DB6PR08MB2824;20:JHjTPMariQXjgrMhTbSAo3I+7PpMvhYmyf56El7RIzSmxksV/hIz2TmPzU50aED0I7JN9PqRBisgqOTuygL/PLY43a7fFvAZdX13OGcy898uItrbO5EAaOg7u4GRUgN7aKo0MStA2Vse8z27uAMbP5kRXe/w97/+47pXe4CuTjw= X-OriginatorOrg: virtuozzo.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 Sep 2017 14:26:47.1317 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB6PR08MB2824 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 09/19/2017 04:54 PM, Dmitry Vyukov wrote: > On Tue, Sep 19, 2017 at 3:52 PM, Andrey Ryabinin > wrote: >> >> >> On 09/19/2017 03:57 PM, Dmitry Vyukov wrote: >>> On Tue, Sep 19, 2017 at 2:46 PM, Andrey Ryabinin >>> wrote: >>>> As comment says barriers needed for preempt_schedule_irq() case >>>> where in_interrupt() returns false. But we don't use in_interrupt() >>>> since b274c0bb394c ("kcov: properly check if we are in an interrupt"). >>>> >>>> Now we use in_task() which handles preempt_schedule_irq() case properly, >>>> thus no barrier required. >>> >>> >>> Are you sure in_task() handles preempt_schedule_irq() correctly? >>> They seem to differ only by SOFTIRQ_MASK vs SOFTIRQ_OFFSET, and that >>> only differs in local_bh_disable sections. But preempt_schedule_irq() >>> does not seem to have anything to do softirq/local_bh_disable. It's >>> called from real interrupts, right? So I would expect that in_task() >>> returns true in preempt_schedule_irq(). >> >> Indeed, you're right. I checked this only on !PREEMPT kernel, where this worked. >> >> Still, I think that barrier() in __sanitizer_cov_trace_pc() is not needed. AFAIU it needed >> to make sure that load of t->kcov_area isn't moved before load of t->kcov_mode, but I don't >> think that compiler is allowed to make such reorder. That would be a bug in the compiler. > Ugh, it should have saied READ_ONCE(area[0]) instead of t->kcov_area. > > Why? C compiler is allowed to fuse/reorder loads from the same base > object. Also stores can be reordered. > Ok, right. t->kcov_area can be loaded before t->kcov_mode, and it's fine. But deference of the kcov_area (READ_ONCE(area[0])) can't be moved before kcov_mode check. And this barrier intended to prevent such move, right? > >>>> Signed-off-by: Andrey Ryabinin >>>> --- >>>> kernel/kcov.c | 10 ---------- >>>> 1 file changed, 10 deletions(-) >>>> >>>> diff --git a/kernel/kcov.c b/kernel/kcov.c >>>> index 14cc8c1a7cad..b7fbcbef88c1 100644 >>>> --- a/kernel/kcov.c >>>> +++ b/kernel/kcov.c >>>> @@ -71,14 +71,6 @@ void notrace __sanitizer_cov_trace_pc(void) >>>> >>>> ip -= kaslr_offset(); >>>> >>>> - /* >>>> - * There is some code that runs in interrupts but for which >>>> - * in_interrupt() returns false (e.g. preempt_schedule_irq()). >>>> - * READ_ONCE()/barrier() effectively provides load-acquire wrt >>>> - * interrupts, there are paired barrier()/WRITE_ONCE() in >>>> - * kcov_ioctl_locked(). >>>> - */ >>>> - barrier(); >>>> area = t->kcov_area; >>>> /* The first word is number of subsequent PCs. */ >>>> pos = READ_ONCE(area[0]) + 1; >>>> @@ -228,8 +220,6 @@ static int kcov_ioctl_locked(struct kcov *kcov, unsigned int cmd, >>>> /* Cache in task struct for performance. */ >>>> t->kcov_size = kcov->size; >>>> t->kcov_area = kcov->area; >>>> - /* See comment in __sanitizer_cov_trace_pc(). */ >>>> - barrier(); >>>> WRITE_ONCE(t->kcov_mode, kcov->mode); >>>> t->kcov = kcov; >>>> kcov->t = t; >>>> -- >>>> 2.13.5 >>>> >>