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=-6.5 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS autolearn=unavailable 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 BC730C2D0EC for ; Thu, 26 Mar 2020 17:16:11 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 7881220737 for ; Thu, 26 Mar 2020 17:16:11 +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="kJhoAEvL" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7881220737 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 229746B000A; Thu, 26 Mar 2020 13:16:11 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 1B3686B000C; Thu, 26 Mar 2020 13:16:11 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 07B456B000D; Thu, 26 Mar 2020 13:16:11 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0135.hostedemail.com [216.40.44.135]) by kanga.kvack.org (Postfix) with ESMTP id DD4DA6B000A for ; Thu, 26 Mar 2020 13:16:10 -0400 (EDT) Received: from smtpin05.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 097F48249980 for ; Thu, 26 Mar 2020 17:16:11 +0000 (UTC) X-FDA: 76638166542.05.tree42_8d3ec191d5702 X-HE-Tag: tree42_8d3ec191d5702 X-Filterd-Recvd-Size: 4195 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) by imf05.hostedemail.com (Postfix) with ESMTP for ; Thu, 26 Mar 2020 17:16:10 +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=kzeGasS7YYQLPvzBhzXfybjPCnozbeaYoOmZNc1qJpM=; b=kJhoAEvLJs13WCqIrUDgq07Con n33EYLnv/zbKE+if4MQ6Mim05UMpPiTTnU/c68AY3mz089vkkQPt0xySMVedJZnS4zlOUd6bdNzlU iLkLXwvUUDTBtePNzV7EtcQkI3HNSg38QfhfMPGT6s54Dbx4WIj9d1PFg/oLtNaNfYGLG6wBSwJKr 1w31APKig5PViCmMeBocszEw8JjVeBMPzMGNJERL3WItE/s1Dz0S22pH8tIDx8HPSYpmsjcFWMIRw qXjXl6OIRHYEhzghj2HcLetGrvZCQy45OvbqQSC9rZprRrjBCIkFnD3DVO0vuzBwdEKKIQSn5ffYr vItOk+6w==; Received: from willy by bombadil.infradead.org with local (Exim 4.92.3 #3 (Red Hat Linux)) id 1jHW6y-0004lW-M6; Thu, 26 Mar 2020 17:16:08 +0000 Date: Thu, 26 Mar 2020 10:16:08 -0700 From: Matthew Wilcox To: Christoph Hellwig Cc: linux-mm@kvack.org, linux-fsdevel@vger.kernel.org Subject: Re: [PATCH 2/2] mm: Use clear_bit_unlock_is_negative_byte for PageWriteback Message-ID: <20200326171608.GI22483@bombadil.infradead.org> References: <20200326122429.20710-1-willy@infradead.org> <20200326122429.20710-3-willy@infradead.org> <20200326171114.GA6566@infradead.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200326171114.GA6566@infradead.org> 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 Thu, Mar 26, 2020 at 10:11:14AM -0700, Christoph Hellwig wrote: > On Thu, Mar 26, 2020 at 05:24:29AM -0700, Matthew Wilcox wrote: > > From: "Matthew Wilcox (Oracle)" > > > > By moving PG_writeback down into the low bits of the page flags, we can > > use clear_bit_unlock_is_negative_byte() for writeback as well as the > > lock bit. wake_up_page() then has no more callers. Given the other > > code being executed between the clear and the test, this is not going > > to be as dramatic a win as it was for PageLocked, but symmetry between > > the two is nice and lets us remove some code. > > > > Signed-off-by: Matthew Wilcox (Oracle) > > --- > > include/linux/page-flags.h | 6 +++--- > > mm/filemap.c | 19 ++++++------------- > > mm/page-writeback.c | 37 ++++++++++++++++++++----------------- > > 3 files changed, 29 insertions(+), 33 deletions(-) > > > > diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h > > index 222f6f7b2bb3..96c7d220c8cf 100644 > > --- a/include/linux/page-flags.h > > +++ b/include/linux/page-flags.h > > @@ -103,13 +103,14 @@ > > */ > > enum pageflags { > > PG_locked, /* Page is locked. Don't touch. */ > > + PG_writeback, /* Page is under writeback */ > > Do we need a comment why these need to be in the low bits? There's some nice build time assertions that they are, next to all the documentation about why and how it all works: @@ -1266,6 +1259,7 @@ EXPORT_SYMBOL_GPL(add_page_wait_queue); void unlock_page(struct page *page) { BUILD_BUG_ON(PG_waiters != 7); + BUILD_BUG_ON(PG_locked > 7); page = compound_head(page); VM_BUG_ON_PAGE(!PageLocked(page), page); if (clear_bit_unlock_is_negative_byte(PG_locked, &page->flags)) @@ -1279,23 +1273,22 @@ EXPORT_SYMBOL(unlock_page); */ void end_page_writeback(struct page *page) { + BUILD_BUG_ON(PG_writeback > 7); so if anyone moves them out of the bottom byte, they'll see those assertions fail and hopefully read this comment: * Note that this depends on PG_waiters being the sign bit in the byte * that contains PG_locked - thus the BUILD_BUG_ON(). That allows us to * clear the PG_locked bit and test PG_waiters at the same time fairly * portably (architectures that do LL/SC can test any bit, while x86 can * test the sign bit). I'd expect any reviewer to notice a BUILD_BUG_ON() being removed and query it if not explained in the changelog.