> > > > > This can be pre-initialized, correct?
> > > >
> > > > pre-initialized? I am not quite understand your mean.
> > >
> > > I think you can maintain sg as part of device state and init sg with the
> bitmap.
> > >
> >
> > I got it.
> >
> > > > > This is grossly inefficient if you only requested a single page.
> > > > > And it's also allocating memory very aggressively without ever
> > > > > telling the host what is going on.
> > > >
> > > > If only requested a single page, there is no need to send the
> > > > entire page bitmap, This RFC patch has already considered about this.
> > >
> > > where's that addressed in code?
> > >
> >
> > By record the start_pfn and end_pfn.
> >
> > The start_pfn & end_pfn will be updated in set_page_bitmap() and will
> > be used in the function tell_host():
> >
> > ----------------------------------------------------------------------
> > -----------
> > +static void set_page_bitmap(struct virtio_balloon *vb, struct page
> > +*page) {
> > + unsigned int i;
> > + unsigned long *bitmap = vb->page_bitmap;
> > + unsigned long balloon_pfn = page_to_balloon_pfn(page);
> > +
> > + for (i = 0; i < VIRTIO_BALLOON_PAGES_PER_PAGE; i++)
> > + set_bit(balloon_pfn + i, bitmap);
>
> BTW, there's a page size value in header so there is no longer need to set
> multiple bits per page.
Yes, you are right.
>
> > + if (balloon_pfn < vb->start_pfn)
> > + vb->start_pfn = balloon_pfn;
> > + if (balloon_pfn > vb->end_pfn)
> > + vb->end_pfn = balloon_pfn;
> > +}
>
> Sounds good, but you also need to limit by allocated bitmap size.
Why should we limit the page bitmap size? Is it no good to send a large page bitmap?
or to save the memory used for page bitmap? Or some other reason?
>
> >
> > + unsigned long start_pfn, end_pfn, flags = 0, bmap_len;
> > + struct scatterlist sg[5];
> > +
> > + start_pfn = rounddown(vb->start_pfn, BITS_PER_LONG);
> > + end_pfn = roundup(vb->end_pfn, BITS_PER_LONG);
> > + bmap_len = (end_pfn - start_pfn) / BITS_PER_LONG *
> sizeof(long);
> > +
> > + sg_init_table(sg, 5);
> > + sg_set_buf(&sg[0], &flags, sizeof(flags));
> > + sg_set_buf(&sg[1], &start_pfn, sizeof(start_pfn));
> > + sg_set_buf(&sg[2], &page_shift, sizeof(page_shift));
> > + sg_set_buf(&sg[3], &bmap_len, sizeof(bmap_len));
> > + sg_set_buf(&sg[4], vb->page_bitmap +
> > + (start_pfn / BITS_PER_LONG), bmap_len);
>
> Looks wrong. start_pfn should start at offset 0 I think ...
I don't know what is wrong here, could you tell me why?
Thanks!
Liang
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
