Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles
On 2/29/16 06:23, Theodore Ts'o wrote: > On Sun, Feb 28, 2016 at 08:47:23AM +0800, Chen Gang wrote: >> >> Excuse me, when I sent this patch, I did not know who I shall send to, I >> have to reference to "./scripts/get_maintainer.pl". >> >> If any members have no time to care about it (every members' time are >> really expensive), please let me know (can reply directly). > > Yes, everybody's time is very expensive. So why are you wasting it > all with a "last post wins" style of argumentation? A maintainer has > NAK'ed it. Please drop this. > For me, I don't care about "last post wins". But I care about the technical correctness, and for me, we (all of us) need try our best to let the email reply as correct as we can, so can avoid to mislead another readers. So for me, if any members have new ideas, suggestions, or completions, they can still reply at any time (may be next day, next week, or next month ...). > There is a reason why whitespace fixes are often consider to have > extreme negative value, and a deep suspicion that people are doing > this just to say that they have a patch in the kernel, perhaps in the > misapprehension that this will help them get a job. > For me, I don't think so, at least for me, contribution and learning is my main goal in open source community, so I mainly focus on correctness. All of us know when some related maintainer NAK'ed, the related patch, of cause, must be dropped, the reason why I still reply the mail is: I shall try to make the discussion/communication as correct as I can. For "get a job", I guess, the open source community is helpful, but I also suggest: if someone wants to "get a job", he/she should not depend on the open source community (community has no duty for it). > Let me say that if I were a hiring manager, and I did a Google search > on a potential job application, and came across a thread like this, my > reaction would be extremely negative. > For me, job hunter and HR hunter can use open source community, but the open source community's main goal is not for job hunter or HR hunter. I guess, the open source community's goal should be: - Develop the relate product (we can treat it as urgent thing, e.g. new features, bug fix). - Learning and discussing the product related technical issues (we can treat it as important thing, I guess, "coding styles issues" should be one of these issues). Welcome any members ideas, suggestions, and completions for it. Thanks. -- Chen Gang (陈刚) Managing Natural Environments is the Duty of Human Beings.
Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles
On Sun, Feb 28, 2016 at 08:47:23AM +0800, Chen Gang wrote: > > Excuse me, when I sent this patch, I did not know who I shall send to, I > have to reference to "./scripts/get_maintainer.pl". > > If any members have no time to care about it (every members' time are > really expensive), please let me know (can reply directly). Yes, everybody's time is very expensive. So why are you wasting it all with a "last post wins" style of argumentation? A maintainer has NAK'ed it. Please drop this. There is a reason why whitespace fixes are often consider to have extreme negative value, and a deep suspicion that people are doing this just to say that they have a patch in the kernel, perhaps in the misapprehension that this will help them get a job. Let me say that if I were a hiring manager, and I did a Google search on a potential job application, and came across a thread like this, my reaction would be extremely negative. - Ted
Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles
On 2/28/16 21:27, Mel Gorman wrote: > On Sun, Feb 28, 2016 at 08:21:40AM +0800, Chen Gang wrote: >> >> For me, NAK also needs reasons. >> > > You already got the reasons. Not only does a patch of this type interfere > with git blame which is important even in headers but I do not think the > patch actually improves the readability of the code. For example, the > comments move to the line after the defintions which to my eye at least > looks clumsy and weird. > For me, in local headers, they may be often modified, and also may be complex, so the code analyzing maybe also be used often. But in common shared headers in ./include (e.g. gfp.h), most of them are simple enough. - Since common shared headers are usually simple, code analyzing is still useful, but not like the body files or local headers (code analyzing are very useful for body files and local headers). - Common shared headers are quite often read by most programmers, so common shared headers need take more care about its coding styles. - Then for common shared headers, the coding style is 1st. And for __GFP_MOVABLE definition (with ZONE_MOVABLE), I guess, we can keep it no touch (like what I originally said: if the related member stick to, we can keep it no touch). And for me, the other macro definitions which out of 80 columns, can be fixed in normal ways (let the related comments ahead of macro definition ), does this change also have negative effect? >> I guess they are related with this patch, and their NAKs' reason are: mm >> and trivial don't care about this coding style issue, is it correct? >> > > No. Coding style is important but it's a guideline not a law. Yes. For me, vertical split window in vim is very useful, I almost always use this feature when read source code in full screen under Macbook client, when columns are 86+, it will be wrapped (I feel really not quite good). And occasionally (really not often), we may copy/past part of contents in the header files (e.g. constant definition) to the pdf file as appendix. So except the string broken, or "grep -rn xxx * | grep yyy", 80 columns limitation is always helpful to me. > There are > cases where breaking it results in perfectly readable code. At least one > my my own recent patches was flagged by checkpatch as having style issues > but fixing the style was considerably harder to read so I left it. If the > definitions in that header need to change again in the future and there > are style issues then they can be fixed in the context of a functional > change instead of patching style just for the sake of it. > For me, except just modify the related contents, usually, we need devide the patch into 2: one for real modification, the other for coding styles. And in some of common, base, shared headers in ./include (e.g. gfp.h), I guess, most of contents *should* not be changed quite often, so the bad coding styles probably will be alive in a long term. Thanks. -- Chen Gang (陈刚) Managing Natural Environments is the Duty of Human Beings.
Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles
On Sun, Feb 28, 2016 at 08:21:40AM +0800, Chen Gang wrote: > > On 2/28/16 00:53, Theodore Ts'o wrote: > > On Sat, Feb 27, 2016 at 10:32:04PM +0800, Chen Gang wrote: > >> I don't think so. Of cause NOT the "CODE CHURN". It is not correct to > >> make an early decision during discussing. > > > > There is no discussion. If the maintainer has NAK'ed it. That's the > > end of the dicsussion. Period. See: > > > > For me, NAK also needs reasons. > You already got the reasons. Not only does a patch of this type interfere with git blame which is important even in headers but I do not think the patch actually improves the readability of the code. For example, the comments move to the line after the defintions which to my eye at least looks clumsy and weird. > I guess they are related with this patch, and their NAKs' reason are: mm > and trivial don't care about this coding style issue, is it correct? > No. Coding style is important but it's a guideline not a law. There are cases where breaking it results in perfectly readable code. At least one my my own recent patches was flagged by checkpatch as having style issues but fixing the style was considerably harder to read so I left it. If the definitions in that header need to change again in the future and there are style issues then they can be fixed in the context of a functional change instead of patching style just for the sake of it. -- Mel Gorman SUSE Labs
Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles
On 2/28/16 07:14, Jiri Kosina wrote: > On Sat, 27 Feb 2016, Chen Gang wrote: > >>> Mel, as an MM developer, has already NACK'ed the patch, which means >>> you should not send the patch to **any** upstream maintainer for >>> inclusion. >> >> I don't think I "should not ...". I only care about correctness and >> contribution, I don't care about any members ideas and their thinking. >> When we have different ideas or thinking, we need discuss. > > If by "discuss" you mean "30+ email thread about where to put a line > break", please drop me from CC next time this discussion is going to > happen. Thanks. > Excuse me, when I sent this patch, I did not know who I shall send to, I have to reference to "./scripts/get_maintainer.pl". If any members have no time to care about it (every members' time are really expensive), please let me know (can reply directly). Thanks. >> For common shared header files, for me, we should really take more care >> about the coding styles. >> >> - If the common shared header files don't care about the coding styles, >>I guess any body files will have much more excuses for "do not care >>about coding styles". >> >> - That means our kernel whole source files need not care about coding >>styles at all!! >> >> - It is really really VERY BAD!! >> >> If someone only dislike me to send the related patches, I suggest: Let >> another member(s) "run checkpatch -file" on the whole "./include" sub- >> directory, and fix all coding styles issues. > > Which is exactly what you shouldn't do. > For me, I also guess, I am not the suitable member to do that (in fact, I dislike to do like that - "run checkpath -file" on "./include"). > The ultimate goal of the Linux kernel is not 100% strict complicance to > the CodingStyle document no matter what. The ultimate goal is to have a > kernel that is under control. By polluting git blame, you are taking on > aspect of the "under control" away. > Yes, the ultimate goal of CodingStyle is to have a kernel that is under control. For me, most of files in "./include" are common, simple, and shared files, which are not quite related with code analyzing (e.g. git log -p, or git blame), but they are read by others in most times. Is it correct? > Common sense needs to be used; horribly terrible coding style needs to be > fixed, sure. Is 82-characters long line horribly terrible coding style? > No, it's not. > For me, what you said above have effect on body files (in kernel, at least, more than 95% source files are body files, I guess). But in "./include", most of files are the interface inside and outside of our kernel, we need take more care about their coding styles. I often use vertical split window in vim in full screen mode to reading source code, when I read c source files, I often split window vertically for the related header files as reference. Thanks. -- Chen Gang (陈刚) Managing Natural Environments is the Duty of Human Beings.
Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles
On 2/28/16 00:53, Theodore Ts'o wrote: > On Sat, Feb 27, 2016 at 10:32:04PM +0800, Chen Gang wrote: >> I don't think so. Of cause NOT the "CODE CHURN". It is not correct to >> make an early decision during discussing. > > There is no discussion. If the maintainer has NAK'ed it. That's the > end of the dicsussion. Period. See: > For me, NAK also needs reasons. And this issue is about "coding styles issue", I am not quite sure whether trivial patch maintainer and mm maintainer are also the maintainer for "coding styles issues". I guess they are related with this patch, and their NAKs' reason are: mm and trivial don't care about this coding style issue, is it correct? > ftp://ftp.kernel.org/pub/linux/kernel/people/rusty/trivial/template-index.html > > Also note the comment from the above: > >NOTE: This means I'll only take whitespace/indentation fixes from the >author or maintainer. OK, thanks, it is really one proof for us. :-) I guess, the file above almost means: except whitespace/indentation, trivial patches don't consider about the coding styles issues. But can we say coding styles issues are not issues in our kernel? (I guess not) If we can not say, I guess one of your suggestion is useful (maybe be as your suggestion): find one suitable member (I guess I am not), run "checkpatch -file" under "./include", and fix all reported issues. Thanks. -- Chen Gang (陈刚) Managing Natural Environments is the Duty of Human Beings.
Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles
On Sat, 27 Feb 2016, Chen Gang wrote: > > Mel, as an MM developer, has already NACK'ed the patch, which means > > you should not send the patch to **any** upstream maintainer for > > inclusion. > > I don't think I "should not ...". I only care about correctness and > contribution, I don't care about any members ideas and their thinking. > When we have different ideas or thinking, we need discuss. If by "discuss" you mean "30+ email thread about where to put a line break", please drop me from CC next time this discussion is going to happen. Thanks. > For common shared header files, for me, we should really take more care > about the coding styles. > > - If the common shared header files don't care about the coding styles, >I guess any body files will have much more excuses for "do not care >about coding styles". > > - That means our kernel whole source files need not care about coding >styles at all!! > > - It is really really VERY BAD!! > > If someone only dislike me to send the related patches, I suggest: Let > another member(s) "run checkpatch -file" on the whole "./include" sub- > directory, and fix all coding styles issues. Which is exactly what you shouldn't do. The ultimate goal of the Linux kernel is not 100% strict complicance to the CodingStyle document no matter what. The ultimate goal is to have a kernel that is under control. By polluting git blame, you are taking on aspect of the "under control" away. Common sense needs to be used; horribly terrible coding style needs to be fixed, sure. Is 82-characters long line horribly terrible coding style? No, it's not. Thanks, -- Jiri Kosina SUSE Labs
Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles
On Sat, Feb 27, 2016 at 10:32:04PM +0800, Chen Gang wrote: > I don't think so. Of cause NOT the "CODE CHURN". It is not correct to > make an early decision during discussing. There is no discussion. If the maintainer has NAK'ed it. That's the end of the dicsussion. Period. See: ftp://ftp.kernel.org/pub/linux/kernel/people/rusty/trivial/template-index.html Also note the comment from the above: NOTE: This means I'll only take whitespace/indentation fixes from the author or maintainer. - Ted
Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles
On 2/27/16 10:45, Theodore Ts'o wrote: > On Fri, Feb 26, 2016 at 11:26:02PM +0800, Chen Gang wrote: >>> As for coding style, actually IMHO this patch is even _not_ a coding >>> style, more like a code shuffle, indeed. >>> >> >> "80 column limitation" is about coding style, I guess, all of us agree >> with it. > > No, it's been accepted that checkpatch requiring people to reformat > code to within be 80 columns limitation was actively harmful, and it > no longer does that. > > Worse, it now complains when you split a printf string across lines, > so there were patches that split a string across multiple lines to > make checkpatch shut up. And now there are patches that join the > string back together. > > And if you now start submitting patches to split them up again because > you think the 80 column restriction is so darned important, that would > be even ***more*** code churn. > I don't think so. Of cause NOT the "CODE CHURN". It is not correct to make an early decision during discussing. "80 column limitation" is mentioned in "Documentation/CodingStyle", if we have very good reason for it, we can break this limitation (for me, what you said above are really some of good reasons). But in our case (the patch), can anybody find any "good reasons" for it? at least, at present, I can not find: - It is a common shared base header file, it is almost not used for code analyzing (e.g. git diff, git blame). - Is it helpful for "grep xxx filename | grep yyy"? Please check the patch, I can not find (maybe __GFP_MOVABL definition be? but it is still not obvious, if some member stick to, we can keep it no touch). - Could anyone find any good reasons for it within this patch? > Which is one of the reasons why some of us aren't terribly happy with > people who start running checkpatch -file on other people's code and > start submitting patches, either through the trivial patch portal or > not. > For me, as a individual developer, I don't like this way, either. So of cause, I don't care about this way. I am just reading the common shared header files about mm. At least, I can understand some common sense of mm, and also read through the whole other headers to know what they are. When I find something valuable more or less, I shall send related patch for it. > Mel, as an MM developer, has already NACK'ed the patch, which means > you should not send the patch to **any** upstream maintainer for > inclusion. I don't think I "should not ...". I only care about correctness and contribution, I don't care about any members ideas and their thinking. When we have different ideas or thinking, we need discuss. For common shared header files, for me, we should really take more care about the coding styles. - If the common shared header files don't care about the coding styles, I guess any body files will have much more excuses for "do not care about coding styles". - That means our kernel whole source files need not care about coding styles at all!! - It is really really VERY BAD!! If someone only dislike me to send the related patches, I suggest: Let another member(s) "run checkpatch -file" on the whole "./include" sub- directory, and fix all coding styles issues. Thanks. -- Chen Gang (陈刚) Managing Natural Environments is the Duty of Human Beings.
Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles
On Fri, Feb 26, 2016 at 11:26:02PM +0800, Chen Gang wrote: > > As for coding style, actually IMHO this patch is even _not_ a coding > > style, more like a code shuffle, indeed. > > > > "80 column limitation" is about coding style, I guess, all of us agree > with it. No, it's been accepted that checkpatch requiring people to reformat code to within be 80 columns limitation was actively harmful, and it no longer does that. Worse, it now complains when you split a printf string across lines, so there were patches that split a string across multiple lines to make checkpatch shut up. And now there are patches that join the string back together. And if you now start submitting patches to split them up again because you think the 80 column restriction is so darned important, that would be even ***more*** code churn. Which is one of the reasons why some of us aren't terribly happy with people who start running checkpatch -file on other people's code and start submitting patches, either through the trivial patch portal or not. Mel, as an MM developer, has already NACK'ed the patch, which means you should not send the patch to **any** upstream maintainer for inclusion. - Ted
Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles
On 2/26/16 10:32, Jianyu Zhan wrote: > On Fri, Feb 26, 2016 at 6:29 AM, Chen Gang wrote: >> git is a tool mainly for analyzing code, but not mainly for normal >> reading main code. >> >> So for me, the coding styles need not consider about git. > > For you, maybe yes. > > But for most of the developers/learners, git blame does help a lot. > Kernel code was not as complicated as it is now, it is keeping evolving. > Yes. > So basically a history chain is indispensable in studying such a complex > system. > git blame fits in this role. I benefited a lot from using it when I > started to learn the code, > And, a pure coding style fix is sometimes really troublesome as I > have to use git blame > to go another step up along the history chain, which is time > consuming and boring. > > But after all, I bet you will be fond of using it if you dive deeper > into the kernel code studying. > And if you do, you will know why so many developers in this thread > are so upset and allergic > to such coding-style fix. > For me, for discussion, I don't care about "so many developers", I only focus on the proof and the contribution. > As for coding style, actually IMHO this patch is even _not_ a coding > style, more like a code shuffle, indeed. > "80 column limitation" is about coding style, I guess, all of us agree with it. > And for your commit history, I found actually you have already > contributed some quit good patches. For me, I don't care about my history -- except some members find issues related with my original patches, I have duty to analyze the related issues together with the finders. > I don't think it is helpful for a non-layman contributor to keep > generating such code churn. > For me, we are discussing, so it is not quite suitable to make an early conclusion (code churn). For me, I don't care about layman or non-layman, I only focus on the proof and the contribution. Thanks. -- Chen Gang (陈刚) Managing Natural Environments is the Duty of Human Beings.
Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles
On 2/26/16 07:12, SeongJae Park wrote: > > On Fri, 26 Feb 2016, Chen Gang wrote: > >> >> git is a tool mainly for analyzing code, but not mainly for normal >> reading main code. >> >> So for me, the coding styles need not consider about git. > > > It is common to see reject of trivial coding style fixup patch here and > there. Those patches usually be merged for early stage files that only > few people read / write. However, for files that are old and lots of > people read and write, those patches are rejected in usual. I mean, the > negative opinions for this patches are usual in this community. > > I agree that coding style is important and respect your effort. However, > because the code will be seen and written by most kernel hackers, the file > should be maintained to be easily readable and writable by most kernel > hackers, especially, maintainers. What I want to say is, we should > respect maintainers' opinion in usual. > Yes we need consider about the maintainers' options. And my another ideas are replied in the other thread, please check, and welcome any ideas, suggestion, and completions. Thanks. -- Chen Gang (陈刚) Managing Natural Environments is the Duty of Human Beings.
Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles
On 2/26/16 06:39, Jiri Kosina wrote: > On Fri, 26 Feb 2016, Chen Gang wrote: > >> git is a tool mainly for analyzing code, but not mainly for normal >> reading main code. >> >> So for me, the coding styles need not consider about git. > > You are mistaken here. It's very helpful when debugging; For me, 'debugging' is related with debugger (e.g. kdb or kgdb), and 'tracing' is related with dumping log, and code analyzing is related with "git diff" and "git blame". And yes, for me, "git diff" and "git blame" is really very helpful for code analyzing. > usually you want > to find the commit that introduced particular change, and read its > changelog (at least). Having to cross rather pointless changes just adds > time (need to restart git-blame with commit~1 as a base) for no really > good reason. > That is the reason why I am not quite care about body files, I often use "git log -p filename", the cleanup code patch has negative effect with code analyzing (although for me, it should still need to be cleanup). But in our case, it is for the shared header file: - They are often the common base file, the main contents will not be changed quite often, and their contents are usually simple enough ( e.g. gfp.h in our case), they are not often for "code analyzing". - But they are quite often read in normal reading ways by programmers (e.g. open with normal editors). For normal reading, programmers usually care about the contents, not the changes. - So for me, the common shared header files need always take care about coding styles, and need not consider about code analyzing. And if we reject this kind of patch (in our case), I guess, that almost mean: "for the common shared header files, their bad coding styles will be remain for ever". Thanks. -- Chen Gang (陈刚) Managing Natural Environments is the Duty of Human Beings.
Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles
On Fri, Feb 26, 2016 at 6:29 AM, Chen Gang wrote: > git is a tool mainly for analyzing code, but not mainly for normal > reading main code. > > So for me, the coding styles need not consider about git. For you, maybe yes. But for most of the developers/learners, git blame does help a lot. Kernel code was not as complicated as it is now, it is keeping evolving. So basically a history chain is indispensable in studying such a complex system. git blame fits in this role. I benefited a lot from using it when I started to learn the code, And, a pure coding style fix is sometimes really troublesome as I have to use git blame to go another step up along the history chain, which is time consuming and boring. But after all, I bet you will be fond of using it if you dive deeper into the kernel code studying. And if you do, you will know why so many developers in this thread are so upset and allergic to such coding-style fix. As for coding style, actually IMHO this patch is even _not_ a coding style, more like a code shuffle, indeed. And for your commit history, I found actually you have already contributed some quit good patches. I don't think it is helpful for a non-layman contributor to keep generating such code churn. Thanks, Jianyu Zhan
Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles
On Fri, 26 Feb 2016, Chen Gang wrote: On 2/26/16 00:07, Mel Gorman wrote: On Thu, Feb 25, 2016 at 06:26:31AM +0800, cheng...@emindsoft.com.cn wrote: I do not want this patch to go through the trivial tree. It still adds another step to identifying relevant commits through git blame and has limited, if any, benefit to maintainability. "it's preferable to preserve blame than go through a layer of cleanup when looking for the commit that defined particular flags". git blame identifies what commit last altered a line. If a cleanup patch is encountered then the tree before that commit needs to be examined which adds time. It's rare that cleanup patches on their own are useful and this is one of those cases. git is a tool mainly for analyzing code, but not mainly for normal reading main code. So for me, the coding styles need not consider about git. It is common to see reject of trivial coding style fixup patch here and there. Those patches usually be merged for early stage files that only few people read / write. However, for files that are old and lots of people read and write, those patches are rejected in usual. I mean, the negative opinions for this patches are usual in this community. I agree that coding style is important and respect your effort. However, because the code will be seen and written by most kernel hackers, the file should be maintained to be easily readable and writable by most kernel hackers, especially, maintainers. What I want to say is, we should respect maintainers' opinion in usual. As far as I remember, I have seen a document that saying same with others' opinion but couldn't find it. Thanks, SeongJae Park Thanks. -- Chen Gang (陈刚) Managing Natural Environments is the Duty of Human Beings. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majord...@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: mailto:"d...@kvack.org";> em...@kvack.org
Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles
On Fri, 26 Feb 2016, Chen Gang wrote: > > git blame identifies what commit last altered a line. If a cleanup patch > > is encountered then the tree before that commit needs to be examined > > which adds time. It's rare that cleanup patches on their own are useful > > and this is one of those cases. > > git is a tool mainly for analyzing code, but not mainly for normal > reading main code. > > So for me, the coding styles need not consider about git. You are mistaken here. It's very helpful when debugging; usually you want to find the commit that introduced particular change, and read its changelog (at least). Having to cross rather pointless changes just adds time (need to restart git-blame with commit~1 as a base) for no really good reason. Thanks, -- Jiri Kosina SUSE Labs
Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles
On 2/26/16 00:07, Mel Gorman wrote: >>> On Thu, Feb 25, 2016 at 06:26:31AM +0800, cheng...@emindsoft.com.cn wrote: > > I do not want this patch to go through the trivial tree. It still adds > another step to identifying relevant commits through git blame and has > limited, if any, benefit to maintainability. > >> "it's preferable to preserve blame than go through a layer of cleanup >> when looking for the commit that defined particular flags". >> > > git blame identifies what commit last altered a line. If a cleanup patch > is encountered then the tree before that commit needs to be examined > which adds time. It's rare that cleanup patches on their own are useful > and this is one of those cases. > git is a tool mainly for analyzing code, but not mainly for normal reading main code. So for me, the coding styles need not consider about git. Thanks. -- Chen Gang (陈刚) Managing Natural Environments is the Duty of Human Beings.
Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles
On 2/25/16 23:12, Jiri Kosina wrote: > On Thu, 25 Feb 2016, Chen Gang wrote: > >> I can understand for your NAK, it is a trivial patch. > > Not all trivial patches are NAKed :) But they have to be generally useful. > > Shuffling code around, without actually changing / improving it a bit, > just for the sole purpose of formatting, is kind of pointless (especially > given the fact that the current code as-is is easily readable; it's not > like it'd be a horrible mess difficult to understand). > > Sure, it might had been formatted better at the time it was actually > merged. But changing it "just because" after being in tree for long time > doesn't fix any problem really. > OK, thanks. I have replied the related contents in the other thread. Welcome any ideas, suggestions, and completions in the other related thread. Thanks. -- Chen Gang (陈刚) Managing Natural Environments is the Duty of Human Beings.
Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles
On 2/25/16 22:47, Michal Hocko wrote: >> >> For the "comment placement" the common way is below, but still make git >> grep harder: > > if you did git grep ZONE_MOVABLE you would get less information > OK. >> >> -#define __GFP_MOVABLE ((__force gfp_t)___GFP_MOVABLE) /* >> ZONE_MOVABLE allowed */ >> +/* ZONE_MOVABLE allowed */ >> +#define __GFP_MOVABLE ((__force gfp_t)___GFP_MOVABLE) >> >> Then how about: >> >> -#define __GFP_MOVABLE ((__force gfp_t)___GFP_MOVABLE) /* >> ZONE_MOVABLE allowed */ >> +#define __GFP_MOVABLE \ >> ((__force gfp_t)___GFP_MOVABLE) /* ZONE_MOVABLE allowed */ >> >> or: >> >> -#define __GFP_MOVABLE ((__force gfp_t)___GFP_MOVABLE) /* >> ZONE_MOVABLE allowed */ >> +#define __GFP_MOVABLE /* ZONE_MOVABLE allowed */ \ >> ((__force gfp_t)___GFP_MOVABLE) > > Now looks worse then other, really. Please try to think what would be > a benefit of such change. As Mel already pointed out git blame would > take an additional step to get back to the patch which has introduced > them. And what is the advantage? Make 80 characters-per-line rule happy? > I just do not think this is worth changes at all. > For 80 column limitation: - I often use vsp (vertical split window) in vim to reading code in the 2 files, 80 columns limitation can avoid the line wrap, which will let code reading better. - Sometimes we need copy/past the code to a pdf files (e.g. print the interface header file contents to a new document as appendix), or print the code to a physical paper (e.g. write a book). For worth or worthless: The shared header files (e.g. in our case), have more chances to be read or printed than the normal source code files. So for me, we need take more care about the coding styles of them. For git-blame: - It really a good feature! Originally, I did not know about it :-). - Can it instead of sending trivial patch? (I guess not). Thanks. -- Chen Gang (陈刚) Managing Natural Environments is the Duty of Human Beings.
Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles
On Thu, Feb 25, 2016 at 10:38:58PM +0800, Chen Gang wrote: > On 2/25/16 17:27, Mel Gorman wrote: > > On Thu, Feb 25, 2016 at 06:26:31AM +0800, cheng...@emindsoft.com.cn wrote: > >> From: Chen Gang > >> > >> Always notice about 80 columns, and the white space near '|'. > >> > >> Let the wrapped function parameters align as the same styles. > >> > >> Remove redundant statement "enum zone_type z;" in function gfp_zone. > >> > >> Signed-off-by: Chen Gang > > > > NAK from me at least. From my perspective, it's preferrable to preserve > > blame than go through a layer of cleanup when looking for the commit > > that defined particular flags. It's ok to cleanup code at the same time > > definitions change for functional or performance reasons. > > > > I can understand for your NAK, it is a trivial patch. For me, I guess > triv...@kernel.org will care about this kind of patch. > I do not want this patch to go through the trivial tree. It still adds another step to identifying relevant commits through git blame and has limited, if any, benefit to maintainability. > "it's preferable to preserve blame than go through a layer of cleanup > when looking for the commit that defined particular flags". > git blame identifies what commit last altered a line. If a cleanup patch is encountered then the tree before that commit needs to be examined which adds time. It's rare that cleanup patches on their own are useful and this is one of those cases. -- Mel Gorman SUSE Labs
Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles
On Thu, 25 Feb 2016, Chen Gang wrote: > I can understand for your NAK, it is a trivial patch. Not all trivial patches are NAKed :) But they have to be generally useful. Shuffling code around, without actually changing / improving it a bit, just for the sole purpose of formatting, is kind of pointless (especially given the fact that the current code as-is is easily readable; it's not like it'd be a horrible mess difficult to understand). Sure, it might had been formatted better at the time it was actually merged. But changing it "just because" after being in tree for long time doesn't fix any problem really. > And excuse me, I guess my english is not quite well, I am not quite > understand the meaning below, could you provide more details? > > "it's preferable to preserve blame than go through a layer of cleanup > when looking for the commit that defined particular flags". git-blame. When looking at commits touching particular lines, you add an extra hop to the person who is trying to find a (functional) commit that touched a particular line. -- Jiri Kosina SUSE Labs
Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles
On Thu 25-02-16 22:23:38, Chen Gang wrote: > On 2/25/16 16:57, Michal Hocko wrote: > > On Thu 25-02-16 06:26:31, cheng...@emindsoft.com.cn wrote: > >> > >> Always notice about 80 columns, and the white space near '|'. > >> > >> Let the wrapped function parameters align as the same styles. > >> > >> Remove redundant statement "enum zone_type z;" in function gfp_zone. > > > > I do not think this is an improvement. The comment placement is just odd > > and artificially splitting the mask into more lines makes git grep > > harder to use. > > > > Excuse me, I am not quite sure your meaning is the whole contents of the > patch is worthless, or only for the "comment placement"? > > For the "comment placement" the common way is below, but still make git > grep harder: if you did git grep ZONE_MOVABLE you would get less information > > -#define __GFP_MOVABLE((__force gfp_t)___GFP_MOVABLE) /* > ZONE_MOVABLE allowed */ > +/* ZONE_MOVABLE allowed */ > +#define __GFP_MOVABLE((__force gfp_t)___GFP_MOVABLE) > > Then how about: > > -#define __GFP_MOVABLE((__force gfp_t)___GFP_MOVABLE) /* > ZONE_MOVABLE allowed */ > +#define __GFP_MOVABLE\ > ((__force gfp_t)___GFP_MOVABLE) /* ZONE_MOVABLE allowed */ > > or: > > -#define __GFP_MOVABLE((__force gfp_t)___GFP_MOVABLE) /* > ZONE_MOVABLE allowed */ > +#define __GFP_MOVABLE/* ZONE_MOVABLE allowed */ \ > ((__force gfp_t)___GFP_MOVABLE) Now looks worse then other, really. Please try to think what would be a benefit of such change. As Mel already pointed out git blame would take an additional step to get back to the patch which has introduced them. And what is the advantage? Make 80 characters-per-line rule happy? I just do not think this is worth changes at all. -- Michal Hocko SUSE Labs
Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles
On 2/25/16 17:27, Mel Gorman wrote: > On Thu, Feb 25, 2016 at 06:26:31AM +0800, cheng...@emindsoft.com.cn wrote: >> From: Chen Gang >> >> Always notice about 80 columns, and the white space near '|'. >> >> Let the wrapped function parameters align as the same styles. >> >> Remove redundant statement "enum zone_type z;" in function gfp_zone. >> >> Signed-off-by: Chen Gang > > NAK from me at least. From my perspective, it's preferrable to preserve > blame than go through a layer of cleanup when looking for the commit > that defined particular flags. It's ok to cleanup code at the same time > definitions change for functional or performance reasons. > I can understand for your NAK, it is a trivial patch. For me, I guess triv...@kernel.org will care about this kind of patch. If we have another better way than sending trivial patch, that will be OK to me. At present, I am learning mm in my free time, when I feel something is valuable more or less, I will send related patch for it. And excuse me, I guess my english is not quite well, I am not quite understand the meaning below, could you provide more details? "it's preferable to preserve blame than go through a layer of cleanup when looking for the commit that defined particular flags". Thanks. -- Chen Gang (陈刚) Managing Natural Environments is the Duty of Human Beings.
Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles
On 2/25/16 16:57, Michal Hocko wrote: > On Thu 25-02-16 06:26:31, cheng...@emindsoft.com.cn wrote: >> >> Always notice about 80 columns, and the white space near '|'. >> >> Let the wrapped function parameters align as the same styles. >> >> Remove redundant statement "enum zone_type z;" in function gfp_zone. > > I do not think this is an improvement. The comment placement is just odd > and artificially splitting the mask into more lines makes git grep > harder to use. > Excuse me, I am not quite sure your meaning is the whole contents of the patch is worthless, or only for the "comment placement"? For the "comment placement" the common way is below, but still make git grep harder: -#define __GFP_MOVABLE ((__force gfp_t)___GFP_MOVABLE) /* ZONE_MOVABLE allowed */ +/* ZONE_MOVABLE allowed */ +#define __GFP_MOVABLE ((__force gfp_t)___GFP_MOVABLE) Then how about: -#define __GFP_MOVABLE ((__force gfp_t)___GFP_MOVABLE) /* ZONE_MOVABLE allowed */ +#define __GFP_MOVABLE \ ((__force gfp_t)___GFP_MOVABLE) /* ZONE_MOVABLE allowed */ or: -#define __GFP_MOVABLE ((__force gfp_t)___GFP_MOVABLE) /* ZONE_MOVABLE allowed */ +#define __GFP_MOVABLE /* ZONE_MOVABLE allowed */ \ ((__force gfp_t)___GFP_MOVABLE) Thanks. -- Chen Gang (陈刚) Managing Natural Environments is the Duty of Human Beings.
Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles
On 2/25/16 09:01, SeongJae Park wrote: > > Well, the indentation for the comment and the '\' looks odd to me. If > the 80 column limit is necessary, how about moving the comment to above > line of the macro as below? Because comments are usually placed before > the target they are explaining, I believe this may better to read. > > -#define __GFP_MOVABLE((__force gfp_t)___GFP_MOVABLE) /* > ZONE_MOVABLE allowed */ > +/* ZONE_MOVABLE allowed */ > +#define __GFP_MOVABLE((__force gfp_t)___GFP_MOVABLE) > > Maybe the opinion can be applied to below similar changes, too. > At least for me, what you said above is OK (it is a common way). And welcome other members' suggestions. Thanks. -- Chen Gang (陈刚) Managing Natural Environments is the Duty of Human Beings.
Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles
On Thu, Feb 25, 2016 at 06:26:31AM +0800, cheng...@emindsoft.com.cn wrote: > From: Chen Gang > > Always notice about 80 columns, and the white space near '|'. > > Let the wrapped function parameters align as the same styles. > > Remove redundant statement "enum zone_type z;" in function gfp_zone. > > Signed-off-by: Chen Gang NAK from me at least. From my perspective, it's preferrable to preserve blame than go through a layer of cleanup when looking for the commit that defined particular flags. It's ok to cleanup code at the same time definitions change for functional or performance reasons. -- Mel Gorman SUSE Labs
Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles
On Thu 25-02-16 06:26:31, cheng...@emindsoft.com.cn wrote: > From: Chen Gang > > Always notice about 80 columns, and the white space near '|'. > > Let the wrapped function parameters align as the same styles. > > Remove redundant statement "enum zone_type z;" in function gfp_zone. I do not think this is an improvement. The comment placement is just odd and artificially splitting the mask into more lines makes git grep harder to use. > Signed-off-by: Chen Gang > --- > include/linux/gfp.h | 35 --- > 1 file changed, 20 insertions(+), 15 deletions(-) > > diff --git a/include/linux/gfp.h b/include/linux/gfp.h > index 36e0c5e..cf904ef 100644 > --- a/include/linux/gfp.h > +++ b/include/linux/gfp.h > @@ -53,8 +53,10 @@ struct vm_area_struct; > #define __GFP_DMA((__force gfp_t)___GFP_DMA) > #define __GFP_HIGHMEM((__force gfp_t)___GFP_HIGHMEM) > #define __GFP_DMA32 ((__force gfp_t)___GFP_DMA32) > -#define __GFP_MOVABLE((__force gfp_t)___GFP_MOVABLE) /* > ZONE_MOVABLE allowed */ > -#define GFP_ZONEMASK (__GFP_DMA|__GFP_HIGHMEM|__GFP_DMA32|__GFP_MOVABLE) > +#define __GFP_MOVABLE((__force gfp_t)___GFP_MOVABLE) \ > + /* ZONE_MOVABLE allowed */ > +#define GFP_ZONEMASK (__GFP_DMA | __GFP_HIGHMEM | __GFP_DMA32 | \ > + __GFP_MOVABLE) > > /* > * Page mobility and placement hints > @@ -151,9 +153,12 @@ struct vm_area_struct; > */ > #define __GFP_IO ((__force gfp_t)___GFP_IO) > #define __GFP_FS ((__force gfp_t)___GFP_FS) > -#define __GFP_DIRECT_RECLAIM ((__force gfp_t)___GFP_DIRECT_RECLAIM) /* > Caller can reclaim */ > -#define __GFP_KSWAPD_RECLAIM ((__force gfp_t)___GFP_KSWAPD_RECLAIM) /* > kswapd can wake */ > -#define __GFP_RECLAIM ((__force > gfp_t)(___GFP_DIRECT_RECLAIM|___GFP_KSWAPD_RECLAIM)) > +#define __GFP_DIRECT_RECLAIM ((__force gfp_t)___GFP_DIRECT_RECLAIM) \ > + /* Caller can reclaim */ > +#define __GFP_KSWAPD_RECLAIM ((__force gfp_t)___GFP_KSWAPD_RECLAIM) \ > + /* kswapd can wake */ > +#define __GFP_RECLAIM((__force gfp_t)(___GFP_DIRECT_RECLAIM | \ > + ___GFP_KSWAPD_RECLAIM)) > #define __GFP_REPEAT ((__force gfp_t)___GFP_REPEAT) > #define __GFP_NOFAIL ((__force gfp_t)___GFP_NOFAIL) > #define __GFP_NORETRY((__force gfp_t)___GFP_NORETRY) > @@ -262,7 +267,7 @@ struct vm_area_struct; >~__GFP_KSWAPD_RECLAIM) > > /* Convert GFP flags to their corresponding migrate type */ > -#define GFP_MOVABLE_MASK (__GFP_RECLAIMABLE|__GFP_MOVABLE) > +#define GFP_MOVABLE_MASK (__GFP_RECLAIMABLE | __GFP_MOVABLE) > #define GFP_MOVABLE_SHIFT 3 > > static inline int gfpflags_to_migratetype(const gfp_t gfp_flags) > @@ -377,11 +382,10 @@ static inline bool gfpflags_allow_blocking(const gfp_t > gfp_flags) > > static inline enum zone_type gfp_zone(gfp_t flags) > { > - enum zone_type z; > int bit = (__force int) (flags & GFP_ZONEMASK); > + enum zone_type z = (GFP_ZONE_TABLE >> (bit * GFP_ZONES_SHIFT)) & > + ((1 << GFP_ZONES_SHIFT) - 1); > > - z = (GFP_ZONE_TABLE >> (bit * GFP_ZONES_SHIFT)) & > - ((1 << GFP_ZONES_SHIFT) - 1); > VM_BUG_ON((GFP_ZONE_BAD >> bit) & 1); > return z; > } > @@ -428,8 +432,7 @@ __alloc_pages_nodemask(gfp_t gfp_mask, unsigned int order, > struct zonelist *zonelist, nodemask_t *nodemask); > > static inline struct page * > -__alloc_pages(gfp_t gfp_mask, unsigned int order, > - struct zonelist *zonelist) > +__alloc_pages(gfp_t gfp_mask, unsigned int order, struct zonelist *zonelist) > { > return __alloc_pages_nodemask(gfp_mask, order, zonelist, NULL); > } > @@ -453,7 +456,7 @@ __alloc_pages_node(int nid, gfp_t gfp_mask, unsigned int > order) > * online. > */ > static inline struct page *alloc_pages_node(int nid, gfp_t gfp_mask, > - unsigned int order) > + unsigned int order) > { > if (nid == NUMA_NO_NODE) > nid = numa_mem_id(); > @@ -470,8 +473,9 @@ alloc_pages(gfp_t gfp_mask, unsigned int order) > return alloc_pages_current(gfp_mask, order); > } > extern struct page *alloc_pages_vma(gfp_t gfp_mask, int order, > - struct vm_area_struct *vma, unsigned long addr, > - int node, bool hugepage); > + struct vm_area_struct *vma, > + unsigned long addr, int node, > + bool hugepage); > #define alloc_hugepage_vma(gfp_mask, vma, addr, order) \ > alloc_pages_vma(gfp_mask, order, vma, addr, numa_node_id(), true) > #else > @@ -552,7 +556,8 @@ static inline bool pm_suspended_storage(void) > } > #endif /*
Re: [PATCH trivial] include/linux/gfp.h: Improve the coding styles
Hello Chen, On Thu, 25 Feb 2016, cheng...@emindsoft.com.cn wrote: From: Chen Gang Always notice about 80 columns, and the white space near '|'. Let the wrapped function parameters align as the same styles. Remove redundant statement "enum zone_type z;" in function gfp_zone. Signed-off-by: Chen Gang --- include/linux/gfp.h | 35 --- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/include/linux/gfp.h b/include/linux/gfp.h index 36e0c5e..cf904ef 100644 --- a/include/linux/gfp.h +++ b/include/linux/gfp.h @@ -53,8 +53,10 @@ struct vm_area_struct; #define __GFP_DMA ((__force gfp_t)___GFP_DMA) #define __GFP_HIGHMEM ((__force gfp_t)___GFP_HIGHMEM) #define __GFP_DMA32 ((__force gfp_t)___GFP_DMA32) -#define __GFP_MOVABLE ((__force gfp_t)___GFP_MOVABLE) /* ZONE_MOVABLE allowed */ -#define GFP_ZONEMASK (__GFP_DMA|__GFP_HIGHMEM|__GFP_DMA32|__GFP_MOVABLE) +#define __GFP_MOVABLE ((__force gfp_t)___GFP_MOVABLE) \ + /* ZONE_MOVABLE allowed */ Well, the indentation for the comment and the '\' looks odd to me. If the 80 column limit is necessary, how about moving the comment to above line of the macro as below? Because comments are usually placed before the target they are explaining, I believe this may better to read. -#define __GFP_MOVABLE((__force gfp_t)___GFP_MOVABLE) /* ZONE_MOVABLE allowed */ +/* ZONE_MOVABLE allowed */ +#define __GFP_MOVABLE((__force gfp_t)___GFP_MOVABLE) Maybe the opinion can be applied to below similar changes, too. Thanks, SeongJae Park. +#define GFP_ZONEMASK (__GFP_DMA | __GFP_HIGHMEM | __GFP_DMA32 | \ +__GFP_MOVABLE) /* * Page mobility and placement hints @@ -151,9 +153,12 @@ struct vm_area_struct; */ #define __GFP_IO((__force gfp_t)___GFP_IO) #define __GFP_FS((__force gfp_t)___GFP_FS) -#define __GFP_DIRECT_RECLAIM ((__force gfp_t)___GFP_DIRECT_RECLAIM) /* Caller can reclaim */ -#define __GFP_KSWAPD_RECLAIM ((__force gfp_t)___GFP_KSWAPD_RECLAIM) /* kswapd can wake */ -#define __GFP_RECLAIM ((__force gfp_t)(___GFP_DIRECT_RECLAIM|___GFP_KSWAPD_RECLAIM)) +#define __GFP_DIRECT_RECLAIM ((__force gfp_t)___GFP_DIRECT_RECLAIM) \ + /* Caller can reclaim */ +#define __GFP_KSWAPD_RECLAIM ((__force gfp_t)___GFP_KSWAPD_RECLAIM) \ + /* kswapd can wake */ +#define __GFP_RECLAIM ((__force gfp_t)(___GFP_DIRECT_RECLAIM | \ +___GFP_KSWAPD_RECLAIM)) #define __GFP_REPEAT((__force gfp_t)___GFP_REPEAT) #define __GFP_NOFAIL((__force gfp_t)___GFP_NOFAIL) #define __GFP_NORETRY ((__force gfp_t)___GFP_NORETRY) @@ -262,7 +267,7 @@ struct vm_area_struct; ~__GFP_KSWAPD_RECLAIM) /* Convert GFP flags to their corresponding migrate type */ -#define GFP_MOVABLE_MASK (__GFP_RECLAIMABLE|__GFP_MOVABLE) +#define GFP_MOVABLE_MASK (__GFP_RECLAIMABLE | __GFP_MOVABLE) #define GFP_MOVABLE_SHIFT 3 static inline int gfpflags_to_migratetype(const gfp_t gfp_flags) @@ -377,11 +382,10 @@ static inline bool gfpflags_allow_blocking(const gfp_t gfp_flags) static inline enum zone_type gfp_zone(gfp_t flags) { - enum zone_type z; int bit = (__force int) (flags & GFP_ZONEMASK); + enum zone_type z = (GFP_ZONE_TABLE >> (bit * GFP_ZONES_SHIFT)) & + ((1 << GFP_ZONES_SHIFT) - 1); - z = (GFP_ZONE_TABLE >> (bit * GFP_ZONES_SHIFT)) & -((1 << GFP_ZONES_SHIFT) - 1); VM_BUG_ON((GFP_ZONE_BAD >> bit) & 1); return z; } @@ -428,8 +432,7 @@ __alloc_pages_nodemask(gfp_t gfp_mask, unsigned int order, struct zonelist *zonelist, nodemask_t *nodemask); static inline struct page * -__alloc_pages(gfp_t gfp_mask, unsigned int order, - struct zonelist *zonelist) +__alloc_pages(gfp_t gfp_mask, unsigned int order, struct zonelist *zonelist) { return __alloc_pages_nodemask(gfp_mask, order, zonelist, NULL); } @@ -453,7 +456,7 @@ __alloc_pages_node(int nid, gfp_t gfp_mask, unsigned int order) * online. */ static inline struct page *alloc_pages_node(int nid, gfp_t gfp_mask, - unsigned int order) + unsigned int order) { if (nid == NUMA_NO_NODE) nid = numa_mem_id(); @@ -470,8 +473,9 @@ alloc_pages(gfp_t gfp_mask, unsigned int order) return alloc_pages_current(gfp_mask, order); } extern struct page *alloc_pages_vma(gfp_t gfp_mask, int order, - struct vm_area_struct *vma, unsigned long addr, - int node, bool hugepage); + struct vm_area_struct *vma, + unsigned long addr, int node, + bool hugepage); #de
[PATCH trivial] include/linux/gfp.h: Improve the coding styles
From: Chen Gang Always notice about 80 columns, and the white space near '|'. Let the wrapped function parameters align as the same styles. Remove redundant statement "enum zone_type z;" in function gfp_zone. Signed-off-by: Chen Gang --- include/linux/gfp.h | 35 --- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/include/linux/gfp.h b/include/linux/gfp.h index 36e0c5e..cf904ef 100644 --- a/include/linux/gfp.h +++ b/include/linux/gfp.h @@ -53,8 +53,10 @@ struct vm_area_struct; #define __GFP_DMA ((__force gfp_t)___GFP_DMA) #define __GFP_HIGHMEM ((__force gfp_t)___GFP_HIGHMEM) #define __GFP_DMA32((__force gfp_t)___GFP_DMA32) -#define __GFP_MOVABLE ((__force gfp_t)___GFP_MOVABLE) /* ZONE_MOVABLE allowed */ -#define GFP_ZONEMASK (__GFP_DMA|__GFP_HIGHMEM|__GFP_DMA32|__GFP_MOVABLE) +#define __GFP_MOVABLE ((__force gfp_t)___GFP_MOVABLE) \ + /* ZONE_MOVABLE allowed */ +#define GFP_ZONEMASK (__GFP_DMA | __GFP_HIGHMEM | __GFP_DMA32 | \ +__GFP_MOVABLE) /* * Page mobility and placement hints @@ -151,9 +153,12 @@ struct vm_area_struct; */ #define __GFP_IO ((__force gfp_t)___GFP_IO) #define __GFP_FS ((__force gfp_t)___GFP_FS) -#define __GFP_DIRECT_RECLAIM ((__force gfp_t)___GFP_DIRECT_RECLAIM) /* Caller can reclaim */ -#define __GFP_KSWAPD_RECLAIM ((__force gfp_t)___GFP_KSWAPD_RECLAIM) /* kswapd can wake */ -#define __GFP_RECLAIM ((__force gfp_t)(___GFP_DIRECT_RECLAIM|___GFP_KSWAPD_RECLAIM)) +#define __GFP_DIRECT_RECLAIM ((__force gfp_t)___GFP_DIRECT_RECLAIM) \ + /* Caller can reclaim */ +#define __GFP_KSWAPD_RECLAIM ((__force gfp_t)___GFP_KSWAPD_RECLAIM) \ + /* kswapd can wake */ +#define __GFP_RECLAIM ((__force gfp_t)(___GFP_DIRECT_RECLAIM | \ +___GFP_KSWAPD_RECLAIM)) #define __GFP_REPEAT ((__force gfp_t)___GFP_REPEAT) #define __GFP_NOFAIL ((__force gfp_t)___GFP_NOFAIL) #define __GFP_NORETRY ((__force gfp_t)___GFP_NORETRY) @@ -262,7 +267,7 @@ struct vm_area_struct; ~__GFP_KSWAPD_RECLAIM) /* Convert GFP flags to their corresponding migrate type */ -#define GFP_MOVABLE_MASK (__GFP_RECLAIMABLE|__GFP_MOVABLE) +#define GFP_MOVABLE_MASK (__GFP_RECLAIMABLE | __GFP_MOVABLE) #define GFP_MOVABLE_SHIFT 3 static inline int gfpflags_to_migratetype(const gfp_t gfp_flags) @@ -377,11 +382,10 @@ static inline bool gfpflags_allow_blocking(const gfp_t gfp_flags) static inline enum zone_type gfp_zone(gfp_t flags) { - enum zone_type z; int bit = (__force int) (flags & GFP_ZONEMASK); + enum zone_type z = (GFP_ZONE_TABLE >> (bit * GFP_ZONES_SHIFT)) & + ((1 << GFP_ZONES_SHIFT) - 1); - z = (GFP_ZONE_TABLE >> (bit * GFP_ZONES_SHIFT)) & -((1 << GFP_ZONES_SHIFT) - 1); VM_BUG_ON((GFP_ZONE_BAD >> bit) & 1); return z; } @@ -428,8 +432,7 @@ __alloc_pages_nodemask(gfp_t gfp_mask, unsigned int order, struct zonelist *zonelist, nodemask_t *nodemask); static inline struct page * -__alloc_pages(gfp_t gfp_mask, unsigned int order, - struct zonelist *zonelist) +__alloc_pages(gfp_t gfp_mask, unsigned int order, struct zonelist *zonelist) { return __alloc_pages_nodemask(gfp_mask, order, zonelist, NULL); } @@ -453,7 +456,7 @@ __alloc_pages_node(int nid, gfp_t gfp_mask, unsigned int order) * online. */ static inline struct page *alloc_pages_node(int nid, gfp_t gfp_mask, - unsigned int order) + unsigned int order) { if (nid == NUMA_NO_NODE) nid = numa_mem_id(); @@ -470,8 +473,9 @@ alloc_pages(gfp_t gfp_mask, unsigned int order) return alloc_pages_current(gfp_mask, order); } extern struct page *alloc_pages_vma(gfp_t gfp_mask, int order, - struct vm_area_struct *vma, unsigned long addr, - int node, bool hugepage); + struct vm_area_struct *vma, + unsigned long addr, int node, + bool hugepage); #define alloc_hugepage_vma(gfp_mask, vma, addr, order) \ alloc_pages_vma(gfp_mask, order, vma, addr, numa_node_id(), true) #else @@ -552,7 +556,8 @@ static inline bool pm_suspended_storage(void) } #endif /* CONFIG_PM_SLEEP */ -#if (defined(CONFIG_MEMORY_ISOLATION) && defined(CONFIG_COMPACTION)) || defined(CONFIG_CMA) +#if (defined(CONFIG_MEMORY_ISOLATION) && defined(CONFIG_COMPACTION)) || \ + defined(CONFIG_CMA) /* The below functions must be run on a range from a single zone. */ extern int alloc_contig_range(unsigned long start, unsigned long end, uns