RE: [PATCH v19 3/7] xbitmap: add more operations

-----Original Message-----
From: Tetsuo Handa [mailto:penguin-kernel@I-love.SAKURA.ne.jp]
Sent: Sunday, December 17, 2017 6:22 PM
To: Wang, Wei W <redacted>; willy@infradead.org
Cc: virtio-dev@lists.oasis-open.org; linux-kernel@vger.kernel.org; qemu-
devel@nongnu.org; virtualization@lists.linux-foundation.org;
kvm@vger.kernel.org; linux-mm@kvack.org; mst@redhat.com;
mhocko@kernel.org; akpm@linux-foundation.org; mawilcox@microsoft.com;
david@redhat.com; cornelia.huck@de.ibm.com;
mgorman@techsingularity.net; aarcange@redhat.com;
amit.shah@redhat.com; pbonzini@redhat.com;
liliang.opensource@gmail.com; yang.zhang.wz@gmail.com;
quan.xu@aliyun.com; nilal@redhat.com; riel@redhat.com
Subject: Re: [PATCH v19 3/7] xbitmap: add more operations

Wei Wang wrote:

quoted

quoted

But passing GFP_NOWAIT means that we can handle allocation failure.
There is no need to use preload approach when we can handle allocation

failure.

quoted

I think the reason we need xb_preload is because radix tree insertion
needs the memory being preallocated already (it couldn't suffer from
memory failure during the process of inserting, probably because
handling the failure there isn't easy, Matthew may know the backstory
of
this)

According to https://lwn.net/Articles/175432/ , I think that preloading is
needed only when failure to insert an item into a radix tree is a significant
problem.
That is, when failure to insert an item into a radix tree is not a problem, I
think that we don't need to use preloading.

It also mentions that the preload attempts to allocate sufficient memory to *guarantee* that the next radix tree insertion cannot fail.

If we check radix_tree_node_alloc(), the comments there says "this assumes that the caller has performed appropriate preallocation".

So, I think we would get a risk of triggering some issue without preload().

quoted

So, I think we can handle the memory failure with xb_preload, which
stops going into the radix tree APIs, but shouldn't call radix tree
APIs without the related memory preallocated.

It seems to me that virtio-ballon case has no problem without using
preloading.

Why is that?

Best,
Wei