| Message ID | 20201122022205.57229-2-andreimatei1@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 1c26ac6ab3ce47ee2e6342373681dedbb97e21a3 |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf-next,1/2] selftest/bpf: fix link in readme | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for bpf-next |
| netdev/subject_prefix | success | Link |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 47 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
On 11/21/20 6:22 PM, Andrei Matei wrote: > A couple of places in the readme had invalid rst formatting causing the > rendering to be off. This patch fixes them with minimal edits. > > Signed-off-by: Andrei Matei <andreimatei1@gmail.com> > --- > tools/testing/selftests/bpf/README.rst | 28 ++++++++++++++------------ > 1 file changed, 15 insertions(+), 13 deletions(-) I cannot apply patch #2 with my current bpf-next branch. -bash-4.4$ git apply ~/p1.txt -bash-4.4$ git apply ~/p2.txt /home/yhs/p2.txt:34: trailing whitespace. __ https://reviews.llvm.org/D85570 /home/yhs/p2.txt:52: trailing whitespace. __ https://reviews.llvm.org/D78466 /home/yhs/p2.txt:70: trailing whitespace. .. _0: https://reviews.llvm.org/D74572 /home/yhs/p2.txt:71: trailing whitespace. .. _1: https://reviews.llvm.org/D74668 /home/yhs/p2.txt:72: trailing whitespace. .. _2: https://reviews.llvm.org/D85174 error: patch failed: tools/testing/selftests/bpf/README.rst:33 error: tools/testing/selftests/bpf/README.rst: patch does not apply -bash-4.4$ git --version git version 2.24.1 -bash-4.4$ Could you help check what is the issue? Maybe the links are presented differently in the patch vs. in the README.rst? > > diff --git a/tools/testing/selftests/bpf/README.rst b/tools/testing/selftests/bpf/README.rst > index 3b8d8885892d..ca064180d4d0 100644 > --- a/tools/testing/selftests/bpf/README.rst > +++ b/tools/testing/selftests/bpf/README.rst > @@ -33,11 +33,12 @@ The verifier will reject such code with above error. > At insn 18 the r7 is indeed unbounded. The later insn 19 checks the bounds and > the insn 20 undoes map_value addition. It is currently impossible for the > verifier to understand such speculative pointer arithmetic. > -Hence > - https://reviews.llvm.org/D85570 > -addresses it on the compiler side. It was committed on llvm 12. > +Hence `this patch`__ addresses it on the compiler side. It was committed on llvm 12. > + > +__ https://reviews.llvm.org/D85570 > > The corresponding C code > + > .. code-block:: c > > for (int i = 0; i < MAX_CGROUPS_PATH_DEPTH; i++) { > @@ -80,10 +81,11 @@ The symptom for ``bpf_iter/netlink`` looks like > 17: (7b) *(u64 *)(r7 +0) = r2 > only read is supported > > -This is due to a llvm BPF backend bug. The fix > - https://reviews.llvm.org/D78466 > +This is due to a llvm BPF backend bug. `The fix`__ > has been pushed to llvm 10.x release branch and will be > -available in 10.0.1. The fix is available in llvm 11.0.0 trunk. > +available in 10.0.1. The patch is available in llvm 11.0.0 trunk. > + > +__ https://reviews.llvm.org/D78466 > > BPF CO-RE-based tests and Clang version > ======================================= > @@ -97,11 +99,11 @@ them to Clang/LLVM. These sub-tests are going to be skipped if Clang is too > old to support them, they shouldn't cause build failures or runtime test > failures: > > - - __builtin_btf_type_id() ([0], [1], [2]); > - - __builtin_preserve_type_info(), __builtin_preserve_enum_value() ([3], [4]). > +- __builtin_btf_type_id() [0_, 1_, 2_]; > +- __builtin_preserve_type_info(), __builtin_preserve_enum_value() [3_, 4_]. > > - [0] https://reviews.llvm.org/D74572 > - [1] https://reviews.llvm.org/D74668 > - [2] https://reviews.llvm.org/D85174 > - [3] https://reviews.llvm.org/D83878 > - [4] https://reviews.llvm.org/D83242 > +.. _0: https://reviews.llvm.org/D74572 > +.. _1: https://reviews.llvm.org/D74668 > +.. _2: https://reviews.llvm.org/D85174 > +.. _3: https://reviews.llvm.org/D83878 > +.. _4: https://reviews.llvm.org/D83242 >
Hi Yonghong! Thanks for looking at my patch!
This is my first patch to the Linux kernel / first time using an
email-based patch workflow, so I don't know what I'm doing. I hope to
contribute more to BPF in the future, though.
The patches apply fine to me on bpf-next master (am I supposed to be
targeting a different branch?), and I've asked someone else to confirm
too. I've tested with your exact git version. I have a theory about
what might be going wrong for you, see below.
Here's what I'm doing:
# prove I'm on bpf-next master
$ git show | head -n 5
commit 91b2db27d3ff9ad29e8b3108dfbf1e2f49fe9bd3
Author: Song Liu <songliubraving@fb.com>
Date: Thu Nov 19 16:28:33 2020 -0800
bpf: Simplify task_file_seq_get_next()
# Download the patches - these are the "raw" links published by
lore.kernel.org for each of the two emails.
$ curl https://lore.kernel.org/bpf/20201122022205.57229-1-andreimatei1@gmail.com/raw
> p1.patch
$ curl https://lore.kernel.org/bpf/20201122022205.57229-2-andreimatei1@gmail.com/raw
> p2.patch
$ git am p1.patch
Applying: selftest/bpf: fix link in readme
$ git am p2.patch
Applying: selftest/bpf: fix rst formatting in readme
So, it all "works for me". The patches were produced with `git
format-patch` and sent with `git send-email`. Please let me know if I
was supposed to do something else.
With the risk of continuing to not know what I'm talking about, I
perhaps have a guess about why the patches don't apply for you: if you
simply copy-pasted the email into your p2.txt, that might not apply
because a space might be lost from the end of one of the one lines
that I'm deleting. The patch has a line that reads: "-This is due to a
llvm BPF backend bug. The fix ". Notice the space at the end of the
line. At least Gmail doesn't render that space, so if I simply
copy-paste the patch from my browser, I end up with a corrupted line
and so it doesn't apply. Perhaps that's your situation?
Thanks Yonghong, I appreciate your time!
- Andrei
On Mon, Nov 23, 2020 at 2:22 AM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 11/21/20 6:22 PM, Andrei Matei wrote:
> > A couple of places in the readme had invalid rst formatting causing the
> > rendering to be off. This patch fixes them with minimal edits.
> >
> > Signed-off-by: Andrei Matei <andreimatei1@gmail.com>
> > ---
> > tools/testing/selftests/bpf/README.rst | 28 ++++++++++++++------------
> > 1 file changed, 15 insertions(+), 13 deletions(-)
>
> I cannot apply patch #2 with my current bpf-next branch.
>
> -bash-4.4$ git apply ~/p1.txt
> -bash-4.4$ git apply ~/p2.txt
> /home/yhs/p2.txt:34: trailing whitespace.
> __
> https://reviews.llvm.org/D85570
>
> /home/yhs/p2.txt:52: trailing whitespace.
> __
> https://reviews.llvm.org/D78466
>
> /home/yhs/p2.txt:70: trailing whitespace.
> .. _0:
> https://reviews.llvm.org/D74572
>
> /home/yhs/p2.txt:71: trailing whitespace.
> .. _1:
> https://reviews.llvm.org/D74668
>
> /home/yhs/p2.txt:72: trailing whitespace.
> .. _2:
> https://reviews.llvm.org/D85174
>
> error: patch failed: tools/testing/selftests/bpf/README.rst:33
> error: tools/testing/selftests/bpf/README.rst: patch does not apply
> -bash-4.4$ git --version
> git version 2.24.1
> -bash-4.4$
>
> Could you help check what is the issue? Maybe the links are presented
> differently in the patch vs. in the README.rst?
>
> >
> > diff --git a/tools/testing/selftests/bpf/README.rst b/tools/testing/selftests/bpf/README.rst
> > index 3b8d8885892d..ca064180d4d0 100644
> > --- a/tools/testing/selftests/bpf/README.rst
> > +++ b/tools/testing/selftests/bpf/README.rst
> > @@ -33,11 +33,12 @@ The verifier will reject such code with above error.
> > At insn 18 the r7 is indeed unbounded. The later insn 19 checks the bounds and
> > the insn 20 undoes map_value addition. It is currently impossible for the
> > verifier to understand such speculative pointer arithmetic.
> > -Hence
> > - https://reviews.llvm.org/D85570
> > -addresses it on the compiler side. It was committed on llvm 12.
> > +Hence `this patch`__ addresses it on the compiler side. It was committed on llvm 12.
> > +
> > +__ https://reviews.llvm.org/D85570
> >
> > The corresponding C code
> > +
> > .. code-block:: c
> >
> > for (int i = 0; i < MAX_CGROUPS_PATH_DEPTH; i++) {
> > @@ -80,10 +81,11 @@ The symptom for ``bpf_iter/netlink`` looks like
> > 17: (7b) *(u64 *)(r7 +0) = r2
> > only read is supported
> >
> > -This is due to a llvm BPF backend bug. The fix
> > - https://reviews.llvm.org/D78466
> > +This is due to a llvm BPF backend bug. `The fix`__
> > has been pushed to llvm 10.x release branch and will be
> > -available in 10.0.1. The fix is available in llvm 11.0.0 trunk.
> > +available in 10.0.1. The patch is available in llvm 11.0.0 trunk.
> > +
> > +__ https://reviews.llvm.org/D78466
> >
> > BPF CO-RE-based tests and Clang version
> > =======================================
> > @@ -97,11 +99,11 @@ them to Clang/LLVM. These sub-tests are going to be skipped if Clang is too
> > old to support them, they shouldn't cause build failures or runtime test
> > failures:
> >
> > - - __builtin_btf_type_id() ([0], [1], [2]);
> > - - __builtin_preserve_type_info(), __builtin_preserve_enum_value() ([3], [4]).
> > +- __builtin_btf_type_id() [0_, 1_, 2_];
> > +- __builtin_preserve_type_info(), __builtin_preserve_enum_value() [3_, 4_].
> >
> > - [0] https://reviews.llvm.org/D74572
> > - [1] https://reviews.llvm.org/D74668
> > - [2] https://reviews.llvm.org/D85174
> > - [3] https://reviews.llvm.org/D83878
> > - [4] https://reviews.llvm.org/D83242
> > +.. _0: https://reviews.llvm.org/D74572
> > +.. _1: https://reviews.llvm.org/D74668
> > +.. _2: https://reviews.llvm.org/D85174
> > +.. _3: https://reviews.llvm.org/D83878
> > +.. _4: https://reviews.llvm.org/D83242
> >
On 11/24/20 10:29 AM, Andrei Matei wrote: > Hi Yonghong! Thanks for looking at my patch! > This is my first patch to the Linux kernel / first time using an > email-based patch workflow, so I don't know what I'm doing. I hope to > contribute more to BPF in the future, though. Thanks for making contributions to bpf ecosystem! > > The patches apply fine to me on bpf-next master (am I supposed to be > targeting a different branch?), and I've asked someone else to confirm > too. I've tested with your exact git version. I have a theory about > what might be going wrong for you, see below. > > Here's what I'm doing: > > # prove I'm on bpf-next master > $ git show | head -n 5 > commit 91b2db27d3ff9ad29e8b3108dfbf1e2f49fe9bd3 > Author: Song Liu <songliubraving@fb.com> > Date: Thu Nov 19 16:28:33 2020 -0800 > > bpf: Simplify task_file_seq_get_next() > > # Download the patches - these are the "raw" links published by > lore.kernel.org for each of the two emails. > $ curl https://lore.kernel.org/bpf/20201122022205.57229-1-andreimatei1@gmail.com/raw >> p1.patch > $ curl https://lore.kernel.org/bpf/20201122022205.57229-2-andreimatei1@gmail.com/raw >> p2.patch > $ git am p1.patch > Applying: selftest/bpf: fix link in readme > $ git am p2.patch > Applying: selftest/bpf: fix rst formatting in readme > > So, it all "works for me". The patches were produced with `git > format-patch` and sent with `git send-email`. Please let me know if I > was supposed to do something else. > > With the risk of continuing to not know what I'm talking about, I > perhaps have a guess about why the patches don't apply for you: if you > simply copy-pasted the email into your p2.txt, that might not apply > because a space might be lost from the end of one of the one lines > that I'm deleting. The patch has a line that reads: "-This is due to a > llvm BPF backend bug. The fix ". Notice the space at the end of the > line. At least Gmail doesn't render that space, so if I simply > copy-paste the patch from my browser, I end up with a corrupted line > and so it doesn't apply. Perhaps that's your situation? My email client is mozilla thunderbird. I just right click and save the patch email to a file and then try to apply and it failed. I guess this process may not be friendly to http/https links. I tried your above curl method and it works. I will switch to your approach if the patch involves http/https links in the future. Thanks for the detailed procedure! > > Thanks Yonghong, I appreciate your time! You are welcome. I tested your patch with rst rendering. It indeed fixed the formatting. I will ack separately. > > - Andrei > > On Mon, Nov 23, 2020 at 2:22 AM Yonghong Song <yhs@fb.com> wrote: >> >> >> >> On 11/21/20 6:22 PM, Andrei Matei wrote: >>> A couple of places in the readme had invalid rst formatting causing the >>> rendering to be off. This patch fixes them with minimal edits. >>> >>> Signed-off-by: Andrei Matei <andreimatei1@gmail.com> >>> --- >>> tools/testing/selftests/bpf/README.rst | 28 ++++++++++++++------------ >>> 1 file changed, 15 insertions(+), 13 deletions(-) >> >> I cannot apply patch #2 with my current bpf-next branch. >> >> -bash-4.4$ git apply ~/p1.txt >> -bash-4.4$ git apply ~/p2.txt >> /home/yhs/p2.txt:34: trailing whitespace. >> __ >> https://reviews.llvm.org/D85570 >> >> /home/yhs/p2.txt:52: trailing whitespace. >> __ >> https://reviews.llvm.org/D78466 >> >> /home/yhs/p2.txt:70: trailing whitespace. >> .. _0: >> https://reviews.llvm.org/D74572 >> >> /home/yhs/p2.txt:71: trailing whitespace. >> .. _1: >> https://reviews.llvm.org/D74668 >> >> /home/yhs/p2.txt:72: trailing whitespace. >> .. _2: >> https://reviews.llvm.org/D85174 >> >> error: patch failed: tools/testing/selftests/bpf/README.rst:33 >> error: tools/testing/selftests/bpf/README.rst: patch does not apply >> -bash-4.4$ git --version >> git version 2.24.1 >> -bash-4.4$ >> >> Could you help check what is the issue? Maybe the links are presented >> differently in the patch vs. in the README.rst? >> >>> >>> diff --git a/tools/testing/selftests/bpf/README.rst b/tools/testing/selftests/bpf/README.rst >>> index 3b8d8885892d..ca064180d4d0 100644 >>> --- a/tools/testing/selftests/bpf/README.rst >>> +++ b/tools/testing/selftests/bpf/README.rst >>> @@ -33,11 +33,12 @@ The verifier will reject such code with above error. >>> At insn 18 the r7 is indeed unbounded. The later insn 19 checks the bounds and >>> the insn 20 undoes map_value addition. It is currently impossible for the >>> verifier to understand such speculative pointer arithmetic. >>> -Hence >>> - https://reviews.llvm.org/D85570 >>> -addresses it on the compiler side. It was committed on llvm 12. >>> +Hence `this patch`__ addresses it on the compiler side. It was committed on llvm 12. >>> + >>> +__ https://reviews.llvm.org/D85570 >>> >>> The corresponding C code >>> + >>> .. code-block:: c >>> >>> for (int i = 0; i < MAX_CGROUPS_PATH_DEPTH; i++) { >>> @@ -80,10 +81,11 @@ The symptom for ``bpf_iter/netlink`` looks like >>> 17: (7b) *(u64 *)(r7 +0) = r2 >>> only read is supported >>> >>> -This is due to a llvm BPF backend bug. The fix >>> - https://reviews.llvm.org/D78466 >>> +This is due to a llvm BPF backend bug. `The fix`__ >>> has been pushed to llvm 10.x release branch and will be >>> -available in 10.0.1. The fix is available in llvm 11.0.0 trunk. >>> +available in 10.0.1. The patch is available in llvm 11.0.0 trunk. >>> + >>> +__ https://reviews.llvm.org/D78466 >>> >>> BPF CO-RE-based tests and Clang version >>> ======================================= >>> @@ -97,11 +99,11 @@ them to Clang/LLVM. These sub-tests are going to be skipped if Clang is too >>> old to support them, they shouldn't cause build failures or runtime test >>> failures: >>> >>> - - __builtin_btf_type_id() ([0], [1], [2]); >>> - - __builtin_preserve_type_info(), __builtin_preserve_enum_value() ([3], [4]). >>> +- __builtin_btf_type_id() [0_, 1_, 2_]; >>> +- __builtin_preserve_type_info(), __builtin_preserve_enum_value() [3_, 4_]. >>> >>> - [0] https://reviews.llvm.org/D74572 >>> - [1] https://reviews.llvm.org/D74668 >>> - [2] https://reviews.llvm.org/D85174 >>> - [3] https://reviews.llvm.org/D83878 >>> - [4] https://reviews.llvm.org/D83242 >>> +.. _0: https://reviews.llvm.org/D74572 >>> +.. _1: https://reviews.llvm.org/D74668 >>> +.. _2: https://reviews.llvm.org/D85174 >>> +.. _3: https://reviews.llvm.org/D83878 >>> +.. _4: https://reviews.llvm.org/D83242 >>>
On 11/21/20 6:22 PM, Andrei Matei wrote: > A couple of places in the readme had invalid rst formatting causing the > rendering to be off. This patch fixes them with minimal edits. > > Signed-off-by: Andrei Matei <andreimatei1@gmail.com> Acked-by: Yonghong Song <yhs@fb.com>
diff --git a/tools/testing/selftests/bpf/README.rst b/tools/testing/selftests/bpf/README.rst index 3b8d8885892d..ca064180d4d0 100644 --- a/tools/testing/selftests/bpf/README.rst +++ b/tools/testing/selftests/bpf/README.rst @@ -33,11 +33,12 @@ The verifier will reject such code with above error. At insn 18 the r7 is indeed unbounded. The later insn 19 checks the bounds and the insn 20 undoes map_value addition. It is currently impossible for the verifier to understand such speculative pointer arithmetic. -Hence - https://reviews.llvm.org/D85570 -addresses it on the compiler side. It was committed on llvm 12. +Hence `this patch`__ addresses it on the compiler side. It was committed on llvm 12. + +__ https://reviews.llvm.org/D85570 The corresponding C code + .. code-block:: c for (int i = 0; i < MAX_CGROUPS_PATH_DEPTH; i++) { @@ -80,10 +81,11 @@ The symptom for ``bpf_iter/netlink`` looks like 17: (7b) *(u64 *)(r7 +0) = r2 only read is supported -This is due to a llvm BPF backend bug. The fix - https://reviews.llvm.org/D78466 +This is due to a llvm BPF backend bug. `The fix`__ has been pushed to llvm 10.x release branch and will be -available in 10.0.1. The fix is available in llvm 11.0.0 trunk. +available in 10.0.1. The patch is available in llvm 11.0.0 trunk. + +__ https://reviews.llvm.org/D78466 BPF CO-RE-based tests and Clang version ======================================= @@ -97,11 +99,11 @@ them to Clang/LLVM. These sub-tests are going to be skipped if Clang is too old to support them, they shouldn't cause build failures or runtime test failures: - - __builtin_btf_type_id() ([0], [1], [2]); - - __builtin_preserve_type_info(), __builtin_preserve_enum_value() ([3], [4]). +- __builtin_btf_type_id() [0_, 1_, 2_]; +- __builtin_preserve_type_info(), __builtin_preserve_enum_value() [3_, 4_]. - [0] https://reviews.llvm.org/D74572 - [1] https://reviews.llvm.org/D74668 - [2] https://reviews.llvm.org/D85174 - [3] https://reviews.llvm.org/D83878 - [4] https://reviews.llvm.org/D83242 +.. _0: https://reviews.llvm.org/D74572 +.. _1: https://reviews.llvm.org/D74668 +.. _2: https://reviews.llvm.org/D85174 +.. _3: https://reviews.llvm.org/D83878 +.. _4: https://reviews.llvm.org/D83242
A couple of places in the readme had invalid rst formatting causing the rendering to be off. This patch fixes them with minimal edits. Signed-off-by: Andrei Matei <andreimatei1@gmail.com> --- tools/testing/selftests/bpf/README.rst | 28 ++++++++++++++------------ 1 file changed, 15 insertions(+), 13 deletions(-)