Re: [PATCH v2] Documentation: admin-guide: PM: Add cpuidle document
On Wed, 28 Nov 2018 at 12:47, Rafael J. Wysocki wrote: > > From: Rafael J. Wysocki > > Important information is missing from user/admin cpuidle documentation > available today, so add a new user/admin document for cpuidle containing > current and comprehensive information to admin-guide and drop the old > .txt documents it is replacing. > > Signed-off-by: Rafael J. Wysocki > --- Wow! Great work! [...] > +.. _idle-states-representation: > + > +Representation of Idle States > += > + > +For the CPU idle time management purposes all of the physical idle states > +supported by the processor have to be represented as a one-dimensional array > of > +|struct cpuidle_state| objects each allowing an individual (logical) CPU to > ask > +the processor hardware to enter an idle state of certain properties. If > there > +is a hierarchy of units in the processor, one |struct cpuidle_state| object > can > +cover a combination of idle states supported by the units at different > levels of > +the hierarchy. In that case, the `target residency and exit latency > parameters > +of it `_, must reflect the properties of the idle state at the > +deepest level (i.e. the idle state of the unit containing all of the other > +units). > + > +For example, take a processor with two cores in a larger unit referred to as > +a "module" and suppose that asking the hardware to enter a specific idle > state > +(say "X") at the "core" level by one core will trigger the module to try to > +enter a specific idle state of its own (say "MX") if the other core is in > idle > +state "X" already. In other words, asking for idle state "X" at the "core" > +level gives the hardware a license to go as deep as to idle state "MX" at the > +"module" level, but there is no guarantee that this is going to happen (the > core > +asking for idle state "X" may just end up in that state by itself instead). This is a good description of a HW like x86 and ARM64 (using PSCI in platform coordinated mode). However, for "legacy" ARM products, having at least two cores in a module, this description does in many cases *not* match how the HW works. I wonder about other architectures. My point is, I think it would be fair to mention that the above description is specific to certain HWs. In other words, for HWs that leaves the entire state synchronization of the last man standing algorithm to be managed by a FW, the cpuidle framework plays well. But for others it has limitations. > +Then, the target residency of the |struct cpuidle_state| object representing > +idle state "X" must reflect the minimum time to spend in idle state "MX" of > +the module (including the time needed to enter it), because that is the > minimum > +time the CPU needs to be idle to save any energy in case the hardware enters > +that state. Analogously, the exit latency parameter of that object must > cover > +the exit time of idle state "MX" of the module (and usually its entry time > too), > +because that is the maximum delay between a wakeup signal and the time the > CPU > +will start to execute the first new instruction (assuming that both cores in > the > +module will always be ready to execute instructions as soon as the module > +becomes operational as a whole). For the HW working as you described, this is a nice clarification, thanks! [...] > + > +.. _cpu-pm-qos: > + > +Power Management Quality of Service for CPUs > + > + > +The power management quality of service (PM QoS) framework in the Linux > kernel > +allows kernel code and user space processes to set constraints on various > +energy-efficiency features of the kernel to prevent performance from dropping > +below a required level. The PM QoS constraints can be set globally, in > +predefined categories referred to as PM QoS classes, or against individual > +devices. > + > +CPU idle time management can be affected by PM QoS in two ways, through the > +global constraint in the ``PM_QOS_CPU_DMA_LATENCY`` class and through the No objections to the description, it's really nice! However, the naming of this QOS class, is to me, rather confusing. I realize renaming the sysfs node is not an option, but maybe we should make a tree wide change to rename the class to something more descriptive. What do you think? [...] Kind regards Uffe
Re: [PATCH v2] Documentation: admin-guide: PM: Add cpuidle document
On Thu, Nov 29, 2018 at 9:07 AM Ulf Hansson wrote: > > On Wed, 28 Nov 2018 at 12:47, Rafael J. Wysocki wrote: > > > > From: Rafael J. Wysocki > > > > Important information is missing from user/admin cpuidle documentation > > available today, so add a new user/admin document for cpuidle containing > > current and comprehensive information to admin-guide and drop the old > > .txt documents it is replacing. > > > > Signed-off-by: Rafael J. Wysocki > > --- > > Wow! Great work! > > [...] > > > +.. _idle-states-representation: > > + > > +Representation of Idle States > > += > > + > > +For the CPU idle time management purposes all of the physical idle states > > +supported by the processor have to be represented as a one-dimensional > > array of > > +|struct cpuidle_state| objects each allowing an individual (logical) CPU > > to ask > > +the processor hardware to enter an idle state of certain properties. If > > there > > +is a hierarchy of units in the processor, one |struct cpuidle_state| > > object can > > +cover a combination of idle states supported by the units at different > > levels of > > +the hierarchy. In that case, the `target residency and exit latency > > parameters > > +of it `_, must reflect the properties of the idle state at the > > +deepest level (i.e. the idle state of the unit containing all of the other > > +units). > > + > > +For example, take a processor with two cores in a larger unit referred to > > as > > +a "module" and suppose that asking the hardware to enter a specific idle > > state > > +(say "X") at the "core" level by one core will trigger the module to try to > > +enter a specific idle state of its own (say "MX") if the other core is in > > idle > > +state "X" already. In other words, asking for idle state "X" at the "core" > > +level gives the hardware a license to go as deep as to idle state "MX" at > > the > > +"module" level, but there is no guarantee that this is going to happen > > (the core > > +asking for idle state "X" may just end up in that state by itself instead). > > This is a good description of a HW like x86 and ARM64 (using PSCI in > platform coordinated mode). > > However, for "legacy" ARM products, having at least two cores in a > module, this description does in many cases *not* match how the HW > works. I wonder about other architectures. Well, this is an example and it is very clearly presented this way IMO (it says "for example" upfront and then "suppose" etc). Basically, the purpose of it is to explain that the parameters shown in sysfs may be effective. > My point is, I think it would be fair to mention that the above > description is specific to certain HWs. In other words, for HWs that > leaves the entire state synchronization of the last man standing > algorithm to be managed by a FW, the cpuidle framework plays well. But > for others it has limitations. I can add it after this entire paragraph I think. Something like "There are processors without direct coordination between different levels of the hierarchy of units inside them, however. In those cases asking for an idle state at the "core" level does not automatically affect the "module" level, for example, in any way and the CPUIdle driver is responsible for the entire handling of the hierarchy. Then, the definition of the idle state objects is entirely up to the driver, but still the physical properties of the idle state that the processor hardware finally goes into must always follow the parameters used by the governor for idle state selection (for instance, the actual exit latency of that idle state must not exceed the exit latency parameter of the idle state object selected by the governor)." > > > +Then, the target residency of the |struct cpuidle_state| object > > representing > > +idle state "X" must reflect the minimum time to spend in idle state "MX" of > > +the module (including the time needed to enter it), because that is the > > minimum > > +time the CPU needs to be idle to save any energy in case the hardware > > enters > > +that state. Analogously, the exit latency parameter of that object must > > cover > > +the exit time of idle state "MX" of the module (and usually its entry time > > too), > > +because that is the maximum delay between a wakeup signal and the time the > > CPU > > +will start to execute the first new instruction (assuming that both cores > > in the > > +module will always be ready to execute instructions as soon as the module > > +becomes operational as a whole). > > For the HW working as you described, this is a nice clarification, thanks! > > [...] > > > + > > +.. _cpu-pm-qos: > > + > > +Power Management Quality of Service for CPUs > > + > > + > > +The power management quality of service (PM QoS) framework in the Linux > > kernel > > +allows kernel code and user space processes to set constraints on various > > +energy-efficiency features of the kernel to p
Re: [PATCH v4 1/5] cgroup: subtree_control bypass mode for bypassable controllers
Hi Waiman, Thank you for the patch! Perhaps something to improve: url: https://github.com/0day-ci/linux/commits/Waiman-Long/cgroup-Introducing-bypass-mode/20181123-030552 base: https://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git for-next smatch warnings: kernel/cgroup/cgroup.c:4893 css_create() error: we previously assumed 'parent' could be null (see line 4864) # https://github.com/0day-ci/linux/commit/8b68fd4330e043645667a5d3306398f8f88f9ff2 git remote add linux-review https://github.com/0day-ci/linux git remote update linux-review git checkout 8b68fd4330e043645667a5d3306398f8f88f9ff2 vim +/parent +4893 kernel/cgroup/cgroup.c a31f2d3ff kernel/cgroup.cTejun Heo2012-11-19 4840 c81c925ad kernel/cgroup.cTejun Heo2013-12-06 4841 /** 6cd0f5bba kernel/cgroup.cTejun Heo2016-03-03 4842 * css_create - create a cgroup_subsys_state c81c925ad kernel/cgroup.cTejun Heo2013-12-06 4843 * @cgrp: the cgroup new css will be associated with c81c925ad kernel/cgroup.cTejun Heo2013-12-06 4844 * @ss: the subsys of new css c81c925ad kernel/cgroup.cTejun Heo2013-12-06 4845 * c81c925ad kernel/cgroup.cTejun Heo2013-12-06 4846 * Create a new css associated with @cgrp - @ss pair. On success, the new 6cd0f5bba kernel/cgroup.cTejun Heo2016-03-03 4847 * css is online and installed in @cgrp. This function doesn't create the 6cd0f5bba kernel/cgroup.cTejun Heo2016-03-03 4848 * interface files. Returns 0 on success, -errno on failure. c81c925ad kernel/cgroup.cTejun Heo2013-12-06 4849 */ 6cd0f5bba kernel/cgroup.cTejun Heo2016-03-03 4850 static struct cgroup_subsys_state *css_create(struct cgroup *cgrp, 6cd0f5bba kernel/cgroup.cTejun Heo2016-03-03 4851 struct cgroup_subsys *ss) c81c925ad kernel/cgroup.cTejun Heo2013-12-06 4852 { d51f39b05 kernel/cgroup.cTejun Heo2014-05-16 4853 struct cgroup *parent = cgroup_parent(cgrp); 8b68fd433 kernel/cgroup/cgroup.c Waiman Long 2018-11-20 4854 struct cgroup_subsys_state *parent_css = NULL; c81c925ad kernel/cgroup.cTejun Heo2013-12-06 4855 struct cgroup_subsys_state *css; c81c925ad kernel/cgroup.cTejun Heo2013-12-06 4856 int err; c81c925ad kernel/cgroup.cTejun Heo2013-12-06 4857 c81c925ad kernel/cgroup.cTejun Heo2013-12-06 4858 lockdep_assert_held(&cgroup_mutex); c81c925ad kernel/cgroup.cTejun Heo2013-12-06 4859 8b68fd433 kernel/cgroup/cgroup.c Waiman Long 2018-11-20 4860 /* 8b68fd433 kernel/cgroup/cgroup.c Waiman Long 2018-11-20 4861 * As cgroup may be in bypass mode, need to skip over ancestor 8b68fd433 kernel/cgroup/cgroup.c Waiman Long 2018-11-20 4862 * cgroups with NULL CSS. 8b68fd433 kernel/cgroup/cgroup.c Waiman Long 2018-11-20 4863 */ 8b68fd433 kernel/cgroup/cgroup.c Waiman Long 2018-11-20 @4864 for (; parent && !parent_css; parent = cgroup_parent(parent)) ^ 8b68fd433 kernel/cgroup/cgroup.c Waiman Long 2018-11-20 4865 parent_css = cgroup_css(parent, ss); When we exit this loop it means either parent is NULL or parent_css is non-NULL. 8b68fd433 kernel/cgroup/cgroup.c Waiman Long 2018-11-20 4866 1fed1b2e3 kernel/cgroup.cTejun Heo2014-05-16 4867 css = ss->css_alloc(parent_css); e7e15b87f kernel/cgroup.cTejun Heo2016-06-21 4868 if (!css) e7e15b87f kernel/cgroup.cTejun Heo2016-06-21 4869 css = ERR_PTR(-ENOMEM); c81c925ad kernel/cgroup.cTejun Heo2013-12-06 4870 if (IS_ERR(css)) 6cd0f5bba kernel/cgroup.cTejun Heo2016-03-03 4871 return css; c81c925ad kernel/cgroup.cTejun Heo2013-12-06 4872 8b68fd433 kernel/cgroup/cgroup.c Waiman Long 2018-11-20 4873 init_and_link_css(css, ss, cgrp, parent_css); a2bed8209 kernel/cgroup.cTejun Heo2014-05-04 4874 2aad2a86f kernel/cgroup.cTejun Heo2014-09-24 4875 err = percpu_ref_init(&css->refcnt, css_release, 0, GFP_KERNEL); c81c925ad kernel/cgroup.cTejun Heo2013-12-06 4876 if (err) 3eb59ec64 kernel/cgroup.cLi Zefan 2014-03-18 4877 goto err_free_css; c81c925ad kernel/cgroup.cTejun Heo2013-12-06 4878 cf780b7dc kernel/cgroup.cVladimir Davydov 2015-08-03 4879 err = cgroup_idr_alloc(&ss->css_idr, NULL, 2, 0, GFP_KERNEL); 15a4c835e kernel/cgroup.cTejun Heo2014-05-04 4880 if (err < 0) b00c52dae kernel/cgroup.cWenwei Tao 20
Re: [PATCH 0/9] drm: remove deprecated functions
On Mon, Nov 26, 2018 at 3:12 PM Daniel Vetter wrote: > On Sat, Nov 24, 2018 at 10:17:13PM +0100, Linus Walleij wrote: > > It was especially scary. > > > > But I think I managed to apply the patches and push the > > branch now. > > Except when you're racing with someone else you should only see conflicts > with stuff you've just pushed. Or if someone forgot to fix up their mess. > What was the conflict? dim push-branches was complaining that one of the commits was missing the proper committer sign-off, it was the bottom commit under mine (IIRC "drm/atomic-helper: WARN if fake_commit->hw_done is not completed as expected") and dim update-branches seemed to rebase and fix up my patches and then everything was fine. I just felt slightly out of control :D Yours, Linus Walleij
Re: [PATCH 0/9] drm: remove deprecated functions
On Thu, Nov 29, 2018 at 3:45 PM Linus Walleij wrote: > > On Mon, Nov 26, 2018 at 3:12 PM Daniel Vetter wrote: > > On Sat, Nov 24, 2018 at 10:17:13PM +0100, Linus Walleij wrote: > > > > It was especially scary. > > > > > > But I think I managed to apply the patches and push the > > > branch now. > > > > Except when you're racing with someone else you should only see conflicts > > with stuff you've just pushed. Or if someone forgot to fix up their mess. > > What was the conflict? > > dim push-branches was complaining that one of the commits was > missing the proper committer sign-off, it was the bottom commit under > mine (IIRC "drm/atomic-helper: WARN if fake_commit->hw_done is not > completed as expected") > and dim update-branches seemed to rebase and fix up my patches > and then everything was fine. This sounds like you (or dim?) accidentaly amended that commit (which changes the committer and results in the warning), and a rebase would indeed have fixed that. If the first patch conflicts this can happen because dim apply-branch doesn't bail out correctly. Or at least did in the past, I recently fixed that in commit ee299e510ae468aab27610bcbc4fdd4de932f74b Author: Daniel Vetter Date: Wed Oct 17 08:53:00 2018 +0200 dim: make apply-patch fail again > I just felt slightly out of control :D If your dim didn't have the above commit and you had a conflict it's all explained. Otherwise I'm not sure what's been going on ... -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH] drm: TODO: Add DRM_MODESET_LOCK_ALL* conversion to todo.rst
From: Sean Paul We should also get the drivers using the helpers. Seems like a good intermediate TODO item. Cc: Daniel Vetter Signed-off-by: Sean Paul --- Documentation/gpu/todo.rst | 12 1 file changed, 12 insertions(+) diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst index ab347dec50796..1a6dc586fb156 100644 --- a/Documentation/gpu/todo.rst +++ b/Documentation/gpu/todo.rst @@ -241,6 +241,18 @@ struct drm_gem_object_funcs GEM objects can now have a function table instead of having the callbacks on the DRM driver struct. This is now the preferred way and drivers can be moved over. +Use DRM_MODESET_LOCK_ALL_* helpers instead of boilerplate +- + +For cases where drivers are attempting to grab the modeset locks with a local +acquire context. Replace the boilerplate code surrounding +drm_modeset_lock_all_ctx() with DRM_MODESET_LOCK_ALL_BEGIN() and +DRM_MODESET_LOCK_ALL_END() instead. + +As a reference, take a look at the conversions already completed in drm core. + +Contact: Sean Paul, respective driver maintainers + Core refactorings = -- Sean Paul, Software Engineer, Google / Chromium OS
[PATCH] Documentation: drm: Remove dangling pointer from drm-mm.rst
From: Sean Paul Fixes htmldocs build error: Error: Cannot open file ../drivers/gpu/drm/drm_global.c Fixes: 2bb42410b1bd ("drm: Remove drm_global.{c,h} v2") Cc: Thomas Zimmermann Cc: Christian König Cc: Junwei Zhang Cc: Alex Deucher Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: Sean Paul Cc: David Airlie Cc: Christian Koenig Cc: Huang Rui Cc: dri-de...@lists.freedesktop.org Signed-off-by: Sean Paul --- Documentation/gpu/drm-mm.rst | 3 --- 1 file changed, 3 deletions(-) diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst index d0f3c6b032009..54a696d961a7c 100644 --- a/Documentation/gpu/drm-mm.rst +++ b/Documentation/gpu/drm-mm.rst @@ -79,9 +79,6 @@ count for the TTM, which will call your initialization function. See the radeon_ttm.c file for an example of usage. -.. kernel-doc:: drivers/gpu/drm/drm_global.c - :export: - The Graphics Execution Manager (GEM) -- Sean Paul, Software Engineer, Google / Chromium OS
Re: [PATCH] Documentation: drm: Remove dangling pointer from drm-mm.rst
Am 29.11.18 um 16:55 schrieb Sean Paul: > From: Sean Paul > > Fixes htmldocs build error: > Error: Cannot open file ../drivers/gpu/drm/drm_global.c > > Fixes: 2bb42410b1bd ("drm: Remove drm_global.{c,h} v2") > Cc: Thomas Zimmermann > Cc: Christian König > Cc: Junwei Zhang > Cc: Alex Deucher > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Sean Paul > Cc: David Airlie > Cc: Christian Koenig > Cc: Huang Rui > Cc: dri-de...@lists.freedesktop.org > Signed-off-by: Sean Paul Reviewed-by: Christian König > --- > Documentation/gpu/drm-mm.rst | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst > index d0f3c6b032009..54a696d961a7c 100644 > --- a/Documentation/gpu/drm-mm.rst > +++ b/Documentation/gpu/drm-mm.rst > @@ -79,9 +79,6 @@ count for the TTM, which will call your initialization > function. > > See the radeon_ttm.c file for an example of usage. > > -.. kernel-doc:: drivers/gpu/drm/drm_global.c > - :export: > - > > The Graphics Execution Manager (GEM) >
Re: [PATCH] Documentation: drm: Remove dangling pointer from drm-mm.rst
On Thu, Nov 29, 2018 at 10:58 AM Koenig, Christian wrote: > > Am 29.11.18 um 16:55 schrieb Sean Paul: > > From: Sean Paul > > > > Fixes htmldocs build error: > > Error: Cannot open file ../drivers/gpu/drm/drm_global.c > > > > Fixes: 2bb42410b1bd ("drm: Remove drm_global.{c,h} v2") > > Cc: Thomas Zimmermann > > Cc: Christian König > > Cc: Junwei Zhang > > Cc: Alex Deucher > > Cc: Maarten Lankhorst > > Cc: Maxime Ripard > > Cc: Sean Paul > > Cc: David Airlie > > Cc: Christian Koenig > > Cc: Huang Rui > > Cc: dri-de...@lists.freedesktop.org > > Signed-off-by: Sean Paul > > Reviewed-by: Christian König > Thanks for the review. Perhaps the best course of action for this patch is to have Dave pick it directly and then we can backmerge it into amdgpu/drm-misc-next from there? I could also spin a topic branch that we could all merge. Thoughts? Sean > > --- > > Documentation/gpu/drm-mm.rst | 3 --- > > 1 file changed, 3 deletions(-) > > > > diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst > > index d0f3c6b032009..54a696d961a7c 100644 > > --- a/Documentation/gpu/drm-mm.rst > > +++ b/Documentation/gpu/drm-mm.rst > > @@ -79,9 +79,6 @@ count for the TTM, which will call your initialization > > function. > > > > See the radeon_ttm.c file for an example of usage. > > > > -.. kernel-doc:: drivers/gpu/drm/drm_global.c > > - :export: > > - > > > > The Graphics Execution Manager (GEM) > > >
Re: [PATCH] Documentation: drm: Remove dangling pointer from drm-mm.rst
Am 29.11.18 um 17:11 schrieb Sean Paul: On Thu, Nov 29, 2018 at 10:58 AM Koenig, Christian wrote: Am 29.11.18 um 16:55 schrieb Sean Paul: From: Sean Paul Fixes htmldocs build error: Error: Cannot open file ../drivers/gpu/drm/drm_global.c Fixes: 2bb42410b1bd ("drm: Remove drm_global.{c,h} v2") Cc: Thomas Zimmermann Cc: Christian König Cc: Junwei Zhang Cc: Alex Deucher Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: Sean Paul Cc: David Airlie Cc: Christian Koenig Cc: Huang Rui Cc: dri-de...@lists.freedesktop.org Signed-off-by: Sean Paul Reviewed-by: Christian König Thanks for the review. Perhaps the best course of action for this patch is to have Dave pick it directly and then we can backmerge it into amdgpu/drm-misc-next from there? I could also spin a topic branch that we could all merge. Thoughts? My preference is that I pick it up and push it to Dave through Alex fixes push. But just because the patch which broke this came the same way as well. Either way works for me, Christian. Sean --- Documentation/gpu/drm-mm.rst | 3 --- 1 file changed, 3 deletions(-) diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst index d0f3c6b032009..54a696d961a7c 100644 --- a/Documentation/gpu/drm-mm.rst +++ b/Documentation/gpu/drm-mm.rst @@ -79,9 +79,6 @@ count for the TTM, which will call your initialization function. See the radeon_ttm.c file for an example of usage. -.. kernel-doc:: drivers/gpu/drm/drm_global.c - :export: - The Graphics Execution Manager (GEM) ___ dri-devel mailing list dri-de...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] Documentation: drm: Remove dangling pointer from drm-mm.rst
On Thu, Nov 29, 2018 at 11:14 AM Christian König wrote: > > Am 29.11.18 um 17:11 schrieb Sean Paul: > > On Thu, Nov 29, 2018 at 10:58 AM Koenig, Christian > > wrote: > >> Am 29.11.18 um 16:55 schrieb Sean Paul: > >>> From: Sean Paul > >>> > >>> Fixes htmldocs build error: > >>> Error: Cannot open file ../drivers/gpu/drm/drm_global.c > >>> > >>> Fixes: 2bb42410b1bd ("drm: Remove drm_global.{c,h} v2") > >>> Cc: Thomas Zimmermann > >>> Cc: Christian König > >>> Cc: Junwei Zhang > >>> Cc: Alex Deucher > >>> Cc: Maarten Lankhorst > >>> Cc: Maxime Ripard > >>> Cc: Sean Paul > >>> Cc: David Airlie > >>> Cc: Christian Koenig > >>> Cc: Huang Rui > >>> Cc: dri-de...@lists.freedesktop.org > >>> Signed-off-by: Sean Paul > >> Reviewed-by: Christian König > >> > > Thanks for the review. Perhaps the best course of action for this > > patch is to have Dave pick it directly and then we can backmerge it > > into amdgpu/drm-misc-next from there? I could also spin a topic branch > > that we could all merge. > > > > Thoughts? > > My preference is that I pick it up and push it to Dave through Alex > fixes push. > > But just because the patch which broke this came the same way as well. > Yeah, unfortunately it's proliferated, and I'd rather not add hops to getting drm-misc-next fixed. Let's just do a topic branch and set a PR to Dave from that. We can both merge the topic branch asap and let Dave pick it up at his leisure. Sean > Either way works for me, > Christian. > > > > > Sean > > > >>> --- > >>>Documentation/gpu/drm-mm.rst | 3 --- > >>>1 file changed, 3 deletions(-) > >>> > >>> diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst > >>> index d0f3c6b032009..54a696d961a7c 100644 > >>> --- a/Documentation/gpu/drm-mm.rst > >>> +++ b/Documentation/gpu/drm-mm.rst > >>> @@ -79,9 +79,6 @@ count for the TTM, which will call your initialization > >>> function. > >>> > >>>See the radeon_ttm.c file for an example of usage. > >>> > >>> -.. kernel-doc:: drivers/gpu/drm/drm_global.c > >>> - :export: > >>> - > >>> > >>>The Graphics Execution Manager (GEM) > >>> > > ___ > > dri-devel mailing list > > dri-de...@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel >
Re: [PATCH] Documentation: drm: Remove dangling pointer from drm-mm.rst
Am 29.11.18 um 17:21 schrieb Sean Paul: > On Thu, Nov 29, 2018 at 11:14 AM Christian König > wrote: >> Am 29.11.18 um 17:11 schrieb Sean Paul: >>> On Thu, Nov 29, 2018 at 10:58 AM Koenig, Christian >>> wrote: Am 29.11.18 um 16:55 schrieb Sean Paul: > From: Sean Paul > > Fixes htmldocs build error: > Error: Cannot open file ../drivers/gpu/drm/drm_global.c > > Fixes: 2bb42410b1bd ("drm: Remove drm_global.{c,h} v2") > Cc: Thomas Zimmermann > Cc: Christian König > Cc: Junwei Zhang > Cc: Alex Deucher > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Sean Paul > Cc: David Airlie > Cc: Christian Koenig > Cc: Huang Rui > Cc: dri-de...@lists.freedesktop.org > Signed-off-by: Sean Paul Reviewed-by: Christian König >>> Thanks for the review. Perhaps the best course of action for this >>> patch is to have Dave pick it directly and then we can backmerge it >>> into amdgpu/drm-misc-next from there? I could also spin a topic branch >>> that we could all merge. >>> >>> Thoughts? >> My preference is that I pick it up and push it to Dave through Alex >> fixes push. >> >> But just because the patch which broke this came the same way as well. >> > Yeah, unfortunately it's proliferated, and I'd rather not add hops to > getting drm-misc-next fixed. Let's just do a topic branch and set a PR > to Dave from that. We can both merge the topic branch asap and let > Dave pick it up at his leisure. Works for me as well, Christian. > > Sean > >> Either way works for me, >> Christian. >> >>> Sean >>> > --- > Documentation/gpu/drm-mm.rst | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst > index d0f3c6b032009..54a696d961a7c 100644 > --- a/Documentation/gpu/drm-mm.rst > +++ b/Documentation/gpu/drm-mm.rst > @@ -79,9 +79,6 @@ count for the TTM, which will call your initialization > function. > > See the radeon_ttm.c file for an example of usage. > > -.. kernel-doc:: drivers/gpu/drm/drm_global.c > - :export: > - > > The Graphics Execution Manager (GEM) > >>> ___ >>> dri-devel mailing list >>> dri-de...@lists.freedesktop.org >>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: TODO: Add DRM_MODESET_LOCK_ALL* conversion to todo.rst
On Thu, Nov 29, 2018 at 10:52:40AM -0500, Sean Paul wrote: > From: Sean Paul > > We should also get the drivers using the helpers. Seems like a good > intermediate TODO item. > > Cc: Daniel Vetter > Signed-off-by: Sean Paul Acked-by: Daniel Vetter > --- > Documentation/gpu/todo.rst | 12 > 1 file changed, 12 insertions(+) > > diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst > index ab347dec50796..1a6dc586fb156 100644 > --- a/Documentation/gpu/todo.rst > +++ b/Documentation/gpu/todo.rst > @@ -241,6 +241,18 @@ struct drm_gem_object_funcs > GEM objects can now have a function table instead of having the callbacks on > the > DRM driver struct. This is now the preferred way and drivers can be moved > over. > > +Use DRM_MODESET_LOCK_ALL_* helpers instead of boilerplate > +- > + > +For cases where drivers are attempting to grab the modeset locks with a local > +acquire context. Replace the boilerplate code surrounding > +drm_modeset_lock_all_ctx() with DRM_MODESET_LOCK_ALL_BEGIN() and > +DRM_MODESET_LOCK_ALL_END() instead. Maybe add: "This should also be done for all places where drm_modest_lock_all() is still used." Cheers, Daniel > + > +As a reference, take a look at the conversions already completed in drm core. > + > +Contact: Sean Paul, respective driver maintainers > + > Core refactorings > = > > -- > Sean Paul, Software Engineer, Google / Chromium OS > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [PATCH v12 23/25] kasan, arm64: select HAVE_ARCH_KASAN_SW_TAGS
On Tue, Nov 27, 2018 at 05:55:41PM +0100, Andrey Konovalov wrote: > Now, that all the necessary infrastructure code has been introduced, > select HAVE_ARCH_KASAN_SW_TAGS for arm64 to enable software tag-based > KASAN mode. > > Signed-off-by: Andrey Konovalov > --- > arch/arm64/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index 787d7850e064..8b331dcfb48e 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -111,6 +111,7 @@ config ARM64 > select HAVE_ARCH_JUMP_LABEL > select HAVE_ARCH_JUMP_LABEL_RELATIVE > select HAVE_ARCH_KASAN if !(ARM64_16K_PAGES && ARM64_VA_BITS_48) > + select HAVE_ARCH_KASAN_SW_TAGS if !(ARM64_16K_PAGES && ARM64_VA_BITS_48) Can you do if HAVE_ARCH_KASAN instead? Will
Re: [PATCH v12 20/25] kasan, arm64: add brk handler for inline instrumentation
On Tue, Nov 27, 2018 at 05:55:38PM +0100, Andrey Konovalov wrote: > Tag-based KASAN inline instrumentation mode (which embeds checks of shadow > memory into the generated code, instead of inserting a callback) generates > a brk instruction when a tag mismatch is detected. > > This commit adds a tag-based KASAN specific brk handler, that decodes the > immediate value passed to the brk instructions (to extract information > about the memory access that triggered the mismatch), reads the register > values (x0 contains the guilty address) and reports the bug. > > Reviewed-by: Andrey Ryabinin > Reviewed-by: Dmitry Vyukov > Signed-off-by: Andrey Konovalov > --- > arch/arm64/include/asm/brk-imm.h | 2 + > arch/arm64/kernel/traps.c| 68 +++- > include/linux/kasan.h| 3 ++ > 3 files changed, 71 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/include/asm/brk-imm.h > b/arch/arm64/include/asm/brk-imm.h > index ed693c5bcec0..2945fe6cd863 100644 > --- a/arch/arm64/include/asm/brk-imm.h > +++ b/arch/arm64/include/asm/brk-imm.h > @@ -16,10 +16,12 @@ > * 0x400: for dynamic BRK instruction > * 0x401: for compile time BRK instruction > * 0x800: kernel-mode BUG() and WARN() traps > + * 0x9xx: tag-based KASAN trap (allowed values 0x900 - 0x9ff) > */ > #define FAULT_BRK_IMM0x100 > #define KGDB_DYN_DBG_BRK_IMM 0x400 > #define KGDB_COMPILED_DBG_BRK_IMM0x401 > #define BUG_BRK_IMM 0x800 > +#define KASAN_BRK_IMM0x900 > > #endif > diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c > index 5f4d9acb32f5..04bdc53716ef 100644 > --- a/arch/arm64/kernel/traps.c > +++ b/arch/arm64/kernel/traps.c > @@ -35,6 +35,7 @@ > #include > #include > #include > +#include > > #include > #include > @@ -284,10 +285,14 @@ void arm64_notify_die(const char *str, struct pt_regs > *regs, > } > } > > -void arm64_skip_faulting_instruction(struct pt_regs *regs, unsigned long > size) > +void __arm64_skip_faulting_instruction(struct pt_regs *regs, unsigned long > size) > { > regs->pc += size; > +} > > +void arm64_skip_faulting_instruction(struct pt_regs *regs, unsigned long > size) > +{ > + __arm64_skip_faulting_instruction(regs, size); > /* >* If we were single stepping, we want to get the step exception after >* we return from the trap. > @@ -959,7 +964,7 @@ static int bug_handler(struct pt_regs *regs, unsigned int > esr) > } > > /* If thread survives, skip over the BUG instruction and continue: */ > - arm64_skip_faulting_instruction(regs, AARCH64_INSN_SIZE); > + __arm64_skip_faulting_instruction(regs, AARCH64_INSN_SIZE); Why do you want to avoid the single-step logic here? Given that we're skipping over the brk instruction, why wouldn't you want that to trigger a step exception if single-step is enabled? Will
Re: [PATCH v2] Documentation: admin-guide: PM: Add cpuidle document
On Thu, 29 Nov 2018 at 11:28, Rafael J. Wysocki wrote: > > On Thu, Nov 29, 2018 at 9:07 AM Ulf Hansson wrote: > > > > On Wed, 28 Nov 2018 at 12:47, Rafael J. Wysocki wrote: > > > > > > From: Rafael J. Wysocki > > > > > > Important information is missing from user/admin cpuidle documentation > > > available today, so add a new user/admin document for cpuidle containing > > > current and comprehensive information to admin-guide and drop the old > > > .txt documents it is replacing. > > > > > > Signed-off-by: Rafael J. Wysocki > > > --- > > > > Wow! Great work! > > > > [...] > > > > > +.. _idle-states-representation: > > > + > > > +Representation of Idle States > > > += > > > + > > > +For the CPU idle time management purposes all of the physical idle states > > > +supported by the processor have to be represented as a one-dimensional > > > array of > > > +|struct cpuidle_state| objects each allowing an individual (logical) CPU > > > to ask > > > +the processor hardware to enter an idle state of certain properties. If > > > there > > > +is a hierarchy of units in the processor, one |struct cpuidle_state| > > > object can > > > +cover a combination of idle states supported by the units at different > > > levels of > > > +the hierarchy. In that case, the `target residency and exit latency > > > parameters > > > +of it `_, must reflect the properties of the idle state at > > > the > > > +deepest level (i.e. the idle state of the unit containing all of the > > > other > > > +units). > > > + > > > +For example, take a processor with two cores in a larger unit referred > > > to as > > > +a "module" and suppose that asking the hardware to enter a specific idle > > > state > > > +(say "X") at the "core" level by one core will trigger the module to try > > > to > > > +enter a specific idle state of its own (say "MX") if the other core is > > > in idle > > > +state "X" already. In other words, asking for idle state "X" at the > > > "core" > > > +level gives the hardware a license to go as deep as to idle state "MX" > > > at the > > > +"module" level, but there is no guarantee that this is going to happen > > > (the core > > > +asking for idle state "X" may just end up in that state by itself > > > instead). > > > > This is a good description of a HW like x86 and ARM64 (using PSCI in > > platform coordinated mode). > > > > However, for "legacy" ARM products, having at least two cores in a > > module, this description does in many cases *not* match how the HW > > works. I wonder about other architectures. > > Well, this is an example and it is very clearly presented this way IMO > (it says "for example" upfront and then "suppose" etc). > > Basically, the purpose of it is to explain that the parameters shown > in sysfs may be effective. > > > My point is, I think it would be fair to mention that the above > > description is specific to certain HWs. In other words, for HWs that > > leaves the entire state synchronization of the last man standing > > algorithm to be managed by a FW, the cpuidle framework plays well. But > > for others it has limitations. > > I can add it after this entire paragraph I think. Something like > > "There are processors without direct coordination between different > levels of the hierarchy of units inside them, however. In those cases > asking for an idle state at the "core" level does not automatically > affect the "module" level, for example, in any way and the CPUIdle > driver is responsible for the entire handling of the hierarchy. Then, > the definition of the idle state objects is entirely up to the driver, > but still the physical properties of the idle state that the processor > hardware finally goes into must always follow the parameters used by > the governor for idle state selection (for instance, the actual exit > latency of that idle state must not exceed the exit latency parameter > of the idle state object selected by the governor)." I like this! This leaves some room for understanding that cpuidle has limitations when in comes down to these kind of platforms, which seems like reasonable statement to include. With that, feel free to add: Reviewed-by: Ulf Hansson > > > > > > +Then, the target residency of the |struct cpuidle_state| object > > > representing > > > +idle state "X" must reflect the minimum time to spend in idle state "MX" > > > of > > > +the module (including the time needed to enter it), because that is the > > > minimum > > > +time the CPU needs to be idle to save any energy in case the hardware > > > enters > > > +that state. Analogously, the exit latency parameter of that object must > > > cover > > > +the exit time of idle state "MX" of the module (and usually its entry > > > time too), > > > +because that is the maximum delay between a wakeup signal and the time > > > the CPU > > > +will start to execute the first new instruction (assuming that both > > > cores in the > > > +module will always be
Re: [PATCH v8 0/8] arm64: untag user pointers passed to the kernel
Hi Andrey, On Thu, Nov 08, 2018 at 03:48:10PM +0100, Andrey Konovalov wrote: > On Thu, Nov 8, 2018 at 3:36 PM, Andrey Konovalov > wrote: > > Changes in v8: > > - Rebased onto 65102238 (4.20-rc1). > > - Added a note to the cover letter on why syscall wrappers/shims that untag > > user pointers won't work. > > - Added a note to the cover letter that this patchset has been merged into > > the Pixel 2 kernel tree. > > - Documentation fixes, in particular added a list of syscalls that don't > > support tagged user pointers. > > I've changed the documentation to be more specific, please take a look. > > I haven't done anything about adding a way for the user to find out > that the kernel supports this ABI extension. I don't know what would > the the preferred way to do this, and we haven't received any comments > on that from anybody else. Probing "on some innocuous syscall > currently returning -EFAULT on tagged pointer arguments" works though, > as you mentioned. We've had some internal discussions and also talked to some people at Plumbers. I think the best option is to introduce an AT_FLAGS bit to describe the ABI relaxation on tagged pointers. Vincenzo is going to propose a patch on top of this series. > As mentioned in the cover letter, this patchset has been merged into > the Pixel 2 kernel tree. I just hope it's not enabled on production kernels, it would introduce a user ABI that may differ from what ends up upstream. -- Catalin
Re: [PATCH v8 1/8] arm64: add type casts to untagged_addr macro
On Thu, Nov 08, 2018 at 03:36:08PM +0100, Andrey Konovalov wrote: > This patch makes the untagged_addr macro accept all kinds of address types > (void *, unsigned long, etc.) and allows not to specify type casts in each > place where it is used. This is done by using __typeof__. > > Signed-off-by: Andrey Konovalov > --- > arch/arm64/include/asm/uaccess.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/include/asm/uaccess.h > b/arch/arm64/include/asm/uaccess.h > index 07c34087bd5e..c1325271e368 100644 > --- a/arch/arm64/include/asm/uaccess.h > +++ b/arch/arm64/include/asm/uaccess.h > @@ -101,7 +101,8 @@ static inline unsigned long __range_ok(const void __user > *addr, unsigned long si > * up with a tagged userland pointer. Clear the tag to get a sane pointer to > * pass on to access_ok(), for instance. > */ > -#define untagged_addr(addr) sign_extend64(addr, 55) > +#define untagged_addr(addr) \ > + ((__typeof__(addr))sign_extend64((__u64)(addr), 55)) Nitpick: same comment as here (use u64): http://lkml.kernel.org/r/20181123173739.osgvnnhmptdgt...@lakrids.cambridge.arm.com Acked-by: Catalin Marinas (not acking the whole series just yet, only specific patches to remember what I reviewed)
Re: [PATCH v8 2/8] uaccess: add untagged_addr definition for other arches
On Thu, Nov 08, 2018 at 03:36:09PM +0100, Andrey Konovalov wrote: > diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h > index efe79c1cdd47..c045b4eff95e 100644 > --- a/include/linux/uaccess.h > +++ b/include/linux/uaccess.h > @@ -13,6 +13,10 @@ > > #include > > +#ifndef untagged_addr > +#define untagged_addr(addr) addr > +#endif Nitpick: add braces around (addr). Otherwise: Acked-by: Catalin Marinas
Re: [PATCH v8 3/8] arm64: untag user addresses in access_ok and __uaccess_mask_ptr
On Thu, Nov 08, 2018 at 03:36:10PM +0100, Andrey Konovalov wrote: > copy_from_user (and a few other similar functions) are used to copy data > from user memory into the kernel memory or vice versa. Since a user can > provided a tagged pointer to one of the syscalls that use copy_from_user, > we need to correctly handle such pointers. > > Do this by untagging user pointers in access_ok and in __uaccess_mask_ptr, > before performing access validity checks. > > Signed-off-by: Andrey Konovalov Reviewed-by: Catalin Marinas
[PATCH v2] drm: TODO: Add DRM_MODESET_LOCK_ALL* conversion to todo.rst
From: Sean Paul We should also get the drivers using the helpers. Seems like a good intermediate TODO item. Changes in v2: - Add line about replacing drm_modest_lock_all() (Daniel) Acked-by: Daniel Vetter Signed-off-by: Sean Paul --- Documentation/gpu/todo.rst | 15 +++ 1 file changed, 15 insertions(+) diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst index ab347dec50796..14191b64446df 100644 --- a/Documentation/gpu/todo.rst +++ b/Documentation/gpu/todo.rst @@ -241,6 +241,21 @@ struct drm_gem_object_funcs GEM objects can now have a function table instead of having the callbacks on the DRM driver struct. This is now the preferred way and drivers can be moved over. +Use DRM_MODESET_LOCK_ALL_* helpers instead of boilerplate +- + +For cases where drivers are attempting to grab the modeset locks with a local +acquire context. Replace the boilerplate code surrounding +drm_modeset_lock_all_ctx() with DRM_MODESET_LOCK_ALL_BEGIN() and +DRM_MODESET_LOCK_ALL_END() instead. + +This should also be done for all places where drm_modest_lock_all() is still +used. + +As a reference, take a look at the conversions already completed in drm core. + +Contact: Sean Paul, respective driver maintainers + Core refactorings = -- Sean Paul, Software Engineer, Google / Chromium OS
Re: [PATCH v2] drm: TODO: Add DRM_MODESET_LOCK_ALL* conversion to todo.rst
On Thu, Nov 29, 2018 at 02:58:33PM -0500, Sean Paul wrote: > From: Sean Paul > > We should also get the drivers using the helpers. Seems like a good > intermediate TODO item. > > Changes in v2: > - Add line about replacing drm_modest_lock_all() (Daniel) > > Acked-by: Daniel Vetter Pushed this to -misc-next > Signed-off-by: Sean Paul > --- > Documentation/gpu/todo.rst | 15 +++ > 1 file changed, 15 insertions(+) > > diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst > index ab347dec50796..14191b64446df 100644 > --- a/Documentation/gpu/todo.rst > +++ b/Documentation/gpu/todo.rst > @@ -241,6 +241,21 @@ struct drm_gem_object_funcs > GEM objects can now have a function table instead of having the callbacks on > the > DRM driver struct. This is now the preferred way and drivers can be moved > over. > > +Use DRM_MODESET_LOCK_ALL_* helpers instead of boilerplate > +- > + > +For cases where drivers are attempting to grab the modeset locks with a local > +acquire context. Replace the boilerplate code surrounding > +drm_modeset_lock_all_ctx() with DRM_MODESET_LOCK_ALL_BEGIN() and > +DRM_MODESET_LOCK_ALL_END() instead. > + > +This should also be done for all places where drm_modest_lock_all() is still > +used. > + > +As a reference, take a look at the conversions already completed in drm core. > + > +Contact: Sean Paul, respective driver maintainers > + > Core refactorings > = > > -- > Sean Paul, Software Engineer, Google / Chromium OS > -- Sean Paul, Software Engineer, Google / Chromium OS
Re: [PATCH v8 04/14] integrity: Introduce struct evm_xattr
On Fri, 2018-11-16 at 18:07 -0200, Thiago Jung Bauermann wrote: > Even though struct evm_ima_xattr_data includes a fixed-size array to hold a > SHA1 digest, most of the code ignores the array and uses the struct to mean > "type indicator followed by data of unspecified size" and tracks the real > size of what the struct represents in a separate length variable. > > The only exception to that is the EVM code, which correctly uses the > definition of struct evm_ima_xattr_data. > > So make this explicit in the code by removing the length specification from > the array in struct evm_ima_xattr_data. Also, change the name of the > element from digest to data since in most places the array doesn't hold a > digest. > > A separate struct evm_xattr is introduced, with the original definition of > evm_ima_xattr_data to be used in the places that actually expect that > definition. , specifically the EVM HMAC code. > > Signed-off-by: Thiago Jung Bauermann Other than commenting the evm_xattr usage is limited to HMAC before the structure definition, this looks good. Reviewed-by: Mimi Zohar > --- > security/integrity/evm/evm_main.c | 8 > security/integrity/ima/ima_appraise.c | 7 --- > security/integrity/integrity.h| 5 + > 3 files changed, 13 insertions(+), 7 deletions(-) > > diff --git a/security/integrity/evm/evm_main.c > b/security/integrity/evm/evm_main.c > index 7f3f54d89a6e..a1b42d10efc7 100644 > --- a/security/integrity/evm/evm_main.c > +++ b/security/integrity/evm/evm_main.c > @@ -169,7 +169,7 @@ static enum integrity_status evm_verify_hmac(struct > dentry *dentry, > /* check value type */ > switch (xattr_data->type) { > case EVM_XATTR_HMAC: > - if (xattr_len != sizeof(struct evm_ima_xattr_data)) { > + if (xattr_len != sizeof(struct evm_xattr)) { > evm_status = INTEGRITY_FAIL; > goto out; > } > @@ -179,7 +179,7 @@ static enum integrity_status evm_verify_hmac(struct > dentry *dentry, > xattr_value_len, &digest); > if (rc) > break; > - rc = crypto_memneq(xattr_data->digest, digest.digest, > + rc = crypto_memneq(xattr_data->data, digest.digest, > SHA1_DIGEST_SIZE); > if (rc) > rc = -EINVAL; > @@ -523,7 +523,7 @@ int evm_inode_init_security(struct inode *inode, >const struct xattr *lsm_xattr, >struct xattr *evm_xattr) > { > - struct evm_ima_xattr_data *xattr_data; > + struct evm_xattr *xattr_data; > int rc; > > if (!evm_key_loaded() || !evm_protected_xattr(lsm_xattr->name)) > @@ -533,7 +533,7 @@ int evm_inode_init_security(struct inode *inode, > if (!xattr_data) > return -ENOMEM; > > - xattr_data->type = EVM_XATTR_HMAC; > + xattr_data->data.type = EVM_XATTR_HMAC; > rc = evm_init_hmac(inode, lsm_xattr, xattr_data->digest); > if (rc < 0) > goto out; > diff --git a/security/integrity/ima/ima_appraise.c > b/security/integrity/ima/ima_appraise.c > index deec1804a00a..8bcef90939f8 100644 > --- a/security/integrity/ima/ima_appraise.c > +++ b/security/integrity/ima/ima_appraise.c > @@ -167,7 +167,8 @@ enum hash_algo ima_get_hash_algo(struct > evm_ima_xattr_data *xattr_value, > return sig->hash_algo; > break; > case IMA_XATTR_DIGEST_NG: > - ret = xattr_value->digest[0]; > + /* first byte contains algorithm id */ > + ret = xattr_value->data[0]; > if (ret < HASH_ALGO__LAST) > return ret; > break; > @@ -175,7 +176,7 @@ enum hash_algo ima_get_hash_algo(struct > evm_ima_xattr_data *xattr_value, > /* this is for backward compatibility */ > if (xattr_len == 21) { > unsigned int zero = 0; > - if (!memcmp(&xattr_value->digest[16], &zero, 4)) > + if (!memcmp(&xattr_value->data[16], &zero, 4)) > return HASH_ALGO_MD5; > else > return HASH_ALGO_SHA1; > @@ -274,7 +275,7 @@ int ima_appraise_measurement(enum ima_hooks func, > /* xattr length may be longer. md5 hash in previous > version occupied 20 bytes in xattr, instead of 16 >*/ > - rc = memcmp(&xattr_value->digest[hash_start], > + rc = memcmp(&xattr_value->data[hash_start], > iint->ima_hash->digest, > iint->ima_hash->length); > else > diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h > index e60473b13a8d..20ac02bf1b84 100644 > --- a/securit
[PATCH v3] Documentation: admin-guide: PM: Add cpuidle document
From: Rafael J. Wysocki Important information is missing from user/admin cpuidle documentation available today, so add a new user/admin document for cpuidle containing current and comprehensive information to admin-guide and drop the old .txt documents it is replacing. Signed-off-by: Rafael J. Wysocki Reviewed-by: Viresh Kumar Reviewed-by: Ulf Hansson --- v2 -> v3: * Add paragraph covering processors without automatic coordination between different herarchy levels in hardware (Ulf). * Add Reviewed-by tags from Viresh and Ulf. -> v2: * Fix typos. * Improve description of the state0, state1, etc directories to cover the relationship between their numbers and the represented idle states depth (Viresh). --- Documentation/admin-guide/pm/cpuidle.rst | 615 + Documentation/admin-guide/pm/working-state.rst |1 Documentation/cpuidle/core.txt | 23 Documentation/cpuidle/sysfs.txt| 98 --- 4 files changed, 616 insertions(+), 121 deletions(-) Index: linux-pm/Documentation/admin-guide/pm/cpuidle.rst === --- /dev/null +++ linux-pm/Documentation/admin-guide/pm/cpuidle.rst @@ -0,0 +1,615 @@ +.. |struct cpuidle_state| replace:: :c:type:`struct cpuidle_state ` +.. |cpufreq| replace:: :doc:`CPU Performance Scaling ` + + +CPU Idle Time Management + + +:: + + Copyright (c) 2018 Intel Corp., Rafael J. Wysocki + +Concepts + + +Modern processors are generally able to enter states in which the execution of +a program is suspended and instructions belonging to it are not fetched from +memory or executed. Those states are the *idle* states of the processor. + +Since part of the processor hardware is not used in idle states, entering them +generally allows power drawn by the processor to be reduced and, in consequence, +it is an opportunity to save energy. + +CPU idle time management is an energy-efficiency feature concerned about using +the idle states of processors for this purpose. + +Logical CPUs + + +CPU idle time management operates on CPUs as seen by the *CPU scheduler* (that +is the part of the kernel responsible for the distribution of computational +work in the system). In its view, CPUs are *logical* units. That is, they need +not be separate physical entities and may just be interfaces appearing to +software as individual single-core processors. In other words, a CPU is an +entity which appears to be fetching instructions that belong to one sequence +(program) from memory and executing them, but it need not work this way +physically. Generally, three different cases can be consider here. + +First, if the whole processor can only follow one sequence of instructions (one +program) at a time, it is a CPU. In that case, if the hardware is asked to +enter an idle state, that applies to the processor as a whole. + +Second, if the processor is multi-core, each core in it is able to follow at +least one program at a time. The cores need not be entirely independent of each +other (for example, they may share caches), but still most of the time they +work physically in parallel with each other, so if each of them executes only +one program, those programs run mostly independently of each other at the same +time. The entire cores are CPUs in that case and if the hardware is asked to +enter an idle state, that applies to the core that asked for it in the first +place, but it also may apply to a larger unit (say a "package" or a "cluster") +that the core belongs to (in fact, it may apply to an entire hierarchy of larger +units containing the core). Namely, if all of the cores in the larger unit +except for one have been put into idle states at the "core level" and the +remaining core asks the processor to enter an idle state, that may trigger it +to put the whole larger unit into an idle state which also will affect the +other cores in that unit. + +Finally, each core in a multi-core processor may be able to follow more than one +program in the same time frame (that is, each core may be able to fetch +instructions from multiple locations in memory and execute them in the same time +frame, but not necessarily entirely in parallel with each other). In that case +the cores present themselves to software as "bundles" each consisting of +multiple individual single-core "processors", referred to as *hardware threads* +(or hyper-threads specifically on Intel hardware), that each can follow one +sequence of instructions. Then, the hardware threads are CPUs from the CPU idle +time management perspective and if the processor is asked to enter an idle state +by one of them, the hardware thread (or CPU) that asked for it is stopped, but +nothing more happens, unless all of the other hardware threads within the same +core also have asked the processor to enter an idle state. In that situ
Re: [PATCH v3] Documentation: admin-guide: PM: Add cpuidle document
On Thu, 29 Nov 2018 22:22:04 +0100 "Rafael J. Wysocki" wrote: > Important information is missing from user/admin cpuidle documentation > available today, so add a new user/admin document for cpuidle containing > current and comprehensive information to admin-guide and drop the old > .txt documents it is replacing. I've not had a chance to read through it in depth, but a first scan looks good. I assume this is going up via your tree? Thanks, jon
Re: [PATCH v3] Documentation: admin-guide: PM: Add cpuidle document
On Thu, Nov 29, 2018 at 10:40 PM Jonathan Corbet wrote: > > On Thu, 29 Nov 2018 22:22:04 +0100 > "Rafael J. Wysocki" wrote: > > > Important information is missing from user/admin cpuidle documentation > > available today, so add a new user/admin document for cpuidle containing > > current and comprehensive information to admin-guide and drop the old > > .txt documents it is replacing. > > I've not had a chance to read through it in depth, but a first scan looks > good. Thanks! > I assume this is going up via your tree? Yes, I would like to take it as there may be new patches on top of it. Cheers, Rafael