Re: using cache for virtio allocations?

["Michael S. Tsirkin" <mst@redhat.com>]

On Thu, May 03, 2012 at 10:48:53AM +0200, Sasha Levin wrote:
> On Thu, May 3, 2012 at 10:44 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Thu, May 03, 2012 at 10:38:56AM +0200, Sasha Levin wrote:
> >> On Thu, May 3, 2012 at 9:32 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> >> > On Thu, May 03, 2012 at 07:51:18AM +0200, Sasha Levin wrote:
> >> >> On Thu, May 3, 2012 at 7:29 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> >> >> > Sasha, didn't you have a patch to allocate
> >> >> > things using cache in virtio core?
> >> >> > What happened to it?
> >> >> >
> >> >> > Thanks,
> >> >> > MST
> >> >>
> >> >> It got stuck due to several things, and I got sidetracked, sorry. Here
> >> >> are the outstanding issues:
> >> >>
> >> >> 1. Since now we can allocate a descriptor either using kmalloc or from
> >> >> the cache, we need a new flag in vring_desc to know how to free it, it
> >> >> seems a bit too intrusive,
> >> >> and I couldn't thing of a better
> >> >> alternative.
> >> >
> >> > Since that is guest visible it does not sound great, I agree.
> >> >
> >> > Three ideas:
> >> > 1. The logic looks at descriptor size so can we just read
> >> >   desc.len before free and rerun the same math?
> >>
> >> It'll break every time the value is changed (either by the user or by
> >> some dynamic algorithm thingie).
> >
> > Yes but did you intend to implement such complex logic?
> > If not let's not over-engineer.
> 
> I did intend to allow him to change the value while the device is
> running, if we don't want to allow that then it's easy.
> 
> >> > 2. For -net the requests are up to max_skb_frags + 2 in size, right?
> >> >   Does it make sense to just use cache for net, always?
> >> >   That would mean a per device flag.
> >>
> >> Yup, it could work.
> >>
> >> > 3. Allocate a bit more and stick extra data before the 1st descriptor.
> >>
> >> I guess it'll work, but it just seems a bit ugly :)
> >
> > An understatement.
> >
> >> >> 2. Rusty has pointed out that no one is going to modify the default
> >> >> value we set, and we don't really have a good default value to put
> >> >> there (at least, we haven't agreed on a specific value). Also, you
> >> >> have noted that it should be a per-device value, which complicates
> >> >> this question further since we probably want a different value for
> >> >> each device type.
> >> >>
> >> >> While the first one can be solved easily with a blessing from the
> >> >> maintainers, the second one will require testing on various platforms,
> >> >> configurations and devices to select either the best "magic" value, or
> >> >> the best algorithm to play with threshold.
> >> >
> >> > Not sure about platforms but for devices that's right.
> >> > But this really only means we only change what we tested.
> >> > eg see what is good for net and change net in a way
> >> > that others will keep using old code.
> >>
> >> It'll work only if there will be someone following up and actually
> >> testing it, since regular users won't be testing it at all (with it
> >> being defaulted to off and everything).
> >
> > Not sure I understand. Whatever patch gets applied will be
> > tested beforehand.
> 
> I thought you meant that we apply the patch with threshold set at
> 0/disabled, and based on future tests we will enable it for specific
> devices and set best values for threshold, no?

Exactly the opposite. I meant each driver sets the value
and we test it to find a good value. Drivers that don't
do anything use existing kmalloc code.