linux-kernel - Re: [RFC PATCH 2/3] mm/memory_hotplug: Create __shrink_pages and move it to offline

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:   Wed, 8 Aug 2018 11:45:02 +0200
From:   Oscar Salvador <osalvador@...hadventures.net>
To:     Jerome Glisse <jglisse@...hat.com>
Cc:     Michal Hocko <mhocko@...nel.org>, akpm@...ux-foundation.org,
        dan.j.williams@...el.com, david@...hat.com, yasu.isimatu@...il.com,
        logang@...tatee.com, dave.jiang@...el.com, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org, Oscar Salvador <osalvador@...e.de>
Subject: Re: [RFC PATCH 2/3] mm/memory_hotplug: Create __shrink_pages and
 move it to offline_pages

On Tue, Aug 07, 2018 at 11:18:10AM -0400, Jerome Glisse wrote:
> Correct, you should not call release_mem_region_adjustable() the device
> region is not part of regular iomem resource as it might not necessarily
> be enumerated through known ways to the kernel (ie only the device driver
> can discover the region and core kernel do not know about it).
> 
> One of the issue to adding this region to iomem resource is that they
> really need to be ignored by core kernel because you can not assume that
> CPU can actually access them. Moreover, if CPU can access them it is
> likely that CPU can not do atomic operation on them (ie what happens on
> a CPU atomic instruction is undefined). So they are _special_ and only
> make sense to be use in conjunction with a device driver.
> 
> 
> Also in the case they do exist in iomem resource it is as PCIE BAR so
> as IORESOURCE_IO (iirc) and thus release_mem_region_adjustable() would
> return -EINVAL. Thought nothing bad happens because of that, only a
> warning message that might confuse the user.

Just to see if I understand this correctly.
I guess that these regions are being registered via devm_request_mem_region() calls.
Among other callers, devm_request_mem_region() is being called from:

dax_pmem_probe
hmm_devmem_add

AFAICS from the code, those regions will inherit the flags from the parent, which is iomem_resource:

#define devm_request_mem_region(dev,start,n,name) \
	__devm_request_region(dev, &iomem_resource, (start), (n), (name))

struct resource iomem_resource = {
	.name	= "PCI mem",
	.start	= 0,
	.end	= -1,
	.flags	= IORESOURCE_MEM,
};


struct resource * __request_region()
{
	...
	...
	res->flags = resource_type(parent) | resource_ext_type(parent);
	res->flags |= IORESOURCE_BUSY | flags;
	res->desc = parent->desc;
	...
	...
}

So the regions will not be tagged as IORESOURCE_IO but IORESOURCE_MEM.
>From the first glance release_mem_region_adjustable() looks like it does
more things than __release_region(), and I did not check it deeply
but maybe we can make it work.

Thanks
-- 
Oscar Salvador
SUSE L3