* Re: [PATCH v2 0/6] NFS: Add support for the v4.2 READ_PLUS operation
From: Chuck Lever @ 2020-02-20 18:29 UTC (permalink / raw)
To: Anna Schumaker; +Cc: Trond.Myklebust, Linux NFS Mailing List
In-Reply-To: <c20cd5a113d19b8287bcc3db9c1b594ecb0e61e4.camel@gmail.com>
> On Feb 20, 2020, at 1:25 PM, Anna Schumaker <schumaker.anna@gmail.com> wrote:
>
> On Thu, 2020-02-20 at 12:40 -0500, Chuck Lever wrote:
>> Hi Anna-
>>
>>> On Feb 14, 2020, at 4:12 PM, schumaker.anna@gmail.com wrote:
>>>
>>> From: Anna Schumaker <Anna.Schumaker@Netapp.com>
>>>
>>> These patches add client support for the READ_PLUS operation, which
>>> breaks read requests into several "data" and "hole" segments when
>>> replying to the client. I also add a "noreadplus" mount option to allow
>>> users to disable the new operation if it becomes a problem, similar to
>>> the "nordirplus" mount option that we already have.
>>
>> Hrm, I went looking for the patch that adds "noreadplus", but I
>> don't see it in this series?
>
> You suggested dropping that patch in the v1 posting and waiting to see if
> anybody asks for it.
Yes, recalling that now.
I requested that because I don't like to add administrative interfaces
if there isn't a clear need. I think we might have one now!
>> Wondering if, to start off, the default should be "noreadplus"
>> until our feet are under us. Just a thought.
>
> I could re-add the patch with this as the default if that's the way everybody
> wants to go.
Yep, I'm interested in other opinions too.
> Anna
>
>>
>>
>>> Here are the results of some performance tests I ran on Netapp lab
>>> machines. I tested by reading various 2G files from a few different
>>> undelying filesystems and across several NFS versions. I used the
>>> `vmtouch` utility to make sure files were only cached when we wanted
>>> them to be. In addition to 100% data and 100% hole cases, I also tested
>>> with files that alternate between data and hole segments. These files
>>> have either 4K, 8K, 16K, or 32K segment sizes and start with either data
>>> or hole segments. So the file mixed-4d has a 4K segment size beginning
>>> with a data segment, but mixed-32h hase 32K segments beginning with a
>>> hole. The units are in seconds, with the first number for each NFS
>>> version being the uncached read time and the second number is for when
>>> the file is cached on the server.
>>>
>>> ext4 | v3 | v4.0 | v4.1 | v4.2
>>> |
>>> ----------|-----------------|-----------------|-----------------|-----------
>>> ------|
>>> data | 22.909 : 18.253 | 22.934 : 18.252 | 22.902 : 18.253 | 23.485 :
>>> 18.253 |
>>> hole | 18.256 : 18.253 | 18.255 : 18.252 | 18.256 : 18.253 | 0.708
>>> : 0.709 |
>>> mixed-4d | 28.261 : 18.253 | 29.616 : 18.252 | 28.341 : 18.252 | 24.508
>>> : 9.150 |
>>> mixed-8d | 27.956 : 18.253 | 28.404 : 18.252 | 28.320 : 18.252 | 23.967
>>> : 9.140 |
>>> mixed-16d | 28.172 : 18.253 | 27.946 : 18.252 | 27.627 : 18.252 | 23.043
>>> : 9.134 |
>>> mixed-32d | 25.350 : 18.253 | 24.406 : 18.252 | 24.384 : 18.253 | 20.698
>>> : 9.132 |
>>> mixed-4h | 28.913 : 18.253 | 28.564 : 18.252 | 27.996 : 18.252 | 21.837
>>> : 9.150 |
>>> mixed-8h | 28.625 : 18.253 | 27.833 : 18.252 | 27.798 : 18.253 | 21.710
>>> : 9.140 |
>>> mixed-16h | 27.975 : 18.253 | 27.662 : 18.252 | 27.795 : 18.253 | 20.585
>>> : 9.134 |
>>> mixed-32h | 25.958 : 18.253 | 25.491 : 18.252 | 24.856 : 18.252 | 21.018
>>> : 9.132 |
>>>
>>> xfs | v3 | v4.0 | v4.1 | v4.2
>>> |
>>> ----------|-----------------|-----------------|-----------------|-----------
>>> ------|
>>> data | 22.041 : 18.253 | 22.618 : 18.252 | 23.067 : 18.253 | 23.496 :
>>> 18.253 |
>>> hole | 18.256 : 18.253 | 18.255 : 18.252 | 18.256 : 18.253 | 0.723
>>> : 0.708 |
>>> mixed-4d | 29.417 : 18.253 | 28.503 : 18.252 | 28.671 : 18.253 | 24.957
>>> : 9.150 |
>>> mixed-8d | 29.080 : 18.253 | 29.401 : 18.252 | 29.251 : 18.252 | 24.625
>>> : 9.140 |
>>> mixed-16d | 27.638 : 18.253 | 28.606 : 18.252 | 27.871 : 18.253 | 25.511
>>> : 9.135 |
>>> mixed-32d | 24.967 : 18.253 | 25.239 : 18.252 | 25.434 : 18.252 | 21.728
>>> : 9.132 |
>>> mixed-4h | 34.816 : 18.253 | 36.243 : 18.252 | 35.837 : 18.252 | 32.332
>>> : 9.150 |
>>> mixed-8h | 43.469 : 18.253 | 44.009 : 18.252 | 43.810 : 18.253 | 37.962
>>> : 9.140 |
>>> mixed-16h | 29.280 : 18.253 | 28.563 : 18.252 | 28.241 : 18.252 | 22.116
>>> : 9.134 |
>>> mixed-32h | 29.428 : 18.253 | 29.378 : 18.252 | 28.808 : 18.253 | 27.378
>>> : 9.134 |
>>>
>>> btrfs | v3 | v4.0 | v4.1 | v4.2
>>> |
>>> ----------|-----------------|-----------------|-----------------|-----------
>>> ------|
>>> data | 25.547 : 18.253 | 25.053 : 18.252 | 24.209 : 18.253 | 32.121 :
>>> 18.253 |
>>> hole | 18.256 : 18.253 | 18.255 : 18.252 | 18.256 : 18.252 | 0.702
>>> : 0.724 |
>>> mixed-4d | 19.016 : 18.253 | 18.822 : 18.252 | 18.955 : 18.253 | 18.697
>>> : 9.150 |
>>> mixed-8d | 19.186 : 18.253 | 19.444 : 18.252 | 18.841 : 18.253 | 18.452
>>> : 9.140 |
>>> mixed-16d | 18.480 : 18.253 | 19.010 : 18.252 | 19.167 : 18.252 | 16.000
>>> : 9.134 |
>>> mixed-32d | 18.635 : 18.253 | 18.565 : 18.252 | 18.550 : 18.252 | 15.930
>>> : 9.132 |
>>> mixed-4h | 19.079 : 18.253 | 18.990 : 18.252 | 19.157 : 18.253 | 27.834
>>> : 9.150 |
>>> mixed-8h | 18.613 : 18.253 | 19.234 : 18.252 | 18.616 : 18.253 | 20.177
>>> : 9.140 |
>>> mixed-16h | 18.590 : 18.253 | 19.221 : 18.252 | 19.654 : 18.253 | 17.273
>>> : 9.135 |
>>> mixed-32h | 18.768 : 18.253 | 19.122 : 18.252 | 18.535 : 18.252 | 15.791
>>> : 9.132 |
>>>
>>> ext3 | v3 | v4.0 | v4.1 | v4.2
>>> |
>>> ----------|-----------------|-----------------|-----------------|-----------
>>> ------|
>>> data | 34.292 : 18.253 | 33.810 : 18.252 | 33.450 : 18.253 | 33.390 :
>>> 18.254 |
>>> hole | 18.256 : 18.253 | 18.255 : 18.252 | 18.256 : 18.253 | 0.718
>>> : 0.728 |
>>> mixed-4d | 46.818 : 18.253 | 47.140 : 18.252 | 48.385 : 18.253 | 42.887
>>> : 9.150 |
>>> mixed-8d | 58.554 : 18.253 | 59.277 : 18.252 | 59.673 : 18.253 | 56.760
>>> : 9.140 |
>>> mixed-16d | 44.631 : 18.253 | 44.291 : 18.252 | 44.729 : 18.253 | 40.237
>>> : 9.135 |
>>> mixed-32d | 39.110 : 18.253 | 38.735 : 18.252 | 38.902 : 18.252 | 35.270
>>> : 9.132 |
>>> mixed-4h | 56.396 : 18.253 | 56.387 : 18.252 | 56.573 : 18.253 | 67.661
>>> : 9.150 |
>>> mixed-8h | 58.483 : 18.253 | 58.484 : 18.252 | 59.099 : 18.253 | 77.958
>>> : 9.140 |
>>> mixed-16h | 42.511 : 18.253 | 42.338 : 18.252 | 42.356 : 18.252 | 51.805
>>> : 9.135 |
>>> mixed-32h | 38.419 : 18.253 | 38.504 : 18.252 | 38.643 : 18.252 | 40.411
>>> : 9.132 |
>>>
>>>
>>> Changes since v1:
>>> - Rebase to 5.6-rc1
>>> - Drop the mount option patch for now
>>> - Fix fallback to READ when the server doesn't support READ_PLUS
>>>
>>> Any questions?
>>> Anna
>>>
>>>
>>> Anna Schumaker (6):
>>> SUNRPC: Split out a function for setting current page
>>> SUNRPC: Add the ability to expand holes in data pages
>>> SUNRPC: Add the ability to shift data to a specific offset
>>> NFS: Add READ_PLUS data segment support
>>> NFS: Add READ_PLUS hole segment decoding
>>> NFS: Decode multiple READ_PLUS segments
>>>
>>> fs/nfs/nfs42xdr.c | 169 +++++++++++++++++++++++++
>>> fs/nfs/nfs4proc.c | 43 ++++++-
>>> fs/nfs/nfs4xdr.c | 1 +
>>> include/linux/nfs4.h | 2 +-
>>> include/linux/nfs_fs_sb.h | 1 +
>>> include/linux/nfs_xdr.h | 2 +-
>>> include/linux/sunrpc/xdr.h | 2 +
>>> net/sunrpc/xdr.c | 244 ++++++++++++++++++++++++++++++++++++-
>>> 8 files changed, 457 insertions(+), 7 deletions(-)
>>>
>>> --
>>> 2.25.0
>>>
>>
>> --
>> Chuck Lever
>> chucklever@gmail.com
--
Chuck Lever
chucklever@gmail.com
^ permalink raw reply
* Re: [PATCH 3/3] efi/arm: rewrite FDT param discovery routines
From: Leif Lindholm @ 2020-02-20 18:30 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: linux-efi, linux-arm-kernel, atish.patra
In-Reply-To: <20200219152440.11561-4-ardb@kernel.org>
On Wed, Feb 19, 2020 at 16:24:40 +0100, Ard Biesheuvel wrote:
> The efi_get_fdt_params() routine uses the early OF device tree
> traversal helpers, that iterate over each node in the DT and invoke
> a caller provided callback that can inspect the node's contents and
> look for the required data. This requires a special param struct to
> be passed around, with pointers into param enumeration structs that
> contain (and duplicate) property names and offsets into yet another
> struct that carries the collected data.
>
> Since we know the data we look for is either under /hypervisor/uefi
> or under /chosen, it is much simpler to use the libfdt routines
> directly, and try to grab a reference to either node directly, and
> read each property in sequence.
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
> drivers/firmware/efi/fdtparams.c | 203 ++++++++------------
> 1 file changed, 85 insertions(+), 118 deletions(-)
>
> diff --git a/drivers/firmware/efi/fdtparams.c b/drivers/firmware/efi/fdtparams.c
> index 7a384b307c56..23af4062e913 100644
> --- a/drivers/firmware/efi/fdtparams.c
> +++ b/drivers/firmware/efi/fdtparams.c
> @@ -5,154 +5,121 @@
> #include <linux/module.h>
> #include <linux/init.h>
> #include <linux/efi.h>
> -#include <linux/of.h>
> +#include <linux/libfdt.h>
> #include <linux/of_fdt.h>
>
> -#include <asm/early_ioremap.h>
> +#include <asm/unaligned.h>
>
> -#define UEFI_PARAM(name, prop, field) \
> - { \
> - { name }, \
> - { prop }, \
> - offsetof(struct efi_fdt_params, field), \
> - sizeof_field(struct efi_fdt_params, field) \
> - }
> -
> -struct efi_fdt_params {
> - u64 system_table;
> - u64 mmap;
> - u32 mmap_size;
> - u32 desc_size;
> - u32 desc_ver;
> -};
> -
> -struct params {
> - const char name[32];
> - const char propname[32];
> - int offset;
> - int size;
> +enum {
> + SYSTAB,
> + MMBASE,
> + MMSIZE,
> + DCSIZE,
> + DCVERS,
> };
>
> -static __initdata struct params fdt_params[] = {
> - UEFI_PARAM("System Table", "linux,uefi-system-table", system_table),
> - UEFI_PARAM("MemMap Address", "linux,uefi-mmap-start", mmap),
> - UEFI_PARAM("MemMap Size", "linux,uefi-mmap-size", mmap_size),
> - UEFI_PARAM("MemMap Desc. Size", "linux,uefi-mmap-desc-size", desc_size),
> - UEFI_PARAM("MemMap Desc. Version", "linux,uefi-mmap-desc-ver", desc_ver)
> +static __initconst const char name[][22] = {
I was going to complain about that 22, and I still think it looks a
bit iffy, but I can't find a compiler that doesn't throw a warning if
the value changes, so I guess I'm being over sensitive.
> + [SYSTAB] = "System Table ",
> + [MMBASE] = "MemMap Address ",
> + [MMSIZE] = "MemMap Size ",
> + [DCSIZE] = "MemMap Desc. Size ",
> + [DCVERS] = "MemMap Desc. Version ",
> };
>
> -static __initdata struct params xen_fdt_params[] = {
> - UEFI_PARAM("System Table", "xen,uefi-system-table", system_table),
> - UEFI_PARAM("MemMap Address", "xen,uefi-mmap-start", mmap),
> - UEFI_PARAM("MemMap Size", "xen,uefi-mmap-size", mmap_size),
> - UEFI_PARAM("MemMap Desc. Size", "xen,uefi-mmap-desc-size", desc_size),
> - UEFI_PARAM("MemMap Desc. Version", "xen,uefi-mmap-desc-ver", desc_ver)
> -};
> -
> -#define EFI_FDT_PARAMS_SIZE ARRAY_SIZE(fdt_params)
> -
> -static __initdata struct {
> - const char *uname;
> - const char *subnode;
> - struct params *params;
> +static __initconst const struct {
> + const char path[17];
> + const char params[5][26];
And I guess the same is true here, but the 5 *could* theoretically be
replaced by a compile-time calculation of DCVERS - SYSTAB + 1. That
may not be less error prone however. Finishing the enum on a
somethingMAX entry might make it slightly better?
But the error checking isn't really what I'm after, but getting rid of
the opaqueness. Could we have comments about which specific string
lengths we're matching against here for us bears of very little brain?
/
Leif
> } dt_params[] = {
> - { "hypervisor", "uefi", xen_fdt_params },
> - { "chosen", NULL, fdt_params },
> -};
> -
> -struct param_info {
> - int found;
> - void *params;
> - const char *missing;
> +#ifdef CONFIG_XEN
> + {
> + "/hypervisor/uefi",
> + {
> + [SYSTAB] = "xen,uefi-system-table",
> + [MMBASE] = "xen,uefi-mmap-start",
> + [MMSIZE] = "xen,uefi-mmap-size",
> + [DCSIZE] = "xen,uefi-mmap-desc-size",
> + [DCVERS] = "xen,uefi-mmap-desc-ver",
> + }
> + },
> +#endif
> + {
> + "/chosen",
> + {
> + [SYSTAB] = "linux,uefi-system-table",
> + [MMBASE] = "linux,uefi-mmap-start",
> + [MMSIZE] = "linux,uefi-mmap-size",
> + [DCSIZE] = "linux,uefi-mmap-desc-size",
> + [DCVERS] = "linux,uefi-mmap-desc-ver",
> + }
> + }
> };
>
> -static int __init __find_uefi_params(unsigned long node,
> - struct param_info *info,
> - struct params *params)
> +static int __init efi_get_fdt_prop(const void *fdt, int node, int dtp, int pp,
> + void *var, int size)
> {
> const void *prop;
> - void *dest;
> + int len;
> u64 val;
> - int i, len;
>
> - for (i = 0; i < EFI_FDT_PARAMS_SIZE; i++) {
> - prop = of_get_flat_dt_prop(node, params[i].propname, &len);
> - if (!prop) {
> - info->missing = params[i].name;
> - return 0;
> - }
> -
> - dest = info->params + params[i].offset;
> - info->found++;
> + prop = fdt_getprop(fdt, node, dt_params[dtp].params[pp], &len);
> + if (!prop) {
> + pr_err("Can't find property '%s' in device tree!\n",
> + dt_params[dtp].params[pp]);
> + return 1;
> + }
>
> - val = of_read_number(prop, len / sizeof(u32));
> + val = (len == 4) ? (u64)be32_to_cpup(prop) : get_unaligned_be64(prop);
>
> - if (params[i].size == sizeof(u32))
> - *(u32 *)dest = val;
> - else
> - *(u64 *)dest = val;
> + if (size == 8)
> + *(u64 *)var = val;
> + else
> + *(u32 *)var = (val <= U32_MAX) ? val : U32_MAX; // saturate
>
> - if (efi_enabled(EFI_DBG))
> - pr_info(" %s: 0x%0*llx\n", params[i].name,
> - params[i].size * 2, val);
> - }
> + if (efi_enabled(EFI_DBG))
> + pr_info(" %s: 0x%0*llx\n", name[pp], size * 2, val);
>
> - return 1;
> + return 0;
> }
>
> -static int __init fdt_find_uefi_params(unsigned long node, const char *uname,
> - int depth, void *data)
> +u64 __init efi_get_fdt_params(struct efi_memory_map_data *mm)
> {
> - struct param_info *info = data;
> + const void *fdt = initial_boot_params;
> + unsigned long systab;
> + struct {
> + void *var;
> + int size;
> + } target[] = {
> + [SYSTAB] = { &systab, sizeof(systab) },
> + [MMBASE] = { &mm->phys_map, sizeof(mm->phys_map) },
> + [MMSIZE] = { &mm->size, sizeof(mm->size) },
> + [DCSIZE] = { &mm->desc_size, sizeof(mm->desc_size) },
> + [DCVERS] = { &mm->desc_version, sizeof(mm->desc_version) },
> + };
> int i;
>
> + BUILD_BUG_ON(ARRAY_SIZE(target) != ARRAY_SIZE(name));
> + BUILD_BUG_ON(ARRAY_SIZE(target) != ARRAY_SIZE(dt_params[0].params));
> +
> for (i = 0; i < ARRAY_SIZE(dt_params); i++) {
> - const char *subnode = dt_params[i].subnode;
> + int node;
> + int j;
>
> - if (depth != 1 || strcmp(uname, dt_params[i].uname) != 0) {
> - info->missing = dt_params[i].params[0].name;
> + node = fdt_path_offset(fdt, dt_params[i].path);
> + if (node < 0)
> continue;
> - }
>
> - if (subnode) {
> - int err = of_get_flat_dt_subnode_by_name(node, subnode);
> + if (efi_enabled(EFI_DBG))
> + pr_info("Getting UEFI parameters from %s in DT:\n",
> + dt_params[i].path);
>
> - if (err < 0)
> + for (j = 0; j < ARRAY_SIZE(target); j++) {
> + if (efi_get_fdt_prop(fdt, node, i, j, target[j].var,
> + target[j].size)) {
> return 0;
> -
> - node = err;
> + }
> }
> -
> - return __find_uefi_params(node, info, dt_params[i].params);
> + return systab;
> }
> -
> + pr_info("UEFI not found.\n");
> return 0;
> }
> -
> -u64 __init efi_get_fdt_params(struct efi_memory_map_data *memmap)
> -{
> - struct efi_fdt_params params;
> - struct param_info info;
> - int ret;
> -
> - pr_info("Getting EFI parameters from FDT:\n");
> -
> - info.found = 0;
> - info.params = ¶ms;
> -
> - ret = of_scan_flat_dt(fdt_find_uefi_params, &info);
> - if (!info.found) {
> - pr_info("UEFI not found.\n");
> - return 0;
> - } else if (!ret) {
> - pr_err("Can't find '%s' in device tree!\n", info.missing);
> - return 0;
> - }
> -
> - memmap->desc_version = params.desc_ver;
> - memmap->desc_size = params.desc_size;
> - memmap->size = params.mmap_size;
> - memmap->phys_map = params.mmap;
> -
> - return params.system_table;
> -}
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH 0/3] Optimize VM's downtime while do checkpoint in COLO
From: Dr. David Alan Gilbert @ 2020-02-20 18:27 UTC (permalink / raw)
To: Hailiang Zhang; +Cc: danielcho, chen.zhang, qemu-devel, quintela
In-Reply-To: <20200217012049.22988-1-zhang.zhanghailiang@huawei.com>
* Hailiang Zhang (zhang.zhanghailiang@huawei.com) wrote:
> Hi,
>
> This is an untested serial that tries to reduce VM's pause time
> while do checkpoint in COLO state.
>
> The second patch tries to reduce the total number of dirty pages
> while do checkpoint with VM been paused, instead of sending all
> dirty pages while VM been pause, it sends part of dirty pages during
> the gap time of two checkpoints when SVM and PVM are running.
>
> The third patch tries to reduce the pause time of backup ram into
> cache in secondary part.
This is quite nice; I've asked for a clarification on the last one, but
it's only a tidy up.
I guess this really helps continuous-COLO as well, because it means
that restarting to the sync to the new secondary is a lot simpler?
Dave
>
> Hailiang Zhang (3):
> migration/colo: wrap incoming checkpoint process into new helper
> COLO: Migrate dirty pages during the gap of checkpointing
> COLO: Optimize memory back-up process
>
> migration/colo.c | 332 +++++++++++++++++++++++++----------------
> migration/migration.h | 1 +
> migration/ram.c | 35 ++++-
> migration/ram.h | 1 +
> migration/trace-events | 1 +
> qapi/migration.json | 4 +-
> 6 files changed, 234 insertions(+), 140 deletions(-)
>
> --
> 2.21.0
>
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply
* Re: [PATCH 3/3] efi/arm: rewrite FDT param discovery routines
From: Leif Lindholm @ 2020-02-20 18:30 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: atish.patra, linux-efi, linux-arm-kernel
In-Reply-To: <20200219152440.11561-4-ardb@kernel.org>
On Wed, Feb 19, 2020 at 16:24:40 +0100, Ard Biesheuvel wrote:
> The efi_get_fdt_params() routine uses the early OF device tree
> traversal helpers, that iterate over each node in the DT and invoke
> a caller provided callback that can inspect the node's contents and
> look for the required data. This requires a special param struct to
> be passed around, with pointers into param enumeration structs that
> contain (and duplicate) property names and offsets into yet another
> struct that carries the collected data.
>
> Since we know the data we look for is either under /hypervisor/uefi
> or under /chosen, it is much simpler to use the libfdt routines
> directly, and try to grab a reference to either node directly, and
> read each property in sequence.
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
> drivers/firmware/efi/fdtparams.c | 203 ++++++++------------
> 1 file changed, 85 insertions(+), 118 deletions(-)
>
> diff --git a/drivers/firmware/efi/fdtparams.c b/drivers/firmware/efi/fdtparams.c
> index 7a384b307c56..23af4062e913 100644
> --- a/drivers/firmware/efi/fdtparams.c
> +++ b/drivers/firmware/efi/fdtparams.c
> @@ -5,154 +5,121 @@
> #include <linux/module.h>
> #include <linux/init.h>
> #include <linux/efi.h>
> -#include <linux/of.h>
> +#include <linux/libfdt.h>
> #include <linux/of_fdt.h>
>
> -#include <asm/early_ioremap.h>
> +#include <asm/unaligned.h>
>
> -#define UEFI_PARAM(name, prop, field) \
> - { \
> - { name }, \
> - { prop }, \
> - offsetof(struct efi_fdt_params, field), \
> - sizeof_field(struct efi_fdt_params, field) \
> - }
> -
> -struct efi_fdt_params {
> - u64 system_table;
> - u64 mmap;
> - u32 mmap_size;
> - u32 desc_size;
> - u32 desc_ver;
> -};
> -
> -struct params {
> - const char name[32];
> - const char propname[32];
> - int offset;
> - int size;
> +enum {
> + SYSTAB,
> + MMBASE,
> + MMSIZE,
> + DCSIZE,
> + DCVERS,
> };
>
> -static __initdata struct params fdt_params[] = {
> - UEFI_PARAM("System Table", "linux,uefi-system-table", system_table),
> - UEFI_PARAM("MemMap Address", "linux,uefi-mmap-start", mmap),
> - UEFI_PARAM("MemMap Size", "linux,uefi-mmap-size", mmap_size),
> - UEFI_PARAM("MemMap Desc. Size", "linux,uefi-mmap-desc-size", desc_size),
> - UEFI_PARAM("MemMap Desc. Version", "linux,uefi-mmap-desc-ver", desc_ver)
> +static __initconst const char name[][22] = {
I was going to complain about that 22, and I still think it looks a
bit iffy, but I can't find a compiler that doesn't throw a warning if
the value changes, so I guess I'm being over sensitive.
> + [SYSTAB] = "System Table ",
> + [MMBASE] = "MemMap Address ",
> + [MMSIZE] = "MemMap Size ",
> + [DCSIZE] = "MemMap Desc. Size ",
> + [DCVERS] = "MemMap Desc. Version ",
> };
>
> -static __initdata struct params xen_fdt_params[] = {
> - UEFI_PARAM("System Table", "xen,uefi-system-table", system_table),
> - UEFI_PARAM("MemMap Address", "xen,uefi-mmap-start", mmap),
> - UEFI_PARAM("MemMap Size", "xen,uefi-mmap-size", mmap_size),
> - UEFI_PARAM("MemMap Desc. Size", "xen,uefi-mmap-desc-size", desc_size),
> - UEFI_PARAM("MemMap Desc. Version", "xen,uefi-mmap-desc-ver", desc_ver)
> -};
> -
> -#define EFI_FDT_PARAMS_SIZE ARRAY_SIZE(fdt_params)
> -
> -static __initdata struct {
> - const char *uname;
> - const char *subnode;
> - struct params *params;
> +static __initconst const struct {
> + const char path[17];
> + const char params[5][26];
And I guess the same is true here, but the 5 *could* theoretically be
replaced by a compile-time calculation of DCVERS - SYSTAB + 1. That
may not be less error prone however. Finishing the enum on a
somethingMAX entry might make it slightly better?
But the error checking isn't really what I'm after, but getting rid of
the opaqueness. Could we have comments about which specific string
lengths we're matching against here for us bears of very little brain?
/
Leif
> } dt_params[] = {
> - { "hypervisor", "uefi", xen_fdt_params },
> - { "chosen", NULL, fdt_params },
> -};
> -
> -struct param_info {
> - int found;
> - void *params;
> - const char *missing;
> +#ifdef CONFIG_XEN
> + {
> + "/hypervisor/uefi",
> + {
> + [SYSTAB] = "xen,uefi-system-table",
> + [MMBASE] = "xen,uefi-mmap-start",
> + [MMSIZE] = "xen,uefi-mmap-size",
> + [DCSIZE] = "xen,uefi-mmap-desc-size",
> + [DCVERS] = "xen,uefi-mmap-desc-ver",
> + }
> + },
> +#endif
> + {
> + "/chosen",
> + {
> + [SYSTAB] = "linux,uefi-system-table",
> + [MMBASE] = "linux,uefi-mmap-start",
> + [MMSIZE] = "linux,uefi-mmap-size",
> + [DCSIZE] = "linux,uefi-mmap-desc-size",
> + [DCVERS] = "linux,uefi-mmap-desc-ver",
> + }
> + }
> };
>
> -static int __init __find_uefi_params(unsigned long node,
> - struct param_info *info,
> - struct params *params)
> +static int __init efi_get_fdt_prop(const void *fdt, int node, int dtp, int pp,
> + void *var, int size)
> {
> const void *prop;
> - void *dest;
> + int len;
> u64 val;
> - int i, len;
>
> - for (i = 0; i < EFI_FDT_PARAMS_SIZE; i++) {
> - prop = of_get_flat_dt_prop(node, params[i].propname, &len);
> - if (!prop) {
> - info->missing = params[i].name;
> - return 0;
> - }
> -
> - dest = info->params + params[i].offset;
> - info->found++;
> + prop = fdt_getprop(fdt, node, dt_params[dtp].params[pp], &len);
> + if (!prop) {
> + pr_err("Can't find property '%s' in device tree!\n",
> + dt_params[dtp].params[pp]);
> + return 1;
> + }
>
> - val = of_read_number(prop, len / sizeof(u32));
> + val = (len == 4) ? (u64)be32_to_cpup(prop) : get_unaligned_be64(prop);
>
> - if (params[i].size == sizeof(u32))
> - *(u32 *)dest = val;
> - else
> - *(u64 *)dest = val;
> + if (size == 8)
> + *(u64 *)var = val;
> + else
> + *(u32 *)var = (val <= U32_MAX) ? val : U32_MAX; // saturate
>
> - if (efi_enabled(EFI_DBG))
> - pr_info(" %s: 0x%0*llx\n", params[i].name,
> - params[i].size * 2, val);
> - }
> + if (efi_enabled(EFI_DBG))
> + pr_info(" %s: 0x%0*llx\n", name[pp], size * 2, val);
>
> - return 1;
> + return 0;
> }
>
> -static int __init fdt_find_uefi_params(unsigned long node, const char *uname,
> - int depth, void *data)
> +u64 __init efi_get_fdt_params(struct efi_memory_map_data *mm)
> {
> - struct param_info *info = data;
> + const void *fdt = initial_boot_params;
> + unsigned long systab;
> + struct {
> + void *var;
> + int size;
> + } target[] = {
> + [SYSTAB] = { &systab, sizeof(systab) },
> + [MMBASE] = { &mm->phys_map, sizeof(mm->phys_map) },
> + [MMSIZE] = { &mm->size, sizeof(mm->size) },
> + [DCSIZE] = { &mm->desc_size, sizeof(mm->desc_size) },
> + [DCVERS] = { &mm->desc_version, sizeof(mm->desc_version) },
> + };
> int i;
>
> + BUILD_BUG_ON(ARRAY_SIZE(target) != ARRAY_SIZE(name));
> + BUILD_BUG_ON(ARRAY_SIZE(target) != ARRAY_SIZE(dt_params[0].params));
> +
> for (i = 0; i < ARRAY_SIZE(dt_params); i++) {
> - const char *subnode = dt_params[i].subnode;
> + int node;
> + int j;
>
> - if (depth != 1 || strcmp(uname, dt_params[i].uname) != 0) {
> - info->missing = dt_params[i].params[0].name;
> + node = fdt_path_offset(fdt, dt_params[i].path);
> + if (node < 0)
> continue;
> - }
>
> - if (subnode) {
> - int err = of_get_flat_dt_subnode_by_name(node, subnode);
> + if (efi_enabled(EFI_DBG))
> + pr_info("Getting UEFI parameters from %s in DT:\n",
> + dt_params[i].path);
>
> - if (err < 0)
> + for (j = 0; j < ARRAY_SIZE(target); j++) {
> + if (efi_get_fdt_prop(fdt, node, i, j, target[j].var,
> + target[j].size)) {
> return 0;
> -
> - node = err;
> + }
> }
> -
> - return __find_uefi_params(node, info, dt_params[i].params);
> + return systab;
> }
> -
> + pr_info("UEFI not found.\n");
> return 0;
> }
> -
> -u64 __init efi_get_fdt_params(struct efi_memory_map_data *memmap)
> -{
> - struct efi_fdt_params params;
> - struct param_info info;
> - int ret;
> -
> - pr_info("Getting EFI parameters from FDT:\n");
> -
> - info.found = 0;
> - info.params = ¶ms;
> -
> - ret = of_scan_flat_dt(fdt_find_uefi_params, &info);
> - if (!info.found) {
> - pr_info("UEFI not found.\n");
> - return 0;
> - } else if (!ret) {
> - pr_err("Can't find '%s' in device tree!\n", info.missing);
> - return 0;
> - }
> -
> - memmap->desc_version = params.desc_ver;
> - memmap->desc_size = params.desc_size;
> - memmap->size = params.mmap_size;
> - memmap->phys_map = params.mmap;
> -
> - return params.system_table;
> -}
> --
> 2.17.1
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] IB/core, cache: Replace zero-length array with flexible-array member
From: Gustavo A. R. Silva @ 2020-02-20 18:09 UTC (permalink / raw)
To: Jason Gunthorpe; +Cc: Doug Ledford, linux-rdma, linux-kernel, Dan Carpenter
In-Reply-To: <20200220173756.GA19347@ziepe.ca>
On 2/20/20 11:37, Jason Gunthorpe wrote:
>
> I squished the four drivers/infiniband patches together and added a
> few more, see below. Applied to for-next
>
Great! Thank you. :)
> It would be fanatstic to follow this up with some analysis to find
> cases where sizeof() is performed on a struct with a [] flex array -
> these days people should be using struct_size() (interested DanC?)
>
I've been working on both things:
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/log/?qt=grep&q=struct_size%28%29+helper
Thanks
--
Gustavo
^ permalink raw reply
* Re: [PATCH v2 4/6] NFS: Add READ_PLUS data segment support
From: Chuck Lever @ 2020-02-20 18:30 UTC (permalink / raw)
To: Anna Schumaker; +Cc: Trond.Myklebust, Linux NFS Mailing List
In-Reply-To: <cbff77a5175b448e955fafc45b5c80b3dcff0f5b.camel@gmail.com>
> On Feb 20, 2020, at 1:28 PM, Anna Schumaker <schumaker.anna@gmail.com> wrote:
>
> On Thu, 2020-02-20 at 09:55 -0500, Chuck Lever wrote:
>>> On Feb 20, 2020, at 9:42 AM, Anna Schumaker <schumaker.anna@gmail.com>
>>> wrote:
>>>
>>> On Fri, 2020-02-14 at 17:28 -0500, Chuck Lever wrote:
>>>>> On Feb 14, 2020, at 4:12 PM, schumaker.anna@gmail.com wrote:
>>>>>
>>>>> From: Anna Schumaker <Anna.Schumaker@Netapp.com>
>>>>>
>>>>> This patch adds client support for decoding a single NFS4_CONTENT_DATA
>>>>> segment returned by the server. This is the simplest implementation
>>>>> possible, since it does not account for any hole segments in the reply.
>>>>>
>>>>> Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
>>>>> ---
>>>>> fs/nfs/nfs42xdr.c | 138 ++++++++++++++++++++++++++++++++++++++
>>>>> fs/nfs/nfs4proc.c | 43 +++++++++++-
>>>>> fs/nfs/nfs4xdr.c | 1 +
>>>>> include/linux/nfs4.h | 2 +-
>>>>> include/linux/nfs_fs_sb.h | 1 +
>>>>> include/linux/nfs_xdr.h | 2 +-
>>>>> 6 files changed, 182 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/fs/nfs/nfs42xdr.c b/fs/nfs/nfs42xdr.c
>>>>> index c03f3246d6c5..bf118ecabe2c 100644
>>>>> --- a/fs/nfs/nfs42xdr.c
>>>>> +++ b/fs/nfs/nfs42xdr.c
>>>>> @@ -45,6 +45,15 @@
>>>>> #define encode_deallocate_maxsz (op_encode_hdr_maxsz + \
>>>>> encode_fallocate_maxsz)
>>>>> #define decode_deallocate_maxsz (op_decode_hdr_maxsz)
>>>>> +#define encode_read_plus_maxsz (op_encode_hdr_maxsz + \
>>>>> + encode_stateid_maxsz + 3)
>>>>> +#define NFS42_READ_PLUS_SEGMENT_SIZE (1 /* data_content4 */ + \
>>>>> + 2 /* data_info4.di_offset */ +
>>>>> \
>>>>> + 2 /* data_info4.di_length */)
>>>>> +#define decode_read_plus_maxsz (op_decode_hdr_maxsz + \
>>>>> + 1 /* rpr_eof */ + \
>>>>> + 1 /* rpr_contents count */ + \
>>>>> + NFS42_READ_PLUS_SEGMENT_SIZE)
>>>>> #define encode_seek_maxsz (op_encode_hdr_maxsz + \
>>>>> encode_stateid_maxsz + \
>>>>> 2 /* offset */ + \
>>>>> @@ -128,6 +137,14 @@
>>>>> decode_putfh_maxsz + \
>>>>> decode_deallocate_maxsz + \
>>>>> decode_getattr_maxsz)
>>>>> +#define NFS4_enc_read_plus_sz (compound_encode_hdr_maxsz + \
>>>>> + encode_sequence_maxsz + \
>>>>> + encode_putfh_maxsz + \
>>>>> + encode_read_plus_maxsz)
>>>>> +#define NFS4_dec_read_plus_sz (compound_decode_hdr_maxsz + \
>>>>> + decode_sequence_maxsz + \
>>>>> + decode_putfh_maxsz + \
>>>>> + decode_read_plus_maxsz)
>>>>> #define NFS4_enc_seek_sz (compound_encode_hdr_maxsz + \
>>>>> encode_sequence_maxsz + \
>>>>> encode_putfh_maxsz + \
>>>>> @@ -252,6 +269,16 @@ static void encode_deallocate(struct xdr_stream
>>>>> *xdr,
>>>>> encode_fallocate(xdr, args);
>>>>> }
>>>>>
>>>>> +static void encode_read_plus(struct xdr_stream *xdr,
>>>>> + const struct nfs_pgio_args *args,
>>>>> + struct compound_hdr *hdr)
>>>>> +{
>>>>> + encode_op_hdr(xdr, OP_READ_PLUS, decode_read_plus_maxsz, hdr);
>>>>> + encode_nfs4_stateid(xdr, &args->stateid);
>>>>> + encode_uint64(xdr, args->offset);
>>>>> + encode_uint32(xdr, args->count);
>>>>> +}
>>>>> +
>>>>> static void encode_seek(struct xdr_stream *xdr,
>>>>> const struct nfs42_seek_args *args,
>>>>> struct compound_hdr *hdr)
>>>>> @@ -446,6 +473,29 @@ static void nfs4_xdr_enc_deallocate(struct rpc_rqst
>>>>> *req,
>>>>> encode_nops(&hdr);
>>>>> }
>>>>>
>>>>> +/*
>>>>> + * Encode READ_PLUS request
>>>>> + */
>>>>> +static void nfs4_xdr_enc_read_plus(struct rpc_rqst *req,
>>>>> + struct xdr_stream *xdr,
>>>>> + const void *data)
>>>>> +{
>>>>> + const struct nfs_pgio_args *args = data;
>>>>> + struct compound_hdr hdr = {
>>>>> + .minorversion = nfs4_xdr_minorversion(&args->seq_args),
>>>>> + };
>>>>> +
>>>>> + encode_compound_hdr(xdr, req, &hdr);
>>>>> + encode_sequence(xdr, &args->seq_args, &hdr);
>>>>> + encode_putfh(xdr, args->fh, &hdr);
>>>>> + encode_read_plus(xdr, args, &hdr);
>>>>> +
>>>>> + rpc_prepare_reply_pages(req, args->pages, args->pgbase,
>>>>> + args->count, hdr.replen);
>>>>> + req->rq_rcv_buf.flags |= XDRBUF_READ;
>>>>
>>>> IMO this line is incorrect.
>>>
>>> You're right, this line causes problems for RDMA with READ_PLUS. I added it
>>> to
>>> match how the other xdr read encoders were set up
>>
>> Ja, I think just removing that line should be sufficient.
>> Better would be replacing it with a comment explaining
>> why this encoder does not set XDRBUF_READ. :-)
>>
>>
>>>> RFC 8267 Section 6.1 does not list any part of the result of READ_PLUS
>>>> as DDP-eligible. There's no way for a client to know how to set up
>>>> Write chunks, unless it knows exactly where the file's holes are in
>>>> advance. Even then... racy.
>>>>
>>>> Just curious, have you tried READ_PLUS with proto=rdma ?
>>>
>>> I haven't done in-depth performance testing, but I have been able to run it.
>>
>> We should figure out whether that will have a regressive
>> impact on NFS/RDMA workloads. I expect that it will, but
>> the client can always set up the Reply chunk so that the
>> READ payload fits precisely in an RDMA segment that lines
>> up with page cache pages. That mitigates some impact.
>>
>> If your patch set already changes NFSv4.2 mounts to always
>> use READ_PLUS in place of READ, it might be prudent for the
>> "proto=rdma" mount option to also set "noreadplus", at least
>> for the time being.
>
> I can make this change.
>
>>
>> The down-side here is that would make NFSv4.2 on RDMA
>> unable to recognize holes in files the same way as it
>> does on TCP, and that's a pretty significant variation
>> in behavior. Does "noreadplus" even deal with that?
>
> Setting "noreadplus" just causes the client to use the READ operation instead,
> so there should be no difference between v4.1 and v4.2 if the option is set.
My concern is the difference between NFSv4.2 with noreadplus
and NFSv4.2 with readplus. The former is not able to detect
holes in files on the server, but the latter is.
Is that worth mentioning in the man page, or in release notes
when NFSv4.2 becomes the default?
> Anna
>
>>
>>
>>> Anna
>>>
>>>>
>>>>> + encode_nops(&hdr);
>>>>> +}
>>>>> +
>>>>> /*
>>>>> * Encode SEEK request
>>>>> */
>>>>> @@ -694,6 +744,67 @@ static int decode_deallocate(struct xdr_stream
>>>>> *xdr,
>>>>> struct nfs42_falloc_res *re
>>>>> return decode_op_hdr(xdr, OP_DEALLOCATE);
>>>>> }
>>>>>
>>>>> +static uint32_t decode_read_plus_data(struct xdr_stream *xdr, struct
>>>>> nfs_pgio_res *res,
>>>>> + uint32_t *eof)
>>>>> +{
>>>>> + __be32 *p;
>>>>> + uint32_t count, recvd;
>>>>> + uint64_t offset;
>>>>> +
>>>>> + p = xdr_inline_decode(xdr, 8 + 4);
>>>>> + if (unlikely(!p))
>>>>> + return -EIO;
>>>>> +
>>>>> + p = xdr_decode_hyper(p, &offset);
>>>>> + count = be32_to_cpup(p);
>>>>> + if (count == 0)
>>>>> + return 0;
>>>>> +
>>>>> + recvd = xdr_read_pages(xdr, count);
>>>>> + if (count > recvd) {
>>>>> + dprintk("NFS: server cheating in read reply: "
>>>>> + "count %u > recvd %u\n", count, recvd);
>>>>> + count = recvd;
>>>>> + *eof = 0;
>>>>> + }
>>>>> +
>>>>> + return count;
>>>>> +}
>>>>> +
>>>>> +static int decode_read_plus(struct xdr_stream *xdr, struct nfs_pgio_res
>>>>> *res)
>>>>> +{
>>>>> + __be32 *p;
>>>>> + uint32_t count, eof, segments, type;
>>>>> + int status;
>>>>> +
>>>>> + status = decode_op_hdr(xdr, OP_READ_PLUS);
>>>>> + if (status)
>>>>> + return status;
>>>>> +
>>>>> + p = xdr_inline_decode(xdr, 4 + 4);
>>>>> + if (unlikely(!p))
>>>>> + return -EIO;
>>>>> +
>>>>> + eof = be32_to_cpup(p++);
>>>>> + segments = be32_to_cpup(p++);
>>>>> + if (segments == 0)
>>>>> + return 0;
>>>>> +
>>>>> + p = xdr_inline_decode(xdr, 4);
>>>>> + if (unlikely(!p))
>>>>> + return -EIO;
>>>>> +
>>>>> + type = be32_to_cpup(p++);
>>>>> + if (type == NFS4_CONTENT_DATA)
>>>>> + count = decode_read_plus_data(xdr, res, &eof);
>>>>> + else
>>>>> + return -EINVAL;
>>>>> +
>>>>> + res->eof = eof;
>>>>> + res->count = count;
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> static int decode_seek(struct xdr_stream *xdr, struct nfs42_seek_res
>>>>> *res)
>>>>> {
>>>>> int status;
>>>>> @@ -870,6 +981,33 @@ static int nfs4_xdr_dec_deallocate(struct rpc_rqst
>>>>> *rqstp,
>>>>> return status;
>>>>> }
>>>>>
>>>>> +/*
>>>>> + * Decode READ_PLUS request
>>>>> + */
>>>>> +static int nfs4_xdr_dec_read_plus(struct rpc_rqst *rqstp,
>>>>> + struct xdr_stream *xdr,
>>>>> + void *data)
>>>>> +{
>>>>> + struct nfs_pgio_res *res = data;
>>>>> + struct compound_hdr hdr;
>>>>> + int status;
>>>>> +
>>>>> + status = decode_compound_hdr(xdr, &hdr);
>>>>> + if (status)
>>>>> + goto out;
>>>>> + status = decode_sequence(xdr, &res->seq_res, rqstp);
>>>>> + if (status)
>>>>> + goto out;
>>>>> + status = decode_putfh(xdr);
>>>>> + if (status)
>>>>> + goto out;
>>>>> + status = decode_read_plus(xdr, res);
>>>>> + if (!status)
>>>>> + status = res->count;
>>>>> +out:
>>>>> + return status;
>>>>> +}
>>>>> +
>>>>> /*
>>>>> * Decode SEEK request
>>>>> */
>>>>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>>>>> index 95d07a3dc5d1..ed3ec8c36273 100644
>>>>> --- a/fs/nfs/nfs4proc.c
>>>>> +++ b/fs/nfs/nfs4proc.c
>>>>> @@ -69,6 +69,10 @@
>>>>>
>>>>> #include "nfs4trace.h"
>>>>>
>>>>> +#ifdef CONFIG_NFS_V4_2
>>>>> +#include "nfs42.h"
>>>>> +#endif /* CONFIG_NFS_V4_2 */
>>>>> +
>>>>> #define NFSDBG_FACILITY NFSDBG_PROC
>>>>>
>>>>> #define NFS4_BITMASK_SZ 3
>>>>> @@ -5199,28 +5203,60 @@ static bool nfs4_read_stateid_changed(struct
>>>>> rpc_task *task,
>>>>> return true;
>>>>> }
>>>>>
>>>>> +static bool nfs4_read_plus_not_supported(struct rpc_task *task,
>>>>> + struct nfs_pgio_header *hdr)
>>>>> +{
>>>>> + struct nfs_server *server = NFS_SERVER(hdr->inode);
>>>>> + struct rpc_message *msg = &task->tk_msg;
>>>>> +
>>>>> + if (msg->rpc_proc == &nfs4_procedures[NFSPROC4_CLNT_READ_PLUS]
>>>>> &&
>>>>> + server->caps & NFS_CAP_READ_PLUS && task->tk_status ==
>>>>> -ENOTSUPP) {
>>>>> + server->caps &= ~NFS_CAP_READ_PLUS;
>>>>> + msg->rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_READ];
>>>>> + rpc_restart_call_prepare(task);
>>>>> + return true;
>>>>> + }
>>>>> + return false;
>>>>> +}
>>>>> +
>>>>> static int nfs4_read_done(struct rpc_task *task, struct nfs_pgio_header
>>>>> *hdr)
>>>>> {
>>>>> -
>>>>> dprintk("--> %s\n", __func__);
>>>>>
>>>>> if (!nfs4_sequence_done(task, &hdr->res.seq_res))
>>>>> return -EAGAIN;
>>>>> if (nfs4_read_stateid_changed(task, &hdr->args))
>>>>> return -EAGAIN;
>>>>> + if (nfs4_read_plus_not_supported(task, hdr))
>>>>> + return -EAGAIN;
>>>>> if (task->tk_status > 0)
>>>>> nfs_invalidate_atime(hdr->inode);
>>>>> return hdr->pgio_done_cb ? hdr->pgio_done_cb(task, hdr) :
>>>>> nfs4_read_done_cb(task, hdr);
>>>>> }
>>>>>
>>>>> +#ifdef CONFIG_NFS_V4_2
>>>>> +static void nfs42_read_plus_support(struct nfs_server *server, struct
>>>>> rpc_message *msg)
>>>>> +{
>>>>> + if (server->caps & NFS_CAP_READ_PLUS)
>>>>> + msg->rpc_proc =
>>>>> &nfs4_procedures[NFSPROC4_CLNT_READ_PLUS];
>>>>> + else
>>>>> + msg->rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_READ];
>>>>> +}
>>>>> +#else
>>>>> +static void nfs42_read_plus_support(struct nfs_server *server, struct
>>>>> rpc_message *msg)
>>>>> +{
>>>>> + msg->rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_READ];
>>>>> +}
>>>>> +#endif /* CONFIG_NFS_V4_2 */
>>>>> +
>>>>> static void nfs4_proc_read_setup(struct nfs_pgio_header *hdr,
>>>>> struct rpc_message *msg)
>>>>> {
>>>>> hdr->timestamp = jiffies;
>>>>> if (!hdr->pgio_done_cb)
>>>>> hdr->pgio_done_cb = nfs4_read_done_cb;
>>>>> - msg->rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_READ];
>>>>> + nfs42_read_plus_support(NFS_SERVER(hdr->inode), msg);
>>>>> nfs4_init_sequence(&hdr->args.seq_args, &hdr->res.seq_res, 0,
>>>>> 0);
>>>>> }
>>>>>
>>>>> @@ -9970,7 +10006,8 @@ static const struct nfs4_minor_version_ops
>>>>> nfs_v4_2_minor_ops = {
>>>>> | NFS_CAP_SEEK
>>>>> | NFS_CAP_LAYOUTSTATS
>>>>> | NFS_CAP_CLONE
>>>>> - | NFS_CAP_LAYOUTERROR,
>>>>> + | NFS_CAP_LAYOUTERROR
>>>>> + | NFS_CAP_READ_PLUS,
>>>>> .init_client = nfs41_init_client,
>>>>> .shutdown_client = nfs41_shutdown_client,
>>>>> .match_stateid = nfs41_match_stateid,
>>>>> diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
>>>>> index 47817ef0aadb..68b2917d0537 100644
>>>>> --- a/fs/nfs/nfs4xdr.c
>>>>> +++ b/fs/nfs/nfs4xdr.c
>>>>> @@ -7584,6 +7584,7 @@ const struct rpc_procinfo nfs4_procedures[] = {
>>>>> PROC42(COPY_NOTIFY, enc_copy_notify, dec_copy_notify)
>>>>> ,
>>>>> PROC(LOOKUPP, enc_lookupp, dec_lookupp),
>>>>> PROC42(LAYOUTERROR, enc_layouterror, dec_layouterror)
>>>>> ,
>>>>> + PROC42(READ_PLUS, enc_read_plus, dec_read_plus),
>>>>> };
>>>>>
>>>>> static unsigned int nfs_version4_counts[ARRAY_SIZE(nfs4_procedures)];
>>>>> diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
>>>>> index 82d8fb422092..c1eeef52545c 100644
>>>>> --- a/include/linux/nfs4.h
>>>>> +++ b/include/linux/nfs4.h
>>>>> @@ -540,8 +540,8 @@ enum {
>>>>>
>>>>> NFSPROC4_CLNT_LOOKUPP,
>>>>> NFSPROC4_CLNT_LAYOUTERROR,
>>>>> -
>>>>> NFSPROC4_CLNT_COPY_NOTIFY,
>>>>> + NFSPROC4_CLNT_READ_PLUS,
>>>>> };
>>>>>
>>>>> /* nfs41 types */
>>>>> diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
>>>>> index 465fa98258a3..11248c5a7b24 100644
>>>>> --- a/include/linux/nfs_fs_sb.h
>>>>> +++ b/include/linux/nfs_fs_sb.h
>>>>> @@ -281,5 +281,6 @@ struct nfs_server {
>>>>> #define NFS_CAP_OFFLOAD_CANCEL (1U << 25)
>>>>> #define NFS_CAP_LAYOUTERROR (1U << 26)
>>>>> #define NFS_CAP_COPY_NOTIFY (1U << 27)
>>>>> +#define NFS_CAP_READ_PLUS (1U << 28)
>>>>>
>>>>> #endif
>>>>> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
>>>>> index 94c77ed55ce1..8efbf3d8b263 100644
>>>>> --- a/include/linux/nfs_xdr.h
>>>>> +++ b/include/linux/nfs_xdr.h
>>>>> @@ -655,7 +655,7 @@ struct nfs_pgio_args {
>>>>> struct nfs_pgio_res {
>>>>> struct nfs4_sequence_res seq_res;
>>>>> struct nfs_fattr * fattr;
>>>>> - __u32 count;
>>>>> + __u64 count;
>>>>> __u32 op_status;
>>>>> union {
>>>>> struct {
>>>>> --
>>>>> 2.25.0
>>>>>
>>>>
>>>> --
>>>> Chuck Lever
>>
>> --
>> Chuck Lever
--
Chuck Lever
^ permalink raw reply
* Re: [PATCH RFT 0/2/2] mtd: hyperbus: add Renesas RPC-IF driver
From: Sergei Shtylyov @ 2020-02-20 18:30 UTC (permalink / raw)
To: Vignesh Raghavendra, Behme Dirk (CM/ESO2), Miquel Raynal,
Richard Weinberger, linux-mtd, Mark Brown
In-Reply-To: <aef44eb7-78df-8505-ad37-d67fd499532e@ti.com>
On 02/18/2020 02:11 PM, Vignesh Raghavendra wrote:
[...]
>>> Looking around, there seems to be more than one SPI controllers, apart
>>> from Renesas, which also support SPI NOR and HyperFlash protocol within
>>> a single IP block. E.g.: Cadence xSPI controller [1]. Therefore, we need
>>> a generic framework to support these kind of controllers.
>>>
>>> One way would be to extend spi_mem_op to support above template along
>>> with a new field to distinguish SPI NOR vs HyperFlash protocol. HyperBus
>>> core can then register a spi_device and use spi-mem ops to talk to
>>> controller driver.
>>> So, I suggest making Renesas RPC-IF backend a full fledged spi-mem
>>> driver (instead of driver/memory) and use extended spi_mem_op to support
>>> HyperFlash.
>>
>>
>> From Renesas Hyperflash user point of view, I wonder if a two step
>> approach would be possible and acceptable, here?
>>
>> Being a user of the Renesas Hyperflash, I want a driver for that. And,
>> of course, I want it "now" ;)
>>
>> So I wonder if it would be a valid option to have a functioning Renesas
>> Hypeflash driver, first. And in a second step abstract that in a more
>> generic way to support additional controllers. While in parallel having
>> a functional driver for the Renesas people, already.
>
> AFAICS, the backend driver is not merged and is still in RFC phase.
It was still marked RFC back in December and I haven't received any
feedback since, other than Dirk's request. Where have you been? Well,
I should have CCed linux-mtd back then... :-/
> Therefore I don't see any benefit of two step approach here. Besides
> you'll have to throw away this new driver (hyperbus/rpc-if.c) entirely
> later on.
Why did you create this directory for, anyway? :-/
> How difficult is it to rewrite backend to be spi-mem driver? There is
> already has a spi_mem_ops frontend implementation, so I don't see much
> of an issue.
Really? This may be not much of an issue with coding this but it's
certainly time consuming (I'm sure there's s/th to think about yet in
this case)? My management (and also me, so far) believes I'm in the
final stage with these drivers... what should I say to my boss now?
> Extending hyperbus core to use spi-mem should also straight forward
> Would involve moving this patch into core file.
Seriously, only "moving"?
>> Is the support for [1] a more or less theoretical one, at the moment? Or
>> are there users of that which need support "now", too?
>
> Its not theoretical, I do see patches for xSPI controller here:
> https://patchwork.kernel.org/cover/11354193/
Which (surprise!) only adds support for the SPI part...
> So, its best to sort this out now so as to avoid possible backward
> compatibility issues (especially with DT bindings)
What DT issues do you mean exactly? I think that other than changing
the "home" dir for the bindings, there'd little to change. The "front ends"
don't deal with the DT probing...
> Regards
> Vignesh
MBR, Sergei
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply
* Re: [PATCH net 0/3] s390/qeth: fixes 2020-02-20
From: David Miller @ 2020-02-20 18:31 UTC (permalink / raw)
To: jwi; +Cc: netdev, linux-s390, heiko.carstens, ubraun
In-Reply-To: <20200220145456.11571-1-jwi@linux.ibm.com>
From: Julian Wiedmann <jwi@linux.ibm.com>
Date: Thu, 20 Feb 2020 15:54:53 +0100
> please apply the following patch series for qeth to netdev's net tree.
>
> This corrects three minor issues:
> 1) return a more fitting errno when VNICC cmds are not supported,
> 2) remove a bogus WARN in the NAPI code, and
> 3) be _very_ pedantic about the RX copybreak.
Series applied, thanks.
^ permalink raw reply
* Re: Questions about logic_pio
From: John Garry @ 2020-02-20 18:32 UTC (permalink / raw)
To: Jiaxun Yang
Cc: xuwei (O), bhelgaas, andyshevchenko, Arnd Bergmann, linux-kernel,
Linux Mips
In-Reply-To: <e54a9936-5cf3-777d-3e91-58d2be96bf1c@huawei.com>
On 20/02/2020 17:39, John Garry wrote:
> On 20/02/2020 15:12, Jiaxun Yang wrote:
>>
>> ---- 在 星期四, 2020-02-20 22:23:57 John Garry
>> <john.garry@huawei.com> 撰写 ----
>> > > Also Cc MIPS list to check other's opinions.
>> > >
>> > > Hi John.
>> > >
>> >
>> > Hi Jiaxun Yang,
>> >
>> > > Thanks for your kind explanation, however, I think this way is
>> > > violating how I/O ports supposed to work, at least in MIPS world.
>> >
>> > For a bit more history, please understand that the core PCI code was
>> > managing non-native IO port space in the same way before we added the
>> > logic PIO framework. The only real functional change here was that we
>> > introduced the indirect-io region within the IO port space, under
>> > CONFIG_INDIRECT_PIO.
>>
>> I'm going to do more investigation. Thanks.
>>
>> >
>> > >
>> > > > >>
>> > > > >> After dig into logic pio logic, I found that logic pio is
>> trying to "allocate" an io_start
>> > > > >> for MMIO ranges, the allocation starts from 0x0. And
>> later the io_start is used to calculate
>> > > > >> cpu_address. In my opinion, for direct MMIO access,
>> logic_pio address should always
>> > > > >> equal to hw address,
>> > > >
>> > > > I'm not sure what you mean by simply the hw address.
>> > > >
>> > >
>> > > I meant hw_start should always equal to io_start.
>> > >
>> > >
>> > > MIPS have their own wrapped inl/outl functions,
>> >
>> > Can you please point me to these? I could not find them in arch/mips
>>
>> They are built by __BUILD_IOPORT_PFX(bus, bwlq, type) macro.
>> Just using mips_io_port_base + offset to handle inl/outl, the same way
>> PCI_IOBASE.
>
> Right, so I had a glance through the code and mips has it own management
> of this IO port space. And, like you say, mips_io_port_base is
> equivalent to PCI_IOBASE.
>
>>
>> >
>> > I will also note that arch/mips/include/asm/io.h does not include
>> > asm-generic io.h today
>>
>> Yes, and I'm attempting to take advantage of asm-generic.
>
> I just don't think it's as simple as saying we want to take advantage of
> asm-generic. asm-generic io.h includes logic_pio.h, which uses logical
> PIO to manage IO port space and relies on PIO_IOBASE. This is
> incompatible with having some other framework - like mips_io_port_base -
> managing IO port space at the same time.
>
> The core PCI code relies on logical PIO to manage IO port space for when
> PCI_IOBASE is defined.
>
>>
>> >
>> > doing the samething with
>> > > PCI_IOBASE enabled one. I was just trying to use PCI_IOBASE
>> instead.
>> > >
>> > > Originally, the I/O ports layout seems like this:
>> > >
>> > > 00000020-00000021 : pic1
>> > > 00000060-0000006f : i8042
>> > > 00000070-00000077 : rtc0
>> > > 000000a0-000000a1 : pic2
>> > > 00000170-00000177 : pata_atiixp
>> > > 000001f0-000001f7 : pata_atiixp
>> > > 00000376-00000376 : pata_atiixp
>> > > 000003f6-000003f6 : pata_atiixp
>> > > 00000800-000008ff : acpi
>> > > 00001000-00001008 : piix4_smbus
>> > > 00004000-0003ffff : pci io space
>> > > 00004000-00004fff : PCI Bus 0000:01
>> > > 00004000-000040ff : 0000:01:05.0
>> > > 00005000-00005fff : PCI Bus 0000:03
>> > > 00005000-0000501f : 0000:03:00.0
>> > >
>> > > But with PCI_IOBASE defined, I got this:
>> > >
>> > > host bridge /bus@10000000/pci@10000000 ranges:
>> > > MEM 0x0040000000..0x007fffffff -> 0x0040000000
>> > > IO 0x0000004000..0x0000007fff -> 0x0000004000
>> > > resource collision: [io 0x0000-0x3fff] conflicts with pic1 [io
>> 0x0020-0x0021]
>> > >
>> > > Because io_start was allocated to 0x0 by Logic PIO.
>> > >
>> > > There are a lot of devices that have fixed ioports thanks to
>> x86's legacy.
>> >
>> > Well, yes, I'm not so surprised.
>> >
>> > So if MIPS does not have native IO port access, then surely you need
>> > some host bridge to translate host CPU MMIO accesses to port I/O
>> > accesses, right? Where are these CPU addresses defined?
>>
>> It is defined by the variable mips_io_port_base.
>>
>> >
>> > > For example, in my hardware, ioports for RTC, PIC, I8042 are
>> unmoveable,
>> > > and they can't be managed by logic pio subsystem. > Also, the
>> PCI Hostbridge got implied by DeviceTree that it's I/O range
>> > > started from 0x4000 in bus side
>> >
>> > which bus is this?
>>
>> They're all located under "ISA Range". Just an MMIO range that will
>> resend
>> the request to ISA I/O. --ioports for both PCI and some legacy devices.
>>
>> In that range, base + 0x0000 to 0x4000 is preserved for PIO devices
>> (e.g.) I8259
>> and base + 0x4000 to MMIO_LIMIT are for PCI devices under host bridge.
>> For the host bridge, ioports it can decode starts from 0x4000.
>>
>> My intentional behavior is that when I'm specifying in dts that the IO
>> Range of PCI host
>> bridge is 0x4000 to 0x7fff, it would request the IO_RESOURCE start
>> from 0x4000
>> to 0x7fff, also tell the host driver to decode 0x4000 to 0x7fff in IO
>> BAR, And let the drivers
>> access 0x4000 to 0x7fff via inl/outl, rather than allocate from PIO
>> 0x0 to 0x3fff.
>>
>> >
>> > , but then, Logic PIO remapped to PCI_IOBASE + 0x0.
>> > > The real address should be PCI_IOBASE + 0x4000,
>> >
>> > You seem to be using two methods to manage IO port space, and they
>> seem
>> > to be conflicting.
>>
>> So... Are there any way to handle these unmoveable devices in logic
>> pio world?
>
> When you say that they are unmovable, they are at a fixed address on
> this "ISA Range", right? If so, yes, you should be able to handle it in
> logical PIO. You just need to deal with translating logical PIO
> addresses to ISA bus addresses. We do this very thing in our LPC driver
> - see drivers/bus/hisi_lpc.c
I will add this may not cover your need, as you probably cannot deal
with any logical PIO <-> ISA translation without modifying the device
driver. For this, we may need to reserve the first 0x4000 in logical PIO
space for this sort of legacy host.
That would not be a bad thing - see
https://lore.kernel.org/linux-pci/1560262374-67875-1-git-send-email-john.garry@huawei.com/
>
> This driver deals with legacy IO ports where we need to bitbang
> accesses, i.e. we don't support MMIO for this.
>>
>> >
>> > > hardware never got correctly informed about that. And there is
>> still no way to
>> > > transform to correct address as it's inside the MMIO_LIMIT.
>> > >
>> > > So the question comes to why we're allocating io_start for MMIO
>> PCI_IOBASE
>> > > rather than just check the range provided doesn't overlap each
>> other or exceed
>> > > the MMIO_LIMIT.
>> >
>> > When PCI_IOBASE is defined, we work on the basis that any IO port
>> range
>> > in the system is registered for a logical PIO region, which
>> manages the
>> > actual IO port addresses - see logic_pio_trans_cpuaddr().
>>
>> The port is not the actual port.. It makes me confusing about what
>> it's actually doing..
>> Sorry but probably I'm still thinking in a vintage way -- need some
>> hints about how to
>> deal with these legacy cases in a modern way.
>>
>> Thanks.
>>
>> >
>> > Thanks,
>> > John
>> >
>>
>
>
^ permalink raw reply
* Re: BUG: git clean -d cannot remove files from read-only directories
From: Junio C Hamano @ 2020-02-20 18:32 UTC (permalink / raw)
To: Adam Milazzo; +Cc: git@vger.kernel.org
In-Reply-To: <BL0PR2101MB10898F2F84B06B1A6B23228380130@BL0PR2101MB1089.namprd21.prod.outlook.com>
Adam Milazzo <Adam.Milazzo@microsoft.com> writes:
> Repro steps:
> 1. git init
> 2. mkdir d
> 3. touch d/a
> 4. chmod -w d
> 5. git clean -fd
>
> Actual result:
> Git doesn't remove anything, saying "warning: failed to remove d/a".
>
> Expected result:
> Git should remove the subdirectory 'd' along with its
> contents. Note that git can remove a read-only file (touch b;
> chmod -w b; git clean -f) with no problem.
It is how UNIX-like filesystem works, isn't it?
As long as a directory D is writable/executable, any regular file
inside it can be removed regardless of the perm-bits of the file.
mkdir d
touch d/a
chmod -w d
rm d/a
would not let you remove the file d/a from d/, exactly because you
cannot modify d/ (it is not writable).
^ permalink raw reply
* Re: About r8169 regression 5.4
From: Heiner Kallweit @ 2020-02-20 18:32 UTC (permalink / raw)
To: Vincas Dargis, Salvatore Bonaccorso; +Cc: netdev
In-Reply-To: <d2b5d904-61e1-6c14-f137-d4d5a803dcf6@gmail.com>
On 20.02.2020 18:36, Vincas Dargis wrote:
> 2020-02-19 23:54, Heiner Kallweit rašė:
>> Realtek responded that they are not aware of a HW issue with RTL8411b. They will try to reproduce the error,
>> in addition they ask to test whether same issue occurs with their own driver, r8168.
>> Would be great if you could give r8168 a try. Most distributions provide it as an optional package.
>> Worst case it can be downloaded from Realtek's website, then it needs to be compiled.
>>
>
> I have installed r8168-dkms:
>
> ```
> apt policy r8168-dkms
> r8168-dkms:
> Installed: 8.048.00-1
> Candidate: 8.048.00-1
> Version table:
> *** 8.048.00-1 500
> 500 http://debian.mirror.vu.lt/debian unstable/non-free amd64 Packages
> 500 http://debian.mirror.vu.lt/debian unstable/non-free i386 Packages
> 100 /var/lib/dpkg/status
> ```
>
> Rebooted, and still (rather fast) got same timeout after maybe couple of minutes of running on same 5.4 withoyt ethtool fixes:
>
> Feb 20 19:24:54 vinco kernel: [ 228.808802] ------------[ cut here ]------------
> Feb 20 19:24:54 vinco kernel: [ 228.808832] NETDEV WATCHDOG: enp5s0f1 (r8168): transmit queue 0 timed out
> Feb 20 19:24:54 vinco kernel: [ 228.808865] WARNING: CPU: 7 PID: 0 at net/sched/sch_generic.c:447 dev_watchdog+0x248/0x250
> Feb 20 19:24:54 vinco kernel: [ 228.808871] Modules linked in: xt_recent ipt_REJECT nf_reject_ipv4 xt_multiport xt_conntrack xt_hashlimit xt_addrtype xt_iface(OE) xt_mark nft_chain_nat xt_comment xt_CT xt_owner xt_tcpudp nft_compat nft_counter xt_NFLOG nf_log_ipv4 nf_log_common xt_LOG nf_nat_tftp nf_nat_snmp_basic nf_conntrack_snmp nf_nat_sip nf_nat_pptp nf_nat_irc nf_nat_h323 nf_nat_ftp nf_nat_amanda ts_kmp nf_conntrack_amanda nf_nat nf_conntrack_sane nf_conntrack_tftp nf_conntrack_sip nf_conntrack_pptp nf_conntrack_netlink nf_conntrack_netbios_ns nf_conntrack_broadcast nf_conntrack_irc nf_conntrack_h323 nf_conntrack_ftp nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 nf_tables rfcomm vboxnetadp(OE) vboxnetflt(OE) xfrm_user xfrm_algo vboxdrv(OE) l2tp_ppp l2tp_netlink l2tp_core ip6_udp_tunnel udp_tunnel pppox ppp_generic slhc bnep nfnetlink_log nfnetlink ipmi_devintf ipmi_msghandler intel_rapl_msr intel_rapl_common bbswitch(OE) x86_pkg_temp_thermal intel_powerclamp coretemp
> kvm_intel kvm irqbypass btusb btrtl iwlmvm btbcm
> Feb 20 19:24:54 vinco kernel: [ 228.808935] binfmt_misc btintel bluetooth crct10dif_pclmul nls_ascii ghash_clmulni_intel mac80211 nls_cp437 snd_hda_codec_realtek snd_hda_codec_generic vfat drbg ledtrig_audio libarc4 snd_hda_codec_hdmi fat uvcvideo aesni_intel iwlwifi videobuf2_vmalloc ansi_cprng crypto_simd videobuf2_memops videobuf2_v4l2 snd_hda_intel cryptd glue_helper snd_intel_nhlt videobuf2_common snd_hda_codec videodev snd_hda_core intel_cstate ecdh_generic ecc cfg80211 mc intel_uncore efi_pstore snd_hwdep snd_pcm snd_timer asus_nb_wmi joydev asus_wmi snd sparse_keymap iTCO_wdt rtsx_pci_ms iTCO_vendor_support pcspkr intel_rapl_perf serio_raw sg efivars soundcore memstick watchdog rfkill ie31200_edac evdev ac asus_wireless parport_pc ppdev lp parport efivarfs ip_tables x_tables autofs4 ext4 crc16 mbcache jbd2 btrfs xor zstd_decompress zstd_compress raid6_pq libcrc32c crc32c_generic sr_mod cdrom sd_mod hid_logitech_hidpp hid_logitech_dj hid_generic usbhid hid i915
> rtsx_pci_sdmmc mmc_core i2c_algo_bit ahci
> Feb 20 19:24:54 vinco kernel: [ 228.809003] drm_kms_helper libahci xhci_pci crc32_pclmul mxm_wmi libata xhci_hcd drm crc32c_intel ehci_pci ehci_hcd psmouse scsi_mod usbcore rtsx_pci lpc_ich i2c_i801 mfd_core r8168(OE) usb_common video wmi battery button
> Feb 20 19:24:54 vinco kernel: [ 228.809025] CPU: 7 PID: 0 Comm: swapper/7 Tainted: P OE 5.4.0-4-amd64 #1 Debian 5.4.19-1
> Feb 20 19:24:54 vinco kernel: [ 228.809027] Hardware name: ASUSTeK COMPUTER INC. N551JM/N551JM, BIOS N551JM.205 02/13/2015
> Feb 20 19:24:54 vinco kernel: [ 228.809034] RIP: 0010:dev_watchdog+0x248/0x250
> Feb 20 19:24:54 vinco kernel: [ 228.809038] Code: 85 c0 75 e5 eb 9f 4c 89 ef c6 05 58 1d a8 00 01 e8 0d e4 fa ff 44 89 e1 4c 89 ee 48 c7 c7 f0 cc d2 9a 48 89 c2 e8 76 40 a0 ff <0f> 0b eb 80 0f 1f 40 00 0f 1f 44 00 00 41 57 41 56 49 89 d6 41 55
> Feb 20 19:24:54 vinco kernel: [ 228.809040] RSP: 0018:ffffc0e74020ce68 EFLAGS: 00010286
> Feb 20 19:24:54 vinco kernel: [ 228.809043] RAX: 0000000000000000 RBX: ffff9f660df8e200 RCX: 000000000000083f
> Feb 20 19:24:54 vinco kernel: [ 228.809044] RDX: 0000000000000000 RSI: 00000000000000f6 RDI: 000000000000083f
> Feb 20 19:24:54 vinco kernel: [ 228.809046] RBP: ffff9f660d30045c R08: ffff9f661edd7688 R09: 0000000000000004
> Feb 20 19:24:54 vinco kernel: [ 228.809048] R10: 0000000000000000 R11: 0000000000000001 R12: 0000000000000000
> Feb 20 19:24:54 vinco kernel: [ 228.809049] R13: ffff9f660d300000 R14: ffff9f660d300480 R15: 0000000000000001
> Feb 20 19:24:54 vinco kernel: [ 228.809052] FS: 0000000000000000(0000) GS:ffff9f661edc0000(0000) knlGS:0000000000000000
> Feb 20 19:24:54 vinco kernel: [ 228.809054] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> Feb 20 19:24:54 vinco kernel: [ 228.809055] CR2: 00005f1110bf6da4 CR3: 0000000185e0a001 CR4: 00000000001626e0
> Feb 20 19:24:54 vinco kernel: [ 228.809057] Call Trace:
> Feb 20 19:24:54 vinco kernel: [ 228.809060] <IRQ>
> Feb 20 19:24:54 vinco kernel: [ 228.809068] ? pfifo_fast_enqueue+0x150/0x150
> Feb 20 19:24:54 vinco kernel: [ 228.809073] call_timer_fn+0x2d/0x130
> Feb 20 19:24:54 vinco kernel: [ 228.809077] __run_timers.part.0+0x16f/0x260
> Feb 20 19:24:54 vinco kernel: [ 228.809084] ? tick_sched_handle+0x22/0x60
> Feb 20 19:24:54 vinco kernel: [ 228.809089] ? tick_sched_timer+0x38/0x80
> Feb 20 19:24:54 vinco kernel: [ 228.809093] ? tick_sched_do_timer+0x60/0x60
> Feb 20 19:24:54 vinco kernel: [ 228.809096] run_timer_softirq+0x26/0x50
> Feb 20 19:24:54 vinco kernel: [ 228.809102] __do_softirq+0xe6/0x2e9
> Feb 20 19:24:54 vinco kernel: [ 228.809111] irq_exit+0xa6/0xb0
> Feb 20 19:24:54 vinco kernel: [ 228.809115] smp_apic_timer_interrupt+0x76/0x130
> Feb 20 19:24:54 vinco kernel: [ 228.809118] apic_timer_interrupt+0xf/0x20
> Feb 20 19:24:54 vinco kernel: [ 228.809120] </IRQ>
> Feb 20 19:24:54 vinco kernel: [ 228.809126] RIP: 0010:cpuidle_enter_state+0xc4/0x450
> Feb 20 19:24:54 vinco kernel: [ 228.809129] Code: e8 b1 54 ad ff 80 7c 24 0f 00 74 17 9c 58 0f 1f 44 00 00 f6 c4 02 0f 85 61 03 00 00 31 ff e8 a3 74 b3 ff fb 66 0f 1f 44 00 00 <45> 85 e4 0f 88 8c 02 00 00 49 63 cc 4c 2b 6c 24 10 48 8d 04 49 48
> Feb 20 19:24:54 vinco kernel: [ 228.809131] RSP: 0018:ffffc0e7400cfe68 EFLAGS: 00000246 ORIG_RAX: ffffffffffffff13
> Feb 20 19:24:54 vinco kernel: [ 228.809133] RAX: ffff9f661edea6c0 RBX: ffffffff9aeb92e0 RCX: 000000000000001f
> Feb 20 19:24:54 vinco kernel: [ 228.809135] RDX: 0000000000000000 RSI: 000000003351882d RDI: 0000000000000000
> Feb 20 19:24:54 vinco kernel: [ 228.809137] RBP: ffff9f661edf4a00 R08: 000000354610e2fa R09: 0000000000029fa0
> Feb 20 19:24:54 vinco kernel: [ 228.809138] R10: ffff9f661ede95a0 R11: ffff9f661ede9580 R12: 0000000000000005
> Feb 20 19:24:54 vinco kernel: [ 228.809140] R13: 000000354610e2fa R14: 0000000000000005 R15: ffff9f661caa8f00
> Feb 20 19:24:54 vinco kernel: [ 228.809145] ? cpuidle_enter_state+0x9f/0x450
> Feb 20 19:24:54 vinco kernel: [ 228.809149] cpuidle_enter+0x29/0x40
> Feb 20 19:24:54 vinco kernel: [ 228.809155] do_idle+0x1dc/0x270
> Feb 20 19:24:54 vinco kernel: [ 228.809162] cpu_startup_entry+0x19/0x20
> Feb 20 19:24:54 vinco kernel: [ 228.809168] start_secondary+0x15f/0x1b0
> Feb 20 19:24:54 vinco kernel: [ 228.809174] secondary_startup_64+0xa4/0xb0
> Feb 20 19:24:54 vinco kernel: [ 228.809179] ---[ end trace f2c0113df7c88e86 ]---
> Feb 20 19:24:57 vinco kernel: [ 231.448423] r8168: enp5s0f1: link up
>
> Full kernl.log is attached.
>
> Interestingly, network did started working again after some time. Does that "link up" mean card was reset successfully or something?
Thanks a lot again for testing! I didn't check in detail, but most likely r8168 has the same NIC reset procedure
after a tx timeout. "link up" means that at least the link on PHY level was successfully re-established.
It would be great if you could test one more thing. Few chip versions have a hw issue with tx checksumming
for very small packets. Maybe your chip version suffers from the same issue.
Could you please test the following patch (with all features enabled, TSO and checksumming)?
diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 8442b8767..bee90af57 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -4345,6 +4345,7 @@ static netdev_features_t rtl8169_features_check(struct sk_buff *skb,
case RTL_GIGA_MAC_VER_12:
case RTL_GIGA_MAC_VER_17:
case RTL_GIGA_MAC_VER_34:
+ case RTL_GIGA_MAC_VER_44:
features &= ~NETIF_F_CSUM_MASK;
break;
default:
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v6 4/7] clocksource: Ingenic: Add high resolution timer support for SMP.
From: Paul Cercueil @ 2020-02-20 18:33 UTC (permalink / raw)
To: 周琰杰 (Zhou Yanjie)
Cc: linux-mips, linux-kernel, linux-clk, devicetree, tglx, ralf,
paulburton, jiaxun.yang, chenhc, sboyd, mturquette, mark.rutland,
robh+dt, daniel.lezcano, geert+renesas, krzk, ebiederm,
miquel.raynal, keescook, sernia.zhou, zhenwenjin, dongsheng.qiu
In-Reply-To: <1582215889-113034-6-git-send-email-zhouyanjie@wanyeetech.com>
Hi Zhou,
Le ven., févr. 21, 2020 at 00:24, 周琰杰 (Zhou Yanjie)
<zhouyanjie@wanyeetech.com> a écrit :
> Enable clock event handling on per CPU core basis.
> Make sure that interrupts raised on the first core execute
> event handlers on the correct CPU core.
>
> Tested-by: H. Nikolaus Schaller <hns@goldelico.com>
> Tested-by: Paul Boddie <paul@boddie.org.uk>
> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
> ---
>
> Notes:
> v1->v2:
> 1.Adjust function naming to make it more reasonable.
> 2.Replace function smp_call_function_single() with
> smp_call_function_single_async() in order to resolve
> the warning below:
>
> [ 0.350942] smp: Brought up 1 node, 2 CPUs
> [ 0.365497] ------------[ cut here ]------------
> [ 0.365522] WARNING: CPU: 0 PID: 1 at kernel/smp.c:300
> smp_call_function_single+0x110/0x200
> [ 0.365533] CPU: 0 PID: 1 Comm: swapper/0 Not tainted
> 5.5.0-rc1+ #5
> [ 0.365537] Stack : 00000000 59c73bcd 00000037 80074e80
> 80000000 80670000 805a0000 80620590
> [ 0.365557] 8065ce38 8fc0dc8c 806d0000 00000000
> 80670000 00000001 8fc0dc20 59c73bcd
> [ 0.365574] 00000000 00000000 806f0000 80670000
> 00000000 806dab00 00000000 2d302e35
> [ 0.365591] 203a6d6d 806e0000 806e0000 70617773
> 80670000 00000000 00000000 00000009
> [ 0.365610] 00000000 8fc94e20 8fc0de30 80690000
> 00000018 803592dc 00000000 806d0000
> [ 0.365627] ...
> [ 0.365634] Call Trace:
> [ 0.365647] [<8001b9a0>] show_stack+0x6c/0x12c
> [ 0.365663] [<804aed20>] dump_stack+0x98/0xc8
> [ 0.365673] [<8003044c>] __warn+0xc4/0xe8
> [ 0.365682] [<800304f4>] warn_slowpath_fmt+0x84/0xb8
> [ 0.365690] [<800a886c>] smp_call_function_single+0x110/0x200
> [ 0.365703] ---[ end trace 5785856ca39c79d5 ]---
> [ 0.365557] 8065ce38 8fc0dc8c 806d0000 00000000
> 80670000 00000001 8fc0dc20 59c73bcd
> [ 0.365574] 00000000 00000000 806f0000 80670000
> 00000000 806dab00 00000000 2d302e35
> [ 0.365591] 203a6d6d 806e0000 806e0000 70617773
> 80670000 00000000 00000000 00000009
> [ 0.365610] 00000000 8fc94e20 8fc0de30 80690000
> 00000018 803592dc 00000000 806d0000
> [ 0.365627] ...
> [ 0.365634] Call Trace:
> [ 0.365647] [<8001b9a0>] show_stack+0x6c/0x12c
> [ 0.365663] [<804aed20>] dump_stack+0x98/0xc8
> [ 0.365673] [<8003044c>] __warn+0xc4/0xe8
> [ 0.365682] [<800304f4>] warn_slowpath_fmt+0x84/0xb8
> [ 0.365690] [<800a886c>] smp_call_function_single+0x110/0x200
> [ 0.365703] ---[ end trace 5785856ca39c79d5 ]---
>
> v2->v3:
> No Change.
>
> v3->v4:
> Rebase on top of kernel 5.6-rc1.
>
> v4->v5:
> Move the check for (evt->event_handler) from
> "ingenic_per_cpu_event_handler"
> to "ingenic_tcu_cevt_cb".
>
> v5->v6:
> No change.
>
> drivers/clocksource/ingenic-timer.c | 113
> +++++++++++++++++++++++++++++-------
> 1 file changed, 91 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/clocksource/ingenic-timer.c
> b/drivers/clocksource/ingenic-timer.c
> index 4bbdb3d..e396326 100644
> --- a/drivers/clocksource/ingenic-timer.c
> +++ b/drivers/clocksource/ingenic-timer.c
> @@ -1,7 +1,8 @@
> // SPDX-License-Identifier: GPL-2.0
> /*
> - * JZ47xx SoCs TCU IRQ driver
> + * XBurst SoCs TCU IRQ driver
> * Copyright (C) 2019 Paul Cercueil <paul@crapouillou.net>
> + * Copyright (C) 2020 周琰杰 (Zhou Yanjie)
> <zhouyanjie@wanyeetech.com>
> */
>
> #include <linux/bitops.h>
> @@ -21,18 +22,23 @@
>
> #include <dt-bindings/clock/ingenic,tcu.h>
>
> +static DEFINE_PER_CPU(call_single_data_t, ingenic_cevt_csd);
> +
> struct ingenic_soc_info {
> unsigned int num_channels;
> };
>
> struct ingenic_tcu {
> struct regmap *map;
> + struct device_node *np;
> struct clk *timer_clk, *cs_clk;
> + unsigned int timer_local[NR_CPUS];
> unsigned int timer_channel, cs_channel;
> struct clock_event_device cevt;
> struct clocksource cs;
> - char name[4];
> + char name[8];
> unsigned long pwm_channels_mask;
> + int cpu;
> };
>
> static struct ingenic_tcu *ingenic_tcu;
> @@ -81,6 +87,24 @@ static int ingenic_tcu_cevt_set_next(unsigned long
> next,
> return 0;
> }
>
> +static void ingenic_per_cpu_event_handler(void *info)
> +{
> + struct clock_event_device *cevt = (struct clock_event_device *)
> info;
> +
> + cevt->event_handler(cevt);
> +}
> +
> +static void ingenic_tcu_per_cpu_cb(struct clock_event_device *evt)
> +{
> + struct ingenic_tcu *tcu = to_ingenic_tcu(evt);
> + call_single_data_t *csd;
> +
> + csd = &per_cpu(ingenic_cevt_csd, tcu->cpu);
> + csd->info = (void *) evt;
> + csd->func = ingenic_per_cpu_event_handler;
> + smp_call_function_single_async(tcu->cpu, csd);
> +}
> +
> static irqreturn_t ingenic_tcu_cevt_cb(int irq, void *dev_id)
> {
> struct clock_event_device *evt = dev_id;
> @@ -89,7 +113,7 @@ static irqreturn_t ingenic_tcu_cevt_cb(int irq,
> void *dev_id)
> regmap_write(tcu->map, TCU_REG_TECR, BIT(tcu->timer_channel));
>
> if (evt->event_handler)
> - evt->event_handler(evt);
> + ingenic_tcu_per_cpu_cb(evt);
>
> return IRQ_HANDLED;
> }
> @@ -105,14 +129,21 @@ static struct clk * __init
> ingenic_tcu_get_clock(struct device_node *np, int id)
> return of_clk_get_from_provider(&args);
> }
>
> -static int __init ingenic_tcu_timer_init(struct device_node *np,
> - struct ingenic_tcu *tcu)
> +static int ingenic_tcu_setup_per_cpu_cevt(struct device_node *np,
> + unsigned int channel)
> {
> - unsigned int timer_virq, channel = tcu->timer_channel;
> + unsigned int timer_virq;
> struct irq_domain *domain;
> + struct ingenic_tcu *tcu;
> unsigned long rate;
> int err;
>
> + tcu = kzalloc(sizeof(*tcu), GFP_KERNEL);
> + if (!tcu)
> + return -ENOMEM;
> +
> + tcu->map = ingenic_tcu->map;
> +
> tcu->timer_clk = ingenic_tcu_get_clock(np, channel);
> if (IS_ERR(tcu->timer_clk))
> return PTR_ERR(tcu->timer_clk);
> @@ -139,13 +170,15 @@ static int __init ingenic_tcu_timer_init(struct
> device_node *np,
> goto err_clk_disable;
> }
>
> - snprintf(tcu->name, sizeof(tcu->name), "TCU");
> + snprintf(tcu->name, sizeof(tcu->name), "TCU%u", channel);
>
> err = request_irq(timer_virq, ingenic_tcu_cevt_cb, IRQF_TIMER,
> tcu->name, &tcu->cevt);
> if (err)
> goto err_irq_dispose_mapping;
>
> + tcu->cpu = smp_processor_id();
> + tcu->timer_channel = channel;
> tcu->cevt.cpumask = cpumask_of(smp_processor_id());
> tcu->cevt.features = CLOCK_EVT_FEAT_ONESHOT;
> tcu->cevt.name = tcu->name;
> @@ -166,6 +199,25 @@ static int __init ingenic_tcu_timer_init(struct
> device_node *np,
> return err;
> }
>
> +static int ingenic_tcu_setup_cevt(unsigned int cpu)
> +{
> + int ret;
> +
> + ret = ingenic_tcu_setup_per_cpu_cevt(ingenic_tcu->np,
> + ingenic_tcu->timer_local[cpu]);
> + if (ret)
> + goto err_tcu_clocksource_cleanup;
> +
> + return 0;
> +
> +err_tcu_clocksource_cleanup:
> + clocksource_unregister(&ingenic_tcu->cs);
> + clk_disable_unprepare(ingenic_tcu->cs_clk);
> + clk_put(ingenic_tcu->cs_clk);
> + kfree(ingenic_tcu);
> + return ret;
> +}
> +
> static int __init ingenic_tcu_clocksource_init(struct device_node
> *np,
> struct ingenic_tcu *tcu)
> {
> @@ -239,6 +291,7 @@ static int __init ingenic_tcu_init(struct
> device_node *np)
> const struct ingenic_soc_info *soc_info = id->data;
> struct ingenic_tcu *tcu;
> struct regmap *map;
> + unsigned cpu = 0;
> long rate;
> int ret;
>
> @@ -252,13 +305,18 @@ static int __init ingenic_tcu_init(struct
> device_node *np)
> if (!tcu)
> return -ENOMEM;
>
> - /* Enable all TCU channels for PWM use by default except channels
> 0/1 */
> - tcu->pwm_channels_mask = GENMASK(soc_info->num_channels - 1, 2);
> + /*
> + * Enable all TCU channels for PWM use by default except channels
> 0/1,
> + * and channel 2 if target CPU is JZ4780 and SMP is selected.
> + */
> + tcu->pwm_channels_mask = GENMASK(soc_info->num_channels - 1,
> + NR_CPUS + 1);
> of_property_read_u32(np, "ingenic,pwm-channels-mask",
> (u32 *)&tcu->pwm_channels_mask);
>
> - /* Verify that we have at least two free channels */
> - if (hweight8(tcu->pwm_channels_mask) > soc_info->num_channels - 2) {
> + /* Verify that we have at least NR_CPUS + 1 free channels */
> + if (hweight8(tcu->pwm_channels_mask) >
> + soc_info->num_channels - NR_CPUS + 1) {
NR_CPUS can be up to 256. You want to use num_online_cpus() here, I
believe.
> pr_crit("%s: Invalid PWM channel mask: 0x%02lx\n", __func__,
> tcu->pwm_channels_mask);
> ret = -EINVAL;
> @@ -266,13 +324,27 @@ static int __init ingenic_tcu_init(struct
> device_node *np)
> }
>
> tcu->map = map;
> + tcu->np = np;
> ingenic_tcu = tcu;
>
> - tcu->timer_channel = find_first_zero_bit(&tcu->pwm_channels_mask,
> + tcu->timer_local[cpu] = find_first_zero_bit(&tcu->pwm_channels_mask,
> soc_info->num_channels);
> - tcu->cs_channel = find_next_zero_bit(&tcu->pwm_channels_mask,
> - soc_info->num_channels,
> - tcu->timer_channel + 1);
> +
> + if (NR_CPUS > 1) {
> + for (cpu = 1; cpu < NR_CPUS; cpu++)
> + tcu->timer_local[cpu] = find_next_zero_bit(
> + &tcu->pwm_channels_mask,
> + soc_info->num_channels,
> + tcu->timer_local[cpu - 1] + 1);
> +
> + tcu->cs_channel = find_next_zero_bit(&tcu->pwm_channels_mask,
> + soc_info->num_channels,
> + tcu->timer_local[cpu-1] + 1);
> + } else {
> + tcu->cs_channel = find_next_zero_bit(&tcu->pwm_channels_mask,
> + soc_info->num_channels,
> + tcu->timer_local[cpu] + 1);
> + }
I believe you can factorize the code a bit here - don't check for
NR_CPUS > 1, and have a for (cpu = 0; cpu < num_online_cpus(); cpu++)
loop. You could use a temporary variable to store the 'next bit' value.
-Paul
>
> ret = ingenic_tcu_clocksource_init(np, tcu);
> if (ret) {
> @@ -280,9 +352,10 @@ static int __init ingenic_tcu_init(struct
> device_node *np)
> goto err_free_ingenic_tcu;
> }
>
> - ret = ingenic_tcu_timer_init(np, tcu);
> - if (ret)
> - goto err_tcu_clocksource_cleanup;
> + /* Setup clock events on each CPU core */
> + ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "Ingenic XBurst:
> online",
> + ingenic_tcu_setup_cevt, NULL);
> + WARN_ON(ret < 0);
>
> /* Register the sched_clock at the end as there's no way to undo it
> */
> rate = clk_get_rate(tcu->cs_clk);
> @@ -290,10 +363,6 @@ static int __init ingenic_tcu_init(struct
> device_node *np)
>
> return 0;
>
> -err_tcu_clocksource_cleanup:
> - clocksource_unregister(&tcu->cs);
> - clk_disable_unprepare(tcu->cs_clk);
> - clk_put(tcu->cs_clk);
> err_free_ingenic_tcu:
> kfree(tcu);
> return ret;
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH v26 10/22] x86/sgx: Linux Enclave Driver
From: Jordan Hand @ 2020-02-20 18:33 UTC (permalink / raw)
To: Sean Christopherson
Cc: Jarkko Sakkinen, linux-kernel, x86, linux-sgx, akpm, dave.hansen,
nhorman, npmccallum, haitao.huang, andriy.shevchenko, tglx,
kai.svahn, bp, josh, luto, kai.huang, rientjes, cedric.xing,
puiterwijk, linux-security-module, Suresh Siddha, Haitao Huang
In-Reply-To: <20200220181345.GD3972@linux.intel.com>
On 2/20/20 10:13 AM, Sean Christopherson wrote:
> On Tue, Feb 18, 2020 at 07:26:31PM -0800, Jordan Hand wrote:
>> During mprotect (in mm/mprotect.c line 525) the following checks if
>> READ_IMPLIES_EXECUTE and a PROT_READ is being requested. If so and
>> VM_MAYEXEC is set, it also adds PROT_EXEC to the request.
>>
>> if (rier && (vma->vm_flags & VM_MAYEXEC))
>> prot |= PROT_EXEC;
>>
>> But if we look at sgx_encl_page_alloc(), we see vm_max_prot_bits is set
>> without taking VM_MAYEXEC into account:
>>
>> encl_page->vm_max_prot_bits = calc_vm_prot_bits(prot, 0);
>>
>> sgx_encl_may_map() checks that the requested protection can be added with:
>>
>> if (!page || (~page->vm_max_prot_bits & vm_prot_bits))
>> return -EACCESS
>>
>> This means that for any process where READ_IMPLIES_EXECUTE is set and
>> page where (vma->vm_flags & VM_MAYEXEC) == true, mmap/mprotect calls to
>> that request PROT_READ on a page that was not added with PROT_EXEC will
>> fail.
>
> I could've sworn this was discussed on the SGX list at one point, but
> apparently we only discussed it internally. Anyways...
>
> More than likely, the READ_IMPLIES_EXECUTE (RIE) crud rears its head
> because part of the enclave loader is written in assembly. Unless
> explicitly told otherwise, the linker assumes that any program with
> assembly code may need an executable stack, which leads to the RIE
> personality being set for the process. Here's a fantastic write up for
> more details: https://www.airs.com/blog/archives/518
>
> There are essentially two paths we can take:
>
> 1) Exempt EPC pages from RIE during mmap()/mprotect(), i.e. don't add
> PROT_EXEC for enclaves.
>
> 2) Punt the issue to userspace.
>
> Option (1) is desirable in some ways:
>
> - Enclaves will get an executable stack if and only if the loader/creator
> intentionally configures it to have an executable stack.
>
> - Separates enclaves from the personality of the loader.
>
> - Userspace doesn't have to do anything for the common case of not
> wanting an executable stack for its enclaves.
>
> The big down side to (1) is that it'd require an ugly hook in architecture
> agnostic code. And arguably, it reduces the overall security of the
> platform (more below).
>
> For (2), userspace has a few options:
>
> a) Tell the linker the enclave loader doesn't need RIE, either via a .note
> in assembly files or via the global "-z noexecstack" flag.
>
> b) Spawn a separate process to run/map the enclave if the enclave loader
> needs RIE.
>
> c) Require enclaves to allow PROT_EXEC on all pages. Note, this is an
> absolutely terrible idea and only included for completeness.
>
> As shown by the lack of a mmap()/mprotect() hook in this series to squash
> RIE, we chose option (2). Given that enclave loaders are not legacy code
> and hopefully following decent coding practices, option (2a) should suffice
> for all loaders. The security benefit mentioned above is that forcing
> enclave loaders to squash RIE eliminates an exectuable stack as an attack
> vector on the loader.
I see your point and I do agree that there are security benefits to (2a)
and I think we could do that for our loader. That said, it does concern
me that this breaks perfectly valid userspace behavior. If a userspace
process decides to use RIE, I don't know that the SGX driver should
disobey that decision.
So option (3) would be to just honor RIE for enclave pages and when page
permissions are set to PROT_READ in sgx_encl_page_alloc and RIE is set,
also add PROT_EXEC.
I understand your concerns that this using RIE is bad security practice
and I'm not convinced that (3) is the way to go, but from a philosophy
perspective I don't know that the kernel should be in the business of
stopping userspace from doing valid things.
If option (3) can't/shouldn't be done for some reason, option (1) at
least keeps from breaking expected userspace behavior. But I do agree
that (1) is ugly to implement.
-Jordan
^ permalink raw reply
* Re: [PATCH 5/8] xfs_db: check that metadata updates have been committed
From: Darrick J. Wong @ 2020-02-20 18:34 UTC (permalink / raw)
To: Brian Foster; +Cc: sandeen, linux-xfs
In-Reply-To: <20200220175857.GI48977@bfoster>
On Thu, Feb 20, 2020 at 12:58:57PM -0500, Brian Foster wrote:
> On Thu, Feb 20, 2020 at 08:58:40AM -0800, Darrick J. Wong wrote:
> > On Thu, Feb 20, 2020 at 09:06:23AM -0500, Brian Foster wrote:
> > > On Wed, Feb 19, 2020 at 05:42:13PM -0800, Darrick J. Wong wrote:
> > > > From: Darrick J. Wong <darrick.wong@oracle.com>
> > > >
> > > > Add a new function that will ensure that everything we scribbled on has
> > > > landed on stable media, and report the results.
> > > >
> > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > > > ---
> > > > db/init.c | 14 ++++++++++++++
> > > > 1 file changed, 14 insertions(+)
> > > >
> > > >
> > > > diff --git a/db/init.c b/db/init.c
> > > > index 0ac37368..e92de232 100644
> > > > --- a/db/init.c
> > > > +++ b/db/init.c
> > > > @@ -184,6 +184,7 @@ main(
> > > > char *input;
> > > > char **v;
> > > > int start_iocur_sp;
> > > > + int d, l, r;
> > > >
> > > > init(argc, argv);
> > > > start_iocur_sp = iocur_sp;
> > > > @@ -216,6 +217,19 @@ main(
> > > > */
> > > > while (iocur_sp > start_iocur_sp)
> > > > pop_cur();
> > > > +
> > > > + libxfs_flush_devices(mp, &d, &l, &r);
> > > > + if (d)
> > > > + fprintf(stderr, _("%s: cannot flush data device (%d).\n"),
> > > > + progname, d);
> > > > + if (l)
> > > > + fprintf(stderr, _("%s: cannot flush log device (%d).\n"),
> > > > + progname, l);
> > > > + if (r)
> > > > + fprintf(stderr, _("%s: cannot flush realtime device (%d).\n"),
> > > > + progname, r);
> > > > +
> > > > +
> > >
> > > Seems like we could reduce some boilerplate by passing progname into
> > > libxfs_flush_devices() and letting it dump out of the error messages,
> > > unless there's some future code that cares about individual device error
> > > state.
> >
> > Such a program could call libxfs_flush_devices directly, as we do here.
> >
>
> Right.. but does anything actually care about that level of granularity
> right now beyond having a nicer error message?
No, afaict.
> > Also, progname is defined in libxfs so we don't even need to pass it as
> > an argument.
> >
>
> Ok.
>
> > I had originally thought that we should try not to add fprintf calls to
> > libxfs because libraries aren't really supposed to be doing things like
> > that, but perhaps you're right that all of this should be melded into
> > something else.
> >
>
> Yeah, fair point, though I guess it depends on the particular library.
I mean... is libxfs even a real library? :)
> > > That said, it also seems the semantics of libxfs_flush_devices() are a
> > > bit different from convention. Just below we invoke
> > > libxfs_device_close() for each device (rather than for all three), and
> > > device_close() also happens to call fsync() and platform_flush_device()
> > > itself...
> >
> > Yeah, the division of responsibilities is a little hazy here -- I would
> > think that unmounting a filesystem should flush all the memory caches
> > and then the disk cache, but OTOH it's the utility that opens the
> > devices and should therefore flush and close them.
> >
> > I dunno. My current thinking is that libxfs_umount should call
> > libxfs_flush_devices() and print error messages as necessary, and return
> > error codes as appropriate. xfs_repair can then check the umount return
> > value and translate that into exit(1) as required. The device_close
> > functions will fsync a second time, but that shouldn't be a big deal
> > because we haven't dirtied anything in the meantime.
> >
> > Thoughts?
> >
>
> I was thinking of having a per-device libxfs_device_flush() along the
> lines of libxfs_device_close() and separating out that functionality,
> but one could argue we're also a bit inconsistent between libxfs_init()
> opening the devices and having to close them individually.
Yeah, I don't understand why libxfs_destroy doesn't empty out the same
struct libxfs_init that libxfs_init populates. Or why we have a global
variable named "x", or why the buffer cache is a global variable.
However, those sound like refactoring for another series.
> I think
> having libxfs_umount() do a proper purge -> flush and returning any
> errors instead is a fair tradeoff for simplicity. Removing the
> flush_devices() API also eliminates risk of somebody incorrectly
> attempting the flush after the umount frees the buftarg structures
> (without reinitializing pointers :P).
Ok, I'll add a separate patch to null out the xfs_mount so that any
further use (afaict there aren't any) will crash immediately on reuse.
--D
> Brian
>
> > --D
> >
> > > Brian
> > >
> > > > libxfs_umount(mp);
> > > > if (x.ddev)
> > > > libxfs_device_close(x.ddev);
> > > >
> > >
> >
>
^ permalink raw reply
* Re: [PATCH v2 4/6] NFS: Add READ_PLUS data segment support
From: Anna Schumaker @ 2020-02-20 18:35 UTC (permalink / raw)
To: Chuck Lever; +Cc: Trond.Myklebust, Linux NFS Mailing List
In-Reply-To: <93E71BBA-029B-44AE-B580-0332E157D0A2@oracle.com>
On Thu, 2020-02-20 at 13:30 -0500, Chuck Lever wrote:
> > On Feb 20, 2020, at 1:28 PM, Anna Schumaker <schumaker.anna@gmail.com>
> > wrote:
> >
> > On Thu, 2020-02-20 at 09:55 -0500, Chuck Lever wrote:
> > > > On Feb 20, 2020, at 9:42 AM, Anna Schumaker <schumaker.anna@gmail.com>
> > > > wrote:
> > > >
> > > > On Fri, 2020-02-14 at 17:28 -0500, Chuck Lever wrote:
> > > > > > On Feb 14, 2020, at 4:12 PM, schumaker.anna@gmail.com wrote:
> > > > > >
> > > > > > From: Anna Schumaker <Anna.Schumaker@Netapp.com>
> > > > > >
> > > > > > This patch adds client support for decoding a single
> > > > > > NFS4_CONTENT_DATA
> > > > > > segment returned by the server. This is the simplest implementation
> > > > > > possible, since it does not account for any hole segments in the
> > > > > > reply.
> > > > > >
> > > > > > Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
> > > > > > ---
> > > > > > fs/nfs/nfs42xdr.c | 138
> > > > > > ++++++++++++++++++++++++++++++++++++++
> > > > > > fs/nfs/nfs4proc.c | 43 +++++++++++-
> > > > > > fs/nfs/nfs4xdr.c | 1 +
> > > > > > include/linux/nfs4.h | 2 +-
> > > > > > include/linux/nfs_fs_sb.h | 1 +
> > > > > > include/linux/nfs_xdr.h | 2 +-
> > > > > > 6 files changed, 182 insertions(+), 5 deletions(-)
> > > > > >
> > > > > > diff --git a/fs/nfs/nfs42xdr.c b/fs/nfs/nfs42xdr.c
> > > > > > index c03f3246d6c5..bf118ecabe2c 100644
> > > > > > --- a/fs/nfs/nfs42xdr.c
> > > > > > +++ b/fs/nfs/nfs42xdr.c
> > > > > > @@ -45,6 +45,15 @@
> > > > > > #define encode_deallocate_maxsz (op_encode_hdr_maxsz + \
> > > > > > encode_fallocate_maxsz)
> > > > > > #define decode_deallocate_maxsz (op_decode_hdr_maxsz)
> > > > > > +#define encode_read_plus_maxsz (op_encode_hdr_maxsz + \
> > > > > > + encode_stateid_maxsz + 3)
> > > > > > +#define NFS42_READ_PLUS_SEGMENT_SIZE (1 /* data_content4 */ +
> > > > > > \
> > > > > > + 2 /* data_info4.di_offset */ +
> > > > > > \
> > > > > > + 2 /* data_info4.di_length */)
> > > > > > +#define decode_read_plus_maxsz (op_decode_hdr_maxsz + \
> > > > > > + 1 /* rpr_eof */ + \
> > > > > > + 1 /* rpr_contents count */ + \
> > > > > > + NFS42_READ_PLUS_SEGMENT_SIZE)
> > > > > > #define encode_seek_maxsz (op_encode_hdr_maxsz + \
> > > > > > encode_stateid_maxsz + \
> > > > > > 2 /* offset */ + \
> > > > > > @@ -128,6 +137,14 @@
> > > > > > decode_putfh_maxsz + \
> > > > > > decode_deallocate_maxsz + \
> > > > > > decode_getattr_maxsz)
> > > > > > +#define NFS4_enc_read_plus_sz (compound_encode_hdr_max
> > > > > > sz + \
> > > > > > + encode_sequence_maxsz + \
> > > > > > + encode_putfh_maxsz + \
> > > > > > + encode_read_plus_maxsz)
> > > > > > +#define NFS4_dec_read_plus_sz (compound_decode_hdr_max
> > > > > > sz + \
> > > > > > + decode_sequence_maxsz + \
> > > > > > + decode_putfh_maxsz + \
> > > > > > + decode_read_plus_maxsz)
> > > > > > #define NFS4_enc_seek_sz (compound_encode_hdr_maxsz + \
> > > > > > encode_sequence_maxsz + \
> > > > > > encode_putfh_maxsz + \
> > > > > > @@ -252,6 +269,16 @@ static void encode_deallocate(struct xdr_stream
> > > > > > *xdr,
> > > > > > encode_fallocate(xdr, args);
> > > > > > }
> > > > > >
> > > > > > +static void encode_read_plus(struct xdr_stream *xdr,
> > > > > > + const struct nfs_pgio_args *args,
> > > > > > + struct compound_hdr *hdr)
> > > > > > +{
> > > > > > + encode_op_hdr(xdr, OP_READ_PLUS, decode_read_plus_maxsz, hdr);
> > > > > > + encode_nfs4_stateid(xdr, &args->stateid);
> > > > > > + encode_uint64(xdr, args->offset);
> > > > > > + encode_uint32(xdr, args->count);
> > > > > > +}
> > > > > > +
> > > > > > static void encode_seek(struct xdr_stream *xdr,
> > > > > > const struct nfs42_seek_args *args,
> > > > > > struct compound_hdr *hdr)
> > > > > > @@ -446,6 +473,29 @@ static void nfs4_xdr_enc_deallocate(struct
> > > > > > rpc_rqst
> > > > > > *req,
> > > > > > encode_nops(&hdr);
> > > > > > }
> > > > > >
> > > > > > +/*
> > > > > > + * Encode READ_PLUS request
> > > > > > + */
> > > > > > +static void nfs4_xdr_enc_read_plus(struct rpc_rqst *req,
> > > > > > + struct xdr_stream *xdr,
> > > > > > + const void *data)
> > > > > > +{
> > > > > > + const struct nfs_pgio_args *args = data;
> > > > > > + struct compound_hdr hdr = {
> > > > > > + .minorversion = nfs4_xdr_minorversion(&args->seq_args),
> > > > > > + };
> > > > > > +
> > > > > > + encode_compound_hdr(xdr, req, &hdr);
> > > > > > + encode_sequence(xdr, &args->seq_args, &hdr);
> > > > > > + encode_putfh(xdr, args->fh, &hdr);
> > > > > > + encode_read_plus(xdr, args, &hdr);
> > > > > > +
> > > > > > + rpc_prepare_reply_pages(req, args->pages, args->pgbase,
> > > > > > + args->count, hdr.replen);
> > > > > > + req->rq_rcv_buf.flags |= XDRBUF_READ;
> > > > >
> > > > > IMO this line is incorrect.
> > > >
> > > > You're right, this line causes problems for RDMA with READ_PLUS. I added
> > > > it
> > > > to
> > > > match how the other xdr read encoders were set up
> > >
> > > Ja, I think just removing that line should be sufficient.
> > > Better would be replacing it with a comment explaining
> > > why this encoder does not set XDRBUF_READ. :-)
> > >
> > >
> > > > > RFC 8267 Section 6.1 does not list any part of the result of READ_PLUS
> > > > > as DDP-eligible. There's no way for a client to know how to set up
> > > > > Write chunks, unless it knows exactly where the file's holes are in
> > > > > advance. Even then... racy.
> > > > >
> > > > > Just curious, have you tried READ_PLUS with proto=rdma ?
> > > >
> > > > I haven't done in-depth performance testing, but I have been able to run
> > > > it.
> > >
> > > We should figure out whether that will have a regressive
> > > impact on NFS/RDMA workloads. I expect that it will, but
> > > the client can always set up the Reply chunk so that the
> > > READ payload fits precisely in an RDMA segment that lines
> > > up with page cache pages. That mitigates some impact.
> > >
> > > If your patch set already changes NFSv4.2 mounts to always
> > > use READ_PLUS in place of READ, it might be prudent for the
> > > "proto=rdma" mount option to also set "noreadplus", at least
> > > for the time being.
> >
> > I can make this change.
> >
> > > The down-side here is that would make NFSv4.2 on RDMA
> > > unable to recognize holes in files the same way as it
> > > does on TCP, and that's a pretty significant variation
> > > in behavior. Does "noreadplus" even deal with that?
> >
> > Setting "noreadplus" just causes the client to use the READ operation
> > instead,
> > so there should be no difference between v4.1 and v4.2 if the option is set.
>
> My concern is the difference between NFSv4.2 with noreadplus
> and NFSv4.2 with readplus. The former is not able to detect
> holes in files on the server, but the latter is.
The client could still use lseek to detect holes. The client throws away the
hole information after xdr decoding, and zeroes out the corresponding pages for
the page cache.
>
> Is that worth mentioning in the man page, or in release notes
> when NFSv4.2 becomes the default?
>
>
> > Anna
> >
> > >
> > > > Anna
> > > >
> > > > > > + encode_nops(&hdr);
> > > > > > +}
> > > > > > +
> > > > > > /*
> > > > > > * Encode SEEK request
> > > > > > */
> > > > > > @@ -694,6 +744,67 @@ static int decode_deallocate(struct xdr_stream
> > > > > > *xdr,
> > > > > > struct nfs42_falloc_res *re
> > > > > > return decode_op_hdr(xdr, OP_DEALLOCATE);
> > > > > > }
> > > > > >
> > > > > > +static uint32_t decode_read_plus_data(struct xdr_stream *xdr,
> > > > > > struct
> > > > > > nfs_pgio_res *res,
> > > > > > + uint32_t *eof)
> > > > > > +{
> > > > > > + __be32 *p;
> > > > > > + uint32_t count, recvd;
> > > > > > + uint64_t offset;
> > > > > > +
> > > > > > + p = xdr_inline_decode(xdr, 8 + 4);
> > > > > > + if (unlikely(!p))
> > > > > > + return -EIO;
> > > > > > +
> > > > > > + p = xdr_decode_hyper(p, &offset);
> > > > > > + count = be32_to_cpup(p);
> > > > > > + if (count == 0)
> > > > > > + return 0;
> > > > > > +
> > > > > > + recvd = xdr_read_pages(xdr, count);
> > > > > > + if (count > recvd) {
> > > > > > + dprintk("NFS: server cheating in read reply: "
> > > > > > + "count %u > recvd %u\n", count, recvd);
> > > > > > + count = recvd;
> > > > > > + *eof = 0;
> > > > > > + }
> > > > > > +
> > > > > > + return count;
> > > > > > +}
> > > > > > +
> > > > > > +static int decode_read_plus(struct xdr_stream *xdr, struct
> > > > > > nfs_pgio_res
> > > > > > *res)
> > > > > > +{
> > > > > > + __be32 *p;
> > > > > > + uint32_t count, eof, segments, type;
> > > > > > + int status;
> > > > > > +
> > > > > > + status = decode_op_hdr(xdr, OP_READ_PLUS);
> > > > > > + if (status)
> > > > > > + return status;
> > > > > > +
> > > > > > + p = xdr_inline_decode(xdr, 4 + 4);
> > > > > > + if (unlikely(!p))
> > > > > > + return -EIO;
> > > > > > +
> > > > > > + eof = be32_to_cpup(p++);
> > > > > > + segments = be32_to_cpup(p++);
> > > > > > + if (segments == 0)
> > > > > > + return 0;
> > > > > > +
> > > > > > + p = xdr_inline_decode(xdr, 4);
> > > > > > + if (unlikely(!p))
> > > > > > + return -EIO;
> > > > > > +
> > > > > > + type = be32_to_cpup(p++);
> > > > > > + if (type == NFS4_CONTENT_DATA)
> > > > > > + count = decode_read_plus_data(xdr, res, &eof);
> > > > > > + else
> > > > > > + return -EINVAL;
> > > > > > +
> > > > > > + res->eof = eof;
> > > > > > + res->count = count;
> > > > > > + return 0;
> > > > > > +}
> > > > > > +
> > > > > > static int decode_seek(struct xdr_stream *xdr, struct nfs42_seek_res
> > > > > > *res)
> > > > > > {
> > > > > > int status;
> > > > > > @@ -870,6 +981,33 @@ static int nfs4_xdr_dec_deallocate(struct
> > > > > > rpc_rqst
> > > > > > *rqstp,
> > > > > > return status;
> > > > > > }
> > > > > >
> > > > > > +/*
> > > > > > + * Decode READ_PLUS request
> > > > > > + */
> > > > > > +static int nfs4_xdr_dec_read_plus(struct rpc_rqst *rqstp,
> > > > > > + struct xdr_stream *xdr,
> > > > > > + void *data)
> > > > > > +{
> > > > > > + struct nfs_pgio_res *res = data;
> > > > > > + struct compound_hdr hdr;
> > > > > > + int status;
> > > > > > +
> > > > > > + status = decode_compound_hdr(xdr, &hdr);
> > > > > > + if (status)
> > > > > > + goto out;
> > > > > > + status = decode_sequence(xdr, &res->seq_res, rqstp);
> > > > > > + if (status)
> > > > > > + goto out;
> > > > > > + status = decode_putfh(xdr);
> > > > > > + if (status)
> > > > > > + goto out;
> > > > > > + status = decode_read_plus(xdr, res);
> > > > > > + if (!status)
> > > > > > + status = res->count;
> > > > > > +out:
> > > > > > + return status;
> > > > > > +}
> > > > > > +
> > > > > > /*
> > > > > > * Decode SEEK request
> > > > > > */
> > > > > > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> > > > > > index 95d07a3dc5d1..ed3ec8c36273 100644
> > > > > > --- a/fs/nfs/nfs4proc.c
> > > > > > +++ b/fs/nfs/nfs4proc.c
> > > > > > @@ -69,6 +69,10 @@
> > > > > >
> > > > > > #include "nfs4trace.h"
> > > > > >
> > > > > > +#ifdef CONFIG_NFS_V4_2
> > > > > > +#include "nfs42.h"
> > > > > > +#endif /* CONFIG_NFS_V4_2 */
> > > > > > +
> > > > > > #define NFSDBG_FACILITY NFSDBG_PROC
> > > > > >
> > > > > > #define NFS4_BITMASK_SZ 3
> > > > > > @@ -5199,28 +5203,60 @@ static bool nfs4_read_stateid_changed(struct
> > > > > > rpc_task *task,
> > > > > > return true;
> > > > > > }
> > > > > >
> > > > > > +static bool nfs4_read_plus_not_supported(struct rpc_task *task,
> > > > > > + struct nfs_pgio_header *hdr)
> > > > > > +{
> > > > > > + struct nfs_server *server = NFS_SERVER(hdr->inode);
> > > > > > + struct rpc_message *msg = &task->tk_msg;
> > > > > > +
> > > > > > + if (msg->rpc_proc == &nfs4_procedures[NFSPROC4_CLNT_READ_PLUS]
> > > > > > &&
> > > > > > + server->caps & NFS_CAP_READ_PLUS && task->tk_status ==
> > > > > > -ENOTSUPP) {
> > > > > > + server->caps &= ~NFS_CAP_READ_PLUS;
> > > > > > + msg->rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_READ];
> > > > > > + rpc_restart_call_prepare(task);
> > > > > > + return true;
> > > > > > + }
> > > > > > + return false;
> > > > > > +}
> > > > > > +
> > > > > > static int nfs4_read_done(struct rpc_task *task, struct
> > > > > > nfs_pgio_header
> > > > > > *hdr)
> > > > > > {
> > > > > > -
> > > > > > dprintk("--> %s\n", __func__);
> > > > > >
> > > > > > if (!nfs4_sequence_done(task, &hdr->res.seq_res))
> > > > > > return -EAGAIN;
> > > > > > if (nfs4_read_stateid_changed(task, &hdr->args))
> > > > > > return -EAGAIN;
> > > > > > + if (nfs4_read_plus_not_supported(task, hdr))
> > > > > > + return -EAGAIN;
> > > > > > if (task->tk_status > 0)
> > > > > > nfs_invalidate_atime(hdr->inode);
> > > > > > return hdr->pgio_done_cb ? hdr->pgio_done_cb(task, hdr) :
> > > > > > nfs4_read_done_cb(task, hdr);
> > > > > > }
> > > > > >
> > > > > > +#ifdef CONFIG_NFS_V4_2
> > > > > > +static void nfs42_read_plus_support(struct nfs_server *server,
> > > > > > struct
> > > > > > rpc_message *msg)
> > > > > > +{
> > > > > > + if (server->caps & NFS_CAP_READ_PLUS)
> > > > > > + msg->rpc_proc =
> > > > > > &nfs4_procedures[NFSPROC4_CLNT_READ_PLUS];
> > > > > > + else
> > > > > > + msg->rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_READ];
> > > > > > +}
> > > > > > +#else
> > > > > > +static void nfs42_read_plus_support(struct nfs_server *server,
> > > > > > struct
> > > > > > rpc_message *msg)
> > > > > > +{
> > > > > > + msg->rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_READ];
> > > > > > +}
> > > > > > +#endif /* CONFIG_NFS_V4_2 */
> > > > > > +
> > > > > > static void nfs4_proc_read_setup(struct nfs_pgio_header *hdr,
> > > > > > struct rpc_message *msg)
> > > > > > {
> > > > > > hdr->timestamp = jiffies;
> > > > > > if (!hdr->pgio_done_cb)
> > > > > > hdr->pgio_done_cb = nfs4_read_done_cb;
> > > > > > - msg->rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_READ];
> > > > > > + nfs42_read_plus_support(NFS_SERVER(hdr->inode), msg);
> > > > > > nfs4_init_sequence(&hdr->args.seq_args, &hdr->res.seq_res, 0,
> > > > > > 0);
> > > > > > }
> > > > > >
> > > > > > @@ -9970,7 +10006,8 @@ static const struct nfs4_minor_version_ops
> > > > > > nfs_v4_2_minor_ops = {
> > > > > > | NFS_CAP_SEEK
> > > > > > | NFS_CAP_LAYOUTSTATS
> > > > > > | NFS_CAP_CLONE
> > > > > > - | NFS_CAP_LAYOUTERROR,
> > > > > > + | NFS_CAP_LAYOUTERROR
> > > > > > + | NFS_CAP_READ_PLUS,
> > > > > > .init_client = nfs41_init_client,
> > > > > > .shutdown_client = nfs41_shutdown_client,
> > > > > > .match_stateid = nfs41_match_stateid,
> > > > > > diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
> > > > > > index 47817ef0aadb..68b2917d0537 100644
> > > > > > --- a/fs/nfs/nfs4xdr.c
> > > > > > +++ b/fs/nfs/nfs4xdr.c
> > > > > > @@ -7584,6 +7584,7 @@ const struct rpc_procinfo nfs4_procedures[] =
> > > > > > {
> > > > > > PROC42(COPY_NOTIFY, enc_copy_notify, dec_copy_notify)
> > > > > > ,
> > > > > > PROC(LOOKUPP, enc_lookupp, dec_lookupp)
> > > > > > ,
> > > > > > PROC42(LAYOUTERROR, enc_layouterror, dec_layouterror)
> > > > > > ,
> > > > > > + PROC42(READ_PLUS, enc_read_plus, dec_read_plus),
> > > > > > };
> > > > > >
> > > > > > static unsigned int
> > > > > > nfs_version4_counts[ARRAY_SIZE(nfs4_procedures)];
> > > > > > diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
> > > > > > index 82d8fb422092..c1eeef52545c 100644
> > > > > > --- a/include/linux/nfs4.h
> > > > > > +++ b/include/linux/nfs4.h
> > > > > > @@ -540,8 +540,8 @@ enum {
> > > > > >
> > > > > > NFSPROC4_CLNT_LOOKUPP,
> > > > > > NFSPROC4_CLNT_LAYOUTERROR,
> > > > > > -
> > > > > > NFSPROC4_CLNT_COPY_NOTIFY,
> > > > > > + NFSPROC4_CLNT_READ_PLUS,
> > > > > > };
> > > > > >
> > > > > > /* nfs41 types */
> > > > > > diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
> > > > > > index 465fa98258a3..11248c5a7b24 100644
> > > > > > --- a/include/linux/nfs_fs_sb.h
> > > > > > +++ b/include/linux/nfs_fs_sb.h
> > > > > > @@ -281,5 +281,6 @@ struct nfs_server {
> > > > > > #define NFS_CAP_OFFLOAD_CANCEL (1U << 25)
> > > > > > #define NFS_CAP_LAYOUTERROR (1U << 26)
> > > > > > #define NFS_CAP_COPY_NOTIFY (1U << 27)
> > > > > > +#define NFS_CAP_READ_PLUS (1U << 28)
> > > > > >
> > > > > > #endif
> > > > > > diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> > > > > > index 94c77ed55ce1..8efbf3d8b263 100644
> > > > > > --- a/include/linux/nfs_xdr.h
> > > > > > +++ b/include/linux/nfs_xdr.h
> > > > > > @@ -655,7 +655,7 @@ struct nfs_pgio_args {
> > > > > > struct nfs_pgio_res {
> > > > > > struct nfs4_sequence_res seq_res;
> > > > > > struct nfs_fattr * fattr;
> > > > > > - __u32 count;
> > > > > > + __u64 count;
> > > > > > __u32 op_status;
> > > > > > union {
> > > > > > struct {
> > > > > > --
> > > > > > 2.25.0
> > > > > >
> > > > >
> > > > > --
> > > > > Chuck Lever
> > >
> > > --
> > > Chuck Lever
>
> --
> Chuck Lever
>
>
>
^ permalink raw reply
* Re: [PATCH v3 2/3] mm: Charge active memcg when no mm is set
From: Chris Down @ 2020-02-20 18:35 UTC (permalink / raw)
To: Dan Schatzberg
Cc: Jens Axboe, Tejun Heo, Li Zefan, Johannes Weiner, Michal Hocko,
Vladimir Davydov, Andrew Morton, Hugh Dickins, Roman Gushchin,
Shakeel Butt, Yang Shi, Thomas Gleixner, open list:BLOCK LAYER,
open list, open list:CONTROL GROUP (CGROUP),
open list:CONTROL GROUP - MEMORY RESOURCE CONTROLLER (MEMCG)
In-Reply-To: <0a27b6fcbd1f7af104d7f4cf0adc6a31e0e7dd19.1582216294.git.schatzberg.dan@gmail.com>
Dan Schatzberg writes:
>memalloc_use_memcg() worked for kernel allocations but was silently
>ignored for user pages.
>
>This patch establishes a precedence order for who gets charged:
>
>1. If there is a memcg associated with the page already, that memcg is
> charged. This happens during swapin.
>
>2. If an explicit mm is passed, mm->memcg is charged. This happens
> during page faults, which can be triggered in remote VMs (eg gup).
>
>3. Otherwise consult the current process context. If it has configured
> a current->active_memcg, use that. Otherwise, current->mm->memcg.
>
>Previously, if a NULL mm was passed to mem_cgroup_try_charge (case 3) it
>would always charge the root cgroup. Now it looks up the current
>active_memcg first (falling back to charging the root cgroup if not
>set).
>
>Signed-off-by: Dan Schatzberg <schatzberg.dan@gmail.com>
>Acked-by: Johannes Weiner <hannes@cmpxchg.org>
>Acked-by: Tejun Heo <tj@kernel.org>
Acked-by: Chris Down <chris@chrisdown.name>
Thanks! The clarification the v2 thread for this made things clear to me.
^ permalink raw reply
* Re: drm_dp_mst_topology.c and old compilers
From: Paul E. McKenney @ 2020-02-20 18:36 UTC (permalink / raw)
To: Chris Wilson; +Cc: Dave Airlie, LKML, Maling list - DRI developers
In-Reply-To: <158218553821.8112.10047864129562395990@skylake-alporthouse-com>
On Thu, Feb 20, 2020 at 07:58:58AM +0000, Chris Wilson wrote:
> Quoting Alex Deucher (2020-02-20 02:52:32)
> > On Wed, Feb 19, 2020 at 7:42 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> > >
> > > Hello!
> > >
> > > A box with GCC 4.8.3 compiler didn't like drm_dp_mst_topology.c. The
> > > following (lightly tested) patch makes it happy and seems OK for newer
> > > compilers as well.
> > >
> > > Is this of interest?
> >
> > How about a memset instead? That should be consistent across compilers.
>
> The kernel has adopted the gccism: struct drm_dp_desc desc = {};
> git grep '= {}' | wc -l: 2046
> git grep '= { }' | wc -l: 694
> -Chris
And this works well, a big "thank you!" to all three of you!
Please see below for the updated patch.
Thanx, Paul
------------------------------------------------------------------------
commit 78c0e53a98a9772a99e46806f8fcbe1140d667a4
Author: Paul E. McKenney <paulmck@kernel.org>
Date: Wed Feb 19 16:42:47 2020 -0800
EXP drm: Make drm_dp_mst_dsc_aux_for_port() safe for old compilers
Older compilers either want two extra pairs of curly braces around the
initializer for local variable desc, or they want a single pair of curly
braces with nothing inside. Current Linux-kernel practice favors the
latter, so this commit makes it so.
Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Suggested-by: Joe Perches <joe@perches.com>
Suggested-by: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 20cdaf3..b123f60 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -5396,7 +5396,7 @@ struct drm_dp_aux *drm_dp_mst_dsc_aux_for_port(struct drm_dp_mst_port *port)
{
struct drm_dp_mst_port *immediate_upstream_port;
struct drm_dp_mst_port *fec_port;
- struct drm_dp_desc desc = { 0 };
+ struct drm_dp_desc desc = { };
u8 endpoint_fec;
u8 endpoint_dsc;
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related
* Re: drm_dp_mst_topology.c and old compilers
From: Paul E. McKenney @ 2020-02-20 18:36 UTC (permalink / raw)
To: Chris Wilson
Cc: Alex Deucher, Dave Airlie, LKML, Maling list - DRI developers
In-Reply-To: <158218553821.8112.10047864129562395990@skylake-alporthouse-com>
On Thu, Feb 20, 2020 at 07:58:58AM +0000, Chris Wilson wrote:
> Quoting Alex Deucher (2020-02-20 02:52:32)
> > On Wed, Feb 19, 2020 at 7:42 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> > >
> > > Hello!
> > >
> > > A box with GCC 4.8.3 compiler didn't like drm_dp_mst_topology.c. The
> > > following (lightly tested) patch makes it happy and seems OK for newer
> > > compilers as well.
> > >
> > > Is this of interest?
> >
> > How about a memset instead? That should be consistent across compilers.
>
> The kernel has adopted the gccism: struct drm_dp_desc desc = {};
> git grep '= {}' | wc -l: 2046
> git grep '= { }' | wc -l: 694
> -Chris
And this works well, a big "thank you!" to all three of you!
Please see below for the updated patch.
Thanx, Paul
------------------------------------------------------------------------
commit 78c0e53a98a9772a99e46806f8fcbe1140d667a4
Author: Paul E. McKenney <paulmck@kernel.org>
Date: Wed Feb 19 16:42:47 2020 -0800
EXP drm: Make drm_dp_mst_dsc_aux_for_port() safe for old compilers
Older compilers either want two extra pairs of curly braces around the
initializer for local variable desc, or they want a single pair of curly
braces with nothing inside. Current Linux-kernel practice favors the
latter, so this commit makes it so.
Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Suggested-by: Joe Perches <joe@perches.com>
Suggested-by: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 20cdaf3..b123f60 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -5396,7 +5396,7 @@ struct drm_dp_aux *drm_dp_mst_dsc_aux_for_port(struct drm_dp_mst_port *port)
{
struct drm_dp_mst_port *immediate_upstream_port;
struct drm_dp_mst_port *fec_port;
- struct drm_dp_desc desc = { 0 };
+ struct drm_dp_desc desc = { };
u8 endpoint_fec;
u8 endpoint_dsc;
^ permalink raw reply related
* Re: [PATCH v6 1/2] mm: Add MREMAP_DONTUNMAP to mremap().
From: Brian Geffon @ 2020-02-20 18:36 UTC (permalink / raw)
To: Minchan Kim
Cc: Andrew Morton, Michael S . Tsirkin, Arnd Bergmann, LKML, linux-mm,
Linux API, Andy Lutomirski, Will Deacon, Andrea Arcangeli,
Sonny Rao, Joel Fernandes, Yu Zhao, Jesse Barnes, Florian Weimer,
Kirill A . Shutemov
In-Reply-To: <20200220171554.GA44866@google.com>
Hi Minchan,
> And here we got error if the addr is in non-anonymous-private vma so the
> syscall will fail but old vma is gone? I guess it's not your intention?
This is exactly what happens today in several situations, because
vma_to_resize is called unconditionally. For example if the old vma
has VM_HUGETLB and old_len < new_len it would have unmapped a portion
and then in vma_to_resize returned -EINVAL, similarly when old_len = 0
with a non-sharable mapping it will have called do_munmap only to fail
in vma_to_resize, if the vma has VM_DONTEXPAND set and you shrink the
size with old_len < new_len it would return -EFAULT after having done
the unmap on the decreased portion. So I followed the pattern to keep
the change simple and maintain consistency with existing behavior.
But with that being said, Kirill made the point that resizing a VMA
while also using MREMAP_DONTUNMAP doesn't have any clear use case and
I agree with that, I'm unable to think of a situation where you'd want
to resize a VMA and use MREMAP_DONTUNMAP. So I'm tempted to mail a new
version which returns -EINVAL if old_len != new_len that would resolve
this concern here as nothing would be unmapped ever at the old
position add it would clean up the change to very few lines of code.
What do you think?
Thank you for taking the time to review.
Brian
^ permalink raw reply
* Re: [PATCH v2 0/6] Support Argon2 KDF in LUKS2
From: Leif Lindholm @ 2020-02-20 18:38 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: grub-devel, Daniel Kiper, gmazyland, agraf, pjones, mjg59,
phcoder
In-Reply-To: <cover.1582221462.git.ps@pks.im>
Hi Patrick,
On Thu, Feb 20, 2020 at 19:00:48 +0100, Patrick Steinhardt wrote:
> this is the second version of my patchset to add support for Argon2
> encryption keys for LUKS2.
>
> The most important change is that I've now verbosely imported the argon2
> code from the official reference implementation instead of from the
> cryptsetup project. The diff between both isn't that big in the end, and
> including from crypsetup's upstream seems a bit cleaner to me. There
> were several transformations required to use GRUB's types and functions
> as well as stripping of unused stuff, which I've now documented the dev
> manual. This also fixes my previously mistaken license headers.
>
> One thing I'm not sure about here is whether it's fine to declare the
> argon2 mod's license as GPLv3. The code is licensed under CC0/Apache
> 2.0, where the latter is compatible with GPLv3. But I don't know whether
> it's legit to just say "Yeah, this mod is a GPLv3 one".
>
> I didn't address the comment made by Leif yet with regards to grabbing
> memory. I ain't got much of a clue of GRUB's memory subsystem, so I'd
> gladly accept help there. Otherwise I'll have to dig a bit deeper.
That's fair enough. I think we could do something halfway clever to
resolve that, or we could do something quick and simple, but either
would be better than moving to reserving 50%.
So could you reply to my email on that thread with some info with
regards to the specific memory requirements, and whether they are
precise or "this much seems to always work"?
Regards,
Leif
> The range diff compared to the previous version of this patch set is
> attached to this mail.
>
> Patrick
>
>
> Patrick Steinhardt (6):
> efi: Allocate half of available memory by default
> types.h: add UINT-related macros needed for Argon2
> argon2: Import Argon2 from cryptsetup
> luks2: Add missing newline to debug message
> luks2: Discern Argon2i and Argon2id
> luks2: Support key derival via Argon2
>
> Makefile.util.def | 6 +-
> docs/grub-dev.texi | 64 +++
> grub-core/Makefile.core.def | 10 +-
> grub-core/disk/luks2.c | 28 +-
> grub-core/kern/efi/mm.c | 4 +-
> grub-core/lib/argon2/argon2.c | 232 ++++++++
> grub-core/lib/argon2/argon2.h | 264 +++++++++
> grub-core/lib/argon2/blake2/blake2-impl.h | 151 +++++
> grub-core/lib/argon2/blake2/blake2.h | 89 +++
> grub-core/lib/argon2/blake2/blake2b.c | 388 +++++++++++++
> .../lib/argon2/blake2/blamka-round-ref.h | 56 ++
> grub-core/lib/argon2/core.c | 525 ++++++++++++++++++
> grub-core/lib/argon2/core.h | 228 ++++++++
> grub-core/lib/argon2/ref.c | 190 +++++++
> include/grub/types.h | 8 +
> 15 files changed, 2231 insertions(+), 12 deletions(-)
> create mode 100644 grub-core/lib/argon2/argon2.c
> create mode 100644 grub-core/lib/argon2/argon2.h
> create mode 100644 grub-core/lib/argon2/blake2/blake2-impl.h
> create mode 100644 grub-core/lib/argon2/blake2/blake2.h
> create mode 100644 grub-core/lib/argon2/blake2/blake2b.c
> create mode 100644 grub-core/lib/argon2/blake2/blamka-round-ref.h
> create mode 100644 grub-core/lib/argon2/core.c
> create mode 100644 grub-core/lib/argon2/core.h
> create mode 100644 grub-core/lib/argon2/ref.c
>
> Range-diff against v1:
> 1: 53cdfdc27 = 1: 15bdf830e efi: Allocate half of available memory by default
> 2: c55946ca5 < -: --------- argon2: Import Argon2 from cryptsetup
> -: --------- > 2: e81db7d95 types.h: add UINT-related macros needed for Argon2
> -: --------- > 3: 50aff9670 argon2: Import Argon2 from cryptsetup
> 3: c17cd2197 ! 4: af3f85665 disk: luks2: Add missing newline to debug message
> @@ Metadata
> Author: Patrick Steinhardt <ps@pks.im>
>
> ## Commit message ##
> - disk: luks2: Add missing newline to debug message
> + luks2: Add missing newline to debug message
>
> The debug message printed when decryption with a keyslot fails is
> missing its trailing newline. Add it to avoid mangling it with
> subsequent output.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> + Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
>
> ## grub-core/disk/luks2.c ##
> @@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t disk,
> 4: 390728cea ! 5: 89abe827b disk: luks2: Discern Argon2i and Argon2id
> @@ Metadata
> Author: Patrick Steinhardt <ps@pks.im>
>
> ## Commit message ##
> - disk: luks2: Discern Argon2i and Argon2id
> + luks2: Discern Argon2i and Argon2id
>
> While GRUB is already able to parse both Argon2i and Argon2id parameters
> from the LUKS2 header, it doesn't discern both types. This commit
> 5: ec4389627 ! 6: 70a354e0b disk: luks2: Support key derival via Argon2
> @@ Metadata
> Author: Patrick Steinhardt <ps@pks.im>
>
> ## Commit message ##
> - disk: luks2: Support key derival via Argon2
> + luks2: Support key derival via Argon2
>
> One addition with LUKS2 was support of the key derival function Argon2
> in addition to the previously supported PBKDF2 algortihm. In order to
> @@ Makefile.util.def: library = {
> common = grub-core/kern/partition.c;
> common = grub-core/lib/crypto.c;
> + common = grub-core/lib/argon2/argon2.c;
> ++ common = grub-core/lib/argon2/core.c;
> ++ common = grub-core/lib/argon2/ref.c;
> + common = grub-core/lib/argon2/blake2/blake2b.c;
> common = grub-core/lib/json/json.c;
> common = grub-core/disk/luks.c;
> @@ grub-core/disk/luks2.c: luks2_decrypt_key (grub_uint8_t *out_key,
> case LUKS2_KDF_TYPE_ARGON2ID:
> - ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Argon2 not supported");
> - goto err;
> -+ ret = grub_crypto_argon2 (passphrase, passphraselen, salt, saltlen,
> -+ k->kdf.u.argon2.time, k->kdf.u.argon2.memory, k->kdf.u.argon2.cpus,
> -+ k->kdf.type == LUKS2_KDF_TYPE_ARGON2I ? GRUB_ARGON2_I : GRUB_ARGON2_ID,
> -+ GRUB_ARGON2_VERSION_NUMBER,
> -+ area_key, k->area.key_size);
> ++ ret = argon2_hash (k->kdf.u.argon2.time, k->kdf.u.argon2.memory, k->kdf.u.argon2.cpus,
> ++ passphrase, passphraselen, salt, saltlen, area_key, k->area.key_size,
> ++ k->kdf.type == LUKS2_KDF_TYPE_ARGON2I ? Argon2_i : Argon2_id,
> ++ ARGON2_VERSION_NUMBER);
> + if (ret)
> + {
> -+ grub_dprintf ("luks2", "Argon2 failed: %s\n", grub_errmsg);
> ++ grub_dprintf ("luks2", "Argon2 failed: %s\n", argon2_error_message (ret));
> + goto err;
> + }
> + break;
> --
> 2.25.1
>
^ permalink raw reply
* Re: [External] Re: [PATCH] thinkpad_acpi: Add sysfs entry for lcdshadow feature
From: Rajat Jain @ 2020-02-20 18:39 UTC (permalink / raw)
To: Mark Pearson
Cc: Andy Shevchenko, Nitin Joshi, Mat King, Jani Nikula,
Daniel Thompson, Jingoo Han, Henrique de Moraes Holschuh,
Darren Hart, Andy Shevchenko, Thinkpad-acpi devel ML,
Platform Driver, Nitin Joshi1, Benjamin Berg,
Linux Kernel Mailing List, dri-devel, Greg Kroah-Hartman
In-Reply-To: <HK2PR0302MB25937E2946BF38583B3A905DBD130@HK2PR0302MB2593.apcprd03.prod.outlook.com>
Hi Mark,
On Thu, Feb 20, 2020 at 7:14 AM Mark Pearson <mpearson@lenovo.com> wrote:
>
> Hi Andy
>
> > -----Original Message-----
> > From: Andy Shevchenko <andy.shevchenko@gmail.com>
> > Sent: Thursday, February 20, 2020 5:43 AM
> >
> > On Thu, Feb 20, 2020 at 9:48 AM Nitin Joshi <nitjoshi@gmail.com> wrote:
> > >
> > > This feature is supported on some Thinkpad products like T490s, Thinkpad
> > > X1 yoga 4th Gen etc . The lcdshadow feature can be enabled and disabled
> > > when user press "Fn" + "D" key. Currently, no user feedback is given for
> > > this action. Adding as sysfs entry allows userspace to show an On Screen
> > > Display whenever the setting changes.
> > >
> > > Summary of changes is mentioned below :
> > >
> > > - Added TP_HKEY_EV_LCDSHADOW_CHANGED for consistency inside the
> > driver
> > > - Added unmapped LCDSHADOW to keymap
> > > - Added lcdshadow_get function to read value using ACPI
> > > - Added lcdshadow_refresh function to re-read value and send notification
> > > - Added sysfs group creation to tpaci_lcdshadow_init
> > > - Added lcdshadow_exit to remove sysfs group again
> > > - Implemented lcdshadow_enable_show/lcdshadow_enable_store
> > > - Added handler to tpacpi_driver_event to update refresh lcdshadow
> > > - Explicitly call tpacpi_driver_event for extended keyset
> >
> > Adding custom PrivacyGuard support to this driver was my mistake,
> > There is a discussion [1] how to do this in generic way to cover other
> > possible users.
> > I Cc this to people from that discussion.
> >
> > [1]: https://lore.kernel.org/dri-
> > devel/CAL_quvRknSSVvXN3q_Se0hrziw2oTNS3ENNoeHYhvciCRq9Yww@mail
> > .gmail.com/
> >
> Thanks for the pointer to that thread - really useful and interesting, we weren't aware there was an ongoing exercise to do this.
>
> I work with Nitin as part of the Linux team at Lenovo. We're trying to get more directly and actively involved in the open source community to improve the Linux experience on Lenovo devices and of course want to make sure we contribute the right way. We're all still pretty new so pointers and help are very much appreciated (we've been getting some great support from the distros to get us started).
>
> For this particular issue what is the best way to contribute and get involved? We'd like to make it so ePrivacy can be used more easily from Linux. I agree a more generic way of controlling it would be good.
> I looked at the proposed patch from Rajat (https://lkml.org/lkml/2019/10/22/967) - it seems like a good solution to me. We can help with testing that on our platforms if that would be useful.
Thanks you, just so that you know, the latest patchset is at:
https://lkml.org/lkml/2019/12/20/794
It would be great to get some additional testing if possible. I can
send a sample ACPI (for our platform) in case it helps.
>
> I need to understand how we connect that implementation with the ACPI controls we have (as I believe what we have are thinkpad specific and not to a drm spec; we need to confirm that). We also have the ACPI events that notify if ePrivacy was changed by the hotkeys and that seems like something that should be done in thinkpad_acpi.c and not the drm code.
Not sure if the two need to be connected somehow (or if handling the
event is actually not important and polling is acceptable)?
So there was some brief discussion about this on my patches - but
atleast on the platforms I have seen, there was no way to change the
privacy screen out of software / kernel control. Essentially, if there
are hotkeys, they would send an input event to the kernel, who'd send
them to userspace, who'd use the DRM method to toggle the privacy
screen. Thus the current version of the patch only supports
controlling the privacy screen via set() method. The get() method just
returns the cached value.I hope that works for you.
Jani, I'm waiting on your inputs here
https://lkml.org/lkml/2020/1/24/1932 in order to send the next
iteration of my patch. Can you please let me know if you have any
comments.
Thanks & Best Regards,
Rajat
>
> As a note Nitin has been working with the Red Hat folk and is looking at the user space aspect of this (in particularl gnome settings) as well.
>
> Thanks
> Mark Pearson
^ permalink raw reply
* Re: [Bug] virtio-gpu broken with qemu/kvm on arm64 on kernel 5.5+
From: Chia-I Wu @ 2020-02-20 18:40 UTC (permalink / raw)
To: Guillaume Gardet
Cc: Daniel Vetter, nd, Gerd Hoffmann, dri-devel@lists.freedesktop.org,
Catalin Marinas
In-Reply-To: <VI1PR0802MB22374AD8B80182D2F186ABB483130@VI1PR0802MB2237.eurprd08.prod.outlook.com>
On Thu, Feb 20, 2020 at 4:44 AM Guillaume Gardet
<Guillaume.Gardet@arm.com> wrote:
>
> Hi,
>
> With (guest) kernel 5.5+ with qemu/kvm on arm64, I get lots of display corruptions leading to this kind of screen:
> https://openqa.opensuse.org/tests/1174521#step/yast2_i/24
Looking at the screenshot, it seems cacheline-related?
There was a change of memory type
https://lists.freedesktop.org/archives/dri-devel/2019-August/233456.html
While the guest memory type is ignored on Intel, it is honored on ARM.
This attempt to fix it
https://lists.freedesktop.org/archives/dri-devel/2019-December/248271.html
does not seem to land.
>
> I git bisected it to commit c66df701e783bc666593e6e665f13670760883ee
> **********
> drm/virtio: switch from ttm to gem shmem helpers
>
> virtio-gpu basically needs a sg_table for the bo, to tell the host where
> the backing pages for the object are. So the gem shmem helpers are a
> perfect fit. Some drm_gem_object_funcs need thin wrappers to update the
> host state, but otherwise the helpers handle everything just fine.
>
> Once the fencing was sorted the switch was surprisingly easy and for the
> most part just removing the ttm code.
>
> v4: fix drm_gem_object_funcs name.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Reviewed-by: Chia-I Wu <olvaffe@gmail.com>
> Link: http://patchwork.freedesktop.org/patch/msgid/20190829103301.3539-15-kraxel@redhat.com
> **********
>
> I also tested kernel 5.6-rc2 which has the same bug.
> Without kvm, the display is fine.
>
> Regards,
> Guillaume
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* [cifs:for-next 4/6] fs/cifs/smb2pdu.h:1616:1: error: expected identifier or '(' before '}' token
From: kbuild test robot @ 2020-02-20 18:41 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 2319 bytes --]
tree: git://git.samba.org/sfrench/cifs-2.6.git for-next
head: 93fc7de81fe90b31c3f99fb86dcc79649ef028b9
commit: 86eac7701750ff65108ecf8022514dbcedaa080d [4/6] cifs: rename posix create rsp
config: powerpc-defconfig (attached as .config)
compiler: powerpc64-linux-gcc (GCC) 7.5.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
git checkout 86eac7701750ff65108ecf8022514dbcedaa080d
# save the attached .config to linux build tree
GCC_VERSION=7.5.0 make.cross ARCH=powerpc
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
Note: the cifs/for-next HEAD 93fc7de81fe90b31c3f99fb86dcc79649ef028b9 builds fine.
It only hurts bisectibility.
All errors (new ones prefixed by >>):
In file included from fs/cifs/cifsglob.h:32:0,
from fs/cifs/cifsfs.c:46:
>> fs/cifs/smb2pdu.h:1616:1: error: expected identifier or '(' before '}' token
};
^
vim +1616 fs/cifs/smb2pdu.h
730928c8f4be88 Ronnie Sahlberg 2018-08-08 1605
ab3459d8f0ef52 Steve French 2020-02-06 1606 /* equivalent of the contents of SMB3.1.1 POSIX open context response */
86eac7701750ff Aurelien Aptel 2020-02-08 1607 struct create_posix_rsp {
ab3459d8f0ef52 Steve French 2020-02-06 1608 __le32 nlink;
ab3459d8f0ef52 Steve French 2020-02-06 1609 __le32 reparse_tag;
ab3459d8f0ef52 Steve French 2020-02-06 1610 __le32 mode;
86eac7701750ff Aurelien Aptel 2020-02-08 1611 /*
86eac7701750ff Aurelien Aptel 2020-02-08 1612 * var sized owner SID
86eac7701750ff Aurelien Aptel 2020-02-08 1613 * var sized group SID
86eac7701750ff Aurelien Aptel 2020-02-08 1614 */
86eac7701750ff Aurelien Aptel 2020-02-08 1615 } __packed;
ab3459d8f0ef52 Steve French 2020-02-06 @1616 };
:::::: The code at line 1616 was first introduced by commit
:::::: ab3459d8f0ef52c38119ed58c4c29139efc7022c smb3: print warning once if posix context returned on open
:::::: TO: Steve French <stfrench@microsoft.com>
:::::: CC: Steve French <stfrench@microsoft.com>
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 25773 bytes --]
^ permalink raw reply
* Re: [meta-virtualization] [PATCH] Xen: fix do_deploy dependencies
From: Christopher Clark @ 2020-02-20 18:41 UTC (permalink / raw)
To: Bruce Ashfield
Cc: Bertrand Marquis, meta-virtualization@lists.yoctoproject.org, nd,
Doug Goldstein
In-Reply-To: <CADkTA4MFBHuXrySuG02TzOkmEitwMwXXs-JPccZj--7-NR9RDA@mail.gmail.com>
On Wed, Feb 19, 2020 at 6:00 AM Bruce Ashfield <bruce.ashfield@gmail.com> wrote:
>
> On Wed, Feb 19, 2020 at 8:47 AM Bertrand Marquis <Bertrand.Marquis@arm.com> wrote:
> >
> > On 19 Feb 2020, at 13:31, Bruce Ashfield <bruce.ashfield@gmail.com> wrote:
> >
> > On Wed, Feb 19, 2020 at 8:09 AM Bertrand Marquis <Bertrand.Marquis@arm.com> wrote:
> >
> > On 14 Feb 2020, at 10:17, Bertrand Marquis via Lists.Yoctoproject.Org <bertrand.marquis=arm.com@lists.yoctoproject.org> wrote:
> >
> > > > > do_deploy is using the files installed during do_install and it must be
> > > > > executed after it but before do_build. This is fixing SSTATE behaviour
> > > > > which had previously an empty scene in some cases.
> > >
> > > > Any chance somebody could check and ack on this ?
> > >
> > > There's an entire update / re-work of Xen on the mailing list. I won't
> > > be merging any Xen patches until we have that sorted out.
> >
> > Ok I will re-push my patch once the changes in Xen are merged then.
>
> the patch does look fine, and I've tagged it for merging. I just
> wanted to get the big changes in before jiggling anything (even if it
> shouldn't have an impact).
Thanks for the report, Bertrand. I've taken a look at this. The
addition of "before do_build" looks OK to me, and should be sufficient
to address the behaviour reported about an empty scene observed in
some cases -- some more details about that observation that prompted
this patch, and some testing to confirm that just adding "before
do_build" addition addresses it would be helpful.
I'm not convinced about the change to move it to "after do_install" though.
do_populate_sysroot already runs after do_install, so no change is
needed to ensure that the binaries are available by the time the
deploy task runs, but more importantly I think that the deploy task
should run, as it currently does, after do_populate_sysroot to ensure
that the same binaries that are made available to other recipes via
sysroots are also those that are deployed. Scheduling it after the
do_populate_sysroot means that if the deploy task is run, the sysroot
copies are also up to date.
Christopher
> > While there, it would make sense to also push this in zeus branch.
> >
> > —
> > Bertrand Marquis
> >
> >
> > Change-Id: I04b1f652c697ed0bc7e262571dd03e123ac096f6
> > Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
> > ---
> > recipes-extended/xen/xen.inc | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/recipes-extended/xen/xen.inc b/recipes-extended/xen/xen.inc
> > index 36ab78e..41b1983 100644
> > --- a/recipes-extended/xen/xen.inc
> > +++ b/recipes-extended/xen/xen.inc
> > @@ -1099,7 +1099,9 @@ do_deploy() {
> > fi
> > }
> >
> > -addtask deploy after do_populate_sysroot
> > +# do_deploy is using files created during do_install so force it to be called
> > +# after do_install but before do_build
> > +addtask deploy before do_build after do_install
> >
> > # Enable use of menuconfig directly from bitbake and also within the devshell
> > OE_TERMINAL_EXPORTS += "HOST_EXTRACFLAGS HOSTLDFLAGS TERMINFO"
> > --
> > 2.17.1
^ permalink raw reply
* [PATCH v2 0/2] of: unittest: add overlay gpio test to catch gpio hog problem
From: frowand.list @ 2020-02-20 18:40 UTC (permalink / raw)
To: Rob Herring, Geert Uytterhoeven, pantelis.antoniou
Cc: devicetree, linux-kernel, Alan Tull
From: Frank Rowand <frank.rowand@sony.com>
Geert reports that gpio hog nodes are not properly processed when
the gpio hog node is added via an overlay reply and provides an
patch to fix the problem [1].
Add a unittest that shows the problem. Unittest will report "1 failed"
test before applying Geert's patch and "0 failed" after applying
Geert's patch.
I did not have a development system for which it would be easy to
experiment with applying an overlay containing a gpio hog, so I
instead created this unittest that uses a fake gpio node.
Some tests in the devicetree unittests result in printk messages
from the code being tested. It can be difficult to determine
whether the messages are the result of unittest or are potentially
reporting bugs that should be fixed. The most recent example of
a person asking whether to be concerned about these messages is [2].
Patch 2 adds annotations for all messages triggered by unittests,
except KERN_DEBUG messages. (KERN_DEBUG is a special case due to the
possible interaction of CONFIG_DYNAMIC_DEBUG.)
The annotations added in patch 2/2 add a small amount of verbosity
to the console output. I have created a proof of concept tool to
explore (1) how test harnesses could use the annotations and
(2) how to make the resulting console output easier to read and
understand as a human being. The tool 'of_unittest_expect' is
available at https://github.com/frowand/dt_tools
The format of the annotations is expected to change when unittests
are converted to use the kunit infrastructure when the broader
testing community has an opportunity to discuss the implementation
of annotations of test triggered messages.
[1] https://lore.kernel.org/r/20200220130149.26283-1-geert+renesas@glider.be
[2] https://lore.kernel.org/r/6021ac63-b5e0-ed3d-f964-7c6ef579cd68@huawei.com
Frank Rowand (2):
of: unittest: add overlay gpio test to catch gpio hog problem
of: unittest: annotate warnings triggered by unittest
drivers/of/unittest-data/Makefile | 8 +-
drivers/of/unittest-data/overlay_gpio_01.dts | 23 +
drivers/of/unittest-data/overlay_gpio_02a.dts | 16 +
drivers/of/unittest-data/overlay_gpio_02b.dts | 16 +
drivers/of/unittest-data/overlay_gpio_03.dts | 23 +
drivers/of/unittest-data/overlay_gpio_04a.dts | 16 +
drivers/of/unittest-data/overlay_gpio_04b.dts | 16 +
drivers/of/unittest.c | 628 ++++++++++++++++++++++++--
8 files changed, 715 insertions(+), 31 deletions(-)
create mode 100644 drivers/of/unittest-data/overlay_gpio_01.dts
create mode 100644 drivers/of/unittest-data/overlay_gpio_02a.dts
create mode 100644 drivers/of/unittest-data/overlay_gpio_02b.dts
create mode 100644 drivers/of/unittest-data/overlay_gpio_03.dts
create mode 100644 drivers/of/unittest-data/overlay_gpio_04a.dts
create mode 100644 drivers/of/unittest-data/overlay_gpio_04b.dts
--
Frank Rowand <frank.rowand@sony.com>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.