linux-kernel - Re: [PATCH v6 06/12] mm/hotplug: Add mem-hotplug restrictions for remove

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, 23 Apr 2019 23:21:20 +0200
From:   David Hildenbrand <david@...hat.com>
To:     Dan Williams <dan.j.williams@...el.com>, akpm@...ux-foundation.org
Cc:     Michal Hocko <mhocko@...e.com>,
        Logan Gunthorpe <logang@...tatee.com>, linux-mm@...ck.org,
        linux-nvdimm@...ts.01.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v6 06/12] mm/hotplug: Add mem-hotplug restrictions for
 remove_memory()

On 17.04.19 20:39, Dan Williams wrote:
> Teach the arch_remove_memory() path to consult the same 'struct
> mhp_restrictions' context as was specified at arch_add_memory() time.
> 
> No functional change, this is a preparation step for teaching
> __remove_pages() about how and when to allow sub-section hot-remove, and
> a cleanup for an unnecessary "is_dev_zone()" special case.

I am not yet sure if this is the right thing to do. When adding memory,
we obviously have to specify the "how". When removing memory, we usually
should be able to look such stuff up.


>  void __remove_pages(struct zone *zone, unsigned long phys_start_pfn,
> -		    unsigned long nr_pages, struct vmem_altmap *altmap)
> +		unsigned long nr_pages, struct mhp_restrictions *restrictions)
>  {
>  	unsigned long i;
> -	unsigned long map_offset = 0;
>  	int sections_to_remove;
> +	unsigned long map_offset = 0;
> +	struct vmem_altmap *altmap = restrictions->altmap;
>  
> -	/* In the ZONE_DEVICE case device driver owns the memory region */
> -	if (is_dev_zone(zone)) {
> -		if (altmap)
> -			map_offset = vmem_altmap_offset(altmap);
> -	}
> +	if (altmap)
> +		map_offset = vmem_altmap_offset(altmap);
>  

Why weren't we able to use this exact same hunk before? (after my
resource deletion cleanup of course)

IOW, do we really need struct mhp_restrictions here?

After I factor out memory device handling into the caller of
arch_remove_memory(), also the next patch ("mm/sparsemem: Prepare for
sub-section ranges") should no longer need it. Or am I missing something?

-- 

Thanks,

David / dhildenb