On Tue, Mar 21, 2017 at 08:10:05PM +0900, Minchan Kim wrote: > Hi Joonsoo, > > On Thu, Mar 16, 2017 at 11:46:35AM +0900, [email protected] wrote: > > From: Joonsoo Kim <[email protected]> > > > > Zspage is always movable and is used through zs_map_object() function > > which returns directly accessible pointer that contains content of > > zspage. It is independent on the user's allocation flag. > > Therefore, it's better to always set movable/highmem flag to the zspage. > > After that, we don't need __GFP_MOVABLE/__GFP_HIGHMEM clearing in > > cache_alloc_handle()/cache_alloc_zspage() since there is no zs_malloc > > caller who specifies __GFP_MOVABLE/__GFP_HIGHMEM. > > Hmm, I wanted this when you pointed out to me firstly but when I think > again, I don't see it's improvement. Sorry for that. > The zs_malloc is exported symbol and it has gfp_t argument so user can > do whatever he want with any zone modifiers flags. IOW, if someuser want > to allocate pages from {normal|dma} zone by whatever reason, he can > omit __GFP_HIGHMEM from the gfp flag to fullfill the goal.
Hello, I don't think that such flexibility makes things better. User cannot fully understand what flags are the best since it highly depends on implementation detail. For example, __GFP_MOVABLE is needed to optimize memory fragmentation and user cannot know it and there is no reason that user need to know it. __GFP_HIGHMEM is the similar case. He cannot know that he can pass __GFP_HIGHMEM without knowing the implementation detail and he cannot know the impact of __GFP_HIGHMEM here. So, I think that adding these flags in zsmalloc can be justified. Anyway, this patch isn't so important for this series so if you don't like it, I will drop it. Thanks.
