linux-kernel - Re: [PATCH v2 1/1] [PATCH] mm/gup: avoid an unnecessary allocation call for FOLL

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: <80500348-e86a-4b8b-94a5-1953aa4ebe12@redhat.com>
Date: Tue, 5 Nov 2024 09:47:23 +0100
From: David Hildenbrand <david@...hat.com>
To: John Hubbard <jhubbard@...dia.com>,
 Andrew Morton <akpm@...ux-foundation.org>
Cc: LKML <linux-kernel@...r.kernel.org>, linux-mm@...ck.org,
 Vivek Kasireddy <vivek.kasireddy@...el.com>, Dave Airlie
 <airlied@...hat.com>, Gerd Hoffmann <kraxel@...hat.com>,
 Matthew Wilcox <willy@...radead.org>, Christoph Hellwig <hch@...radead.org>,
 Jason Gunthorpe <jgg@...dia.com>, Peter Xu <peterx@...hat.com>,
 Arnd Bergmann <arnd@...db.de>, Daniel Vetter <daniel.vetter@...ll.ch>,
 Dongwon Kim <dongwon.kim@...el.com>, Hugh Dickins <hughd@...gle.com>,
 Junxiao Chang <junxiao.chang@...el.com>,
 Mike Kravetz <mike.kravetz@...cle.com>, Oscar Salvador <osalvador@...e.de>,
 linux-stable@...r.kernel.org
Subject: Re: [PATCH v2 1/1] [PATCH] mm/gup: avoid an unnecessary allocation
 call for FOLL_LONGTERM cases

On 05.11.24 04:29, John Hubbard wrote:
> commit 53ba78de064b ("mm/gup: introduce
> check_and_migrate_movable_folios()") created a new constraint on the
> pin_user_pages*() API family: a potentially large internal allocation
> must now occur, for FOLL_LONGTERM cases.
> 
> A user-visible consequence has now appeared: user space can no longer
> pin more than 2GB of memory anymore on x86_64. That's because, on a 4KB
> PAGE_SIZE system, when user space tries to (indirectly, via a device
> driver that calls pin_user_pages()) pin 2GB, this requires an allocation
> of a folio pointers array of MAX_PAGE_ORDER size, which is the limit for
> kmalloc().
> 
> In addition to the directly visible effect described above, there is
> also the problem of adding an unnecessary allocation. The **pages array
> argument has already been allocated, and there is no need for a
> redundant **folios array allocation in this case.
> 
> Fix this by avoiding the new allocation entirely. This is done by
> referring to either the original page[i] within **pages, or to the
> associated folio. Thanks to David Hildenbrand for suggesting this
> approach and for providing the initial implementation (which I've tested
> and adjusted slightly) as well.
> 
> Fixes: 53ba78de064b ("mm/gup: introduce check_and_migrate_movable_folios()")
> Suggested-by: David Hildenbrand <david@...hat.com>
> Cc: Vivek Kasireddy <vivek.kasireddy@...el.com>
> Cc: Dave Airlie <airlied@...hat.com>
> Cc: Gerd Hoffmann <kraxel@...hat.com>
> Cc: Matthew Wilcox <willy@...radead.org>
> Cc: Christoph Hellwig <hch@...radead.org>
> Cc: Jason Gunthorpe <jgg@...dia.com>
> Cc: Peter Xu <peterx@...hat.com>
> Cc: Arnd Bergmann <arnd@...db.de>
> Cc: Daniel Vetter <daniel.vetter@...ll.ch>
> Cc: Dongwon Kim <dongwon.kim@...el.com>
> Cc: Hugh Dickins <hughd@...gle.com>
> Cc: Junxiao Chang <junxiao.chang@...el.com>
> Cc: Mike Kravetz <mike.kravetz@...cle.com>
> Cc: Oscar Salvador <osalvador@...e.de>
> Cc: linux-stable@...r.kernel.org
> Signed-off-by: John Hubbard <jhubbard@...dia.com>
> ---



>   
>   	return ret;
>   }
>   
> +static long
> +check_and_migrate_movable_pages_or_folios(struct pages_or_folios *pofs)
> +{
> +	LIST_HEAD(movable_folio_list);
> +	unsigned long collected;
> +
> +	collected =
> +		collect_longterm_unpinnable_folios(&movable_folio_list, pofs);

Nit: We're allowed to use more than 80 characters :)

(I would prefer the old way it was split across more lines if we really 
want to split; this way here is less common)


Thanks!

Acked-by: David Hildenbrand <david@...hat.com>

-- 
Cheers,

David / dhildenb