On 07/09, Khalid Masum wrote: > Hi, > > On Sun, Jul 9, 2023 at 8:38 PM Anh Tuan Phan <tuananhlfc@xxxxxxxxx> wrote: > > > > Hi Stanislav, > > > > I have updated the Documentation according to your suggestion. Please > > see it in the below patch. Thanks! > > > > On 7/7/23 23:57, Stanislav Fomichev wrote: > > > On 07/07, Anh Tuan Phan wrote: > > >> > > >> > > >> On 7/7/23 01:16, Stanislav Fomichev wrote: > > >>> On 07/06, Anh Tuan Phan wrote: > > >>>> Update the Documentation to mention that some samples require pahole > > >>>> v1.16 and kernel built with CONFIG_DEBUG_INFO_BTF=y > > >>>> > > >>>> Signed-off-by: Anh Tuan Phan <tuananhlfc@xxxxxxxxx> > > >>>> --- > > >>>> samples/bpf/README.rst | 7 +++++++ > > >>>> 1 file changed, 7 insertions(+) > > >>>> > > >>>> diff --git a/samples/bpf/README.rst b/samples/bpf/README.rst > > >>>> index 57f93edd1957..631592b83d60 100644 > > >>>> --- a/samples/bpf/README.rst > > >>>> +++ b/samples/bpf/README.rst > > >>>> @@ -14,6 +14,9 @@ Compiling requires having installed: > > >>>> Note that LLVM's tool 'llc' must support target 'bpf', list version > > >>>> and supported targets with command: ``llc --version`` > > >>>> > > >>>> +Some samples require pahole version 1.16 as a dependency. See > > >>>> +https://docs.kernel.org/bpf/bpf_devel_QA.html for reference. > > >>>> + > > >>> > > >>> Any reason no to add pahole 1.16 to this section above? > > >>>> Compiling requires having installed: > > >>> * clang >= version 3.4.0 > > >>> * llvm >= version 3.7.1 > > >>> * pahole >= version 1.16 > > >>> > > >>> Although clang 3.4 probably won't get you anywhere these days. The > > >>> whole README seems a bit outdated :-) > > >>> > > >> > > >> Put pahole requirement as your idea is better, thanks for suggestion. > > >> Will update it and clang version as well. For clang version, I think I > > >> can update min version as 11.0.0 (reference from > > >> https://www.kernel.org/doc/html/next/process/changes.html). Do you see > > >> any other potential outdated things in this document? I follow the above > > >> steps and it help me compile the sample code successfully. > > > > > > Maybe we can reference that doc instead here? Otherwise that copy-pasted > > > 11.0.0 will also get old. Just mention here that we need > > > clang/llvm/pahole to compile the samples and for specific versions > > > put a link to process/changes.rst > > > > > >>>> Clean and configuration > > >>>> ----------------------- > > >>>> > > >>>> @@ -28,6 +31,10 @@ Configure kernel, defconfig for instance:: > > >>>> > > >>>> make defconfig > > >>>> > > >>>> +Some samples require support for BPF Type Format (BTF). To enable it, > > >>>> open the > > >>>> +generated config file, or use menuconfig (by "make menuconfig") to > > >>>> enable the > > >>>> +following configs: CONFIG_BPF_SYSCALL and CONFIG_DEBUG_INFO_BTF. > > >>>> + > > >>> > > >>> This is usually enabled by default, so why special case it here? > > >>> Maybe, if you want some hints about the config, we should add > > >>> a reference to tools/testing/selftests/bpf/config ? > > >>> > > >> > > >> The config CONFIG_DEBUG_INFO_BTF is disabled for some distros at least > > >> for mine. I ran "make defconfig" and it's not enabled by default so I > > >> think it worth to mention it here to help novice get started. I'll > > >> update it to reference to tools/testing/selftests/bpf/config . > > >> > > >>>> Kernel headers > > >>>> -------------- > > >>>> > > >>>> -- > > >>>> 2.34.1 > > > > Signed-off-by: Anh Tuan Phan <tuananhlfc@xxxxxxxxx> > > --- > > > > Change from the original patch: > > > > - Move pahole to the list installed requirements > > - Remove minimal version and link the related doc > > - Add a reference of kernel configuration > > > > samples/bpf/README.rst | 10 +++++++--- > > 1 file changed, 7 insertions(+), 3 deletions(-) > > > > diff --git a/samples/bpf/README.rst b/samples/bpf/README.rst > > index 57f93edd1957..e18500753ba5 100644 > > --- a/samples/bpf/README.rst > > +++ b/samples/bpf/README.rst > > @@ -8,9 +8,12 @@ Build dependencies > > ================== > > > > Compiling requires having installed: > > - * clang >= version 3.4.0 > > - * llvm >= version 3.7.1 > > + * clang > > + * llvm > > + * pahole > > > > +The minimal version of the above software is referenced in > > +https://www.kernel.org/doc/html/next/process/changes.html. > > I think it is better to not use docs from linux-next as it keeps changing > too frequently. How about using the latest documentation's link instead? :) > > https://www.kernel.org/doc/html/latest/process/changes.html +1 We should put Documentation/process/changes.rst here (or whatever the correct path). The tooling that generates html from rst will put a proper link.
