(2010/04/27 22:46), Takuya Yoshikawa wrote:
(2010/04/27 22:18), Avi Kivity wrote:
Furthermore, the reduced allocations seem to produce good effects for
other cases too. Actually, I observed that the time for the ioctl was
more stable than the original one and the average time for dirty slots
was also reduced by some extent.
Can you explain why the dirty slots were improved?
I cannot do exactly, but vmalloc() might affect the following allocations?
Oh, this might be the effect of tlb flush or something?
- I'm not so confident about mm, but vmalloc(),vfree() does this kind of thing?
If so, the effect has more generic sense, and my explanation was misleading
in that sense.
And one more difference is from clear_user(). This probably uses registers as
source as opposed to copy_to_user()'s case.
But actually, I could check how much one vmalloc() consumed our time,
and this will be completely removed by our "moving dirty bitmaps to user
space".
Better to remove this explanation about dirty slots?
- slots = kzalloc(sizeof(struct kvm_memslots), GFP_KERNEL);
- if (!slots)
- goto out_free;
+ r = -ENOMEM;
+ dirty_bitmap = vmalloc(n);
+ if (!dirty_bitmap)
+ goto out;
+ memset(dirty_bitmap, 0, n);
Better to keep r = -ENOMEM here, so if something else is inserted, we
don't lose the error. It logically belongs with the allocation, not
inherited.
Oh, I read your similar comment to other person. Sorry, I should have.
BTW, maybe good news for us!
1. I found one place in which set_bit_to_user() is locally defined using
current helpers.
So we become easier to explain why set_bit_user_non_atomic() is needed.
2. I found copy_user helper of x86_32 asm which may be useful for
copy_in_user().
This is used both for *_from_user() and *_to_user() and my patch worked
with
it in more than enough speed.
With Fernando, I'm checking if this is OK for worst fault cases too.
Thanks,
Takuya
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
