On Thu, Jan 12, 2017 at 11:06:06AM +0000, Daniel P. Berrange wrote:
On Thu, Jan 12, 2017 at 08:48:01AM -0200, Marcelo Tosatti wrote:
> On Thu, Jan 12, 2017 at 09:44:36AM +0800, 乔立勇(Eli Qiao) wrote:
> > hi, It's really good to have you get involved to support CAT in
> > libvirt/OpenStack.
> > replied inlines.
> >
> > 2017-01-11 20:19 GMT+08:00 Marcelo Tosatti <mtosatti(a)redhat.com>:
> >
> > >
> > > Hi,
> > >
> > > Comments/questions related to:
> > >
https://www.redhat.com/archives/libvir-list/2017-January/msg00354.html
> > >
> > > 1) root s2600wt:~/linux# virsh cachetune kvm02 --l3.count 2
> > >
> > > How does allocation of code/data look like?
> > >
> >
> > My plan's expose new options:
> >
> > virsh cachetune kvm02 --l3data.count 2 --l3code.count 2
> >
> > Please notes, you can use only l3 or l3data/l3code(if enable cdp while
> > mount resctrl fs)
>
> Fine. However, you should be able to emulate a type=both reservation
> (non cdp) by writing a schemata file with the same CBM bits:
>
> L3code:0=0x000ff;1=0x000ff
> L3data:0=0x000ff;1=0x000ff
>
> (*)
>
> I don't see how this interface enables that possibility.
>
> I suppose it would be easier for mgmt software to have it
> done automatically:
>
> virsh cachetune kvm02 --l3 size_in_kbytes.
>
> Would create the reservations as (*) in resctrlfs, in
> case host is CDP enabled.
You'll be able to query libvirt to determine whether you have
l3, or l3data + l3code. So mgmt app can decide to emulate
"type=both", if it sees l3data+l3code as separate items.
No it can't, because the interface does not allow you to specify
whether l3data and l3code should intersect each other (and by how much).
Unless you do that. Which IMO is overkill.
Or a parameter (option 1):
virsh cachetune kvm02 --l3data size_in_kbytes --l3code size_in_kbytes
--share-l3
meaning the reservations share space.
OR (option 2):
virsh cachetune kvm02 --l3 size_in_kbytes
(with internal translation to l3data and l3code reservations in the
same space).
I don't see any point in having option 1, option 2 is simpler and
removes a tunable from the interface.
Do you have a reason behind the statement that mgmt app should
decide this?
> > > 4) Usefulness of exposing minimum unit size.
> > >
> > > Rather than specify unit sizes (which forces the user
> > > to convert every time the command is executed), why not specify
> > > in kbytes and round up?
> > >
> >
> > I accept this, I propose to expose minimum unit size because of I'd like
to
> > let using specify the unit count(which as you say this is not good),
> >
> > as you know the minimum unit size is decided by hard ware
> > eg
> > on a host, we have 56320 KiB cache, and the max cbm length is 20 (fffff),
> > so the minimum cache should be 56320/20 = 2816 KiB
> >
> > if we allow use specify cache size instead of cache unit count, user may
> > set the cache as 2817 KiB, and we should round up it to 2816 * 2, there
> > will be 2815 KiB wasted.
>
> Yes but the user can know the wasted amount if necessary, if you expose
> the cache unit size (again, i doubt this will happen in practice because
> the granularity of the CBM bits is small compared to the cache size).
>
> The problem with the cache unit count specification is that it does not
> work across different hosts: if a user saves the "cache unit count"
> value manually in a XML file, then uses that XML file on a different
> host, the reservation on the new host can become smaller than desired,
> which violates expectations.
Yes, public APIs should always use an actual size, usually KB in most
of our APIs, but sometimes bytes.
> > Anyway , I am open to using KiB size and let libvirt to calculate the cbm
> > bits, am thinking if we need to tell the actual_cache_size is up to 5632
> > KiB even they wants 2816 KiB cache.
>
> Another thing i did on resctrltool is to have a safety margin for
> allocations: do not let the user allocate all of the cache (that is
> leave 0 bytes for the default group). I used one cache unit as the
> minimum:
>
> if ret == ERR_LOW_SPACE:
> print "Warning: free space on default mask is <= %d\n" %
> (kbytes_per_bit_of_cbm)
> print "use --force to force"
Libvirt explicitly aims to avoid making policy decisions like this.
As your "--force" message shows there, it means you then have to
add in ways to get around the policy. Libvirt just tries to provide
the mechanism and leave it to the app to decide on usage policy.
Actually what i said is nonsense the kernel does it already.