From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id BE084C46CA1 for ; Mon, 18 Sep 2023 15:01:39 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 307D56B03A8; Mon, 18 Sep 2023 11:01:39 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 2B8426B03AE; Mon, 18 Sep 2023 11:01:39 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 180096B03B0; Mon, 18 Sep 2023 11:01:39 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 095DB6B03A8 for ; Mon, 18 Sep 2023 11:01:39 -0400 (EDT) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id CE17DB3C78 for ; Mon, 18 Sep 2023 15:01:38 +0000 (UTC) X-FDA: 81250032276.20.D8E18ED Received: from mail-io1-f49.google.com (mail-io1-f49.google.com [209.85.166.49]) by imf02.hostedemail.com (Postfix) with ESMTP id 01C6E8004F for ; Mon, 18 Sep 2023 15:01:33 +0000 (UTC) Authentication-Results: imf02.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=En2cAoRm; spf=pass (imf02.hostedemail.com: domain of yury.norov@gmail.com designates 209.85.166.49 as permitted sender) smtp.mailfrom=yury.norov@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1695049294; h=from:from:sender: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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=7Pyv4oO8rrHEd4wiz5Er/b8SPSG4uhIlASx2HO5yMlQ=; b=AKU000LtOnpvZssvLm+4stNj/Jvoxim586uOs9cvyEFB49vIjQ9f+rfGr1RtaizgHVDMT6 0PITqQ2BvUG4gfiKvIEEJhbcxdlM/ZizCzlORWbNEzWZWZsJiKcLl0oQv1NNBkT0PqCuYp ea+/9tc0V5Iz89ocZ98MgvJgdjZ5sC4= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1695049294; a=rsa-sha256; cv=none; b=b4rPKDvUa4lpjjlKzzEKVmb2B9qXpQNXSSNKE3HAJcwxbZw9ZZitCLmqngtGhZYETfl3IY xdcN1AAKXhSS/RwSobSJJaUgcsWGmlOSe4vKmW8BZ6WaaPwfRX/kxU5Ui2uf3kHoPLC32F V88saPKvH1ZhD/qT6vx/aQfVxPhObkE= ARC-Authentication-Results: i=1; imf02.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=En2cAoRm; spf=pass (imf02.hostedemail.com: domain of yury.norov@gmail.com designates 209.85.166.49 as permitted sender) smtp.mailfrom=yury.norov@gmail.com; dmarc=pass (policy=none) header.from=gmail.com Received: by mail-io1-f49.google.com with SMTP id ca18e2360f4ac-79536bc6697so155991239f.1 for ; Mon, 18 Sep 2023 08:01:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1695049293; x=1695654093; darn=kvack.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=7Pyv4oO8rrHEd4wiz5Er/b8SPSG4uhIlASx2HO5yMlQ=; b=En2cAoRm2GGOUMF/yHQtia6ZHYmd0RB90QkVSbVwBmR52U40ZYRH1ELSPWYlLtNYHM jyIQtFwMMfega+KMcj0NY2O/ED+nOy/LC8E9naveVFoFpm+aBm4h9XdAVtWajUEAd9Z2 BguhK82t8sPOMrOyk0nXrZ3YaCsmA/HsiuyKWvf24GY2iVXVoReMwGVUkcH79Qjy9fj9 gAOGvTRh1Lxo5QwUsa4iAhqEFba9uQqNRc1+wu5Jh4xoNQiSUk/Z7bqGH3xhyeoi6mr7 TGxr3dtJOnK9qaX6rHmXDzO+eUPJB/cuHBNrx3izcpojpDnI+w9UAJ7Z9CtRWq6MzKvo KmXg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1695049293; x=1695654093; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=7Pyv4oO8rrHEd4wiz5Er/b8SPSG4uhIlASx2HO5yMlQ=; b=QTJ+J7vWMswCbYa2eavcwPdXWuqd8VNLk/cGXYqZytptel6mhFaTXMxYrEBWYe9jng Yafqu4vyfRuROJzJLYIEWWoe7bzFandPhdIQZmG1vpApqFlXV8DTNlqom4Hfy1RRrN9T GvBNiuJrDu1kqXj2IP1dGdj2Fi4zVNlfUfbQjulsKzt/zt59wg72azHiQ9u2GSYGLIdh t0oAJTxnhSqwhdDaFYurteqhsUL/fABgyBoedTnDnG4XvjZG73RTZ1v0IsGHD2jX0KWo bmqiuEfJeWQh1ueeCURV7ChQnHGW3RTCbgzDZJagKfiR9BWCYFd999XfKwoVbSM0SEop cgZQ== X-Gm-Message-State: AOJu0Yx1mSOXMPS17xtF0vQX0NxJQIs7XOpKinKi92XQOBRqnlzqO5rR f5EmMg9EcFLZs3Q1lqWP9H0= X-Google-Smtp-Source: AGHT+IF7JTS/i+m9/oQRFXfvKeREFGeGsE6KslGHKMNcg+tSAckFmk+2b7p+zXlHPu/4uvCOHinHTA== X-Received: by 2002:a6b:dc0c:0:b0:795:13ea:477a with SMTP id s12-20020a6bdc0c000000b0079513ea477amr11908098ioc.8.1695049290863; Mon, 18 Sep 2023 08:01:30 -0700 (PDT) Received: from localhost ([216.228.127.130]) by smtp.gmail.com with ESMTPSA id h2-20020a6b7a02000000b00786aa1eb582sm2818737iom.31.2023.09.18.08.01.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 18 Sep 2023 08:01:29 -0700 (PDT) Date: Mon, 18 Sep 2023 07:59:03 -0700 From: Yury Norov To: Mirsad Todorovac Cc: Jan Kara , Philipp Stanner , linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, Matthew Wilcox , Chris Mason , Andrew Morton , Josef Bacik , David Sterba , linux-btrfs@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH v1 1/1] xarray: fix the data-race in xas_find_chunk() by using READ_ONCE() Message-ID: References: <20230918044739.29782-1-mirsad.todorovac@alu.unizg.hr> <20230918094116.2mgquyxhnxcawxfu@quack3> <22ca3ad4-42ef-43bc-51d0-78aaf274977b@alu.unizg.hr> <20230918113840.h3mmnuyer44e5bc5@quack3> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Queue-Id: 01C6E8004F X-Rspam-User: X-Rspamd-Server: rspam11 X-Stat-Signature: szkg5cxicrrfngdg4cetqxc5se8g6imz X-HE-Tag: 1695049293-245930 X-HE-Meta: U2FsdGVkX1+XFti0VMrGDpcexF3fxpF4i8HR0HApMgt3IBY5mMp/nUY4x5zjBZ4Vj7rFIEbzWIOfiC5HtDriQE4+zTK2cOF5JjzOZoWqUNaEZ0FOwd5P4vHqbAAFJi0ZaFfp1qFQiAqd44AVsoHm9zh4jDm/VQb+geIOC7E5YnN73LVk09j3ZNcn1KR0bT25UZz6WDQiJ2S3HW5GX69u9cDhDlVAPGe1z8b9nygCp9UldM7Ora0WT+pXz0GgblyfjnaVHu4ImlfTMgcT+ztHZPuR3SUpjfBCXnKUBTAcbcK/EsCjT2Uueg3oSMnvm9TjPBD5N0B8GJivXtEb/k3G5IFcWNM9eL5IzEZd14oKDRsH30tUGdRFkOen5+m/mJ7jIks1h6BpjBydkljW+s2BEngKxHhylomHfXtSjx/kJaDbsRvk4Bs/oYIxo/F5HhTIXLOyWaxod26jO8ViRAHYWNeV9vHxMjFehDUDAd/IU0Sz3YhEtREIK+enjjbsfGQBb/0ekF3PFT4DbxmNj5sZ/ePreUJm497bn9Qh13wxpqsj/CtgrtAb+1OlUaZi1ksRpxks1UZqT0igf3fam4J0o2/HRvMVYwPpsjiiUGxvJozNGG6HKPxqyFVK7kQieKtNzlcEyauGZeC1k+sJ4dg6j34BKbQlH95+WyzEYMphihdoP28Hea1lcmJsmb4AO1QfnRxI5AOGNmLd6Q4ab8lShjSnM8CuTeThqLFrI0Gw7p4ZFpI+lBApWmxUt0EXPeIaW8P82NmtUjm/lpYy3LXoZjUrTu5yGb1W6k1DbULpdiT1/9qyiOqRdP0IYlgLnpHCstXA3TIPq5q2zgROTYLvFo9UMY2/spFOlCYQMwXCa41alTOzkod8F1uYJgvfn9fOivqF0Rxkob7KVKnV9yPt+WeIT9jwv1IYfb6tUU8fcMG0X9cm0T7KpOvRYzGOVkDjiycQ1JtotLTD7vXXLud Xfw9L1St nVLbjIXXvGTL6jHUeIdpRvSLF8lpLRKgcsnnNhy9uKNXPkXa7swQNtwOtDS6PLaMUwTVvWEXT8K/6QPeds+2PCHzJV9nUcKkFvtlXnYiZwc/R60YqYdax9Nrwl4oqTrBah5ZDpKeNxGufR+oDBl4r3uz3RW09ZeZLx6JF/N9tBHzxLtlee9HSugDA0zJqrRhJaSlU9ylk+CbxR4cwDl5D7pW3Y5XMkupdq3VuzXcKT9tyb45pLbAd4ttgChf9G19CgCpzM1RTx+8DaBgl/usQnkiDp9WiFQoElaN4gpAN5y8Zy8fkOb9v+jgSyYGFm7X+WdbUfmkoDm2/YKEYZyAdv14BQebq65EqGWldeXk4KRMKTrtuyruF8AXGbEN6yCAg05UkvsbYbil9kIgQbQghYuIwMSQ5/b+Nf4dZoxjDQR7EbDl5UOJUnzXStMTcPVUmPHh7j0/w0CZ3/nOaX0YFPlSO7NeYWCdyK7pPZb+CEVg02S953TkTylhe+s2+kBWTFI1QHliqOa/pQ4l7gWjXUokNTSUW1/mIAxyS5EKdpQvcBlQ= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Mon, Sep 18, 2023 at 02:46:02PM +0200, Mirsad Todorovac wrote: ... > Ah, I see. This is definitely not good. But I managed to fix and test the find_next_bit() > family, but this seems that simply > > ------------------------------------------- > include/linux/xarray.h | 8 -------- > 1 file changed, 8 deletions(-) > > diff --git a/include/linux/xarray.h b/include/linux/xarray.h > index 1715fd322d62..89918b65b00d 100644 > --- a/include/linux/xarray.h > +++ b/include/linux/xarray.h > @@ -1718,14 +1718,6 @@ static inline unsigned int xas_find_chunk(struct xa_state *xas, bool advance, > if (advance) > offset++; > - if (XA_CHUNK_SIZE == BITS_PER_LONG) { > - if (offset < XA_CHUNK_SIZE) { > - unsigned long data = READ_ONCE(*addr) & (~0UL << offset); > - if (data) > - return __ffs(data); > - } > - return XA_CHUNK_SIZE; > - } > return find_next_bit(addr, XA_CHUNK_SIZE, offset); > } This looks correct. As per my understanding, the removed part is the 1-word bitmap optimization for find_next_bit. If so, it's not needed because find_next_bit() bears this optimization itself. ... > -------------------------------------------------------- > lib/find_bit.c | 33 +++++++++++++++++---------------- > 1 file changed, 17 insertions(+), 16 deletions(-) > > diff --git a/lib/find_bit.c b/lib/find_bit.c > index 32f99e9a670e..56244e4f744e 100644 > --- a/lib/find_bit.c > +++ b/lib/find_bit.c > @@ -18,6 +18,7 @@ > #include > #include > #include > +#include > /* > * Common helper for find_bit() function family > @@ -98,7 +99,7 @@ out: \ > */ > unsigned long _find_first_bit(const unsigned long *addr, unsigned long size) > { > - return FIND_FIRST_BIT(addr[idx], /* nop */, size); > + return FIND_FIRST_BIT(READ_ONCE(addr[idx]), /* nop */, size); > } > EXPORT_SYMBOL(_find_first_bit); > #endif ... That doesn't look correct. READ_ONCE() implies that there's another thread modifying the bitmap concurrently. This is not the true for vast majority of bitmap API users, and I expect that forcing READ_ONCE() would affect performance for them. Bitmap functions, with a few rare exceptions like set_bit(), are not thread-safe and require users to perform locking/synchronization where needed. If you really need READ_ONCE, I think it's better to implement a new flavor of the function(s) separately, like: find_first_bit_read_once() Thanks, Yury