| Message ID | 20200421212004.6146-1-robh@kernel.org |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [1/2] dt-bindings: Fix command line length limit calling dt-mk-schema | expand |
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 <laurent.pinchart@ideasonboard.com> > Cc: Masahiro Yamada <masahiroy@kernel.org> > Signed-off-by: Rob Herring <robh@kernel.org> Quite a bit slower than v5.6 when passing DT_SCHEMA_FILES, but reasonable, and working now :-) Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > 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 >$@.tmp, $(real-prereqs)) \ > + cat $@.tmp | xargs $(DT_MK_SCHEMA) $(DT_MK_SCHEMA_FLAGS) -o $@ > > DT_DOCS = $(addprefix $(src)/, \ > $(shell \
On Tue, Apr 21, 2020 at 6:37 PM Laurent Pinchart <laurent.pinchart@ideasonboard.com> 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 <laurent.pinchart@ideasonboard.com> > > Cc: Masahiro Yamada <masahiroy@kernel.org> > > Signed-off-by: Rob Herring <robh@kernel.org> > > 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 <robh@kernel.org> 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 <laurent.pinchart@ideasonboard.com> > Cc: Masahiro Yamada <masahiroy@kernel.org> > Signed-off-by: Rob Herring <robh@kernel.org> > --- > 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 >$@.tmp, $(real-prereqs)) \ > + cat $@.tmp | 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 >
On Wed, Apr 22, 2020 at 12:59 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > Hi Rob, > > > On Wed, Apr 22, 2020 at 6:20 AM Rob Herring <robh@kernel.org> 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 <laurent.pinchart@ideasonboard.com> > > Cc: Masahiro Yamada <masahiroy@kernel.org> > > Signed-off-by: Rob Herring <robh@kernel.org> > > --- > > 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 >$@.tmp, $(real-prereqs)) \ > > + cat $@.tmp | 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
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 >$@.tmp, $(real-prereqs)) \ + cat $@.tmp | xargs $(DT_MK_SCHEMA) $(DT_MK_SCHEMA_FLAGS) -o $@ DT_DOCS = $(addprefix $(src)/, \ $(shell \
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 <laurent.pinchart@ideasonboard.com> Cc: Masahiro Yamada <masahiroy@kernel.org> Signed-off-by: Rob Herring <robh@kernel.org> --- Documentation/devicetree/bindings/.gitignore | 2 +- Documentation/devicetree/bindings/Makefile | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-)