Re: [v8 0/4] cgroup-aware OOM killer
I'm excited to see this being discussed again - it's been years since the last attempt. I've tried to stay out of the conversation, but I feel obligated say something and then go back to lurking. On Tue, Sep 26, 2017 at 10:26 AM, Johannes Weiner wrote: > On Tue, Sep 26, 2017 at 03:30:40PM +0200, Michal Hocko wrote: >> On Tue 26-09-17 13:13:00, Roman Gushchin wrote: >> > On Tue, Sep 26, 2017 at 01:21:34PM +0200, Michal Hocko wrote: >> > > On Tue 26-09-17 11:59:25, Roman Gushchin wrote: >> > > > On Mon, Sep 25, 2017 at 10:25:21PM +0200, Michal Hocko wrote: >> > > > > On Mon 25-09-17 19:15:33, Roman Gushchin wrote: >> > > > > [...] >> > > > > > I'm not against this model, as I've said before. It feels logical, >> > > > > > and will work fine in most cases. >> > > > > > >> > > > > > In this case we can drop any mount/boot options, because it >> > > > > > preserves >> > > > > > the existing behavior in the default configuration. A big >> > > > > > advantage. >> > > > > >> > > > > I am not sure about this. We still need an opt-in, ragardless, >> > > > > because >> > > > > selecting the largest process from the largest memcg != selecting the >> > > > > largest task (just consider memcgs with many processes example). >> > > > >> > > > As I understand Johannes, he suggested to compare individual processes >> > > > with >> > > > group_oom mem cgroups. In other words, always select a killable entity >> > > > with >> > > > the biggest memory footprint. >> > > > >> > > > This is slightly different from my v8 approach, where I treat leaf >> > > > memcgs >> > > > as indivisible memory consumers independent on group_oom setting, so >> > > > by default I'm selecting the biggest task in the biggest memcg. >> > > >> > > My reading is that he is actually proposing the same thing I've been >> > > mentioning. Simply select the biggest killable entity (leaf memcg or >> > > group_oom hierarchy) and either kill the largest task in that entity >> > > (for !group_oom) or the whole memcg/hierarchy otherwise. >> > >> > He wrote the following: >> > "So I'm leaning toward the second model: compare all oomgroups and >> > standalone tasks in the system with each other, independent of the >> > failed hierarchical control structure. Then kill the biggest of them." >> >> I will let Johannes to comment but I believe this is just a >> misunderstanding. If we compared only the biggest task from each memcg >> then we are basically losing our fairness objective, aren't we? > > Sorry about the confusion. > > Yeah I was making the case for what Michal proposed, to kill the > biggest terminal consumer, which is either a task or an oomgroup. > > You'd basically iterate through all the tasks and cgroups in the > system and pick the biggest task that isn't in an oom group or the > biggest oom group and then kill that. > > Yeah, you'd have to compare the memory footprints of tasks with the > memory footprints of cgroups. These aren't defined identically, and > tasks don't get attributed every type of allocation that a cgroup > would. But it should get us in the ballpark, and I cannot picture a > scenario where this would lead to a completely undesirable outcome. That last sentence: > I cannot picture a scenario where this would lead to a completely undesirable > outcome. I feel like David has offered examples here, and many of us at Google have offered examples as long ago as 2013 (if I recall) of cases where the proposed heuristic is EXACTLY WRONG. We need OOM behavior to kill in a deterministic order configured by policy. Sometimes, I would literally prefer to kill every other cgroup before killing "the big one". The policy is *all* that matters for shared clusters of varying users and priorities. We did this in Borg, and it works REALLY well. Has for years. Now that the world is adopting Kubernetes we need it again, only it's much harder to carry a kernel patch in this case. -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] scripts: kernel-doc: fix nexted handling
Hi Mauro,
[auto build test WARNING on linus/master]
[also build test WARNING on v4.14-rc2 next-20170926]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Mauro-Carvalho-Chehab/scripts-kernel-doc-fix-nexted-handling/20170927-091127
reproduce: make htmldocs
All warnings (new ones prefixed by >>):
WARNING: convert(1) not found, for SVG to PDF conversion install ImageMagick
(https://www.imagemagick.org)
kernel/trace/blktrace.c:824: warning: No description found for parameter
'cgid'
>> include/net/cfg80211.h:2056: warning: Excess struct/union/enum/typedef
>> member 'band_pref' description in 'cfg80211_bss_selection'
>> include/net/cfg80211.h:2056: warning: Excess struct/union/enum/typedef
>> member 'adjust' description in 'cfg80211_bss_selection'
>> include/net/cfg80211.h:4115: warning: Excess struct/union/enum/typedef
>> member 'bssid' description in 'wireless_dev'
>> include/net/cfg80211.h:2056: warning: Excess struct/union/enum/typedef
>> member 'band_pref' description in 'cfg80211_bss_selection'
>> include/net/cfg80211.h:2056: warning: Excess struct/union/enum/typedef
>> member 'adjust' description in 'cfg80211_bss_selection'
>> include/net/cfg80211.h:4115: warning: Excess struct/union/enum/typedef
>> member 'bssid' description in 'wireless_dev'
>> include/net/cfg80211.h:2056: warning: Excess struct/union/enum/typedef
>> member 'band_pref' description in 'cfg80211_bss_selection'
>> include/net/cfg80211.h:2056: warning: Excess struct/union/enum/typedef
>> member 'adjust' description in 'cfg80211_bss_selection'
>> include/net/cfg80211.h:4115: warning: Excess struct/union/enum/typedef
>> member 'bssid' description in 'wireless_dev'
>> include/net/cfg80211.h:2056: warning: Excess struct/union/enum/typedef
>> member 'band_pref' description in 'cfg80211_bss_selection'
>> include/net/cfg80211.h:2056: warning: Excess struct/union/enum/typedef
>> member 'adjust' description in 'cfg80211_bss_selection'
>> include/net/cfg80211.h:4115: warning: Excess struct/union/enum/typedef
>> member 'bssid' description in 'wireless_dev'
>> include/net/cfg80211.h:2056: warning: Excess struct/union/enum/typedef
>> member 'band_pref' description in 'cfg80211_bss_selection'
>> include/net/cfg80211.h:2056: warning: Excess struct/union/enum/typedef
>> member 'adjust' description in 'cfg80211_bss_selection'
>> include/net/cfg80211.h:4115: warning: Excess struct/union/enum/typedef
>> member 'bssid' description in 'wireless_dev'
>> include/net/cfg80211.h:2056: warning: Excess struct/union/enum/typedef
>> member 'band_pref' description in 'cfg80211_bss_selection'
>> include/net/cfg80211.h:2056: warning: Excess struct/union/enum/typedef
>> member 'adjust' description in 'cfg80211_bss_selection'
>> include/net/cfg80211.h:4115: warning: Excess struct/union/enum/typedef
>> member 'bssid' description in 'wireless_dev'
>> include/net/cfg80211.h:2056: warning: Excess struct/union/enum/typedef
>> member 'band_pref' description in 'cfg80211_bss_selection'
>> include/net/cfg80211.h:2056: warning: Excess struct/union/enum/typedef
>> member 'adjust' description in 'cfg80211_bss_selection'
vim +2056 include/net/cfg80211.h
04a773ad Johannes Berg2009-04-19 2041
04a773ad Johannes Berg2009-04-19 2042 /**
38de03d2 Arend van Spriel 2016-03-02 2043 * struct cfg80211_bss_selection -
connection parameters for BSS selection.
38de03d2 Arend van Spriel 2016-03-02 2044 *
38de03d2 Arend van Spriel 2016-03-02 2045 * @behaviour: requested BSS
selection behaviour.
38de03d2 Arend van Spriel 2016-03-02 2046 * @param: parameters for
requestion behaviour.
38de03d2 Arend van Spriel 2016-03-02 2047 * @band_pref: preferred band for
%NL80211_BSS_SELECT_ATTR_BAND_PREF.
38de03d2 Arend van Spriel 2016-03-02 2048 * @adjust: parameters for
%NL80211_BSS_SELECT_ATTR_RSSI_ADJUST.
38de03d2 Arend van Spriel 2016-03-02 2049 */
38de03d2 Arend van Spriel 2016-03-02 2050 struct cfg80211_bss_selection {
38de03d2 Arend van Spriel 2016-03-02 2051 enum nl80211_bss_select_attr
behaviour;
38de03d2 Arend van Spriel 2016-03-02 2052 union {
57fbcce3 Johannes Berg2016-04-12 2053 enum nl80211_band
band_pref;
38de03d2 Arend van Spriel 2016-03-02 2054 struct
cfg80211_bss_select_adjust adjust;
38de03d2 Arend van Spriel 2016-03-02 2055 } param;
38de03d2 Arend van Spriel 2016-03-02 @2056 };
38de03d2 Arend van Spriel 2016-03-02 2057
:: The code at line 2056 was first introduced by commit
:: 38de03d2a28925b489c11546804e2f5418cc17a4 nl80211: add feature for BSS
selection support
:: TO: Arend van Spriel
:: CC: Johannes Berg
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
Re: [PATCH 07/10] docs: kernel-doc.rst: add documentation about man pages
On 09/26/17 10:59, Mauro Carvalho Chehab wrote:
> kernel-doc-nano-HOWTO.txt has a chapter about man pages
> production. While we don't have a working "make manpages"
> target, add it.
>
> Signed-off-by: Mauro Carvalho Chehab
> ---
> Documentation/doc-guide/kernel-doc.rst | 61
> ++
> 1 file changed, 47 insertions(+), 14 deletions(-)
>
> diff --git a/Documentation/doc-guide/kernel-doc.rst
> b/Documentation/doc-guide/kernel-doc.rst
> index 9777aa53e3dd..50473f0db345 100644
> --- a/Documentation/doc-guide/kernel-doc.rst
> +++ b/Documentation/doc-guide/kernel-doc.rst
> @@ -377,7 +377,6 @@ cross-references.
> For further details, please refer to the `Sphinx C Domain`_ documentation.
>
>
> -
> In-line member documentation comments
> ~
>
> @@ -391,19 +390,19 @@ on a line of their own, like all other kernel-doc
> comments::
> * @foo: The Foo member.
> */
>struct foo {
> -int foo;
> -/**
> - * @bar: The Bar member.
> - */
> -int bar;
> -/**
> - * @baz: The Baz member.
> - *
> - * Here, the member description may contain several paragraphs.
> - */
> -int baz;
> -/** @foobar: Single line description. */
> -int foobar;
> + int foo;
> + /**
> + * @bar: The Bar member.
> + */
> + int bar;
> + /**
> + * @baz: The Baz member.
> + *
> + * Here, the member description may contain several paragraphs.
> + */
> + int baz;
> + /** @foobar: Single line description. */
> + int foobar;
>}
The above doesn't belong in this patch. (??)
>
> @@ -452,3 +451,37 @@ file.
>
> Data structures visible in kernel include files should also be documented
> using
> kernel-doc formatted comments.
> +
> +How to use kernel-doc to generate man pages
> +---
> +
> +If you just want to use kernel-doc to generate man pages you can do this
> +from the Kernel git tree::
> +
> + $ scripts/kernel-doc -man $(git grep -l '/\*\*' |grep -v Documentation/) |
> ./split-man.pl /tmp/man
> +
> +Using the small ``split-man.pl`` script below::
> +
> +
> + #!/usr/bin/perl
> +
> + if ($#ARGV < 0) {
> + die "where do I put the results?\n";
> + }
> +
> + mkdir $ARGV[0],0777;
> + $state = 0;
> + while () {
> + if (/^\.TH \"[^\"]*\" 9 \"([^\"]*)\"/) {
> + if ($state == 1) { close OUT }
> + $state = 1;
> + $fn = "$ARGV[0]/$1.9";
> + print STDERR "Creating $fn\n";
> + open OUT, ">$fn" or die "can't open $fn: $!\n";
> + print OUT $_;
> + } elsif ($state != 0) {
> + print OUT $_;
> + }
> + }
> +
> + close OUT;
>
--
~Randy
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/10] docs: kernel-doc.rst: better describe kernel-doc arguments
On 09/26/17 10:59, Mauro Carvalho Chehab wrote: > Add a new section to describe kernel-doc arguments, > adding examples about how identation should happen, as failing > to do that causes Sphinx to do the wrong thing. > > Signed-off-by: Mauro Carvalho Chehab > --- > Documentation/doc-guide/kernel-doc.rst | 44 > +++--- > 1 file changed, 41 insertions(+), 3 deletions(-) > > diff --git a/Documentation/doc-guide/kernel-doc.rst > b/Documentation/doc-guide/kernel-doc.rst > index b24854b5d6be..7a3f5c710c0b 100644 > --- a/Documentation/doc-guide/kernel-doc.rst > +++ b/Documentation/doc-guide/kernel-doc.rst > @@ -112,16 +112,17 @@ Example kernel-doc function comment:: > >/** > * foobar() - Brief description of foobar. > - * @arg: Description of argument of foobar. > + * @argument1: Description of parameter argument1 of foobar. > + * @argument1: Description of parameter argument2 of foobar. @argument2: > * > * Longer description of foobar. > * > * Return: Description of return value of foobar. > */ > - int foobar(int arg) > + int foobar(int argument1, char *argument2) -- ~Randy -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[lwn:docs-next 9/9] htmldocs: include/net/cfg80211.h:3278: warning: Excess enum value 'WIPHY_FLAG_SUPPORTS_SCHED_SCAN' description in 'wiphy_flags'
tree: git://git.lwn.net/linux-2.6 docs-next head: 5cb5c31cdf246099f7d48a57f448b05b7941cd6a commit: 5cb5c31cdf246099f7d48a57f448b05b7941cd6a [9/9] scripts/kernel-doc: warn on excess enum value descriptions reproduce: make htmldocs All warnings (new ones prefixed by >>): WARNING: convert(1) not found, for SVG to PDF conversion install ImageMagick (https://www.imagemagick.org) kernel/trace/blktrace.c:818: warning: No description found for parameter 'cgid' >> include/net/cfg80211.h:3278: warning: Excess enum value >> 'WIPHY_FLAG_SUPPORTS_SCHED_SCAN' description in 'wiphy_flags' >> include/net/cfg80211.h:3278: warning: Excess enum value >> 'WIPHY_FLAG_SUPPORTS_SCHED_SCAN' description in 'wiphy_flags' >> include/net/cfg80211.h:3278: warning: Excess enum value >> 'WIPHY_FLAG_SUPPORTS_SCHED_SCAN' description in 'wiphy_flags' >> include/net/cfg80211.h:3278: warning: Excess enum value >> 'WIPHY_FLAG_SUPPORTS_SCHED_SCAN' description in 'wiphy_flags' >> include/net/cfg80211.h:3278: warning: Excess enum value >> 'WIPHY_FLAG_SUPPORTS_SCHED_SCAN' description in 'wiphy_flags' >> include/net/cfg80211.h:3278: warning: Excess enum value >> 'WIPHY_FLAG_SUPPORTS_SCHED_SCAN' description in 'wiphy_flags' >> include/net/cfg80211.h:3278: warning: Excess enum value >> 'WIPHY_FLAG_SUPPORTS_SCHED_SCAN' description in 'wiphy_flags' >> include/net/cfg80211.h:3278: warning: Excess enum value >> 'WIPHY_FLAG_SUPPORTS_SCHED_SCAN' description in 'wiphy_flags' >> include/net/cfg80211.h:3278: warning: Excess enum value >> 'WIPHY_FLAG_SUPPORTS_SCHED_SCAN' description in 'wiphy_flags' >> include/net/cfg80211.h:3278: warning: Excess enum value >> 'WIPHY_FLAG_SUPPORTS_SCHED_SCAN' description in 'wiphy_flags' >> include/net/cfg80211.h:3278: warning: Excess enum value >> 'WIPHY_FLAG_SUPPORTS_SCHED_SCAN' description in 'wiphy_flags' >> include/net/cfg80211.h:3278: warning: Excess enum value >> 'WIPHY_FLAG_SUPPORTS_SCHED_SCAN' description in 'wiphy_flags' >> include/net/cfg80211.h:3278: warning: Excess enum value >> 'WIPHY_FLAG_SUPPORTS_SCHED_SCAN' description in 'wiphy_flags' >> include/net/cfg80211.h:3278: warning: Excess enum value >> 'WIPHY_FLAG_SUPPORTS_SCHED_SCAN' description in 'wiphy_flags' >> include/net/cfg80211.h:3278: warning: Excess enum value >> 'WIPHY_FLAG_SUPPORTS_SCHED_SCAN' description in 'wiphy_flags' >> include/net/cfg80211.h:3278: warning: Excess enum value >> 'WIPHY_FLAG_SUPPORTS_SCHED_SCAN' description in 'wiphy_flags' >> include/net/cfg80211.h:3278: warning: Excess enum value >> 'WIPHY_FLAG_SUPPORTS_SCHED_SCAN' description in 'wiphy_flags' >> include/net/cfg80211.h:3278: warning: Excess enum value >> 'WIPHY_FLAG_SUPPORTS_SCHED_SCAN' description in 'wiphy_flags' >> include/net/cfg80211.h:3278: warning: Excess enum value >> 'WIPHY_FLAG_SUPPORTS_SCHED_SCAN' description in 'wiphy_flags' >> include/net/cfg80211.h:3278: warning: Excess enum value >> 'WIPHY_FLAG_SUPPORTS_SCHED_SCAN' description in 'wiphy_flags' vim +3278 include/net/cfg80211.h 704232c27 Johannes Berg 2007-04-23 3205 d32365537 Johannes Berg 2009-04-20 3206 /* d32365537 Johannes Berg 2009-04-20 3207 * wireless hardware and networking interfaces structures d32365537 Johannes Berg 2009-04-20 3208 * and registration/helper functions d32365537 Johannes Berg 2009-04-20 3209 */ d32365537 Johannes Berg 2009-04-20 3210 d32365537 Johannes Berg 2009-04-20 3211 /** 5be83de54 Johannes Berg 2009-11-19 3212 * enum wiphy_flags - wiphy capability flags 5be83de54 Johannes Berg 2009-11-19 3213 * 5be83de54 Johannes Berg 2009-11-19 3214 * @WIPHY_FLAG_NETNS_OK: if not set, do not allow changing the netns of this 5be83de54 Johannes Berg 2009-11-19 3215 *wiphy at all 5be83de54 Johannes Berg 2009-11-19 3216 * @WIPHY_FLAG_PS_ON_BY_DEFAULT: if set to true, powersave will be enabled 5be83de54 Johannes Berg 2009-11-19 3217 *by default -- this flag will be set depending on the kernel's default 5be83de54 Johannes Berg 2009-11-19 3218 *on wiphy_new(), but can be changed by the driver if it has a good 5be83de54 Johannes Berg 2009-11-19 3219 *reason to override the default 9bc383de3 Johannes Berg 2009-11-19 3220 * @WIPHY_FLAG_4ADDR_AP: supports 4addr mode even on AP (with a single station 9bc383de3 Johannes Berg 2009-11-19 3221 *on a VLAN interface) 9bc383de3 Johannes Berg 2009-11-19 3222 * @WIPHY_FLAG_4ADDR_STATION: supports 4addr mode even as a station c0692b8fe Johannes Berg 2010-08-27 3223 * @WIPHY_FLAG_CONTROL_PORT_PROTOCOL: This device supports setting the c0692b8fe Johannes Berg 2010-08-27 3224 *control port protocol ethertype. The device also honours the c0692b8fe Johannes Berg 2010-08-27 3225 *control_port_no_encrypt flag. e31b82136 Johannes Berg 2010-10-05 3226 * @WIPHY_FLAG_IBSS_RSN: The device supports IBSS RSN. 15d5dda62 Javier Cardon
Re: [v8 0/4] cgroup-aware OOM killer
On Tue, 26 Sep 2017, Michal Hocko wrote: > > No, I agree that we shouldn't compare sibling memory cgroups based on > > different criteria depending on whether group_oom is set or not. > > > > I think it would be better to compare siblings based on the same criteria > > independent of group_oom if the user has mounted the hierarchy with the > > new mode (I think we all agree that the mount option is needed). It's > > very easy to describe to the user and the selection is simple to > > understand. > > I disagree. Just take the most simplistic example when cgroups reflect > some other higher level organization - e.g. school with teachers, > students and admins as the top level cgroups to control the proper cpu > share load. Now you want to have a fair OOM selection between different > entities. Do you consider selecting students all the time as an expected > behavior just because their are the largest group? This just doesn't > make any sense to me. > Are you referring to this? root /\ studentsadmins / \/\ A BCD If the cumulative usage of all students exceeds the cumulative usage of all admins, yes, the choice is to kill from the /students tree. This has been Roman's design from the very beginning. If the preference is to kill the single largest process, which may be attached to either subtree, you would not have opted-in to the new heuristic. > > Then, once a cgroup has been chosen as the victim cgroup, > > kill the process with the highest badness, allowing the user to influence > > that with /proc/pid/oom_score_adj just as today, if group_oom is disabled; > > otherwise, kill all eligible processes if enabled. > > And now, what should be the semantic of group_oom on an intermediate > (non-leaf) memcg? Why should we compare it to other killable entities? > Roman was mentioning a setup where a _single_ workload consists of a > deeper hierarchy which has to be shut down at once. It absolutely makes > sense to consider the cumulative memory of that hierarchy when we are > going to kill it all. > If group_oom is enabled on an intermediate memcg, I think the intuitive way to handle it would be that all descendants are also implicitly or explicitly group_oom. It is compared to sibling cgroups based on cumulative usage at the time of oom and the largest is chosen and iterated. The point is to separate out the selection heuristic (policy) from group_oom (mechanism) so that we don't bias or prefer subtrees based on group_oom, which makes this much more complex. > But what you are proposing is something different from oom_score_adj. > That only sets bias to the killable entities while priorities on > intermediate non-killable memcgs controls how the whole oom hierarchy > is traversed. So a non-killable intermediate memcg can hugely influence > what gets killed in the end. Why is there an intermediate non-killable memcg allowed? Cgroup oom priorities should not be allowed to disable oom killing, it should only set a priority. The only reason an intermediate cgroup should be non-killable is if there are no processes attached, but I don't think anyone is arguing we should just do nothing in that scenario. The point is that the user has infleunce over the decisionmaking with a per-process heuristic with oom_score_adj and should also have influence over the decisionmaking with a per-cgroup heuristic. > This is IMHO a tricky and I would even dare > to claim a wrong semantic. I can see priorities being very useful on > killable entities for sure. I am not entirely sure what would be the > best approach yet and that is why I've suggested that to postpone to > after we settle with a simple approach first. Bringing priorities back > to the discussion again will not help to move that forward I am afraid. > I agree to keep it as simple as possible, especially since some users want specific victim selection, it should be clear to document, and it shouldn't be influenced by some excessive amount of usage in another subtree the user has no control over (/admins over /students) to prevent the user from defining that it really wants to be the first oom victim or the admin from defining it really prefers something else killed first. My suggestion is that Roman's implementation is clear, well defined, and has real-world usecases and it should be the direction that this moves in. I think victim selection and group_oom are distinct and should not influence the decisionmaking. I think that oom_priority should influence the decisionmaking. When mounted with the new option, as the oom hierarchy is iterated, compare all sibling cgroups regarding cumulative size unless an oom priority overrides that (either user specifying it wants to be oom killed or admin specifying it prefers something else). When a victim memcg is chosen, use group_oom to determine what should be killed, otherwise choose by oom_score_adj. I can't imagine h
Re: [PATCH 09/10] scripts: kernel-doc: parse next structs/unions
Em Tue, 26 Sep 2017 14:59:19 -0300
Mauro Carvalho Chehab escreveu:
> There are several places within the Kernel tree with nested
> structs/unions, like this one:
>
> struct ingenic_cgu_clk_info {
> const char *name;
> enum {
> CGU_CLK_NONE = 0,
> CGU_CLK_EXT = BIT(0),
> CGU_CLK_PLL = BIT(1),
> CGU_CLK_GATE = BIT(2),
> CGU_CLK_MUX = BIT(3),
> CGU_CLK_MUX_GLITCHFREE = BIT(4),
> CGU_CLK_DIV = BIT(5),
> CGU_CLK_FIXDIV = BIT(6),
> CGU_CLK_CUSTOM = BIT(7),
> } type;
> int parents[4];
> union {
> struct ingenic_cgu_pll_info pll;
> struct {
> struct ingenic_cgu_gate_info gate;
> struct ingenic_cgu_mux_info mux;
> struct ingenic_cgu_div_info div;
> struct ingenic_cgu_fixdiv_info fixdiv;
> };
> struct ingenic_cgu_custom_info custom;
> };
> };
>
> Currently, such struct is documented as:
>
> **Definition**
>
> ::
> struct ingenic_cgu_clk_info {
> const char * name;
> };
>
> **Members**
>
> ``name``
> name of the clock
>
> With is obvioulsy wrong. It also generates an error:
> drivers/clk/ingenic/cgu.h:169: warning: No description found for
> parameter 'enum'
>
> However, there's nothing wrong with this kernel-doc markup: everything
> is documented there.
>
> It makes sense to document all fields there. So, add a
> way for the core to parse those structs.
>
> With this patch, all documented fields will properly generate
> documentation.
>
> Signed-off-by: Mauro Carvalho Chehab
> ---
> Documentation/doc-guide/kernel-doc.rst | 46 +
> scripts/kernel-doc | 120
> ++---
> 2 files changed, 112 insertions(+), 54 deletions(-)
>
> diff --git a/Documentation/doc-guide/kernel-doc.rst
> b/Documentation/doc-guide/kernel-doc.rst
> index 50473f0db345..3916a28b82b7 100644
> --- a/Documentation/doc-guide/kernel-doc.rst
> +++ b/Documentation/doc-guide/kernel-doc.rst
> @@ -281,6 +281,52 @@ comment block.
> The kernel-doc data structure comments describe each member of the structure,
> in order, with the member descriptions.
>
> +Nested structs/unions
> +~
> +
> +It is possible to document nested structs unions, like::
> +
> + /**
> + * struct nested_foobar - a struct with nested unions and structs
> + * @arg1: - first argument of anonymous union/anonymous struct
> + * @arg2: - second argument of anonymous union/anonymous struct
> + * @arg3: - third argument of anonymous union/anonymous struct
> + * @arg4: - fourth argument of anonymous union/anonymous struct
> + * @bar.st1.arg1 - first argument of struct st1 on union bar
> + * @bar.st1.arg2 - second argument of struct st1 on union bar
> + * @bar.st2.arg1 - first argument of struct st2 on union bar
> + * @bar.st2.arg2 - second argument of struct st2 on union bar
> + struct nested_foobar {
> +/* Anonymous union/struct*/
> +union {
> + struct {
> +int arg1;
> +int arg2;
> + }
> + struct {
> +void *arg3;
> +int arg4;
> + }
> + }
> + union {
> + struct {
> +int arg1;
> +int arg2;
> + } st1;
> + struct {
> +void *arg1;
> +int arg2;
> + } st2;
> + } bar;
> + };
> +
> +.. note::
> +
> + #) When documenting nested structs or unions, if the struct/union ``foo``
> + is named, the argument ``bar`` inside it should be documented as
> + ``@foo.bar:``
> + #) When the nested struct/union is anonymous, the argument ``bar`` on it
> + should be documented as ``@bar:``
>
> Typedef documentation
> -
> diff --git a/scripts/kernel-doc b/scripts/kernel-doc
> index b6f3f6962897..880a196c7dc7 100755
> --- a/scripts/kernel-doc
> +++ b/scripts/kernel-doc
> @@ -210,7 +210,7 @@ my $anon_struct_union = 0;
> my $type_constant = '\b``([^\`]+)``\b';
> my $type_constant2 = '\%([-_\w]+)';
> my $type_func = '(\w+)\(\)';
> -my $type_param = '\@(\w+(\.\.\.)?)';
> +my $type_param = '\@(\w[\.\w]*(\.\.\.)?)';
> my $type_fp_param = '\@(\w+)\(\)'; # Special RST handling for func ptr
> params
> my $type_env = '(\$\w+)';
> my $type_enum = '\&(enum\s*([_\w]+))';
> @@ -663,32 +663,12 @@ sub output_struct_man(%) {
> print ".SH NAME\n";
> print $args{'type'} . " " . $args{'struct'} . " \\- " . $args{'purpose'}
> . "\n";
>
> +my $declaration = $args{'definition'};
> +$declaration =~ s/\t/ /g;
> +$declaration =~ s/\n/"\n.br\n.BI \"/g;
> print ".SH SYNOPSIS\n";
> print $args{'type'} . " " . $args{'struct'} . " {\n.br\n";
> -
> -foreach my $parameter (@{$args{'parameterlist'}}) {
> - if ($parameter =~ /^#/) {
> - print ".BI \"$parameter\"\n.br\n";
> - next;
> - }
> - my $parame
Re: [PATCH] scripts: kernel-doc: parse next structs/unions
Em Tue, 26 Sep 2017 18:56:10 +0200
Markus Heiser escreveu:
> > Am 25.09.2017 um 20:22 schrieb Mauro Carvalho Chehab
> > :
> >
> > Jon,
> >
> > Please let me know what you think about this approach. IMHO, it is a way
> > better than what we do right now. This is more a proof of concept, as the
> > patch is not complete.
> >
> > With it, we can now document both anonymous and named unions/structs.
> >
> > For named structs, the name of the fields should be in the format:
> > foo.bar
> >
> > What's missing on this PoC:
> >
> > 1) I didn't check if @foo.bar: would work, though. Probably the parser
> > for it should be added to allow this new syntax.
>
> Below you will find my with a "@sys_clk.rate: lorem ipsum".
> With I expected a output in the **Members** section for sys_clk.rate but
> I got none :o
The parser line for @foo.bar: was not working, as I was expecting. The
version I posted today handles it well:
``sys_clk.rate``
lorem ipsum
It didn't handled - nor warned - about the lack of documentation for
this one:
const struct omap_vfsm_instance *vfsm;
but this is unrelated.
> Anyway your concept is good and so I applied it to my py-version. If you
> are interested in follow the patches titled ...
>
> kernel-doc: fix nested & sub-nested handling (continued
>
> at https://github.com/return42/linuxdoc/commits/master
>
> I also added some comments from my (py-) experience to your perl
> implementation below.
>
> Here are some links which illustrate how your concept could work:
>
> documentation with example:
>
> https://return42.github.io/linuxdoc/linuxdoc-howto/kernel-doc-syntax.html#structs-unions
>
> how the example is rendered:
>
> https://return42.github.io/linuxdoc/linuxdoc-howto/all-in-a-tumble.html#example-my-struct
> >
> > + my $declaration = $members;
> > +
> > + # Split nested struct/union elements as newer ones
> > + my $cont = 1;
> > + while ($cont) {
> > + $cont = 0;
>
> Is the outer loop needed ..
>
> > + while ($members =~
> > m/(struct|union)([^{};]+){([^{}]*)}([^{}\;]*)\;/) {
>
> this (inner) loop seems enough to me.
>
> > + my $newmember = "$1 $4;";
> > + my $id = $4;
>
> This won't work if you have e.g.
>
>union car {int foo;} bar1, bar2, *bbar3;
>
> where $id will be "bar1, bar2, *bbar3", you need to split
> this string and iterate over the id's.
That's true, if we have cases like that. I hope not!
Seriously, in the above case, the developer should very likely declare
the union outside the main struct and have its own kernel-doc markup.
My personal taste would be to not explicitly support the above. If it
hits some code, try to argue with the developer first, before patching
kernel-doc to such weird stuff.
> > + my $content = $3;
> > + $id =~ s/[:\[].*//;
> > + foreach my $arg (split /;/, $content) {
> > + next if ($arg =~ m/^\s*$/);
> > + my $type = $arg;
> > + my $name = $arg;
> > + $type =~ s/\s\S+$//;
> > + $name =~ s/.*\s//;
> > + next if (($name =~ m/^\s*$/));
> > + if ($id =~ m/^\s*$/) {
> > + # anonymous struct/union
> > + $newmember .= "$type $name;";
> > + } else {
> > + $newmember .= "$type $id.$name;";
> > + }
> > + }
> > + $members =~
> > s/(struct|union)([^{};]+){([^{}]*)}([^{}\;]*)\;/$newmember/;
>
> I'am not so familiar with Perl regexpr, does this replace only the
> first matching occurrence with $newmember or all?
Just the first match. if we wanted to apply it multiple times, we need
to add a "g" at the end, e. g.:
$members =~
s/(struct|union)([^{};]+){([^{}]*)}([^{}\;]*)\;/$newmember/g;
>
> > + $cont = 1;
> > + };
> > + };
> > +
> > + # Ignore other nested elements, like enums
> > + $members =~ s/({[^\{\}]*})//g;
> > + $nested = $decl_type;
> > + $nested =~ s/\/\*.*?\*\///gos;
> > +
> > create_parameterlist($members, ';', $file);
> > check_sections($file, $declaration_name, "struct", $sectcheck,
> > $struct_actual, $nested);
> >
> > + # Adjust declaration for better display
> > + $declaration =~ s/([{,;])/$1\n/g;
> > + $declaration =~ s/}\s+;/};/g;
> > + my @def_args = split /\n/, $declaration;
> > + my $level = 2;
> > + $declaration = "";
> > + foreach my $clause (@def_args) {
> > + $clause =~ s/^\s+//;
> > + $clause =~ s/\s+$//;
> > + $clause =~ s/\s+/ /;
> > + $level-- if ($clause =~ m/}/ && $level > 2);
> > + $declaration .= "\t" x $level . $clause . "\n" if ($clause);
>
> why in
Re: [PATCH v2] scripts: kernel-doc: fix nexted handling
Em Tue, 26 Sep 2017 14:45:08 +0200
Markus Heiser escreveu:
> > Am 25.09.2017 um 20:41 schrieb Mauro Carvalho Chehab
> > :
>
> >>> + $cont = 1;
> >>> + };
> >>> + };
> >>> + # Ignore other nested elements, like enums
> >>> + $members =~ s/({[^\{\}]*})//g;
> >>> + $nested = $decl_type;
> >>
> >> What is the latter good for? I guess the 'nested' trick to suppress
> >> such 'excess' warnings from nested types is no longer needed .. right?
> >
> > For things like:
> >
> > enum { foo, bar } type;
> >
> > Granted, a good documentation should also describe "foo" and "bar",
> > but that could be easily done by moving enums out of the struct, or
> > by add descriptions for "foo" and "bar" at @type: markup.
>
>
> Hm .. I suppose you are misunderstanding me. I didn't asked about
> $members, I asked about $nested. There is only one place where
> $nested is used, and this is in the check_sections function ...
>
> @@ -2531,9 +2527,7 @@ sub check_sections($$) {
> } else {
> - if ($nested !~ m/\Q$sects[$sx]\E/) {
> - print STDERR "${file}:$.: warning: " .
> - "Excess struct/union/enum/typedef
> member " .
> - "'$sects[$sx]' " .
> - "description in '$decl_name'\n";
> - ++$warnings;
> - }
> +print STDERR "${file}:$.: warning: " .
> +"Excess struct/union/enum/typedef member " .
> +"'$sects[$sx]' " .
> +"description in '$decl_name'\n";
> +++$warnings;
> }
>
> Since this is the only place where $nested is use, we can drop all
> the occurrence of $nested in the kernel-doc script .. or I'am
> totally wrong?
Ah, now I understood you! Yeah, this can be removed. I'll put it into
a separate cleanup patch.
See below.
Regards,
Mauro
[PATCH] scripts: kernel-doc: get rid of $nested parameter
The check_sections() function has a $nested parameter, meant
to identify when a nested struct is present. As we now have
a logic that handles it, get rid of such parameter.
Signed-off-by: Mauro Carvalho Chehab
diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index 880a196c7dc7..cff66ee91f2c 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -979,7 +979,6 @@ sub dump_union($$) {
sub dump_struct($$) {
my $x = shift;
my $file = shift;
-my $nested;
if ($x =~ /(struct|union)\s+(\w+)\s*{(.*)}/) {
my $decl_type = $1;
@@ -1034,11 +1033,9 @@ sub dump_struct($$) {
# Ignore other nested elements, like enums
$members =~ s/({[^\{\}]*})//g;
- $nested = $decl_type;
- $nested =~ s/\/\*.*?\*\///gos;
create_parameterlist($members, ';', $file);
- check_sections($file, $declaration_name, "struct", $sectcheck,
$struct_actual, $nested);
+ check_sections($file, $declaration_name, "struct", $sectcheck,
$struct_actual);
# Adjust declaration for better display
$declaration =~ s/([{;])/$1\n/g;
@@ -1334,8 +1331,8 @@ sub push_parameter($$$) {
$parametertypes{$param} = $type;
}
-sub check_sections($$) {
- my ($file, $decl_name, $decl_type, $sectcheck, $prmscheck, $nested) =
@_;
+sub check_sections($) {
+ my ($file, $decl_name, $decl_type, $sectcheck, $prmscheck) = @_;
my @sects = split ' ', $sectcheck;
my @prms = split ' ', $prmscheck;
my $err;
@@ -1369,14 +1366,6 @@ sub check_sections($$) {
"'$sects[$sx]' " .
"description in '$decl_name'\n";
++$warnings;
- } else {
- if ($nested !~ m/\Q$sects[$sx]\E/) {
- print STDERR "${file}:$.: warning: " .
- "Excess struct/union/enum/typedef
member " .
- "'$sects[$sx]' " .
- "description in '$decl_name'\n";
- ++$warnings;
- }
}
}
}
@@ -1487,7 +1476,7 @@ sub dump_function($$) {
}
my $prms = join " ", @parameterlist;
- check_sections($file, $declaration_name, "function", $sectcheck, $prms,
"");
+ check_sections($file, $declaration_name, "function", $sectcheck, $prms);
# This check emits a lot of warnings at the moment, because many
# functions don't have a 'Return' doc section. So until the number
Thanks,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.ke
Re: [PATCH] arm64: fix documentation on kernel pages mappings to HYP VA
Ping? On Wed, Sep 13, 2017 at 09:08:30PM +0300, Yury Norov wrote: > The Documentation/arm64/memory.txt says: > When using KVM, the hypervisor maps kernel pages in EL2, at a fixed > offset from the kernel VA (top 24bits of the kernel VA set to zero): > > In fact, kernel addresses are transleted to HYP with kern_hyp_va macro, > which has more options, and none of them assumes clearing of top 24bits > of the kernel VA. > > Signed-off-by: Yury Norov > --- > Documentation/arm64/memory.txt | 15 +-- > 1 file changed, 9 insertions(+), 6 deletions(-) > > diff --git a/Documentation/arm64/memory.txt b/Documentation/arm64/memory.txt > index d7273a5f6456..c39895d7e3a2 100644 > --- a/Documentation/arm64/memory.txt > +++ b/Documentation/arm64/memory.txt > @@ -86,9 +86,12 @@ Translation table lookup with 64KB pages: > +-> [63] TTBR0/1 > > > -When using KVM, the hypervisor maps kernel pages in EL2, at a fixed > -offset from the kernel VA (top 24bits of the kernel VA set to zero): > - > -StartEnd SizeUse > > -0040 007f 256GB kernel objects > mapped in HYP > +When using KVM without Virtualization Host Extensions, the hypervisor maps > +kernel pages in EL2, at a fixed offset from the kernel VA. Namely, top 16 > +or 25 bits of the kernel VA set to zero depending on ARM64_VA_BITS_48 or > +ARM64_VA_BITS_39 config option selected; or top 17 or 26 bits of the kernel > +VA set to zero if CPU has Reduced HYP mapping offset capability. See > +kern_hyp_va macro. > + > +When using KVM with Virtualization Host Extensions, no additional mappings > +created as host kernel already operates in EL2. > -- > 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 10/10] [RFC] w1_netlink.h: add support for nested structs
Describe nested struct/union fields
NOTE: This is a pure test patch, meant to validate if the
parsing logic for nested structs is working properly.
I've no idea if the random text I added there is correct!
Signed-off-by: Mauro Carvalho Chehab
---
drivers/w1/w1_netlink.h | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/w1/w1_netlink.h b/drivers/w1/w1_netlink.h
index a36661cd1f05..e781d1109cd7 100644
--- a/drivers/w1/w1_netlink.h
+++ b/drivers/w1/w1_netlink.h
@@ -60,6 +60,10 @@ enum w1_netlink_message_types {
* @status: kernel feedback for success 0 or errno failure value
* @len: length of data following w1_netlink_msg
* @id: union holding master bus id (msg.id) and slave device id (id[8]).
+ * @id.id: Slave ID (8 bytes)
+ * @id.mst: master bus identification
+ * @id.mst.id: master bus ID
+ * @id.mst.res: master bus reserved
* @data: start address of any following data
*
* The base message structure for w1 messages over netlink.
--
2.13.5
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 01/10] scripts: kernel-doc: get rid of unused output formats
Since there isn't any docbook code anymore upstream, we can get rid of several output formats: - docbook/xml, html, html5 and list formats were used by the old build system; - As ReST is text, there's not much sense on outputting on a different text format. After this patch, only man and rst output formats are supported. Signed-off-by: Mauro Carvalho Chehab --- scripts/kernel-doc | 1182 +--- 1 file changed, 4 insertions(+), 1178 deletions(-) diff --git a/scripts/kernel-doc b/scripts/kernel-doc index 9d3eafea58f0..69757ee9db4c 100755 --- a/scripts/kernel-doc +++ b/scripts/kernel-doc @@ -51,13 +51,8 @@ The documentation comments are identified by "/**" opening comment mark. See Documentation/kernel-doc-nano-HOWTO.txt for the documentation comment syntax. Output format selection (mutually exclusive): - -docbook Output DocBook format. - -htmlOutput HTML format. - -html5 Output HTML5 format. - -listOutput symbol list format. This is for use by docproc. -man Output troff manual page format. This is the default. -rst Output reStructuredText format. - -textOutput plain text format. Output selection (mutually exclusive): -export Only output documentation for symbols that have been @@ -224,84 +219,11 @@ my $type_typedef = '\&(typedef\s*([_\w]+))'; my $type_union = '\&(union\s*([_\w]+))'; my $type_member = '\&([_\w]+)(\.|->)([_\w]+)'; my $type_fallback = '\&([_\w]+)'; -my $type_enum_xml = '\&(enum\s*([_\w]+))'; -my $type_struct_xml = '\&(struct\s*([_\w]+))'; -my $type_typedef_xml = '\&(typedef\s*([_\w]+))'; -my $type_union_xml = '\&(union\s*([_\w]+))'; -my $type_member_xml = '\&([_\w]+)(\.|-\>)([_\w]+)'; -my $type_fallback_xml = '\&([_\w]+)'; my $type_member_func = $type_member . '\(\)'; # Output conversion substitutions. # One for each output format -# these work fairly well -my @highlights_html = ( - [$type_constant, "\$1"], - [$type_constant2, "\$1"], - [$type_func, "\$1"], - [$type_enum_xml, "\$1"], - [$type_struct_xml, "\$1"], - [$type_typedef_xml, "\$1"], - [$type_union_xml, "\$1"], - [$type_env, "\$1"], - [$type_param, "\$1"], - [$type_member_xml, "\$1\$2\$3"], - [$type_fallback_xml, "\$1"] - ); -my $local_lt = "lt:"; -my $local_gt = "gt:"; -my $blankline_html = $local_lt . "p" . $local_gt; # was "" - -# html version 5 -my @highlights_html5 = ( -[$type_constant, "\$1"], -[$type_constant2, "\$1"], -[$type_func, "\$1"], -[$type_enum_xml, "\$1"], -[$type_struct_xml, "\$1"], -[$type_typedef_xml, "\$1"], -[$type_union_xml, "\$1"], -[$type_env, "\$1"], -[$type_param, "\$1]"], -[$type_member_xml, "\$1\$2\$3"], -[$type_fallback_xml, "\$1"] - ); -my $blankline_html5 = $local_lt . "br /" . $local_gt; - -# XML, docbook format -my @highlights_xml = ( - ["([^=])\\\"([^\\\"<]+)\\\"", "\$1\$2"], - [$type_constant, "\$1"], - [$type_constant2, "\$1"], - [$type_enum_xml, "\$1"], - [$type_struct_xml, "\$1"], - [$type_typedef_xml, "\$1"], - [$type_union_xml, "\$1"], - [$type_param, "\$1"], - [$type_func, "\$1"], - [$type_env, "\$1"], - [$type_member_xml, "\$1\$2\$3"], - [$type_fallback_xml, "\$1"] -); -my $blankline_xml = $local_lt . "/para" . $local_gt . $local_lt . "para" . $local_gt . "\n"; - -# gnome, docbook format -my @highlights_gnome = ( -[$type_constant, "\$1"], -[$type_constant2, "\$1"], -[$type_func, "\$1"], -[$type_enum, "\$1"], -[$type_struct, "\$1"], -[$type_typedef, "\$1"], -[$type_union, "\$1"], -[$type_env, "\$1"], -[$type_param, "\$1" ], -[$type_member, "\$1\$2\$3"], -[$type_fallback, "\$1"] - ); -my $blankline_gnome = "\n"; - # these are pretty rough my @highlights_man = ( [$type_constant, "\$1"], @@ -317,21 +239,6 @@ my @highlights_man = (
[PATCH 05/10] docs: kernel-doc.rst: improve structs chapter
There is a mess on this chapter: it suggests that even enums and unions should be documented with "struct". That's not the way it should be ;-) Fix it and move it to happen earlier. Signed-off-by: Mauro Carvalho Chehab Signed-off-by: Mauro Carvalho Chehab --- Documentation/doc-guide/kernel-doc.rst | 48 +- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/Documentation/doc-guide/kernel-doc.rst b/Documentation/doc-guide/kernel-doc.rst index 9b69dfe928d8..68cb1b496c73 100644 --- a/Documentation/doc-guide/kernel-doc.rst +++ b/Documentation/doc-guide/kernel-doc.rst @@ -258,6 +258,30 @@ named ``Return``. as a new section heading, with probably won't produce the desired effect. +Structure, union, and enumeration documentation +--- + +The general format of a struct, union, and enum kernel-doc comment is:: + + /** + * struct struct_name - Brief description. + * @argument: Description of member member_name. + * + * Description of the structure. + */ + +On the above, ``struct`` is used to mean structs. You can also use ``union`` +and ``enum`` to describe unions and enums. ``argument`` is used +to mean struct and union member names as well as enumerations in an enum. + +The brief description following the structure name may span multiple lines, and +ends with a member description, a blank comment line, or the end of the +comment block. + +The kernel-doc data structure comments describe each member of the structure, +in order, with the member descriptions. + + Highlights and cross-references --- @@ -331,30 +355,6 @@ cross-references. For further details, please refer to the `Sphinx C Domain`_ documentation. -Structure, union, and enumeration documentation - -The general format of a struct, union, and enum kernel-doc comment is:: - - /** - * struct struct_name - Brief description. - * @member_name: Description of member member_name. - * - * Description of the structure. - */ - -Below, "struct" is used to mean structs, unions and enums, and "member" is used -to mean struct and union members as well as enumerations in an enum. - -The brief description following the structure name may span multiple lines, and -ends with a ``@member:`` description, a blank comment line, or the end of the -comment block. - -The kernel-doc data structure comments describe each member of the structure, in -order, with the ``@member:`` descriptions. The ``@member:`` descriptions must -begin on the very next line following the opening brief function description -line, with no intervening blank comment lines. The ``@member:`` descriptions may -span multiple lines. The continuation lines may contain indentation. In-line member documentation comments ~ -- 2.13.5 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 09/10] scripts: kernel-doc: parse next structs/unions
There are several places within the Kernel tree with nested
structs/unions, like this one:
struct ingenic_cgu_clk_info {
const char *name;
enum {
CGU_CLK_NONE = 0,
CGU_CLK_EXT = BIT(0),
CGU_CLK_PLL = BIT(1),
CGU_CLK_GATE = BIT(2),
CGU_CLK_MUX = BIT(3),
CGU_CLK_MUX_GLITCHFREE = BIT(4),
CGU_CLK_DIV = BIT(5),
CGU_CLK_FIXDIV = BIT(6),
CGU_CLK_CUSTOM = BIT(7),
} type;
int parents[4];
union {
struct ingenic_cgu_pll_info pll;
struct {
struct ingenic_cgu_gate_info gate;
struct ingenic_cgu_mux_info mux;
struct ingenic_cgu_div_info div;
struct ingenic_cgu_fixdiv_info fixdiv;
};
struct ingenic_cgu_custom_info custom;
};
};
Currently, such struct is documented as:
**Definition**
::
struct ingenic_cgu_clk_info {
const char * name;
};
**Members**
``name``
name of the clock
With is obvioulsy wrong. It also generates an error:
drivers/clk/ingenic/cgu.h:169: warning: No description found for
parameter 'enum'
However, there's nothing wrong with this kernel-doc markup: everything
is documented there.
It makes sense to document all fields there. So, add a
way for the core to parse those structs.
With this patch, all documented fields will properly generate
documentation.
Signed-off-by: Mauro Carvalho Chehab
---
Documentation/doc-guide/kernel-doc.rst | 46 +
scripts/kernel-doc | 120 ++---
2 files changed, 112 insertions(+), 54 deletions(-)
diff --git a/Documentation/doc-guide/kernel-doc.rst
b/Documentation/doc-guide/kernel-doc.rst
index 50473f0db345..3916a28b82b7 100644
--- a/Documentation/doc-guide/kernel-doc.rst
+++ b/Documentation/doc-guide/kernel-doc.rst
@@ -281,6 +281,52 @@ comment block.
The kernel-doc data structure comments describe each member of the structure,
in order, with the member descriptions.
+Nested structs/unions
+~
+
+It is possible to document nested structs unions, like::
+
+ /**
+ * struct nested_foobar - a struct with nested unions and structs
+ * @arg1: - first argument of anonymous union/anonymous struct
+ * @arg2: - second argument of anonymous union/anonymous struct
+ * @arg3: - third argument of anonymous union/anonymous struct
+ * @arg4: - fourth argument of anonymous union/anonymous struct
+ * @bar.st1.arg1 - first argument of struct st1 on union bar
+ * @bar.st1.arg2 - second argument of struct st1 on union bar
+ * @bar.st2.arg1 - first argument of struct st2 on union bar
+ * @bar.st2.arg2 - second argument of struct st2 on union bar
+ struct nested_foobar {
+/* Anonymous union/struct*/
+union {
+ struct {
+int arg1;
+int arg2;
+ }
+ struct {
+void *arg3;
+int arg4;
+ }
+ }
+ union {
+ struct {
+int arg1;
+int arg2;
+ } st1;
+ struct {
+void *arg1;
+int arg2;
+ } st2;
+ } bar;
+ };
+
+.. note::
+
+ #) When documenting nested structs or unions, if the struct/union ``foo``
+ is named, the argument ``bar`` inside it should be documented as
+ ``@foo.bar:``
+ #) When the nested struct/union is anonymous, the argument ``bar`` on it
+ should be documented as ``@bar:``
Typedef documentation
-
diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index b6f3f6962897..880a196c7dc7 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -210,7 +210,7 @@ my $anon_struct_union = 0;
my $type_constant = '\b``([^\`]+)``\b';
my $type_constant2 = '\%([-_\w]+)';
my $type_func = '(\w+)\(\)';
-my $type_param = '\@(\w+(\.\.\.)?)';
+my $type_param = '\@(\w[\.\w]*(\.\.\.)?)';
my $type_fp_param = '\@(\w+)\(\)'; # Special RST handling for func ptr params
my $type_env = '(\$\w+)';
my $type_enum = '\&(enum\s*([_\w]+))';
@@ -663,32 +663,12 @@ sub output_struct_man(%) {
print ".SH NAME\n";
print $args{'type'} . " " . $args{'struct'} . " \\- " . $args{'purpose'} .
"\n";
+my $declaration = $args{'definition'};
+$declaration =~ s/\t/ /g;
+$declaration =~ s/\n/"\n.br\n.BI \"/g;
print ".SH SYNOPSIS\n";
print $args{'type'} . " " . $args{'struct'} . " {\n.br\n";
-
-foreach my $parameter (@{$args{'parameterlist'}}) {
- if ($parameter =~ /^#/) {
- print ".BI \"$parameter\"\n.br\n";
- next;
- }
- my $parameter_name = $parameter;
- $parameter_name =~ s/\[.*//;
-
- ($args{'parameterdescs'}{$parameter_name} ne $undescribed) || next;
- $type = $args{'parametertypes'}{$parameter};
- if ($type =~ m/([^\(]*\(\*)\s*\)\s*\(([^\)]*)\)/) {
- # pointer-to-function
- print ".BI \"" . $1 . "\" " . $p
[PATCH 00/10] kernel-doc: add supported to document nested structs/unions
Right now, it is not possible to document nested struct and nested unions. kernel-doc simply ignore them. Add support to document them. This series starts with a patch getting rid of the now unused output formats for kernel-doc: since we got rid of all DocBook stuff, we should not need them anymore. The reason for dropping it (despite cleaning up), is that it doesn't make sense to invest time on adding new features for formats that aren't used anymore. The next 8 patches on this series improve kernel-doc documentation and finally get rid of its old documentation (kernel-doc-nano-HOWTO.txt). Patch 9/10 is the most interesting one in this series: it adds support for nested structures and unions. Patch 10/10 is just an example from a random header with kernel-doc markups. There's no special reason for selecting this file, and the comments there are likely wrong. So, please use it only as a way to test the new parser logic from patch 9/10. Mauro Carvalho Chehab (10): scripts: kernel-doc: get rid of unused output formats docs: kernel-doc.rst: better describe kernel-doc arguments docs: kernel-doc.rst: improve private members description docs: kernel-doc.rst: improve function documentation section docs: kernel-doc.rst: improve structs chapter docs: kernel-doc: improve typedef documentation docs: kernel-doc.rst: add documentation about man pages docs: get rid of kernel-doc-nano-HOWTO.txt scripts: kernel-doc: parse next structs/unions [RFC] w1_netlink.h: add support for nested structs --- Before this series, I send a few PoC patches. They were all replaced by patch 9/10. Documentation/00-INDEX |2 - Documentation/doc-guide/kernel-doc.rst | 387 ++--- Documentation/kernel-doc-nano-HOWTO.txt | 322 drivers/w1/w1_netlink.h |4 + scripts/kernel-doc | 1304 ++- 5 files changed, 346 insertions(+), 1673 deletions(-) delete mode 100644 Documentation/kernel-doc-nano-HOWTO.txt -- 2.13.5 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 06/10] docs: kernel-doc: improve typedef documentation
Add documentation about typedefs for function prototypes and move it to happen earlier. Signed-off-by: Mauro Carvalho Chehab --- Documentation/doc-guide/kernel-doc.rst | 32 ++-- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/Documentation/doc-guide/kernel-doc.rst b/Documentation/doc-guide/kernel-doc.rst index 68cb1b496c73..9777aa53e3dd 100644 --- a/Documentation/doc-guide/kernel-doc.rst +++ b/Documentation/doc-guide/kernel-doc.rst @@ -282,6 +282,28 @@ The kernel-doc data structure comments describe each member of the structure, in order, with the member descriptions. +Typedef documentation +- + +The general format of a typedef kernel-doc comment is:: + + /** + * typedef type_name - Brief description. + * + * Description of the type. + */ + +Typedefs with function prototypes can also be documented:: + + /** + * typedef type_name - Brief description. + * @arg1: description of arg1 + * @arg2: description of arg2 + * + * Description of the type. + */ + typedef void (*type_name)(struct v4l2_ctrl *arg1, void *arg2); + Highlights and cross-references --- @@ -384,16 +406,6 @@ on a line of their own, like all other kernel-doc comments:: int foobar; } -Typedef documentation -- - -The general format of a typedef kernel-doc comment is:: - - /** - * typedef type_name - Brief description. - * - * Description of the type. - */ Overview documentation comments --- -- 2.13.5 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 04/10] docs: kernel-doc.rst: improve function documentation section
Move its contents to happen earlier and improve the description of return values, adding a subsection to it. Most of the contents there came from kernel-doc-nano-HOWTO.txt. Signed-off-by: Mauro Carvalho Chehab --- Documentation/doc-guide/kernel-doc.rst | 100 - 1 file changed, 61 insertions(+), 39 deletions(-) diff --git a/Documentation/doc-guide/kernel-doc.rst b/Documentation/doc-guide/kernel-doc.rst index f1eb00899084..9b69dfe928d8 100644 --- a/Documentation/doc-guide/kernel-doc.rst +++ b/Documentation/doc-guide/kernel-doc.rst @@ -197,6 +197,67 @@ Example:: int d; }; +Function documentation +-- + +The general format of a function and function-like macro kernel-doc comment is:: + + /** + * function_name() - Brief description of function. + * @arg1: Describe the first argument. + * @arg2: Describe the second argument. + *One can provide multiple line descriptions + *for arguments. + * + * A longer description, with more discussion of the function function_name() + * that might be useful to those using or modifying it. Begins with an + * empty comment line, and may include additional embedded empty + * comment lines. + * + * The longer description may have multiple paragraphs. + * + * Return: Describe the return value of foobar. + * + * The return value description can also have multiple paragraphs, and should + * be placed at the end of the comment block. + */ + +The brief description following the function name may span multiple lines, and +ends with an argument description, a blank comment line, or the end of the +comment block. + +Return values +~ + +The return value, if any, should be described in a dedicated section +named ``Return``. + +.. note:: + + #) The multi-line descriptive text you provide does *not* recognize + line breaks, so if you try to format some text nicely, as in:: + + * Return: + * 0 - OK + * -EINVAL - invalid argument + * -ENOMEM - out of memory + + this will all run together and produce:: + + Return: 0 - OK -EINVAL - invalid argument -ENOMEM - out of memory + + So, in order to produce the desired line breaks, you need to use a + ReST list, e. g.:: + + * Return: + * * 0- OK to runtime suspend the device + * * -EBUSY - Device should not be runtime suspended + + #) If the descriptive text you provide has lines that begin with + some phrase followed by a colon, each of those phrases will be taken + as a new section heading, with probably won't produce the desired + effect. + Highlights and cross-references --- @@ -269,45 +330,6 @@ cross-references. For further details, please refer to the `Sphinx C Domain`_ documentation. -Function documentation --- - -The general format of a function and function-like macro kernel-doc comment is:: - - /** - * function_name() - Brief description of function. - * @arg1: Describe the first argument. - * @arg2: Describe the second argument. - *One can provide multiple line descriptions - *for arguments. - * - * A longer description, with more discussion of the function function_name() - * that might be useful to those using or modifying it. Begins with an - * empty comment line, and may include additional embedded empty - * comment lines. - * - * The longer description may have multiple paragraphs. - * - * Return: Describe the return value of foobar. - * - * The return value description can also have multiple paragraphs, and should - * be placed at the end of the comment block. - */ - -The brief description following the function name may span multiple lines, and -ends with an ``@argument:`` description, a blank comment line, or the end of the -comment block. - -The kernel-doc function comments describe each parameter to the function, in -order, with the ``@argument:`` descriptions. The ``@argument:`` descriptions -must begin on the very next line following the opening brief function -description line, with no intervening blank comment lines. The ``@argument:`` -descriptions may span multiple lines. The continuation lines may contain -indentation. If a function parameter is ``...`` (varargs), it should be listed -in kernel-doc notation as: ``@...:``. - -The return value, if any, should be described in a dedicated section at the end -of the comment starting with "Return:". Structure, union, and enumeration documentation --- -- 2.13.5 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 08/10] docs: get rid of kernel-doc-nano-HOWTO.txt
Everything there is already described at Documentation/doc-guide/kernel-doc.rst. So, there's no reason why to keep it anymore. Signed-off-by: Mauro Carvalho Chehab --- Documentation/00-INDEX | 2 - Documentation/kernel-doc-nano-HOWTO.txt | 322 scripts/kernel-doc | 2 +- 3 files changed, 1 insertion(+), 325 deletions(-) delete mode 100644 Documentation/kernel-doc-nano-HOWTO.txt diff --git a/Documentation/00-INDEX b/Documentation/00-INDEX index 3bec49c33bbb..aca4f00ec69b 100644 --- a/Documentation/00-INDEX +++ b/Documentation/00-INDEX @@ -228,8 +228,6 @@ isdn/ - directory with info on the Linux ISDN support, and supported cards. kbuild/ - directory with info about the kernel build process. -kernel-doc-nano-HOWTO.txt - - outdated info about kernel-doc documentation. kdump/ - directory with mini HowTo on getting the crash dump code to work. doc-guide/ diff --git a/Documentation/kernel-doc-nano-HOWTO.txt b/Documentation/kernel-doc-nano-HOWTO.txt deleted file mode 100644 index c23e2c5ab80d.. --- a/Documentation/kernel-doc-nano-HOWTO.txt +++ /dev/null @@ -1,322 +0,0 @@ -NOTE: this document is outdated and will eventually be removed. See -Documentation/doc-guide/ for current information. - -kernel-doc nano-HOWTO -= - -How to format kernel-doc comments -- - -In order to provide embedded, 'C' friendly, easy to maintain, -but consistent and extractable documentation of the functions and -data structures in the Linux kernel, the Linux kernel has adopted -a consistent style for documenting functions and their parameters, -and structures and their members. - -The format for this documentation is called the kernel-doc format. -It is documented in this Documentation/kernel-doc-nano-HOWTO.txt file. - -This style embeds the documentation within the source files, using -a few simple conventions. The scripts/kernel-doc perl script, the -Documentation/sphinx/kerneldoc.py Sphinx extension and other tools understand -these conventions, and are used to extract this embedded documentation -into various documents. - -In order to provide good documentation of kernel functions and data -structures, please use the following conventions to format your -kernel-doc comments in Linux kernel source. - -We definitely need kernel-doc formatted documentation for functions -that are exported to loadable modules using EXPORT_SYMBOL. - -We also look to provide kernel-doc formatted documentation for -functions externally visible to other kernel files (not marked -"static"). - -We also recommend providing kernel-doc formatted documentation -for private (file "static") routines, for consistency of kernel -source code layout. But this is lower priority and at the -discretion of the MAINTAINER of that kernel source file. - -Data structures visible in kernel include files should also be -documented using kernel-doc formatted comments. - -The opening comment mark "/**" is reserved for kernel-doc comments. -Only comments so marked will be considered by the kernel-doc scripts, -and any comment so marked must be in kernel-doc format. Do not use -"/**" to be begin a comment block unless the comment block contains -kernel-doc formatted comments. The closing comment marker for -kernel-doc comments can be either "*/" or "**/", but "*/" is -preferred in the Linux kernel tree. - -Kernel-doc comments should be placed just before the function -or data structure being described. - -Example kernel-doc function comment: - -/** - * foobar() - short function description of foobar - * @arg1: Describe the first argument to foobar. - * @arg2: Describe the second argument to foobar. - * One can provide multiple line descriptions - * for arguments. - * - * A longer description, with more discussion of the function foobar() - * that might be useful to those using or modifying it. Begins with - * empty comment line, and may include additional embedded empty - * comment lines. - * - * The longer description can have multiple paragraphs. - * - * Return: Describe the return value of foobar. - */ - -The short description following the subject can span multiple lines -and ends with an @argument description, an empty line or the end of -the comment block. - -The @argument descriptions must begin on the very next line following -this opening short function description line, with no intervening -empty comment lines. - -If a function parameter is "..." (varargs), it should be listed in -kernel-doc notation as: - * @...: description - -The return value, if any, should be described in a dedicated section -named "Return". - -Example kernel-doc data structure comment. - -/** - * struct blah - the basic blah structure - * @mem1: describe the first member of struct blah - * @mem2: describe the second member of struct blah, - * perhaps with more lines an
[PATCH 03/10] docs: kernel-doc.rst: improve private members description
The private members section can now be moved to be together
with the arguments section. Move it there and add an example
about the usage of public:
Signed-off-by: Mauro Carvalho Chehab
---
Documentation/doc-guide/kernel-doc.rst | 56 ++
1 file changed, 30 insertions(+), 26 deletions(-)
diff --git a/Documentation/doc-guide/kernel-doc.rst
b/Documentation/doc-guide/kernel-doc.rst
index 7a3f5c710c0b..f1eb00899084 100644
--- a/Documentation/doc-guide/kernel-doc.rst
+++ b/Documentation/doc-guide/kernel-doc.rst
@@ -167,6 +167,36 @@ notation as::
* @...: description
+Private members
+~~~
+
+Inside a struct or union description, you can use the ``private:`` and
+``public:`` comment tags. Structure fields that are inside a ``private:``
+area are not listed in the generated output documentation.
+
+The ``private:`` and ``public:`` tags must begin immediately following a
+``/*`` comment marker. They may optionally include comments between the
+``:`` and the ending ``*/`` marker.
+
+Example::
+
+ /**
+ * struct my_struct - short description
+ * @a: first member
+ * @b: second member
+ * @d: fourth member
+ *
+ * Longer description
+ */
+ struct my_struct {
+ int a;
+ int b;
+ /* private: internal use only */
+ int c;
+ /* public: the next one is public */
+ int d;
+ };
+
Highlights and cross-references
---
@@ -332,32 +362,6 @@ on a line of their own, like all other kernel-doc
comments::
int foobar;
}
-Private members
-~~~
-
-Inside a struct description, you can use the "private:" and "public:" comment
-tags. Structure fields that are inside a "private:" area are not listed in the
-generated output documentation. The "private:" and "public:" tags must begin
-immediately following a ``/*`` comment marker. They may optionally include
-comments between the ``:`` and the ending ``*/`` marker.
-
-Example::
-
- /**
- * struct my_struct - short description
- * @a: first member
- * @b: second member
- *
- * Longer description
- */
- struct my_struct {
- int a;
- int b;
- /* private: internal use only */
- int c;
- };
-
-
Typedef documentation
-
--
2.13.5
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 07/10] docs: kernel-doc.rst: add documentation about man pages
kernel-doc-nano-HOWTO.txt has a chapter about man pages
production. While we don't have a working "make manpages"
target, add it.
Signed-off-by: Mauro Carvalho Chehab
---
Documentation/doc-guide/kernel-doc.rst | 61 ++
1 file changed, 47 insertions(+), 14 deletions(-)
diff --git a/Documentation/doc-guide/kernel-doc.rst
b/Documentation/doc-guide/kernel-doc.rst
index 9777aa53e3dd..50473f0db345 100644
--- a/Documentation/doc-guide/kernel-doc.rst
+++ b/Documentation/doc-guide/kernel-doc.rst
@@ -377,7 +377,6 @@ cross-references.
For further details, please refer to the `Sphinx C Domain`_ documentation.
-
In-line member documentation comments
~
@@ -391,19 +390,19 @@ on a line of their own, like all other kernel-doc
comments::
* @foo: The Foo member.
*/
struct foo {
-int foo;
-/**
- * @bar: The Bar member.
- */
-int bar;
-/**
- * @baz: The Baz member.
- *
- * Here, the member description may contain several paragraphs.
- */
-int baz;
-/** @foobar: Single line description. */
-int foobar;
+ int foo;
+ /**
+* @bar: The Bar member.
+*/
+ int bar;
+ /**
+* @baz: The Baz member.
+*
+* Here, the member description may contain several paragraphs.
+*/
+ int baz;
+ /** @foobar: Single line description. */
+ int foobar;
}
@@ -452,3 +451,37 @@ file.
Data structures visible in kernel include files should also be documented using
kernel-doc formatted comments.
+
+How to use kernel-doc to generate man pages
+---
+
+If you just want to use kernel-doc to generate man pages you can do this
+from the Kernel git tree::
+
+ $ scripts/kernel-doc -man $(git grep -l '/\*\*' |grep -v Documentation/) |
./split-man.pl /tmp/man
+
+Using the small ``split-man.pl`` script below::
+
+
+ #!/usr/bin/perl
+
+ if ($#ARGV < 0) {
+ die "where do I put the results?\n";
+ }
+
+ mkdir $ARGV[0],0777;
+ $state = 0;
+ while () {
+ if (/^\.TH \"[^\"]*\" 9 \"([^\"]*)\"/) {
+ if ($state == 1) { close OUT }
+ $state = 1;
+ $fn = "$ARGV[0]/$1.9";
+ print STDERR "Creating $fn\n";
+ open OUT, ">$fn" or die "can't open $fn: $!\n";
+ print OUT $_;
+ } elsif ($state != 0) {
+ print OUT $_;
+ }
+ }
+
+ close OUT;
--
2.13.5
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 02/10] docs: kernel-doc.rst: better describe kernel-doc arguments
Add a new section to describe kernel-doc arguments, adding examples about how identation should happen, as failing to do that causes Sphinx to do the wrong thing. Signed-off-by: Mauro Carvalho Chehab --- Documentation/doc-guide/kernel-doc.rst | 44 +++--- 1 file changed, 41 insertions(+), 3 deletions(-) diff --git a/Documentation/doc-guide/kernel-doc.rst b/Documentation/doc-guide/kernel-doc.rst index b24854b5d6be..7a3f5c710c0b 100644 --- a/Documentation/doc-guide/kernel-doc.rst +++ b/Documentation/doc-guide/kernel-doc.rst @@ -112,16 +112,17 @@ Example kernel-doc function comment:: /** * foobar() - Brief description of foobar. - * @arg: Description of argument of foobar. + * @argument1: Description of parameter argument1 of foobar. + * @argument1: Description of parameter argument2 of foobar. * * Longer description of foobar. * * Return: Description of return value of foobar. */ - int foobar(int arg) + int foobar(int argument1, char *argument2) The format is similar for documentation for structures, enums, paragraphs, -etc. See the sections below for details. +etc. See the sections below for specific details of each type. The kernel-doc structure is extracted from the comments, and proper `Sphinx C Domain`_ function and type descriptions with anchors are generated for them. The @@ -130,6 +131,43 @@ cross-references. See below for details. .. _Sphinx C Domain: http://www.sphinx-doc.org/en/stable/domains.html + +Parameters and member arguments +--- + +The kernel-doc function comments describe each parameter to the function and +function typedefs or each member of struct/union, in order, with the +``@argument:`` descriptions. For each non-private member argument, one +``@argument`` definition is needed. + +The ``@argument:`` descriptions begin on the very next line following +the opening brief function description line, with no intervening blank +comment lines. + +The ``@argument:`` descriptions may span multiple lines. + +.. note:: + + If the ``@argument`` description has multiple lines, the continuation + of the description should be starting exactly at the same column as + the previous line, e. g.:: + + * @argument: some long description + * that continues on next lines + + or:: + + * @argument: + *some long description + *that continues on next lines + +If a function or typedef parameter argument is ``...`` (e. g. a variable +number of arguments), its description should be listed in kernel-doc +notation as:: + + * @...: description + + Highlights and cross-references --- -- 2.13.5 -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 0/2] kbuild: Cache exploratory calls to the compiler
This two-patch series attempts to speed incremental builds of the kernel up by a bit. How much of a speedup you get depends a lot on your environment, specifically the speed of your workstation and how fast it takes to invoke the compiler. In the Chrome OS build environment you get a really big win. For an incremental build (via emerge) I measured a speedup from ~1 minute to ~35 seconds. ...but Chrome OS calls the compiler through a number of wrapper scripts and also calls the kernel make at least twice for an emerge (during compile stage and install stage), so it's a bit of a worst case. Perhaps a more realistic measure of the speedup others might see is running "time make help > /dev/null" outside of the Chrome OS build environment on my system. When I do this I see that it took more than 1.0 seconds before and less than 0.2 seconds after. So presumably this has the ability to shave ~0.8 seconds off an incremental build for most folks out there. While 0.8 seconds savings isn't huge, it does make incremental builds feel a lot snappier. Please note that I make no illusions of being a Makefile expert nor do I have any belief that I fully understand the Linux kernel build system. Please take this patch series as the start of a discussion about whether others feel like this type of speedup is worthwhile and how to best accomplish it. Specific things to note: - I'm happy to paint the bikeshed any color that maintainers want. If you'd like the cache named differently, in a slightly different format, or you want me to adjust the spacing / names of Makefile stuff then please just let me know. - If this is totally the wrong approach and you have a better idea then let me know. If you want something that's super complicated to explain then feel free to post a replacement patch and I'm happy to test. - This patch definitely needs extra testing. I've tested it on a very limited build environment and it seems to be working fine, but I could believe that with some weird compiler options or on certain architectures you might need some extra escaping here and there. Douglas Anderson (2): kbuild: Add a cache for generated variables kbuild: Cache a few more calls to the compiler Documentation/kbuild/makefiles.txt | 21 ++ Makefile | 9 ++- scripts/Kbuild.include | 133 +++-- 3 files changed, 142 insertions(+), 21 deletions(-) -- 2.14.1.821.g8fa685d3b7-goog -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 1/2] kbuild: Add a cache for generated variables
While timing a "no-op" build of the kernel (incrementally building the kernel even though nothing changed) in the Chrome OS build system I found that it was much slower than I expected. Digging into things a bit, I found that quite a bit of the time was spent invoking the C compiler even though we weren't actually building anything. Currently in the Chrome OS build system the C compiler is called through a number of wrappers (one of which is written in python!) and can take upwards of 100 ms to invoke even if we're not doing anything difficult, so these invocations of the compiler were taking a lot of time. Worse the invocations couldn't seem to take advantage of the multiple cores on my system. Certainly it seems like we could make the compiler invocations in the Chrome OS build system faster, but only to a point. Inherently invoking a program as big as a C compiler is a fairly heavy operation. Thus even if we can speed the compiler calls it made sense to track down what was happening. It turned out that all the compiler invocations were coming from usages like this in the kernel's Makefile: KBUILD_CFLAGS += $(call cc-option,-fno-delete-null-pointer-checks,) Due to the way cc-option and similar statements work the above contains an implicit call to the C compiler. ...and due to the fact that we're storing the result in KBUILD_CFLAGS, a simply expanded variable, the call will happen every time the Makefile is parsed, even if there are no users of KBUILD_CFLAGS. Rather than redoing this computation every time, it makes a lot of sense to cache the result of all of the Makefile's compiler calls just like we do when we compile a ".c" file to a ".o" file. Conceptually this is quite a simple idea. ...and since the calls to invoke the compiler and similar tools are centrally located in the Kbuild.include file this doesn't even need to be super invasive. Implementing the cache in a simple-to-use and efficient way is not quite as simple as it first sounds, though. To get maximum speed we really want the cache in a format that make can natively understand and make doesn't really have an ability to load/parse files. ...but make _can_ import other Makefiles, so the solution is to store the cache in Makefile format. This requires coming up with a valid/unique Makefile variable name for each value to be cached, but that's solvable with some cleverness. After this change, we'll automatically create a "Makefile-cache.o" file that will contain our cached variables. We'll load this on each invocation of make and will avoid recomputing anything that's already in our cache. The cache is stored in a format that it shouldn't need any invalidation since anything that might change should affect the "key" and any old cached value won't be used. The cache will be cleaned by virtue of the ".o" suffix by a "make clean". Signed-off-by: Douglas Anderson --- Documentation/kbuild/makefiles.txt | 21 ++ scripts/Kbuild.include | 133 +++-- 2 files changed, 135 insertions(+), 19 deletions(-) diff --git a/Documentation/kbuild/makefiles.txt b/Documentation/kbuild/makefiles.txt index 329e740adea7..679e8483cb4f 100644 --- a/Documentation/kbuild/makefiles.txt +++ b/Documentation/kbuild/makefiles.txt @@ -581,6 +581,27 @@ more details, with real examples. LDFLAGS_vmlinux += $(call ld-option, -X) +--- 3.14 $(LD) support function cache + +One thing to realize about all the calls to the above support functions +is that each use of them requires a full invocation of an external tool, like +the C compiler, assembler, or linker. If nothing else that invocation will +cause a fork/exec/shared library link. In some build environments, however, it +could also involve traversing thorough one or more wrappers. To put some +numbers on it, I've measured compiler invocations of as fast as 4ms or +as slow as 150ms. + +Many of the above support functions are used in places where they are +evaluated on each invocation of Make before anything else can run. Even on +a simple command like "make help" we need to evaluate every single one of the +variables. + +To avoid this slow overhead we can cache the result of these invocations. +If we store this cache in a way that's easy for Make to use (like in Makefile +format) then it will be very quick and we'll save a lot of time with each +invocation of make. + + === 4 Host Program support Kbuild supports building executables on the host for use during the diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include index 9ffd3dda3889..de88120f1763 100644 --- a/scripts/Kbuild.include +++ b/scripts/Kbuild.include @@ -8,6 +8,8 @@ squote := ' empty := space := $(empty) $(empty) space_escape := _-_SPACE_-_ +right_paren := ) +left_paren := ( ### # Name of target with a '.' as filename prefix. foo/bar.o => foo/.bar.o @@ -69,6 +71,53 @@ endef # gcc support functions # See documentation in Documentation/kbuild/makefil
Re: [v8 0/4] cgroup-aware OOM killer
On Tue, Sep 26, 2017 at 03:30:40PM +0200, Michal Hocko wrote: > On Tue 26-09-17 13:13:00, Roman Gushchin wrote: > > On Tue, Sep 26, 2017 at 01:21:34PM +0200, Michal Hocko wrote: > > > On Tue 26-09-17 11:59:25, Roman Gushchin wrote: > > > > On Mon, Sep 25, 2017 at 10:25:21PM +0200, Michal Hocko wrote: > > > > > On Mon 25-09-17 19:15:33, Roman Gushchin wrote: > > > > > [...] > > > > > > I'm not against this model, as I've said before. It feels logical, > > > > > > and will work fine in most cases. > > > > > > > > > > > > In this case we can drop any mount/boot options, because it > > > > > > preserves > > > > > > the existing behavior in the default configuration. A big advantage. > > > > > > > > > > I am not sure about this. We still need an opt-in, ragardless, because > > > > > selecting the largest process from the largest memcg != selecting the > > > > > largest task (just consider memcgs with many processes example). > > > > > > > > As I understand Johannes, he suggested to compare individual processes > > > > with > > > > group_oom mem cgroups. In other words, always select a killable entity > > > > with > > > > the biggest memory footprint. > > > > > > > > This is slightly different from my v8 approach, where I treat leaf > > > > memcgs > > > > as indivisible memory consumers independent on group_oom setting, so > > > > by default I'm selecting the biggest task in the biggest memcg. > > > > > > My reading is that he is actually proposing the same thing I've been > > > mentioning. Simply select the biggest killable entity (leaf memcg or > > > group_oom hierarchy) and either kill the largest task in that entity > > > (for !group_oom) or the whole memcg/hierarchy otherwise. > > > > He wrote the following: > > "So I'm leaning toward the second model: compare all oomgroups and > > standalone tasks in the system with each other, independent of the > > failed hierarchical control structure. Then kill the biggest of them." > > I will let Johannes to comment but I believe this is just a > misunderstanding. If we compared only the biggest task from each memcg > then we are basically losing our fairness objective, aren't we? Sorry about the confusion. Yeah I was making the case for what Michal proposed, to kill the biggest terminal consumer, which is either a task or an oomgroup. You'd basically iterate through all the tasks and cgroups in the system and pick the biggest task that isn't in an oom group or the biggest oom group and then kill that. Yeah, you'd have to compare the memory footprints of tasks with the memory footprints of cgroups. These aren't defined identically, and tasks don't get attributed every type of allocation that a cgroup would. But it should get us in the ballpark, and I cannot picture a scenario where this would lead to a completely undesirable outcome. -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] scripts: kernel-doc: parse next structs/unions
> Am 25.09.2017 um 20:22 schrieb Mauro Carvalho Chehab
> :
>
> Jon,
>
> Please let me know what you think about this approach. IMHO, it is a way
> better than what we do right now. This is more a proof of concept, as the
> patch is not complete.
>
> With it, we can now document both anonymous and named unions/structs.
>
> For named structs, the name of the fields should be in the format:
> foo.bar
>
> What's missing on this PoC:
>
> 1) I didn't check if @foo.bar: would work, though. Probably the parser
> for it should be added to allow this new syntax.
Below you will find my with a "@sys_clk.rate: lorem ipsum".
With I expected a output in the **Members** section for sys_clk.rate but
I got none :o
Anyway your concept is good and so I applied it to my py-version. If you
are interested in follow the patches titled ...
kernel-doc: fix nested & sub-nested handling (continued
at https://github.com/return42/linuxdoc/commits/master
I also added some comments from my (py-) experience to your perl
implementation below.
Here are some links which illustrate how your concept could work:
documentation with example:
https://return42.github.io/linuxdoc/linuxdoc-howto/kernel-doc-syntax.html#structs-unions
how the example is rendered:
https://return42.github.io/linuxdoc/linuxdoc-howto/all-in-a-tumble.html#example-my-struct
>
> 2) I only changed the ReST output to preserve the struct format with
> nested fields.
>
> For (2), I'm thinking is we should still have all those output formats for
> kernel-doc. IMHO, I would keep just RST (and perhaps man - while we don't
> have a "make man" targed working.
>
> Depending on your comments, I'll proceed addressing (1) and (2)
> and doing more tests with it.
>
>
>
> scripts/kernel-doc | 83 +++---
> 1 file changed, 54 insertions(+), 29 deletions(-)
>
> diff --git a/scripts/kernel-doc b/scripts/kernel-doc
> index 9d3eafea58f0..5ca81b879088 100755
> --- a/scripts/kernel-doc
> +++ b/scripts/kernel-doc
> @@ -2035,29 +2035,9 @@ sub output_struct_rst(%) {
>
> print "**Definition**\n\n";
> print "::\n\n";
> -print " " . $args{'type'} . " " . $args{'struct'} . " {\n";
> -foreach $parameter (@{$args{'parameterlist'}}) {
> - if ($parameter =~ /^#/) {
> - print " " . "$parameter\n";
> - next;
> - }
> -
> - my $parameter_name = $parameter;
> - $parameter_name =~ s/\[.*//;
> -
> - ($args{'parameterdescs'}{$parameter_name} ne $undescribed) || next;
> - $type = $args{'parametertypes'}{$parameter};
> - if ($type =~ m/([^\(]*\(\*)\s*\)\s*\(([^\)]*)\)/) {
> - # pointer-to-function
> - print "$1 $parameter) ($2);\n";
> - } elsif ($type =~ m/^(.*?)\s*(:.*)/) {
> - # bitfield
> - print "$1 $parameter$2;\n";
> - } else {
> - print "" . $type . " " . $parameter . ";\n";
> - }
> -}
> -print " };\n\n";
> +my $declaration = $args{'definition'};
> +$declaration =~ s/\t/ /g;
> +print " " . $args{'type'} . " " . $args{'struct'} . " {\n$declaration
> };\n\n";
>
> print "**Members**\n\n";
> $lineprefix = " ";
> @@ -2168,20 +2148,15 @@ sub dump_struct($$) {
> my $nested;
>
> if ($x =~ /(struct|union)\s+(\w+)\s*{(.*)}/) {
> - #my $decl_type = $1;
> + my $decl_type = $1;
> $declaration_name = $2;
> my $members = $3;
>
> - # ignore embedded structs or unions
> - $members =~ s/({.*})//g;
> - $nested = $1;
> -
> # ignore members marked private:
> $members =~ s/\/\*\s*private:.*?\/\*\s*public:.*?\*\///gosi;
> $members =~ s/\/\*\s*private:.*//gosi;
> # strip comments:
> $members =~ s/\/\*.*?\*\///gos;
> - $nested =~ s/\/\*.*?\*\///gos;
> # strip kmemcheck_bitfield_{begin,end}.*;
> $members =~ s/kmemcheck_bitfield_.*?;//gos;
> # strip attributes
> @@ -2193,13 +2168,63 @@ sub dump_struct($$) {
> # replace DECLARE_HASHTABLE
> $members =~ s/DECLARE_HASHTABLE\s*\(([^,)]+), ([^,)]+)\)/unsigned long
> $1\[1 << (($2) - 1)\]/gos;
>
> + my $declaration = $members;
> +
> + # Split nested struct/union elements as newer ones
> + my $cont = 1;
> + while ($cont) {
> + $cont = 0;
Is the outer loop needed ..
> + while ($members =~
> m/(struct|union)([^{};]+){([^{}]*)}([^{}\;]*)\;/) {
this (inner) loop seems enough to me.
> + my $newmember = "$1 $4;";
> + my $id = $4;
This won't work if you have e.g.
union car {int foo;} bar1, bar2, *bbar3;
where $id will be "bar1, bar2, *bbar3", you need to split
this string and iterate over the id's.
> + my $content = $3;
> + $id =~ s/[:\[].*//;
> + foreach my $arg (split /;/, $content) {
> + next if ($arg =~ m/^\s*$/);
> + my $type = $arg;
Re: [v8 0/4] cgroup-aware OOM killer
On Tue 26-09-17 13:13:00, Roman Gushchin wrote: > On Tue, Sep 26, 2017 at 01:21:34PM +0200, Michal Hocko wrote: > > On Tue 26-09-17 11:59:25, Roman Gushchin wrote: > > > On Mon, Sep 25, 2017 at 10:25:21PM +0200, Michal Hocko wrote: > > > > On Mon 25-09-17 19:15:33, Roman Gushchin wrote: > > > > [...] > > > > > I'm not against this model, as I've said before. It feels logical, > > > > > and will work fine in most cases. > > > > > > > > > > In this case we can drop any mount/boot options, because it preserves > > > > > the existing behavior in the default configuration. A big advantage. > > > > > > > > I am not sure about this. We still need an opt-in, ragardless, because > > > > selecting the largest process from the largest memcg != selecting the > > > > largest task (just consider memcgs with many processes example). > > > > > > As I understand Johannes, he suggested to compare individual processes > > > with > > > group_oom mem cgroups. In other words, always select a killable entity > > > with > > > the biggest memory footprint. > > > > > > This is slightly different from my v8 approach, where I treat leaf memcgs > > > as indivisible memory consumers independent on group_oom setting, so > > > by default I'm selecting the biggest task in the biggest memcg. > > > > My reading is that he is actually proposing the same thing I've been > > mentioning. Simply select the biggest killable entity (leaf memcg or > > group_oom hierarchy) and either kill the largest task in that entity > > (for !group_oom) or the whole memcg/hierarchy otherwise. > > He wrote the following: > "So I'm leaning toward the second model: compare all oomgroups and > standalone tasks in the system with each other, independent of the > failed hierarchical control structure. Then kill the biggest of them." I will let Johannes to comment but I believe this is just a misunderstanding. If we compared only the biggest task from each memcg then we are basically losing our fairness objective, aren't we? -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3 v12] printk: Add monotonic, boottime, and realtime timestamps
On 09/26/2017 07:48 AM, Petr Mladek wrote:
> On Mon 2017-09-18 13:51:00, Prarit Bhargava wrote:
>> printk.time=1/CONFIG_PRINTK_TIME=1 adds a unmodified local hardware clock
>> timestamp to printk messages. The local hardware clock loses time each
>> day making it difficult to determine exactly when an issue has occurred in
>> the kernel log, and making it difficult to determine how kernel and
>> hardware issues relate to each other in real time.
>>
>> Make printk output different timestamps by adding options for no
>> timestamp, the local hardware clock, the monotonic clock, the boottime
>> clock, and the real clock. Allow a user to pick one of the clocks by
>> using the printk.time kernel parameter. Output the type of clock in
>> /sys/module/printk/parameters/time so userspace programs can interpret the
>> timestamp.
>>
>> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
>> index 512f7c2baedd..5e0bf2ef02f7 100644
>> --- a/kernel/printk/printk.c
>> +++ b/kernel/printk/printk.c
>> @@ -1201,14 +1204,130 @@ static inline void boot_delay_msec(int level)
>> +static int printk_time = CONFIG_PRINTK_TIME_TYPE;
>> +
>> +static int printk_set_ts_source(enum timestamp_sources ts_source)
>> +{
>> +int err = 0;
>
>
>> @@ -1861,6 +1980,7 @@ static size_t msg_print_text(const struct printk_log
>> *msg,
>> bool syslog, char *buf, size_t size) { return 0; }
>> static bool suppress_message_printing(int level) { return false; }
>>
>> +static int printk_time;
>
> I worried if the variable should have got initialized. But it seems to
> be a relic from an older version. The variable is not longer used and
> needed when CONFIG_PRINTK is not defined. It is proved by gcc:
>
> CC kernel/printk/printk.o
> kernel/printk/printk.c:1983:12: warning: ‘printk_time’ defined but not used
> [-Wunused-variable]
> static int printk_time;
>
I didn't catch that :(. tglx, want a v14?
P.
>
>> #endif /* CONFIG_PRINTK */
>>
>> #ifdef CONFIG_EARLY_PRINTK
>
> Otherwise that patch looks fine. With the unused variable removed,
> feel free to use:
>
> Reviewed-by: Petr Mladek
>
> Best Regards,
> Petr
>
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] scripts: kernel-doc: fix nexted handling
> Am 25.09.2017 um 20:41 schrieb Mauro Carvalho Chehab
> :
>>> + $cont = 1;
>>> + };
>>> + };
>>> + # Ignore other nested elements, like enums
>>> + $members =~ s/({[^\{\}]*})//g;
>>> + $nested = $decl_type;
>>
>> What is the latter good for? I guess the 'nested' trick to suppress
>> such 'excess' warnings from nested types is no longer needed .. right?
>
> For things like:
>
> enum { foo, bar } type;
>
> Granted, a good documentation should also describe "foo" and "bar",
> but that could be easily done by moving enums out of the struct, or
> by add descriptions for "foo" and "bar" at @type: markup.
Hm .. I suppose you are misunderstanding me. I didn't asked about
$members, I asked about $nested. There is only one place where
$nested is used, and this is in the check_sections function ...
@@ -2531,9 +2527,7 @@ sub check_sections($$) {
} else {
- if ($nested !~ m/\Q$sects[$sx]\E/) {
- print STDERR "${file}:$.: warning: " .
- "Excess struct/union/enum/typedef
member " .
- "'$sects[$sx]' " .
- "description in '$decl_name'\n";
- ++$warnings;
- }
+print STDERR "${file}:$.: warning: " .
+"Excess struct/union/enum/typedef member " .
+"'$sects[$sx]' " .
+"description in '$decl_name'\n";
+++$warnings;
}
Since this is the only place where $nested is use, we can drop all
the occurrence of $nested in the kernel-doc script .. or I'am
totally wrong?
-- Markus
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [v8 0/4] cgroup-aware OOM killer
On Tue, Sep 26, 2017 at 01:21:34PM +0200, Michal Hocko wrote: > On Tue 26-09-17 11:59:25, Roman Gushchin wrote: > > On Mon, Sep 25, 2017 at 10:25:21PM +0200, Michal Hocko wrote: > > > On Mon 25-09-17 19:15:33, Roman Gushchin wrote: > > > [...] > > > > I'm not against this model, as I've said before. It feels logical, > > > > and will work fine in most cases. > > > > > > > > In this case we can drop any mount/boot options, because it preserves > > > > the existing behavior in the default configuration. A big advantage. > > > > > > I am not sure about this. We still need an opt-in, ragardless, because > > > selecting the largest process from the largest memcg != selecting the > > > largest task (just consider memcgs with many processes example). > > > > As I understand Johannes, he suggested to compare individual processes with > > group_oom mem cgroups. In other words, always select a killable entity with > > the biggest memory footprint. > > > > This is slightly different from my v8 approach, where I treat leaf memcgs > > as indivisible memory consumers independent on group_oom setting, so > > by default I'm selecting the biggest task in the biggest memcg. > > My reading is that he is actually proposing the same thing I've been > mentioning. Simply select the biggest killable entity (leaf memcg or > group_oom hierarchy) and either kill the largest task in that entity > (for !group_oom) or the whole memcg/hierarchy otherwise. He wrote the following: "So I'm leaning toward the second model: compare all oomgroups and standalone tasks in the system with each other, independent of the failed hierarchical control structure. Then kill the biggest of them." > > > While the approach suggested by Johannes looks clear and reasonable, > > I'm slightly concerned about possible implementation issues, > > which I've described below: > > > > > > > > > The only thing, I'm slightly concerned, that due to the way how we > > > > calculate > > > > the memory footprint for tasks and memory cgroups, we will have a number > > > > of weird edge cases. For instance, when putting a single process into > > > > the group_oom memcg will alter the oom_score significantly and result > > > > in significantly different chances to be killed. An obvious example will > > > > be a task with oom_score_adj set to any non-extreme (other than 0 and > > > > -1000) > > > > value, but it can also happen in case of constrained alloc, for > > > > instance. > > > > > > I am not sure I understand. Are you talking about root memcg comparing > > > to other memcgs? > > > > Not only, but root memcg in this case will be another complication. We can > > also use the same trick for all memcg (define memcg oom_score as maximum > > oom_score > > of the belonging tasks), it will turn group_oom into pure container cleanup > > solution, without changing victim selection algorithm > > I fail to see the problem to be honest. Simply evaluate the memcg_score > you have so far with one minor detail. You only check memcgs which have > tasks (rather than check for leaf node check) or it is group_oom. An > intermediate memcg will get a cumulative size of the whole subhierarchy > and then you know you can skip the subtree because any subtree can be larger. > > > But, again, I'm not against approach suggested by Johannes. I think that > > overall > > it's the best possible semantics, if we're not taking some implementation > > details > > into account. > > I do not see those implementation details issues and let me repeat do > not develop a semantic based on implementation details. There are no problems in "select the biggest leaf or group_oom memcg, then kill the biggest task or all tasks depending on group_oom" approach, which you're describing. Comparing tasks and memcgs (what Johannes is suggesting) may have some issues. Thanks! -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3 v12] printk: Add monotonic, boottime, and realtime timestamps
On Mon 2017-09-18 13:51:00, Prarit Bhargava wrote:
> printk.time=1/CONFIG_PRINTK_TIME=1 adds a unmodified local hardware clock
> timestamp to printk messages. The local hardware clock loses time each
> day making it difficult to determine exactly when an issue has occurred in
> the kernel log, and making it difficult to determine how kernel and
> hardware issues relate to each other in real time.
>
> Make printk output different timestamps by adding options for no
> timestamp, the local hardware clock, the monotonic clock, the boottime
> clock, and the real clock. Allow a user to pick one of the clocks by
> using the printk.time kernel parameter. Output the type of clock in
> /sys/module/printk/parameters/time so userspace programs can interpret the
> timestamp.
>
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 512f7c2baedd..5e0bf2ef02f7 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -1201,14 +1204,130 @@ static inline void boot_delay_msec(int level)
> +static int printk_time = CONFIG_PRINTK_TIME_TYPE;
> +
> +static int printk_set_ts_source(enum timestamp_sources ts_source)
> +{
> + int err = 0;
> @@ -1861,6 +1980,7 @@ static size_t msg_print_text(const struct printk_log
> *msg,
>bool syslog, char *buf, size_t size) { return 0; }
> static bool suppress_message_printing(int level) { return false; }
>
> +static int printk_time;
I worried if the variable should have got initialized. But it seems to
be a relic from an older version. The variable is not longer used and
needed when CONFIG_PRINTK is not defined. It is proved by gcc:
CC kernel/printk/printk.o
kernel/printk/printk.c:1983:12: warning: ‘printk_time’ defined but not used
[-Wunused-variable]
static int printk_time;
> #endif /* CONFIG_PRINTK */
>
> #ifdef CONFIG_EARLY_PRINTK
Otherwise that patch looks fine. With the unused variable removed,
feel free to use:
Reviewed-by: Petr Mladek
Best Regards,
Petr
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [v8 0/4] cgroup-aware OOM killer
On Tue 26-09-17 11:59:25, Roman Gushchin wrote: > On Mon, Sep 25, 2017 at 10:25:21PM +0200, Michal Hocko wrote: > > On Mon 25-09-17 19:15:33, Roman Gushchin wrote: > > [...] > > > I'm not against this model, as I've said before. It feels logical, > > > and will work fine in most cases. > > > > > > In this case we can drop any mount/boot options, because it preserves > > > the existing behavior in the default configuration. A big advantage. > > > > I am not sure about this. We still need an opt-in, ragardless, because > > selecting the largest process from the largest memcg != selecting the > > largest task (just consider memcgs with many processes example). > > As I understand Johannes, he suggested to compare individual processes with > group_oom mem cgroups. In other words, always select a killable entity with > the biggest memory footprint. > > This is slightly different from my v8 approach, where I treat leaf memcgs > as indivisible memory consumers independent on group_oom setting, so > by default I'm selecting the biggest task in the biggest memcg. My reading is that he is actually proposing the same thing I've been mentioning. Simply select the biggest killable entity (leaf memcg or group_oom hierarchy) and either kill the largest task in that entity (for !group_oom) or the whole memcg/hierarchy otherwise. > While the approach suggested by Johannes looks clear and reasonable, > I'm slightly concerned about possible implementation issues, > which I've described below: > > > > > > The only thing, I'm slightly concerned, that due to the way how we > > > calculate > > > the memory footprint for tasks and memory cgroups, we will have a number > > > of weird edge cases. For instance, when putting a single process into > > > the group_oom memcg will alter the oom_score significantly and result > > > in significantly different chances to be killed. An obvious example will > > > be a task with oom_score_adj set to any non-extreme (other than 0 and > > > -1000) > > > value, but it can also happen in case of constrained alloc, for instance. > > > > I am not sure I understand. Are you talking about root memcg comparing > > to other memcgs? > > Not only, but root memcg in this case will be another complication. We can > also use the same trick for all memcg (define memcg oom_score as maximum > oom_score > of the belonging tasks), it will turn group_oom into pure container cleanup > solution, without changing victim selection algorithm I fail to see the problem to be honest. Simply evaluate the memcg_score you have so far with one minor detail. You only check memcgs which have tasks (rather than check for leaf node check) or it is group_oom. An intermediate memcg will get a cumulative size of the whole subhierarchy and then you know you can skip the subtree because any subtree can be larger. > But, again, I'm not against approach suggested by Johannes. I think that > overall > it's the best possible semantics, if we're not taking some implementation > details > into account. I do not see those implementation details issues and let me repeat do not develop a semantic based on implementation details. -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [v8 0/4] cgroup-aware OOM killer
On Mon, Sep 25, 2017 at 10:25:21PM +0200, Michal Hocko wrote: > On Mon 25-09-17 19:15:33, Roman Gushchin wrote: > [...] > > I'm not against this model, as I've said before. It feels logical, > > and will work fine in most cases. > > > > In this case we can drop any mount/boot options, because it preserves > > the existing behavior in the default configuration. A big advantage. > > I am not sure about this. We still need an opt-in, ragardless, because > selecting the largest process from the largest memcg != selecting the > largest task (just consider memcgs with many processes example). As I understand Johannes, he suggested to compare individual processes with group_oom mem cgroups. In other words, always select a killable entity with the biggest memory footprint. This is slightly different from my v8 approach, where I treat leaf memcgs as indivisible memory consumers independent on group_oom setting, so by default I'm selecting the biggest task in the biggest memcg. While the approach suggested by Johannes looks clear and reasonable, I'm slightly concerned about possible implementation issues, which I've described below: > > > The only thing, I'm slightly concerned, that due to the way how we calculate > > the memory footprint for tasks and memory cgroups, we will have a number > > of weird edge cases. For instance, when putting a single process into > > the group_oom memcg will alter the oom_score significantly and result > > in significantly different chances to be killed. An obvious example will > > be a task with oom_score_adj set to any non-extreme (other than 0 and -1000) > > value, but it can also happen in case of constrained alloc, for instance. > > I am not sure I understand. Are you talking about root memcg comparing > to other memcgs? Not only, but root memcg in this case will be another complication. We can also use the same trick for all memcg (define memcg oom_score as maximum oom_score of the belonging tasks), it will turn group_oom into pure container cleanup solution, without changing victim selection algorithm But, again, I'm not against approach suggested by Johannes. I think that overall it's the best possible semantics, if we're not taking some implementation details into account. -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [v8 0/4] cgroup-aware OOM killer
On Mon 25-09-17 15:21:03, David Rientjes wrote: > On Mon, 25 Sep 2017, Johannes Weiner wrote: > > > > True but we want to have the semantic reasonably understandable. And it > > > is quite hard to explain that the oom killer hasn't selected the largest > > > memcg just because it happened to be in a deeper hierarchy which has > > > been configured to cover a different resource. > > > > Going back to Michal's example, say the user configured the following: > > > >root > > /\ > > A D > > / \ > >B C > > > > A global OOM event happens and we find this: > > - A > D > > - B, C, D are oomgroups > > > > What the user is telling us is that B, C, and D are compound memory > > consumers. They cannot be divided into their task parts from a memory > > point of view. > > > > However, the user doesn't say the same for A: the A subtree summarizes > > and controls aggregate consumption of B and C, but without groupoom > > set on A, the user says that A is in fact divisible into independent > > memory consumers B and C. > > > > If we don't have to kill all of A, but we'd have to kill all of D, > > does it make sense to compare the two? > > > > No, I agree that we shouldn't compare sibling memory cgroups based on > different criteria depending on whether group_oom is set or not. > > I think it would be better to compare siblings based on the same criteria > independent of group_oom if the user has mounted the hierarchy with the > new mode (I think we all agree that the mount option is needed). It's > very easy to describe to the user and the selection is simple to > understand. I disagree. Just take the most simplistic example when cgroups reflect some other higher level organization - e.g. school with teachers, students and admins as the top level cgroups to control the proper cpu share load. Now you want to have a fair OOM selection between different entities. Do you consider selecting students all the time as an expected behavior just because their are the largest group? This just doesn't make any sense to me. > Then, once a cgroup has been chosen as the victim cgroup, > kill the process with the highest badness, allowing the user to influence > that with /proc/pid/oom_score_adj just as today, if group_oom is disabled; > otherwise, kill all eligible processes if enabled. And now, what should be the semantic of group_oom on an intermediate (non-leaf) memcg? Why should we compare it to other killable entities? Roman was mentioning a setup where a _single_ workload consists of a deeper hierarchy which has to be shut down at once. It absolutely makes sense to consider the cumulative memory of that hierarchy when we are going to kill it all. > That, to me, is a very clear semantic and I believe it addresses Roman's > usecase. My desire to have oom priorities amongst siblings is so that > userspace can influence which cgroup is chosen, just as it can influence > which process is chosen. But what you are proposing is something different from oom_score_adj. That only sets bias to the killable entities while priorities on intermediate non-killable memcgs controls how the whole oom hierarchy is traversed. So a non-killable intermediate memcg can hugely influence what gets killed in the end. This is IMHO a tricky and I would even dare to claim a wrong semantic. I can see priorities being very useful on killable entities for sure. I am not entirely sure what would be the best approach yet and that is why I've suggested that to postpone to after we settle with a simple approach first. Bringing priorities back to the discussion again will not help to move that forward I am afraid. -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
