From: Avi Kivity on
On 07/06/2010 01:48 PM, Xiao Guangrong wrote:
> This function not only return the gfn's page but also the page number
> after @gfn in the slot
>
>
> @@ -923,15 +923,25 @@ static unsigned long gfn_to_hva_memslot(struct kvm_memory_slot *slot, gfn_t gfn)
> return slot->userspace_addr + (gfn - slot->base_gfn) * PAGE_SIZE;
> }
>
> -unsigned long gfn_to_hva(struct kvm *kvm, gfn_t gfn)
> +static unsigned long gfn_to_hva_many(struct kvm *kvm, gfn_t gfn, int *entry)
> {
>

'entry' better be gfn_t. We limit gfns in a slot, but that's
artificial, let's use the correct type.

Also suggest nr_pages as the name.

> @@ -1011,6 +1021,23 @@ pfn_t gfn_to_pfn_memslot(struct kvm *kvm,
> return hva_to_pfn(kvm, addr, false);
> }
>
> +int gfn_to_page_many_atomic(struct kvm *kvm, gfn_t gfn,
> + struct page **pages, int nr_pages, bool *enough)
> +{
> + unsigned long addr;
> + int entry, ret;
> +
> + addr = gfn_to_hva_many(kvm, gfn,&entry);
> + if (kvm_is_error_hva(addr))
> + return -1;
> +
> + entry = min(entry, nr_pages);
> + *enough = (entry == nr_pages) ? true : false;
>

s/ ? true : false//; :)

Why not return 0 if !enough?

> + ret = __get_user_pages_fast(addr, entry, 1, pages);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(gfn_to_page_many_atomic);
> +
> struct page *gfn_to_page(struct kvm *kvm, gfn_t gfn)
> {
> pfn_t pfn;
>


--
error compiling committee.c: too many arguments to function

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Xiao Guangrong on


Avi Kivity wrote:
> On 07/06/2010 01:48 PM, Xiao Guangrong wrote:
>> This function not only return the gfn's page but also the page number
>> after @gfn in the slot
>>
>>
>> @@ -923,15 +923,25 @@ static unsigned long gfn_to_hva_memslot(struct
>> kvm_memory_slot *slot, gfn_t gfn)
>> return slot->userspace_addr + (gfn - slot->base_gfn) * PAGE_SIZE;
>> }
>>
>> -unsigned long gfn_to_hva(struct kvm *kvm, gfn_t gfn)
>> +static unsigned long gfn_to_hva_many(struct kvm *kvm, gfn_t gfn, int
>> *entry)
>> {
>>
>
> 'entry' better be gfn_t. We limit gfns in a slot, but that's
> artificial, let's use the correct type.
>
> Also suggest nr_pages as the name.
>

OK, will fix it in the next version.

>> @@ -1011,6 +1021,23 @@ pfn_t gfn_to_pfn_memslot(struct kvm *kvm,
>> return hva_to_pfn(kvm, addr, false);
>> }
>>
>> +int gfn_to_page_many_atomic(struct kvm *kvm, gfn_t gfn,
>> + struct page **pages, int nr_pages, bool *enough)
>> +{
>> + unsigned long addr;
>> + int entry, ret;
>> +
>> + addr = gfn_to_hva_many(kvm, gfn,&entry);
>> + if (kvm_is_error_hva(addr))
>> + return -1;
>> +
>> + entry = min(entry, nr_pages);
>> + *enough = (entry == nr_pages) ? true : false;
>>
>
> s/ ? true : false//; :)

Yeah, it's better.

>
> Why not return 0 if !enough?
>

I think it's better that handle the reset pages in the slot, for example,
we expect 16 pages are consecutive, but only 12 pages in the slot, the better
way is handle the reset 12 pages not throw those away.

>> + ret = __get_user_pages_fast(addr, entry, 1, pages);
>> + return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(gfn_to_page_many_atomic);
>> +
>> struct page *gfn_to_page(struct kvm *kvm, gfn_t gfn)
>> {
>> pfn_t pfn;
>>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Avi Kivity on
On 07/12/2010 05:55 AM, Xiao Guangrong wrote:
>
>>> @@ -1011,6 +1021,23 @@ pfn_t gfn_to_pfn_memslot(struct kvm *kvm,
>>> return hva_to_pfn(kvm, addr, false);
>>> }
>>>
>>> +int gfn_to_page_many_atomic(struct kvm *kvm, gfn_t gfn,
>>> + struct page **pages, int nr_pages, bool *enough)
>>> +{
>>> + unsigned long addr;
>>> + int entry, ret;
>>> +
>>> + addr = gfn_to_hva_many(kvm, gfn,&entry);
>>> + if (kvm_is_error_hva(addr))
>>> + return -1;
>>> +
>>> + entry = min(entry, nr_pages);
>>> + *enough = (entry == nr_pages) ? true : false;
>>>
>>>
>
>> Why not return 0 if !enough?
>>
>>
> I think it's better that handle the reset pages in the slot, for example,
> we expect 16 pages are consecutive, but only 12 pages in the slot, the better
> way is handle the reset 12 pages not throw those away.

It will almost never happen, let's remove edge cases.




--
error compiling committee.c: too many arguments to function

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Xiao Guangrong on


Avi Kivity wrote:

>>>
>> I think it's better that handle the reset pages in the slot, for example,
>> we expect 16 pages are consecutive, but only 12 pages in the slot, the
>> better
>> way is handle the reset 12 pages not throw those away.
>
> It will almost never happen, let's remove edge cases.
>

OK, will update it.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/