linux-kernel - Re: [PATCH v2 00/28] Convert GUP to folios

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]

Message-ID: <20220110153103.GH6467@ziepe.ca>
Date:   Mon, 10 Jan 2022 11:31:03 -0400
From:   Jason Gunthorpe <jgg@...pe.ca>
To:     "Matthew Wilcox (Oracle)" <willy@...radead.org>
Cc:     linux-mm@...ck.org, John Hubbard <jhubbard@...dia.com>,
        Christoph Hellwig <hch@...radead.org>,
        William Kucharski <william.kucharski@...cle.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 00/28] Convert GUP to folios


On Mon, Jan 10, 2022 at 04:23:38AM +0000, Matthew Wilcox (Oracle) wrote:
> This patch series is against my current folio for-next branch.  I know
> it won't apply to sfr's next tree, and it's not for-next material yet.
> I intend to submit it for 5.18 after I've rebased it to one of the
> 5.17-rc releases.
> 
> The overall effect of this (ignoring the primary "preparing for folios
> that are not PAGE or PMD sized" purpose) is to reduce the size of gup.o
> by ~700 bytes in the config I normally test with.
> 
> This patchset just converts existing implementations to use folios.
> There's no new API for consumers here to provide information in a more
> efficient (address, length) format.  That will be a separate patchset.
> 
> In v2, I've tried to address all the comments from the reviews I got
> on v1.  Apologies if I missed anything; I got a lot of good feedback.
> Primarily I separated out the folio changes (later) from the "While
> I'm looking at this ..." changes (earlier).  I'm not sure the story
> of the patchset is necessarily coherent this way, but it should be
> easier to review.
> 
> Another big change is that compound_pincount is now available to all
> compound pages, not just those that are order-2-or-higher.  Patch 11.
> 
> I did notice one bug in my original patchset which I'm disappointed GCC
> didn't diagnose:
> 
> 		pages[nr++] = nth_page(page, nr);
> 
> Given the massive reorg of the patchset, I didn't feel right using
> anyone's SoB from v1 on any of the patches.  But, despite the increased
> number of patches, I hope it's easier to review than v1.
> 
> And I'd dearly love a better name than 'folio_nth'.  page_nth() is
> a temporary construct, so doesn't need a better name.  If you need
> context, it's in the gup_folio_range_next() patch and its job
> is to tell you, given a page and a folio, what # page it is within
> a folio (so a number between [0 and folio_nr_pages())).

folio_tail_index() ?

Series still looks good to me, though I checked each patch
carefully than the prior series:

Reviewed-by: Jason Gunthorpe <jgg@...dia.com>

Jason