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 C5CC4C3F68F for ; Thu, 6 Feb 2020 14:36:30 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 93B70214AF for ; Thu, 6 Feb 2020 14:36:30 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 93B70214AF 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 30EF36B0007; Thu, 6 Feb 2020 09:36:30 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 2BF066B0008; Thu, 6 Feb 2020 09:36:30 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1FC436B000A; Thu, 6 Feb 2020 09:36:30 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0131.hostedemail.com [216.40.44.131]) by kanga.kvack.org (Postfix) with ESMTP id 0610D6B0007 for ; Thu, 6 Feb 2020 09:36:30 -0500 (EST) Received: from smtpin08.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id AAFC0181AEF07 for ; Thu, 6 Feb 2020 14:36:29 +0000 (UTC) X-FDA: 76459952898.08.slope30_72df11be7ed0b X-HE-Tag: slope30_72df11be7ed0b X-Filterd-Recvd-Size: 3420 Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by imf14.hostedemail.com (Postfix) with ESMTP for ; Thu, 6 Feb 2020 14:36:29 +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 CDD29AC91; Thu, 6 Feb 2020 14:36:27 +0000 (UTC) Received: by quack2.suse.cz (Postfix, from userid 1000) id 862E01E0DEB; Thu, 6 Feb 2020 15:36:27 +0100 (CET) Date: Thu, 6 Feb 2020 15:36:27 +0100 From: Jan Kara To: Jason Gunthorpe Cc: Matthew Wilcox , Jan Kara , linux-fsdevel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH 8/8] xarray: Don't clear marks in xas_store() Message-ID: <20200206143627.GA26114@quack2.suse.cz> References: <20200204142514.15826-1-jack@suse.cz> <20200204142514.15826-9-jack@suse.cz> <20200205184344.GB28298@ziepe.ca> <20200205215904.GT8731@bombadil.infradead.org> <20200206134904.GD25297@ziepe.ca> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200206134904.GD25297@ziepe.ca> User-Agent: Mutt/1.10.1 (2018-07-13) X-Bogosity: Ham, tests=bogofilter, spamicity=0.000001, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Thu 06-02-20 09:49:04, Jason Gunthorpe wrote: > On Wed, Feb 05, 2020 at 01:59:04PM -0800, Matthew Wilcox wrote: > > > > How is RCU mark reading used anyhow? > > > > We iterate over pages in the page cache with, eg, the dirty bit set. > > This bug will lead to the loop terminating early and failing to find > > dirty pages that it should. > > But the inhernetly weak sync of marks and pointers means that > iteration will miss some dirty pages and return some clean pages. It > is all OK for some reason? Yes. The reason is - the definitive source of dirtiness is in page flags so if iteration returns more pages, we don't care. WRT missing pages we only need to make sure pages that were dirty before the iteration started are returned and the current code fulfills that. > > > Actually the clearing of marks by xa_store(, NULL) is creating a very > > > subtle bug in drivers/infiniband/core/device.c :( Can you add a Fixes > > > line too: > > > > > > ib_set_client_data() is assuming the marks for the entry will not > > > change, but if the caller passed in NULL they get wrongly reset, and > > > three call sites pass in NULL: > > > drivers/infiniband/ulp/srpt/ib_srpt.c > > > net/rds/ib.c > > > net/smc/smc_ib.c > > > Fixes: 0df91bb67334 ("RDMA/devices: Use xarray to store the client_data") > > > > There's no bug here. > > > > If you're actually storing NULL in the array, then the marks would go > > away. That's inherent -- imagine you have an array with a single entry > > at 64. Then you store NULL there. That causes the node to be deleted, > > and the marks must necessarily disappear with it -- there's nowhere to > > store them! > > Ah, it is allocating! These little behavior differences are tricky to > remember over with infrequent use :( Yeah, that's why I'd prefer if NULL was not "special value" at all and if someone wanted to remove index from xarray he'd always have to use a special function. My patches go towards that direction but not the full way because there's still xa_cmpxchg() whose users use the fact that NULL is in fact 'erase'. Honza -- Jan Kara SUSE Labs, CR