linux-kernel - Re: [PATCH v2 6/6] shmem: add large folios support to the write path

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 19 Sep 2023 16:01:19 +0100
From:   Matthew Wilcox <willy@...radead.org>
To:     Daniel Gomez <da.gomez@...sung.com>
Cc:     "minchan@...nel.org" <minchan@...nel.org>,
        "senozhatsky@...omium.org" <senozhatsky@...omium.org>,
        "axboe@...nel.dk" <axboe@...nel.dk>,
        "djwong@...nel.org" <djwong@...nel.org>,
        "hughd@...gle.com" <hughd@...gle.com>,
        "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
        "mcgrof@...nel.org" <mcgrof@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-block@...r.kernel.org" <linux-block@...r.kernel.org>,
        "linux-xfs@...r.kernel.org" <linux-xfs@...r.kernel.org>,
        "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "gost.dev@...sung.com" <gost.dev@...sung.com>,
        Pankaj Raghav <p.raghav@...sung.com>
Subject: Re: [PATCH v2 6/6] shmem: add large folios support to the write path

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.

>  	else
> -		folio = shmem_alloc_folio(gfp, info, index, *order);
> +		folio = 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.