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

[Wang, Wei W]

> -----Original Message-----
> From: Tetsuo Handa [mailto:penguin-kernel@xxxxxxxxxxxxxxxxxxx]
> Sent: Sunday, December 17, 2017 6:22 PM
> To: Wang, Wei W <wei.w.wang@xxxxxxxxx>; willy@xxxxxxxxxxxxx
> Cc: virtio-dev@xxxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; qemu-
> devel@xxxxxxxxxx; virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx;
> kvm@xxxxxxxxxxxxxxx; linux-mm@xxxxxxxxx; mst@xxxxxxxxxx;
> mhocko@xxxxxxxxxx; akpm@xxxxxxxxxxxxxxxxxxxx; mawilcox@xxxxxxxxxxxxx;
> david@xxxxxxxxxx; cornelia.huck@xxxxxxxxxx;
> mgorman@xxxxxxxxxxxxxxxxxxx; aarcange@xxxxxxxxxx;
> amit.shah@xxxxxxxxxx; pbonzini@xxxxxxxxxx;
> liliang.opensource@xxxxxxxxx; yang.zhang.wz@xxxxxxxxx;
> quan.xu@xxxxxxxxxx; nilal@xxxxxxxxxx; riel@xxxxxxxxxx
> Subject: Re: [PATCH v19 3/7] xbitmap: add more operations
> 
> Wei Wang wrote:
> > > 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.
> >
> > 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().

> >
> > 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