LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 03/10] mm: Convert p[te|md]_numa users to p[te|md]_protnone_numa
From: Aneesh Kumar K.V @ 2014-11-21  5:28 UTC (permalink / raw)
  To: Mel Gorman, Linux Kernel
  Cc: Rik van Riel, Linus Torvalds, Hugh Dickins, Linux-MM, Ingo Molnar,
	Paul Mackerras, Mel Gorman, Sasha Levin, Dave Jones, LinuxPPC-dev,
	Kirill Shutemov
In-Reply-To: <1416478790-27522-4-git-send-email-mgorman@suse.de>

Mel Gorman <mgorman@suse.de> writes:

> Convert existing users of pte_numa and friends to the new helper. Note
> that the kernel is broken after this patch is applied until the other
> page table modifiers are also altered. This patch layout is to make
> review easier.
>

.....

> diff --git a/arch/powerpc/mm/pgtable.c b/arch/powerpc/mm/pgtable.c
> index c90e602..b5d58d3 100644
> --- a/arch/powerpc/mm/pgtable.c
> +++ b/arch/powerpc/mm/pgtable.c
> @@ -173,7 +173,13 @@ void set_pte_at(struct mm_struct *mm, unsigned long addr, pte_t *ptep,
>  		pte_t pte)
>  {
>  #ifdef CONFIG_DEBUG_VM
> -	WARN_ON(pte_val(*ptep) & _PAGE_PRESENT);
> +	/*
> +	 * When handling numa faults, we already have the pte marked
> +	 * _PAGE_PRESENT, but we can be sure that it is not in hpte.
> +	 * Hence we can use set_pte_at for them.
> +	 */
> +	WARN_ON((pte_val(*ptep) & (_PAGE_PRESENT | _PAGE_USER)) ==
> +		(_PAGE_PRESENT | _PAGE_USER));
>  #endif


This can be VM_WARN_ON with #ifdef removed.

>  	/* Note: mm->context.id might not yet have been assigned as
>  	 * this context might not have been activated yet when this
> diff --git a/arch/powerpc/mm/pgtable_64.c b/arch/powerpc/mm/pgtable_64.c

-aneesh

^ permalink raw reply

* [PATCH] Powerpc/Oprofile:Disable pagefaults during user stack read
From: Jiang Lu @ 2014-11-21  3:23 UTC (permalink / raw)
  To: linuxppc-dev, lu.jiang

A page fault occurred during reading user stack in oprofile backtrace
would lead following calltrace:

WARNING: at linux/kernel/smp.c:210
Modules linked in:
CPU: 5 PID: 736 Comm: sh Tainted: G W 3.14.23-WR7.0.0.0_standard #1 
task: c0000000f6208bc0 ti: c00000007c72c000 task.ti: c00000007c72c000 
NIP: c0000000000ed6e4 LR: c0000000000ed5b8 CTR: 0000000000000000 
REGS: c00000007c72f050 TRAP: 0700 Tainted: G W (3.14.23-WR7.0.0 
tandard) 
MSR: 0000000080021000 <CE,ME> CR: 48222482 XER: 00000000 
SOFTE: 0 
GPR00: c0000000000ed5b8 c00000007c72f2d0 c0000000010aa048 0000000000000005 
GPR04: c000000000fdb820 c00000007c72f410 0000000000000001 0000000000000005 
GPR08: c0000000010b5768 c000000000f8a048 0000000000000001 0000000000000000 
GPR12: 0000000048222482 c00000000fffe580 0000000022222222 0000000010129664 
GPR16: 0000000010143cc0 0000000000000000 0000000044444444 0000000000000000 
GPR20: c00000007c7221d8 c0000000f638e3c8 000003f15a20120d 0000000000000001 
GPR24: 000000005a20120d c00000007c722000 c00000007cdedda8 00003fffef23b160 
GPR28: 0000000000000001 c00000007c72f410 c000000000fdb820 0000000000000006 
NIP [c0000000000ed6e4] .smp_call_function_single+0x18c/0x248 
LR [c0000000000ed5b8] .smp_call_function_single+0x60/0x248 
Call Trace: 
[c00000007c72f2d0] [c0000000000ed5b8] .smp_call_function_single+0x60/0x248 (unreliable) 
[c00000007c72f3a0] [c000000000030810] .__flush_tlb_page+0x164/0x1b0 
[c00000007c72f460] [c00000000002e054] .ptep_set_access_flags+0xb8/0x168 
[c00000007c72f500] [c0000000001ad3d8] .handle_mm_fault+0x4a8/0xbac 
[c00000007c72f5e0] [c000000000bb3238] .do_page_fault+0x3b8/0x868 
[c00000007c72f810] [c00000000001e1d0] storage_fault_common+0x20/0x44 
 Exception: 301 at .__copy_tofrom_user_base+0x54/0x5b0 
    LR = .op_powerpc_backtrace+0x190/0x20c 
[c00000007c72fb00] [c000000000a2ec34] .op_powerpc_backtrace+0x204/0x20c (unreliable) 
[c00000007c72fbc0] [c000000000a2b5fc] .oprofile_add_ext_sample+0xe8/0x118 
[c00000007c72fc70] [c000000000a2eee0] .fsl_emb_handle_interrupt+0x20c/0x27c 
[c00000007c72fd30] [c000000000a2e440] .op_handle_interrupt+0x44/0x58 
[c00000007c72fdb0] [c000000000016d68] .performance_monitor_exception+0x74/0x90 
[c00000007c72fe30] [c00000000001d8b4] exc_0x260_common+0xfc/0x100 

performance_monitor_exception() is executed in a context with interrupt
disabled and preemption enabled. When there is a user space page fault
happened, do_page_fault() invoke in_atomic() to decide whether kernel
should handle such page fault. in_atomic() only check preempt_count.
So need call pagefault_disable() to disable preemption before reading
user stack.

Signed-off-by: Jiang Lu <lu.jiang@windriver.com>
---
 arch/powerpc/oprofile/backtrace.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/oprofile/backtrace.c b/arch/powerpc/oprofile/backtrace.c
index f75301f..cae8ee7 100644
--- a/arch/powerpc/oprofile/backtrace.c
+++ b/arch/powerpc/oprofile/backtrace.c
@@ -10,7 +10,7 @@
 #include <linux/oprofile.h>
 #include <linux/sched.h>
 #include <asm/processor.h>
-#include <asm/uaccess.h>
+#include <linux/uaccess.h>
 #include <asm/compat.h>
 
 #define STACK_SP(STACK)		*(STACK)
@@ -104,6 +104,7 @@ void op_powerpc_backtrace(struct pt_regs * const regs, unsigned int depth)
 			first_frame = 0;
 		}
 	} else {
+		pagefault_disable();
 #ifdef CONFIG_PPC64
 		if (!is_32bit_task()) {
 			while (depth--) {
@@ -112,7 +113,7 @@ void op_powerpc_backtrace(struct pt_regs * const regs, unsigned int depth)
 					break;
 				first_frame = 0;
 			}
-
+			pagefault_enable();
 			return;
 		}
 #endif
@@ -123,5 +124,6 @@ void op_powerpc_backtrace(struct pt_regs * const regs, unsigned int depth)
 				break;
 			first_frame = 0;
 		}
+		pagefault_enable();
 	}
 }
-- 
1.9.1

^ permalink raw reply related

* Re: [RFC PATCH 00/16] Refine PCI host bridge scan interfaces
From: Yijing Wang @ 2014-11-21  2:58 UTC (permalink / raw)
  To: Liviu Dudau, Tomasz Nowicki
  Cc: Tony Luck, Russell King, Arnd Bergmann, linux-pci, x86,
	linux-kernel, Xinwei Hu, Thierry Reding, Suravee.Suthikulpanit,
	Bjorn Helgaas, linux-ia64, Thomas Gleixner, Wuyun, linuxppc-dev,
	linux-arm-kernel
In-Reply-To: <20141120163918.GE9162@bart.dudau.co.uk>

On 2014/11/21 0:39, Liviu Dudau wrote:
> On Thu, Nov 20, 2014 at 01:53:48PM +0100, Tomasz Nowicki wrote:
>> On 20.11.2014 13:08, Liviu Dudau wrote:
>>> On Thu, Nov 20, 2014 at 12:54:48PM +0100, Tomasz Nowicki wrote:
>>>> On 17.11.2014 15:13, Arnd Bergmann wrote:
>>>>> On Monday 17 November 2014 18:21:34 Yijing Wang wrote:
>>>>>> This series is based Linux 3.18-rc1 and Lorenzo Pieralisi's
>>>>>> arm PCI domain cleanup patches, link:
>>>>>> https://patchwork.ozlabs.org/patch/407585/
>>>>>>
>>>>>> Current pci scan interfaces like pci_scan_root_bus() and directly
>>>>>> call pci_create_root_bus()/pci_scan_child_bus() lack flexiblity.
>>>>>> Some platform infos like PCI domain and msi_chip have to be
>>>>>> associated to PCI bus by some arch specific function.
>>>>>> We want to make a generic pci_host_bridge, and make it hold
>>>>>> the platform infos or hook. Then we could eliminate the lots
>>>>>> of arch pci_domain_nr, also we could associate some platform
>>>>>> ops something like pci_get_msi_chip(struct pci_dev *dev)
>>>>>> with pci_host_bridge to avoid introduce arch weak functions.
>>>>>>
>>>>>> This RFC version not for all platforms, just applied the new
>>>>>> scan interface in x86/arm/powerpc/ia64, I will refresh other
>>>>>> platforms after the core pci scan interfaces are ok.
>>>>>
>>>>> I think overall this is a good direction to take, in particular
>>>>> moving more things into struct pci_host_bridge so we can
>>>>> slim down the architecture specific code.
>>>>>
>>>>> I don't particularly like the way you use the 'pci_host_info'
>>>>> to pass callback pointers and some of the generic information.
>>>>> This duplicates some of the issues we are currently trying
>>>>> to untangle in the arm32 code to make drivers easier to share
>>>>> between architectures.
>>>>>
>>>>> As a general approach, I'd rather see generic helper functions
>>>>> being exported by the PCI core that a driver may or may not
>>>>> call.
>>>>> The way you split the interface between things that happen
>>>>> before scanning the buses (pci_create_host_bridge) and
>>>>> the actual scanning (__pci_create_root_bus, pci_scan_child_bus)
>>>>> seems very helpful and I think we can expand that concept further:
>>>>>
>>>>> - The normal pci_create_host_bridge() function can contain
>>>>>   all of the DT scanning functions (finding bus/mem/io resources,
>>>>>   finding the msi-parent), while drivers that don't depend on DT
>>>>>   for this information can call the same function and fill the
>>>>>   same things after they have the pci_host_bridge pointer.
>>>>
>>>> How about finding PCI domain number (in the DT way) within
>>>> pci_create_host_bridge() too ?
>>>
>>> It is an idea worth pursuing for the 99% of the cases. I would like
>>> to understand the 1% of the time when we want a domain number to be
>>> shared between two host bridges or the time when we want more than
>>> one domain per bridge.
>> Even though we have shared domain, this should be resolved via DT calls, do
>> I miss something ?
> 
> If we only going to hold one domain number per host bridge, then no, you're
> not missing anything.
> 
>>
>>>
>>> Is that possible? Is it useful? Is it already in practice?
>> This is good question... IMO:
>> 1. Two host bridges can shared domain number if they are children of the
>> same parent host bridge.
>> 2. But I can not find good explanation for more than one domain per bridge.
> 
> Splitting a root bus into two or more "segments" ?

It seems impossible.

> 
> Best regards,
> Liviu
> 
>>
>> Tomasz
>>
>>
> 


-- 
Thanks!
Yijing

^ permalink raw reply

* Re: [RFC PATCH 07/16] PCI: Separate pci_host_bridge creation out of pci_create_root_bus()
From: Yijing Wang @ 2014-11-21  2:53 UTC (permalink / raw)
  To: Liviu Dudau
  Cc: Liviu Dudau, Tony Luck, Russell King, linux-pci@vger.kernel.org,
	x86@kernel.org, linux-kernel@vger.kernel.org, huxinwei@huawei.com,
	Thierry Reding, suravee.suthikulpanit@amd.com, Bjorn Helgaas,
	linux-ia64@vger.kernel.org, Thomas Gleixner, Wuyun,
	linuxppc-dev@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <20141120094741.GA17214@e106497-lin.cambridge.arm.com>

>>> pci_create_host_bridge() can get pci_host_bridge ops while pci_create_root_bus() gets
>>> the bus ops. For find out the MSI controller, the domain number and any other HB
>>> specific stuff, you use the HB ops. For config R/W acceses you use bus ops.
>>>
>>
>> I want to unexport pci_create_root_bus() if we have pci_create_host_bridge().
> 
> That's fine, but the function signature doesn't change, right? And I think we should keep
> pci_scan_root_bus() with the current signature as well (except change sysdata into a
> struct pci_host_bridge *). We create the host bridge in one step, stuff it with all the
> data that we need for scanning the root and associated busses, and then call pci_scan_root_bus().
> pci_host_bridge has the ops from pci_create_host_bridge() and the root bus from pci_scan_root_bus().

Liviu, could you have some draft patches about replace sysdata with pci_host_bridge, so we could
discuss the details base it. :)

> 
> Hope this makes sense.
> 
>>
>>
>>>>>> -struct pci_bus *pci_create_root_bus(struct device *parent, u32 db,
>>>>>> -               struct pci_ops *ops, void *sysdata, struct list_head *resources)
>>>>>> +struct pci_bus *__pci_create_root_bus(struct pci_host_bridge *bridge)
>>>>>>  {
>>>>>>         int error;
>>>>>> -       struct pci_host_bridge *bridge;
>>>>>>         struct pci_bus *b, *b2;
>>>>>> -       struct pci_host_bridge_window *window, *n;
>>>>>> +       struct pci_host_bridge_window *window;
>>>>>>         struct resource *res;
>>>>>>         resource_size_t offset;
>>>>>>         char bus_addr[64];
>>>>>>         char *fmt;
>>>>>> -       u8      bus = PCI_BUSNUM(db);
>>>>>> +       struct device *parent = bridge->dev.parent;
>>>>>>
>>>>>>         b = pci_alloc_bus(NULL);
>>>>>>         if (!b)
>>>>>>                 return NULL;
>>>>>>
>>>>>> -       b->sysdata = sysdata;
>>>>>> -       b->ops = ops;
>>>>>> -       b->number = b->busn_res.start = bus;
>>>>>> +       b->sysdata = bridge->sysdata;
>>>>>
>>>>> I think bridge should be the b->sysdata here.
>>>>
>>>> ? what's the meaning?
>>>
>>> Currently, bus->sysdata holds a pointer to the arch/driver host bridge structure, as passed
>>> in pci_create_root_bus(). If you agree with my idea of wrapping the host bridge driver
>>> structure around the pci_host_bridge, then we will always have a way of retrieving that
>>> information; but for backwards compatibility we could set bus->sysdata to be the bridge.
>>> Then existing macros that convert sysdata to pci_controller can be reused after being
>>> updated.
>>
>> I think wrapping the host bridge driver structure around the pci_host_bridge could help
>> us to find the private pci_controller, but in this case, pci_host_bridge is not a pointer
>> so we put all pci_host_bridge init stuff in host drivers ? 
> 
> Of course we put pci_host_bridge init *calls* in the host drivers because we need to do host
> driver specific initialisation there anyway. 
> 
>> And if we still use arch spec
>> macros convert ssydata to pci_controller, so what's the greatest advantage ?
> 
> Agree, the macro is not useful unless we have a common pci_controller structure. Sorry about
> the noise.
> 
>>
>> If we move domain and msi_chip out of sysdata, what's other things in sysdata still need us
>> to convert in PCI core ?
> 
> I've never said we need to keep sysdata. From my discussion with Bjorn a year ago the idea was
> to put the sysdata members into pci_host_bridge structure and remove sysdata member from pci_bus.
> pci_scan_root_bus() can still get a parameter called sysdata if we want, but it will be a
> struct pci_host_bridge * type.

Fine.

> 
> Best regards,
> Liviu
> 
>>
>>
>>
>>>
>>> Best regards,
>>> Liviu
>>>
>>>>
>>>>>
>>>>>> +       b->ops = bridge->ops;
>>>>>
>>>>> See comment above why I don't think this is necessary.
>>>>>
>>>>>> +       b->number = b->busn_res.start = bridge->busnum;
>>>>>>         pci_bus_assign_domain_nr(b, parent);
>>>>>> -       b2 = pci_find_bus(pci_domain_nr(b), bus);
>>>>>> +       bridge->domain = pci_domain_nr(b);
>>>>>
>>>>> Do you really want to overwrite the bridge's domain with the one from a bus that
>>>>> could possibly be rejected a couple of lines further down?
>>>>>
>>>>> As an asside: if we are doing the split of pci_host_bridge from root bus creation
>>>>> it is worth in my opinion to move the domain setup in pci_create_host_bridge()
>>>>> and stop fiddling with it here.
>>>>
>>>>
>>>> Hi Liviu, these lines just temporary, I will remove it after all host drivers
>>>> save its domain in pci_host_bridge.
>>>>
>>>>
>>>>>
>>>>> Otherwise it looks to me like you are heading in the right direction.
>>>>
>>>> Thanks!
>>>> Yijing.
>>>>
>>>>>
>>>>> Best regards,
>>>>> Liviu
>>>>>
>>>>>> +       b2 = pci_find_bus(pci_domain_nr(b), bridge->busnum);
>>>>>>         if (b2) {
>>>>>>                 /* If we already got to this bus through a different bridge, ignore it */
>>>>>>                 dev_dbg(&b2->dev, "bus already known\n");
>>>>>>                 goto err_out;
>>>>>>         }
>>>>>>
>>>>>> -       bridge = pci_alloc_host_bridge(b);
>>>>>> -       if (!bridge)
>>>>>> -               goto err_out;
>>>>>> -
>>>>>> -       bridge->dev.parent = parent;
>>>>>> -       bridge->dev.release = pci_release_host_bridge_dev;
>>>>>> -       dev_set_name(&bridge->dev, "pci%04x:%02x", pci_domain_nr(b), bus);
>>>>>> -       error = pcibios_root_bridge_prepare(bridge);
>>>>>> -       if (error) {
>>>>>> -               kfree(bridge);
>>>>>> -               goto err_out;
>>>>>> -       }
>>>>>> -
>>>>>> -       error = device_register(&bridge->dev);
>>>>>> -       if (error) {
>>>>>> -               put_device(&bridge->dev);
>>>>>> -               goto err_out;
>>>>>> -       }
>>>>>>         b->bridge = get_device(&bridge->dev);
>>>>>>         device_enable_async_suspend(b->bridge);
>>>>>>         pci_set_bus_of_node(b);
>>>>>> @@ -1950,7 +1899,7 @@ struct pci_bus *pci_create_root_bus(struct device *parent, u32 db,
>>>>>>
>>>>>>         b->dev.class = &pcibus_class;
>>>>>>         b->dev.parent = b->bridge;
>>>>>> -       dev_set_name(&b->dev, "%04x:%02x", pci_domain_nr(b), bus);
>>>>>> +       dev_set_name(&b->dev, "%04x:%02x", pci_domain_nr(b), b->number);
>>>>>>         error = device_register(&b->dev);
>>>>>>         if (error)
>>>>>>                 goto class_dev_reg_err;
>>>>>> @@ -1966,12 +1915,11 @@ struct pci_bus *pci_create_root_bus(struct device *parent, u32 db,
>>>>>>                 printk(KERN_INFO "PCI host bridge to bus %s\n", dev_name(&b->dev));
>>>>>>
>>>>>>         /* Add initial resources to the bus */
>>>>>> -       list_for_each_entry_safe(window, n, resources, list) {
>>>>>> -               list_move_tail(&window->list, &bridge->windows);
>>>>>> +       list_for_each_entry(window, &bridge->windows, list) {
>>>>>>                 res = window->res;
>>>>>>                 offset = window->offset;
>>>>>>                 if (res->flags & IORESOURCE_BUS)
>>>>>> -                       pci_bus_insert_busn_res(b, bus, res->end);
>>>>>> +                       pci_bus_insert_busn_res(b, b->number, res->end);
>>>>>>                 else
>>>>>>                         pci_bus_add_resource(b, res, 0);
>>>>>>                 if (offset) {
>>>>>> @@ -2001,6 +1949,25 @@ err_out:
>>>>>>         return NULL;
>>>>>>  }
>>>>>>
>>>>>> +struct pci_bus *pci_create_root_bus(struct device *parent, u32 bus,
>>>>>> +               struct pci_ops *ops, void *sysdata, struct list_head *resources)
>>>>>> +{
>>>>>> +       struct pci_host_bridge *host;
>>>>>> +
>>>>>> +       host = pci_create_host_bridge(parent, bus, ops,
>>>>>> +                       sysdata ,resources);
>>>>>> +       if (!host)
>>>>>> +               return NULL;
>>>>>> +
>>>>>> +       host->bus = __pci_create_root_bus(host);
>>>>>> +       if (!host->bus) {
>>>>>> +               pci_free_host_bridge(host);
>>>>>> +               return NULL;
>>>>>> +       }
>>>>>> +
>>>>>> +       return host->bus;
>>>>>> +}
>>>>>> +
>>>>>>  int pci_bus_insert_busn_res(struct pci_bus *b, int bus, int bus_max)
>>>>>>  {
>>>>>>         struct resource *res = &b->busn_res;
>>>>>> @@ -2069,40 +2036,37 @@ struct pci_bus *pci_scan_root_bus(struct device *parent, u32 db,
>>>>>>  {
>>>>>>         struct pci_host_bridge_window *window;
>>>>>>         bool found = false;
>>>>>> -       struct pci_bus *b;
>>>>>> -       LIST_HEAD(default_res);
>>>>>> +       struct pci_host_bridge *host;
>>>>>>         int max;
>>>>>>
>>>>>> -       if (!resources) {
>>>>>> -               pci_add_resource(&default_res, &ioport_resource);
>>>>>> -               pci_add_resource(&default_res, &iomem_resource);
>>>>>> -               pci_add_resource(&default_res, &busn_resource);
>>>>>> -       } else {
>>>>>> -               list_for_each_entry(window, resources, list)
>>>>>> -                       if (window->res->flags & IORESOURCE_BUS) {
>>>>>> -                               found = true;
>>>>>> -                               break;
>>>>>> -                       }
>>>>>> -       }
>>>>>> +       host = pci_create_host_bridge(parent, db, ops, sysdata, resources);
>>>>>> +       if (!host)
>>>>>> +               return NULL;
>>>>>>
>>>>>> -       b = pci_create_root_bus(parent, db, ops, sysdata,
>>>>>> -                       resources ? resources : &default_res);
>>>>>> -       if (!b)
>>>>>> +       list_for_each_entry(window, &host->windows, list)
>>>>>> +               if (window->res->flags & IORESOURCE_BUS) {
>>>>>> +                       found = true;
>>>>>> +                       break;
>>>>>> +               }
>>>>>> +
>>>>>> +       host->bus = __pci_create_root_bus(host);
>>>>>> +       if (!host->bus) {
>>>>>> +               pci_free_host_bridge(host);
>>>>>>                 return NULL;
>>>>>> +       }
>>>>>>
>>>>>>         if (!found) {
>>>>>> -               dev_info(&b->dev,
>>>>>> +               dev_info(&host->bus->dev,
>>>>>>                  "No busn resource found for root bus, will use [bus %02x-ff]\n",
>>>>>>                         PCI_BUSNUM(db));
>>>>>> -               pci_bus_insert_busn_res(b, PCI_BUSNUM(db), 255);
>>>>>> +               pci_bus_insert_busn_res(host->bus, PCI_BUSNUM(db), 255);
>>>>>>         }
>>>>>>
>>>>>> -       max = pci_scan_child_bus(b);
>>>>>> -
>>>>>> +       max = pci_scan_child_bus(host->bus);
>>>>>>         if (!found)
>>>>>> -               pci_bus_update_busn_res_end(b, max);
>>>>>> +               pci_bus_update_busn_res_end(host->bus, max);
>>>>>>
>>>>>> -       return b;
>>>>>> +       return host->bus;
>>>>>>  }
>>>>>>  EXPORT_SYMBOL(pci_scan_root_bus);
>>>>>>
>>>>>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>>>>>> index 8b11b38..daa7f40 100644
>>>>>> --- a/include/linux/pci.h
>>>>>> +++ b/include/linux/pci.h
>>>>>> @@ -402,7 +402,12 @@ struct pci_host_bridge_window {
>>>>>>  struct pci_host_bridge {
>>>>>>         struct device dev;
>>>>>>         struct pci_bus *bus;            /* root bus */
>>>>>> +       struct list_head list;
>>>>>>         struct list_head windows;       /* pci_host_bridge_windows */
>>>>>> +       int busnum;
>>>>>> +       int domain;
>>>>>> +       void *sysdata;
>>>>>> +       struct pci_ops *ops;
>>>>>>         void (*release_fn)(struct pci_host_bridge *);
>>>>>>         void *release_data;
>>>>>>  };
>>>>>> @@ -413,7 +418,9 @@ void pci_set_host_bridge_release(struct pci_host_bridge *bridge,
>>>>>>                      void *release_data);
>>>>>>
>>>>>>  int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge);
>>>>>> -
>>>>>> +struct pci_host_bridge *pci_create_host_bridge(
>>>>>> +               struct device *parent, u32 db, struct pci_ops *ops,
>>>>>> +               void *sys, struct list_head *resources);
>>>>>>  /*
>>>>>>   * The first PCI_BRIDGE_RESOURCE_NUM PCI bus resources (those that correspond
>>>>>>   * to P2P or CardBus bridge windows) go in a table.  Additional ones (for
>>>>>> @@ -770,6 +777,8 @@ void pci_bus_add_devices(const struct pci_bus *bus);
>>>>>>  struct pci_bus *pci_create_root_bus(struct device *parent, u32 bus,
>>>>>>                                     struct pci_ops *ops, void *sysdata,
>>>>>>                                     struct list_head *resources);
>>>>>> +struct pci_bus *__pci_create_root_bus(struct pci_host_bridge *host);
>>>>>> +void pci_free_host_bridge(struct pci_host_bridge *host);
>>>>>>  int pci_bus_insert_busn_res(struct pci_bus *b, int bus, int busmax);
>>>>>>  int pci_bus_update_busn_res_end(struct pci_bus *b, int busmax);
>>>>>>  void pci_bus_release_busn_res(struct pci_bus *b);
>>>>>> --
>>>>>> 1.7.1
>>>>>>
>>>>>> --
>>>>>> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
>>>>>> the body of a message to majordomo@vger.kernel.org
>>>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>>>>
>>>>>
>>>>
>>>>
>>>> --
>>>> Thanks!
>>>> Yijing
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>>
>>>
>>
>>
>> --
>> Thanks!
>> Yijing
>>
>>
> 


-- 
Thanks!
Yijing

^ permalink raw reply

* Re: [PATCH V9 08/18] powrepc/pci: Refactor pci_dn
From: Wei Yang @ 2014-11-21  1:46 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: linux-pci, Wei Yang, benh, linuxppc-dev, gwshan
In-Reply-To: <20141120190541.GA5110@google.com>

On Thu, Nov 20, 2014 at 12:05:41PM -0700, Bjorn Helgaas wrote:
>On Thu, Nov 20, 2014 at 03:20:57PM +0800, Wei Yang wrote:
>> On Wed, Nov 19, 2014 at 04:30:24PM -0700, Bjorn Helgaas wrote:
>> >On Sun, Nov 02, 2014 at 11:41:24PM +0800, Wei Yang wrote:
>> >> From: Gavin Shan <gwshan@linux.vnet.ibm.com>
>> >> 
>> >> pci_dn is the extension of PCI device node and it's created from
>> >> device node. Unfortunately, VFs that are enabled dynamically by
>> >> PF's driver and they don't have corresponding device nodes, and
>> >> pci_dn. The patch refactors pci_dn to support VFs:
>> >> 
>> >>    * pci_dn is organized as a hierarchy tree. VF's pci_dn is put
>> >>      to the child list of pci_dn of PF's bridge. pci_dn of other
>> >>      device put to the child list of pci_dn of its upstream bridge.
>> >> 
>> >>    * VF's pci_dn is expected to be created dynamically when applying
>> >>      final fixup to PF. VF's pci_dn will be destroyed when releasing
>> >>      PF's pci_dev instance. pci_dn of other device is still created
>> >>      from device node as before.
>> >> 
>> >>    * For one particular PCI device (VF or not), its pci_dn can be
>> >>      found from pdev->dev.archdata.firmware_data, PCI_DN(devnode),
>> >>      or parent's list. The fast path (fetching pci_dn through PCI
>> >>      device instance) is populated during early fixup time.
>> >> 
>> >> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>> >> ---
>> >> ...
>> >
>> >> +struct pci_dn *add_dev_pci_info(struct pci_dev *pdev)
>> >> +{
>> >> +#ifdef CONFIG_PCI_IOV
>> >> +	struct pci_dn *parent, *pdn;
>> >> +	int i;
>> >> +
>> >> +	/* Only support IOV for now */
>> >> +	if (!pdev->is_physfn)
>> >> +		return pci_get_pdn(pdev);
>> >> +
>> >> +	/* Check if VFs have been populated */
>> >> +	pdn = pci_get_pdn(pdev);
>> >> +	if (!pdn || (pdn->flags & PCI_DN_FLAG_IOV_VF))
>> >> +		return NULL;
>> >> +
>> >> +	pdn->flags |= PCI_DN_FLAG_IOV_VF;
>> >> +	parent = pci_bus_to_pdn(pdev->bus);
>> >> +	if (!parent)
>> >> +		return NULL;
>> >> +
>> >> +	for (i = 0; i < pci_sriov_get_totalvfs(pdev); i++) {
>> >> +		pdn = add_one_dev_pci_info(parent, NULL,
>> >> +					   pci_iov_virtfn_bus(pdev, i),
>> >> +					   pci_iov_virtfn_devfn(pdev, i));
>> >
>> >I'm not sure this makes sense, but I certainly don't know this code, so
>> >maybe I'm missing something.
>> >
>> >pci_iov_virtfn_bus() and pci_iov_virtfn_devfn() depend on
>> >pdev->sriov->stride and pdev->sriov->offset.  These are read from VF Stride
>> >and First VF Offset in the SR-IOV capability by sriov_init(), which is
>> >called before add_dev_pci_info():
>> >
>> >  pci_scan_child_bus
>> >    pci_scan_slot
>> >      pci_scan_single_device
>> >	pci_device_add
>> >	  pci_init_capabilities
>> >	    pci_iov_init(PF)
>> >	      sriov_init(PF, pos)
>> >		pci_write_config_word(dev, pos + PCI_SRIOV_NUM_VF, 0)
>> >		pci_read_config_word(dev, pos + PCI_SRIOV_VF_OFFSET, &offset)
>> >		pci_read_config_word(dev, pos + PCI_SRIOV_VF_STRIDE, &stride)
>> >		iov->offset = offset
>> >		iov->stride = stride
>> >
>> >  pci_bus_add_devices
>> >    pci_bus_add_device
>> >      pci_fixup_device(pci_fixup_final)
>> >	add_dev_pci_info
>> >	  pci_iov_virtfn_bus
>> >	    return ... + sriov->offset + (sriov->stride * id) ...
>> >
>> >But both First VF Offset and VF Stride change when ARI Capable Hierarchy or
>> >NumVFs changes (SR-IOV spec sec 3.3.9, 3.3.10).  We set NumVFs to zero in
>> >sriov_init() above.  We will change NumVFs to something different when a
>> >driver calls pci_enable_sriov():
>> >
>> >  pci_enable_sriov
>> >    sriov_enable
>> >      pci_write_config_word(dev, iov->pos + PCI_SRIOV_NUM_VF, nr_virtfn)
>> >
>> >Now First VF Offset and VF Stride have changed from what they were when we
>> >called pci_iov_virtfn_bus() above.
>> >
>> 
>> Oops, I see the ARI would affect those value, while missed the NumVFs also
>> would.
>> 
>> Let's look at the problem one by one.
>> 
>> 1. The ARI capability.
>> ===============================================================================
>> The kernel initialize the capability like this:
>> 
>> pci_init_capabilities()
>> 	pci_configure_ari()
>> 	pci_iov_init()
>> 		iov->offset = offset
>> 		iov->stride = stride
>> 
>> When offset/stride is retrieved at this point, the ARI capability is taken
>> into consideration.
>
>PCI_SRIOV_CTRL_ARI is currently only changed at the time we enumerate the
>PF, so I don't think this is really a problem.
>
>> 2. The PF's NumVFs field
>> ===============================================================================
>> 2.1 Potential problem in current code
>> ===============================================================================
>> First, is current pci code has some potential problem?
>> 
>> sriov_enable()
>> 	pci_read_config_word(dev, iov->pos + PCI_SRIOV_VF_OFFSET, &offset);
>> 	pci_read_config_word(dev, iov->pos + PCI_SRIOV_VF_STRIDE, &stride);
>> 	iov->offset = offset;
>> 	iov->stride = stride;
>> 	pci_write_config_word(dev, iov->pos + PCI_SRIOV_NUM_VF, nr_virtfn);
>> 	virtfn_add()
>> 		...
>> 		virtfn->devfn = pci_iov_virtfn_devfn(dev, id);
>> 
>> The sriov_enable() retrieve the offset/stride then write the NumVFs. According
>> to the SPEC, at this moment the offset/stride may change. While I don't see
>> some code to retrieve and store those value again. And these fields will be
>> used in virtfn_add().
>> 
>> If my understanding is correct, I suggest to move the retrieve and store
>> operation after NumVFs is written.
>
>Yep, it looks like the existing code has similar problems.  We might want
>to add a simple function that writes PCI_SRIOV_NUM_VF, then reads
>PCI_SRIOV_VF_OFFSET and PCI_SRIOV_VF_STRIDE and refreshes the cached values
>in dev->sriov.
>
>Then we'd at least know that virtfn_bus() and virtfn_devfn() return values
>that are correct until the next NumVFs change.
>

Ok, I will write a function to wrap it.

>> 2.2 The IOV bus range may not be correct in pci_scan_child_bus()?
>> ===============================================================================
>> In current pci core, when enumerating the pci tree, we do like this:
>> 
>> pci_scan_child_bus()
>> 	pci_scan_slot()
>> 		pci_scan_single_device()
>> 			pci_device_add()
>> 				pci_init_capabilities()
>> 					pci_iov_init()
>> 	max += pci_iov_bus_range(bus);
>> 		busnr = pci_iov_virtfn_bus(dev, dev->sriov->total_VFs - 1);
>> 	max = pci_scan_bridge(bus, dev, max, pass);
>> 
>> From this point, we see pci core reserve some bus range for VFs. This
>> calculation is based on the offset/stride at this moment. And do the
>> enumeration with the new bus number.
>> 
>> sriov_enable() could be called several times from driver to enable SRIOV, and
>> with different nr_virtfn. If each time the NumVFs written, the offset/stride
>> will change. This means we may try to address an extra bus we didn't reserve?
>> Or this means it is out of control?
>
>This looks like a problem, too.  I don't have a good suggestion for fixing
>it.

How about enumerating all possible NumVFs and select the maximum?

I am not sure what will happen if the FW sets a different number? So FW should
listen to kernel, right?

I could write a code with this logic and test, while I am afraid this will
break some platfrom in some case.

>
>> 2.3 How can I reserve bus range in FW?
>> ===============================================================================
>> This question comes from the previous one.
>> 
>> Based on my understanding, current pci core will rely on the bus number in HW
>> if pcibios_assign_all_busses() is not set. If we want to support those VFs
>> sits on different bus with PF, we need to reserve bus range and write the
>> correct secondary/subordinate in bridge. Otherwise, those VFs on different bus
>> may not be addressed.
>> 
>> Currently I am writing the code in FW to reserve the range with the same
>> mechanism in pci core. While as you mentioned the offset/stride may change
>> after sriov_enable(), I am confused whether this is the correct way.
>
>If your firmware knows something about the device and can compute the
>number of buses it will likely need, it can set up bridges with appropriate
>bus number ranges, and Linux will generally leave those alone.
>

Yep, this is what I am trying to do.

>I don't know the best way to figure out the number of buses, though.  It
>seems like you almost need to experimentally set NumVFs and read the
>resulting offset/stride, because I think it's really up to the device to
>decide how to number the VFs.  Maybe pci_iov_bus_range() needs to do
>something similar.

Got it, I will add this logic.

>
>> 2.4 The potential problem for [Patch 08/18]
>> ===============================================================================
>> According to the SPEC, the offset/stride will change after each
>> sriov_enable(). This means the bus/devfn will change after each
>> sriov_enable().
>> 
>> My current thought is to fix it up in virtfn_add(). If the total VF number
>> will not change, we could create those pci_dn at the beginning and fix the
>> bus/devfn at each time the VF is truely created.
>
>By "fix it up," I assume you mean call an arch function that does the
>pci_pdn setup you need.
>
>It sounds reasonable to do this either in virtfn_add()/virtfn_remove() or
>at the points where we write PCI_SRIOV_CTRL_VFE, i.e., in sriov_init(),
>sriov_enable(), sriov_disable(), and sriov_restore_state().  From a
>hardware point of view, the VFs exist whenever PCI_SRIOV_CTRL_VFE is set,
>so it might be nice to have this setup connected to that.

If my understanding is correct, we could wrap up the configuration write/read
on PCI_SRIOV_CTRL and when it involves PCI_SRIOV_CTRL_VFE, do the fix up?

>
>Bjorn

-- 
Richard Yang
Help you, Help me

^ permalink raw reply

* Re: [PATCH V9 08/18] powrepc/pci: Refactor pci_dn
From: Gavin Shan @ 2014-11-21  0:04 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: linux-pci, Wei Yang, benh, linuxppc-dev, gwshan
In-Reply-To: <20141120190541.GA5110@google.com>

On Thu, Nov 20, 2014 at 12:05:41PM -0700, Bjorn Helgaas wrote:
>On Thu, Nov 20, 2014 at 03:20:57PM +0800, Wei Yang wrote:
>> On Wed, Nov 19, 2014 at 04:30:24PM -0700, Bjorn Helgaas wrote:
>> >On Sun, Nov 02, 2014 at 11:41:24PM +0800, Wei Yang wrote:
>> >> From: Gavin Shan <gwshan@linux.vnet.ibm.com>
>> >> 
>> >> pci_dn is the extension of PCI device node and it's created from
>> >> device node. Unfortunately, VFs that are enabled dynamically by
>> >> PF's driver and they don't have corresponding device nodes, and
>> >> pci_dn. The patch refactors pci_dn to support VFs:
>> >> 
>> >>    * pci_dn is organized as a hierarchy tree. VF's pci_dn is put
>> >>      to the child list of pci_dn of PF's bridge. pci_dn of other
>> >>      device put to the child list of pci_dn of its upstream bridge.
>> >> 
>> >>    * VF's pci_dn is expected to be created dynamically when applying
>> >>      final fixup to PF. VF's pci_dn will be destroyed when releasing
>> >>      PF's pci_dev instance. pci_dn of other device is still created
>> >>      from device node as before.
>> >> 
>> >>    * For one particular PCI device (VF or not), its pci_dn can be
>> >>      found from pdev->dev.archdata.firmware_data, PCI_DN(devnode),
>> >>      or parent's list. The fast path (fetching pci_dn through PCI
>> >>      device instance) is populated during early fixup time.
>> >> 
>> >> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>> >> ---
>> >> ...
>> >
>> >> +struct pci_dn *add_dev_pci_info(struct pci_dev *pdev)
>> >> +{
>> >> +#ifdef CONFIG_PCI_IOV
>> >> +	struct pci_dn *parent, *pdn;
>> >> +	int i;
>> >> +
>> >> +	/* Only support IOV for now */
>> >> +	if (!pdev->is_physfn)
>> >> +		return pci_get_pdn(pdev);
>> >> +
>> >> +	/* Check if VFs have been populated */
>> >> +	pdn = pci_get_pdn(pdev);
>> >> +	if (!pdn || (pdn->flags & PCI_DN_FLAG_IOV_VF))
>> >> +		return NULL;
>> >> +
>> >> +	pdn->flags |= PCI_DN_FLAG_IOV_VF;
>> >> +	parent = pci_bus_to_pdn(pdev->bus);
>> >> +	if (!parent)
>> >> +		return NULL;
>> >> +
>> >> +	for (i = 0; i < pci_sriov_get_totalvfs(pdev); i++) {
>> >> +		pdn = add_one_dev_pci_info(parent, NULL,
>> >> +					   pci_iov_virtfn_bus(pdev, i),
>> >> +					   pci_iov_virtfn_devfn(pdev, i));
>> >
>> >I'm not sure this makes sense, but I certainly don't know this code, so
>> >maybe I'm missing something.
>> >
>> >pci_iov_virtfn_bus() and pci_iov_virtfn_devfn() depend on
>> >pdev->sriov->stride and pdev->sriov->offset.  These are read from VF Stride
>> >and First VF Offset in the SR-IOV capability by sriov_init(), which is
>> >called before add_dev_pci_info():
>> >
>> >  pci_scan_child_bus
>> >    pci_scan_slot
>> >      pci_scan_single_device
>> >	pci_device_add
>> >	  pci_init_capabilities
>> >	    pci_iov_init(PF)
>> >	      sriov_init(PF, pos)
>> >		pci_write_config_word(dev, pos + PCI_SRIOV_NUM_VF, 0)
>> >		pci_read_config_word(dev, pos + PCI_SRIOV_VF_OFFSET, &offset)
>> >		pci_read_config_word(dev, pos + PCI_SRIOV_VF_STRIDE, &stride)
>> >		iov->offset = offset
>> >		iov->stride = stride
>> >
>> >  pci_bus_add_devices
>> >    pci_bus_add_device
>> >      pci_fixup_device(pci_fixup_final)
>> >	add_dev_pci_info
>> >	  pci_iov_virtfn_bus
>> >	    return ... + sriov->offset + (sriov->stride * id) ...
>> >
>> >But both First VF Offset and VF Stride change when ARI Capable Hierarchy or
>> >NumVFs changes (SR-IOV spec sec 3.3.9, 3.3.10).  We set NumVFs to zero in
>> >sriov_init() above.  We will change NumVFs to something different when a
>> >driver calls pci_enable_sriov():
>> >
>> >  pci_enable_sriov
>> >    sriov_enable
>> >      pci_write_config_word(dev, iov->pos + PCI_SRIOV_NUM_VF, nr_virtfn)
>> >
>> >Now First VF Offset and VF Stride have changed from what they were when we
>> >called pci_iov_virtfn_bus() above.
>> >
>> 
>> Oops, I see the ARI would affect those value, while missed the NumVFs also
>> would.
>> 
>> Let's look at the problem one by one.
>> 
>> 1. The ARI capability.
>> ===============================================================================
>> The kernel initialize the capability like this:
>> 
>> pci_init_capabilities()
>> 	pci_configure_ari()
>> 	pci_iov_init()
>> 		iov->offset = offset
>> 		iov->stride = stride
>> 
>> When offset/stride is retrieved at this point, the ARI capability is taken
>> into consideration.
>
>PCI_SRIOV_CTRL_ARI is currently only changed at the time we enumerate the
>PF, so I don't think this is really a problem.
>
>> 2. The PF's NumVFs field
>> ===============================================================================
>> 2.1 Potential problem in current code
>> ===============================================================================
>> First, is current pci code has some potential problem?
>> 
>> sriov_enable()
>> 	pci_read_config_word(dev, iov->pos + PCI_SRIOV_VF_OFFSET, &offset);
>> 	pci_read_config_word(dev, iov->pos + PCI_SRIOV_VF_STRIDE, &stride);
>> 	iov->offset = offset;
>> 	iov->stride = stride;
>> 	pci_write_config_word(dev, iov->pos + PCI_SRIOV_NUM_VF, nr_virtfn);
>> 	virtfn_add()
>> 		...
>> 		virtfn->devfn = pci_iov_virtfn_devfn(dev, id);
>> 
>> The sriov_enable() retrieve the offset/stride then write the NumVFs. According
>> to the SPEC, at this moment the offset/stride may change. While I don't see
>> some code to retrieve and store those value again. And these fields will be
>> used in virtfn_add().
>> 
>> If my understanding is correct, I suggest to move the retrieve and store
>> operation after NumVFs is written.
>
>Yep, it looks like the existing code has similar problems.  We might want
>to add a simple function that writes PCI_SRIOV_NUM_VF, then reads
>PCI_SRIOV_VF_OFFSET and PCI_SRIOV_VF_STRIDE and refreshes the cached values
>in dev->sriov.
>
>Then we'd at least know that virtfn_bus() and virtfn_devfn() return values
>that are correct until the next NumVFs change.
>
>> 2.2 The IOV bus range may not be correct in pci_scan_child_bus()?
>> ===============================================================================
>> In current pci core, when enumerating the pci tree, we do like this:
>> 
>> pci_scan_child_bus()
>> 	pci_scan_slot()
>> 		pci_scan_single_device()
>> 			pci_device_add()
>> 				pci_init_capabilities()
>> 					pci_iov_init()
>> 	max += pci_iov_bus_range(bus);
>> 		busnr = pci_iov_virtfn_bus(dev, dev->sriov->total_VFs - 1);
>> 	max = pci_scan_bridge(bus, dev, max, pass);
>> 
>> From this point, we see pci core reserve some bus range for VFs. This
>> calculation is based on the offset/stride at this moment. And do the
>> enumeration with the new bus number.
>> 
>> sriov_enable() could be called several times from driver to enable SRIOV, and
>> with different nr_virtfn. If each time the NumVFs written, the offset/stride
>> will change. This means we may try to address an extra bus we didn't reserve?
>> Or this means it is out of control?
>
>This looks like a problem, too.  I don't have a good suggestion for fixing
>it.
>
>> 2.3 How can I reserve bus range in FW?
>> ===============================================================================
>> This question comes from the previous one.
>> 
>> Based on my understanding, current pci core will rely on the bus number in HW
>> if pcibios_assign_all_busses() is not set. If we want to support those VFs
>> sits on different bus with PF, we need to reserve bus range and write the
>> correct secondary/subordinate in bridge. Otherwise, those VFs on different bus
>> may not be addressed.
>> 
>> Currently I am writing the code in FW to reserve the range with the same
>> mechanism in pci core. While as you mentioned the offset/stride may change
>> after sriov_enable(), I am confused whether this is the correct way.
>
>If your firmware knows something about the device and can compute the
>number of buses it will likely need, it can set up bridges with appropriate
>bus number ranges, and Linux will generally leave those alone.
>
>I don't know the best way to figure out the number of buses, though.  It
>seems like you almost need to experimentally set NumVFs and read the
>resulting offset/stride, because I think it's really up to the device to
>decide how to number the VFs.  Maybe pci_iov_bus_range() needs to do
>something similar.
>

Yep, it's the reasonable way to probe maximal number of buses for the
upstream bridge of PF. I guess Richard need implement similar thing in
firmware.

>> 2.4 The potential problem for [Patch 08/18]
>> ===============================================================================
>> According to the SPEC, the offset/stride will change after each
>> sriov_enable(). This means the bus/devfn will change after each
>> sriov_enable().
>> 
>> My current thought is to fix it up in virtfn_add(). If the total VF number
>> will not change, we could create those pci_dn at the beginning and fix the
>> bus/devfn at each time the VF is truely created.
>
>By "fix it up," I assume you mean call an arch function that does the
>pci_pdn setup you need.
>
>It sounds reasonable to do this either in virtfn_add()/virtfn_remove() or
>at the points where we write PCI_SRIOV_CTRL_VFE, i.e., in sriov_init(),
>sriov_enable(), sriov_disable(), and sriov_restore_state().  From a
>hardware point of view, the VFs exist whenever PCI_SRIOV_CTRL_VFE is set,
>so it might be nice to have this setup connected to that.
>

Yes, Both ways can fix the issue. For couple reasons, I want add weak
pcibios_virtfn_add(), which is called in virtfn_add() if you agree.

- PCI_SRIOV_CTRL_VFE might be set somewhere except the functions you pointed.
  Set/clear PCI_SRIOV_CTRL_VFE will invoke background work to check pci_dn
  and add/remove accordingly. It would be overhead which we can avoid.
- We plan to support EEH for VFs in future. virtfn_add() way matches with
  current EEH implementation better. EEH device and PE are created based
  on (struct pci_dev), and EEH devices and PE can be destroied in time in
  pcibios_release_device(), which is invoked by virtfn_remove(). So we only
  need one weak function. In contrast, we have to create EEH device and PE
  for VFs a bit early before any VFs are instantiated, and destroy them a
  bit late after all VFs are offline.

Thanks,
Gavin

>Bjorn
>

^ permalink raw reply

* Re: [PATCH 0/10] Replace _PAGE_NUMA with PAGE_NONE protections v2
From: Sasha Levin @ 2014-11-20 21:50 UTC (permalink / raw)
  To: Mel Gorman, Linux Kernel
  Cc: Rik van Riel, Linus Torvalds, Hugh Dickins, Linux-MM, Ingo Molnar,
	Paul Mackerras, Aneesh Kumar, Dave Jones, LinuxPPC-dev,
	Kirill Shutemov
In-Reply-To: <1416478790-27522-1-git-send-email-mgorman@suse.de>

On 11/20/2014 05:19 AM, Mel Gorman wrote:
> V1 failed while running under kvm-tools very quickly and a second report
> indicated that it happens on bare metal as well. This version survived
> an overnight run of trinity running under kvm-tools here but verification
> from Sasha would be appreciated.

Hi Mel,

I tried giving it a spin, but it won't apply at all on the latest -mm
tree:

$ git am -3 numa/*
Applying: mm: numa: Do not dereference pmd outside of the lock during NUMA hinting fault
Applying: mm: Add p[te|md] protnone helpers for use by NUMA balancing
Applying: mm: Convert p[te|md]_numa users to p[te|md]_protnone_numa
fatal: sha1 information is lacking or useless (mm/huge_memory.c).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.

Did I miss a prerequisite?


Thanks,
Sasha

^ permalink raw reply

* Re: [PATCH 02/10] mm: Add p[te|md] protnone helpers for use by NUMA balancing
From: Linus Torvalds @ 2014-11-20 19:54 UTC (permalink / raw)
  To: Mel Gorman
  Cc: Rik van Riel, Hugh Dickins, Linux Kernel, Linux-MM, Ingo Molnar,
	Paul Mackerras, Aneesh Kumar, Sasha Levin, Dave Jones,
	LinuxPPC-dev, Kirill Shutemov
In-Reply-To: <1416478790-27522-3-git-send-email-mgorman@suse.de>

On Thu, Nov 20, 2014 at 2:19 AM, Mel Gorman <mgorman@suse.de> wrote:
> This is a preparatory patch that introduces protnone helpers for automatic
> NUMA balancing.

Oh, I hadn't noticed that you had renamed these things. It was
probably already true in your V1 version.

I do *not* think that "pte_protnone_numa()" makes sense as a name. It
only confuses people to think that there is still/again something
NUMA-special about the PTE. The whole point of the protnone changes
was to make it really very very clear that from a hardware standpoint,
this is *exactly* about protnone, and nothing else.

The fact that we then use protnone PTE's for numa faults is a VM
internal issue, it should *not* show up in the architecture page table
helpers.

I'm not NAK'ing this name, but I really think it's a very important
part of the whole patch series - to stop the stupid confusion about
NUMA entries. As far as the page tables are concerned, this has
absolutely _zero_ to do with NUMA.

We made that mistake once. We're fixing it. Let the naming *show* that
it's fixed, and this is "pte_protnone()".

The places that use this for NUMA handling might have a comment or
something. But they'll be in the VM where this matters, not in the
architecture page table description files. The comment would be
something like "if the vma is accessible, but the PTE is marked
protnone, this is a autonuma entry".

                    Linus

^ permalink raw reply

* Re: [PATCH V9 08/18] powrepc/pci: Refactor pci_dn
From: Bjorn Helgaas @ 2014-11-20 19:05 UTC (permalink / raw)
  To: Wei Yang; +Cc: linux-pci, benh, linuxppc-dev, gwshan
In-Reply-To: <20141120072057.GC8562@richard>

On Thu, Nov 20, 2014 at 03:20:57PM +0800, Wei Yang wrote:
> On Wed, Nov 19, 2014 at 04:30:24PM -0700, Bjorn Helgaas wrote:
> >On Sun, Nov 02, 2014 at 11:41:24PM +0800, Wei Yang wrote:
> >> From: Gavin Shan <gwshan@linux.vnet.ibm.com>
> >> 
> >> pci_dn is the extension of PCI device node and it's created from
> >> device node. Unfortunately, VFs that are enabled dynamically by
> >> PF's driver and they don't have corresponding device nodes, and
> >> pci_dn. The patch refactors pci_dn to support VFs:
> >> 
> >>    * pci_dn is organized as a hierarchy tree. VF's pci_dn is put
> >>      to the child list of pci_dn of PF's bridge. pci_dn of other
> >>      device put to the child list of pci_dn of its upstream bridge.
> >> 
> >>    * VF's pci_dn is expected to be created dynamically when applying
> >>      final fixup to PF. VF's pci_dn will be destroyed when releasing
> >>      PF's pci_dev instance. pci_dn of other device is still created
> >>      from device node as before.
> >> 
> >>    * For one particular PCI device (VF or not), its pci_dn can be
> >>      found from pdev->dev.archdata.firmware_data, PCI_DN(devnode),
> >>      or parent's list. The fast path (fetching pci_dn through PCI
> >>      device instance) is populated during early fixup time.
> >> 
> >> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
> >> ---
> >> ...
> >
> >> +struct pci_dn *add_dev_pci_info(struct pci_dev *pdev)
> >> +{
> >> +#ifdef CONFIG_PCI_IOV
> >> +	struct pci_dn *parent, *pdn;
> >> +	int i;
> >> +
> >> +	/* Only support IOV for now */
> >> +	if (!pdev->is_physfn)
> >> +		return pci_get_pdn(pdev);
> >> +
> >> +	/* Check if VFs have been populated */
> >> +	pdn = pci_get_pdn(pdev);
> >> +	if (!pdn || (pdn->flags & PCI_DN_FLAG_IOV_VF))
> >> +		return NULL;
> >> +
> >> +	pdn->flags |= PCI_DN_FLAG_IOV_VF;
> >> +	parent = pci_bus_to_pdn(pdev->bus);
> >> +	if (!parent)
> >> +		return NULL;
> >> +
> >> +	for (i = 0; i < pci_sriov_get_totalvfs(pdev); i++) {
> >> +		pdn = add_one_dev_pci_info(parent, NULL,
> >> +					   pci_iov_virtfn_bus(pdev, i),
> >> +					   pci_iov_virtfn_devfn(pdev, i));
> >
> >I'm not sure this makes sense, but I certainly don't know this code, so
> >maybe I'm missing something.
> >
> >pci_iov_virtfn_bus() and pci_iov_virtfn_devfn() depend on
> >pdev->sriov->stride and pdev->sriov->offset.  These are read from VF Stride
> >and First VF Offset in the SR-IOV capability by sriov_init(), which is
> >called before add_dev_pci_info():
> >
> >  pci_scan_child_bus
> >    pci_scan_slot
> >      pci_scan_single_device
> >	pci_device_add
> >	  pci_init_capabilities
> >	    pci_iov_init(PF)
> >	      sriov_init(PF, pos)
> >		pci_write_config_word(dev, pos + PCI_SRIOV_NUM_VF, 0)
> >		pci_read_config_word(dev, pos + PCI_SRIOV_VF_OFFSET, &offset)
> >		pci_read_config_word(dev, pos + PCI_SRIOV_VF_STRIDE, &stride)
> >		iov->offset = offset
> >		iov->stride = stride
> >
> >  pci_bus_add_devices
> >    pci_bus_add_device
> >      pci_fixup_device(pci_fixup_final)
> >	add_dev_pci_info
> >	  pci_iov_virtfn_bus
> >	    return ... + sriov->offset + (sriov->stride * id) ...
> >
> >But both First VF Offset and VF Stride change when ARI Capable Hierarchy or
> >NumVFs changes (SR-IOV spec sec 3.3.9, 3.3.10).  We set NumVFs to zero in
> >sriov_init() above.  We will change NumVFs to something different when a
> >driver calls pci_enable_sriov():
> >
> >  pci_enable_sriov
> >    sriov_enable
> >      pci_write_config_word(dev, iov->pos + PCI_SRIOV_NUM_VF, nr_virtfn)
> >
> >Now First VF Offset and VF Stride have changed from what they were when we
> >called pci_iov_virtfn_bus() above.
> >
> 
> Oops, I see the ARI would affect those value, while missed the NumVFs also
> would.
> 
> Let's look at the problem one by one.
> 
> 1. The ARI capability.
> ===============================================================================
> The kernel initialize the capability like this:
> 
> pci_init_capabilities()
> 	pci_configure_ari()
> 	pci_iov_init()
> 		iov->offset = offset
> 		iov->stride = stride
> 
> When offset/stride is retrieved at this point, the ARI capability is taken
> into consideration.

PCI_SRIOV_CTRL_ARI is currently only changed at the time we enumerate the
PF, so I don't think this is really a problem.

> 2. The PF's NumVFs field
> ===============================================================================
> 2.1 Potential problem in current code
> ===============================================================================
> First, is current pci code has some potential problem?
> 
> sriov_enable()
> 	pci_read_config_word(dev, iov->pos + PCI_SRIOV_VF_OFFSET, &offset);
> 	pci_read_config_word(dev, iov->pos + PCI_SRIOV_VF_STRIDE, &stride);
> 	iov->offset = offset;
> 	iov->stride = stride;
> 	pci_write_config_word(dev, iov->pos + PCI_SRIOV_NUM_VF, nr_virtfn);
> 	virtfn_add()
> 		...
> 		virtfn->devfn = pci_iov_virtfn_devfn(dev, id);
> 
> The sriov_enable() retrieve the offset/stride then write the NumVFs. According
> to the SPEC, at this moment the offset/stride may change. While I don't see
> some code to retrieve and store those value again. And these fields will be
> used in virtfn_add().
> 
> If my understanding is correct, I suggest to move the retrieve and store
> operation after NumVFs is written.

Yep, it looks like the existing code has similar problems.  We might want
to add a simple function that writes PCI_SRIOV_NUM_VF, then reads
PCI_SRIOV_VF_OFFSET and PCI_SRIOV_VF_STRIDE and refreshes the cached values
in dev->sriov.

Then we'd at least know that virtfn_bus() and virtfn_devfn() return values
that are correct until the next NumVFs change.

> 2.2 The IOV bus range may not be correct in pci_scan_child_bus()?
> ===============================================================================
> In current pci core, when enumerating the pci tree, we do like this:
> 
> pci_scan_child_bus()
> 	pci_scan_slot()
> 		pci_scan_single_device()
> 			pci_device_add()
> 				pci_init_capabilities()
> 					pci_iov_init()
> 	max += pci_iov_bus_range(bus);
> 		busnr = pci_iov_virtfn_bus(dev, dev->sriov->total_VFs - 1);
> 	max = pci_scan_bridge(bus, dev, max, pass);
> 
> From this point, we see pci core reserve some bus range for VFs. This
> calculation is based on the offset/stride at this moment. And do the
> enumeration with the new bus number.
> 
> sriov_enable() could be called several times from driver to enable SRIOV, and
> with different nr_virtfn. If each time the NumVFs written, the offset/stride
> will change. This means we may try to address an extra bus we didn't reserve?
> Or this means it is out of control?

This looks like a problem, too.  I don't have a good suggestion for fixing
it.

> 2.3 How can I reserve bus range in FW?
> ===============================================================================
> This question comes from the previous one.
> 
> Based on my understanding, current pci core will rely on the bus number in HW
> if pcibios_assign_all_busses() is not set. If we want to support those VFs
> sits on different bus with PF, we need to reserve bus range and write the
> correct secondary/subordinate in bridge. Otherwise, those VFs on different bus
> may not be addressed.
> 
> Currently I am writing the code in FW to reserve the range with the same
> mechanism in pci core. While as you mentioned the offset/stride may change
> after sriov_enable(), I am confused whether this is the correct way.

If your firmware knows something about the device and can compute the
number of buses it will likely need, it can set up bridges with appropriate
bus number ranges, and Linux will generally leave those alone.

I don't know the best way to figure out the number of buses, though.  It
seems like you almost need to experimentally set NumVFs and read the
resulting offset/stride, because I think it's really up to the device to
decide how to number the VFs.  Maybe pci_iov_bus_range() needs to do
something similar.

> 2.4 The potential problem for [Patch 08/18]
> ===============================================================================
> According to the SPEC, the offset/stride will change after each
> sriov_enable(). This means the bus/devfn will change after each
> sriov_enable().
> 
> My current thought is to fix it up in virtfn_add(). If the total VF number
> will not change, we could create those pci_dn at the beginning and fix the
> bus/devfn at each time the VF is truely created.

By "fix it up," I assume you mean call an arch function that does the
pci_pdn setup you need.

It sounds reasonable to do this either in virtfn_add()/virtfn_remove() or
at the points where we write PCI_SRIOV_CTRL_VFE, i.e., in sriov_init(),
sriov_enable(), sriov_disable(), and sriov_restore_state().  From a
hardware point of view, the VFs exist whenever PCI_SRIOV_CTRL_VFE is set,
so it might be nice to have this setup connected to that.

Bjorn

^ permalink raw reply

* Re: [RFC PATCH 00/16] Refine PCI host bridge scan interfaces
From: Liviu Dudau @ 2014-11-20 16:39 UTC (permalink / raw)
  To: Tomasz Nowicki
  Cc: Tony Luck, Russell King, Arnd Bergmann, linux-pci, x86,
	linux-kernel, Xinwei Hu, Yijing Wang, Thierry Reding,
	Suravee.Suthikulpanit, Bjorn Helgaas, linux-ia64, Thomas Gleixner,
	Wuyun, linuxppc-dev, linux-arm-kernel
In-Reply-To: <546DE45C.6010306@linaro.org>

On Thu, Nov 20, 2014 at 01:53:48PM +0100, Tomasz Nowicki wrote:
> On 20.11.2014 13:08, Liviu Dudau wrote:
> >On Thu, Nov 20, 2014 at 12:54:48PM +0100, Tomasz Nowicki wrote:
> >>On 17.11.2014 15:13, Arnd Bergmann wrote:
> >>>On Monday 17 November 2014 18:21:34 Yijing Wang wrote:
> >>>>This series is based Linux 3.18-rc1 and Lorenzo Pieralisi's
> >>>>arm PCI domain cleanup patches, link:
> >>>>https://patchwork.ozlabs.org/patch/407585/
> >>>>
> >>>>Current pci scan interfaces like pci_scan_root_bus() and directly
> >>>>call pci_create_root_bus()/pci_scan_child_bus() lack flexiblity.
> >>>>Some platform infos like PCI domain and msi_chip have to be
> >>>>associated to PCI bus by some arch specific function.
> >>>>We want to make a generic pci_host_bridge, and make it hold
> >>>>the platform infos or hook. Then we could eliminate the lots
> >>>>of arch pci_domain_nr, also we could associate some platform
> >>>>ops something like pci_get_msi_chip(struct pci_dev *dev)
> >>>>with pci_host_bridge to avoid introduce arch weak functions.
> >>>>
> >>>>This RFC version not for all platforms, just applied the new
> >>>>scan interface in x86/arm/powerpc/ia64, I will refresh other
> >>>>platforms after the core pci scan interfaces are ok.
> >>>
> >>>I think overall this is a good direction to take, in particular
> >>>moving more things into struct pci_host_bridge so we can
> >>>slim down the architecture specific code.
> >>>
> >>>I don't particularly like the way you use the 'pci_host_info'
> >>>to pass callback pointers and some of the generic information.
> >>>This duplicates some of the issues we are currently trying
> >>>to untangle in the arm32 code to make drivers easier to share
> >>>between architectures.
> >>>
> >>>As a general approach, I'd rather see generic helper functions
> >>>being exported by the PCI core that a driver may or may not
> >>>call.
> >>>The way you split the interface between things that happen
> >>>before scanning the buses (pci_create_host_bridge) and
> >>>the actual scanning (__pci_create_root_bus, pci_scan_child_bus)
> >>>seems very helpful and I think we can expand that concept further:
> >>>
> >>>- The normal pci_create_host_bridge() function can contain
> >>>   all of the DT scanning functions (finding bus/mem/io resources,
> >>>   finding the msi-parent), while drivers that don't depend on DT
> >>>   for this information can call the same function and fill the
> >>>   same things after they have the pci_host_bridge pointer.
> >>
> >>How about finding PCI domain number (in the DT way) within
> >>pci_create_host_bridge() too ?
> >
> >It is an idea worth pursuing for the 99% of the cases. I would like
> >to understand the 1% of the time when we want a domain number to be
> >shared between two host bridges or the time when we want more than
> >one domain per bridge.
> Even though we have shared domain, this should be resolved via DT calls, do
> I miss something ?

If we only going to hold one domain number per host bridge, then no, you're
not missing anything.

> 
> >
> >Is that possible? Is it useful? Is it already in practice?
> This is good question... IMO:
> 1. Two host bridges can shared domain number if they are children of the
> same parent host bridge.
> 2. But I can not find good explanation for more than one domain per bridge.

Splitting a root bus into two or more "segments" ?

Best regards,
Liviu

> 
> Tomasz
> 
> 

-- 
-------------------
   .oooO
   (   )
    \ (  Oooo.
     \_) (   )
          ) /
         (_/

 One small step
   for me ...

^ permalink raw reply

* [PATCH] spi: fsl-spi: Don't use cpm_command on CPM1
From: Christophe Leroy @ 2014-11-20 16:24 UTC (permalink / raw)
  To: Mark Brown; +Cc: linuxppc-dev, linux-kernel, linux-spi

On CPM1, when the SPI parameter RAM is relocated to somewhere else than the
default location, in accordance with freescale documentation
(refer micropatch SPI application note EB662), init RX/TX params command shall
not be used because it doesn't take into account the new location, and
overwrites data that is in original location of SPI param ram at addresses 
SCC2 param base +  (u32*)0x88 (u16*)0x90 (u32*)0x98 (u16*)0xA0, hence breaking
activity on SCC2 if SCC2 is used in a mode like QMC for instance.

Therefore, the action shall be done manually as described by freescale and as
was already partly done by the driver.

Reported-by: Patrick Vasseur <patrick.vasseur@c-s.fr>
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Tested-by: Patrick Vasseur <patrick.vasseur@c-s.fr>

---
 drivers/spi/spi-fsl-cpm.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-fsl-cpm.c b/drivers/spi/spi-fsl-cpm.c
index 54b0637..da97988 100644
--- a/drivers/spi/spi-fsl-cpm.c
+++ b/drivers/spi/spi-fsl-cpm.c
@@ -56,12 +56,15 @@ void fsl_spi_cpm_reinit_txrx(struct mpc8xxx_spi *mspi)
 		qe_issue_cmd(QE_INIT_TX_RX, mspi->subblock,
 			     QE_CR_PROTOCOL_UNSPECIFIED, 0);
 	} else {
-		cpm_command(CPM_SPI_CMD, CPM_CR_INIT_TRX);
 		if (mspi->flags & SPI_CPM1) {
+			out_be32(&mspi->pram->rstate, 0);
 			out_be16(&mspi->pram->rbptr,
 				 in_be16(&mspi->pram->rbase));
+			out_be32(&mspi->pram->tstate, 0);
 			out_be16(&mspi->pram->tbptr,
 				 in_be16(&mspi->pram->tbase));
+		} else {
+			cpm_command(CPM_SPI_CMD, CPM_CR_INIT_TRX);
 		}
 	}
 }
-- 
2.1.0

^ permalink raw reply related

* Re: [PATCH v2] i2c: Driver to expose PowerNV platform i2c busses
From: Neelesh Gupta @ 2014-11-20 14:22 UTC (permalink / raw)
  To: linuxppc-dev, linux-i2c, wsa
In-Reply-To: <20141116171605.4750.17472.stgit@localhost.localdomain>


On 11/16/2014 10:47 PM, Neelesh Gupta wrote:
> The patch exposes the available i2c busses on the PowerNV platform
> to the kernel and implements the bus driver to support i2c and
> smbus commands.
> The driver uses the platform device infrastructure to probe the busses
> on the platform and registers them with the i2c driver framework.
>
> Signed-off-by: Neelesh Gupta <neelegup@linux.vnet.ibm.com>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---

Hi Wolfram,

Did you get the chance to look at it?
The patch addresses the comments and discussions on v1.

Thanks,
Neelesh

>   arch/powerpc/include/asm/opal.h                |   29 ++
>   arch/powerpc/platforms/powernv/opal-wrappers.S |    1
>   arch/powerpc/platforms/powernv/opal.c          |   11 +
>   drivers/i2c/busses/Kconfig                     |   11 +
>   drivers/i2c/busses/Makefile                    |    1
>   drivers/i2c/busses/i2c-opal.c                  |  295 ++++++++++++++++++++++++
>   6 files changed, 348 insertions(+)
>   create mode 100644 drivers/i2c/busses/i2c-opal.c
>
> diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
> index 9124b0e..537807b 100644
> --- a/arch/powerpc/include/asm/opal.h
> +++ b/arch/powerpc/include/asm/opal.h
>

^ permalink raw reply

* Re: [RFC PATCH 00/16] Refine PCI host bridge scan interfaces
From: Arnd Bergmann @ 2014-11-20 13:15 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Liviu Dudau, Tony Luck, Russell King, linux-pci, x86,
	linux-kernel, Tomasz Nowicki, Xinwei Hu, Bjorn Helgaas,
	Thierry Reding, Suravee.Suthikulpanit, Yijing Wang, linux-ia64,
	Thomas Gleixner, Wuyun, linux-arm-kernel
In-Reply-To: <546DD804.4010105@linaro.org>

On Thursday 20 November 2014 13:01:08 Tomasz Nowicki wrote:
> On 18.11.2014 13:27, Arnd Bergmann wrote:
> > On Tuesday 18 November 2014 20:17:57 Yijing Wang wrote:
> >>
> >>>>
> >>>> I hope platforms with ACPI or DT could both use pci_create_host_bridge().
> >>>> Why we need to use two different ways to process it ?
> >>>
> >>> These are completely different use cases:
> >>>
> >>> a) For DT, we want loadable device drivers that start by probing a host
> >>>     bridge device which was added through the DT platform code. The
> >>>     driver is self-contained, and eventually we want to be able to unload
> >>>     it. We have lots of different per-soc drivers that require different
> >>>     quirks
> >>>
> >>> b) For ACPI, the interface is defined in the ACPI spec across architectures
> >>>     and SoCs, we don't have host bridge drivers and the code that initializes
> >>>     the PCI is required early during boot and called from architecture
> >>>     code. There is no parent device, as ACPI sees PCI as a fundamental building
> >>>     block by itself, and there are no drivers because the firmware does
> >>>     the initial hardware setup, so we only have to access the config space.
> >>
> >> Hmmm, I'm a little confused, so why you think ACPI host driver should not use
> >> pci_create_host_bridge(), because ACPI PCI driver has no parent device ?
> >
> > It's one of the difference. Having a parent device can certainly make your
> > life simpler, since you have devm_kzalloc(), dev_info(), etc. Coming from
> > the other end, I think ACPI needs PCI to be available during early boot,
> > at a time where we might not want pci_create_host_bridge() to do the
> > right thing.
> 
> Device pointer is not required for ACPI, struct acpi_device is all we 
> need to get all that info. If pci_create_host_bridge() would be DT 
> specific, it would be nice to have sth similar for ACPI but that is out 
> of this patch set scope.

My point was more that we don't need to have something like it for ACPI,
since we don't get random drivers that need to be probed that way,
just one common implementation that calls into the PCI core. We should
of course share the common bits with pci_create_host_bridge() in some
form, but that can be done by moving the x86 pci_acpi_scan_root
function and/or acpi_pci_root_add() to a common place in drivers/pci
and then refactoring the internals.

	Arnd

^ permalink raw reply

* RE: [PATCH] [v2] power/fsl: add MDIO dt binding for FMan
From: Shaohui Xie @ 2014-11-20 13:00 UTC (permalink / raw)
  To: linuxppc-dev@lists.ozlabs.org, devicetree@vger.kernel.org,
	Scott Wood
  Cc: Igal.Liberman@freescale.com, Emilian Medve
In-Reply-To: <1415958800-9867-1-git-send-email-shh.xie@gmail.com>

Ping.

Best Regards,=20
Shaohui Xie

> -----Original Message-----
> From: shh.xie@gmail.com [mailto:shh.xie@gmail.com]
> Sent: Friday, November 14, 2014 5:53 PM
> To: linuxppc-dev@lists.ozlabs.org; devicetree@vger.kernel.org; Wood
> Scott-B07421
> Cc: Medve Emilian-EMMEDVE1; Liberman Igal-B31950; Xie Shaohui-B21989
> Subject: [PATCH] [v2] power/fsl: add MDIO dt binding for FMan
>=20
> From: Shaohui Xie <Shaohui.Xie@freescale.com>
>=20
> This binding is for FMan MDIO, it covers FMan v2 & FMan v3.
>=20
> Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com>
> ---
> changes in V2:
> addressed comments from Scott in V1.
>=20
>  .../devicetree/bindings/powerpc/fsl/fman.txt       | 69
> ++++++++++++++++++++++
>  1 file changed, 69 insertions(+)
>=20
> diff --git a/Documentation/devicetree/bindings/powerpc/fsl/fman.txt
> b/Documentation/devicetree/bindings/powerpc/fsl/fman.txt
> index edeea16..1523a87 100644
> --- a/Documentation/devicetree/bindings/powerpc/fsl/fman.txt
> +++ b/Documentation/devicetree/bindings/powerpc/fsl/fman.txt
> @@ -7,6 +7,7 @@ CONTENTS
>    - FMan MURAM Node
>    - FMan dTSEC/XGEC/mEMAC Node
>    - FMan IEEE 1588 Node
> +  - FMan MDIO Node
>    - Example
>=20
>=20
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> =3D=3D=3D=3D
> @@ -357,6 +358,68 @@ ptp-timer@fe000 {
>  };
>=20
>=20
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> =3D=3D=3D=3D
> +FMan MDIO Node
> +
> +DESCRIPTION
> +
> +The MDIO is a bus to which the PHY devices are connected.
> +
> +PROPERTIES
> +
> +- compatible
> +		Usage: required
> +		Value type: <stringlist>
> +		Definition: A standard property.
> +		Must include "fsl,fman-mdio" for 1 Gb/s MDIO from FMan v2.
> +		Must include "fsl,fman-xmdio" for 10 Gb/s MDIO from FMan v2.
> +		Must include "fsl,fman-memac-mdio" for 1/10 Gb/s MDIO from
> +		FMan v3.
> +
> +- reg
> +		Usage: required
> +		Value type: <prop-encoded-array>
> +		Definition: A standard property.
> +
> +- bus-frequency
> +		Usage: optional
> +		Value type: <u32>
> +		Definition: Specifies external MDIO bus clock speed which is
> +		different from MDIO standard 2.5MHz. Should be defined for
> SoCs
> +		on which the standard one cannot work.
> +
> +- interrupts
> +		Usage: optional
> +		Value type: <prop-encoded-array>
> +		Definition: Event interrupt of external MDIO controller.
> +		1 Gb/s MDIO and 10 Gb/s MDIO has one interrupt respectively.
> +
> +- fsl,fman-internal-mdio
> +		Usage: required for internal MDIO
> +		Value type: boolean
> +		Definition: Fman has internal MDIO for internal PCS(Physical
> +		Coding Sublayer) PHYs and external MDIO for external PHYs.
> +		The settings and programming routines for internal/external
> +		MDIO are different. Must be included for internal MDIO.
> +
> +EXAMPLE
> +
> +Example for FMan v2 external MDIO:
> +
> +mdio@f1000 {
> +	compatible =3D "fsl,fman-xmdio";
> +	reg =3D <0xf1000 0x1000>;
> +	bus-frequency =3D <20000>;
> +};
> +
> +Example for FMan v3 internal MDIO:
> +
> +mdio@f1000 {
> +	compatible =3D "fsl,fman-memac-mdio";
> +	reg =3D <0xf1000 0x1000>;
> +	fsl,fman-internal-mdio;
> +};
> +
> +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> =3D=3D=3D=3D=3D
>  Example
>=20
>  fman@400000 {
> @@ -531,4 +594,10 @@ fman@400000 {
>  		compatible =3D "fsl,fman-ptp-timer";
>  		reg =3D <0xfe000 0x1000>;
>  	};
> +
> +	mdio@f1000 {
> +		compatible =3D "fsl,fman-xmdio";
> +		reg =3D <0xf1000 0x1000>;
> +		bus-frequency =3D <20000>;
> +	};
>  };
> --
> 1.8.4.1

^ permalink raw reply

* Re: [RFC PATCH 00/16] Refine PCI host bridge scan interfaces
From: Tomasz Nowicki @ 2014-11-20 12:53 UTC (permalink / raw)
  To: Liviu Dudau
  Cc: Tony Luck, Russell King, Arnd Bergmann, linux-pci, x86,
	linux-kernel, Xinwei Hu, Yijing Wang, Thierry Reding,
	Suravee.Suthikulpanit, Bjorn Helgaas, linux-ia64, Thomas Gleixner,
	Wuyun, linuxppc-dev, linux-arm-kernel
In-Reply-To: <20141120120850.GD9162@bart.dudau.co.uk>

On 20.11.2014 13:08, Liviu Dudau wrote:
> On Thu, Nov 20, 2014 at 12:54:48PM +0100, Tomasz Nowicki wrote:
>> On 17.11.2014 15:13, Arnd Bergmann wrote:
>>> On Monday 17 November 2014 18:21:34 Yijing Wang wrote:
>>>> This series is based Linux 3.18-rc1 and Lorenzo Pieralisi's
>>>> arm PCI domain cleanup patches, link:
>>>> https://patchwork.ozlabs.org/patch/407585/
>>>>
>>>> Current pci scan interfaces like pci_scan_root_bus() and directly
>>>> call pci_create_root_bus()/pci_scan_child_bus() lack flexiblity.
>>>> Some platform infos like PCI domain and msi_chip have to be
>>>> associated to PCI bus by some arch specific function.
>>>> We want to make a generic pci_host_bridge, and make it hold
>>>> the platform infos or hook. Then we could eliminate the lots
>>>> of arch pci_domain_nr, also we could associate some platform
>>>> ops something like pci_get_msi_chip(struct pci_dev *dev)
>>>> with pci_host_bridge to avoid introduce arch weak functions.
>>>>
>>>> This RFC version not for all platforms, just applied the new
>>>> scan interface in x86/arm/powerpc/ia64, I will refresh other
>>>> platforms after the core pci scan interfaces are ok.
>>>
>>> I think overall this is a good direction to take, in particular
>>> moving more things into struct pci_host_bridge so we can
>>> slim down the architecture specific code.
>>>
>>> I don't particularly like the way you use the 'pci_host_info'
>>> to pass callback pointers and some of the generic information.
>>> This duplicates some of the issues we are currently trying
>>> to untangle in the arm32 code to make drivers easier to share
>>> between architectures.
>>>
>>> As a general approach, I'd rather see generic helper functions
>>> being exported by the PCI core that a driver may or may not
>>> call.
>>> The way you split the interface between things that happen
>>> before scanning the buses (pci_create_host_bridge) and
>>> the actual scanning (__pci_create_root_bus, pci_scan_child_bus)
>>> seems very helpful and I think we can expand that concept further:
>>>
>>> - The normal pci_create_host_bridge() function can contain
>>>    all of the DT scanning functions (finding bus/mem/io resources,
>>>    finding the msi-parent), while drivers that don't depend on DT
>>>    for this information can call the same function and fill the
>>>    same things after they have the pci_host_bridge pointer.
>>
>> How about finding PCI domain number (in the DT way) within
>> pci_create_host_bridge() too ?
>
> It is an idea worth pursuing for the 99% of the cases. I would like
> to understand the 1% of the time when we want a domain number to be
> shared between two host bridges or the time when we want more than
> one domain per bridge.
Even though we have shared domain, this should be resolved via DT calls, 
do I miss something ?

>
> Is that possible? Is it useful? Is it already in practice?
This is good question... IMO:
1. Two host bridges can shared domain number if they are children of the 
same parent host bridge.
2. But I can not find good explanation for more than one domain per bridge.

Tomasz

^ permalink raw reply

* Re: [RFC PATCH 00/16] Refine PCI host bridge scan interfaces
From: Liviu Dudau @ 2014-11-20 12:08 UTC (permalink / raw)
  To: Tomasz Nowicki
  Cc: Tony Luck, Russell King, Arnd Bergmann, linux-pci, x86,
	linux-kernel, Xinwei Hu, Yijing Wang, Thierry Reding,
	Suravee.Suthikulpanit, Bjorn Helgaas, linux-ia64, Thomas Gleixner,
	Wuyun, linuxppc-dev, linux-arm-kernel
In-Reply-To: <546DD688.60705@linaro.org>

On Thu, Nov 20, 2014 at 12:54:48PM +0100, Tomasz Nowicki wrote:
> On 17.11.2014 15:13, Arnd Bergmann wrote:
> >On Monday 17 November 2014 18:21:34 Yijing Wang wrote:
> >>This series is based Linux 3.18-rc1 and Lorenzo Pieralisi's
> >>arm PCI domain cleanup patches, link:
> >>https://patchwork.ozlabs.org/patch/407585/
> >>
> >>Current pci scan interfaces like pci_scan_root_bus() and directly
> >>call pci_create_root_bus()/pci_scan_child_bus() lack flexiblity.
> >>Some platform infos like PCI domain and msi_chip have to be
> >>associated to PCI bus by some arch specific function.
> >>We want to make a generic pci_host_bridge, and make it hold
> >>the platform infos or hook. Then we could eliminate the lots
> >>of arch pci_domain_nr, also we could associate some platform
> >>ops something like pci_get_msi_chip(struct pci_dev *dev)
> >>with pci_host_bridge to avoid introduce arch weak functions.
> >>
> >>This RFC version not for all platforms, just applied the new
> >>scan interface in x86/arm/powerpc/ia64, I will refresh other
> >>platforms after the core pci scan interfaces are ok.
> >
> >I think overall this is a good direction to take, in particular
> >moving more things into struct pci_host_bridge so we can
> >slim down the architecture specific code.
> >
> >I don't particularly like the way you use the 'pci_host_info'
> >to pass callback pointers and some of the generic information.
> >This duplicates some of the issues we are currently trying
> >to untangle in the arm32 code to make drivers easier to share
> >between architectures.
> >
> >As a general approach, I'd rather see generic helper functions
> >being exported by the PCI core that a driver may or may not
> >call.
> >The way you split the interface between things that happen
> >before scanning the buses (pci_create_host_bridge) and
> >the actual scanning (__pci_create_root_bus, pci_scan_child_bus)
> >seems very helpful and I think we can expand that concept further:
> >
> >- The normal pci_create_host_bridge() function can contain
> >   all of the DT scanning functions (finding bus/mem/io resources,
> >   finding the msi-parent), while drivers that don't depend on DT
> >   for this information can call the same function and fill the
> >   same things after they have the pci_host_bridge pointer.
> 
> How about finding PCI domain number (in the DT way) within
> pci_create_host_bridge() too ?

It is an idea worth pursuing for the 99% of the cases. I would like
to understand the 1% of the time when we want a domain number to be
shared between two host bridges or the time when we want more than
one domain per bridge.

Is that possible? Is it useful? Is it already in practice?

Best regards,
Liviu

> 
> Tomasz
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

-- 
-------------------
   .oooO
   (   )
    \ (  Oooo.
     \_) (   )
          ) /
         (_/

 One small step
   for me ...

^ permalink raw reply

* Re: [RFC PATCH 00/16] Refine PCI host bridge scan interfaces
From: Tomasz Nowicki @ 2014-11-20 12:01 UTC (permalink / raw)
  To: Arnd Bergmann, Yijing Wang
  Cc: Liviu Dudau, Tony Luck, Russell King, linux-pci, x86,
	linux-kernel, Xinwei Hu, Thierry Reding, Suravee.Suthikulpanit,
	Bjorn Helgaas, linux-ia64, Thomas Gleixner, Wuyun, linuxppc-dev,
	linux-arm-kernel
In-Reply-To: <1534030.bkpIToWlHq@wuerfel>

On 18.11.2014 13:27, Arnd Bergmann wrote:
> On Tuesday 18 November 2014 20:17:57 Yijing Wang wrote:
>>
>>>>
>>>> I hope platforms with ACPI or DT could both use pci_create_host_bridge().
>>>> Why we need to use two different ways to process it ?
>>>
>>> These are completely different use cases:
>>>
>>> a) For DT, we want loadable device drivers that start by probing a host
>>>     bridge device which was added through the DT platform code. The
>>>     driver is self-contained, and eventually we want to be able to unload
>>>     it. We have lots of different per-soc drivers that require different
>>>     quirks
>>>
>>> b) For ACPI, the interface is defined in the ACPI spec across architectures
>>>     and SoCs, we don't have host bridge drivers and the code that initializes
>>>     the PCI is required early during boot and called from architecture
>>>     code. There is no parent device, as ACPI sees PCI as a fundamental building
>>>     block by itself, and there are no drivers because the firmware does
>>>     the initial hardware setup, so we only have to access the config space.
>>
>> Hmmm, I'm a little confused, so why you think ACPI host driver should not use
>> pci_create_host_bridge(), because ACPI PCI driver has no parent device ?
>
> It's one of the difference. Having a parent device can certainly make your
> life simpler, since you have devm_kzalloc(), dev_info(), etc. Coming from
> the other end, I think ACPI needs PCI to be available during early boot,
> at a time where we might not want pci_create_host_bridge() to do the
> right thing.

Device pointer is not required for ACPI, struct acpi_device is all we 
need to get all that info. If pci_create_host_bridge() would be DT 
specific, it would be nice to have sth similar for ACPI but that is out 
of this patch set scope.

Tomasz

^ permalink raw reply

* Re: [RFC PATCH 00/16] Refine PCI host bridge scan interfaces
From: Tomasz Nowicki @ 2014-11-20 11:54 UTC (permalink / raw)
  To: Arnd Bergmann, linux-arm-kernel
  Cc: Liviu Dudau, Tony Luck, Russell King, linux-pci, x86,
	linux-kernel, Xinwei Hu, Yijing Wang, Thierry Reding,
	Suravee.Suthikulpanit, Bjorn Helgaas, linux-ia64, Thomas Gleixner,
	Wuyun, linuxppc-dev
In-Reply-To: <1463511.o4kE8TX3Bd@wuerfel>

On 17.11.2014 15:13, Arnd Bergmann wrote:
> On Monday 17 November 2014 18:21:34 Yijing Wang wrote:
>> This series is based Linux 3.18-rc1 and Lorenzo Pieralisi's
>> arm PCI domain cleanup patches, link:
>> https://patchwork.ozlabs.org/patch/407585/
>>
>> Current pci scan interfaces like pci_scan_root_bus() and directly
>> call pci_create_root_bus()/pci_scan_child_bus() lack flexiblity.
>> Some platform infos like PCI domain and msi_chip have to be
>> associated to PCI bus by some arch specific function.
>> We want to make a generic pci_host_bridge, and make it hold
>> the platform infos or hook. Then we could eliminate the lots
>> of arch pci_domain_nr, also we could associate some platform
>> ops something like pci_get_msi_chip(struct pci_dev *dev)
>> with pci_host_bridge to avoid introduce arch weak functions.
>>
>> This RFC version not for all platforms, just applied the new
>> scan interface in x86/arm/powerpc/ia64, I will refresh other
>> platforms after the core pci scan interfaces are ok.
>
> I think overall this is a good direction to take, in particular
> moving more things into struct pci_host_bridge so we can
> slim down the architecture specific code.
>
> I don't particularly like the way you use the 'pci_host_info'
> to pass callback pointers and some of the generic information.
> This duplicates some of the issues we are currently trying
> to untangle in the arm32 code to make drivers easier to share
> between architectures.
>
> As a general approach, I'd rather see generic helper functions
> being exported by the PCI core that a driver may or may not
> call.
> The way you split the interface between things that happen
> before scanning the buses (pci_create_host_bridge) and
> the actual scanning (__pci_create_root_bus, pci_scan_child_bus)
> seems very helpful and I think we can expand that concept further:
>
> - The normal pci_create_host_bridge() function can contain
>    all of the DT scanning functions (finding bus/mem/io resources,
>    finding the msi-parent), while drivers that don't depend on DT
>    for this information can call the same function and fill the
>    same things after they have the pci_host_bridge pointer.

How about finding PCI domain number (in the DT way) within 
pci_create_host_bridge() too ?

Tomasz

^ permalink raw reply

* Re: [PATCH 03/10] mm: Convert p[te|md]_numa users to p[te|md]_protnone_numa
From: Mel Gorman @ 2014-11-20 10:47 UTC (permalink / raw)
  To: David Laight
  Cc: Rik van Riel, LinuxPPC-dev, Hugh Dickins, Linux Kernel, Linux-MM,
	Ingo Molnar, Paul Mackerras, Aneesh Kumar, Dave Jones,
	Sasha Levin, Linus Torvalds, Kirill Shutemov
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D1C9F48CB@AcuExch.aculab.com>

On Thu, Nov 20, 2014 at 10:38:56AM +0000, David Laight wrote:
> From:  Mel Gorman
> > Convert existing users of pte_numa and friends to the new helper. Note
> > that the kernel is broken after this patch is applied until the other
> > page table modifiers are also altered. This patch layout is to make
> > review easier.
> 
> Doesn't that break bisection?
> 

Yes, for automatic NUMA balancing at least. The patch structure is to
to make reviewers job easier and besides, bisecting within patches 2-6
is pointless. If desired, I can collapse patches 2-6 together for the
final submission.

-- 
Mel Gorman
SUSE Labs

^ permalink raw reply

* RE: [PATCH 03/10] mm: Convert p[te|md]_numa users to p[te|md]_protnone_numa
From: David Laight @ 2014-11-20 10:38 UTC (permalink / raw)
  To: 'Mel Gorman', Linux Kernel
  Cc: Rik van Riel, LinuxPPC-dev, Hugh Dickins, Linux-MM, Ingo Molnar,
	Paul Mackerras, Aneesh Kumar, Dave Jones, Sasha Levin,
	Linus Torvalds, Kirill Shutemov
In-Reply-To: <1416478790-27522-4-git-send-email-mgorman@suse.de>

RnJvbTogIE1lbCBHb3JtYW4NCj4gQ29udmVydCBleGlzdGluZyB1c2VycyBvZiBwdGVfbnVtYSBh
bmQgZnJpZW5kcyB0byB0aGUgbmV3IGhlbHBlci4gTm90ZQ0KPiB0aGF0IHRoZSBrZXJuZWwgaXMg
YnJva2VuIGFmdGVyIHRoaXMgcGF0Y2ggaXMgYXBwbGllZCB1bnRpbCB0aGUgb3RoZXINCj4gcGFn
ZSB0YWJsZSBtb2RpZmllcnMgYXJlIGFsc28gYWx0ZXJlZC4gVGhpcyBwYXRjaCBsYXlvdXQgaXMg
dG8gbWFrZQ0KPiByZXZpZXcgZWFzaWVyLg0KDQpEb2Vzbid0IHRoYXQgYnJlYWsgYmlzZWN0aW9u
Pw0KDQoJRGF2aWQNCg0K

^ permalink raw reply

* [PATCH 10/10] mm: numa: Avoid unnecessary TLB flushes when setting NUMA hinting entries
From: Mel Gorman @ 2014-11-20 10:19 UTC (permalink / raw)
  To: Linux Kernel
  Cc: Rik van Riel, Linus Torvalds, Hugh Dickins, Linux-MM, Ingo Molnar,
	Paul Mackerras, Aneesh Kumar, Sasha Levin, Dave Jones,
	LinuxPPC-dev, Kirill Shutemov, Mel Gorman
In-Reply-To: <1416478790-27522-1-git-send-email-mgorman@suse.de>

If a PTE or PMD is already marked NUMA when scanning to mark entries
for NUMA hinting then it is not necessary to update the entry and
incur a TLB flush penalty. Avoid the avoidhead where possible.

Signed-off-by: Mel Gorman <mgorman@suse.de>
---
 mm/huge_memory.c | 14 ++++++++------
 mm/mprotect.c    |  4 ++++
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 6458229f..a7ea9b8 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -1524,12 +1524,14 @@ int change_huge_pmd(struct vm_area_struct *vma, pmd_t *pmd,
 			return 0;
 		}
 
-		ret = 1;
-		entry = pmdp_get_and_clear(mm, addr, pmd);
-		entry = pmd_modify(entry, newprot);
-		ret = HPAGE_PMD_NR;
-		set_pmd_at(mm, addr, pmd, entry);
-		BUG_ON(pmd_write(entry));
+		if (!prot_numa || !pmd_protnone_numa(*pmd)) {
+			ret = 1;
+			entry = pmdp_get_and_clear(mm, addr, pmd);
+			entry = pmd_modify(entry, newprot);
+			ret = HPAGE_PMD_NR;
+			set_pmd_at(mm, addr, pmd, entry);
+			BUG_ON(pmd_write(entry));
+		}
 		spin_unlock(ptl);
 	}
 
diff --git a/mm/mprotect.c b/mm/mprotect.c
index 33dfafb..eb890d0 100644
--- a/mm/mprotect.c
+++ b/mm/mprotect.c
@@ -86,6 +86,10 @@ static unsigned long change_pte_range(struct vm_area_struct *vma, pmd_t *pmd,
 				page = vm_normal_page(vma, addr, oldpte);
 				if (!page || PageKsm(page))
 					continue;
+
+				/* Avoid TLB flush if possible */
+				if (pte_protnone_numa(oldpte))
+					continue;
 			}
 
 			ptent = ptep_modify_prot_start(mm, addr, pte);
-- 
2.1.2

^ permalink raw reply related

* [PATCH 09/10] mm: numa: Add paranoid check around pte_protnone_numa
From: Mel Gorman @ 2014-11-20 10:19 UTC (permalink / raw)
  To: Linux Kernel
  Cc: Rik van Riel, Linus Torvalds, Hugh Dickins, Linux-MM, Ingo Molnar,
	Paul Mackerras, Aneesh Kumar, Sasha Levin, Dave Jones,
	LinuxPPC-dev, Kirill Shutemov, Mel Gorman
In-Reply-To: <1416478790-27522-1-git-send-email-mgorman@suse.de>

pte_protnone_numa is only safe to use after VMA checks for PROT_NONE are
complete. Treating a real PROT_NONE PTE as a NUMA hinting fault is going
to result in strangeness so add a check for it. BUG_ON looks like overkill
but if this is hit then it's a serious bug that could result in corruption
so do not even try recovering. It would have been more comprehensive to
check VMA flags in pte_protnone_numa but it would have made the API ugly
just for a debugging check.

Signed-off-by: Mel Gorman <mgorman@suse.de>
---
 mm/huge_memory.c | 3 +++
 mm/memory.c      | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 3013eb8..6458229f 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -1274,6 +1274,9 @@ int do_huge_pmd_numa_page(struct mm_struct *mm, struct vm_area_struct *vma,
 	bool migrated = false;
 	int flags = 0;
 
+	/* A PROT_NONE fault should not end up here */
+	BUG_ON(!(vma->vm_flags & (VM_READ | VM_EXEC | VM_WRITE)));
+
 	ptl = pmd_lock(mm, pmdp);
 	if (unlikely(!pmd_same(pmd, *pmdp)))
 		goto out_unlock;
diff --git a/mm/memory.c b/mm/memory.c
index a725c08..42d652d 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3115,6 +3115,9 @@ static int do_numa_page(struct mm_struct *mm, struct vm_area_struct *vma,
 	bool migrated = false;
 	int flags = 0;
 
+	/* A PROT_NONE fault should not end up here */
+	BUG_ON(!(vma->vm_flags & (VM_READ | VM_EXEC | VM_WRITE)));
+
 	/*
 	* The "pte" at this point cannot be used safely without
 	* validation through pte_unmap_same(). It's of NUMA type but
-- 
2.1.2

^ permalink raw reply related

* [PATCH 08/10] x86: mm: Restore original pte_special check
From: Mel Gorman @ 2014-11-20 10:19 UTC (permalink / raw)
  To: Linux Kernel
  Cc: Rik van Riel, Linus Torvalds, Hugh Dickins, Linux-MM, Ingo Molnar,
	Paul Mackerras, Aneesh Kumar, Sasha Levin, Dave Jones,
	LinuxPPC-dev, Kirill Shutemov, Mel Gorman
In-Reply-To: <1416478790-27522-1-git-send-email-mgorman@suse.de>

Commit b38af4721f59 ("x86,mm: fix pte_special versus pte_numa") adjusted
the pte_special check to take into account that a special pte had SPECIAL
and neither PRESENT nor PROTNONE. Now that NUMA hinting PTEs are no
longer modifying _PAGE_PRESENT it should be safe to restore the original
pte_special behaviour.

Signed-off-by: Mel Gorman <mgorman@suse.de>
---
 arch/x86/include/asm/pgtable.h | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
index f8799e0..5241332 100644
--- a/arch/x86/include/asm/pgtable.h
+++ b/arch/x86/include/asm/pgtable.h
@@ -131,13 +131,7 @@ static inline int pte_exec(pte_t pte)
 
 static inline int pte_special(pte_t pte)
 {
-	/*
-	 * See CONFIG_NUMA_BALANCING pte_numa in include/asm-generic/pgtable.h.
-	 * On x86 we have _PAGE_BIT_NUMA == _PAGE_BIT_GLOBAL+1 ==
-	 * __PAGE_BIT_SOFTW1 == _PAGE_BIT_SPECIAL.
-	 */
-	return (pte_flags(pte) & _PAGE_SPECIAL) &&
-		(pte_flags(pte) & (_PAGE_PRESENT|_PAGE_PROTNONE));
+	return pte_flags(pte) & _PAGE_SPECIAL;
 }
 
 static inline unsigned long pte_pfn(pte_t pte)
-- 
2.1.2

^ permalink raw reply related

* [PATCH 07/10] mm: numa: Do not trap faults on the huge zero page
From: Mel Gorman @ 2014-11-20 10:19 UTC (permalink / raw)
  To: Linux Kernel
  Cc: Rik van Riel, Linus Torvalds, Hugh Dickins, Linux-MM, Ingo Molnar,
	Paul Mackerras, Aneesh Kumar, Sasha Levin, Dave Jones,
	LinuxPPC-dev, Kirill Shutemov, Mel Gorman
In-Reply-To: <1416478790-27522-1-git-send-email-mgorman@suse.de>

Faults on the huge zero page are pointless and there is a BUG_ON
to catch them during fault time. This patch reintroduces a check
that avoids marking the zero page PAGE_NONE.

Signed-off-by: Mel Gorman <mgorman@suse.de>
---
 include/linux/huge_mm.h |  3 ++-
 mm/huge_memory.c        | 13 ++++++++++++-
 mm/memory.c             |  1 -
 mm/mprotect.c           | 15 ++++++++++++++-
 4 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
index 554bbe3..ad9051b 100644
--- a/include/linux/huge_mm.h
+++ b/include/linux/huge_mm.h
@@ -31,7 +31,8 @@ extern int move_huge_pmd(struct vm_area_struct *vma,
 			 unsigned long new_addr, unsigned long old_end,
 			 pmd_t *old_pmd, pmd_t *new_pmd);
 extern int change_huge_pmd(struct vm_area_struct *vma, pmd_t *pmd,
-			unsigned long addr, pgprot_t newprot);
+			unsigned long addr, pgprot_t newprot,
+			int prot_numa);
 
 enum transparent_hugepage_flag {
 	TRANSPARENT_HUGEPAGE_FLAG,
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 668f1a3..3013eb8 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -1502,7 +1502,7 @@ out:
  *  - HPAGE_PMD_NR is protections changed and TLB flush necessary
  */
 int change_huge_pmd(struct vm_area_struct *vma, pmd_t *pmd,
-		unsigned long addr, pgprot_t newprot)
+		unsigned long addr, pgprot_t newprot, int prot_numa)
 {
 	struct mm_struct *mm = vma->vm_mm;
 	spinlock_t *ptl;
@@ -1510,6 +1510,17 @@ int change_huge_pmd(struct vm_area_struct *vma, pmd_t *pmd,
 
 	if (__pmd_trans_huge_lock(pmd, vma, &ptl) == 1) {
 		pmd_t entry;
+
+		/*
+		 * Avoid trapping faults against the zero page. The read-only
+		 * data is likely to be read-cached on the local CPU and
+		 * local/remote hits to the zero page are not interesting.
+		 */
+		if (prot_numa && is_huge_zero_pmd(*pmd)) {
+			spin_unlock(ptl);
+			return 0;
+		}
+
 		ret = 1;
 		entry = pmdp_get_and_clear(mm, addr, pmd);
 		entry = pmd_modify(entry, newprot);
diff --git a/mm/memory.c b/mm/memory.c
index 900127b..a725c08 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3142,7 +3142,6 @@ static int do_numa_page(struct mm_struct *mm, struct vm_area_struct *vma,
 		pte_unmap_unlock(ptep, ptl);
 		return 0;
 	}
-	BUG_ON(is_zero_pfn(page_to_pfn(page)));
 
 	/*
 	 * Avoid grouping on DSO/COW pages in specific and RO pages
diff --git a/mm/mprotect.c b/mm/mprotect.c
index dc65c0f..33dfafb 100644
--- a/mm/mprotect.c
+++ b/mm/mprotect.c
@@ -75,6 +75,19 @@ static unsigned long change_pte_range(struct vm_area_struct *vma, pmd_t *pmd,
 		oldpte = *pte;
 		if (pte_present(oldpte)) {
 			pte_t ptent;
+
+			/*
+			 * Avoid trapping faults against the zero or KSM
+			 * pages. See similar comment in change_huge_pmd.
+			 */
+			if (prot_numa) {
+				struct page *page;
+
+				page = vm_normal_page(vma, addr, oldpte);
+				if (!page || PageKsm(page))
+					continue;
+			}
+
 			ptent = ptep_modify_prot_start(mm, addr, pte);
 			ptent = pte_modify(ptent, newprot);
 
@@ -141,7 +154,7 @@ static inline unsigned long change_pmd_range(struct vm_area_struct *vma,
 				split_huge_page_pmd(vma, addr, pmd);
 			else {
 				int nr_ptes = change_huge_pmd(vma, pmd, addr,
-						newprot);
+						newprot, prot_numa);
 
 				if (nr_ptes) {
 					if (nr_ptes == HPAGE_PMD_NR) {
-- 
2.1.2

^ permalink raw reply related

* [PATCH 06/10] mm: Remove remaining references to NUMA hinting bits and helpers
From: Mel Gorman @ 2014-11-20 10:19 UTC (permalink / raw)
  To: Linux Kernel
  Cc: Rik van Riel, Linus Torvalds, Hugh Dickins, Linux-MM, Ingo Molnar,
	Paul Mackerras, Aneesh Kumar, Sasha Levin, Dave Jones,
	LinuxPPC-dev, Kirill Shutemov, Mel Gorman
In-Reply-To: <1416478790-27522-1-git-send-email-mgorman@suse.de>

This patch removes the NUMA PTE bits and associated helpers. As a side-effect
it increases the maximum possible swap space on x86-64.

One potential source of problems is races between the marking of PTEs
PROT_NONE, NUMA hinting faults and migration. It must be guaranteed that
a PTE being protected is not faulted in parallel, seen as a pte_none and
corrupting memory. The base case is safe but transhuge has problems in the
past due to an different migration mechanism and a dependance on page lock
to serialise migrations and warrants a closer look.

task_work hinting update			parallel fault
------------------------			--------------
change_pmd_range
  change_huge_pmd
    __pmd_trans_huge_lock
      pmdp_get_and_clear
						__handle_mm_fault
						pmd_none
						  do_huge_pmd_anonymous_page
						  read? pmd_lock blocks until hinting complete, fail !pmd_none test
						  write? __do_huge_pmd_anonymous_page acquires pmd_lock, checks pmd_none
      pmd_modify
      set_pmd_at

task_work hinting update			parallel migration
------------------------			------------------
change_pmd_range
  change_huge_pmd
    __pmd_trans_huge_lock
      pmdp_get_and_clear
						__handle_mm_fault
						  do_huge_pmd_numa_page
						    migrate_misplaced_transhuge_page
						    pmd_lock waits for updates to complete, recheck pmd_same
      pmd_modify
      set_pmd_at

Both of those are safe and the case where a transhuge page is inserted
during a protection update is unchanged. The case where two processes try
migrating at the same time is unchanged by this series so should still be
ok. I could not find a case where we are accidentally depending on the
PTE not being cleared and flushed. If one is missed, it'll manifest as
corruption problems that start triggering shortly after this series is
merged and only happen when NUMA balancing is enabled.

Signed-off-by: Mel Gorman <mgorman@suse.de>
---
 arch/powerpc/include/asm/pgtable.h    |  54 +-----------
 arch/powerpc/include/asm/pte-common.h |   5 --
 arch/powerpc/include/asm/pte-hash64.h |   6 --
 arch/x86/include/asm/pgtable.h        |  22 +----
 arch/x86/include/asm/pgtable_64.h     |   5 --
 arch/x86/include/asm/pgtable_types.h  |  41 +--------
 include/asm-generic/pgtable.h         | 155 ----------------------------------
 include/linux/swapops.h               |   2 +-
 8 files changed, 7 insertions(+), 283 deletions(-)

diff --git a/arch/powerpc/include/asm/pgtable.h b/arch/powerpc/include/asm/pgtable.h
index 452c3b4..2e074e7 100644
--- a/arch/powerpc/include/asm/pgtable.h
+++ b/arch/powerpc/include/asm/pgtable.h
@@ -49,64 +49,12 @@ static inline int pmd_protnone_numa(pmd_t pmd)
 {
 	return pte_protnone_numa(pmd_pte(pmd));
 }
-
-static inline int pte_present(pte_t pte)
-{
-	return pte_val(pte) & _PAGE_NUMA_MASK;
-}
-
-#define pte_present_nonuma pte_present_nonuma
-static inline int pte_present_nonuma(pte_t pte)
-{
-	return pte_val(pte) & (_PAGE_PRESENT);
-}
-
-#define ptep_set_numa ptep_set_numa
-static inline void ptep_set_numa(struct mm_struct *mm, unsigned long addr,
-				 pte_t *ptep)
-{
-	if ((pte_val(*ptep) & _PAGE_PRESENT) == 0)
-		VM_BUG_ON(1);
-
-	pte_update(mm, addr, ptep, _PAGE_PRESENT, _PAGE_NUMA, 0);
-	return;
-}
-
-#define pmdp_set_numa pmdp_set_numa
-static inline void pmdp_set_numa(struct mm_struct *mm, unsigned long addr,
-				 pmd_t *pmdp)
-{
-	if ((pmd_val(*pmdp) & _PAGE_PRESENT) == 0)
-		VM_BUG_ON(1);
-
-	pmd_hugepage_update(mm, addr, pmdp, _PAGE_PRESENT, _PAGE_NUMA);
-	return;
-}
-
-/*
- * Generic NUMA pte helpers expect pteval_t and pmdval_t types to exist
- * which was inherited from x86. For the purposes of powerpc pte_basic_t and
- * pmd_t are equivalent
- */
-#define pteval_t pte_basic_t
-#define pmdval_t pmd_t
-static inline pteval_t ptenuma_flags(pte_t pte)
-{
-	return pte_val(pte) & _PAGE_NUMA_MASK;
-}
-
-static inline pmdval_t pmdnuma_flags(pmd_t pmd)
-{
-	return pmd_val(pmd) & _PAGE_NUMA_MASK;
-}
-
-# else
+#endif /* CONFIG_NUMA_BALANCING */
 
 static inline int pte_present(pte_t pte)
 {
 	return pte_val(pte) & _PAGE_PRESENT;
 }
-#endif /* CONFIG_NUMA_BALANCING */
 
 /* Conversion functions: convert a page and protection to a page entry,
  * and a page entry and page directory to the page they refer to.
diff --git a/arch/powerpc/include/asm/pte-common.h b/arch/powerpc/include/asm/pte-common.h
index e040c35..8d1569c 100644
--- a/arch/powerpc/include/asm/pte-common.h
+++ b/arch/powerpc/include/asm/pte-common.h
@@ -98,11 +98,6 @@ extern unsigned long bad_call_to_PMD_PAGE_SIZE(void);
 			 _PAGE_USER | _PAGE_ACCESSED | \
 			 _PAGE_RW | _PAGE_HWWRITE | _PAGE_DIRTY | _PAGE_EXEC)
 
-#ifdef CONFIG_NUMA_BALANCING
-/* Mask of bits that distinguish present and numa ptes */
-#define _PAGE_NUMA_MASK (_PAGE_NUMA|_PAGE_PRESENT)
-#endif
-
 /*
  * We define 2 sets of base prot bits, one for basic pages (ie,
  * cacheable kernel and user pages) and one for non cacheable
diff --git a/arch/powerpc/include/asm/pte-hash64.h b/arch/powerpc/include/asm/pte-hash64.h
index 2505d8e..55aea0c 100644
--- a/arch/powerpc/include/asm/pte-hash64.h
+++ b/arch/powerpc/include/asm/pte-hash64.h
@@ -27,12 +27,6 @@
 #define _PAGE_RW		0x0200 /* software: user write access allowed */
 #define _PAGE_BUSY		0x0800 /* software: PTE & hash are busy */
 
-/*
- * Used for tracking numa faults
- */
-#define _PAGE_NUMA	0x00000010 /* Gather numa placement stats */
-
-
 /* No separate kernel read-only */
 #define _PAGE_KERNEL_RW		(_PAGE_RW | _PAGE_DIRTY) /* user access blocked by key */
 #define _PAGE_KERNEL_RO		 _PAGE_KERNEL_RW
diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
index 613cd00..f8799e0 100644
--- a/arch/x86/include/asm/pgtable.h
+++ b/arch/x86/include/asm/pgtable.h
@@ -299,7 +299,7 @@ static inline pmd_t pmd_mkwrite(pmd_t pmd)
 
 static inline pmd_t pmd_mknotpresent(pmd_t pmd)
 {
-	return pmd_clear_flags(pmd, _PAGE_PRESENT);
+	return pmd_clear_flags(pmd, _PAGE_PRESENT | _PAGE_PROTNONE);
 }
 
 #ifdef CONFIG_HAVE_ARCH_SOFT_DIRTY
@@ -457,13 +457,6 @@ static inline int pte_same(pte_t a, pte_t b)
 
 static inline int pte_present(pte_t a)
 {
-	return pte_flags(a) & (_PAGE_PRESENT | _PAGE_PROTNONE |
-			       _PAGE_NUMA);
-}
-
-#define pte_present_nonuma pte_present_nonuma
-static inline int pte_present_nonuma(pte_t a)
-{
 	return pte_flags(a) & (_PAGE_PRESENT | _PAGE_PROTNONE);
 }
 
@@ -473,7 +466,7 @@ static inline bool pte_accessible(struct mm_struct *mm, pte_t a)
 	if (pte_flags(a) & _PAGE_PRESENT)
 		return true;
 
-	if ((pte_flags(a) & (_PAGE_PROTNONE | _PAGE_NUMA)) &&
+	if ((pte_flags(a) & _PAGE_PROTNONE) &&
 			mm_tlb_flush_pending(mm))
 		return true;
 
@@ -493,8 +486,7 @@ static inline int pmd_present(pmd_t pmd)
 	 * the _PAGE_PSE flag will remain set at all times while the
 	 * _PAGE_PRESENT bit is clear).
 	 */
-	return pmd_flags(pmd) & (_PAGE_PRESENT | _PAGE_PROTNONE | _PAGE_PSE |
-				 _PAGE_NUMA);
+	return pmd_flags(pmd) & (_PAGE_PRESENT | _PAGE_PROTNONE | _PAGE_PSE);
 }
 
 #ifdef CONFIG_NUMA_BALANCING
@@ -569,11 +561,6 @@ static inline pte_t *pte_offset_kernel(pmd_t *pmd, unsigned long address)
 
 static inline int pmd_bad(pmd_t pmd)
 {
-#ifdef CONFIG_NUMA_BALANCING
-	/* pmd_numa check */
-	if ((pmd_flags(pmd) & (_PAGE_NUMA|_PAGE_PRESENT)) == _PAGE_NUMA)
-		return 0;
-#endif
 	return (pmd_flags(pmd) & ~_PAGE_USER) != _KERNPG_TABLE;
 }
 
@@ -892,19 +879,16 @@ static inline void update_mmu_cache_pmd(struct vm_area_struct *vma,
 #ifdef CONFIG_HAVE_ARCH_SOFT_DIRTY
 static inline pte_t pte_swp_mksoft_dirty(pte_t pte)
 {
-	VM_BUG_ON(pte_present_nonuma(pte));
 	return pte_set_flags(pte, _PAGE_SWP_SOFT_DIRTY);
 }
 
 static inline int pte_swp_soft_dirty(pte_t pte)
 {
-	VM_BUG_ON(pte_present_nonuma(pte));
 	return pte_flags(pte) & _PAGE_SWP_SOFT_DIRTY;
 }
 
 static inline pte_t pte_swp_clear_soft_dirty(pte_t pte)
 {
-	VM_BUG_ON(pte_present_nonuma(pte));
 	return pte_clear_flags(pte, _PAGE_SWP_SOFT_DIRTY);
 }
 #endif
diff --git a/arch/x86/include/asm/pgtable_64.h b/arch/x86/include/asm/pgtable_64.h
index 4572b2f..06ffca8 100644
--- a/arch/x86/include/asm/pgtable_64.h
+++ b/arch/x86/include/asm/pgtable_64.h
@@ -146,12 +146,7 @@ static inline int pgd_large(pgd_t pgd) { return 0; }
 
 /* Encode and de-code a swap entry */
 #define SWP_TYPE_BITS (_PAGE_BIT_FILE - _PAGE_BIT_PRESENT - 1)
-#ifdef CONFIG_NUMA_BALANCING
-/* Automatic NUMA balancing needs to be distinguishable from swap entries */
-#define SWP_OFFSET_SHIFT (_PAGE_BIT_PROTNONE + 2)
-#else
 #define SWP_OFFSET_SHIFT (_PAGE_BIT_PROTNONE + 1)
-#endif
 
 #define MAX_SWAPFILES_CHECK() BUILD_BUG_ON(MAX_SWAPFILES_SHIFT > SWP_TYPE_BITS)
 
diff --git a/arch/x86/include/asm/pgtable_types.h b/arch/x86/include/asm/pgtable_types.h
index 0778964..d299cdd 100644
--- a/arch/x86/include/asm/pgtable_types.h
+++ b/arch/x86/include/asm/pgtable_types.h
@@ -27,14 +27,6 @@
 #define _PAGE_BIT_SOFT_DIRTY	_PAGE_BIT_SOFTW3 /* software dirty tracking */
 #define _PAGE_BIT_NX           63       /* No execute: only valid after cpuid check */
 
-/*
- * Swap offsets on configurations that allow automatic NUMA balancing use the
- * bits after _PAGE_BIT_GLOBAL. To uniquely distinguish NUMA hinting PTEs from
- * swap entries, we use the first bit after _PAGE_BIT_GLOBAL and shrink the
- * maximum possible swap space from 16TB to 8TB.
- */
-#define _PAGE_BIT_NUMA		(_PAGE_BIT_GLOBAL+1)
-
 /* If _PAGE_BIT_PRESENT is clear, we use these: */
 /* - if the user mapped it with PROT_NONE; pte_present gives true */
 #define _PAGE_BIT_PROTNONE	_PAGE_BIT_GLOBAL
@@ -78,21 +70,6 @@
 #endif
 
 /*
- * _PAGE_NUMA distinguishes between a numa hinting minor fault and a page
- * that is not present. The hinting fault gathers numa placement statistics
- * (see pte_numa()). The bit is always zero when the PTE is not present.
- *
- * The bit picked must be always zero when the pmd is present and not
- * present, so that we don't lose information when we set it while
- * atomically clearing the present bit.
- */
-#ifdef CONFIG_NUMA_BALANCING
-#define _PAGE_NUMA	(_AT(pteval_t, 1) << _PAGE_BIT_NUMA)
-#else
-#define _PAGE_NUMA	(_AT(pteval_t, 0))
-#endif
-
-/*
  * Tracking soft dirty bit when a page goes to a swap is tricky.
  * We need a bit which can be stored in pte _and_ not conflict
  * with swap entry format. On x86 bits 6 and 7 are *not* involved
@@ -125,8 +102,8 @@
 /* Set of bits not changed in pte_modify */
 #define _PAGE_CHG_MASK	(PTE_PFN_MASK | _PAGE_PCD | _PAGE_PWT |		\
 			 _PAGE_SPECIAL | _PAGE_ACCESSED | _PAGE_DIRTY |	\
-			 _PAGE_SOFT_DIRTY | _PAGE_NUMA)
-#define _HPAGE_CHG_MASK (_PAGE_CHG_MASK | _PAGE_PSE | _PAGE_NUMA)
+			 _PAGE_SOFT_DIRTY)
+#define _HPAGE_CHG_MASK (_PAGE_CHG_MASK | _PAGE_PSE)
 
 #define _PAGE_CACHE_MASK	(_PAGE_PCD | _PAGE_PWT)
 #define _PAGE_CACHE_WB		(0)
@@ -324,20 +301,6 @@ static inline pteval_t pte_flags(pte_t pte)
 	return native_pte_val(pte) & PTE_FLAGS_MASK;
 }
 
-#ifdef CONFIG_NUMA_BALANCING
-/* Set of bits that distinguishes present, prot_none and numa ptes */
-#define _PAGE_NUMA_MASK (_PAGE_NUMA|_PAGE_PROTNONE|_PAGE_PRESENT)
-static inline pteval_t ptenuma_flags(pte_t pte)
-{
-	return pte_flags(pte) & _PAGE_NUMA_MASK;
-}
-
-static inline pmdval_t pmdnuma_flags(pmd_t pmd)
-{
-	return pmd_flags(pmd) & _PAGE_NUMA_MASK;
-}
-#endif /* CONFIG_NUMA_BALANCING */
-
 #define pgprot_val(x)	((x).pgprot)
 #define __pgprot(x)	((pgprot_t) { (x) } )
 
diff --git a/include/asm-generic/pgtable.h b/include/asm-generic/pgtable.h
index 7e74122..323e914 100644
--- a/include/asm-generic/pgtable.h
+++ b/include/asm-generic/pgtable.h
@@ -233,10 +233,6 @@ static inline int pmd_same(pmd_t pmd_a, pmd_t pmd_b)
 # define pte_accessible(mm, pte)	((void)(pte), 1)
 #endif
 
-#ifndef pte_present_nonuma
-#define pte_present_nonuma(pte) pte_present(pte)
-#endif
-
 #ifndef flush_tlb_fix_spurious_fault
 #define flush_tlb_fix_spurious_fault(vma, address) flush_tlb_page(vma, address)
 #endif
@@ -696,157 +692,6 @@ static inline int pmd_protnone_numa(pmd_t pmd)
 }
 #endif /* CONFIG_NUMA_BALANCING */
 
-#ifdef CONFIG_NUMA_BALANCING
-/*
- * _PAGE_NUMA distinguishes between an unmapped page table entry, an entry that
- * is protected for PROT_NONE and a NUMA hinting fault entry. If the
- * architecture defines __PAGE_PROTNONE then it should take that into account
- * but those that do not can rely on the fact that the NUMA hinting scanner
- * skips inaccessible VMAs.
- *
- * pte/pmd_present() returns true if pte/pmd_numa returns true. Page
- * fault triggers on those regions if pte/pmd_numa returns true
- * (because _PAGE_PRESENT is not set).
- */
-#ifndef pte_numa
-static inline int pte_numa(pte_t pte)
-{
-	return ptenuma_flags(pte) == _PAGE_NUMA;
-}
-#endif
-
-#ifndef pmd_numa
-static inline int pmd_numa(pmd_t pmd)
-{
-	return pmdnuma_flags(pmd) == _PAGE_NUMA;
-}
-#endif
-
-/*
- * pte/pmd_mknuma sets the _PAGE_ACCESSED bitflag automatically
- * because they're called by the NUMA hinting minor page fault. If we
- * wouldn't set the _PAGE_ACCESSED bitflag here, the TLB miss handler
- * would be forced to set it later while filling the TLB after we
- * return to userland. That would trigger a second write to memory
- * that we optimize away by setting _PAGE_ACCESSED here.
- */
-#ifndef pte_mknonnuma
-static inline pte_t pte_mknonnuma(pte_t pte)
-{
-	pteval_t val = pte_val(pte);
-
-	val &= ~_PAGE_NUMA;
-	val |= (_PAGE_PRESENT|_PAGE_ACCESSED);
-	return __pte(val);
-}
-#endif
-
-#ifndef pmd_mknonnuma
-static inline pmd_t pmd_mknonnuma(pmd_t pmd)
-{
-	pmdval_t val = pmd_val(pmd);
-
-	val &= ~_PAGE_NUMA;
-	val |= (_PAGE_PRESENT|_PAGE_ACCESSED);
-
-	return __pmd(val);
-}
-#endif
-
-#ifndef pte_mknuma
-static inline pte_t pte_mknuma(pte_t pte)
-{
-	pteval_t val = pte_val(pte);
-
-	VM_BUG_ON(!(val & _PAGE_PRESENT));
-
-	val &= ~_PAGE_PRESENT;
-	val |= _PAGE_NUMA;
-
-	return __pte(val);
-}
-#endif
-
-#ifndef ptep_set_numa
-static inline void ptep_set_numa(struct mm_struct *mm, unsigned long addr,
-				 pte_t *ptep)
-{
-	pte_t ptent = *ptep;
-
-	ptent = pte_mknuma(ptent);
-	set_pte_at(mm, addr, ptep, ptent);
-	return;
-}
-#endif
-
-#ifndef pmd_mknuma
-static inline pmd_t pmd_mknuma(pmd_t pmd)
-{
-	pmdval_t val = pmd_val(pmd);
-
-	val &= ~_PAGE_PRESENT;
-	val |= _PAGE_NUMA;
-
-	return __pmd(val);
-}
-#endif
-
-#ifndef pmdp_set_numa
-static inline void pmdp_set_numa(struct mm_struct *mm, unsigned long addr,
-				 pmd_t *pmdp)
-{
-	pmd_t pmd = *pmdp;
-
-	pmd = pmd_mknuma(pmd);
-	set_pmd_at(mm, addr, pmdp, pmd);
-	return;
-}
-#endif
-#else
-static inline int pmd_numa(pmd_t pmd)
-{
-	return 0;
-}
-
-static inline int pte_numa(pte_t pte)
-{
-	return 0;
-}
-
-static inline pte_t pte_mknonnuma(pte_t pte)
-{
-	return pte;
-}
-
-static inline pmd_t pmd_mknonnuma(pmd_t pmd)
-{
-	return pmd;
-}
-
-static inline pte_t pte_mknuma(pte_t pte)
-{
-	return pte;
-}
-
-static inline void ptep_set_numa(struct mm_struct *mm, unsigned long addr,
-				 pte_t *ptep)
-{
-	return;
-}
-
-
-static inline pmd_t pmd_mknuma(pmd_t pmd)
-{
-	return pmd;
-}
-
-static inline void pmdp_set_numa(struct mm_struct *mm, unsigned long addr,
-				 pmd_t *pmdp)
-{
-	return ;
-}
-#endif /* CONFIG_NUMA_BALANCING */
-
 #endif /* CONFIG_MMU */
 
 #endif /* !__ASSEMBLY__ */
diff --git a/include/linux/swapops.h b/include/linux/swapops.h
index 6adfb7b..2b1fa56 100644
--- a/include/linux/swapops.h
+++ b/include/linux/swapops.h
@@ -54,7 +54,7 @@ static inline pgoff_t swp_offset(swp_entry_t entry)
 /* check whether a pte points to a swap entry */
 static inline int is_swap_pte(pte_t pte)
 {
-	return !pte_none(pte) && !pte_present_nonuma(pte) && !pte_file(pte);
+	return !pte_none(pte) && !pte_file(pte);
 }
 #endif
 
-- 
2.1.2

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox