On Fri 25-05-18 09:43:09, Huaisheng HS1 Ye wrote: > From: Michal Hocko [mailto:mhocko@xxxxxxxxxx] > Sent: Thursday, May 24, 2018 8:19 PM> > > > Let me try to reply your questions. > > > Exactly, GFP_ZONE_TABLE is too complicated. I think there are two advantages > > > from the series of patches. > > > > > > 1. XOR operation is simple and efficient, GFP_ZONE_TABLE/BAD need to do twice > > > shift operations, the first is for getting a zone_type and the second is for > > > checking the to be returned type is a correct or not. But with these patch XOR > > > operation just needs to use once. Because the bottom 3 bits of GFP bitmask have > > > been used to represent the encoded zone number, we can say there is no bad zone > > > number if all callers could use it without buggy way. Of course, the returned > > > zone type in gfp_zone needs to be no more than ZONE_MOVABLE. > > > > But you are losing the ability to check for wrong usage. And it seems > > that the sad reality is that the existing code do screw up. > > In my opinion, originally there shouldn't be such many wrong > combinations of these bottom 3 bits. For any user, whether or > driver and fs, they should make a decision that which zone is they > preferred. Matthew's idea is great, because with it the user must > offer an unambiguous flag to gfp zone bits. Well, I would argue that those shouldn't really care about any zones at all. All they should carea bout is whether they really need a low mem zone (aka directly accessible to the kernel), highmem or they are the allocation is generally movable. Mixing zones into the picture just makes the whole thing more complicated and error prone. [...] > > That being said. I am not saying that I am in love with GFP_ZONE_TABLE. > > It always makes my head explode when I look there but it seems to work > > with the current code and it is optimized for it. If you want to change > > this then you should make sure you describe reasons _why_ this is an > > improvement. And I would argue that "we can have more zones" is a > > relevant one. > > Yes, GFP_ZONE_TABLE is too complicated. The patches have 4 advantages as below. > > * The address zone modifiers have new operation method, that is, user should decide which zone is preferred at first, then give the encoded zone number to bottom 3 bits in GFP mask. That is much direct and clear than before. > > * No bad zone combination, because user should choose just one address zone modifier always. > * Better performance and efficiency, current gfp_zone has to take shifting operation twice for GFP_ZONE_TABLE and GFP_ZONE_BAD. With these patches, gfp_zone() just needs one XOR. > * Up to 8 zones can be used. At least it isn't a disadvantage, right? This should be a part of the changelog. Please note that you should provide some number if you claim performance benefits. The complexity will always be subjective. -- Michal Hocko SUSE Labs
