As the number of schemas has increased, we're starting to hit the error
"execvp: /bin/sh: Argument list too long". This is due to passing all the
schema files on the command line to dt-mk-schema. It currently is only
with out of tree builds and is intermittent depending on the file path
lengths.
Commit 2ba06cd8565b ("kbuild: Always validate DT binding examples") made
hitting this proplem more likely since the example validation now always
gets the full list of schemas.
Fix this by putting the schema file list into a temp file and using xargs.
Reported-by: Laurent Pinchart <[email protected]>
Cc: Masahiro Yamada <[email protected]>
Signed-off-by: Rob Herring <[email protected]>
---
Documentation/devicetree/bindings/.gitignore | 2 +-
Documentation/devicetree/bindings/Makefile | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/.gitignore b/Documentation/devicetree/bindings/.gitignore
index 5c6d8ea1a09c..0a6aef915fa4 100644
--- a/Documentation/devicetree/bindings/.gitignore
+++ b/Documentation/devicetree/bindings/.gitignore
@@ -1,3 +1,3 @@
# SPDX-License-Identifier: GPL-2.0-only
*.example.dts
-processed-schema*.yaml
+processed-schema*.yaml*
diff --git a/Documentation/devicetree/bindings/Makefile b/Documentation/devicetree/bindings/Makefile
index 1df680d07461..1c1cad860b7c 100644
--- a/Documentation/devicetree/bindings/Makefile
+++ b/Documentation/devicetree/bindings/Makefile
@@ -14,7 +14,8 @@ $(obj)/%.example.dts: $(src)/%.yaml FORCE
DT_TMP_SCHEMA := $(obj)/processed-schema-examples.yaml
quiet_cmd_mk_schema = SCHEMA $@
- cmd_mk_schema = $(DT_MK_SCHEMA) $(DT_MK_SCHEMA_FLAGS) -o $@ $(real-prereqs)
+ cmd_mk_schema = $(file >[email protected], $(real-prereqs)) \
+ cat [email protected] | xargs $(DT_MK_SCHEMA) $(DT_MK_SCHEMA_FLAGS) -o $@
DT_DOCS = $(addprefix $(src)/, \
$(shell \
--
2.20.1
In commit 2ba06cd8565b ("kbuild: Always validate DT binding examples"),
the core schemas (from dtschema repo) were inadvertently disabled for
dtbs_checks. Re-enable them.
Fixes: 2ba06cd8565b ("kbuild: Always validate DT binding examples")
Cc: Masahiro Yamada <[email protected]>
Signed-off-by: Rob Herring <[email protected]>
---
Documentation/devicetree/bindings/Makefile | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/Makefile b/Documentation/devicetree/bindings/Makefile
index 1c1cad860b7c..c66e8329a645 100644
--- a/Documentation/devicetree/bindings/Makefile
+++ b/Documentation/devicetree/bindings/Makefile
@@ -2,6 +2,7 @@
DT_DOC_CHECKER ?= dt-doc-validate
DT_EXTRACT_EX ?= dt-extract-example
DT_MK_SCHEMA ?= dt-mk-schema
+DT_MK_SCHEMA_USERONLY_FLAG := $(if $(DT_SCHEMA_FILES), -u)
quiet_cmd_chk_binding = CHKDT $(patsubst $(srctree)/%,%,$<)
cmd_chk_binding = $(DT_DOC_CHECKER) -u $(srctree)/$(src) $< ; \
@@ -38,7 +39,7 @@ override DTC_FLAGS := \
$(obj)/processed-schema-examples.yaml: $(DT_DOCS) FORCE
$(call if_changed,mk_schema)
-$(obj)/processed-schema.yaml: DT_MK_SCHEMA_FLAGS := -u
+$(obj)/processed-schema.yaml: DT_MK_SCHEMA_FLAGS := $(DT_MK_SCHEMA_USERONLY_FLAG)
$(obj)/processed-schema.yaml: $(DT_SCHEMA_FILES) FORCE
$(call if_changed,mk_schema)
--
2.20.1
Hi Rob,
Thank you for the patch.
On Tue, Apr 21, 2020 at 04:20:03PM -0500, Rob Herring wrote:
> As the number of schemas has increased, we're starting to hit the error
> "execvp: /bin/sh: Argument list too long". This is due to passing all the
> schema files on the command line to dt-mk-schema. It currently is only
> with out of tree builds and is intermittent depending on the file path
> lengths.
>
> Commit 2ba06cd8565b ("kbuild: Always validate DT binding examples") made
> hitting this proplem more likely since the example validation now always
> gets the full list of schemas.
>
> Fix this by putting the schema file list into a temp file and using xargs.
>
> Reported-by: Laurent Pinchart <[email protected]>
> Cc: Masahiro Yamada <[email protected]>
> Signed-off-by: Rob Herring <[email protected]>
Quite a bit slower than v5.6 when passing DT_SCHEMA_FILES, but
reasonable, and working now :-)
Tested-by: Laurent Pinchart <[email protected]>
> ---
> Documentation/devicetree/bindings/.gitignore | 2 +-
> Documentation/devicetree/bindings/Makefile | 3 ++-
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/.gitignore b/Documentation/devicetree/bindings/.gitignore
> index 5c6d8ea1a09c..0a6aef915fa4 100644
> --- a/Documentation/devicetree/bindings/.gitignore
> +++ b/Documentation/devicetree/bindings/.gitignore
> @@ -1,3 +1,3 @@
> # SPDX-License-Identifier: GPL-2.0-only
> *.example.dts
> -processed-schema*.yaml
> +processed-schema*.yaml*
> diff --git a/Documentation/devicetree/bindings/Makefile b/Documentation/devicetree/bindings/Makefile
> index 1df680d07461..1c1cad860b7c 100644
> --- a/Documentation/devicetree/bindings/Makefile
> +++ b/Documentation/devicetree/bindings/Makefile
> @@ -14,7 +14,8 @@ $(obj)/%.example.dts: $(src)/%.yaml FORCE
> DT_TMP_SCHEMA := $(obj)/processed-schema-examples.yaml
>
> quiet_cmd_mk_schema = SCHEMA $@
> - cmd_mk_schema = $(DT_MK_SCHEMA) $(DT_MK_SCHEMA_FLAGS) -o $@ $(real-prereqs)
> + cmd_mk_schema = $(file >[email protected], $(real-prereqs)) \
> + cat [email protected] | xargs $(DT_MK_SCHEMA) $(DT_MK_SCHEMA_FLAGS) -o $@
>
> DT_DOCS = $(addprefix $(src)/, \
> $(shell \
--
Regards,
Laurent Pinchart
On Tue, Apr 21, 2020 at 6:37 PM Laurent Pinchart
<[email protected]> wrote:
>
> Hi Rob,
>
> Thank you for the patch.
>
> On Tue, Apr 21, 2020 at 04:20:03PM -0500, Rob Herring wrote:
> > As the number of schemas has increased, we're starting to hit the error
> > "execvp: /bin/sh: Argument list too long". This is due to passing all the
> > schema files on the command line to dt-mk-schema. It currently is only
> > with out of tree builds and is intermittent depending on the file path
> > lengths.
> >
> > Commit 2ba06cd8565b ("kbuild: Always validate DT binding examples") made
> > hitting this proplem more likely since the example validation now always
> > gets the full list of schemas.
> >
> > Fix this by putting the schema file list into a temp file and using xargs.
> >
> > Reported-by: Laurent Pinchart <[email protected]>
> > Cc: Masahiro Yamada <[email protected]>
> > Signed-off-by: Rob Herring <[email protected]>
>
> Quite a bit slower than v5.6 when passing DT_SCHEMA_FILES, but
> reasonable, and working now :-)
That's expected. It's validating with ~700 vs. 1 schema. The problem
was folks only checking with DT_SCHEMA_FILES set, but a new schema can
affect another example or existing schemas (including core schema) may
fail on the new example.
Rob
Hi Rob,
On Wed, Apr 22, 2020 at 6:20 AM Rob Herring <[email protected]> wrote:
>
> As the number of schemas has increased, we're starting to hit the error
> "execvp: /bin/sh: Argument list too long". This is due to passing all the
> schema files on the command line to dt-mk-schema. It currently is only
> with out of tree builds and is intermittent depending on the file path
> lengths.
>
> Commit 2ba06cd8565b ("kbuild: Always validate DT binding examples") made
> hitting this proplem more likely since the example validation now always
> gets the full list of schemas.
>
> Fix this by putting the schema file list into a temp file and using xargs.
>
> Reported-by: Laurent Pinchart <[email protected]>
> Cc: Masahiro Yamada <[email protected]>
> Signed-off-by: Rob Herring <[email protected]>
> ---
> Documentation/devicetree/bindings/.gitignore | 2 +-
> Documentation/devicetree/bindings/Makefile | 3 ++-
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/.gitignore b/Documentation/devicetree/bindings/.gitignore
> index 5c6d8ea1a09c..0a6aef915fa4 100644
> --- a/Documentation/devicetree/bindings/.gitignore
> +++ b/Documentation/devicetree/bindings/.gitignore
> @@ -1,3 +1,3 @@
> # SPDX-License-Identifier: GPL-2.0-only
> *.example.dts
> -processed-schema*.yaml
> +processed-schema*.yaml*
> diff --git a/Documentation/devicetree/bindings/Makefile b/Documentation/devicetree/bindings/Makefile
> index 1df680d07461..1c1cad860b7c 100644
> --- a/Documentation/devicetree/bindings/Makefile
> +++ b/Documentation/devicetree/bindings/Makefile
> @@ -14,7 +14,8 @@ $(obj)/%.example.dts: $(src)/%.yaml FORCE
> DT_TMP_SCHEMA := $(obj)/processed-schema-examples.yaml
>
> quiet_cmd_mk_schema = SCHEMA $@
> - cmd_mk_schema = $(DT_MK_SCHEMA) $(DT_MK_SCHEMA_FLAGS) -o $@ $(real-prereqs)
> + cmd_mk_schema = $(file >[email protected], $(real-prereqs)) \
> + cat [email protected] | xargs $(DT_MK_SCHEMA) $(DT_MK_SCHEMA_FLAGS) -o $@
>
The built-in function $(file ...) is supported on GNU Make 4.0 or later.
The current minimal version is GNU Make 3.81.
If you want to use this function, you must update
Documentation/process/changes.rst first.
I am pretty sure some conservative distros
still stick to GNU Make 3.8*
but I am open to raising the minimal version
if it is useful.
But, does this code work in the first place?
When a very long command is given, xargs
splits it into smaller chunks, and invokes
the command multiple times, right?
So, it boils down to this question:
Are the following two commands work equivalently?
"dt-mk-schema -o processed-schema-examples.yaml foo.yaml &&
dt-mk-schema -o processed-schema-examples.yaml bar.yaml &&
dt-mk-schema -o processed-schema-examples.yaml baz.yaml"
"dt-mk-schema -o processed-schema-examples.yaml foo.yaml bar.yaml baz.yaml"
I think the answer is no.
I confirmed the produced processed-schema-examples.yaml is broken.
In a normal case, it is more than 50000 lines,
but if xargs split the command, it is much smaller.
masahiro@oscar:~/ref/linux$ make -j24 O=foo/bar dt_binding_check
masahiro@oscar:~/ref/linux$ wc
foo/bar/Documentation/devicetree/bindings/processed-schema-examples.yaml
51461 115967 1516186
foo/bar/Documentation/devicetree/bindings/processed-schema-examples.yaml
masahiro@oscar:~/ref/this/is/a/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/src/path/to/linux$
make -j24 O=foo/bar dt_binding_check
masahiro@oscar:~/ref/this/is/a/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/long/src/path/to/linux$
wc foo/bar/Documentation/devicetree/bindings/processed-schema-examples.yaml
9625 21510 291993
foo/bar/Documentation/devicetree/bindings/processed-schema-examples.yaml
Can dt-mk-schema read the list of schema files
by other means?
> DT_DOCS = $(addprefix $(src)/, \
> $(shell \
> --
> 2.20.1
>
--
Best Regards
Masahiro Yamada
On Wed, Apr 22, 2020 at 6:20 AM Rob Herring <[email protected]> wrote:
>
> In commit 2ba06cd8565b ("kbuild: Always validate DT binding examples"),
> the core schemas (from dtschema repo) were inadvertently disabled for
> dtbs_checks. Re-enable them.
>
> Fixes: 2ba06cd8565b ("kbuild: Always validate DT binding examples")
> Cc: Masahiro Yamada <[email protected]>
> Signed-off-by: Rob Herring <[email protected]>
Reviewed-by: Masahiro Yamada <[email protected]>
> ---
> Documentation/devicetree/bindings/Makefile | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/Makefile b/Documentation/devicetree/bindings/Makefile
> index 1c1cad860b7c..c66e8329a645 100644
> --- a/Documentation/devicetree/bindings/Makefile
> +++ b/Documentation/devicetree/bindings/Makefile
> @@ -2,6 +2,7 @@
> DT_DOC_CHECKER ?= dt-doc-validate
> DT_EXTRACT_EX ?= dt-extract-example
> DT_MK_SCHEMA ?= dt-mk-schema
> +DT_MK_SCHEMA_USERONLY_FLAG := $(if $(DT_SCHEMA_FILES), -u)
>
> quiet_cmd_chk_binding = CHKDT $(patsubst $(srctree)/%,%,$<)
> cmd_chk_binding = $(DT_DOC_CHECKER) -u $(srctree)/$(src) $< ; \
> @@ -38,7 +39,7 @@ override DTC_FLAGS := \
> $(obj)/processed-schema-examples.yaml: $(DT_DOCS) FORCE
> $(call if_changed,mk_schema)
>
> -$(obj)/processed-schema.yaml: DT_MK_SCHEMA_FLAGS := -u
> +$(obj)/processed-schema.yaml: DT_MK_SCHEMA_FLAGS := $(DT_MK_SCHEMA_USERONLY_FLAG)
> $(obj)/processed-schema.yaml: $(DT_SCHEMA_FILES) FORCE
> $(call if_changed,mk_schema)
>
> --
> 2.20.1
>
--
Best Regards
Masahiro Yamada
On Wed, Apr 22, 2020 at 12:59 AM Masahiro Yamada <[email protected]> wrote:
>
> Hi Rob,
>
>
> On Wed, Apr 22, 2020 at 6:20 AM Rob Herring <[email protected]> wrote:
> >
> > As the number of schemas has increased, we're starting to hit the error
> > "execvp: /bin/sh: Argument list too long". This is due to passing all the
> > schema files on the command line to dt-mk-schema. It currently is only
> > with out of tree builds and is intermittent depending on the file path
> > lengths.
> >
> > Commit 2ba06cd8565b ("kbuild: Always validate DT binding examples") made
> > hitting this proplem more likely since the example validation now always
> > gets the full list of schemas.
> >
> > Fix this by putting the schema file list into a temp file and using xargs.
> >
> > Reported-by: Laurent Pinchart <[email protected]>
> > Cc: Masahiro Yamada <[email protected]>
> > Signed-off-by: Rob Herring <[email protected]>
> > ---
> > Documentation/devicetree/bindings/.gitignore | 2 +-
> > Documentation/devicetree/bindings/Makefile | 3 ++-
> > 2 files changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/.gitignore b/Documentation/devicetree/bindings/.gitignore
> > index 5c6d8ea1a09c..0a6aef915fa4 100644
> > --- a/Documentation/devicetree/bindings/.gitignore
> > +++ b/Documentation/devicetree/bindings/.gitignore
> > @@ -1,3 +1,3 @@
> > # SPDX-License-Identifier: GPL-2.0-only
> > *.example.dts
> > -processed-schema*.yaml
> > +processed-schema*.yaml*
> > diff --git a/Documentation/devicetree/bindings/Makefile b/Documentation/devicetree/bindings/Makefile
> > index 1df680d07461..1c1cad860b7c 100644
> > --- a/Documentation/devicetree/bindings/Makefile
> > +++ b/Documentation/devicetree/bindings/Makefile
> > @@ -14,7 +14,8 @@ $(obj)/%.example.dts: $(src)/%.yaml FORCE
> > DT_TMP_SCHEMA := $(obj)/processed-schema-examples.yaml
> >
> > quiet_cmd_mk_schema = SCHEMA $@
> > - cmd_mk_schema = $(DT_MK_SCHEMA) $(DT_MK_SCHEMA_FLAGS) -o $@ $(real-prereqs)
> > + cmd_mk_schema = $(file >[email protected], $(real-prereqs)) \
> > + cat [email protected] | xargs $(DT_MK_SCHEMA) $(DT_MK_SCHEMA_FLAGS) -o $@
> >
>
>
> The built-in function $(file ...) is supported on GNU Make 4.0 or later.
> The current minimal version is GNU Make 3.81.
>
> If you want to use this function, you must update
> Documentation/process/changes.rst first.
>
> I am pretty sure some conservative distros
> still stick to GNU Make 3.8*
> but I am open to raising the minimal version
> if it is useful.
I'd like to avoid that. I've come up with another solution.
> But, does this code work in the first place?
>
> When a very long command is given, xargs
> splits it into smaller chunks, and invokes
> the command multiple times, right?
>
>
> So, it boils down to this question:
>
> Are the following two commands work equivalently?
>
>
> "dt-mk-schema -o processed-schema-examples.yaml foo.yaml &&
> dt-mk-schema -o processed-schema-examples.yaml bar.yaml &&
> dt-mk-schema -o processed-schema-examples.yaml baz.yaml"
>
> "dt-mk-schema -o processed-schema-examples.yaml foo.yaml bar.yaml baz.yaml"
>
>
> I think the answer is no.
>
> I confirmed the produced processed-schema-examples.yaml is broken.
Indeed. We need to output to stdout and append the file instead.
Sending a v2 out now.
[...]
> Can dt-mk-schema read the list of schema files
> by other means?
Not currently. Happy to add it, but I'd rather not require everyone
update right away.
Rob