On Mon, Oct 26, 2026 at 12:19 AM Michael S. Tsirkin <[email protected]> wrote: > On Sun, Oct 25, 2026 at 04:56:41PM -0700, Ruben Hakobyan wrote: > > On Fri, Oct 23, 2026 at 11:04 AM Michael S. Tsirkin <[email protected]> wrote: > > > > > > Great patch, thanks! > > > > > > On Fri, Oct 23, 2026 at 01:19:19AM -0700, Ruben Hakobyan wrote: > > > > The virtio_balloon driver reports a scatter-gather list of free pages > > > > to the VMM so it may discard unused pages. > > ... > > > > > > It's not just a question of convenient, is it? > > > Rust is buggy wrt to spec as it allows only U32_MAX. > > > So it is also to make it work. > > > So should we maybe fix Rust to be spec compliant and allow 2^32 too? > > > > > > I think we should definitely recommend U32_MAX in the spec > > > because otherwise it can not fit in the used length. > > > > > > > Thanks for the review! > > > > Sounds good to me, I will CC [email protected] as > > per your recommendation. > > > > Agreed that Rust-VMM should also be fixed in case 2^32 needs to > > stay the hard limit. I think here for the driver, we can be more > > conservative as long as we stay below 2^32. > > > > ... > > > > > > --- > > > > The wording in the spec is a bit ambiguous because it uses "chain > > > > length" for both descriptor counts and the total size of the memory > > > > ranges referenced by the descriptors. My read is that the byte limit > > > > applies to the sum of descriptor lengths, which is also what rust-vmm > > > > checks. > > > > > > > > > quote what you are referring to please, also in the commit log > > > not after ---. > > > > I will update with quotes in v2, thanks. > > > > > > > > I would also CC [email protected] - spec should recommend > > > U32_MAX. > > > > > > > > > > > Another option I considered was to limit reporting capacity based > > > > on the largest possible block. E.g. for 64 KiB pages, the largest > > > > block is 512 MiB, so limit the reporting capacity to 7 to stay > > > > below 4GiB. Although simpler, I found that the time to reclaim > > > > was quite a bit longer. > > > > > > > > > > > > So I was going to apply this actually: > > > > > > https://lore.kernel.org/all/b541ff170218bd5d45b551b5132e7cf2c490d761.1780906288.git....@redhat.com/ > > > > > > which makes the question completely moot. > > > > > > Does this have any effect on performance? > > > > > > Thanks! > > > > Thanks for sharing this. Yes, this would also make sure we stay below > > the limit. Regarding performance, I tested this on QEMU and it calls > > virtio_notify() after each completion. > > Who, QEMU does?
Yes QEMU, sorry for the ambiguous wording. > > > Using one chain per entry > > creates more chains, so QEMU can use more CPU time to reclaim the same > > amount of memory. > > > > If we go with this, I think a QEMU patch to batch notifications > > could reduce the overhead. Alternatively, I think we could create > > larger chains here but make sure they stay within the 2^32 limit, > > and keeping your approach of queuing them all before kicking. > > > > I am happy to share a patch with my thinking here. > > > > ... > > Oh. It's just the driver being silly and never disabling > callbacks. > > pls just add virtqueue_enable_cb_delayed and this will fix it. > Ah great, I tried with virtqueue_enable_cb_delayed() and yeah it largely fixed it. So regarding the patch in https://lore.kernel.org/all/b541ff170218bd5d45b551b5132e7cf2c490d761.1780906288.git....@redhat.com/ I see it's in a larger series, so just wanted to check if you're planning to send this patch separately? Or let me know if you'd like me to do it. Thanks!
