On Thu, May 3, 2012 at 10:44 AM, Michael S. Tsirkin <mst@xxxxxxxxxx> 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@xxxxxxxxxx> 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@xxxxxxxxxx> 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? -- 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
