* Re: [PATCH v26 01/12] Linux Random Number Generator
From: kbuild test robot @ 2019-11-24 22:44 UTC (permalink / raw)
To: Stephan Müller
Cc: kbuild-all, Arnd Bergmann, Greg Kroah-Hartman, linux-crypto, LKML,
linux-api, Eric W. Biederman, Alexander E. Patrakov,
Ahmed S. Darwish, Theodore Y. Ts'o, Willy Tarreau,
Matthew Garrett, Vito Caputo, Andreas Dilger, Jan Kara,
Ray Strode, William Jon McCann, zhangjs, Andy Lutomirski,
Florian Weimer, Lennart Poettering, Ni
In-Reply-To: <4092523.WvKr7FP7fE@positron.chronox.de>
[-- Attachment #1: Type: text/plain, Size: 2717 bytes --]
Hi "Stephan,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on char-misc/char-misc-testing]
[also build test ERROR on v5.4-rc8 next-20191122]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Stephan-M-ller/dev-random-a-new-approach-with-full-SP800-90B/20191125-042152
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git b78cda795ac83333293f1bfa3165572a47e550c2
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gcc (GCC) 7.4.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.4.0 make.cross ARCH=sh
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All error/warnings (new ones prefixed by >>):
In file included from include/linux/printk.h:6:0,
from include/linux/kernel.h:15,
from include/asm-generic/bug.h:19,
from arch/sh/include/asm/bug.h:112,
from include/linux/bug.h:5,
from include/linux/mmdebug.h:5,
from include/linux/percpu.h:5,
from include/asm-generic/irq_regs.h:11,
from ./arch/sh/include/generated/asm/irq_regs.h:1,
from drivers/char/lrng/lrng_pool.c:10:
>> drivers/char/lrng/lrng_pool.c:216:15: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
core_initcall(lrng_init_time_source);
^
include/linux/init.h:197:50: note: in definition of macro '___define_initcall'
__attribute__((__section__(#__sec ".init"))) = fn;
^~
include/linux/init.h:218:28: note: in expansion of macro '__define_initcall'
#define core_initcall(fn) __define_initcall(fn, 1)
^~~~~~~~~~~~~~~~~
>> drivers/char/lrng/lrng_pool.c:216:1: note: in expansion of macro 'core_initcall'
core_initcall(lrng_init_time_source);
^~~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +216 drivers/char/lrng/lrng_pool.c
215
> 216 core_initcall(lrng_init_time_source);
217
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 52266 bytes --]
^ permalink raw reply
* Re: [PATCH v17 08/13] namei: LOOKUP_BENEATH: O_BENEATH-like scoped resolution
From: Al Viro @ 2019-11-25 0:26 UTC (permalink / raw)
To: Aleksa Sarai
Cc: Song Liu, linux-ia64, linux-doc, Peter Zijlstra, Rasmus Villemoes,
Alexei Starovoitov, linux-kernel, David Howells, linux-kselftest,
sparclinux, containers, Christian Brauner, linux-api, Shuah Khan,
linux-arch, linux-s390, Tycho Andersen, Daniel Borkmann,
Jonathan Corbet, Jiri Olsa, linux-sh, Alexander Shishkin,
Ingo Molnar, linux-arm-kernel, Yonghong Song, linux-mips
In-Reply-To: <20191117011713.13032-9-cyphar@cyphar.com>
On Sun, Nov 17, 2019 at 12:17:08PM +1100, Aleksa Sarai wrote:
> + if (unlikely(nd->flags & LOOKUP_IS_SCOPED)) {
> + /*
> + * Do a final check to ensure that the path didn't escape. Note
> + * that this should already be guaranteed by all of the other
> + * LOOKUP_IS_SCOPED checks (and delaying this check this late
> + * does open the door to some possible timing-based attacks).
> + */
> + if (WARN_ON(!path_is_under(&nd->path, &nd->root)))
> + return -EXDEV;
I don't like that. What it gives is an ability to race that with
rename(), with user-triggered WARN_ON. You *can't* promise that result of
lookup is in a subtree, simply because it can get moved just as you've
declared it to be in the clear.
Anyone who relies upon that is delusional; it really can't be done.
What warranties LOOKUP_IS_SCOPED is really supposed to provide? That we do not
attempt to walk out of the subtree rooted at the start point? Fine, but this
is not what this test does. What are you trying to achieve there? If it's
"what we'd got was at one point in our subtree", the test is more or less
right, but WARN_ON isn't.
^ permalink raw reply
* Re: [PATCH v17 10/13] namei: LOOKUP_{IN_ROOT,BENEATH}: permit limited ".." resolution
From: Al Viro @ 2019-11-25 0:35 UTC (permalink / raw)
To: Aleksa Sarai
Cc: Song Liu, linux-ia64, linux-doc, Peter Zijlstra, Rasmus Villemoes,
Alexei Starovoitov, linux-kernel, David Howells, linux-kselftest,
sparclinux, containers, Christian Brauner, linux-api, Shuah Khan,
linux-arch, linux-s390, Tycho Andersen, Daniel Borkmann,
Jonathan Corbet, Jiri Olsa, linux-sh, Alexander Shishkin,
Ingo Molnar, linux-arm-kernel, Yonghong Song, linux-mips
In-Reply-To: <20191117011713.13032-11-cyphar@cyphar.com>
On Sun, Nov 17, 2019 at 12:17:10PM +1100, Aleksa Sarai wrote:
> + if (unlikely(nd->flags & LOOKUP_IS_SCOPED)) {
> + /*
> + * If there was a racing rename or mount along our
> + * path, then we can't be sure that ".." hasn't jumped
> + * above nd->root (and so userspace should retry or use
> + * some fallback).
> + */
> + if (unlikely(read_seqretry(&mount_lock, nd->m_seq)))
> + return -EAGAIN;
> + if (unlikely(read_seqretry(&rename_lock, nd->r_seq)))
> + return -EAGAIN;
> + }
Looks like excessive barriers to me - it's
rmb
check mount_lock.sequence
rmb
check rename_lock.sequence
> @@ -2266,6 +2274,10 @@ static const char *path_init(struct nameidata *nd, unsigned flags)
> nd->last_type = LAST_ROOT; /* if there are only slashes... */
> nd->flags = flags | LOOKUP_JUMPED | LOOKUP_PARENT;
> nd->depth = 0;
> +
> + nd->m_seq = read_seqbegin(&mount_lock);
> + nd->r_seq = read_seqbegin(&rename_lock);
Same here, pretty much (fetch/rmb/fetch/rmb)
^ permalink raw reply
* Re: [PATCH v17 11/13] open: introduce openat2(2) syscall
From: Al Viro @ 2019-11-25 0:37 UTC (permalink / raw)
To: Aleksa Sarai
Cc: Song Liu, linux-ia64, linux-doc, Peter Zijlstra, Rasmus Villemoes,
Alexei Starovoitov, linux-kernel, David Howells, linux-kselftest,
sparclinux, containers, linux-api, Shuah Khan, linux-arch,
linux-s390, Tycho Andersen, Daniel Borkmann, Jonathan Corbet,
Jiri Olsa, linux-sh, Alexander Shishkin, Ingo Molnar,
linux-arm-kernel, Yonghong Song, linux-mips, Andrii Nakryiko, bpf
In-Reply-To: <20191117011713.13032-12-cyphar@cyphar.com>
On Sun, Nov 17, 2019 at 12:17:11PM +1100, Aleksa Sarai wrote:
> +/* how->upgrade flags for openat2(2). */
> +/* First bit is reserved for a future UPGRADE_NOEXEC flag. */
> +#define UPGRADE_NOREAD 0x02 /* Block re-opening with MAY_READ. */
> +#define UPGRADE_NOWRITE 0x04 /* Block re-opening with MAY_WRITE. */
I'd drop that chunk, for the lack of ->upgrade...
^ permalink raw reply
* Re: [PATCH v17 00/13] open: introduce openat2(2) syscall
From: Al Viro @ 2019-11-25 0:39 UTC (permalink / raw)
To: Aleksa Sarai
Cc: Song Liu, linux-ia64, linux-doc, Peter Zijlstra, Rasmus Villemoes,
Alexei Starovoitov, linux-kernel, David Howells, linux-kselftest,
sparclinux, containers, linux-api, Shuah Khan, linux-arch,
linux-s390, Tycho Andersen, Daniel Borkmann, Jonathan Corbet,
Jiri Olsa, linux-sh, Alexander Shishkin, Ingo Molnar,
linux-arm-kernel, Yonghong Song, linux-mips, Andrii Nakryiko, bpf
In-Reply-To: <20191117011713.13032-1-cyphar@cyphar.com>
Generally I can live with that - the only serious issue is
with that WARN_ON() and the logics of the check in it.
^ permalink raw reply
* Re: [PATCH v6 0/2] add performance reporting support to FPGA DFL drivers
From: Wu Hao @ 2019-11-25 3:34 UTC (permalink / raw)
To: mdf, will, mark.rutland, linux-fpga, linux-kernel
Cc: linux-api, atull, gregkh, hao.wu
In-Reply-To: <1573622695-25607-1-git-send-email-hao.wu@intel.com>
On Wed, Nov 13, 2019 at 01:24:53PM +0800, Wu Hao wrote:
> Hi Moritz and all,
Hi Will and Mark,
Could you please help us on review this patchset? as this patchset mainly
introduced a new perf driver following the similar way as drivers/perf/*.
Hi Moritz,
Could you please help us on review the other parts?
This patchset has been submitted for a long time but didn't receive any
comment after v4. we appreciate any review comments! thanks in advance. :)
Hao
>
> This patchset adds performance reporting support for FPGA DFL drivers. It
> introduces one pmu to expose userspace interfaces via standard perf API.
> User could use standard perf tool to access perf events exposed via pmu.
>
> This patchset is generated based on fpga/for-next branch.
>
> Main changes from v5:
> - use dev_ext_attribute instead of fme_perf_event_attr.
> - use is_visible function to decide which events to expose per
> hardware capability, and add event_init checking for all events.
>
> Main changes from v4:
> - rebase and clean up.
> - update Kconfig for PERF_EVENTS dependency.
>
> Main changes from v3:
> - add more descriptions in doc, including how to use perf tool for these
> hardware counters. (patch #1)
> - use standard perf API instead of sysfs entries. (patch #2)
>
> Wu Hao (1):
> fpga: dfl: fme: add performance reporting support
>
> Xu Yilun (1):
> Documentation: fpga: dfl: add description for performance reporting
> support
>
> Documentation/fpga/dfl.rst | 83 ++++
> drivers/fpga/Makefile | 1 +
> drivers/fpga/dfl-fme-main.c | 4 +
> drivers/fpga/dfl-fme-perf.c | 943 ++++++++++++++++++++++++++++++++++++++++++++
> drivers/fpga/dfl-fme.h | 2 +
> drivers/fpga/dfl.h | 2 +
> 6 files changed, 1035 insertions(+)
> create mode 100644 drivers/fpga/dfl-fme-perf.c
>
> --
> 1.8.3.1
^ permalink raw reply
* Re: [PATCH v17 08/13] namei: LOOKUP_BENEATH: O_BENEATH-like scoped resolution
From: Aleksa Sarai @ 2019-11-25 6:03 UTC (permalink / raw)
To: Al Viro
Cc: Jeff Layton, J. Bruce Fields, Arnd Bergmann, David Howells,
Shuah Khan, Shuah Khan, Ingo Molnar, Peter Zijlstra,
Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Yonghong Song, Andrii Nakryiko, Jonathan Corbet,
Christian Brauner, David Drysdale, Andy Lutomirski,
Linus Torvalds, Eric Biederman, Andrew Morton, Kees Cook,
Jann Horn, Tycho Andersen
In-Reply-To: <20191125002604.GE4203@ZenIV.linux.org.uk>
[-- Attachment #1: Type: text/plain, Size: 2877 bytes --]
On 2019-11-25, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Sun, Nov 17, 2019 at 12:17:08PM +1100, Aleksa Sarai wrote:
>
> > + if (unlikely(nd->flags & LOOKUP_IS_SCOPED)) {
> > + /*
> > + * Do a final check to ensure that the path didn't escape. Note
> > + * that this should already be guaranteed by all of the other
> > + * LOOKUP_IS_SCOPED checks (and delaying this check this late
> > + * does open the door to some possible timing-based attacks).
> > + */
> > + if (WARN_ON(!path_is_under(&nd->path, &nd->root)))
> > + return -EXDEV;
>
> I don't like that. What it gives is an ability to race that with
> rename(), with user-triggered WARN_ON. You *can't* promise that result of
> lookup is in a subtree, simply because it can get moved just as you've
> declared it to be in the clear.
>
> Anyone who relies upon that is delusional; it really can't be done.
> What warranties LOOKUP_IS_SCOPED is really supposed to provide? That we do not
> attempt to walk out of the subtree rooted at the start point? Fine, but this
> is not what this test does. What are you trying to achieve there? If it's
> "what we'd got was at one point in our subtree", the test is more or less
> right, but WARN_ON isn't.
You're right that (looking at this again) this chunk doesn't make too
much sense -- though I still think it wouldn't be a bad idea to include
it without the WARN_ON.
The reason I added it was as an attempt to have a last-chance check to
make sure we don't hand out a file descriptor to userspace that is
outside nd->root as a result of some yet-unknown namei bug (hence the
WARN_ON). But you're quite right that I overlooked that user-space could
trigger this maliciously.
Regarding the warranties LOOKUP_IS_SCOPED is supposed to provide --
arguably the guarantee is meant to be "you never step outside the root
during lookup" but that should already be implemented with the
handle_dots() checks -- and it's not something you could easily check at
the end of a lookup anyway. The idea was that (if for some reason those
checks were bypassed), at the very least you wouldn't silently get a
file descriptor which is completely outside the root.
Looking at this again, I can see the argument that check this shouldn't
be done at all -- but I will admit that I feel more comfortable with the
guarantees of LOOKUP_IS_SCOPED if we had some kind of last-chance check
to avoid a privileged process opening /etc/shadow on the host. Then
again, libpathrs (which I assume will be the primary consumer of
LOOKUP_IN_ROOT) already does a double-check in userspace after getting
the file descriptor from openat2().
All of that being said, I'd be happy to drop it entirely if you feel
it's unnecessary.
--
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v26 01/12] Linux Random Number Generator
From: Stephan Mueller @ 2019-11-25 6:29 UTC (permalink / raw)
To: kbuild test robot
Cc: kbuild-all, Arnd Bergmann, Greg Kroah-Hartman, linux-crypto, LKML,
linux-api, Eric W. Biederman, Alexander E. Patrakov,
Ahmed S. Darwish, Theodore Y. Ts'o, Willy Tarreau,
Matthew Garrett, Vito Caputo, Andreas Dilger, Jan Kara,
Ray Strode, William Jon McCann, zhangjs, Andy Lutomirski,
Florian Weimer, Lennart Poettering, Ni
In-Reply-To: <201911250614.ZLzn1B8q%lkp@intel.com>
Am Sonntag, 24. November 2019, 23:44:13 CET schrieb kbuild test robot:
Hi kbuild,
> Hi "Stephan,
>
> Thank you for the patch! Yet something to improve:
>
> [auto build test ERROR on char-misc/char-misc-testing]
> [also build test ERROR on v5.4-rc8 next-20191122]
> [if your patch is applied to the wrong git tree, please drop us a note to
> help improve the system. BTW, we also suggest to use '--base' option to
> specify the base tree in git format-patch, please see
> https://stackoverflow.com/a/37406982]
>
> url:
> https://github.com/0day-ci/linux/commits/Stephan-M-ller/dev-random-a-new-ap
> proach-with-full-SP800-90B/20191125-042152 base:
> https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git
> b78cda795ac83333293f1bfa3165572a47e550c2 config: sh-allmodconfig (attached
> as .config)
> compiler: sh4-linux-gcc (GCC) 7.4.0
> reproduce:
> wget
> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
> ~/bin/make.cross chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> GCC_VERSION=7.4.0 make.cross ARCH=sh
>
> If you fix the issue, kindly add following tag
> Reported-by: kbuild test robot <lkp@intel.com>
>
> All error/warnings (new ones prefixed by >>):
>
> In file included from include/linux/printk.h:6:0,
> from include/linux/kernel.h:15,
> from include/asm-generic/bug.h:19,
> from arch/sh/include/asm/bug.h:112,
> from include/linux/bug.h:5,
> from include/linux/mmdebug.h:5,
> from include/linux/percpu.h:5,
> from include/asm-generic/irq_regs.h:11,
> from ./arch/sh/include/generated/asm/irq_regs.h:1,
>
> from drivers/char/lrng/lrng_pool.c:10:
> >> drivers/char/lrng/lrng_pool.c:216:15: error: initialization from
> >> incompatible pointer type [-Werror=incompatible-pointer-types]
The function prototype needs to return an int - I wonder why that did not show
in my x86 compile run.
It will be fixed in the next set.
> core_initcall(lrng_init_time_source);
> ^
> include/linux/init.h:197:50: note: in definition of macro
> '___define_initcall' __attribute__((__section__(#__sec ".init"))) = fn;
> ^~
> include/linux/init.h:218:28: note: in expansion of macro
> '__define_initcall' #define core_initcall(fn) __define_initcall(fn, 1)
> ^~~~~~~~~~~~~~~~~
>
> >> drivers/char/lrng/lrng_pool.c:216:1: note: in expansion of macro
> >> 'core_initcall'
> core_initcall(lrng_init_time_source);
> ^~~~~~~~~~~~~
> cc1: some warnings being treated as errors
>
> vim +216 drivers/char/lrng/lrng_pool.c
>
> 215
>
> > 216 core_initcall(lrng_init_time_source);
>
> 217
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology
> Center https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel
> Corporation
Ciao
Stephan
^ permalink raw reply
* Re: [PATCH v6 0/2] add performance reporting support to FPGA DFL drivers
From: Will Deacon @ 2019-11-25 8:01 UTC (permalink / raw)
To: Wu Hao
Cc: mdf, mark.rutland, linux-fpga, linux-kernel, linux-api, atull,
gregkh
In-Reply-To: <20191125033412.GB890@hao-dev>
On Mon, Nov 25, 2019 at 11:34:12AM +0800, Wu Hao wrote:
> Hi Will and Mark,
>
> Could you please help us on review this patchset? as this patchset mainly
> introduced a new perf driver following the similar way as drivers/perf/*.
Why is it not under drivers/perf/, then?
> This patchset has been submitted for a long time but didn't receive any
> comment after v4. we appreciate any review comments! thanks in advance. :)
Hmm, not sure I saw the previous versions. Guessing I wasn't on cc?
Will
^ permalink raw reply
* Re: [PATCH v6 0/2] add performance reporting support to FPGA DFL drivers
From: Wu Hao @ 2019-11-25 8:08 UTC (permalink / raw)
To: Will Deacon
Cc: mdf, mark.rutland, linux-fpga, linux-kernel, linux-api, atull,
gregkh
In-Reply-To: <20191125080127.GC1809@willie-the-truck>
On Mon, Nov 25, 2019 at 08:01:28AM +0000, Will Deacon wrote:
> On Mon, Nov 25, 2019 at 11:34:12AM +0800, Wu Hao wrote:
> > Hi Will and Mark,
> >
> > Could you please help us on review this patchset? as this patchset mainly
> > introduced a new perf driver following the similar way as drivers/perf/*.
>
> Why is it not under drivers/perf/, then?
Hi Will
Thanks for the quick response. This is one sub feature for DFL based FPGAs,
and we plan to put this sub feature together with others, including related
documentation. It only registers a standard perf pmu for its userspace
interfaces.
>
> > This patchset has been submitted for a long time but didn't receive any
> > comment after v4. we appreciate any review comments! thanks in advance. :)
>
> Hmm, not sure I saw the previous versions. Guessing I wasn't on cc?
We switched to perf API from v4, and started ccing you and Mark from v5. :)
Thanks
Hao
>
> Will
^ permalink raw reply
* Re: [PATCH][v5] x86/resctrl: Add task resctrl information display
From: Michal Hocko @ 2019-11-25 9:32 UTC (permalink / raw)
To: Chen Yu
Cc: x86, Chen Yu, Thomas Gleixner, Borislav Petkov, Ingo Molnar,
H. Peter Anvin, Alexey Dobriyan, Andrew Morton, Reinette Chatre,
Fenghua Yu, Tony Luck, Linus Torvalds, linux-fsdevel,
linux-kernel, linux-api
In-Reply-To: <20191125040001.28943-1-yu.c.chen@intel.com>
[Cc linux-api]
On Mon 25-11-19 12:00:01, Chen Yu wrote:
> Monitoring tools that want to find out which resctrl control
> and monitor groups a task belongs to must currently read
> the "tasks" file in every group until they locate the process
> ID.
>
> Add an additional file /proc/{pid}/resctrl to provide this
> information.
>
> The output is as followed according to Thomas's suggestion,
> for example:
>
> 1) ""
> Resctrl is not available.
>
> 2) "/"
> Task is part of the root group, task is not associated to
> any monitoring group.
>
> 3) "/mon_groups/mon0"
> Task is part of the root group and monitoring group mon0.
>
> 4) "/group0"
> Task is part of control group group0, task is not associated
> to any monitoring group.
>
> 5) "/group0/mon_groups/mon1"
> Task is part of control group group0 and monitoring group mon1.
>
> Tested-by: Jinshi Chen <jinshi.chen@intel.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: Alexey Dobriyan <adobriyan@gmail.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Reinette Chatre <reinette.chatre@intel.com>
> Cc: Fenghua Yu <fenghua.yu@intel.com>
> Cc: Tony Luck <tony.luck@intel.com>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: linux-fsdevel@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> ---
> v2: Reduce indentation level in proc_resctrl_show()
> according to Boris's suggestion.
> Create the include/linux/resctrl.h header and
> declare proc_resctrl_show() in this file, so
> that other architectures would probably use it
> in the future. Different architectures should
> implement architectural specific proc_resctrl_show()
> accordingly.
>
> v3: Return empty string if the resctrl filesystem has
> not been mounted per Boris's suggestion.
> Rename the config from CPU_RESCTRL to PROC_CPU_RESCTRL
> to better represent its usage. Move PROC_CPU_RESCTRL
> from arch/Kconfig to fs/proc/Kconfig.
> And let PROC_CPU_RESCTRL to be depended on PROC_FS.
>
> v4: According to Thomas's suggestion, changed the output
> from multiple lines to one single line.
>
> v5: According to Alexey's feedback, removed the header file
> proc_fs.h in resctrl.h, and changed seq_puts() to
> seq_putc() for simplicity.
> ---
> arch/x86/Kconfig | 1 +
> arch/x86/kernel/cpu/resctrl/rdtgroup.c | 78 ++++++++++++++++++++++++++
> fs/proc/Kconfig | 4 ++
> fs/proc/base.c | 7 +++
> include/linux/resctrl.h | 14 +++++
> 5 files changed, 104 insertions(+)
> create mode 100644 include/linux/resctrl.h
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 8ef85139553f..252364d18887 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -455,6 +455,7 @@ config X86_CPU_RESCTRL
> bool "x86 CPU resource control support"
> depends on X86 && (CPU_SUP_INTEL || CPU_SUP_AMD)
> select KERNFS
> + select PROC_CPU_RESCTRL if PROC_FS
> help
> Enable x86 CPU resource control support.
>
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index 2e3b06d6bbc6..f786e7626a65 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -725,6 +725,84 @@ static int rdtgroup_tasks_show(struct kernfs_open_file *of,
> return ret;
> }
>
> +#ifdef CONFIG_PROC_CPU_RESCTRL
> +
> +/*
> + * A task can only be part of one control
> + * group and of one monitoring group which
> + * is associated to that control group.
> + * So one line is simple and clear enough:
> + *
> + * 1) ""
> + * Resctrl is not available.
> + *
> + * 2) "/"
> + * Task is part of the root group, and it is
> + * not associated to any monitoring group.
> + *
> + * 3) "/mon_groups/mon0"
> + * Task is part of the root group and monitoring
> + * group mon0.
> + *
> + * 4) "/group0"
> + * Task is part of control group group0, and it is
> + * not associated to any monitoring group.
> + *
> + * 5) "/group0/mon_groups/mon1"
> + * Task is part of control group group0 and monitoring
> + * group mon1.
> + */
> +int proc_resctrl_show(struct seq_file *s, struct pid_namespace *ns,
> + struct pid *pid, struct task_struct *tsk)
> +{
> + struct rdtgroup *rdtg;
> + int ret = 0;
> +
> + mutex_lock(&rdtgroup_mutex);
> +
> + /* Return empty if resctrl has not been mounted. */
> + if (!static_branch_unlikely(&rdt_enable_key))
> + goto unlock;
> +
> + list_for_each_entry(rdtg, &rdt_all_groups, rdtgroup_list) {
> + struct rdtgroup *crg;
> +
> + /*
> + * Task information is only relevant for shareable
> + * and exclusive groups.
> + */
> + if (rdtg->mode != RDT_MODE_SHAREABLE &&
> + rdtg->mode != RDT_MODE_EXCLUSIVE)
> + continue;
> +
> + if (rdtg->closid != tsk->closid)
> + continue;
> +
> + seq_printf(s, "/%s", rdtg->kn->name);
> + list_for_each_entry(crg, &rdtg->mon.crdtgrp_list,
> + mon.crdtgrp_list) {
> + if (tsk->rmid != crg->mon.rmid)
> + continue;
> + seq_printf(s, "%smon_groups/%s",
> + rdtg == &rdtgroup_default ? "" : "/",
> + crg->kn->name);
> + break;
> + }
> + seq_putc(s, '\n');
> + goto unlock;
> + }
> + /*
> + * The above search should succeed. Otherwise return
> + * with an error.
> + */
> + ret = -ENOENT;
> +unlock:
> + mutex_unlock(&rdtgroup_mutex);
> +
> + return ret;
> +}
> +#endif
> +
> static int rdt_last_cmd_status_show(struct kernfs_open_file *of,
> struct seq_file *seq, void *v)
> {
> diff --git a/fs/proc/Kconfig b/fs/proc/Kconfig
> index cb5629bd5fff..ae96a339d24d 100644
> --- a/fs/proc/Kconfig
> +++ b/fs/proc/Kconfig
> @@ -103,3 +103,7 @@ config PROC_CHILDREN
> config PROC_PID_ARCH_STATUS
> def_bool n
> depends on PROC_FS
> +
> +config PROC_CPU_RESCTRL
> + def_bool n
> + depends on PROC_FS
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index ebea9501afb8..0e4b8bf2b986 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -94,6 +94,7 @@
> #include <linux/sched/debug.h>
> #include <linux/sched/stat.h>
> #include <linux/posix-timers.h>
> +#include <linux/resctrl.h>
> #include <trace/events/oom.h>
> #include "internal.h"
> #include "fd.h"
> @@ -3060,6 +3061,9 @@ static const struct pid_entry tgid_base_stuff[] = {
> #endif
> #ifdef CONFIG_CGROUPS
> ONE("cgroup", S_IRUGO, proc_cgroup_show),
> +#endif
> +#ifdef CONFIG_PROC_CPU_RESCTRL
> + ONE("resctrl", S_IRUGO, proc_resctrl_show),
> #endif
> ONE("oom_score", S_IRUGO, proc_oom_score),
> REG("oom_adj", S_IRUGO|S_IWUSR, proc_oom_adj_operations),
> @@ -3460,6 +3464,9 @@ static const struct pid_entry tid_base_stuff[] = {
> #endif
> #ifdef CONFIG_CGROUPS
> ONE("cgroup", S_IRUGO, proc_cgroup_show),
> +#endif
> +#ifdef CONFIG_PROC_CPU_RESCTRL
> + ONE("resctrl", S_IRUGO, proc_resctrl_show),
> #endif
> ONE("oom_score", S_IRUGO, proc_oom_score),
> REG("oom_adj", S_IRUGO|S_IWUSR, proc_oom_adj_operations),
> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
> new file mode 100644
> index 000000000000..daf5cf64c6a6
> --- /dev/null
> +++ b/include/linux/resctrl.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _RESCTRL_H
> +#define _RESCTRL_H
> +
> +#ifdef CONFIG_PROC_CPU_RESCTRL
> +
> +int proc_resctrl_show(struct seq_file *m,
> + struct pid_namespace *ns,
> + struct pid *pid,
> + struct task_struct *tsk);
> +
> +#endif
> +
> +#endif /* _RESCTRL_H */
> --
> 2.17.1
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH v17 10/13] namei: LOOKUP_{IN_ROOT,BENEATH}: permit limited ".." resolution
From: Aleksa Sarai @ 2019-11-25 13:21 UTC (permalink / raw)
To: Al Viro
Cc: Jeff Layton, J. Bruce Fields, Arnd Bergmann, David Howells,
Shuah Khan, Shuah Khan, Ingo Molnar, Peter Zijlstra,
Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Yonghong Song, Andrii Nakryiko, Jonathan Corbet,
Christian Brauner, Jann Horn, Linus Torvalds, Eric Biederman,
Andy Lutomirski, Andrew Morton, Kees Cook, Tycho Andersen,
David Drysdale
In-Reply-To: <20191125003501.GF4203@ZenIV.linux.org.uk>
[-- Attachment #1: Type: text/plain, Size: 2300 bytes --]
On 2019-11-25, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Sun, Nov 17, 2019 at 12:17:10PM +1100, Aleksa Sarai wrote:
> > + if (unlikely(nd->flags & LOOKUP_IS_SCOPED)) {
> > + /*
> > + * If there was a racing rename or mount along our
> > + * path, then we can't be sure that ".." hasn't jumped
> > + * above nd->root (and so userspace should retry or use
> > + * some fallback).
> > + */
> > + if (unlikely(read_seqretry(&mount_lock, nd->m_seq)))
> > + return -EAGAIN;
> > + if (unlikely(read_seqretry(&rename_lock, nd->r_seq)))
> > + return -EAGAIN;
> > + }
>
> Looks like excessive barriers to me - it's
> rmb
> check mount_lock.sequence
> rmb
> check rename_lock.sequence
If you like, I can switch this to
smp_rmb();
if (unlikely(__read_seqcount_retry(&mount_lock.seqcount, nd->m_seq)))
return -EAGAIN;
if (unlikely(__read_seqcount_retry(&rename_lock.seqcount, nd->r_seq)))
return -EAGAIN;
Though I think it makes it more noisy (and this code-path will only be
hit for ".." and LOOKUP_IS_SCOPED).
> > @@ -2266,6 +2274,10 @@ static const char *path_init(struct nameidata *nd, unsigned flags)
> > nd->last_type = LAST_ROOT; /* if there are only slashes... */
> > nd->flags = flags | LOOKUP_JUMPED | LOOKUP_PARENT;
> > nd->depth = 0;
> > +
> > + nd->m_seq = read_seqbegin(&mount_lock);
> > + nd->r_seq = read_seqbegin(&rename_lock);
>
> Same here, pretty much (fetch/rmb/fetch/rmb)
Unless I'm mistaken, wouldn't we have to do
seqcount_lockdep_reader_access() explicitly -- so it would end up
looking something like:
seqcount_lockdep_reader_access(&mount_lock.seqcount);
nd->m_seq = __read_seqcount_begin(&mount_lock.seqcount);
seqcount_lockdep_reader_access(&mount_lock.seqcount);
nd->r_seq = __read_seqcount_begin(&rename_lock.seqcount);
smp_rmb();
Given this code only runs once at the start of each lookup, I'm not sure
it makes much sense to expand it like that and make it look uglier.
If you really want to avoid the duplicate memory barriers in the common
case I could instead gate the rename_lock grab behind LOOKUP_IS_SCOPED
(since that's the only time it's used).
--
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* [GIT PULL] pipe: Notification queue preparation
From: David Howells @ 2019-11-25 22:39 UTC (permalink / raw)
To: torvalds
Cc: dhowells, Rasmus Villemoes, Greg Kroah-Hartman, Peter Zijlstra,
raven, Christian Brauner, keyrings, linux-usb, linux-block,
linux-security-module, linux-fsdevel, linux-api, linux-kernel
Hi Linus,
Can you pull this please? This is my set of preparatory patches for
building a general notification queue on top of pipes. It makes a number
of significant changes:
(1) It removes the nr_exclusive argument from __wake_up_sync_key() as this
is always 1. This prepares for step 2.
(2) Adds wake_up_interruptible_sync_poll_locked() so that poll can be
woken up from a function that's holding the poll waitqueue spinlock.
[btw, I realise that I haven't un-sync'd the
wake_up_interruptible_sync_poll() calls as you tentatively suggested.
I can send a follow up patch to fix that if you still want it]
(3) Change the pipe buffer ring to be managed in terms of unbounded head
and tail indices rather than bounded index and length. This means
that reading the pipe only needs to modify one index, not two.
(4) A selection of helper functions are provided to query the state of the
pipe buffer, plus a couple to apply updates to the pipe indices.
(5) The pipe ring is allowed to have kernel-reserved slots. This allows
many notification messages to be spliced in by the kernel without
allowing userspace to pin too many pages if it writes to the same
pipe.
(6) Advance the head and tail indices inside the pipe waitqueue lock and
use step 2 to poke poll without having to take the lock twice.
(7) Rearrange pipe_write() to preallocate the buffer it is going to write
into and then drop the spinlock. This allows kernel notifications to
then be added the ring whilst it is filling the buffer it allocated.
The read side is stalled because the pipe mutex is still held.
(8) Don't wake up readers on a pipe if there was already data in it when
we added more.
(9) Don't wake up writers on a pipe if the ring wasn't full before we
removed a buffer.
PATCHES BENCHMARK BEST TOTAL BYTES AVG BYTES STDDEV
======= =============== =============== =============== =============== ===============
- pipe 307457969 36348556755 302904639 10622403
- splice 287117614 26933658717 224447155 160777958
- vmsplice 435180375 51302964090 427524700 19083037
rm-nrx pipe 311091179 37093181356 309109844 7221622
rm-nrx splice 285628049 27916298942 232635824 158296431
rm-nrx vmsplice 417703153 47570362546 396419687 33960822
wakesl pipe 310698731 36772541631 306437846 8249347
wakesl splice 286193726 28600435451 238336962 141169318
wakesl vmsplice 436175803 50723895824 422699131 40724240
ht pipe 305534565 36426079543 303550662 5673885
ht splice 243632025 23319439010 194328658 150479853
ht vmsplice 432825176 49101781001 409181508 44102509
k-rsv pipe 308691523 36652267561 305435563 12972559
k-rsv splice 244793528 23625172865 196876440 125319143
k-rsv vmsplice 436119082 49460808579 412173404 55547525
r-adv-t pipe 310094218 36860182219 307168185 8081101
r-adv-t splice 285527382 27085052687 225708772 206918887
r-adv-t vmsplice 336885948 40128756927 334406307 5895935
r-cond pipe 308727804 36635828180 305298568 9976806
r-cond splice 284467568 28445793054 237048275 200284329
r-cond vmsplice 449679489 51134833848 426123615 66790875
w-preal pipe 307416578 36662086426 305517386 6216663
w-preal splice 282655051 28455249109 237127075 194154549
w-preal vmsplice 437002601 47832160621 398601338 96513019
w-redun pipe 307279630 36329750422 302747920 8913567
w-redun splice 284324488 27327152734 227726272 219735663
w-redun vmsplice 451141971 51485257719 429043814 51388217
w-ckful pipe 305055247 36374947350 303124561 5400728
w-ckful splice 281575308 26841554544 223679621 215942886
w-ckful vmsplice 436653588 47564907110 396374225 82255342
The patches column indicates the point in the patchset at which the benchmarks
were taken:
0 No patches
rm-nrx "Remove the nr_exclusive argument from __wake_up_sync_key()"
wakesl "Add wake_up_interruptible_sync_poll_locked()"
ht "pipe: Use head and tail pointers for the ring, not cursor and length"
k-rsv "pipe: Allow pipes to have kernel-reserved slots"
r-adv-t "pipe: Advance tail pointer inside of wait spinlock in pipe_read()"
r-cond "pipe: Conditionalise wakeup in pipe_read()"
w-preal "pipe: Rearrange sequence in pipe_write() to preallocate slot"
w-redun "pipe: Remove redundant wakeup from pipe_write()"
w-ckful "pipe: Check for ring full inside of the spinlock in pipe_write()"
Changes:
(*) Fix some bugs spotted by kbuild.
ver #3:
(*) Get rid of pipe_commit_{read,write}.
(*) Port the virtio_console driver.
(*) Fix pipe_zero().
(*) Amend some comments.
(*) Added an additional patch that changes the threshold at which readers
wake writers for Konstantin Khlebnikov.
ver #2:
(*) Split the notification patches out into a separate branch.
(*) Removed the nr_exclusive parameter from __wake_up_sync_key().
(*) Renamed the locked wakeup function.
(*) Add helpers for empty, full, occupancy.
(*) Split the addition of ->max_usage out into its own patch.
(*) Fixed some bits pointed out by Rasmus Villemoes.
ver #1:
(*) Build on top of standard pipes instead of having a driver.
David
---
The following changes since commit da0c9ea146cbe92b832f1b0f694840ea8eb33cce:
Linux 5.4-rc2 (2019-10-06 14:27:30 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git tags/notifications-pipe-prep-20191115
for you to fetch changes up to 3c0edea9b29f9be6c093f236f762202b30ac9431:
pipe: Remove sync on wake_ups (2019-11-15 16:22:54 +0000)
----------------------------------------------------------------
Pipework for general notification queue
----------------------------------------------------------------
David Howells (12):
pipe: Reduce #inclusion of pipe_fs_i.h
Remove the nr_exclusive argument from __wake_up_sync_key()
Add wake_up_interruptible_sync_poll_locked()
pipe: Use head and tail pointers for the ring, not cursor and length
pipe: Allow pipes to have kernel-reserved slots
pipe: Advance tail pointer inside of wait spinlock in pipe_read()
pipe: Conditionalise wakeup in pipe_read()
pipe: Rearrange sequence in pipe_write() to preallocate slot
pipe: Remove redundant wakeup from pipe_write()
pipe: Check for ring full inside of the spinlock in pipe_write()
pipe: Increase the writer-wakeup threshold to reduce context-switch count
pipe: Remove sync on wake_ups
drivers/char/virtio_console.c | 16 ++-
fs/exec.c | 1 -
fs/fuse/dev.c | 31 +++--
fs/ocfs2/aops.c | 1 -
fs/pipe.c | 232 +++++++++++++++++++++---------------
fs/splice.c | 190 +++++++++++++++++------------
include/linux/pipe_fs_i.h | 64 +++++++++-
include/linux/uio.h | 4 +-
include/linux/wait.h | 11 +-
kernel/exit.c | 2 +-
kernel/sched/wait.c | 37 ++++--
lib/iov_iter.c | 269 ++++++++++++++++++++++++------------------
security/smack/smack_lsm.c | 1 -
13 files changed, 529 insertions(+), 330 deletions(-)
^ permalink raw reply
* Re: [RFC PATCH v3 03/12] fs: add RWF_ENCODED for reading/writing compressed data
From: Nikolay Borisov @ 2019-11-26 13:53 UTC (permalink / raw)
To: Omar Sandoval, linux-fsdevel, linux-btrfs
Cc: Dave Chinner, Jann Horn, Amir Goldstein, Aleksa Sarai, linux-api,
kernel-team
In-Reply-To: <07f9cc1969052e94818fa50019e7589d206d1d18.1574273658.git.osandov@fb.com>
On 20.11.19 г. 20:24 ч., Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
<snip>
>
> +enum {
> + ENCODED_IOV_COMPRESSION_NONE,
> +#define ENCODED_IOV_COMPRESSION_NONE ENCODED_IOV_COMPRESSION_NONE
> + ENCODED_IOV_COMPRESSION_ZLIB,
> +#define ENCODED_IOV_COMPRESSION_ZLIB ENCODED_IOV_COMPRESSION_ZLIB
> + ENCODED_IOV_COMPRESSION_LZO,
> +#define ENCODED_IOV_COMPRESSION_LZO ENCODED_IOV_COMPRESSION_LZO
> + ENCODED_IOV_COMPRESSION_ZSTD,
> +#define ENCODED_IOV_COMPRESSION_ZSTD ENCODED_IOV_COMPRESSION_ZSTD
> + ENCODED_IOV_COMPRESSION_TYPES = ENCODED_IOV_COMPRESSION_ZSTD,
This looks very dodgy, what am I missing?
<snip>
^ permalink raw reply
* Re: [RFC PATCH v3 04/12] btrfs: get rid of trivial __btrfs_lookup_bio_sums() wrappers
From: Nikolay Borisov @ 2019-11-26 13:56 UTC (permalink / raw)
To: Omar Sandoval, linux-fsdevel, linux-btrfs
Cc: Dave Chinner, Jann Horn, Amir Goldstein, Aleksa Sarai, linux-api,
kernel-team
In-Reply-To: <bca47beb2f4eef766accebef683137e94313f7d3.1574273658.git.osandov@fb.com>
On 20.11.19 г. 20:24 ч., Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
>
> Currently, we have two wrappers for __btrfs_lookup_bio_sums():
> btrfs_lookup_bio_sums_dio(), which is used for direct I/O, and
> btrfs_lookup_bio_sums(), which is used everywhere else. The only
> difference is that the _dio variant looks up csums starting at the given
> offset instead of using the page index, which isn't actually direct
> I/O-specific. Let's clean up the signature and return value of
> __btrfs_lookup_bio_sums(), rename it to btrfs_lookup_bio_sums(), and get
> rid of the trivial helpers.
>
> Signed-off-by: Omar Sandoval <osandov@fb.com>
Overall looks good but 2 nits, see below.
In any case:
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
> ---
> fs/btrfs/compression.c | 4 ++--
> fs/btrfs/ctree.h | 4 +---
> fs/btrfs/file-item.c | 35 +++++++++++++++++------------------
> fs/btrfs/inode.c | 6 +++---
> 4 files changed, 23 insertions(+), 26 deletions(-)
>
> diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
> index b05b361e2062..4df6f0c58dc9 100644
> --- a/fs/btrfs/compression.c
> +++ b/fs/btrfs/compression.c
> @@ -660,7 +660,7 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
>
> if (!(BTRFS_I(inode)->flags & BTRFS_INODE_NODATASUM)) {
> ret = btrfs_lookup_bio_sums(inode, comp_bio,
> - sums);
> + false, 0, sums);
> BUG_ON(ret); /* -ENOMEM */
> }
>
> @@ -689,7 +689,7 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
> BUG_ON(ret); /* -ENOMEM */
>
> if (!(BTRFS_I(inode)->flags & BTRFS_INODE_NODATASUM)) {
> - ret = btrfs_lookup_bio_sums(inode, comp_bio, sums);
> + ret = btrfs_lookup_bio_sums(inode, comp_bio, false, 0, sums);
> BUG_ON(ret); /* -ENOMEM */
> }
>
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index fe2b8765d9e6..4bc40bf49b0e 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -2787,9 +2787,7 @@ struct btrfs_dio_private;
> int btrfs_del_csums(struct btrfs_trans_handle *trans,
> struct btrfs_fs_info *fs_info, u64 bytenr, u64 len);
> blk_status_t btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio,
> - u8 *dst);
> -blk_status_t btrfs_lookup_bio_sums_dio(struct inode *inode, struct bio *bio,
> - u64 logical_offset);
> + bool at_offset, u64 offset, u8 *dst);
> int btrfs_insert_file_extent(struct btrfs_trans_handle *trans,
> struct btrfs_root *root,
> u64 objectid, u64 pos,
> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> index 1a599f50837b..a87c40502267 100644
> --- a/fs/btrfs/file-item.c
> +++ b/fs/btrfs/file-item.c
> @@ -148,8 +148,21 @@ int btrfs_lookup_file_extent(struct btrfs_trans_handle *trans,
> return ret;
> }
>
> -static blk_status_t __btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio,
> - u64 logical_offset, u8 *dst, int dio)
> +/**
> + * btrfs_lookup_bio_sums - Look up checksums for a bio.
> + * @inode: inode that the bio is for.
> + * @bio: bio embedded in btrfs_io_bio.
> + * @at_offset: If true, look up checksums for the extent at @c offset.
nit: that @c is an editing artifact? On the other hand rather than
having an explicit bool signifying whether we want a specific offset
can't we simply check if offset is != 0 ?
<snip>
^ permalink raw reply
* Re: [RFC PATCH v3 05/12] btrfs: don't advance offset for compressed bios in btrfs_csum_one_bio()
From: Nikolay Borisov @ 2019-11-26 14:18 UTC (permalink / raw)
To: Omar Sandoval, linux-fsdevel, linux-btrfs
Cc: Dave Chinner, Jann Horn, Amir Goldstein, Aleksa Sarai, linux-api,
kernel-team
In-Reply-To: <a669365a9165b18814c635f61ed566fdcd47a96f.1574273658.git.osandov@fb.com>
On 20.11.19 г. 20:24 ч., Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
>
> btrfs_csum_one_bio() loops over each sector in the bio while keeping a
'sector' here is ambiguous it really loops over every fs block (which in
btrfs is also known as sector). SO perhaps change the wording in the
changelog but also in the function instead of nr_sectors perhahps it
could be renamed to blockcount?
> cursor of its current logical position in the file in order to look up
> the ordered extent to add the checksums to. However, this doesn't make
> much sense for compressed extents, as a sector on disk does not
> correspond to a sector of decompressed file data. It happens to work
> because 1) the compressed bio always covers one ordered extent and 2)
> the size of the bio is always less than the size of the ordered extent.
> However, the second point will not always be true for encoded writes.
>
> Let's add a boolean parameter to btrfs_csum_one_bio() to indicate that
> it can assume that the bio only covers one ordered extent. Since we're
> already changing the signature, let's make contig bool instead of int,
> too.
>
> Signed-off-by: Omar Sandoval <osandov@fb.com>
> ---
> fs/btrfs/compression.c | 5 +++--
> fs/btrfs/ctree.h | 2 +-
> fs/btrfs/file-item.c | 19 +++++++++++--------
> fs/btrfs/inode.c | 8 ++++----
> 4 files changed, 19 insertions(+), 15 deletions(-)
>
> diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
> index 4df6f0c58dc9..05b6e404a291 100644
> --- a/fs/btrfs/compression.c
> +++ b/fs/btrfs/compression.c
> @@ -374,7 +374,8 @@ blk_status_t btrfs_submit_compressed_write(struct inode *inode, u64 start,
> BUG_ON(ret); /* -ENOMEM */
>
> if (!skip_sum) {
> - ret = btrfs_csum_one_bio(inode, bio, start, 1);
> + ret = btrfs_csum_one_bio(inode, bio, start,
> + true, true);
> BUG_ON(ret); /* -ENOMEM */
> }
>
> @@ -405,7 +406,7 @@ blk_status_t btrfs_submit_compressed_write(struct inode *inode, u64 start,
> BUG_ON(ret); /* -ENOMEM */
>
> if (!skip_sum) {
> - ret = btrfs_csum_one_bio(inode, bio, start, 1);
> + ret = btrfs_csum_one_bio(inode, bio, start, true, true);
> BUG_ON(ret); /* -ENOMEM */
> }
>
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 4bc40bf49b0e..c32741879088 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -2802,7 +2802,7 @@ int btrfs_csum_file_blocks(struct btrfs_trans_handle *trans,
> struct btrfs_root *root,
> struct btrfs_ordered_sum *sums);
> blk_status_t btrfs_csum_one_bio(struct inode *inode, struct bio *bio,
> - u64 file_start, int contig);
> + u64 file_start, bool contig, bool one_ordered);
> int btrfs_lookup_csums_range(struct btrfs_root *root, u64 start, u64 end,
> struct list_head *list, int search_commit);
> void btrfs_extent_item_to_extent_map(struct btrfs_inode *inode,
> diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> index a87c40502267..c95772949b00 100644
> --- a/fs/btrfs/file-item.c
> +++ b/fs/btrfs/file-item.c
> @@ -423,13 +423,14 @@ int btrfs_lookup_csums_range(struct btrfs_root *root, u64 start, u64 end,
> * @inode: Owner of the data inside the bio
> * @bio: Contains the data to be checksummed
> * @file_start: offset in file this bio begins to describe
> - * @contig: Boolean. If true/1 means all bio vecs in this bio are
> - * contiguous and they begin at @file_start in the file. False/0
> - * means this bio can contains potentially discontigous bio vecs
> - * so the logical offset of each should be calculated separately.
> + * @contig: If true, all bio vecs in @bio are contiguous and they begin at
> + * @file_start in the file. If false, @bio may contain
> + * discontigous bio vecs, so the logical offset of each should be
> + * calculated separately (@file_start is ignored).
> + * @one_ordered: If true, @bio only refers to one ordered extent.
> */
> blk_status_t btrfs_csum_one_bio(struct inode *inode, struct bio *bio,
> - u64 file_start, int contig)
> + u64 file_start, bool contig, bool one_ordered)
> {
> struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
> SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
> @@ -482,8 +483,9 @@ blk_status_t btrfs_csum_one_bio(struct inode *inode, struct bio *bio,
> - 1);
>
> for (i = 0; i < nr_sectors; i++) {
> - if (offset >= ordered->file_offset + ordered->len ||
> - offset < ordered->file_offset) {
> + if (!one_ordered &&
> + (offset >= ordered->file_offset + ordered->len ||
> + offset < ordered->file_offset)) {
> unsigned long bytes_left;
>
> sums->len = this_sum_bytes;
> @@ -515,7 +517,8 @@ blk_status_t btrfs_csum_one_bio(struct inode *inode, struct bio *bio,
> kunmap_atomic(data);
> crypto_shash_final(shash, (char *)(sums->sums + index));
> index += csum_size;
> - offset += fs_info->sectorsize;
> + if (!one_ordered)
> + offset += fs_info->sectorsize;
> this_sum_bytes += fs_info->sectorsize;
> total_bytes += fs_info->sectorsize;
> }
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index ad5bffb24199..4c1ed6dddfd8 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -2039,7 +2039,7 @@ static blk_status_t btrfs_submit_bio_start(void *private_data, struct bio *bio,
> struct inode *inode = private_data;
> blk_status_t ret = 0;
>
> - ret = btrfs_csum_one_bio(inode, bio, 0, 0);
> + ret = btrfs_csum_one_bio(inode, bio, 0, false, false);
> BUG_ON(ret); /* -ENOMEM */
> return 0;
> }
> @@ -2104,7 +2104,7 @@ static blk_status_t btrfs_submit_bio_hook(struct inode *inode, struct bio *bio,
> 0, inode, btrfs_submit_bio_start);
> goto out;
> } else if (!skip_sum) {
> - ret = btrfs_csum_one_bio(inode, bio, 0, 0);
> + ret = btrfs_csum_one_bio(inode, bio, 0, false, false);
> if (ret)
> goto out;
> }
> @@ -8272,7 +8272,7 @@ static blk_status_t btrfs_submit_bio_start_direct_io(void *private_data,
> {
> struct inode *inode = private_data;
> blk_status_t ret;
> - ret = btrfs_csum_one_bio(inode, bio, offset, 1);
> + ret = btrfs_csum_one_bio(inode, bio, offset, true, false);
> BUG_ON(ret); /* -ENOMEM */
> return 0;
> }
> @@ -8379,7 +8379,7 @@ static inline blk_status_t btrfs_submit_dio_bio(struct bio *bio,
> * If we aren't doing async submit, calculate the csum of the
> * bio now.
> */
> - ret = btrfs_csum_one_bio(inode, bio, file_offset, 1);
> + ret = btrfs_csum_one_bio(inode, bio, file_offset, true, false);
> if (ret)
> goto err;
> } else {
>
^ permalink raw reply
* Re: [RFC PATCH v3 06/12] btrfs: remove dead snapshot-aware defrag code
From: Nikolay Borisov @ 2019-11-26 15:13 UTC (permalink / raw)
To: Omar Sandoval, linux-fsdevel, linux-btrfs
Cc: Dave Chinner, Jann Horn, Amir Goldstein, Aleksa Sarai, linux-api,
kernel-team
In-Reply-To: <a33a0fddb55101eec47b815672aa42b6f85b3830.1574273658.git.osandov@fb.com>
On 20.11.19 г. 20:24 ч., Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
>
> Snapshot-aware defrag has been disabled since commit 8101c8dbf624
> ("Btrfs: disable snapshot aware defrag for now") almost 6 years ago.
> Let's remove the dead code. If someone is up to the task of bringing it
> back, they can dig it up from git.
>
> This is logically a revert of commit 38c227d87c49 ("Btrfs:
> snapshot-aware defrag") except that now we have to clear the
> EXTENT_DEFRAG bit to avoid need_force_cow() returning true forever.
>
> Signed-off-by: Omar Sandoval <osandov@fb.com>
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
^ permalink raw reply
* Re: [RFC PATCH v3 03/12] fs: add RWF_ENCODED for reading/writing compressed data
From: Omar Sandoval @ 2019-11-26 17:36 UTC (permalink / raw)
To: Nikolay Borisov
Cc: linux-fsdevel, linux-btrfs, Dave Chinner, Jann Horn,
Amir Goldstein, Aleksa Sarai, linux-api, kernel-team
In-Reply-To: <d1886c1f-f19e-f3a7-32d6-8803a71a510c@suse.com>
On Tue, Nov 26, 2019 at 03:53:02PM +0200, Nikolay Borisov wrote:
>
>
> On 20.11.19 г. 20:24 ч., Omar Sandoval wrote:
> > From: Omar Sandoval <osandov@fb.com>
>
> <snip>
>
> >
> > +enum {
> > + ENCODED_IOV_COMPRESSION_NONE,
> > +#define ENCODED_IOV_COMPRESSION_NONE ENCODED_IOV_COMPRESSION_NONE
> > + ENCODED_IOV_COMPRESSION_ZLIB,
> > +#define ENCODED_IOV_COMPRESSION_ZLIB ENCODED_IOV_COMPRESSION_ZLIB
> > + ENCODED_IOV_COMPRESSION_LZO,
> > +#define ENCODED_IOV_COMPRESSION_LZO ENCODED_IOV_COMPRESSION_LZO
> > + ENCODED_IOV_COMPRESSION_ZSTD,
> > +#define ENCODED_IOV_COMPRESSION_ZSTD ENCODED_IOV_COMPRESSION_ZSTD
> > + ENCODED_IOV_COMPRESSION_TYPES = ENCODED_IOV_COMPRESSION_ZSTD,
>
> This looks very dodgy, what am I missing?
This is a somewhat common trick so that enum values can be checked for
with ifdef/ifndef. See include/uapi/linux.in.h, for example.
^ permalink raw reply
* Re: [RFC PATCH v3 04/12] btrfs: get rid of trivial __btrfs_lookup_bio_sums() wrappers
From: Omar Sandoval @ 2019-11-26 17:42 UTC (permalink / raw)
To: Nikolay Borisov
Cc: linux-fsdevel, linux-btrfs, Dave Chinner, Jann Horn,
Amir Goldstein, Aleksa Sarai, linux-api, kernel-team
In-Reply-To: <dc600214-0f19-b321-8573-6193b5f47e16@suse.com>
On Tue, Nov 26, 2019 at 03:56:31PM +0200, Nikolay Borisov wrote:
>
>
> On 20.11.19 г. 20:24 ч., Omar Sandoval wrote:
> > From: Omar Sandoval <osandov@fb.com>
> >
> > Currently, we have two wrappers for __btrfs_lookup_bio_sums():
> > btrfs_lookup_bio_sums_dio(), which is used for direct I/O, and
> > btrfs_lookup_bio_sums(), which is used everywhere else. The only
> > difference is that the _dio variant looks up csums starting at the given
> > offset instead of using the page index, which isn't actually direct
> > I/O-specific. Let's clean up the signature and return value of
> > __btrfs_lookup_bio_sums(), rename it to btrfs_lookup_bio_sums(), and get
> > rid of the trivial helpers.
> >
> > Signed-off-by: Omar Sandoval <osandov@fb.com>
>
> Overall looks good but 2 nits, see below.
>
> In any case:
>
> Reviewed-by: Nikolay Borisov <nborisov@suse.com>
Thanks!
> > ---
> > fs/btrfs/compression.c | 4 ++--
> > fs/btrfs/ctree.h | 4 +---
> > fs/btrfs/file-item.c | 35 +++++++++++++++++------------------
> > fs/btrfs/inode.c | 6 +++---
> > 4 files changed, 23 insertions(+), 26 deletions(-)
> >
> > diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
> > index b05b361e2062..4df6f0c58dc9 100644
> > --- a/fs/btrfs/compression.c
> > +++ b/fs/btrfs/compression.c
> > @@ -660,7 +660,7 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
> >
> > if (!(BTRFS_I(inode)->flags & BTRFS_INODE_NODATASUM)) {
> > ret = btrfs_lookup_bio_sums(inode, comp_bio,
> > - sums);
> > + false, 0, sums);
> > BUG_ON(ret); /* -ENOMEM */
> > }
> >
> > @@ -689,7 +689,7 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
> > BUG_ON(ret); /* -ENOMEM */
> >
> > if (!(BTRFS_I(inode)->flags & BTRFS_INODE_NODATASUM)) {
> > - ret = btrfs_lookup_bio_sums(inode, comp_bio, sums);
> > + ret = btrfs_lookup_bio_sums(inode, comp_bio, false, 0, sums);
> > BUG_ON(ret); /* -ENOMEM */
> > }
> >
> > diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> > index fe2b8765d9e6..4bc40bf49b0e 100644
> > --- a/fs/btrfs/ctree.h
> > +++ b/fs/btrfs/ctree.h
> > @@ -2787,9 +2787,7 @@ struct btrfs_dio_private;
> > int btrfs_del_csums(struct btrfs_trans_handle *trans,
> > struct btrfs_fs_info *fs_info, u64 bytenr, u64 len);
> > blk_status_t btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio,
> > - u8 *dst);
> > -blk_status_t btrfs_lookup_bio_sums_dio(struct inode *inode, struct bio *bio,
> > - u64 logical_offset);
> > + bool at_offset, u64 offset, u8 *dst);
> > int btrfs_insert_file_extent(struct btrfs_trans_handle *trans,
> > struct btrfs_root *root,
> > u64 objectid, u64 pos,
> > diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
> > index 1a599f50837b..a87c40502267 100644
> > --- a/fs/btrfs/file-item.c
> > +++ b/fs/btrfs/file-item.c
> > @@ -148,8 +148,21 @@ int btrfs_lookup_file_extent(struct btrfs_trans_handle *trans,
> > return ret;
> > }
> >
> > -static blk_status_t __btrfs_lookup_bio_sums(struct inode *inode, struct bio *bio,
> > - u64 logical_offset, u8 *dst, int dio)
> > +/**
> > + * btrfs_lookup_bio_sums - Look up checksums for a bio.
> > + * @inode: inode that the bio is for.
> > + * @bio: bio embedded in btrfs_io_bio.
> > + * @at_offset: If true, look up checksums for the extent at @c offset.
>
> nit: that @c is an editing artifact?
Oops, I mixed up kernel-doc with Doxygen. Fixed, thanks.
> On the other hand rather than
> having an explicit bool signifying whether we want a specific offset
> can't we simply check if offset is != 0 ?
Zero is a perfectly valid offset to have an extent at, but we could do
(u64)-1 instead. I'm not sure what's cleaner.
^ permalink raw reply
* Re: [RFC PATCH v3 05/12] btrfs: don't advance offset for compressed bios in btrfs_csum_one_bio()
From: Omar Sandoval @ 2019-11-26 17:50 UTC (permalink / raw)
To: Nikolay Borisov
Cc: linux-fsdevel, linux-btrfs, Dave Chinner, Jann Horn,
Amir Goldstein, Aleksa Sarai, linux-api, kernel-team
In-Reply-To: <9669273e-5a73-540f-2091-5ce64e093062@suse.com>
On Tue, Nov 26, 2019 at 04:18:45PM +0200, Nikolay Borisov wrote:
>
>
> On 20.11.19 г. 20:24 ч., Omar Sandoval wrote:
> > From: Omar Sandoval <osandov@fb.com>
> >
> > btrfs_csum_one_bio() loops over each sector in the bio while keeping a
>
> 'sector' here is ambiguous it really loops over every fs block (which in
> btrfs is also known as sector). SO perhaps change the wording in the
> changelog but also in the function instead of nr_sectors perhahps it
> could be renamed to blockcount?
Fixed, thanks.
^ permalink raw reply
* Re: [PATCH v26 12/12] LRNG - add interface for gathering of raw entropy (fwd)
From: Julia Lawall @ 2019-11-26 18:04 UTC (permalink / raw)
To: Stephan Müller
Cc: Greg Kroah-Hartman, linux-crypto, LKML, linux-api,
Eric W. Biederman, Alexander E. Patrakov, Ahmed S. Darwish,
Theodore Y. Ts'o, Willy Tarreau, Matthew Garrett, Vito Caputo,
Andreas Dilger, Jan Kara, Ray Strode, William Jon McCann, zhangjs,
Andy Lutomirski, Florian Weimer, Lennart Poettering,
Nicolai Stange, "Peter, Matthias" <matth>
[-- Attachment #1: Type: text/plain, Size: 5386 bytes --]
Hello,
Should something be done about the lock on line 162?
julia
---------- Forwarded message ----------
Date: Tue, 26 Nov 2019 01:12:49 +0800
From: kbuild test robot <lkp@intel.com>
To: kbuild@lists.01.org
Cc: Julia Lawall <julia.lawall@lip6.fr>
Subject: Re: [PATCH v26 12/12] LRNG - add interface for gathering of raw entropy
In-Reply-To: <3742813.K7GG538zxB@positron.chronox.de>
References: <3742813.K7GG538zxB@positron.chronox.de>
Hi "Stephan,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on char-misc/char-misc-testing]
[also build test WARNING on v5.4 next-20191125]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Stephan-M-ller/dev-random-a-new-approach-with-full-SP800-90B/20191125-042152
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git b78cda795ac83333293f1bfa3165572a47e550c2
:::::: branch date: 21 hours ago
:::::: commit date: 21 hours ago
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
Reported-by: Julia Lawall <julia.lawall@lip6.fr>
>> drivers/char/lrng/lrng_testing.c:176:1-7: preceding lock on line 134
# https://github.com/0day-ci/linux/commit/d69edb724ef2677392bfcbaf4fb6750306f8a1f1
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout d69edb724ef2677392bfcbaf4fb6750306f8a1f1
vim +176 drivers/char/lrng/lrng_testing.c
d69edb724ef267 Stephan Müller 2019-11-23 125
d69edb724ef267 Stephan Müller 2019-11-23 126 static int lrng_raw_entropy_reader(u8 *outbuf, u32 outbuflen)
d69edb724ef267 Stephan Müller 2019-11-23 127 {
d69edb724ef267 Stephan Müller 2019-11-23 128 unsigned long flags;
d69edb724ef267 Stephan Müller 2019-11-23 129 int collected_data = 0;
d69edb724ef267 Stephan Müller 2019-11-23 130
d69edb724ef267 Stephan Müller 2019-11-23 131 lrng_raw_entropy_init();
d69edb724ef267 Stephan Müller 2019-11-23 132
d69edb724ef267 Stephan Müller 2019-11-23 133 while (outbuflen) {
d69edb724ef267 Stephan Müller 2019-11-23 @134 spin_lock_irqsave(&lrng_raw_lock, flags);
d69edb724ef267 Stephan Müller 2019-11-23 135
d69edb724ef267 Stephan Müller 2019-11-23 136 /* We have no data or reached the writer. */
d69edb724ef267 Stephan Müller 2019-11-23 137 if (!lrng_rb_writer || (lrng_rb_writer == lrng_rb_reader)) {
d69edb724ef267 Stephan Müller 2019-11-23 138
d69edb724ef267 Stephan Müller 2019-11-23 139 spin_unlock_irqrestore(&lrng_raw_lock, flags);
d69edb724ef267 Stephan Müller 2019-11-23 140
d69edb724ef267 Stephan Müller 2019-11-23 141 /*
d69edb724ef267 Stephan Müller 2019-11-23 142 * Now we gathered all boot data, enable regular data
d69edb724ef267 Stephan Müller 2019-11-23 143 * collection.
d69edb724ef267 Stephan Müller 2019-11-23 144 */
d69edb724ef267 Stephan Müller 2019-11-23 145 if (boot_test) {
d69edb724ef267 Stephan Müller 2019-11-23 146 boot_test = 0;
d69edb724ef267 Stephan Müller 2019-11-23 147 goto out;
d69edb724ef267 Stephan Müller 2019-11-23 148 }
d69edb724ef267 Stephan Müller 2019-11-23 149
d69edb724ef267 Stephan Müller 2019-11-23 150 wait_event_interruptible(lrng_raw_read_wait,
d69edb724ef267 Stephan Müller 2019-11-23 151 lrng_raw_have_data());
d69edb724ef267 Stephan Müller 2019-11-23 152 if (signal_pending(current)) {
d69edb724ef267 Stephan Müller 2019-11-23 153 collected_data = -ERESTARTSYS;
d69edb724ef267 Stephan Müller 2019-11-23 154 goto out;
d69edb724ef267 Stephan Müller 2019-11-23 155 }
d69edb724ef267 Stephan Müller 2019-11-23 156
d69edb724ef267 Stephan Müller 2019-11-23 157 continue;
d69edb724ef267 Stephan Müller 2019-11-23 158 }
d69edb724ef267 Stephan Müller 2019-11-23 159
d69edb724ef267 Stephan Müller 2019-11-23 160 /* We copy out word-wise */
d69edb724ef267 Stephan Müller 2019-11-23 161 if (outbuflen < sizeof(u32))
d69edb724ef267 Stephan Müller 2019-11-23 162 goto out;
d69edb724ef267 Stephan Müller 2019-11-23 163
d69edb724ef267 Stephan Müller 2019-11-23 164 memcpy(outbuf, &lrng_testing_rb[lrng_rb_reader], sizeof(u32));
d69edb724ef267 Stephan Müller 2019-11-23 165 lrng_rb_reader++;
d69edb724ef267 Stephan Müller 2019-11-23 166
d69edb724ef267 Stephan Müller 2019-11-23 167 spin_unlock_irqrestore(&lrng_raw_lock, flags);
d69edb724ef267 Stephan Müller 2019-11-23 168
d69edb724ef267 Stephan Müller 2019-11-23 169 outbuf += sizeof(u32);
d69edb724ef267 Stephan Müller 2019-11-23 170 outbuflen -= sizeof(u32);
d69edb724ef267 Stephan Müller 2019-11-23 171 collected_data += sizeof(u32);
d69edb724ef267 Stephan Müller 2019-11-23 172 }
d69edb724ef267 Stephan Müller 2019-11-23 173
d69edb724ef267 Stephan Müller 2019-11-23 174 out:
d69edb724ef267 Stephan Müller 2019-11-23 175 lrng_raw_entropy_fini();
d69edb724ef267 Stephan Müller 2019-11-23 @176 return collected_data;
d69edb724ef267 Stephan Müller 2019-11-23 177 }
d69edb724ef267 Stephan Müller 2019-11-23 178
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation
^ permalink raw reply
* Re: [PATCH v26 12/12] LRNG - add interface for gathering of raw entropy (fwd)
From: Stephan Mueller @ 2019-11-26 19:27 UTC (permalink / raw)
To: Julia Lawall
Cc: Greg Kroah-Hartman, linux-crypto, LKML, linux-api,
Eric W. Biederman, Alexander E. Patrakov, Ahmed S. Darwish,
Theodore Y. Ts'o, Willy Tarreau, Matthew Garrett, Vito Caputo,
Andreas Dilger, Jan Kara, Ray Strode, William Jon McCann, zhangjs,
Andy Lutomirski, Florian Weimer, Lennart Poettering,
Nicolai Stange, "Peter, Matthias" <matth>
In-Reply-To: <alpine.DEB.2.21.1911261903060.2605@hadrien>
Am Dienstag, 26. November 2019, 19:04:10 CET schrieb Julia Lawall:
Hi Julia,
> Hello,
>
> Should something be done about the lock on line 162?
Yes, thank you for the report. There should be an unlock, naturally.
Ciao
Stephan
^ permalink raw reply
* [GIT PULL] pipe: General notification queue
From: David Howells @ 2019-11-26 23:18 UTC (permalink / raw)
To: torvalds
Cc: dhowells, Rasmus Villemoes, Greg Kroah-Hartman, Peter Zijlstra,
raven, Christian Brauner, keyrings, linux-usb, linux-block,
linux-security-module, linux-fsdevel, linux-api, linux-kernel
Hi Linus,
Can you consider pulling my general notification queue patchset after
you've pulled the preparatory pipework patchset? Or should it be deferred
to the next window?
The general notification queue is built on top a pipe, splicing kernel
events into the pipe from a variety of event sources. A 'notification'
mode pipe must be linked to the desired sources before it will start
getting any events delivered through it. Event sources include:
(1) Keys/keyrings, such as linking and unlinking keys and changing their
attributes.
(2) General device events (single common queue) including:
- Block layer events, such as device errors
- USB subsystem events, such as device attach/remove, device reset,
device errors.
(3) I have patches for adding superblock and mount topology watches also,
but in a separate branch as there are other dependencies.
LSM hooks are included:
(1) A set of hooks are provided that allow an LSM to rule on whether or
not a watch may be set. Each of these hooks takes a different
"watched object" parameter, so they're not really shareable. The LSM
should use current's credentials. [Wanted by SELinux & Smack]
(2) A hook is provided to allow an LSM to rule on whether or not a
particular message may be posted to a particular queue. This is given
the credentials from the event generator (which may be the system) and
the watch setter. [Wanted by Smack]
I've provided SELinux and Smack with implementations of some of these hooks.
USE CASES
=========
(1) Key/keyring notifications.
If you have your kerberos tickets in a file/directory, your gnome desktop
will monitor that using something like fanotify and tell you if your
credentials cache changes.
We also have the ability to cache your kerberos tickets in the session,
user or persistent keyring so that it isn't left around on disk across a
reboot or logout. Keyrings, however, cannot currently be monitored
asynchronously, so the desktop has to poll for it - not so good on a
laptop.
This source will allow the desktop to avoid the need to poll.
(2) USB notifications.
GregKH was looking for a way to do USB notifications as I was looking to
find additional sources to implement. I'm not sure how he wants to use
them, but I'll let him speak to that himself.
(3) Block notifications.
This one I was thinking that I could make something like ddrescue better
by letting it get notifications this way. This was a target of
convenience since I had a dodgy disk I was trying to rescue.
It could also potentially be used help systemd, say, detect broken
devices and avoid trying to unmount them when trying to reboot the machine.
I can drop this for now if you prefer.
(4) Mount notifications.
This one is wanted to avoid repeated trawling of /proc/mounts or similar
to work out changes to the mount object attributes and mount topology.
I'm told that the proc file holding the namespace_sem is a point of
contention, especially as the process of generating the text descriptions
of the mounts/superblocks can be quite involved.
The notifications directly indicate the mounts involved in any particular
event and what the change was. You can poll /proc/mounts, but all you
know is that something changed; you don't know what and you don't know
how and reading that file may race with multiple changed being effected.
I pair this with a new fsinfo() system call that allows, amongst other
things, the ability to retrieve in one go an { id, change counter } tuple
from all the children of a specified mount, allowing buffer overruns to
be cleaned up quickly.
It's not just Red Hat that's potentially interested in this:
https://lore.kernel.org/linux-fsdevel/293c9bd3-f530-d75e-c353-ddeabac27cf6@6wind.com/
(5) Superblock notifications.
This one is provided to allow systemd or the desktop to more easily
detect events such as I/O errors and EDQUOT/ENOSPC.
DESIGN DECISIONS
================
(1) The notification queue is built on top of a standard pipe. Messages
are effectively spliced in. The pipe is opened with a special flag:
pipe2(fds, O_NOTIFICATION_PIPE);
The special flag has the same value as O_EXCL (which doesn't seem like
it will ever be applicable in this context)[?]. It is given up front
to make it a lot easier to prohibit splice and co. from accessing the
pipe.
[?] Should this be done some other way? I'd rather not use up a new
O_* flag if I can avoid it - should I add a pipe3() system call
instead?
The pipe is then configured::
ioctl(fds[1], IOC_WATCH_QUEUE_SET_SIZE, queue_depth);
ioctl(fds[1], IOC_WATCH_QUEUE_SET_FILTER, &filter);
Messages are then read out of the pipe using read().
[?] Should this use fcntl() rather than ioctl()?
(2) It should be possible to allow write() to insert data into the
notification pipes too, but this is currently disabled as the kernel
has to be able to insert messages into the pipe *without* holding
pipe->mutex and the code to make this work needs careful auditing.
(3) sendfile(), splice() and vmsplice() are disabled on notification pipes
because of the pipe->mutex issue and also because they sometimes want
to revert what they just did - but one or more notification messages
might've been interleaved in the ring.
(4) The kernel inserts messages with the wait queue spinlock held. This
means that pipe_read() and pipe_write() have to take the spinlock to
update the queue pointers.
(5) Records in the buffer are binary, typed and have a length so that they
can be of varying size.
This allows multiple heterogeneous sources to share a common buffer;
there are 16 million types available, of which I've used just a few,
so there is scope for others to be used. Tags may be specified when a
watchpoint is created to help distinguish the sources.
(6) Records are filterable as types have up to 256 subtypes that can be
individually filtered. Other filtration is also available.
(7) Notification pipes don't interfere with each other; each may be bound
to a different set of event sources. Any particular notification will
be copied to all the queues that are currently watching for it - and
only those that are watching for it.
(8) When recording a notification, the kernel will not sleep, but will
rather mark a queue as having lost a message if there's insufficient
space. read() will fabricate a loss notification message at an
appropriate point later.
(9) The notification pipe is created and then watchpoints are attached to
it. Examples of this include:
keyctl_watch_key(KEY_SPEC_SESSION_KEYRING, fds[1], 0x01);
watch_devices(fds[1], 0x02, 0);
where in both cases, fd indicates the queue and the number after is a
tag between 0 and 255.
(10) Watches are removed if either the notification pipe is destroyed or
the watched object is destroyed. In the latter case, a message will
be generated indicating the enforced watch removal.
Things I want to avoid:
(1) Introducing features that make the core VFS dependent on the network
stack or networking namespaces (ie. usage of netlink).
(2) Dumping all this stuff into dmesg and having a daemon that sits there
parsing the output and distributing it as this then puts the
responsibility for security into userspace and makes handling
namespaces tricky. Further, dmesg might not exist or might be
inaccessible inside a container.
(3) Letting users see events they shouldn't be able to see.
TESTING AND MANPAGES
====================
(*) The keyutils tree has a pipe-watch branch that has keyctl commands for
making use of notifications. Proposed manual pages can also be found
on this branch, though a couple of them really need to go to the main
manpages repository instead.
If the kernel supports the watching of keys, then running "make test"
on that branch will cause the testing infrastructure to spawn a
monitoring process on the side that monitors a notifications pipe for
all the key/keyring changes induced by the tests and they'll all be
checked off to make sure they happened.
https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/keyutils.git/log/?h=pipe-watch
Changes:
(*) Fix some bits found by kbuild.
ver #2:
(*) Declare O_NOTIFICATION_PIPE to use and switch it to be the same value
as O_EXCL rather then O_TMPFILE (the latter is a bit nasty in its
implementation).
ver #1:
(*) Build on top of standard pipes instead of having a driver.
David
---
The following changes since commit 3c0edea9b29f9be6c093f236f762202b30ac9431:
pipe: Remove sync on wake_ups (2019-11-15 16:22:54 +0000)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git tags/notifications-pipe-core-20191115
for you to fetch changes up to 72fb99936e50f6c17490aeb445c87406a1265d52:
smack: Implement the watch_key and post_notification hooks (2019-11-15 16:23:58 +0000)
----------------------------------------------------------------
General notification queue built on pipes
----------------------------------------------------------------
David Howells (14):
uapi: General notification queue definitions
security: Add hooks to rule on setting a watch
security: Add a hook for the point of notification insertion
pipe: Add O_NOTIFICATION_PIPE
pipe: Add general notification queue support
keys: Add a notification facility
Add sample notification program
pipe: Allow buffers to be marked read-whole-or-error for notifications
pipe: Add notification lossage handling
Add a general, global device notification watch list
block: Add block layer notifications
usb: Add USB subsystem notifications
selinux: Implement the watch_key security hook
smack: Implement the watch_key and post_notification hooks
Documentation/ioctl/ioctl-number.rst | 1 +
Documentation/security/keys/core.rst | 58 +++
Documentation/watch_queue.rst | 385 ++++++++++++++++
arch/alpha/kernel/syscalls/syscall.tbl | 1 +
arch/arm/tools/syscall.tbl | 1 +
arch/arm64/include/asm/unistd.h | 2 +-
arch/arm64/include/asm/unistd32.h | 2 +
arch/ia64/kernel/syscalls/syscall.tbl | 1 +
arch/m68k/kernel/syscalls/syscall.tbl | 1 +
arch/microblaze/kernel/syscalls/syscall.tbl | 1 +
arch/mips/kernel/syscalls/syscall_n32.tbl | 1 +
arch/mips/kernel/syscalls/syscall_n64.tbl | 1 +
arch/mips/kernel/syscalls/syscall_o32.tbl | 1 +
arch/parisc/kernel/syscalls/syscall.tbl | 1 +
arch/powerpc/kernel/syscalls/syscall.tbl | 1 +
arch/s390/kernel/syscalls/syscall.tbl | 1 +
arch/sh/kernel/syscalls/syscall.tbl | 1 +
arch/sparc/kernel/syscalls/syscall.tbl | 1 +
arch/x86/entry/syscalls/syscall_32.tbl | 1 +
arch/x86/entry/syscalls/syscall_64.tbl | 1 +
arch/xtensa/kernel/syscalls/syscall.tbl | 1 +
block/Kconfig | 9 +
block/blk-core.c | 29 ++
drivers/base/Kconfig | 9 +
drivers/base/Makefile | 1 +
drivers/base/watch.c | 90 ++++
drivers/usb/core/Kconfig | 9 +
drivers/usb/core/devio.c | 47 ++
drivers/usb/core/hub.c | 4 +
fs/pipe.c | 242 +++++++---
fs/splice.c | 12 +-
include/linux/blkdev.h | 15 +
include/linux/device.h | 7 +
include/linux/key.h | 3 +
include/linux/lsm_audit.h | 1 +
include/linux/lsm_hooks.h | 38 ++
include/linux/pipe_fs_i.h | 27 +-
include/linux/security.h | 32 +-
include/linux/syscalls.h | 1 +
include/linux/usb.h | 18 +
include/linux/watch_queue.h | 127 ++++++
include/uapi/asm-generic/unistd.h | 4 +-
include/uapi/linux/keyctl.h | 2 +
include/uapi/linux/watch_queue.h | 158 +++++++
init/Kconfig | 12 +
kernel/Makefile | 1 +
kernel/sys_ni.c | 1 +
kernel/watch_queue.c | 658 ++++++++++++++++++++++++++++
samples/Kconfig | 7 +
samples/Makefile | 1 +
samples/watch_queue/Makefile | 7 +
samples/watch_queue/watch_test.c | 251 +++++++++++
security/keys/Kconfig | 9 +
security/keys/compat.c | 3 +
security/keys/gc.c | 5 +
security/keys/internal.h | 30 +-
security/keys/key.c | 38 +-
security/keys/keyctl.c | 99 ++++-
security/keys/keyring.c | 20 +-
security/keys/request_key.c | 4 +-
security/security.c | 23 +
security/selinux/hooks.c | 14 +
security/smack/smack_lsm.c | 82 +++-
63 files changed, 2506 insertions(+), 108 deletions(-)
create mode 100644 Documentation/watch_queue.rst
create mode 100644 drivers/base/watch.c
create mode 100644 include/linux/watch_queue.h
create mode 100644 include/uapi/linux/watch_queue.h
create mode 100644 kernel/watch_queue.c
create mode 100644 samples/watch_queue/Makefile
create mode 100644 samples/watch_queue/watch_test.c
^ permalink raw reply
* Re: [PATCH v6 2/2] fpga: dfl: fme: add performance reporting support
From: Moritz Fischer @ 2019-11-27 1:45 UTC (permalink / raw)
To: Wu Hao
Cc: mdf, will, mark.rutland, linux-fpga, linux-kernel, linux-api,
atull, gregkh, Luwei Kang, Xu Yilun
In-Reply-To: <1573622695-25607-3-git-send-email-hao.wu@intel.com>
Hi Hao,
On Wed, Nov 13, 2019 at 01:24:55PM +0800, Wu Hao wrote:
> This patch adds support for performance reporting private feature
> for FPGA Management Engine (FME). Now it supports several different
> performance counters, including 'basic', 'cache', 'fabric', 'vtd'
> and 'vtd_sip'. It allows user to use standard linux tools to access
> these performance counters.
>
> e.g. List all events by "perf list"
>
> perf list | grep fme
>
> fme0/cache_read_hit/ [Kernel PMU event]
> fme0/cache_read_miss/ [Kernel PMU event]
> ...
>
> fme0/fab_mmio_read/ [Kernel PMU event]
> fme0/fab_mmio_write/ [Kernel PMU event]
> ...
>
> fme0/fab_port_mmio_read,portid=?/ [Kernel PMU event]
> fme0/fab_port_mmio_write,portid=?/ [Kernel PMU event]
> ...
>
> fme0/vtd_port_devtlb_1g_fill,portid=?/ [Kernel PMU event]
> fme0/vtd_port_devtlb_2m_fill,portid=?/ [Kernel PMU event]
> ...
>
> fme0/vtd_sip_iotlb_1g_hit/ [Kernel PMU event]
> fme0/vtd_sip_iotlb_1g_miss/ [Kernel PMU event]
> ...
>
> fme0/clock [Kernel PMU event]
> ...
>
> e.g. check increased counter value after run one application using
> "perf stat" command.
>
> perf stat -e fme0/fab_mmio_read/,fme0/fab_mmio_write/, ./test
>
> Performance counter stats for './test':
>
> 1 fme0/fab_mmio_read/
> 2 fme0/fab_mmio_write/
>
> 1.009496520 seconds time elapsed
>
> Please note that fabric counters support both fab_* and fab_port_*, but
> actually they are sharing one set of performance counters in hardware.
> If user wants to monitor overall data events on fab_* then fab_port_*
> can't be supported at the same time, see example below:
>
> perf stat -e fme0/fab_mmio_read/,fme0/fab_port_mmio_write,portid=0/
>
> Performance counter stats for 'system wide':
>
> 0 fme0/fab_mmio_read/
> <not supported> fme0/fab_port_mmio_write,portid=0/
>
> 2.141064085 seconds time elapsed
>
> Signed-off-by: Luwei Kang <luwei.kang@intel.com>
> Signed-off-by: Xu Yilun <yilun.xu@intel.com>
> Signed-off-by: Wu Hao <hao.wu@intel.com>
> ---
> v3: replace scnprintf with sprintf in sysfs interfaces.
> update sysfs doc kernel version and date.
> fix sysfs doc issue for fabric counter.
> refine PERF_OBJ_ATTR_* macro, doesn't count on __ATTR anymore.
> introduce PERF_OBJ_ATTR_F_* macro, as it needs to use different
> filenames for some of the sysfs attributes.
> remove kobject_del when destroy kobject, kobject_put is enough.
> do sysfs_remove_groups first when destroying perf_obj.
> WARN_ON_ONCE in case internal parms are wrong in read_*_count().
> v4: rework this patch to use standard perf API as user interfaces.
> v5: rebase and clean up.
> v6: use dev_ext_attribute instead of creating new fme_perf_attribute
> use is_visible function to decide which events to expose per
> hardware capability, and add event_init checking for all events.
> ---
> drivers/fpga/Makefile | 1 +
> drivers/fpga/dfl-fme-main.c | 4 +
> drivers/fpga/dfl-fme-perf.c | 943 ++++++++++++++++++++++++++++++++++++++++++++
> drivers/fpga/dfl-fme.h | 2 +
> drivers/fpga/dfl.h | 2 +
> 5 files changed, 952 insertions(+)
> create mode 100644 drivers/fpga/dfl-fme-perf.c
>
> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> index 4865b74..d8e21df 100644
> --- a/drivers/fpga/Makefile
> +++ b/drivers/fpga/Makefile
> @@ -40,6 +40,7 @@ obj-$(CONFIG_FPGA_DFL_FME_REGION) += dfl-fme-region.o
> obj-$(CONFIG_FPGA_DFL_AFU) += dfl-afu.o
>
> dfl-fme-objs := dfl-fme-main.o dfl-fme-pr.o dfl-fme-error.o
> +dfl-fme-objs += dfl-fme-perf.o
> dfl-afu-objs := dfl-afu-main.o dfl-afu-region.o dfl-afu-dma-region.o
> dfl-afu-objs += dfl-afu-error.o
>
> diff --git a/drivers/fpga/dfl-fme-main.c b/drivers/fpga/dfl-fme-main.c
> index 7c930e6..53a1447 100644
> --- a/drivers/fpga/dfl-fme-main.c
> +++ b/drivers/fpga/dfl-fme-main.c
> @@ -580,6 +580,10 @@ static int fme_power_mgmt_init(struct platform_device *pdev,
> .ops = &fme_power_mgmt_ops,
> },
> {
> + .id_table = fme_perf_id_table,
> + .ops = &fme_perf_ops,
> + },
> + {
> .ops = NULL,
> },
> };
> diff --git a/drivers/fpga/dfl-fme-perf.c b/drivers/fpga/dfl-fme-perf.c
> new file mode 100644
> index 0000000..5e51727
> --- /dev/null
> +++ b/drivers/fpga/dfl-fme-perf.c
> @@ -0,0 +1,943 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for FPGA Management Engine (FME) Global Performance Reporting
> + *
> + * Copyright 2019 Intel Corporation, Inc.
> + *
> + * Authors:
> + * Kang Luwei <luwei.kang@intel.com>
> + * Xiao Guangrong <guangrong.xiao@linux.intel.com>
> + * Wu Hao <hao.wu@intel.com>
> + * Xu Yilun <yilun.xu@intel.com>
> + * Joseph Grecco <joe.grecco@intel.com>
> + * Enno Luebbers <enno.luebbers@intel.com>
> + * Tim Whisonant <tim.whisonant@intel.com>
> + * Ananda Ravuri <ananda.ravuri@intel.com>
> + * Mitchel, Henry <henry.mitchel@intel.com>
> + */
> +
> +#include <linux/perf_event.h>
> +#include "dfl.h"
> +#include "dfl-fme.h"
> +
> +/*
> + * Performance Counter Registers for Cache.
> + *
> + * Cache Events are listed below as CACHE_EVNT_*.
> + */
> +#define CACHE_CTRL 0x8
> +#define CACHE_RESET_CNTR BIT_ULL(0)
> +#define CACHE_FREEZE_CNTR BIT_ULL(8)
> +#define CACHE_CTRL_EVNT GENMASK_ULL(19, 16)
> +#define CACHE_EVNT_RD_HIT 0x0
> +#define CACHE_EVNT_WR_HIT 0x1
> +#define CACHE_EVNT_RD_MISS 0x2
> +#define CACHE_EVNT_WR_MISS 0x3
> +#define CACHE_EVNT_RSVD 0x4
> +#define CACHE_EVNT_HOLD_REQ 0x5
> +#define CACHE_EVNT_DATA_WR_PORT_CONTEN 0x6
> +#define CACHE_EVNT_TAG_WR_PORT_CONTEN 0x7
> +#define CACHE_EVNT_TX_REQ_STALL 0x8
> +#define CACHE_EVNT_RX_REQ_STALL 0x9
> +#define CACHE_EVNT_EVICTIONS 0xa
> +#define CACHE_EVNT_MAX CACHE_EVNT_EVICTIONS
> +#define CACHE_CHANNEL_SEL BIT_ULL(20)
> +#define CACHE_CHANNEL_RD 0
> +#define CACHE_CHANNEL_WR 1
> +#define CACHE_CNTR0 0x10
> +#define CACHE_CNTR1 0x18
> +#define CACHE_CNTR_EVNT_CNTR GENMASK_ULL(47, 0)
> +#define CACHE_CNTR_EVNT GENMASK_ULL(63, 60)
> +
> +/*
> + * Performance Counter Registers for Fabric.
> + *
> + * Fabric Events are listed below as FAB_EVNT_*
> + */
> +#define FAB_CTRL 0x20
> +#define FAB_RESET_CNTR BIT_ULL(0)
> +#define FAB_FREEZE_CNTR BIT_ULL(8)
> +#define FAB_CTRL_EVNT GENMASK_ULL(19, 16)
> +#define FAB_EVNT_PCIE0_RD 0x0
> +#define FAB_EVNT_PCIE0_WR 0x1
> +#define FAB_EVNT_PCIE1_RD 0x2
> +#define FAB_EVNT_PCIE1_WR 0x3
> +#define FAB_EVNT_UPI_RD 0x4
> +#define FAB_EVNT_UPI_WR 0x5
> +#define FAB_EVNT_MMIO_RD 0x6
> +#define FAB_EVNT_MMIO_WR 0x7
> +#define FAB_EVNT_MAX FAB_EVNT_MMIO_WR
> +#define FAB_PORT_ID GENMASK_ULL(21, 20)
> +#define FAB_PORT_FILTER BIT_ULL(23)
> +#define FAB_PORT_FILTER_DISABLE 0
> +#define FAB_PORT_FILTER_ENABLE 1
> +#define FAB_CNTR 0x28
> +#define FAB_CNTR_EVNT_CNTR GENMASK_ULL(59, 0)
> +#define FAB_CNTR_EVNT GENMASK_ULL(63, 60)
> +
> +/*
> + * Performance Counter Registers for Clock.
> + *
> + * Clock Counter can't be reset or frozen by SW.
> + */
> +#define CLK_CNTR 0x30
> +#define BASIC_EVNT_CLK 0x0
> +#define BASIC_EVNT_MAX BASIC_EVNT_CLK
> +
> +/*
> + * Performance Counter Registers for IOMMU / VT-D.
> + *
> + * VT-D Events are listed below as VTD_EVNT_* and VTD_SIP_EVNT_*
> + */
> +#define VTD_CTRL 0x38
> +#define VTD_RESET_CNTR BIT_ULL(0)
> +#define VTD_FREEZE_CNTR BIT_ULL(8)
> +#define VTD_CTRL_EVNT GENMASK_ULL(19, 16)
> +#define VTD_EVNT_AFU_MEM_RD_TRANS 0x0
> +#define VTD_EVNT_AFU_MEM_WR_TRANS 0x1
> +#define VTD_EVNT_AFU_DEVTLB_RD_HIT 0x2
> +#define VTD_EVNT_AFU_DEVTLB_WR_HIT 0x3
> +#define VTD_EVNT_DEVTLB_4K_FILL 0x4
> +#define VTD_EVNT_DEVTLB_2M_FILL 0x5
> +#define VTD_EVNT_DEVTLB_1G_FILL 0x6
> +#define VTD_EVNT_MAX VTD_EVNT_DEVTLB_1G_FILL
> +#define VTD_CNTR 0x40
> +#define VTD_CNTR_EVNT_CNTR GENMASK_ULL(47, 0)
> +#define VTD_CNTR_EVNT GENMASK_ULL(63, 60)
> +
> +#define VTD_SIP_CTRL 0x48
> +#define VTD_SIP_RESET_CNTR BIT_ULL(0)
> +#define VTD_SIP_FREEZE_CNTR BIT_ULL(8)
> +#define VTD_SIP_CTRL_EVNT GENMASK_ULL(19, 16)
> +#define VTD_SIP_EVNT_IOTLB_4K_HIT 0x0
> +#define VTD_SIP_EVNT_IOTLB_2M_HIT 0x1
> +#define VTD_SIP_EVNT_IOTLB_1G_HIT 0x2
> +#define VTD_SIP_EVNT_SLPWC_L3_HIT 0x3
> +#define VTD_SIP_EVNT_SLPWC_L4_HIT 0x4
> +#define VTD_SIP_EVNT_RCC_HIT 0x5
> +#define VTD_SIP_EVNT_IOTLB_4K_MISS 0x6
> +#define VTD_SIP_EVNT_IOTLB_2M_MISS 0x7
> +#define VTD_SIP_EVNT_IOTLB_1G_MISS 0x8
> +#define VTD_SIP_EVNT_SLPWC_L3_MISS 0x9
> +#define VTD_SIP_EVNT_SLPWC_L4_MISS 0xa
> +#define VTD_SIP_EVNT_RCC_MISS 0xb
> +#define VTD_SIP_EVNT_MAX VTD_SIP_EVNT_SLPWC_L4_MISS
> +#define VTD_SIP_CNTR 0X50
> +#define VTD_SIP_CNTR_EVNT_CNTR GENMASK_ULL(47, 0)
> +#define VTD_SIP_CNTR_EVNT GENMASK_ULL(63, 60)
> +
> +#define PERF_TIMEOUT 30
> +
> +#define PERF_MAX_PORT_NUM 1U
> +
> +/**
> + * struct fme_perf_priv - priv data structure for fme perf driver
> + *
> + * @dev: parent device.
> + * @ioaddr: mapped base address of mmio region.
> + * @pmu: pmu data structure for fme perf counters.
> + * @id: id of this fme performance report private feature.
> + * @fab_users: current user number on fabric counters.
> + * @fab_port_id: used to indicate current working mode of fabric counters.
> + * @fab_lock: lock to protect fabric counters working mode.
> + */
> +struct fme_perf_priv {
> + struct device *dev;
> + void __iomem *ioaddr;
> + struct pmu pmu;
> + u64 id;
> +
> + u32 fab_users;
> + u32 fab_port_id;
> + spinlock_t fab_lock;
> +};
> +
> +/**
> + * struct fme_perf_event_ops - callbacks for fme perf events
> + *
> + * @event_init: callback invoked during event init.
> + * @event_destroy: callback invoked during event destroy.
> + * @read_counter: callback to read hardware counters.
> + */
> +struct fme_perf_event_ops {
> + int (*event_init)(struct fme_perf_priv *priv, u32 event, u32 portid);
> + void (*event_destroy)(struct fme_perf_priv *priv, u32 event,
> + u32 portid);
> + u64 (*read_counter)(struct fme_perf_priv *priv, u32 event, u32 portid);
> +};
> +
> +#define to_fme_perf_priv(_pmu) container_of(_pmu, struct fme_perf_priv, pmu)
> +
> +static cpumask_t fme_perf_cpumask = CPU_MASK_CPU0;
> +
> +static ssize_t cpumask_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + return cpumap_print_to_pagebuf(true, buf, &fme_perf_cpumask);
> +}
> +static DEVICE_ATTR_RO(cpumask);
> +
> +static struct attribute *fme_perf_cpumask_attrs[] = {
> + &dev_attr_cpumask.attr,
> + NULL,
> +};
> +
> +static struct attribute_group fme_perf_cpumask_group = {
> + .attrs = fme_perf_cpumask_attrs,
> +};
> +
> +#define FME_EVENT_MASK GENMASK_ULL(11, 0)
> +#define FME_EVENT_SHIFT 0
> +#define FME_EVTYPE_MASK GENMASK_ULL(15, 12)
> +#define FME_EVTYPE_SHIFT 12
> +#define FME_EVTYPE_BASIC 0
> +#define FME_EVTYPE_CACHE 1
> +#define FME_EVTYPE_FABRIC 2
> +#define FME_EVTYPE_VTD 3
> +#define FME_EVTYPE_VTD_SIP 4
> +#define FME_EVTYPE_MAX FME_EVTYPE_VTD_SIP
> +#define FME_PORTID_MASK GENMASK_ULL(23, 16)
> +#define FME_PORTID_SHIFT 16
> +#define FME_PORTID_ROOT (0xffU)
> +
> +#define get_event(_config) FIELD_GET(FME_EVENT_MASK, _config)
> +#define get_evtype(_config) FIELD_GET(FME_EVTYPE_MASK, _config)
> +#define get_portid(_config) FIELD_GET(FME_PORTID_MASK, _config)
> +
> +PMU_FORMAT_ATTR(event, "config:0-11");
> +PMU_FORMAT_ATTR(evtype, "config:12-15");
> +PMU_FORMAT_ATTR(portid, "config:16-23");
> +
> +static struct attribute *fme_perf_format_attrs[] = {
> + &format_attr_event.attr,
> + &format_attr_evtype.attr,
> + &format_attr_portid.attr,
> + NULL,
> +};
> +
> +static struct attribute_group fme_perf_format_group = {
> + .name = "format",
> + .attrs = fme_perf_format_attrs,
> +};
> +
> +static struct attribute *fme_perf_events_attrs_empty[] = {
> + NULL,
> +};
> +
> +static struct attribute_group fme_perf_events_group = {
> + .name = "events",
> + .attrs = fme_perf_events_attrs_empty,
> +};
> +
> +static const struct attribute_group *fme_perf_groups[] = {
> + &fme_perf_format_group,
> + &fme_perf_cpumask_group,
> + &fme_perf_events_group,
> + NULL,
> +};
> +
> +static bool is_portid_root(u32 portid)
> +{
> + return portid == FME_PORTID_ROOT;
> +}
> +
> +static bool is_portid_port(u32 portid)
> +{
> + return portid < PERF_MAX_PORT_NUM;
> +}
> +
> +static bool is_portid_root_or_port(u32 portid)
> +{
> + return is_portid_root(portid) || is_portid_port(portid);
> +}
> +
> +static int basic_event_init(struct fme_perf_priv *priv, u32 event, u32 portid)
> +{
> + if (event <= BASIC_EVNT_MAX && is_portid_root(portid))
> + return 0;
> +
> + return -EINVAL;
> +}
> +
> +static u64 basic_read_event_counter(struct fme_perf_priv *priv,
> + u32 event, u32 portid)
> +{
> + void __iomem *base = priv->ioaddr;
> +
> + return readq(base + CLK_CNTR);
> +}
> +
> +static int cache_event_init(struct fme_perf_priv *priv, u32 event, u32 portid)
> +{
> + if (priv->id == FME_FEATURE_ID_GLOBAL_IPERF &&
> + event <= CACHE_EVNT_MAX && is_portid_root(portid))
> + return 0;
> +
> + return -EINVAL;
> +}
> +
> +static u64 cache_read_event_counter(struct fme_perf_priv *priv,
> + u32 event, u32 portid)
> +{
> + void __iomem *base = priv->ioaddr;
> + u64 v, count;
> + u8 channel;
> +
> + if (event == CACHE_EVNT_WR_HIT || event == CACHE_EVNT_WR_MISS ||
> + event == CACHE_EVNT_DATA_WR_PORT_CONTEN ||
> + event == CACHE_EVNT_TAG_WR_PORT_CONTEN)
> + channel = CACHE_CHANNEL_WR;
> + else
> + channel = CACHE_CHANNEL_RD;
> +
> + /* set channel access type and cache event code. */
> + v = readq(base + CACHE_CTRL);
> + v &= ~(CACHE_CHANNEL_SEL | CACHE_CTRL_EVNT);
> + v |= FIELD_PREP(CACHE_CHANNEL_SEL, channel);
> + v |= FIELD_PREP(CACHE_CTRL_EVNT, event);
> + writeq(v, base + CACHE_CTRL);
> +
> + if (readq_poll_timeout_atomic(base + CACHE_CNTR0, v,
> + FIELD_GET(CACHE_CNTR_EVNT, v) == event,
> + 1, PERF_TIMEOUT)) {
> + dev_err(priv->dev, "timeout, unmatched cache event code in counter register.\n");
> + return 0;
> + }
> +
> + v = readq(base + CACHE_CNTR0);
> + count = FIELD_GET(CACHE_CNTR_EVNT_CNTR, v);
> + v = readq(base + CACHE_CNTR1);
> + count += FIELD_GET(CACHE_CNTR_EVNT_CNTR, v);
> +
> + return count;
> +}
> +
> +static bool is_fabric_event_supported(struct fme_perf_priv *priv, u32 event,
> + u32 portid)
> +{
> + if (event > FAB_EVNT_MAX || !is_portid_root_or_port(portid))
> + return false;
> +
> + if (priv->id == FME_FEATURE_ID_GLOBAL_DPERF &&
> + (event == FAB_EVNT_PCIE1_RD || event == FAB_EVNT_UPI_RD ||
> + event == FAB_EVNT_PCIE1_WR || event == FAB_EVNT_UPI_WR))
> + return false;
> +
> + return true;
> +}
> +
> +static int fabric_event_init(struct fme_perf_priv *priv, u32 event, u32 portid)
> +{
> + void __iomem *base = priv->ioaddr;
> + int ret = 0;
> + u64 v;
> +
> + if (!is_fabric_event_supported(priv, event, portid))
> + return -EINVAL;
> +
> + /*
> + * as fabric counter set only can be in either overall or port mode.
> + * In overall mode, it counts overall data for FPGA, and in port mode,
> + * it is configured to monitor on one individual port.
> + *
> + * so every time, a new event is initialized, driver checks
> + * current working mode and if someone is using this counter set.
> + */
> + spin_lock(&priv->fab_lock);
> + if (priv->fab_users && priv->fab_port_id != portid) {
> + dev_dbg(priv->dev, "conflict fabric event monitoring mode.\n");
> + ret = -EOPNOTSUPP;
> + goto exit;
> + }
> +
> + priv->fab_users++;
> +
> + /*
> + * skip if current working mode matches, otherwise change the working
> + * mode per input port_id, to monitor overall data or another port.
> + */
> + if (priv->fab_port_id == portid)
> + goto exit;
> +
> + priv->fab_port_id = portid;
> +
> + v = readq(base + FAB_CTRL);
> + v &= ~(FAB_PORT_FILTER | FAB_PORT_ID);
> +
> + if (is_portid_root(portid)) {
> + v |= FIELD_PREP(FAB_PORT_FILTER, FAB_PORT_FILTER_DISABLE);
> + } else {
> + v |= FIELD_PREP(FAB_PORT_FILTER, FAB_PORT_FILTER_ENABLE);
> + v |= FIELD_PREP(FAB_PORT_ID, portid);
> + }
> + writeq(v, base + FAB_CTRL);
> +
> +exit:
> + spin_unlock(&priv->fab_lock);
> + return ret;
> +}
> +
> +static void fabric_event_destroy(struct fme_perf_priv *priv, u32 event,
> + u32 portid)
> +{
> + spin_lock(&priv->fab_lock);
> + priv->fab_users--;
> + spin_unlock(&priv->fab_lock);
> +}
> +
> +static u64 fabric_read_event_counter(struct fme_perf_priv *priv, u32 event,
> + u32 portid)
> +{
> + void __iomem *base = priv->ioaddr;
> + u64 v;
> +
> + v = readq(base + FAB_CTRL);
> + v &= ~FAB_CTRL_EVNT;
> + v |= FIELD_PREP(FAB_CTRL_EVNT, event);
> + writeq(v, base + FAB_CTRL);
> +
> + if (readq_poll_timeout_atomic(base + FAB_CNTR, v,
> + FIELD_GET(FAB_CNTR_EVNT, v) == event,
> + 1, PERF_TIMEOUT)) {
> + dev_err(priv->dev, "timeout, unmatched fab event code in counter register.\n");
> + return 0;
> + }
> +
> + v = readq(base + FAB_CNTR);
> + return FIELD_GET(FAB_CNTR_EVNT_CNTR, v);
> +}
> +
> +static int vtd_event_init(struct fme_perf_priv *priv, u32 event, u32 portid)
> +{
> + if (priv->id == FME_FEATURE_ID_GLOBAL_IPERF &&
> + event <= VTD_EVNT_MAX && is_portid_port(portid))
> + return 0;
> +
> + return -EINVAL;
> +}
> +
> +static u64 vtd_read_event_counter(struct fme_perf_priv *priv, u32 event,
> + u32 portid)
> +{
> + void __iomem *base = priv->ioaddr;
> + u64 v;
> +
> + event += (portid * (VTD_EVNT_MAX + 1));
> +
> + v = readq(base + VTD_CTRL);
> + v &= ~VTD_CTRL_EVNT;
> + v |= FIELD_PREP(VTD_CTRL_EVNT, event);
> + writeq(v, base + VTD_CTRL);
> +
> + if (readq_poll_timeout_atomic(base + VTD_CNTR, v,
> + FIELD_GET(VTD_CNTR_EVNT, v) == event,
> + 1, PERF_TIMEOUT)) {
> + dev_err(priv->dev, "timeout, unmatched vtd event code in counter register.\n");
> + return 0;
> + }
> +
> + v = readq(base + VTD_CNTR);
> + return FIELD_GET(VTD_CNTR_EVNT_CNTR, v);
> +}
> +
> +static int vtd_sip_event_init(struct fme_perf_priv *priv, u32 event, u32 portid)
> +{
> + if (priv->id == FME_FEATURE_ID_GLOBAL_IPERF &&
> + event <= VTD_SIP_EVNT_MAX && is_portid_root(portid))
> + return 0;
> +
> + return -EINVAL;
> +}
> +
> +static u64 vtd_sip_read_event_counter(struct fme_perf_priv *priv, u32 event,
> + u32 portid)
> +{
> + void __iomem *base = priv->ioaddr;
> + u64 v;
> +
> + v = readq(base + VTD_SIP_CTRL);
> + v &= ~VTD_SIP_CTRL_EVNT;
> + v |= FIELD_PREP(VTD_SIP_CTRL_EVNT, event);
> + writeq(v, base + VTD_SIP_CTRL);
> +
> + if (readq_poll_timeout_atomic(base + VTD_SIP_CNTR, v,
> + FIELD_GET(VTD_SIP_CNTR_EVNT, v) == event,
> + 1, PERF_TIMEOUT)) {
> + dev_err(priv->dev, "timeout, unmatched vtd sip event code in counter register\n");
> + return 0;
> + }
> +
> + v = readq(base + VTD_SIP_CNTR);
> + return FIELD_GET(VTD_SIP_CNTR_EVNT_CNTR, v);
> +}
> +
> +static struct fme_perf_event_ops fme_perf_event_ops[] = {
> + [FME_EVTYPE_BASIC] = {.event_init = basic_event_init,
> + .read_counter = basic_read_event_counter,},
> + [FME_EVTYPE_CACHE] = {.event_init = cache_event_init,
> + .read_counter = cache_read_event_counter,},
> + [FME_EVTYPE_FABRIC] = {.event_init = fabric_event_init,
> + .event_destroy = fabric_event_destroy,
> + .read_counter = fabric_read_event_counter,},
> + [FME_EVTYPE_VTD] = {.event_init = vtd_event_init,
> + .read_counter = vtd_read_event_counter,},
> + [FME_EVTYPE_VTD_SIP] = {.event_init = vtd_sip_event_init,
> + .read_counter = vtd_sip_read_event_counter,},
> +};
> +
> +static ssize_t fme_perf_event_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct dev_ext_attribute *eattr;
> + unsigned long config;
> + char *ptr = buf;
> +
> + eattr = container_of(attr, struct dev_ext_attribute, attr);
> + config = (unsigned long)eattr->var;
> +
> + ptr += sprintf(ptr, "event=0x%02x", (unsigned int)get_event(config));
> + ptr += sprintf(ptr, ",evtype=0x%02x", (unsigned int)get_evtype(config));
> +
> + if (is_portid_root(get_portid(config)))
> + ptr += sprintf(ptr, ",portid=0x%02x\n", FME_PORTID_ROOT);
> + else
> + ptr += sprintf(ptr, ",portid=?\n");
> +
> + return (ssize_t)(ptr - buf);
> +}
> +
> +#define FME_EVENT_ATTR(_name) \
> + __ATTR(_name, 0444, fme_perf_event_show, NULL)
> +
> +#define FME_PORT_EVENT_CONFIG(_event, _type) \
> + (void *)((((_event) << FME_EVENT_SHIFT) & FME_EVENT_MASK) | \
> + (((_type) << FME_EVTYPE_SHIFT) & FME_EVTYPE_MASK))
> +
> +#define FME_EVENT_CONFIG(_event, _type) \
> + (void *)((((_event) << FME_EVENT_SHIFT) & FME_EVENT_MASK) | \
> + (((_type) << FME_EVTYPE_SHIFT) & FME_EVTYPE_MASK) | \
> + (FME_PORTID_ROOT << FME_PORTID_SHIFT))
> +
> +/* FME Perf Basic Events */
> +#define FME_EVENT_BASIC(_name, _event) \
> +static struct dev_ext_attribute fme_perf_event_##_name = { \
> + .attr = FME_EVENT_ATTR(_name), \
> + .var = FME_EVENT_CONFIG(_event, FME_EVTYPE_BASIC), \
> +}
> +
> +FME_EVENT_BASIC(clock, BASIC_EVNT_CLK);
> +
> +static struct attribute *fme_perf_basic_events_attrs[] = {
> + &fme_perf_event_clock.attr.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group fme_perf_basic_events_group = {
> + .name = "events",
> + .attrs = fme_perf_basic_events_attrs,
> +};
> +
> +/* FME Perf Fabric Events */
> +#define FME_EVENT_FABRIC(_name, _event) \
> +static struct dev_ext_attribute fme_perf_event_fab_##_name = { \
> + .attr = FME_EVENT_ATTR(fab_##_name), \
> + .var = FME_EVENT_CONFIG(_event, FME_EVTYPE_FABRIC), \
> +}
> +
> +#define FME_EVENT_FABRIC_PORT(_name, _event) \
> +static struct dev_ext_attribute fme_perf_event_fab_port_##_name = { \
> + .attr = FME_EVENT_ATTR(fab_port_##_name), \
> + .var = FME_PORT_EVENT_CONFIG(_event, FME_EVTYPE_FABRIC), \
> +}
> +
> +FME_EVENT_FABRIC(pcie0_read, FAB_EVNT_PCIE0_RD);
> +FME_EVENT_FABRIC(pcie0_write, FAB_EVNT_PCIE0_WR);
> +FME_EVENT_FABRIC(pcie1_read, FAB_EVNT_PCIE1_RD);
> +FME_EVENT_FABRIC(pcie1_write, FAB_EVNT_PCIE1_WR);
> +FME_EVENT_FABRIC(upi_read, FAB_EVNT_UPI_RD);
> +FME_EVENT_FABRIC(upi_write, FAB_EVNT_UPI_WR);
> +FME_EVENT_FABRIC(mmio_read, FAB_EVNT_MMIO_RD);
> +FME_EVENT_FABRIC(mmio_write, FAB_EVNT_MMIO_WR);
> +
> +FME_EVENT_FABRIC_PORT(pcie0_read, FAB_EVNT_PCIE0_RD);
> +FME_EVENT_FABRIC_PORT(pcie0_write, FAB_EVNT_PCIE0_WR);
> +FME_EVENT_FABRIC_PORT(pcie1_read, FAB_EVNT_PCIE1_RD);
> +FME_EVENT_FABRIC_PORT(pcie1_write, FAB_EVNT_PCIE1_WR);
> +FME_EVENT_FABRIC_PORT(upi_read, FAB_EVNT_UPI_RD);
> +FME_EVENT_FABRIC_PORT(upi_write, FAB_EVNT_UPI_WR);
> +FME_EVENT_FABRIC_PORT(mmio_read, FAB_EVNT_MMIO_RD);
> +FME_EVENT_FABRIC_PORT(mmio_write, FAB_EVNT_MMIO_WR);
> +
> +static struct attribute *fme_perf_fabric_events_attrs[] = {
> + &fme_perf_event_fab_pcie0_read.attr.attr,
> + &fme_perf_event_fab_pcie0_write.attr.attr,
> + &fme_perf_event_fab_pcie1_read.attr.attr,
> + &fme_perf_event_fab_pcie1_write.attr.attr,
> + &fme_perf_event_fab_upi_read.attr.attr,
> + &fme_perf_event_fab_upi_write.attr.attr,
> + &fme_perf_event_fab_mmio_read.attr.attr,
> + &fme_perf_event_fab_mmio_write.attr.attr,
> + &fme_perf_event_fab_port_pcie0_read.attr.attr,
> + &fme_perf_event_fab_port_pcie0_write.attr.attr,
> + &fme_perf_event_fab_port_pcie1_read.attr.attr,
> + &fme_perf_event_fab_port_pcie1_write.attr.attr,
> + &fme_perf_event_fab_port_upi_read.attr.attr,
> + &fme_perf_event_fab_port_upi_write.attr.attr,
> + &fme_perf_event_fab_port_mmio_read.attr.attr,
> + &fme_perf_event_fab_port_mmio_write.attr.attr,
> + NULL,
> +};
> +
> +static umode_t fme_perf_fabric_events_visible(struct kobject *kobj,
> + struct attribute *attr, int n)
> +{
> + struct pmu *pmu = dev_get_drvdata(kobj_to_dev(kobj));
> + struct fme_perf_priv *priv = to_fme_perf_priv(pmu);
> + struct dev_ext_attribute *eattr;
> + u64 var;
> +
> + eattr = container_of(attr, struct dev_ext_attribute, attr.attr);
> + var = (u64)eattr->var;
> +
> + if (is_fabric_event_supported(priv, get_event(var), get_portid(var)))
> + return attr->mode;
> +
> + return 0;
> +}
> +
> +static const struct attribute_group fme_perf_fabric_events_group = {
> + .name = "events",
> + .attrs = fme_perf_fabric_events_attrs,
> + .is_visible = fme_perf_fabric_events_visible,
> +};
> +
> +/* FME Perf Cache Events */
> +#define FME_EVENT_CACHE(_name, _event) \
> +static struct dev_ext_attribute fme_perf_event_cache_##_name = { \
> + .attr = FME_EVENT_ATTR(cache_##_name), \
> + .var = FME_EVENT_CONFIG(_event, FME_EVTYPE_CACHE), \
> +}
> +
> +FME_EVENT_CACHE(read_hit, CACHE_EVNT_RD_HIT);
> +FME_EVENT_CACHE(read_miss, CACHE_EVNT_RD_MISS);
> +FME_EVENT_CACHE(write_hit, CACHE_EVNT_WR_HIT);
> +FME_EVENT_CACHE(write_miss, CACHE_EVNT_WR_MISS);
> +FME_EVENT_CACHE(hold_request, CACHE_EVNT_HOLD_REQ);
> +FME_EVENT_CACHE(tx_req_stall, CACHE_EVNT_TX_REQ_STALL);
> +FME_EVENT_CACHE(rx_req_stall, CACHE_EVNT_RX_REQ_STALL);
> +FME_EVENT_CACHE(eviction, CACHE_EVNT_EVICTIONS);
> +FME_EVENT_CACHE(data_write_port_contention, CACHE_EVNT_DATA_WR_PORT_CONTEN);
> +FME_EVENT_CACHE(tag_write_port_contention, CACHE_EVNT_TAG_WR_PORT_CONTEN);
> +
> +static struct attribute *fme_perf_cache_events_attrs[] = {
> + &fme_perf_event_cache_read_hit.attr.attr,
> + &fme_perf_event_cache_read_miss.attr.attr,
> + &fme_perf_event_cache_write_hit.attr.attr,
> + &fme_perf_event_cache_write_miss.attr.attr,
> + &fme_perf_event_cache_hold_request.attr.attr,
> + &fme_perf_event_cache_tx_req_stall.attr.attr,
> + &fme_perf_event_cache_rx_req_stall.attr.attr,
> + &fme_perf_event_cache_eviction.attr.attr,
> + &fme_perf_event_cache_data_write_port_contention.attr.attr,
> + &fme_perf_event_cache_tag_write_port_contention.attr.attr,
> + NULL,
> +};
> +
> +static umode_t fme_perf_events_visible(struct kobject *kobj,
> + struct attribute *attr, int n)
> +{
> + struct pmu *pmu = dev_get_drvdata(kobj_to_dev(kobj));
> + struct fme_perf_priv *priv = to_fme_perf_priv(pmu);
> +
> + return (priv->id == FME_FEATURE_ID_GLOBAL_IPERF) ? attr->mode : 0;
> +}
> +
> +static const struct attribute_group fme_perf_cache_events_group = {
> + .name = "events",
> + .attrs = fme_perf_cache_events_attrs,
> + .is_visible = fme_perf_events_visible,
> +};
> +
> +/* FME Perf VTD Events */
> +#define FME_EVENT_VTD_PORT(_name, _event) \
> +static struct dev_ext_attribute fme_perf_event_vtd_port_##_name = { \
> + .attr = FME_EVENT_ATTR(vtd_port_##_name), \
> + .var = FME_PORT_EVENT_CONFIG(_event, FME_EVTYPE_VTD), \
> +}
> +
> +FME_EVENT_VTD_PORT(read_transaction, VTD_EVNT_AFU_MEM_RD_TRANS);
> +FME_EVENT_VTD_PORT(write_transaction, VTD_EVNT_AFU_MEM_WR_TRANS);
> +FME_EVENT_VTD_PORT(devtlb_read_hit, VTD_EVNT_AFU_DEVTLB_RD_HIT);
> +FME_EVENT_VTD_PORT(devtlb_write_hit, VTD_EVNT_AFU_DEVTLB_WR_HIT);
> +FME_EVENT_VTD_PORT(devtlb_4k_fill, VTD_EVNT_DEVTLB_4K_FILL);
> +FME_EVENT_VTD_PORT(devtlb_2m_fill, VTD_EVNT_DEVTLB_2M_FILL);
> +FME_EVENT_VTD_PORT(devtlb_1g_fill, VTD_EVNT_DEVTLB_1G_FILL);
> +
> +static struct attribute *fme_perf_vtd_events_attrs[] = {
> + &fme_perf_event_vtd_port_read_transaction.attr.attr,
> + &fme_perf_event_vtd_port_write_transaction.attr.attr,
> + &fme_perf_event_vtd_port_devtlb_read_hit.attr.attr,
> + &fme_perf_event_vtd_port_devtlb_write_hit.attr.attr,
> + &fme_perf_event_vtd_port_devtlb_4k_fill.attr.attr,
> + &fme_perf_event_vtd_port_devtlb_2m_fill.attr.attr,
> + &fme_perf_event_vtd_port_devtlb_1g_fill.attr.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group fme_perf_vtd_events_group = {
> + .name = "events",
> + .attrs = fme_perf_vtd_events_attrs,
> + .is_visible = fme_perf_events_visible,
> +};
> +
> +/* FME Perf VTD SIP Events */
> +#define FME_EVENT_VTD_SIP(_name, _event) \
> +static struct dev_ext_attribute fme_perf_event_vtd_sip_##_name = { \
> + .attr = FME_EVENT_ATTR(vtd_sip_##_name), \
> + .var = FME_EVENT_CONFIG(_event, FME_EVTYPE_VTD_SIP), \
> +}
> +
> +FME_EVENT_VTD_SIP(iotlb_4k_hit, VTD_SIP_EVNT_IOTLB_4K_HIT);
> +FME_EVENT_VTD_SIP(iotlb_2m_hit, VTD_SIP_EVNT_IOTLB_2M_HIT);
> +FME_EVENT_VTD_SIP(iotlb_1g_hit, VTD_SIP_EVNT_IOTLB_1G_HIT);
> +FME_EVENT_VTD_SIP(slpwc_l3_hit, VTD_SIP_EVNT_SLPWC_L3_HIT);
> +FME_EVENT_VTD_SIP(slpwc_l4_hit, VTD_SIP_EVNT_SLPWC_L4_HIT);
> +FME_EVENT_VTD_SIP(rcc_hit, VTD_SIP_EVNT_RCC_HIT);
> +FME_EVENT_VTD_SIP(iotlb_4k_miss, VTD_SIP_EVNT_IOTLB_4K_MISS);
> +FME_EVENT_VTD_SIP(iotlb_2m_miss, VTD_SIP_EVNT_IOTLB_2M_MISS);
> +FME_EVENT_VTD_SIP(iotlb_1g_miss, VTD_SIP_EVNT_IOTLB_1G_MISS);
> +FME_EVENT_VTD_SIP(slpwc_l3_miss, VTD_SIP_EVNT_SLPWC_L3_MISS);
> +FME_EVENT_VTD_SIP(slpwc_l4_miss, VTD_SIP_EVNT_SLPWC_L4_MISS);
> +FME_EVENT_VTD_SIP(rcc_miss, VTD_SIP_EVNT_RCC_MISS);
> +
> +static struct attribute *fme_perf_vtd_sip_events_attrs[] = {
> + &fme_perf_event_vtd_sip_iotlb_4k_hit.attr.attr,
> + &fme_perf_event_vtd_sip_iotlb_2m_hit.attr.attr,
> + &fme_perf_event_vtd_sip_iotlb_1g_hit.attr.attr,
> + &fme_perf_event_vtd_sip_slpwc_l3_hit.attr.attr,
> + &fme_perf_event_vtd_sip_slpwc_l4_hit.attr.attr,
> + &fme_perf_event_vtd_sip_rcc_hit.attr.attr,
> + &fme_perf_event_vtd_sip_iotlb_4k_miss.attr.attr,
> + &fme_perf_event_vtd_sip_iotlb_2m_miss.attr.attr,
> + &fme_perf_event_vtd_sip_iotlb_1g_miss.attr.attr,
> + &fme_perf_event_vtd_sip_slpwc_l3_miss.attr.attr,
> + &fme_perf_event_vtd_sip_slpwc_l4_miss.attr.attr,
> + &fme_perf_event_vtd_sip_rcc_miss.attr.attr,
> + NULL,
> +};
> +
> +static const struct attribute_group fme_perf_vtd_sip_events_group = {
> + .name = "events",
> + .attrs = fme_perf_vtd_sip_events_attrs,
> + .is_visible = fme_perf_events_visible,
> +};
> +
> +static const struct attribute_group *fme_perf_events_groups[] = {
> + &fme_perf_basic_events_group,
> + &fme_perf_cache_events_group,
> + &fme_perf_fabric_events_group,
> + &fme_perf_vtd_events_group,
> + &fme_perf_vtd_sip_events_group,
> + NULL,
> +};
> +
> +static struct fme_perf_event_ops *get_event_ops(u32 evtype)
> +{
> + if (evtype > FME_EVTYPE_MAX)
> + return NULL;
> +
> + return &fme_perf_event_ops[evtype];
> +}
> +
> +static void fme_perf_event_destroy(struct perf_event *event)
> +{
> + struct fme_perf_event_ops *ops = get_event_ops(event->hw.event_base);
> + struct fme_perf_priv *priv = to_fme_perf_priv(event->pmu);
> +
> + if (ops->event_destroy)
> + ops->event_destroy(priv, event->hw.idx, event->hw.config_base);
> +}
> +
> +static int fme_perf_event_init(struct perf_event *event)
> +{
> + struct fme_perf_priv *priv = to_fme_perf_priv(event->pmu);
> + struct hw_perf_event *hwc = &event->hw;
> + struct fme_perf_event_ops *ops;
> + u32 eventid, evtype, portid;
> +
> + /* test the event attr type check for PMU enumeration */
> + if (event->attr.type != event->pmu->type)
> + return -ENOENT;
> +
> + /*
> + * fme counters are shared across all cores.
> + * Therefore, it does not support per-process mode.
> + * Also, it does not support event sampling mode.
> + */
> + if (is_sampling_event(event) || event->attach_state & PERF_ATTACH_TASK)
> + return -EINVAL;
> +
> + if (event->cpu < 0)
> + return -EINVAL;
> +
> + eventid = get_event(event->attr.config);
> + portid = get_portid(event->attr.config);
> + evtype = get_evtype(event->attr.config);
> + if (evtype > FME_EVTYPE_MAX)
> + return -EINVAL;
> +
> + hwc->event_base = evtype;
> + hwc->idx = (int)eventid;
> + hwc->config_base = portid;
> +
> + event->destroy = fme_perf_event_destroy;
> +
> + dev_dbg(priv->dev, "%s event=0x%x, evtype=0x%x, portid=0x%x,\n",
> + __func__, eventid, evtype, portid);
> +
> + ops = get_event_ops(evtype);
> + if (ops->event_init)
> + return ops->event_init(priv, eventid, portid);
> +
> + return 0;
> +}
> +
> +static void fme_perf_event_update(struct perf_event *event)
> +{
> + struct fme_perf_event_ops *ops = get_event_ops(event->hw.event_base);
> + struct fme_perf_priv *priv = to_fme_perf_priv(event->pmu);
> + struct hw_perf_event *hwc = &event->hw;
> + u64 now, prev, delta;
> +
> + now = ops->read_counter(priv, (u32)hwc->idx, hwc->config_base);
> + prev = local64_read(&hwc->prev_count);
> + delta = now - prev;
> +
> + local64_add(delta, &event->count);
> +}
> +
> +static void fme_perf_event_start(struct perf_event *event, int flags)
> +{
> + struct fme_perf_event_ops *ops = get_event_ops(event->hw.event_base);
> + struct fme_perf_priv *priv = to_fme_perf_priv(event->pmu);
> + struct hw_perf_event *hwc = &event->hw;
> + u64 count;
> +
> + count = ops->read_counter(priv, (u32)hwc->idx, hwc->config_base);
> + local64_set(&hwc->prev_count, count);
> +}
> +
> +static void fme_perf_event_stop(struct perf_event *event, int flags)
> +{
> + fme_perf_event_update(event);
> +}
> +
> +static int fme_perf_event_add(struct perf_event *event, int flags)
> +{
> + if (flags & PERF_EF_START)
> + fme_perf_event_start(event, flags);
> +
> + return 0;
> +}
> +
> +static void fme_perf_event_del(struct perf_event *event, int flags)
> +{
> + fme_perf_event_stop(event, PERF_EF_UPDATE);
> +}
> +
> +static void fme_perf_event_read(struct perf_event *event)
> +{
> + fme_perf_event_update(event);
> +}
> +
> +static void fme_perf_setup_hardware(struct fme_perf_priv *priv)
> +{
> + void __iomem *base = priv->ioaddr;
> + u64 v;
> +
> + /* read and save current working mode for fabric counters */
> + v = readq(base + FAB_CTRL);
> +
> + if (FIELD_GET(FAB_PORT_FILTER, v) == FAB_PORT_FILTER_DISABLE)
> + priv->fab_port_id = FME_PORTID_ROOT;
> + else
> + priv->fab_port_id = FIELD_GET(FAB_PORT_ID, v);
> +}
> +
> +static int fme_perf_pmu_register(struct platform_device *pdev,
> + struct fme_perf_priv *priv)
> +{
> + struct pmu *pmu = &priv->pmu;
> + char *name;
> + int ret;
> +
> + spin_lock_init(&priv->fab_lock);
> +
> + fme_perf_setup_hardware(priv);
> +
> + pmu->task_ctx_nr = perf_invalid_context;
> + pmu->attr_groups = fme_perf_groups;
> + pmu->attr_update = fme_perf_events_groups;
> + pmu->event_init = fme_perf_event_init;
> + pmu->add = fme_perf_event_add;
> + pmu->del = fme_perf_event_del;
> + pmu->start = fme_perf_event_start;
> + pmu->stop = fme_perf_event_stop;
> + pmu->read = fme_perf_event_read;
> + pmu->capabilities = PERF_PMU_CAP_NO_INTERRUPT |
> + PERF_PMU_CAP_NO_EXCLUDE;
> +
> + name = devm_kasprintf(priv->dev, GFP_KERNEL, "fme%d", pdev->id);
> +
> + ret = perf_pmu_register(pmu, name, -1);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static void fme_perf_pmu_unregister(struct fme_perf_priv *priv)
> +{
> + perf_pmu_unregister(&priv->pmu);
> +}
> +
> +static int fme_perf_init(struct platform_device *pdev,
> + struct dfl_feature *feature)
> +{
> + struct fme_perf_priv *priv;
> + int ret;
> +
> + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + priv->dev = &pdev->dev;
> + priv->ioaddr = feature->ioaddr;
> + priv->id = feature->id;
> +
> + ret = fme_perf_pmu_register(pdev, priv);
> + if (ret)
> + return ret;
> +
> + feature->priv = priv;
> + return 0;
> +}
> +
> +static void fme_perf_uinit(struct platform_device *pdev,
> + struct dfl_feature *feature)
> +{
> + struct fme_perf_priv *priv = feature->priv;
> +
> + fme_perf_pmu_unregister(priv);
> +}
> +
> +const struct dfl_feature_id fme_perf_id_table[] = {
> + {.id = FME_FEATURE_ID_GLOBAL_IPERF,},
> + {.id = FME_FEATURE_ID_GLOBAL_DPERF,},
> + {0,}
> +};
> +
> +const struct dfl_feature_ops fme_perf_ops = {
> + .init = fme_perf_init,
> + .uinit = fme_perf_uinit,
> +};
> diff --git a/drivers/fpga/dfl-fme.h b/drivers/fpga/dfl-fme.h
> index 6685c8e..4195dd6 100644
> --- a/drivers/fpga/dfl-fme.h
> +++ b/drivers/fpga/dfl-fme.h
> @@ -38,5 +38,7 @@ struct dfl_fme {
> extern const struct dfl_feature_ops fme_global_err_ops;
> extern const struct dfl_feature_id fme_global_err_id_table[];
> extern const struct attribute_group fme_global_err_group;
> +extern const struct dfl_feature_ops fme_perf_ops;
> +extern const struct dfl_feature_id fme_perf_id_table[];
>
> #endif /* __DFL_FME_H */
> diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
> index 9f0e656..d312678 100644
> --- a/drivers/fpga/dfl.h
> +++ b/drivers/fpga/dfl.h
> @@ -197,12 +197,14 @@ struct dfl_feature_driver {
> * feature dev (platform device)'s reources.
> * @ioaddr: mapped mmio resource address.
> * @ops: ops of this sub feature.
> + * @priv: priv data of this feature.
> */
> struct dfl_feature {
> u64 id;
> int resource_index;
> void __iomem *ioaddr;
> const struct dfl_feature_ops *ops;
> + void *priv;
> };
>
> #define DEV_STATUS_IN_USE 0
> --
> 1.8.3.1
>
For the fpga part, looks good to me. Let me take a look at the rest.
Thanks,
Moritz
^ permalink raw reply
* Re: [RFC PATCH v3 03/12] fs: add RWF_ENCODED for reading/writing compressed data
From: Nikolay Borisov @ 2019-11-27 9:00 UTC (permalink / raw)
To: Omar Sandoval
Cc: linux-fsdevel, linux-btrfs, Dave Chinner, Jann Horn,
Amir Goldstein, Aleksa Sarai, linux-api, kernel-team
In-Reply-To: <20191126173607.GA657777@vader>
On 26.11.19 г. 19:36 ч., Omar Sandoval wrote:
> On Tue, Nov 26, 2019 at 03:53:02PM +0200, Nikolay Borisov wrote:
>>
>>
>> On 20.11.19 г. 20:24 ч., Omar Sandoval wrote:
>>> From: Omar Sandoval <osandov@fb.com>
>>
>> <snip>
>>
>>>
>>> +enum {
>>> + ENCODED_IOV_COMPRESSION_NONE,
>>> +#define ENCODED_IOV_COMPRESSION_NONE ENCODED_IOV_COMPRESSION_NONE
>>> + ENCODED_IOV_COMPRESSION_ZLIB,
>>> +#define ENCODED_IOV_COMPRESSION_ZLIB ENCODED_IOV_COMPRESSION_ZLIB
>>> + ENCODED_IOV_COMPRESSION_LZO,
>>> +#define ENCODED_IOV_COMPRESSION_LZO ENCODED_IOV_COMPRESSION_LZO
>>> + ENCODED_IOV_COMPRESSION_ZSTD,
>>> +#define ENCODED_IOV_COMPRESSION_ZSTD ENCODED_IOV_COMPRESSION_ZSTD
>>> + ENCODED_IOV_COMPRESSION_TYPES = ENCODED_IOV_COMPRESSION_ZSTD,
>>
>> This looks very dodgy, what am I missing?
>
> This is a somewhat common trick so that enum values can be checked for
> with ifdef/ifndef. See include/uapi/linux.in.h, for example.
I cannot seem to have this file on my system (or any .in.h file for that
matter in the kernel source dir).
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox