[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: [PATCH v21 2/5 RESEND] virtio-balloon: VIRTIO_BALLOON_F_SG
On 01/09/2018 10:42 PM, Tetsuo Handa wrote:
Wei Wang wrote:- enable OOM to free inflated pages maintained in the local temporary list.I do want to see it before applying this patch.
Fine, then what do you think of the method I shared in your post here: https://patchwork.kernel.org/patch/10140731/
Michael, could we merge patch 3-5 first?
Please carefully check how the xbitmap implementation works, and you will find that you are adding a lot of redundant operations with a bug.
This version mainly added some test cases, and it passes the test run without any issue. Appreciate it if your comments could be more specific, that would make the discussion more effective, for example, I deliberately added "xb_find_set(xb1, 2, ULONG_MAX - 3)" for the overflow test, not sure if this is the "bug" you referred to, but I'm glad to hear your different thought.
I agree that some tests may be repeated in some degree, since we test the implementation from different aspects, for example, xbitmap_check_bit_range() may have already performed xb_zero() while we specifically have another xbitmap_check_zero_bits() which may test something that has already been tested when checking bit range. But I think testing twice is better than omission. Also, I left the "Regualr test1: node=NULL" case though the new implementation doesn't explicitly use "node" as before, but that node=NULL is still a radix tree implementation internally and that case looks special to me, so maybe not bad to cover in the test.
You are also welcome to send a patch to remove the redundant one if you think that's an issue. Thanks.