From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (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 319F33A6B7C for ; Tue, 31 Mar 2026 19:22:27 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774984947; cv=none; b=dKleAQJ9nrQ+cgQge5X7vn60rnAM+vhrlgQCT709/NbNPRLUhdqlOCa1PMDT3KysNqWBVvcWpYA8fzjftf+GgzeqHDMZX1UHsjVxEnQZVOXSn3MIgEkfchp1GGU2jxE56reEqeKdGCbpJBFfprARdvNmjpCTpCNCnI6ZJfF0clU= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774984947; c=relaxed/simple; bh=NtJAEnQasgoVLEwwLKfhue0JsEEDLd5R00KCgN7cw0A=; h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID: MIME-Version:Content-Type; b=RMHi4ixyyIcJxLDk0me2GMVqb0016BCZyMRNn9udQbD7pFM/CbOf6nzRJFQ46n4jMqKEdsSMO2GyHrdhcs8lq6Rdq+bDNYrkgNms5D51BY0Xzvd4wrcUzD7pgyqpj5awdqwOt70pvJIv9oMy+MEdrkK263qXB/uSfFwPCqnDcT0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=gcbU/Z1m; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="gcbU/Z1m" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 3E981C19423; Tue, 31 Mar 2026 19:22:25 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1774984946; bh=NtJAEnQasgoVLEwwLKfhue0JsEEDLd5R00KCgN7cw0A=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=gcbU/Z1m/yB1XtkHD2LSFGULkoFEoe3dUhITr7wVVwAdkQYpd6dINPpe8Yc9jOU9x unRVg8KWYX8VQQnQ/joChCQqQztIePJPisfL1DDj62nKyX4JIxcJ9vJiK+kqpifmNX 6mTlncZH03LT71ndzNMc8F95zA8RM0D2zCf4l+8WUeGYDBORH5CCHWqV8mKKZzns11 UrdbVhjhSivOl8DtYmbDL/EHYGt7fk2heDrAXtQQfiuXXfwlBVf4Nku5ycP20BvcED 3PJTXaMiZzJ0TqXgUWfUktP/aGTc9ucp9o6vApE7R0gabvpoiEhKs6Hyw2MkumK7lx /gEho0SUT+2oA== From: Pratyush Yadav To: Pasha Tatashin Cc: Pratyush Yadav , rppt@kernel.org, akpm@linux-foundation.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, dmatlack@google.com, skhawaja@google.com Subject: Re: [PATCH v3 02/10] liveupdate: Synchronize lazy initialization of FLB private state In-Reply-To: (Pasha Tatashin's message of "Tue, 31 Mar 2026 12:41:54 -0400") References: <20260327033335.696621-1-pasha.tatashin@soleen.com> <20260327033335.696621-3-pasha.tatashin@soleen.com> <2vxzecl0i8rv.fsf@kernel.org> Date: Tue, 31 Mar 2026 19:22:23 +0000 Message-ID: <2vxz1pgziz2o.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; charset=utf-8 Content-Transfer-Encoding: quoted-printable On Tue, Mar 31 2026, Pasha Tatashin wrote: > On Tue, Mar 31, 2026 at 6:38=E2=80=AFAM Pratyush Yadav wrote: >> >> On Fri, Mar 27 2026, Pasha Tatashin wrote: >> >> > The luo_flb_get_private() function, which is responsible for lazily >> > initializing the private state of FLB objects, can be called >> > concurrently from multiple threads. This creates a data >> > race on the 'initialized' flag and can lead to multiple executions of >> > mutex_init() and INIT_LIST_HEAD() on the same memory. >> > >> > Introduce a static spinlock (luo_flb_init_lock) local to the function >> > to synchronize the initialization path. Use smp_load_acquire() and >> > smp_store_release() for memory ordering between the fast path and the >> > slow path. >> > >> > Signed-off-by: Pasha Tatashin >> >> Reviewed-by: Pratyush Yadav > > Thank you. > >> >> But... wouldn't it be a whole lot simpler if we introduce a >> DEFINE_LUO_FLB() and get rid of luo_flb_get_private() entirely: > > We are planning some updates to this code in the near future: > > 1. David Matlack will send a patch in the next version of VFIO LUO > preservation series to add liveupdate_flb_put_incoming() so that > caller can protect from race with release of the last FD release that > will also release FLB. > This change will increment FLB 'count' not only when FH uses it, but > also when the object is being accessed. > > 2. After I plan to convert 'count' to use kref, and also decouble init > and liveupdate_flb_get_incoming(), this will allow to access FLB > object with interrupts disabled, as requested by Sami for the IOMMU > work. I think, that while working on this 2nd change, we can also do > some clean-ups if necessary. Okay, makes sense. I really think this initialization of private state on first use is ugly and it should be gotten rid of. So please, whenever you plan to do this refactor, include something like the static initialization I mentioned. > > Pasha > >> >> #define DEFINE_LUO_FLB(_name, _ops, _compatible) \ >> struct liveupdate_flb _name =3D { = \ >> .ops =3D _ops, = \ >> .compatible =3D _compatible, = \ >> .private =3D { = \ >> .list =3D LIST_HEAD_INIT(_name.private.list), = \ >> .list =3D LIST_HEAD_INIT(_name.private.list), = \ >> / ... >> }, \ >> } >> >> I can't get sparse to work so not sure if I need some special syntax to >> initialize stuff in .private, but I reckon we can get something working. >> --=20 Regards, Pratyush Yadav