include/asm-x86: checkpatch cleanups - formatting only
Re: [PATCH 109/148] include/asm-x86/serial.h: checkpatch cleanups - formatting only
In reply to David MillerQuoted 6 lines · David Miller
There are mountains of more useful stuff to be working on (much of it automated, but unlike checkpatch work doesn't result in crap) rather than 148 patches of checkpatch vomit.
Fixing sparse warnings properly fixes real issues, whereas fixing checkpatch stuff creates garbage 9 times out of 10.
Yes, I agree with you in this.
What I don't agree with is that it's useless. It may help track down some issues in yet prepared patch (it's checkpatch, not checkcode and it should be used in that manner).
Re: [PATCH 109/148] include/asm-x86/serial.h: checkpatch cleanups - formatting only
From: Jiri Slaby <[email protected]>
Date: Sun, 23 Mar 2008 13:24:23 +0100
Quoted 5 lines · David Miller
Quoted 4 lines
There are mountains of more useful stuff to be working on (much of it automated, but unlike checkpatch work doesn't result in crap) rather than 148 patches of checkpatch vomit.
Quoted 8 lines
Quoted 2 lines
Fixing sparse warnings properly fixes real issues, whereas fixing checkpatch stuff creates garbage 9 times out of 10.
Yes, I agree with you in this.
What I don't agree with is that it's useless. It may help track down some issues in yet prepared patch (it's checkpatch, not checkcode and it should be used in that manner).
I strongly disagree still.
Half the warnings I get when I have run checkpatch on things I've written were crap.
It's an automaton, and that's why people like it. It tells you exactly what to do, and people like to be able to turn their brains off like that.
checkpatch [was: include/asm-x86/serial.h: checkpatch cleanups - formatting only]
Quoted 10 lines · David Miller
From: Jiri Slaby <[email protected]>
Date: Sun, 23 Mar 2008 13:24:23 +0100Quoted 7 lines · David Miller
Quoted 6 lines
There are mountains of more useful stuff to be working on (much of it automated, but unlike checkpatch work doesn't result in crap) rather than 148 patches of checkpatch vomit.
Fixing sparse warnings properly fixes real issues, whereas fixing checkpatch stuff creates garbage 9 times out of 10.
Quoted 8 lines
Quoted 3 lines
Yes, I agree with you in this.
What I don't agree with is that it's useless. It may help track down some issues in yet prepared patch (it's checkpatch, not checkcode and it should be used in that manner).
I strongly disagree still.
Half the warnings I get when I have run checkpatch on things I've written were crap.
Could you be more concrete here? I often get only "more than 80 columns used blah blah" in the header files and this sucks, yes. This check removal was discussed some time ago on the list, seems like the result was to let it be.
Quoted 3 lines
It's an automaton, and that's why people like it. It tells you exactly what to do, and people like to be able to turn their brains off like that.
It just spits out warnings/errors like compiler or some static analyzer, maybe I'm terribly missing something, what exactly do you mind on the output?
Re: checkpatch [was: include/asm-x86/serial.h: checkpatch cleanups - formatting only]
Quoted 4 lines · Jiri Slaby
It just spits out warnings/errors like compiler or some static analyzer, maybe I'm terribly missing something, what exactly do you mind on the output?
A good example are the trailing white space warnings in there. They are just useless and a waste of time. That is something that really should just be done automatically by maintainer scripts somewhere on merge (and a lot of maintainers do it automatically), not waste the valuable time of a human.
There are also a couple of other pointless warnings. e.g. I dislike some of the style warnings -- they are far too broad. Or there can be good reasons to violate them occasionally. e.g. I would rather put not a space around an operator occasionally instead of splitting an expression to avoid going over 80 characers. Or the EXPORT_SYMBOL warning. WTF is that good for? Or the warning about if () ... else { ... }.
Also checkpatch.pl --file seems to be a good candidate for one of the worst ideas ever merged (to be fair it was difficult to see it would become that bad in advance -- but it got)
On the other hand a few of the pattern matches are ful.
To fix the worst problems of checkpatch.pl one could probably do something like this:
- Get rid of --file
- Drop at least 50% of the style warnings (or make them not trigger unless a special option is given) and review all the others.
- Concentrate on the really useful things like checking for deprecated functions and pattern matching for a few common mistakes.
-Andi
Re: checkpatch [was: include/asm-x86/serial.h: checkpatch cleanups - formatting only]
Quoted 10 lines · Andi Kleen
Quoted 3 lines · Jiri Slaby
It just spits out warnings/errors like compiler or some static analyzer, maybe I'm terribly missing something, what exactly do you mind on the output?
A good example are the trailing white space warnings in there. They are just useless and a waste of time. That is something that really should just be done automatically by maintainer scripts somewhere on merge (and a lot of maintainers do it automatically), not waste the valuable time of a human.
Agreed.
Quoted 4 lines
There are also a couple of other pointless warnings. e.g. I dislike
some of the style warnings -- they are far too broad. Or there can be good reasons to violate them occasionally. e.g. I would rather put not a space around an operator occasionally instead
of splitting an expression to avoid going over 80 characers.
Me too, there will ever be certain circumstances where the rules just doesn't apply and you can't teach the scripts in that way (turning off doubtful warnings sounds reasonable anyway).
Quoted 1 line
Or the
EXPORT_SYMBOLwarning. WTF is that good for? Or the warning about if () ... else { ... }.
Understand you points, but most of it is strict implementation of CodingStyle. People, who don't prefer coding style as described in that document, won't like checkpatch either.
Quoted 3 lines
Also checkpatch.pl --file seems to be a good candidate for one of the worst ideas ever merged (to be fair it was difficult to see it would become that bad in advance -- but it got)
Definitely agree.
Re: checkpatch [was: include/asm-x86/serial.h: checkpatch cleanups - formatting only]
Quoted 4 lines · Jiri Slaby
Understand you points, but most of it is strict implementation of CodingStyle. People, who don't prefer coding style as described in that document, won't like checkpatch either.
I used to like CodingStyle. Recently I've started to have doubts. Basically, the file used to contain roughly this paraphrase:
"I know coding style is highly personly, but when possible I'd prefer to
receive patches in this style..."
Over time the particular style has been refined more and more and the tone of the document became a little more like:
"And the Lord sayeth, thou shalt not diverge from His style."
And that is just going too far. Keeping the style roughly similar is useful. Several of the rules in CodingStyle are about writing code in a way that avoids bugs. Clearly useful. But if you go too far you end in religious zealotry.
Finding the right balance between following common rules and bending them where appropriate is important - here as much as anywhere.
Jörn
Signature · 3 lines
The grand essentials of happiness are: something to do, something to love, and something to hope for. -- Allan K. Chalmers
Re: checkpatch [was: include/asm-x86/serial.h: checkpatch cleanups - formatting only]
In reply to Andi KleenQuoted 3 lines · Andi Kleen
Also checkpatch.pl --file seems to be a good candidate for one of the worst ideas ever merged (to be fair it was difficult to see it would become that bad in advance -- but it got)
I have found checkpatch --file useful when taking large amounts of out of tree code (a kernel arch subdir and a number of drivers) and trying to get them into better shape for inclusion. In that situation it's quite a good way of seeing how far off that goal I am. Running it on existing in-tree code is more problematic.
Re: [PATCH 109/148] include/asm-x86/serial.h: checkpatch cleanups - formatting only
In reply to David MillerQuoted 4 lines · David Miller
I strongly disagree still.
Half the warnings I get when I have run checkpatch on things I've written were crap.
could you please give me a file name as an example that i could double-check myself? Thanks,
Ingo
Re: [PATCH 109/148] include/asm-x86/serial.h: checkpatch cleanups - formatting only
From: Ingo Molnar <[email protected]>
Date: Tue, 25 Mar 2008 09:44:57 +0100
Quoted 9 lines · David Miller
Quoted 5 lines
I strongly disagree still.
Half the warnings I get when I have run checkpatch on things I've written were crap.
could you please give me a file name as an example that i could double-check myself? Thanks,
I can't because I pacified it to cut down the review noise for the patch in question last time it happened.
I can tell you one more example of things I strongly disagree with that it does, for example, such as telling me how to document spinlocks in datastructures.
It wants a comment right above the spinlock_t member, but this totally ignores that perhaps I put a huge comment explaining the locking semantics elsewhere.
It's a black and white tool in a grey world, it just sucks. And I'd be fine with that if people used it as a guide but people, especially kernel newbies, treat it as gospel and a way to contribe "useful" patches. They aren't useful, they're crap. Fix a bug instead of this automaton whitespace noise.
Re: [PATCH 109/148] include/asm-x86/serial.h: checkpatch cleanups - formatting only
Quoted 7 lines · David Miller
I can tell you one more example of things I strongly disagree with that it does, for example, such as telling me how to document spinlocks in datastructures.
It wants a comment right above the
spinlock_tmember, but this totally ignores that perhaps I put a huge comment explaining the locking semantics elsewhere.
firstly, this warning from checkpatch.pl is off by default.
There are 3 checkpatch warning categories: ERROR, WARNING, CHECK. spinlock_t without a warning is in this third category and you wont even see that warning unless you very explicitly do:
checkpatch.pl --subjective
Secondly, even about this "checkpatch.pl --subjective" check you are wrong. As someone who had to decode (way!) too many lockdep backtraces in various kernel code that i didnt author and didnt maintain, i can tell it you with very strong authority that even in this case it's a minimum requirement to put a comment right to that lock:
/*
* Regarding the locking rules, see the big comment block above in
* this file:
*/
or:
/* See net/core/sock.c for the locking rules: */
_Way_ too many times do i have to wonder where the heck a given lock is documented. You _wrote_ and maintain a good portion of that code, so to you it's seemingly an annoyance and nuisance. To everyone else, it's must-have information. Locks are at the heart of kernel data structures, not having at least a minimal pointer at them is really bad.
(sidenote: the scheduler has one deficiency there and i've fixed it in my tree. this warning should be moved from the 'check' category into the warning category.)
Ingo
Re: [PATCH 109/148] include/asm-x86/serial.h: checkpatch cleanups - formatting only
In reply to David MillerQuoted 5 lines · David Miller
Quoted 2 lines
could you please give me a file name as an example that i could double-check myself? Thanks,
I can't because I pacified it to cut down the review noise for the patch in question last time it happened.
ok, so i'll have to come up with some hard data about code you maintain i guess - please provide contrary data if you have it.
Using the "code-quality" script mentioned at:
http://people.redhat.com/mingo/x86.git/README
on the Sparc64 tree:
$ code-quality `find arch/sparc64/ include/asm-sparc64/ -name '*.c'` |
tee ~/quality-sparc.txt
$ sort -n -k 4 ~/quality-sparc.txt | tail -20shows the following "20 worst files":
errors lines of code errors/KLOC
arch/sparc64/kernel/sys_sparc32.c 48 1034 46.4 arch/sparc64/kernel/signal32.c 65 1392 46.6 arch/sparc64/kernel/sys_sunos32.c 73 1360 53.6 arch/sparc64/solaris/misc.c 43 787 54.6 arch/sparc64/solaris/socket.c 30 462 64.9 arch/sparc64/kernel/binfmt_aout32.c 34 420 80.9 arch/sparc64/oprofile/init.c 2 24 83.3 arch/sparc64/solaris/fs.c 64 746 85.7 arch/sparc64/prom/devops.c 4 42 95.2 arch/sparc64/prom/console.c 8 75 106.6 arch/sparc64/solaris/ioctl.c 89 826 107.7 arch/sparc64/kernel/cpu.c 17 155 109.6 arch/sparc64/solaris/ipc.c 19 127 149.6 arch/sparc64/solaris/timod.c 157 977 160.6 arch/sparc64/kernel/sunos_ioctl32.c 49 276 177.5 arch/sparc64/solaris/signal.c 81 430 188.3 arch/sparc64/solaris/socksys.c 39 204 191.1 arch/sparc64/prom/tree.c 58 300 193.3 arch/sparc64/math-emu/math.c 215 514 418.2 arch/sparc64/boot/piggyback.c 47 110 427.2
most of these are certainly legacy stuff that dont matter to active maintenance anymore (nobody will ever change math-emu/math.c i hope), but for example looking at:
scripts/checkpatch.pl --file arch/sparc64/kernel/cpu.c [...]
total: 17 errors, 2 warnings, 154 lines checked
all 17 errors and both warnings are legitimate complaints and if i was maintaining that file i'd accept any patch that cleans those problems up, in a heartbeat.
And today's mainstream files might become tomorrow's legacy files. To come up with a hypothetical example: had you applied checkpatch.pl on all changes to this file in the past 15 years (checkpatch.pl only exists for less than 5 years so this is hypothetical), you'd have a squeaky-clean file and no need for any annoying "monkey patches".
or take a look at the 65 errors that arch/sparc64/kernel/signal32.c produces:
total: 65 errors, 33 warnings, 1391 lines checked
i've just checked all of the 65 errors and all look legitimate at first sight and should be fixed.
even looking at "best of breed" arch/sparc64 code, arch/sparc64/mm/*.c gives 34 errors:
scripts/checkpatch.pl --file arch/sparc64/mm/*.c | grep ERROR | wc -l 34
all of which seem legitimate complaints.
so the score right now: checkpatch versus DaveM 116:0 ;-)
i'm sure there are false positives in it too, but in my experience (from the scheduler and from arch/x86) there's less than 1 false positive for every 100 legitimate errors that checkpatch.pl it finds. So for the 1500 errors reported by checkpatch.pl for the whole sparc64 tree:
errors lines of code errors/KLOC
arch/sparc64/ 1460 49801 29.3
i estimate that there will be less than 15 false positive "ERROR" lines, and my random sample of about 100 errors confirms that rate. That's a diminishing rate IMO.
as a comparison, core kernel code has this 'style quality':
errors lines of code errors/KLOC
kernel/ 727 100364 7.2
in my experience, subsystems (of any significant size - i.e. above a few KLOC) where errors/KLOC is "single digit", has very clean code in general. Subsystems that have low-double-digit errors/KLOC are OK-ish, subsystems with high-double-digit or triple-digit errors/KLOC are messy.
There can be fluctuations and artifacts, and obviously this is just another (arbitrary) static metric that has no forced relationship with real code quality - but in my experience it's surprisingly close to reality - closer than any other code metric i've seen.
Ingo
Re: [PATCH 109/148] include/asm-x86/serial.h: checkpatch cleanups - formatting only
From: Ingo Molnar <[email protected]>
Date: Tue, 25 Mar 2008 14:17:08 +0100
Quoted 4 lines
There can be fluctuations and artifacts, and obviously this is just another (arbitrary) static metric that has no forced relationship with real code quality - but in my experience it's surprisingly close to reality - closer than any other code metric i've seen.
And yet you used it to claim that the sparc64 port is an unmaintainable pile of poo.
Thanks but no thanks, you just proved even more to me why checkpatch is crap.
Re: [PATCH 109/148] include/asm-x86/serial.h: checkpatch cleanups - formatting only
Quoted 10 lines · David Miller
From: Ingo Molnar <[email protected]>
Date: Tue, 25 Mar 2008 14:17:08 +0100Quoted 4 lines
There can be fluctuations and artifacts, and obviously this is just another (arbitrary) static metric that has no forced relationship with real code quality - but in my experience it's surprisingly close to reality - closer than any other code metric i've seen.
And yet you used it to claim that the sparc64 port is an unmaintainable pile of poo.
i did not claim that at all, the checkpatch average of sparc64 is pretty good:
errors lines of code errors/KLOC
arch/sparc64/ 1457 49785 29.2
Sparc64 is one of the cleanest architectures, in terms of average checkpatch.pl code-quality.
the list i generated is the '20 worst files' (out of 90 sparc64 files) those are the files in need of cleanups (according to that metric) - and a quick manual glance confirmed that impression.
Or is it your position that every single file in sparc64 is squeaky clean and that each of the 1457 cleanliness problems that checkpatch.pl reported there is bogus?
Ingo
Re: [PATCH 109/148] include/asm-x86/serial.h: checkpatch cleanups - formatting only
From: Ingo Molnar <[email protected]>
Date: Wed, 26 Mar 2008 11:25:14 +0100
Quoted 3 lines
Or is it your position that every single file in sparc64 is squeaky clean and that each of the 1457 cleanliness problems that checkpatch.pl reported there is bogus?
No but last time I checked sparc64 was one of the few arch's that build themselves with -Werror.
Re: [PATCH 109/148] include/asm-x86/serial.h: checkpatch cleanups - formatting only
In reply to David MillerQuoted 10 lines · David Miller
Quoted 2 lines
I disagree. It's just misuse in this case (like using Lindent on whole tree).
Unlike sparse, this thing encourages the kind of behavior seen here.
And even worse it becomes monkey see monkey do.
There are mountains of more useful stuff to be working on (much of it automated, but unlike checkpatch work doesn't result in crap) rather than 148 patches of checkpatch vomit.
Joe should not have spammed lkml like this - but let me take up the general issue of checkpatch that you touch upon - even if the incident at hand puts checkpatch into an unfavorable light.
Quoted 2 lines
Fixing sparse warnings properly fixes real issues, whereas fixing checkpatch stuff creates garbage 9 times out of 10.
actually, my experience has been that 99% of the arch/x86 sparse annotations dont fix any "real" issue but remove a warning. The remaining 1% still very much makes it worth though (they can prevent a security hole, a bad bug, endianness incompatibility, etc.), so we've taken a large amount of sparse annotations in arch/x86 in the past few months - while fixing exactly zero "real" bugs in the process.
Same goes for checkpatch: almost no individual checkpatch change _looks_ worthwile in isolation, but the combined effect very much shows and avoids real bugs. (While Sparse is 'type functional' and checkpatch is 'style only' - still both avoid real bugs - see below why.)
Lets consider the "end result" that we are aiming for. One example of a "checkpatch clean" codebase is the scheduler (kernel/sched*.[ch]):
$ code-quality kernel/sched*.[ch]
errors lines of code errors/KLOC
kernel/sched.c 0 8490 0 kernel/sched_debug.c 0 402 0 kernel/sched_fair.c 0 1475 0 kernel/sched_idletask.c 0 130 0 kernel/sched_rt.c 0 1341 0 kernel/sched_stats.h 0 238 0
The value of a "checkpatch clean" codebase is significant to me as a maintainer. No matter where i look at the (rather sizable) scheduler codebase, the style is uniform and changes all look the same and are much easier to review. Since 2.6.22 we've managed to do about 500 changes to this 10 KLOC codebase, with a very low regression rate - that is more than 10 times the rate of change of the kernel as a whole.
We couldnt achieve that without broad "visual uniformity". Human vision is based on pattern matching, which brain capacity has a physical limit. Reducing the complexity of that process, and helping the "flow of eye movement during review" is just as important as to clean up the logic of code - it gives us better chances to find a bad bug during review.
We here on lkml are all quite good at filtering out unnecessary visual noise when reviewing patches and writing code, but i prefer to reserve that brain capacity towards understanding the code and finding mistakes in it.
So i minimize all visual distractions in my physical work environment (i optimize the field of view i have at the monitor), i minimize visual distractions in the editor i use (no GUI for example, just plain fullscreen text view with no borders, no menus, etc.), and an important part of that is that i also minimize all unnecessary distractions in the _code_ itself that i maintain.
But if you look at the git log of the scheduler in of the past 5 months, you'll see a striking lack of trivial, checkpatch generated "monkey" patches.
Why? Because all the patches that are applied are checkpatch-clean from the get go, so there's no need for trivialities. There were certainly some checkpatch "trivialities" early in the process (despite the scheduler being very clean to begin with), but the transients have subsided meanwhile and what we have is a squeaky-clean codebase in action. In this model of maintenance, checkpatch ends up being just a 'background force' that never truly comes to the surface to bother us with explicit trivialities. In other words: there's _zero_ room for "monkey patches".
Note that there are no "problems to development patches" caused by scheduler cleanups either - because there are essentially _no_ cleanup patches at all in the scheduler - almost all patches we apply to the scheduler are clean.
arch/x86 is on a similar path:
errors LOC err/KLOC
-----------------------------
v2.6.24-rc1 arch/x86/ 8695 117423 74.0 v2.6.24-x86.git arch/x86/ [21 Nov 2007] 5190 117156 44.2 v2.6.24-x86.git arch/x86/ [18 Dec 2007] 4057 117213 34.6 v2.6.24-x86.git arch/x86/ [ 8 Jan 2008] 3650 117987 30.9 v2.6.24-x86.git arch/x86/ [ 4 Feb 2008] 3334 133542 24.9 v2.6.25-x86.git arch/x86/ [21 Feb 2008] 2724 136963 19.8 v2.6.25-x86.git arch/x86/ [ 1 Mar 2008] 2155 136404 15.7 v2.6.25-x86.git arch/x86/ [21 Mar 2008] 1979 137205 14.4
and once we reach a "zero" state, the flow of "explicit" checkpatch patches comes to a virtual standstill - just like it did for the scheduler. And we broke up the "please dont do this to my outstanding development patches" Catch-22 (which is also a way too easy place for lazy developers to hide behind) by doing backports/forward ports along more intrusive cleanups.
On a more conceptual angle: "coding style", despite being entirely "non-functional" (it does not affect the generated code), is still very much an integral part of the code because source code is fundamentally about "knowledge" - and extra style noise in knowledge can never possibly increase the quality of that knowledge. There are strong links between code correctness and typography/aesthetics.
So, in the specific example of the scheduler subsystem, i've only observed advantages to checkpatch and zero downsides. Could anyone give me _any_ objective reason why i shouldnt be using checkpatch for the scheduler? More broadly, could anyone give me an objective reason why we shouldnt be doing it for arch/x86? And even more broadly, could anyone give me an objective reason why we shouldnt be doing it for all actively maintained areas of the kernel?
Ingo
Re: [PATCH 109/148] include/asm-x86/serial.h: checkpatch cleanups - formatting only
Quoted 8 lines · Ingo Molnar
So, in the specific example of the scheduler subsystem, i've only observed advantages to checkpatch and zero downsides. Could anyone give me _any_ objective reason why i shouldnt be using checkpatch for the scheduler? More broadly, could anyone give me an objective reason why we shouldnt be doing it for
arch/x86? And even more broadly, could anyone give me an objective reason why we shouldnt be doing it for all actively maintained areas of the kernel?
Disagreement between checkpatch and maintainers preferred style. I've had a patch that fixed a bug and - while in the region - "cleaned up" the style for a single line. This line no longer matches the rest of the file and creates the kind of visual distraction you complain about.
In short, for a file with an active maintainer whatever the maintainer prefers should be done. Doing a full checkpatch sweep against a maintainers wishes is madness, doing a partial "cleanup" is worse. Of course when a maintainer likes checkpatch, as you do, there is no disagreement to deal with. :)
Jörn
Signature · 2 lines
I don't understand it. Nobody does. -- Richard P. Feynman
Re: [PATCH 109/148] include/asm-x86/serial.h: checkpatch cleanups - formatting only
Quoted 10 lines · Jörn Engel
Quoted 8 lines · Ingo Molnar
So, in the specific example of the scheduler subsystem, i've only observed advantages to checkpatch and zero downsides. Could anyone give me _any_ objective reason why i shouldnt be using checkpatch for the scheduler? More broadly, could anyone give me an objective reason why we shouldnt be doing it for
arch/x86? And even more broadly, could anyone give me an objective reason why we shouldnt be doing it for all actively maintained areas of the kernel?
Quoted 5 lines
Disagreement between checkpatch and maintainers preferred style. I've had a patch that fixed a bug and - while in the region - "cleaned up" the style for a single line. This line no longer matches the rest of the file and creates the kind of visual distraction you complain about.
well, once a subsystem is "checkpatch-clean" (which my challenge above obviously assumes), there's no such "partial style" problem. It obviously also assumes that the maintainer agrees that having consistent coding style across all of Linux is a long-term advantage.
The current visual inconsistency between subsystems makes the Linux kernel appear rather unpleasant and unprofessional to new kernel developers. This is not just embarrasing to us (we want to write the best OS on the planet), it is also actively harmful: such a "consistent style does not matter" stance turns away people who have taste and tends to attract people who have no taste - which i'm sure you'll agree with results in a deadly spiral if it gets strong enough.
So to turn around the argument: could you give me any reason why differing coding style between subsystems, _often in blatant violation of Documentation/CodingStyle_, is somehow "good" for Linux in the long run? I listed numerous first-hand advantages that style consistency brings and i listed numerous disadvantages created by inconsistency. So i'm waiting for the list of counter-arguments - there _must_ be some objective ones, besides the obvious "kernel old-timers are lazy to change their ways" argument =B-)
In my experience in almost all cases the "style disagreement" between a subsystem maintainer and checkpatch is due to the _maintainer_ being wrong about seemingly unimportant (to him) style details.
And that very much includes myself: i used to have such "disagreement" with checkpatch errors and i used to be annoyed at the style nitpicking of checkpatch. And yes, it takes a certain amount of time for a long-time kernel hacker like me to realize that a lot of code i wrote in the past needs a good clean-up ;-)
These style differences are certainly not "wrong enough" to inconvenience or displace an active maintainer (and i never made that point), but they are nevertheless a death by a thousand cuts that the general kernel is suffering from right now, and i'd be a fool not to point it out. These seemingly unimportant style details add up to a hodgepodge of coding style that makes life difficult for people who have to look at many different parts of the kernel that they _dont maintain_.
That's why i asked about specific examples that we can talk about - and i gave specific examples and filenames. The checkpatch maintainer (Andy Whitcroft) has certainly shown flexibility to fix false positives ASAP.
Quoted 2 lines
Of course when a maintainer likes checkpatch, as you do, there is no disagreement to deal with. :)
note, all those patches are "Subject: x86: " and 99% of them is maintained by us x86 maintainers.
Ingo
Re: [PATCH 109/148] include/asm-x86/serial.h: checkpatch cleanups - formatting only
Quoted 8 lines · Ingo Molnar
The current visual inconsistency between subsystems makes the Linux kernel appear rather unpleasant and unprofessional to new kernel developers. This is not just embarrasing to us (we want to write the best OS on the planet), it is also actively harmful: such a "consistent style does not matter" stance turns away people who have taste and tends to attract people who have no taste - which i'm sure you'll agree with results in a deadly spiral if it gets strong enough.
I disagree with that assertion. My favorite example of where CodingStyle has gone too far is this:
for (i=0; i<10; i++)
While the official document demands four extra spaces, I _hate_ them. Whitespace offers visual grouping. The lack of whitespace around the binary operators emphasizes that one kind of grouping is stonger than another. Ever since this binary operator testament was added to our Holy Canon, I started violating the coding style on purpose. Imo this is beyond silly.
So do I have bad taste and should I leave the kernel in favor of someone else with better taste that is currently turned away by me? Maybe. Show me that person and I'll consider gardening. Until then I'll continue to violate the style. Just to spite the fundamentalist movement.
Quoted 8 lines
So to turn around the argument: could you give me any reason why differing coding style between subsystems,
_oftenin blatant violation ofDocumentation/CodingStyle_, is somehow "good" for Linux in the long run? I listed numerous first-hand advantages that style consistency brings and i listed numerous disadvantages created by inconsistency. So i'm waiting for the list of counter-arguments - there _must_ be some objective ones, besides the obvious "kernel old-timers are lazy to change their ways" argument =B-)
When you reject useful patches based on "this is not our preferred style", you piss people off. That is a significant reason why people choose to spend their time elsewhere. In certain cases having people abandon the kernel may be a net gain, in many it is a loss.
So unless you are willing to maintain every single driver in the kernel, pissing all the maintainers off that happen to disagree with the canonical style - if only in detail - is not a good recipe.
Don't get me wrong, I certainly see advantages in checkpatch and keeping the style consistent. But there are limits, where the gains no longer justify the cost. And the limits will never be clearly defined. Are some variable names better than others - sure. Can you write a rule for checkpatch to ensure good names - hardly. And yet, variable names are part of the style.
Quoted 3 lines
These style differences are certainly not "wrong enough" to inconvenience or displace an active maintainer (and i never made that point)
It seems we both agree then. And for the record, your mail could easily be interpreted as if you had made that point. Thanks for clarifying things.
Jörn
Signature · 2 lines
I don't understand it. Nobody does. -- Richard P. Feynman
Re: [PATCH 109/148] include/asm-x86/serial.h: checkpatch cleanups - formatting only
Quoted 11 lines · Jörn Engel
Quoted 8 lines
The current visual inconsistency between subsystems makes the Linux kernel appear rather unpleasant and unprofessional to new kernel developers. This is not just embarrasing to us (we want to write the best OS on the planet), it is also actively harmful: such a "consistent style does not matter" stance turns away people who have taste and tends to attract people who have no taste - which i'm sure you'll agree with results in a deadly spiral if it gets strong enough.
I disagree with that assertion. My favorite example of where CodingStyle has gone too far is this:
Quoted 7 lines
for (i=0; i<10; i++)While the official document demands four extra spaces, I _hate_ them. Whitespace offers visual grouping. The lack of whitespace around the binary operators emphasizes that one kind of grouping is stonger than another. Ever since this binary operator testament was added to our Holy Canon, I started violating the coding style on purpose. Imo this is beyond silly.
you picked an borderline case without showing the full effects of your choice of style - but still even in this example you are wrong i believe. Look at how inconsistent this looks:
for (i=0; i<10; i++) {
l = 10;
if (k <= 10)
k = 11;
}(the inconsistent 'i=0' versus 'l = 10')
so in your style we'd have to write it as:
for (i=0; i<10; i++) {
l=10;
if (k<=10)
k=11;
}which, on one hand, looks unprofessional (in fixed width font), but on the other hand, the literals and operators are way too "close" to each other and operators are easily missed and mis-identified visually - causing bugs.
For example the 'k' and the '<=' operator may look visually similar and can be "blended", making it easy to skip over 'k=10' versus 'k<=10' - while 'k = 10' clearly stands apart from 'k <= 10'. [and syntax highlighting does not help with this particular problem]
in Documentation/CodingStyle it looks:
for (i = 0; i < 10; i++) {
l = 10;
if (k <= 10)
k = 11;
}which is certainly reasonable and groups safely.
yes - you can have arguments one way or another, but there's nothing worse than maintainers each going towards their own _arbitrary_ and often clearly inferior coding style, which is inconsistent within the same file.
I at least make the point that i'm trying to converge to Documentation/CodingStyle. Is it arbitrary? Yes, to a fair degree, but it certainly conveys a very strong, unambiguous sense of taste.
Ingo
Re: [PATCH 109/148] include/asm-x86/serial.h: checkpatch cleanups - formatting only
Quoted 3 lines · Ingo Molnar
you picked an borderline case without showing the full effects of your choice of style - but still even in this example you are wrong i believe. [...]
and let me give an example with the your very own code that you wrote and maintain, drivers/mtd/devices/block2mtd.c:
errors lines of code errors/KLOCdrivers/mtd/devices/block2mtd.c 10 490 20.4
that's pretty OK code, but not perfect, the 10 errors are:
ERROR: do not use C99 // comments
ERROR: need spaces around that '=' (ctx:VxV)
ERROR: need spaces around that '<' (ctx:VxV)
ERROR: do not use C99 // comments
ERROR: do not use C99 // comments
ERROR: do not use C99 // comments
ERROR: do not use C99 // comments
ERROR: do not use C99 // comments
ERROR: do not use C99 // comments
ERROR: do not initialise statics to 0 or NULL
so just because you disagreed with those 2 errors that relate to '=' and '<' (and where accoding to CodingStyle checkpatch.pl is correct), you disregarded the other 8 very valid complaints that checkpatch.pl had. (the final one even negatively affects the size of the kernel)
and this is the experience i made in general: the checkpatch.pl benefits far outweigh the costs, even if you disagree with a particular rule of checkpatch.pl. When you came to Linux you already had to change your coding style quite radically, correct?
Ingo
Re: [PATCH 109/148] include/asm-x86/serial.h: checkpatch cleanups - formatting only
Quoted 9 lines · Ingo Molnar
and let me give an example with the your very own code that you wrote and maintain,
drivers/mtd/devices/block2mtd.c:errors lines of code errors/KLOC
drivers/mtd/devices/block2mtd.c10 490 20.4that's pretty OK code, but not perfect, the 10 errors are:
Quoted 10 lines
ERROR: do not use C99 // comments
ERROR: need spaces around that '=' (ctx:VxV)
ERROR: need spaces around that '<' (ctx:VxV)
ERROR: do not use C99 // comments
ERROR: do not use C99 // comments
ERROR: do not use C99 // comments
ERROR: do not use C99 // comments
ERROR: do not use C99 // comments
ERROR: do not use C99 // comments
ERROR: do not initialise statics to 0 or NULL
The last should and will be fixed. The // I don't really care about. Send a patch if you do.
Going over my logfs patch, I found several things that are either false positives or rather questionable in my book. <adds Andy to Cc:>
(foo*) should be (foo *)
What does that extra space gain us?
ERROR: no space before that close parenthesis ')'
#2565: FILE: fs/logfs/gc.c:294:
+ seg_ofs + sizeof(oh) < super->s_segsize; ) {
Actual code is this:
for (seg_ofs = LOGFS_SEGMENT_HEADERSIZE;
seg_ofs + sizeof(oh) < super->s_segsize; ) {The for() loop is missing one of its three terms. I assume this is one of the effects of not having a perfect C parser.
ERROR: trailing statements should be on next line
#5000: FILE: fs/logfs/readwrite.c:203:
+ } else while (unlikely(TestSetPageLocked(page))) {
We have an explicit exception for "else if". "else while" makes imo just as much (or as little) sense.
ERROR: need space after that ',' (ctx:VxV)
#5801: FILE: fs/logfs/readwrite.c:1004:
+ ret = logfs_segment_read(inode, ipage, this_wc->ofs, bix,level);
One of those examples where a missing space is the lesser of two or three evils.
ERROR: need consistent spacing around '|' (ctx:WxV) #8293: FILE: fs/logfs/dev_mtd.c:376:
+ |SLAB_MEM_SPREAD|SLAB_PANIC),
I have no idea what this is about. Original code:
mtd_cache = kmem_cache_create("mtd_cache", sizeof(struct mtd_inode), 0,
(SLAB_HWCACHE_ALIGN|SLAB_RECLAIM_ACCOUNT
|SLAB_MEM_SPREAD|SLAB_PANIC),
Jörn
Signature · 2 lines
Victory in war is not repetitious. -- Sun Tzu
Re: [PATCH 109/148] include/asm-x86/serial.h: checkpatch cleanups - formatting only
Quoted 24 lines · Jörn Engel
Quoted 9 lines · Ingo Molnar
and let me give an example with the your very own code that you wrote and maintain,
drivers/mtd/devices/block2mtd.c:errors lines of code errors/KLOC
drivers/mtd/devices/block2mtd.c10 490 20.4that's pretty OK code, but not perfect, the 10 errors are:
Quoted 10 lines
ERROR: do not use C99 // comments
ERROR: need spaces around that '=' (ctx:VxV)
ERROR: need spaces around that '<' (ctx:VxV)
ERROR: do not use C99 // comments
ERROR: do not use C99 // comments
ERROR: do not use C99 // comments
ERROR: do not use C99 // comments
ERROR: do not use C99 // comments
ERROR: do not use C99 // comments
ERROR: do not initialise statics to 0 or NULLThe last should and will be fixed. The // I don't really care about. Send a patch if you do.
Quoted 6 lines
Going over my logfs patch, I found several things that are either false positives or rather questionable in my book. <adds Andy to Cc:>
(foo*) should be (foo *)
What does that extra space gain us?
It really gains us nothing, however that is not really the point. The point is that consistancy is good, with the space is the more normal 'C' usage, without for 'C++'; something to do with the implication that (foo *) is a pointer to a foo (separate things), and (foo*) is a thing of type pointer to foo (one thing) which is more object oriented.
The "norm" is with and so it makes sense to maintain it that way. A lot of the layout and style choises are arbitrary, and disliked by many of us, but we follow the style to maintain that common feel.
Quoted 10 lines
ERROR: no space before that close parenthesis ')'
#2565: FILE:fs/logfs/gc.c:294:
+seg_ofs+ sizeof(oh) <super->s_segsize; ) {Actual code is this:
for (seg_ofs = LOGFS_SEGMENT_HEADERSIZE; seg_ofs + sizeof(oh) < super->s_segsize; ) {The
for()loop is missing one of its three terms. I assume this is one of the effects of not having a perfect C parser.
Yes that is a false positive. I'll have a look at fixing it.
Quoted 6 lines
ERROR: trailing statements should be on next line
#5000: FILE:fs/logfs/readwrite.c:203:
+ } else while (unlikely(TestSetPageLocked(page))) {We have an explicit exception for "else if". "else while" makes imo just as much (or as little) sense.
"else if" is at least creating an additional arm of the same control structure. else while is mixing two different paradigms.
Quoted 6 lines
ERROR: need space after that ',' (ctx:VxV)
#5801: FILE:fs/logfs/readwrite.c:1004:
+ ret =logfs_segment_read(inode, ipage, this_wc->ofs, bix,level);One of those examples where a missing space is the lesser of two or three evils.
Because to add the space would mean breaking the line to avoid exceeding 80 characters?
Quoted 3 lines
ERROR: need consistent spacing around '|' (ctx:WxV) #8293: FILE:
fs/logfs/dev_mtd.c:376:
+ |SLAB_MEM_SPREAD|SLAB_PANIC),
That is a false positive triggered by the ' |S' at the start of
the line. This one is fixed in the head of my tree. The more normal form would be for that leading | to be on the end of the previous line, and the exception for that was already there.
Quoted 5 lines
I have no idea what this is about. Original code:
mtd_cache= kmem_cache_create("mtd_cache",sizeof(struct mtd_inode), 0,
(SLAB_HWCACHE_ALIGN|SLAB_RECLAIM_ACCOUNT
|SLAB_MEM_SPREAD|SLAB_PANIC),
-apw
Re: [PATCH 109/148] include/asm-x86/serial.h: checkpatch cleanups - formatting only
Quoted 11 lines · Andy Whitcroft
Quoted 3 lines · Jörn Engel
(foo*) should be (foo *)
What does that extra space gain us?It really gains us nothing, however that is not really the point. The point is that consistancy is good, with the space is the more normal 'C' usage, without for 'C++'; something to do with the implication that (foo *) is a pointer to a foo (separate things), and (foo*) is a thing of type pointer to foo (one thing) which is more object oriented.
Quoted 3 lines
The "norm" is with and so it makes sense to maintain it that way. A lot of the layout and style choises are arbitrary, and disliked by many of us, but we follow the style to maintain that common feel.
Then I'll happily ignore it. Not having the space gains me one column. It is absolutely minimal, sure. But when the alternative is based on pure whim...
Quoted 9 lines
Quoted 6 lines
ERROR: trailing statements should be on next line
#5000: FILE:fs/logfs/readwrite.c:203:
+ } else while (unlikely(TestSetPageLocked(page))) {We have an explicit exception for "else if". "else while" makes imo just as much (or as little) sense.
"else if" is at least creating an additional arm of the same control structure. else while is mixing two different paradigms.
Fairly weak grounds to argue on. Not that mine are much stronger, I just default to less (shorter, fewer indentations, etc.) when lacking a reason to use more (characters, lines, indentations, etc.).
Quoted 9 lines
Quoted 6 lines
ERROR: need space after that ',' (ctx:VxV)
#5801: FILE:fs/logfs/readwrite.c:1004:
+ ret =logfs_segment_read(inode, ipage, this_wc->ofs, bix,level);One of those examples where a missing space is the lesser of two or three evils.
Because to add the space would mean breaking the line to avoid exceeding 80 characters?
Or breaking the line. Either of those choices sucks. Well, breaking the line is often the lesser of those evils, but in this particular function it looks worse to me - and I have to stare at it often enough to care.
The best strategy usually is to rethink the code and reduce the indentation, number of arguments or length of identifiers. I just don't see a good way of doing that without resorting to
ret = logfs_segment_read(i, p, w->o, b, l);
Probably nothing checkpatch should worry about. Although I would have been happy to have finer-grained options to enable/disable particular warnings on the command line. Right now I commented out several lines in checkpatch.pl.
Jörn
Signature · 2 lines
Joern's library part 7: http://www.usenix.org/publications/library/proceedings/neworl/full_papers/mckusick.a
Re: [PATCH 109/148] include/asm-x86/serial.h: checkpatch cleanups - formatting only
Quoted 10 lines · Jörn Engel
Quoted 10 lines
Quoted 2 lines
(foo*) should be (foo *)
What does that extra space gain us?It really gains us nothing, however that is not really the point. The point is that consistancy is good, with the space is the more normal 'C' usage, without for 'C++'; something to do with the implication that (foo *) is a pointer to a foo (separate things), and (foo*) is a thing of type pointer to foo (one thing) which is more object oriented.
Quoted 7 lines
Quoted 3 lines
The "norm" is with and so it makes sense to maintain it that way. A lot of the layout and style choises are arbitrary, and disliked by many of us, but we follow the style to maintain that common feel.
Then I'll happily ignore it. Not having the space gains me one column. It is absolutely minimal, sure. But when the alternative is based on pure whim...
you seem to be confused here. Consistency is not a 'whim', and it is often just about "arbitrary" choices that also have some ergonomic component as well (but sometimes not). The following form:
if(x)
{
MyFunction1();
MyFunction2();
}can be argued to be just as clean (and just as ergonomic) as:
if (x) {
my_function_1();
my_function_2();
}if you use it consistently throughout the _whole_ codebase.
what does not make sense is to _mix_ different coding styles, especially within the same source code block - which you do in that specific file.
or is it your argument that consistent coding style is bad? That argument has been settled long ago with the creation of Documentation/CodingStyle. All kernel code is supposed to follow that style, unless the resulting line of code looks clearly _wrong_. Your arguments seem to center around "hey, my way it looks similarly good so i'll do that instead because i'm the maintainer" - and that argument does not fly. CodingStyle is definitely not gospel and common sense should be applied, but _arbitrarily_ and _intentionally_ deviating from it is considered bad manners and hurts Linux as a whole.
granted, especially in the driver space, where there's a lack of maintenance resources, all such secondary rules are weaker - because a somewhat quirky maintainer is still much better to Linux than no maintainer. But the closer you get to the core kernel, the higher the code quality stakes get, and the stricter (and the more consistent) these style requirements become as well.
Ingo
Re: [PATCH 109/148] include/asm-x86/serial.h: checkpatch cleanups - formatting only
Quoted 10 lines · Ingo Molnar
Quoted 4 lines · Jörn Engel
Then I'll happily ignore it. Not having the space gains me one column. It is absolutely minimal, sure. But when the alternative is based on pure whim...
you seem to be confused here. Consistency is not a 'whim', and it is often just about "arbitrary" choices that also have some ergonomic component as well (but sometimes not). The following form:
I have an objective reason to prefer one over the other. Your only reason is consistency. If consistency was everything, we might as well let the often-abused 1000 monkeys type up a coding style document and stick to that.
But most of our rules exist for reasons beyond mere consistency. Following the rules, on average, makes the code better. In particular the "less" rules (fewer lines, less indentation, etc.) result in more code fitting any arbitrary editor window. Which means more control flow our mind can ponder about without scrolling.
Do you have a non-consistency based reason to prefer the longer version? If not, then we should settle on the short version, which does have a minimal advantage.
Jörn
Signature · 2 lines
Joern's library part 13: http://www.chip-architect.com/
Re: [PATCH 109/148] include/asm-x86/serial.h: checkpatch cleanups - formatting only
From: Jörn Engel <[email protected]>
Date: Wed, 26 Mar 2008 12:41:42 +0100
Quoted 2 lines
Do you have a non-consistency based reason to prefer the longer version?
Inconstent spacing fools people's eyes and leads to bugs, more often than not.
After 15 years of kernel development, I can remember at least 10 or so multi-week-debugging sessions that could have been curtailed had I not mis-read a poorly spaced C statement.
It matters in practical terms, not just consistency terms, trust me.
Re: [PATCH 109/148] include/asm-x86/serial.h: checkpatch cleanups - formatting only
Quoted 7 lines · David Miller
Quoted 3 lines
Do you have a non-consistency based reason to prefer the longer version?
Inconstent spacing fools people's eyes and leads to bugs, more often than not.
Quoted 7 lines
After 15 years of kernel development, I can remember at least 10 or so multi-week-debugging sessions that could have been curtailed had I not mis-read a poorly spaced C statement.
It matters in practical terms, not just consistency terms, trust me.
Sure, I buy that. What I'm arguing here is why we have to be consistently long instead of consistently short. CodingStyle seems to be silent on the question. And a quick grep shows that while being in the minority, I seem to be in a sizeable minority:
joern@Dublin:/usr/src/kernel/logfs$ sgrep ' \*)' .|wc 51687 346050 4213259 joern@Dublin:/usr/src/kernel/logfs$ sgrep '[^ ]\*)' .|wc 5838 33165 472462
What is the reason why (void *)foo is better than (void*)foo? Just that fact that by random chance one them them became more common in our codebase and the minority always has to give in?
Jörn
Signature · 2 lines
A victorious army first wins and then seeks battle. -- Sun Tzu
Re: [PATCH 109/148] include/asm-x86/serial.h: checkpatch cleanups - formatting only
From: Jörn Engel <[email protected]>
Date: Wed, 26 Mar 2008 12:58:53 +0100
Quoted 3 lines
Sure, I buy that. What I'm arguing here is why we have to be consistently long instead of consistently short. CodingStyle seems to be silent on the question.
Becauseitmoreeasilyallowsyoureyestoseethedifferentoperators.
Re: [PATCH 109/148] include/asm-x86/serial.h: checkpatch cleanups - formatting only
Quoted 6 lines · David Miller
Quoted 4 lines
Sure, I buy that. What I'm arguing here is why we have to be consistently long instead of consistently short. CodingStyle seems to be silent on the question.
Becauseitmoreeasilyallowsyoureyestoseethedifferentoperators.
How many different operators are there in a cast? ;)
But you seem to be arguing the i=0 case. Took me a while to notice that. Fair enough.
Jörn
Signature · 4 lines
Maintenance in other professions and of other articles is concerned with the return of the item to its original state; in Software, maintenance is concerned with moving an item away from its original state. -- Les Belady
Re: [PATCH 109/148] include/asm-x86/serial.h: checkpatch cleanups - formatting only
In reply to Jörn EngelQuoted 3 lines · Jörn Engel
What is the reason why (void *)foo is better than (void*)foo? Just that fact that by random chance one them them became more common in our codebase and the minority always has to give in?
It doesn't matter. This is the crux of all interminable coding style discussions. There is no objectively best coding style. Pick one, stick to it. That's all that matters.
Linux happens to have a particular coding style, GNU has another. When I hack Linux code I use one style, when I hack GNU coee I use another. Arguing about it is a waste of time.
Re: [PATCH 109/148] include/asm-x86/serial.h: checkpatch cleanups - formatting only
In reply to Jörn EngelHi,
[Some CCs dropped. I don't think they're truly interested.]
On 3/26/08, Jörn Engel <[email protected]> wrote:> What is the reason why (void *)foo is better than (void*)foo? Just that> fact that by random chance one them them became more common in our> codebase and the minority always has to give in? I think the rationale for using (void *) over (void*) is that weusually write a space before the * in other places (again, it's aboutconsistency). Or do you also write void*a; in variable declarations?
Vegard���{.n�+�������+%�����ݶ��w��{.n�+����{��G�����{ay� ʇڙ�,j��f���h��������z_�(�階�ݢj"���m������G������?����&���~��iO��z��v�^�m���� ������?�I�
PATCH 1/1 bkl2mtd: cleanup
In reply to Jörn EngelQuoted 4 lines · Jörn Engel
Quoted 1 line
ERROR: do not initialise statics to 0 or NULL
The last should and will be fixed. The // I don't really care about. Send a patch if you do.
sure, find clean-up patch below. (Some of the changes are for just tiny nuances, not mentioned in CodingStyle nor flagged by checkpatch)
Ingo
------------------------>
Subject: bkl2mtd: cleanup
From: Ingo Molnar <[email protected]>
Date: Wed Mar 26 10:40:47 CET 2008
Before:
total: 10 errors, 4 warnings, 488 lines checked
After:
total: 0 errors, 0 warnings, 497 lines checked
No code changed, except the small shrink due to the block2mtd_init_called initialization change:
drivers/mtd/devices/block2mtd.o:
text data bss dec hex filename 3097 16 124 3237 ca5 block2mtd.o.before 3093 16 124 3233 ca1 block2mtd.o.after
md5 changes only due to the different section that the block2mtd_init_called got into:
bacbb932ec90b514f8adf654afa29b44 block2mtd.o.before.asm 05e1252749f7294f45c79c44554ec6ba block2mtd.o.after.asm
--- drivers/mtd/devices/block2mtd.c | 151 +++++++++++++++++++++------------------- 1 file changed, 80 insertions(+), 71 deletions(-)
drivers/mtd/devices/block2mtd.c +80 -71
@@ -1,6 +1,4 @@/* -* $Id: block2mtd.c,v 1.30 2005/11/29 14:48:32 gleixner Exp $ -* * block2mtd.c - create an mtd from a block device * * Copyright (C) 2001,2002 Simon Evans <[email protected]>
@@ -20,7 +18,7 @@#include <linux/mutex.h> #include <linux/mount.h> -#define VERSION "$Revision: 1.30 $" +#define VERSION "Revision: 1.30" #define ERROR(fmt, args...) printk(KERN_ERR "block2mtd: " fmt "\n" , ## args)
@@ -29,13 +27,13 @@/* Info for the block device */ struct block2mtd_dev { -struct list_head list; -struct block_device *blkdev; -struct mtd_info mtd; -struct mutex write_mutex; +struct list_head list; +struct block_device *blkdev; +struct mtd_info mtd; +/* serializes writes with each other and also with erase: */ +struct mutex write_mutex; }; -/* Static info about the MTD, used in cleanup_module */ static LIST_HEAD (blkmtd_device_list);
@@ -49,11 +47,11 @@ static struct page *page_read(struct addstatic int _block2mtd_erase (struct block2mtd_dev *dev,loff_t to,size_t len) {struct address_space *mapping = dev->blkdev->bd_inode->i_mapping; -struct page *page; -int index = to >>PAGE_SHIFT ;// page index +int index = to >>PAGE_SHIFT ;/* page index */ int pages = len >>PAGE_SHIFT ; -u_long *p; +struct page *page;u_long *max; +u_long *p;while (pages) { page =page_read (mapping, index);
@@ -63,7 +61,7 @@ static int _block2mtd_erase(struct blockreturn PTR_ERR (page); max =page_address (page) +PAGE_SIZE ; -for (p=page_address (page); p<max; p++) +for (p =page_address (page); p < max; p++) {if (*p !=-1UL ) {lock_page (page);memset (page_address (page),0xff ,PAGE_SIZE );
@@ -71,6 +69,7 @@ static int _block2mtd_erase(struct blockunlock_page (page);break ; } + }page_cache_release (page); pages--;
@@ -78,6 +77,7 @@ static int _block2mtd_erase(struct block }return 0 ; } +static int block2mtd_erase (struct mtd_info *mtd,struct erase_info *instr) {struct block2mtd_dev *dev = mtd->priv;
@@ -86,17 +86,21 @@ static int block2mtd_erase(struct mtd_inint err; instr->state =MTD_ERASING ; +mutex_lock (&dev->write_mutex); err =_block2mtd_erase (dev, from, len);mutex_unlock (&dev->write_mutex); +if (err) {ERROR ("erase failed err = %d" , err); instr->state =MTD_ERASE_FAILED ; - }else + }else { instr->state =MTD_ERASE_DONE ; + } instr->state =MTD_ERASE_DONE ;mtd_erase_callback (instr); +return err; }
@@ -105,13 +109,14 @@ static int block2mtd_read(struct mtd_inf size_t *retlen, u_char *buf) { struct block2mtd_dev *dev = mtd->priv; -struct page *page;int index = from >>PAGE_SHIFT ;int offset = from & (PAGE_SIZE -1 ); +struct page *page;int cpylen;if (from > mtd->size)return -EINVAL ; +if (from + len > mtd->size) len = mtd->size - from;
@@ -120,9 +125,9 @@ static int block2mtd_read(struct mtd_infwhile (len) {if ((offset + len) >PAGE_SIZE ) - cpylen =PAGE_SIZE - offset;// multiple pages + cpylen =PAGE_SIZE - offset;/* multiple pages */ else - cpylen = len;// this page + cpylen = len;/* this page */ len = len - cpylen; page =page_read (dev->blkdev->bd_inode->i_mapping, index);
@@ -145,22 +150,23 @@ static int block2mtd_read(struct mtd_inf/* write data to the underlying device */ -static int _block2mtd_write (struct block2mtd_dev *dev,const u_char *buf, -loff_t to,size_t len,size_t *retlen) +static int +_block2mtd_write (struct block2mtd_dev *dev,const u_char *buf,loff_t to, +size_t len,size_t *retlen) { -struct page *page;struct address_space *mapping = dev->blkdev->bd_inode->i_mapping; -int index = to >>PAGE_SHIFT ;// page index -int offset = to & ~PAGE_MASK ;// page offset +int index = to >>PAGE_SHIFT ;/* page index */ +int offset = to & ~PAGE_MASK ;/* page offset */ +struct page *page;int cpylen;if (retlen) *retlen =0 ;while (len) {if ((offset+len) >PAGE_SIZE ) - cpylen =PAGE_SIZE - offset;// multiple pages + cpylen =PAGE_SIZE - offset;/* multiple pages */ else - cpylen = len;// this page + cpylen = len;/* this page */ len = len - cpylen; page =page_read (mapping, index);
@@ -187,8 +193,8 @@ static int _block2mtd_write(struct blockreturn 0 ; } - -static int block2mtd_write (struct mtd_info *mtd,loff_t to,size_t len, +static int +block2mtd_write (struct mtd_info *mtd,loff_t to,size_t len,size_t *retlen,const u_char *buf) {struct block2mtd_dev *dev = mtd->priv;
@@ -198,12 +204,14 @@ static int block2mtd_write(struct mtd_inreturn 0 ;if (to >= mtd->size)return -ENOSPC ; +if (to + len > mtd->size) len = mtd->size - to;mutex_lock (&dev->write_mutex); err =_block2mtd_write (dev, buf, to, len, retlen);mutex_unlock (&dev->write_mutex); +if (err >0 ) err =0 ;return err;
@@ -214,8 +222,8 @@ static int block2mtd_write(struct mtd_instatic void block2mtd_sync (struct mtd_info *mtd) {struct block2mtd_dev *dev = mtd->priv; +sync_blockdev (dev->blkdev); -return ; }
@@ -253,14 +261,14 @@ static struct block2mtd_dev *add_device( bdev =open_bdev_excl (devname,O_RDWR ,NULL );#ifndef MODULE if (IS_ERR (bdev)) { - -/* We might not have rootfs mounted at this point. Try -to resolve the device name by other means. */ - +/* +* We might not have rootfs mounted at this point. Try +* to resolve the device name by other means. +*/ dev_t devt =name_to_dev_t (devname); -if (devt) { + +if (devt) bdev =open_by_devnum (devt,FMODE_WRITE |FMODE_READ ); - } }#endif
@@ -286,18 +294,18 @@ static struct block2mtd_dev *add_device(sprintf (dev->mtd.name,"block2mtd: %s" , devname); - dev->mtd.size = dev->blkdev->bd_inode->i_size &PAGE_MASK ; - dev->mtd.erasesize = erase_size; - dev->mtd.writesize =1 ; - dev->mtd.type =MTD_RAM ; - dev->mtd.flags =MTD_CAP_RAM ; - dev->mtd.erase = block2mtd_erase; - dev->mtd.write = block2mtd_write; - dev->mtd.writev = default_mtd_writev; - dev->mtd.sync = block2mtd_sync; - dev->mtd.read = block2mtd_read; - dev->mtd.priv = dev; - dev->mtd.owner =THIS_MODULE ; + dev->mtd.size = dev->blkdev->bd_inode->i_size &PAGE_MASK ; + dev->mtd.erasesize = erase_size; + dev->mtd.writesize =1 ; + dev->mtd.type =MTD_RAM ; + dev->mtd.flags =MTD_CAP_RAM ; + dev->mtd.erase = block2mtd_erase; + dev->mtd.write = block2mtd_write; + dev->mtd.writev = default_mtd_writev; + dev->mtd.sync = block2mtd_sync; + dev->mtd.read = block2mtd_read; + dev->mtd.priv = dev; + dev->mtd.owner =THIS_MODULE ;if (add_mtd_device (&dev->mtd)) {/* Device didnt get added, so free the entry */
@@ -311,11 +319,12 @@ static struct block2mtd_dev *add_device( devinit_err:block2mtd_free_device (dev); +return NULL ; } - -/* This function works similar to reguler strtoul. In addition, it +/* +* This function works similar to reguler strtoul. In addition, it * allows some suffixes for a more human-readable number format: * ki, Ki, kiB, KiB - multiply result with 1024 * Mi, MiB - multiply result with 1024^2
@@ -324,6 +333,7 @@ devinit_err:static int ustrtoul (const char *cp,char **endp,unsigned int base) {unsigned long result =simple_strtoul (cp, endp, base); +switch (**endp) {case 'G' : result *=1024 ;
@@ -343,7 +353,6 @@ static int ustrtoul(const char *cp, charreturn result; } -static int parse_num (size_t *num,const char *token) {char *endp;
@@ -354,10 +363,10 @@ static int parse_num(size_t *num, constreturn -EINVAL ; *num = n; +return 0 ; } -static inline void kill_final_newline (char *str) {char *newline =strrchr (str,'\n' );
@@ -365,25 +374,25 @@ static inline void kill_final_newline(ch *newline =0 ; } - #define parse_err(fmt, args...) do { \ ERROR("block2mtd: " fmt "\n", ## args); \ return 0; \ } while (0) +#define BLK2MTD_PARAM_SIZE (80 + 12)/* 80 for device, 12 for erase size */ +#ifndef MODULE -static int block2mtd_init_called =0 ; -static char block2mtd_paramline[80 +12 ];/* 80 for device, 12 for erase size */ +static int block2mtd_init_called; +static char block2mtd_paramline[BLK2MTD_PARAM_SIZE ];#endif -static int block2mtd_setup2 (const char *val) { -char buf[80 +12 ];/* 80 for device, 12 for erase size */ +size_t erase_size =PAGE_SIZE ; +char buf[BLK2MTD_PARAM_SIZE ];char *str = buf;char *token[2 ];char *name; -size_t erase_size =PAGE_SIZE ;int i, ret;if (strnlen (val,sizeof (buf)) >=sizeof (buf))
@@ -407,9 +416,8 @@ static int block2mtd_setup2(const char *if (token[1 ]) { ret =parse_num (&erase_size, token[1 ]); -if (ret) { +if (ret)parse_err ("illegal erase size" ); - } }add_device (name, erase_size);
@@ -423,34 +431,36 @@ static int block2mtd_setup(const char *v#ifdef MODULE return block2mtd_setup2 (val);#else -/* If more parameters are later passed in via -/sys/module/block2mtd/parameters/block2mtd -and block2mtd_init() has already been called, -we can parse the argument now. */ - +/* +* If more parameters are later passed in via +* /sys/module/block2mtd/parameters/block2mtd +* and block2mtd_init() has already been called, +* we can parse the argument now: +*/ if (block2mtd_init_called)return block2mtd_setup2 (val); -/* During early boot stage, we only save the parameters -here. We must parse them later: if the param passed -from kernel boot command line, block2mtd_setup() is -called so early that it is not possible to resolve -the device (even kmalloc() fails). Deter that work to -block2mtd_setup2(). */ - +/* +* During early boot stage, we only save the parameters +* here. We must parse them later: if the param passed +* from kernel boot command line, block2mtd_setup() is +* called so early that it is not possible to resolve +* the device (even kmalloc() fails). Deter that work to +* block2mtd_setup2(): +*/ strlcpy (block2mtd_paramline, val,sizeof (block2mtd_paramline));return 0 ;#endif } -module_param_call (block2mtd, block2mtd_setup,NULL ,NULL ,0200 );MODULE_PARM_DESC (block2mtd,"Device to use. \"block2mtd=<dev>[,<erasesize>]\"" );static int __init block2mtd_init (void ) {int ret =0 ; +INFO ("version " VERSION );#ifndef MODULE
@@ -462,7 +472,6 @@ static int __init block2mtd_init(void)return ret; } - static void __devexit block2mtd_exit(void) {struct list_head *pos, *next;
@@ -470,6 +479,7 @@ static void __devexit block2mtd_exit(voi/* Remove the MTD devices */ list_for_each_safe (pos, next, &blkmtd_device_list) {struct block2mtd_dev *dev =list_entry (pos,typeof (*dev), list); +block2mtd_sync (&dev->mtd);del_mtd_device (&dev->mtd); INFO("mtd%d: [%s] removed", dev->mtd.index,
@@ -479,7 +489,6 @@ static void __devexit block2mtd_exit(voi } } -module_init (block2mtd_init);module_exit (block2mtd_exit);
Re: [patch] bkl2mtd: cleanup
drivers/mtd/devices/block2mtd.c line 35
Quoted 12 lines · Ingo Molnar
/* Info for the block device */
structblock2mtd_dev{drivers/mtd/devices/block2mtd.c +5
-4-struct list_head list; -struct block_device *blkdev; -struct mtd_info mtd; -struct mutex write_mutex; +struct list_head list; +struct block_device *blkdev; +struct mtd_info mtd; +/* serializes writes with each other and also with erase: */ +struct mutex write_mutex; };
Why the hell?
drivers/mtd/devices/block2mtd.c static int block2mtd_read(struct mtd_inf · line 155
Quoted 5 lines
drivers/mtd/devices/block2mtd.c +3
-2-static int _block2mtd_write (struct block2mtd_dev *dev,const u_char *buf, -loff_t to,size_t len,size_t *retlen) +static int +_block2mtd_write (struct block2mtd_dev *dev,const u_char *buf,loff_t to, +size_t len,size_t *retlen)
That's actually worse... BTW, single-underscore-... for identifiers? Odd.
drivers/mtd/devices/block2mtd.c static int _block2mtd_write(struct block · line 198
Quoted 4 lines
drivers/mtd/devices/block2mtd.c +2
-1-static int block2mtd_write (struct mtd_info *mtd,loff_t to,size_t len, +static int +block2mtd_write (struct mtd_info *mtd,loff_t to,size_t len,size_t *retlen,const u_char *buf)
Again, why split it that way?
Quoted 14 lines
drivers/mtd/devices/block2mtd.c +2
-12- dev->mtd.size = dev->blkdev->bd_inode->i_size &PAGE_MASK ; - dev->mtd.erasesize = erase_size; - dev->mtd.writesize =1 ; - dev->mtd.type =MTD_RAM ; - dev->mtd.flags =MTD_CAP_RAM ; - dev->mtd.erase = block2mtd_erase; - dev->mtd.write = block2mtd_write; - dev->mtd.writev = default_mtd_writev; - dev->mtd.sync = block2mtd_sync; - dev->mtd.read = block2mtd_read; - dev->mtd.priv = dev; - dev->mtd.owner =THIS_MODULE ; + dev->mtd.size = dev->blkdev->bd_inode->i_size &PAGE_MASK ; + dev->mtd.erasesize = erase_size;
drivers/mtd/devices/block2mtd.c static struct block2mtd_dev *add_device( · line 308
Quoted 10 lines
drivers/mtd/devices/block2mtd.c +10
-0+ dev->mtd.writesize =1 ; + dev->mtd.type =MTD_RAM ; + dev->mtd.flags =MTD_CAP_RAM ; + dev->mtd.erase = block2mtd_erase; + dev->mtd.write = block2mtd_write; + dev->mtd.writev = default_mtd_writev; + dev->mtd.sync = block2mtd_sync; + dev->mtd.read = block2mtd_read; + dev->mtd.priv = dev; + dev->mtd.owner =THIS_MODULE ;
Bogus.
drivers/mtd/devices/block2mtd.c static int block2mtd_setup2(const char * · line 421
Quoted 5 lines
drivers/mtd/devices/block2mtd.c +1
-2-if (ret) { +if (ret)parse_err ("illegal erase size" ); - } }
BTW, I certainly wouldn't inflict that on patches; at some point in series the body of if may shrink to one line only to get longer in the next patch. Removing and restoring {} would only add noise.
OTOH, you've missed quite a few of my pet peeves, starting with u_char in the quoted part...
Re: [patch] bkl2mtd: cleanup
Quoted 8 lines · Al Viro
drivers/mtd/devices/block2mtd.c static int block2mtd_read(struct mtd_inf · line 155
Quoted 6 lines
drivers/mtd/devices/block2mtd.c +3
-2-static int _block2mtd_write (struct block2mtd_dev *dev,const u_char *buf, -loff_t to,size_t len,size_t *retlen) +static int +_block2mtd_write (struct block2mtd_dev *dev,const u_char *buf,loff_t to, +size_t len,size_t *retlen)That's actually worse... BTW, single-underscore-... for identifiers? Odd.
Indeed. Must have been my fault.
Quoted 2 lines
OTOH, you've missed quite a few of my pet peeves, starting with
u_charin the quoted part...
A large-scale s/u_char/void/ over mtd read and write functions wouldn't hurt, no doubt. Currently every other driver contains a cast to (u_char*) or three for no gain afaics.
Jörn
Signature · 3 lines
Never argue with idiots - first they drag you down to their level, then they beat you with experience. -- unknown
Re: [patch] bkl2mtd: cleanup
In reply to Al ViroQuoted 15 lines · Al Viro
drivers/mtd/devices/block2mtd.c line 35
Quoted 12 lines · Ingo Molnar
/* Info for the block device */
structblock2mtd_dev{drivers/mtd/devices/block2mtd.c +5
-4-struct list_head list; -struct block_device *blkdev; -struct mtd_info mtd; -struct mutex write_mutex; +struct list_head list; +struct block_device *blkdev; +struct mtd_info mtd; +/* serializes writes with each other and also with erase: */ +struct mutex write_mutex; };Why the hell?
the vertical alignment? For the same reason some of the key VFS data structures in include/linux/fs.h are aligned vertically:
struct inode
struct file
struct super_block
struct address_space
struct block_device
there are several advantages of aligning structure fields vertically (and the same applies to bulk initializers of structures), should i list them?
Ingo
Re: [patch] bkl2mtd: cleanup
In reply to Al ViroQuoted 9 lines · Al Viro
drivers/mtd/devices/block2mtd.c static int block2mtd_setup2(const char * · line 421
Quoted 5 lines
drivers/mtd/devices/block2mtd.c +1
-2-if (ret) { +if (ret)parse_err ("illegal erase size" ); - } }BTW, I certainly wouldn't inflict that on patches; at some point in series the body of if may shrink to one line only to get longer in the next patch. Removing and restoring {} would only add noise.
agreed, we dont enforce it strictly - it is the end result that must be clean.
[ OTOH, when i create a patch series i personally always try to make
each patch clean, because one never knows when a later patch gets delayed or dropped altogether. ]
Ingo
Re: [patch] bkl2mtd: cleanup
In reply to Al ViroQuoted 8 lines · Al Viro
drivers/mtd/devices/block2mtd.c static int block2mtd_read(struct mtd_inf · line 155
Quoted 5 lines
drivers/mtd/devices/block2mtd.c +3
-2-static int _block2mtd_write (struct block2mtd_dev *dev,const u_char *buf, -loff_t to,size_t len,size_t *retlen) +static int +_block2mtd_write (struct block2mtd_dev *dev,const u_char *buf,loff_t to, +size_t len,size_t *retlen)That's actually worse... BTW, single-underscore-... for identifiers? Odd.
(yep, that's odd.)
Quoted 6 lines
drivers/mtd/devices/block2mtd.c static int _block2mtd_write(struct block · line 198
Quoted 4 lines
drivers/mtd/devices/block2mtd.c +2
-1-static int block2mtd_write (struct mtd_info *mtd,loff_t to,size_t len, +static int +block2mtd_write (struct mtd_info *mtd,loff_t to,size_t len,size_t *retlen,const u_char *buf)Again, why split it that way?
these are really nuances, so unless you are interested in such nuances nowhere found in CodingStyle, stop reading here :-)
i personally try to minimize the number and complexity of function prototype patterns, while still trying to keep the linecount low. So if a function prototype wants to be multi-line, it's not a "simple one-line function prototype" anymore, so i use the same template for everything:
type
function_name(vars ...
more vars ...)
{
[ having the 'type' separately makes it easy to judge the return type of
a function (especially with syntax highlighting active). Aligned variables are an efficient extension of the 'line' concept that does not mix the function_name with the variables. ]
incidentally, a natural simplified variant of that is the following:
type
function_name(vars...)
{
which tends to stay cleanly 2-line and looks tidier and shorter than the:
type function_name(vars...
more vars ...)
{
form. The preferred form is of course:
type function_name(vars...)
Ingo
Re: [patch] bkl2mtd: cleanup
Quoted 6 lines · Ingo Molnar
type
function_name(vars...)
{which tends to stay cleanly 2-line and looks tidier and shorter than the:
Quoted 7 lines
type function_name(vars...
more vars ...){
form. The preferred form is of course:
type
function_name(vars...)
Anyway I wouldn't mix that bsd style with this one in one particular file. (Beside the fact I dislike the BSD one.)
Joe Perches
maintainerRe: [patch] bkl2mtd: cleanup
In reply to Ingo MolnarQuoted 7 lines · Ingo Molnar
not a "simple one-line function prototype" anymore,
so i use the same template for everything:type
function_name(vars ...more vars ...){
It seems the most common linux style keeps type and function on the same line where possible and arguments on subsequent lines when necessary.
type function_name(args...)
type function_name(args_to_col80,
more_args...)
Perhaps that should be your default template.
style of function definitions (Re: [patch] bkl2mtd: cleanup)
In reply to Ingo MolnarIngo Molnar @ Wed, 26 Mar 2008 12:10:21 +0100
Quoted 3 lines · Al Viro
[]
Quoted 9 lines
Quoted 6 lines
drivers/mtd/devices/block2mtd.c static int _block2mtd_write(struct block · line 198
Quoted 4 lines
drivers/mtd/devices/block2mtd.c +2
-1-static int block2mtd_write (struct mtd_info *mtd,loff_t to,size_t len, +static int +block2mtd_write (struct mtd_info *mtd,loff_t to,size_t len,size_t *retlen,const u_char *buf)Again, why split it that way?
these are really nuances, so unless you are interested in such nuances nowhere found in CodingStyle, stop reading here :-)
Nuances, or not, there are all kinds of stuff in Linux. Very small journey from simple grep and trivial multi-line sed to a huge discovery can be found here:
http://kernelnewbies.org/olecom#Function_definitions
No matter what coding style is, the winner is "linux-2.6/fs/xfs", and i doubt anyone can fix that :)
Maybe meta-patching by sed scripts in git? Just kidding. --
-o--=O`C
#oo'L O
<___=E M
Re: style of function definitions (Re: [patch] bkl2mtd: cleanup)
Quoted 11 lines · Oleg Verych
[]
Quoted 9 lines
Quoted 6 lines
drivers/mtd/devices/block2mtd.c static int _block2mtd_write(struct block · line 198
Quoted 4 lines
drivers/mtd/devices/block2mtd.c +2
-1-static int block2mtd_write (struct mtd_info *mtd,loff_t to,size_t len, +static int +block2mtd_write (struct mtd_info *mtd,loff_t to,size_t len,size_t *retlen,const u_char *buf)Again, why split it that way?
these are really nuances, so unless you are interested in such nuances nowhere found in CodingStyle, stop reading here :-)
Quoted 8 lines
Nuances, or not, there are all kinds of stuff in Linux. Very small journey from simple
grepand trivial multi-linesedto a huge discovery can be found here:http://kernelnewbies.org/olecom#Function_definitions
No matter what coding style is, the winner is "linux-2.6/fs/xfs", and i doubt anyone can fix that :)
They at least have an excuse of having a historical unix background :)
Re: style of function definitions (Re: [patch] bkl2mtd: cleanup)
Jan Engelhardt @ Sun, Mar 30, 2008 at 6:31 AM:
Quoted 4 lines
Quoted 2 lines
No matter what coding style is, the winner is "linux-2.6/fs/xfs", and i doubt anyone can fix that :)
They at least have an excuse of having a historical unix background :)
i.e. zoo of proprietary versions, expensive, without reliable implementation of basic tools and compilers, etc.
Historical UNIX(R) isn't that good at all. Now it's POSIX(R) forcing all new development back.
http://kernelnewbies.org/olecom#trailing_comments_crap
http://kernelnewbies.org/olecom#trailing_multiline_comments_crap
Q: why every parameter is on new line?
A: thus we have more actual code line count
Q: why trailing comments on every parameter?
A: our C compiler have (had) 8 characters limit for variable names
Q: don't you think streaming editor can handle that?
A: our tools have not such thing
Q: you have multi-line trailing comments there, don't you know
it complicates line-oriented grep-like text processing?
why don't you write freely in comment block before function definition?
A: why do you care?
Signature · 3 lines
-o--=O`C #oo'L O <___=E M
Re: [PATCH 109/148] include/asm-x86/serial.h: checkpatch cleanups - formatting only
In reply to Ingo MolnarFrom: Ingo Molnar <[email protected]>
Date: Tue, 25 Mar 2008 14:45:56 +0100
Quoted 5 lines
and let me give an example with the your very own code that you wrote and maintain,
drivers/mtd/devices/block2mtd.c:errors lines of code errors/KLOC
drivers/mtd/devices/block2mtd.c10 490 20.4
Ingo, this is devolving into a "code I maintain is great, code you maintain sucks, checkpatch says so" kind of discussion, please stop.
You're not making any friends by making your arguments this way.
Re: [PATCH 109/148] include/asm-x86/serial.h: checkpatch cleanups - formatting only
Quoted 7 lines · David Miller
Quoted 2 lines
errors lines of code errors/KLOC
drivers/mtd/devices/block2mtd.c10 490 20.4Ingo, this is devolving into a "code I maintain is great, code you maintain sucks, checkpatch says so" kind of discussion, please stop.
You're not making any friends by making your arguments this way.
To be fair, block2mtd does suck, even conceptually. But that would digress too far.
Jörn
Signature · 4 lines
Rules of Optimization: Rule 1: Don't do it. Rule 2 (for experts only): Don't do it yet. -- M.A. Jackson
Re: [PATCH 109/148] include/asm-x86/serial.h: checkpatch cleanups - formatting only
In reply to David MillerQuoted 11 lines · David Miller
From: Ingo Molnar <[email protected]>
Date: Tue, 25 Mar 2008 14:45:56 +0100Quoted 5 lines
and let me give an example with the your very own code that you wrote and maintain,
drivers/mtd/devices/block2mtd.c:errors lines of code errors/KLOC
drivers/mtd/devices/block2mtd.c10 490 20.4Ingo, this is devolving into a "code I maintain is great, code you maintain sucks, checkpatch says so" kind of discussion, please stop.
Firstly, had you read my previous mails you'd realize that a score of 20.4 errors/KLOC is pretty clean code for a driver. (and a manual view of drivers/mtd/devices/block2mtd.c confirmed that first quick impression.)
Secondly, it was you who claimed that checkpatch.pl warnings were bogus, i asked for hard data which you refused to provide, so i had to come up with examples to counter that.
What did you expect me to express my checkpatch.pl experience about, if not about code that i maintain? How did you expect me to show to you that the warnings are correct, if not code that you maintain? Why should i talk about any code that neither you nor i am directly interested in? Do you want me to talk about FreeBSD, so that i dont hurt anyone's feelings? Feel free to use my code as an example to make your points.
Quoted 1 line
You're not making any friends by making your arguments this way.
just to make it clear, i'm not here on lkml to "make friends", i'm here to make Linux better. The two goals are not fundamentally incompatible but sometimes they do clash ;-)
Ingo
Re: [PATCH 109/148] include/asm-x86/serial.h: checkpatch cleanups - formatting only
In reply to Jörn EngelQuoted 11 lines · Jörn Engel
Quoted 8 lines
So to turn around the argument: could you give me any reason why differing coding style between subsystems,
_oftenin blatant violation ofDocumentation/CodingStyle_, is somehow "good" for Linux in the long run? I listed numerous first-hand advantages that style consistency brings and i listed numerous disadvantages created by inconsistency. So i'm waiting for the list of counter-arguments - there _must_ be some objective ones, besides the obvious "kernel old-timers are lazy to change their ways" argument =B-)When you reject useful patches based on "this is not our preferred style", you piss people off. [...]
we dont actually do that for newbies and newbies are in fact happy to write cleaner code - so the rest of your argument which depends on this premise fails. (Most of the time i fix it up silently myself or if a style error comes in a pattern, i ask the person to send future patches with that small detail fixed.)
my experience with checkpatch.pl is the exact opposite of what you fear:
it _widened_ the contributor base: a good number of newbies felt encouraged that an objective piece of tool reports an "error" in a file that was written by otherwise "much more knowledgable" kernel hackers. checkpatch.pl is basically the "yes, really, you are right, this piece of code in the Linux kernel is indeed crap" review tool that reinforces newbies. It lowers the bar of entry to kernel hacking, and it does so for exactly those pieces of code that we want newbies to be active on:
barely maintained source code.
Whoever is afraid of an "army of checkpatch wielding newbies" who'll never rise above their newbie status fails to consider two important
factors: 1) checkpatch errors are a finite resource to feed on and they dont re-grow in a well-maintained subsystem 2) they need to look back a few years when they themselves were newbies and were in need of some easy kernel projects just to familiarize themselves with the kernel and its contribution environment. (My first-ever contribution to the Linux kernel was a trivial patch.)
( i only remember one patch ever being rejected due to checkpatch
failures, it came from a kernel old-timer who sent absolutely horrible patches and who _should have known better_. Kernel old-timers are "multipliers", they write more code and influence more people's code, so it's expected of them to write absolutely squeaky-clean code. )
so at least for the scheduler and for arch/x86 there's absolutely zero friction between checkpatch.pl use and newbies - and if you look at the nicely evolving arch/x86 contributor statistics you'll have to come to the same conclusion i believe.
Ingo
Re: [PATCH 109/148] include/asm-x86/serial.h: checkpatch cleanups - formatting only
In reply to Jörn EngelQuoted 2 lines · J??rn Engel
CodingStyle has gone too far is this:
for (i=0; i<10; i++)
it's a very good argument why we need a consistant style. The above is unreadable crap that hurts my eyes.
Re: [PATCH 109/148] include/asm-x86/serial.h: checkpatch cleanups - formatting only
Quoted 6 lines · Christoph Hellwig
Quoted 2 lines · J??rn Engel
CodingStyle has gone too far is this:
for (i=0; i<10; i++)it's a very good argument why we need a consistant style. The above is unreadable crap that hurts my eyes.
same for me.
and PLEASE, folks, even if you _dont_ find that line unreadable, and even if you have full power and control over your own subsystem that you maintain and can NAK cleanup patches at whim, still _PLEASE_ follow the Linux coding style because it inconsistency hurts the eyes of a substantial proportion of kernel developers. Other folks might have to fix bugs in your code, other folks might want to reuse your code or you might go into a different subsystem and give maintainership to someone else, etc. etc. Consistent coding style is one of the few concepts that only has advantages and no disadvantages at all.
Ingo
Re: [PATCH 109/148] include/asm-x86/serial.h: checkpatch cleanups - formatting only
In reply to Ingo MolnarQuoted 7 lines · Ingo Molnar
On a more conceptual angle: "coding style", despite being entirely "non-functional" (it does not affect the generated code), is still very much an integral part of the code because source code is fundamentally about "knowledge" - and extra style noise in knowledge can never possibly increase the quality of that knowledge. There are strong links between code correctness and typography/aesthetics.
You assert that all the time, but it is just that: an assertation. I assert that code style is only a small part of code correctness. Also just an assertation. Who is more right? Probably the truth is somewhere inbetween. At least I think it is nearer my position than yours @)
Also regarding the rules enforced by checkpatch I think there is a wide range on how much they impact readability: e.g. if someone uses the wrong bracket style consistently that is somewhat disrupting. I agree.
But is trailing white space disrupting to code reading in any way? Very doubtful.
Most rules are somewhere inbetween. They vary widely in how much they impact readability.
Also sometimes the rules conflict. Example: the 80 column rule often conflicts with the "always space around operator" rule. That is because expressions split over multiple lines are harder to read than an expression on a single line (at least here) and at least I would rather trade a few missing spaces around operators than to have a multi-line expression.
It's always a trade-off and checkpatch.pl is not very good (read it doesn't really handle) trade-offs.
Quoted 7 lines
So, in the specific example of the scheduler subsystem, i've only observed advantages to checkpatch and zero downsides. Could anyone give me _any_ objective reason why i shouldnt be using checkpatch for the scheduler? More broadly, could anyone give me an objective reason why we shouldnt be doing it for
arch/x86? And even more broadly, could anyone give me an objective reason why we shouldnt be doing it for all actively maintained areas of the kernel?
For new code being added (like your CFS scheduler) it is fine, but for old code it has the problem of conflicting with other actually useful changes on the same areas which are pending. And doing merges into such changing code bases is always somewhat error prone because the people who do it are also only human and can apply subtle typos etc.
Strictly seen each such merge requires a whole new testing cycle and doing such a testing cycle just for someone's checkpatch changes is really a waste of time and seriously impacting real progress. The only saving grace is that it will hopefully only happen once per file, but the point still holds. There are a lot of different files in Linux, so it has the potential to be a serious problem.
That is an objective (not just random assertation) reason against doing extensive changes of existing files like Joe's patchkit.
I think it would be fair at least if people doing this asked first at least:
- Does anybody have pending changes against file X, perhaps
also checking mm and linux-next
and then wait a bit and if someone says he has pending changes then not do the reformatting until the pending changes get merged.
Or better really only do it on new code.
-Andi
Re: [PATCH 109/148] include/asm-x86/serial.h: checkpatch cleanups - formatting only
Quoted 9 lines · Andi Kleen
Quoted 7 lines · Ingo Molnar
On a more conceptual angle: "coding style", despite being entirely "non-functional" (it does not affect the generated code), is still very much an integral part of the code because source code is fundamentally about "knowledge" - and extra style noise in knowledge can never possibly increase the quality of that knowledge. There are strong links between code correctness and typography/aesthetics.
Quoted 5 lines
You assert that all the time, but it is just that: an assertation. I assert that code style is only a small part of code correctness. Also just an assertation. Who is more right? Probably the truth is somewhere inbetween. At least I think it is nearer my position than yours @)
I think someone with a consistent code style generates better code than someone who can't even make up his mind on how to write it, let alone what to write.
With Linux we have many people, many of us with a preferred style, most of which will differ from the suggested style.
Much like natural languages, many of us don't speak English as their native tongue, mine is Dutch, yours is German. Still we communicate with each other using English, because that is the de-facto standard on LKML [ and of course because my German truly sucks ;-) ].
Similarly for coding style, if we all stick to one style its easier to read (and hopefully fix) each others code. All checkpatch.pl attempts is help us do that.
Sure it complains about sometimes trivial things - but if truly trivial making the change isn't hard - or might even not be done in favour of 'better' taste. Its a guide, not hard rules, it suggests we inspect a certain piece of code again, to see if we really intended to write it so.
Does it get things wrong; Yes I do think so. The change in patch 120/148 is utter nonsense IMHO.
Checkpatch will also not complain about larger things that make a bigger difference; for instance it will happily let you write a 200 line function, even though the code would have been so much better in 10 smaller functions.
Does it help; Yes, I do think it does. If only people would apply common sense along with it...
Quoted 7 lines
Also regarding the rules enforced by checkpatch I think there is a wide range on how much they impact readability: e.g. if someone uses the wrong bracket style consistently that is somewhat disrupting. I agree.
But is trailing white space disrupting to code reading in any way? Very doubtful.
The trailing and leading whitespace thingies are trivial to fix (and quilt refresh --strip-trailing-whitespace does half of it already, so anybody using quilt doesn't have any excuse in ever seeing that error anyway).
There is something to say to send Linus a script that fixes up the whole tree and be done with it. These are petty things indeed.
Quoted 10 lines
Most rules are somewhere inbetween. They vary widely in how much they impact readability.
Also sometimes the rules conflict. Example: the 80 column rule often conflicts with the "always space around operator" rule. That is because expressions split over multiple lines are harder to read than an expression on a single line (at least here) and at least I would rather trade a few missing spaces around operators than to have a multi-line expression.
Quoted 2 lines
It's always a trade-off and checkpatch.pl is not very good (read it doesn't really handle) trade-offs.
Which is why it is a guide, a human using the tool can use his good taste and discretion by choosing which suggestions to take and which to politely ignore.
Quoted 8 lines
Quoted 7 lines
So, in the specific example of the scheduler subsystem, i've only observed advantages to checkpatch and zero downsides. Could anyone give me _any_ objective reason why i shouldnt be using checkpatch for the scheduler? More broadly, could anyone give me an objective reason why we shouldnt be doing it for
arch/x86? And even more broadly, could anyone give me an objective reason why we shouldnt be doing it for all actively maintained areas of the kernel?
Quoted 5 lines
For new code being added (like your CFS scheduler) it is fine, but for old code it has the problem of conflicting with other actually useful changes on the same areas which are pending. And doing merges into such changing code bases is always somewhat error prone because the people who do it are also only human and can apply subtle typos etc.
Sure, and I think that when there is actual work pending its 'actively' maintained and will eventually gravitate towards cleaniness anyway.
Quoted 6 lines
Strictly seen each such merge requires a whole new testing cycle and doing such a testing cycle just for someone's checkpatch changes is really a waste of time and seriously impacting real progress. The only saving grace is that it will hopefully only happen once per file, but the point still holds. There are a lot of different files in Linux, so it has the potential to be a serious problem.
The good thing is that most of these checkpatch patches could be validated by comparing object code of the affected translation units - they're that trivial.
When generating 100% identical code, there is no issue.
Quoted 2 lines
That is an objective (not just random assertation) reason against doing extensive changes of existing files like Joe's patchkit.
Building a single allyesconfig for x86_32 and x86_64 before and after and getting identical binaries is pretty strong.
Although I think this patch series doesn't manage to accomplish it, if only because it moves __LINE__ statements around.
Quoted 5 lines
I think it would be fair at least if people doing this asked first at least:
- Does anybody have pending changes against file X, perhaps
also checking mm and linux-next
and then wait a bit and if someone says he has pending changes then not do the reformatting until the pending changes get merged.
Sure, cleanups like this should never bother real work - good chance that the real work already cleans up most issues anyway.
Quoted 1 line
Or better really only do it on new code.
It might be an incentive to touch (and get to know and appreciate and eventually clean up or start maintaining) otherwise dead code. I don't think we should stop such incentives (even if they are very small).
Re: [PATCH 109/148] include/asm-x86/serial.h: checkpatch cleanups - formatting only
Quoted 5 lines
Quoted 2 lines
That is an objective (not just random assertation) reason against doing extensive changes of existing files like Joe's patchkit.
Building a single allyesconfig for
x86_32andx86_64before and after and getting identical binaries is pretty strong.
checkpatch does not necessarily result in the same binaries. First there is the build date and then there might be changes like KERN_* prefixes added etc.
And there might be code which is not covered under a single configuration, e.g. when both 32bit and 64bit x86 is changed.
-Andi
Re: [PATCH 109/148] include/asm-x86/serial.h: checkpatch cleanups - formatting only
Quoted 5 lines · Andi Kleen
Quoted 5 lines
Quoted 2 lines
That is an objective (not just random assertation) reason against doing extensive changes of existing files like Joe's patchkit.
Building a single allyesconfig for
x86_32andx86_64before and after and getting identical binaries is pretty strong.
true, but it's usually simplier to just compile the affected file.
Quoted 6 lines
checkpatch does not necessarily result in the same binaries. First there is the build date and then there might be changes like KERN_* prefixes added etc.
And there might be code which is not covered under a single configuration, e.g. when both 32bit and 64bit x86 is changed.
in the last series of coding style patches i sent to both ingo and bart i worked as follow:
- worked on files with agreement of the maintainer (or after he asked me to do the cleanup)
- separated changes that modified the binary from the pure style changes. -all the patch were compile tested and when possible a size/md5sum verificatio was performed and added to the changelog.
i learned this "rules" learning from my mistakes and in the end it worked well so i think the problem is in how people are using the tool, not in the tool itself.
ciao,
--
Paolo
http://paolo.ciarrocchi.googlepages.com/
Joe Perches
maintainerPATCH 110/148 include/asm-x86/setup.h: checkpatch cleanups - formatting only
In reply to Joe Perches--- include/asm-x86/setup.h | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
include/asm-x86/setup.h +2 -2
@@ -55,8 +55,8 @@ struct e820entry;char *__init machine_specific_memory_setup (void );char *memory_setup (void ); -int __init copy_e820_map (struct e820entry * biosmap,int nr_map); -int __init sanitize_e820_map (struct e820entry * biosmap,char * pnr_map); +int __init copy_e820_map (struct e820entry *biosmap,int nr_map); +int __init sanitize_e820_map (struct e820entry *biosmap,char *pnr_map);void __init add_memory_region (unsigned long long start,unsigned long long size,int type);
Signature · 1 line
1.5.4.rc2
Joe Perches
maintainerPATCH 111/148 include/asm-x86/sigcontext32.h: checkpatch cleanups - formatting only
In reply to Joe Perches--- include/asm-x86/sigcontext32.h | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
include/asm-x86/sigcontext32.h +2 -2
@@ -26,7 +26,7 @@ struct _fpstate_ia32 {__u32 cw;__u32 sw;__u32 tag;/* not compatible to 64bit twd */ -__u32 ipoff; +__u32 ipoff;__u32 cssel;__u32 dataoff;__u32 datasel;
@@ -39,7 +39,7 @@ struct _fpstate_ia32 {__u32 mxcsr;__u32 reserved;struct _fpxreg _fxsr_st[8 ]; -struct _xmmreg _xmm[8 ];/* It's actually 16 */ +struct _xmmreg _xmm[8 ];/* It's actually 16 */ __u32 padding[56 ]; };
Signature · 1 line
1.5.4.rc2
Joe Perches
maintainerPATCH 112/148 include/asm-x86/sigcontext.h: checkpatch cleanups - formatting only
In reply to Joe Perches--- include/asm-x86/sigcontext.h | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-)
include/asm-x86/sigcontext.h +4 -3
@@ -79,7 +79,7 @@ struct sigcontext {unsigned long flags;unsigned long sp_at_signal;unsigned short ss, __ssh; -struct _fpstate __user * fpstate; +struct _fpstate __user *fpstate;unsigned long oldmask;unsigned long cr2; };
@@ -107,7 +107,7 @@ struct sigcontext {unsigned long eflags;unsigned long esp_at_signal;unsigned short ss, __ssh; -struct _fpstate __user * fpstate; +struct _fpstate __user *fpstate;unsigned long oldmask;unsigned long cr2; };
@@ -121,7 +121,8 @@ struct sigcontext {struct _fpstate {__u16 cwd;__u16 swd; -__u16 twd;/* Note this is not the same as the 32bit/x87/FSAVE twd */ +__u16 twd;/* Note this is not the same as the +32bit/x87/FSAVE twd */ __u16 fop;__u64 rip;__u64 rdp;
Signature · 1 line
1.5.4.rc2
Joe Perches
maintainerPATCH 113/148 include/asm-x86/signal.h: checkpatch cleanups - formatting only
In reply to Joe Perches--- include/asm-x86/signal.h | 48 +++++++++++++++++++++++----------------------- 1 files changed, 24 insertions(+), 24 deletions(-)
include/asm-x86/signal.h +24 -24
@@ -185,61 +185,61 @@ typedef struct sigaltstack {#define __HAVE_ARCH_SIG_BITOPS -#define sigaddset (set,sig) \ - (__builtin_constantp(sig) ? \ - __const_sigaddset((set),(sig)) : \ - __gen_sigaddset((set),(sig))) +#define sigaddset (set,sig) \ + (__builtin_constantp(sig) \ + ? __const_sigaddset((set), (sig)) \ + : __gen_sigaddset((set), (sig))) -static __inline__void __gen_sigaddset (sigset_t *set,int _sig) +static inline void __gen_sigaddset (sigset_t *set,int _sig) { - __asm__("btsl %1,%0" :"+m" (*set) :"Ir" (_sig -1 ) :"cc" ); + asm("btsl %1,%0" :"+m" (*set) :"Ir" (_sig -1 ) :"cc" ); } -static __inline__void __const_sigaddset (sigset_t *set,int _sig) +static inline void __const_sigaddset (sigset_t *set,int _sig) {unsigned long sig = _sig -1 ; set->sig[sig / _NSIG_BPW] |=1 << (sig % _NSIG_BPW); } -#define sigdelset (set,sig) \ - (__builtin_constant_p(sig) ? \ - __const_sigdelset((set),(sig)) : \ - __gen_sigdelset((set),(sig))) +#define sigdelset (set, sig) \ + (__builtin_constant_p(sig) \ + ? __const_sigdelset((set), (sig)) \ + : __gen_sigdelset((set), (sig))) -static __inline__void __gen_sigdelset (sigset_t *set,int _sig) +static inline void __gen_sigdelset (sigset_t *set,int _sig) { - __asm__("btrl %1,%0" :"+m" (*set) :"Ir" (_sig -1 ) :"cc" ); + asm("btrl %1,%0" :"+m" (*set) :"Ir" (_sig -1 ) :"cc" ); } -static __inline__void __const_sigdelset (sigset_t *set,int _sig) +static inline void __const_sigdelset (sigset_t *set,int _sig) {unsigned long sig = _sig -1 ; set->sig[sig / _NSIG_BPW] &= ~(1 << (sig % _NSIG_BPW)); } -static __inline__int __const_sigismember (sigset_t *set,int _sig) +static inline int __const_sigismember (sigset_t *set,int _sig) {unsigned long sig = _sig -1 ;return 1 & (set->sig[sig / _NSIG_BPW] >> (sig % _NSIG_BPW)); } -static __inline__int __gen_sigismember (sigset_t *set,int _sig) +static inline int __gen_sigismember (sigset_t *set,int _sig) {int ret; - __asm__("btl %2,%1\n\tsbbl %0,%0" - :"=r" (ret) :"m" (*set),"Ir" (_sig-1 ) :"cc" ); + asm("btl %2,%1\n\tsbbl %0,%0" + :"=r" (ret) :"m" (*set),"Ir" (_sig-1 ) :"cc" );return ret; } -#define sigismember (set,sig) \ - (__builtin_constant_p(sig) ? \ - __const_sigismember((set),(sig)) : \ - __gen_sigismember((set),(sig))) +#define sigismember (set, sig) \ + (__builtin_constant_p(sig) \ + ? __const_sigismember((set), (sig)) \ + : __gen_sigismember((set), (sig))) -static __inline__int sigfindinword (unsigned long word) +static inline int sigfindinword (unsigned long word) { - __asm__("bsfl %1,%0" :"=r" (word) :"rm" (word) :"cc" ); + asm("bsfl %1,%0" :"=r" (word) :"rm" (word) :"cc" );return word; }
Signature · 1 line
1.5.4.rc2
Joe Perches
maintainerPATCH 114/148 include/asm-x86/smp_32.h: checkpatch cleanups - formatting only
In reply to Joe Perches--- include/asm-x86/smp_32.h | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)
include/asm-x86/smp_32.h +3 -3
@@ -18,8 +18,8 @@extern cpumask_t cpu_callin_map; -extern void (*mtrr_hook) (void ); -extern void zap_low_mappings (void ); +extern void (*mtrr_hook)(void ); +extern void zap_low_mappings (void );#ifdef CONFIG_SMP /*
@@ -44,7 +44,7 @@ static inline int num_booting_cpus(void)#ifdef CONFIG_X86_LOCAL_APIC -static __inlineint logical_smp_processor_id (void ) +static inline int logical_smp_processor_id (void ) {/* we don't want to mark this access volatile - bad code generation */ return GET_APIC_LOGICAL_ID (*(u32 *)(APIC_BASE +APIC_LDR ));
Signature · 1 line
1.5.4.rc2
Joe Perches
maintainerPATCH 115/148 include/asm-x86/smp_64.h: checkpatch cleanups - formatting only
In reply to Joe Perches--- include/asm-x86/smp_64.h | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)
include/asm-x86/smp_64.h +3 -3
@@ -24,9 +24,9 @@ extern int smp_call_function_mask(cpumask_t mask, void (*func)(void *),#define raw_smp_processor_id () read_pda(cpunumber)#define stack_smp_processor_id () \ - ({ \ +({ \ struct thread_info *ti; \ - __asm__("andq %%rsp,%0; ":"=r" (ti) : "0" (CURRENT_MASK)); \ + asm("andq %%rsp,%0; ":"=r" (ti) : "0" (CURRENT_MASK)); \ ti->cpu; \ })
@@ -46,7 +46,7 @@ static inline int num_booting_cpus(void)#define safe_smp_processor_id () smp_processor_id() -static __inlineint logical_smp_processor_id (void ) +static inline int logical_smp_processor_id (void ) {/* we don't want to mark this access volatile - bad code generation */ return GET_APIC_LOGICAL_ID (*(u32 *)(APIC_BASE +APIC_LDR ));
Signature · 1 line
1.5.4.rc2
Joe Perches
maintainerPATCH 116/148 include/asm-x86/spinlock.h: checkpatch cleanups - formatting only
In reply to Joe Perches--- include/asm-x86/spinlock.h | 105 +++++++++++++++++++++----------------------- 1 files changed, 50 insertions(+), 55 deletions(-)
include/asm-x86/spinlock.h +50 -55
@@ -82,7 +82,7 @@ static inline void __raw_spin_lock(raw_spinlock_t *lock) {short inc =0x0100 ; - __asm__ __volatile__ ( + asmvolatile (LOCK_PREFIX "xaddw %w0, %1\n" "1:\t" "cmpb %h0, %b0\n\t"
@@ -92,9 +92,9 @@ static inline void __raw_spin_lock(raw_spinlock_t *lock)/* don't need lfence here, because loads are in-order */ "jmp 1b\n" "2:" - :"+Q" (inc), "+m" (lock->slock) + : "+Q" (inc), "+m" (lock->slock) : - :"memory", "cc"); + : "memory", "cc"); } #define __raw_spin_lock_flags(lock, flags) __raw_spin_lock(lock)
@@ -104,30 +104,28 @@ static inline int __raw_spin_trylock(raw_spinlock_t *lock)int tmp;short new; - asmvolatile ( -"movw %2,%w0\n\t" -"cmpb %h0,%b0\n\t" -"jne 1f\n\t" -"movw %w0,%w1\n\t" -"incb %h1\n\t" -"lock ; cmpxchgw %w1,%2\n\t" -"1:" -"sete %b1\n\t" -"movzbl %b1,%0\n\t" - :"=&a" (tmp),"=Q" (new),"+m" (lock->slock) - : - :"memory" ,"cc" ); + asmvolatile ("movw %2,%w0\n\t" +"cmpb %h0,%b0\n\t" +"jne 1f\n\t" +"movw %w0,%w1\n\t" +"incb %h1\n\t" +"lock ; cmpxchgw %w1,%2\n\t" +"1:" +"sete %b1\n\t" +"movzbl %b1,%0\n\t" + :"=&a" (tmp),"=Q" (new),"+m" (lock->slock) + : + :"memory" ,"cc" ); return tmp; }static inline void __raw_spin_unlock (raw_spinlock_t *lock) { - __asm__ __volatile__( -UNLOCK_LOCK_PREFIX "incb %0" - :"+m" (lock->slock) - : - :"memory" ,"cc" ); + asmvolatile (UNLOCK_LOCK_PREFIX "incb %0" + :"+m" (lock->slock) + : + :"memory" ,"cc" ); }#else static inline int __raw_spin_is_locked (raw_spinlock_t *lock)
@@ -149,21 +147,20 @@ static inline void __raw_spin_lock(raw_spinlock_t *lock)int inc =0x00010000 ;int tmp; - __asm__ __volatile__ ( -"lock ; xaddl %0, %1\n" -"movzwl %w0, %2\n\t" -"shrl $16, %0\n\t" -"1:\t" -"cmpl %0, %2\n\t" -"je 2f\n\t" -"rep ; nop\n\t" -"movzwl %1, %2\n\t" -/* don't need lfence here, because loads are in-order */ -"jmp 1b\n" -"2:" - :"+Q" (inc),"+m" (lock->slock),"=r" (tmp) - : - :"memory" ,"cc" ); + asmvolatile ("lock ; xaddl %0, %1\n" +"movzwl %w0, %2\n\t" +"shrl $16, %0\n\t" +"1:\t" +"cmpl %0, %2\n\t" +"je 2f\n\t" +"rep ; nop\n\t" +"movzwl %1, %2\n\t" +/* don't need lfence here, because loads are in-order */ +"jmp 1b\n" +"2:" + :"+Q" (inc),"+m" (lock->slock),"=r" (tmp) + : + :"memory" ,"cc" ); }#define __raw_spin_lock_flags (lock, flags) __raw_spin_lock(lock)
@@ -173,31 +170,29 @@ static inline int __raw_spin_trylock(raw_spinlock_t *lock)int tmp;int new; - asmvolatile ( -"movl %2,%0\n\t" -"movl %0,%1\n\t" -"roll $16, %0\n\t" -"cmpl %0,%1\n\t" -"jne 1f\n\t" -"addl $0x00010000, %1\n\t" -"lock ; cmpxchgl %1,%2\n\t" -"1:" -"sete %b1\n\t" -"movzbl %b1,%0\n\t" - :"=&a" (tmp),"=r" (new),"+m" (lock->slock) - : - :"memory" ,"cc" ); + asmvolatile ("movl %2,%0\n\t" +"movl %0,%1\n\t" +"roll $16, %0\n\t" +"cmpl %0,%1\n\t" +"jne 1f\n\t" +"addl $0x00010000, %1\n\t" +"lock ; cmpxchgl %1,%2\n\t" +"1:" +"sete %b1\n\t" +"movzbl %b1,%0\n\t" + :"=&a" (tmp),"=r" (new),"+m" (lock->slock) + : + :"memory" ,"cc" ); return tmp; }static inline void __raw_spin_unlock (raw_spinlock_t *lock) { - __asm__ __volatile__( -UNLOCK_LOCK_PREFIX "incw %0" - :"+m" (lock->slock) - : - :"memory" ,"cc" ); + asmvolatile (UNLOCK_LOCK_PREFIX "incw %0" + :"+m" (lock->slock) + : + :"memory" ,"cc" ); }#endif
Signature · 1 line
1.5.4.rc2
Joe Perches
maintainerPATCH 117/148 include/asm-x86/srat.h: checkpatch cleanups - formatting only
In reply to Joe Perches--- include/asm-x86/srat.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
include/asm-x86/srat.h +1 -1
@@ -1,5 +1,5 @@/* -* Some of the code in this file has been gleaned from the 64 bit +* Some of the code in this file has been gleaned from the 64 bit * discontigmem support code base. * * Copyright (C) 2002, IBM Corp.
Signature · 1 line
1.5.4.rc2
Joe Perches
maintainerPATCH 118/148 include/asm-x86/string_32.h: checkpatch cleanups - formatting only
In reply to Joe Perches--- include/asm-x86/string_32.h | 141 +++++++++++++++++++++---------------------- 1 files changed, 70 insertions(+), 71 deletions(-)
include/asm-x86/string_32.h +70 -71
@@ -3,7 +3,7 @@#ifdef __KERNEL__ -/* Let gcc decide wether to inline or use the out of line functions */ +/* Let gcc decide whether to inline or use the out of line functions */ #define __HAVE_ARCH_STRCPYextern char *strcpy (char *dest,const char *src);
@@ -32,16 +32,15 @@ extern size_t strlen(const char *s); static __always_inline void *__memcpy(void *to, const void *from, size_t n) {int d0, d1, d2; - __asm__ __volatile__( -"rep ; movsl\n\t" -"movl %4,%%ecx\n\t" -"andl $3,%%ecx\n\t" -"jz 1f\n\t" -"rep ; movsb\n\t" -"1:" - :"=&c" (d0),"=&D" (d1),"=&S" (d2) - :"0" (n/4 ),"g" (n),"1" ((long )to),"2" ((long )from) - :"memory" ); + asmvolatile ("rep ; movsl\n\t" +"movl %4,%%ecx\n\t" +"andl $3,%%ecx\n\t" +"jz 1f\n\t" +"rep ; movsb\n\t" +"1:" + :"=&c" (d0),"=&D" (d1),"=&S" (d2) + :"0" (n /4 ),"g" (n),"1" ((long )to),"2" ((long )from) + :"memory" );return to; }
@@ -74,10 +73,12 @@ static __always_inline void *__constant_memcpy(void *to, const void *from, size_ *(int *)to = *(int *)from; *((char *)to +4 ) = *((char *)from +4 );return to; -case 6 : *(int *)to = *(int *)from; +case 6 : + *(int *)to = *(int *)from; *((short *)to +2 ) = *((short *)from +2 );return to; -case 8 : *(int *)to = *(int *)from; +case 8 : + *(int *)to = *(int *)from; *((int *)to +1 ) = *((int *)from +1 );return to;#endif
@@ -88,54 +89,55 @@ static __always_inline void *__constant_memcpy(void *to, const void *from, size_if (n >=5 *4 ) {/* large block: use rep prefix */ int ecx; - __asm__ __volatile__( -"rep ; movsl" - :"=&c" (ecx),"=&D" (edi),"=&S" (esi) - :"0" (n/4 ),"1" (edi),"2" (esi) - :"memory" + asmvolatile ("rep ; movsl" + :"=&c" (ecx),"=&D" (edi),"=&S" (esi) + :"0" (n /4 ),"1" (edi),"2" (esi) + :"memory" ); }else {/* small block: don't clobber ecx + smaller code */ if (n >=4 *4 ) - __asm__ __volatile__("movsl" - :"=&D" (edi),"=&S" (esi) - :"0" (edi),"1" (esi) - :"memory" ); + asmvolatile ("movsl" + :"=&D" (edi),"=&S" (esi) + :"0" (edi),"1" (esi) + :"memory" );if (n >=3 *4 ) - __asm__ __volatile__("movsl" - :"=&D" (edi),"=&S" (esi) - :"0" (edi),"1" (esi) - :"memory" ); + asmvolatile ("movsl" + :"=&D" (edi),"=&S" (esi) + :"0" (edi),"1" (esi) + :"memory" );if (n >=2 *4 ) - __asm__ __volatile__("movsl" - :"=&D" (edi),"=&S" (esi) - :"0" (edi),"1" (esi) - :"memory" ); + asmvolatile ("movsl" + :"=&D" (edi),"=&S" (esi) + :"0" (edi),"1" (esi) + :"memory" );if (n >=1 *4 ) - __asm__ __volatile__("movsl" - :"=&D" (edi),"=&S" (esi) - :"0" (edi),"1" (esi) - :"memory" ); + asmvolatile ("movsl" + :"=&D" (edi),"=&S" (esi) + :"0" (edi),"1" (esi) + :"memory" ); }switch (n %4 ) {/* tail */ case 0 :return to;case 1 : - __asm__ __volatile__("movsb" - :"=&D" (edi),"=&S" (esi) - :"0" (edi),"1" (esi) - :"memory" ); + asmvolatile ("movsb" + :"=&D" (edi),"=&S" (esi) + :"0" (edi),"1" (esi) + :"memory" );return to; -case 2 : __asm__ __volatile__("movsw" - :"=&D" (edi),"=&S" (esi) - :"0" (edi),"1" (esi) - :"memory" ); +case 2 : + asmvolatile ("movsw" + :"=&D" (edi),"=&S" (esi) + :"0" (edi),"1" (esi) + :"memory" );return to; -default : __asm__ __volatile__("movsw\n\tmovsb" - :"=&D" (edi),"=&S" (esi) - :"0" (edi),"1" (esi) - :"memory" ); +default : + asmvolatile ("movsw\n\tmovsb" + :"=&D" (edi),"=&S" (esi) + :"0" (edi),"1" (esi) + :"memory" );return to; } }
@@ -193,12 +195,11 @@ extern void *memchr(const void *cs, int c, size_t count);static inline void *__memset_generic (void *s,char c,size_t count) {int d0, d1; - __asm__ __volatile__( -"rep\n\t" -"stosb" - :"=&c" (d0),"=&D" (d1) - :"a" (c),"1" (s),"0" (count) - :"memory" ); + asmvolatile ("rep\n\t" +"stosb" + :"=&c" (d0),"=&D" (d1) + :"a" (c),"1" (s),"0" (count) + :"memory" );return s; }
@@ -213,18 +214,17 @@ static inline void *__memset_generic(void *s, char c, size_t count) static __always_inline void *__constant_c_memset(void *s, unsigned long c, size_t count) {int d0, d1; - __asm__ __volatile__( -"rep ; stosl\n\t" -"testb $2,%b3\n\t" -"je 1f\n\t" -"stosw\n" -"1:\ttestb $1,%b3\n\t" -"je 2f\n\t" -"stosb\n" -"2:" - :"=&c" (d0),"=&D" (d1) - :"a" (c),"q" (count),"0" (count/4 ),"1" ((long )s) - :"memory" ); + asmvolatile ("rep ; stosl\n\t" +"testb $2,%b3\n\t" +"je 1f\n\t" +"stosw\n" +"1:\ttestb $1,%b3\n\t" +"je 2f\n\t" +"stosb\n" +"2:" + :"=&c" (d0),"=&D" (d1) + :"a" (c),"q" (count),"0" (count/4 ),"1" ((long )s) + :"memory" );return s; }
@@ -260,13 +260,12 @@ static __always_inline void *__constant_c_and_count_memset(void *s, unsigned lonreturn s; } -#define COMMON (x) \ - __asm__ __volatile__( \ - "rep ; stosl" \ - x \ - : "=&c" (d0), "=&D" (d1) \ - : "a" (pattern), "0" (count/4), "1" ((long)s) \ - : "memory") +#define COMMON (x) \ + asm volatile("rep ; stosl" \ + x \ + : "=&c" (d0), "=&D" (d1) \ + : "a" (pattern), "0" (count/4), "1" ((long)s) \ + : "memory") {int d0, d1;
Signature · 1 line
1.5.4.rc2
Joe Perches
maintainerPATCH 119/148 include/asm-x86/string_64.h: checkpatch cleanups - formatting only
In reply to Joe Perches--- include/asm-x86/string_64.h | 66 +++++++++++++++++++++--------------------- 1 files changed, 33 insertions(+), 33 deletions(-)
include/asm-x86/string_64.h +33 -33
@@ -3,26 +3,24 @@#ifdef __KERNEL__ -/* Written 2002 by Andi Kleen */ +/* Written 2002 by Andi Kleen */ -/* Only used for special circumstances. Stolen from i386/string.h */ -static __always_inline void * -__inline_memcpy (void * to,const void * from,size_t n) +/* Only used for special circumstances. Stolen from i386/string.h */ +static __always_inline void *__inline_memcpy(void *to, const void *from, size_t n) { -unsigned long d0, d1, d2; -__asm__ __volatile__( -"rep ; movsl\n\t" -"testb $2,%b4\n\t" -"je 1f\n\t" -"movsw\n" -"1:\ttestb $1,%b4\n\t" -"je 2f\n\t" -"movsb\n" -"2:" - :"=&c" (d0),"=&D" (d1),"=&S" (d2) - :"0" (n/4 ),"q" (n),"1" ((long ) to),"2" ((long ) from) - :"memory" ); -return (to); +unsigned long d0, d1, d2; + asmvolatile ("rep ; movsl\n\t" +"testb $2,%b4\n\t" +"je 1f\n\t" +"movsw\n" +"1:\ttestb $1,%b4\n\t" +"je 2f\n\t" +"movsb\n" +"2:" + :"=&c" (d0),"=&D" (d1),"=&S" (d2) + :"0" (n /4 ),"q" (n),"1" ((long )to),"2" ((long )from) + :"memory" ); +return to; }/* Even with __builtin_ the compiler may decide to use the out of line
@@ -32,28 +30,30 @@ return (to);#if (__GNUC__ ==4 && __GNUC_MINOR__ >=3 ) || __GNUC__ >4 extern void *memcpy (void *to,const void *from,size_t len);#else -extern void *__memcpy (void *to,const void *from,size_t len); -#define memcpy (dst,src,len) \ - ({ size_t __len = (len); \ - void *__ret; \ - if (__builtin_constant_p(len) && __len >= 64) \ - __ret = __memcpy((dst),(src),__len); \ - else \ - __ret = __builtin_memcpy((dst),(src),__len); \ - __ret; }) +extern void *__memcpy (void *to,const void *from,size_t len); +#define memcpy (dst, src, len) \ +({ \ + size_t __len = (len); \ + void *__ret; \ + if (__builtin_constant_p(len) && __len >= 64) \ + __ret = __memcpy((dst), (src), __len); \ + else \ + __ret = __builtin_memcpy((dst), (src), __len); \ + __ret; \ +})#endif #define __HAVE_ARCH_MEMSETvoid *memset (void *s,int c,size_t n);#define __HAVE_ARCH_MEMMOVE -void *memmove (void * dest,const void *src,size_t count); +void *memmove (void *dest,const void *src,size_t count); -int memcmp (const void * cs,const void * ct,size_t count); -size_t strlen (const char * s); -char *strcpy (char * dest,const char *src); -char *strcat (char * dest,const char * src); -int strcmp (const char * cs,const char * ct); +int memcmp (const void *cs,const void *ct,size_t count); +size_t strlen (const char *s); +char *strcpy (char *dest,const char *src); +char *strcat (char *dest,const char *src); +int strcmp (const char *cs,const char *ct);#endif /* __KERNEL__ */
Signature · 1 line
1.5.4.rc2
Joe Perches
maintainerPATCH 120/148 include/asm-x86/suspend_32.h: checkpatch cleanups - formatting only
In reply to Joe Perches--- include/asm-x86/suspend_32.h | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-)
include/asm-x86/suspend_32.h +6 -6
@@ -10,7 +10,7 @@ static inline int arch_prepare_suspend(void) { return 0; }/* image of the saved processor state */ struct saved_context { -u16 es, fs, gs, ss; +u16 es, fs, gs, ss;unsigned long cr0, cr2, cr3, cr4;struct desc_ptr gdt;struct desc_ptr idt;
@@ -32,11 +32,11 @@ extern unsigned long saved_edi;static inline void acpi_save_register_state (unsigned long return_point) { saved_eip = return_point; - asmvolatile ("movl %%esp,%0" :"=m" (saved_esp)); - asmvolatile ("movl %%ebp,%0" :"=m" (saved_ebp)); - asmvolatile ("movl %%ebx,%0" :"=m" (saved_ebx)); - asmvolatile ("movl %%edi,%0" :"=m" (saved_edi)); - asmvolatile ("movl %%esi,%0" :"=m" (saved_esi)); + asmvolatile ("movl %%esp,%0" :"=m" (saved_esp)); + asmvolatile ("movl %%ebp,%0" :"=m" (saved_ebp)); + asmvolatile ("movl %%ebx,%0" :"=m" (saved_ebx)); + asmvolatile ("movl %%edi,%0" :"=m" (saved_edi)); + asmvolatile ("movl %%esi,%0" :"=m" (saved_esi)); }#define acpi_restore_register_state () do {} while (0)
Signature · 1 line
1.5.4.rc2
Re: [PATCH 120/148] include/asm-x86/suspend_32.h: checkpatch cleanups - formatting only
Quoted 6 lines · Joe Perches
include/asm-x86/suspend_32.h +0
-0@@ -32,11 +32,11 @@ extern unsigned long saved_edi;static inline void acpi_save_register_state (unsigned long return_point) { saved_eip = return_point;
include/asm-x86/suspend_32.h extern unsigned long saved_edi; · line 39
Quoted 10 lines
include/asm-x86/suspend_32.h +5
-5- asmvolatile ("movl %%esp,%0" :"=m" (saved_esp)); - asmvolatile ("movl %%ebp,%0" :"=m" (saved_ebp)); - asmvolatile ("movl %%ebx,%0" :"=m" (saved_ebx)); - asmvolatile ("movl %%edi,%0" :"=m" (saved_edi)); - asmvolatile ("movl %%esi,%0" :"=m" (saved_esi)); + asmvolatile ("movl %%esp,%0" :"=m" (saved_esp)); + asmvolatile ("movl %%ebp,%0" :"=m" (saved_ebp)); + asmvolatile ("movl %%ebx,%0" :"=m" (saved_ebx)); + asmvolatile ("movl %%edi,%0" :"=m" (saved_edi)); + asmvolatile ("movl %%esi,%0" :"=m" (saved_esi));
Since when "volatile" became a function?
Re: [PATCH 120/148] include/asm-x86/suspend_32.h: checkpatch cleanups - formatting only
Quoted 11 lines · Alexey Dobriyan
Quoted 10 lines · Joe Perches
include/asm-x86/suspend_32.h +0
-4@@ -32,11 +32,11 @@ extern unsigned long saved_edi;static inline void acpi_save_register_state (unsigned long return_point) { saved_eip = return_point; - asmvolatile ("movl %%esp,%0" :"=m" (saved_esp)); - asmvolatile ("movl %%ebp,%0" :"=m" (saved_ebp)); - asmvolatile ("movl %%ebx,%0" :"=m" (saved_ebx)); - asmvolatile ("movl %%edi,%0" :"=m" (saved_edi));
Quoted 8 lines
include/asm-x86/suspend_32.h extern unsigned long saved_edi; · line 39
Quoted 6 lines
include/asm-x86/suspend_32.h +5
-1- asmvolatile ("movl %%esi,%0" :"=m" (saved_esi)); + asmvolatile ("movl %%esp,%0" :"=m" (saved_esp)); + asmvolatile ("movl %%ebp,%0" :"=m" (saved_ebp)); + asmvolatile ("movl %%ebx,%0" :"=m" (saved_ebx)); + asmvolatile ("movl %%edi,%0" :"=m" (saved_edi)); + asmvolatile ("movl %%esi,%0" :"=m" (saved_esi));Since when "volatile" became a function?
Agreed, this is crack.
Joe, please find something useful to contribute. Having a few cleanup patches at the beginning of a useful series isn't an issue, but such series as posted here are utterly annoying, and by the above just plain stupid.
checkpatch.pl is a guide, not a replacement for common sense.
Joe Perches
maintainerPATCH 121/148 include/asm-x86/suspend_64.h: checkpatch cleanups - formatting only
In reply to Joe Perches--- include/asm-x86/suspend_64.h | 5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-)
include/asm-x86/suspend_64.h +2 -3
@@ -9,8 +9,7 @@#include <asm/desc.h> #include <asm/i387.h> -static inline int -arch_prepare_suspend (void ) +static inline int arch_prepare_suspend (void ) {return 0 ; }
@@ -25,7 +24,7 @@ arch_prepare_suspend(void)*/ struct saved_context {struct pt_regs regs; -u16 ds, es, fs, gs, ss; +u16 ds, es, fs, gs, ss;unsigned long gs_base, gs_kernel_base, fs_base;unsigned long cr0, cr2, cr3, cr4, cr8;unsigned long efer;
Signature · 1 line
1.5.4.rc2
Joe Perches
maintainerPATCH 122/148 include/asm-x86/swiotlb.h: checkpatch cleanups - formatting only
In reply to Joe Perches--- include/asm-x86/swiotlb.h | 28 ++++++++++++++-------------- 1 files changed, 14 insertions(+), 14 deletions(-)
include/asm-x86/swiotlb.h +14 -14
@@ -8,15 +8,15 @@extern dma_addr_t swiotlb_map_single (struct device *hwdev,void *ptr,size_t size,int dir);extern void *swiotlb_alloc_coherent (struct device *hwdev,size_t size, -dma_addr_t *dma_handle,gfp_t flags); +dma_addr_t *dma_handle,gfp_t flags);extern void swiotlb_unmap_single (struct device *hwdev,dma_addr_t dev_addr, -size_t size,int dir); +size_t size,int dir);extern void swiotlb_sync_single_for_cpu (struct device *hwdev, -dma_addr_t dev_addr, -size_t size,int dir); +dma_addr_t dev_addr, +size_t size,int dir);extern void swiotlb_sync_single_for_device (struct device *hwdev, -dma_addr_t dev_addr, -size_t size,int dir); +dma_addr_t dev_addr, +size_t size,int dir); extern void swiotlb_sync_single_range_for_cpu(struct device *hwdev, dma_addr_t dev_addr, unsigned long offset,
@@ -26,18 +26,18 @@ extern void swiotlb_sync_single_range_for_device(struct device *hwdev,unsigned long offset, size_t size, int dir);extern void swiotlb_sync_sg_for_cpu (struct device *hwdev, -struct scatterlist *sg,int nelems, -int dir); +struct scatterlist *sg,int nelems, +int dir);extern void swiotlb_sync_sg_for_device (struct device *hwdev, -struct scatterlist *sg,int nelems, -int dir); +struct scatterlist *sg,int nelems, +int dir);extern int swiotlb_map_sg (struct device *hwdev,struct scatterlist *sg, -int nents,int direction); +int nents,int direction);extern void swiotlb_unmap_sg (struct device *hwdev,struct scatterlist *sg, -int nents,int direction); +int nents,int direction);extern int swiotlb_dma_mapping_error (dma_addr_t dma_addr); -extern void swiotlb_free_coherent (struct device *hwdev,size_t size, -void *vaddr,dma_addr_t dma_handle); +extern void swiotlb_free_coherent (struct device *hwdev,size_t size, +void *vaddr,dma_addr_t dma_handle);extern int swiotlb_dma_supported (struct device *hwdev,u64 mask);extern void swiotlb_init (void );
Signature · 1 line
1.5.4.rc2
Joe Perches
maintainerPATCH 123/148 include/asm-x86/sync_bitops.h: checkpatch cleanups - formatting only
In reply to Joe Perches--- include/asm-x86/sync_bitops.h | 56 ++++++++++++++++++++-------------------- 1 files changed, 28 insertions(+), 28 deletions(-)
include/asm-x86/sync_bitops.h +28 -28
@@ -13,7 +13,7 @@* bit 0 is the LSB of addr; bit 32 is the LSB of (addr+1). */ -#define ADDR (*(volatile long *) addr) +#define ADDR (*(volatile long *)addr)/** * sync_set_bit - Atomically set a bit in memory
@@ -26,12 +26,12 @@* Note that @nr may be almost arbitrarily large; this function is not * restricted to acting on a single-word quantity. */ -static inline void sync_set_bit (int nr,volatile unsigned long * addr) +static inline void sync_set_bit (int nr,volatile unsigned long *addr) { - __asm__ __volatile__("lock; btsl %1,%0" - :"+m" (ADDR ) - :"Ir" (nr) - :"memory" ); + asmvolatile ("lock; btsl %1,%0" + :"+m" (ADDR ) + :"Ir" (nr) + :"memory" ); }/**
@@ -44,12 +44,12 @@ static inline void sync_set_bit(int nr, volatile unsigned long * addr)* you should call smp_mb__before_clear_bit() and/or smp_mb__after_clear_bit() * in order to ensure changes are visible on other processors. */ -static inline void sync_clear_bit (int nr,volatile unsigned long * addr) +static inline void sync_clear_bit (int nr,volatile unsigned long *addr) { - __asm__ __volatile__("lock; btrl %1,%0" - :"+m" (ADDR ) - :"Ir" (nr) - :"memory" ); + asmvolatile ("lock; btrl %1,%0" + :"+m" (ADDR ) + :"Ir" (nr) + :"memory" ); }/**
@@ -61,12 +61,12 @@ static inline void sync_clear_bit(int nr, volatile unsigned long * addr)* Note that @nr may be almost arbitrarily large; this function is not * restricted to acting on a single-word quantity. */ -static inline void sync_change_bit (int nr,volatile unsigned long * addr) +static inline void sync_change_bit (int nr,volatile unsigned long *addr) { - __asm__ __volatile__("lock; btcl %1,%0" - :"+m" (ADDR ) - :"Ir" (nr) - :"memory" ); + asmvolatile ("lock; btcl %1,%0" + :"+m" (ADDR ) + :"Ir" (nr) + :"memory" ); }/**
@@ -77,13 +77,13 @@ static inline void sync_change_bit(int nr, volatile unsigned long * addr)* This operation is atomic and cannot be reordered. * It also implies a memory barrier. */ -static inline int sync_test_and_set_bit (int nr,volatile unsigned long * addr) +static inline int sync_test_and_set_bit (int nr,volatile unsigned long *addr) {int oldbit; - __asm__ __volatile__("lock; btsl %2,%1\n\tsbbl %0,%0" - :"=r" (oldbit),"+m" (ADDR ) - :"Ir" (nr) :"memory" ); + asmvolatile ("lock; btsl %2,%1\n\tsbbl %0,%0" + :"=r" (oldbit),"+m" (ADDR ) + :"Ir" (nr) :"memory" );return oldbit; }
@@ -95,13 +95,13 @@ static inline int sync_test_and_set_bit(int nr, volatile unsigned long * addr)* This operation is atomic and cannot be reordered. * It also implies a memory barrier. */ -static inline int sync_test_and_clear_bit (int nr,volatile unsigned long * addr) +static inline int sync_test_and_clear_bit (int nr,volatile unsigned long *addr) {int oldbit; - __asm__ __volatile__("lock; btrl %2,%1\n\tsbbl %0,%0" - :"=r" (oldbit),"+m" (ADDR ) - :"Ir" (nr) :"memory" ); + asmvolatile ("lock; btrl %2,%1\n\tsbbl %0,%0" + :"=r" (oldbit),"+m" (ADDR ) + :"Ir" (nr) :"memory" );return oldbit; }
@@ -113,13 +113,13 @@ static inline int sync_test_and_clear_bit(int nr, volatile unsigned long * addr)* This operation is atomic and cannot be reordered. * It also implies a memory barrier. */ -static inline int sync_test_and_change_bit (int nr,volatile unsigned long * addr) +static inline int sync_test_and_change_bit (int nr,volatile unsigned long *addr) {int oldbit; - __asm__ __volatile__("lock; btcl %2,%1\n\tsbbl %0,%0" - :"=r" (oldbit),"+m" (ADDR ) - :"Ir" (nr) :"memory" ); + asmvolatile ("lock; btcl %2,%1\n\tsbbl %0,%0" + :"=r" (oldbit),"+m" (ADDR ) + :"Ir" (nr) :"memory" );return oldbit; }
Signature · 1 line
1.5.4.rc2
Joe Perches
maintainerPATCH 124/148 include/asm-x86/system.h: checkpatch cleanups - formatting only
In reply to Joe Perches--- include/asm-x86/system.h | 223 ++++++++++++++++++++++++---------------------- 1 files changed, 116 insertions(+), 107 deletions(-)
include/asm-x86/system.h +116 -107
@@ -38,35 +38,33 @@ do { \*/ \unsigned long ebx, ecx, edx, esi, edi; \ \ - asmvolatile ( \ -"pushfl \n\t" /* save flags */ \ -"pushl %%ebp \n\t" /* save EBP */ \ -"movl %%esp,%[prev_sp] \n\t" /* save ESP */ \ -"movl %[next_sp],%%esp \n\t" /* restore ESP */ \ -"movl $1f,%[prev_ip] \n\t" /* save EIP */ \ -"pushl %[next_ip] \n\t" /* restore EIP */ \ -"jmp __switch_to \n" /* regparm call */ \ -"1: \t" \ -"popl %%ebp \n\t" /* restore EBP */ \ -"popfl \n" /* restore flags */ \ + asmvolatile ("pushfl\n\t" /* save flags */ \ +"pushl %%ebp\n\t" /* save EBP */ \ +"movl %%esp,%[prev_sp]\n\t" /* save ESP */ \ +"movl %[next_sp],%%esp\n\t" /* restore ESP */ \ +"movl $1f,%[prev_ip]\n\t" /* save EIP */ \ +"pushl %[next_ip]\n\t" /* restore EIP */ \ +"jmp __switch_to\n" /* regparm call */ \ +"1:\t" \ +"popl %%ebp\n\t" /* restore EBP */ \ +"popfl\n" /* restore flags */ \ \ -/* output parameters */ \ - : [prev_sp]"=m" (prev->thread.sp), \ - [prev_ip]"=m" (prev->thread.ip), \ -"=a" (last), \ +/* output parameters */ \ + : [prev_sp]"=m" (prev->thread.sp), \ + [prev_ip]"=m" (prev->thread.ip), \ +"=a" (last), \ \ -/* clobbered output registers: */ \ -"=b" (ebx),"=c" (ecx),"=d" (edx), \ -"=S" (esi),"=D" (edi) \ - \ -/* input parameters: */ \ - : [next_sp]"m" (next->thread.sp), \ - [next_ip]"m" (next->thread.ip), \ - \ -/* regparm parameters for __switch_to(): */ \ - [prev]"a" (prev), \ - [next]"d" (next) \ - ); \ +/* clobbered output registers: */ \ +"=b" (ebx),"=c" (ecx),"=d" (edx), \ +"=S" (esi),"=D" (edi) \ + \ +/* input parameters: */ \ + : [next_sp]"m" (next->thread.sp), \ + [next_ip]"m" (next->thread.ip), \ + \ +/* regparm parameters for __switch_to(): */ \ + [prev]"a" (prev), \ + [next]"d" (next)); \ }while (0 )/*
@@ -81,66 +79,77 @@ do { \#define SAVE_CONTEXT "pushf ; pushq %%rbp ; movq %%rsi,%%rbp\n\t"#define RESTORE_CONTEXT "movq %%rbp,%%rsi ; popq %%rbp ; popf\t" -#define __EXTRA_CLOBBER \ - , "rcx", "rbx", "rdx", "r8", "r9", "r10", "r11", \ - "r12", "r13", "r14", "r15" +#define __EXTRA_CLOBBER \ + , "rcx", "rbx", "rdx", "r8", "r9", "r10", "r11" \ + , "r12", "r13", "r14", "r15"/* Save restore flags to clear handle leaking NT */ #define switch_to (prev, next, last) \ - asm volatile(SAVE_CONTEXT \ - "movq %%rsp,%P[threadrsp](%[prev])\n\t"/* save RSP */ \ - "movq %P[threadrsp](%[next]),%%rsp\n\t" /* restore RSP */ \ - "call __switch_to\n\t" \ - ".globl thread_return\n" \ - "thread_return:\n\t" \ - "movq %%gs:%P[pda_pcurrent],%%rsi\n\t" \ - "movq %P[task_canary](%%rsi),%%r8\n\t" \ - "movq %%r8,%%gs:%P[pda_canary]\n\t" \ - "movq %P[thread_info](%%rsi),%%r8\n\t" \ - LOCK_PREFIX "btr %[tif_fork],%P[ti_flags](%%r8)\n\t" \ - "movq %%rax,%%rdi\n\t" \ - "jc ret_from_fork\n\t" \ - RESTORE_CONTEXT \ - : "=a" (last) \ -: [next] " S " (next), [prev] " D " (prev), \ -[threadrsp] " i" (offsetof(struct task_struct, thread.sp)), \ -[ti_flags] " i" (offsetof(struct thread_info, flags)), \ -[tif_fork] " i" (TIF_FORK), \ -[thread_info] " i" (offsetof(struct task_struct, stack)), \ -[task_canary] " i" (offsetof(struct task_struct, stack_canary)),\ -[pda_pcurrent] " i" (offsetof(struct x8664_pda, pcurrent)), \ -[pda_canary] " i" (offsetof(struct x8664_pda, stack_canary))\ - : "memory", "cc" __EXTRA_CLOBBER) + asm volatile(SAVE_CONTEXT \ + "movq %%rsp,%P[threadrsp](%[prev])\n\t"/* save RSP */ \ + "movq %P[threadrsp](%[next]),%%rsp\n\t" /* restore RSP */ \ + "call __switch_to\n\t" \ + ".globl thread_return\n" \ + "thread_return:\n\t" \ + "movq %%gs:%P[pda_pcurrent],%%rsi\n\t" \ + "movq %P[task_canary](%%rsi),%%r8\n\t" \ + "movq %%r8,%%gs:%P[pda_canary]\n\t" \ + "movq %P[thread_info](%%rsi),%%r8\n\t" \ + LOCK_PREFIX "btr %[tif_fork],%P[ti_flags](%%r8)\n\t" \ + "movq %%rax,%%rdi\n\t" \ + "jc ret_from_fork\n\t" \ + RESTORE_CONTEXT \ + : "=a" (last) \ +: [next] " S " (next), [prev] " D " (prev), \ +[threadrsp] \ +" i" (offsetof(struct task_struct, thread.sp)), \ +[ti_flags] \ +" i" (offsetof(struct thread_info, flags)), \ +[tif_fork] \ +" i" (TIF_FORK), \ +[thread_info] \ +" i" (offsetof(struct task_struct, stack)), \ +[task_canary] \ +" i" (offsetof(struct task_struct, stack_canary)), \ +[pda_pcurrent] \ +" i" (offsetof(struct x8664_pda, pcurrent)), \ +[pda_canary] \ +" i" (offsetof(struct x8664_pda, stack_canary)) \ + : "memory", "cc" __EXTRA_CLOBBER) #endif #ifdef __KERNEL__ -#define _set_base(addr, base) do { unsigned long __pr; \ -__asm__ __volatile__ ("movw %%dx,%1\n\t" \ -"rorl $16,%%edx\n\t" \ -"movb %%dl,%2\n\t" \ -"movb %%dh,%3" \ - :"=&d" (__pr) \ - :"m" (*((addr)+2 )), \ -"m" (*((addr)+4 )), \ -"m" (*((addr)+7 )), \ -"0" (base) \ - ); } while (0) - -#define _set_limit (addr, limit) do { unsigned long __lr; \ -__asm__ __volatile__ ("movw %%dx,%1\n\t" \ - "rorl $16,%%edx\n\t" \ - "movb %2,%%dh\n\t" \ - "andb $0xf0,%%dh\n\t" \ - "orb %%dh,%%dl\n\t" \ - "movb %%dl,%2" \ - :"=&d" (__lr) \ - :"m" (*(addr)), \ - "m" (*((addr)+6)), \ - "0" (limit) \ - ); } while (0) - -#define set_base (ldt, base) _set_base(((char *)&(ldt)) , (base)) -#define set_limit (ldt, limit) _set_limit(((char *)&(ldt)) , ((limit)-1)) +#define _set_base(addr, base) \ +do { \ +unsigned long __pr; \ + asmvolatile ("movw %%dx,%1\n\t" \ +"rorl $16,%%edx\n\t" \ +"movb %%dl,%2\n\t" \ +"movb %%dh,%3" \ + :"=&d" (__pr) \ + :"m" (*((addr)+2 )), \ +"m" (*((addr)+4 )), \ +"m" (*((addr)+7 )), \ +"0" (base)); \ +} while (0) + +#define _set_limit (addr, limit) \ +do { \ + unsigned long __lr; \ + asm volatile ("movw %%dx,%1\n\t" \ + "rorl $16,%%edx\n\t" \ + "movb %2,%%dh\n\t" \ + "andb $0xf0,%%dh\n\t" \ + "orb %%dh,%%dl\n\t" \ + "movb %%dl,%2" \ + : "=&d" (__lr) \ + : "m" (*(addr)), \ + "m" (*((addr)+6)), \ + "0" (limit)); \ +} while (0) + +#define set_base (ldt, base) _set_base(((char *)&(ldt)), (base)) +#define set_limit (ldt, limit) _set_limit(((char *)&(ldt)), ((limit) - 1))extern void load_gs_index (unsigned );
@@ -150,35 +159,34 @@ extern void load_gs_index(unsigned);*/ #define loadsegment (seg, value) \ asm volatile("\n" \ - "1:\t" \ - "movl %k0,%%" #seg "\n" \ - "2:\n" \ - ".section .fixup,\"ax\"\n" \ - "3:\t" \ - "movl %k1, %%" #seg "\n\t" \ - "jmp 2b\n" \ - ".previous\n" \ - _ASM_EXTABLE(1b,3b) \ - : :"r" (value), "r" (0)) + "1:\t" \ + "movl %k0,%%" #seg "\n" \ + "2:\n" \ + ".section .fixup,\"ax\"\n" \ + "3:\t" \ + "movl %k1, %%" #seg "\n\t" \ + "jmp 2b\n" \ + ".previous\n" \ + _ASM_EXTABLE(1b,3b) \ + : :"r" (value), "r" (0))/* * Save a segment register away */ -#define savesegment (seg, value) \ +#define savesegment (seg, value) \ asm volatile("mov %%" #seg ",%0":"=rm" (value))static inline unsigned long get_limit (unsigned long segment) {unsigned long __limit; - __asm__("lsll %1,%0" - :"=r" (__limit):"r" (segment)); -return __limit+1 ; + asm("lsll %1,%0" :"=r" (__limit) :"r" (segment)); +return __limit +1 ; }static inline void native_clts (void ) { - asmvolatile ("clts" ); + asmvolatile ("clts" ); }/*
@@ -193,43 +201,43 @@ static unsigned long __force_order;static inline unsigned long native_read_cr0 (void ) {unsigned long val; - asmvolatile ("mov %%cr0,%0\n\t" :"=r" (val),"=m" (__force_order)); + asmvolatile ("mov %%cr0,%0\n\t" :"=r" (val),"=m" (__force_order));return val; }static inline void native_write_cr0 (unsigned long val) { - asmvolatile ("mov %0,%%cr0" : :"r" (val),"m" (__force_order)); + asmvolatile ("mov %0,%%cr0" : :"r" (val),"m" (__force_order)); }static inline unsigned long native_read_cr2 (void ) {unsigned long val; - asmvolatile ("mov %%cr2,%0\n\t" :"=r" (val),"=m" (__force_order)); + asmvolatile ("mov %%cr2,%0\n\t" :"=r" (val),"=m" (__force_order));return val; }static inline void native_write_cr2 (unsigned long val) { - asmvolatile ("mov %0,%%cr2" : :"r" (val),"m" (__force_order)); + asmvolatile ("mov %0,%%cr2" : :"r" (val),"m" (__force_order)); }static inline unsigned long native_read_cr3 (void ) {unsigned long val; - asmvolatile ("mov %%cr3,%0\n\t" :"=r" (val),"=m" (__force_order)); + asmvolatile ("mov %%cr3,%0\n\t" :"=r" (val),"=m" (__force_order));return val; }static inline void native_write_cr3 (unsigned long val) { - asmvolatile ("mov %0,%%cr3" : :"r" (val),"m" (__force_order)); + asmvolatile ("mov %0,%%cr3" : :"r" (val),"m" (__force_order)); }static inline unsigned long native_read_cr4 (void ) {unsigned long val; - asmvolatile ("mov %%cr4,%0\n\t" :"=r" (val),"=m" (__force_order)); + asmvolatile ("mov %%cr4,%0\n\t" :"=r" (val),"=m" (__force_order));return val; }
@@ -241,7 +249,7 @@ static inline unsigned long native_read_cr4_safe(void)#ifdef CONFIG_X86_32 asm volatile("1: mov %%cr4, %0\n" "2:\n" - _ASM_EXTABLE(1b,2b) + _ASM_EXTABLE(1b, 2b) : "=r" (val), "=m" (__force_order) : "0" (0));#else val =native_read_cr4 ();
@@ -251,7 +259,7 @@ static inline unsigned long native_read_cr4_safe(void)static inline void native_write_cr4 (unsigned long val) { - asmvolatile ("mov %0,%%cr4" : :"r" (val),"m" (__force_order)); + asmvolatile ("mov %0,%%cr4" : :"r" (val),"m" (__force_order)); }#ifdef CONFIG_X86_64
@@ -272,6 +280,7 @@ static inline void native_wbinvd(void) { asmvolatile ("wbinvd" : : :"memory" ); } +#ifdef CONFIG_PARAVIRT #include <asm/paravirt.h> #else
@@ -304,7 +313,7 @@ static inline void clflush(volatile void *__p) asmvolatile ("clflush %0" :"+m" (*(volatile char __force *)__p)); } -#define nop () __asm__ __volatile__ ("nop") +#define nop () asm volatile ("nop")void disable_hlt (void );void enable_hlt (void );
@@ -403,7 +412,7 @@ void default_idle(void);# define smp_wmb () barrier()#endif #define smp_read_barrier_depends () read_barrier_depends() -#define set_mb (var, value) do { (void) xchg(&var, value); } while (0) +#define set_mb (var, value) do { (void)xchg(&var, value); } while (0)#else #define smp_mb () barrier()#define smp_rmb () barrier()
Signature · 1 line
1.5.4.rc2
Joe Perches
maintainerPATCH 125/148 include/asm-x86/tce.h: checkpatch cleanups - formatting only
In reply to Joe Perches--- include/asm-x86/tce.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
include/asm-x86/tce.h +1 -1
@@ -39,7 +39,7 @@ struct iommu_table;#define TCE_RPN_MASK 0x0000fffffffff000ULLextern void tce_build (struct iommu_table *tbl,unsigned long index, -unsigned int npages,unsigned long uaddr,int direction); +unsigned int npages,unsigned long uaddr,int direction);extern void tce_free (struct iommu_table *tbl,long index,unsigned int npages);extern void *__init alloc_tce_table (void );extern void __init free_tce_table (void *tbl);
Signature · 1 line
1.5.4.rc2
Joe Perches
maintainerPATCH 126/148 include/asm-x86/termios.h: checkpatch cleanups - formatting only
In reply to Joe Perches--- include/asm-x86/termios.h | 9 +++++---- 1 files changed, 5 insertions(+), 4 deletions(-)
include/asm-x86/termios.h +5 -4
@@ -54,10 +54,11 @@ struct termio {/* * Translate a "termio" structure into a "termios". Ugh. */ -#define SET_LOW_TERMIOS_BITS (termios, termio, x) { \ - unsigned short __tmp; \ - get_user(__tmp,&(termio)->x); \ - *(unsigned short *) &(termios)->x = __tmp; \ +#define SET_LOW_TERMIOS_BITS (termios, termio, x) \ +{ \ + unsigned short __tmp; \ + get_user(__tmp, &(termio)->x); \ + *(unsigned short *)&(termios)->x = __tmp; \ } static inline int user_termio_to_kernel_termios(struct ktermios *termios,
Signature · 1 line
1.5.4.rc2
Alan Cox
NAKRe: [PATCH 126/148] include/asm-x86/termios.h: checkpatch cleanups - formatting only
Quoted 2 lines · On Sun, 23 Mar 2008 01:03:41 -0700 Joe Perches <[email protected]> wrote:
Signed-off-by Joe Perches <[email protected]>
NAK - all serial/tty changes in this set.
Please leave the tty code alone for now (all of it). You are getting in the way of far more important work and the changes you make add no real value so should be done when nothing else is happening
Joe Perches
maintainerPATCH 127/148 include/asm-x86/thread_info_32.h: checkpatch cleanups - formatting only
In reply to Joe Perches--- include/asm-x86/thread_info_32.h | 88 ++++++++++++++++++++------------------ 1 files changed, 46 insertions(+), 42 deletions(-)
include/asm-x86/thread_info_32.h +46 -42
@@ -20,7 +20,8 @@* low level task data that entry.S needs immediate access to * - this struct should fit entirely inside of one cache line * - this struct shares the supervisor stack pages -* - if the contents of this structure are changed, the assembly constants must also be changed +* - if the contents of this structure are changed, +* the assembly constants must also be changed */ #ifndef __ASSEMBLY__
@@ -30,18 +31,16 @@ struct thread_info {unsigned long flags;/* low level flags */ unsigned long status;/* thread-synchronous flags */ __u32 cpu;/* current CPU */ -int preempt_count;/* 0 => preemptable, <0 => BUG */ - - +int preempt_count;/* 0 => preemptable, +<0 => BUG */ mm_segment_t addr_limit;/* thread address space: -0-0xBFFFFFFF for user-thead -0-0xFFFFFFFF for kernel-thread +0-0xBFFFFFFF user-thread +0-0xFFFFFFFF kernel-thread */ void *sysenter_return;struct restart_block restart_block; - -unsigned long previous_esp;/* ESP of the previous stack in case -of nested (IRQ) stacks +unsigned long previous_esp;/* ESP of the previous stack in +case of nested (IRQ) stacks */ __u8 supervisor_stack[0 ]; };
@@ -90,22 +89,23 @@ register unsigned long current_stack_pointer asm("esp") __used;/* how to get the thread information struct from C */ static inline struct thread_info *current_thread_info (void ) { -return (struct thread_info *)(current_stack_pointer & ~(THREAD_SIZE -1 )); +return (struct thread_info *) + (current_stack_pointer & ~(THREAD_SIZE -1 )); }/* thread information allocation */ #ifdef CONFIG_DEBUG_STACK_USAGE -#define alloc_thread_info (tsk) ((struct thread_info *) \ - __get_free_pages(GFP_KERNEL| __GFP_ZERO, get_order(THREAD_SIZE))) +#define alloc_thread_info (tsk) ((struct thread_info *) \ + __get_free_pages(GFP_KERNEL | __GFP_ZERO, get_order(THREAD_SIZE)))#else -#define alloc_thread_info (tsk) ((struct thread_info *) \ +#define alloc_thread_info (tsk) ((struct thread_info *) \ __get_free_pages(GFP_KERNEL, get_order(THREAD_SIZE)))#endif #else /* !__ASSEMBLY__ */ /* how to get the thread information struct from ASM */ -#define GET_THREAD_INFO (reg) \ +#define GET_THREAD_INFO (reg) \ movl $-THREAD_SIZE, reg; \ andl %esp, reg
@@ -117,14 +117,16 @@ static inline struct thread_info *current_thread_info(void)/* * thread information flags -* - these are process state flags that various assembly files may need to access +* - these are process state flags that various +* assembly files may need to access * - pending work-to-be-done flags are in LSW * - other flags in MSW */ #define TIF_SYSCALL_TRACE 0/* syscall trace active */ #define TIF_SIGPENDING 1/* signal pending */ #define TIF_NEED_RESCHED 2/* rescheduling necessary */ -#define TIF_SINGLESTEP 3/* restore singlestep on return to user mode */ +#define TIF_SINGLESTEP 3/* restore singlestep on return to +user mode */ #define TIF_IRET 4/* return with iret */ #define TIF_SYSCALL_EMU 5/* syscall emulation active */ #define TIF_SYSCALL_AUDIT 6/* syscall auditing active */
@@ -141,36 +143,36 @@ static inline struct thread_info *current_thread_info(void)#define TIF_DS_AREA_MSR 23/* uses thread_struct.ds_area_msr */ #define TIF_BTS_TRACE_TS 24/* record scheduling event timestamps */ -#define _TIF_SYSCALL_TRACE (1<<TIF_SYSCALL_TRACE) -#define _TIF_SIGPENDING (1<<TIF_SIGPENDING) -#define _TIF_NEED_RESCHED (1<<TIF_NEED_RESCHED) -#define _TIF_SINGLESTEP (1<<TIF_SINGLESTEP) -#define _TIF_IRET (1<<TIF_IRET) -#define _TIF_SYSCALL_EMU (1<<TIF_SYSCALL_EMU) -#define _TIF_SYSCALL_AUDIT (1<<TIF_SYSCALL_AUDIT) -#define _TIF_SECCOMP (1<<TIF_SECCOMP) -#define _TIF_RESTORE_SIGMASK (1<<TIF_RESTORE_SIGMASK) -#define _TIF_HRTICK_RESCHED (1<<TIF_HRTICK_RESCHED) -#define _TIF_DEBUG (1<<TIF_DEBUG) -#define _TIF_IO_BITMAP (1<<TIF_IO_BITMAP) -#define _TIF_FREEZE (1<<TIF_FREEZE) -#define _TIF_NOTSC (1<<TIF_NOTSC) -#define _TIF_FORCED_TF (1<<TIF_FORCED_TF) -#define _TIF_DEBUGCTLMSR (1<<TIF_DEBUGCTLMSR) -#define _TIF_DS_AREA_MSR (1<<TIF_DS_AREA_MSR) -#define _TIF_BTS_TRACE_TS (1<<TIF_BTS_TRACE_TS) +#define _TIF_SYSCALL_TRACE (1 << TIF_SYSCALL_TRACE) +#define _TIF_SIGPENDING (1 << TIF_SIGPENDING) +#define _TIF_NEED_RESCHED (1 << TIF_NEED_RESCHED) +#define _TIF_SINGLESTEP (1 << TIF_SINGLESTEP) +#define _TIF_IRET (1 << TIF_IRET) +#define _TIF_SYSCALL_EMU (1 << TIF_SYSCALL_EMU) +#define _TIF_SYSCALL_AUDIT (1 << TIF_SYSCALL_AUDIT) +#define _TIF_SECCOMP (1 << TIF_SECCOMP) +#define _TIF_RESTORE_SIGMASK (1 << TIF_RESTORE_SIGMASK) +#define _TIF_HRTICK_RESCHED (1 << TIF_HRTICK_RESCHED) +#define _TIF_DEBUG (1 << TIF_DEBUG) +#define _TIF_IO_BITMAP (1 << TIF_IO_BITMAP) +#define _TIF_FREEZE (1 << TIF_FREEZE) +#define _TIF_NOTSC (1 << TIF_NOTSC) +#define _TIF_FORCED_TF (1 << TIF_FORCED_TF) +#define _TIF_DEBUGCTLMSR (1 << TIF_DEBUGCTLMSR) +#define _TIF_DS_AREA_MSR (1 << TIF_DS_AREA_MSR) +#define _TIF_BTS_TRACE_TS (1 << TIF_BTS_TRACE_TS)/* work to do on interrupt/exception return */ -#define _TIF_WORK_MASK \ - (0x0000FFFF & ~(_TIF_SYSCALL_TRACE | _TIF_SYSCALL_AUDIT | \ - _TIF_SECCOMP | _TIF_SYSCALL_EMU)) +#define _TIF_WORK_MASK \ + (0x0000FFFF & ~(_TIF_SYSCALL_TRACE | _TIF_SYSCALL_AUDIT | \ + _TIF_SECCOMP | _TIF_SYSCALL_EMU))/* work to do on any return to u-space */ #define _TIF_ALLWORK_MASK (0x0000FFFF & ~_TIF_SECCOMP)/* flags to check in __switch_to() */ -#define _TIF_WORK_CTXSW \ - (_TIF_IO_BITMAP | _TIF_NOTSC | _TIF_DEBUGCTLMSR | \ - _TIF_DS_AREA_MSR | _TIF_BTS_TRACE_TS) +#define _TIF_WORK_CTXSW \ + (_TIF_IO_BITMAP | _TIF_NOTSC | _TIF_DEBUGCTLMSR | \ + _TIF_DS_AREA_MSR | _TIF_BTS_TRACE_TS)#define _TIF_WORK_CTXSW_PREV _TIF_WORK_CTXSW#define _TIF_WORK_CTXSW_NEXT (_TIF_WORK_CTXSW | _TIF_DEBUG)
@@ -182,8 +184,10 @@ static inline struct thread_info *current_thread_info(void)* ever touches our thread-synchronous status, so we don't * have to worry about atomic accesses. */ -#define TS_USEDFPU 0x0001/* FPU was used by this task this quantum (SMP) */ -#define TS_POLLING 0x0002/* True if in idle loop and not sleeping */ +#define TS_USEDFPU 0x0001/* FPU was used by this task +this quantum (SMP) */ +#define TS_POLLING 0x0002/* True if in idle loop +and not sleeping */ #define tsk_is_polling (t) (task_thread_info(t)->status & TS_POLLING)
Signature · 1 line
1.5.4.rc2
Joe Perches
maintainerPATCH 128/148 include/asm-x86/thread_info_64.h: checkpatch cleanups - formatting only
In reply to Joe Perches--- include/asm-x86/thread_info_64.h | 78 ++++++++++++++++++++------------------ 1 files changed, 41 insertions(+), 37 deletions(-)
include/asm-x86/thread_info_64.h +41 -37
@@ -29,9 +29,9 @@ struct thread_info {__u32 flags;/* low level flags */ __u32 status;/* thread synchronous flags */ __u32 cpu;/* current CPU */ -int preempt_count;/* 0 => preemptable, <0 => BUG */ - -mm_segment_t addr_limit; +int preempt_count;/* 0 => preemptable, +<0 => BUG */ +mm_segment_t addr_limit;struct restart_block restart_block;#ifdef CONFIG_IA32_EMULATION void __user *sysenter_return;
@@ -61,17 +61,17 @@ struct thread_info {#define init_stack (init_thread_union.stack)static inline struct thread_info *current_thread_info (void ) -{ +{struct thread_info *ti; ti = (void *)(read_pda (kernelstack) +PDA_STACKOFFSET -THREAD_SIZE ); -return ti; +return ti; }/* do not use in interrupt context */ static inline struct thread_info *stack_thread_info (void ) {struct thread_info *ti; - __asm__("andq %%rsp,%0; " :"=r" (ti) :"0" (~(THREAD_SIZE -1 ))); + asm("andq %%rsp,%0; " :"=r" (ti) :"0" (~(THREAD_SIZE -1 )));return ti; }
@@ -82,8 +82,8 @@ static inline struct thread_info *stack_thread_info(void)#define THREAD_FLAGS GFP_KERNEL#endif -#define alloc_thread_info (tsk) \ - ((struct thread_info *) __get_free_pages(THREAD_FLAGS, THREAD_ORDER)) +#define alloc_thread_info (tsk) \ + ((struct thread_info *)__get_free_pages(THREAD_FLAGS, THREAD_ORDER))#else /* !__ASSEMBLY__ */
@@ -96,7 +96,8 @@ static inline struct thread_info *stack_thread_info(void)/* * thread information flags -* - these are process state flags that various assembly files may need to access +* - these are process state flags that various assembly files +* may need to access * - pending work-to-be-done flags are in LSW * - other flags in MSW * Warning: layout of LSW is hardcoded in entry.S
@@ -112,7 +113,7 @@ static inline struct thread_info *stack_thread_info(void)#define TIF_MCE_NOTIFY 10/* notify userspace of an MCE */ #define TIF_HRTICK_RESCHED 11/* reprogram hrtick timer */ /* 16 free */ -#define TIF_IA32 17/* 32bit process */ +#define TIF_IA32 17/* 32bit process */ #define TIF_FORK 18/* ret_from_fork */ #define TIF_ABI_PENDING 19#define TIF_MEMDIE 20
@@ -124,39 +125,40 @@ static inline struct thread_info *stack_thread_info(void)#define TIF_DS_AREA_MSR 26/* uses thread_struct.ds_area_msr */ #define TIF_BTS_TRACE_TS 27/* record scheduling event timestamps */ -#define _TIF_SYSCALL_TRACE (1<<TIF_SYSCALL_TRACE) -#define _TIF_SIGPENDING (1<<TIF_SIGPENDING) -#define _TIF_SINGLESTEP (1<<TIF_SINGLESTEP) -#define _TIF_NEED_RESCHED (1<<TIF_NEED_RESCHED) -#define _TIF_IRET (1<<TIF_IRET) -#define _TIF_SYSCALL_AUDIT (1<<TIF_SYSCALL_AUDIT) -#define _TIF_SECCOMP (1<<TIF_SECCOMP) -#define _TIF_RESTORE_SIGMASK (1<<TIF_RESTORE_SIGMASK) -#define _TIF_MCE_NOTIFY (1<<TIF_MCE_NOTIFY) -#define _TIF_HRTICK_RESCHED (1<<TIF_HRTICK_RESCHED) -#define _TIF_IA32 (1<<TIF_IA32) -#define _TIF_FORK (1<<TIF_FORK) -#define _TIF_ABI_PENDING (1<<TIF_ABI_PENDING) -#define _TIF_DEBUG (1<<TIF_DEBUG) -#define _TIF_IO_BITMAP (1<<TIF_IO_BITMAP) -#define _TIF_FREEZE (1<<TIF_FREEZE) -#define _TIF_FORCED_TF (1<<TIF_FORCED_TF) -#define _TIF_DEBUGCTLMSR (1<<TIF_DEBUGCTLMSR) -#define _TIF_DS_AREA_MSR (1<<TIF_DS_AREA_MSR) -#define _TIF_BTS_TRACE_TS (1<<TIF_BTS_TRACE_TS) +#define _TIF_SYSCALL_TRACE (1 << TIF_SYSCALL_TRACE) +#define _TIF_SIGPENDING (1 << TIF_SIGPENDING) +#define _TIF_SINGLESTEP (1 << TIF_SINGLESTEP) +#define _TIF_NEED_RESCHED (1 << TIF_NEED_RESCHED) +#define _TIF_IRET (1 << TIF_IRET) +#define _TIF_SYSCALL_AUDIT (1 << TIF_SYSCALL_AUDIT) +#define _TIF_SECCOMP (1 << TIF_SECCOMP) +#define _TIF_RESTORE_SIGMASK (1 << TIF_RESTORE_SIGMASK) +#define _TIF_MCE_NOTIFY (1 << TIF_MCE_NOTIFY) +#define _TIF_HRTICK_RESCHED (1 << TIF_HRTICK_RESCHED) +#define _TIF_IA32 (1 << TIF_IA32) +#define _TIF_FORK (1 << TIF_FORK) +#define _TIF_ABI_PENDING (1 << TIF_ABI_PENDING) +#define _TIF_DEBUG (1 << TIF_DEBUG) +#define _TIF_IO_BITMAP (1 << TIF_IO_BITMAP) +#define _TIF_FREEZE (1 << TIF_FREEZE) +#define _TIF_FORCED_TF (1 << TIF_FORCED_TF) +#define _TIF_DEBUGCTLMSR (1 << TIF_DEBUGCTLMSR) +#define _TIF_DS_AREA_MSR (1 << TIF_DS_AREA_MSR) +#define _TIF_BTS_TRACE_TS (1 << TIF_BTS_TRACE_TS)/* work to do on interrupt/exception return */ -#define _TIF_WORK_MASK \ - (0x0000FFFF & ~(_TIF_SYSCALL_TRACE|_TIF_SYSCALL_AUDIT|_TIF_SINGLESTEP|_TIF_SECCOMP)) +#define _TIF_WORK_MASK \ + (0x0000FFFF & \ + ~(_TIF_SYSCALL_TRACE|_TIF_SYSCALL_AUDIT|_TIF_SINGLESTEP|_TIF_SECCOMP))/* work to do on any return to user space */ #define _TIF_ALLWORK_MASK (0x0000FFFF & ~_TIF_SECCOMP) -#define _TIF_DO_NOTIFY_MASK \ +#define _TIF_DO_NOTIFY_MASK \ (_TIF_SIGPENDING|_TIF_SINGLESTEP|_TIF_MCE_NOTIFY|_TIF_HRTICK_RESCHED)/* flags to check in __switch_to() */ -#define _TIF_WORK_CTXSW \ - (_TIF_IO_BITMAP|_TIF_DEBUGCTLMSR|_TIF_DS_AREA_MSR|_TIF_BTS_TRACE_TS) +#define _TIF_WORK_CTXSW \ + (_TIF_IO_BITMAP|_TIF_DEBUGCTLMSR|_TIF_DS_AREA_MSR|_TIF_BTS_TRACE_TS)#define _TIF_WORK_CTXSW_PREV _TIF_WORK_CTXSW#define _TIF_WORK_CTXSW_NEXT (_TIF_WORK_CTXSW|_TIF_DEBUG)
@@ -169,9 +171,11 @@ static inline struct thread_info *stack_thread_info(void)* ever touches our thread-synchronous status, so we don't * have to worry about atomic accesses. */ -#define TS_USEDFPU 0x0001/* FPU was used by this task this quantum (SMP) */ +#define TS_USEDFPU 0x0001/* FPU was used by this task +this quantum (SMP) */ #define TS_COMPAT 0x0002/* 32bit syscall active */ -#define TS_POLLING 0x0004/* true if in idle loop and not sleeping */ +#define TS_POLLING 0x0004/* true if in idle loop +and not sleeping */ #define tsk_is_polling (t) (task_thread_info(t)->status & TS_POLLING)
Signature · 1 line
1.5.4.rc2
Joe Perches
maintainerPATCH 129/148 include/asm-x86/thread_info.h: checkpatch cleanups - formatting only
In reply to Joe Perches--- include/asm-x86/thread_info.h | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
include/asm-x86/thread_info.h +2 -1
@@ -8,6 +8,7 @@#ifndef __ASSEMBLY__extern void arch_task_cache_init (void );extern void free_thread_info (struct thread_info *ti); -extern int arch_dup_task_struct (struct task_struct *dst,struct task_struct *src); +extern int arch_dup_task_struct (struct task_struct *dst, +struct task_struct *src);#endif #endif /* _ASM_X86_THREAD_INFO_H */
Signature · 1 line
1.5.4.rc2
Joe Perches
maintainerPATCH 130/148 include/asm-x86/tlbflush.h: checkpatch cleanups - formatting only
In reply to Joe Perches--- include/asm-x86/tlbflush.h | 5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-)
include/asm-x86/tlbflush.h +2 -3
@@ -32,7 +32,7 @@ static inline void __native_flush_tlb_global(void)static inline void __native_flush_tlb_single (unsigned long addr) { - __asm__ __volatile__("invlpg (%0)" ::"r" (addr) :"memory" ); + asmvolatile ("invlpg (%0)" ::"r" (addr) :"memory" ); }static inline void __flush_tlb_all (void )
@@ -134,8 +134,7 @@ void native_flush_tlb_others(const cpumask_t *cpumask, struct mm_struct *mm,#define TLBSTATE_LAZY 2 #ifdef CONFIG_X86_32 -struct tlb_state -{ +struct tlb_state { struct mm_struct *active_mm; int state; char __cacheline_padding[L1_CACHE_BYTES-8];
Signature · 1 line
1.5.4.rc2
Joe Perches
maintainerPATCH 131/148 include/asm-x86/topology.h: checkpatch cleanups - formatting only
In reply to Joe Perches--- include/asm-x86/topology.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
include/asm-x86/topology.h +1 -1
@@ -72,7 +72,7 @@ static inline int cpu_to_node(int cpu)#ifdef CONFIG_DEBUG_PER_CPU_MAPS if (x86_cpu_to_node_map_early_ptr) {printk ("KERN_NOTICE cpu_to_node(%d): usage too early!\n" , - (int )cpu); + (int )cpu);dump_stack ();return ((int *)x86_cpu_to_node_map_early_ptr)[cpu]; }
Signature · 1 line
1.5.4.rc2
Joe Perches
maintainerPATCH 132/148 include/asm-x86/tsc.h: checkpatch cleanups - formatting only
In reply to Joe Perches--- include/asm-x86/tsc.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
include/asm-x86/tsc.h +1 -1
@@ -42,7 +42,7 @@ static inline cycles_t vget_cycles(void)if (!cpu_has_tsc)return 0 ;#endif -return (cycles_t )__native_read_tsc (); +return (cycles_t )__native_read_tsc (); }extern void tsc_init (void );
Signature · 1 line
1.5.4.rc2
Joe Perches
maintainerPATCH 133/148 include/asm-x86/uaccess_32.h: checkpatch cleanups - formatting only
In reply to Joe Perches--- include/asm-x86/uaccess_32.h | 316 +++++++++++++++++++++++++----------------- 1 files changed, 187 insertions(+), 129 deletions(-)
include/asm-x86/uaccess_32.h +187 -129
@@ -32,7 +32,7 @@#define get_fs () (current_thread_info()->addr_limit)#define set_fs (x) (current_thread_info()->addr_limit = (x)) -#define segment_eq (a,b) ((a).seg == (b).seg) +#define segment_eq (a, b) ((a).seg == (b).seg)/* * movsl can be slow when source and dest are not both 8-byte aligned
@@ -43,7 +43,9 @@ extern struct movsl_mask { } ____cacheline_aligned_in_smp movsl_mask;#endif -#define __addr_ok (addr) ((unsigned long __force)(addr) < (current_thread_info()->addr_limit.seg)) +#define __addr_ok (addr) \ + ((unsigned long __force)(addr) < \ + (current_thread_info()->addr_limit.seg))/* * Test whether a block of memory is a valid user space address.
@@ -54,13 +56,16 @@ extern struct movsl_mask {* * This needs 33-bit arithmetic. We have a carry... */ -#define __range_ok (addr,size) ({ \ - unsigned long flag,roksum; \ - __chk_user_ptr(addr); \ - asm("addl %3,%1 ; sbbl %0,%0; cmpl %1,%4; sbbl $0,%0" \ - :"=&r" (flag), "=r" (roksum) \ - :"1" (addr),"g" ((int)(size)),"rm" (current_thread_info()->addr_limit.seg)); \ - flag; }) +#define __range_ok (addr, size) \ +({ \ + unsigned long flag, roksum; \ + __chk_user_ptr(addr); \ + asm("addl %3,%1 ; sbbl %0,%0; cmpl %1,%4; sbbl $0,%0" \ + :"=&r" (flag), "=r" (roksum) \ + :"1" (addr), "g" ((int)(size)), \ + "rm" (current_thread_info()->addr_limit.seg)); \ + flag; \ +})/** * access_ok: - Checks if a user space pointer is valid
@@ -81,7 +86,7 @@ extern struct movsl_mask {* checks that the pointer is in the user space range - after calling * this function, memory access functions may still return -EFAULT. */ -#define access_ok (type,addr,size) (likely(__range_ok(addr,size) == 0)) +#define access_ok (type, addr, size) (likely(__range_ok(addr, size) == 0))/* * The exception table consists of pairs of addresses: the first is the
@@ -96,8 +101,7 @@ extern struct movsl_mask {* on our cache or tlb entries. */ -struct exception_table_entry -{ +struct exception_table_entry {unsigned long insn, fixup; };
@@ -122,13 +126,15 @@ extern void __get_user_1(void);extern void __get_user_2 (void );extern void __get_user_4 (void ); -#define __get_user_x (size,ret,x,ptr) \ - __asm__ __volatile__("call __get_user_" #size \ - :"=a" (ret),"=d" (x) \ - :"0" (ptr)) +#define __get_user_x (size, ret, x, ptr) \ + asm volatile("call __get_user_" #size \ + :"=a" (ret),"=d" (x) \ + :"0" (ptr)) + +/* Careful: we have to cast the result to the type of the pointer +* for sign reasons */ -/* Careful: we have to cast the result to the type of the pointer for sign reasons */ /** * get_user: - Get a simple variable from user space. * @x: Variable to store result.
@@ -146,15 +152,24 @@ extern void __get_user_4(void);* Returns zero on success, or -EFAULT on error. * On error, the variable @x is set to zero. */ -#define get_user (x,ptr) \ -({ int __ret_gu; \ +#define get_user (x, ptr) \ +({ \ + int __ret_gu; \ unsigned long __val_gu; \ __chk_user_ptr(ptr); \ - switch(sizeof (*(ptr))) { \ - case 1: __get_user_x(1,__ret_gu,__val_gu,ptr); break; \ - case 2: __get_user_x(2,__ret_gu,__val_gu,ptr); break; \ - case 4: __get_user_x(4,__ret_gu,__val_gu,ptr); break; \ - default: __get_user_x(X,__ret_gu,__val_gu,ptr); break; \ + switch (sizeof(*(ptr))) { \ + case 1: \ + __get_user_x(1, __ret_gu, __val_gu, ptr); \ + break; \ + case 2: \ + __get_user_x(2, __ret_gu, __val_gu, ptr); \ + break; \ + case 4: \ + __get_user_x(4, __ret_gu, __val_gu, ptr); \ + break; \ + default: \ + __get_user_x(X, __ret_gu, __val_gu, ptr); \ + break; \ } \ (x) = (__typeof__(*(ptr)))__val_gu; \ __ret_gu; \
@@ -171,11 +186,25 @@ extern void __put_user_2(void);extern void __put_user_4 (void );extern void __put_user_8 (void ); -#define __put_user_1 (x, ptr) __asm__ __volatile__("call __put_user_1":"=a" (__ret_pu):"0" ((typeof(*(ptr)))(x)), "c" (ptr)) -#define __put_user_2 (x, ptr) __asm__ __volatile__("call __put_user_2":"=a" (__ret_pu):"0" ((typeof(*(ptr)))(x)), "c" (ptr)) -#define __put_user_4 (x, ptr) __asm__ __volatile__("call __put_user_4":"=a" (__ret_pu):"0" ((typeof(*(ptr)))(x)), "c" (ptr)) -#define __put_user_8 (x, ptr) __asm__ __volatile__("call __put_user_8":"=a" (__ret_pu):"A" ((typeof(*(ptr)))(x)), "c" (ptr)) -#define __put_user_X (x, ptr) __asm__ __volatile__("call __put_user_X":"=a" (__ret_pu):"c" (ptr)) +#define __put_user_1 (x, ptr) \ + asm volatile("call __put_user_1" : "=a" (__ret_pu) \ + : "0" ((typeof(*(ptr)))(x)), "c" (ptr)) + +#define __put_user_2 (x, ptr) \ + asm volatile("call __put_user_2" : "=a" (__ret_pu) \ + : "0" ((typeof(*(ptr)))(x)), "c" (ptr)) + +#define __put_user_4 (x, ptr) \ + asm volatile("call __put_user_4" : "=a" (__ret_pu) \ + : "0" ((typeof(*(ptr)))(x)), "c" (ptr)) + +#define __put_user_8 (x, ptr) \ + asm volatile("call __put_user_8" : "=a" (__ret_pu) \ + : "A" ((typeof(*(ptr)))(x)), "c" (ptr)) + +#define __put_user_X (x, ptr) \ + asm volatile("call __put_user_X" : "=a" (__ret_pu) \ + : "c" (ptr))/** * put_user: - Write a simple value into user space.
@@ -195,32 +224,43 @@ extern void __put_user_8(void);*/ #ifdef CONFIG_X86_WP_WORKS_OK -#define put_user (x,ptr) \ -({ int __ret_pu; \ +#define put_user (x, ptr) \ +({ \ + int __ret_pu; \ __typeof__(*(ptr)) __pu_val; \ __chk_user_ptr(ptr); \ __pu_val = x; \ - switch(sizeof(*(ptr))) { \ - case 1: __put_user_1(__pu_val, ptr); break; \ - case 2: __put_user_2(__pu_val, ptr); break; \ - case 4: __put_user_4(__pu_val, ptr); break; \ - case 8: __put_user_8(__pu_val, ptr); break; \ - default:__put_user_X(__pu_val, ptr); break; \ + switch (sizeof(*(ptr))) { \ + case 1: \ + __put_user_1(__pu_val, ptr); \ + break; \ + case 2: \ + __put_user_2(__pu_val, ptr); \ + break; \ + case 4: \ + __put_user_4(__pu_val, ptr); \ + break; \ + case 8: \ + __put_user_8(__pu_val, ptr); \ + break; \ + default: \ + __put_user_X(__pu_val, ptr); \ + break; \ } \ __ret_pu; \ })#else -#define put_user (x,ptr) \ +#define put_user (x, ptr) \ ({ \ - int __ret_pu; \ - __typeof__(*(ptr)) __pus_tmp = x; \ - __ret_pu=0; \ - if(unlikely(__copy_to_user_ll(ptr, &__pus_tmp, \ - sizeof(*(ptr))) != 0)) \ - __ret_pu=-EFAULT; \ - __ret_pu; \ - }) + int __ret_pu; \ + __typeof__(*(ptr))__pus_tmp = x; \ + __ret_pu = 0; \ + if (unlikely(__copy_to_user_ll(ptr, &__pus_tmp, \ + sizeof(*(ptr))) != 0)) \ + __ret_pu = -EFAULT; \ + __ret_pu; \ +})#endif
@@ -245,8 +285,8 @@ extern void __put_user_8(void);* Returns zero on success, or -EFAULT on error. * On error, the variable @x is set to zero. */ -#define __get_user (x,ptr) \ - __get_user_nocheck((x),(ptr),sizeof(*(ptr))) +#define __get_user (x, ptr) \ + __get_user_nocheck((x), (ptr), sizeof(*(ptr)))/**
@@ -268,54 +308,62 @@ extern void __put_user_8(void);* * Returns zero on success, or -EFAULT on error. */ -#define __put_user (x,ptr) \ - __put_user_nocheck((__typeof__(*(ptr)))(x),(ptr),sizeof(*(ptr))) +#define __put_user (x, ptr) \ + __put_user_nocheck((__typeof__(*(ptr)))(x), (ptr), sizeof(*(ptr))) -#define __put_user_nocheck (x,ptr,size) \ +#define __put_user_nocheck (x, ptr, size) \ ({ \ long __pu_err; \ - __put_user_size((x),(ptr),(size),__pu_err,-EFAULT); \ + __put_user_size((x), (ptr), (size), __pu_err, -EFAULT); \ __pu_err; \ }) -#define __put_user_u64 (x, addr, err) \ - __asm__ __volatile__( \ - "1: movl %%eax,0(%2)\n" \ - "2: movl %%edx,4(%2)\n" \ - "3:\n" \ - ".section .fixup,\"ax\"\n" \ - "4: movl %3,%0\n" \ - " jmp 3b\n" \ - ".previous\n" \ - _ASM_EXTABLE(1b,4b) \ - _ASM_EXTABLE(2b,4b) \ - : "=r"(err) \ - : "A" (x), "r" (addr), "i"(-EFAULT), "0"(err)) +#define __put_user_u64 (x, addr, err) \ + asm volatile("1: movl %%eax,0(%2)\n" \ + "2: movl %%edx,4(%2)\n" \ + "3:\n" \ + ".section .fixup,\"ax\"\n" \ + "4: movl %3,%0\n" \ + " jmp 3b\n" \ + ".previous\n" \ + _ASM_EXTABLE(1b, 4b) \ + _ASM_EXTABLE(2b, 4b) \ + : "=r" (err) \ + : "A" (x), "r" (addr), "i" (-EFAULT), "0" (err))#ifdef CONFIG_X86_WP_WORKS_OK -#define __put_user_size (x,ptr,size,retval,errret) \ +#define __put_user_size (x, ptr, size, retval, errret) \ do { \ retval = 0; \ __chk_user_ptr(ptr); \ switch (size) { \ - case 1: __put_user_asm(x,ptr,retval,"b","b","iq",errret);break; \ - case 2: __put_user_asm(x,ptr,retval,"w","w","ir",errret);break; \ - case 4: __put_user_asm(x,ptr,retval,"l","","ir",errret); break; \ - case 8: __put_user_u64((__typeof__(*ptr))(x),ptr,retval); break;\ - default: __put_user_bad(); \ + case 1: \ + __put_user_asm(x, ptr, retval, "b", "b", "iq", errret); \ + break; \ + case 2: \ + __put_user_asm(x, ptr, retval, "w", "w", "ir", errret); \ + break; \ + case 4: \ + __put_user_asm(x, ptr, retval, "l", "", "ir", errret); \ + break; \ + case 8: \ + __put_user_u64((__typeof__(*ptr))(x), ptr, retval); \ + break; \ + default: \ + __put_user_bad(); \ } \ } while (0)#else -#define __put_user_size (x,ptr,size,retval,errret) \ +#define __put_user_size (x, ptr, size, retval, errret) \ do { \ - __typeof__(*(ptr)) __pus_tmp = x; \ + __typeof__(*(ptr))__pus_tmp = x; \ retval = 0; \ \ - if(unlikely(__copy_to_user_ll(ptr, &__pus_tmp, size) != 0)) \ + if (unlikely(__copy_to_user_ll(ptr, &__pus_tmp, size) != 0)) \ retval = errret; \ } while (0)
@@ -329,65 +377,70 @@ struct __large_struct { unsigned long buf[100]; };* aliasing issues. */ #define __put_user_asm (x, addr, err, itype, rtype, ltype, errret) \ - __asm__ __volatile__( \ - "1: mov"itype" %"rtype"1,%2\n" \ - "2:\n" \ - ".section .fixup,\"ax\"\n" \ - "3: movl %3,%0\n" \ - " jmp 2b\n" \ - ".previous\n" \ - _ASM_EXTABLE(1b,3b) \ - : "=r"(err) \ - : ltype (x), "m"(__m(addr)), "i"(errret), "0"(err)) - - -#define __get_user_nocheck (x,ptr,size) \ -({ \ - long __gu_err; \ - unsigned long __gu_val; \ - __get_user_size(__gu_val,(ptr),(size),__gu_err,-EFAULT);\ - (x) = (__typeof__(*(ptr)))__gu_val; \ - __gu_err; \ + asm volatile("1: mov"itype" %"rtype"1,%2\n" \ + "2:\n" \ + ".section .fixup,\"ax\"\n" \ + "3: movl %3,%0\n" \ + " jmp 2b\n" \ + ".previous\n" \ + _ASM_EXTABLE(1b, 3b) \ + : "=r"(err) \ + : ltype (x), "m" (__m(addr)), "i" (errret), "0" (err)) + + +#define __get_user_nocheck (x, ptr, size) \ +({ \ + long __gu_err; \ + unsigned long __gu_val; \ + __get_user_size(__gu_val, (ptr), (size), __gu_err, -EFAULT); \ + (x) = (__typeof__(*(ptr)))__gu_val; \ + __gu_err; \ })extern long __get_user_bad (void ); -#define __get_user_size (x,ptr,size,retval,errret) \ +#define __get_user_size (x, ptr, size, retval, errret) \ do { \ retval = 0; \ __chk_user_ptr(ptr); \ switch (size) { \ - case 1: __get_user_asm(x,ptr,retval,"b","b","=q",errret);break; \ - case 2: __get_user_asm(x,ptr,retval,"w","w","=r",errret);break; \ - case 4: __get_user_asm(x,ptr,retval,"l","","=r",errret);break; \ - default: (x) = __get_user_bad(); \ + case 1: \ + __get_user_asm(x, ptr, retval, "b", "b", "=q", errret); \ + break; \ + case 2: \ + __get_user_asm(x, ptr, retval, "w", "w", "=r", errret); \ + break; \ + case 4: \ + __get_user_asm(x, ptr, retval, "l", "", "=r", errret); \ + break; \ + default: \ + (x) = __get_user_bad(); \ } \ } while (0)#define __get_user_asm (x, addr, err, itype, rtype, ltype, errret) \ - __asm__ __volatile__( \ - "1: mov"itype" %2,%"rtype"1\n" \ - "2:\n" \ - ".section .fixup,\"ax\"\n" \ - "3: movl %3,%0\n" \ - " xor"itype" %"rtype"1,%"rtype"1\n" \ - " jmp 2b\n" \ - ".previous\n" \ - _ASM_EXTABLE(1b,3b) \ - : "=r"(err), ltype (x) \ - : "m"(__m(addr)), "i"(errret), "0"(err)) - - -unsigned long __must_check __copy_to_user_ll (void __user *to, -const void *from,unsigned long n); -unsigned long __must_check __copy_from_user_ll (void *to, -const void __user *from,unsigned long n); -unsigned long __must_check __copy_from_user_ll_nozero (void *to, -const void __user *from,unsigned long n); -unsigned long __must_check __copy_from_user_ll_nocache (void *to, -const void __user *from,unsigned long n); -unsigned long __must_check __copy_from_user_ll_nocache_nozero (void *to, -const void __user *from,unsigned long n); + asm volatile("1: mov"itype" %2,%"rtype"1\n" \ + "2:\n" \ + ".section .fixup,\"ax\"\n" \ + "3: movl %3,%0\n" \ + " xor"itype" %"rtype"1,%"rtype"1\n" \ + " jmp 2b\n" \ + ".previous\n" \ + _ASM_EXTABLE(1b, 3b) \ + : "=r" (err), ltype (x) \ + : "m" (__m(addr)), "i" (errret), "0" (err)) + + +unsigned long __must_check __copy_to_user_ll + (void __user *to,const void *from,unsigned long n); +unsigned long __must_check __copy_from_user_ll + (void *to,const void __user *from,unsigned long n); +unsigned long __must_check __copy_from_user_ll_nozero + (void *to,const void __user *from,unsigned long n); +unsigned long __must_check __copy_from_user_ll_nocache + (void *to,const void __user *from,unsigned long n); +unsigned long __must_check __copy_from_user_ll_nocache_nozero + (void *to,const void __user *from,unsigned long n);/** * __copy_to_user_inatomic: - Copy a block of data into user space, with less checking.
@@ -416,13 +469,16 @@ __copy_to_user_inatomic(void __user *to, const void *from, unsigned long n)switch (n) {case 1 : -__put_user_size (*(u8 *)from, (u8 __user *)to,1 , ret,1 ); +__put_user_size (*(u8 *)from, (u8 __user *)to, +1 , ret,1 );return ret;case 2 : -__put_user_size (*(u16 *)from, (u16 __user *)to,2 , ret,2 ); +__put_user_size (*(u16 *)from, (u16 __user *)to, +2 , ret,2 );return ret;case 4 : -__put_user_size (*(u32 *)from, (u32 __user *)to,4 , ret,4 ); +__put_user_size (*(u32 *)from, (u32 __user *)to, +4 , ret,4 );return ret; } }
@@ -545,19 +601,21 @@ static __always_inline unsigned long __copy_from_user_nocache(void *to, }static __always_inline unsigned long -__copy_from_user_inatomic_nocache (void *to,const void __user *from,unsigned long n) +__copy_from_user_inatomic_nocache (void *to,const void __user *from, +unsigned long n) {return __copy_from_user_ll_nocache_nozero (to, from, n); }unsigned long __must_check copy_to_user (void __user *to, -const void *from,unsigned long n); +const void *from,unsigned long n);unsigned long __must_check copy_from_user (void *to, -const void __user *from,unsigned long n); +const void __user *from, +unsigned long n);long __must_check strncpy_from_user (char *dst,const char __user *src, -long count); +long count);long __must_check __strncpy_from_user (char *dst, -const char __user *src,long count); +const char __user *src,long count);/** * strlen_user: - Get the size of a string in user space.
Signature · 1 line
1.5.4.rc2
Joe Perches
maintainerPATCH 134/148 include/asm-x86/uaccess_64.h: checkpatch cleanups - formatting only
In reply to Joe Perches--- include/asm-x86/uaccess_64.h | 376 +++++++++++++++++++++++++----------------- 1 files changed, 227 insertions(+), 149 deletions(-)
include/asm-x86/uaccess_64.h +227 -149
@@ -29,23 +29,27 @@#define get_fs () (current_thread_info()->addr_limit)#define set_fs (x) (current_thread_info()->addr_limit = (x)) -#define segment_eq (a,b) ((a).seg == (b).seg) +#define segment_eq (a, b) ((a).seg == (b).seg) -#define __addr_ok (addr) (!((unsigned long)(addr) & (current_thread_info()->addr_limit.seg))) +#define __addr_ok (addr) (!((unsigned long)(addr) & \ + (current_thread_info()->addr_limit.seg)))/* * Uhhuh, this needs 65-bit arithmetic. We have a carry.. */ -#define __range_not_ok (addr,size) ({ \ - unsigned long flag,roksum; \ - __chk_user_ptr(addr); \ - asm("# range_ok\n\r" \ - "addq %3,%1 ; sbbq %0,%0 ; cmpq %1,%4 ; sbbq $0,%0" \ - :"=&r" (flag), "=r" (roksum) \ - :"1" (addr),"g" ((long)(size)),"g" (current_thread_info()->addr_limit.seg)); \ - flag; }) +#define __range_not_ok (addr, size) \ +({ \ + unsigned long flag, roksum; \ + __chk_user_ptr(addr); \ + asm("# range_ok\n\r" \ + "addq %3,%1 ; sbbq %0,%0 ; cmpq %1,%4 ; sbbq $0,%0" \ + : "=&r" (flag), "=r" (roksum) \ + : "1" (addr), "g" ((long)(size)), \ + "g" (current_thread_info()->addr_limit.seg)); \ + flag; +}) -#define access_ok (type, addr, size) (__range_not_ok(addr,size) == 0) +#define access_ok (type, addr, size) (__range_not_ok(addr, size) == 0)/* * The exception table consists of pairs of addresses: the first is the
@@ -60,8 +64,7 @@* on our cache or tlb entries. */ -struct exception_table_entry -{ +struct exception_table_entry {unsigned long insn, fixup; };
@@ -84,23 +87,36 @@ extern int fixup_exception(struct pt_regs *regs);* accesses to the same area of user memory). */ -#define __get_user_x (size,ret,x,ptr) \ - asm volatile("call __get_user_" #size \ - :"=a" (ret),"=d" (x) \ - :"c" (ptr) \ - :"r8") +#define __get_user_x (size, ret, x, ptr) \ + asm volatile("call __get_user_" #size \ + : "=a" (ret),"=d" (x) \ + : "c" (ptr) \ + : "r8") + +/* Careful: we have to cast the result to the type of the pointer +* for sign reasons */ -/* Careful: we have to cast the result to the type of the pointer for sign reasons */ -#define get_user (x,ptr) \ -({ unsigned long __val_gu; \ - int __ret_gu; \ +#define get_user (x, ptr) \ +({ \ + unsigned long __val_gu; \ + int __ret_gu; \ __chk_user_ptr(ptr); \ - switch(sizeof (*(ptr))) { \ - case 1: __get_user_x(1,__ret_gu,__val_gu,ptr); break; \ - case 2: __get_user_x(2,__ret_gu,__val_gu,ptr); break; \ - case 4: __get_user_x(4,__ret_gu,__val_gu,ptr); break; \ - case 8: __get_user_x(8,__ret_gu,__val_gu,ptr); break; \ - default: __get_user_bad(); break; \ + switch (sizeof(*(ptr))) { \ + case 1: \ + __get_user_x(1, __ret_gu, __val_gu, ptr); \ + break; \ + case 2: \ + __get_user_x(2, __ret_gu, __val_gu, ptr); \ + break; \ + case 4: \ + __get_user_x(4, __ret_gu, __val_gu, ptr); \ + break; \ + case 8: \ + __get_user_x(8, __ret_gu, __val_gu, ptr); \ + break; \ + default: \ + __get_user_bad(); \ + break; \ } \ (x) = (__force typeof(*(ptr)))__val_gu; \ __ret_gu; \
@@ -112,55 +128,73 @@ extern void __put_user_4(void);extern void __put_user_8 (void );extern void __put_user_bad (void ); -#define __put_user_x (size,ret,x,ptr) \ - asm volatile("call __put_user_" #size \ - :"=a" (ret) \ - :"c" (ptr),"d" (x) \ - :"r8") +#define __put_user_x (size, ret, x, ptr) \ + asm volatile("call __put_user_" #size \ + :"=a" (ret) \ + :"c" (ptr),"d" (x) \ + :"r8") -#define put_user (x,ptr) \ - __put_user_check((__typeof__(*(ptr)))(x),(ptr),sizeof(*(ptr))) +#define put_user (x, ptr) \ + __put_user_check((__typeof__(*(ptr)))(x), (ptr), sizeof(*(ptr))) -#define __get_user (x,ptr) \ - __get_user_nocheck((x),(ptr),sizeof(*(ptr))) -#define __put_user (x,ptr) \ - __put_user_nocheck((__typeof__(*(ptr)))(x),(ptr),sizeof(*(ptr))) +#define __get_user (x, ptr) \ + __get_user_nocheck((x), (ptr), sizeof(*(ptr))) +#define __put_user (x, ptr) \ + __put_user_nocheck((__typeof__(*(ptr)))(x), (ptr), sizeof(*(ptr)))#define __get_user_unaligned __get_user#define __put_user_unaligned __put_user -#define __put_user_nocheck (x,ptr,size) \ +#define __put_user_nocheck (x, ptr, size) \ ({ \ int __pu_err; \ - __put_user_size((x),(ptr),(size),__pu_err); \ + __put_user_size((x), (ptr), (size), __pu_err); \ __pu_err; \ }) -#define __put_user_check (x,ptr,size) \ -({ \ - int __pu_err; \ - typeof(*(ptr)) __user *__pu_addr = (ptr); \ - switch (size) { \ - case 1: __put_user_x(1,__pu_err,x,__pu_addr); break; \ - case 2: __put_user_x(2,__pu_err,x,__pu_addr); break; \ - case 4: __put_user_x(4,__pu_err,x,__pu_addr); break; \ - case 8: __put_user_x(8,__pu_err,x,__pu_addr); break; \ - default: __put_user_bad(); \ - } \ - __pu_err; \ +#define __put_user_check (x, ptr, size) \ +({ \ + int __pu_err; \ + typeof(*(ptr)) __user *__pu_addr = (ptr); \ + switch (size) { \ + case 1: \ + __put_user_x(1, __pu_err, x, __pu_addr); \ + break; \ + case 2: \ + __put_user_x(2, __pu_err, x, __pu_addr); \ + break; \ + case 4: \ + __put_user_x(4, __pu_err, x, __pu_addr); \ + break; \ + case 8: \ + __put_user_x(8, __pu_err, x, __pu_addr); \ + break; \ + default: \ + __put_user_bad(); \ + } \ + __pu_err; \ }) -#define __put_user_size (x,ptr,size,retval) \ +#define __put_user_size (x, ptr, size, retval) \ do { \ retval = 0; \ __chk_user_ptr(ptr); \ switch (size) { \ - case 1: __put_user_asm(x,ptr,retval,"b","b","iq",-EFAULT); break;\ - case 2: __put_user_asm(x,ptr,retval,"w","w","ir",-EFAULT); break;\ - case 4: __put_user_asm(x,ptr,retval,"l","k","ir",-EFAULT); break;\ - case 8: __put_user_asm(x,ptr,retval,"q","","Zr",-EFAULT); break;\ - default: __put_user_bad(); \ + case 1: \ + __put_user_asm(x, ptr, retval, "b", "b", "iq", -EFAULT);\ + break; \ + case 2: \ + __put_user_asm(x, ptr, retval, "w", "w", "ir", -EFAULT);\ + break; \ + case 4: \ + __put_user_asm(x, ptr, retval, "l", "k", "ir", -EFAULT);\ + break; \ + case 8: \ + __put_user_asm(x, ptr, retval, "q", "", "Zr", -EFAULT); \ + break; \ + default: \ + __put_user_bad(); \ } \ } while (0)
@@ -174,23 +208,22 @@ struct __large_struct { unsigned long buf[100]; };* aliasing issues. */ #define __put_user_asm (x, addr, err, itype, rtype, ltype, errno) \ - asm volatile( \ - "1: mov"itype" %"rtype"1,%2\n" \ - "2:\n" \ - ".section .fixup,\"ax\"\n" \ - "3: mov %3,%0\n" \ - " jmp 2b\n" \ - ".previous\n" \ - _ASM_EXTABLE(1b,3b) \ - : "=r"(err) \ - : ltype (x), "m"(__m(addr)), "i"(errno), "0"(err)) - - -#define __get_user_nocheck (x,ptr,size) \ + asm volatile("1: mov"itype" %"rtype"1,%2\n" \ + "2:\n" \ + ".section .fixup, \"ax\"\n" \ + "3: mov %3,%0\n" \ + " jmp 2b\n" \ + ".previous\n" \ + _ASM_EXTABLE(1b, 3b) \ + : "=r"(err) \ + : ltype (x), "m" (__m(addr)), "i" (errno), "0" (err)) + + +#define __get_user_nocheck (x, ptr, size) \ ({ \ int __gu_err; \ unsigned long __gu_val; \ - __get_user_size(__gu_val,(ptr),(size),__gu_err); \ + __get_user_size(__gu_val, (ptr), (size), __gu_err); \ (x) = (__force typeof(*(ptr)))__gu_val; \ __gu_err; \ })
@@ -201,31 +234,39 @@ extern int __get_user_4(void);extern int __get_user_8 (void );extern int __get_user_bad (void ); -#define __get_user_size (x,ptr,size,retval) \ +#define __get_user_size (x, ptr, size, retval) \ do { \ retval = 0; \ __chk_user_ptr(ptr); \ switch (size) { \ - case 1: __get_user_asm(x,ptr,retval,"b","b","=q",-EFAULT); break;\ - case 2: __get_user_asm(x,ptr,retval,"w","w","=r",-EFAULT); break;\ - case 4: __get_user_asm(x,ptr,retval,"l","k","=r",-EFAULT); break;\ - case 8: __get_user_asm(x,ptr,retval,"q","","=r",-EFAULT); break;\ - default: (x) = __get_user_bad(); \ + case 1: \ + __get_user_asm(x, ptr, retval, "b", "b", "=q", -EFAULT);\ + break; \ + case 2: \ + __get_user_asm(x, ptr, retval, "w", "w", "=r", -EFAULT);\ + break; \ + case 4: \ + __get_user_asm(x, ptr, retval, "l", "k", "=r", -EFAULT);\ + break; \ + case 8: \ + __get_user_asm(x, ptr, retval, "q", "", "=r", -EFAULT); \ + break; \ + default: \ + (x) = __get_user_bad(); \ } \ } while (0)#define __get_user_asm (x, addr, err, itype, rtype, ltype, errno) \ - asm volatile( \ - "1: mov"itype" %2,%"rtype"1\n" \ - "2:\n" \ - ".section .fixup,\"ax\"\n" \ - "3: mov %3,%0\n" \ - " xor"itype" %"rtype"1,%"rtype"1\n" \ - " jmp 2b\n" \ - ".previous\n" \ - _ASM_EXTABLE(1b,3b) \ - : "=r"(err), ltype (x) \ - : "m"(__m(addr)), "i"(errno), "0"(err)) + asm volatile("1: mov"itype" %2,%"rtype"1\n" \ + "2:\n" \ + ".section .fixup, \"ax\"\n" \ + "3: mov %3,%0\n" \ + " xor"itype" %"rtype"1,%"rtype"1\n" \ + " jmp 2b\n" \ + ".previous\n" \ + _ASM_EXTABLE(1b, 3b) \ + : "=r" (err), ltype (x) \ + : "m" (__m(addr)), "i"(errno), "0"(err))/* * Copy To/From Userspace
@@ -244,110 +285,142 @@ copy_in_user(void __user *to, const void __user *from, unsigned len);static __always_inline __must_check int __copy_from_user(void *dst, const void __user *src, unsigned size) -{ +{int ret =0 ;if (!__builtin_constant_p (size)) -return copy_user_generic (dst,(__force void *)src,size); -switch (size) { -case 1 :__get_user_asm (*(u8 *)dst,(u8 __user *)src,ret,"b" ,"b" ,"=q" ,1 ); +return copy_user_generic (dst, (__force void *)src, size); +switch (size) { +case 1 :__get_user_asm (*(u8 *)dst, (u8 __user *)src, + ret,"b" ,"b" ,"=q" ,1 );return ret; -case 2 :__get_user_asm (*(u16 *)dst,(u16 __user *)src,ret,"w" ,"w" ,"=r" ,2 ); +case 2 :__get_user_asm (*(u16 *)dst, (u16 __user *)src, + ret,"w" ,"w" ,"=r" ,2 );return ret; -case 4 :__get_user_asm (*(u32 *)dst,(u32 __user *)src,ret,"l" ,"k" ,"=r" ,4 ); +case 4 :__get_user_asm (*(u32 *)dst, (u32 __user *)src, + ret,"l" ,"k" ,"=r" ,4 ); +return ret; +case 8 :__get_user_asm (*(u64 *)dst, (u64 __user *)src, + ret,"q" ,"" ,"=r" ,8 );return ret; -case 8 :__get_user_asm (*(u64 *)dst,(u64 __user *)src,ret,"q" ,"" ,"=r" ,8 ); -return ret;case 10 : -__get_user_asm (*(u64 *)dst,(u64 __user *)src,ret,"q" ,"" ,"=r" ,16 ); -if (unlikely (ret))return ret; -__get_user_asm (*(u16 *)(8 +(char *)dst),(u16 __user *)(8 +(char __user *)src),ret,"w" ,"w" ,"=r" ,2 ); -return ret; +__get_user_asm (*(u64 *)dst, (u64 __user *)src, + ret,"q" ,"" ,"=r" ,16 ); +if (unlikely (ret)) +return ret; +__get_user_asm (*(u16 *)(8 + (char *)dst), + (u16 __user *)(8 + (char __user *)src), + ret,"w" ,"w" ,"=r" ,2 ); +return ret;case 16 : -__get_user_asm (*(u64 *)dst,(u64 __user *)src,ret,"q" ,"" ,"=r" ,16 ); -if (unlikely (ret))return ret; -__get_user_asm (*(u64 *)(8 +(char *)dst),(u64 __user *)(8 +(char __user *)src),ret,"q" ,"" ,"=r" ,8 ); -return ret; +__get_user_asm (*(u64 *)dst, (u64 __user *)src, + ret,"q" ,"" ,"=r" ,16 ); +if (unlikely (ret)) +return ret; +__get_user_asm (*(u64 *)(8 + (char *)dst), + (u64 __user *)(8 + (char __user *)src), + ret,"q" ,"" ,"=r" ,8 ); +return ret;default : -return copy_user_generic (dst,(__force void *)src,size); +return copy_user_generic (dst, (__force void *)src, size); } -} +}static __always_inline __must_check int __copy_to_user(void __user *dst, const void *src, unsigned size) -{ +{int ret =0 ;if (!__builtin_constant_p (size)) -return copy_user_generic ((__force void *)dst,src,size); -switch (size) { -case 1 :__put_user_asm (*(u8 *)src,(u8 __user *)dst,ret,"b" ,"b" ,"iq" ,1 ); +return copy_user_generic ((__force void *)dst, src, size); +switch (size) { +case 1 :__put_user_asm (*(u8 *)src, (u8 __user *)dst, + ret,"b" ,"b" ,"iq" ,1 );return ret; -case 2 :__put_user_asm (*(u16 *)src,(u16 __user *)dst,ret,"w" ,"w" ,"ir" ,2 ); +case 2 :__put_user_asm (*(u16 *)src, (u16 __user *)dst, + ret,"w" ,"w" ,"ir" ,2 );return ret; -case 4 :__put_user_asm (*(u32 *)src,(u32 __user *)dst,ret,"l" ,"k" ,"ir" ,4 ); +case 4 :__put_user_asm (*(u32 *)src, (u32 __user *)dst, + ret,"l" ,"k" ,"ir" ,4 ); +return ret; +case 8 :__put_user_asm (*(u64 *)src, (u64 __user *)dst, + ret,"q" ,"" ,"ir" ,8 );return ret; -case 8 :__put_user_asm (*(u64 *)src,(u64 __user *)dst,ret,"q" ,"" ,"ir" ,8 ); -return ret;case 10 : -__put_user_asm (*(u64 *)src,(u64 __user *)dst,ret,"q" ,"" ,"ir" ,10 ); -if (unlikely (ret))return ret; +__put_user_asm (*(u64 *)src, (u64 __user *)dst, + ret,"q" ,"" ,"ir" ,10 ); +if (unlikely (ret)) +return ret; asm("" :::"memory" ); -__put_user_asm (4 [(u16 *)src],4 +(u16 __user *)dst,ret,"w" ,"w" ,"ir" ,2 ); -return ret; +__put_user_asm (4 [(u16 *)src],4 + (u16 __user *)dst, + ret,"w" ,"w" ,"ir" ,2 ); +return ret;case 16 : -__put_user_asm (*(u64 *)src,(u64 __user *)dst,ret,"q" ,"" ,"ir" ,16 ); -if (unlikely (ret))return ret; +__put_user_asm (*(u64 *)src, (u64 __user *)dst, + ret,"q" ,"" ,"ir" ,16 ); +if (unlikely (ret)) +return ret; asm("" :::"memory" ); -__put_user_asm (1 [(u64 *)src],1 +(u64 __user *)dst,ret,"q" ,"" ,"ir" ,8 ); -return ret; +__put_user_asm (1 [(u64 *)src],1 + (u64 __user *)dst, + ret,"q" ,"" ,"ir" ,8 ); +return ret;default : -return copy_user_generic ((__force void *)dst,src,size); +return copy_user_generic ((__force void *)dst, src, size); } -} +}static __always_inline __must_check int __copy_in_user(void __user *dst, const void __user *src, unsigned size) -{ +{int ret =0 ;if (!__builtin_constant_p (size)) -return copy_user_generic ((__force void *)dst,(__force void *)src,size); -switch (size) { -case 1 : { +return copy_user_generic ((__force void *)dst, + (__force void *)src, size); +switch (size) { +case 1 : {u8 tmp; -__get_user_asm (tmp,(u8 __user *)src,ret,"b" ,"b" ,"=q" ,1 ); +__get_user_asm (tmp, (u8 __user *)src, + ret,"b" ,"b" ,"=q" ,1 );if (likely (!ret)) -__put_user_asm (tmp,(u8 __user *)dst,ret,"b" ,"b" ,"iq" ,1 ); +__put_user_asm (tmp, (u8 __user *)dst, + ret,"b" ,"b" ,"iq" ,1 );return ret; } -case 2 : { +case 2 : {u16 tmp; -__get_user_asm (tmp,(u16 __user *)src,ret,"w" ,"w" ,"=r" ,2 ); +__get_user_asm (tmp, (u16 __user *)src, + ret,"w" ,"w" ,"=r" ,2 );if (likely (!ret)) -__put_user_asm (tmp,(u16 __user *)dst,ret,"w" ,"w" ,"ir" ,2 ); +__put_user_asm (tmp, (u16 __user *)dst, + ret,"w" ,"w" ,"ir" ,2 );return ret; } -case 4 : { +case 4 : {u32 tmp; -__get_user_asm (tmp,(u32 __user *)src,ret,"l" ,"k" ,"=r" ,4 ); +__get_user_asm (tmp, (u32 __user *)src, + ret,"l" ,"k" ,"=r" ,4 );if (likely (!ret)) -__put_user_asm (tmp,(u32 __user *)dst,ret,"l" ,"k" ,"ir" ,4 ); +__put_user_asm (tmp, (u32 __user *)dst, + ret,"l" ,"k" ,"ir" ,4 );return ret; } -case 8 : { +case 8 : {u64 tmp; -__get_user_asm (tmp,(u64 __user *)src,ret,"q" ,"" ,"=r" ,8 ); +__get_user_asm (tmp, (u64 __user *)src, + ret,"q" ,"" ,"=r" ,8 );if (likely (!ret)) -__put_user_asm (tmp,(u64 __user *)dst,ret,"q" ,"" ,"ir" ,8 ); +__put_user_asm (tmp, (u64 __user *)dst, + ret,"q" ,"" ,"ir" ,8 );return ret; }default : -return copy_user_generic ((__force void *)dst,(__force void *)src,size); +return copy_user_generic ((__force void *)dst, + (__force void *)src, size); } -} +} -__must_check long +__must_check long strncpy_from_user (char *dst,const char __user *src,long count); -__must_check long +__must_check long __strncpy_from_user (char *dst,const char __user *src,long count);__must_check long strnlen_user (const char __user *str,long n);__must_check long __strnlen_user (const char __user *str,long n);
@@ -355,7 +428,8 @@ __must_check long strlen_user(const char __user *str);__must_check unsigned long clear_user (void __user *mem,unsigned long len);__must_check unsigned long __clear_user (void __user *mem,unsigned long len); -__must_check long __copy_from_user_inatomic (void *dst,const void __user *src,unsigned size); +__must_check long __copy_from_user_inatomic (void *dst,const void __user *src, +unsigned size); static __must_check __always_inline int__copy_to_user_inatomic (void __user *dst,const void *src,unsigned size)
@@ -364,15 +438,19 @@ __copy_to_user_inatomic(void __user *dst, const void *src, unsigned size) }#define ARCH_HAS_NOCACHE_UACCESS 1 -extern long __copy_user_nocache (void *dst,const void __user *src,unsigned size,int zerorest); +extern long __copy_user_nocache (void *dst,const void __user *src, +unsigned size,int zerorest); -static inline int __copy_from_user_nocache (void *dst,const void __user *src,unsigned size) +static inline int __copy_from_user_nocache (void *dst,const void __user *src, +unsigned size) {might_sleep ();return __copy_user_nocache (dst, src, size,1 ); } -static inline int __copy_from_user_inatomic_nocache (void *dst,const void __user *src,unsigned size) +static inline int __copy_from_user_inatomic_nocache (void *dst, +const void __user *src, +unsigned size) {return __copy_user_nocache (dst, src, size,0 ); }
Signature · 1 line
1.5.4.rc2
Re: [PATCH 134/148] include/asm-x86/uaccess_64.h: checkpatch cleanups - formatting only
include/asm-x86/uaccess_64.h line 50
Quoted 4 lines · Joe Perches
include/asm-x86/uaccess_64.h +4
-0+ : "1" (addr), "g" ((long)(size)), \ + "g" (current_thread_info()->addr_limit.seg)); \ + flag; +})
hm, you apparently never built this on 64-bit x86? The above has a trivial typo.
Ingo
Joe Perches
maintainerRe: [PATCH 134/148] include/asm-x86/uaccess_64.h: checkpatch cleanups - formatting only
Quoted 1 line · Ingo Molnar
hm, you apparently never built this on 64-bit x86?
Correct. x86(_32) defconfig and allyesconfig only.
Quoted 1 line
The above has a trivial typo.
Careless.
I'll set up an x86-64 cross-compiler.
Before building I did
s/__LINE__/0/g
to minimize the md5sum differences
and md5sum/diff and objdump -Dx/diff and inspected the objects before and after.
Re: [PATCH 134/148] include/asm-x86/uaccess_64.h: checkpatch cleanups - formatting only
Quoted 9 lines · Joe Perches
Quoted 1 line · Ingo Molnar
hm, you apparently never built this on 64-bit x86?
Correct.
x86(_32)defconfig and allyesconfig only.Quoted 1 line
The above has a trivial typo.
Careless.
I'll set up an x86-64 cross-compiler.
yeah - 64-bit allyesconfig (with DEBUG_INFO disabled - it just slows down the build) should trigger most of the problems.
nevertheless i have most of your other patches in x86.git/latest right now, you can pick it up via:
http://people.redhat.com/mingo/x86.git/README
Quoted 6 lines
Before building I did
s/__LINE__/0/g
to minimize the md5sum differencesand md5sum/diff and objdump -Dx/diff and inspected the objects before and after.
that's a nice trick - i never figured out a good way to skip such type of build differences.
Ingo
Joe Perches
maintainerPATCH 135/148 include/asm-x86/unaligned.h: checkpatch cleanups - formatting only
In reply to Joe Perches--- include/asm-x86/unaligned.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
include/asm-x86/unaligned.h +1 -1
@@ -32,6 +32,6 @@* * Note that unaligned accesses can be very expensive on some architectures. */ -#define put_unaligned (val, ptr) ((void)( *(ptr) = (val) )) +#define put_unaligned (val, ptr) ((void)(*(ptr) = (val)))#endif /* _ASM_X86_UNALIGNED_H */
Signature · 1 line
1.5.4.rc2
Joe Perches
maintainerPATCH 136/148 include/asm-x86/unistd_32.h: checkpatch cleanups - formatting only
In reply to Joe Perches--- include/asm-x86/unistd_32.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
include/asm-x86/unistd_32.h +1 -1
@@ -81,7 +81,7 @@#define __NR_sigpending 73#define __NR_sethostname 74#define __NR_setrlimit 75 -#define __NR_getrlimit 76/* Back compatible 2Gig limited rlimit */ +#define __NR_getrlimit 76/* Back compatible 2Gig limited rlimit */ #define __NR_getrusage 77#define __NR_gettimeofday 78#define __NR_settimeofday 79
Signature · 1 line
1.5.4.rc2
Joe Perches
maintainerPATCH 137/148 include/asm-x86/unistd_64.h: checkpatch cleanups - formatting only
In reply to Joe Perches--- include/asm-x86/unistd_64.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
include/asm-x86/unistd_64.h +1 -1
@@ -2,7 +2,7 @@#define _ASM_X86_64_UNISTD_H_#ifndef __SYSCALL -#define __SYSCALL (a,b) +#define __SYSCALL (a, b)#endif /*
Signature · 1 line
1.5.4.rc2
Joe Perches
maintainerPATCH 138/148 include/asm-x86/user_32.h: checkpatch cleanups - formatting only
In reply to Joe Perches--- include/asm-x86/user_32.h | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)
include/asm-x86/user_32.h +3 -3
@@ -100,10 +100,10 @@ struct user_regs_struct {struct user {/* We start with the registers, to mimic the way that "memory" is returned from the ptrace(3,...) function. */ -struct user_regs_struct regs;/* Where the registers are actually stored */ +struct user_regs_struct regs;/* Where the registers are actually stored */ /* ptrace does not yet supply these. Someday.... */ int u_fpvalid;/* True if math co-processor being used. */ -/* for this mess. Not yet used. */ +/* for this mess. Not yet used. */ struct user_i387_struct i387;/* Math Co-processor registers. */ /* The rest of this junk is to help gdb figure out what goes where */ unsigned long int u_tsize;/* Text segment size (pages). */
@@ -118,7 +118,7 @@ struct user{int reserved;/* No longer used */ unsigned long u_ar0;/* Used by gdb to help find the values for */ /* the registers. */ -struct user_i387_struct * u_fpstate;/* Math Co-processor pointer. */ +struct user_i387_struct *u_fpstate;/* Math Co-processor pointer. */ unsigned long magic;/* To uniquely identify a core file */ char u_comm[32 ];/* User command that was responsible */ int u_debugreg[8 ];
Signature · 1 line
1.5.4.rc2
Joe Perches
maintainerPATCH 139/148 include/asm-x86/user32.h: checkpatch cleanups - formatting only
In reply to Joe Perches--- include/asm-x86/user32.h | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-)
include/asm-x86/user32.h +4 -3
@@ -1,7 +1,8 @@ #ifndef USER32_H #define USER32_H 1 -/* IA32 compatible user structures for ptrace. These should be used for 32bit coredumps too. */ +/* IA32 compatible user structures for ptrace. + * These should be used for 32bit coredumps too. */ struct user_i387_ia32_struct { u32 cwd;
@@ -42,9 +43,9 @@ struct user_regs_struct32 { };struct user32 { -struct user_regs_struct32 regs;/* Where the registers are actually stored */ +struct user_regs_struct32 regs;/* Where the registers are actually stored */ int u_fpvalid;/* True if math co-processor being used. */ -/* for this mess. Not yet used. */ +/* for this mess. Not yet used. */ struct user_i387_ia32_struct i387;/* Math Co-processor registers. */ /* The rest of this junk is to help gdb figure out what goes where */ __u32 u_tsize;/* Text segment size (pages). */
Signature · 1 line
1.5.4.rc2
Joe Perches
maintainerPATCH 140/148 include/asm-x86/user_64.h: checkpatch cleanups - formatting only
In reply to Joe Perches--- include/asm-x86/user_64.h | 16 +++++++++------- 1 files changed, 9 insertions(+), 7 deletions(-)
include/asm-x86/user_64.h +9 -7
@@ -45,12 +45,13 @@*/ /* This matches the 64bit FXSAVE format as defined by AMD. It is the same -as the 32bit format defined by Intel, except that the selector:offset pairs for -data and eip are replaced with flat 64bit pointers. */ +as the 32bit format defined by Intel, except that the selector:offset pairs +for data and eip are replaced with flat 64bit pointers. */ struct user_i387_struct {unsigned short cwd;unsigned short swd; -unsigned short twd;/* Note this is not the same as the 32bit/x87/FSAVE twd */ +unsigned short twd;/* Note this is not the same as +the 32bit/x87/FSAVE twd */ unsigned short fop;__u64 rip;__u64 rdp;
@@ -97,13 +98,14 @@ struct user_regs_struct {/* When the kernel dumps core, it starts by dumping the user struct - this will be used by gdb to figure out where the data and stack segments are within the file, and what virtual addresses to use. */ -struct user { + +struct user {/* We start with the registers, to mimic the way that "memory" is returned from the ptrace(3,...) function. */ -struct user_regs_struct regs;/* Where the registers are actually stored */ +struct user_regs_struct regs;/* Where the registers are actually stored */ /* ptrace does not yet supply these. Someday.... */ int u_fpvalid;/* True if math co-processor being used. */ -/* for this mess. Not yet used. */ +/* for this mess. Not yet used. */ int pad0;struct user_i387_struct i387;/* Math Co-processor registers. */ /* The rest of this junk is to help gdb figure out what goes where */
@@ -120,7 +122,7 @@ struct user{int pad1;unsigned long u_ar0;/* Used by gdb to help find the values for */ /* the registers. */ -struct user_i387_struct * u_fpstate;/* Math Co-processor pointer. */ +struct user_i387_struct *u_fpstate;/* Math Co-processor pointer. */ unsigned long magic;/* To uniquely identify a core file */ char u_comm[32 ];/* User command that was responsible */ unsigned long u_debugreg[8 ];
Signature · 1 line
1.5.4.rc2
Joe Perches
maintainerPATCH 141/148 include/asm-x86/vdso.h: checkpatch cleanups - formatting only
In reply to Joe Perches--- include/asm-x86/vdso.h | 16 ++++++++++------ 1 files changed, 10 insertions(+), 6 deletions(-)
include/asm-x86/vdso.h +10 -6
@@ -8,9 +8,11 @@ extern const char VDSO64_PRELINK[]; * Given a pointer to the vDSO image, find the pointer to VDSO64_name * as that symbol is defined in the vDSO sources or linker script. */ -#define VDSO64_SYMBOL (base, name) ({ \ - extern const char VDSO64_##name[]; \ - (void *) (VDSO64_##name - VDSO64_PRELINK + (unsigned long) (base)); }) +#define VDSO64_SYMBOL(base, name) +({ \ + extern const char VDSO64_##name[]; \ + (void *)(VDSO64_##name - VDSO64_PRELINK + (unsigned long)(base)); \ +}) #endif #if defined CONFIG_X86_32 || defined CONFIG_COMPAT
@@ -20,9 +22,11 @@ extern const char VDSO32_PRELINK[];* Given a pointer to the vDSO image, find the pointer to VDSO32_name * as that symbol is defined in the vDSO sources or linker script. */ -#define VDSO32_SYMBOL (base, name) ({ \ - extern const char VDSO32_##name[]; \ - (void *) (VDSO32_##name - VDSO32_PRELINK + (unsigned long) (base)); }) +#define VDSO32_SYMBOL (base, name) \ +({ \ + extern const char VDSO32_##name[]; \ + (void *)(VDSO32_##name - VDSO32_PRELINK + (unsigned long)(base)); \ +})#endif /*
Signature · 1 line
1.5.4.rc2
Joe Perches
maintainerPATCH 142/148 include/asm-x86/vga.h: checkpatch cleanups - formatting only
In reply to Joe Perches--- include/asm-x86/vga.h | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
include/asm-x86/vga.h +2 -2
@@ -12,9 +12,9 @@* access the videoram directly without any black magic. */ -#define VGA_MAP_MEM (x,s) (unsigned long)phys_to_virt(x) +#define VGA_MAP_MEM (x, s) (unsigned long)phys_to_virt(x)#define vga_readb (x) (*(x)) -#define vga_writeb (x,y) (*(y) = (x)) +#define vga_writeb (x, y) (*(y) = (x))#endif
Signature · 1 line
1.5.4.rc2
Joe Perches
maintainerPATCH 143/148 include/asm-x86/vm86.h: checkpatch cleanups - formatting only
In reply to Joe Perches--- include/asm-x86/vm86.h | 9 ++++++--- 1 files changed, 6 insertions(+), 3 deletions(-)
include/asm-x86/vm86.h +6 -3
@@ -42,9 +42,11 @@#define VM86_ARG (retval) ((retval) >> 8)#define VM86_SIGNAL 0/* return due to signal */ -#define VM86_UNKNOWN 1/* unhandled GP fault - IO-instruction or similar */ +#define VM86_UNKNOWN 1/* unhandled GP fault +- IO-instruction or similar */ #define VM86_INTx 2/* int3/int x instruction (ARG = x) */ -#define VM86_STI 3/* sti/popf/iret instruction enabled virtual interrupts */ +#define VM86_STI 3/* sti/popf/iret instruction enabled +virtual interrupts */ /* * Additional return values when invoking new vm86()
@@ -205,7 +207,8 @@ void release_vm86_irqs(struct task_struct *);#define handle_vm86_fault (a, b)#define release_vm86_irqs (a) -static inline int handle_vm86_trap (struct kernel_vm86_regs *a,long b,int c) { +static inline int handle_vm86_trap (struct kernel_vm86_regs *a,long b,int c) +{return 0 ; }
Signature · 1 line
1.5.4.rc2
Joe Perches
maintainerPATCH 144/148 include/asm-x86/vmi.h: checkpatch cleanups - formatting only
In reply to Joe Perches--- include/asm-x86/vmi.h | 88 ++++++++++++++++++++++++------------------------ 1 files changed, 44 insertions(+), 44 deletions(-)
include/asm-x86/vmi.h +44 -44
@@ -155,9 +155,9 @@#ifndef __ASSEMBLY__struct vmi_relocation_info { -unsigned char *eip; -unsigned char type; -unsigned char reserved[3 ]; +unsigned char *eip; +unsigned char type; +unsigned char reserved[3 ]; };#endif
@@ -173,53 +173,53 @@ struct vmi_relocation_info {#ifndef __ASSEMBLY__struct vrom_header { -u16 rom_signature;// option ROM signature -u8 rom_length;// ROM length in 512 byte chunks -u8 rom_entry[4 ];// 16-bit code entry point -u8 rom_pad0;// 4-byte align pad -u32 vrom_signature;// VROM identification signature -u8 api_version_min;// Minor version of API -u8 api_version_maj;// Major version of API -u8 jump_slots;// Number of jump slots -u8 reserved1;// Reserved for expansion -u32 virtual_top;// Hypervisor virtual address start -u16 reserved2;// Reserved for expansion -u16 license_offs;// Offset to License string -u16 pci_header_offs;// Offset to PCI OPROM header -u16 pnp_header_offs;// Offset to PnP OPROM header -u32 rom_pad3;// PnP reserverd / VMI reserved -u8 reserved[96 ];// Reserved for headers -char vmi_init[8 ];// VMI_Init jump point -char get_reloc[8 ];// VMI_GetRelocationInfo jump point +u16 rom_signature;/* option ROM signature */ +u8 rom_length;/* ROM length in 512 byte chunks */ +u8 rom_entry[4 ];/* 16-bit code entry point */ +u8 rom_pad0;/* 4-byte align pad */ +u32 vrom_signature;/* VROM identification signature */ +u8 api_version_min;/* Minor version of API */ +u8 api_version_maj;/* Major version of API */ +u8 jump_slots;/* Number of jump slots */ +u8 reserved1;/* Reserved for expansion */ +u32 virtual_top;/* Hypervisor virtual address start */ +u16 reserved2;/* Reserved for expansion */ +u16 license_offs;/* Offset to License string */ +u16 pci_header_offs;/* Offset to PCI OPROM header */ +u16 pnp_header_offs;/* Offset to PnP OPROM header */ +u32 rom_pad3;/* PnP reserverd / VMI reserved */ +u8 reserved[96 ];/* Reserved for headers */ +char vmi_init[8 ];/* VMI_Init jump point */ +char get_reloc[8 ];/* VMI_GetRelocationInfo jump point */ } __attribute__((packed));struct pnp_header { -char sig[4 ]; -char rev; -char size; -short next; -short res; -long devID; -unsigned short manufacturer_offset; -unsigned short product_offset; +char sig[4 ]; +char rev; +char size; +short next; +short res; +long devID; +unsigned short manufacturer_offset; +unsigned short product_offset; } __attribute__((packed));struct pci_header { -char sig[4 ]; -short vendorID; -short deviceID; -short vpdData; -short size; -char rev; -char class; -char subclass; -char interface; -short chunks; -char rom_version_min; -char rom_version_maj; -char codetype; -char lastRom; -short reserved; +char sig[4 ]; +short vendorID; +short deviceID; +short vpdData; +short size; +char rev; +char class; +char subclass; +char interface; +short chunks; +char rom_version_min; +char rom_version_maj; +char codetype; +char lastRom; +short reserved; } __attribute__((packed));/* Function prototypes for bootstrapping */
Signature · 1 line
1.5.4.rc2
Joe Perches
maintainerPATCH 145/148 include/asm-x86/voyager.h: checkpatch cleanups - formatting only
In reply to Joe Perches--- include/asm-x86/voyager.h | 51 +++++++++++++++++++++++++++----------------- 1 files changed, 31 insertions(+), 20 deletions(-)
include/asm-x86/voyager.h +31 -20
@@ -91,8 +91,7 @@#define VOYAGER_WRITE_CONFIG 0x2#define VOYAGER_BYPASS 0xff -typedef struct voyager_asic -{ +typedef struct voyager_asic { __u8 asic_addr; /* ASIC address; Level 4 */ __u8 asic_type; /* ASIC type */ __u8 asic_id; /* ASIC id */
@@ -113,7 +112,7 @@ typedef struct voyager_module {__u16 largest_reg;/* Largest register in the scan path */ __u16 smallest_reg;/* Smallest register in the scan path */ voyager_asic_t *asic;/* First ASIC in scan path (CAT_I) */ -struct voyager_module *submodule;/* Submodule pointer */ +struct voyager_module *submodule;/* Submodule pointer */ struct voyager_module *next;/* Next module in linked list */ } voyager_module_t;
@@ -135,7 +134,7 @@ typedef struct voyager_eeprom_hdr {__u16 cct_offset;__u16 log_length;/* length of err log */ __u16 xsum_end;/* offset to end of -checksum */ +checksum */ __u8 reserved[4 ];__u8 sflag;/* starting sentinal */ __u8 part_number[13 ];/* prom part number */
@@ -148,7 +147,8 @@ typedef struct voyager_eeprom_hdr { -#define VOYAGER_EPROM_SIZE_OFFSET ((__u16)(&(((voyager_eprom_hdr_t *)0)->ee_size))) +#define VOYAGER_EPROM_SIZE_OFFSET \ + ((__u16)(&(((voyager_eprom_hdr_t *)0)->ee_size)))#define VOYAGER_XSUM_END_OFFSET 0x2a/* the following three definitions are for internal table layouts
@@ -199,7 +199,7 @@ typedef struct voyager_asic_data_table {#define VOYAGER_WCBIC_TOM_L 0x4#define VOYAGER_WCBIC_TOM_H 0x5 -/* register defines for Voyager Memory Contol (VMC) +/* register defines for Voyager Memory Contol (VMC) * these are present on L4 machines only */ #define VOYAGER_VMC1 0x81#define VOYAGER_VMC2 0x91
@@ -334,7 +334,7 @@ typedef struct {struct QuadDescription {__u8 Type;/* for type 0 (DYADIC or MONADIC) all fields -* will be zero except for slot */ +* will be zero except for slot */ __u8 StructureVersion;__u32 CPI_BaseAddress;__u32 LARC_BankSize;
@@ -342,7 +342,7 @@ struct QuadDescription {__u8 Slot;/* Processor slots 1 - 4 */ } __attribute__((packed)); -struct ProcBoardInfo { +struct ProcBoardInfo {__u8 Type;__u8 StructureVersion;__u8 NumberOfBoards;
@@ -382,19 +382,30 @@ struct CPU_Info {* packed in it by our friend the compiler. */ typedef struct { -__u8 Mailbox_SUS;/* Written to by SUS to give commands/response to the OS */ -__u8 Mailbox_OS;/* Written to by the OS to give commands/response to SUS */ -__u8 SUS_MailboxVersion;/* Tells the OS which iteration of the interface SUS supports */ -__u8 OS_MailboxVersion;/* Tells SUS which iteration of the interface the OS supports */ -__u32 OS_Flags;/* Flags set by the OS as info for SUS */ -__u32 SUS_Flags;/* Flags set by SUS as info for the OS */ -__u32 WatchDogPeriod;/* Watchdog period (in seconds) which the DP uses to see if the OS is dead */ +__u8 Mailbox_SUS;/* Written to by SUS to give +commands/response to the OS */ +__u8 Mailbox_OS;/* Written to by the OS to give +commands/response to SUS */ +__u8 SUS_MailboxVersion;/* Tells the OS which iteration of the +interface SUS supports */ +__u8 OS_MailboxVersion;/* Tells SUS which iteration of the +interface the OS supports */ +__u32 OS_Flags;/* Flags set by the OS as info for +SUS */ +__u32 SUS_Flags;/* Flags set by SUS as info +for the OS */ +__u32 WatchDogPeriod;/* Watchdog period (in seconds) which +the DP uses to see if the OS +is dead */ __u32 WatchDogCount;/* Updated by the OS on every tic. */ -__u32 MemoryFor_SUS_ErrorLog;/* Flat 32 bit address which tells SUS where to stuff the SUS error log on a dump */ -MC_SlotInformation_t MC_SlotInfo[NUMBER_OF_MC_BUSSES *SLOTS_PER_MC_BUS ];/* Storage for MCA POS data */ +__u32 MemoryFor_SUS_ErrorLog;/* Flat 32 bit address which tells SUS +where to stuff the SUS error log +on a dump */ +MC_SlotInformation_t MC_SlotInfo[NUMBER_OF_MC_BUSSES *SLOTS_PER_MC_BUS ]; +/* Storage for MCA POS data */ /* All new SECOND_PASS_INTERFACE fields added from this point */ -struct ProcBoardInfo *BoardData; -struct CPU_Info *CPU_Data; +struct ProcBoardInfo *BoardData; +struct CPU_Info *CPU_Data;/* All new fields must be added from this point */ }Voyager_KernelSUS_Mbox_t ;
@@ -478,7 +489,7 @@ struct voyager_SUS {__u32 SUS_errorlog;/* lots of system configuration stuff under here */ }; - +/* Variables exported by voyager_smp */ extern __u32 voyager_extended_vic_processors;extern __u32 voyager_allowed_boot_processors;
Signature · 1 line
1.5.4.rc2
Joe Perches
maintainerPATCH 146/148 include/asm-x86/vsyscall.h: checkpatch cleanups - formatting only
In reply to Joe Perches--- include/asm-x86/vsyscall.h | 21 +++++++++++++-------- 1 files changed, 13 insertions(+), 8 deletions(-)
include/asm-x86/vsyscall.h +13 -8
@@ -11,20 +11,25 @@ enum vsyscall_num {#define VSYSCALL_SIZE 1024#define VSYSCALL_END (-2UL << 20)#define VSYSCALL_MAPPED_PAGES 1 -#define VSYSCALL_ADDR (vsyscall_nr) (VSYSCALL_START+VSYSCALL_SIZE*(vsyscall_nr)) +#define VSYSCALL_ADDR (vsyscall_nr) \ + (VSYSCALL_START + VSYSCALL_SIZE * (vsyscall_nr))#ifdef __KERNEL__#include <linux/seqlock.h> -#define __section_vgetcpu_mode __attribute__ ((unused, __section__ (".vgetcpu_mode"), aligned(16))) -#define __section_jiffies __attribute__ ((unused, __section__ (".jiffies"), aligned(16))) +#define __section_vgetcpu_mode \ + __attribute__ ((unused, __section__ (".vgetcpu_mode"), aligned(16))) +#define __section_jiffies \ + __attribute__ ((unused, __section__ (".jiffies"), aligned(16)))/* Definitions for CONFIG_GENERIC_TIME definitions */ -#define __section_vsyscall_gtod_data __attribute__ \ - ((unused, __section__ (".vsyscall_gtod_data"),aligned(16))) -#define __section_vsyscall_clock __attribute__ \ - ((unused, __section__ (".vsyscall_clock"),aligned(16))) -#define __vsyscall_fn __attribute__ ((unused,__section__(".vsyscall_fn"))) +#define __section_vsyscall_gtod_data \ + __attribute__ ((unused, __section__ (".vsyscall_gtod_data"), \ + aligned(16))) +#define __section_vsyscall_clock \ + __attribute__ ((unused, __section__ (".vsyscall_clock"), aligned(16))) +#define __vsyscall_fn \ + __attribute__ ((unused, __section__(".vsyscall_fn")))#define VGETCPU_RDTSCP 1#define VGETCPU_LSL 2
Signature · 1 line
1.5.4.rc2
Joe Perches
maintainerPATCH 147/148 include/asm-x86/xor_32.h: checkpatch cleanups - formatting only
In reply to Joe Perches--- include/asm-x86/xor_32.h | 494 +++++++++++++++++++++++----------------------- 1 files changed, 248 insertions(+), 246 deletions(-)
include/asm-x86/xor_32.h +248 -246
@@ -16,12 +16,12 @@* Copyright (C) 1998 Ingo Molnar. */ -#define LD (x,y) " movq 8*("#x")(%1), %%mm"#y" ;\n" -#define ST (x,y) " movq %%mm"#y", 8*("#x")(%1) ;\n" -#define XO1 (x,y) " pxor 8*("#x")(%2), %%mm"#y" ;\n" -#define XO2 (x,y) " pxor 8*("#x")(%3), %%mm"#y" ;\n" -#define XO3 (x,y) " pxor 8*("#x")(%4), %%mm"#y" ;\n" -#define XO4 (x,y) " pxor 8*("#x")(%5), %%mm"#y" ;\n" +#define LD (x, y) " movq 8*("#x")(%1), %%mm"#y" ;\n" +#define ST (x, y) " movq %%mm"#y", 8*("#x")(%1) ;\n" +#define XO1 (x, y) " pxor 8*("#x")(%2), %%mm"#y" ;\n" +#define XO2 (x, y) " pxor 8*("#x")(%3), %%mm"#y" ;\n" +#define XO3 (x, y) " pxor 8*("#x")(%4), %%mm"#y" ;\n" +#define XO4 (x, y) " pxor 8*("#x")(%5), %%mm"#y" ;\n"#include <asm/i387.h>
@@ -32,24 +32,24 @@ xor_pII_mmx_2(unsigned long bytes, unsigned long *p1, unsigned long *p2)kernel_fpu_begin (); - __asm__ __volatile__ ( + asm volatile(#undef BLOCK -#define BLOCK (i) \ - LD(i,0) \ - LD(i+1,1) \ - LD(i+2,2) \ - LD(i+3,3) \ - XO1(i,0) \ - ST(i,0) \ - XO1(i+1,1) \ - ST(i+1,1) \ - XO1(i+2,2) \ - ST(i+2,2) \ - XO1(i+3,3) \ - ST(i+3,3) +#define BLOCK (i) \ + LD(i, 0) \ + LD(i + 1, 1) \ + LD(i + 2, 2) \ + LD(i + 3, 3) \ + XO1(i, 0) \ + ST(i, 0) \ + XO1(i+1, 1) \ + ST(i+1, 1) \ + XO1(i + 2, 2) \ + ST(i + 2, 2) \ + XO1(i + 3, 3) \ + ST(i + 3, 3)" .align 32 ;\n" -" 1: ;\n" +" 1: ;\n" BLOCK(0)BLOCK (4 )
@@ -76,25 +76,25 @@ xor_pII_mmx_3(unsigned long bytes, unsigned long *p1, unsigned long *p2,kernel_fpu_begin (); - __asm__ __volatile__ ( + asm volatile(#undef BLOCK -#define BLOCK (i) \ - LD(i,0) \ - LD(i+1,1) \ - LD(i+2,2) \ - LD(i+3,3) \ - XO1(i,0) \ - XO1(i+1,1) \ - XO1(i+2,2) \ - XO1(i+3,3) \ - XO2(i,0) \ - ST(i,0) \ - XO2(i+1,1) \ - ST(i+1,1) \ - XO2(i+2,2) \ - ST(i+2,2) \ - XO2(i+3,3) \ - ST(i+3,3) +#define BLOCK (i) \ + LD(i, 0) \ + LD(i + 1, 1) \ + LD(i + 2, 2) \ + LD(i + 3, 3) \ + XO1(i, 0) \ + XO1(i + 1, 1) \ + XO1(i + 2, 2) \ + XO1(i + 3, 3) \ + XO2(i, 0) \ + ST(i, 0) \ + XO2(i + 1, 1) \ + ST(i + 1, 1) \ + XO2(i + 2, 2) \ + ST(i + 2, 2) \ + XO2(i + 3, 3) \ + ST(i + 3, 3)" .align 32 ;\n" " 1: ;\n"
@@ -125,29 +125,29 @@ xor_pII_mmx_4(unsigned long bytes, unsigned long *p1, unsigned long *p2,kernel_fpu_begin (); - __asm__ __volatile__ ( + asm volatile(#undef BLOCK -#define BLOCK (i) \ - LD(i,0) \ - LD(i+1,1) \ - LD(i+2,2) \ - LD(i+3,3) \ - XO1(i,0) \ - XO1(i+1,1) \ - XO1(i+2,2) \ - XO1(i+3,3) \ - XO2(i,0) \ - XO2(i+1,1) \ - XO2(i+2,2) \ - XO2(i+3,3) \ - XO3(i,0) \ - ST(i,0) \ - XO3(i+1,1) \ - ST(i+1,1) \ - XO3(i+2,2) \ - ST(i+2,2) \ - XO3(i+3,3) \ - ST(i+3,3) +#define BLOCK (i) \ + LD(i, 0) \ + LD(i + 1, 1) \ + LD(i + 2, 2) \ + LD(i + 3, 3) \ + XO1(i, 0) \ + XO1(i + 1, 1) \ + XO1(i + 2, 2) \ + XO1(i + 3, 3) \ + XO2(i, 0) \ + XO2(i + 1, 1) \ + XO2(i + 2, 2) \ + XO2(i + 3, 3) \ + XO3(i, 0) \ + ST(i, 0) \ + XO3(i + 1, 1) \ + ST(i + 1, 1) \ + XO3(i + 2, 2) \ + ST(i + 2, 2) \ + XO3(i + 3, 3) \ + ST(i + 3, 3)" .align 32 ;\n" " 1: ;\n"
@@ -186,35 +186,35 @@ xor_pII_mmx_5(unsigned long bytes, unsigned long *p1, unsigned long *p2,because we modify p4 and p5 there, but we can't mark them as read/write, otherwise we'd overflow the 10-asm-operands limit of GCC < 3.1. */ - __asm__ ("" :"+r" (p4),"+r" (p5)); + asm("" :"+r" (p4),"+r" (p5)); - __asm__ __volatile__ ( + asm volatile(#undef BLOCK -#define BLOCK (i) \ - LD(i,0) \ - LD(i+1,1) \ - LD(i+2,2) \ - LD(i+3,3) \ - XO1(i,0) \ - XO1(i+1,1) \ - XO1(i+2,2) \ - XO1(i+3,3) \ - XO2(i,0) \ - XO2(i+1,1) \ - XO2(i+2,2) \ - XO2(i+3,3) \ - XO3(i,0) \ - XO3(i+1,1) \ - XO3(i+2,2) \ - XO3(i+3,3) \ - XO4(i,0) \ - ST(i,0) \ - XO4(i+1,1) \ - ST(i+1,1) \ - XO4(i+2,2) \ - ST(i+2,2) \ - XO4(i+3,3) \ - ST(i+3,3) +#define BLOCK (i) \ + LD(i, 0) \ + LD(i + 1, 1) \ + LD(i + 2, 2) \ + LD(i + 3, 3) \ + XO1(i, 0) \ + XO1(i + 1, 1) \ + XO1(i + 2, 2) \ + XO1(i + 3, 3) \ + XO2(i, 0) \ + XO2(i + 1, 1) \ + XO2(i + 2, 2) \ + XO2(i + 3, 3) \ + XO3(i, 0) \ + XO3(i + 1, 1) \ + XO3(i + 2, 2) \ + XO3(i + 3, 3) \ + XO4(i, 0) \ + ST(i, 0) \ + XO4(i + 1, 1) \ + ST(i + 1, 1) \ + XO4(i + 2, 2) \ + ST(i + 2, 2) \ + XO4(i + 3, 3) \ + ST(i + 3, 3)" .align 32 ;\n" " 1: ;\n"
@@ -233,13 +233,13 @@ xor_pII_mmx_5(unsigned long bytes, unsigned long *p1, unsigned long *p2," jnz 1b ;\n" : "+r" (lines)," +r" (p1), " +r" (p2), " +r" (p3) - : "r" (p4), "r" (p5) + : "r" (p4), "r" (p5) : "memory");/* p4 and p5 were modified, and now the variables are dead. Clobber them just to be sure nobody does something stupid like assuming they have some legal value. */ - __asm__ ("" : "=r" (p4), "=r" (p5)); + asm("" : "=r" (p4), "=r" (p5));kernel_fpu_end (); }
@@ -259,7 +259,7 @@ xor_p5_mmx_2(unsigned long bytes, unsigned long *p1, unsigned long *p2)kernel_fpu_begin (); - __asm__ __volatile__ ( + asmvolatile (" .align 32 ;\n" " 1: ;\n" " movq (%1), %%mm0 ;\n"
@@ -286,7 +286,7 @@ xor_p5_mmx_2(unsigned long bytes, unsigned long *p1, unsigned long *p2)" pxor 56(%2), %%mm7 ;\n" " movq %%mm6, 48(%1) ;\n" " movq %%mm7, 56(%1) ;\n" - +" addl $64, %1 ;\n" " addl $64, %2 ;\n" " decl %0 ;\n"
@@ -307,7 +307,7 @@ xor_p5_mmx_3(unsigned long bytes, unsigned long *p1, unsigned long *p2,kernel_fpu_begin (); - __asm__ __volatile__ ( + asmvolatile (" .align 32,0x90 ;\n" " 1: ;\n" " movq (%1), %%mm0 ;\n"
@@ -342,7 +342,7 @@ xor_p5_mmx_3(unsigned long bytes, unsigned long *p1, unsigned long *p2," pxor 56(%3), %%mm7 ;\n" " movq %%mm6, 48(%1) ;\n" " movq %%mm7, 56(%1) ;\n" - +" addl $64, %1 ;\n" " addl $64, %2 ;\n" " addl $64, %3 ;\n"
@@ -364,7 +364,7 @@ xor_p5_mmx_4(unsigned long bytes, unsigned long *p1, unsigned long *p2,kernel_fpu_begin (); - __asm__ __volatile__ ( + asmvolatile (" .align 32,0x90 ;\n" " 1: ;\n" " movq (%1), %%mm0 ;\n"
@@ -407,7 +407,7 @@ xor_p5_mmx_4(unsigned long bytes, unsigned long *p1, unsigned long *p2," pxor 56(%4), %%mm7 ;\n" " movq %%mm6, 48(%1) ;\n" " movq %%mm7, 56(%1) ;\n" - +" addl $64, %1 ;\n" " addl $64, %2 ;\n" " addl $64, %3 ;\n"
@@ -436,9 +436,9 @@ xor_p5_mmx_5(unsigned long bytes, unsigned long *p1, unsigned long *p2,because we modify p4 and p5 there, but we can't mark them as read/write, otherwise we'd overflow the 10-asm-operands limit of GCC < 3.1. */ - __asm__ ("" :"+r" (p4),"+r" (p5)); + asm("" :"+r" (p4),"+r" (p5)); - __asm__ __volatile__ ( + asmvolatile (" .align 32,0x90 ;\n" " 1: ;\n" " movq (%1), %%mm0 ;\n"
@@ -489,7 +489,7 @@ xor_p5_mmx_5(unsigned long bytes, unsigned long *p1, unsigned long *p2," pxor 56(%5), %%mm7 ;\n" " movq %%mm6, 48(%1) ;\n" " movq %%mm7, 56(%1) ;\n" - +" addl $64, %1 ;\n" " addl $64, %2 ;\n" " addl $64, %3 ;\n"
@@ -505,7 +505,7 @@ xor_p5_mmx_5(unsigned long bytes, unsigned long *p1, unsigned long *p2,/* p4 and p5 were modified, and now the variables are dead. Clobber them just to be sure nobody does something stupid like assuming they have some legal value. */ - __asm__ ("" :"=r" (p4),"=r" (p5)); + asm("" :"=r" (p4),"=r" (p5));kernel_fpu_end (); }
@@ -531,11 +531,12 @@ static struct xor_block_template xor_block_p5_mmx = {* Copyright (C) 1999 Zach Brown (with obvious credit due Ingo) */ -#define XMMS_SAVE do { \ +#define XMMS_SAVE \ +do { \ preempt_disable(); \ cr0 = read_cr0(); \ clts(); \ - __asm__ __volatile__ ( \ + asm volatile( \ "movups %%xmm0,(%0) ;\n\t" \ "movups %%xmm1,0x10(%0) ;\n\t" \ "movups %%xmm2,0x20(%0) ;\n\t" \
@@ -543,10 +544,11 @@ static struct xor_block_template xor_block_p5_mmx = { : \ : "r" (xmm_save) \ : "memory"); \ -} while(0) +} while (0) -#define XMMS_RESTORE do { \ - __asm__ __volatile__ ( \ +#define XMMS_RESTORE \ +do { \ + asm volatile( \ "sfence ;\n\t" \ "movups (%0),%%xmm0 ;\n\t" \ "movups 0x10(%0),%%xmm1 ;\n\t" \
@@ -557,76 +559,76 @@ static struct xor_block_template xor_block_p5_mmx = { : "memory"); \write_cr0 (cr0); \preempt_enable (); \ -} while(0) +} while (0)#define ALIGN16 __attribute__((aligned(16)))#define OFFS (x) "16*("#x")"#define PF_OFFS (x) "256+16*("#x")"#define PF0 (x) " prefetchnta "PF_OFFS(x)"(%1) ;\n" -#define LD (x,y) " movaps "OFFS(x)"(%1), %%xmm"#y" ;\n" -#define ST (x,y) " movaps %%xmm"#y", "OFFS(x)"(%1) ;\n" +#define LD (x, y) " movaps "OFFS(x)"(%1), %%xmm"#y" ;\n" +#define ST (x, y) " movaps %%xmm"#y", "OFFS(x)"(%1) ;\n"#define PF1 (x) " prefetchnta "PF_OFFS(x)"(%2) ;\n"#define PF2 (x) " prefetchnta "PF_OFFS(x)"(%3) ;\n"#define PF3 (x) " prefetchnta "PF_OFFS(x)"(%4) ;\n"#define PF4 (x) " prefetchnta "PF_OFFS(x)"(%5) ;\n"#define PF5 (x) " prefetchnta "PF_OFFS(x)"(%6) ;\n" -#define XO1 (x,y) " xorps "OFFS(x)"(%2), %%xmm"#y" ;\n" -#define XO2 (x,y) " xorps "OFFS(x)"(%3), %%xmm"#y" ;\n" -#define XO3 (x,y) " xorps "OFFS(x)"(%4), %%xmm"#y" ;\n" -#define XO4 (x,y) " xorps "OFFS(x)"(%5), %%xmm"#y" ;\n" -#define XO5 (x,y) " xorps "OFFS(x)"(%6), %%xmm"#y" ;\n" +#define XO1 (x, y) " xorps "OFFS(x)"(%2), %%xmm"#y" ;\n" +#define XO2 (x, y) " xorps "OFFS(x)"(%3), %%xmm"#y" ;\n" +#define XO3 (x, y) " xorps "OFFS(x)"(%4), %%xmm"#y" ;\n" +#define XO4 (x, y) " xorps "OFFS(x)"(%5), %%xmm"#y" ;\n" +#define XO5 (x, y) " xorps "OFFS(x)"(%6), %%xmm"#y" ;\n"static void xor_sse_2 (unsigned long bytes,unsigned long *p1,unsigned long *p2) { -unsigned long lines = bytes >>8 ; +unsigned long lines = bytes >>8 ; char xmm_save[16*4] ALIGN16;int cr0;XMMS_SAVE ; - __asm__ __volatile__ ( + asm volatile(#undef BLOCK -#define BLOCK (i) \ - LD(i,0) \ - LD(i+1,1) \ +#define BLOCK (i) \ + LD(i, 0) \ + LD(i + 1, 1) \ PF1(i) \ - PF1(i+2) \ - LD(i+2,2) \ - LD(i+3,3) \ - PF0(i+4) \ - PF0(i+6) \ - XO1(i,0) \ - XO1(i+1,1) \ - XO1(i+2,2) \ - XO1(i+3,3) \ - ST(i,0) \ - ST(i+1,1) \ - ST(i+2,2) \ - ST(i+3,3) \ + PF1(i + 2) \ + LD(i + 2, 2) \ + LD(i + 3, 3) \ + PF0(i + 4) \ + PF0(i + 6) \ + XO1(i, 0) \ + XO1(i + 1, 1) \ + XO1(i + 2, 2) \ + XO1(i + 3, 3) \ + ST(i, 0) \ + ST(i + 1, 1) \ + ST(i + 2, 2) \ + ST(i + 3, 3) \ PF0(0) PF0(2)" .align 32 ;\n" -" 1: ;\n" +" 1: ;\n" BLOCK(0) BLOCK(4) BLOCK(8) BLOCK(12) -" addl $256, %1 ;\n" -" addl $256, %2 ;\n" -" decl %0 ;\n" -" jnz 1b ;\n" +" addl $256, %1 ;\n" +" addl $256, %2 ;\n" +" decl %0 ;\n" +" jnz 1b ;\n" : "+r" (lines)," +r" (p1), " +r" (p2) : - : "memory"); + : "memory");XMMS_RESTORE ; }
@@ -635,59 +637,59 @@ static void xor_sse_3(unsigned long bytes, unsigned long *p1, unsigned long *p2, unsigned long *p3) { - unsigned long lines = bytes >> 8; + unsigned long lines = bytes >> 8; char xmm_save[16*4] ALIGN16;int cr0;XMMS_SAVE ; - __asm__ __volatile__ ( + asm volatile(#undef BLOCK#define BLOCK (i) \ PF1(i) \ - PF1(i+2) \ + PF1(i + 2) \ LD(i,0) \ - LD(i+1,1) \ - LD(i+2,2) \ - LD(i+3,3) \ + LD(i + 1, 1) \ + LD(i + 2, 2) \ + LD(i + 3, 3) \ PF2(i) \ - PF2(i+2) \ - PF0(i+4) \ - PF0(i+6) \ + PF2(i + 2) \ + PF0(i + 4) \ + PF0(i + 6) \ XO1(i,0) \ - XO1(i+1,1) \ - XO1(i+2,2) \ - XO1(i+3,3) \ + XO1(i + 1, 1) \ + XO1(i + 2, 2) \ + XO1(i + 3, 3) \ XO2(i,0) \ - XO2(i+1,1) \ - XO2(i+2,2) \ - XO2(i+3,3) \ + XO2(i + 1, 1) \ + XO2(i + 2, 2) \ + XO2(i + 3, 3) \ ST(i,0) \ - ST(i+1,1) \ - ST(i+2,2) \ - ST(i+3,3) \ + ST(i + 1, 1) \ + ST(i + 2, 2) \ + ST(i + 3, 3) \ PF0(0) PF0(2)" .align 32 ;\n" -" 1: ;\n" +" 1: ;\n" BLOCK(0) BLOCK(4) BLOCK(8) BLOCK(12) -" addl $256, %1 ;\n" -" addl $256, %2 ;\n" -" addl $256, %3 ;\n" -" decl %0 ;\n" -" jnz 1b ;\n" +" addl $256, %1 ;\n" +" addl $256, %2 ;\n" +" addl $256, %3 ;\n" +" decl %0 ;\n" +" jnz 1b ;\n" : "+r" (lines)," +r" (p1), " +r"(p2), " +r"(p3) : - : "memory" ); + : "memory" );XMMS_RESTORE ; }
@@ -696,66 +698,66 @@ static void xor_sse_4(unsigned long bytes, unsigned long *p1, unsigned long *p2, unsigned long *p3, unsigned long *p4) { - unsigned long lines = bytes >> 8; + unsigned long lines = bytes >> 8; char xmm_save[16*4] ALIGN16;int cr0;XMMS_SAVE ; - __asm__ __volatile__ ( + asm volatile(#undef BLOCK#define BLOCK (i) \ PF1(i) \ - PF1(i+2) \ + PF1(i + 2) \ LD(i,0) \ - LD(i+1,1) \ - LD(i+2,2) \ - LD(i+3,3) \ + LD(i + 1, 1) \ + LD(i + 2, 2) \ + LD(i + 3, 3) \ PF2(i) \ - PF2(i+2) \ + PF2(i + 2) \ XO1(i,0) \ - XO1(i+1,1) \ - XO1(i+2,2) \ - XO1(i+3,3) \ + XO1(i + 1, 1) \ + XO1(i + 2, 2) \ + XO1(i + 3, 3) \ PF3(i) \ - PF3(i+2) \ - PF0(i+4) \ - PF0(i+6) \ + PF3(i + 2) \ + PF0(i + 4) \ + PF0(i + 6) \ XO2(i,0) \ - XO2(i+1,1) \ - XO2(i+2,2) \ - XO2(i+3,3) \ + XO2(i + 1, 1) \ + XO2(i + 2, 2) \ + XO2(i + 3, 3) \ XO3(i,0) \ - XO3(i+1,1) \ - XO3(i+2,2) \ - XO3(i+3,3) \ + XO3(i + 1, 1) \ + XO3(i + 2, 2) \ + XO3(i + 3, 3) \ ST(i,0) \ - ST(i+1,1) \ - ST(i+2,2) \ - ST(i+3,3) \ + ST(i + 1, 1) \ + ST(i + 2, 2) \ + ST(i + 3, 3) \ PF0(0) PF0(2)" .align 32 ;\n" -" 1: ;\n" +" 1: ;\n" BLOCK(0) BLOCK(4) BLOCK(8) BLOCK(12) -" addl $256, %1 ;\n" -" addl $256, %2 ;\n" -" addl $256, %3 ;\n" -" addl $256, %4 ;\n" -" decl %0 ;\n" -" jnz 1b ;\n" +" addl $256, %1 ;\n" +" addl $256, %2 ;\n" +" addl $256, %3 ;\n" +" addl $256, %4 ;\n" +" decl %0 ;\n" +" jnz 1b ;\n" : "+r" (lines)," +r" (p1), " +r" (p2), " +r" (p3), " +r" (p4) : - : "memory" ); + : "memory" );XMMS_RESTORE ; }
@@ -764,7 +766,7 @@ static void xor_sse_5(unsigned long bytes, unsigned long *p1, unsigned long *p2, unsigned long *p3, unsigned long *p4, unsigned long *p5) { - unsigned long lines = bytes >> 8; + unsigned long lines = bytes >> 8; char xmm_save[16*4] ALIGN16;int cr0;
@@ -776,65 +778,65 @@ xor_sse_5(unsigned long bytes, unsigned long *p1, unsigned long *p2,because we modify p4 and p5 there, but we can't mark them as read/write, otherwise we'd overflow the 10-asm-operands limit of GCC < 3.1. */ - __asm__ ("" :"+r" (p4),"+r" (p5)); + asm("" :"+r" (p4),"+r" (p5)); - __asm__ __volatile__ ( + asm volatile(#undef BLOCK#define BLOCK (i) \ PF1(i) \ - PF1(i+2) \ + PF1(i + 2) \ LD(i,0) \ - LD(i+1,1) \ - LD(i+2,2) \ - LD(i+3,3) \ + LD(i + 1, 1) \ + LD(i + 2, 2) \ + LD(i + 3, 3) \ PF2(i) \ - PF2(i+2) \ + PF2(i + 2) \ XO1(i,0) \ - XO1(i+1,1) \ - XO1(i+2,2) \ - XO1(i+3,3) \ + XO1(i + 1, 1) \ + XO1(i + 2, 2) \ + XO1(i + 3, 3) \ PF3(i) \ - PF3(i+2) \ + PF3(i + 2) \ XO2(i,0) \ - XO2(i+1,1) \ - XO2(i+2,2) \ - XO2(i+3,3) \ + XO2(i + 1, 1) \ + XO2(i + 2, 2) \ + XO2(i + 3, 3) \ PF4(i) \ - PF4(i+2) \ - PF0(i+4) \ - PF0(i+6) \ + PF4(i + 2) \ + PF0(i + 4) \ + PF0(i + 6) \ XO3(i,0) \ - XO3(i+1,1) \ - XO3(i+2,2) \ - XO3(i+3,3) \ + XO3(i + 1, 1) \ + XO3(i + 2, 2) \ + XO3(i + 3, 3) \ XO4(i,0) \ - XO4(i+1,1) \ - XO4(i+2,2) \ - XO4(i+3,3) \ + XO4(i + 1, 1) \ + XO4(i + 2, 2) \ + XO4(i + 3, 3) \ ST(i,0) \ - ST(i+1,1) \ - ST(i+2,2) \ - ST(i+3,3) \ + ST(i + 1, 1) \ + ST(i + 2, 2) \ + ST(i + 3, 3) \ PF0(0) PF0(2)" .align 32 ;\n" -" 1: ;\n" +" 1: ;\n" BLOCK(0) BLOCK(4) BLOCK(8) BLOCK(12) -" addl $256, %1 ;\n" -" addl $256, %2 ;\n" -" addl $256, %3 ;\n" -" addl $256, %4 ;\n" -" addl $256, %5 ;\n" -" decl %0 ;\n" -" jnz 1b ;\n" +" addl $256, %1 ;\n" +" addl $256, %2 ;\n" +" addl $256, %3 ;\n" +" addl $256, %4 ;\n" +" addl $256, %5 ;\n" +" decl %0 ;\n" +" jnz 1b ;\n" : "+r" (lines)," +r" (p1), " +r" (p2), " +r" (p3) : "r" (p4), "r" (p5)
@@ -843,17 +845,17 @@ xor_sse_5(unsigned long bytes, unsigned long *p1, unsigned long *p2,/* p4 and p5 were modified, and now the variables are dead. Clobber them just to be sure nobody does something stupid like assuming they have some legal value. */ - __asm__ ("" :"=r" (p4),"=r" (p5)); + asm("" :"=r" (p4),"=r" (p5));XMMS_RESTORE ; }static struct xor_block_template xor_block_pIII_sse = { - .name ="pIII_sse" , - .do_2 = xor_sse_2, - .do_3 = xor_sse_3, - .do_4 = xor_sse_4, - .do_5 = xor_sse_5, + .name ="pIII_sse" , + .do_2 = xor_sse_2, + .do_3 = xor_sse_3, + .do_4 = xor_sse_4, + .do_5 = xor_sse_5, };/* Also try the generic routines. */
@@ -861,21 +863,21 @@ static struct xor_block_template xor_block_pIII_sse = {#undef XOR_TRY_TEMPLATES#define XOR_TRY_TEMPLATES \ - do { \ - xor_speed(&xor_block_8regs); \ - xor_speed(&xor_block_8regs_p); \ - xor_speed(&xor_block_32regs); \ - xor_speed(&xor_block_32regs_p); \ - if (cpu_has_xmm) \ - xor_speed(&xor_block_pIII_sse); \ - if (cpu_has_mmx) { \ - xor_speed(&xor_block_pII_mmx); \ - xor_speed(&xor_block_p5_mmx); \ - } \ - } while (0) +do { \ + xor_speed(&xor_block_8regs); \ + xor_speed(&xor_block_8regs_p); \ + xor_speed(&xor_block_32regs); \ + xor_speed(&xor_block_32regs_p); \ + if (cpu_has_xmm) \ + xor_speed(&xor_block_pIII_sse); \ + if (cpu_has_mmx) { \ + xor_speed(&xor_block_pII_mmx); \ + xor_speed(&xor_block_p5_mmx); \ + } \ +} while (0)/* We force the use of the SSE xor block because it can write around L2. We may also be able to load into the L1 only depending on how the cpu deals with a load to a line that is being prefetched. */ -#define XOR_SELECT_TEMPLATE (FASTEST ) \ +#define XOR_SELECT_TEMPLATE (FASTEST ) \ (cpu_has_xmm ? &xor_block_pIII_sse : FASTEST)
Signature · 1 line
1.5.4.rc2
Joe Perches
maintainerPATCH 148/148 include/asm-x86/xor_64.h: checkpatch cleanups - formatting only
In reply to Joe Perches--- include/asm-x86/xor_64.h | 294 +++++++++++++++++++++++----------------------- 1 files changed, 149 insertions(+), 145 deletions(-)
include/asm-x86/xor_64.h +149 -145
@@ -24,20 +24,23 @@*/ /* -* x86-64 changes / gcc fixes from Andi Kleen. +* x86-64 changes / gcc fixes from Andi Kleen. * Copyright 2002 Andi Kleen, SuSE Labs. * * This hasn't been optimized for the hammer yet, but there are likely * no advantages to be gotten from x86-64 here anyways. */ -typedef struct {unsigned long a,b; } __attribute__((aligned (16 )))xmm_store_t ; +typedef struct { +unsigned long a, b; +} __attribute__((aligned (16 )))xmm_store_t ; -/* Doesn't use gcc to save the XMM registers, because there is no easy way to +/* Doesn't use gcc to save the XMM registers, because there is no easy way to tell it to do a clts before the register saving. */ -#define XMMS_SAVE do { \ +#define XMMS_SAVE \ +do { \ preempt_disable(); \ - asm volatile ( \ + asm volatile( \ "movq %%cr0,%0 ;\n\t" \ "clts ;\n\t" \ "movups %%xmm0,(%1) ;\n\t" \
@@ -47,10 +50,11 @@ typedef struct { unsigned long a,b; } __attribute__((aligned(16))) xmm_store_t; : "=&r" (cr0) \ : "r" (xmm_save) \ : "memory"); \ -} while(0) +} while (0) -#define XMMS_RESTORE do { \ - asm volatile ( \ +#define XMMS_RESTORE \ +do { \ + asm volatile( \ "sfence ;\n\t" \ "movups (%1),%%xmm0 ;\n\t" \ "movups 0x10(%1),%%xmm1 ;\n\t" \
@@ -61,72 +65,72 @@ typedef struct { unsigned long a,b; } __attribute__((aligned(16))) xmm_store_t; : "r" (cr0), "r" (xmm_save) \ : "memory"); \preempt_enable (); \ -} while(0) +} while (0)#define OFFS (x) "16*("#x")"#define PF_OFFS (x) "256+16*("#x")"#define PF0 (x) " prefetchnta "PF_OFFS(x)"(%[p1]) ;\n" -#define LD (x,y) " movaps "OFFS(x)"(%[p1]), %%xmm"#y" ;\n" -#define ST (x,y) " movaps %%xmm"#y", "OFFS(x)"(%[p1]) ;\n" +#define LD (x, y) " movaps "OFFS(x)"(%[p1]), %%xmm"#y" ;\n" +#define ST (x, y) " movaps %%xmm"#y", "OFFS(x)"(%[p1]) ;\n"#define PF1 (x) " prefetchnta "PF_OFFS(x)"(%[p2]) ;\n"#define PF2 (x) " prefetchnta "PF_OFFS(x)"(%[p3]) ;\n"#define PF3 (x) " prefetchnta "PF_OFFS(x)"(%[p4]) ;\n"#define PF4 (x) " prefetchnta "PF_OFFS(x)"(%[p5]) ;\n"#define PF5 (x) " prefetchnta "PF_OFFS(x)"(%[p6]) ;\n" -#define XO1 (x,y) " xorps "OFFS(x)"(%[p2]), %%xmm"#y" ;\n" -#define XO2 (x,y) " xorps "OFFS(x)"(%[p3]), %%xmm"#y" ;\n" -#define XO3 (x,y) " xorps "OFFS(x)"(%[p4]), %%xmm"#y" ;\n" -#define XO4 (x,y) " xorps "OFFS(x)"(%[p5]), %%xmm"#y" ;\n" -#define XO5 (x,y) " xorps "OFFS(x)"(%[p6]), %%xmm"#y" ;\n" +#define XO1 (x, y) " xorps "OFFS(x)"(%[p2]), %%xmm"#y" ;\n" +#define XO2 (x, y) " xorps "OFFS(x)"(%[p3]), %%xmm"#y" ;\n" +#define XO3 (x, y) " xorps "OFFS(x)"(%[p4]), %%xmm"#y" ;\n" +#define XO4 (x, y) " xorps "OFFS(x)"(%[p5]), %%xmm"#y" ;\n" +#define XO5 (x, y) " xorps "OFFS(x)"(%[p6]), %%xmm"#y" ;\n"static void xor_sse_2 (unsigned long bytes,unsigned long *p1,unsigned long *p2) { -unsigned int lines = bytes >>8 ; +unsigned int lines = bytes >>8 ;unsigned long cr0;xmm_store_t xmm_save[4 ];XMMS_SAVE ; - asm volatile ( + asm volatile(#undef BLOCK#define BLOCK (i) \ - LD(i,0) \ - LD(i+1,1) \ + LD(i, 0) \ + LD(i + 1, 1) \ PF1(i) \ - PF1(i+2) \ - LD(i+2,2) \ - LD(i+3,3) \ - PF0(i+4) \ - PF0(i+6) \ - XO1(i,0) \ - XO1(i+1,1) \ - XO1(i+2,2) \ - XO1(i+3,3) \ - ST(i,0) \ - ST(i+1,1) \ - ST(i+2,2) \ - ST(i+3,3) \ + PF1(i + 2) \ + LD(i + 2, 2) \ + LD(i + 3, 3) \ + PF0(i + 4) \ + PF0(i + 6) \ + XO1(i, 0) \ + XO1(i + 1, 1) \ + XO1(i + 2, 2) \ + XO1(i + 3, 3) \ + ST(i, 0) \ + ST(i + 1, 1) \ + ST(i + 2, 2) \ + ST(i + 3, 3) \ PF0(0) PF0(2)" .align 32 ;\n" -" 1: ;\n" +" 1: ;\n" BLOCK(0) BLOCK(4) BLOCK(8) BLOCK(12) -" addq %[inc], %[p1] ;\n" -" addq %[inc], %[p2] ;\n" +" addq %[inc], %[p1] ;\n" +" addq %[inc], %[p2] ;\n" " decl %[cnt] ; jnz 1b" : [p1] "+r" (p1), [p2] "+r" (p2), [cnt] "+r" (lines) - : [inc] "r" (256UL) - : "memory"); + : [inc] "r" (256UL) + : "memory");XMMS_RESTORE ; }
@@ -141,52 +145,52 @@ xor_sse_3(unsigned long bytes, unsigned long *p1, unsigned long *p2,XMMS_SAVE ; - __asm__ __volatile__ ( + asm volatile(#undef BLOCK#define BLOCK (i) \ PF1(i) \ - PF1(i+2) \ - LD(i,0) \ - LD(i+1,1) \ - LD(i+2,2) \ - LD(i+3,3) \ + PF1(i + 2) \ + LD(i, 0) \ + LD(i + 1, 1) \ + LD(i + 2, 2) \ + LD(i + 3, 3) \ PF2(i) \ - PF2(i+2) \ - PF0(i+4) \ - PF0(i+6) \ - XO1(i,0) \ - XO1(i+1,1) \ - XO1(i+2,2) \ - XO1(i+3,3) \ - XO2(i,0) \ - XO2(i+1,1) \ - XO2(i+2,2) \ - XO2(i+3,3) \ - ST(i,0) \ - ST(i+1,1) \ - ST(i+2,2) \ - ST(i+3,3) \ + PF2(i + 2) \ + PF0(i + 4) \ + PF0(i + 6) \ + XO1(i, 0) \ + XO1(i + 1, 1) \ + XO1(i + 2, 2) \ + XO1(i + 3, 3) \ + XO2(i, 0) \ + XO2(i + 1, 1) \ + XO2(i + 2, 2) \ + XO2(i + 3, 3) \ + ST(i, 0) \ + ST(i + 1, 1) \ + ST(i + 2, 2) \ + ST(i + 3, 3) \ PF0(0) PF0(2)" .align 32 ;\n" -" 1: ;\n" +" 1: ;\n" BLOCK(0) BLOCK(4) BLOCK(8) BLOCK(12) -" addq %[inc], %[p1] ;\n" -" addq %[inc], %[p2] ;\n" -" addq %[inc], %[p3] ;\n" +" addq %[inc], %[p1] ;\n" +" addq %[inc], %[p2] ;\n" +" addq %[inc], %[p3] ;\n" " decl %[cnt] ; jnz 1b" : [cnt] "+r" (lines), [p1] "+r" (p1), [p2] "+r" (p2), [p3] "+r" (p3) : [inc] "r" (256UL) - : "memory"); + : "memory");XMMS_RESTORE ; }
@@ -195,64 +199,64 @@ xor_sse_4(unsigned long bytes, unsigned long *p1, unsigned long *p2, unsigned long *p3, unsigned long *p4) { unsigned int lines = bytes >> 8; -xmm_store_t xmm_save[4 ]; +xmm_store_t xmm_save[4 ];unsigned long cr0;XMMS_SAVE ; - __asm__ __volatile__ ( + asm volatile(#undef BLOCK#define BLOCK (i) \ PF1(i) \ - PF1(i+2) \ - LD(i,0) \ - LD(i+1,1) \ - LD(i+2,2) \ - LD(i+3,3) \ + PF1(i + 2) \ + LD(i, 0) \ + LD(i + 1, 1) \ + LD(i + 2, 2) \ + LD(i + 3, 3) \ PF2(i) \ - PF2(i+2) \ - XO1(i,0) \ - XO1(i+1,1) \ - XO1(i+2,2) \ - XO1(i+3,3) \ + PF2(i + 2) \ + XO1(i, 0) \ + XO1(i + 1, 1) \ + XO1(i + 2, 2) \ + XO1(i + 3, 3) \ PF3(i) \ - PF3(i+2) \ - PF0(i+4) \ - PF0(i+6) \ - XO2(i,0) \ - XO2(i+1,1) \ - XO2(i+2,2) \ - XO2(i+3,3) \ - XO3(i,0) \ - XO3(i+1,1) \ - XO3(i+2,2) \ - XO3(i+3,3) \ - ST(i,0) \ - ST(i+1,1) \ - ST(i+2,2) \ - ST(i+3,3) \ + PF3(i + 2) \ + PF0(i + 4) \ + PF0(i + 6) \ + XO2(i, 0) \ + XO2(i + 1, 1) \ + XO2(i + 2, 2) \ + XO2(i + 3, 3) \ + XO3(i, 0) \ + XO3(i + 1, 1) \ + XO3(i + 2, 2) \ + XO3(i + 3, 3) \ + ST(i, 0) \ + ST(i + 1, 1) \ + ST(i + 2, 2) \ + ST(i + 3, 3) \ PF0(0) PF0(2)" .align 32 ;\n" -" 1: ;\n" +" 1: ;\n" BLOCK(0) BLOCK(4) BLOCK(8) BLOCK(12) -" addq %[inc], %[p1] ;\n" -" addq %[inc], %[p2] ;\n" -" addq %[inc], %[p3] ;\n" -" addq %[inc], %[p4] ;\n" +" addq %[inc], %[p1] ;\n" +" addq %[inc], %[p2] ;\n" +" addq %[inc], %[p3] ;\n" +" addq %[inc], %[p4] ;\n" " decl %[cnt] ; jnz 1b" : [cnt] "+c" (lines), [p1] "+r" (p1), [p2] "+r" (p2), [p3] "+r" (p3), [p4] "+r" (p4) : [inc] "r" (256UL) - : "memory" ); + : "memory" );XMMS_RESTORE ; }
@@ -261,70 +265,70 @@ static void xor_sse_5(unsigned long bytes, unsigned long *p1, unsigned long *p2, unsigned long *p3, unsigned long *p4, unsigned long *p5) { - unsigned int lines = bytes >> 8; + unsigned int lines = bytes >> 8;xmm_store_t xmm_save[4 ];unsigned long cr0;XMMS_SAVE ; - __asm__ __volatile__ ( + asm volatile(#undef BLOCK#define BLOCK (i) \ PF1(i) \ - PF1(i+2) \ - LD(i,0) \ - LD(i+1,1) \ - LD(i+2,2) \ - LD(i+3,3) \ + PF1(i + 2) \ + LD(i, 0) \ + LD(i + 1, 1) \ + LD(i + 2, 2) \ + LD(i + 3, 3) \ PF2(i) \ - PF2(i+2) \ - XO1(i,0) \ - XO1(i+1,1) \ - XO1(i+2,2) \ - XO1(i+3,3) \ + PF2(i + 2) \ + XO1(i, 0) \ + XO1(i + 1, 1) \ + XO1(i + 2, 2) \ + XO1(i + 3, 3) \ PF3(i) \ - PF3(i+2) \ - XO2(i,0) \ - XO2(i+1,1) \ - XO2(i+2,2) \ - XO2(i+3,3) \ + PF3(i + 2) \ + XO2(i, 0) \ + XO2(i + 1, 1) \ + XO2(i + 2, 2) \ + XO2(i + 3, 3) \ PF4(i) \ - PF4(i+2) \ - PF0(i+4) \ - PF0(i+6) \ - XO3(i,0) \ - XO3(i+1,1) \ - XO3(i+2,2) \ - XO3(i+3,3) \ - XO4(i,0) \ - XO4(i+1,1) \ - XO4(i+2,2) \ - XO4(i+3,3) \ - ST(i,0) \ - ST(i+1,1) \ - ST(i+2,2) \ - ST(i+3,3) \ + PF4(i + 2) \ + PF0(i + 4) \ + PF0(i + 6) \ + XO3(i, 0) \ + XO3(i + 1, 1) \ + XO3(i + 2, 2) \ + XO3(i + 3, 3) \ + XO4(i, 0) \ + XO4(i + 1, 1) \ + XO4(i + 2, 2) \ + XO4(i + 3, 3) \ + ST(i, 0) \ + ST(i + 1, 1) \ + ST(i + 2, 2) \ + ST(i + 3, 3) \ PF0(0) PF0(2)" .align 32 ;\n" -" 1: ;\n" +" 1: ;\n" BLOCK(0) BLOCK(4) BLOCK(8) BLOCK(12) -" addq %[inc], %[p1] ;\n" -" addq %[inc], %[p2] ;\n" -" addq %[inc], %[p3] ;\n" -" addq %[inc], %[p4] ;\n" -" addq %[inc], %[p5] ;\n" +" addq %[inc], %[p1] ;\n" +" addq %[inc], %[p2] ;\n" +" addq %[inc], %[p3] ;\n" +" addq %[inc], %[p4] ;\n" +" addq %[inc], %[p5] ;\n" " decl %[cnt] ; jnz 1b" : [cnt] "+c" (lines), - [p1] "+r" (p1), [p2] "+r" (p2), [p3] "+r" (p3), [p4] "+r" (p4), + [p1] "+r" (p1), [p2] "+r" (p2), [p3] "+r" (p3), [p4] "+r" (p4), [p5] "+r" (p5) : [inc] "r" (256UL) : "memory");
@@ -333,18 +337,18 @@ xor_sse_5(unsigned long bytes, unsigned long *p1, unsigned long *p2, }static struct xor_block_template xor_block_sse = { - .name ="generic_sse" , - .do_2 = xor_sse_2, - .do_3 = xor_sse_3, - .do_4 = xor_sse_4, - .do_5 = xor_sse_5, + .name ="generic_sse" , + .do_2 = xor_sse_2, + .do_3 = xor_sse_3, + .do_4 = xor_sse_4, + .do_5 = xor_sse_5, };#undef XOR_TRY_TEMPLATES -#define XOR_TRY_TEMPLATES \ - do { \ - xor_speed(&xor_block_sse); \ - } while (0) +#define XOR_TRY_TEMPLATES \ +do { \ + xor_speed(&xor_block_sse); \ +} while (0)/* We force the use of the SSE xor block because it can write around L2. We may also be able to load into the L1 only depending on how the cpu
Signature · 1 line
1.5.4.rc2
Quoted 1 line · Joe Perches
Cleanup to standardize formatting of .h files
thanks Joe, this is a good first step. I'll keep the 180 patches separated until it's in x86/testing, to ease bisection in the case of any (unlikely) problems - but after that we'll probably combine them into one single commit to not pollute the changelogs too much. In the future you might want to put them into a git tree and ask me to pull them, to not pollute lkml with 180 patches. (but, this is the least of our problems really)
Ingo
Quoted 7 lines · Joe Perches
Ingo's code-quality script totals for
include/asm-x86Errors LOC
Before: 1457 31320 After: 252 31729Changes:
btw., i'd prefer to see more structural cleanups as well. For example, to convert macros that generate code (i.e. just about everything except constants) to inlines. For example in include/asm-x86/processor.h, to convert get_debugreg/set_debugreg or task_pt_regs to inline functions.
Ingo