| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-03-19 | |||
| 17:29:08 | openstackgerrit | Matthew Booth proposed openstack/nova master: Eventlet monkey patching should be as early as possible https://review.openstack.org/626952 | |
| 17:29:10 | mdbooth | Thar she blows. | |
| 17:29:18 | sean-k-mooney | or maybe we only do that for realtime i can check | |
| 17:29:28 | mdbooth | Lets see if that passes muster. | |
| 17:30:03 | aspiers | sean-k-mooney: ah OK, is that also done for device pass-through? | |
| 17:30:47 | sean-k-mooney | no its not done for device pass-though | |
| 17:32:37 | sean-k-mooney | aspiers: this is the only time we current lock the memory explcitly whic is for realtime guests | |
| 17:32:39 | sean-k-mooney | http://git.openstack.org/cgit/openstack/nova/tree/nova/virt/libvirt/driver.py#n4810 | |
| 17:34:20 | sean-k-mooney | the way dpdk work i am prtty sure the hugepage does not migrate to different phyical pages on the host as it is used for dma transfer for the vhost-user nics | |
| 17:34:54 | sean-k-mooney | aspiers: in anycase if you have hardware you can test it on it would be worth testing with hugepages | |
| 17:36:04 | sean-k-mooney | aspiers: but if dan's and brijesh's assertion regarding the rom+pflash issue are correct it may not be enough | |
| 17:36:04 | aspiers | sean-k-mooney: AMD already tried hugepages in one of their earliest iterations | |
| 17:36:15 | aspiers | that's what I just heard | |
| 17:36:49 | aspiers | they are also saying that mlock() is just a hint not a guarantee | |
| 17:37:31 | sean-k-mooney | ok well in that case i dont know what the best path forword is on the hardlimit issue | |
| 17:37:55 | aspiers | one suggestion was to expose the right hardlimit via QEMU / libvirt | |
| 17:38:01 | aspiers | so nova could just query it | |
| 17:39:29 | sean-k-mooney | aspiers: if qemu/libvirt can provide it that sounds resonable but it would make the relevent libvirt/qemu version that provides that the minium qemu/libivrt for that feature | |
| 17:39:47 | aspiers | correct, there's already a minimum version requirement anyway | |
| 17:40:06 | aspiers | I think everyone is aware of the issue now though, so hopefully Dan and Brijesh can figure something out :) | |
| 17:45:47 | sean-k-mooney | aspiers: so reading dan's comment again if we just remove the hard_limmit entirly then that may be suffienct to adress his orignial comment here https://review.openstack.org/#/c/641994/2/specs/train/approved/amd-sev-libvirt-support.rst@167 | |
| 17:46:32 | sean-k-mooney | ah but that wont work as that is how you are pinning the memory | |
| 17:46:47 | aspiers | right | |
| 17:49:15 | aspiers | bryan_stephenson: catch up on the conversation via http://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2019-03-19.log.html#t2019-03-19T17:13:27 | |
| 17:50:22 | sean-k-mooney | aspiers: honestly looking at the documentation of hard_limit i think relying on it for the behavior you desire is undocumented and not part of the contract at least form the libvirt perspective | |
| 17:50:56 | sean-k-mooney | aspiers: hard_limit does not guarntee teh meoeoy will not be swapped or that it will be preallocated | |
| 17:51:37 | sean-k-mooney | aspiers: thos poperties are conntoeld via allocation and locked in the memory backing | |
| 17:53:20 | bbobrov | lets just ask on the libvirt mailing list | |
| 17:53:31 | bbobrov | (hi) | |
| 17:53:37 | aspiers | I agree with bbobrov | |
| 17:53:46 | aspiers | this probably requires libvirt changes | |
| 17:54:35 | aspiers | bbobrov: Do you want to kick off the thread there? | |
| 17:54:39 | sean-k-mooney | bbobrov: aspiers sure let us know what they say | |
| 17:55:25 | bbobrov | aspiers: yep, i will do it, but starting tomorrow | |
| 17:55:28 | aspiers | we can just link to the spec review and say "please find us a solution" :-) | |
| 17:55:33 | aspiers | bbobrov: great thanks | |
| 17:55:33 | sean-k-mooney | bbobrov: aspiers but as i said the behavior that is being asserted for hard_limit. i.e. that it prevents the page form migrating is not documented in teh libvirt docs https://libvirt.org/formatdomain.html#elementsMemoryTuning | |
| 17:55:42 | aspiers | sean-k-mooney: ack | |
| 17:56:16 | aspiers | https://libvirt.org/formatdomain.html#elementsMemoryBacking seems to be the bit for pinning but if that just does an mlock() then apparently it's not enough | |
| 17:56:23 | aspiers | from what I heard earlier | |
| 17:56:36 | aspiers | although that seems strange to me, since then why would it be good enough for realtime? | |
| 17:56:57 | aspiers | sean-k-mooney: real-time also needs a hard guarantee, right? | |
| 17:57:06 | bbobrov | why have we decided to use memtune for pinning in the first place? | |
| 17:57:22 | aspiers | because that's what the libvirt/QEMU guys recommended initially | |
| 17:57:27 | bbobrov | if there is no direct indication that it actually pins memory | |
| 17:57:35 | aspiers | they said it did | |
| 17:57:40 | bbobrov | is there a mailing list thread about it? | |
| 17:57:47 | aspiers | yes, on the internal list | |
| 17:58:02 | sean-k-mooney | aspiers: not entirly. for minium latency variance yes but it wont break the feature if there was host page migration in the background but it may break SLAs | |
| 17:58:08 | bbobrov | eh, internal lists | |
| 17:58:20 | aspiers | bbobrov: our sev list | |
| 17:58:39 | bbobrov | yeah, i got it, still ba | |
| 17:58:41 | bbobrov | *bad | |
| 17:59:10 | sean-k-mooney | bbobrov: well this is what the amdese repo does https://github.com/AMDESE/AMDSEV/blob/master/xmls/sample.xml#L218 | |
| 17:59:17 | sean-k-mooney | it jsue set hard_limit | |
| 17:59:57 | sean-k-mooney | there is no other memory tuning or pinning but i would not have assumed that pinned any memory | |
| 18:00:43 | aspiers | "The value of the domain/memtune/hard_limit element will be used to setrlimit(RLIMIT_MEMLOCK, hard_limit) on the qemu process and in memory.limit_in_bytes setting of the processes memory controller (/sys/fs/cgroup/memory/machine.slice/machine-<vmid><vmname>.scope/memory.limit_in_bytes)" | |
| 18:00:56 | aspiers | that's from our libvirt guy | |
| 18:01:28 | aspiers | also https://libvirt.org/git/?p=libvirt.git;a=blob;f=src/qemu/qemu_domain.c;h=ba3fff607a93533b9b47956cc2cfa70237e7c041;hb=HEAD#l10134 | |
| 18:02:35 | gibi | mriedem: I've replied in https://review.openstack.org/#/c/640390/5/doc/source/admin/config-qos-min-bw.rst@98 | |
| 18:07:26 | bryan_stephenson | Accoring to https://libvirt.org/formatdomain.html#elementsMemoryBacking setting "locked" reserves/locks the physical memory. One use is for DMA, which would only be useful if the memory did not move. Do we think that memory might move locations even if it is locked? | |
| 18:07:49 | aspiers | bryan_stephenson: that's what Brijesh said on the call. he said mlock(2) was just a hint | |
| 18:08:02 | aspiers | the man page makes it sound like more than that, but I don't know | |
| 18:08:15 | bryan_stephenson | Then how does DMA not get goofed up? | |
| 18:08:33 | sean-k-mooney | bryan_stephenson we use hugepage memory for dma trasfer with dpdk so i think either hugepage or setting locked expclitly would be suffient | |
| 18:08:45 | aspiers | Honestly it's beyond my paygrade ;-) | |
| 18:09:20 | bryan_stephenson | So the "locked" just calls mlock() which may not be sufficient. Is that a correct understanding? | |
| 18:09:34 | aspiers | That's what I think I heard from Brijesh | |
| 18:12:06 | sean-k-mooney | aspiers: so looking at that funtion it runtrun Unlimited if the memory is locked to this would not caluatle a valide limit for realtime guests | |
| 18:12:08 | sean-k-mooney | https://libvirt.org/git/?p=libvirt.git;a=blob;f=src/qemu/qemu_domain.c;h=ba3fff607a93533b9b47956cc2cfa70237e7c041;hb=HEAD#l10049 | |
| 18:12:28 | openstackgerrit | Dan Smith proposed openstack/nova-specs master: Add request-filter-image-types spec https://review.openstack.org/644625 | |
| 18:14:03 | efried | aspiers: Want me to throw a procedural -2 on the bottom SEV patch so you don't have to keep chasing -Ws? | |
| 18:14:18 | openstackgerrit | Merged openstack/nova master: Remove additional policy configuration details from policy doc https://review.openstack.org/644423 | |
| 18:14:25 | openstackgerrit | Merged openstack/nova master: Clarify policy shortcomings in policy enforcement doc https://review.openstack.org/643960 | |
| 18:14:31 | sean-k-mooney | aspiers: so based on https://github.com/libvirt/libvirt/blob/0ec6343a069b21178d4580688a8380dbb6d76620/docs/news-2013.html.in#L1526 | |
| 18:14:56 | sean-k-mooney | aspiers: libvirt used to set RLIMIT_MEMLOCK when you set the locked value | |
| 18:14:59 | aspiers | efried: that would be nice thanks, but depends on which you think is the bottom patch ;-) | |
| 18:15:08 | efried | aspiers: https://review.openstack.org/#/c/633855/ ? | |
| 18:15:19 | sean-k-mooney | aspiers: and it still does https://github.com/libvirt/libvirt/blob/b6aacfc435ce3b7e2665ddf5a422d2153bca88b8/src/util/virprocess.c#L749 | |
| 18:15:49 | efried | aspiers: Done, lmk if there are any stragglers not in the series that you'd like similarly tagged. | |
| 18:15:57 | aspiers | efried: yeah, I think 644554 could maybe merge pre-Stein without doing harm | |
| 18:16:05 | efried | agree | |
| 18:16:05 | aspiers | sean-k-mooney: reading | |
| 18:16:21 | efried | aspiers: if you can get that efried guy to approve it. | |
| 18:16:25 | aspiers | :) | |
| 18:16:33 | aspiers | efried: sounds like a tall order | |
| 18:17:19 | efried | aspiers: I almost did a full rewrite of that method, but I'll settle for nixing backslashes. | |
| 18:18:11 | aspiers | hrm, nix them how? | |
| 18:18:57 | efried | aspiers: Use parens, or use an if, or... | |
| 18:19:29 | aspiers | OK | |
| 18:20:04 | efried | it's just a style rule that nova likes to follow. I'm pretty sure there was a time I didn't care, but now I'm brainwashed. | |
| 18:20:09 | aspiers | sure | |
| 18:22:50 | aspiers | sean-k-mooney: seems like a key question is whether mlock(2) really is enough for SEV or not | |
| 18:26:01 | aspiers | sean-k-mooney, bbobrov: I guess there's also #libvirt | |
| 18:30:16 | sean-k-mooney | aspiers: i think its calling https://linux.die.net/man/2/setrlimit but yes let use know what they determin | |
| 18:33:24 | sean-k-mooney | but as i said before im pretry sure hard_limit is not actully locking the memory its just limiting how much memory can be locked by an unprivalaged process | |
| 18:33:58 | aspiers | sean-k-mooney: yes, I think it needs the combination of setrlimit and mlock | |
| 18:35:18 | sean-k-mooney | which https://github.com/AMDESE/AMDSEV/blob/master/xmls/sample.xml does not do :) | |
| 18:35:44 | sean-k-mooney | anyway im going to grab something to eat | |
| 18:35:49 | sean-k-mooney | be back in a bit | |