Re: [PATCH v2 00/10] x86: Clean up percpu operations
On Mon, Jul 13, 2020 at 5:31 PM Nick Desaulniers wrote: > > So I did: Ack, I have the series, and it looks fine to me. > Actually, looks like a lot of merged PRs come from github! Grizzly > Adams *did* have a beard! Oh, I think github is an _excellent_ hosting service. I just can't stand their web workflow for creating commits and merging code. At least at some point it was really easy to generate totally horrible commit messages using the web interface without the proper header line etc. I _think_ that got fixed. But the merge codeflow still doesn't work at all for the kernel. (To be fair, I've used it for _other_ projects, and there it can work really really well, together with all the test infrastructure etc). They also have a very spammy email system where people can add me to their projects and "invite" me, and I get email about it. But again - as a hosting site for kernel pulls, it's one of the better ones. So you definitely don't need a kernel.org account to send me pull requests, github is in fact much better than some others (infradead is very slow to pull for me, for example). A kernel.org pull doesn't strictly need a signed tag - I do kernel.org pulls over ssh, and I trust the user security there. Any other hosting service I _do_ require that the pull requests are proper signed tags, though. But even that difference is largely gone: for kernel.org pulls I've very heavily favored signed tags, and I think almost all of them are using them by now. Linus
Re: [PATCH v2 00/10] x86: Clean up percpu operations
On Mon, Jul 13, 2020 at 3:58 PM Nick Desaulniers wrote: > > On Mon, Jul 13, 2020 at 3:40 PM Linus Torvalds > wrote: > > > > On Mon, Jul 13, 2020 at 3:24 PM Nick Desaulniers > > wrote: > > > > > > Otherwise, is there any additional feedback on this series or is it good > > > to go? > > > > I've lost sight of the series. I'm sure it is fine, but maybe you can > > resend it to me (in private, if it's already been going out on the > > mailing lists and everybody else is completely fed up with it). > > Is there a fast way that maintainters amend ACKs to each commit in a series? For future travelers (more so myself, since I don't sync my shell history between machines, and I'm a big fan of aggressively sharing knowledge. See also the section "Information as Power" and the anecdote about tank manuals: https://www.meforum.org/441/why-arabs-lose-wars). `b4` has a pretty cool feature. When I was fetching this series, it was warning: ``` NOTE: Some trailers were sent to the cover letter: Tested-by: Nick Desaulniers Acked-by: Peter Zijlstra (Intel) Reviewed-by: Nick Desaulniers NOTE: Rerun with -t to apply them to all patches ``` So I did: ``` $ b4 am -t https://lore.kernel.org/lkml/cakwvodmsap8ib+h5jxihywsjfrtq_krjnh7eqcge7p-ljk7...@mail.gmail.com/T/\#t -o - | git am $ git filter-branch -f --msg-filter 'cat - && echo "Signed-off-by: Nick Desaulniers "" $@";' HEAD~10..HEAD > > > > > And no, pointing to the "plus this hunk" with a web link isn't what I > > was looking for ;) > > So you're not accepting pull requests yet on github? I jest. Actually, looks like a lot of merged PRs come from github! Grizzly Adams *did* have a beard! https://www.youtube.com/watch?v=pdwJC9HvKLU Sent as a series of emails via: $ git format-patch -o linus_i386 HEAD~11 $ git send-email --to="Linus Torvalds " --suppress-cc=all linus_i386 Though, I'm sure a pull request would have been more formal. -- Thanks, ~Nick Desaulniers
Re: [PATCH v2 00/10] x86: Clean up percpu operations
On Mon, Jul 13, 2020 at 3:40 PM Linus Torvalds wrote: > > On Mon, Jul 13, 2020 at 3:24 PM Nick Desaulniers > wrote: > > > > Otherwise, is there any additional feedback on this series or is it good to > > go? > > I've lost sight of the series. I'm sure it is fine, but maybe you can > resend it to me (in private, if it's already been going out on the > mailing lists and everybody else is completely fed up with it). Is there a fast way that maintainters amend ACKs to each commit in a series? > > And no, pointing to the "plus this hunk" with a web link isn't what I > was looking for ;) So you're not accepting pull requests yet on github? I jest. -- Thanks, ~Nick Desaulniers
Re: [PATCH v2 00/10] x86: Clean up percpu operations
On Mon, Jul 13, 2020 at 3:24 PM Nick Desaulniers wrote: > > Otherwise, is there any additional feedback on this series or is it good to > go? I've lost sight of the series. I'm sure it is fine, but maybe you can resend it to me (in private, if it's already been going out on the mailing lists and everybody else is completely fed up with it). And no, pointing to the "plus this hunk" with a web link isn't what I was looking for ;) Linus
Re: [PATCH v2 00/10] x86: Clean up percpu operations
On Wed, Jul 8, 2020 at 12:36 PM Nick Desaulniers wrote: > > On Mon, Jun 1, 2020 at 2:00 PM Nick Desaulniers > wrote: > > > > On Sat, May 30, 2020 at 3:11 PM Brian Gerst wrote: > > > > > > The core percpu operations already have a switch on the width of the > > > data type, which resulted in an extra amount of dead code being > > > generated with the x86 operations having another switch. This patch set > > > rewrites the x86 ops to remove the switch. Additional cleanups are to > > > use named assembly operands, and to cast variables to the width used in > > > the assembly to make Clang happy. > > > > Thanks for all of the work that went into this series. I think I've > > reviewed all of them. > > With this series plus this hunk: > > https://github.com/ClangBuiltLinux/continuous-integration/blob/master/patches/llvm-all/linux-next/x86/x86-support-i386-with-Clang.patch#L219-L237 > > I can build and boot i386_defconfig with Clang! So for the series: > > > > Reviewed-by: Nick Desaulniers > > Tested-by: Nick Desaulniers > > tglx, Ingo, Boris, Linus, > Do you all have thoughts on this series? I can understand "let > sleeping dogs lie" but some Android folks are really interested in > i386 testing, and randconfigs/allnoconfigs are doing i386 builds which > are currently broken w/ Clang. This series gets us closer to having > test coverage of this ISA with another toolchain, FWIW. I'm trying to organize an LLVM micro conference at plumbers. I think a session on "clang and remaining i386 blockers" might be of interest, which would cover why the existing code pattern is problematic for compiler portability. This series in particular would be brought up. Are you all planning on attending plumbers this year? Might such a session be of interest? Otherwise, is there any additional feedback on this series or is it good to go? -- Thanks, ~Nick Desaulniers
Re: [PATCH v2 00/10] x86: Clean up percpu operations
On Wed, Jul 8, 2020 at 12:36 PM Nick Desaulniers wrote: > > On Mon, Jun 1, 2020 at 2:00 PM Nick Desaulniers > wrote: > > > > On Sat, May 30, 2020 at 3:11 PM Brian Gerst wrote: > > > > > > The core percpu operations already have a switch on the width of the > > > data type, which resulted in an extra amount of dead code being > > > generated with the x86 operations having another switch. This patch set > > > rewrites the x86 ops to remove the switch. Additional cleanups are to > > > use named assembly operands, and to cast variables to the width used in > > > the assembly to make Clang happy. > > > > Thanks for all of the work that went into this series. I think I've > > reviewed all of them. > > With this series plus this hunk: > > https://github.com/ClangBuiltLinux/continuous-integration/blob/master/patches/llvm-all/linux-next/x86/x86-support-i386-with-Clang.patch#L219-L237 > > I can build and boot i386_defconfig with Clang! So for the series: > > > > Reviewed-by: Nick Desaulniers > > Tested-by: Nick Desaulniers > > tglx, Ingo, Boris, Linus, > Do you all have thoughts on this series? I can understand "let > sleeping dogs lie" but some Android folks are really interested in > i386 testing, and randconfigs/allnoconfigs are doing i386 builds which > are currently broken w/ Clang. This series gets us closer to having > test coverage of this ISA with another toolchain, FWIW. Oh, was https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6ec4476ac825 alluding to this, perchance? -- Thanks, ~Nick Desaulniers
Re: [PATCH v2 00/10] x86: Clean up percpu operations
On Sat, May 30, 2020 at 06:11:17PM -0400, Brian Gerst wrote: > The core percpu operations already have a switch on the width of the > data type, which resulted in an extra amount of dead code being > generated with the x86 operations having another switch. This patch set > rewrites the x86 ops to remove the switch. Additional cleanups are to > use named assembly operands, and to cast variables to the width used in > the assembly to make Clang happy. > > Changes from v1: > - Add separate patch for XADD constraint fix > - Fixed sparse truncation warning > - Add cleanup of percpu_stable_op() > - Add patch to Remove PER_CPU() > > Brian Gerst (10): > x86/percpu: Introduce size abstraction macros > x86/percpu: Clean up percpu_to_op() > x86/percpu: Clean up percpu_from_op() > x86/percpu: Clean up percpu_add_op() > x86/percpu: Remove "e" constraint from XADD > x86/percpu: Clean up percpu_add_return_op() > x86/percpu: Clean up percpu_xchg_op() > x86/percpu: Clean up percpu_cmpxchg_op() > x86/percpu: Clean up percpu_stable_op() > x86/percpu: Remove unused PER_CPU() macro > > arch/x86/include/asm/percpu.h | 510 -- > 1 file changed, 172 insertions(+), 338 deletions(-) Nice! Acked-by: Peter Zijlstra (Intel)
Re: [PATCH v2 00/10] x86: Clean up percpu operations
On Mon, Jun 1, 2020 at 2:00 PM Nick Desaulniers wrote: > > On Sat, May 30, 2020 at 3:11 PM Brian Gerst wrote: > > > > The core percpu operations already have a switch on the width of the > > data type, which resulted in an extra amount of dead code being > > generated with the x86 operations having another switch. This patch set > > rewrites the x86 ops to remove the switch. Additional cleanups are to > > use named assembly operands, and to cast variables to the width used in > > the assembly to make Clang happy. > > Thanks for all of the work that went into this series. I think I've > reviewed all of them. > With this series plus this hunk: > https://github.com/ClangBuiltLinux/continuous-integration/blob/master/patches/llvm-all/linux-next/x86/x86-support-i386-with-Clang.patch#L219-L237 > I can build and boot i386_defconfig with Clang! So for the series: > > Reviewed-by: Nick Desaulniers > Tested-by: Nick Desaulniers tglx, Ingo, Boris, Linus, Do you all have thoughts on this series? I can understand "let sleeping dogs lie" but some Android folks are really interested in i386 testing, and randconfigs/allnoconfigs are doing i386 builds which are currently broken w/ Clang. This series gets us closer to having test coverage of this ISA with another toolchain, FWIW. -- Thanks, ~Nick Desaulniers
Re: [PATCH v2 00/10] x86: Clean up percpu operations
On Sat, May 30, 2020 at 3:11 PM Brian Gerst wrote: > > The core percpu operations already have a switch on the width of the > data type, which resulted in an extra amount of dead code being > generated with the x86 operations having another switch. This patch set > rewrites the x86 ops to remove the switch. Additional cleanups are to > use named assembly operands, and to cast variables to the width used in > the assembly to make Clang happy. Thanks for all of the work that went into this series. I think I've reviewed all of them. With this series plus this hunk: https://github.com/ClangBuiltLinux/continuous-integration/blob/master/patches/llvm-all/linux-next/x86/x86-support-i386-with-Clang.patch#L219-L237 I can build and boot i386_defconfig with Clang! So for the series: Reviewed-by: Nick Desaulniers Tested-by: Nick Desaulniers > > Changes from v1: > - Add separate patch for XADD constraint fix > - Fixed sparse truncation warning > - Add cleanup of percpu_stable_op() > - Add patch to Remove PER_CPU() > > Brian Gerst (10): > x86/percpu: Introduce size abstraction macros > x86/percpu: Clean up percpu_to_op() > x86/percpu: Clean up percpu_from_op() > x86/percpu: Clean up percpu_add_op() > x86/percpu: Remove "e" constraint from XADD > x86/percpu: Clean up percpu_add_return_op() > x86/percpu: Clean up percpu_xchg_op() > x86/percpu: Clean up percpu_cmpxchg_op() > x86/percpu: Clean up percpu_stable_op() > x86/percpu: Remove unused PER_CPU() macro > > arch/x86/include/asm/percpu.h | 510 -- > 1 file changed, 172 insertions(+), 338 deletions(-) > > > base-commit: 229aaa8c059f2c908e0561453509f996f2b2d5c4 > -- > 2.25.4 > -- Thanks, ~Nick Desaulniers
[PATCH v2 00/10] x86: Clean up percpu operations
The core percpu operations already have a switch on the width of the data type, which resulted in an extra amount of dead code being generated with the x86 operations having another switch. This patch set rewrites the x86 ops to remove the switch. Additional cleanups are to use named assembly operands, and to cast variables to the width used in the assembly to make Clang happy. Changes from v1: - Add separate patch for XADD constraint fix - Fixed sparse truncation warning - Add cleanup of percpu_stable_op() - Add patch to Remove PER_CPU() Brian Gerst (10): x86/percpu: Introduce size abstraction macros x86/percpu: Clean up percpu_to_op() x86/percpu: Clean up percpu_from_op() x86/percpu: Clean up percpu_add_op() x86/percpu: Remove "e" constraint from XADD x86/percpu: Clean up percpu_add_return_op() x86/percpu: Clean up percpu_xchg_op() x86/percpu: Clean up percpu_cmpxchg_op() x86/percpu: Clean up percpu_stable_op() x86/percpu: Remove unused PER_CPU() macro arch/x86/include/asm/percpu.h | 510 -- 1 file changed, 172 insertions(+), 338 deletions(-) base-commit: 229aaa8c059f2c908e0561453509f996f2b2d5c4 -- 2.25.4
