Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) client-ip=2620:137:e000::3:3;
From:   Daniel Gomez <da.gomez@samsung.com>
To:     Matthew Wilcox <willy@infradead.org>
CC:     "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>,
        "mcgrof@kernel.org" <mcgrof@kernel.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 v2 6/6] shmem: add large folios support to the write
 path
Thread-Topic: [PATCH v2 6/6] shmem: add large folios support to the write
        path
Thread-Index: AQHZ6wEBFE+DTgi9xUu5khR159FX3LAiLYSAgAAYPoA=
Date:   Tue, 19 Sep 2023 16:28:07 +0000
Message-ID: <20230919162805.m6hbrmwq4gm4iiv5@sarkhan>
In-Reply-To: <ZQm3vywitP+UdIHF@casper.infradead.org>
Accept-Language: en-US, en-GB
Content-Language: en-US
x-ms-exchange-messagesentrepresentingtype: 1
x-ms-exchange-transport-fromentityheader: Hosted
x-originating-ip: [106.110.32.103]
Content-Type: text/plain; charset="us-ascii"
Content-ID: <A4B5720378D78244931EB5E1F49DD8DC@scsc.local>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrEKsWRmVeSWpSXmKPExsWy7djP87qSZzhTDeY+ZLGYs34Nm8Xqu/1s
        Fpef8Fk8/dTHYrH3lrbFnr0nWSwu75rDZnFvzX9Wi11/drBb3JjwlNFi2df37Ba7Ny5is/j9
        Yw6bA6/H7IaLLB4LNpV6bF6h5XH5bKnHplWdbB6bPk1i9zgx4zeLx+dNcgEcUVw2Kak5mWWp
        Rfp2CVwZE07/Zi14KV7x+uRC5gbG4/xdjJwcEgImEnc+fGbsYuTiEBJYwSjx/fw/NgjnC6PE
        hIs32CGcz4wSG54tZINpWdm6ACqxnFFi4ukXbHBVO1edZwWpEhI4wyhxvF8SIrGSUeLv1GPs
        IAk2AU2JfSc3AdkcHCICGhJvthiB1DALHGWVWLJ2EwtIjbCAv8Tyg2/B6kUEAiTa+q8wQdhW
        Ei0n5zKD2CwCqhJ9s24wg8zhFTCVWNZZAmJyAl238asTSAWjgKzEo5W/wKYwC4hL3Hoynwni
        AUGJRbP3MEPYYhL/dj2EekxH4uz1J4wQtoHE1qX7WCBsJYk/HQsZIeboSCzY/YkNwraUWLFs
        BpStLbFs4WuwmbxA80/OfALVO5NL4tIvO5DTJARcJJq+ZECEhSVeHd/CPoFRZxaS62Yh2TAL
        yYZZSDbMQrJhASPrKkbx1NLi3PTUYqO81HK94sTc4tK8dL3k/NxNjMBkd/rf8S87GJe/+qh3
        iJGJg/EQowQHs5II70xDtlQh3pTEyqrUovz4otKc1OJDjNIcLErivNq2J5OFBNITS1KzU1ML
        UotgskwcnFINTG2pn2bKRXx4tVP/5sKDBvc+qz6SFTAufdncpmlpde2YfGHT/Sv+J26qlm4z
        3cexTvPsv22T9qyRO6HrVh9rEvvvQ0VuFL/Qr2s3fk99Wrp6f3sf5yeziS5Xf9+xemMU5/u5
        RWBH+l9O+2Me89Rf33a7MDfiVXVJvz/fcef4N1xzD0od21f4R+CVYYuvr5uU+g49fe3Dnpsa
        IhK/ZTB0nX9r6vB5z747E7JeTVrLFnx737x7hftYFicu99t7miN/+cv1tkevl9zYFPef4az3
        uTWxq7OzFO+K/V59+m1KlNmSPzbXOGdI3g80kpmjU5VYu03V3fhEydqKq8ZvU57ULTow88Uy
        uZ+TYh64374u+S9EiaU4I9FQi7moOBEAwXhDjeUDAAA=
X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrNKsWRmVeSWpSXmKPExsVy+t/xu7oSZzhTDTYv0rKYs34Nm8Xqu/1s
        Fpef8Fk8/dTHYrH3lrbFnr0nWSwu75rDZnFvzX9Wi11/drBb3JjwlNFi2df37Ba7Ny5is/j9
        Yw6bA6/H7IaLLB4LNpV6bF6h5XH5bKnHplWdbB6bPk1i9zgx4zeLx+dNcgEcUXo2RfmlJakK
        GfnFJbZK0YYWRnqGlhZ6RiaWeobG5rFWRqZK+nY2Kak5mWWpRfp2CXoZE07/Zi14KV7x+uRC
        5gbG4/xdjJwcEgImEitbF7B3MXJxCAksZZTY/2cuE0RCRmLjl6usELawxJ9rXWwQRR8ZJU5+
        O8oE4ZxhlPj/7BYjSJWQwEpGiZXH5EFsNgFNiX0nNwGN5eAQEdCQeLPFCKSeWeAoq8SStZtY
        QGqEBXwl7k3qAtsgIuAn8fPRTSjbSqLl5FxmEJtFQFWib9YNZpA5vAKmEss6SyBWvWaU6J7o
        DRLmBPpg41cnkDCjgKzEo5W/2EFsZgFxiVtP5kP9IiCxZM95ZghbVOLl439Qf+lInL3+hBHC
        NpDYunQfC4StJPGnYyEjxBwdiQW7P7FB2JYSK5bNgLK1JZYtfA02k1dAUOLkzCcsExhlZiFZ
        PQtJ+ywk7bOQtM9C0r6AkXUVo0hqaXFuem6xkV5xYm5xaV66XnJ+7iZGYCrbduznlh2MK199
        1DvEyMTBeIhRgoNZSYR3piFbqhBvSmJlVWpRfnxRaU5q8SFGU2DITWSWEk3OBybTvJJ4QzMD
        U0MTM0sDU0szYyVxXs+CjkQhgfTEktTs1NSC1CKYPiYOTqkGpv49/REhfTdlxG99YdA6pvj6
        1+quDLfN4jMm3dZY8Gl10oKzO/Rr+h9dyfk0fesCa2dv0703etc+OvPYoO74TbbCCqmtOQWm
        EVzLphbyP5tV+zsp6/eGrWwn5znPn7Jy4q/JvT5/yzUZWrn/TjQROPaScZI5oxGbxvt+1jlv
        194QfXssJzRj38uTi+Zn7myLc3k+Y4b0xuLFBUrxHcX18oV/BUPy63n1rhyNi/+qfqxfNeKY
        HfesGaph04xUpixXtvnfvv+m4KR133ac2rZn2fHPwkHc3f3eeX3lYiIVt/wW7hb97WPDwHHl
        2Le3PwsLb03O6P5j9Dnu2rRZJfbPFi1rKVllYcv8gUl8YelW/ltKLMUZiYZazEXFiQCeQcBG
        7gMAAA==
X-CMS-MailID: 20230919162808eucas1p2d773186479b84364e8adf1a286a92af0
X-Msg-Generator: CA
X-RootMTR: 20230919135556eucas1p19920c52d4af0809499eac6bbf4466117
X-EPHeader: CA
CMS-TYPE: 201P
X-CMS-RootMailID: 20230919135556eucas1p19920c52d4af0809499eac6bbf4466117
References: <20230919135536.2165715-1-da.gomez@samsung.com>
        <CGME20230919135556eucas1p19920c52d4af0809499eac6bbf4466117@eucas1p1.samsung.com>
        <20230919135536.2165715-7-da.gomez@samsung.com>
        <ZQm3vywitP+UdIHF@casper.infradead.org>
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable
	autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lipwig.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (lipwig.vger.email [0.0.0.0]); Tue, 19 Sep 2023 09:28:35 -0700 (PDT)

On Tue, Sep 19, 2023 at 04:01:19PM +0100, Matthew Wilcox wrote:
> On Tue, Sep 19, 2023 at 01:55:54PM +0000, Daniel Gomez wrote:
> > Add large folio support for shmem write path matching the same high
> > order preference mechanism used for iomap buffered IO path as used in
> > __filemap_get_folio() with a difference on the max order permitted
> > (being PMD_ORDER-1) to respect the huge mount option when large folio
> > is supported.
>
> I'm strongly opposed to "respecting the huge mount option".  We're
> determining the best order to use for the folios.  Artificially limiting
> the size because the sysadmin read an article from 2005 that said to
> use this option is STUPID.

Then, I would still have the conflict on what to do when the order is
same as huge. I guess huge does not make sense in this new scenario?
unless we add large folios controls as proposal in linux-MM meeting
notes [1]. But I'm missing a bit of context so it's not clear to me
what to do next.

[1] https://lore.kernel.org/all/4966f496-9f71-460c-b2ab-8661384ce626@arm.co=
m/T/#u

In that sense, I wanted to have a big picture of what was this new
strategy implying in terms of folio order when adding to page cache,
so I added tracing for it (same as in readahead). With bpftrace I
can see the following (notes added to explain each field) after running
fsx up to 119M:

@c: 363049108  /* total folio order being traced */
@order[8]: 2 /* order 8 being used 2 times (add_to_page_cache) */
@order[5]: 3249587 */ order 5 being used 3249587 times
(add_to_page_cache) */
@order[4]: 5972205
@order[3]: 8890418
@order[2]: 10380055
@order[0]: 334556841
@order_2: /* linear histogram of folio order */
[0, 1)          334556841 |@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@=
@@@@|
[1, 2)                 0  |                                                =
    |
[2, 3)          10380055  |@                                               =
    |
[3, 4)           8890418  |@                                               =
    |
[4, 5)           5972205  |                                                =
    |
[5, 6)           3249587  |                                                =
    |
[6, 7)                 0  |                                                =
    |
[7, 8)                 0  |                                                =
    |
[8, 9)                 2  |                                                =
    |

I guess that's not te best workload to see this but would tracing be also
interesting to add to the series?
>
> >  	else
> > -		folio =3D shmem_alloc_folio(gfp, info, index, *order);
> > +		folio =3D shmem_alloc_folio(gfp, info, index, order);
>
> Why did you introduce it as *order, only to change it back to order
> in this patch?  It feels like you just fixed up patch 6 rather than
> percolating the changes all the way back to where they should have
> been done.  This makes the reviewer's life hard.
>

Sorry about that. I missed it in my changes.=