| Message ID | 20240613122803.1031511-1-maobibo@loongson.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: Discard zero mask with function kvm_dirty_ring_reset | expand |
On Thu, Jun 13, 2024, Bibo Mao wrote: > Function kvm_reset_dirty_gfn may be called with parameters cur_slot / > cur_offset / mask are all zero, it does not represent real dirty page. > It is not necessary to clear dirty page in this condition. Also return > value of macro __fls() is undefined if mask is zero which is called in > funciton kvm_reset_dirty_gfn(). Here just discard it. > > Signed-off-by: Bibo Mao <maobibo@loongson.cn> > --- > virt/kvm/dirty_ring.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/virt/kvm/dirty_ring.c b/virt/kvm/dirty_ring.c > index 86d267db87bb..05f4c1c40cc7 100644 > --- a/virt/kvm/dirty_ring.c > +++ b/virt/kvm/dirty_ring.c > @@ -147,14 +147,16 @@ int kvm_dirty_ring_reset(struct kvm *kvm, struct kvm_dirty_ring *ring) > continue; > } > } > - kvm_reset_dirty_gfn(kvm, cur_slot, cur_offset, mask); > + if (mask) > + kvm_reset_dirty_gfn(kvm, cur_slot, cur_offset, mask); > cur_slot = next_slot; > cur_offset = next_offset; > mask = 1; > first_round = false; > } > > - kvm_reset_dirty_gfn(kvm, cur_slot, cur_offset, mask); > + if (mask) > + kvm_reset_dirty_gfn(kvm, cur_slot, cur_offset, mask); Given that mask must be checked before __fls(), just do: diff --git a/virt/kvm/dirty_ring.c b/virt/kvm/dirty_ring.c index 86d267db87bb..7bc74969a819 100644 --- a/virt/kvm/dirty_ring.c +++ b/virt/kvm/dirty_ring.c @@ -55,6 +55,9 @@ static void kvm_reset_dirty_gfn(struct kvm *kvm, u32 slot, u64 offset, u64 mask) struct kvm_memory_slot *memslot; int as_id, id; + if (!mask) + return; + as_id = slot >> 16; id = (u16)slot;
On 2024/6/14 上午12:43, Sean Christopherson wrote: > On Thu, Jun 13, 2024, Bibo Mao wrote: >> Function kvm_reset_dirty_gfn may be called with parameters cur_slot / >> cur_offset / mask are all zero, it does not represent real dirty page. >> It is not necessary to clear dirty page in this condition. Also return >> value of macro __fls() is undefined if mask is zero which is called in >> funciton kvm_reset_dirty_gfn(). Here just discard it. >> >> Signed-off-by: Bibo Mao <maobibo@loongson.cn> >> --- >> virt/kvm/dirty_ring.c | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/virt/kvm/dirty_ring.c b/virt/kvm/dirty_ring.c >> index 86d267db87bb..05f4c1c40cc7 100644 >> --- a/virt/kvm/dirty_ring.c >> +++ b/virt/kvm/dirty_ring.c >> @@ -147,14 +147,16 @@ int kvm_dirty_ring_reset(struct kvm *kvm, struct kvm_dirty_ring *ring) >> continue; >> } >> } >> - kvm_reset_dirty_gfn(kvm, cur_slot, cur_offset, mask); >> + if (mask) >> + kvm_reset_dirty_gfn(kvm, cur_slot, cur_offset, mask); >> cur_slot = next_slot; >> cur_offset = next_offset; >> mask = 1; >> first_round = false; >> } >> >> - kvm_reset_dirty_gfn(kvm, cur_slot, cur_offset, mask); >> + if (mask) >> + kvm_reset_dirty_gfn(kvm, cur_slot, cur_offset, mask); > > Given that mask must be checked before __fls(), just do: That is ok for me. To be frankly I am not familiar with kvm common code, I submit this patch just when I look through the migration source code. Regards Bibo Mao > > diff --git a/virt/kvm/dirty_ring.c b/virt/kvm/dirty_ring.c > index 86d267db87bb..7bc74969a819 100644 > --- a/virt/kvm/dirty_ring.c > +++ b/virt/kvm/dirty_ring.c > @@ -55,6 +55,9 @@ static void kvm_reset_dirty_gfn(struct kvm *kvm, u32 slot, u64 offset, u64 mask) > struct kvm_memory_slot *memslot; > int as_id, id; > > + if (!mask) > + return; > + > as_id = slot >> 16; > id = (u16)slot; >
Queued the alternative patch that returns early from kvm_reset_dirty_gfn, thanks. Paolo
diff --git a/virt/kvm/dirty_ring.c b/virt/kvm/dirty_ring.c index 86d267db87bb..05f4c1c40cc7 100644 --- a/virt/kvm/dirty_ring.c +++ b/virt/kvm/dirty_ring.c @@ -147,14 +147,16 @@ int kvm_dirty_ring_reset(struct kvm *kvm, struct kvm_dirty_ring *ring) continue; } } - kvm_reset_dirty_gfn(kvm, cur_slot, cur_offset, mask); + if (mask) + kvm_reset_dirty_gfn(kvm, cur_slot, cur_offset, mask); cur_slot = next_slot; cur_offset = next_offset; mask = 1; first_round = false; } - kvm_reset_dirty_gfn(kvm, cur_slot, cur_offset, mask); + if (mask) + kvm_reset_dirty_gfn(kvm, cur_slot, cur_offset, mask); /* * The request KVM_REQ_DIRTY_RING_SOFT_FULL will be cleared
Function kvm_reset_dirty_gfn may be called with parameters cur_slot / cur_offset / mask are all zero, it does not represent real dirty page. It is not necessary to clear dirty page in this condition. Also return value of macro __fls() is undefined if mask is zero which is called in funciton kvm_reset_dirty_gfn(). Here just discard it. Signed-off-by: Bibo Mao <maobibo@loongson.cn> --- virt/kvm/dirty_ring.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) base-commit: 83a7eefedc9b56fe7bfeff13b6c7356688ffa670