dt-bindings: Fix command line length limit calling dt-mk-schema

PATCH Superseded Rob Herring 3 participants 7 messages Apr 21, 2020 – Apr 22, 2020 Device tree bindings

Share & tools

Rob Herring

author maintainer

2020-04-21 21:20 UTC #0 lore raw

PATCH 1/2 dt-bindings: Fix command line length limit calling dt-mk-schema

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(-)

Documentation/devicetree/bindings/.gitignore +1 -1

index 5c6d8ea1a09c..0a6aef915fa4 100644
@@ -1,3 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0-only
 *.example.dts
-processed-schema*.yaml
+processed-schema*.yaml*

Documentation/devicetree/bindings/Makefile +2 -1

index 1df680d07461..1c1cad860b7c 100644
@@ -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 \

Signature · 1 line

2.20.1

Rob Herring

author maintainer

2020-04-21 21:20 UTC #1 lore raw

PATCH 2/2 dt-bindings: Re-enable core schemas for dtbs_check

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(-)

Documentation/devicetree/bindings/Makefile +2 -1

index 1c1cad860b7c..c66e8329a645 100644
@@ -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)

Signature · 1 line

2.20.1

Masahiro Yamada

Reviewed-by on v1 2/2

2020-04-22 06:12 UTC #2 lore raw

Re: [PATCH 2/2] dt-bindings: Re-enable core schemas for dtbs_check

Quoted 8 lines · Rob Herring

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]>

Quoted 28 lines

---
 Documentation/devicetree/bindings/Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Documentation/devicetree/bindings/Makefile +2 -2

index 1c1cad860b7c..c66e8329a645 100644
@@ -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

Signature · 2 lines

Best Regards
Masahiro Yamada

Laurent Pinchart

Tested-by on v1 1/2

2020-04-21 23:37 UTC #3 lore raw

In reply to Rob Herring

Hi Rob,

Thank you for the patch.

Quoted 10 lines · Rob Herring

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.

Quoted 5 lines

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]>

Quoted 28 lines

---
 Documentation/devicetree/bindings/.gitignore | 2 +-
 Documentation/devicetree/bindings/Makefile   | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

Documentation/devicetree/bindings/.gitignore +1 -1

index 5c6d8ea1a09c..0a6aef915fa4 100644
@@ -1,3 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0-only
 *.example.dts
-processed-schema*.yaml
+processed-schema*.yaml*

Documentation/devicetree/bindings/Makefile +2 -1

index 1df680d07461..1c1cad860b7c 100644
@@ -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 \

Signature · 3 lines

Regards,

Laurent Pinchart

Rob Herring

author maintainer

2020-04-22 00:22 UTC #4 lore raw

Quoted 16 lines · Laurent Pinchart

Hi Rob,
Thank you for the patch.
Quoted 10 lines · Rob Herring
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.

Quoted 8 lines

Quoted 5 lines
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

Masahiro Yamada

2020-04-22 05:59 UTC #5 lore raw

In reply to Rob Herring

Hi Rob,

Quoted 35 lines · Rob Herring

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(-)
Documentation/devicetree/bindings/.gitignore +1 -1
index 5c6d8ea1a09c..0a6aef915fa4 100644
@@ -1,3 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0-only
 *.example.dts
-processed-schema*.yaml
+processed-schema*.yaml*
Documentation/devicetree/bindings/Makefile +0 -0
index 1df680d07461..1c1cad860b7c 100644

Documentation/devicetree/bindings/Makefile $(obj)/%.example.dts: $(src)/%.yaml FORCE · line 19

Quoted 7 lines

Documentation/devicetree/bindings/Makefile +2 -1

@@ -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?

Quoted 5 lines

Documentation/devicetree/bindings/Makefile +0 -1
 DT_DOCS = $(addprefix $(src)/, \
        $(shell \
--
2.20.1

Signature · 2 lines

Best Regards
Masahiro Yamada

Rob Herring

author maintainer

2020-04-22 18:56 UTC #6 lore raw

Quoted 52 lines · Masahiro Yamada

Hi Rob,
Quoted 35 lines · Rob Herring
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(-)
Documentation/devicetree/bindings/.gitignore +1 -1
index 5c6d8ea1a09c..0a6aef915fa4 100644
@@ -1,3 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0-only
 *.example.dts
-processed-schema*.yaml
+processed-schema*.yaml*
Documentation/devicetree/bindings/Makefile +0 -0
index 1df680d07461..1c1cad860b7c 100644
Documentation/devicetree/bindings/Makefile $(obj)/%.example.dts: $(src)/%.yaml FORCE · line 19
Quoted 7 lines
Documentation/devicetree/bindings/Makefile +2 -1
@@ -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.

Quoted 7 lines

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.

Quoted 16 lines

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"

Quoted 6 lines

"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.

[...]

Quoted 2 lines

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