On 6/4/19 9:55 AM, Ira Weiny wrote:
> On Tue, Jun 04, 2019 at 12:08:08AM -0700, Christoph Hellwig wrote:
>> On Mon, Jun 03, 2019 at 04:56:10PM -0700, Ira Weiny wrote:
>>> On Mon, Jun 03, 2019 at 09:42:06AM -0700, Christoph Hellwig wrote:
>>>>> +#if defined(CONFIG_CMA)
>>>>
>>>> You can just use #ifdef here.
>>>>
>>>>> +static inline int reject_cma_pages(int nr_pinned, unsigned int gup_flags,
>>>>> + struct page **pages)
>>>>
>>>> Please use two instead of one tab to indent the continuing line of
>>>> a function declaration.
>>>>
>>>>> +{
>>>>> + if (unlikely(gup_flags & FOLL_LONGTERM)) {
>>>>
>>>> IMHO it would be a little nicer if we could move this into the caller.
>>>
>>> FWIW we already had this discussion and thought it better to put this here.
>>>
>>> https://lkml.org/lkml/2019/5/30/1565
>>
>> I don't see any discussion like this. FYI, this is what I mean,
>> code might be easier than words:
>
> Indeed that is more clear. My apologies.
>
> Ira
>
>>
>>
>> diff --git a/mm/gup.c b/mm/gup.c
>> index ddde097cf9e4..62d770b18e2c 100644
>> --- a/mm/gup.c
>> +++ b/mm/gup.c
>> @@ -2197,6 +2197,27 @@ static int __gup_longterm_unlocked(unsigned long start, int nr_pages,
>> return ret;
>> }
>>
>> +#ifdef CONFIG_CMA
>> +static int reject_cma_pages(struct page **pages, int nr_pinned)
>> +{
>> + int i = 0;
>> +
>> + for (i = 0; i < nr_pinned; i++)
>> + if (is_migrate_cma_page(pages[i])) {
>> + put_user_pages(pages + i, nr_pinned - i);
>> + return i;
>> + }
>> + }
>> +
>> + return nr_pinned;
>> +}
>> +#else
>> +static inline int reject_cma_pages(struct page **pages, int nr_pinned)
>> +{
>> + return nr_pinned;
>> +}
>> +#endif /* CONFIG_CMA */
>> +
>> /**
>> * get_user_pages_fast() - pin user pages in memory
>> * @start: starting user address
>> @@ -2237,6 +2258,9 @@ int get_user_pages_fast(unsigned long start, int nr_pages,
>> ret = nr;
>> }
>>
>> + if (nr && unlikely(gup_flags & FOLL_LONGTERM))
>> + nr = reject_cma_pages(pages, nr);
>> +
Yes, now I see what you meant, and agree that that is cleaner.
thanks,
--
John Hubbard
NVIDIA
>> if (nr < nr_pages) {
>> /* Try to get the remaining pages with get_user_pages */
>> start += nr << PAGE_SHIFT;
>>
