Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Message-ID: <36cfda2f-f35a-c86e-4d65-76d0956a8c6a@cornelisnetworks.com>
Date:   Mon, 4 Apr 2022 13:14:49 -0400
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.7.0
Subject: Re: [PATCH] IB/rdmavt: add lock to call to rvt_error_qp to prevent a
 race condition
Content-Language: en-US
To:     Jason Gunthorpe <jgg@nvidia.com>,
        Niels Dossche <dossche.niels@gmail.com>
Cc:     linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20220228165330.41546-1-dossche.niels@gmail.com>
 <20220404134209.GA2905506@nvidia.com>
From:   Dennis Dalessandro <dennis.dalessandro@cornelisnetworks.com>
In-Reply-To: <20220404134209.GA2905506@nvidia.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?K3FJS004N0J3aHdPbGhIQ0Y0ZzhlR0JsQ0NadVVibGZDWmlDS3dNYWUwMk9T?=
 =?utf-8?B?OWRzcllzZXBzYnE4SmM5VVQ4YWNqelQ5VWdxbE1kMnVndmYxUDNGaWR5M25x?=
 =?utf-8?B?YXN0aEw5bUZGeGk4dGFtTWZ3UTJjTUlBYmo3K0hCRU9aWG1oaGlEVlZNOXFn?=
 =?utf-8?B?OXA3c2pQU3IvQlV4YmI0eHBrUzZCdXZMWU5Za2NoWnluYU1yWUhFZ1JibDZ1?=
 =?utf-8?B?R3A1VDUvclhlUFVROGtXU3NYNWN2UzlnMWZLZmRsaXMvNG02NllZVU90blJv?=
 =?utf-8?B?cmNlUGNQMWlhVm03SmxaWGtLeXVTNEhpT01KK2VxNk1iak00d001N1dybVor?=
 =?utf-8?B?aWxwbWNBVGp2S1lmT2FSakdSKzdyL1MxWEphVlg0WXFzRHBxVjM3UThVS0pD?=
 =?utf-8?B?bUJTbFlvQmx0dndoSVprV2NXNXIzR1BqdlRMTDd2OWhXdUg1MDJ6NmphNWVq?=
 =?utf-8?B?UWtKZUhSa0hSSVBmWDRDclgyRlFNTzE5MUJDWE1OQkkyZ3Fsa2ZGRE1KSVZz?=
 =?utf-8?B?UjRRcGwwVUMwMWIwQmY4Y21hWDR4WjNpcXJ6akJxdzZSTFpqbDFJTThpK1NL?=
 =?utf-8?B?a01UNngxam9lSUFtTU52STZwOElnRWkrc24rUHN3ZWZWZG1IY051UEpJb2ZH?=
 =?utf-8?B?NWhNN08vWkpUWitPVXE0cW1PbkduZDR6U3BrZkdBNlo3ekxjVU9YWURYQkdx?=
 =?utf-8?B?YzBqUnB0Lzc2ejhSWkM2SnRHeXhrUEM1NjFzbUplcWliblJjZk1ZdWJCNTJv?=
 =?utf-8?B?cDhBaHZRaG5wakhaeVNYdkI3K0J3SnJ4bW5Qb0dGeExEdGQ3K2htR2p1cjVM?=
 =?utf-8?B?ZFBZUmZtQmhTUVljcTllZ1hoK0FvbmpEMjhFaGRod1kxemVNTms2QnJZYmdP?=
 =?utf-8?B?bHhwb1hyZHJhSUMxS09mMU8yNEhQSUFYc0k1S0pLNUNFdml2QlozQzYrbER2?=
 =?utf-8?B?UHQ1TWZvVmJiUmtRa21KMUVzaE9wbTNnckF5cGcwZVRyZXcxcE5Uc3EvZjE3?=
 =?utf-8?B?L3FrV3M5SVI2NFdZS094N0JGNEE5eEExS3ZwK0Q0cG9qRVlweWFmbG5yNlMr?=
 =?utf-8?B?ckdtMEl0Rmh1RXZhTEN4MFR0RHlZMXBGS2VWVEZldWdxUTM2VWQyZVg5NWlD?=
 =?utf-8?B?b005ZGxWQVlZandRR1JWU1hIWWNqTThBZTk2MytXWGZIVTY4ZStGeVVWdDJh?=
 =?utf-8?B?dWNOdENTb2Q1dWxKQ1hmVmo1T2pjaExUV2ZabWw5d0NtdVRuZFJwemRlV2Jr?=
 =?utf-8?B?OVlSNlM0NW9TRnY3NnJZMVF6L05CV2E3WmtMdHhoNnpWMWFyNUNvNlN1VXp2?=
 =?utf-8?B?NEE1M3pvRllsRGtYOERhTEFGbDEydm45eHVEWG5PT2VhaXJKYlRVUzJNRWRE?=
 =?utf-8?B?c1o0b3BucUVCNWU0ZlUrZXlINnROK1lQc2QrdkhCbTBKYWZqdG1BT2NyVW1t?=
 =?utf-8?B?WmNkcVErNVVuRCsyNjJBS0dNQWJoZ1l6VC9jdlVrekNEKzJJVDR4QlJHTGg3?=
 =?utf-8?B?eWgwSUZvcXZQaGhvd0M5TFhyQ1BSSkJRZmhrZGlnSmxzelZycG9ZOGRPSzU4?=
 =?utf-8?B?MmVXNDN3MS9IazQzQjV4ZGh2TjlaQ3IyaDQ5clNKYmd1TGpmM29oZGF6YWRR?=
 =?utf-8?B?UCtSYnRxZTNJUVJES3UwOFdvTXh3THFURU9Ub0lLaGMvbTVpMGt4WFN6TEhL?=
 =?utf-8?B?c3duWXJqSVZ2clZ1QVRJUUlCSWVMNmlKVldjRmNxMkpCME11SzR6cG51N1E3?=
 =?utf-8?B?bjM3QmZqZUxNMHBOcWxUcFA1dStidXRMaU04dUJqYzZ1NTZuZzlpSUNlOStn?=
 =?utf-8?B?dkxtRDlxOUcyRVJVWlJrWGkrM2hhbnM1OEhscUxtTmNRdTNxWGlpc0NmTURt?=
 =?utf-8?B?ckxsVzRhTVAwc1ZXam9yeHNVRjQ4WjJtNE9iS0RMS3Q2TXArTGkwaGhtWm9o?=
 =?utf-8?B?VklBR2QzV01FOUt6YkZUL1ZTV0tBSWptM0hXNWJUWVh0RGNLTko2NFF5dHUx?=
 =?utf-8?B?K01VZTY2L1JrdGNxUkFpanVoNGNlTTVDcG5HUWFRT2N4ZWFrWXFVNlB1Q1ZJ?=
 =?utf-8?B?WEUxVjFCdzBOYWxBWjBSN3FsT202bmRzN3k2blVwbmovOFpTTTVqdjhkSUU2?=
 =?utf-8?B?bWs5ZUJJbG1HbFVlRDloVHRVSWs4QmF4RnJIKzBpaEk4Mk9ZSngrR0s5bWpQ?=
 =?utf-8?B?aXFJL0dmQkM5b0FXOUtNMEphWWxHa0tjT2Exakk5dnlvbG83eWwvNnVaaTVy?=
 =?utf-8?B?RjYyV2d1QjFFQTFyOXlVdUtxQ1ByZ3dHZS9jMU9ybWFzb1NqbjNGMGhwQlNI?=
 =?utf-8?B?TzFmdUFwQ3BueTVuRVFOUHp2TEpLdEk3SE40ck8vcEZNRFpDcGdhdjNsaUtQ?=
 =?utf-8?Q?bRs4VLFzch/MYCXDO6h4UMSjv5K39wbG5fiyh?=
X-OriginatorOrg: cornelisnetworks.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 6b33ea2e-54ce-428e-d770-08da165ea2b2
X-MS-Exchange-CrossTenant-AuthSource: PH0PR01MB6439.prod.exchangelabs.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Apr 2022 17:14:54.2295
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4dbdb7da-74ee-4b45-8747-ef5ce5ebe68a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: vkWVbkA74XIDopc68Vz0/uDhtYJiRtWvpvqvkzCPCnQZlnHOhgl8VTgQy1xEdgOdTEvNc0WO6qQBChABBmbnWgT4nakRqd/gVZKfJPMgM05oj2TCBHXsHsoQEpTIgUy4
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN6PR01MB2835
X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,NICE_REPLY_A,RDNS_NONE,SPF_HELO_NONE,
	T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no
	version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
	lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 4/4/22 9:42 AM, Jason Gunthorpe wrote:
> On Mon, Feb 28, 2022 at 05:53:30PM +0100, Niels Dossche wrote:
>> The documentation of the function rvt_error_qp says both r_lock and
>> s_lock need to be held when calling that function.
>> It also asserts using lockdep that both of those locks are held.
>> However, the commit I referenced in Fixes accidentally makes the call
>> to rvt_error_qp in rvt_ruc_loopback no longer covered by r_lock.
>> This results in the lockdep assertion failing and also possibly in a
>> race condition.
>>
>> Fixes: d757c60eca9b ("IB/rdmavt: Fix concurrency panics in QP post_send and modify to error")
>> Signed-off-by: Niels Dossche <dossche.niels@gmail.com>
>> ---
>>  drivers/infiniband/sw/rdmavt/qp.c | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> Dennis?

For this one:

Acked-by: Dennis Dalessandro <dennis.dalessandro@cornelisnetworks.com>

> And this too:
> 
> https://patchwork.kernel.org/project/linux-rdma/patch/20220228195144.71946-1-dossche.niels@gmail.com/

I think is correct as well. Not thrilled with the lock, unlock, lock. Stuff, but
this whole unwind code needs a once-over so for this patch as well:

Acked-by: Dennis Dalessandro <dennis.dalessandro@cornelisnetworks.com>

Thanks Niels for finding and fixing these issues!

-Denny