Re: [Xen-devel] [External] Re: [RFC PATCH v2 00/12] get rid of GFP_ZONE_TABLE/BAD

[Michal Hocko <mhocko@xxxxxxxxxx>]

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

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel