| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-03-10 | |||
| 12:49:32 | alex_xu | cool | |
| 12:50:58 | sean-k-mooney | im more or less ok with the patch other then that but still want to see if gibi is ok with defining the constanct before they are need | |
| 12:51:52 | alex_xu | yea, definitely need gibi's feedback | |
| 12:51:55 | sean-k-mooney | we may also need some extra test coverage for the vnic_types i havent fully review the last patch | |
| 13:07:21 | openstackgerrit | Lee Yarwood proposed openstack/nova master: WIP zuul: Replace grenade and nova-grenade-multinode with grenade-multinode https://review.opendev.org/c/openstack/nova/+/778885 | |
| 13:18:55 | sean-k-mooney | artom: so since your pci socket policy patch conflit with both my port numa patch and vdpa series we proably should set them up in a chain | |
| 13:19:28 | artom | sean-k-mooney, sure. Which one of yours is more likely to land at this point? | |
| 13:19:36 | artom | I guess put that one on top of my socket stuff? | |
| 13:19:37 | sean-k-mooney | vdpa obviously goes at the end since that is the lesast likely to be ready | |
| 13:19:45 | artom | Yeah, what I thought :) | |
| 13:19:51 | sean-k-mooney | ya the port numa one | |
| 13:20:15 | artom | I'm in the process of adding some patches to my series, I'll let you know when it's pushed? | |
| 13:20:54 | sean-k-mooney | would you be oke putting your patch on top of the port numa one? if i rebase it on top of master to pic up the patches of your that already merged | |
| 13:21:45 | sean-k-mooney | hum i guess yours already had 2 +2s buyt you need to change it for the multi socket thing | |
| 13:22:16 | sean-k-mooney | i dont think there are outstanding quesion left for mine. | |
| 13:22:24 | artom | sean-k-mooney, mind linking your series? | |
| 13:22:45 | sean-k-mooney | its one patch or the prot numa polices https://review.opendev.org/c/openstack/nova/+/773792 | |
| 13:22:47 | artom | Ah, there's only the one? https://review.opendev.org/c/openstack/nova/+/773792 | |
| 13:23:12 | sean-k-mooney | ya i should have a second for numa vswitch but i wont get to that until next cycle so just one for sriov | |
| 13:23:43 | artom | There aren't serious conflicts there, though | |
| 13:23:44 | sean-k-mooney | i was going to say maybe defer to stephenfin and gibi for order | |
| 13:23:57 | artom | Maybe some trivial "context code" in the tests | |
| 13:23:58 | sean-k-mooney | correct its minor | |
| 13:24:09 | sean-k-mooney | just dont wnat them to fight in the gate | |
| 13:24:21 | sean-k-mooney | sicne one of the two will merge first | |
| 13:24:30 | sean-k-mooney | and the other need to be respun | |
| 13:24:46 | artom | sean-k-mooney, lemme drop the kids off | |
| 13:24:51 | artom | Back in ~1 hour | |
| 13:26:09 | sean-k-mooney | sure ill wait for stephenfin or gibi to comment and ill do whatever they say | |
| 13:26:36 | sean-k-mooney | if they say put user first ill rebase on top of https://review.opendev.org/c/openstack/nova/+/772779 | |
| 13:43:03 | stephenfin | sean-k-mooney: what's the question? | |
| 13:44:47 | sean-k-mooney | stephenfin: artoms socket patch conflict with both my port numa and vdpa patches so i thin we need to stack them | |
| 13:45:05 | sean-k-mooney | stephenfin: question is port numa first or socket | |
| 13:45:11 | sean-k-mooney | with vpda last | |
| 13:45:21 | stephenfin | socket | |
| 13:45:23 | stephenfin | it's closest to merge | |
| 13:45:27 | stephenfin | in fact it's approved | |
| 13:45:40 | sean-k-mooney | right but ig cant merge since it broken | |
| 13:46:03 | stephenfin | huh? | |
| 13:46:04 | sean-k-mooney | artom need to fix suport form multipel socekt on one numa node | |
| 13:46:17 | sean-k-mooney | by making it a list and taking the first socket | |
| 13:46:26 | sean-k-mooney | to make it work in the gate vms | |
| 13:46:31 | stephenfin | can't that be a follow-up? | |
| 13:46:48 | stephenfin | as it's an edge condition | |
| 13:46:54 | sean-k-mooney | i gues but i also am not happy with the numa topology object being passed as a copy to the pci tracker | |
| 13:46:59 | stephenfin | is there real hardware with that design, out of curiosity? | |
| 13:47:11 | sean-k-mooney | we should have passed a dict of numa:socket | |
| 13:47:16 | stephenfin | yeah, neither was I but I couldn't find a better way | |
| 13:47:21 | stephenfin | that not much better | |
| 13:47:30 | sean-k-mooney | well we should have passed it to the function | |
| 13:47:31 | stephenfin | it's still information that can get out of sync | |
| 13:47:39 | sean-k-mooney | that cant | |
| 13:47:57 | sean-k-mooney | we dont supprot memory hotplug on the compute host | |
| 13:48:00 | stephenfin | and the NUMATopology can, for what we need? | |
| 13:48:09 | sean-k-mooney | so the numa node to sockt mapping wont get out of daty | |
| 13:48:13 | stephenfin | *what we need it for there? | |
| 13:48:24 | sean-k-mooney | stephenfin: its a copy so the pinned cpus and mempages wil be out of date | |
| 13:48:34 | stephenfin | but we don't use that, right? | |
| 13:48:56 | stephenfin | so we're simply passing too much information, some of which will get out-of-date | |
| 13:48:59 | sean-k-mooney | right but we dont have any comment stating that it will be incorrect and if we started trying to use it it could break | |
| 13:49:07 | sean-k-mooney | stephenfin: yep | |
| 13:49:14 | stephenfin | okay, fair | |
| 13:49:33 | stephenfin | I'm happy to look at alternatives so | |
| 13:49:47 | sean-k-mooney | i would have prefered not to store it in the class too and pass it to the filter evne if that involed passing it donw the layres but the too mugh datat it my main concnern | |
| 13:49:48 | stephenfin | but I wouldn't block on it, because of how close feature freeze is | |
| 13:50:07 | stephenfin | yeah, artom and I went over that in the review | |
| 13:50:12 | sean-k-mooney | ya so we coudl reduce the amount of data we pass in a folowup | |
| 13:50:20 | stephenfin | I believe him when he said it was too ugly | |
| 13:50:46 | sean-k-mooney | i just dislike the fact i need to change my mental model | |
| 13:51:03 | sean-k-mooney | the filter were not ment to use any data not passed into them | |
| 13:51:14 | sean-k-mooney | which is why they were class metods | |
| 13:51:17 | sean-k-mooney | to signal that | |
| 13:51:23 | stephenfin | did you see my other idea? | |
| 13:51:28 | stephenfin | from https://review.opendev.org/c/openstack/nova/+/774149/12 | |
| 13:51:33 | stephenfin | "Spitballing. How about adding 'host_socket' and maybe 'host_cell' attributes to 'InstanceNUMACell'? The latter isn't really necessary, since we use 'id' for that right now, but it would let us decouple this in the future (it's a confusing design, IMO). That former would let us avoid passing host NUMA information through to the PCI manager." | |
| 13:51:42 | sean-k-mooney | nope i only saw this after the patches were merged | |
| 13:52:04 | stephenfin | Well then, thoughts on that? Unnecessary duplication or potentially useful? | |
| 13:52:35 | stephenfin | host_cell should probably read host_node too | |
| 13:52:37 | sean-k-mooney | well i orginally wanted to put the socket in the extra_info dict in the pcidevice | |
| 13:52:54 | sean-k-mooney | so we did not need the lookup | |
| 13:53:02 | sean-k-mooney | but let me read that a few times | |
| 13:53:57 | sean-k-mooney | stephenfin: am adding host_socket to instanceNumaCell wont help unfortunetly | |
| 13:54:09 | sean-k-mooney | stephenfin: we dont have the socket of the pci device | |
| 13:54:25 | sean-k-mooney | if we did that and we added host_socket to the pci extra info then yes | |
| 13:54:39 | yonglihe | sean-k-mooney, alex_xu gibi: thanks, guys. | |
| 13:54:45 | stephenfin | oh, right | |
| 13:54:50 | stephenfin | darn | |
| 13:55:16 | stephenfin | then nvm | |
| 13:55:37 | sean-k-mooney | so what artom has merged in the first to pathces works its just got sharp edges | |
| 13:56:02 | sean-k-mooney | and if we reduced it to a dict of numa node to list of sockets | |
| 13:56:19 | sean-k-mooney | and then jsut did mapping[cell.id][0] | |
| 13:56:28 | sean-k-mooney | i think that would be enough | |
| 13:56:42 | sean-k-mooney | and we coudl do that in a follow up | |
| 13:57:24 | sean-k-mooney | so add a properyt or function to the host numa object to return that mappign dict and pass that in to the pci_tracker when we contuct it | |
| 13:57:48 | sean-k-mooney | that also get rid of some of the nested loops | |
| 13:58:24 | sean-k-mooney | stephenfin: ill rebase my port patch on his then but what do you think of ^ | |
| 13:58:42 | stephenfin | wfm | |
| 13:58:51 | stephenfin | but as a follow-up, IMO | |
| 13:58:58 | stephenfin | just to de-risk the whole enterprise | |
| 13:59:11 | sean-k-mooney | sure | |