On Tue, Mar 21, 2023 at 02:01:59PM -0300, Leonardo Bras Soares Passos wrote: > On Tue, Mar 21, 2023 at 4:49 AM Conor Dooley wrote: > > On Tue, Mar 21, 2023 at 03:30:35AM -0300, Leonardo Brás wrote: > > > On Sun, 2023-03-19 at 20:35 +0000, Conor Dooley wrote: > > > > On Sat, Mar 18, 2023 at 05:00:54AM -0300, Leonardo Bras wrote: > > > > > While studying riscv's cmpxchg.h file, I got really interested in > > > > > understanding how RISCV asm implemented the different versions of > > > > > {cmp,}xchg. > > > > > > > > > > When I understood the pattern, it made sense for me to remove the > > > > > duplications and create macros to make it easier to understand what exactly > > > > > changes between the versions: Instruction sufixes & barriers. > > > > > > > > > > I split those changes in 3 levels for each cmpxchg and xchg, resulting a > > > > > total of 6 patches. I did this so it becomes easier to review and remove > > > > > the last levels if desired, but I have no issue squashing them if it's > > > > > better. > > > > > > > > > > Please provide comments. > > > > > > > > > > Thanks! > > > > > Leo > > > > > > > > > > Leonardo Bras (6): > > > > > riscv/cmpxchg: Deduplicate cmpxchg() asm functions > > > > > riscv/cmpxchg: Deduplicate cmpxchg() macros > > > > > riscv/cmpxchg: Deduplicate arch_cmpxchg() macros > > > > > > > > > riscv/cmpxchg: Deduplicate xchg() asm functions > > > > > > > > FWIW, this patch seems to break the build pretty badly: > > > > https://patchwork.kernel.org/project/linux-riscv/patch/20230318080059.1109286-5-leobras@redhat.com/ > > > > > > Thanks for pointing out! > > > > > > It was an intermediary error: > > > Sufix for amoswap on acquire version was "d.aqrl" instead of the > > > correct".d.aqrl", and that caused the fail. > > > > > > I did not notice anything because the next commit made it more general, and thus > > > removed this line of code. I will send a v2-RFC shortly. > > > > > > I see that patch 4/6 has 5 fails, but on each one of them I can see: > > > "I: build output in /ci/workspace/[...]", or > > > ""I: build output in /tmp/[...]". > > > > I don't push the built artifacts anywhere, just the brief logs - > > although the "failed to build" log isn't very helpful other than telling > > you the build broke. > > That seems like a bug w.r.t. where tuxmake prints its output. I'll try > > to fix that. > > Thanks for that :) I've pushed what I think is a fix, the wrong log file was being grepped for errors in the case of a failed build. > > > I could not find any reference to where this is saved, though. > > > Could you point where can I find the error output, just for the sake of further > > > debugging? > > > > > > > > > > > Patches 1 & 5 also add quite a lot of sparse issues (like 1000), but I > > > > think that may be more of an artifact of the testing process as opposed > > > > to something caused by this patchset. > > > > > > For those I can see the build output diffs. Both added error lines to > > > conchuod/build_rv64_gcc_allmodconfig. > > > I tried to mimic this with [make allmodconfig + gcc build with > > > CROSS_COMPILE=riscv64-linux-gnu- ] but I could not get any error in any patch. > > > > If you can't replicate them, it's probably because they come from > > sparse. I only really mentioned it here in case you went looking at the > > other patches in the series and were wondering why things were so amiss. > > Oh, it makes sense. > > > > > > For patch 1/6 it removes as much error lines (-) as it adds (+), and the error > > > messages are mostly the same, apart for an line number. > > > > I don't see a line number difference, but rather an increase in the > > number of times the same error lines have appeared in the output. > > I don't find the single-line output from sparse to really be very > > helpful, I should probably have a bit of a think how to present that > > information better. > > Oh, I see. > The number at the beginning relates to the number of times the error happens. > Ok it makes sense to me now :) > > > > > > For patch 5/6 it actually adds many more lines, but tracking (some of) the > > > errors gave me no idea why. > > > > Probably just sparse being unhappy with some way the macros were > > changed - but some of it ("Should it be static?" bits) look very much > > like the patch causing things to be rebuilt only for the "after the > > patch" build, but somehow not for the "before" build. > > Humm, not sure I could understand that last part: > What I get is that the patch 5/6 is causing things to be rebuilt, and > it was not like that on 1-4/6. > Is that what you said? > If so, and you are doing it as an incremental build, changing the > macros in 5/6 should be triggering rebuilds, but it does not make > sense to not be rebuilt in 1-4/6 as they change the same macros. Right, it is an incremental build. First it builds the tree with a patch applied, then it checks out HEAD~1 and builds that tree. Then it goes back to the tree with the patch applied and builds it again. The output from builds 2 & 3 are compared to see if any errors snuck in. In theory, that should ensure that, as builds 2 & 3 have had the same diff to the previous one just in opposite directions, the same files get rebuilt - but I am a little worried that ccache gets involved sometimes and leads to the before/after builds not being exactly the same. They may very well be real issues as your refactor has caused the casting in the macros to change or w/e. Not my area of expertise by any stretch of the imagination, but the lkp sparse is out of date & doesn't run any more so I figure it's better to be running the stuff, even if it does sometimes result in a splurge of errors like this than forget about poor aul sparse. Cheers, Conor.