From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 755DB3905F0 for ; Tue, 18 Aug 2026 09:17:10 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787044631; cv=none; b=W67pKHf9L7hbNssAVBSUvpMgYXOHkYEpVIgkwF6kKD6yon7zeIW5+uDZ4/6jz98lXdEkkAfUvvqsxWoEU0bXC2FfcyoEblgPNNHN+5qSRB+1QTgc2IrOh+pO97YSl1Ex8P0rCU4IWHyT35oVprK67qD+DGulm8duCmhxL70bvVo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787044631; c=relaxed/simple; bh=8yOIYI2cRyM4ukqnNWlzvgGK4HgoxnbtqEjr5Si9yIw=; h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID: MIME-Version:Content-Type; b=rpFE9J4Rn7CK8VdAH5ubErib+kXGT2LXXuuyDlJ01dcjsf+aw+m7ioEYRNrnvBPHgzUuhFsB8SF4+xIFpMPpvjhfDlNhV+9+sPRL1TfMrXDUrhCv9gabFWPtsnTtvxRoSZQek1QcCg/FDnZpzmY8VEVwmJk0+wEkPwr7XSNfRFQ= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=Sp+d8/Up; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="Sp+d8/Up" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 8E5751F000E9; Tue, 18 Aug 2026 09:17:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1787044630; bh=CgxvZuvkwadEpFuIclkSP8Q6+jXLcm9MUedkH6duLsc=; h=From:To:Cc:Subject:In-Reply-To:References:Date; b=Sp+d8/UpGyNfg9ewcl0Xff86tUjKVbiovZXBlh3Ez8eoIN3BHBBPccNWCWgOFcs7r LVZEt5YFnKAmBxjwOWsKZ14WMGgXL99MEGoY8psZcp9Va/MxxfPaJUDoWcUM56Kk21 yFQZxRnckxSwrP5xQXVRDsbGZSyaFjGQUsTRMMpaA1QGx0xqRlBR6oaT+60odvl2u+ JQH3UW2CSjfU/6pE7mweMMg7erA82fC2Pq6GSQbODg+g7jDCbuugtfcSxPUWKS2+GN M4Kgfe9jfmjsZ55GZQM2ppBjHrz1aoxwZudGG0lU/6cI+eYbucLvLxMsOcfnbp13BY HUJTI/5305zig== From: Pratyush Yadav To: Chenghao Duan Cc: Pratyush Yadav , pasha.tatashin@soleen.com, rppt@kernel.org, graf@amazon.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org, kexec@lists.infradead.org, jianghaoran@kylinos.cn Subject: Re: [PATCH] liveupdate: use scoped_guard for mutex in session operations In-Reply-To: <20260817025146.GA133735@chenghao-pc> (Chenghao Duan's message of "Mon, 17 Aug 2026 10:51:46 +0800") References: <20260814095506.631274-1-duanchenghao@kylinos.cn> <2vxzzeyo23ol.fsf@kernel.org> <20260817025146.GA133735@chenghao-pc> Date: Tue, 18 Aug 2026 11:17:07 +0200 Message-ID: <2vxzfr0b23fw.fsf@kernel.org> User-Agent: Gnus/5.13 (Gnus v5.13) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain On Mon, Aug 17 2026, Chenghao Duan wrote: > On Fri, Aug 14, 2026 at 04:10:34PM +0200, Pratyush Yadav wrote: >> On Fri, Aug 14 2026, Chenghao Duan wrote: >> >> > Replace manually paired mutex_lock/unlock with scoped_guard to align >> > with the coding style of the rest of the codebase and simplify locking >> > paths. >> >> No. This is done explicitly because we don't want to mix gotos with the >> automatic cleanup-style locking. I don't think we should change this. >> > > Thank you for the clarification. I understand your concern about mixing > `goto`-based error handling with automatic cleanup-style locking. > > I was wondering if these particular cases might still be suitable for > using `scoped_guard()`. The patch only changes two manually paired mutex > lock/unlock instances, and in both cases, the guard scope is limited to > the operation that requires the mutex. The lock is released before the > subsequent error-handling logic is executed, so the `goto` paths do not > cross the scope of the guard. > > We could also use an explicit `scoped_guard { ... }` scope to make the > lifetime of the guard more obvious and easier to review. > > Of course, I may be missing some broader considerations. If there are > other reasons why these cases should retain the explicit lock/unlock > pattern, I would be happy to follow your guidance. That's one of the recommendations of the cleanup API. From include/linux/cleanup.h: Lastly, given that the benefit of cleanup helpers is removal of "goto", and that the "goto" statement can jump between scopes, the expectation is that usage of "goto" and cleanup helpers is never mixed in the same function. I.e. for a given routine, convert all resources that need a "goto" cleanup to scope-based cleanup, or convert none of them. Which sort of makes sense I think. Also, cleanup-style guards can cause compilation failures too. For example, if you do: err = sanity_check(); if (err) goto out; guard(mutex)(&my_lock); ... This will fail to compile. Now this problem doesn't exist with scoped guards as far as I can tell, but I do see why it is recommended to not mix them with gotos. -- Regards, Pratyush Yadav