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 X-Spam-Level: X-Spam-Status: No, score=-2.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2290CC35247 for ; Mon, 3 Feb 2020 14:27:48 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id E56612082E for ; Mon, 3 Feb 2020 14:27:47 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org E56612082E Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=suse.cz Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 6E2A16B066C; Mon, 3 Feb 2020 09:27:47 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 6926A6B066D; Mon, 3 Feb 2020 09:27:47 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5A8206B066E; Mon, 3 Feb 2020 09:27:47 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0044.hostedemail.com [216.40.44.44]) by kanga.kvack.org (Postfix) with ESMTP id 459086B066C for ; Mon, 3 Feb 2020 09:27:47 -0500 (EST) Received: from smtpin04.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id 13DAD2463 for ; Mon, 3 Feb 2020 14:27:47 +0000 (UTC) X-FDA: 76449044574.04.order19_9b5ab47eb030 X-HE-Tag: order19_9b5ab47eb030 X-Filterd-Recvd-Size: 2781 Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by imf05.hostedemail.com (Postfix) with ESMTP for ; Mon, 3 Feb 2020 14:27:46 +0000 (UTC) X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id EBE89AE1C; Mon, 3 Feb 2020 14:27:44 +0000 (UTC) Received: by quack2.suse.cz (Postfix, from userid 1000) id 8E3491E0D5D; Mon, 3 Feb 2020 15:09:37 +0100 (CET) Date: Mon, 3 Feb 2020 15:09:37 +0100 From: Jan Kara To: Matthew Wilcox Cc: linux-mm@kvack.org, linux-fsdevel@vger.kernel.org Subject: Race in xarray tagged iteration Message-ID: <20200203140937.GA18591@quack2.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.10.1 (2018-07-13) 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: Hello Matthew! Lately I've been looking into speeding up page cache truncation that got slowed down by the conversion of page cache to xarray as we spoke about back in February / March [1]. Now I have relatively simple patch giving me around 6% improvement in truncation speeds on my test machine but when testing it and debugging issues, I've found out that current xarray tagged iteration is racy: TASK1 TASK2 page_cache_delete() find_get_pages_range_tag() xas_for_each_marked() xas_find_marked() off = xas_find_chunk() xas_store(&xas, NULL) xas_init_marks(&xas); ... rcu_assign_pointer(*slot, NULL); entry = xa_entry(off); So xas_for_each_marked() can return NULL entries as tagged thus aborting xas_for_each_marked() iteration prematurely (data loss possible). Now I have a patch to change xas_for_each_marked() to not get confused by NULL entries (because that is IMO a fragile design anyway and easy to avoid AFAICT) but that still leaves us with find_get_pages_range_tag() getting NULL as tagged entry and that causes oops there. I see two options how to fix this and I'm not quite decided which is better: 1) Just add NULL checking to find_get_pages_range_tag() similarly to how it currently checks xa_is_value(). Quick grepping seems to show that that place is the only place that uses tagged iteration under RCU. It is cheap but kind of ugly. 2) Make sure xas_find_marked() and xas_next_marked() do recheck marks after loading the entry. This is more convenient for the callers but potentially more expensive since we'd have to add some barriers there. What's your opinion? I'm leaning more towards 1) but I'm not completely decided... Honza [1] https://lore.kernel.org/linux-mm/20190226165628.GB24711@quack2.suse.cz -- Jan Kara SUSE Labs, CR