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=-0.6 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS 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 381C6C0044D for ; Sat, 14 Mar 2020 19:55:02 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id B62BC20774 for ; Sat, 14 Mar 2020 19:55:01 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="tZb1OlDu" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B62BC20774 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=infradead.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 1F9216B0005; Sat, 14 Mar 2020 15:55:01 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 1838C6B0006; Sat, 14 Mar 2020 15:55:01 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 04A5F6B0007; Sat, 14 Mar 2020 15:55:00 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0064.hostedemail.com [216.40.44.64]) by kanga.kvack.org (Postfix) with ESMTP id DA0F16B0005 for ; Sat, 14 Mar 2020 15:55:00 -0400 (EDT) Received: from smtpin04.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id 8B795181AEF3F for ; Sat, 14 Mar 2020 19:55:00 +0000 (UTC) X-FDA: 76595021160.04.sky92_656e6f488341e X-HE-Tag: sky92_656e6f488341e X-Filterd-Recvd-Size: 3380 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) by imf06.hostedemail.com (Postfix) with ESMTP for ; Sat, 14 Mar 2020 19:54:56 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20170209; h=In-Reply-To:Content-Type:MIME-Version :References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=uEmqxb+PLDhAqiQj+TB1T58aNriZfkwYBvEAt6vKntk=; b=tZb1OlDuQGZDCFk3bfbZFM2yZ+ turf0R5msUmaz6ORycgL+6YjsdxEgTUhhJqlqugR8FcfXWGyYw+BaMnYlpnaJD5YLXcc2lz9FsfUD 7dmu/mGfZXX32c8j55Djb2CCmbZYB3DIsLWlb3TKjBPnUBKpXmSAjGJvhMivptM4wBceoL6d9ho3O aa+mwEZsGXQgHUyitZx2f1GNmT2L2ptiT/cjbu5vCG2CKAz9wR08Xr5/TAwMqRJlBBeIKnz3rRsLH Egc0mZ294nrpoKbXSgH2XijCZIkS0/Hw/y6VsA+FuLLJPY01HEPveyp5SSu3HoEf5Hqgrw477TsBQ XZ4BHVdg==; Received: from willy by bombadil.infradead.org with local (Exim 4.92.3 #3 (Red Hat Linux)) id 1jDCs1-00032O-Pr; Sat, 14 Mar 2020 19:54:53 +0000 Date: Sat, 14 Mar 2020 12:54:53 -0700 From: Matthew Wilcox To: Jan Kara Cc: linux-fsdevel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH 2/8] xarray: Provide xas_erase() helper Message-ID: <20200314195453.GS22433@bombadil.infradead.org> References: <20200204142514.15826-1-jack@suse.cz> <20200204142514.15826-3-jack@suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200204142514.15826-3-jack@suse.cz> X-Bogosity: Ham, tests=bogofilter, spamicity=0.000012, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Tue, Feb 04, 2020 at 03:25:08PM +0100, Jan Kara wrote: > Currently xas_store() clears marks when stored value is NULL. This is > somewhat counter-intuitive and also causes measurable performance impact > when mark clearing is not needed (e.g. because marks are already clear). > So provide xas_erase() helper (similarly to existing xa_erase()) which > stores NULL at given index and also takes care of clearing marks. Use > this helper from __xa_erase() and item_kill_tree() in tools/testing. In > the following patches, callers that use the mark-clearing property of > xas_store() will be converted to xas_erase() and remaining users can > enjoy better performance. I (finally!) figured out what I don't like about this series. You're changing the semantics of xas_store() without changing the name, so if we have any new users in flight they'll use the new semantics when they're expecting the old. Further, while you've split the patches nicely for review, they're not good for bisection because the semantic change comes right at the end of the series, so any problem due to this series is going to bisect to the end, and not tell us anything useful. What I think this series should do instead is: Patch 1: +#define xas_store(xas, entry) xas_erase(xas, entry) -void *xas_store(struct xa_state *xas, void *entry) +void *__xas_store(struct xa_state *xas, void *entry) - if (!entry) - xas_init_marks(xas); +void *xas_erase(struct xa_state *xas, void *entry) +{ + xas_init_marks(xas); + return __xas_store(xas, entry); +} (also documentation changes) Patches 2-n: Change each user of xas_store() to either xas_erase() or __xas_store() Patch n+1: -#define xas_store(xas, entry) xas_erase(xas, entry) Does that make sense? I'll code it up next week unless you want to.