Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) client-ip=2620:137:e000::3:5;
Message-ID: <d0a37147-ff96-44b3-bee4-7004f0af5199@suse.com>
Date:   Mon, 30 Oct 2023 18:17:16 +0200
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] KVM: x86: User mutex guards to eliminate
 __kvm_x86_vendor_init()
Content-Language: en-US
To:     Sean Christopherson <seanjc@google.com>
Cc:     pbonzini@redhat.com, x86@kernel.org, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20231030141728.1406118-1-nik.borisov@suse.com>
 <ZT_UtjWSKCwgBxb_@google.com>
From:   Nikolay Borisov <nik.borisov@suse.com>
In-Reply-To: <ZT_UtjWSKCwgBxb_@google.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?OXJYSGFMUkRiMDF6QlNtOXZRNzYwSFZoejFrNExVYWlwUjZUMHk0VktFMXF6?=
 =?utf-8?B?SW1QZlFMRmkrcnlKNUpDeDhXLzFKODJheEZCTnNRT3p2ZGFWNFdSeGhSUkE0?=
 =?utf-8?B?Sy9QZDUvVmFjanVFZ2x3US8xRVZWbWpFN0dPM25vengvZEc5eDZrZ0JKNS9R?=
 =?utf-8?B?WHFLNDNxMGUwdjRBMkpkaU1EaWlNZkxSWmlwM0Z1cExIY2RYQ3pQNDEwZlVa?=
 =?utf-8?B?VW50OHR4NU11ZWJ4MTFuTklSdjZ1b2J1OUxPVDl4VkV6WFhBSUNCQ2NOTTk4?=
 =?utf-8?B?TDVvTlBZcnpOcEpPdmxGZUZOUGRNbmlIQ2JRT090OWJzYUI1SWN4R3lqNmRX?=
 =?utf-8?B?aCtBN0g2ZDd5VmU3UzFqL1dFVVNBOFN1UmFLUUdzelBnUEllQjFVZDZQU1A2?=
 =?utf-8?B?b0hGS0tiRjJpY1lOd2swRGNhSWFXWFBzR0kzYlFRMG5pTEM2czFWV1hWeFRu?=
 =?utf-8?B?a21tTXN4aW45emFoK1MxTUoxL1ltM09NOFJZTDNUQlozMmxMcTBncVF3Y1Zx?=
 =?utf-8?B?TlkyQlNVZUN0MVIxdkpvUVBCcWQ5cmNrSTNKakQvQzE2TUtqWUl2NktPWlRR?=
 =?utf-8?B?aVFFcFJhWWVvYWhyQ01JUmZpMTRoem9OQkh5Sk92R0xmUzN1cmR5UTZYcVVi?=
 =?utf-8?B?RXJzRUFVaGs5NGlBZDJtZWU3cXpEbFlGbTYyYW42WnZhNk5CNGFOWkNoQ2pt?=
 =?utf-8?B?ZiszRVBLSFRkUzd3b0ZLYUV4aHQ2QXl6SnhTVlord01mK1FNdmIvRkpzeFBr?=
 =?utf-8?B?anV1RFJlTDZJUnR0MjZVVkhxaXFNaStxU1ZaUWtoWUZBNm1pR2cxUmZ1eE0y?=
 =?utf-8?B?QlpjVEVRbGw1VDZoL1AvWU9qeGZwWllBbGs3eEFpa05BdXlWOS96YzgxZ1Qy?=
 =?utf-8?B?N0kwMTdKS0JkTVEvQ0RyOXdGUlhSdG0wcG9IYy9CVlJVM0M5akxkOHY4cldj?=
 =?utf-8?B?Q0Z5Zkt0Y001SVBGYlg3YUVyb0FLbE9sVUwvUG1USGZUSS9uUHYyZzJabmZE?=
 =?utf-8?B?ak16VXZiN1ZUb2N3aDlBSEtFYUFmd29tV0VMWmJYeFlEa2k0TmVDT3NFZWJi?=
 =?utf-8?B?N1NDTHU3VzdEampSeExqQjZCaTlobHhyYytFcmFzekF0ckVyZTlCWWZkUDMz?=
 =?utf-8?B?WTQxRlRuWkN3b2ZCeHRadHNqcGc0NXJQT1pWQzREaEVGSDVVZ3FaOHZ2Qkkv?=
 =?utf-8?B?aU9CNkhhWm5PTC9iQWcvWHQwRFdYTnQ1MklZODh0Q3ZNckZ1bndkekRCVTFD?=
 =?utf-8?B?M1hRdkVTZzBmSW81N2tXTjlUUlc2YVFiMWIrd0pYci9aQ2pDZG54K04vWHEw?=
 =?utf-8?B?andqK3lrNmJKSk11eFBCMG1QTTd2THJKOElQdjRYTGJraDU5b3BEZlhUWWg1?=
 =?utf-8?B?UUd0bUpvbHJPRitQUXZ6UVZJRzNoa3F3T3UvTGVFc2d2OXZSWm50bHhDaWxS?=
 =?utf-8?B?RXRuRVczQnF4cUJqRlZ0S2s4aTdSeis0RWdtVmpURmppTWdSaDlNZGRyY3pm?=
 =?utf-8?B?OUxGcC9TVHM5bUFJVWhBWGlycUlBblNLZHVlMHJ3bE01K3BOSWVPQzhUSEta?=
 =?utf-8?B?U3pKN1FaSEVXWXMrdzVtT1Bacm9pSlIya1Jzb004bG1CaHV1RjBnR2xxWjh4?=
 =?utf-8?B?SVNERzNhQmt5QzRnQ3JxQkRnRGdONkIrR0tjR1lMZUFZUnlaMFU5WWhIa0p4?=
 =?utf-8?B?YW5mV3hRTTYranZ6cUF4cVkwUXhYYWxoaUJEd2J1RkhIc3RHUnZUcVROUW5n?=
 =?utf-8?B?N3F0c3BxcTVyMmV5RjZXcGlLTFZLUkNKOXNwcWJGN214dDJrM0VPclBSZzBN?=
 =?utf-8?B?VWZQeVdMVGpVL25qaitjV2xLTFYrWlZqQktodTFsUFE2c2JuQ0pqaDl4SWx6?=
 =?utf-8?B?UnpSWkR6cUdGNGo0cW9BcHByUnVSVDI0N3Q4TXl1ZTFIT3czZFJkTklVaUhN?=
 =?utf-8?B?dWZpdWQ1Mk5DZTFjd3kyTUFJRE9VS1ZQNFlNKzY3cE1FREhXRDNoNUxSaEZJ?=
 =?utf-8?B?N3ZCeE40a3V6UFhPRWYwdVN2a3kwMTU5NGFnTGM3WXJsTWxiS2FTL0tTTUIx?=
 =?utf-8?B?MElWMUdxVjN1enRHSEUyNlY2dFZ5UEVHRVlFRWZvSTZnd2lpQXpyUDg4dVZG?=
 =?utf-8?B?eDFLWkErUWZYcUl1UTVFMEtPQnFLbjVMVXVKMUdLTUllVFVQak1IWFgwQXhm?=
 =?utf-8?Q?cB7lundLN/vTZs+c7Q2D7SkPoIdOjx+KZz/JVNSQ9gFe?=
X-OriginatorOrg: suse.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 5daf183a-6d80-44d4-ec89-08dbd963b1ab
X-MS-Exchange-CrossTenant-AuthSource: PA4PR04MB7790.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Oct 2023 16:17:21.1775
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: f7a17af6-1c5c-4a36-aa8b-f5be247aa4ba
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 8SQLgfPyGaaIDPqMMf+3B0ZJYrtw3z+1CCis+T/fu3S7rjq8ohoF9HjK2mXeCTZfbiRGOIIFbmst3g5sm//SCw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS5PR04MB9754
X-Spam-Status: No, score=-0.8 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,
	SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no
	version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on groat.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (groat.vger.email [0.0.0.0]); Mon, 30 Oct 2023 09:17:36 -0700 (PDT)


On 30.10.23 г. 18:07 ч., Sean Christopherson wrote:
> On Mon, Oct 30, 2023, Nikolay Borisov wrote:
>> Current separation between (__){0,1}kvm_x86_vendor_init() is superfluos as
> 
> superfluous
> 
> But this intro is actively misleading.  The double-underscore variant most definitely
> isn't superfluous, e.g. it eliminates the need for gotos reduces the probability
> of incorrect error codes, bugs in the error handling, etc.  It _becomes_ superflous
> after switching to guard(mutex).
> 
> IMO, this is one of the instances where the "problem, then solution" appoach is
> counter-productive.  If there are no objections, I'll massage the change log to
> the below when applying (for 6.8, in a few weeks).
> 
>    Use the recently introduced guard(mutex) infrastructure acquire and
>    automatically release vendor_module_lock when the guard goes out of scope.
>    Drop the inner __kvm_x86_vendor_init(), its sole purpose was to simplify
>    releasing vendor_module_lock in error paths.
> 
>    No functional change intended.

Thanks, I'm fine with this changelog.

> 
>> the the underscore version doesn't have any other callers.
>>
>> Instead, use the newly added cleanup infrastructure to ensure that
>> kvm_x86_vendor_init() holds the vendor_module_lock throughout its
>> exectuion and that in case of error in the middle it's released. No
>> functional changes.
>