Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.34 as permitted sender) client-ip=23.128.96.34;
Date:   Mon, 18 Sep 2023 19:24:55 +0100
From:   Matthew Wilcox <willy@infradead.org>
To:     Luis Chamberlain <mcgrof@kernel.org>
Cc:     Daniel Gomez <da.gomez@samsung.com>,
        "minchan@kernel.org" <minchan@kernel.org>,
        "senozhatsky@chromium.org" <senozhatsky@chromium.org>,
        "axboe@kernel.dk" <axboe@kernel.dk>,
        "djwong@kernel.org" <djwong@kernel.org>,
        "hughd@google.com" <hughd@google.com>,
        "akpm@linux-foundation.org" <akpm@linux-foundation.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
        "linux-xfs@vger.kernel.org" <linux-xfs@vger.kernel.org>,
        "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
        "linux-mm@kvack.org" <linux-mm@kvack.org>,
        "gost.dev@samsung.com" <gost.dev@samsung.com>,
        Pankaj Raghav <p.raghav@samsung.com>
Subject: Re: [PATCH 1/6] filemap: make the folio order calculation shareable
Message-ID: <ZQiV96mTMiSXF9yf@casper.infradead.org>
References: <20230915095042.1320180-1-da.gomez@samsung.com>
 <CGME20230915095124eucas1p1eb0e0ef883f6316cf14c349404a51150@eucas1p1.samsung.com>
 <20230915095042.1320180-2-da.gomez@samsung.com>
 <ZQRet4w5VSbvKvKB@casper.infradead.org>
 <ZQiSPEKRJSkeh3Fe@bombadil.infradead.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZQiSPEKRJSkeh3Fe@bombadil.infradead.org>
Precedence: bulk

On Mon, Sep 18, 2023 at 11:09:00AM -0700, Luis Chamberlain wrote:
> On Fri, Sep 15, 2023 at 02:40:07PM +0100, Matthew Wilcox wrote:
> > On Fri, Sep 15, 2023 at 09:51:23AM +0000, Daniel Gomez wrote:
> > > To make the code that clamps the folio order in the __filemap_get_folio
> > > routine reusable to others, move and merge it to the fgf_set_order
> > > new subroutine (mapping_size_order), so when mapping the size at a
> > > given index, the order calculated is already valid and ready to be
> > > used when order is retrieved from fgp_flags with FGF_GET_ORDER.
> > > 
> > > Signed-off-by: Daniel Gomez <da.gomez@samsung.com>
> > > ---
> > >  fs/iomap/buffered-io.c  |  6 ++++--
> > >  include/linux/pagemap.h | 42 ++++++++++++++++++++++++++++++++++++-----
> > >  mm/filemap.c            |  8 --------
> > >  3 files changed, 41 insertions(+), 15 deletions(-)
> > 
> > That seems like a lot of extra code to add in order to avoid copying
> > six lines of code and one comment into the shmem code.
> > 
> > It's not wrong, but it seems like a bad tradeoff to me.
> 
> The suggestion to merge came from me, mostly based on later observations
> that in the future we may want to extend this with a min order to ensure
> the index is aligned the the order. This check would only be useful for
> buffred IO for iomap, readahead. It has me wondering if buffer-heads
> support for large order folios come around would we a similar check
> there?
> 
> So Willy, you would know better if and when a shared piece of code would
> be best with all these things in mind.

In my mind, this is fundamentally code which belongs in the page cache
rather than in individual filesystems.  The fly in the ointment is that
shmem has forked the page cache in order to do its own slightly
specialised thing.  I don't see the buffer_head connection; shmem is
an extremely special case, and we shouldn't mess around with other
filesystems to avoid changing shmem.

Ideally, we'd reunify (parts of) shmem and the regular page cache, but
that's a lot of work, probably involving the swap layer changing.