* Re: [PATCH -next 016/491] KERNEL VIRTUAL MACHINE FOR POWERPC (KVM/powerpc): Use fallthrough;
From: Paul Mackerras @ 2020-03-19 1:18 UTC (permalink / raw)
To: Joe Perches; +Cc: linuxppc-dev, linux-kernel, kvm-ppc
In-Reply-To: <37a5342c67e1b68b9ad06aca8da245b0ff409692.1583896348.git.joe@perches.com>
On Tue, Mar 10, 2020 at 09:51:30PM -0700, Joe Perches wrote:
> Convert the various uses of fallthrough comments to fallthrough;
>
> Done via script
> Link: https://lore.kernel.org/lkml/b56602fcf79f849e733e7b521bb0e17895d390fa.1582230379.git.joe.com/
>
> Signed-off-by: Joe Perches <joe@perches.com>
The subject line should look like "KVM: PPC: Use fallthrough".
Apart from that,
Acked-by: Paul Mackerras <paulus@ozlabs.org>
How are these patches going upstream? Do you want me to take this via
my tree?
Paul.
^ permalink raw reply
* Re: [PATCH v3 3/9] powerpc/vas: Add VAS user space API
From: Daniel Axtens @ 2020-03-19 1:16 UTC (permalink / raw)
To: Haren Myneni, herbert; +Cc: mikey, sukadev, linuxppc-dev, linux-crypto, npiggin
In-Reply-To: <1583541215.9256.35.camel@hbabu-laptop>
Haren Myneni <haren@linux.ibm.com> writes:
> On power9, userspace can send GZIP compression requests directly to NX
> once kernel establishes NX channel / window with VAS. This patch provides
> user space API which allows user space to establish channel using open
> VAS_TX_WIN_OPEN ioctl, mmap and close operations.
>
> Each window corresponds to file descriptor and application can open
> multiple windows. After the window is opened, VAS_TX_WIN_OPEN icoctl to
> open a window on specific VAS instance, mmap() system call to map
> the hardware address of engine's request queue into the application's
> virtual address space.
>
> Then the application can then submit one or more requests to the the
> engine by using the copy/paste instructions and pasting the CRBs to
> the virtual address (aka paste_address) returned by mmap().
>
> Only NX GZIP coprocessor type is supported right now and allow GZIP
> engine access via /dev/crypto/nx-gzip device node.
>
> Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
> Signed-off-by: Haren Myneni <haren@linux.ibm.com>
> ---
> arch/powerpc/include/asm/vas.h | 11 ++
> arch/powerpc/platforms/powernv/Makefile | 2 +-
> arch/powerpc/platforms/powernv/vas-api.c | 290 ++++++++++++++++++++++++++++
> arch/powerpc/platforms/powernv/vas-window.c | 6 +-
> arch/powerpc/platforms/powernv/vas.h | 2 +
> 5 files changed, 307 insertions(+), 4 deletions(-)
> create mode 100644 arch/powerpc/platforms/powernv/vas-api.c
>
> diff --git a/arch/powerpc/include/asm/vas.h b/arch/powerpc/include/asm/vas.h
> index f93e6b0..e064953 100644
> --- a/arch/powerpc/include/asm/vas.h
> +++ b/arch/powerpc/include/asm/vas.h
> @@ -163,4 +163,15 @@ struct vas_window *vas_tx_win_open(int vasid, enum vas_cop_type cop,
> */
> int vas_paste_crb(struct vas_window *win, int offset, bool re);
>
> +/*
> + * Register / unregister coprocessor type to VAS API which will be exported
> + * to user space. Applications can use this API to open / close window
> + * which can be used to send / receive requests directly to cooprcessor.
> + *
> + * Only NX GZIP coprocessor type is supported now, but this API can be
> + * used for others in future.
> + */
> +int vas_register_coproc_api(struct module *mod);
> +void vas_unregister_coproc_api(void);
> +
> #endif /* __ASM_POWERPC_VAS_H */
> diff --git a/arch/powerpc/platforms/powernv/Makefile b/arch/powerpc/platforms/powernv/Makefile
> index 395789f..fe3f0fb 100644
> --- a/arch/powerpc/platforms/powernv/Makefile
> +++ b/arch/powerpc/platforms/powernv/Makefile
> @@ -17,7 +17,7 @@ obj-$(CONFIG_MEMORY_FAILURE) += opal-memory-errors.o
> obj-$(CONFIG_OPAL_PRD) += opal-prd.o
> obj-$(CONFIG_PERF_EVENTS) += opal-imc.o
> obj-$(CONFIG_PPC_MEMTRACE) += memtrace.o
> -obj-$(CONFIG_PPC_VAS) += vas.o vas-window.o vas-debug.o vas-fault.o
> +obj-$(CONFIG_PPC_VAS) += vas.o vas-window.o vas-debug.o vas-fault.o vas-api.o
> obj-$(CONFIG_OCXL_BASE) += ocxl.o
> obj-$(CONFIG_SCOM_DEBUGFS) += opal-xscom.o
> obj-$(CONFIG_PPC_SECURE_BOOT) += opal-secvar.o
> diff --git a/arch/powerpc/platforms/powernv/vas-api.c b/arch/powerpc/platforms/powernv/vas-api.c
> new file mode 100644
> index 0000000..3473a4a
> --- /dev/null
> +++ b/arch/powerpc/platforms/powernv/vas-api.c
> @@ -0,0 +1,290 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * VAS user space API for its accelerators (Only NX-GZIP is supported now)
> + * Copyright (C) 2019 Haren Myneni, IBM Corp
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/device.h>
> +#include <linux/cdev.h>
> +#include <linux/fs.h>
> +#include <linux/slab.h>
> +#include <linux/uaccess.h>
> +#include <asm/vas.h>
> +#include <uapi/asm/vas-api.h>
> +#include "vas.h"
> +
> +/*
> + * The driver creates the device node that can be used as follows:
> + * For NX-GZIP
> + *
> + * fd = open("/dev/crypto/nx-gzip", O_RDWR);
> + * rc = ioctl(fd, VAS_TX_WIN_OPEN, &attr);
> + * paste_addr = mmap(NULL, PAGE_SIZE, prot, MAP_SHARED, fd, 0ULL).
> + * vas_copy(&crb, 0, 1);
> + * vas_paste(paste_addr, 0, 1);
> + * close(fd) or exit process to close window.
> + *
> + * where "vas_copy" and "vas_paste" are defined in copy-paste.h.
> + * copy/paste returns to the user space directly. So refer NX hardware
> + * documententation for excat copy/paste usage and completion / error
> + * conditions.
> + */
> +
> +static char *coproc_dev_name = "nx-gzip";
> +static atomic_t coproc_instid = ATOMIC_INIT(0);
> +
> +/*
> + * Wrapper object for the nx-gzip device - there is just one instance of
> + * this node for the whole system.
> + */
> +static struct coproc_dev {
> + struct cdev cdev;
> + struct device *device;
> + char *name;
> + dev_t devt;
> + struct class *class;
> +} coproc_device;
> +
> +/*
> + * One instance per open of a nx-gzip device. Each coproc_instance is
> + * associated with a VAS window after the caller issues
> + * VAS_GZIP_TX_WIN_OPEN ioctl.
> + */
> +struct coproc_instance {
> + int id;
> + struct vas_window *txwin;
> +};
> +
> +static char *coproc_devnode(struct device *dev, umode_t *mode)
> +{
> + return kasprintf(GFP_KERNEL, "crypto/%s", dev_name(dev));
> +}
> +
> +static int coproc_open(struct inode *inode, struct file *fp)
> +{
> + struct coproc_instance *instance;
> +
> + instance = kzalloc(sizeof(*instance), GFP_KERNEL);
> + if (!instance)
> + return -ENOMEM;
> +
> + instance->id = atomic_inc_return(&coproc_instid);
> +
> + fp->private_data = instance;
> + return 0;
> +}
> +
> +static int coproc_ioc_tx_win_open(struct file *fp, unsigned long arg)
> +{
> + int rc, vasid;
> + struct vas_tx_win_attr txattr;
> + struct vas_tx_win_open_attr uattr;
> + void __user *uptr = (void __user *)arg;
> + struct vas_window *txwin;
> + struct coproc_instance *nxti = fp->private_data;
> +
> + if (!nxti)
> + return -EINVAL;
> +
> + /*
> + * One window for file descriptor
> + */
> + if (nxti->txwin)
> + return -EEXIST;
> +
> + rc = copy_from_user(&uattr, uptr, sizeof(uattr));
> + if (rc) {
> + pr_err("%s(): copy_from_user() returns %d\n", __func__, rc);
> + return -EFAULT;
> + }
> +
> + if (uattr.version != 1) {
> + pr_err("Invalid version\n");
> + return -EINVAL;
> + }
> +
> + vasid = uattr.vas_id;
> +
> + memset(&txattr, 0, sizeof(struct vas_tx_win_attr));
> + vas_init_tx_win_attr(&txattr, VAS_COP_TYPE_GZIP);
> +
> + txattr.lpid = mfspr(SPRN_LPID);
> + txattr.pidr = mfspr(SPRN_PID);
> + txattr.user_win = true;
> + txattr.rsvd_txbuf_count = false;
> + txattr.pswid = false;
> + /*
> + * txattr.wcreds_max is set to VAS_WCREDS_DEFAULT (1024) in
> + * vas-window.c, but can be changed specific to GZIP depends
> + * on user space need.
> + * If needed to set txattr.wcreds_max here.
> + */
> +
> + pr_devel("Pid %d: Opening txwin, PIDR %ld\n", txattr.pidr,
> + mfspr(SPRN_PID));
> +
> + txwin = vas_tx_win_open(vasid, VAS_COP_TYPE_GZIP, &txattr);
> + if (IS_ERR(txwin)) {
> + pr_err("%s() vas_tx_win_open() failed, %ld\n", __func__,
> + PTR_ERR(txwin));
> + return PTR_ERR(txwin);
> + }
> +
> + nxti->txwin = txwin;
> +
> + return 0;
> +}
> +
> +static int coproc_release(struct inode *inode, struct file *fp)
> +{
> + struct coproc_instance *instance;
> +
> + instance = fp->private_data;
> +
> + if (instance && instance->txwin) {
> + vas_win_close(instance->txwin);
> + instance->txwin = NULL;
> + }
> +
> + /*
> + * We don't know here if user has other receive windows
> + * open, so we can't really call clear_thread_tidr().
> + * So, once the process calls set_thread_tidr(), the
> + * TIDR value sticks around until process exits, resulting
> + * in an extra copy in restore_sprs().
> + */
> +
> + kfree(instance);
> + fp->private_data = NULL;
> + atomic_dec(&coproc_instid);
> +
> + return 0;
> +}
> +
> +static int coproc_mmap(struct file *fp, struct vm_area_struct *vma)
> +{
> + int rc;
> + pgprot_t prot;
> + u64 paste_addr;
> + unsigned long pfn;
> + struct coproc_instance *instance = fp->private_data;
> +
> + if ((vma->vm_end - vma->vm_start) > PAGE_SIZE) {
> + pr_debug("%s(): size 0x%zx, PAGE_SIZE 0x%zx\n", __func__,
> + (vma->vm_end - vma->vm_start), PAGE_SIZE);
> + return -EINVAL;
> + }
> +
> + /* Ensure instance has an open send window */
> + if (!instance->txwin) {
> + pr_err("%s(): No send window open?\n", __func__);
> + return -EINVAL;
> + }
> +
> + vas_win_paste_addr(instance->txwin, &paste_addr, NULL);
> + pfn = paste_addr >> PAGE_SHIFT;
> +
> + /* flags, page_prot from cxl_mmap(), except we want cachable */
> + vma->vm_flags |= VM_IO | VM_PFNMAP;
> + vma->vm_page_prot = pgprot_cached(vma->vm_page_prot);
> +
> + prot = __pgprot(pgprot_val(vma->vm_page_prot) | _PAGE_DIRTY);
> +
> + rc = remap_pfn_range(vma, vma->vm_start, pfn + vma->vm_pgoff,
> + vma->vm_end - vma->vm_start, prot);
> +
> + pr_devel("%s(): paste addr %llx at %lx, rc %d\n", __func__,
> + paste_addr, vma->vm_start, rc);
> +
> + return rc;
> +}
> +
> +static long coproc_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
> +{
> + switch (cmd) {
> + case VAS_TX_WIN_OPEN:
> + return coproc_ioc_tx_win_open(fp, arg);
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static struct file_operations coproc_fops = {
> + .open = coproc_open,
> + .release = coproc_release,
> + .mmap = coproc_mmap,
> + .unlocked_ioctl = coproc_ioctl,
> +};
> +
checkpatch didn't run on this series via snowpatch because it doesn't
understand that the other series needs to be applied first. So I ran
checkpatch myself, and it reported:
WARNING: struct file_operations should normally be const
#287: FILE: arch/powerpc/platforms/powernv/vas-api.c:213:
+static struct file_operations coproc_fops = {
> +/*
> + * Supporting only nx-gzip coprocessor type now, but this API code
> + * extended to other coprocessor types later.
> + */
> +int vas_register_coproc_api(struct module *mod)
> +{
> + int rc = -EINVAL;
> + dev_t devno;
> +
> + rc = alloc_chrdev_region(&coproc_device.devt, 1, 1, "nx-gzip");
> + if (rc) {
> + pr_err("Unable to allocate coproc major number: %i\n", rc);
> + return rc;
> + }
> +
> + pr_devel("NX-GZIP device allocated, dev [%i,%i]\n",
> + MAJOR(coproc_device.devt), MINOR(coproc_device.devt));
> +
> + coproc_device.class = class_create(mod, "nx-gzip");
> + if (IS_ERR(coproc_device.class)) {
> + rc = PTR_ERR(coproc_device.class);
> + pr_err("Unable to create NX-GZIP class %d\n", rc);
> + goto err_class;
> + }
> + coproc_device.class->devnode = coproc_devnode;
> +
> + coproc_fops.owner = mod;
> + cdev_init(&coproc_device.cdev, &coproc_fops);
> +
> + devno = MKDEV(MAJOR(coproc_device.devt), 0);
> + rc = cdev_add(&coproc_device.cdev, devno, 1);
> + if (rc) {
> + pr_err("cdev_add() failed %d\n", rc);
> + goto err_cdev;
> + }
> +
> + coproc_device.device = device_create(coproc_device.class, NULL,
> + devno, NULL, coproc_dev_name, MINOR(devno));
> + if (IS_ERR(coproc_device.device)) {
> + rc = PTR_ERR(coproc_device.device);
> + pr_err("Unable to create coproc-%d %d\n", MINOR(devno), rc);
> + goto err;
> + }
> +
> + pr_devel("%s: Added dev [%d,%d]\n", __func__, MAJOR(devno),
> + MINOR(devno));
> +
> + return 0;
> +
> +err:
> + cdev_del(&coproc_device.cdev);
> +err_cdev:
> + class_destroy(coproc_device.class);
> +err_class:
> + unregister_chrdev_region(coproc_device.devt, 1);
> + return rc;
> +}
> +EXPORT_SYMBOL_GPL(vas_register_coproc_api);
> +
> +void vas_unregister_coproc_api(void)
> +{
> + dev_t devno;
> +
> + cdev_del(&coproc_device.cdev);
> + devno = MKDEV(MAJOR(coproc_device.devt), 0);
> + device_destroy(coproc_device.class, devno);
> +
> + class_destroy(coproc_device.class);
> + unregister_chrdev_region(coproc_device.devt, 1);
> +}
> +EXPORT_SYMBOL_GPL(vas_unregister_coproc_api);
> diff --git a/arch/powerpc/platforms/powernv/vas-window.c b/arch/powerpc/platforms/powernv/vas-window.c
> index e9ab851..7484296 100644
> --- a/arch/powerpc/platforms/powernv/vas-window.c
> +++ b/arch/powerpc/platforms/powernv/vas-window.c
> @@ -26,7 +26,7 @@
> * Compute the paste address region for the window @window using the
> * ->paste_base_addr and ->paste_win_id_shift we got from device tree.
> */
> -static void compute_paste_address(struct vas_window *window, u64 *addr, int *len)
> +void vas_win_paste_addr(struct vas_window *window, u64 *addr, int *len)
> {
> int winid;
> u64 base, shift;
> @@ -80,7 +80,7 @@ static void *map_paste_region(struct vas_window *txwin)
> goto free_name;
>
> txwin->paste_addr_name = name;
> - compute_paste_address(txwin, &start, &len);
> + vas_win_paste_addr(txwin, &start, &len);
>
> if (!request_mem_region(start, len, name)) {
> pr_devel("%s(): request_mem_region(0x%llx, %d) failed\n",
> @@ -138,7 +138,7 @@ static void unmap_paste_region(struct vas_window *window)
> u64 busaddr_start;
>
> if (window->paste_kaddr) {
> - compute_paste_address(window, &busaddr_start, &len);
> + vas_win_paste_addr(window, &busaddr_start, &len);
> unmap_region(window->paste_kaddr, busaddr_start, len);
> window->paste_kaddr = NULL;
> kfree(window->paste_addr_name);
> diff --git a/arch/powerpc/platforms/powernv/vas.h b/arch/powerpc/platforms/powernv/vas.h
> index 8c39a7d..a10abed 100644
> --- a/arch/powerpc/platforms/powernv/vas.h
> +++ b/arch/powerpc/platforms/powernv/vas.h
> @@ -431,6 +431,8 @@ struct vas_winctx {
> extern void vas_return_credit(struct vas_window *window, bool tx);
> extern struct vas_window *vas_pswid_to_window(struct vas_instance *vinst,
> uint32_t pswid);
> +extern void vas_win_paste_addr(struct vas_window *window, u64 *addr,
> + int *len);
>
> static inline int vas_window_pid(struct vas_window *window)
> {
> --
> 1.8.3.1
^ permalink raw reply
* Re: [PATCH v2 1/4] mm: Check for node_online in node_present_pages
From: Michael Ellerman @ 2020-03-19 1:11 UTC (permalink / raw)
To: Vlastimil Babka, Michal Hocko, Srikar Dronamraju
Cc: Sachin Sant, Nathan Lynch, Bharata B Rao, linux-mm, Kirill Tkhai,
Mel Gorman, Joonsoo Kim, Andrew Morton, linuxppc-dev,
Christopher Lameter
In-Reply-To: <87tv2ldw1k.fsf@mpe.ellerman.id.au>
Michael Ellerman <mpe@ellerman.id.au> writes:
> Vlastimil Babka <vbabka@suse.cz> writes:
>> On 3/18/20 11:02 AM, Michal Hocko wrote:
>>> On Wed 18-03-20 12:58:07, Srikar Dronamraju wrote:
>>>> Calling a kmalloc_node on a possible node which is not yet onlined can
>>>> lead to panic. Currently node_present_pages() doesn't verify the node is
>>>> online before accessing the pgdat for the node. However pgdat struct may
>>>> not be available resulting in a crash.
>>>>
>>>> NIP [c0000000003d55f4] ___slab_alloc+0x1f4/0x760
>>>> LR [c0000000003d5b94] __slab_alloc+0x34/0x60
>>>> Call Trace:
>>>> [c0000008b3783960] [c0000000003d5734] ___slab_alloc+0x334/0x760 (unreliable)
>>>> [c0000008b3783a40] [c0000000003d5b94] __slab_alloc+0x34/0x60
>>>> [c0000008b3783a70] [c0000000003d6fa0] __kmalloc_node+0x110/0x490
>>>> [c0000008b3783af0] [c0000000003443d8] kvmalloc_node+0x58/0x110
>>>> [c0000008b3783b30] [c0000000003fee38] mem_cgroup_css_online+0x108/0x270
>>>> [c0000008b3783b90] [c000000000235aa8] online_css+0x48/0xd0
>>>> [c0000008b3783bc0] [c00000000023eaec] cgroup_apply_control_enable+0x2ec/0x4d0
>>>> [c0000008b3783ca0] [c000000000242318] cgroup_mkdir+0x228/0x5f0
>>>> [c0000008b3783d10] [c00000000051e170] kernfs_iop_mkdir+0x90/0xf0
>>>> [c0000008b3783d50] [c00000000043dc00] vfs_mkdir+0x110/0x230
>>>> [c0000008b3783da0] [c000000000441c90] do_mkdirat+0xb0/0x1a0
>>>> [c0000008b3783e20] [c00000000000b278] system_call+0x5c/0x68
>>>>
>>>> Fix this by verifying the node is online before accessing the pgdat
>>>> structure. Fix the same for node_spanned_pages() too.
>>>>
>>>> Cc: Andrew Morton <akpm@linux-foundation.org>
>>>> Cc: linux-mm@kvack.org
>>>> Cc: Mel Gorman <mgorman@suse.de>
>>>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>>>> Cc: Sachin Sant <sachinp@linux.vnet.ibm.com>
>>>> Cc: Michal Hocko <mhocko@kernel.org>
>>>> Cc: Christopher Lameter <cl@linux.com>
>>>> Cc: linuxppc-dev@lists.ozlabs.org
>>>> Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
>>>> Cc: Kirill Tkhai <ktkhai@virtuozzo.com>
>>>> Cc: Vlastimil Babka <vbabka@suse.cz>
>>>> Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
>>>> Cc: Bharata B Rao <bharata@linux.ibm.com>
>>>> Cc: Nathan Lynch <nathanl@linux.ibm.com>
>>>>
>>>> Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
>>>> Tested-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
>>>> Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
>>>> ---
>>>> include/linux/mmzone.h | 6 ++++--
>>>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
>>>> index f3f264826423..88078a3b95e5 100644
>>>> --- a/include/linux/mmzone.h
>>>> +++ b/include/linux/mmzone.h
>>>> @@ -756,8 +756,10 @@ typedef struct pglist_data {
>>>> atomic_long_t vm_stat[NR_VM_NODE_STAT_ITEMS];
>>>> } pg_data_t;
>>>>
>>>> -#define node_present_pages(nid) (NODE_DATA(nid)->node_present_pages)
>>>> -#define node_spanned_pages(nid) (NODE_DATA(nid)->node_spanned_pages)
>>>> +#define node_present_pages(nid) \
>>>> + (node_online(nid) ? NODE_DATA(nid)->node_present_pages : 0)
>>>> +#define node_spanned_pages(nid) \
>>>> + (node_online(nid) ? NODE_DATA(nid)->node_spanned_pages : 0)
>>>
>>> I believe this is a wrong approach. We really do not want to special
>>> case all the places which require NODE_DATA. Can we please go and
>>> allocate pgdat for all possible nodes?
>>>
>>> The current state of memory less hacks subtle bugs poping up here and
>>> there just prove that we should have done that from the very begining
>>> IMHO.
>>
>> Yes. So here's an alternative proposal for fixing the current situation in SLUB,
>> before the long-term solution of having all possible nodes provide valid pgdat
>> with zonelists:
>>
>> - fix SLUB with the hunk at the end of this mail - the point is to use NUMA_NO_NODE
>> as fallback instead of node_to_mem_node()
>> - this removes all uses of node_to_mem_node (luckily it's just SLUB),
>> kill it completely instead of trying to fix it up
>> - patch 1/4 is not needed with the fix
>> - perhaps many of your other patches are alss not needed
>> - once we get the long-term solution, some of the !node_online() checks can be removed
>
> Seems like a nice solution to me :)
>
>> ----8<----
>> diff --git a/mm/slub.c b/mm/slub.c
>> index 17dc00e33115..1d4f2d7a0080 100644
>> --- a/mm/slub.c
>> +++ b/mm/slub.c
>> @@ -1511,7 +1511,7 @@ static inline struct page *alloc_slab_page(struct kmem_cache *s,
>> struct page *page;
>> unsigned int order = oo_order(oo);
>>
>> - if (node == NUMA_NO_NODE)
>> + if (node == NUMA_NO_NODE || !node_online(node))
>
> Why don't we need the node_present_pages() check here?
>
>> page = alloc_pages(flags, order);
>> else
>> page = __alloc_pages_node(node, flags, order);
>> @@ -1973,8 +1973,6 @@ static void *get_partial(struct kmem_cache *s, gfp_t flags, int node,
>>
>> if (node == NUMA_NO_NODE)
>> searchnode = numa_mem_id();
>> - else if (!node_present_pages(node))
>> - searchnode = node_to_mem_node(node);
>>
>> object = get_partial_node(s, get_node(s, searchnode), c, flags);
>> if (object || node != NUMA_NO_NODE)
>> @@ -2568,12 +2566,15 @@ static void *___slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node,
>> redo:
>>
>> if (unlikely(!node_match(page, node))) {
>> - int searchnode = node;
>> -
>> - if (node != NUMA_NO_NODE && !node_present_pages(node))
>> - searchnode = node_to_mem_node(node);
>> -
>> - if (unlikely(!node_match(page, searchnode))) {
>> + /*
>> + * node_match() false implies node != NUMA_NO_NODE
>> + * but if the node is not online and has no pages, just
> ^
> this should be 'or' ?
Sorry I see you've already fixed this in the version you posted.
cheers
^ permalink raw reply
* Re: [PATCH] powerpc/vdso: Fix multiple issues with sys_call_table
From: Michael Ellerman @ 2020-03-19 1:10 UTC (permalink / raw)
To: Anton Blanchard, linuxppc-dev, linux-kernel; +Cc: Nicholas Piggin
In-Reply-To: <20200306135705.7f80fcad@kryten.localdomain>
Anton Blanchard <anton@ozlabs.org> writes:
> The VDSO exports a bitmap of valid syscalls. vdso_setup_syscall_map()
> sets this up, but there are both little and big endian bugs. The issue
> is with:
>
> if (sys_call_table[i] != sys_ni_syscall)
>
> On little endian, instead of comparing pointers to the two functions,
> we compare the first two instructions of each function. If a function
> happens to have the same first two instructions as sys_ni_syscall, then
> we have a spurious match and mark the instruction as not implemented.
> Fix this by removing the inline declarations.
>
> On big endian we have a further issue where sys_ni_syscall is a function
> descriptor and sys_call_table[] holds pointers to the instruction text.
> Fix this by using dereference_kernel_function_descriptor().
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Anton Blanchard <anton@ozlabs.org>
That's some pretty epic breakage.
Is it even worth keeping, or should we just rip it out and declare that
the syscall map is junk? Userspace can hardly rely on it given it's been
this broken for so long.
If not it would be really nice to have a selftest of this stuff so we
can verify it works and not break it again in future.
cheers
> ---
> diff --git a/arch/powerpc/kernel/vdso.c b/arch/powerpc/kernel/vdso.c
> index b9a108411c0d..d186b729026e 100644
> --- a/arch/powerpc/kernel/vdso.c
> +++ b/arch/powerpc/kernel/vdso.c
> @@ -17,6 +17,7 @@
> #include <linux/elf.h>
> #include <linux/security.h>
> #include <linux/memblock.h>
> +#include <linux/syscalls.h>
>
> #include <asm/pgtable.h>
> #include <asm/processor.h>
> @@ -30,6 +31,7 @@
> #include <asm/vdso.h>
> #include <asm/vdso_datapage.h>
> #include <asm/setup.h>
> +#include <asm/syscall.h>
>
> #undef DEBUG
>
> @@ -644,19 +646,16 @@ static __init int vdso_setup(void)
> static void __init vdso_setup_syscall_map(void)
> {
> unsigned int i;
> - extern unsigned long *sys_call_table;
> -#ifdef CONFIG_PPC64
> - extern unsigned long *compat_sys_call_table;
> -#endif
> - extern unsigned long sys_ni_syscall;
> + unsigned long ni_syscall;
>
> + ni_syscall = (unsigned long)dereference_kernel_function_descriptor(sys_ni_syscall);
>
> for (i = 0; i < NR_syscalls; i++) {
> #ifdef CONFIG_PPC64
> - if (sys_call_table[i] != sys_ni_syscall)
> + if (sys_call_table[i] != ni_syscall)
> vdso_data->syscall_map_64[i >> 5] |=
> 0x80000000UL >> (i & 0x1f);
> - if (compat_sys_call_table[i] != sys_ni_syscall)
> + if (compat_sys_call_table[i] != ni_syscall)
> vdso_data->syscall_map_32[i >> 5] |=
> 0x80000000UL >> (i & 0x1f);
> #else /* CONFIG_PPC64 */
^ permalink raw reply
* [PATCH v2] qtpm2: Export tpm2_get_cc_attrs_tbl for ibmvtpm driver as module
From: Stefan Berger @ 2020-03-19 1:00 UTC (permalink / raw)
To: jarkko.sakkinen, linux-integrity
Cc: sachinp, linux-kernel, linux-next, linuxppc-dev, Stefan Berger
From: Stefan Berger <stefanb@linux.ibm.com>
This patch fixes the following problem when the ibmvtpm driver
is built as a module:
ERROR: modpost: "tpm2_get_cc_attrs_tbl" [drivers/char/tpm/tpm_ibmvtpm.ko] undefined!
make[1]: *** [scripts/Makefile.modpost:94: __modpost] Error 1
make: *** [Makefile:1298: modules] Error 2
Fixes: 18b3670d79ae ("tpm: ibmvtpm: Add support for TPM2")
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
Tested-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
---
drivers/char/tpm/tpm2-cmd.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 76f67b155bd5..eff1f12d981a 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -681,6 +681,7 @@ int tpm2_get_cc_attrs_tbl(struct tpm_chip *chip)
rc = -ENODEV;
return rc;
}
+EXPORT_SYMBOL_GPL(tpm2_get_cc_attrs_tbl);
/**
* tpm2_startup - turn on the TPM
--
2.23.0
^ permalink raw reply related
* Re: [PATCH 1/4] hugetlbfs: add arch_hugetlb_valid_size
From: kbuild test robot @ 2020-03-19 0:48 UTC (permalink / raw)
To: Mike Kravetz
Cc: linux-s390, kbuild-all, linux-doc, linux-kernel, linux-mm,
sparclinux, linux-riscv, linuxppc-dev, linux-arm-kernel
In-Reply-To: <20200318220634.32100-2-mike.kravetz@oracle.com>
[-- Attachment #1: Type: text/plain, Size: 4748 bytes --]
Hi Mike,
I love your patch! Yet something to improve:
[auto build test ERROR on next-20200318]
[also build test ERROR on v5.6-rc6]
[cannot apply to arm64/for-next/core powerpc/next sparc/master linus/master sparc-next/master v5.6-rc6 v5.6-rc5 v5.6-rc4]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Mike-Kravetz/Clean-up-hugetlb-boot-command-line-processing/20200319-060943
base: 47780d7892b77e922bbe19b5dea99cde06b2f0e5
config: riscv-allyesconfig (attached as .config)
compiler: riscv64-linux-gcc (GCC) 9.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=9.2.0 make.cross ARCH=riscv
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All error/warnings (new ones prefixed by >>):
arch/riscv/mm/hugetlbpage.c: In function 'arch_hugetlb_valid_size':
>> arch/riscv/mm/hugetlbpage.c:19:39: error: 'ps' undeclared (first use in this function)
19 | else if (IS_ENABLED(CONFIG_64BIT) && ps == PUD_SIZE)
| ^~
arch/riscv/mm/hugetlbpage.c:19:39: note: each undeclared identifier is reported only once for each function it appears in
>> arch/riscv/mm/hugetlbpage.c:20:3: error: 'retrurn' undeclared (first use in this function)
20 | retrurn true;
| ^~~~~~~
>> arch/riscv/mm/hugetlbpage.c:20:10: error: expected ';' before 'true'
20 | retrurn true;
| ^~~~~
| ;
In file included from include/linux/printk.h:7,
from include/linux/kernel.h:15,
from include/asm-generic/bug.h:19,
from arch/riscv/include/asm/bug.h:75,
from include/linux/bug.h:5,
from arch/riscv/include/asm/cmpxchg.h:9,
from arch/riscv/include/asm/atomic.h:19,
from include/linux/atomic.h:7,
from include/linux/mm_types_task.h:13,
from include/linux/mm_types.h:5,
from include/linux/hugetlb.h:5,
from arch/riscv/mm/hugetlbpage.c:2:
arch/riscv/mm/hugetlbpage.c: In function 'setup_hugepagesz':
include/linux/kern_levels.h:5:18: warning: format '%lu' expects argument of type 'long unsigned int', but argument 2 has type 'long long unsigned int' [-Wformat=]
5 | #define KERN_SOH "\001" /* ASCII Start Of Header */
| ^~~~~~
include/linux/kern_levels.h:11:18: note: in expansion of macro 'KERN_SOH'
11 | #define KERN_ERR KERN_SOH "3" /* error conditions */
| ^~~~~~~~
include/linux/printk.h:304:9: note: in expansion of macro 'KERN_ERR'
304 | printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~~
arch/riscv/mm/hugetlbpage.c:35:2: note: in expansion of macro 'pr_err'
35 | pr_err("hugepagesz: Unsupported page size %lu M\n", ps >> 20);
| ^~~~~~
arch/riscv/mm/hugetlbpage.c:35:46: note: format string is defined here
35 | pr_err("hugepagesz: Unsupported page size %lu M\n", ps >> 20);
| ~~^
| |
| long unsigned int
| %llu
arch/riscv/mm/hugetlbpage.c: In function 'arch_hugetlb_valid_size':
>> arch/riscv/mm/hugetlbpage.c:23:1: warning: control reaches end of non-void function [-Wreturn-type]
23 | }
| ^
vim +/ps +19 arch/riscv/mm/hugetlbpage.c
14
15 bool __init arch_hugetlb_valid_size(unsigned long long size)
16 {
17 if (size == HPAGE_SIZE)
18 return true;
> 19 else if (IS_ENABLED(CONFIG_64BIT) && ps == PUD_SIZE)
> 20 retrurn true;
21 else
22 return false;
> 23 }
24
25 static __init int setup_hugepagesz(char *opt)
26 {
27 unsigned long long ps = memparse(opt, &opt);
28
29 if (arch_hugetlb_valid_size(ps)) {
30 hugetlb_add_hstate(ilog2(ps) - PAGE_SHIFT);
31 return 1;
32 }
33
34 hugetlb_bad_size();
> 35 pr_err("hugepagesz: Unsupported page size %lu M\n", ps >> 20);
36 return 0;
37
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 62790 bytes --]
^ permalink raw reply
* Re: [patch V2 11/15] completion: Use simple wait queues
From: Thomas Gleixner @ 2020-03-19 0:44 UTC (permalink / raw)
To: Joel Fernandes
Cc: Randy Dunlap, Peter Zijlstra, linux-pci,
Sebastian Andrzej Siewior, Oleg Nesterov, Will Deacon,
Ingo Molnar, Davidlohr Bueso, Paul E . McKenney, Logan Gunthorpe,
Arnd Bergmann, linuxppc-dev, Steven Rostedt, Bjorn Helgaas,
Kurt Schwemmer, Kalle Valo, Felipe Balbi, Greg Kroah-Hartman,
linux-usb, linux-wireless, LKML, netdev, Linus Torvalds,
David S. Miller
In-Reply-To: <20200319003351.GA211584@google.com>
Joel,
Joel Fernandes <joel@joelfernandes.org> writes:
> On Wed, Mar 18, 2020 at 09:43:13PM +0100, Thomas Gleixner wrote:
>> The spinlock in the wait queue head cannot be replaced by a raw_spinlock
>> because:
>>
>> - wait queues can have custom wakeup callbacks, which acquire other
>> spinlock_t locks and have potentially long execution times
>
> Cool, makes sense.
>
>> - wake_up() walks an unbounded number of list entries during the wake up
>> and may wake an unbounded number of waiters.
>
> Just to clarify here, wake_up() will really wake up just 1 waiter if all the
> waiters on the queue are exclusive right? So in such scenario at least, the
> "unbounded number of waiters" would not be an issue if everything waiting was
> exclusive and waitqueue with wake_up() was used. Please correct me if I'm
> wrong about that though.
Correct.
> So the main reasons to avoid waitqueue in favor of swait (as you mentioned)
> would be the sleep-while-atomic issue in truly atomic context on RT, and the
> fact that callbacks can take a long time.
Yes.
Thanks,
tglx
^ permalink raw reply
* Re: [patch V2 11/15] completion: Use simple wait queues
From: Joel Fernandes @ 2020-03-19 0:33 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Randy Dunlap, Peter Zijlstra, linux-pci,
Sebastian Andrzej Siewior, Oleg Nesterov, Will Deacon,
Ingo Molnar, Davidlohr Bueso, Paul E . McKenney, Logan Gunthorpe,
Arnd Bergmann, linuxppc-dev, Steven Rostedt, Bjorn Helgaas,
Kurt Schwemmer, Kalle Valo, Felipe Balbi, Greg Kroah-Hartman,
linux-usb, linux-wireless, LKML, netdev, Linus Torvalds,
David S. Miller
In-Reply-To: <20200318204408.521507446@linutronix.de>
Hi Thomas,
On Wed, Mar 18, 2020 at 09:43:13PM +0100, Thomas Gleixner wrote:
> From: Thomas Gleixner <tglx@linutronix.de>
>
> completion uses a wait_queue_head_t to enqueue waiters.
>
> wait_queue_head_t contains a spinlock_t to protect the list of waiters
> which excludes it from being used in truly atomic context on a PREEMPT_RT
> enabled kernel.
>
> The spinlock in the wait queue head cannot be replaced by a raw_spinlock
> because:
>
> - wait queues can have custom wakeup callbacks, which acquire other
> spinlock_t locks and have potentially long execution times
Cool, makes sense.
> - wake_up() walks an unbounded number of list entries during the wake up
> and may wake an unbounded number of waiters.
Just to clarify here, wake_up() will really wake up just 1 waiter if all the
waiters on the queue are exclusive right? So in such scenario at least, the
"unbounded number of waiters" would not be an issue if everything waiting was
exclusive and waitqueue with wake_up() was used. Please correct me if I'm
wrong about that though.
So the main reasons to avoid waitqueue in favor of swait (as you mentioned)
would be the sleep-while-atomic issue in truly atomic context on RT, and the
fact that callbacks can take a long time.
>
> For simplicity and performance reasons complete() should be usable on
> PREEMPT_RT enabled kernels.
>
> completions do not use custom wakeup callbacks and are usually single
> waiter, except for a few corner cases.
>
> Replace the wait queue in the completion with a simple wait queue (swait),
> which uses a raw_spinlock_t for protecting the waiter list and therefore is
> safe to use inside truly atomic regions on PREEMPT_RT.
>
> There is no semantical or functional change:
>
> - completions use the exclusive wait mode which is what swait provides
>
> - complete() wakes one exclusive waiter
>
> - complete_all() wakes all waiters while holding the lock which protects
> the wait queue against newly incoming waiters. The conversion to swait
> preserves this behaviour.
>
> complete_all() might cause unbound latencies with a large number of waiters
> being woken at once, but most complete_all() usage sites are either in
> testing or initialization code or have only a really small number of
> concurrent waiters which for now does not cause a latency problem. Keep it
> simple for now.
>
> The fixup of the warning check in the USB gadget driver is just a straight
> forward conversion of the lockless waiter check from one waitqueue type to
> the other.
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org>
thanks,
- Joel
> ---
> V2: Split out the orinoco and usb gadget parts and amended change log
> ---
> drivers/usb/gadget/function/f_fs.c | 2 +-
> include/linux/completion.h | 8 ++++----
> kernel/sched/completion.c | 36 +++++++++++++++++++-----------------
> 3 files changed, 24 insertions(+), 22 deletions(-)
>
> --- a/drivers/usb/gadget/function/f_fs.c
> +++ b/drivers/usb/gadget/function/f_fs.c
> @@ -1703,7 +1703,7 @@ static void ffs_data_put(struct ffs_data
> pr_info("%s(): freeing\n", __func__);
> ffs_data_clear(ffs);
> BUG_ON(waitqueue_active(&ffs->ev.waitq) ||
> - waitqueue_active(&ffs->ep0req_completion.wait) ||
> + swait_active(&ffs->ep0req_completion.wait) ||
> waitqueue_active(&ffs->wait));
> destroy_workqueue(ffs->io_completion_wq);
> kfree(ffs->dev_name);
> --- a/include/linux/completion.h
> +++ b/include/linux/completion.h
> @@ -9,7 +9,7 @@
> * See kernel/sched/completion.c for details.
> */
>
> -#include <linux/wait.h>
> +#include <linux/swait.h>
>
> /*
> * struct completion - structure used to maintain state for a "completion"
> @@ -25,7 +25,7 @@
> */
> struct completion {
> unsigned int done;
> - wait_queue_head_t wait;
> + struct swait_queue_head wait;
> };
>
> #define init_completion_map(x, m) __init_completion(x)
> @@ -34,7 +34,7 @@ static inline void complete_acquire(stru
> static inline void complete_release(struct completion *x) {}
>
> #define COMPLETION_INITIALIZER(work) \
> - { 0, __WAIT_QUEUE_HEAD_INITIALIZER((work).wait) }
> + { 0, __SWAIT_QUEUE_HEAD_INITIALIZER((work).wait) }
>
> #define COMPLETION_INITIALIZER_ONSTACK_MAP(work, map) \
> (*({ init_completion_map(&(work), &(map)); &(work); }))
> @@ -85,7 +85,7 @@ static inline void complete_release(stru
> static inline void __init_completion(struct completion *x)
> {
> x->done = 0;
> - init_waitqueue_head(&x->wait);
> + init_swait_queue_head(&x->wait);
> }
>
> /**
> --- a/kernel/sched/completion.c
> +++ b/kernel/sched/completion.c
> @@ -29,12 +29,12 @@ void complete(struct completion *x)
> {
> unsigned long flags;
>
> - spin_lock_irqsave(&x->wait.lock, flags);
> + raw_spin_lock_irqsave(&x->wait.lock, flags);
>
> if (x->done != UINT_MAX)
> x->done++;
> - __wake_up_locked(&x->wait, TASK_NORMAL, 1);
> - spin_unlock_irqrestore(&x->wait.lock, flags);
> + swake_up_locked(&x->wait);
> + raw_spin_unlock_irqrestore(&x->wait.lock, flags);
> }
> EXPORT_SYMBOL(complete);
>
> @@ -58,10 +58,12 @@ void complete_all(struct completion *x)
> {
> unsigned long flags;
>
> - spin_lock_irqsave(&x->wait.lock, flags);
> + WARN_ON(irqs_disabled());
> +
> + raw_spin_lock_irqsave(&x->wait.lock, flags);
> x->done = UINT_MAX;
> - __wake_up_locked(&x->wait, TASK_NORMAL, 0);
> - spin_unlock_irqrestore(&x->wait.lock, flags);
> + swake_up_all_locked(&x->wait);
> + raw_spin_unlock_irqrestore(&x->wait.lock, flags);
> }
> EXPORT_SYMBOL(complete_all);
>
> @@ -70,20 +72,20 @@ do_wait_for_common(struct completion *x,
> long (*action)(long), long timeout, int state)
> {
> if (!x->done) {
> - DECLARE_WAITQUEUE(wait, current);
> + DECLARE_SWAITQUEUE(wait);
>
> - __add_wait_queue_entry_tail_exclusive(&x->wait, &wait);
> do {
> if (signal_pending_state(state, current)) {
> timeout = -ERESTARTSYS;
> break;
> }
> + __prepare_to_swait(&x->wait, &wait);
> __set_current_state(state);
> - spin_unlock_irq(&x->wait.lock);
> + raw_spin_unlock_irq(&x->wait.lock);
> timeout = action(timeout);
> - spin_lock_irq(&x->wait.lock);
> + raw_spin_lock_irq(&x->wait.lock);
> } while (!x->done && timeout);
> - __remove_wait_queue(&x->wait, &wait);
> + __finish_swait(&x->wait, &wait);
> if (!x->done)
> return timeout;
> }
> @@ -100,9 +102,9 @@ static inline long __sched
>
> complete_acquire(x);
>
> - spin_lock_irq(&x->wait.lock);
> + raw_spin_lock_irq(&x->wait.lock);
> timeout = do_wait_for_common(x, action, timeout, state);
> - spin_unlock_irq(&x->wait.lock);
> + raw_spin_unlock_irq(&x->wait.lock);
>
> complete_release(x);
>
> @@ -291,12 +293,12 @@ bool try_wait_for_completion(struct comp
> if (!READ_ONCE(x->done))
> return false;
>
> - spin_lock_irqsave(&x->wait.lock, flags);
> + raw_spin_lock_irqsave(&x->wait.lock, flags);
> if (!x->done)
> ret = false;
> else if (x->done != UINT_MAX)
> x->done--;
> - spin_unlock_irqrestore(&x->wait.lock, flags);
> + raw_spin_unlock_irqrestore(&x->wait.lock, flags);
> return ret;
> }
> EXPORT_SYMBOL(try_wait_for_completion);
> @@ -322,8 +324,8 @@ bool completion_done(struct completion *
> * otherwise we can end up freeing the completion before complete()
> * is done referencing it.
> */
> - spin_lock_irqsave(&x->wait.lock, flags);
> - spin_unlock_irqrestore(&x->wait.lock, flags);
> + raw_spin_lock_irqsave(&x->wait.lock, flags);
> + raw_spin_unlock_irqrestore(&x->wait.lock, flags);
> return true;
> }
> EXPORT_SYMBOL(completion_done);
>
^ permalink raw reply
* Re: [PATCH v2 1/4] mm: Check for node_online in node_present_pages
From: Michael Ellerman @ 2020-03-19 0:32 UTC (permalink / raw)
To: Vlastimil Babka, Michal Hocko, Srikar Dronamraju
Cc: Sachin Sant, Nathan Lynch, Bharata B Rao, linux-mm, Kirill Tkhai,
Mel Gorman, Joonsoo Kim, Andrew Morton, linuxppc-dev,
Christopher Lameter
In-Reply-To: <2d7c55ed-0f67-bd47-e478-9726734abcc9@suse.cz>
Vlastimil Babka <vbabka@suse.cz> writes:
> On 3/18/20 11:02 AM, Michal Hocko wrote:
>> On Wed 18-03-20 12:58:07, Srikar Dronamraju wrote:
>>> Calling a kmalloc_node on a possible node which is not yet onlined can
>>> lead to panic. Currently node_present_pages() doesn't verify the node is
>>> online before accessing the pgdat for the node. However pgdat struct may
>>> not be available resulting in a crash.
>>>
>>> NIP [c0000000003d55f4] ___slab_alloc+0x1f4/0x760
>>> LR [c0000000003d5b94] __slab_alloc+0x34/0x60
>>> Call Trace:
>>> [c0000008b3783960] [c0000000003d5734] ___slab_alloc+0x334/0x760 (unreliable)
>>> [c0000008b3783a40] [c0000000003d5b94] __slab_alloc+0x34/0x60
>>> [c0000008b3783a70] [c0000000003d6fa0] __kmalloc_node+0x110/0x490
>>> [c0000008b3783af0] [c0000000003443d8] kvmalloc_node+0x58/0x110
>>> [c0000008b3783b30] [c0000000003fee38] mem_cgroup_css_online+0x108/0x270
>>> [c0000008b3783b90] [c000000000235aa8] online_css+0x48/0xd0
>>> [c0000008b3783bc0] [c00000000023eaec] cgroup_apply_control_enable+0x2ec/0x4d0
>>> [c0000008b3783ca0] [c000000000242318] cgroup_mkdir+0x228/0x5f0
>>> [c0000008b3783d10] [c00000000051e170] kernfs_iop_mkdir+0x90/0xf0
>>> [c0000008b3783d50] [c00000000043dc00] vfs_mkdir+0x110/0x230
>>> [c0000008b3783da0] [c000000000441c90] do_mkdirat+0xb0/0x1a0
>>> [c0000008b3783e20] [c00000000000b278] system_call+0x5c/0x68
>>>
>>> Fix this by verifying the node is online before accessing the pgdat
>>> structure. Fix the same for node_spanned_pages() too.
>>>
>>> Cc: Andrew Morton <akpm@linux-foundation.org>
>>> Cc: linux-mm@kvack.org
>>> Cc: Mel Gorman <mgorman@suse.de>
>>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>>> Cc: Sachin Sant <sachinp@linux.vnet.ibm.com>
>>> Cc: Michal Hocko <mhocko@kernel.org>
>>> Cc: Christopher Lameter <cl@linux.com>
>>> Cc: linuxppc-dev@lists.ozlabs.org
>>> Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
>>> Cc: Kirill Tkhai <ktkhai@virtuozzo.com>
>>> Cc: Vlastimil Babka <vbabka@suse.cz>
>>> Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
>>> Cc: Bharata B Rao <bharata@linux.ibm.com>
>>> Cc: Nathan Lynch <nathanl@linux.ibm.com>
>>>
>>> Reported-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
>>> Tested-by: Sachin Sant <sachinp@linux.vnet.ibm.com>
>>> Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
>>> ---
>>> include/linux/mmzone.h | 6 ++++--
>>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
>>> index f3f264826423..88078a3b95e5 100644
>>> --- a/include/linux/mmzone.h
>>> +++ b/include/linux/mmzone.h
>>> @@ -756,8 +756,10 @@ typedef struct pglist_data {
>>> atomic_long_t vm_stat[NR_VM_NODE_STAT_ITEMS];
>>> } pg_data_t;
>>>
>>> -#define node_present_pages(nid) (NODE_DATA(nid)->node_present_pages)
>>> -#define node_spanned_pages(nid) (NODE_DATA(nid)->node_spanned_pages)
>>> +#define node_present_pages(nid) \
>>> + (node_online(nid) ? NODE_DATA(nid)->node_present_pages : 0)
>>> +#define node_spanned_pages(nid) \
>>> + (node_online(nid) ? NODE_DATA(nid)->node_spanned_pages : 0)
>>
>> I believe this is a wrong approach. We really do not want to special
>> case all the places which require NODE_DATA. Can we please go and
>> allocate pgdat for all possible nodes?
>>
>> The current state of memory less hacks subtle bugs poping up here and
>> there just prove that we should have done that from the very begining
>> IMHO.
>
> Yes. So here's an alternative proposal for fixing the current situation in SLUB,
> before the long-term solution of having all possible nodes provide valid pgdat
> with zonelists:
>
> - fix SLUB with the hunk at the end of this mail - the point is to use NUMA_NO_NODE
> as fallback instead of node_to_mem_node()
> - this removes all uses of node_to_mem_node (luckily it's just SLUB),
> kill it completely instead of trying to fix it up
> - patch 1/4 is not needed with the fix
> - perhaps many of your other patches are alss not needed
> - once we get the long-term solution, some of the !node_online() checks can be removed
Seems like a nice solution to me :)
> ----8<----
> diff --git a/mm/slub.c b/mm/slub.c
> index 17dc00e33115..1d4f2d7a0080 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -1511,7 +1511,7 @@ static inline struct page *alloc_slab_page(struct kmem_cache *s,
> struct page *page;
> unsigned int order = oo_order(oo);
>
> - if (node == NUMA_NO_NODE)
> + if (node == NUMA_NO_NODE || !node_online(node))
Why don't we need the node_present_pages() check here?
> page = alloc_pages(flags, order);
> else
> page = __alloc_pages_node(node, flags, order);
> @@ -1973,8 +1973,6 @@ static void *get_partial(struct kmem_cache *s, gfp_t flags, int node,
>
> if (node == NUMA_NO_NODE)
> searchnode = numa_mem_id();
> - else if (!node_present_pages(node))
> - searchnode = node_to_mem_node(node);
>
> object = get_partial_node(s, get_node(s, searchnode), c, flags);
> if (object || node != NUMA_NO_NODE)
> @@ -2568,12 +2566,15 @@ static void *___slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node,
> redo:
>
> if (unlikely(!node_match(page, node))) {
> - int searchnode = node;
> -
> - if (node != NUMA_NO_NODE && !node_present_pages(node))
> - searchnode = node_to_mem_node(node);
> -
> - if (unlikely(!node_match(page, searchnode))) {
> + /*
> + * node_match() false implies node != NUMA_NO_NODE
> + * but if the node is not online and has no pages, just
^
this should be 'or' ?
> + * ignore the constraint
> + */
> + if ((!node_online(node) || !node_present_pages(node))) {
> + node = NUMA_NO_NODE;
> + goto redo;
> + } else {
> stat(s, ALLOC_NODE_MISMATCH);
> deactivate_slab(s, page, c->freelist, c);
> goto new_slab;
cheers
^ permalink raw reply
* Re: [PATCH 4/4] hugetlbfs: clean up command line processing
From: Randy Dunlap @ 2020-03-19 0:20 UTC (permalink / raw)
To: Mike Kravetz, linux-mm, linux-kernel, linux-arm-kernel,
linuxppc-dev, linux-riscv, linux-s390, sparclinux, linux-doc
Cc: Albert Ou, Vasily Gorbik, Jonathan Corbet, Dave Hansen,
Heiko Carstens, David S . Miller, Christian Borntraeger,
Ingo Molnar, Palmer Dabbelt, Paul Walmsley, Catalin Marinas,
Paul Mackerras, Andrew Morton, Longpeng, Will Deacon,
Thomas Gleixner
In-Reply-To: <20200318220634.32100-5-mike.kravetz@oracle.com>
Hi Mike,
On 3/18/20 3:06 PM, Mike Kravetz wrote:
> With all hugetlb page processing done in a single file clean up code.
> - Make code match desired semantics
> - Update documentation with semantics
> - Make all warnings and errors messages start with 'HugeTLB:'.
> - Consistently name command line parsing routines.
> - Add comments to code
> - Describe some of the subtle interactions
> - Describe semantics of command line arguments
>
> Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
> ---
> Documentation/admin-guide/mm/hugetlbpage.rst | 26 +++++++
> mm/hugetlb.c | 78 +++++++++++++++-----
> 2 files changed, 87 insertions(+), 17 deletions(-)
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index cc85b4f156ca..2b9bf01db2b6 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -3214,8 +3238,15 @@ static int __init hugetlb_nrpages_setup(char *s)
>
> return 1;
> }
> -__setup("hugepages=", hugetlb_nrpages_setup);
> +__setup("hugepages=", hugepages_setup);
>
> +/*
> + * hugepagesz command line processing
> + * A specific huge page size can only be specified once with hugepagesz.
> + * hugepagesz is followed by hugepages on the commnad line. The global
typo: command
> + * variable 'parsed_valid_hugepagesz' is used to determine if prior
> + * hugepagesz argument was valid.
> + */
> static int __init hugepagesz_setup(char *s)
> {
> unsigned long long size;
Does any of this need to be updated? (from Documentation/admin-guide/kernel-parameters.txt)
hugepagesz= [HW,IA-64,PPC,X86-64] The size of the HugeTLB pages.
On x86-64 and powerpc, this option can be specified
multiple times interleaved with hugepages= to reserve
huge pages of different sizes. Valid pages sizes on
x86-64 are 2M (when the CPU supports "pse") and 1G
(when the CPU supports the "pdpe1gb" cpuinfo flag).
--
~Randy
^ permalink raw reply
* Re: [PATCH 1/4] hugetlbfs: add arch_hugetlb_valid_size
From: Dave Hansen @ 2020-03-18 23:36 UTC (permalink / raw)
To: Mike Kravetz, linux-mm, linux-kernel, linux-arm-kernel,
linuxppc-dev, linux-riscv, linux-s390, sparclinux, linux-doc
Cc: Albert Ou, Andrew Morton, Vasily Gorbik, Jonathan Corbet,
Catalin Marinas, Dave Hansen, Heiko Carstens,
Christian Borntraeger, Ingo Molnar, Palmer Dabbelt, Paul Walmsley,
Paul Mackerras, Thomas Gleixner, Longpeng, Will Deacon,
David S.Miller
In-Reply-To: <5aceea6a-8dc0-a44b-80c6-94511b5c75ca@oracle.com>
On 3/18/20 3:52 PM, Mike Kravetz wrote:
> Sounds good. I'll incorporate those changes into a v2, unless someone
> else with has a different opinion.
>
> BTW, this patch should not really change the way the code works today.
> It is mostly a movement of code. Unless I am missing something, the
> existing code will always allow setup of PMD_SIZE hugetlb pages.
Hah, I totally skipped over the old code in the diff.
It looks like we'll disable hugetblfs *entirely* if PSE isn't supported.
I think this is actually wrong, but nobody ever noticed. I think you'd
have to be running as a guest under a hypervisor that's lying about PSE
not being supported *and* care about 1GB pages. Nobody does that.
^ permalink raw reply
* Re: [PATCH 12/15] powerpc/watchpoint: Prepare handler to handle more than one watcnhpoint
From: Michael Ellerman @ 2020-03-18 23:36 UTC (permalink / raw)
To: Segher Boessenkool, Christophe Leroy
Cc: Ravi Bangoria, mikey, apopple, peterz, fweisbec, oleg, npiggin,
linux-kernel, paulus, jolsa, naveen.n.rao, linuxppc-dev, mingo
In-Reply-To: <20200318212726.GN22482@gate.crashing.org>
Segher Boessenkool <segher@kernel.crashing.org> writes:
> On Wed, Mar 18, 2020 at 12:44:52PM +0100, Christophe Leroy wrote:
>> Le 18/03/2020 à 12:35, Michael Ellerman a écrit :
>> >Christophe Leroy <christophe.leroy@c-s.fr> writes:
>> >>Le 09/03/2020 à 09:58, Ravi Bangoria a écrit :
>> >>>Currently we assume that we have only one watchpoint supported by hw.
>> >>>Get rid of that assumption and use dynamic loop instead. This should
>> >>>make supporting more watchpoints very easy.
>> >>
>> >>I think using 'we' is to be avoided in commit message.
>> >
>> >Hmm, is it?
>> >
>> >I use 'we' all the time. Which doesn't mean it's correct, but I think it
>> >reads OK.
>> >
>> >cheers
>>
>> From
>> https://www.kernel.org/doc/html/latest/process/submitting-patches.html :
>>
>> Describe your changes in imperative mood, e.g. “make xyzzy do frotz”
>> instead of “[This patch] makes xyzzy do frotz” or “[I] changed xyzzy
>> to do frotz”, as if you are giving orders to the codebase to change its
>> behaviour.
>
> That is what is there already? "Get rid of ...".
>
> You cannot describe the current situation with an imperative.
Yeah, I think the use of 'we' and the imperative mood are separate
things.
ie. this uses 'we' to describe the current behaviour and then the
imperative mood to describe the change that's being made:
Currently we assume xyzzy always does bar, which is incorrect.
Change it to do frotz.
cheers
^ permalink raw reply
* Re: [PATCH 1/4] hugetlbfs: add arch_hugetlb_valid_size
From: Mike Kravetz @ 2020-03-18 22:52 UTC (permalink / raw)
To: Dave Hansen, linux-mm, linux-kernel, linux-arm-kernel,
linuxppc-dev, linux-riscv, linux-s390, sparclinux, linux-doc
Cc: Albert Ou, Andrew Morton, Vasily Gorbik, Jonathan Corbet,
Catalin Marinas, Dave Hansen, Heiko Carstens,
Christian Borntraeger, Ingo Molnar, Palmer Dabbelt, Paul Walmsley,
Paul Mackerras, Thomas Gleixner, Longpeng, Will Deacon,
David S.Miller
In-Reply-To: <831a0773-1ba6-4d72-44b9-7472123b8528@intel.com>
On 3/18/20 3:15 PM, Dave Hansen wrote:
> Hi Mike,
>
> The series looks like a great idea to me. One nit on the x86 bits,
> though...
>
>> diff --git a/arch/x86/mm/hugetlbpage.c b/arch/x86/mm/hugetlbpage.c
>> index 5bfd5aef5378..51e6208fdeec 100644
>> --- a/arch/x86/mm/hugetlbpage.c
>> +++ b/arch/x86/mm/hugetlbpage.c
>> @@ -181,16 +181,25 @@ hugetlb_get_unmapped_area(struct file *file, unsigned long addr,
>> #endif /* CONFIG_HUGETLB_PAGE */
>>
>> #ifdef CONFIG_X86_64
>> +bool __init arch_hugetlb_valid_size(unsigned long long size)
>> +{
>> + if (size == PMD_SIZE)
>> + return true;
>> + else if (size == PUD_SIZE && boot_cpu_has(X86_FEATURE_GBPAGES))
>> + return true;
>> + else
>> + return false;
>> +}
>
> I'm pretty sure it's possible to have a system without 2M/PMD page
> support. We even have a handy-dandy comment about it in
> arch/x86/include/asm/required-features.h:
>
> #ifdef CONFIG_X86_64
> #ifdef CONFIG_PARAVIRT
> /* Paravirtualized systems may not have PSE or PGE available */
> #define NEED_PSE 0
> ...
>
> I *think* you need an X86_FEATURE_PSE check here to be totally correct.
>
> if (size == PMD_SIZE && cpu_feature_enabled(X86_FEATURE_PSE))
> return true;
>
> BTW, I prefer cpu_feature_enabled() to boot_cpu_has() because it
> includes disabled-features checking. I don't think any of it matters
> for these specific features, but I generally prefer it on principle.
Sounds good. I'll incorporate those changes into a v2, unless someone
else with has a different opinion.
BTW, this patch should not really change the way the code works today.
It is mostly a movement of code. Unless I am missing something, the
existing code will always allow setup of PMD_SIZE hugetlb pages.
--
Mike Kravetz
^ permalink raw reply
* Re: [PATCH 1/4] hugetlbfs: add arch_hugetlb_valid_size
From: Mike Kravetz @ 2020-03-18 22:38 UTC (permalink / raw)
To: Will Deacon
Cc: linux-doc, Catalin Marinas, Dave Hansen, Heiko Carstens, linux-mm,
Paul Mackerras, sparclinux, linux-riscv, linux-s390,
Jonathan Corbet, Christian Borntraeger, Ingo Molnar, Longpeng,
Albert Ou, Vasily Gorbik, Paul Walmsley, Thomas Gleixner,
linux-arm-kernel, linux-kernel, Palmer Dabbelt, Andrew Morton,
linuxppc-dev, David S.Miller
In-Reply-To: <20200318220954.GD8477@willie-the-truck>
On 3/18/20 3:09 PM, Will Deacon wrote:
> On Wed, Mar 18, 2020 at 03:06:31PM -0700, Mike Kravetz wrote:
>> The architecture independent routine hugetlb_default_setup sets up
>> the default huge pages size. It has no way to verify if the passed
>> value is valid, so it accepts it and attempts to validate at a later
>> time. This requires undocumented cooperation between the arch specific
>> and arch independent code.
>>
>> For architectures that support more than one huge page size, provide
>> a routine arch_hugetlb_valid_size to validate a huge page size.
>> hugetlb_default_setup can use this to validate passed values.
>>
>> arch_hugetlb_valid_size will also be used in a subsequent patch to
>> move processing of the "hugepagesz=" in arch specific code to a common
>> routine in arch independent code.
>>
>> Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
>> ---
>> arch/arm64/include/asm/hugetlb.h | 2 ++
>> arch/arm64/mm/hugetlbpage.c | 19 ++++++++++++++-----
>> arch/powerpc/include/asm/hugetlb.h | 3 +++
>> arch/powerpc/mm/hugetlbpage.c | 20 +++++++++++++-------
>> arch/riscv/include/asm/hugetlb.h | 3 +++
>> arch/riscv/mm/hugetlbpage.c | 28 ++++++++++++++++++----------
>> arch/s390/include/asm/hugetlb.h | 3 +++
>> arch/s390/mm/hugetlbpage.c | 18 +++++++++++++-----
>> arch/sparc/include/asm/hugetlb.h | 3 +++
>> arch/sparc/mm/init_64.c | 23 ++++++++++++++++-------
>> arch/x86/include/asm/hugetlb.h | 3 +++
>> arch/x86/mm/hugetlbpage.c | 21 +++++++++++++++------
>> include/linux/hugetlb.h | 7 +++++++
>> mm/hugetlb.c | 16 +++++++++++++---
>> 14 files changed, 126 insertions(+), 43 deletions(-)
>>
>> diff --git a/arch/arm64/include/asm/hugetlb.h b/arch/arm64/include/asm/hugetlb.h
>> index 2eb6c234d594..3248f35213ee 100644
>> --- a/arch/arm64/include/asm/hugetlb.h
>> +++ b/arch/arm64/include/asm/hugetlb.h
<snip>
>> +
>> +static __init int setup_hugepagesz(char *opt)
>> +{
>> + unsigned long long ps = memparse(opt, &opt);
>> +
>> + if arch_hugetlb_valid_size(ps)) {
>
> Please compile your changes if you're touching multiple architectures. You
> can get cross-compiler binaries from:
>
My apologies. I only cross compiled the result of the series on each
architecture. The above code is obviously bad.
--
Mike Kravetz
^ permalink raw reply
* Re: [patch V2 08/15] Documentation: Add lock ordering and nesting documentation
From: Paul E. McKenney @ 2020-03-18 22:31 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Randy Dunlap, Peter Zijlstra, linux-pci,
Sebastian Andrzej Siewior, Oleg Nesterov, Joel Fernandes,
Will Deacon, Ingo Molnar, Davidlohr Bueso, Logan Gunthorpe,
Arnd Bergmann, linuxppc-dev, Steven Rostedt, Bjorn Helgaas,
Kurt Schwemmer, Kalle Valo, Felipe Balbi, Greg Kroah-Hartman,
linux-usb, linux-wireless, LKML, netdev, Linus Torvalds,
David S. Miller
In-Reply-To: <20200318204408.211530902@linutronix.de>
On Wed, Mar 18, 2020 at 09:43:10PM +0100, Thomas Gleixner wrote:
> From: Thomas Gleixner <tglx@linutronix.de>
>
> The kernel provides a variety of locking primitives. The nesting of these
> lock types and the implications of them on RT enabled kernels is nowhere
> documented.
>
> Add initial documentation.
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Mostly native-English-speaker services below, so please feel free to
ignore. The one place I made a substantive change, I marked it "@@@".
I only did about half of this document, but should this prove useful,
I will do the other half later.
Thanx, Paul
> ---
> V2: Addressed review comments from Randy
> ---
> Documentation/locking/index.rst | 1
> Documentation/locking/locktypes.rst | 298 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 299 insertions(+)
> create mode 100644 Documentation/locking/locktypes.rst
>
> --- a/Documentation/locking/index.rst
> +++ b/Documentation/locking/index.rst
> @@ -7,6 +7,7 @@ locking
> .. toctree::
> :maxdepth: 1
>
> + locktypes
> lockdep-design
> lockstat
> locktorture
> --- /dev/null
> +++ b/Documentation/locking/locktypes.rst
> @@ -0,0 +1,298 @@
> +.. _kernel_hacking_locktypes:
> +
> +==========================
> +Lock types and their rules
> +==========================
> +
> +Introduction
> +============
> +
> +The kernel provides a variety of locking primitives which can be divided
> +into two categories:
> +
> + - Sleeping locks
> + - Spinning locks
> +
> +This document describes the lock types at least at the conceptual level and
> +provides rules for nesting of lock types also under the aspect of PREEMPT_RT.
I suggest something like this:
This document conceptually describes these lock types and provides rules
for their nesting, including the rules for use under PREEMPT_RT.
> +
> +Lock categories
> +===============
> +
> +Sleeping locks
> +--------------
> +
> +Sleeping locks can only be acquired in preemptible task context.
> +
> +Some of the implementations allow try_lock() attempts from other contexts,
> +but that has to be really evaluated carefully including the question
> +whether the unlock can be done from that context safely as well.
> +
> +Note that some lock types change their implementation details when
> +debugging is enabled, so this should be really only considered if there is
> +no other option.
How about something like this?
Although implementations allow try_lock() from other contexts, it is
necessary to carefully evaluate the safety of unlock() as well as of
try_lock(). Furthermore, it is also necessary to evaluate the debugging
versions of these primitives. In short, don't acquire sleeping locks
from other contexts unless there is no other option.
> +Sleeping lock types:
> +
> + - mutex
> + - rt_mutex
> + - semaphore
> + - rw_semaphore
> + - ww_mutex
> + - percpu_rw_semaphore
> +
> +On a PREEMPT_RT enabled kernel the following lock types are converted to
> +sleeping locks:
On PREEMPT_RT kernels, these lock types are converted to sleeping locks:
> + - spinlock_t
> + - rwlock_t
> +
> +Spinning locks
> +--------------
> +
> + - raw_spinlock_t
> + - bit spinlocks
> +
> +On a non PREEMPT_RT enabled kernel the following lock types are spinning
> +locks as well:
On non-PREEMPT_RT kernels, these lock types are also spinning locks:
> + - spinlock_t
> + - rwlock_t
> +
> +Spinning locks implicitly disable preemption and the lock / unlock functions
> +can have suffixes which apply further protections:
> +
> + =================== ====================================================
> + _bh() Disable / enable bottom halves (soft interrupts)
> + _irq() Disable / enable interrupts
> + _irqsave/restore() Save and disable / restore interrupt disabled state
> + =================== ====================================================
> +
> +
> +rtmutex
> +=======
> +
> +RT-mutexes are mutexes with support for priority inheritance (PI).
> +
> +PI has limitations on non PREEMPT_RT enabled kernels due to preemption and
> +interrupt disabled sections.
> +
> +On a PREEMPT_RT enabled kernel most of these sections are fully
> +preemptible. This is possible because PREEMPT_RT forces most executions
> +into task context, especially interrupt handlers and soft interrupts, which
> +allows to substitute spinlock_t and rwlock_t with RT-mutex based
> +implementations.
PI clearly cannot preempt preemption-disabled or interrupt-disabled
regions of code, even on PREEMPT_RT kernels. Instead, PREEMPT_RT kernels
execute most such regions of code in preemptible task context, especially
interrupt handlers and soft interrupts. This conversion allows spinlock_t
and rwlock_t to be implemented via RT-mutexes.
> +
> +raw_spinlock_t and spinlock_t
> +=============================
> +
> +raw_spinlock_t
> +--------------
> +
> +raw_spinlock_t is a strict spinning lock implementation regardless of the
> +kernel configuration including PREEMPT_RT enabled kernels.
> +
> +raw_spinlock_t is to be used only in real critical core code, low level
> +interrupt handling and places where protecting (hardware) state is required
> +to be safe against preemption and eventually interrupts.
> +
> +Another reason to use raw_spinlock_t is when the critical section is tiny
> +to avoid the overhead of spinlock_t on a PREEMPT_RT enabled kernel in the
> +contended case.
raw_spinlock_t is a strict spinning lock implementation in all kernels,
including PREEMPT_RT kernels. Use raw_spinlock_t only in real critical
core code, low level interrupt handling and places where disabling
preemption or interrupts is required, for example, to safely access
hardware state. raw_spinlock_t can sometimes also be used when the
critical section is tiny and the lock is lightly contended, thus avoiding
RT-mutex overhead.
@@@ I added the point about the lock being lightly contended.
> +spinlock_t
> +----------
> +
> +The semantics of spinlock_t change with the state of CONFIG_PREEMPT_RT.
> +
> +On a non PREEMPT_RT enabled kernel spinlock_t is mapped to raw_spinlock_t
> +and has exactly the same semantics.
> +
> +spinlock_t and PREEMPT_RT
> +-------------------------
> +
> +On a PREEMPT_RT enabled kernel spinlock_t is mapped to a separate
> +implementation based on rt_mutex which changes the semantics:
> +
> + - Preemption is not disabled
> +
> + - The hard interrupt related suffixes for spin_lock / spin_unlock
> + operations (_irq, _irqsave / _irqrestore) do not affect the CPUs
CPU's
> + interrupt disabled state
> +
> + - The soft interrupt related suffix (_bh()) is still disabling the
> + execution of soft interrupts, but contrary to a non PREEMPT_RT enabled
> + kernel, which utilizes the preemption count, this is achieved by a per
> + CPU bottom half locking mechanism.
- The soft interrupt related suffix (_bh()) still disables softirq
handlers. However, unlike non-PREEMPT_RT kernels (which disable
preemption to get this effect), PREEMPT_RT kernels use a per-CPU
per-bottom-half locking mechanism.
> +All other semantics of spinlock_t are preserved:
> +
> + - Migration of tasks which hold a spinlock_t is prevented. On a non
> + PREEMPT_RT enabled kernel this is implicit due to preemption disable.
> + PREEMPT_RT has a separate mechanism to achieve this. This ensures that
> + pointers to per CPU variables stay valid even if the task is preempted.
> +
> + - Task state preservation. The task state is not affected when a lock is
> + contended and the task has to schedule out and wait for the lock to
> + become available. The lock wake up restores the task state unless there
> + was a regular (not lock related) wake up on the task. This ensures that
> + the task state rules are always correct independent of the kernel
> + configuration.
> +
> +rwlock_t
> +========
> +
> +rwlock_t is a multiple readers and single writer lock mechanism.
> +
> +On a non PREEMPT_RT enabled kernel rwlock_t is implemented as a spinning
> +lock and the suffix rules of spinlock_t apply accordingly. The
> +implementation is fair and prevents writer starvation.
> +
> +rwlock_t and PREEMPT_RT
> +-----------------------
> +
> +On a PREEMPT_RT enabled kernel rwlock_t is mapped to a separate
> +implementation based on rt_mutex which changes the semantics:
> +
> + - Same changes as for spinlock_t
> +
> + - The implementation is not fair and can cause writer starvation under
> + certain circumstances. The reason for this is that a writer cannot grant
> + its priority to multiple readers. Readers which are blocked on a writer
> + fully support the priority inheritance protocol.
> +
> +
> +PREEMPT_RT caveats
> +==================
> +
> +spinlock_t and rwlock_t
> +-----------------------
> +
> +The substitution of spinlock_t and rwlock_t on PREEMPT_RT enabled kernels
> +with RT-mutex based implementations has a few implications.
> +
> +On a non PREEMPT_RT enabled kernel the following code construct is
> +perfectly fine::
> +
> + local_irq_disable();
> + spin_lock(&lock);
> +
> +and fully equivalent to::
> +
> + spin_lock_irq(&lock);
> +
> +Same applies to rwlock_t and the _irqsave() suffix variant.
> +
> +On a PREEMPT_RT enabled kernel this breaks because the RT-mutex
> +substitution expects a fully preemptible context.
> +
> +The preferred solution is to use :c:func:`spin_lock_irq()` or
> +:c:func:`spin_lock_irqsave()` and their unlock counterparts.
> +
> +PREEMPT_RT also offers a local_lock mechanism to substitute the
> +local_irq_disable/save() constructs in cases where a separation of the
> +interrupt disabling and the locking is really unavoidable. This should be
> +restricted to very rare cases.
> +
> +
> +raw_spinlock_t
> +--------------
> +
> +Locking of a raw_spinlock_t disables preemption and eventually interrupts.
> +Therefore code inside the critical region has to be careful to avoid calls
> +into code which takes a regular spinlock_t or rwlock_t. A prime example is
> +memory allocation.
> +
> +On a non PREEMPT_RT enabled kernel the following code construct is
> +perfectly fine code::
> +
> + raw_spin_lock(&lock);
> + p = kmalloc(sizeof(*p), GFP_ATOMIC);
> +
> +On a PREEMPT_RT enabled kernel this breaks because the memory allocator is
> +fully preemptible and therefore does not support allocations from truly
> +atomic contexts.
> +
> +Contrary to that the following code construct is perfectly fine on
> +PREEMPT_RT as spin_lock() does not disable preemption::
> +
> + spin_lock(&lock);
> + p = kmalloc(sizeof(*p), GFP_ATOMIC);
> +
> +Most places which use GFP_ATOMIC allocations are safe on PREEMPT_RT as the
> +execution is forced into thread context and the lock substitution is
> +ensuring preemptibility.
> +
> +
> +bit spinlocks
> +-------------
> +
> +Bit spinlocks are problematic for PREEMPT_RT as they cannot be easily
> +substituted by an RT-mutex based implementation for obvious reasons.
> +
> +The semantics of bit spinlocks are preserved on a PREEMPT_RT enabled kernel
> +and the caveats vs. raw_spinlock_t apply.
> +
> +Some bit spinlocks are substituted by regular spinlock_t for PREEMPT_RT but
> +this requires conditional (#ifdef'ed) code changes at the usage side while
> +the spinlock_t substitution is simply done by the compiler and the
> +conditionals are restricted to header files and core implementation of the
> +locking primitives and the usage sites do not require any changes.
> +
> +
> +Lock type nesting rules
> +=======================
> +
> +The most basic rules are:
> +
> + - Lock types of the same lock category (sleeping, spinning) can nest
> + arbitrarily as long as they respect the general lock ordering rules to
> + prevent deadlocks.
> +
> + - Sleeping lock types cannot nest inside spinning lock types.
> +
> + - Spinning lock types can nest inside sleeping lock types.
> +
> +These rules apply in general independent of CONFIG_PREEMPT_RT.
> +
> +As PREEMPT_RT changes the lock category of spinlock_t and rwlock_t from
> +spinning to sleeping this has obviously restrictions how they can nest with
> +raw_spinlock_t.
> +
> +This results in the following nest ordering:
> +
> + 1) Sleeping locks
> + 2) spinlock_t and rwlock_t
> + 3) raw_spinlock_t and bit spinlocks
> +
> +Lockdep is aware of these constraints to ensure that they are respected.
> +
> +
> +Owner semantics
> +===============
> +
> +Most lock types in the Linux kernel have strict owner semantics, i.e. the
> +context (task) which acquires a lock has to release it.
> +
> +There are two exceptions:
> +
> + - semaphores
> + - rwsems
> +
> +semaphores have no strict owner semantics for historical reasons. They are
> +often used for both serialization and waiting purposes. That's generally
> +discouraged and should be replaced by separate serialization and wait
> +mechanisms.
> +
> +rwsems have grown interfaces which allow non owner release for special
> +purposes. This usage is problematic on PREEMPT_RT because PREEMPT_RT
> +substitutes all locking primitives except semaphores with RT-mutex based
> +implementations to provide priority inheritance for all lock types except
> +the truly spinning ones. Priority inheritance on ownerless locks is
> +obviously impossible.
> +
> +For now the rwsem non-owner release excludes code which utilizes it from
> +being used on PREEMPT_RT enabled kernels. In same cases this can be
> +mitigated by disabling portions of the code, in other cases the complete
> +functionality has to be disabled until a workable solution has been found.
>
^ permalink raw reply
* Re: [PATCH 2/5] selftests/powerpc: Add header files for NX compresion/decompression
From: Daniel Axtens @ 2020-03-18 22:29 UTC (permalink / raw)
To: Raphael Moreira Zinsly, linuxppc-dev, linux-crypto
Cc: abali, haren, herbert, Raphael Moreira Zinsly
In-Reply-To: <20200316180714.18631-3-rzinsly@linux.ibm.com>
Raphael Moreira Zinsly <rzinsly@linux.ibm.com> writes:
> Add files to be able to compress and decompress files using the
> powerpc NX-GZIP engine.
>
> Signed-off-by: Bulent Abali <abali@us.ibm.com>
> Signed-off-by: Raphael Moreira Zinsly <rzinsly@linux.ibm.com>
> ---
> .../powerpc/nx-gzip/inc/copy-paste.h | 54 ++
> .../selftests/powerpc/nx-gzip/inc/nx_dbg.h | 95 +++
> .../selftests/powerpc/nx-gzip/inc/nxu.h | 644 ++++++++++++++++++
> 3 files changed, 793 insertions(+)
> create mode 100644 tools/testing/selftests/powerpc/nx-gzip/inc/copy-paste.h
> create mode 100644 tools/testing/selftests/powerpc/nx-gzip/inc/nx_dbg.h
> create mode 100644 tools/testing/selftests/powerpc/nx-gzip/inc/nxu.h
>
> diff --git a/tools/testing/selftests/powerpc/nx-gzip/inc/copy-paste.h b/tools/testing/selftests/powerpc/nx-gzip/inc/copy-paste.h
> new file mode 100644
> index 000000000000..107139b6c7df
> --- /dev/null
> +++ b/tools/testing/selftests/powerpc/nx-gzip/inc/copy-paste.h
> @@ -0,0 +1,54 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +
> +#include "nx-helpers.h"
> +
> +/*
> + * Macros taken from arch/powerpc/include/asm/ppc-opcode.h and other
> + * header files.
> + */
> +#define ___PPC_RA(a) (((a) & 0x1f) << 16)
> +#define ___PPC_RB(b) (((b) & 0x1f) << 11)
> +
> +#define PPC_INST_COPY 0x7c20060c
> +#define PPC_INST_PASTE 0x7c20070d
> +
> +#define PPC_COPY(a, b) stringify_in_c(.long PPC_INST_COPY | \
> + ___PPC_RA(a) | ___PPC_RB(b))
> +#define PPC_PASTE(a, b) stringify_in_c(.long PPC_INST_PASTE | \
> + ___PPC_RA(a) | ___PPC_RB(b))
> +#define CR0_SHIFT 28
> +#define CR0_MASK 0xF
> +/*
> + * Copy/paste instructions:
> + *
> + * copy RA,RB
> + * Copy contents of address (RA) + effective_address(RB)
> + * to internal copy-buffer.
> + *
> + * paste RA,RB
> + * Paste contents of internal copy-buffer to the address
> + * (RA) + effective_address(RB)
> + */
> +static inline int vas_copy(void *crb, int offset)
> +{
> + asm volatile(PPC_COPY(%0, %1)";"
> + :
> + : "b" (offset), "b" (crb)
> + : "memory");
> +
> + return 0;
> +}
> +
> +static inline int vas_paste(void *paste_address, int offset)
> +{
> + u32 cr;
> +
> + cr = 0;
> + asm volatile(PPC_PASTE(%1, %2)";"
> + "mfocrf %0, 0x80;"
> + : "=r" (cr)
> + : "b" (offset), "b" (paste_address)
> + : "memory", "cr0");
> +
> + return (cr >> CR0_SHIFT) & CR0_MASK;
> +}
> diff --git a/tools/testing/selftests/powerpc/nx-gzip/inc/nx_dbg.h b/tools/testing/selftests/powerpc/nx-gzip/inc/nx_dbg.h
> new file mode 100644
> index 000000000000..f2c0eee2317e
> --- /dev/null
> +++ b/tools/testing/selftests/powerpc/nx-gzip/inc/nx_dbg.h
> @@ -0,0 +1,95 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later
> + *
> + * Copyright 2020 IBM Corporation
> + *
> + */
> +
> +#ifndef _NXU_DBG_H_
> +#define _NXU_DBG_H_
> +
> +#include <sys/file.h>
> +#include <stdint.h>
> +#include <stdio.h>
> +#include <time.h>
> +#include <pthread.h>
> +
> +extern FILE * nx_gzip_log;
> +extern int nx_gzip_trace;
> +extern unsigned int nx_gzip_inflate_impl;
> +extern unsigned int nx_gzip_deflate_impl;
> +extern unsigned int nx_gzip_inflate_flags;
> +extern unsigned int nx_gzip_deflate_flags;
> +
> +extern int nx_dbg;
> +pthread_mutex_t mutex_log;
> +
> +#define nx_gzip_trace_enabled() (nx_gzip_trace & 0x1)
> +#define nx_gzip_hw_trace_enabled() (nx_gzip_trace & 0x2)
> +#define nx_gzip_sw_trace_enabled() (nx_gzip_trace & 0x4)
> +#define nx_gzip_gather_statistics() (nx_gzip_trace & 0x8)
> +#define nx_gzip_per_stream_stat() (nx_gzip_trace & 0x10)
> +
> +#define prt(fmt, ...) do { \
> + pthread_mutex_lock(&mutex_log); \
> + flock(nx_gzip_log->_fileno, LOCK_EX); \
> + time_t t; struct tm *m; time(&t); m = localtime(&t); \
> + fprintf(nx_gzip_log, "[%04d/%02d/%02d %02d:%02d:%02d] " \
> + "pid %d: " fmt, \
> + (int)m->tm_year + 1900, (int)m->tm_mon+1, (int)m->tm_mday, \
> + (int)m->tm_hour, (int)m->tm_min, (int)m->tm_sec, \
> + (int)getpid(), ## __VA_ARGS__); \
> + fflush(nx_gzip_log); \
> + flock(nx_gzip_log->_fileno, LOCK_UN); \
> + pthread_mutex_unlock(&mutex_log); \
> +} while (0)
> +
> +/* Use in case of an error */
> +#define prt_err(fmt, ...) do { if (nx_dbg >= 0) { \
> + prt("%s:%u: Error: "fmt, \
> + __FILE__, __LINE__, ## __VA_ARGS__); \
> +}} while (0)
> +
> +/* Use in case of an warning */
> +#define prt_warn(fmt, ...) do { if (nx_dbg >= 1) { \
> + prt("%s:%u: Warning: "fmt, \
> + __FILE__, __LINE__, ## __VA_ARGS__); \
> +}} while (0)
> +
> +/* Informational printouts */
> +#define prt_info(fmt, ...) do { if (nx_dbg >= 2) { \
> + prt("Info: "fmt, ## __VA_ARGS__); \
> +}} while (0)
> +
> +/* Trace zlib wrapper code */
> +#define prt_trace(fmt, ...) do { if (nx_gzip_trace_enabled()) { \
> + prt("### "fmt, ## __VA_ARGS__); \
> +}} while (0)
> +
> +/* Trace statistics */
> +#define prt_stat(fmt, ...) do { if (nx_gzip_gather_statistics()) { \
> + prt("### "fmt, ## __VA_ARGS__); \
> +}} while (0)
> +
> +/* Trace zlib hardware implementation */
> +#define hw_trace(fmt, ...) do { \
> + if (nx_gzip_hw_trace_enabled()) \
> + fprintf(nx_gzip_log, "hhh " fmt, ## __VA_ARGS__); \
> + } while (0)
> +
> +/* Trace zlib software implementation */
> +#define sw_trace(fmt, ...) do { \
> + if (nx_gzip_sw_trace_enabled()) \
> + fprintf(nx_gzip_log, "sss " fmt, ## __VA_ARGS__); \
> + } while (0)
> +
> +
> +/**
> + * str_to_num - Convert string into number and copy with endings like
> + * KiB for kilobyte
> + * MiB for megabyte
> + * GiB for gigabyte
> + */
> +uint64_t str_to_num(char *str);
> +void nx_lib_debug(int onoff);
> +
> +#endif /* _NXU_DBG_H_ */
> diff --git a/tools/testing/selftests/powerpc/nx-gzip/inc/nxu.h b/tools/testing/selftests/powerpc/nx-gzip/inc/nxu.h
> new file mode 100644
> index 000000000000..faa95ffc162a
> --- /dev/null
> +++ b/tools/testing/selftests/powerpc/nx-gzip/inc/nxu.h
> @@ -0,0 +1,644 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later
> + *
> + * Hardware interface of the NX-GZIP compression accelerator
> + *
> + * Copyright (C) IBM Corporation, 2020
> + *
> + * Author: Bulent Abali <abali@us.ibm.com>
> + *
> + */
> +
> +#ifndef _NXU_H
> +#define _NXU_H
> +
> +#include <stdint.h>
> +#include <endian.h>
> +
> +/* deflate */
> +#define LLSZ 286
> +#define DSZ 30
> +
> +/* nx */
> +#define DHTSZ 18
> +#define DHT_MAXSZ 288
> +#define MAX_DDE_COUNT 256
> +
> +/* util */
> +#ifdef NXDBG
> +#define NXPRT(X) do { X; } while (0)
> +#else
> +#define NXPRT(X) do { ; } while (0)
> +#endif
> +
> +#ifdef NXTIMER
> +#include <sys/platform/ppc.h>
> +#define NX_CLK(X) do { X; } while (0)
> +#define nx_get_time() __ppc_get_timebase()
> +#define nx_get_freq() __ppc_get_timebase_freq()
> +#else
> +#define NX_CLK(X) do { ; } while (0)
> +#define nx_get_time() (-1)
> +#define nx_get_freq() (-1)
> +#endif
> +
> +/*
> + * Definitions of acronyms used here. See
> + * P9 NX Gzip Accelerator User's Manual for details
> + *
If I google "P9 NX Gzip Accelerator User's Manual" I just find the
POWER9 Processor User's Manual which seems to be a different
thing... following the links in that document to a GitHub repo I think
the document I'm looking for is
https://github.com/abalib/power-gzip/blob/master/power_nx_gzip_um.pdf
If that's right and the URL isstable, could you include a link in source
code? (Should perhaps the power-gzip repository be moved from a personal
github account to the open-power organisation?)
Regards,
Daniel
> + * adler/crc: 32 bit checksums appended to stream tail
> + * ce: completion extension
> + * cpb: coprocessor parameter block (metadata)
> + * crb: coprocessor request block (command)
> + * csb: coprocessor status block (status)
> + * dht: dynamic huffman table
> + * dde: data descriptor element (address, length)
> + * ddl: list of ddes
> + * dh/fh: dynamic and fixed huffman types
> + * fc: coprocessor function code
> + * histlen: history/dictionary length
> + * history: sliding window of up to 32KB of data
> + * lzcount: Deflate LZ symbol counts
> + * rembytecnt: remaining byte count
> + * sfbt: source final block type; last block's type during decomp
> + * spbc: source processed byte count
> + * subc: source unprocessed bit count
> + * tebc: target ending bit count; valid bits in the last byte
> + * tpbc: target processed byte count
> + * vas: virtual accelerator switch; the user mode interface
> + */
> +
> +typedef union {
> + uint32_t word[4];
> + uint64_t dword[2];
> +} nx_qw_t __attribute__((aligned (16)));
> +
> +/*
> + * Note: NX registers with fewer than 32 bits are declared by
> + * convention as uint32_t variables in unions. If *_offset and *_mask
> + * are defined for a variable, then use get_ put_ macros to
> + * conveniently access the register fields for endian conversions.
> + */
> +
> +typedef struct {
> + /* Data Descriptor Element, Section 6.4 */
> + union {
> + uint32_t dde_count;
> + /* When dde_count == 0 ddead is a pointer to a data buffer;
> + * ddebc is the buffer length bytes.
> + * When dde_count > 0 dde is an indirect dde; ddead is a pointer
> + * to a contiguous list of direct ddes; ddebc is the total length
> + * of all data pointed to by the list of direct ddes.
> + * Note that only one level of indirection is permitted.
> + * See Section 6.4 of the user manual for additional details
> + */
> + };
> + uint32_t ddebc; /* dde byte count */
> + uint64_t ddead; /* dde address */
> +} nx_dde_t __attribute__((aligned (16)));
> +
> +typedef struct {
> + /* Coprocessor Status Block, Section 6.6 */
> + union {
> + uint32_t csb_v;
> + /* Valid bit. v must be set to 0 by the program
> + * before submitting the coprocessor command.
> + * Software can poll for the v bit
> + */
> +
> + uint32_t csb_f;
> + /* 16B CSB size. Written to 0 by DMA when it writes the CPB */
> +
> + uint32_t csb_cs;
> + /* cs completion sequence; unused */
> +
> + uint32_t csb_cc;
> + /* cc completion code; cc != 0 exception occurred */
> +
> + uint32_t csb_ce;
> + /* ce completion extension */
> +
> + };
> + uint32_t tpbc;
> + /* target processed byte count TPBC */
> +
> + uint64_t fsaddr;
> + /* Section 6.12.1 CSB NonZero error summary. FSA Failing storage
> + * address. Address where error occurred. When available, written
> + * to A field of CSB
> + */
> +} nx_csb_t __attribute__((aligned (16)));
> +
> +typedef struct {
> + /* Coprocessor Completion Block, Section 6.7 */
> +
> + uint32_t reserved[3];
> + union {
> + /* When crb.c==0 (no ccb defined) it is reserved;
> + * When crb.c==1 (ccb defined) it is cm
> + */
> +
> + uint32_t ccb_cm;
> + /* Signal interrupt of crb.c==1 and cm==1 */
> +
> + uint32_t word;
> + /* generic access to the 32bit word */
> + };
> +} nx_ccb_t __attribute__((aligned (16)));
> +
> +typedef struct {
> + /*
> + * CRB operand of the paste coprocessor instruction is stamped
> + * in quadword 4 with the information shown here as its written
> + * in to the receive FIFO of the coprocessor
> + */
> +
> + union {
> + uint32_t vas_buf_num;
> + /* Verification only vas buffer number which correlates to
> + * the low order bits of the atag in the paste command
> + */
> +
> + uint32_t send_wc_id;
> + /* Pointer to Send Window Context that provides for NX address
> + * translation information, such as MSR and LPCR bits, job completion
> + * interrupt RA, PSWID, and job utilization counter.
> + */
> +
> + };
> + union {
> + uint32_t recv_wc_id;
> + /* Pointer to Receive Window Context. NX uses this to return
> + * credits to a Receive FIFO as entries are dequeued.
> + */
> +
> + };
> + uint32_t reserved2;
> + union {
> + uint32_t vas_invalid;
> + /* Invalid bit. If this bit is 1 the CRB is discarded by
> + * NX upon fetching from the receive FIFO. If this bit is 0
> + * the CRB is processed normally. The bit is stamped to 0
> + * by VAS and may be written to 1 by hypervisor while
> + * the CRB is in the receive FIFO (in memory).
> + */
> +
> + };
> +} vas_stamped_crb_t;
> +
> +typedef struct {
> + /*
> + * A CRB that has a translation fault is stamped by NX in quadword 4
> + * and pasted to the Fault Send Window in VAS.
> + */
> + uint64_t fsa;
> + union {
> + uint32_t nxsf_t;
> + uint32_t nxsf_fs;
> + };
> + uint32_t pswid;
> +} nx_stamped_fault_crb_t;
> +
> +typedef union {
> + vas_stamped_crb_t vas;
> + nx_stamped_fault_crb_t nx;
> +} stamped_crb_t;
> +
> +typedef struct {
> + /*
> + * Coprocessor Parameter Block In/Out are used to pass metadata
> + * to/from accelerator. Tables 6.5 and 6.6 of the user manual.
> + */
> +
> + /* CPBInput */
> +
> + struct {
> + union {
> + nx_qw_t qw0;
> + struct {
> + uint32_t in_adler; /* bits 0:31 */
> + uint32_t in_crc; /* bits 32:63 */
> + union {
> + uint32_t in_histlen; /* bits 64:75 */
> + uint32_t in_subc; /* bits 93:95 */
> + };
> + union {
> + uint32_t in_sfbt; /* bits 108:111 */
> + uint32_t in_rembytecnt; /* bits 112:127 */
> + uint32_t in_dhtlen; /* bits 116:127 */
> + };
> + };
> + };
> + union {
> + nx_qw_t in_dht[DHTSZ]; /* qw[1:18] */
> + char in_dht_char[DHT_MAXSZ]; /* byte access */
> + };
> + nx_qw_t reserved[5]; /* qw[19:23] */
> + };
> +
> + /* CPBOutput */
> +
> + volatile struct {
> + union {
> + nx_qw_t qw24;
> + struct {
> + uint32_t out_adler; /* bits 0:31 qw[24] */
> + uint32_t out_crc; /* bits 32:63 qw[24] */
> + union {
> + uint32_t out_tebc; /* bits 77:79 qw[24] */
> + uint32_t out_subc; /* bits 80:95 qw[24] */
> + };
> + union {
> + uint32_t out_sfbt; /* bits 108:111 qw[24] */
> + uint32_t out_rembytecnt; /* bits 112:127 qw[24] */
> + uint32_t out_dhtlen; /* bits 116:127 qw[24] */
> + };
> + };
> + };
> + union {
> + nx_qw_t qw25[79]; /* qw[25:103] */
> + /* qw[25] compress no lzcounts or wrap */
> + uint32_t out_spbc_comp_wrap;
> + uint32_t out_spbc_wrap; /* qw[25] wrap */
> + uint32_t out_spbc_comp; /* qw[25] compress no lzcounts */
> + uint32_t out_lzcount[LLSZ+DSZ]; /* 286 LL and 30 D symbol counts */
> + struct {
> + nx_qw_t out_dht[DHTSZ]; /* qw[25:42] */
> + uint32_t out_spbc_decomp; /* qw[43] decompress */
> + };
> + };
> + /* qw[104] compress with lzcounts */
> + uint32_t out_spbc_comp_with_count;
> + };
> +} nx_gzip_cpb_t __attribute__((aligned (128)));
> +
> +typedef struct {
> + union { /* byte[0:3] */
> + uint32_t gzip_fc; /* bits[24-31] */
> + };
> + uint32_t reserved1; /* byte[4:7] */
> + union {
> + uint64_t csb_address; /* byte[8:15] */
> + struct {
> + uint32_t reserved2;
> + union {
> + uint32_t crb_c;
> + /* c==0 no ccb defined */
> +
> + uint32_t crb_at;
> + /* at==0 address type is ignored;
> + * all addrs effective assumed.
> + */
> +
> + };
> + };
> + };
> + nx_dde_t source_dde; /* byte[16:31] */
> + nx_dde_t target_dde; /* byte[32:47] */
> + volatile nx_ccb_t ccb; /* byte[48:63] */
> + volatile union {
> + /* byte[64:239] shift csb by 128 bytes out of the crb; csb was in crb
> + * earlier; JReilly says csb written with partial inject.
> + */
> + nx_qw_t reserved64[11];
> + stamped_crb_t stamp; /* byte[64:79] */
> + };
> + volatile nx_csb_t csb;
> +} nx_gzip_crb_t __attribute__((aligned (128)));
> +
> +
> +typedef struct {
> + nx_gzip_crb_t crb;
> + nx_gzip_cpb_t cpb;
> +} nx_gzip_crb_cpb_t __attribute__((aligned (2048)));
> +
> +
> +/*
> + * NX hardware convention has the msb bit on the left numbered 0.
> + * The defines below has *_offset defined as the right most bit
> + * position of a field. x of size_mask(x) is the field width in bits.
> + */
> +
> +#define size_mask(x) ((1U<<(x))-1)
> +
> +/*
> + * Offsets and Widths within the containing 32 bits of the various NX
> + * gzip hardware registers. Use the getnn/putnn macros to access
> + * these regs
> + */
> +
> +#define dde_count_mask size_mask(8)
> +#define dde_count_offset 23
> +
> +/* CSB */
> +
> +#define csb_v_mask size_mask(1)
> +#define csb_v_offset 0
> +#define csb_f_mask size_mask(1)
> +#define csb_f_offset 6
> +#define csb_cs_mask size_mask(8)
> +#define csb_cs_offset 15
> +#define csb_cc_mask size_mask(8)
> +#define csb_cc_offset 23
> +#define csb_ce_mask size_mask(8)
> +#define csb_ce_offset 31
> +
> +/* CCB */
> +
> +#define ccb_cm_mask size_mask(3)
> +#define ccb_cm_offset 31
> +
> +/* VAS stamped CRB fields */
> +
> +#define vas_buf_num_mask size_mask(6)
> +#define vas_buf_num_offset 5
> +#define send_wc_id_mask size_mask(16)
> +#define send_wc_id_offset 31
> +#define recv_wc_id_mask size_mask(16)
> +#define recv_wc_id_offset 31
> +#define vas_invalid_mask size_mask(1)
> +#define vas_invalid_offset 31
> +
> +/* NX stamped fault CRB fields */
> +
> +#define nxsf_t_mask size_mask(1)
> +#define nxsf_t_offset 23
> +#define nxsf_fs_mask size_mask(8)
> +#define nxsf_fs_offset 31
> +
> +/* CPB input */
> +
> +#define in_histlen_mask size_mask(12)
> +#define in_histlen_offset 11
> +#define in_dhtlen_mask size_mask(12)
> +#define in_dhtlen_offset 31
> +#define in_subc_mask size_mask(3)
> +#define in_subc_offset 31
> +#define in_sfbt_mask size_mask(4)
> +#define in_sfbt_offset 15
> +#define in_rembytecnt_mask size_mask(16)
> +#define in_rembytecnt_offset 31
> +
> +/* CPB output */
> +
> +#define out_tebc_mask size_mask(3)
> +#define out_tebc_offset 15
> +#define out_subc_mask size_mask(16)
> +#define out_subc_offset 31
> +#define out_sfbt_mask size_mask(4)
> +#define out_sfbt_offset 15
> +#define out_rembytecnt_mask size_mask(16)
> +#define out_rembytecnt_offset 31
> +#define out_dhtlen_mask size_mask(12)
> +#define out_dhtlen_offset 31
> +
> +/* CRB */
> +
> +#define gzip_fc_mask size_mask(8)
> +#define gzip_fc_offset 31
> +#define crb_c_mask size_mask(1)
> +#define crb_c_offset 28
> +#define crb_at_mask size_mask(1)
> +#define crb_at_offset 30
> +#define csb_address_mask ~(15UL) /* mask off bottom 4b */
> +
> +/*
> + * Access macros for the registers. Do not access registers directly
> + * because of the endian conversion. P9 processor may run either as
> + * Little or Big endian. However the NX coprocessor regs are always
> + * big endian.
> + * Use the 32 and 64b macros to access respective
> + * register sizes.
> + * Use nn forms for the register fields shorter than 32 bits.
> + */
> +
> +#define getnn(ST, REG) ((be32toh(ST.REG) >> (31-REG##_offset)) \
> + & REG##_mask)
> +#define getpnn(ST, REG) ((be32toh((ST)->REG) >> (31-REG##_offset)) \
> + & REG##_mask)
> +#define get32(ST, REG) (be32toh(ST.REG))
> +#define getp32(ST, REG) (be32toh((ST)->REG))
> +#define get64(ST, REG) (be64toh(ST.REG))
> +#define getp64(ST, REG) (be64toh((ST)->REG))
> +
> +#define unget32(ST, REG) (get32(ST, REG) & ~((REG##_mask) \
> + << (31-REG##_offset)))
> +/* get 32bits less the REG field */
> +
> +#define ungetp32(ST, REG) (getp32(ST, REG) & ~((REG##_mask) \
> + << (31-REG##_offset)))
> +/* get 32bits less the REG field */
> +
> +#define clear_regs(ST) do { memset((void *)(&(ST)), 0, sizeof(ST)); \
> + } while (0)
> +#define clear_dde(ST) do { ST.dde_count = ST.ddebc = 0; ST.ddead = 0; \
> + } while (0)
> +#define clearp_dde(ST) do { (ST)->dde_count = (ST)->ddebc = 0; \
> + (ST)->ddead = 0; \
> + } while (0)
> +#define clear_struct(ST) do { memset((void *)(&(ST)), 0, sizeof(ST)); \
> + } while (0)
> +
> +#define putnn(ST, REG, X) do { ST.REG = htobe32(unget32(ST, REG) | (((X) \
> + & REG##_mask) << (31-REG##_offset))); \
> + } while (0)
> +#define putpnn(ST, REG, X) do { (ST)->REG = htobe32(ungetp32(ST, REG) \
> + | (((X) & REG##_mask) << (31-REG##_offset))); \
> + } while (0)
> +
> +#define put32(ST, REG, X) do { ST.REG = htobe32(X); } while (0)
> +#define putp32(ST, REG, X) do { (ST)->REG = htobe32(X); } while (0)
> +#define put64(ST, REG, X) do { ST.REG = htobe64(X); } while (0)
> +#define putp64(ST, REG, X) do { (ST)->REG = htobe64(X); } while (0)
> +
> +/*
> + * Completion extension ce(0) ce(1) ce(2). Bits ce(3-7)
> + * unused. Section 6.6 Figure 6.7.
> + */
> +
> +#define get_csb_ce(ST) ((uint32_t)getnn(ST, csb_ce))
> +#define get_csb_ce_ms3b(ST) (get_csb_ce(ST) >> 5)
> +#define put_csb_ce_ms3b(ST, X) do { putnn(ST, csb_ce, ((uint32_t)(X) << 5)); \
> + } while (0)
> +
> +#define CSB_CE_PARTIAL 0x4
> +#define CSB_CE_TERMINATE 0x2
> +#define CSB_CE_TPBC_VALID 0x1
> +
> +#define csb_ce_termination(X) (!!((X) & CSB_CE_TERMINATE))
> +/* termination, output buffers may be modified, SPBC/TPBC invalid Fig.6-7 */
> +
> +#define csb_ce_check_completion(X) (!csb_ce_termination(X))
> +/* if not terminated then check full or partial completion */
> +
> +#define csb_ce_partial_completion(X) (!!((X) & CSB_CE_PARTIAL))
> +#define csb_ce_full_completion(X) (!csb_ce_partial_completion(X))
> +#define csb_ce_tpbc_valid(X) (!!((X) & CSB_CE_TPBC_VALID))
> +/* TPBC indicates successfully stored data count */
> +
> +#define csb_ce_default_err(X) csb_ce_termination(X)
> +/* most error CEs have CE(0)=0 and CE(1)=1 */
> +
> +#define csb_ce_cc3_partial(X) csb_ce_partial_completion(X)
> +/* some CC=3 are partially completed, Table 6-8 */
> +
> +#define csb_ce_cc64(X) ((X)&(CSB_CE_PARTIAL \
> + | CSB_CE_TERMINATE) == 0)
> +/* Compression: when TPBC>SPBC then CC=64 Table 6-8; target didn't
> + * compress smaller than source.
> + */
> +
> +/* Decompress SFBT combinations Tables 5-3, 6-4, 6-6 */
> +
> +#define SFBT_BFINAL 0x1
> +#define SFBT_LIT 0x4
> +#define SFBT_FHT 0x5
> +#define SFBT_DHT 0x6
> +#define SFBT_HDR 0x7
> +
> +/*
> + * NX gzip function codes. Table 6.2.
> + * Bits 0:4 are the FC. Bit 5 is used by the DMA controller to
> + * select one of the two Byte Count Limits.
> + */
> +
> +#define GZIP_FC_LIMIT_MASK 0x01
> +#define GZIP_FC_COMPRESS_FHT 0x00
> +#define GZIP_FC_COMPRESS_DHT 0x02
> +#define GZIP_FC_COMPRESS_FHT_COUNT 0x04
> +#define GZIP_FC_COMPRESS_DHT_COUNT 0x06
> +#define GZIP_FC_COMPRESS_RESUME_FHT 0x08
> +#define GZIP_FC_COMPRESS_RESUME_DHT 0x0a
> +#define GZIP_FC_COMPRESS_RESUME_FHT_COUNT 0x0c
> +#define GZIP_FC_COMPRESS_RESUME_DHT_COUNT 0x0e
> +#define GZIP_FC_DECOMPRESS 0x10
> +#define GZIP_FC_DECOMPRESS_SINGLE_BLK_N_SUSPEND 0x12
> +#define GZIP_FC_DECOMPRESS_RESUME 0x14
> +#define GZIP_FC_DECOMPRESS_RESUME_SINGLE_BLK_N_SUSPEND 0x16
> +#define GZIP_FC_WRAP 0x1e
> +
> +#define fc_is_compress(fc) (((fc) & 0x10) == 0)
> +#define fc_has_count(fc) (fc_is_compress(fc) && (((fc) & 0x4) != 0))
> +
> +/* CSB.CC Error codes */
> +
> +#define ERR_NX_OK 0
> +#define ERR_NX_ALIGNMENT 1
> +#define ERR_NX_OPOVERLAP 2
> +#define ERR_NX_DATA_LENGTH 3
> +#define ERR_NX_TRANSLATION 5
> +#define ERR_NX_PROTECTION 6
> +#define ERR_NX_EXTERNAL_UE7 7
> +#define ERR_NX_INVALID_OP 8
> +#define ERR_NX_PRIVILEGE 9
> +#define ERR_NX_INTERNAL_UE 10
> +#define ERR_NX_EXTERN_UE_WR 12
> +#define ERR_NX_TARGET_SPACE 13
> +#define ERR_NX_EXCESSIVE_DDE 14
> +#define ERR_NX_TRANSL_WR 15
> +#define ERR_NX_PROTECT_WR 16
> +#define ERR_NX_SUBFUNCTION 17
> +#define ERR_NX_FUNC_ABORT 18
> +#define ERR_NX_BYTE_MAX 19
> +#define ERR_NX_CORRUPT_CRB 20
> +#define ERR_NX_INVALID_CRB 21
> +#define ERR_NX_INVALID_DDE 30
> +#define ERR_NX_SEGMENTED_DDL 31
> +#define ERR_NX_DDE_OVERFLOW 33
> +#define ERR_NX_TPBC_GT_SPBC 64
> +#define ERR_NX_MISSING_CODE 66
> +#define ERR_NX_INVALID_DIST 67
> +#define ERR_NX_INVALID_DHT 68
> +#define ERR_NX_EXTERNAL_UE90 90
> +#define ERR_NX_WDOG_TIMER 224
> +#define ERR_NX_AT_FAULT 250
> +#define ERR_NX_INTR_SERVER 252
> +#define ERR_NX_UE253 253
> +#define ERR_NX_NO_HW 254
> +#define ERR_NX_HUNG_OP 255
> +#define ERR_NX_END 256
> +
> +/* initial values for non-resume operations */
> +#define INIT_CRC 0 /* crc32(0L, Z_NULL, 0) */
> +#define INIT_ADLER 1 /* adler32(0L, Z_NULL, 0) adler is initalized to 1 */
> +
> +/* prototypes */
> +#ifdef NX_JOB_CALLBACK
> +int nxu_run_job(nx_gzip_crb_cpb_t *c, void *handle,
> + int (*callback)(const void *));
> +#else
> +int nxu_run_job(nx_gzip_crb_cpb_t *c, void *handle);
> +#endif
> +
> +
> +/* caller supplies a print buffer 4*sizeof(crb) */
> +
> +char *nx_crb_str(nx_gzip_crb_t *crb, char *prbuf);
> +char *nx_cpb_str(nx_gzip_cpb_t *cpb, char *prbuf);
> +char *nx_prt_hex(void *cp, int sz, char *prbuf);
> +char *nx_lzcount_str(nx_gzip_cpb_t *cpb, char *prbuf);
> +char *nx_strerror(int e);
> +
> +#ifdef NX_SIM
> +#include <stdio.h>
> +int nx_sim_init(void *ctx);
> +int nx_sim_end(void *ctx);
> +int nxu_run_sim_job(nx_gzip_crb_cpb_t *c, void *ctx);
> +#endif /* NX_SIM */
> +
> +/* Deflate stream manipulation */
> +
> +#define set_final_bit(x) do { x |= (unsigned char)1; } while (0)
> +#define clr_final_bit(x) do { x &= ~(unsigned char)1; } while (0)
> +
> +#define append_empty_fh_blk(p, b) do { *(p) = (2 | (1&(b))); *((p)+1) = 0; \
> + } while (0)
> +/* append 10 bits 0000001b 00...... ;
> + * assumes appending starts on a byte boundary; b is the final bit.
> + */
> +
> +
> +#ifdef NX_842
> +
> +/* 842 Engine */
> +
> +typedef struct {
> + union { /* byte[0:3] */
> + uint32_t eft_fc; /* bits[29-31] */
> + };
> + uint32_t reserved1; /* byte[4:7] */
> + union {
> + uint64_t csb_address; /* byte[8:15] */
> + struct {
> + uint32_t reserved2;
> + union {
> + uint32_t crb_c;
> + /* c==0 no ccb defined */
> +
> + uint32_t crb_at;
> + /* at==0 address type is ignored;
> + all addrs effective assumed */
> +
> + };
> + };
> + };
> + nx_dde_t source_dde; /* byte[16:31] */
> + nx_dde_t target_dde; /* byte[32:47] */
> + nx_ccb_t ccb; /* byte[48:63] */
> + union {
> + nx_qw_t reserved64[3]; /* byte[64:96] */
> + };
> + nx_csb_t csb;
> +} nx_eft_crb_t __attribute__((aligned (128)));
> +
> +/* 842 CRB */
> +
> +#define EFT_FC_MASK size_mask(3)
> +#define EFT_FC_OFFSET 31
> +#define EFT_FC_COMPRESS 0x0
> +#define EFT_FC_COMPRESS_WITH_CRC 0x1
> +#define EFT_FC_DECOMPRESS 0x2
> +#define EFT_FC_DECOMPRESS_WITH_CRC 0x3
> +#define EFT_FC_BLK_DATA_MOVE 0x4
> +#endif /* NX_842 */
> +
> +#endif /* _NXU_H */
> --
> 2.21.0
^ permalink raw reply
* Re: [patch V2 11/15] completion: Use simple wait queues
From: Logan Gunthorpe @ 2020-03-18 22:28 UTC (permalink / raw)
To: Thomas Gleixner, LKML
Cc: Randy Dunlap, Peter Zijlstra, linux-pci,
Sebastian Andrzej Siewior, Joel Fernandes, Will Deacon,
Ingo Molnar, Davidlohr Bueso, Paul E . McKenney, Arnd Bergmann,
linuxppc-dev, Steven Rostedt, Bjorn Helgaas, Kurt Schwemmer,
Kalle Valo, Felipe Balbi, Greg Kroah-Hartman, linux-usb,
linux-wireless, Oleg Nesterov, netdev, Linus Torvalds,
David S. Miller
In-Reply-To: <20200318204408.521507446@linutronix.de>
On 2020-03-18 2:43 p.m., Thomas Gleixner wrote:
> There is no semantical or functional change:
>
> - completions use the exclusive wait mode which is what swait provides
>
> - complete() wakes one exclusive waiter
>
> - complete_all() wakes all waiters while holding the lock which protects
> the wait queue against newly incoming waiters. The conversion to swait
> preserves this behaviour.
>
> complete_all() might cause unbound latencies with a large number of waiters
> being woken at once, but most complete_all() usage sites are either in
> testing or initialization code or have only a really small number of
> concurrent waiters which for now does not cause a latency problem. Keep it
> simple for now.
Seems like it would be worth adding a note for this to the
complete_all() doc string. Otherwise developers will not likely find out
about this issue and may not keep it as simple as you'd like.
Logan
^ permalink raw reply
* Re: [PATCH 4/5] selftests/powerpc: Add NX-GZIP engine decompress testcase
From: Daniel Axtens @ 2020-03-18 22:19 UTC (permalink / raw)
To: Raphael M Zinsly, linuxppc-dev, linux-crypto; +Cc: abali, haren, herbert
In-Reply-To: <016517d3-b341-172c-dc53-7c3351c93aa7@linux.ibm.com>
Raphael M Zinsly <rzinsly@linux.ibm.com> writes:
> Thanks for the reviews Daniel, I'll use your testcases and address the
> issues you found, I still have some questions bellow:
>
> On 18/03/2020 03:18, Daniel Axtens wrote:
>> Raphael Moreira Zinsly <rzinsly@linux.ibm.com> writes:
>>
>>> Include a decompression testcase for the powerpc NX-GZIP
>>> engine.
>>
>> I compiled gzip with the AFL++ fuzzer and generated a corpus of tests to
>> run against this decompressor. I also fuzzed the decompressor
>> directly. I found a few issues. I _think_ they're just in the userspace
>> but I'm a bit too early in the process to know.
>>
>> I realise this is self-test code but:
>> a) it stops me testing more deeply, and
> I don't understand what do you mean by that, what did you couldn't test?
I'm trying to stress-test the accellerator by fuzzing it. If it hangs
with an infinite loop rather than cleanly exiting, that inhibits my
ability to stress-test it.
>> b) it looks like some of this code is shared with https://github.com/libnxz/power-gzip/
> Is that an issue?
> This selftest were develop by the same team that develop the userspace
> library, the first version of this tests were pushed there in order for
> the team to review. It uses some of the headers of the library to access
> the accelerator and part of the code in the samples.
What I mean is that if there's a bug in code we copied, we should also
report it to the developers of the library so that it gets fixed in both
places.
Regards,
Daniel
> Regards,
> Raphael
>
>>
>> The issues I've found are:
>>
>> 1) In the ERR_NX_DATA_LENGTH case, the decompressor doesn't check that
>> you're making forward progress, so you can provoke it into an
>> infinite loop.
>>
>> Here's an _extremely_ ugly fix:
>>
>> diff --git a/tools/testing/selftests/powerpc/nx-gzip/gunz_test.c b/tools/testing/selftests/powerpc/nx-gzip/gunz_test.c
>> index 653de92698cc..236a1f567656 100644
>> --- a/tools/testing/selftests/powerpc/nx-gzip/gunz_test.c
>> +++ b/tools/testing/selftests/powerpc/nx-gzip/gunz_test.c
>> @@ -343,6 +343,8 @@ int decompress_file(int argc, char **argv, void *devhandle)
>> nx_dde_t dde_out[6] __attribute__((aligned (128)));
>> int pgfault_retries;
>>
>> + int last_first_used = 0;
>> +
>> /* when using mmap'ed files */
>> off_t input_file_offset;
>>
>> @@ -642,6 +644,11 @@ int decompress_file(int argc, char **argv, void *devhandle)
>> first_used = fifo_used_first_bytes(cur_in, used_in, fifo_in_len);
>> last_used = fifo_used_last_bytes(cur_in, used_in, fifo_in_len);
>>
>> + if (first_used > 0 && last_first_used > 0) {
>> + assert(first_used != last_first_used);
>> + }
>> + last_first_used = first_used;
>> +
>> if (first_used > 0)
>> nx_append_dde(ddl_in, fifo_in + cur_in, first_used);
>>
>>
>> 2) It looks like you can provoke an out-of-bounds write. I've seen both
>> infinte loops printing something that seems to come from the file
>> content like:
>>
>> 57201: Got signal 11 si_code 3, si_addr 0xcacacacacacacac8
>>
>> or a less bizzare address like
>>
>> 19285: Got signal 11 si_code 1, si_addr 0x7fffcf1b0000
>>
>> Depending on the build I've also seen the stack smasher protection fire.
>>
>> I don't understand the code well enough to figure out how this comes to
>> be just yet.
>>
>> I've included a few test cases as attachments. I've preconverted them
>> with xxd to avoid anything that might flag suspicious gzip files!
>> Decompress them then use `xxd -r attachment testcase.gz` to convert them
>> back.
>>
>> Regards,
>> Daniel
>>
>>
>>
>>
>>>
>>> Signed-off-by: Bulent Abali <abali@us.ibm.com>
>>> Signed-off-by: Raphael Moreira Zinsly <rzinsly@linux.ibm.com>
>>> ---
>>> .../selftests/powerpc/nx-gzip/Makefile | 7 +-
>>> .../selftests/powerpc/nx-gzip/gunz_test.c | 1058 +++++++++++++++++
>>> 2 files changed, 1062 insertions(+), 3 deletions(-)
>>> create mode 100644 tools/testing/selftests/powerpc/nx-gzip/gunz_test.c
>>>
>>> diff --git a/tools/testing/selftests/powerpc/nx-gzip/Makefile b/tools/testing/selftests/powerpc/nx-gzip/Makefile
>>> index ab903f63bbbd..82abc19a49a0 100644
>>> --- a/tools/testing/selftests/powerpc/nx-gzip/Makefile
>>> +++ b/tools/testing/selftests/powerpc/nx-gzip/Makefile
>>> @@ -1,9 +1,9 @@
>>> CC = gcc
>>> CFLAGS = -O3
>>> INC = ./inc
>>> -SRC = gzfht_test.c
>>> +SRC = gzfht_test.c gunz_test.c
>>> OBJ = $(SRC:.c=.o)
>>> -TESTS = gzfht_test
>>> +TESTS = gzfht_test gunz_test
>>> EXTRA_SOURCES = gzip_vas.c
>>>
>>> all: $(TESTS)
>>> @@ -16,6 +16,7 @@ $(TESTS): $(OBJ)
>>>
>>> run_tests: $(TESTS)
>>> ./gzfht_test gzip_vas.c
>>> + ./gunz_test gzip_vas.c.nx.gz
>>>
>>> clean:
>>> - rm -f $(TESTS) *.o *~ *.gz
>>> + rm -f $(TESTS) *.o *~ *.gz *.gunzip
>>> diff --git a/tools/testing/selftests/powerpc/nx-gzip/gunz_test.c b/tools/testing/selftests/powerpc/nx-gzip/gunz_test.c
>>> new file mode 100644
>>> index 000000000000..653de92698cc
>>> --- /dev/null
>>> +++ b/tools/testing/selftests/powerpc/nx-gzip/gunz_test.c
>>> @@ -0,0 +1,1058 @@
>>> +/* SPDX-License-Identifier: GPL-2.0-or-later
>>> + *
>>> + * P9 gunzip sample code for demonstrating the P9 NX hardware
>>> + * interface. Not intended for productive uses or for performance or
>>> + * compression ratio measurements. Note also that /dev/crypto/gzip,
>>> + * VAS and skiboot support are required
>>> + *
>>> + * Copyright 2020 IBM Corp.
>>> + *
>>> + * Author: Bulent Abali <abali@us.ibm.com>
>>> + *
>>> + * https://github.com/libnxz/power-gzip for zlib api and other utils
>>> + * Definitions of acronyms used here. See
>>> + * P9 NX Gzip Accelerator User's Manual for details
>>> + *
>>> + * adler/crc: 32 bit checksums appended to stream tail
>>> + * ce: completion extension
>>> + * cpb: coprocessor parameter block (metadata)
>>> + * crb: coprocessor request block (command)
>>> + * csb: coprocessor status block (status)
>>> + * dht: dynamic huffman table
>>> + * dde: data descriptor element (address, length)
>>> + * ddl: list of ddes
>>> + * dh/fh: dynamic and fixed huffman types
>>> + * fc: coprocessor function code
>>> + * histlen: history/dictionary length
>>> + * history: sliding window of up to 32KB of data
>>> + * lzcount: Deflate LZ symbol counts
>>> + * rembytecnt: remaining byte count
>>> + * sfbt: source final block type; last block's type during decomp
>>> + * spbc: source processed byte count
>>> + * subc: source unprocessed bit count
>>> + * tebc: target ending bit count; valid bits in the last byte
>>> + * tpbc: target processed byte count
>>> + * vas: virtual accelerator switch; the user mode interface
>>> + */
>>> +
>>> +#include <stdio.h>
>>> +#include <stdlib.h>
>>> +#include <string.h>
>>> +#include <unistd.h>
>>> +#include <stdint.h>
>>> +#include <sys/types.h>
>>> +#include <sys/stat.h>
>>> +#include <sys/time.h>
>>> +#include <sys/fcntl.h>
>>> +#include <sys/mman.h>
>>> +#include <endian.h>
>>> +#include <bits/endian.h>
>>> +#include <sys/ioctl.h>
>>> +#include <assert.h>
>>> +#include <errno.h>
>>> +#include <signal.h>
>>> +#include "nxu.h"
>>> +#include "nx.h"
>>> +
>>> +int nx_dbg = 0;
>>> +FILE *nx_gzip_log = NULL;
>>> +
>>> +#define NX_MIN(X, Y) (((X) < (Y))?(X):(Y))
>>> +#define NX_MAX(X, Y) (((X) > (Y))?(X):(Y))
>>> +
>>> +#define mb() asm volatile("sync" ::: "memory")
>>> +#define rmb() asm volatile("lwsync" ::: "memory")
>>> +#define wmb() rmb()
>>> +
>>> +const int fifo_in_len = 1<<24;
>>> +const int fifo_out_len = 1<<24;
>>> +const int page_sz = 1<<16;
>>> +const int line_sz = 1<<7;
>>> +const int window_max = 1<<15;
>>> +const int retry_max = 50;
>>> +
>>> +extern void *nx_fault_storage_address;
>>> +extern void *nx_function_begin(int function, int pri);
>>> +extern int nx_function_end(void *handle);
>>> +
>>> +/*
>>> + * Fault in pages prior to NX job submission. wr=1 may be required to
>>> + * touch writeable pages. System zero pages do not fault-in the page as
>>> + * intended. Typically set wr=1 for NX target pages and set wr=0 for
>>> + * NX source pages.
>>> + */
>>> +static int nx_touch_pages(void *buf, long buf_len, long page_len, int wr)
>>> +{
>>> + char *begin = buf;
>>> + char *end = (char *) buf + buf_len - 1;
>>> + volatile char t;
>>> +
>>> + assert(buf_len >= 0 && !!buf);
>>> +
>>> + NXPRT(fprintf(stderr, "touch %p %p len 0x%lx wr=%d\n", buf,
>>> + buf + buf_len, buf_len, wr));
>>> +
>>> + if (buf_len <= 0 || buf == NULL)
>>> + return -1;
>>> +
>>> + do {
>>> + t = *begin;
>>> + if (wr)
>>> + *begin = t;
>>> + begin = begin + page_len;
>>> + } while (begin < end);
>>> +
>>> + /* When buf_sz is small or buf tail is in another page. */
>>> + t = *end;
>>> + if (wr)
>>> + *end = t;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +void sigsegv_handler(int sig, siginfo_t *info, void *ctx)
>>> +{
>>> + fprintf(stderr, "%d: Got signal %d si_code %d, si_addr %p\n", getpid(),
>>> + sig, info->si_code, info->si_addr);
>>> +
>>> + nx_fault_storage_address = info->si_addr;
>>> +}
>>> +
>>> +/*
>>> + * Adds an (address, len) pair to the list of ddes (ddl) and updates
>>> + * the base dde. ddl[0] is the only dde in a direct dde which
>>> + * contains a single (addr,len) pair. For more pairs, ddl[0] becomes
>>> + * the indirect (base) dde that points to a list of direct ddes.
>>> + * See Section 6.4 of the NX-gzip user manual for DDE description.
>>> + * Addr=NULL, len=0 clears the ddl[0]. Returns the total number of
>>> + * bytes in ddl. Caller is responsible for allocting the array of
>>> + * nx_dde_t *ddl. If N addresses are required in the scatter-gather
>>> + * list, the ddl array must have N+1 entries minimum.
>>> + */
>>> +static inline uint32_t nx_append_dde(nx_dde_t *ddl, void *addr, uint32_t len)
>>> +{
>>> + uint32_t ddecnt;
>>> + uint32_t bytes;
>>> +
>>> + if (addr == NULL && len == 0) {
>>> + clearp_dde(ddl);
>>> + return 0;
>>> + }
>>> +
>>> + NXPRT(fprintf(stderr, "%d: nx_append_dde addr %p len %x\n", __LINE__,
>>> + addr, len));
>>> +
>>> + /* Number of ddes in the dde list ; == 0 when it is a direct dde */
>>> + ddecnt = getpnn(ddl, dde_count);
>>> + bytes = getp32(ddl, ddebc);
>>> +
>>> + if (ddecnt == 0 && bytes == 0) {
>>> + /* First dde is unused; make it a direct dde */
>>> + bytes = len;
>>> + putp32(ddl, ddebc, bytes);
>>> + putp64(ddl, ddead, (uint64_t) addr);
>>> + } else if (ddecnt == 0) {
>>> + /* Converting direct to indirect dde
>>> + * ddl[0] becomes head dde of ddl
>>> + * copy direct to indirect first.
>>> + */
>>> + ddl[1] = ddl[0];
>>> +
>>> + /* Add the new dde next */
>>> + clear_dde(ddl[2]);
>>> + put32(ddl[2], ddebc, len);
>>> + put64(ddl[2], ddead, (uint64_t) addr);
>>> +
>>> + /* Ddl head points to 2 direct ddes */
>>> + ddecnt = 2;
>>> + putpnn(ddl, dde_count, ddecnt);
>>> + bytes = bytes + len;
>>> + putp32(ddl, ddebc, bytes);
>>> + /* Pointer to the first direct dde */
>>> + putp64(ddl, ddead, (uint64_t) &ddl[1]);
>>> + } else {
>>> + /* Append a dde to an existing indirect ddl */
>>> + ++ddecnt;
>>> + clear_dde(ddl[ddecnt]);
>>> + put64(ddl[ddecnt], ddead, (uint64_t) addr);
>>> + put32(ddl[ddecnt], ddebc, len);
>>> +
>>> + putpnn(ddl, dde_count, ddecnt);
>>> + bytes = bytes + len;
>>> + putp32(ddl, ddebc, bytes); /* byte sum of all dde */
>>> + }
>>> + return bytes;
>>> +}
>>> +
>>> +/*
>>> + * Touch specified number of pages represented in number bytes
>>> + * beginning from the first buffer in a dde list.
>>> + * Do not touch the pages past buf_sz-th byte's page.
>>> + *
>>> + * Set buf_sz = 0 to touch all pages described by the ddep.
>>> + */
>>> +static int nx_touch_pages_dde(nx_dde_t *ddep, long buf_sz, long page_sz,
>>> + int wr)
>>> +{
>>> + uint32_t indirect_count;
>>> + uint32_t buf_len;
>>> + long total;
>>> + uint64_t buf_addr;
>>> + nx_dde_t *dde_list;
>>> + int i;
>>> +
>>> + assert(!!ddep);
>>> +
>>> + indirect_count = getpnn(ddep, dde_count);
>>> +
>>> + NXPRT(fprintf(stderr, "nx_touch_pages_dde dde_count %d request len \
>>> + 0x%lx\n", indirect_count, buf_sz));
>>> +
>>> + if (indirect_count == 0) {
>>> + /* Direct dde */
>>> + buf_len = getp32(ddep, ddebc);
>>> + buf_addr = getp64(ddep, ddead);
>>> +
>>> + NXPRT(fprintf(stderr, "touch direct ddebc 0x%x ddead %p\n",
>>> + buf_len, (void *)buf_addr));
>>> +
>>> + if (buf_sz == 0)
>>> + nx_touch_pages((void *)buf_addr, buf_len, page_sz, wr);
>>> + else
>>> + nx_touch_pages((void *)buf_addr, NX_MIN(buf_len,
>>> + buf_sz), page_sz, wr);
>>> +
>>> + return ERR_NX_OK;
>>> + }
>>> +
>>> + /* Indirect dde */
>>> + if (indirect_count > MAX_DDE_COUNT)
>>> + return ERR_NX_EXCESSIVE_DDE;
>>> +
>>> + /* First address of the list */
>>> + dde_list = (nx_dde_t *) getp64(ddep, ddead);
>>> +
>>> + if (buf_sz == 0)
>>> + buf_sz = getp32(ddep, ddebc);
>>> +
>>> + total = 0;
>>> + for (i = 0; i < indirect_count; i++) {
>>> + buf_len = get32(dde_list[i], ddebc);
>>> + buf_addr = get64(dde_list[i], ddead);
>>> + total += buf_len;
>>> +
>>> + NXPRT(fprintf(stderr, "touch loop len 0x%x ddead %p total \
>>> + 0x%lx\n", buf_len, (void *)buf_addr, total));
>>> +
>>> + /* Touching fewer pages than encoded in the ddebc */
>>> + if (total > buf_sz) {
>>> + buf_len = NX_MIN(buf_len, total - buf_sz);
>>> + nx_touch_pages((void *)buf_addr, buf_len, page_sz, wr);
>>> + NXPRT(fprintf(stderr, "touch loop break len 0x%x \
>>> + ddead %p\n", buf_len, (void *)buf_addr));
>>> + break;
>>> + }
>>> + nx_touch_pages((void *)buf_addr, buf_len, page_sz, wr);
>>> + }
>>> + return ERR_NX_OK;
>>> +}
>>> +
>>> +/*
>>> + * Src and dst buffers are supplied in scatter gather lists.
>>> + * NX function code and other parameters supplied in cmdp.
>>> + */
>>> +static int nx_submit_job(nx_dde_t *src, nx_dde_t *dst, nx_gzip_crb_cpb_t *cmdp,
>>> + void *handle)
>>> +{
>>> + int cc;
>>> + uint64_t csbaddr;
>>> +
>>> + memset((void *)&cmdp->crb.csb, 0, sizeof(cmdp->crb.csb));
>>> +
>>> + cmdp->crb.source_dde = *src;
>>> + cmdp->crb.target_dde = *dst;
>>> +
>>> + /* Status, output byte count in tpbc */
>>> + csbaddr = ((uint64_t) &cmdp->crb.csb) & csb_address_mask;
>>> + put64(cmdp->crb, csb_address, csbaddr);
>>> +
>>> + /* NX reports input bytes in spbc; cleared */
>>> + cmdp->cpb.out_spbc_comp_wrap = 0;
>>> + cmdp->cpb.out_spbc_comp_with_count = 0;
>>> + cmdp->cpb.out_spbc_decomp = 0;
>>> +
>>> + /* Clear output */
>>> + put32(cmdp->cpb, out_crc, INIT_CRC);
>>> + put32(cmdp->cpb, out_adler, INIT_ADLER);
>>> +
>>> + cc = nxu_run_job(cmdp, handle);
>>> +
>>> + if (!cc)
>>> + cc = getnn(cmdp->crb.csb, csb_cc); /* CC Table 6-8 */
>>> +
>>> + return cc;
>>> +}
>>> +
>>> +/* fifo queue management */
>>> +#define fifo_used_bytes(used) (used)
>>> +#define fifo_free_bytes(used, len) ((len)-(used))
>>> +/* amount of free bytes in the first and last parts */
>>> +#define fifo_free_first_bytes(cur, used, len) ((((cur)+(used)) <= (len)) \
>>> + ? (len)-((cur)+(used)) : 0)
>>> +#define fifo_free_last_bytes(cur, used, len) ((((cur)+(used)) <= (len)) \
>>> + ? (cur) : (len)-(used))
>>> +/* amount of used bytes in the first and last parts */
>>> +#define fifo_used_first_bytes(cur, used, len) ((((cur)+(used)) <= (len)) \
>>> + ? (used) : (len)-(cur))
>>> +#define fifo_used_last_bytes(cur, used, len) ((((cur)+(used)) <= (len)) \
>>> + ? 0 : ((used)+(cur))-(len))
>>> +/* first and last free parts start here */
>>> +#define fifo_free_first_offset(cur, used) ((cur)+(used))
>>> +#define fifo_free_last_offset(cur, used, len) \
>>> + fifo_used_last_bytes(cur, used, len)
>>> +/* first and last used parts start here */
>>> +#define fifo_used_first_offset(cur) (cur)
>>> +#define fifo_used_last_offset(cur) (0)
>>> +
>>> +int decompress_file(int argc, char **argv, void *devhandle)
>>> +{
>>> + FILE *inpf;
>>> + FILE *outf;
>>> +
>>> + int c, expect, i, cc, rc = 0;
>>> + char gzfname[1024];
>>> +
>>> + /* Queuing, file ops, byte counting */
>>> + char *fifo_in, *fifo_out;
>>> + int used_in, cur_in, used_out, cur_out, read_sz, n;
>>> + int first_free, last_free, first_used, last_used;
>>> + int first_offset, last_offset;
>>> + int write_sz, free_space, source_sz;
>>> + int source_sz_estimate, target_sz_estimate;
>>> + uint64_t last_comp_ratio; /* 1000 max */
>>> + uint64_t total_out;
>>> + int is_final, is_eof;
>>> +
>>> + /* nx hardware */
>>> + int sfbt, subc, spbc, tpbc, nx_ce, fc, resuming = 0;
>>> + int history_len = 0;
>>> + nx_gzip_crb_cpb_t cmd, *cmdp;
>>> + nx_dde_t *ddl_in;
>>> + nx_dde_t dde_in[6] __attribute__((aligned (128)));
>>> + nx_dde_t *ddl_out;
>>> + nx_dde_t dde_out[6] __attribute__((aligned (128)));
>>> + int pgfault_retries;
>>> +
>>> + /* when using mmap'ed files */
>>> + off_t input_file_offset;
>>> +
>>> + if (argc > 2) {
>>> + fprintf(stderr, "usage: %s <fname> or stdin\n", argv[0]);
>>> + fprintf(stderr, " writes to stdout or <fname>.nx.gunzip\n");
>>> + return -1;
>>> + }
>>> +
>>> + if (argc == 1) {
>>> + inpf = stdin;
>>> + outf = stdout;
>>> + } else if (argc == 2) {
>>> + char w[1024];
>>> + char *wp;
>>> + inpf = fopen(argv[1], "r");
>>> + if (inpf == NULL) {
>>> + perror(argv[1]);
>>> + return -1;
>>> + }
>>> +
>>> + /* Make a new file name to write to. Ignoring '.gz' */
>>> + wp = (NULL != (wp = strrchr(argv[1], '/'))) ? ++wp : argv[1];
>>> + strcpy(w, wp);
>>> + strcat(w, ".nx.gunzip");
>>> +
>>> + outf = fopen(w, "w");
>>> + if (outf == NULL) {
>>> + perror(w);
>>> + return -1;
>>> + }
>>> + }
>>> +
>>> +#define GETINPC(X) fgetc(X)
>>> +
>>> + /* Decode the gzip header */
>>> + c = GETINPC(inpf); expect = 0x1f; /* ID1 */
>>> + if (c != expect)
>>> + goto err1;
>>> +
>>> + c = GETINPC(inpf); expect = 0x8b; /* ID2 */
>>> + if (c != expect)
>>> + goto err1;
>>> +
>>> + c = GETINPC(inpf); expect = 0x08; /* CM */
>>> + if (c != expect)
>>> + goto err1;
>>> +
>>> + int flg = GETINPC(inpf); /* FLG */
>>> + if (flg & 0b11100000 || flg & 0b100)
>>> + goto err2;
>>> +
>>> + fprintf(stderr, "gzHeader FLG %x\n", flg);
>>> +
>>> + /* Read 6 bytes; ignoring the MTIME, XFL, OS fields in this
>>> + * sample code.
>>> + */
>>> + for (i = 0; i < 6; i++) {
>>> + char tmp[10];
>>> + if (EOF == (tmp[i] = GETINPC(inpf)))
>>> + goto err3;
>>> + fprintf(stderr, "%02x ", tmp[i]);
>>> + if (i == 5)
>>> + fprintf(stderr, "\n");
>>> + }
>>> + fprintf(stderr, "gzHeader MTIME, XFL, OS ignored\n");
>>> +
>>> + /* FNAME */
>>> + if (flg & 0b1000) {
>>> + int k = 0;
>>> + do {
>>> + if (EOF == (c = GETINPC(inpf)))
>>> + goto err3;
>>> + gzfname[k++] = c;
>>> + } while (c);
>>> + fprintf(stderr, "gzHeader FNAME: %s\n", gzfname);
>>> + }
>>> +
>>> + /* FHCRC */
>>> + if (flg & 0b10) {
>>> + c = GETINPC(inpf); c = GETINPC(inpf);
>>> + fprintf(stderr, "gzHeader FHCRC: ignored\n");
>>> + }
>>> +
>>> + used_in = cur_in = used_out = cur_out = 0;
>>> + is_final = is_eof = 0;
>>> +
>>> + /* Allocate one page larger to prevent page faults due to NX
>>> + * overfetching.
>>> + * Either do this (char*)(uintptr_t)aligned_alloc or use
>>> + * -std=c11 flag to make the int-to-pointer warning go away.
>>> + */
>>> + assert((fifo_in = (char *)(uintptr_t)aligned_alloc(line_sz,
>>> + fifo_in_len + page_sz)) != NULL);
>>> + assert((fifo_out = (char *)(uintptr_t)aligned_alloc(line_sz,
>>> + fifo_out_len + page_sz + line_sz)) != NULL);
>>> + /* Leave unused space due to history rounding rules */
>>> + fifo_out = fifo_out + line_sz;
>>> + nx_touch_pages(fifo_out, fifo_out_len, page_sz, 1);
>>> +
>>> + ddl_in = &dde_in[0];
>>> + ddl_out = &dde_out[0];
>>> + cmdp = &cmd;
>>> + memset(&cmdp->crb, 0, sizeof(cmdp->crb));
>>> +
>>> +read_state:
>>> +
>>> + /* Read from .gz file */
>>> +
>>> + NXPRT(fprintf(stderr, "read_state:\n"));
>>> +
>>> + if (is_eof != 0)
>>> + goto write_state;
>>> +
>>> + /* We read in to fifo_in in two steps: first: read in to from
>>> + * cur_in to the end of the buffer. last: if free space wrapped
>>> + * around, read from fifo_in offset 0 to offset cur_in.
>>> + */
>>> +
>>> + /* Reset fifo head to reduce unnecessary wrap arounds */
>>> + cur_in = (used_in == 0) ? 0 : cur_in;
>>> +
>>> + /* Free space total is reduced by a gap */
>>> + free_space = NX_MAX(0, fifo_free_bytes(used_in, fifo_in_len)
>>> + - line_sz);
>>> +
>>> + /* Free space may wrap around as first and last */
>>> + first_free = fifo_free_first_bytes(cur_in, used_in, fifo_in_len);
>>> + last_free = fifo_free_last_bytes(cur_in, used_in, fifo_in_len);
>>> +
>>> + /* Start offsets of the free memory */
>>> + first_offset = fifo_free_first_offset(cur_in, used_in);
>>> + last_offset = fifo_free_last_offset(cur_in, used_in, fifo_in_len);
>>> +
>>> + /* Reduce read_sz because of the line_sz gap */
>>> + read_sz = NX_MIN(free_space, first_free);
>>> + n = 0;
>>> + if (read_sz > 0) {
>>> + /* Read in to offset cur_in + used_in */
>>> + n = fread(fifo_in + first_offset, 1, read_sz, inpf);
>>> + used_in = used_in + n;
>>> + free_space = free_space - n;
>>> + assert(n <= read_sz);
>>> + if (n != read_sz) {
>>> + /* Either EOF or error; exit the read loop */
>>> + is_eof = 1;
>>> + goto write_state;
>>> + }
>>> + }
>>> +
>>> + /* If free space wrapped around */
>>> + if (last_free > 0) {
>>> + /* Reduce read_sz because of the line_sz gap */
>>> + read_sz = NX_MIN(free_space, last_free);
>>> + n = 0;
>>> + if (read_sz > 0) {
>>> + n = fread(fifo_in + last_offset, 1, read_sz, inpf);
>>> + used_in = used_in + n; /* Increase used space */
>>> + free_space = free_space - n; /* Decrease free space */
>>> + assert(n <= read_sz);
>>> + if (n != read_sz) {
>>> + /* Either EOF or error; exit the read loop */
>>> + is_eof = 1;
>>> + goto write_state;
>>> + }
>>> + }
>>> + }
>>> +
>>> + /* At this point we have used_in bytes in fifo_in with the
>>> + * data head starting at cur_in and possibly wrapping around.
>>> + */
>>> +
>>> +write_state:
>>> +
>>> + /* Write decompressed data to output file */
>>> +
>>> + NXPRT(fprintf(stderr, "write_state:\n"));
>>> +
>>> + if (used_out == 0)
>>> + goto decomp_state;
>>> +
>>> + /* If fifo_out has data waiting, write it out to the file to
>>> + * make free target space for the accelerator used bytes in
>>> + * the first and last parts of fifo_out.
>>> + */
>>> +
>>> + first_used = fifo_used_first_bytes(cur_out, used_out, fifo_out_len);
>>> + last_used = fifo_used_last_bytes(cur_out, used_out, fifo_out_len);
>>> +
>>> + write_sz = first_used;
>>> +
>>> + n = 0;
>>> + if (write_sz > 0) {
>>> + n = fwrite(fifo_out + cur_out, 1, write_sz, outf);
>>> + used_out = used_out - n;
>>> + /* Move head of the fifo */
>>> + cur_out = (cur_out + n) % fifo_out_len;
>>> + assert(n <= write_sz);
>>> + if (n != write_sz) {
>>> + fprintf(stderr, "error: write\n");
>>> + rc = -1;
>>> + goto err5;
>>> + }
>>> + }
>>> +
>>> + if (last_used > 0) { /* If more data available in the last part */
>>> + write_sz = last_used; /* Keep it here for later */
>>> + n = 0;
>>> + if (write_sz > 0) {
>>> + n = fwrite(fifo_out, 1, write_sz, outf);
>>> + used_out = used_out - n;
>>> + cur_out = (cur_out + n) % fifo_out_len;
>>> + assert(n <= write_sz);
>>> + if (n != write_sz) {
>>> + fprintf(stderr, "error: write\n");
>>> + rc = -1;
>>> + goto err5;
>>> + }
>>> + }
>>> + }
>>> +
>>> +decomp_state:
>>> +
>>> + /* NX decompresses input data */
>>> +
>>> + NXPRT(fprintf(stderr, "decomp_state:\n"));
>>> +
>>> + if (is_final)
>>> + goto finish_state;
>>> +
>>> + /* Address/len lists */
>>> + clearp_dde(ddl_in);
>>> + clearp_dde(ddl_out);
>>> +
>>> + /* FC, CRC, HistLen, Table 6-6 */
>>> + if (resuming) {
>>> + /* Resuming a partially decompressed input.
>>> + * The key to resume is supplying the 32KB
>>> + * dictionary (history) to NX, which is basically
>>> + * the last 32KB of output produced.
>>> + */
>>> + fc = GZIP_FC_DECOMPRESS_RESUME;
>>> +
>>> + cmdp->cpb.in_crc = cmdp->cpb.out_crc;
>>> + cmdp->cpb.in_adler = cmdp->cpb.out_adler;
>>> +
>>> + /* Round up the history size to quadword. Section 2.10 */
>>> + history_len = (history_len + 15) / 16;
>>> + putnn(cmdp->cpb, in_histlen, history_len);
>>> + history_len = history_len * 16; /* bytes */
>>> +
>>> + if (history_len > 0) {
>>> + /* Chain in the history buffer to the DDE list */
>>> + if (cur_out >= history_len) {
>>> + nx_append_dde(ddl_in, fifo_out
>>> + + (cur_out - history_len),
>>> + history_len);
>>> + } else {
>>> + nx_append_dde(ddl_in, fifo_out
>>> + + ((fifo_out_len + cur_out)
>>> + - history_len),
>>> + history_len - cur_out);
>>> + /* Up to 32KB history wraps around fifo_out */
>>> + nx_append_dde(ddl_in, fifo_out, cur_out);
>>> + }
>>> +
>>> + }
>>> + } else {
>>> + /* First decompress job */
>>> + fc = GZIP_FC_DECOMPRESS;
>>> +
>>> + history_len = 0;
>>> + /* Writing 0 clears out subc as well */
>>> + cmdp->cpb.in_histlen = 0;
>>> + total_out = 0;
>>> +
>>> + put32(cmdp->cpb, in_crc, INIT_CRC);
>>> + put32(cmdp->cpb, in_adler, INIT_ADLER);
>>> + put32(cmdp->cpb, out_crc, INIT_CRC);
>>> + put32(cmdp->cpb, out_adler, INIT_ADLER);
>>> +
>>> + /* Assuming 10% compression ratio initially; use the
>>> + * most recently measured compression ratio as a
>>> + * heuristic to estimate the input and output
>>> + * sizes. If we give too much input, the target buffer
>>> + * overflows and NX cycles are wasted, and then we
>>> + * must retry with smaller input size. 1000 is 100%.
>>> + */
>>> + last_comp_ratio = 100UL;
>>> + }
>>> + cmdp->crb.gzip_fc = 0;
>>> + putnn(cmdp->crb, gzip_fc, fc);
>>> +
>>> + /*
>>> + * NX source buffers
>>> + */
>>> + first_used = fifo_used_first_bytes(cur_in, used_in, fifo_in_len);
>>> + last_used = fifo_used_last_bytes(cur_in, used_in, fifo_in_len);
>>> +
>>> + if (first_used > 0)
>>> + nx_append_dde(ddl_in, fifo_in + cur_in, first_used);
>>> +
>>> + if (last_used > 0)
>>> + nx_append_dde(ddl_in, fifo_in, last_used);
>>> +
>>> + /*
>>> + * NX target buffers
>>> + */
>>> + first_free = fifo_free_first_bytes(cur_out, used_out, fifo_out_len);
>>> + last_free = fifo_free_last_bytes(cur_out, used_out, fifo_out_len);
>>> +
>>> + /* Reduce output free space amount not to overwrite the history */
>>> + int target_max = NX_MAX(0, fifo_free_bytes(used_out, fifo_out_len)
>>> + - (1<<16));
>>> +
>>> + NXPRT(fprintf(stderr, "target_max %d (0x%x)\n", target_max,
>>> + target_max));
>>> +
>>> + first_free = NX_MIN(target_max, first_free);
>>> + if (first_free > 0) {
>>> + first_offset = fifo_free_first_offset(cur_out, used_out);
>>> + nx_append_dde(ddl_out, fifo_out + first_offset, first_free);
>>> + }
>>> +
>>> + if (last_free > 0) {
>>> + last_free = NX_MIN(target_max - first_free, last_free);
>>> + if (last_free > 0) {
>>> + last_offset = fifo_free_last_offset(cur_out, used_out,
>>> + fifo_out_len);
>>> + nx_append_dde(ddl_out, fifo_out + last_offset,
>>> + last_free);
>>> + }
>>> + }
>>> +
>>> + /* Target buffer size is used to limit the source data size
>>> + * based on previous measurements of compression ratio.
>>> + */
>>> +
>>> + /* source_sz includes history */
>>> + source_sz = getp32(ddl_in, ddebc);
>>> + assert(source_sz > history_len);
>>> + source_sz = source_sz - history_len;
>>> +
>>> + /* Estimating how much source is needed to 3/4 fill a
>>> + * target_max size target buffer. If we overshoot, then NX
>>> + * must repeat the job with smaller input and we waste
>>> + * bandwidth. If we undershoot then we use more NX calls than
>>> + * necessary.
>>> + */
>>> +
>>> + source_sz_estimate = ((uint64_t)target_max * last_comp_ratio * 3UL)
>>> + / 4000;
>>> +
>>> + if (source_sz_estimate < source_sz) {
>>> + /* Target might be small, therefore limiting the
>>> + * source data.
>>> + */
>>> + source_sz = source_sz_estimate;
>>> + target_sz_estimate = target_max;
>>> + } else {
>>> + /* Source file might be small, therefore limiting target
>>> + * touch pages to a smaller value to save processor cycles.
>>> + */
>>> + target_sz_estimate = ((uint64_t)source_sz * 1000UL)
>>> + / (last_comp_ratio + 1);
>>> + target_sz_estimate = NX_MIN(2 * target_sz_estimate,
>>> + target_max);
>>> + }
>>> +
>>> + source_sz = source_sz + history_len;
>>> +
>>> + /* Some NX condition codes require submitting the NX job again.
>>> + * Kernel doesn't handle NX page faults. Expects user code to
>>> + * touch pages.
>>> + */
>>> + pgfault_retries = retry_max;
>>> +
>>> +restart_nx:
>>> +
>>> + putp32(ddl_in, ddebc, source_sz);
>>> +
>>> + /* Fault in pages */
>>> + nx_touch_pages_dde(ddl_in, 0, page_sz, 0);
>>> + nx_touch_pages_dde(ddl_out, target_sz_estimate, page_sz, 1);
>>> +
>>> + /* Send job to NX */
>>> + cc = nx_submit_job(ddl_in, ddl_out, cmdp, devhandle);
>>> +
>>> + switch (cc) {
>>> +
>>> + case ERR_NX_TRANSLATION:
>>> +
>>> + /* We touched the pages ahead of time. In the most common case
>>> + * we shouldn't be here. But may be some pages were paged out.
>>> + * Kernel should have placed the faulting address to fsaddr.
>>> + */
>>> + NXPRT(fprintf(stderr, "ERR_NX_TRANSLATION %p\n",
>>> + (void *)cmdp->crb.csb.fsaddr));
>>> +
>>> + /* Touch 1 byte, read-only */
>>> + nx_touch_pages((void *)cmdp->crb.csb.fsaddr, 1, page_sz, 0);
>>> +
>>> + if (pgfault_retries == retry_max) {
>>> + /* Try once with exact number of pages */
>>> + --pgfault_retries;
>>> + goto restart_nx;
>>> + } else if (pgfault_retries > 0) {
>>> + /* If still faulting try fewer input pages
>>> + * assuming memory outage
>>> + */
>>> + if (source_sz > page_sz)
>>> + source_sz = NX_MAX(source_sz / 2, page_sz);
>>> + --pgfault_retries;
>>> + goto restart_nx;
>>> + } else {
>>> + fprintf(stderr, "cannot make progress; too many page \
>>> + fault retries cc= %d\n", cc);
>>> + rc = -1;
>>> + goto err5;
>>> + }
>>> +
>>> + case ERR_NX_DATA_LENGTH:
>>> +
>>> + NXPRT(fprintf(stderr, "ERR_NX_DATA_LENGTH; not an error \
>>> + usually; stream may have trailing data\n"));
>>> +
>>> + /* Not an error in the most common case; it just says
>>> + * there is trailing data that we must examine.
>>> + *
>>> + * CC=3 CE(1)=0 CE(0)=1 indicates partial completion
>>> + * Fig.6-7 and Table 6-8.
>>> + */
>>> + nx_ce = get_csb_ce_ms3b(cmdp->crb.csb);
>>> +
>>> + if (!csb_ce_termination(nx_ce) &&
>>> + csb_ce_partial_completion(nx_ce)) {
>>> + /* Check CPB for more information
>>> + * spbc and tpbc are valid
>>> + */
>>> + sfbt = getnn(cmdp->cpb, out_sfbt); /* Table 6-4 */
>>> + subc = getnn(cmdp->cpb, out_subc); /* Table 6-4 */
>>> + spbc = get32(cmdp->cpb, out_spbc_decomp);
>>> + tpbc = get32(cmdp->crb.csb, tpbc);
>>> + assert(target_max >= tpbc);
>>> +
>>> + goto ok_cc3; /* not an error */
>>> + } else {
>>> + /* History length error when CE(1)=1 CE(0)=0. */
>>> + rc = -1;
>>> + fprintf(stderr, "history length error cc= %d\n", cc);
>>> + goto err5;
>>> + }
>>> +
>>> + case ERR_NX_TARGET_SPACE:
>>> +
>>> + /* Target buffer not large enough; retry smaller input
>>> + * data; give at least 1 byte. SPBC/TPBC are not valid.
>>> + */
>>> + assert(source_sz > history_len);
>>> + source_sz = ((source_sz - history_len + 2) / 2) + history_len;
>>> + NXPRT(fprintf(stderr, "ERR_NX_TARGET_SPACE; retry with \
>>> + smaller input data src %d hist %d\n", source_sz,
>>> + history_len));
>>> + goto restart_nx;
>>> +
>>> + case ERR_NX_OK:
>>> +
>>> + /* This should not happen for gzip formatted data;
>>> + * we need trailing crc and isize
>>> + */
>>> + fprintf(stderr, "ERR_NX_OK\n");
>>> + spbc = get32(cmdp->cpb, out_spbc_decomp);
>>> + tpbc = get32(cmdp->crb.csb, tpbc);
>>> + assert(target_max >= tpbc);
>>> + assert(spbc >= history_len);
>>> + source_sz = spbc - history_len;
>>> + goto offsets_state;
>>> +
>>> + default:
>>> + fprintf(stderr, "error: cc= %d\n", cc);
>>> + rc = -1;
>>> + goto err5;
>>> + }
>>> +
>>> +ok_cc3:
>>> +
>>> + NXPRT(fprintf(stderr, "cc3: sfbt: %x\n", sfbt));
>>> +
>>> + assert(spbc > history_len);
>>> + source_sz = spbc - history_len;
>>> +
>>> + /* Table 6-4: Source Final Block Type (SFBT) describes the
>>> + * last processed deflate block and clues the software how to
>>> + * resume the next job. SUBC indicates how many input bits NX
>>> + * consumed but did not process. SPBC indicates how many
>>> + * bytes of source were given to the accelerator including
>>> + * history bytes.
>>> + */
>>> +
>>> + switch (sfbt) {
>>> + int dhtlen;
>>> +
>>> + case 0b0000: /* Deflate final EOB received */
>>> +
>>> + /* Calculating the checksum start position. */
>>> +
>>> + source_sz = source_sz - subc / 8;
>>> + is_final = 1;
>>> + break;
>>> +
>>> + /* Resume decompression cases are below. Basically
>>> + * indicates where NX has suspended and how to resume
>>> + * the input stream.
>>> + */
>>> +
>>> + case 0b1000: /* Within a literal block; use rembytecount */
>>> + case 0b1001: /* Within a literal block; use rembytecount; bfinal=1 */
>>> +
>>> + /* Supply the partially processed source byte again */
>>> + source_sz = source_sz - ((subc + 7) / 8);
>>> +
>>> + /* SUBC LS 3bits: number of bits in the first source byte need
>>> + * to be processed.
>>> + * 000 means all 8 bits; Table 6-3
>>> + * Clear subc, histlen, sfbt, rembytecnt, dhtlen
>>> + */
>>> + cmdp->cpb.in_subc = 0;
>>> + cmdp->cpb.in_sfbt = 0;
>>> + putnn(cmdp->cpb, in_subc, subc % 8);
>>> + putnn(cmdp->cpb, in_sfbt, sfbt);
>>> + putnn(cmdp->cpb, in_rembytecnt, getnn(cmdp->cpb,
>>> + out_rembytecnt));
>>> + break;
>>> +
>>> + case 0b1010: /* Within a FH block; */
>>> + case 0b1011: /* Within a FH block; bfinal=1 */
>>> +
>>> + source_sz = source_sz - ((subc + 7) / 8);
>>> +
>>> + /* Clear subc, histlen, sfbt, rembytecnt, dhtlen */
>>> + cmdp->cpb.in_subc = 0;
>>> + cmdp->cpb.in_sfbt = 0;
>>> + putnn(cmdp->cpb, in_subc, subc % 8);
>>> + putnn(cmdp->cpb, in_sfbt, sfbt);
>>> + break;
>>> +
>>> + case 0b1100: /* Within a DH block; */
>>> + case 0b1101: /* Within a DH block; bfinal=1 */
>>> +
>>> + source_sz = source_sz - ((subc + 7) / 8);
>>> +
>>> + /* Clear subc, histlen, sfbt, rembytecnt, dhtlen */
>>> + cmdp->cpb.in_subc = 0;
>>> + cmdp->cpb.in_sfbt = 0;
>>> + putnn(cmdp->cpb, in_subc, subc % 8);
>>> + putnn(cmdp->cpb, in_sfbt, sfbt);
>>> +
>>> + dhtlen = getnn(cmdp->cpb, out_dhtlen);
>>> + putnn(cmdp->cpb, in_dhtlen, dhtlen);
>>> + assert(dhtlen >= 42);
>>> +
>>> + /* Round up to a qword */
>>> + dhtlen = (dhtlen + 127) / 128;
>>> +
>>> + while (dhtlen > 0) { /* Copy dht from cpb.out to cpb.in */
>>> + --dhtlen;
>>> + cmdp->cpb.in_dht[dhtlen] = cmdp->cpb.out_dht[dhtlen];
>>> + }
>>> + break;
>>> +
>>> + case 0b1110: /* Within a block header; bfinal=0; */
>>> + /* Also given if source data exactly ends (SUBC=0) with
>>> + * EOB code with BFINAL=0. Means the next byte will
>>> + * contain a block header.
>>> + */
>>> + case 0b1111: /* within a block header with BFINAL=1. */
>>> +
>>> + source_sz = source_sz - ((subc + 7) / 8);
>>> +
>>> + /* Clear subc, histlen, sfbt, rembytecnt, dhtlen */
>>> + cmdp->cpb.in_subc = 0;
>>> + cmdp->cpb.in_sfbt = 0;
>>> + putnn(cmdp->cpb, in_subc, subc % 8);
>>> + putnn(cmdp->cpb, in_sfbt, sfbt);
>>> + }
>>> +
>>> +offsets_state:
>>> +
>>> + /* Adjust the source and target buffer offsets and lengths */
>>> +
>>> + NXPRT(fprintf(stderr, "offsets_state:\n"));
>>> +
>>> + /* Delete input data from fifo_in */
>>> + used_in = used_in - source_sz;
>>> + cur_in = (cur_in + source_sz) % fifo_in_len;
>>> + input_file_offset = input_file_offset + source_sz;
>>> +
>>> + /* Add output data to fifo_out */
>>> + used_out = used_out + tpbc;
>>> +
>>> + assert(used_out <= fifo_out_len);
>>> +
>>> + total_out = total_out + tpbc;
>>> +
>>> + /* Deflate history is 32KB max. No need to supply more
>>> + * than 32KB on a resume.
>>> + */
>>> + history_len = (total_out > window_max) ? window_max : total_out;
>>> +
>>> + /* To estimate expected expansion in the next NX job; 500 means 50%.
>>> + * Deflate best case is around 1 to 1000.
>>> + */
>>> + last_comp_ratio = (1000UL * ((uint64_t)source_sz + 1))
>>> + / ((uint64_t)tpbc + 1);
>>> + last_comp_ratio = NX_MAX(NX_MIN(1000UL, last_comp_ratio), 1);
>>> + NXPRT(fprintf(stderr, "comp_ratio %ld source_sz %d spbc %d tpbc %d\n",
>>> + last_comp_ratio, source_sz, spbc, tpbc));
>>> +
>>> + resuming = 1;
>>> +
>>> +finish_state:
>>> +
>>> + NXPRT(fprintf(stderr, "finish_state:\n"));
>>> +
>>> + if (is_final) {
>>> + if (used_out)
>>> + goto write_state; /* More data to write out */
>>> + else if (used_in < 8) {
>>> + /* Need at least 8 more bytes containing gzip crc
>>> + * and isize.
>>> + */
>>> + rc = -1;
>>> + goto err4;
>>> + } else {
>>> + /* Compare checksums and exit */
>>> + int i;
>>> + char tail[8];
>>> + uint32_t cksum, isize;
>>> + for (i = 0; i < 8; i++)
>>> + tail[i] = fifo_in[(cur_in + i) % fifo_in_len];
>>> + fprintf(stderr, "computed checksum %08x isize %08x\n",
>>> + cmdp->cpb.out_crc, (uint32_t) (total_out
>>> + % (1ULL<<32)));
>>> + cksum = (tail[0] | tail[1]<<8 | tail[2]<<16
>>> + | tail[3]<<24);
>>> + isize = (tail[4] | tail[5]<<8 | tail[6]<<16
>>> + | tail[7]<<24);
>>> + fprintf(stderr, "stored checksum %08x isize %08x\n",
>>> + cksum, isize);
>>> +
>>> + if (cksum == cmdp->cpb.out_crc && isize == (uint32_t)
>>> + (total_out % (1ULL<<32))) {
>>> + rc = 0; goto ok1;
>>> + } else {
>>> + rc = -1; goto err4;
>>> + }
>>> + }
>>> + } else
>>> + goto read_state;
>>> +
>>> + return -1;
>>> +
>>> +err1:
>>> + fprintf(stderr, "error: not a gzip file, expect %x, read %x\n",
>>> + expect, c);
>>> + return -1;
>>> +
>>> +err2:
>>> + fprintf(stderr, "error: the FLG byte is wrong or not handled by this \
>>> + code sample\n");
>>> + return -1;
>>> +
>>> +err3:
>>> + fprintf(stderr, "error: gzip header\n");
>>> + return -1;
>>> +
>>> +err4:
>>> + fprintf(stderr, "error: checksum\n");
>>> +
>>> +err5:
>>> +ok1:
>>> + fprintf(stderr, "decomp is complete: fclose\n");
>>> + fclose(outf);
>>> +
>>> + return rc;
>>> +}
>>> +
>>> +
>>> +int main(int argc, char **argv)
>>> +{
>>> + int rc;
>>> + struct sigaction act;
>>> + void *handle;
>>> +
>>> + act.sa_handler = 0;
>>> + act.sa_sigaction = sigsegv_handler;
>>> + act.sa_flags = SA_SIGINFO;
>>> + act.sa_restorer = 0;
>>> + sigemptyset(&act.sa_mask);
>>> + sigaction(SIGSEGV, &act, NULL);
>>> +
>>> + handle = nx_function_begin(NX_FUNC_COMP_GZIP, 0);
>>> + if (!handle) {
>>> + fprintf(stderr, "Unable to init NX, errno %d\n", errno);
>>> + exit(-1);
>>> + }
>>> +
>>> + rc = decompress_file(argc, argv, handle);
>>> +
>>> + nx_function_end(handle);
>>> +
>>> + return rc;
>>> +}
>>> --
>>> 2.21.0
>
> --
> Raphael Moreira Zinsly
> IBM
> Linux on Power Toolchain
^ permalink raw reply
* Re: [PATCH 1/4] hugetlbfs: add arch_hugetlb_valid_size
From: Dave Hansen @ 2020-03-18 22:15 UTC (permalink / raw)
To: Mike Kravetz, linux-mm, linux-kernel, linux-arm-kernel,
linuxppc-dev, linux-riscv, linux-s390, sparclinux, linux-doc
Cc: Albert Ou, Andrew Morton, Vasily Gorbik, Jonathan Corbet,
Catalin Marinas, Dave Hansen, Heiko Carstens,
Christian Borntraeger, Ingo Molnar, Palmer Dabbelt, Paul Walmsley,
Paul Mackerras, Thomas Gleixner, Longpeng, Will Deacon,
David S . Miller
In-Reply-To: <20200318220634.32100-2-mike.kravetz@oracle.com>
Hi Mike,
The series looks like a great idea to me. One nit on the x86 bits,
though...
> diff --git a/arch/x86/mm/hugetlbpage.c b/arch/x86/mm/hugetlbpage.c
> index 5bfd5aef5378..51e6208fdeec 100644
> --- a/arch/x86/mm/hugetlbpage.c
> +++ b/arch/x86/mm/hugetlbpage.c
> @@ -181,16 +181,25 @@ hugetlb_get_unmapped_area(struct file *file, unsigned long addr,
> #endif /* CONFIG_HUGETLB_PAGE */
>
> #ifdef CONFIG_X86_64
> +bool __init arch_hugetlb_valid_size(unsigned long long size)
> +{
> + if (size == PMD_SIZE)
> + return true;
> + else if (size == PUD_SIZE && boot_cpu_has(X86_FEATURE_GBPAGES))
> + return true;
> + else
> + return false;
> +}
I'm pretty sure it's possible to have a system without 2M/PMD page
support. We even have a handy-dandy comment about it in
arch/x86/include/asm/required-features.h:
#ifdef CONFIG_X86_64
#ifdef CONFIG_PARAVIRT
/* Paravirtualized systems may not have PSE or PGE available */
#define NEED_PSE 0
...
I *think* you need an X86_FEATURE_PSE check here to be totally correct.
if (size == PMD_SIZE && cpu_feature_enabled(X86_FEATURE_PSE))
return true;
BTW, I prefer cpu_feature_enabled() to boot_cpu_has() because it
includes disabled-features checking. I don't think any of it matters
for these specific features, but I generally prefer it on principle.
^ permalink raw reply
* Re: [patch V2 02/15] pci/switchtec: Replace completion wait queue usage for poll
From: Logan Gunthorpe @ 2020-03-18 22:11 UTC (permalink / raw)
To: Thomas Gleixner, LKML
Cc: Randy Dunlap, Peter Zijlstra, linux-pci,
Sebastian Andrzej Siewior, Joel Fernandes, Will Deacon,
Ingo Molnar, Davidlohr Bueso, Arnd Bergmann, Paul E . McKenney,
linuxppc-dev, Steven Rostedt, Bjorn Helgaas, Kurt Schwemmer,
Kalle Valo, Felipe Balbi, Greg Kroah-Hartman, linux-usb,
linux-wireless, Oleg Nesterov, netdev, Linus Torvalds,
David S. Miller
In-Reply-To: <20200318204407.607241357@linutronix.de>
On 2020-03-18 2:43 p.m., Thomas Gleixner wrote:
> From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>
> The poll callback is using the completion wait queue and sticks it into
> poll_wait() to wake up pollers after a command has completed.
>
> This works to some extent, but cannot provide EPOLLEXCLUSIVE support
> because the waker side uses complete_all() which unconditionally wakes up
> all waiters. complete_all() is required because completions internally use
> exclusive wait and complete() only wakes up one waiter by default.
>
> This mixes conceptually different mechanisms and relies on internal
> implementation details of completions, which in turn puts contraints on
> changing the internal implementation of completions.
>
> Replace it with a regular wait queue and store the state in struct
> switchtec_user.
>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
While I've been against open coding the completion in this driver for a
while, I'm convinced by the EPOLLEXCLUSIVE argument for this change.
I've reviewed and lightly tested the change with hardware:
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
Thanks,
Logan
> Cc: Kurt Schwemmer <kurt.schwemmer@microsemi.com>
> Cc: Logan Gunthorpe <logang@deltatee.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: linux-pci@vger.kernel.org
> ---
> V2: Reworded changelog.
> ---
> drivers/pci/switch/switchtec.c | 22 +++++++++++++---------
> 1 file changed, 13 insertions(+), 9 deletions(-)
>
> --- a/drivers/pci/switch/switchtec.c
> +++ b/drivers/pci/switch/switchtec.c
> @@ -52,10 +52,11 @@ struct switchtec_user {
>
> enum mrpc_state state;
>
> - struct completion comp;
> + wait_queue_head_t cmd_comp;
> struct kref kref;
> struct list_head list;
>
> + bool cmd_done;
> u32 cmd;
> u32 status;
> u32 return_code;
> @@ -77,7 +78,7 @@ static struct switchtec_user *stuser_cre
> stuser->stdev = stdev;
> kref_init(&stuser->kref);
> INIT_LIST_HEAD(&stuser->list);
> - init_completion(&stuser->comp);
> + init_waitqueue_head(&stuser->cmd_comp);
> stuser->event_cnt = atomic_read(&stdev->event_cnt);
>
> dev_dbg(&stdev->dev, "%s: %p\n", __func__, stuser);
> @@ -175,7 +176,7 @@ static int mrpc_queue_cmd(struct switcht
> kref_get(&stuser->kref);
> stuser->read_len = sizeof(stuser->data);
> stuser_set_state(stuser, MRPC_QUEUED);
> - reinit_completion(&stuser->comp);
> + stuser->cmd_done = false;
> list_add_tail(&stuser->list, &stdev->mrpc_queue);
>
> mrpc_cmd_submit(stdev);
> @@ -222,7 +223,8 @@ static void mrpc_complete_cmd(struct swi
> memcpy_fromio(stuser->data, &stdev->mmio_mrpc->output_data,
> stuser->read_len);
> out:
> - complete_all(&stuser->comp);
> + stuser->cmd_done = true;
> + wake_up_interruptible(&stuser->cmd_comp);
> list_del_init(&stuser->list);
> stuser_put(stuser);
> stdev->mrpc_busy = 0;
> @@ -529,10 +531,11 @@ static ssize_t switchtec_dev_read(struct
> mutex_unlock(&stdev->mrpc_mutex);
>
> if (filp->f_flags & O_NONBLOCK) {
> - if (!try_wait_for_completion(&stuser->comp))
> + if (!stuser->cmd_done)
> return -EAGAIN;
> } else {
> - rc = wait_for_completion_interruptible(&stuser->comp);
> + rc = wait_event_interruptible(stuser->cmd_comp,
> + stuser->cmd_done);
> if (rc < 0)
> return rc;
> }
> @@ -580,7 +583,7 @@ static __poll_t switchtec_dev_poll(struc
> struct switchtec_dev *stdev = stuser->stdev;
> __poll_t ret = 0;
>
> - poll_wait(filp, &stuser->comp.wait, wait);
> + poll_wait(filp, &stuser->cmd_comp, wait);
> poll_wait(filp, &stdev->event_wq, wait);
>
> if (lock_mutex_and_test_alive(stdev))
> @@ -588,7 +591,7 @@ static __poll_t switchtec_dev_poll(struc
>
> mutex_unlock(&stdev->mrpc_mutex);
>
> - if (try_wait_for_completion(&stuser->comp))
> + if (stuser->cmd_done)
> ret |= EPOLLIN | EPOLLRDNORM;
>
> if (stuser->event_cnt != atomic_read(&stdev->event_cnt))
> @@ -1272,7 +1275,8 @@ static void stdev_kill(struct switchtec_
>
> /* Wake up and kill any users waiting on an MRPC request */
> list_for_each_entry_safe(stuser, tmpuser, &stdev->mrpc_queue, list) {
> - complete_all(&stuser->comp);
> + stuser->cmd_done = true;
> + wake_up_interruptible(&stuser->cmd_comp);
> list_del_init(&stuser->list);
> stuser_put(stuser);
> }
>
^ permalink raw reply
* Re: [PATCH 1/4] hugetlbfs: add arch_hugetlb_valid_size
From: Will Deacon @ 2020-03-18 22:09 UTC (permalink / raw)
To: Mike Kravetz
Cc: linux-doc, Catalin Marinas, Dave Hansen, Heiko Carstens, linux-mm,
Paul Mackerras, sparclinux, linux-riscv, linux-s390,
Jonathan Corbet, Christian Borntraeger, Ingo Molnar, Longpeng,
Albert Ou, Vasily Gorbik, Paul Walmsley, Thomas Gleixner,
linux-arm-kernel, linux-kernel, Palmer Dabbelt, Andrew Morton,
linuxppc-dev, David S . Miller
In-Reply-To: <20200318220634.32100-2-mike.kravetz@oracle.com>
On Wed, Mar 18, 2020 at 03:06:31PM -0700, Mike Kravetz wrote:
> The architecture independent routine hugetlb_default_setup sets up
> the default huge pages size. It has no way to verify if the passed
> value is valid, so it accepts it and attempts to validate at a later
> time. This requires undocumented cooperation between the arch specific
> and arch independent code.
>
> For architectures that support more than one huge page size, provide
> a routine arch_hugetlb_valid_size to validate a huge page size.
> hugetlb_default_setup can use this to validate passed values.
>
> arch_hugetlb_valid_size will also be used in a subsequent patch to
> move processing of the "hugepagesz=" in arch specific code to a common
> routine in arch independent code.
>
> Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
> ---
> arch/arm64/include/asm/hugetlb.h | 2 ++
> arch/arm64/mm/hugetlbpage.c | 19 ++++++++++++++-----
> arch/powerpc/include/asm/hugetlb.h | 3 +++
> arch/powerpc/mm/hugetlbpage.c | 20 +++++++++++++-------
> arch/riscv/include/asm/hugetlb.h | 3 +++
> arch/riscv/mm/hugetlbpage.c | 28 ++++++++++++++++++----------
> arch/s390/include/asm/hugetlb.h | 3 +++
> arch/s390/mm/hugetlbpage.c | 18 +++++++++++++-----
> arch/sparc/include/asm/hugetlb.h | 3 +++
> arch/sparc/mm/init_64.c | 23 ++++++++++++++++-------
> arch/x86/include/asm/hugetlb.h | 3 +++
> arch/x86/mm/hugetlbpage.c | 21 +++++++++++++++------
> include/linux/hugetlb.h | 7 +++++++
> mm/hugetlb.c | 16 +++++++++++++---
> 14 files changed, 126 insertions(+), 43 deletions(-)
>
> diff --git a/arch/arm64/include/asm/hugetlb.h b/arch/arm64/include/asm/hugetlb.h
> index 2eb6c234d594..3248f35213ee 100644
> --- a/arch/arm64/include/asm/hugetlb.h
> +++ b/arch/arm64/include/asm/hugetlb.h
> @@ -59,6 +59,8 @@ extern void huge_pte_clear(struct mm_struct *mm, unsigned long addr,
> extern void set_huge_swap_pte_at(struct mm_struct *mm, unsigned long addr,
> pte_t *ptep, pte_t pte, unsigned long sz);
> #define set_huge_swap_pte_at set_huge_swap_pte_at
> +extern bool __init arch_hugetlb_valid_size(unsigned long long size);
> +#define arch_hugetlb_valid_size arch_hugetlb_valid_size
>
> #include <asm-generic/hugetlb.h>
>
> diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
> index bbeb6a5a6ba6..da30127086d0 100644
> --- a/arch/arm64/mm/hugetlbpage.c
> +++ b/arch/arm64/mm/hugetlbpage.c
> @@ -462,23 +462,32 @@ static int __init hugetlbpage_init(void)
> }
> arch_initcall(hugetlbpage_init);
>
> -static __init int setup_hugepagesz(char *opt)
> +bool __init arch_hugetlb_valid_size(unsigned long long size)
> {
> - unsigned long ps = memparse(opt, &opt);
> -
> - switch (ps) {
> + switch (size) {
> #ifdef CONFIG_ARM64_4K_PAGES
> case PUD_SIZE:
> #endif
> case CONT_PMD_SIZE:
> case PMD_SIZE:
> case CONT_PTE_SIZE:
> + return true;
> + }
> +
> + return false;
> +}
> +
> +static __init int setup_hugepagesz(char *opt)
> +{
> + unsigned long long ps = memparse(opt, &opt);
> +
> + if arch_hugetlb_valid_size(ps)) {
Please compile your changes if you're touching multiple architectures. You
can get cross-compiler binaries from:
https://mirrors.edge.kernel.org/pub/tools/crosstool/
https://toolchains.bootlin.com/
Will
^ permalink raw reply
* [PATCH 4/4] hugetlbfs: clean up command line processing
From: Mike Kravetz @ 2020-03-18 22:06 UTC (permalink / raw)
To: linux-mm, linux-kernel, linux-arm-kernel, linuxppc-dev,
linux-riscv, linux-s390, sparclinux, linux-doc
Cc: Albert Ou, Andrew Morton, Vasily Gorbik, Jonathan Corbet,
Catalin Marinas, Dave Hansen, Heiko Carstens,
Christian Borntraeger, Ingo Molnar, Palmer Dabbelt, Paul Walmsley,
Paul Mackerras, Thomas Gleixner, Longpeng, Will Deacon,
David S . Miller, Mike Kravetz
In-Reply-To: <20200318220634.32100-1-mike.kravetz@oracle.com>
With all hugetlb page processing done in a single file clean up code.
- Make code match desired semantics
- Update documentation with semantics
- Make all warnings and errors messages start with 'HugeTLB:'.
- Consistently name command line parsing routines.
- Add comments to code
- Describe some of the subtle interactions
- Describe semantics of command line arguments
Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
---
Documentation/admin-guide/mm/hugetlbpage.rst | 26 +++++++
mm/hugetlb.c | 78 +++++++++++++++-----
2 files changed, 87 insertions(+), 17 deletions(-)
diff --git a/Documentation/admin-guide/mm/hugetlbpage.rst b/Documentation/admin-guide/mm/hugetlbpage.rst
index 1cc0bc78d10e..afc8888f33c3 100644
--- a/Documentation/admin-guide/mm/hugetlbpage.rst
+++ b/Documentation/admin-guide/mm/hugetlbpage.rst
@@ -100,6 +100,32 @@ with a huge page size selection parameter "hugepagesz=<size>". <size> must
be specified in bytes with optional scale suffix [kKmMgG]. The default huge
page size may be selected with the "default_hugepagesz=<size>" boot parameter.
+Hugetlb boot command line parameter semantics
+hugepagesz - Specify a huge page size. Used in conjunction with hugepages
+ parameter to preallocate a number of huge pages of the specified
+ size. Hence, hugepagesz and hugepages are typically specified in
+ pairs such as:
+ hugepagesz=2M hugepages=512
+ hugepagesz can only be specified once on the command line for a
+ specific huge page size. Valid huge page sizes are architecture
+ dependent.
+hugepages - Specify the number of huge pages to preallocate. This typically
+ follows a valid hugepagesz parameter. However, if hugepages is the
+ first or only hugetlb command line parameter it specifies the number
+ of huge pages of default size to allocate. The number of huge pages
+ of default size specified in this manner can be overwritten by a
+ hugepagesz,hugepages parameter pair for the default size.
+ For example, on an architecture with 2M default huge page size:
+ hugepages=256 hugepagesz=2M hugepages=512
+ will result in 512 2M huge pages being allocated. If a hugepages
+ parameter is preceded by an invalid hugepagesz parameter, it will
+ be ignored.
+default_hugepagesz - Specify the default huge page size. This parameter can
+ only be specified on the command line. No other hugetlb command line
+ parameter is associated with default_hugepagesz. Therefore, it can
+ appear anywhere on the command line. Valid default huge page size is
+ architecture dependent.
+
When multiple huge page sizes are supported, ``/proc/sys/vm/nr_hugepages``
indicates the current number of pre-allocated huge pages of the default size.
Thus, one can use the following command to dynamically allocate/deallocate
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index cc85b4f156ca..2b9bf01db2b6 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -2954,7 +2954,7 @@ static void __init hugetlb_sysfs_init(void)
err = hugetlb_sysfs_add_hstate(h, hugepages_kobj,
hstate_kobjs, &hstate_attr_group);
if (err)
- pr_err("Hugetlb: Unable to add hstate %s", h->name);
+ pr_err("HugeTLB: Unable to add hstate %s", h->name);
}
}
@@ -3058,7 +3058,7 @@ static void hugetlb_register_node(struct node *node)
nhs->hstate_kobjs,
&per_node_hstate_attr_group);
if (err) {
- pr_err("Hugetlb: Unable to add hstate %s for node %d\n",
+ pr_err("HugeTLB: Unable to add hstate %s for node %d\n",
h->name, node->dev.id);
hugetlb_unregister_node(node);
break;
@@ -3109,19 +3109,35 @@ static int __init hugetlb_init(void)
if (!hugepages_supported())
return 0;
- if (!size_to_hstate(default_hstate_size)) {
- if (default_hstate_size != 0) {
- pr_err("HugeTLB: unsupported default_hugepagesz %lu. Reverting to %lu\n",
- default_hstate_size, HPAGE_SIZE);
- }
-
+ /*
+ * Make sure HPAGE_SIZE (HUGETLB_PAGE_ORDER) hstate exists. Some
+ * architectures depend on setup being done here.
+ *
+ * If a valid default huge page size was specified on the command line,
+ * add associated hstate if necessary. If not, set default_hstate_size
+ * to default size. default_hstate_idx is used at runtime to identify
+ * the default huge page size/hstate.
+ */
+ hugetlb_add_hstate(HUGETLB_PAGE_ORDER);
+ if (default_hstate_size)
+ hugetlb_add_hstate(ilog2(default_hstate_size) - PAGE_SHIFT);
+ else
default_hstate_size = HPAGE_SIZE;
- hugetlb_add_hstate(HUGETLB_PAGE_ORDER);
- }
default_hstate_idx = hstate_index(size_to_hstate(default_hstate_size));
+
+ /*
+ * default_hstate_max_huge_pages != 0 indicates a count (hugepages=)
+ * specified before a size (hugepagesz=). Use this count for the
+ * default huge page size, unless a specific value was specified for
+ * this size in a hugepagesz/hugepages pair.
+ */
if (default_hstate_max_huge_pages) {
if (!default_hstate.max_huge_pages)
- default_hstate.max_huge_pages = default_hstate_max_huge_pages;
+ default_hstate.max_huge_pages =
+ default_hstate_max_huge_pages;
+ else
+ pr_warn("HugeTLB: First hugepages=%lu kB ignored\n",
+ default_hstate_max_huge_pages);
}
hugetlb_init_hstates();
@@ -3174,20 +3190,27 @@ void __init hugetlb_add_hstate(unsigned int order)
parsed_hstate = h;
}
-static int __init hugetlb_nrpages_setup(char *s)
+/*
+ * hugepages command line processing
+ * hugepages must normally follows a valid hugepagsz specification. If not,
+ * ignore the hugepages value. hugepages can also be the first huge page
+ * command line option in which case it specifies the number of huge pages
+ * for the default size.
+ */
+static int __init hugepages_setup(char *s)
{
unsigned long *mhp;
static unsigned long *last_mhp;
if (!parsed_valid_hugepagesz) {
- pr_warn("hugepages = %s preceded by "
+ pr_warn("HugeTLB: hugepages = %s preceded by "
"an unsupported hugepagesz, ignoring\n", s);
parsed_valid_hugepagesz = true;
return 1;
}
/*
- * !hugetlb_max_hstate means we haven't parsed a hugepagesz= parameter yet,
- * so this hugepages= parameter goes to the "default hstate".
+ * !hugetlb_max_hstate means we haven't parsed a hugepagesz= parameter
+ * yet, so this hugepages= parameter goes to the "default hstate".
*/
else if (!hugetlb_max_hstate)
mhp = &default_hstate_max_huge_pages;
@@ -3195,7 +3218,8 @@ static int __init hugetlb_nrpages_setup(char *s)
mhp = &parsed_hstate->max_huge_pages;
if (mhp == last_mhp) {
- pr_warn("hugepages= specified twice without interleaving hugepagesz=, ignoring\n");
+ pr_warn("HugeTLB: hugepages= specified twice without interleaving hugepagesz=, ignoring hugepages=%s\n",
+ s);
return 1;
}
@@ -3214,8 +3238,15 @@ static int __init hugetlb_nrpages_setup(char *s)
return 1;
}
-__setup("hugepages=", hugetlb_nrpages_setup);
+__setup("hugepages=", hugepages_setup);
+/*
+ * hugepagesz command line processing
+ * A specific huge page size can only be specified once with hugepagesz.
+ * hugepagesz is followed by hugepages on the commnad line. The global
+ * variable 'parsed_valid_hugepagesz' is used to determine if prior
+ * hugepagesz argument was valid.
+ */
static int __init hugepagesz_setup(char *s)
{
unsigned long long size;
@@ -3230,16 +3261,23 @@ static int __init hugepagesz_setup(char *s)
}
if (size_to_hstate(size)) {
+ parsed_valid_hugepagesz = false;
pr_warn("HugeTLB: hugepagesz %s specified twice, ignoring\n",
saved_s);
return 0;
}
+ parsed_valid_hugepagesz = true;
hugetlb_add_hstate(ilog2(size) - PAGE_SHIFT);
return 1;
}
__setup("hugepagesz=", hugepagesz_setup);
+/*
+ * default_hugepagesz command line input
+ * Only one instance of default_hugepagesz allowed on command line. Do not
+ * add hstate here as that will confuse hugepagesz/hugepages processing.
+ */
static int __init default_hugepagesz_setup(char *s)
{
unsigned long long size;
@@ -3252,6 +3290,12 @@ static int __init default_hugepagesz_setup(char *s)
return 0;
}
+ if (default_hstate_size) {
+ pr_err("HugeTLB: default_hugepagesz previously specified, ignoring %s\n",
+ saved_s);
+ return 0;
+ }
+
default_hstate_size = size;
return 1;
}
--
2.24.1
^ permalink raw reply related
* [PATCH 0/4] Clean up hugetlb boot command line processing
From: Mike Kravetz @ 2020-03-18 22:06 UTC (permalink / raw)
To: linux-mm, linux-kernel, linux-arm-kernel, linuxppc-dev,
linux-riscv, linux-s390, sparclinux, linux-doc
Cc: Albert Ou, Andrew Morton, Vasily Gorbik, Jonathan Corbet,
Catalin Marinas, Dave Hansen, Heiko Carstens,
Christian Borntraeger, Ingo Molnar, Palmer Dabbelt, Paul Walmsley,
Paul Mackerras, Thomas Gleixner, Longpeng, Will Deacon,
David S . Miller, Mike Kravetz
Longpeng(Mike) reported a weird message from hugetlb command line processing
and proposed a solution [1]. While the proposed patch does address the
specific issue, there are other related issues in command line processing.
As hugetlbfs evolved, updates to command line processing have been made to
meet immediate needs and not necessarily in a coordinated manner. The result
is that some processing is done in arch specific code, some is done in arch
independent code and coordination is problematic. Semantics can vary between
architectures.
The following patch series does the following:
- Define arch specific arch_hugetlb_valid_size routine used to validate
passed huge page sizes.
- Move hugepagesz= command line parsing out of arch specific code and into
an arch independent routine.
- Clean up command line processing to follow desired semantics and
document those semantics.
[1] https://lore.kernel.org/linux-mm/20200305033014.1152-1-longpeng2@huawei.com
Mike Kravetz (4):
hugetlbfs: add arch_hugetlb_valid_size
hugetlbfs: move hugepagesz= parsing to arch independent code
hugetlbfs: remove hugetlb_add_hstate() warning for existing hstate
hugetlbfs: clean up command line processing
Documentation/admin-guide/mm/hugetlbpage.rst | 26 ++++
arch/arm64/include/asm/hugetlb.h | 2 +
arch/arm64/mm/hugetlbpage.c | 30 ++---
arch/powerpc/include/asm/hugetlb.h | 3 +
arch/powerpc/mm/hugetlbpage.c | 30 ++---
arch/riscv/include/asm/hugetlb.h | 3 +
arch/riscv/mm/hugetlbpage.c | 24 ++--
arch/s390/include/asm/hugetlb.h | 3 +
arch/s390/mm/hugetlbpage.c | 24 ++--
arch/sparc/include/asm/hugetlb.h | 3 +
arch/sparc/mm/init_64.c | 42 ++-----
arch/x86/include/asm/hugetlb.h | 3 +
arch/x86/mm/hugetlbpage.c | 23 ++--
include/linux/hugetlb.h | 8 +-
mm/hugetlb.c | 126 ++++++++++++++-----
15 files changed, 198 insertions(+), 152 deletions(-)
--
2.24.1
^ permalink raw reply
* [PATCH 1/4] hugetlbfs: add arch_hugetlb_valid_size
From: Mike Kravetz @ 2020-03-18 22:06 UTC (permalink / raw)
To: linux-mm, linux-kernel, linux-arm-kernel, linuxppc-dev,
linux-riscv, linux-s390, sparclinux, linux-doc
Cc: Albert Ou, Andrew Morton, Vasily Gorbik, Jonathan Corbet,
Catalin Marinas, Dave Hansen, Heiko Carstens,
Christian Borntraeger, Ingo Molnar, Palmer Dabbelt, Paul Walmsley,
Paul Mackerras, Thomas Gleixner, Longpeng, Will Deacon,
David S . Miller, Mike Kravetz
In-Reply-To: <20200318220634.32100-1-mike.kravetz@oracle.com>
The architecture independent routine hugetlb_default_setup sets up
the default huge pages size. It has no way to verify if the passed
value is valid, so it accepts it and attempts to validate at a later
time. This requires undocumented cooperation between the arch specific
and arch independent code.
For architectures that support more than one huge page size, provide
a routine arch_hugetlb_valid_size to validate a huge page size.
hugetlb_default_setup can use this to validate passed values.
arch_hugetlb_valid_size will also be used in a subsequent patch to
move processing of the "hugepagesz=" in arch specific code to a common
routine in arch independent code.
Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
---
arch/arm64/include/asm/hugetlb.h | 2 ++
arch/arm64/mm/hugetlbpage.c | 19 ++++++++++++++-----
arch/powerpc/include/asm/hugetlb.h | 3 +++
arch/powerpc/mm/hugetlbpage.c | 20 +++++++++++++-------
arch/riscv/include/asm/hugetlb.h | 3 +++
arch/riscv/mm/hugetlbpage.c | 28 ++++++++++++++++++----------
arch/s390/include/asm/hugetlb.h | 3 +++
arch/s390/mm/hugetlbpage.c | 18 +++++++++++++-----
arch/sparc/include/asm/hugetlb.h | 3 +++
arch/sparc/mm/init_64.c | 23 ++++++++++++++++-------
arch/x86/include/asm/hugetlb.h | 3 +++
arch/x86/mm/hugetlbpage.c | 21 +++++++++++++++------
include/linux/hugetlb.h | 7 +++++++
mm/hugetlb.c | 16 +++++++++++++---
14 files changed, 126 insertions(+), 43 deletions(-)
diff --git a/arch/arm64/include/asm/hugetlb.h b/arch/arm64/include/asm/hugetlb.h
index 2eb6c234d594..3248f35213ee 100644
--- a/arch/arm64/include/asm/hugetlb.h
+++ b/arch/arm64/include/asm/hugetlb.h
@@ -59,6 +59,8 @@ extern void huge_pte_clear(struct mm_struct *mm, unsigned long addr,
extern void set_huge_swap_pte_at(struct mm_struct *mm, unsigned long addr,
pte_t *ptep, pte_t pte, unsigned long sz);
#define set_huge_swap_pte_at set_huge_swap_pte_at
+extern bool __init arch_hugetlb_valid_size(unsigned long long size);
+#define arch_hugetlb_valid_size arch_hugetlb_valid_size
#include <asm-generic/hugetlb.h>
diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
index bbeb6a5a6ba6..da30127086d0 100644
--- a/arch/arm64/mm/hugetlbpage.c
+++ b/arch/arm64/mm/hugetlbpage.c
@@ -462,23 +462,32 @@ static int __init hugetlbpage_init(void)
}
arch_initcall(hugetlbpage_init);
-static __init int setup_hugepagesz(char *opt)
+bool __init arch_hugetlb_valid_size(unsigned long long size)
{
- unsigned long ps = memparse(opt, &opt);
-
- switch (ps) {
+ switch (size) {
#ifdef CONFIG_ARM64_4K_PAGES
case PUD_SIZE:
#endif
case CONT_PMD_SIZE:
case PMD_SIZE:
case CONT_PTE_SIZE:
+ return true;
+ }
+
+ return false;
+}
+
+static __init int setup_hugepagesz(char *opt)
+{
+ unsigned long long ps = memparse(opt, &opt);
+
+ if arch_hugetlb_valid_size(ps)) {
add_huge_page_size(ps);
return 1;
}
hugetlb_bad_size();
- pr_err("hugepagesz: Unsupported page size %lu K\n", ps >> 10);
+ pr_err("hugepagesz: Unsupported page size %llu K\n", ps >> 10);
return 0;
}
__setup("hugepagesz=", setup_hugepagesz);
diff --git a/arch/powerpc/include/asm/hugetlb.h b/arch/powerpc/include/asm/hugetlb.h
index bd6504c28c2f..3b5939016955 100644
--- a/arch/powerpc/include/asm/hugetlb.h
+++ b/arch/powerpc/include/asm/hugetlb.h
@@ -64,6 +64,9 @@ static inline void arch_clear_hugepage_flags(struct page *page)
{
}
+#define arch_hugetlb_valid_size arch_hugetlb_valid_size
+extern bool __init arch_hugetlb_valid_size(unsigned long long size);
+
#include <asm-generic/hugetlb.h>
#else /* ! CONFIG_HUGETLB_PAGE */
diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
index 33b3461d91e8..b78f660252f3 100644
--- a/arch/powerpc/mm/hugetlbpage.c
+++ b/arch/powerpc/mm/hugetlbpage.c
@@ -558,7 +558,7 @@ unsigned long vma_mmu_pagesize(struct vm_area_struct *vma)
return vma_kernel_pagesize(vma);
}
-static int __init add_huge_page_size(unsigned long long size)
+bool __init arch_hugetlb_valid_size(unsigned long long size)
{
int shift = __ffs(size);
int mmu_psize;
@@ -566,20 +566,26 @@ static int __init add_huge_page_size(unsigned long long size)
/* Check that it is a page size supported by the hardware and
* that it fits within pagetable and slice limits. */
if (size <= PAGE_SIZE || !is_power_of_2(size))
- return -EINVAL;
+ return false;
mmu_psize = check_and_get_huge_psize(shift);
if (mmu_psize < 0)
- return -EINVAL;
+ return false;
BUG_ON(mmu_psize_defs[mmu_psize].shift != shift);
- /* Return if huge page size has already been setup */
- if (size_to_hstate(size))
- return 0;
+ return true;
+}
- hugetlb_add_hstate(shift - PAGE_SHIFT);
+static int __init add_huge_page_size(unsigned long long size)
+{
+ int shift = __ffs(size);
+
+ if (!arch_hugetlb_valid_size(size))
+ return -EINVAL;
+ if (!size_to_hstate(size))
+ hugetlb_add_hstate(shift - PAGE_SHIFT);
return 0;
}
diff --git a/arch/riscv/include/asm/hugetlb.h b/arch/riscv/include/asm/hugetlb.h
index 728a5db66597..ebd6f5a35d26 100644
--- a/arch/riscv/include/asm/hugetlb.h
+++ b/arch/riscv/include/asm/hugetlb.h
@@ -5,6 +5,9 @@
#include <asm-generic/hugetlb.h>
#include <asm/page.h>
+extern bool __init arch_hugetlb_valid_size(unsigned long long size);
+#define arch_hugetlb_valid_size arch_hugetlb_valid_size
+
static inline int is_hugepage_only_range(struct mm_struct *mm,
unsigned long addr,
unsigned long len) {
diff --git a/arch/riscv/mm/hugetlbpage.c b/arch/riscv/mm/hugetlbpage.c
index a6189ed36c5f..f1990882f16c 100644
--- a/arch/riscv/mm/hugetlbpage.c
+++ b/arch/riscv/mm/hugetlbpage.c
@@ -12,21 +12,29 @@ int pmd_huge(pmd_t pmd)
return pmd_leaf(pmd);
}
+bool __init arch_hugetlb_valid_size(unsigned long long size)
+{
+ if (size == HPAGE_SIZE)
+ return true;
+ else if (IS_ENABLED(CONFIG_64BIT) && ps == PUD_SIZE)
+ retrurn true;
+ else
+ return false;
+}
+
static __init int setup_hugepagesz(char *opt)
{
- unsigned long ps = memparse(opt, &opt);
+ unsigned long long ps = memparse(opt, &opt);
- if (ps == HPAGE_SIZE) {
- hugetlb_add_hstate(HPAGE_SHIFT - PAGE_SHIFT);
- } else if (IS_ENABLED(CONFIG_64BIT) && ps == PUD_SIZE) {
- hugetlb_add_hstate(PUD_SHIFT - PAGE_SHIFT);
- } else {
- hugetlb_bad_size();
- pr_err("hugepagesz: Unsupported page size %lu M\n", ps >> 20);
- return 0;
+ if (arch_hugetlb_valid_size(ps)) {
+ hugetlb_add_hstate(ilog2(ps) - PAGE_SHIFT);
+ return 1;
}
- return 1;
+ hugetlb_bad_size();
+ pr_err("hugepagesz: Unsupported page size %lu M\n", ps >> 20);
+ return 0;
+
}
__setup("hugepagesz=", setup_hugepagesz);
diff --git a/arch/s390/include/asm/hugetlb.h b/arch/s390/include/asm/hugetlb.h
index de8f0bf5f238..a3dd457d2167 100644
--- a/arch/s390/include/asm/hugetlb.h
+++ b/arch/s390/include/asm/hugetlb.h
@@ -15,6 +15,9 @@
#define hugetlb_free_pgd_range free_pgd_range
#define hugepages_supported() (MACHINE_HAS_EDAT1)
+extern bool __init arch_hugetlb_valid_size(unsigned long long size);
+#define arch_hugetlb_valid_size arch_hugetlb_valid_size
+
void set_huge_pte_at(struct mm_struct *mm, unsigned long addr,
pte_t *ptep, pte_t pte);
pte_t huge_ptep_get(pte_t *ptep);
diff --git a/arch/s390/mm/hugetlbpage.c b/arch/s390/mm/hugetlbpage.c
index 5674710a4841..d92e8c5c3e71 100644
--- a/arch/s390/mm/hugetlbpage.c
+++ b/arch/s390/mm/hugetlbpage.c
@@ -251,16 +251,24 @@ follow_huge_pud(struct mm_struct *mm, unsigned long address,
return pud_page(*pud) + ((address & ~PUD_MASK) >> PAGE_SHIFT);
}
+bool __init arch_hugetlb_valid_size(unsigned long long size)
+{
+ if (MACHINE_HAS_EDAT1 && size == PMD_SIZE)
+ return true;
+ else if (MACHINE_HAS_EDAT2 && size == PUD_SIZE)
+ return true;
+ else
+ return false;
+}
+
static __init int setup_hugepagesz(char *opt)
{
- unsigned long size;
+ unsigned long long size;
char *string = opt;
size = memparse(opt, &opt);
- if (MACHINE_HAS_EDAT1 && size == PMD_SIZE) {
- hugetlb_add_hstate(PMD_SHIFT - PAGE_SHIFT);
- } else if (MACHINE_HAS_EDAT2 && size == PUD_SIZE) {
- hugetlb_add_hstate(PUD_SHIFT - PAGE_SHIFT);
+ if (arch_hugetlb_valid_size(size)) {
+ hugetlb_add_hstate(ilog2(size) - PAGE_SHIFT);
} else {
hugetlb_bad_size();
pr_err("hugepagesz= specifies an unsupported page size %s\n",
diff --git a/arch/sparc/include/asm/hugetlb.h b/arch/sparc/include/asm/hugetlb.h
index 3963f80d1cb3..0d4f4adaffaf 100644
--- a/arch/sparc/include/asm/hugetlb.h
+++ b/arch/sparc/include/asm/hugetlb.h
@@ -10,6 +10,9 @@ struct pud_huge_patch_entry {
unsigned int insn;
};
extern struct pud_huge_patch_entry __pud_huge_patch, __pud_huge_patch_end;
+
+extern bool __init arch_hugetlb_valid_size(unsigned long long size);
+#define arch_hugetlb_valid_size arch_hugetlb_valid_size
#endif
#define __HAVE_ARCH_HUGE_SET_HUGE_PTE_AT
diff --git a/arch/sparc/mm/init_64.c b/arch/sparc/mm/init_64.c
index 1cf0d666dea3..4cc248817b19 100644
--- a/arch/sparc/mm/init_64.c
+++ b/arch/sparc/mm/init_64.c
@@ -360,17 +360,13 @@ static void __init pud_huge_patch(void)
__asm__ __volatile__("flush %0" : : "r" (addr));
}
-static int __init setup_hugepagesz(char *string)
+bool __init arch_hugetlb_valid_size(unsigned long long size)
{
- unsigned long long hugepage_size;
- unsigned int hugepage_shift;
+ unsigned int hugepage_shift = ilog2(size);
unsigned short hv_pgsz_idx;
unsigned int hv_pgsz_mask;
int rc = 0;
- hugepage_size = memparse(string, &string);
- hugepage_shift = ilog2(hugepage_size);
-
switch (hugepage_shift) {
case HPAGE_16GB_SHIFT:
hv_pgsz_mask = HV_PGSZ_MASK_16GB;
@@ -397,7 +393,20 @@ static int __init setup_hugepagesz(char *string)
hv_pgsz_mask = 0;
}
- if ((hv_pgsz_mask & cpu_pgsz_mask) == 0U) {
+ if ((hv_pgsz_mask & cpu_pgsz_mask) == 0U)
+ return false;
+
+ return true;
+}
+
+static int __init setup_hugepagesz(char *string)
+{
+ unsigned long long hugepage_size;
+ int rc = 0;
+
+ hugepage_size = memparse(string, &string);
+
+ if (!arch_hugetlb_valid_size(hugepage_size)) {
hugetlb_bad_size();
pr_err("hugepagesz=%llu not supported by MMU.\n",
hugepage_size);
diff --git a/arch/x86/include/asm/hugetlb.h b/arch/x86/include/asm/hugetlb.h
index f65cfb48cfdd..8ed96e4010ec 100644
--- a/arch/x86/include/asm/hugetlb.h
+++ b/arch/x86/include/asm/hugetlb.h
@@ -7,6 +7,9 @@
#define hugepages_supported() boot_cpu_has(X86_FEATURE_PSE)
+extern bool __init arch_hugetlb_valid_size(unsigned long long size);
+#define arch_hugetlb_valid_size arch_hugetlb_valid_size
+
static inline int is_hugepage_only_range(struct mm_struct *mm,
unsigned long addr,
unsigned long len) {
diff --git a/arch/x86/mm/hugetlbpage.c b/arch/x86/mm/hugetlbpage.c
index 5bfd5aef5378..51e6208fdeec 100644
--- a/arch/x86/mm/hugetlbpage.c
+++ b/arch/x86/mm/hugetlbpage.c
@@ -181,16 +181,25 @@ hugetlb_get_unmapped_area(struct file *file, unsigned long addr,
#endif /* CONFIG_HUGETLB_PAGE */
#ifdef CONFIG_X86_64
+bool __init arch_hugetlb_valid_size(unsigned long long size)
+{
+ if (size == PMD_SIZE)
+ return true;
+ else if (size == PUD_SIZE && boot_cpu_has(X86_FEATURE_GBPAGES))
+ return true;
+ else
+ return false;
+}
+
static __init int setup_hugepagesz(char *opt)
{
- unsigned long ps = memparse(opt, &opt);
- if (ps == PMD_SIZE) {
- hugetlb_add_hstate(PMD_SHIFT - PAGE_SHIFT);
- } else if (ps == PUD_SIZE && boot_cpu_has(X86_FEATURE_GBPAGES)) {
- hugetlb_add_hstate(PUD_SHIFT - PAGE_SHIFT);
+ unsigned long long ps = memparse(opt, &opt);
+
+ if (arch_hugetlb_valid_size(ps)) {
+ hugetlb_add_hstate(ilog2(ps) - PAGE_SHIFT);
} else {
hugetlb_bad_size();
- printk(KERN_ERR "hugepagesz: Unsupported page size %lu M\n",
+ printk(KERN_ERR "hugepagesz: Unsupported page size %llu M\n",
ps >> 20);
return 0;
}
diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index b831e9fa1a26..33343eb980d0 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -678,6 +678,13 @@ static inline spinlock_t *huge_pte_lockptr(struct hstate *h,
return &mm->page_table_lock;
}
+#ifndef arch_hugetlb_valid_size
+static inline bool arch_hugetlb_valid_size(unsigned long long size)
+{
+ return (size == HPAGE_SIZE);
+}
+#endif
+
#ifndef hugepages_supported
/*
* Some platform decide whether they support huge pages at boot
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index d8ebd876871d..2f99359b93af 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -3224,12 +3224,22 @@ static int __init hugetlb_nrpages_setup(char *s)
}
__setup("hugepages=", hugetlb_nrpages_setup);
-static int __init hugetlb_default_setup(char *s)
+static int __init default_hugepagesz_setup(char *s)
{
- default_hstate_size = memparse(s, &s);
+ unsigned long long size;
+ char *saved_s = s;
+
+ size = memparse(s, &s);
+
+ if (!arch_hugetlb_valid_size(size)) {
+ pr_err("HugeTLB: unsupported default_hugepagesz %s\n", saved_s);
+ return 0;
+ }
+
+ default_hstate_size = size;
return 1;
}
-__setup("default_hugepagesz=", hugetlb_default_setup);
+__setup("default_hugepagesz=", default_hugepagesz_setup);
static unsigned int cpuset_mems_nr(unsigned int *array)
{
--
2.24.1
^ permalink raw reply related
* [PATCH 2/4] hugetlbfs: move hugepagesz= parsing to arch independent code
From: Mike Kravetz @ 2020-03-18 22:06 UTC (permalink / raw)
To: linux-mm, linux-kernel, linux-arm-kernel, linuxppc-dev,
linux-riscv, linux-s390, sparclinux, linux-doc
Cc: Albert Ou, Andrew Morton, Vasily Gorbik, Jonathan Corbet,
Catalin Marinas, Dave Hansen, Heiko Carstens,
Christian Borntraeger, Ingo Molnar, Palmer Dabbelt, Paul Walmsley,
Paul Mackerras, Thomas Gleixner, Longpeng, Will Deacon,
David S . Miller, Mike Kravetz
In-Reply-To: <20200318220634.32100-1-mike.kravetz@oracle.com>
Now that architectures provide arch_hugetlb_valid_size(), parsing
of "hugepagesz=" can be done in architecture independent code.
Create a single routine to handle hugepagesz= parsing and remove
all arch specific routines. We can also remove the interface
hugetlb_bad_size() as this is no longer used outside arch independent
code.
This also provides consistent behavior of hugetlbfs command line
options. The hugepagesz= option should only be specified once for
a specific size, but some architectures allow multiple instances.
This appears to be more of an oversight when code was added by some
architectures to set up ALL huge pages sizes.
Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
---
arch/arm64/mm/hugetlbpage.c | 15 ---------------
arch/powerpc/mm/hugetlbpage.c | 15 ---------------
arch/riscv/mm/hugetlbpage.c | 16 ----------------
arch/s390/mm/hugetlbpage.c | 18 ------------------
arch/sparc/mm/init_64.c | 22 ----------------------
arch/x86/mm/hugetlbpage.c | 16 ----------------
include/linux/hugetlb.h | 1 -
mm/hugetlb.c | 24 ++++++++++++++++++------
8 files changed, 18 insertions(+), 109 deletions(-)
diff --git a/arch/arm64/mm/hugetlbpage.c b/arch/arm64/mm/hugetlbpage.c
index da30127086d0..4aa9534a45d7 100644
--- a/arch/arm64/mm/hugetlbpage.c
+++ b/arch/arm64/mm/hugetlbpage.c
@@ -476,18 +476,3 @@ bool __init arch_hugetlb_valid_size(unsigned long long size)
return false;
}
-
-static __init int setup_hugepagesz(char *opt)
-{
- unsigned long long ps = memparse(opt, &opt);
-
- if arch_hugetlb_valid_size(ps)) {
- add_huge_page_size(ps);
- return 1;
- }
-
- hugetlb_bad_size();
- pr_err("hugepagesz: Unsupported page size %llu K\n", ps >> 10);
- return 0;
-}
-__setup("hugepagesz=", setup_hugepagesz);
diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
index b78f660252f3..166960ba1236 100644
--- a/arch/powerpc/mm/hugetlbpage.c
+++ b/arch/powerpc/mm/hugetlbpage.c
@@ -589,21 +589,6 @@ static int __init add_huge_page_size(unsigned long long size)
return 0;
}
-static int __init hugepage_setup_sz(char *str)
-{
- unsigned long long size;
-
- size = memparse(str, &str);
-
- if (add_huge_page_size(size) != 0) {
- hugetlb_bad_size();
- pr_err("Invalid huge page size specified(%llu)\n", size);
- }
-
- return 1;
-}
-__setup("hugepagesz=", hugepage_setup_sz);
-
static int __init hugetlbpage_init(void)
{
bool configured = false;
diff --git a/arch/riscv/mm/hugetlbpage.c b/arch/riscv/mm/hugetlbpage.c
index f1990882f16c..bdf89d7eb714 100644
--- a/arch/riscv/mm/hugetlbpage.c
+++ b/arch/riscv/mm/hugetlbpage.c
@@ -22,22 +22,6 @@ bool __init arch_hugetlb_valid_size(unsigned long long size)
return false;
}
-static __init int setup_hugepagesz(char *opt)
-{
- unsigned long long ps = memparse(opt, &opt);
-
- if (arch_hugetlb_valid_size(ps)) {
- hugetlb_add_hstate(ilog2(ps) - PAGE_SHIFT);
- return 1;
- }
-
- hugetlb_bad_size();
- pr_err("hugepagesz: Unsupported page size %lu M\n", ps >> 20);
- return 0;
-
-}
-__setup("hugepagesz=", setup_hugepagesz);
-
#ifdef CONFIG_CONTIG_ALLOC
static __init int gigantic_pages_init(void)
{
diff --git a/arch/s390/mm/hugetlbpage.c b/arch/s390/mm/hugetlbpage.c
index d92e8c5c3e71..b809762f206e 100644
--- a/arch/s390/mm/hugetlbpage.c
+++ b/arch/s390/mm/hugetlbpage.c
@@ -261,24 +261,6 @@ bool __init arch_hugetlb_valid_size(unsigned long long size)
return false;
}
-static __init int setup_hugepagesz(char *opt)
-{
- unsigned long long size;
- char *string = opt;
-
- size = memparse(opt, &opt);
- if (arch_hugetlb_valid_size(size)) {
- hugetlb_add_hstate(ilog2(size) - PAGE_SHIFT);
- } else {
- hugetlb_bad_size();
- pr_err("hugepagesz= specifies an unsupported page size %s\n",
- string);
- return 0;
- }
- return 1;
-}
-__setup("hugepagesz=", setup_hugepagesz);
-
static unsigned long hugetlb_get_unmapped_area_bottomup(struct file *file,
unsigned long addr, unsigned long len,
unsigned long pgoff, unsigned long flags)
diff --git a/arch/sparc/mm/init_64.c b/arch/sparc/mm/init_64.c
index 4cc248817b19..5c29203fd460 100644
--- a/arch/sparc/mm/init_64.c
+++ b/arch/sparc/mm/init_64.c
@@ -398,28 +398,6 @@ bool __init arch_hugetlb_valid_size(unsigned long long size)
return true;
}
-
-static int __init setup_hugepagesz(char *string)
-{
- unsigned long long hugepage_size;
- int rc = 0;
-
- hugepage_size = memparse(string, &string);
-
- if (!arch_hugetlb_valid_size(hugepage_size)) {
- hugetlb_bad_size();
- pr_err("hugepagesz=%llu not supported by MMU.\n",
- hugepage_size);
- goto out;
- }
-
- add_huge_page_size(hugepage_size);
- rc = 1;
-
-out:
- return rc;
-}
-__setup("hugepagesz=", setup_hugepagesz);
#endif /* CONFIG_HUGETLB_PAGE */
void update_mmu_cache(struct vm_area_struct *vma, unsigned long address, pte_t *ptep)
diff --git a/arch/x86/mm/hugetlbpage.c b/arch/x86/mm/hugetlbpage.c
index 51e6208fdeec..dd3ed09f6c23 100644
--- a/arch/x86/mm/hugetlbpage.c
+++ b/arch/x86/mm/hugetlbpage.c
@@ -191,22 +191,6 @@ bool __init arch_hugetlb_valid_size(unsigned long long size)
return false;
}
-static __init int setup_hugepagesz(char *opt)
-{
- unsigned long long ps = memparse(opt, &opt);
-
- if (arch_hugetlb_valid_size(ps)) {
- hugetlb_add_hstate(ilog2(ps) - PAGE_SHIFT);
- } else {
- hugetlb_bad_size();
- printk(KERN_ERR "hugepagesz: Unsupported page size %llu M\n",
- ps >> 20);
- return 0;
- }
- return 1;
-}
-__setup("hugepagesz=", setup_hugepagesz);
-
#ifdef CONFIG_CONTIG_ALLOC
static __init int gigantic_pages_init(void)
{
diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index 33343eb980d0..47244853ceb4 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -504,7 +504,6 @@ int huge_add_to_page_cache(struct page *page, struct address_space *mapping,
int __init __alloc_bootmem_huge_page(struct hstate *h);
int __init alloc_bootmem_huge_page(struct hstate *h);
-void __init hugetlb_bad_size(void);
void __init hugetlb_add_hstate(unsigned order);
struct hstate *size_to_hstate(unsigned long size);
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 2f99359b93af..cd4ec07080fb 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -3149,12 +3149,6 @@ static int __init hugetlb_init(void)
}
subsys_initcall(hugetlb_init);
-/* Should be called on processing a hugepagesz=... option */
-void __init hugetlb_bad_size(void)
-{
- parsed_valid_hugepagesz = false;
-}
-
void __init hugetlb_add_hstate(unsigned int order)
{
struct hstate *h;
@@ -3224,6 +3218,24 @@ static int __init hugetlb_nrpages_setup(char *s)
}
__setup("hugepages=", hugetlb_nrpages_setup);
+static int __init hugepagesz_setup(char *s)
+{
+ unsigned long long size;
+ char *saved_s = s;
+
+ size = memparse(s, &s);
+
+ if (!arch_hugetlb_valid_size(size)) {
+ parsed_valid_hugepagesz = false;
+ pr_err("HugeTLB: unsupported hugepagesz %s\n", saved_s);
+ return 0;
+ }
+
+ hugetlb_add_hstate(ilog2(size) - PAGE_SHIFT);
+ return 1;
+}
+__setup("hugepagesz=", hugepagesz_setup);
+
static int __init default_hugepagesz_setup(char *s)
{
unsigned long long size;
--
2.24.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox