Re: [PATCH v3 1/1] mm: introduce put_user_page*(), placeholder versions

[John Hubbard]

On 3/8/19 9:57 AM, Jerome Glisse wrote:
[snip] 
> Just a small comments below that would help my life :)
> 
> Reviewed-by: JÃrÃme Glisse <jglisse@xxxxxxxxxx>
> 

Thanks for the review! 

>> ---
>>  include/linux/mm.h | 24 ++++++++++++++
>>  mm/swap.c          | 82 ++++++++++++++++++++++++++++++++++++++++++++++
> 
> Why not putting those functions in gup.c instead of swap.c ?

Yes, gup.c is better for these. And it passes the various cross compiler and
tinyconfig builds locally, so I think I'm not missing any cases. (The swap.c 
location was an artifact of very early approaches, pre-dating the
put_user_pages() name.) 

[snip]

>>  #define SECTION_IN_PAGE_FLAGS
>>  #endif
>> diff --git a/mm/swap.c b/mm/swap.c
>> index 4d7d37eb3c40..a6b4f693f46d 100644
>> --- a/mm/swap.c
>> +++ b/mm/swap.c
>> @@ -133,6 +133,88 @@ void put_pages_list(struct list_head *pages)
>>  }
>>  EXPORT_SYMBOL(put_pages_list);
>>  
>> +typedef int (*set_dirty_func)(struct page *page);
> 
> set_dirty_func_t would be better as it is the rule for typedef to append
> the _t also it make it easier for coccinelle patch.
> 

Done. I'm posting a v4 in a moment, with both of the above, plus
Christopher's "real filesystems" wording change, and your reviewed-by
tag.


thanks,
-- 
John Hubbard
NVIDIA