----- On Nov 6, 2017, at 9:07 PM, Boqun Feng boqun.feng@gmail.com wrote:

> On Mon, Nov 06, 2017 at 03:56:38PM -0500, Mathieu Desnoyers wrote:
> [...]
>> +static int cpu_op_pin_pages(unsigned long addr, unsigned long len,
>> +		struct page ***pinned_pages_ptr, size_t *nr_pinned,
>> +		int write)
>> +{
>> +	struct page *pages[2];
>> +	int ret, nr_pages;
>> +
>> +	if (!len)
>> +		return 0;
>> +	nr_pages = cpu_op_range_nr_pages(addr, len);
>> +	BUG_ON(nr_pages > 2);
>> +	if (*nr_pinned + nr_pages > NR_PINNED_PAGES_ON_STACK) {
> 
> Is this a bug? Seems you will kzalloc() every time if *nr_pinned is
> bigger than NR_PINNED_PAGES_ON_STACK, which will result in memory
> leaking.
> 
> I think the logic here is complex enough for us to introduce a
> structure, like:
> 
>	struct cpu_opv_page_pinner {
>		int nr_pinned;
>		bool is_kmalloc;
>		struct page **pinned_pages;
>	};
> 
> Thoughts?

Good catch !

How about the attached diff ? I'll fold it into the rseq/dev tree.

Thanks,

Mathieu

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com