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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 36FC8C7EE23 for ; Mon, 5 Jun 2023 14:31:29 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232973AbjFEOb1 (ORCPT ); Mon, 5 Jun 2023 10:31:27 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53614 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233076AbjFEObZ (ORCPT ); Mon, 5 Jun 2023 10:31:25 -0400 Received: from casper.infradead.org (casper.infradead.org [IPv6:2001:8b0:10b:1236::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 73A27E8; Mon, 5 Jun 2023 07:31:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.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=6izMVQ9m9HEYT6yx5F2UmGBTPWgGa26OUk67j+agszA=; b=NoBJSmHH9oHu83+wHfrXfLoSWy P/IhwCMWavyo6edAdqB0Gx34lmUEFqSbKXoQ863oNFdz14OiC+1Wwy4onkAWYCkHb3U+/W63uJNWs Z922V1pr1wdCDeJTqbFcVtDzQoq3ZwJpadTDJDHUzHrdG52UtK9krO4PerOpBa+6CHGyyJVL8VDrr UI4a5BhX6PSPUUTAxdL/xVUSHCx/R3Cf6AYxR1rkVrsq6aqUHm2wabWxRm2e0bTdX8kNu+jpNnnC7 yHPl/AAPMdsqfI9IKK/iUIm0HpcgR9beJmmic9mSzppk4M3SUfqRZ2k85mXKoLlEC6sBk36VNguPI F6xbLKPQ==; Received: from willy by casper.infradead.org with local (Exim 4.94.2 #2 (Red Hat Linux)) id 1q6BEt-00C6SO-8g; Mon, 05 Jun 2023 14:31:19 +0000 Date: Mon, 5 Jun 2023 15:31:19 +0100 From: Matthew Wilcox To: Andreas Gruenbacher Cc: "Ritesh Harjani (IBM)" , linux-xfs@vger.kernel.org, linux-fsdevel@vger.kernel.org, Dave Chinner , Brian Foster , Christoph Hellwig , Ojaswin Mujoo , Disha Goel Subject: Re: [PATCHv7 3/6] iomap: Refactor some iop related accessor functions Message-ID: References: <4fe4937718d44c89e0c279175c65921717d9f591.1685962158.git.ritesh.list@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org On Mon, Jun 05, 2023 at 04:15:31PM +0200, Andreas Gruenbacher wrote: > Note that to_iomap_page() does folio_test_private() followed by > folio_get_private(), which doesn't really make sense in places where > we know that iop is defined. Maybe we want to split that up. The plan is to retire the folio private flag entirely. I just haven't got round to cleaning up iomap yet. For folios which we know to be file-backed, we can just test whether folio->private is NULL or not. So I'd do this as: - struct iomap_page *iop = to_iomap_page(folio); + struct iomap_page *iop = folio->private; and not even use folio_get_private() or to_iomap_page() any more. > > + unsigned int first_blk = off >> inode->i_blkbits; > > + unsigned int last_blk = (off + len - 1) >> inode->i_blkbits; > > + unsigned int nr_blks = last_blk - first_blk + 1; > > + unsigned long flags; > > + > > + spin_lock_irqsave(&iop->state_lock, flags); > > + bitmap_set(iop->state, first_blk, nr_blks); > > + if (iop_test_full_uptodate(folio)) > > + folio_mark_uptodate(folio); > > + spin_unlock_irqrestore(&iop->state_lock, flags); > > +} > > + > > +static void iomap_iop_set_range_uptodate(struct inode *inode, > > + struct folio *folio, size_t off, size_t len) > > +{ > > + struct iomap_page *iop = to_iomap_page(folio); > > + > > + if (iop) > > + iop_set_range_uptodate(inode, folio, off, len); > > + else > > + folio_mark_uptodate(folio); > > +} > > This patch passes the inode into iomap_iop_set_range_uptodate() and > removes the iop argument. Can this be done in a separate patch, > please? > > We have a few places like the above, where we look up the iop without > using it. Would a function like folio_has_iop() make more sense? I think this is all a symptom of the unnecessary splitting of iomap_iop_set_range_uptodate and iop_set_range_uptodate.