* [PATCH 06/17] mm: pass the vmem_altmap to arch_remove_memory and __remove_pages
From: Christoph Hellwig @ 2017-12-29 7:53 UTC (permalink / raw)
To: Dan Williams
Cc: Jérôme Glisse, Logan Gunthorpe, Michal Hocko,
linux-nvdimm, linuxppc-dev, x86, linux-mm, linux-kernel
In-Reply-To: <20171229075406.1936-1-hch@lst.de>
We can just pass this on instead of having to do a radix tree lookup
without proper locking 2 levels into the callchain.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
---
arch/ia64/mm/init.c | 4 ++--
arch/powerpc/mm/mem.c | 6 ++----
arch/s390/mm/init.c | 2 +-
arch/sh/mm/init.c | 4 ++--
arch/x86/mm/init_32.c | 4 ++--
arch/x86/mm/init_64.c | 6 ++----
include/linux/memory_hotplug.h | 5 +++--
kernel/memremap.c | 2 +-
mm/hmm.c | 4 ++--
mm/memory_hotplug.c | 8 ++------
10 files changed, 19 insertions(+), 26 deletions(-)
diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
index 2e2e4f532204..6a8ce9e1536e 100644
--- a/arch/ia64/mm/init.c
+++ b/arch/ia64/mm/init.c
@@ -663,7 +663,7 @@ int arch_add_memory(int nid, u64 start, u64 size, struct vmem_altmap *altmap,
}
#ifdef CONFIG_MEMORY_HOTREMOVE
-int arch_remove_memory(u64 start, u64 size)
+int arch_remove_memory(u64 start, u64 size, struct vmem_altmap *altmap)
{
unsigned long start_pfn = start >> PAGE_SHIFT;
unsigned long nr_pages = size >> PAGE_SHIFT;
@@ -671,7 +671,7 @@ int arch_remove_memory(u64 start, u64 size)
int ret;
zone = page_zone(pfn_to_page(start_pfn));
- ret = __remove_pages(zone, start_pfn, nr_pages);
+ ret = __remove_pages(zone, start_pfn, nr_pages, altmap);
if (ret)
pr_warn("%s: Problem encountered in __remove_pages() as"
" ret=%d\n", __func__, ret);
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index e670cfc2766e..22aa528b78a2 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -149,11 +149,10 @@ int arch_add_memory(int nid, u64 start, u64 size, struct vmem_altmap *altmap,
}
#ifdef CONFIG_MEMORY_HOTREMOVE
-int arch_remove_memory(u64 start, u64 size)
+int arch_remove_memory(u64 start, u64 size, struct vmem_altmap *altmap)
{
unsigned long start_pfn = start >> PAGE_SHIFT;
unsigned long nr_pages = size >> PAGE_SHIFT;
- struct vmem_altmap *altmap;
struct page *page;
int ret;
@@ -162,11 +161,10 @@ int arch_remove_memory(u64 start, u64 size)
* when querying the zone.
*/
page = pfn_to_page(start_pfn);
- altmap = to_vmem_altmap((unsigned long) page);
if (altmap)
page += vmem_altmap_offset(altmap);
- ret = __remove_pages(page_zone(page), start_pfn, nr_pages);
+ ret = __remove_pages(page_zone(page), start_pfn, nr_pages, altmap);
if (ret)
return ret;
diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
index e12c5af50cd7..3fa3e5323612 100644
--- a/arch/s390/mm/init.c
+++ b/arch/s390/mm/init.c
@@ -240,7 +240,7 @@ int arch_add_memory(int nid, u64 start, u64 size, struct vmem_altmap *altmap,
}
#ifdef CONFIG_MEMORY_HOTREMOVE
-int arch_remove_memory(u64 start, u64 size)
+int arch_remove_memory(u64 start, u64 size, struct vmem_altmap *altmap)
{
/*
* There is no hardware or firmware interface which could trigger a
diff --git a/arch/sh/mm/init.c b/arch/sh/mm/init.c
index 552afbf55bad..ce0bbaa7e404 100644
--- a/arch/sh/mm/init.c
+++ b/arch/sh/mm/init.c
@@ -510,7 +510,7 @@ EXPORT_SYMBOL_GPL(memory_add_physaddr_to_nid);
#endif
#ifdef CONFIG_MEMORY_HOTREMOVE
-int arch_remove_memory(u64 start, u64 size)
+int arch_remove_memory(u64 start, u64 size, struct vmem_altmap *altmap)
{
unsigned long start_pfn = PFN_DOWN(start);
unsigned long nr_pages = size >> PAGE_SHIFT;
@@ -518,7 +518,7 @@ int arch_remove_memory(u64 start, u64 size)
int ret;
zone = page_zone(pfn_to_page(start_pfn));
- ret = __remove_pages(zone, start_pfn, nr_pages);
+ ret = __remove_pages(zone, start_pfn, nr_pages, altmap);
if (unlikely(ret))
pr_warn("%s: Failed, __remove_pages() == %d\n", __func__,
ret);
diff --git a/arch/x86/mm/init_32.c b/arch/x86/mm/init_32.c
index 8a3091511a71..79cb066f40c0 100644
--- a/arch/x86/mm/init_32.c
+++ b/arch/x86/mm/init_32.c
@@ -839,14 +839,14 @@ int arch_add_memory(int nid, u64 start, u64 size, struct vmem_altmap *altmap,
}
#ifdef CONFIG_MEMORY_HOTREMOVE
-int arch_remove_memory(u64 start, u64 size)
+int arch_remove_memory(u64 start, u64 size, struct vmem_altmap *altmap)
{
unsigned long start_pfn = start >> PAGE_SHIFT;
unsigned long nr_pages = size >> PAGE_SHIFT;
struct zone *zone;
zone = page_zone(pfn_to_page(start_pfn));
- return __remove_pages(zone, start_pfn, nr_pages);
+ return __remove_pages(zone, start_pfn, nr_pages, altmap);
}
#endif
#endif
diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index 594902ef56ef..3c046618cc7e 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -1132,21 +1132,19 @@ kernel_physical_mapping_remove(unsigned long start, unsigned long end)
remove_pagetable(start, end, true);
}
-int __ref arch_remove_memory(u64 start, u64 size)
+int __ref arch_remove_memory(u64 start, u64 size, struct vmem_altmap *altmap)
{
unsigned long start_pfn = start >> PAGE_SHIFT;
unsigned long nr_pages = size >> PAGE_SHIFT;
struct page *page = pfn_to_page(start_pfn);
- struct vmem_altmap *altmap;
struct zone *zone;
int ret;
/* With altmap the first mapped page is offset from @start */
- altmap = to_vmem_altmap((unsigned long) page);
if (altmap)
page += vmem_altmap_offset(altmap);
zone = page_zone(page);
- ret = __remove_pages(zone, start_pfn, nr_pages);
+ ret = __remove_pages(zone, start_pfn, nr_pages, altmap);
WARN_ON_ONCE(ret);
kernel_physical_mapping_remove(start, start + size);
diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index cbdd6d52e877..e71927d0d46b 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -126,9 +126,10 @@ static inline bool movable_node_is_enabled(void)
#ifdef CONFIG_MEMORY_HOTREMOVE
extern bool is_pageblock_removable_nolock(struct page *page);
-extern int arch_remove_memory(u64 start, u64 size);
+extern int arch_remove_memory(u64 start, u64 size,
+ struct vmem_altmap *altmap);
extern int __remove_pages(struct zone *zone, unsigned long start_pfn,
- unsigned long nr_pages);
+ unsigned long nr_pages, struct vmem_altmap *altmap);
#endif /* CONFIG_MEMORY_HOTREMOVE */
/* reasonably generic interface to expand the physical pages */
diff --git a/kernel/memremap.c b/kernel/memremap.c
index 8488cdeead16..380fca1c4a02 100644
--- a/kernel/memremap.c
+++ b/kernel/memremap.c
@@ -304,7 +304,7 @@ static void devm_memremap_pages_release(struct device *dev, void *data)
align_size = ALIGN(resource_size(res), SECTION_SIZE);
mem_hotplug_begin();
- arch_remove_memory(align_start, align_size);
+ arch_remove_memory(align_start, align_size, pgmap->altmap);
mem_hotplug_done();
untrack_pfn(NULL, PHYS_PFN(align_start), align_size);
diff --git a/mm/hmm.c b/mm/hmm.c
index 231aaacd1997..5d17ba89062f 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -838,10 +838,10 @@ static void hmm_devmem_release(struct device *dev, void *data)
mem_hotplug_begin();
if (resource->desc == IORES_DESC_DEVICE_PRIVATE_MEMORY)
- __remove_pages(zone, start_pfn, npages);
+ __remove_pages(zone, start_pfn, npages, NULL);
else
arch_remove_memory(start_pfn << PAGE_SHIFT,
- npages << PAGE_SHIFT);
+ npages << PAGE_SHIFT, NULL);
mem_hotplug_done();
hmm_devmem_radix_release(resource);
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index b36f1822c432..eae6bf47caf7 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -569,7 +569,7 @@ static int __remove_section(struct zone *zone, struct mem_section *ms,
* calling offline_pages().
*/
int __remove_pages(struct zone *zone, unsigned long phys_start_pfn,
- unsigned long nr_pages)
+ unsigned long nr_pages, struct vmem_altmap *altmap)
{
unsigned long i;
unsigned long map_offset = 0;
@@ -577,10 +577,6 @@ int __remove_pages(struct zone *zone, unsigned long phys_start_pfn,
/* In the ZONE_DEVICE case device driver owns the memory region */
if (is_dev_zone(zone)) {
- struct page *page = pfn_to_page(phys_start_pfn);
- struct vmem_altmap *altmap;
-
- altmap = to_vmem_altmap((unsigned long) page);
if (altmap)
map_offset = vmem_altmap_offset(altmap);
} else {
@@ -1890,7 +1886,7 @@ void __ref remove_memory(int nid, u64 start, u64 size)
memblock_free(start, size);
memblock_remove(start, size);
- arch_remove_memory(start, size);
+ arch_remove_memory(start, size, NULL);
try_offline_node(nid);
--
2.14.2
^ permalink raw reply related
* [PATCH 05/17] mm: pass the vmem_altmap to vmemmap_populate
From: Christoph Hellwig @ 2017-12-29 7:53 UTC (permalink / raw)
To: Dan Williams
Cc: Jérôme Glisse, Logan Gunthorpe, Michal Hocko,
linux-nvdimm, linuxppc-dev, x86, linux-mm, linux-kernel
In-Reply-To: <20171229075406.1936-1-hch@lst.de>
We can just pass this on instead of having to do a radix tree lookup
without proper locking a few levels into the callchain.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
---
arch/arm64/mm/mmu.c | 6 ++++--
arch/ia64/mm/discontig.c | 3 ++-
arch/powerpc/mm/init_64.c | 7 ++-----
arch/s390/mm/vmem.c | 3 ++-
arch/sparc/mm/init_64.c | 2 +-
arch/x86/mm/init_64.c | 4 ++--
include/linux/memory_hotplug.h | 3 ++-
include/linux/mm.h | 6 ++++--
mm/memory_hotplug.c | 7 ++++---
mm/sparse-vmemmap.c | 7 ++++---
mm/sparse.c | 20 ++++++++++++--------
11 files changed, 39 insertions(+), 29 deletions(-)
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 267d2b79d52d..ec8952ff13be 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -654,12 +654,14 @@ int kern_addr_valid(unsigned long addr)
}
#ifdef CONFIG_SPARSEMEM_VMEMMAP
#if !ARM64_SWAPPER_USES_SECTION_MAPS
-int __meminit vmemmap_populate(unsigned long start, unsigned long end, int node)
+int __meminit vmemmap_populate(unsigned long start, unsigned long end, int node,
+ struct vmem_altmap *altmap)
{
return vmemmap_populate_basepages(start, end, node);
}
#else /* !ARM64_SWAPPER_USES_SECTION_MAPS */
-int __meminit vmemmap_populate(unsigned long start, unsigned long end, int node)
+int __meminit vmemmap_populate(unsigned long start, unsigned long end, int node,
+ struct vmem_altmap *altmap)
{
unsigned long addr = start;
unsigned long next;
diff --git a/arch/ia64/mm/discontig.c b/arch/ia64/mm/discontig.c
index 9b2d994cddf6..1555aecaaf85 100644
--- a/arch/ia64/mm/discontig.c
+++ b/arch/ia64/mm/discontig.c
@@ -754,7 +754,8 @@ void arch_refresh_nodedata(int update_node, pg_data_t *update_pgdat)
#endif
#ifdef CONFIG_SPARSEMEM_VMEMMAP
-int __meminit vmemmap_populate(unsigned long start, unsigned long end, int node)
+int __meminit vmemmap_populate(unsigned long start, unsigned long end, int node,
+ struct vmem_altmap *altmap)
{
return vmemmap_populate_basepages(start, end, node);
}
diff --git a/arch/powerpc/mm/init_64.c b/arch/powerpc/mm/init_64.c
index a07722531b32..779b74a96b8f 100644
--- a/arch/powerpc/mm/init_64.c
+++ b/arch/powerpc/mm/init_64.c
@@ -183,7 +183,8 @@ static __meminit void vmemmap_list_populate(unsigned long phys,
vmemmap_list = vmem_back;
}
-int __meminit vmemmap_populate(unsigned long start, unsigned long end, int node)
+int __meminit vmemmap_populate(unsigned long start, unsigned long end, int node,
+ struct vmem_altmap *altmap)
{
unsigned long page_size = 1 << mmu_psize_defs[mmu_vmemmap_psize].shift;
@@ -193,16 +194,12 @@ int __meminit vmemmap_populate(unsigned long start, unsigned long end, int node)
pr_debug("vmemmap_populate %lx..%lx, node %d\n", start, end, node);
for (; start < end; start += page_size) {
- struct vmem_altmap *altmap;
void *p;
int rc;
if (vmemmap_populated(start, page_size))
continue;
- /* altmap lookups only work at section boundaries */
- altmap = to_vmem_altmap(SECTION_ALIGN_DOWN(start));
-
p = __vmemmap_alloc_block_buf(page_size, node, altmap);
if (!p)
return -ENOMEM;
diff --git a/arch/s390/mm/vmem.c b/arch/s390/mm/vmem.c
index 3316d463fc29..c44ef0e7c466 100644
--- a/arch/s390/mm/vmem.c
+++ b/arch/s390/mm/vmem.c
@@ -211,7 +211,8 @@ static void vmem_remove_range(unsigned long start, unsigned long size)
/*
* Add a backed mem_map array to the virtual mem_map array.
*/
-int __meminit vmemmap_populate(unsigned long start, unsigned long end, int node)
+int __meminit vmemmap_populate(unsigned long start, unsigned long end, int node,
+ struct vmem_altmap *altmap)
{
unsigned long pgt_prot, sgt_prot;
unsigned long address = start;
diff --git a/arch/sparc/mm/init_64.c b/arch/sparc/mm/init_64.c
index 55ba62957e64..42d27a1a042a 100644
--- a/arch/sparc/mm/init_64.c
+++ b/arch/sparc/mm/init_64.c
@@ -2628,7 +2628,7 @@ EXPORT_SYMBOL(_PAGE_CACHE);
#ifdef CONFIG_SPARSEMEM_VMEMMAP
int __meminit vmemmap_populate(unsigned long vstart, unsigned long vend,
- int node)
+ int node, struct vmem_altmap *altmap)
{
unsigned long pte_base;
diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index e80bb4189254..594902ef56ef 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -1411,9 +1411,9 @@ static int __meminit vmemmap_populate_hugepages(unsigned long start,
return 0;
}
-int __meminit vmemmap_populate(unsigned long start, unsigned long end, int node)
+int __meminit vmemmap_populate(unsigned long start, unsigned long end, int node,
+ struct vmem_altmap *altmap)
{
- struct vmem_altmap *altmap = to_vmem_altmap(start);
int err;
if (boot_cpu_has(X86_FEATURE_PSE))
diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index db276afbefcc..cbdd6d52e877 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -327,7 +327,8 @@ extern void move_pfn_range_to_zone(struct zone *zone, unsigned long start_pfn,
extern int offline_pages(unsigned long start_pfn, unsigned long nr_pages);
extern bool is_memblock_offlined(struct memory_block *mem);
extern void remove_memory(int nid, u64 start, u64 size);
-extern int sparse_add_one_section(struct pglist_data *pgdat, unsigned long start_pfn);
+extern int sparse_add_one_section(struct pglist_data *pgdat,
+ unsigned long start_pfn, struct vmem_altmap *altmap);
extern void sparse_remove_one_section(struct zone *zone, struct mem_section *ms,
unsigned long map_offset);
extern struct page *sparse_decode_mem_map(unsigned long coded_mem_map,
diff --git a/include/linux/mm.h b/include/linux/mm.h
index ea818ff739cd..2f3a7ebecbe2 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2538,7 +2538,8 @@ void sparse_mem_maps_populate_node(struct page **map_map,
unsigned long map_count,
int nodeid);
-struct page *sparse_mem_map_populate(unsigned long pnum, int nid);
+struct page *sparse_mem_map_populate(unsigned long pnum, int nid,
+ struct vmem_altmap *altmap);
pgd_t *vmemmap_pgd_populate(unsigned long addr, int node);
p4d_t *vmemmap_p4d_populate(pgd_t *pgd, unsigned long addr, int node);
pud_t *vmemmap_pud_populate(p4d_t *p4d, unsigned long addr, int node);
@@ -2556,7 +2557,8 @@ static inline void *vmemmap_alloc_block_buf(unsigned long size, int node)
void vmemmap_verify(pte_t *, int, unsigned long, unsigned long);
int vmemmap_populate_basepages(unsigned long start, unsigned long end,
int node);
-int vmemmap_populate(unsigned long start, unsigned long end, int node);
+int vmemmap_populate(unsigned long start, unsigned long end, int node,
+ struct vmem_altmap *altmap);
void vmemmap_populate_print_last(void);
#ifdef CONFIG_MEMORY_HOTPLUG
void vmemmap_free(unsigned long start, unsigned long end);
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index fc0485dcece1..b36f1822c432 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -250,7 +250,7 @@ void __init register_page_bootmem_info_node(struct pglist_data *pgdat)
#endif /* CONFIG_HAVE_BOOTMEM_INFO_NODE */
static int __meminit __add_section(int nid, unsigned long phys_start_pfn,
- bool want_memblock)
+ struct vmem_altmap *altmap, bool want_memblock)
{
int ret;
int i;
@@ -258,7 +258,7 @@ static int __meminit __add_section(int nid, unsigned long phys_start_pfn,
if (pfn_valid(phys_start_pfn))
return -EEXIST;
- ret = sparse_add_one_section(NODE_DATA(nid), phys_start_pfn);
+ ret = sparse_add_one_section(NODE_DATA(nid), phys_start_pfn, altmap);
if (ret < 0)
return ret;
@@ -317,7 +317,8 @@ int __ref __add_pages(int nid, unsigned long phys_start_pfn,
}
for (i = start_sec; i <= end_sec; i++) {
- err = __add_section(nid, section_nr_to_pfn(i), want_memblock);
+ err = __add_section(nid, section_nr_to_pfn(i), altmap,
+ want_memblock);
/*
* EEXIST is finally dealt with by ioresource collision
diff --git a/mm/sparse-vmemmap.c b/mm/sparse-vmemmap.c
index 17acf01791fa..376dcf05a39c 100644
--- a/mm/sparse-vmemmap.c
+++ b/mm/sparse-vmemmap.c
@@ -278,7 +278,8 @@ int __meminit vmemmap_populate_basepages(unsigned long start,
return 0;
}
-struct page * __meminit sparse_mem_map_populate(unsigned long pnum, int nid)
+struct page * __meminit sparse_mem_map_populate(unsigned long pnum, int nid,
+ struct vmem_altmap *altmap)
{
unsigned long start;
unsigned long end;
@@ -288,7 +289,7 @@ struct page * __meminit sparse_mem_map_populate(unsigned long pnum, int nid)
start = (unsigned long)map;
end = (unsigned long)(map + PAGES_PER_SECTION);
- if (vmemmap_populate(start, end, nid))
+ if (vmemmap_populate(start, end, nid, altmap))
return NULL;
return map;
@@ -318,7 +319,7 @@ void __init sparse_mem_maps_populate_node(struct page **map_map,
if (!present_section_nr(pnum))
continue;
- map_map[pnum] = sparse_mem_map_populate(pnum, nodeid);
+ map_map[pnum] = sparse_mem_map_populate(pnum, nodeid, NULL);
if (map_map[pnum])
continue;
ms = __nr_to_section(pnum);
diff --git a/mm/sparse.c b/mm/sparse.c
index 7a5dacaa06e3..5f4a0dac7836 100644
--- a/mm/sparse.c
+++ b/mm/sparse.c
@@ -417,7 +417,8 @@ static void __init sparse_early_usemaps_alloc_node(void *data,
}
#ifndef CONFIG_SPARSEMEM_VMEMMAP
-struct page __init *sparse_mem_map_populate(unsigned long pnum, int nid)
+struct page __init *sparse_mem_map_populate(unsigned long pnum, int nid,
+ struct vmem_altmap *altmap)
{
struct page *map;
unsigned long size;
@@ -472,7 +473,7 @@ void __init sparse_mem_maps_populate_node(struct page **map_map,
if (!present_section_nr(pnum))
continue;
- map_map[pnum] = sparse_mem_map_populate(pnum, nodeid);
+ map_map[pnum] = sparse_mem_map_populate(pnum, nodeid, NULL);
if (map_map[pnum])
continue;
ms = __nr_to_section(pnum);
@@ -500,7 +501,7 @@ static struct page __init *sparse_early_mem_map_alloc(unsigned long pnum)
struct mem_section *ms = __nr_to_section(pnum);
int nid = sparse_early_nid(ms);
- map = sparse_mem_map_populate(pnum, nid);
+ map = sparse_mem_map_populate(pnum, nid, NULL);
if (map)
return map;
@@ -678,10 +679,11 @@ void offline_mem_sections(unsigned long start_pfn, unsigned long end_pfn)
#endif
#ifdef CONFIG_SPARSEMEM_VMEMMAP
-static inline struct page *kmalloc_section_memmap(unsigned long pnum, int nid)
+static inline struct page *kmalloc_section_memmap(unsigned long pnum, int nid,
+ struct vmem_altmap *altmap)
{
/* This will make the necessary allocations eventually. */
- return sparse_mem_map_populate(pnum, nid);
+ return sparse_mem_map_populate(pnum, nid, altmap);
}
static void __kfree_section_memmap(struct page *memmap)
{
@@ -721,7 +723,8 @@ static struct page *__kmalloc_section_memmap(void)
return ret;
}
-static inline struct page *kmalloc_section_memmap(unsigned long pnum, int nid)
+static inline struct page *kmalloc_section_memmap(unsigned long pnum, int nid,
+ struct vmem_altmap *altmap)
{
return __kmalloc_section_memmap();
}
@@ -773,7 +776,8 @@ static void free_map_bootmem(struct page *memmap)
* set. If this is <=0, then that means that the passed-in
* map was not consumed and must be freed.
*/
-int __meminit sparse_add_one_section(struct pglist_data *pgdat, unsigned long start_pfn)
+int __meminit sparse_add_one_section(struct pglist_data *pgdat,
+ unsigned long start_pfn, struct vmem_altmap *altmap)
{
unsigned long section_nr = pfn_to_section_nr(start_pfn);
struct mem_section *ms;
@@ -789,7 +793,7 @@ int __meminit sparse_add_one_section(struct pglist_data *pgdat, unsigned long st
ret = sparse_index_init(section_nr, pgdat->node_id);
if (ret < 0 && ret != -EEXIST)
return ret;
- memmap = kmalloc_section_memmap(section_nr, pgdat->node_id);
+ memmap = kmalloc_section_memmap(section_nr, pgdat->node_id, altmap);
if (!memmap)
return -ENOMEM;
usemap = __kmalloc_section_usemap();
--
2.14.2
^ permalink raw reply related
* [PATCH 04/17] mm: pass the vmem_altmap to arch_add_memory and __add_pages
From: Christoph Hellwig @ 2017-12-29 7:53 UTC (permalink / raw)
To: Dan Williams
Cc: Jérôme Glisse, Logan Gunthorpe, Michal Hocko,
linux-nvdimm, linuxppc-dev, x86, linux-mm, linux-kernel
In-Reply-To: <20171229075406.1936-1-hch@lst.de>
We can just pass this on instead of having to do a radix tree lookup
without proper locking 2 levels into the callchain.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/ia64/mm/init.c | 5 +++--
arch/powerpc/mm/mem.c | 5 +++--
arch/s390/mm/init.c | 5 +++--
arch/sh/mm/init.c | 5 +++--
arch/x86/mm/init_32.c | 5 +++--
arch/x86/mm/init_64.c | 11 ++++++-----
include/linux/memory_hotplug.h | 17 ++++++++++-------
kernel/memremap.c | 3 ++-
mm/hmm.c | 5 +++--
mm/memory_hotplug.c | 7 +++----
10 files changed, 39 insertions(+), 29 deletions(-)
diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
index 7af4e05bb61e..2e2e4f532204 100644
--- a/arch/ia64/mm/init.c
+++ b/arch/ia64/mm/init.c
@@ -647,13 +647,14 @@ mem_init (void)
}
#ifdef CONFIG_MEMORY_HOTPLUG
-int arch_add_memory(int nid, u64 start, u64 size, bool want_memblock)
+int arch_add_memory(int nid, u64 start, u64 size, struct vmem_altmap *altmap,
+ bool want_memblock)
{
unsigned long start_pfn = start >> PAGE_SHIFT;
unsigned long nr_pages = size >> PAGE_SHIFT;
int ret;
- ret = __add_pages(nid, start_pfn, nr_pages, want_memblock);
+ ret = __add_pages(nid, start_pfn, nr_pages, altmap, want_memblock);
if (ret)
printk("%s: Problem encountered in __add_pages() as ret=%d\n",
__func__, ret);
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index 4362b86ef84c..e670cfc2766e 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -127,7 +127,8 @@ int __weak remove_section_mapping(unsigned long start, unsigned long end)
return -ENODEV;
}
-int arch_add_memory(int nid, u64 start, u64 size, bool want_memblock)
+int arch_add_memory(int nid, u64 start, u64 size, struct vmem_altmap *altmap,
+ bool want_memblock)
{
unsigned long start_pfn = start >> PAGE_SHIFT;
unsigned long nr_pages = size >> PAGE_SHIFT;
@@ -144,7 +145,7 @@ int arch_add_memory(int nid, u64 start, u64 size, bool want_memblock)
return -EFAULT;
}
- return __add_pages(nid, start_pfn, nr_pages, want_memblock);
+ return __add_pages(nid, start_pfn, nr_pages, altmap, want_memblock);
}
#ifdef CONFIG_MEMORY_HOTREMOVE
diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
index 671535e64aba..e12c5af50cd7 100644
--- a/arch/s390/mm/init.c
+++ b/arch/s390/mm/init.c
@@ -222,7 +222,8 @@ device_initcall(s390_cma_mem_init);
#endif /* CONFIG_CMA */
-int arch_add_memory(int nid, u64 start, u64 size, bool want_memblock)
+int arch_add_memory(int nid, u64 start, u64 size, struct vmem_altmap *altmap,
+ bool want_memblock)
{
unsigned long start_pfn = PFN_DOWN(start);
unsigned long size_pages = PFN_DOWN(size);
@@ -232,7 +233,7 @@ int arch_add_memory(int nid, u64 start, u64 size, bool want_memblock)
if (rc)
return rc;
- rc = __add_pages(nid, start_pfn, size_pages, want_memblock);
+ rc = __add_pages(nid, start_pfn, size_pages, altmap, want_memblock);
if (rc)
vmem_remove_mapping(start, size);
return rc;
diff --git a/arch/sh/mm/init.c b/arch/sh/mm/init.c
index afc54d593a26..552afbf55bad 100644
--- a/arch/sh/mm/init.c
+++ b/arch/sh/mm/init.c
@@ -485,14 +485,15 @@ void free_initrd_mem(unsigned long start, unsigned long end)
#endif
#ifdef CONFIG_MEMORY_HOTPLUG
-int arch_add_memory(int nid, u64 start, u64 size, bool want_memblock)
+int arch_add_memory(int nid, u64 start, u64 size, struct vmem_altmap *altmap,
+ bool want_memblock)
{
unsigned long start_pfn = PFN_DOWN(start);
unsigned long nr_pages = size >> PAGE_SHIFT;
int ret;
/* We only have ZONE_NORMAL, so this is easy.. */
- ret = __add_pages(nid, start_pfn, nr_pages, want_memblock);
+ ret = __add_pages(nid, start_pfn, nr_pages, altmap, want_memblock);
if (unlikely(ret))
printk("%s: Failed, __add_pages() == %d\n", __func__, ret);
diff --git a/arch/x86/mm/init_32.c b/arch/x86/mm/init_32.c
index 135c9a7898c7..8a3091511a71 100644
--- a/arch/x86/mm/init_32.c
+++ b/arch/x86/mm/init_32.c
@@ -829,12 +829,13 @@ void __init mem_init(void)
}
#ifdef CONFIG_MEMORY_HOTPLUG
-int arch_add_memory(int nid, u64 start, u64 size, bool want_memblock)
+int arch_add_memory(int nid, u64 start, u64 size, struct vmem_altmap *altmap,
+ bool want_memblock)
{
unsigned long start_pfn = start >> PAGE_SHIFT;
unsigned long nr_pages = size >> PAGE_SHIFT;
- return __add_pages(nid, start_pfn, nr_pages, want_memblock);
+ return __add_pages(nid, start_pfn, nr_pages, altmap, want_memblock);
}
#ifdef CONFIG_MEMORY_HOTREMOVE
diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index 8acdc35c2dfa..e80bb4189254 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -772,12 +772,12 @@ static void update_end_of_memory_vars(u64 start, u64 size)
}
}
-int add_pages(int nid, unsigned long start_pfn,
- unsigned long nr_pages, bool want_memblock)
+int add_pages(int nid, unsigned long start_pfn, unsigned long nr_pages,
+ struct vmem_altmap *altmap, bool want_memblock)
{
int ret;
- ret = __add_pages(nid, start_pfn, nr_pages, want_memblock);
+ ret = __add_pages(nid, start_pfn, nr_pages, altmap, want_memblock);
WARN_ON_ONCE(ret);
/* update max_pfn, max_low_pfn and high_memory */
@@ -787,14 +787,15 @@ int add_pages(int nid, unsigned long start_pfn,
return ret;
}
-int arch_add_memory(int nid, u64 start, u64 size, bool want_memblock)
+int arch_add_memory(int nid, u64 start, u64 size, struct vmem_altmap *altmap,
+ bool want_memblock)
{
unsigned long start_pfn = start >> PAGE_SHIFT;
unsigned long nr_pages = size >> PAGE_SHIFT;
init_memory_mapping(start, start + size);
- return add_pages(nid, start_pfn, nr_pages, want_memblock);
+ return add_pages(nid, start_pfn, nr_pages, altmap, want_memblock);
}
#define PAGE_INUSE 0xFD
diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index 58e110aee7ab..db276afbefcc 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -13,6 +13,7 @@ struct pglist_data;
struct mem_section;
struct memory_block;
struct resource;
+struct vmem_altmap;
#ifdef CONFIG_MEMORY_HOTPLUG
/*
@@ -131,18 +132,19 @@ extern int __remove_pages(struct zone *zone, unsigned long start_pfn,
#endif /* CONFIG_MEMORY_HOTREMOVE */
/* reasonably generic interface to expand the physical pages */
-extern int __add_pages(int nid, unsigned long start_pfn,
- unsigned long nr_pages, bool want_memblock);
+extern int __add_pages(int nid, unsigned long start_pfn, unsigned long nr_pages,
+ struct vmem_altmap *altmap, bool want_memblock);
#ifndef CONFIG_ARCH_HAS_ADD_PAGES
static inline int add_pages(int nid, unsigned long start_pfn,
- unsigned long nr_pages, bool want_memblock)
+ unsigned long nr_pages, struct vmem_altmap *altmap,
+ bool want_memblock)
{
- return __add_pages(nid, start_pfn, nr_pages, want_memblock);
+ return __add_pages(nid, start_pfn, nr_pages, altmap, want_memblock);
}
#else /* ARCH_HAS_ADD_PAGES */
-int add_pages(int nid, unsigned long start_pfn,
- unsigned long nr_pages, bool want_memblock);
+int add_pages(int nid, unsigned long start_pfn, unsigned long nr_pages,
+ struct vmem_altmap *altmap, bool want_memblock);
#endif /* ARCH_HAS_ADD_PAGES */
#ifdef CONFIG_NUMA
@@ -318,7 +320,8 @@ extern int walk_memory_range(unsigned long start_pfn, unsigned long end_pfn,
void *arg, int (*func)(struct memory_block *, void *));
extern int add_memory(int nid, u64 start, u64 size);
extern int add_memory_resource(int nid, struct resource *resource, bool online);
-extern int arch_add_memory(int nid, u64 start, u64 size, bool want_memblock);
+extern int arch_add_memory(int nid, u64 start, u64 size,
+ struct vmem_altmap *altmap, bool want_memblock);
extern void move_pfn_range_to_zone(struct zone *zone, unsigned long start_pfn,
unsigned long nr_pages);
extern int offline_pages(unsigned long start_pfn, unsigned long nr_pages);
diff --git a/kernel/memremap.c b/kernel/memremap.c
index 403ab9cdb949..8488cdeead16 100644
--- a/kernel/memremap.c
+++ b/kernel/memremap.c
@@ -382,6 +382,7 @@ void *devm_memremap_pages(struct device *dev, struct resource *res,
if (altmap) {
memcpy(&page_map->altmap, altmap, sizeof(*altmap));
pgmap->altmap = &page_map->altmap;
+ altmap = pgmap->altmap;
}
pgmap->ref = ref;
pgmap->res = &page_map->res;
@@ -427,7 +428,7 @@ void *devm_memremap_pages(struct device *dev, struct resource *res,
goto err_pfn_remap;
mem_hotplug_begin();
- error = arch_add_memory(nid, align_start, align_size, false);
+ error = arch_add_memory(nid, align_start, align_size, altmap, false);
if (!error)
move_pfn_range_to_zone(&NODE_DATA(nid)->node_zones[ZONE_DEVICE],
align_start >> PAGE_SHIFT,
diff --git a/mm/hmm.c b/mm/hmm.c
index ea19742a5d60..231aaacd1997 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -931,10 +931,11 @@ static int hmm_devmem_pages_create(struct hmm_devmem *devmem)
* want the linear mapping and thus use arch_add_memory().
*/
if (devmem->pagemap.type == MEMORY_DEVICE_PUBLIC)
- ret = arch_add_memory(nid, align_start, align_size, false);
+ ret = arch_add_memory(nid, align_start, align_size, NULL,
+ false);
else
ret = add_pages(nid, align_start >> PAGE_SHIFT,
- align_size >> PAGE_SHIFT, false);
+ align_size >> PAGE_SHIFT, NULL, false);
if (ret) {
mem_hotplug_done();
goto error_add_memory;
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 5c6f96e6b334..fc0485dcece1 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -292,18 +292,17 @@ static int __meminit __add_section(int nid, unsigned long phys_start_pfn,
* add the new pages.
*/
int __ref __add_pages(int nid, unsigned long phys_start_pfn,
- unsigned long nr_pages, bool want_memblock)
+ unsigned long nr_pages, struct vmem_altmap *altmap,
+ bool want_memblock)
{
unsigned long i;
int err = 0;
int start_sec, end_sec;
- struct vmem_altmap *altmap;
/* during initialize mem_map, align hot-added range to section */
start_sec = pfn_to_section_nr(phys_start_pfn);
end_sec = pfn_to_section_nr(phys_start_pfn + nr_pages - 1);
- altmap = to_vmem_altmap((unsigned long) pfn_to_page(phys_start_pfn));
if (altmap) {
/*
* Validate altmap is within bounds of the total request
@@ -1148,7 +1147,7 @@ int __ref add_memory_resource(int nid, struct resource *res, bool online)
}
/* call arch's memory hotadd */
- ret = arch_add_memory(nid, start, size, true);
+ ret = arch_add_memory(nid, start, size, NULL, true);
if (ret < 0)
goto error;
--
2.14.2
^ permalink raw reply related
* [PATCH 01/17] memremap: provide stubs for vmem_altmap_offset and vmem_altmap_free
From: Christoph Hellwig @ 2017-12-29 7:53 UTC (permalink / raw)
To: Dan Williams
Cc: Jérôme Glisse, Logan Gunthorpe, Michal Hocko,
linux-nvdimm, linuxppc-dev, x86, linux-mm, linux-kernel
In-Reply-To: <20171229075406.1936-1-hch@lst.de>
Currently all calls to those functions are eliminated by the compiler when
CONFIG_ZONE_DEVICE is not set, but this soon won't be the case.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
---
include/linux/memremap.h | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/include/linux/memremap.h b/include/linux/memremap.h
index 10d23c367048..d5a6736d9737 100644
--- a/include/linux/memremap.h
+++ b/include/linux/memremap.h
@@ -26,9 +26,6 @@ struct vmem_altmap {
unsigned long alloc;
};
-unsigned long vmem_altmap_offset(struct vmem_altmap *altmap);
-void vmem_altmap_free(struct vmem_altmap *altmap, unsigned long nr_pfns);
-
#ifdef CONFIG_ZONE_DEVICE
struct vmem_altmap *to_vmem_altmap(unsigned long memmap_start);
#else
@@ -138,6 +135,9 @@ void *devm_memremap_pages(struct device *dev, struct resource *res,
struct percpu_ref *ref, struct vmem_altmap *altmap);
struct dev_pagemap *find_dev_pagemap(resource_size_t phys);
+unsigned long vmem_altmap_offset(struct vmem_altmap *altmap);
+void vmem_altmap_free(struct vmem_altmap *altmap, unsigned long nr_pfns);
+
static inline bool is_zone_device_page(const struct page *page);
#else
static inline void *devm_memremap_pages(struct device *dev,
@@ -157,7 +157,17 @@ static inline struct dev_pagemap *find_dev_pagemap(resource_size_t phys)
{
return NULL;
}
-#endif
+
+static inline unsigned long vmem_altmap_offset(struct vmem_altmap *altmap)
+{
+ return 0;
+}
+
+static inline void vmem_altmap_free(struct vmem_altmap *altmap,
+ unsigned long nr_pfns)
+{
+}
+#endif /* CONFIG_ZONE_DEVICE */
#if defined(CONFIG_DEVICE_PRIVATE) || defined(CONFIG_DEVICE_PUBLIC)
static inline bool is_device_private_page(const struct page *page)
--
2.14.2
^ permalink raw reply related
* revamp vmem_altmap / dev_pagemap handling V3
From: Christoph Hellwig @ 2017-12-29 7:53 UTC (permalink / raw)
To: Dan Williams
Cc: Jérôme Glisse, Logan Gunthorpe, Michal Hocko,
linux-nvdimm, linuxppc-dev, x86, linux-mm, linux-kernel
Hi all,
this series started with two patches from Logan that now are in the
middle of the series to kill the memremap-internal pgmap structure
and to redo the dev_memreamp_pages interface to be better suitable
for future PCI P2P uses. I reviewed them and noticed that there
isn't really any good reason to keep struct vmem_altmap either,
and that a lot of these alternative device page map access should
be better abstracted out instead of being sprinkled all over the
mm code. But when we got the RCU warnings in V1 I went for yet
another approach, and now struct vmem_altmap is kept for now,
but passed explicitly through the memory hotplug code instead of
having to do unprotected lookups through the radix tree. The
end result is that only the get_user_pages path ever looks up
struct dev_pagemap, and struct vmem_altmap is now always embedded
into struct dev_pagemap, and explicitly passed where needed.
Please review carefully, this has only been tested with my legacy
e820 NVDIMM system.
Chances since V2:
- properly pass altmap from dev_devm_memremap_pages through add_pages
(Dan Williams)
- small changelog updates
- a comment type fix
- dropped the patch to just rely on the radix_tree_insert return value
- initialize pgmap->type
^ permalink raw reply
* [PATCH 03/17] mm: don't export __add_pages
From: Christoph Hellwig @ 2017-12-29 7:53 UTC (permalink / raw)
To: Dan Williams
Cc: Jérôme Glisse, Logan Gunthorpe, Michal Hocko,
linux-nvdimm, linuxppc-dev, x86, linux-mm, linux-kernel
In-Reply-To: <20171229075406.1936-1-hch@lst.de>
This function isn't used by any modules, and is only to be called
from core MM code. This includes the calls for the add_pages wrapper
that might be inlined.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
---
mm/memory_hotplug.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index c52aa05b106c..5c6f96e6b334 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -334,7 +334,6 @@ int __ref __add_pages(int nid, unsigned long phys_start_pfn,
out:
return err;
}
-EXPORT_SYMBOL_GPL(__add_pages);
#ifdef CONFIG_MEMORY_HOTREMOVE
/* find the smallest valid pfn in the range [start_pfn, end_pfn) */
--
2.14.2
^ permalink raw reply related
* [PATCH 02/17] mm: don't export arch_add_memory
From: Christoph Hellwig @ 2017-12-29 7:53 UTC (permalink / raw)
To: Dan Williams
Cc: Jérôme Glisse, Logan Gunthorpe, Michal Hocko,
linux-nvdimm, linuxppc-dev, x86, linux-mm, linux-kernel
In-Reply-To: <20171229075406.1936-1-hch@lst.de>
Only x86_64 and sh export this symbol, and it is not used by any
modular code.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
---
arch/sh/mm/init.c | 1 -
arch/x86/mm/init_64.c | 1 -
2 files changed, 2 deletions(-)
diff --git a/arch/sh/mm/init.c b/arch/sh/mm/init.c
index bf726af5f1a5..afc54d593a26 100644
--- a/arch/sh/mm/init.c
+++ b/arch/sh/mm/init.c
@@ -498,7 +498,6 @@ int arch_add_memory(int nid, u64 start, u64 size, bool want_memblock)
return ret;
}
-EXPORT_SYMBOL_GPL(arch_add_memory);
#ifdef CONFIG_NUMA
int memory_add_physaddr_to_nid(u64 addr)
diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index 4a837289f2ad..8acdc35c2dfa 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -796,7 +796,6 @@ int arch_add_memory(int nid, u64 start, u64 size, bool want_memblock)
return add_pages(nid, start_pfn, nr_pages, want_memblock);
}
-EXPORT_SYMBOL_GPL(arch_add_memory);
#define PAGE_INUSE 0xFD
--
2.14.2
^ permalink raw reply related
* Re: [PATCH v6 2/8] module: use relative references for __ksymtab entries
From: kbuild test robot @ 2017-12-29 6:42 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: kbuild-all, linux-kernel, Ard Biesheuvel, H. Peter Anvin,
Ralf Baechle, Arnd Bergmann, Heiko Carstens, Kees Cook,
Will Deacon, Michael Ellerman, Thomas Garnier, Thomas Gleixner,
Serge E. Hallyn, Bjorn Helgaas, Benjamin Herrenschmidt,
Russell King, Paul Mackerras, Catalin Marinas, David S. Miller,
Petr Mladek, Ingo Molnar, James Morris, Andrew Morton,
Nicolas Pitre, Josh Poimboeuf, Steven Rostedt, Martin Schwidefsky,
Sergey Senozhatsky, Linus Torvalds, Jessica Yu, linux-arm-kernel,
linux-mips, linuxppc-dev, linux-s390, sparclinux, x86,
Ingo Molnar
In-Reply-To: <20171227085033.22389-3-ard.biesheuvel@linaro.org>
[-- Attachment #1: Type: text/plain, Size: 1299 bytes --]
Hi Ard,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v4.15-rc5 next-20171222]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Ard-Biesheuvel/add-support-for-relative-references-in-special-sections/20171228-171634
config: s390-gcov_defconfig (attached as .config)
compiler: s390x-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=s390
All errors (new ones prefixed by >>):
>> arch/s390/kernel/ebcdic.o:(.data+0x118): undefined reference to `__gcov_merge_add'
arch/s390/kernel/ebcdic.o: In function `_GLOBAL__sub_I_00100_0__ascebc':
>> ebcdic.c:(.text.startup+0xe): undefined reference to `__gcov_init'
arch/s390/kernel/ebcdic.o: In function `_GLOBAL__sub_D_00100_1__ascebc':
>> ebcdic.c:(.text.exit+0x8): undefined reference to `__gcov_exit'
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 17457 bytes --]
^ permalink raw reply
* Re: [PATCH v2 2/7] powerpc/kernel: Add uevents in EEH error/resume
From: Benjamin Herrenschmidt @ 2017-12-29 0:02 UTC (permalink / raw)
To: Bjorn Helgaas, Juan Alvarez
Cc: Bryant G. Ly, paulus, mpe, seroyer, alex.williamson, aik, ruscur,
linux-pci, linuxppc-dev, bodong, eli, saeedm, Keith Busch,
Gabriele Paoloni, Dongdong Liu
In-Reply-To: <20171228232253.GC19819@bhelgaas-glaptop.roam.corp.google.com>
On Thu, 2017-12-28 at 17:22 -0600, Bjorn Helgaas wrote:
> Both paths end up calling the pci_error_handlers.error_detected()
> hook.
>
> Drivers are not supposed to care what arch they're running on. If the
> driver supplies an .error_detected() entry point, it's up to the PCI
> core and powerpc code to use it consistently across arches. That
> means the same uevents (if any) should be emitted from both paths.
>
> The best way would be to unify the call of .error_detected() so the
> AER path and the powerpc path do it via the same function. The AER
> report_error_detected() and the powerpc eeh_report_error() do look
> fairly similar, so this seems possible in principle, but I'm not
> holding my breath.
Factoring these callers into a common function that can then do the
uevent for errors makes a lot of sense.
The "resume" path might be trickier, but even then, rather than calling
directly the driver op, it would be easy to have a little wrapper that
does it, which can then also do the uevent.
Ben.
^ permalink raw reply
* Re: [PATCH v6 5/8] kernel: tracepoints: add support for relative references
From: Ard Biesheuvel @ 2017-12-28 23:24 UTC (permalink / raw)
To: Steven Rostedt
Cc: Linux Kernel Mailing List, H. Peter Anvin, Ralf Baechle,
Arnd Bergmann, Heiko Carstens, Kees Cook, Will Deacon,
Michael Ellerman, Thomas Garnier, Thomas Gleixner,
Serge E. Hallyn, Bjorn Helgaas, Benjamin Herrenschmidt,
Russell King, Paul Mackerras, Catalin Marinas, David S. Miller,
Petr Mladek, Ingo Molnar, James Morris, Andrew Morton,
Nicolas Pitre, Josh Poimboeuf, Martin Schwidefsky,
Sergey Senozhatsky, Linus Torvalds, Jessica Yu, linux-arm-kernel,
linux-mips, linuxppc-dev, linux-s390, sparclinux,
the arch/x86 maintainers
In-Reply-To: <20171228104207.117ee0ff@gandalf.local.home>
On 28 December 2017 at 15:42, Steven Rostedt <rostedt@goodmis.org> wrote:
> On Wed, 27 Dec 2017 08:50:30 +0000
> Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
>> To avoid the need for relocating absolute references to tracepoint
>> structures at boot time when running relocatable kernels (which may
>> take a disproportionate amount of space), add the option to emit
>> these tables as relative references instead.
>>
>
> I gave this patch a quick skim over. It appears to not modify anything
> when CONFIG_HAVE_PREL32_RELOCATIONS is not defined. I haven't
> thoroughly reviewed it or tested it. But if it doesn't break anything,
> I'm fine giving you an ack.
>
> Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
>
Thank you Steven.
I should mention though (as you don't appear to recall) that an
earlier version of this patch triggered an issue for you
https://marc.info/?l=linux-arch&m=150584374820168&w=2
but I have never managed to reproduce it, neither at the time nor
currently with this v6.
ard@bezzzef:~/linux-2.6$ sudo tools/testing/selftests/ftrace/ftracetest
=== Ftrace unit tests ===
[1] Basic trace file check [PASS]
[2] Basic test for tracers [PASS]
[3] Basic trace clock test [PASS]
[4] Basic event tracing check [PASS]
[5] event tracing - enable/disable with event level files [PASS]
[6] event tracing - restricts events based on pid [PASS]
[7] event tracing - enable/disable with subsystem level files [PASS]
[8] event tracing - enable/disable with top level files [PASS]
[9] ftrace - function graph filters with stack tracer [PASS]
[10] ftrace - function graph filters [PASS]
[11] ftrace - test for function event triggers [PASS]
[12] ftrace - function glob filters [PASS]
[13] ftrace - function pid filters [PASS]
[14] ftrace - function profiler with function tracing [PASS]
[15] ftrace - test reading of set_ftrace_filter [PASS]
[16] ftrace - test for function traceon/off triggers [PASS]
[17] Test creation and deletion of trace instances while setting an event [PASS]
[18] Test creation and deletion of trace instances [PASS]
[19] Kprobe dynamic event - adding and removing [PASS]
[20] Kprobe dynamic event - busy event check [PASS]
[21] Kprobe dynamic event with arguments [PASS]
[22] Kprobes event arguments with types [PASS]
[23] Kprobe event auto/manual naming [PASS]
[24] Kprobe dynamic event with function tracer [PASS]
[25] Kprobe dynamic event - probing module [PASS]
[26] Kretprobe dynamic event with arguments [PASS]
[27] Kretprobe dynamic event with maxactive [PASS]
[28] Register/unregister many kprobe events [PASS]
[29] event trigger - test event enable/disable trigger [PASS]
[30] event trigger - test trigger filter [PASS]
[31] event trigger - test histogram modifiers [PASS]
[32] event trigger - test histogram trigger [PASS]
[33] event trigger - test multiple histogram triggers [PASS]
[34] event trigger - test snapshot-trigger [PASS]
[35] event trigger - test stacktrace-trigger [PASS]
[36] event trigger - test traceon/off trigger [PASS]
[37] (instance) Basic test for tracers [PASS]
[38] (instance) Basic trace clock test [PASS]
[39] (instance) event tracing - enable/disable with event level files [PASS]
[40] (instance) event tracing - restricts events based on pid [PASS]
[41] (instance) event tracing - enable/disable with subsystem level
files [PASS]
[42] (instance) ftrace - test for function event triggers [PASS]
[43] (instance) ftrace - test for function traceon/off triggers [PASS]
[44] (instance) event trigger - test event enable/disable trigger [PASS]
[45] (instance) event trigger - test trigger filter [PASS]
[46] (instance) event trigger - test histogram modifiers [PASS]
[47] (instance) event trigger - test histogram trigger [PASS]
[48] (instance) event trigger - test multiple histogram triggers [PASS]
# of passed: 48
# of failed: 0
# of unresolved: 0
# of untested: 0
# of unsupported: 0
# of xfailed: 0
# of undefined(test bug): 0
^ permalink raw reply
* Re: [PATCH v2 2/7] powerpc/kernel: Add uevents in EEH error/resume
From: Bjorn Helgaas @ 2017-12-28 23:22 UTC (permalink / raw)
To: Juan Alvarez
Cc: Benjamin Herrenschmidt, Bryant G. Ly, paulus, mpe, seroyer,
alex.williamson, aik, ruscur, linux-pci, linuxppc-dev, bodong,
eli, saeedm, Keith Busch, Gabriele Paoloni, Dongdong Liu
In-Reply-To: <952dac4e-cf22-ed68-1a29-58ac7ed3515a@linux.vnet.ibm.com>
On Wed, Dec 20, 2017 at 09:04:27PM -0600, Juan Alvarez wrote:
> On 12/19/17 12:27 AM, Benjamin Herrenschmidt wrote:
>
> > On Mon, 2017-12-18 at 22:50 -0600, Bjorn Helgaas wrote:
> >> [+cc Keith, Gabriele, Dongdong]
> >>
> >> On Mon, Dec 18, 2017 at 04:38:03PM -0600, Bryant G. Ly wrote:
> >>> Devices can go offline when EEH is reported. This patch adds
> >>> a change to the kernel object and lets udev know of error.
> >>> When device resumes a change is also set reporting device as
> >>> online. Therefore, EEH events are better propagated to user
> >>> space for devices in powerpc arch.
> >> I'm on vacation and can't review this in detail, but I wonder if you
> >> can compare this with the uevents we emit for DPC, AER, and hotplug
> >> events (if any). I hope we don't end up with userspace having to be
> >> aware of the differences between EEH, DPC, AER, etc.
> >>
> >>> From a very quick look, I only see a few uevents even mentioned in
> >> drivers/pci: KOBJ_ADD in __pci_hp_register() and KOBJ_CHANGE in the
> >> SR-IOV code. I'm worried that we're missing some important uevents in
> >> the PCI core. That's not an argument against what you're doing here;
> >> it just would be nice to fill in any missing pieces in the core also,
> >> and hopefully make them consistent with these EEH events.
> > We also need to be careful about what specific EEH activity we are
> > talking about, and if we bring into the picture things like DPDK, it
> > gets even more murky...
> >
> > The basic way EEH is supposed to work for recovery (minus all sort of
> > implementation nasties which hopefully Russell and Sam are trying to
> > cleanup and fix) is that either:
> >
> > - The driver of the device has recovery callbacks, in which
> > case the driver participates in the recovery process, the device
> > doesn't "go away" (though it shouldn't be accessed during that process
> > by other entities, userspace originated config space could be a problem
> > and needs to be blocked...). The recovery typically involves a reset of
> > the device but in sync with the driver.
> >
> > - The driver doesn't have the callbacks. In this case, we
> > simulate an unplug, reset the device, and replug.
> >
> > So it makes sense for the second case to emit the same uevents as a
> > normal PCI(e) hotplug.
> >
> > For the former case I'm less sure.... Do we really need userspace to be
> > notified ? If yes, what for precisely ?
>
> In pSeries SR-IOV environment the management console might need to apply
> certain configuration changes to the PF driver after it has been recovered
> and before the VF drivers are allowed to resume their recovery path.
> I could not think of another way to notify user space of these events.
> I made this assumption because I saw there were no uevents added when
> the device goes offline and come back online in EEH code. It was my
> intention to make the event as generic as possible in EEH component,
> therefore, making this change independent of pSeries SR-IOV.
I don't know what your plan for this is, but we do have two different
paths that use the struct pci_error_handlers hooks that drivers may
supply. There's this AER path that may be used on all arches:
aer_isr
get_e_source # remove from rpc->e_sources[] queue
aer_isr_one_error
aer_process_err_devices
handle_error_source # or aer_recover_work_func
do_recovery # for uncorrectable (fatal/nonfatal) only
broadcast_error_message(dev, ..., report_error_detected)
pci_walk_bus(..., report_error_detected)
report_error_detected
dev->driver->err_handler->error_detected
And there's this powerpc path where you're adding a uevent:
eeh_event_handler
eeh_handle_event
eeh_handle_normal_event
eeh_pe_dev_traverse(pe, eeh_report_error, &result)
eeh_report_error
driver->err_handler->error_detected(dev, pci_channel_io_frozen)
+ kobject_uevent_env(&dev->dev.kobj, KOBJ_CHANGE, envp);
Both paths end up calling the pci_error_handlers.error_detected()
hook.
Drivers are not supposed to care what arch they're running on. If the
driver supplies an .error_detected() entry point, it's up to the PCI
core and powerpc code to use it consistently across arches. That
means the same uevents (if any) should be emitted from both paths.
The best way would be to unify the call of .error_detected() so the
AER path and the powerpc path do it via the same function. The AER
report_error_detected() and the powerpc eeh_report_error() do look
fairly similar, so this seems possible in principle, but I'm not
holding my breath.
Bjorn
^ permalink raw reply
* Re: [PATCH v6 8/8] x86/kernel: jump_table: use relative references
From: Steven Rostedt @ 2017-12-28 16:39 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Linux Kernel Mailing List, H. Peter Anvin, Ralf Baechle,
Arnd Bergmann, Heiko Carstens, Kees Cook, Will Deacon,
Michael Ellerman, Thomas Garnier, Thomas Gleixner,
Serge E. Hallyn, Bjorn Helgaas, Benjamin Herrenschmidt,
Russell King, Paul Mackerras, Catalin Marinas, David S. Miller,
Petr Mladek, Ingo Molnar, James Morris, Andrew Morton,
Nicolas Pitre, Josh Poimboeuf, Martin Schwidefsky,
Sergey Senozhatsky, Linus Torvalds, Jessica Yu, linux-arm-kernel,
linux-mips, linuxppc-dev, linux-s390, sparclinux,
the arch/x86 maintainers
In-Reply-To: <CAKv+Gu9Gza=RN_v_H-G7m7-qKg9B4Xf4GFvd_H-Gut-V3eabmA@mail.gmail.com>
On Thu, 28 Dec 2017 16:26:07 +0000
Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 28 December 2017 at 16:19, Steven Rostedt <rostedt@goodmis.org> wrote:
> > On Wed, 27 Dec 2017 08:50:33 +0000
> > Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> >
> >> static inline jump_label_t jump_entry_code(const struct jump_entry *entry)
> >> {
> >> - return entry->code;
> >> + return (jump_label_t)&entry->code + entry->code;
> >
> > I'm paranoid about doing arithmetic on abstract types. What happens in
> > the future if jump_label_t becomes a pointer? You will get a different
> > result.
> >
>
> In general, I share your concern. In this case, however, jump_label_t
> is typedef'd three lines up and is never used anywhere else.
I would agree if this was in a .c file, but it's in a header file,
which causes me to be more paranoid.
>
> > Could we switch these calculations to something like:
> >
> > return (jump_label_t)((long)&entrty->code + entry->code);
> >
>
> jump_label_t is local to this .h file, so it can be defined as u32 or
> u64 depending on the word size. I don't mind adding the extra cast,
> but I am not sure if your paranoia is justified in this particular
> case. Perhaps we should just use 'unsigned long' throughout?
Actually, that may be better. Have the return value be jump_label_t,
but the cast be "unsigned long". That way it should always work.
static inline jump_label_t jump_entry_code(...)
{
return (unsigned long)&entry->code + entry->code;
}
-- Steve
^ permalink raw reply
* Re: [PATCH v6 8/8] x86/kernel: jump_table: use relative references
From: Ard Biesheuvel @ 2017-12-28 16:26 UTC (permalink / raw)
To: Steven Rostedt
Cc: Linux Kernel Mailing List, H. Peter Anvin, Ralf Baechle,
Arnd Bergmann, Heiko Carstens, Kees Cook, Will Deacon,
Michael Ellerman, Thomas Garnier, Thomas Gleixner,
Serge E. Hallyn, Bjorn Helgaas, Benjamin Herrenschmidt,
Russell King, Paul Mackerras, Catalin Marinas, David S. Miller,
Petr Mladek, Ingo Molnar, James Morris, Andrew Morton,
Nicolas Pitre, Josh Poimboeuf, Martin Schwidefsky,
Sergey Senozhatsky, Linus Torvalds, Jessica Yu, linux-arm-kernel,
linux-mips, linuxppc-dev, linux-s390, sparclinux,
the arch/x86 maintainers
In-Reply-To: <20171228111926.28e82877@gandalf.local.home>
On 28 December 2017 at 16:19, Steven Rostedt <rostedt@goodmis.org> wrote:
> On Wed, 27 Dec 2017 08:50:33 +0000
> Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
>> static inline jump_label_t jump_entry_code(const struct jump_entry *entry)
>> {
>> - return entry->code;
>> + return (jump_label_t)&entry->code + entry->code;
>
> I'm paranoid about doing arithmetic on abstract types. What happens in
> the future if jump_label_t becomes a pointer? You will get a different
> result.
>
In general, I share your concern. In this case, however, jump_label_t
is typedef'd three lines up and is never used anywhere else.
> Could we switch these calculations to something like:
>
> return (jump_label_t)((long)&entrty->code + entry->code);
>
jump_label_t is local to this .h file, so it can be defined as u32 or
u64 depending on the word size. I don't mind adding the extra cast,
but I am not sure if your paranoia is justified in this particular
case. Perhaps we should just use 'unsigned long' throughout?
>> +}
>> +
>> +static inline jump_label_t jump_entry_target(const struct jump_entry *entry)
>> +{
>> + return (jump_label_t)&entry->target + entry->target;
>> }
>>
>> static inline struct static_key *jump_entry_key(const struct jump_entry *entry)
>> {
>> - return (struct static_key *)((unsigned long)entry->key & ~1UL);
>> + unsigned long key = (unsigned long)&entry->key + entry->key;
>> +
>> + return (struct static_key *)(key & ~1UL);
>> }
>>
>> static inline bool jump_entry_is_branch(const struct jump_entry *entry)
>> @@ -99,7 +106,7 @@ static inline void jump_entry_set_module_init(struct jump_entry *entry)
>> entry->code = 0;
>> }
>>
>> -#define jump_label_swap NULL
>> +void jump_label_swap(void *a, void *b, int size);
>>
>> #else /* __ASSEMBLY__ */
>>
>> @@ -114,8 +121,8 @@ static inline void jump_entry_set_module_init(struct jump_entry *entry)
>> .byte STATIC_KEY_INIT_NOP
>> .endif
>> .pushsection __jump_table, "aw"
>> - _ASM_ALIGN
>> - _ASM_PTR .Lstatic_jump_\@, \target, \key
>> + .balign 4
>> + .long .Lstatic_jump_\@ - ., \target - ., \key - .
>> .popsection
>> .endm
>>
>> @@ -130,8 +137,8 @@ static inline void jump_entry_set_module_init(struct jump_entry *entry)
>> .Lstatic_jump_after_\@:
>> .endif
>> .pushsection __jump_table, "aw"
>> - _ASM_ALIGN
>> - _ASM_PTR .Lstatic_jump_\@, \target, \key + 1
>> + .balign 4
>> + .long .Lstatic_jump_\@ - ., \target - ., \key - . + 1
>> .popsection
>> .endm
>>
>> diff --git a/arch/x86/kernel/jump_label.c b/arch/x86/kernel/jump_label.c
>> index e56c95be2808..cc5034b42335 100644
>> --- a/arch/x86/kernel/jump_label.c
>> +++ b/arch/x86/kernel/jump_label.c
>> @@ -52,22 +52,24 @@ static void __jump_label_transform(struct jump_entry *entry,
>> * Jump label is enabled for the first time.
>> * So we expect a default_nop...
>> */
>> - if (unlikely(memcmp((void *)entry->code, default_nop, 5)
>> - != 0))
>> - bug_at((void *)entry->code, __LINE__);
>> + if (unlikely(memcmp((void *)jump_entry_code(entry),
>> + default_nop, 5) != 0))
>> + bug_at((void *)jump_entry_code(entry),
>
> You have the functions already made before this patch. Perhaps we
> should have a separate patch to use them (here and elsewhere) before
> you make the conversion to using relative references. It will help out
> in debugging and bisects. To know if the use of functions is an issue,
> or the conversion of relative references is an issue.
>
> I suggest splitting this into two patches.
>
Fair enough.
>> + __LINE__);
>> } else {
>> /*
>> * ...otherwise expect an ideal_nop. Otherwise
>> * something went horribly wrong.
>> */
>> - if (unlikely(memcmp((void *)entry->code, ideal_nop, 5)
>> - != 0))
>> - bug_at((void *)entry->code, __LINE__);
>> + if (unlikely(memcmp((void *)jump_entry_code(entry),
>> + ideal_nop, 5) != 0))
>> + bug_at((void *)jump_entry_code(entry),
>> + __LINE__);
>> }
>>
>> code.jump = 0xe9;
>> - code.offset = entry->target -
>> - (entry->code + JUMP_LABEL_NOP_SIZE);
>> + code.offset = jump_entry_target(entry) -
>> + (jump_entry_code(entry) + JUMP_LABEL_NOP_SIZE);
>> } else {
>> /*
>> * We are disabling this jump label. If it is not what
>> @@ -76,14 +78,18 @@ static void __jump_label_transform(struct jump_entry *entry,
>> * are converting the default nop to the ideal nop.
>> */
>> if (init) {
>> - if (unlikely(memcmp((void *)entry->code, default_nop, 5) != 0))
>> - bug_at((void *)entry->code, __LINE__);
>> + if (unlikely(memcmp((void *)jump_entry_code(entry),
>> + default_nop, 5) != 0))
>> + bug_at((void *)jump_entry_code(entry),
>> + __LINE__);
>> } else {
>> code.jump = 0xe9;
>> - code.offset = entry->target -
>> - (entry->code + JUMP_LABEL_NOP_SIZE);
>> - if (unlikely(memcmp((void *)entry->code, &code, 5) != 0))
>> - bug_at((void *)entry->code, __LINE__);
>> + code.offset = jump_entry_target(entry) -
>> + (jump_entry_code(entry) + JUMP_LABEL_NOP_SIZE);
>> + if (unlikely(memcmp((void *)jump_entry_code(entry),
>> + &code, 5) != 0))
>> + bug_at((void *)jump_entry_code(entry),
>> + __LINE__);
>> }
>> memcpy(&code, ideal_nops[NOP_ATOMIC5], JUMP_LABEL_NOP_SIZE);
>> }
>> @@ -97,10 +103,13 @@ static void __jump_label_transform(struct jump_entry *entry,
>> *
>> */
>> if (poker)
>> - (*poker)((void *)entry->code, &code, JUMP_LABEL_NOP_SIZE);
>> + (*poker)((void *)jump_entry_code(entry), &code,
>> + JUMP_LABEL_NOP_SIZE);
>> else
>> - text_poke_bp((void *)entry->code, &code, JUMP_LABEL_NOP_SIZE,
>> - (void *)entry->code + JUMP_LABEL_NOP_SIZE);
>> + text_poke_bp((void *)jump_entry_code(entry), &code,
>> + JUMP_LABEL_NOP_SIZE,
>> + (void *)jump_entry_code(entry) +
>> + JUMP_LABEL_NOP_SIZE);
>> }
>>
>> void arch_jump_label_transform(struct jump_entry *entry,
>> @@ -140,4 +149,20 @@ __init_or_module void arch_jump_label_transform_static(struct jump_entry *entry,
>> __jump_label_transform(entry, type, text_poke_early, 1);
>> }
>>
>> +void jump_label_swap(void *a, void *b, int size)
>> +{
>> + long delta = (unsigned long)a - (unsigned long)b;
>> + struct jump_entry *jea = a;
>> + struct jump_entry *jeb = b;
>> + struct jump_entry tmp = *jea;
>> +
>> + jea->code = jeb->code - delta;
>> + jea->target = jeb->target - delta;
>> + jea->key = jeb->key - delta;
>> +
>> + jeb->code = tmp.code + delta;
>> + jeb->target = tmp.target + delta;
>> + jeb->key = tmp.key + delta;
>> +}
>> +
>> #endif
>> diff --git a/tools/objtool/special.c b/tools/objtool/special.c
>> index 84f001d52322..98ae55b39037 100644
>> --- a/tools/objtool/special.c
>> +++ b/tools/objtool/special.c
>> @@ -30,9 +30,9 @@
>> #define EX_ORIG_OFFSET 0
>> #define EX_NEW_OFFSET 4
>>
>> -#define JUMP_ENTRY_SIZE 24
>> +#define JUMP_ENTRY_SIZE 12
>> #define JUMP_ORIG_OFFSET 0
>> -#define JUMP_NEW_OFFSET 8
>> +#define JUMP_NEW_OFFSET 4
>>
>> #define ALT_ENTRY_SIZE 13
>> #define ALT_ORIG_OFFSET 0
>
^ permalink raw reply
* Re: [PATCH v6 8/8] x86/kernel: jump_table: use relative references
From: Steven Rostedt @ 2017-12-28 16:19 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: linux-kernel, H. Peter Anvin, Ralf Baechle, Arnd Bergmann,
Heiko Carstens, Kees Cook, Will Deacon, Michael Ellerman,
Thomas Garnier, Thomas Gleixner, Serge E. Hallyn, Bjorn Helgaas,
Benjamin Herrenschmidt, Russell King, Paul Mackerras,
Catalin Marinas, David S. Miller, Petr Mladek, Ingo Molnar,
James Morris, Andrew Morton, Nicolas Pitre, Josh Poimboeuf,
Martin Schwidefsky, Sergey Senozhatsky, Linus Torvalds,
Jessica Yu, linux-arm-kernel, linux-mips, linuxppc-dev,
linux-s390, sparclinux, x86
In-Reply-To: <20171227085033.22389-9-ard.biesheuvel@linaro.org>
On Wed, 27 Dec 2017 08:50:33 +0000
Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> static inline jump_label_t jump_entry_code(const struct jump_entry *entry)
> {
> - return entry->code;
> + return (jump_label_t)&entry->code + entry->code;
I'm paranoid about doing arithmetic on abstract types. What happens in
the future if jump_label_t becomes a pointer? You will get a different
result.
Could we switch these calculations to something like:
return (jump_label_t)((long)&entrty->code + entry->code);
> +}
> +
> +static inline jump_label_t jump_entry_target(const struct jump_entry *entry)
> +{
> + return (jump_label_t)&entry->target + entry->target;
> }
>
> static inline struct static_key *jump_entry_key(const struct jump_entry *entry)
> {
> - return (struct static_key *)((unsigned long)entry->key & ~1UL);
> + unsigned long key = (unsigned long)&entry->key + entry->key;
> +
> + return (struct static_key *)(key & ~1UL);
> }
>
> static inline bool jump_entry_is_branch(const struct jump_entry *entry)
> @@ -99,7 +106,7 @@ static inline void jump_entry_set_module_init(struct jump_entry *entry)
> entry->code = 0;
> }
>
> -#define jump_label_swap NULL
> +void jump_label_swap(void *a, void *b, int size);
>
> #else /* __ASSEMBLY__ */
>
> @@ -114,8 +121,8 @@ static inline void jump_entry_set_module_init(struct jump_entry *entry)
> .byte STATIC_KEY_INIT_NOP
> .endif
> .pushsection __jump_table, "aw"
> - _ASM_ALIGN
> - _ASM_PTR .Lstatic_jump_\@, \target, \key
> + .balign 4
> + .long .Lstatic_jump_\@ - ., \target - ., \key - .
> .popsection
> .endm
>
> @@ -130,8 +137,8 @@ static inline void jump_entry_set_module_init(struct jump_entry *entry)
> .Lstatic_jump_after_\@:
> .endif
> .pushsection __jump_table, "aw"
> - _ASM_ALIGN
> - _ASM_PTR .Lstatic_jump_\@, \target, \key + 1
> + .balign 4
> + .long .Lstatic_jump_\@ - ., \target - ., \key - . + 1
> .popsection
> .endm
>
> diff --git a/arch/x86/kernel/jump_label.c b/arch/x86/kernel/jump_label.c
> index e56c95be2808..cc5034b42335 100644
> --- a/arch/x86/kernel/jump_label.c
> +++ b/arch/x86/kernel/jump_label.c
> @@ -52,22 +52,24 @@ static void __jump_label_transform(struct jump_entry *entry,
> * Jump label is enabled for the first time.
> * So we expect a default_nop...
> */
> - if (unlikely(memcmp((void *)entry->code, default_nop, 5)
> - != 0))
> - bug_at((void *)entry->code, __LINE__);
> + if (unlikely(memcmp((void *)jump_entry_code(entry),
> + default_nop, 5) != 0))
> + bug_at((void *)jump_entry_code(entry),
You have the functions already made before this patch. Perhaps we
should have a separate patch to use them (here and elsewhere) before
you make the conversion to using relative references. It will help out
in debugging and bisects. To know if the use of functions is an issue,
or the conversion of relative references is an issue.
I suggest splitting this into two patches.
-- Steve
> + __LINE__);
> } else {
> /*
> * ...otherwise expect an ideal_nop. Otherwise
> * something went horribly wrong.
> */
> - if (unlikely(memcmp((void *)entry->code, ideal_nop, 5)
> - != 0))
> - bug_at((void *)entry->code, __LINE__);
> + if (unlikely(memcmp((void *)jump_entry_code(entry),
> + ideal_nop, 5) != 0))
> + bug_at((void *)jump_entry_code(entry),
> + __LINE__);
> }
>
> code.jump = 0xe9;
> - code.offset = entry->target -
> - (entry->code + JUMP_LABEL_NOP_SIZE);
> + code.offset = jump_entry_target(entry) -
> + (jump_entry_code(entry) + JUMP_LABEL_NOP_SIZE);
> } else {
> /*
> * We are disabling this jump label. If it is not what
> @@ -76,14 +78,18 @@ static void __jump_label_transform(struct jump_entry *entry,
> * are converting the default nop to the ideal nop.
> */
> if (init) {
> - if (unlikely(memcmp((void *)entry->code, default_nop, 5) != 0))
> - bug_at((void *)entry->code, __LINE__);
> + if (unlikely(memcmp((void *)jump_entry_code(entry),
> + default_nop, 5) != 0))
> + bug_at((void *)jump_entry_code(entry),
> + __LINE__);
> } else {
> code.jump = 0xe9;
> - code.offset = entry->target -
> - (entry->code + JUMP_LABEL_NOP_SIZE);
> - if (unlikely(memcmp((void *)entry->code, &code, 5) != 0))
> - bug_at((void *)entry->code, __LINE__);
> + code.offset = jump_entry_target(entry) -
> + (jump_entry_code(entry) + JUMP_LABEL_NOP_SIZE);
> + if (unlikely(memcmp((void *)jump_entry_code(entry),
> + &code, 5) != 0))
> + bug_at((void *)jump_entry_code(entry),
> + __LINE__);
> }
> memcpy(&code, ideal_nops[NOP_ATOMIC5], JUMP_LABEL_NOP_SIZE);
> }
> @@ -97,10 +103,13 @@ static void __jump_label_transform(struct jump_entry *entry,
> *
> */
> if (poker)
> - (*poker)((void *)entry->code, &code, JUMP_LABEL_NOP_SIZE);
> + (*poker)((void *)jump_entry_code(entry), &code,
> + JUMP_LABEL_NOP_SIZE);
> else
> - text_poke_bp((void *)entry->code, &code, JUMP_LABEL_NOP_SIZE,
> - (void *)entry->code + JUMP_LABEL_NOP_SIZE);
> + text_poke_bp((void *)jump_entry_code(entry), &code,
> + JUMP_LABEL_NOP_SIZE,
> + (void *)jump_entry_code(entry) +
> + JUMP_LABEL_NOP_SIZE);
> }
>
> void arch_jump_label_transform(struct jump_entry *entry,
> @@ -140,4 +149,20 @@ __init_or_module void arch_jump_label_transform_static(struct jump_entry *entry,
> __jump_label_transform(entry, type, text_poke_early, 1);
> }
>
> +void jump_label_swap(void *a, void *b, int size)
> +{
> + long delta = (unsigned long)a - (unsigned long)b;
> + struct jump_entry *jea = a;
> + struct jump_entry *jeb = b;
> + struct jump_entry tmp = *jea;
> +
> + jea->code = jeb->code - delta;
> + jea->target = jeb->target - delta;
> + jea->key = jeb->key - delta;
> +
> + jeb->code = tmp.code + delta;
> + jeb->target = tmp.target + delta;
> + jeb->key = tmp.key + delta;
> +}
> +
> #endif
> diff --git a/tools/objtool/special.c b/tools/objtool/special.c
> index 84f001d52322..98ae55b39037 100644
> --- a/tools/objtool/special.c
> +++ b/tools/objtool/special.c
> @@ -30,9 +30,9 @@
> #define EX_ORIG_OFFSET 0
> #define EX_NEW_OFFSET 4
>
> -#define JUMP_ENTRY_SIZE 24
> +#define JUMP_ENTRY_SIZE 12
> #define JUMP_ORIG_OFFSET 0
> -#define JUMP_NEW_OFFSET 8
> +#define JUMP_NEW_OFFSET 4
>
> #define ALT_ENTRY_SIZE 13
> #define ALT_ORIG_OFFSET 0
^ permalink raw reply
* Re: [PATCH v6 5/8] kernel: tracepoints: add support for relative references
From: Steven Rostedt @ 2017-12-28 15:42 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: linux-kernel, H. Peter Anvin, Ralf Baechle, Arnd Bergmann,
Heiko Carstens, Kees Cook, Will Deacon, Michael Ellerman,
Thomas Garnier, Thomas Gleixner, Serge E. Hallyn, Bjorn Helgaas,
Benjamin Herrenschmidt, Russell King, Paul Mackerras,
Catalin Marinas, David S. Miller, Petr Mladek, Ingo Molnar,
James Morris, Andrew Morton, Nicolas Pitre, Josh Poimboeuf,
Martin Schwidefsky, Sergey Senozhatsky, Linus Torvalds,
Jessica Yu, linux-arm-kernel, linux-mips, linuxppc-dev,
linux-s390, sparclinux, x86
In-Reply-To: <20171227085033.22389-6-ard.biesheuvel@linaro.org>
On Wed, 27 Dec 2017 08:50:30 +0000
Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> To avoid the need for relocating absolute references to tracepoint
> structures at boot time when running relocatable kernels (which may
> take a disproportionate amount of space), add the option to emit
> these tables as relative references instead.
>
I gave this patch a quick skim over. It appears to not modify anything
when CONFIG_HAVE_PREL32_RELOCATIONS is not defined. I haven't
thoroughly reviewed it or tested it. But if it doesn't break anything,
I'm fine giving you an ack.
Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
-- Steve
^ permalink raw reply
* Re: [PATCH v6 2/8] module: use relative references for __ksymtab entries
From: Ard Biesheuvel @ 2017-12-28 12:39 UTC (permalink / raw)
To: Ingo Molnar
Cc: Linus Torvalds, Linux Kernel Mailing List, H. Peter Anvin,
Ralf Baechle, Arnd Bergmann, Heiko Carstens, Kees Cook,
Will Deacon, Michael Ellerman, Thomas Garnier, Thomas Gleixner,
Serge E. Hallyn, Bjorn Helgaas, Benjamin Herrenschmidt,
Russell King, Paul Mackerras, Catalin Marinas, David S. Miller,
Petr Mladek, Ingo Molnar, James Morris, Andrew Morton,
Nicolas Pitre, Josh Poimboeuf, Steven Rostedt, Martin Schwidefsky,
Sergey Senozhatsky, Jessica Yu, linux-arm-kernel, linux-mips,
ppc-dev, linux-s390, sparclinux, the arch/x86 maintainers
In-Reply-To: <20171228120531.xhcb4dg2qu2z5ssp@gmail.com>
On 28 December 2017 at 12:05, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
>> Annoyingly, we need this because there is a single instance of a
>> special section that ends up in the EFI stub code: we build lib/sort.c
>> again as a EFI libstub object, and given that sort() is exported, we
>> end up with a ksymtab section in the EFI stub. The sort() thing has
>> caused issues before [0], so perhaps I should just clone sort.c into
>> drivers/firmware/efi/libstub and get rid of that hack.
>>
>> [0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=29f9007b3182ab3f328a31da13e6b1c9072f7a95
>
> If the root problem is early bootstrap code randomly using generic facility that
> isn't __init, then we should definitely improve tooling to at least detect these
> problems.
>
> As bootstrap code gets improved (KASLR, more complex decompression, etc. etc.) we
> keep using new bits of generic facilities...
>
> So this should definitely not be hidden by open coding that function (which has
> various other disadvantages as well), but should be turned from silent breakage
> either into non-breakage (and do so not only for sort() but for other generic
> functions as well), or should be turned into a build failure.
>
We already have safeguards in place to ensure that the arm64 EFI stub
(which is essentially the same executable as the kernel proper) only
pulls in code that has been made available to it explicitly. That is
why sort.c is recompiled for the EFI stub, as well as all other C code
that is shared between the stub and the kernel. We also have a build
time check to ensure that the resulting code does not rely on absolute
symbol references, which will be invalid in the UEFI execution
context.
So the only problem is that unneeded ksymtab/kcrctab sections, which
affected ARM for obscure reasons; typically, they just take up some
space. On x86, the kaslr code deals with a similar issue by
#define'ing _LINUX_EXPORT_H before including linux/export.h, which
also gets rid of these sections, but I was a bit reluctant to copy
that pattern. Perhaps we should enhance linux/export.h for reasons
such as these by adding a macro that nops out EXPORT_SYMBOL()
declarations?
^ permalink raw reply
* Re: [PATCH v6 2/8] module: use relative references for __ksymtab entries
From: Ingo Molnar @ 2017-12-28 12:05 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Linus Torvalds, Linux Kernel Mailing List, H. Peter Anvin,
Ralf Baechle, Arnd Bergmann, Heiko Carstens, Kees Cook,
Will Deacon, Michael Ellerman, Thomas Garnier, Thomas Gleixner,
Serge E. Hallyn, Bjorn Helgaas, Benjamin Herrenschmidt,
Russell King, Paul Mackerras, Catalin Marinas, David S. Miller,
Petr Mladek, Ingo Molnar, James Morris, Andrew Morton,
Nicolas Pitre, Josh Poimboeuf, Steven Rostedt, Martin Schwidefsky,
Sergey Senozhatsky, Jessica Yu, linux-arm-kernel, linux-mips,
ppc-dev, linux-s390, sparclinux, the arch/x86 maintainers
In-Reply-To: <CAKv+Gu-wRtfnGfrEjuxR5YkCpZM-nQHZShROEdbcEh=fSiWf5A@mail.gmail.com>
* Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> Annoyingly, we need this because there is a single instance of a
> special section that ends up in the EFI stub code: we build lib/sort.c
> again as a EFI libstub object, and given that sort() is exported, we
> end up with a ksymtab section in the EFI stub. The sort() thing has
> caused issues before [0], so perhaps I should just clone sort.c into
> drivers/firmware/efi/libstub and get rid of that hack.
>
> [0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=29f9007b3182ab3f328a31da13e6b1c9072f7a95
If the root problem is early bootstrap code randomly using generic facility that
isn't __init, then we should definitely improve tooling to at least detect these
problems.
As bootstrap code gets improved (KASLR, more complex decompression, etc. etc.) we
keep using new bits of generic facilities...
So this should definitely not be hidden by open coding that function (which has
various other disadvantages as well), but should be turned from silent breakage
either into non-breakage (and do so not only for sort() but for other generic
functions as well), or should be turned into a build failure.
Thanks,
Ingo
^ permalink raw reply
* Re: [PATCH v6 2/8] module: use relative references for __ksymtab entries
From: Ard Biesheuvel @ 2017-12-27 20:24 UTC (permalink / raw)
To: Linus Torvalds
Cc: Linux Kernel Mailing List, H. Peter Anvin, Ralf Baechle,
Arnd Bergmann, Heiko Carstens, Kees Cook, Will Deacon,
Michael Ellerman, Thomas Garnier, Thomas Gleixner,
Serge E. Hallyn, Bjorn Helgaas, Benjamin Herrenschmidt,
Russell King, Paul Mackerras, Catalin Marinas, David S. Miller,
Petr Mladek, Ingo Molnar, James Morris, Andrew Morton,
Nicolas Pitre, Josh Poimboeuf, Steven Rostedt, Martin Schwidefsky,
Sergey Senozhatsky, Jessica Yu, linux-arm-kernel, linux-mips,
ppc-dev, linux-s390, sparclinux, the arch/x86 maintainers,
Ingo Molnar
In-Reply-To: <CA+55aFzygF6P3v5VxyBucZfn-tg58jeV6qwt0y7QGmmNiKYghQ@mail.gmail.com>
On 27 December 2017 at 20:13, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Wed, Dec 27, 2017 at 12:11 PM, Ard Biesheuvel
> <ard.biesheuvel@linaro.org> wrote:
>>
>> I tried to keep the generic patches generic, so perhaps I should just
>> put the arm64 vmlinux.lds.S change in a patch on its own?
>
> I guess it doesn't matter, but regardless of where it gets introduced
> I would like to see the explanation for where the heck that magical
> ".init.discard.text" comes from. It's definitely not obvious from the
> patches, and is presumably some odd arm64 special case.
>
This has to do with the EFI stub. x86 and ARM link it into the
decompressor, and so the code and data are not annotated as __init
(and doing so would involve modifying a lot of code). arm64 does not
have a decompressor, and so the EFI stub is linked into the kernel
proper. To make sure the code ends up in the .init segment, all
sections are prepended with .init at the object level, using objcopy.
Annoyingly, we need this because there is a single instance of a
special section that ends up in the EFI stub code: we build lib/sort.c
again as a EFI libstub object, and given that sort() is exported, we
end up with a ksymtab section in the EFI stub. The sort() thing has
caused issues before [0], so perhaps I should just clone sort.c into
drivers/firmware/efi/libstub and get rid of that hack.
[0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=29f9007b3182ab3f328a31da13e6b1c9072f7a95
^ permalink raw reply
* Re: [PATCH v6 2/8] module: use relative references for __ksymtab entries
From: Linus Torvalds @ 2017-12-27 20:13 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Linux Kernel Mailing List, H. Peter Anvin, Ralf Baechle,
Arnd Bergmann, Heiko Carstens, Kees Cook, Will Deacon,
Michael Ellerman, Thomas Garnier, Thomas Gleixner,
Serge E. Hallyn, Bjorn Helgaas, Benjamin Herrenschmidt,
Russell King, Paul Mackerras, Catalin Marinas, David S. Miller,
Petr Mladek, Ingo Molnar, James Morris, Andrew Morton,
Nicolas Pitre, Josh Poimboeuf, Steven Rostedt, Martin Schwidefsky,
Sergey Senozhatsky, Jessica Yu, linux-arm-kernel, linux-mips,
ppc-dev, linux-s390, sparclinux, the arch/x86 maintainers,
Ingo Molnar
In-Reply-To: <CAKv+Gu-2NUzsakN2rcM_5fqD0ubr+6ZXSc+5sjZZPbU3wj_Xsg@mail.gmail.com>
On Wed, Dec 27, 2017 at 12:11 PM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
>
> I tried to keep the generic patches generic, so perhaps I should just
> put the arm64 vmlinux.lds.S change in a patch on its own?
I guess it doesn't matter, but regardless of where it gets introduced
I would like to see the explanation for where the heck that magical
".init.discard.text" comes from. It's definitely not obvious from the
patches, and is presumably some odd arm64 special case.
Linus
^ permalink raw reply
* Re: [PATCH v6 2/8] module: use relative references for __ksymtab entries
From: Ard Biesheuvel @ 2017-12-27 20:11 UTC (permalink / raw)
To: Linus Torvalds
Cc: Linux Kernel Mailing List, H. Peter Anvin, Ralf Baechle,
Arnd Bergmann, Heiko Carstens, Kees Cook, Will Deacon,
Michael Ellerman, Thomas Garnier, Thomas Gleixner,
Serge E. Hallyn, Bjorn Helgaas, Benjamin Herrenschmidt,
Russell King, Paul Mackerras, Catalin Marinas, David S. Miller,
Petr Mladek, Ingo Molnar, James Morris, Andrew Morton,
Nicolas Pitre, Josh Poimboeuf, Steven Rostedt, Martin Schwidefsky,
Sergey Senozhatsky, Jessica Yu, linux-arm-kernel, linux-mips,
ppc-dev, linux-s390, sparclinux, the arch/x86 maintainers,
Ingo Molnar
In-Reply-To: <CA+55aFxqJqJq_7VUzBVTppgXFPc-8Ou55iLsZjp3fr6B2gRyTQ@mail.gmail.com>
On 27 December 2017 at 20:07, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Wed, Dec 27, 2017 at 12:50 AM, Ard Biesheuvel
> <ard.biesheuvel@linaro.org> wrote:
>> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
>> index 52e611ab9a6c..fe752d365334 100644
>> --- a/include/linux/compiler.h
>> +++ b/include/linux/compiler.h
>> @@ -327,4 +327,15 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s
>> compiletime_assert(__native_word(t), \
>> "Need native word sized stores/loads for atomicity.")
>>
>> +/*
>> + * Force the compiler to emit 'sym' as a symbol, so that we can reference
>> + * it from inline assembler. Necessary in case 'sym' could be inlined
>> + * otherwise, or eliminated entirely due to lack of references that are
>> + * visibile to the compiler.
>> + */
>> +#define __ADDRESSABLE(sym) \
>> + static void *__attribute__((section(".discard.text"), used)) \
>> + __PASTE(__discard_##sym, __LINE__)(void) \
>> + { return (void *)&sym; } \
>> +
>> #endif /* __LINUX_COMPILER_H */
>
> Isn't this logically the point where you should add the arm64
> vmlinux.lds.S change, and explain how ".discard.text" turns into
> ".init.discard.text" for some odd arm64 reason?
>
I tried to keep the generic patches generic, so perhaps I should just
put the arm64 vmlinux.lds.S change in a patch on its own?
^ permalink raw reply
* Re: [PATCH v6 2/8] module: use relative references for __ksymtab entries
From: Linus Torvalds @ 2017-12-27 20:07 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Linux Kernel Mailing List, H. Peter Anvin, Ralf Baechle,
Arnd Bergmann, Heiko Carstens, Kees Cook, Will Deacon,
Michael Ellerman, Thomas Garnier, Thomas Gleixner,
Serge E. Hallyn, Bjorn Helgaas, Benjamin Herrenschmidt,
Russell King, Paul Mackerras, Catalin Marinas, David S. Miller,
Petr Mladek, Ingo Molnar, James Morris, Andrew Morton,
Nicolas Pitre, Josh Poimboeuf, Steven Rostedt, Martin Schwidefsky,
Sergey Senozhatsky, Jessica Yu, linux-arm-kernel, linux-mips,
ppc-dev, linux-s390, sparclinux, the arch/x86 maintainers,
Ingo Molnar
In-Reply-To: <20171227085033.22389-3-ard.biesheuvel@linaro.org>
On Wed, Dec 27, 2017 at 12:50 AM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> index 52e611ab9a6c..fe752d365334 100644
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -327,4 +327,15 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s
> compiletime_assert(__native_word(t), \
> "Need native word sized stores/loads for atomicity.")
>
> +/*
> + * Force the compiler to emit 'sym' as a symbol, so that we can reference
> + * it from inline assembler. Necessary in case 'sym' could be inlined
> + * otherwise, or eliminated entirely due to lack of references that are
> + * visibile to the compiler.
> + */
> +#define __ADDRESSABLE(sym) \
> + static void *__attribute__((section(".discard.text"), used)) \
> + __PASTE(__discard_##sym, __LINE__)(void) \
> + { return (void *)&sym; } \
> +
> #endif /* __LINUX_COMPILER_H */
Isn't this logically the point where you should add the arm64
vmlinux.lds.S change, and explain how ".discard.text" turns into
".init.discard.text" for some odd arm64 reason?
Linus
^ permalink raw reply
* Re: [PATCH v6 1/8] arch: enable relative relocations for arm64, power, x86, s390 and x86
From: Ard Biesheuvel @ 2017-12-27 19:59 UTC (permalink / raw)
To: Linus Torvalds
Cc: Linux Kernel Mailing List, H. Peter Anvin, Ralf Baechle,
Arnd Bergmann, Heiko Carstens, Kees Cook, Will Deacon,
Michael Ellerman, Thomas Garnier, Thomas Gleixner,
Serge E. Hallyn, Bjorn Helgaas, Benjamin Herrenschmidt,
Russell King, Paul Mackerras, Catalin Marinas, David S. Miller,
Petr Mladek, Ingo Molnar, James Morris, Andrew Morton,
Nicolas Pitre, Josh Poimboeuf, Steven Rostedt, Martin Schwidefsky,
Sergey Senozhatsky, Jessica Yu, linux-arm-kernel, linux-mips,
ppc-dev, linux-s390, sparclinux, the arch/x86 maintainers
In-Reply-To: <CA+55aFyJg=-PfrVV1kw_bqKKd9Uk+q2FS4pqPp-og_DDbhhaFw@mail.gmail.com>
On 27 December 2017 at 19:54, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Wed, Dec 27, 2017 at 12:50 AM, Ard Biesheuvel
> <ard.biesheuvel@linaro.org> wrote:
>> diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
>> index 7da3e5c366a0..49ae5b43fe2b 100644
>> --- a/arch/arm64/kernel/vmlinux.lds.S
>> +++ b/arch/arm64/kernel/vmlinux.lds.S
>> @@ -156,7 +156,7 @@ SECTIONS
>> CON_INITCALL
>> SECURITY_INITCALL
>> INIT_RAM_FS
>> - *(.init.rodata.* .init.bss) /* from the EFI stub */
>> + *(.init.rodata.* .init.bss .init.discard.*) /* EFI stub */
>> }
>> .exit.data : {
>> ARM_EXIT_KEEP(EXIT_DATA)
>
> Weren't you supposed to explain this part in the commit message?
>
Oops. Apologies, I indeed forgot to update the commit log.
> It isn't obvious why this is mixed up with the Kconfig changes, and
> somebody already asked about it. The commit message only talks about
> the Kconfig changes, and then suddenly there's that odd vmlinux.lds.S
> change in there...
>
Yeah. It doesn't make sense to respin right away for just that, so I
will give people some time to respond, and respin in a week or so.
^ permalink raw reply
* Re: [PATCH v6 1/8] arch: enable relative relocations for arm64, power, x86, s390 and x86
From: Linus Torvalds @ 2017-12-27 19:54 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Linux Kernel Mailing List, H. Peter Anvin, Ralf Baechle,
Arnd Bergmann, Heiko Carstens, Kees Cook, Will Deacon,
Michael Ellerman, Thomas Garnier, Thomas Gleixner,
Serge E. Hallyn, Bjorn Helgaas, Benjamin Herrenschmidt,
Russell King, Paul Mackerras, Catalin Marinas, David S. Miller,
Petr Mladek, Ingo Molnar, James Morris, Andrew Morton,
Nicolas Pitre, Josh Poimboeuf, Steven Rostedt, Martin Schwidefsky,
Sergey Senozhatsky, Jessica Yu, linux-arm-kernel, linux-mips,
ppc-dev, linux-s390, sparclinux, the arch/x86 maintainers
In-Reply-To: <20171227085033.22389-2-ard.biesheuvel@linaro.org>
On Wed, Dec 27, 2017 at 12:50 AM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
> index 7da3e5c366a0..49ae5b43fe2b 100644
> --- a/arch/arm64/kernel/vmlinux.lds.S
> +++ b/arch/arm64/kernel/vmlinux.lds.S
> @@ -156,7 +156,7 @@ SECTIONS
> CON_INITCALL
> SECURITY_INITCALL
> INIT_RAM_FS
> - *(.init.rodata.* .init.bss) /* from the EFI stub */
> + *(.init.rodata.* .init.bss .init.discard.*) /* EFI stub */
> }
> .exit.data : {
> ARM_EXIT_KEEP(EXIT_DATA)
Weren't you supposed to explain this part in the commit message?
It isn't obvious why this is mixed up with the Kconfig changes, and
somebody already asked about it. The commit message only talks about
the Kconfig changes, and then suddenly there's that odd vmlinux.lds.S
change in there...
Linus
^ permalink raw reply
* Re: [PATCH v3 0/3] create sysfs representation of ACPI HMAT
From: Brice Goglin @ 2017-12-27 9:10 UTC (permalink / raw)
To: Dan Williams
Cc: Ross Zwisler, Matthew Wilcox, Dave Hansen, Michal Hocko,
linux-kernel@vger.kernel.org, Anaczkowski, Lukasz, Box, David E,
Kogut, Jaroslaw, Koss, Marcin, Koziej, Artur, Lahtinen, Joonas,
Moore, Robert, Nachimuthu, Murugasamy, Odzioba, Lukasz,
Rafael J. Wysocki, Rafael J. Wysocki, Schmauss, Erik,
Verma, Vishal L, Zheng, Lv, Andrew Morton, Balbir Singh,
Jerome Glisse, John Hubbard, Len Brown, Tim Chen, devel,
Linux ACPI, Linux MM, linux-nvdimm@lists.01.org, Linux API,
linuxppc-dev
In-Reply-To: <CAPcyv4j9shdJFrvADa=qW4L-jPJJ4S_TJc_c=aRoW3EmSCCChQ@mail.gmail.com>
Le 22/12/2017 à 23:53, Dan Williams a écrit :
> On Thu, Dec 21, 2017 at 12:31 PM, Brice Goglin <brice.goglin@gmail.com> wrote:
>> Le 20/12/2017 à 23:41, Ross Zwisler a écrit :
> [..]
>> Hello
>>
>> I can confirm that HPC runtimes are going to use these patches (at least
>> all runtimes that use hwloc for topology discovery, but that's the vast
>> majority of HPC anyway).
>>
>> We really didn't like KNL exposing a hacky SLIT table [1]. We had to
>> explicitly detect that specific crazy table to find out which NUMA nodes
>> were local to which cores, and to find out which NUMA nodes were
>> HBM/MCDRAM or DDR. And then we had to hide the SLIT values to the
>> application because the reported latencies didn't match reality. Quite
>> annoying.
>>
>> With Ross' patches, we can easily get what we need:
>> * which NUMA nodes are local to which CPUs? /sys/devices/system/node/
>> can only report a single local node per CPU (doesn't work for KNL and
>> upcoming architectures with HBM+DDR+...)
>> * which NUMA nodes are slow/fast (for both bandwidth and latency)
>> And we can still look at SLIT under /sys/devices/system/node if really
>> needed.
>>
>> And of course having this in sysfs is much better than parsing ACPI
>> tables that are only accessible to root :)
> On this point, it's not clear to me that we should allow these sysfs
> entries to be world readable. Given /proc/iomem now hides physical
> address information from non-root we at least need to be careful not
> to undo that with new sysfs HMAT attributes. Once you need to be root
> for this info, is parsing binary HMAT vs sysfs a blocker for the HPC
> use case?
I don't think it would be a blocker.
> Perhaps we can enlist /proc/iomem or a similar enumeration interface
> to tell userspace the NUMA node and whether the kernel thinks it has
> better or worse performance characteristics relative to base
> system-RAM, i.e. new IORES_DESC_* values. I'm worried that if we start
> publishing absolute numbers in sysfs userspace will default to looking
> for specific magic numbers in sysfs vs asking the kernel for memory
> that has performance characteristics relative to base "System RAM". In
> other words the absolute performance information that the HMAT
> publishes is useful to the kernel, but it's not clear that userspace
> needs that vs a relative indicator for making NUMA node preference
> decisions.
Some HPC users will benchmark the machine to discovery actual
performance numbers anyway.
However, most users won't do this. They will want to know relative
performance of different nodes. If you normalize HMAT values by dividing
them with system-RAM values, that's likely OK. If you just say "that
node is faster than system RAM", it's not precise enough.
Brice
^ permalink raw reply
* [PATCH v6 8/8] x86/kernel: jump_table: use relative references
From: Ard Biesheuvel @ 2017-12-27 8:50 UTC (permalink / raw)
To: linux-kernel
Cc: Ard Biesheuvel, H. Peter Anvin, Ralf Baechle, Arnd Bergmann,
Heiko Carstens, Kees Cook, Will Deacon, Michael Ellerman,
Thomas Garnier, Thomas Gleixner, Serge E. Hallyn, Bjorn Helgaas,
Benjamin Herrenschmidt, Russell King, Paul Mackerras,
Catalin Marinas, David S. Miller, Petr Mladek, Ingo Molnar,
James Morris, Andrew Morton, Nicolas Pitre, Josh Poimboeuf,
Steven Rostedt, Martin Schwidefsky, Sergey Senozhatsky,
Linus Torvalds, Jessica Yu, linux-arm-kernel, linux-mips,
linuxppc-dev, linux-s390, sparclinux, x86
In-Reply-To: <20171227085033.22389-1-ard.biesheuvel@linaro.org>
Similar to the arm64 case, 64-bit x86 can benefit from using 32-bit
relative references rather than 64-bit absolute ones when emitting
struct jump_entry instances. Not only does this reduce the memory
footprint of the entries themselves by 50%, it also removes the need
for carrying relocation metadata on relocatable builds (i.e., for KASLR)
which saves a fair chunk of .init space as well (although the savings
are not as dramatic as on arm64)
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/x86/include/asm/jump_label.h | 35 +++++++-----
arch/x86/kernel/jump_label.c | 59 ++++++++++++++------
tools/objtool/special.c | 4 +-
3 files changed, 65 insertions(+), 33 deletions(-)
diff --git a/arch/x86/include/asm/jump_label.h b/arch/x86/include/asm/jump_label.h
index 009ff2699d07..91c01af96907 100644
--- a/arch/x86/include/asm/jump_label.h
+++ b/arch/x86/include/asm/jump_label.h
@@ -36,8 +36,8 @@ static __always_inline bool arch_static_branch(struct static_key *key, bool bran
asm_volatile_goto("1:"
".byte " __stringify(STATIC_KEY_INIT_NOP) "\n\t"
".pushsection __jump_table, \"aw\" \n\t"
- _ASM_ALIGN "\n\t"
- _ASM_PTR "1b, %l[l_yes], %c0 + %c1 \n\t"
+ ".balign 4\n\t"
+ ".long 1b - ., %l[l_yes] - ., %c0 + %c1 - .\n\t"
".popsection \n\t"
: : "i" (key), "i" (branch) : : l_yes);
@@ -52,8 +52,8 @@ static __always_inline bool arch_static_branch_jump(struct static_key *key, bool
".byte 0xe9\n\t .long %l[l_yes] - 2f\n\t"
"2:\n\t"
".pushsection __jump_table, \"aw\" \n\t"
- _ASM_ALIGN "\n\t"
- _ASM_PTR "1b, %l[l_yes], %c0 + %c1 \n\t"
+ ".balign 4\n\t"
+ ".long 1b - ., %l[l_yes] - ., %c0 + %c1 - .\n\t"
".popsection \n\t"
: : "i" (key), "i" (branch) : : l_yes);
@@ -69,19 +69,26 @@ typedef u32 jump_label_t;
#endif
struct jump_entry {
- jump_label_t code;
- jump_label_t target;
- jump_label_t key;
+ s32 code;
+ s32 target;
+ s32 key;
};
static inline jump_label_t jump_entry_code(const struct jump_entry *entry)
{
- return entry->code;
+ return (jump_label_t)&entry->code + entry->code;
+}
+
+static inline jump_label_t jump_entry_target(const struct jump_entry *entry)
+{
+ return (jump_label_t)&entry->target + entry->target;
}
static inline struct static_key *jump_entry_key(const struct jump_entry *entry)
{
- return (struct static_key *)((unsigned long)entry->key & ~1UL);
+ unsigned long key = (unsigned long)&entry->key + entry->key;
+
+ return (struct static_key *)(key & ~1UL);
}
static inline bool jump_entry_is_branch(const struct jump_entry *entry)
@@ -99,7 +106,7 @@ static inline void jump_entry_set_module_init(struct jump_entry *entry)
entry->code = 0;
}
-#define jump_label_swap NULL
+void jump_label_swap(void *a, void *b, int size);
#else /* __ASSEMBLY__ */
@@ -114,8 +121,8 @@ static inline void jump_entry_set_module_init(struct jump_entry *entry)
.byte STATIC_KEY_INIT_NOP
.endif
.pushsection __jump_table, "aw"
- _ASM_ALIGN
- _ASM_PTR .Lstatic_jump_\@, \target, \key
+ .balign 4
+ .long .Lstatic_jump_\@ - ., \target - ., \key - .
.popsection
.endm
@@ -130,8 +137,8 @@ static inline void jump_entry_set_module_init(struct jump_entry *entry)
.Lstatic_jump_after_\@:
.endif
.pushsection __jump_table, "aw"
- _ASM_ALIGN
- _ASM_PTR .Lstatic_jump_\@, \target, \key + 1
+ .balign 4
+ .long .Lstatic_jump_\@ - ., \target - ., \key - . + 1
.popsection
.endm
diff --git a/arch/x86/kernel/jump_label.c b/arch/x86/kernel/jump_label.c
index e56c95be2808..cc5034b42335 100644
--- a/arch/x86/kernel/jump_label.c
+++ b/arch/x86/kernel/jump_label.c
@@ -52,22 +52,24 @@ static void __jump_label_transform(struct jump_entry *entry,
* Jump label is enabled for the first time.
* So we expect a default_nop...
*/
- if (unlikely(memcmp((void *)entry->code, default_nop, 5)
- != 0))
- bug_at((void *)entry->code, __LINE__);
+ if (unlikely(memcmp((void *)jump_entry_code(entry),
+ default_nop, 5) != 0))
+ bug_at((void *)jump_entry_code(entry),
+ __LINE__);
} else {
/*
* ...otherwise expect an ideal_nop. Otherwise
* something went horribly wrong.
*/
- if (unlikely(memcmp((void *)entry->code, ideal_nop, 5)
- != 0))
- bug_at((void *)entry->code, __LINE__);
+ if (unlikely(memcmp((void *)jump_entry_code(entry),
+ ideal_nop, 5) != 0))
+ bug_at((void *)jump_entry_code(entry),
+ __LINE__);
}
code.jump = 0xe9;
- code.offset = entry->target -
- (entry->code + JUMP_LABEL_NOP_SIZE);
+ code.offset = jump_entry_target(entry) -
+ (jump_entry_code(entry) + JUMP_LABEL_NOP_SIZE);
} else {
/*
* We are disabling this jump label. If it is not what
@@ -76,14 +78,18 @@ static void __jump_label_transform(struct jump_entry *entry,
* are converting the default nop to the ideal nop.
*/
if (init) {
- if (unlikely(memcmp((void *)entry->code, default_nop, 5) != 0))
- bug_at((void *)entry->code, __LINE__);
+ if (unlikely(memcmp((void *)jump_entry_code(entry),
+ default_nop, 5) != 0))
+ bug_at((void *)jump_entry_code(entry),
+ __LINE__);
} else {
code.jump = 0xe9;
- code.offset = entry->target -
- (entry->code + JUMP_LABEL_NOP_SIZE);
- if (unlikely(memcmp((void *)entry->code, &code, 5) != 0))
- bug_at((void *)entry->code, __LINE__);
+ code.offset = jump_entry_target(entry) -
+ (jump_entry_code(entry) + JUMP_LABEL_NOP_SIZE);
+ if (unlikely(memcmp((void *)jump_entry_code(entry),
+ &code, 5) != 0))
+ bug_at((void *)jump_entry_code(entry),
+ __LINE__);
}
memcpy(&code, ideal_nops[NOP_ATOMIC5], JUMP_LABEL_NOP_SIZE);
}
@@ -97,10 +103,13 @@ static void __jump_label_transform(struct jump_entry *entry,
*
*/
if (poker)
- (*poker)((void *)entry->code, &code, JUMP_LABEL_NOP_SIZE);
+ (*poker)((void *)jump_entry_code(entry), &code,
+ JUMP_LABEL_NOP_SIZE);
else
- text_poke_bp((void *)entry->code, &code, JUMP_LABEL_NOP_SIZE,
- (void *)entry->code + JUMP_LABEL_NOP_SIZE);
+ text_poke_bp((void *)jump_entry_code(entry), &code,
+ JUMP_LABEL_NOP_SIZE,
+ (void *)jump_entry_code(entry) +
+ JUMP_LABEL_NOP_SIZE);
}
void arch_jump_label_transform(struct jump_entry *entry,
@@ -140,4 +149,20 @@ __init_or_module void arch_jump_label_transform_static(struct jump_entry *entry,
__jump_label_transform(entry, type, text_poke_early, 1);
}
+void jump_label_swap(void *a, void *b, int size)
+{
+ long delta = (unsigned long)a - (unsigned long)b;
+ struct jump_entry *jea = a;
+ struct jump_entry *jeb = b;
+ struct jump_entry tmp = *jea;
+
+ jea->code = jeb->code - delta;
+ jea->target = jeb->target - delta;
+ jea->key = jeb->key - delta;
+
+ jeb->code = tmp.code + delta;
+ jeb->target = tmp.target + delta;
+ jeb->key = tmp.key + delta;
+}
+
#endif
diff --git a/tools/objtool/special.c b/tools/objtool/special.c
index 84f001d52322..98ae55b39037 100644
--- a/tools/objtool/special.c
+++ b/tools/objtool/special.c
@@ -30,9 +30,9 @@
#define EX_ORIG_OFFSET 0
#define EX_NEW_OFFSET 4
-#define JUMP_ENTRY_SIZE 24
+#define JUMP_ENTRY_SIZE 12
#define JUMP_ORIG_OFFSET 0
-#define JUMP_NEW_OFFSET 8
+#define JUMP_NEW_OFFSET 4
#define ALT_ENTRY_SIZE 13
#define ALT_ORIG_OFFSET 0
--
2.11.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox