From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1425726AbdDZPbO (ORCPT ); Wed, 26 Apr 2017 11:31:14 -0400 Received: from lhrrgout.huawei.com ([194.213.3.17]:25339 "EHLO lhrrgout.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1956536AbdDZPaQ (ORCPT ); Wed, 26 Apr 2017 11:30:16 -0400 Subject: Re: [PATCH 1/1] Remove hardcoding of ___GFP_xxx bitmasks To: Michal Hocko References: <20170426133549.22603-1-igor.stoppa@huawei.com> <20170426133549.22603-2-igor.stoppa@huawei.com> <20170426144750.GH12504@dhcp22.suse.cz> CC: , , From: Igor Stoppa Message-ID: Date: Wed, 26 Apr 2017 18:29:08 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <20170426144750.GH12504@dhcp22.suse.cz> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.122.225.51] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A020201.5900BCFD.0333,ss=1,re=0.000,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0, ip=0.0.0.0, so=2013-06-18 04:22:30, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: 83ee8bf6e889adb052aeaa7585bbeedb Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 26/04/17 17:47, Michal Hocko wrote: > On Wed 26-04-17 16:35:49, Igor Stoppa wrote: >> The bitmasks used for ___GFP_xxx can be defined in terms of an enum, >> which doesn't require manual updates to its values. > > GFP masks are rarely updated so why is this worth doing? I have plans for that [1] - yeah, I should have not written only to ml - but I thought there was sufficient value in this patch to be sent alone. I got into this part of the code because (if I understood correctly) there are no spare bits available from the 32bits mask that is currently in use. Adding a new zone, therefore, would cause the bumping to a 64bits type. If the zone is not strictly needed, some people might prefer to have the option to stick to 32 bits. Which in turn would mean more #ifdefs. Using the enum should provide the same flexibility with a more limited number of #ifdefs in the code. But I really thought that there is a value in the change per-se. Regardless of what other patches might follow. >> As bonus, __GFP_BITS_SHIFT is automatically kept consistent. > > this alone doesn't sound like a huge win to me, to be honest. We already > have ___GFP_$FOO and __GFP_FOO you are adding __GFP_$FOO_SHIFT. This is > too much IMHO. I do not like the #defines being floating and potentially inconsistent with the rest, when they are supposed to represent all the individual bits in a bitmask. One might argue that an error will be detected fairly soon, but to me using an enum to automatically manage the values and counter of items seems preferable. > Also the current mm tree has ___GFP_NOLOCKDEP which is not addressed > here so I suspect you have based your change on the Linus tree. I used your tree from kernel.org - I asked yesterday on #mm if it was a good idea and was told that it should be ok, so I did it, but I can redo the patch with mm. If you prefer to have this patch only as part of the larger patchset, I'm also fine with it. Also, if you could reply to [1], that would be greatly appreciated. Maybe I'm starting from some wrong assumption or there is a better way to achieve what I want. thanks, igor [1] http://marc.info/?l=linux-mm&m=149276346722464&w=2