From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.1 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE, SPF_PASS,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id AFAFBC63777 for ; Tue, 17 Nov 2020 06:05:41 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 60B5D246BF for ; Tue, 17 Nov 2020 06:05:41 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="CZkdAzkh" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1725904AbgKQGFT (ORCPT ); Tue, 17 Nov 2020 01:05:19 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48630 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725879AbgKQGFT (ORCPT ); Tue, 17 Nov 2020 01:05:19 -0500 Received: from mail-ed1-x532.google.com (mail-ed1-x532.google.com [IPv6:2a00:1450:4864:20::532]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6AF26C0613CF; Mon, 16 Nov 2020 22:05:17 -0800 (PST) Received: by mail-ed1-x532.google.com with SMTP id m16so6328220edr.3; Mon, 16 Nov 2020 22:05:17 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:from:to:cc:references:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=1D5+sHmUNH52Iu2Qtq6250gD2qVYhSdcv/pbrGTVi20=; b=CZkdAzkhNLmk2mkDWDyVYvfv47dMHpHsSwz4EfaRRvflDn8i7a8r8JsbWvCuNhOVb8 ShCOmMErN3gntS65gOsz1U6GziWtybq51kDmjZAltHfSY6elL//Flv3t9iXYjXJpcnDK Bp8acYTmYoWk/TF1zh9cIvI2u6yMoG2y/XAgcWl/TaFqQd6tOhcPAJ4u3cUlIWAoxKZh EphfZz6F20+Pz5mDccWpIRdJKK5RldaD2qxeTh22inRZtsUGraMUXoNRu+iwxLnKeDQP ++phiQQvEgWkcH+NtoRosZXZETCciOGnR/d3oYg5L3W3TiR4gJWX3fU6StR8TJxGQo9R 4uNQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:from:to:cc:references:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=1D5+sHmUNH52Iu2Qtq6250gD2qVYhSdcv/pbrGTVi20=; b=KX4PV72qrxwLZqFUdQjMBsbJEdFyEo6wjSINgx/Bqi3U+J1ZN+CaIQ3UNp7ozvMvFC eEKj/OVF3J6qmqnVqGizAfZJ0dR9wFP9U3evhTRKhC7ZLWtE22OPnAjzy8yOUeSYVxHe qVDjuE8aKMBnAr/XjDZY+QYTPay2epOL0+W0rLSbmphREnr33LXVkKotDxiV7gz1N1J7 cXIYOmwKS4OLuEV5TE64OflF3u13gPfxovK3fah3RZ1u3ZGXSFKmWbb301nkOMYxjdVK 5ocK7VJlbXNFbkeQJ8HNBScYYBx1/U+v57WZnBCB9V8Vij12qrOL5NNQEgyfajrzsj5s C2vg== X-Gm-Message-State: AOAM533B6Z8OTc3svU8mjuU4hT3BVPSx9utaJGHWWOVq7bIMR6/f+FcQ eqHe2BbG9s87Ra65xCs3H5V7SFWDAF4= X-Google-Smtp-Source: ABdhPJxnxnaQSEoQtNlRof1LxQkkQ/p6a3aUL7/oSgCOboVrz7x+zyq3MnwOVSQi3/vZhkFSya8DsQ== X-Received: by 2002:aa7:c3c3:: with SMTP id l3mr19371737edr.118.1605593115571; Mon, 16 Nov 2020 22:05:15 -0800 (PST) Received: from [192.168.2.202] (pd9e5afac.dip0.t-ipconnect.de. [217.229.175.172]) by smtp.gmail.com with ESMTPSA id f7sm11282315ejz.23.2020.11.16.22.05.14 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 16 Nov 2020 22:05:14 -0800 (PST) Subject: Re: [PATCH 1/9] platform/surface: Add Surface Aggregator subsystem From: Maximilian Luz To: Andy Shevchenko Cc: Linux Kernel Mailing List , Hans de Goede , Mark Gross , Arnd Bergmann , Greg Kroah-Hartman , Rob Herring , Jiri Slaby , "Rafael J . Wysocki" , Len Brown , =?UTF-8?Q?Bla=c5=be_Hrastnik?= , Dorian Stoll , Platform Driver , "open list:SERIAL DRIVERS" , ACPI Devel Maling List References: <20201115192143.21571-1-luzmaximilian@gmail.com> <20201115192143.21571-2-luzmaximilian@gmail.com> <8768d422-15f1-9fa3-481f-53be8549c395@gmail.com> Message-ID: <596dd647-b874-29f5-5bbf-a02f9d6ac587@gmail.com> Date: Tue, 17 Nov 2020 07:05:13 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.12.0 MIME-Version: 1.0 In-Reply-To: <8768d422-15f1-9fa3-481f-53be8549c395@gmail.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-acpi@vger.kernel.org On 11/16/20 6:03 PM, Maximilian Luz wrote: > On 11/16/20 2:33 PM, Andy Shevchenko wrote: >> On Sun, Nov 15, 2020 at 9:25 PM Maximilian Luz wrote: [...] > READ_ONCE and WRITE_ONCE are used to ensure proper access to state that > can be changed outside of the queue/pending locks (or under any one of > them). In general, I have tried to document all critical access of such > state with an explanation of why it is safe to do so. I've looked at this again and noticed that I can guard the packet timestamp by the pending lock and the packet priority by the queue lock (after first submission). This makes reasoning about access to them significantly easier and removes the need for WRITE_ONCE / READ_ONCE. After that, READ_ONCE is used - to access the controller state for smoke-testing to (hopefully) detect invalid request function usage (note that all other access to this state happens under the controller state lock) - for the "safe counters", where access to the shared value is, after initialization, restricted to the increment function - to update the timeout reaper, where access to the shared value (rtx_timeout.expires) is, after initialization, restricted to its modification function (ssh_ptl_timeout_reaper_mod() / ssh_rtl_timeout_reaper_mod()) and the timer function - to access the request timestamp, which is, after initialization, only set once in the lifetime of a request (all other access is read-only) - to access the 'ptl' reference of the packet, which, after initialization, is only set once, either at packet or request submission (all other access is read-only). Note due to this, READ_ONCE is only required for functions that can run concurrently with ssh_ptl_submit() and ssh_rtl_submit(), i.e. ssh_ptl_cancel() and ssh_rtl_cancel(). - to access request state outside of bit-ops when canceling I'd argue that all of these cases can be checked and verified with a reasonable amount of effort. Cancellation (last two points) is probably the most complex one. Unfortunately, I don't see any way to simplify that part without disallowing cancellation to run concurrently to submission, which is something I'd like to support as this makes implementing asynchronous requests in the future easier. Regards, Max