From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) (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 E7C112571C7 for ; Wed, 29 Apr 2026 19:31:08 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.133.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1777491070; cv=none; b=Tm+GivxBoIvIPApLuqT09Xch/GZvwwmbJSPO+1GUprcFYiO+xAmPDHdoGHZSEP+5ktF5rY5jRpURD1JWrwY+ZnF6MIoC8hOu2KMuZ7wEs+hUdjYp1NiZ/VKydClHKTFapVS0vABx4JMfRvBkn2n6RLw8uozHHKyggKCcUSvcFjo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1777491070; c=relaxed/simple; bh=kXYGMvt1rhykfXQpnEH1UzW5gDV3m1ghchtdMwu8nIY=; h=Message-ID:Subject:From:To:Cc:Date:In-Reply-To:References: Content-Type:MIME-Version; b=V4AoILin+j7m2KrM3bblGbYRbI65yziVBd1xFlMu8G56iyS+57CYUXUQw2occ0w7y4wuIeJkRq6ml0nvJ+A6gelSgDKFu0bZj6Mj/Fzz2/TCD/nNErLSELfW5Vb2fMAU0wCYAWMf9OCTzTdyy1/4ziuEs95EgH0e5jqFpNWuLzM= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=eLryzAzo; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b=KDRnW11r; arc=none smtp.client-ip=170.10.133.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="eLryzAzo"; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b="KDRnW11r" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1777491067; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=pAorgokiyXcdKZvLEWEV9aRso9IqXESGmbQbGoVLsbc=; b=eLryzAzo+zF+HP7mTMjuZFrZ632HLgsrjFyfMYsZ0IaEIZmFCu+qdo6H0hRQAvpDauBjT9 h3iSriMLB/tjYY0QUNiyuxanGzsjseAycUMq3bLRSAye48nVLqel1WKqrN+zqxczLkdezk hISmx9bo6E1Vire2fBI3TMP0Rn33FHk= Received: from mail-yw1-f198.google.com (mail-yw1-f198.google.com [209.85.128.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-664-UEXeuSx-MOuyLM0N9Pd29g-1; Wed, 29 Apr 2026 15:31:06 -0400 X-MC-Unique: UEXeuSx-MOuyLM0N9Pd29g-1 X-Mimecast-MFC-AGG-ID: UEXeuSx-MOuyLM0N9Pd29g_1777491065 Received: by mail-yw1-f198.google.com with SMTP id 00721157ae682-7a00198633fso3391777b3.0 for ; Wed, 29 Apr 2026 12:31:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=google; t=1777491065; x=1778095865; darn=vger.kernel.org; h=mime-version:user-agent:content-transfer-encoding:references :in-reply-to:date:cc:to:from:subject:message-id:from:to:cc:subject :date:message-id:reply-to; bh=pAorgokiyXcdKZvLEWEV9aRso9IqXESGmbQbGoVLsbc=; b=KDRnW11r4xCR4HrlKaowRJsYv7tPWkcbqCfHiHuzMDOQitYxf5aywfNYETlEBjDgZQ HDsXswql9LRzQ211/h6GPD3h+9iQa0CLqC6YMxWPgpIQVwVBrNiiZCOKrJBBj0Y+3Omz I1VEL32iq8EnYee0nx/iNMpb7Xtx1DCfJyZviMKADplsCF0KFWpuFAI11xZf6tkCVuJ4 vX0p5b1GUPe3iSSfN8Zb4s+aQQYXT8a6xHvl5evG8f+7rYM9COsF3WwcfOrjTFGG+Pz4 rXGBpRK7KdKIX81x5SRuqbNJU2xQDl0nM/puXJhmrsM1RDDktNAjemUm/mUQYaCKFmTt 5Wew== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1777491065; x=1778095865; h=mime-version:user-agent:content-transfer-encoding:references :in-reply-to:date:cc:to:from:subject:message-id:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=pAorgokiyXcdKZvLEWEV9aRso9IqXESGmbQbGoVLsbc=; b=epEVKdnEYu/wdfRDkS+Q+L/ksxvurPdrgGzANYI+/jWrY7Vsi7lSKt8w/7zE/biyem rROEAbJjPJYzrqtERVZXxonpQyOKLb60/t50k2aOH92vto0uWJ3cxvOGWRk7isX8VGha lXMPEetcQrJuaDZBWGhIPKHfE77zUYrYP18/Uqka/26y9+LBoW/7fi/ejzCNukrlcqjv AlBe5S8dviwl8aWEkTfJlawa+uFEdVJNqyRmkq8qcZ/lz5u2aHWQLOH3TPIQbJZPQw5Z 0T41Gw4FaJHxb0xyBiUgGa4Ejg7Z2fKhKtXC9sWr0av3WWLnmIkXyAVfDupc8JBa3FZD lwXQ== X-Gm-Message-State: AOJu0Yx20r83xCYqctRI5Zy4v0B3V5zjtKoLAlCX6lm+AmOq+fqUJzvu ymfRsdLL58w5mc8+ty3DdnXOyZYyzH16fmLgEqAzwGnVhuJ353a7yWyI9fbT0KB6sA0O9pNT295 IX4b0DIF2FT/l2lK+2bHR49fidEtGcAcvAgL/9pz/YbTsKyAgHgX3DBDv2EqPiVM2Sg== X-Gm-Gg: AeBDievuZViNT9FU99Fip5Eh2T4d5TEFKZKQZDcSmXPva6NO/OWtGaji6chlqoDDDXI vTENSyHlc/WezFAjY2TUvBeyZG1q0pRbatmwwr0kzVua0pTSlu32FTi4zpIR3wOEIojFP1v1pf0 VUnQrxesQRPwVuYFctXYRtD+SLJ2Eo7IY5hg9wv3eOdGU92qw4EdvWDXWx6boQ+PLtqNGpczwMU T4R7ByiSZYqjGDHLk5bIOwN5ZIuLfeH6vDnHx6mf6ao0IuSd8/ycOW0cfOjfgu2GjlpRtiuY9sN j+BjuoLCQFEVhcbGrY6C3iGD7FxKHR2d3ZC1AOi0zx6qkSX2wTJJrDDmaHV//xGBOlIaYWrIhEs d++jwRaNfpegYwJNp0AXhPpX/keFN7Z4EVXGNLXwEXtte1Sq5O/eLwPWwZ9S1EUg= X-Received: by 2002:a05:690c:9a84:b0:7b3:c611:7ef5 with SMTP id 00721157ae682-7bd527fa466mr1566957b3.6.1777491065602; Wed, 29 Apr 2026 12:31:05 -0700 (PDT) X-Received: by 2002:a05:690c:9a84:b0:7b3:c611:7ef5 with SMTP id 00721157ae682-7bd527fa466mr1566587b3.6.1777491065167; Wed, 29 Apr 2026 12:31:05 -0700 (PDT) Received: from li-4c4c4544-0032-4210-804c-c3c04f423534.ibm.com ([2600:1700:6476:1430::29]) by smtp.gmail.com with ESMTPSA id 00721157ae682-7bd258a54fbsm19181917b3.26.2026.04.29.12.31.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 29 Apr 2026 12:31:04 -0700 (PDT) Message-ID: <9bd559bf25d2e4f7faaab94eb756755f75bb4eff.camel@redhat.com> Subject: Re: [EXTERNAL] [PATCH v3 01/11] ceph: convert inode flags to named bit positions and atomic bitops From: Viacheslav Dubeyko To: Alex Markuze , ceph-devel@vger.kernel.org Cc: linux-kernel@vger.kernel.org, idryomov@gmail.com Date: Wed, 29 Apr 2026 12:31:03 -0700 In-Reply-To: <20260429125206.1512203-2-amarkuze@redhat.com> References: <20260429125206.1512203-1-amarkuze@redhat.com> <20260429125206.1512203-2-amarkuze@redhat.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.60.0 (3.60.0-1.fc44app2) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 On Wed, 2026-04-29 at 12:51 +0000, Alex Markuze wrote: > Define named bit-position constants for all CEPH_I_* inode flags and > derive the bitmask values from them. This gives every flag a named > _BIT constant usable with the test_bit/set_bit/clear_bit family. > The intentionally unused bit position 1 is documented inline. >=20 > Convert all flag modifications to use atomic bitops (set_bit, > clear_bit, test_and_clear_bit). The previous code mixed lockless > atomic ops on some flags (ERROR_WRITE, ODIRECT) with non-atomic > read-modify-write (|=3D / &=3D ~) on other flags sharing the same > unsigned long. A concurrent non-atomic RMW can clobber an > adjacent lockless atomic update -- for example, a lockless > clear_bit(ERROR_WRITE) could be silently resurrected by a > concurrent ci->i_ceph_flags |=3D CEPH_I_FLUSH under the spinlock. > Using atomic bitops for all modifications eliminates this class > of race entirely. >=20 > Flags whose only users are now the _BIT form (ERROR_WRITE, > ASYNC_CHECK_CAPS) have their old mask defines removed to document > that callers must use the _BIT constant with the set_bit/test_bit > family. ERROR_FILELOCK and SHUTDOWN retain their mask defines > because they are still used via bitmask tests in lockless readers > (ceph_inode_is_shutdown, reconnect_caps_cb). >=20 > Flag reads under i_ceph_lock continue to use bitmask tests where > the tested flag is only modified under the same lock; this is safe > because the lock serialises both the read and the write. The > remaining flags continue to use non-atomic bitmask operations under > i_ceph_lock, which is correct and unchanged. >=20 > The lockless reader ceph_inode_is_shutdown() retains the READ_ONCE() > snapshot plus bitmask test pattern -- the single atomic load into a > local variable is correct and avoids a second memory access that > test_bit() would require. It now uses the named CEPH_I_SHUTDOWN > mask constant instead of an inline BIT(). >=20 > The direct assignment in ceph_finish_async_create() is converted > from i_ceph_flags =3D CEPH_I_ASYNC_CREATE to set_bit(). This > inode is I_NEW at this point -- still invisible to other threads > and guaranteed to have zero flags from alloc_inode -- so either > form is safe, but set_bit() keeps the conversion uniform. >=20 > The only remaining direct assignment (alloc_inode zeroing) operates > on an inode that is not yet visible to other threads, so it is safe > without atomic ops. >=20 > The dead precomputed flags variable in ceph_pool_perm_check() is > removed; the check: loop re-reads flags from i_ceph_flags after > the set_bit() calls, keeping a single source of truth. >=20 > Co-developed-by: Viacheslav Dubeyko > Signed-off-by: Viacheslav Dubeyko > Signed-off-by: Alex Markuze > --- > fs/ceph/addr.c | 17 ++++++------ > fs/ceph/caps.c | 24 ++++++++--------- > fs/ceph/file.c | 13 ++++----- > fs/ceph/inode.c | 4 +-- > fs/ceph/locks.c | 22 ++++----------- > fs/ceph/mds_client.c | 3 ++- > fs/ceph/mds_client.h | 2 +- > fs/ceph/snap.c | 2 +- > fs/ceph/super.h | 64 +++++++++++++++++++++++--------------------- > fs/ceph/xattr.c | 2 +- > 10 files changed, 72 insertions(+), 81 deletions(-) >=20 > diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c > index 2090fc78529c..35c5fdb5a448 100644 > --- a/fs/ceph/addr.c > +++ b/fs/ceph/addr.c > @@ -2583,20 +2583,19 @@ int ceph_pool_perm_check(struct inode *inode, int= need) I have realized that we have issue here. We declare flags as int [1]: int ret, flags; However, we assign ci->i_ceph_flags [2] that has unsigned long type [3]: spin_lock(&ci->i_ceph_lock); flags =3D ci->i_ceph_flags; pool =3D ci->i_layout.pool_id; spin_unlock(&ci->i_ceph_lock); I think we need to rework the declaration of flags variable. The rest of the patch looks good. Thanks, Slava. [1] https://elixir.bootlin.com/linux/v7.0.1/source/fs/ceph/addr.c#L2544 [2] https://elixir.bootlin.com/linux/v7.0.1/source/fs/ceph/addr.c#L2564 [3] https://elixir.bootlin.com/linux/v7.0.1/source/fs/ceph/super.h#L383