07:47:07 XanClic joined the channel
08:24:41 kevin joined the channel
16:04:14 kevin: XanClic: Lohnt es sich, die "inflight writes counters" näher anzuschauen oder ist das eh -EMAGIC?
16:05:04 XanClic: kevin, mein Hauptproblem ist, dass es aussieht, als hätte Vladimir das als Proof-of-Concept runtergeschrieben, dachte „hey, das funktioniert ja gut“ und es dann einfach abgeschickt
16:05:07 kevin: Auf den ersten Blick sieht es viel komplizierter aus als die CoRwlock-Lösung, aber dafür wahrscheinlich auch ein bisschen gründlicher
16:05:25 XanClic: will sagen, da fehlt mir die Formalisierung, was das wirklich sein soll und warum das genau tut
16:05:56 XanClic: Ich hab die CoRwlock-Lösung jetzt nicht gesehen
16:06:28 kevin: Ich glaube, ich hätte das nicht so angegangen, dass Cluster bei Refcount 0 trotzdem nicht alloziert werden, sondern dass der Refcount erst dann runtergezählt wird, wenn tatsächlich niemand mehr den L2-Eintrag (oder wem auch immer der Refcount gehört hat) benutzt
16:06:30 XanClic: Also, wenns von der Performance her OK ist, bei jedem Schreibrequest für jeden Cluster zwei malloc()s und einen Eintrag in eine Hashtabelle zu machen (und das wieder weg), dann denke ich aber schon, dass das OK ist
16:06:49 XanClic: Ja, darüber diskutier ich gerade mit ihm
16:07:00 XanClic: Dass er nicht definiert hat, was jetzt ein “free cluster” sein soll
16:07:24 kevin: Aber das würde den Code eher noch länger machen, nur vielleicht mit ein bisschen klarerer Struktur
16:07:49 XanClic: Mir ist egal, was er sich aussucht, er soll sich nur was aussuchen und das dann auch klar implementieren ;)
16:08:02 kevin: Die CoRwlock-Lösung ist halt, dass Discard ein Writer sind und andere Operationen auf dem Cluster Reader und dann wird das einfach serialisiert
16:08:28 XanClic: Mag das halt nicht, dass er update_refcount() mit addend=0 aufruft, nur um das nachzuholen, was vorher aufgeschoben wurde
16:08:53 XanClic: Finde, da sollte er das eher in eine Funktion auslagern, deren klarer Zweck ist, dass sie aufgerufen wird, wenn ein Cluster gerade freigegeben wurde
16:08:57 kevin: Die CoRwlock kostet eventuell ein bisschen Parallelität, die der andere Ansatz erhalten kann, wenn man ständig discardet, aber dann hat man's auch verdient
16:08:58 XanClic: s/da /das /
16:09:06 XanClic: Schon
16:09:20 kevin: update_refcount() mit 0 kommt mir bekannt vor. Macht nicht der Snapshot-Code sowas? :-)
16:09:25 XanClic: Uuuh…
16:09:34 XanClic: .Auf dem Cluster? Man hätte dann einen Lock pro Cluster?
16:09:41 kevin: Nein, pro BDS
16:09:44 XanClic: Ah, OK
16:09:48 XanClic: Von mir aus geht auch das
16:09:59 kevin: Dümmste mögliche Lösung, die eigentlich tun müsste
16:10:04 XanClic: Das Problem ist halt, dass Host-Discards auch passieren können, wenn kein Guest-Discard kommt
16:10:25 XanClic: Wobei das Ergebnis vermutlich das gleiche ist, so oft werden Hostcluster ja nun auch nicht discarded
16:10:31 XanClic: *discardet (?)
16:10:34 XanClic: *verworfen
16:10:40 kevin: Ja, das ist der Teil, der an der anderen Lösung gründlicher sein müsste (hab den Code aber noch nicht wirklich angeschaut)
16:11:16 kevin: Mit internen Snapshots kriegt man die sogar recht häufig
16:11:36 kevin: Oder Kompression. Alles, was internes COW macht halt
16:11:43 XanClic: Das einzige, was mir an seiner Lösung wirklich grundsätzlich nicht gefällt, ist, dass sie halt wie gesagt jeden Cluster einzeln in diese Hashtabelle einträgt
16:12:15 XanClic: Und somit für den Standardfall von nur Schreibzugriffen ohne Allokationen/Freigebungen trotzdem Overhead verursacht
16:12:23 XanClic: Auch wenn er sagt, das sei ja kein Problem. *Schulterzuck*
16:12:31 kevin: Wer liest denn die Hashtabelle? Kommt das oft vor?
16:12:35 XanClic: Ne
16:12:40 XanClic: Nur bei Allokationen und beim Freigeben
16:12:48 XanClic: Na gut, wobei
16:13:02 XanClic: Oder beim Eintragen eben, damit ein bestehender Eintrag verwendet wird, aber da muss man dann zwei Schreibzugriffe auf einen Cluster haben
16:13:05 XanClic: Vermutlich auch nicht so häufig
16:13:46 kevin: Warum machen wir dann nicht eine Liste von aktuell laufenden Requests? Sollte das nicht schneller gehen als malloc/free für normale Requests?
16:13:52 XanClic: Die Alternative wäre vermutlich, eine Liste von Requests zu haben, wo einfach jeder Schreibzugriff was anhängt, dann würde das Iterieren eben teurer, aber immerhin müssten Schreibzugriffe nicht mehr für jeden einzelnen Cluster was eintragen
16:14:11 XanClic: Ist halt die Frage, was besser ist. Ich weiß es nicht.
16:14:48 XanClic: Da Allokation und Freigeben eh Mist für die Performance ist, könnte man sagen, da zu iterieren wär jetzt nicht so schlimm
16:14:57 XanClic: Ist dann aber unberechenbarer
16:15:46 kevin: Hm... Warum fixen wir eigentlich nur Writes? Können nicht Reads auch falsche Daten zurückliefern? Also weder die alten Daten noch Nullen, sondern was auch immer nachher alloziert worden ist?
16:16:05 XanClic: Ja, aber na ja
16:16:21 XanClic: Das passiert doch nur, wenn man was liest, was gleichzeitig discardet wird, oder?
16:16:29 kevin: Vielleicht müsste man das grundsätzlicher angehen, nicht nur ein get_cluster_offset(), sondern ein get/put-Paar
16:16:42 XanClic: Wobei, es gibt doch dieses Flag, dass man Discard benutzt, wenn garantiert wird, dass Nullen zurückkommen… Hm…
16:16:46 kevin: Dann kann man dort ordentlich zählen und muss das nicht an jeder Stelle extra machen
16:16:57 XanClic: (Wollte gerade sagen, wenn man was discardet, dann ist ja auch egal, was dann beim Lesen kommt)
16:17:12 kevin: Ist unter Umständen halt ein Information Leak
16:17:19 XanClic: Hm, stimmt
16:17:45 XanClic: Ja, schade.
16:17:59 XanClic: Warum ist eigentlich immer alles kaputt?
16:19:54 kevin: Hm. Hat ganz bestimmt nichts damit zu tun, dass der Code von uns kommt.
16:19:58 XanClic: :)
16:20:03 XanClic: Jedenfalls, wenn du get_cluster_offset() ein get/put machen möchtest, dann wär das ja ungefähr das, was Vladimir jetzt hat
16:20:03 kevin: Das war bestimmt Fabrice
16:20:34 XanClic: Dann würde man Cluster halt nur nicht als in-use markieren, wenn auf sie geschrieben wird, sondern grundsätzlich in get_cluster_offset()
16:21:12 kevin: Ja, es wäre halt ein Interface, das es schwer macht, es falsch zu benutzen
16:21:48 XanClic: > In the meanwhile Kevin dispelled my "big problems" in "[PATCH v2(RFC) 0/3] qcow2: fix parallel rewrite and discard", so probably next step would be to retry CoRwLock-based approach.
16:21:50 XanClic: oh nein
16:22:13 kevin: Meine größte Sorge mit zusätzlichen Datenstrukturen, die an der einen Stelle benutzt werden und an der anderen nicht, ist dass man irgendwelche relevanten Stellen vergisst (entweder geich oder wenn man sie später hinzufügt)
16:22:19 XanClic: Ja, klar
16:28:36 XanClic: Wobei ich eigentlich das gleiche Problem auch beim CoRwlock sehe, oder nicht?
16:29:02 XanClic: Genau an den Stellen, wo man das nehmen muss, muss man bei dem Hashtablezeug dann eben Einträge in die Hashtabelle schreiben
16:37:48 kevin: Ja, CoRwlock geht nicht die Wurzel an. Das klebt man halt drumrum und dann tut es hoffentlich. Meistens. Oder so.
16:38:12 XanClic: Dann wärs ja eigentlich noch besser, wenn man das in get_cluster_offset() hätte
16:38:13 kevin: Wenn ich die Wahl zwischen zwei Lösungen habe, die beide nicht die Wurzel angehen, dann nehm ich die einfachere
16:39:07 XanClic: Na ja, oder die bessere, was auch immer das heißen mag
16:39:12 kevin: Hm, wenn du get/put_cluster_offset hast, könntest du da drin auch die CoRwlock nehmen, denke ich, ja
16:39:21 XanClic: Oh, stimmt.
16:39:53 kevin: Das wäre vielleicht sogar halbwegs gründlich und trotzdem halbwegs einfach
16:40:02 XanClic: Kann ich nix gegen sagen
16:41:08 kevin: Den einzigen Grund, den ich dagegen sehen könnte, ist dass es Workloads mit parallelen Writes und Discards langsamer machen könnte
16:41:20 kevin: Soweit ich weiß, draint Linux die Queue für Discards...
16:41:35 kevin: Also eher nicht so relevant
16:41:39 XanClic: Ja, das ist halt die Frage
16:42:05 XanClic: Na ja, Vladimir meinte ja, dass es mit dem Compressed-Cache dann möglich ist, dass Discards reinkommen, während der Cache geflusht wird
16:42:18 kevin: Welcher Cache?
16:42:25 XanClic: Der Cache für komprimierte Cluster
16:42:25 kevin: L2-Metadaten?
16:42:32 kevin: Ach so, der Datencache
16:42:35 XanClic: Ja
16:42:41 XanClic: Aber weiß nicht, ob mir das jetzt relevant genug ist, um das als Performanceproblem zu sehen
16:43:07 XanClic: Dann ist das halt so, dass dann mal eins aufs andere warten muss
16:43:15 kevin: Jo
16:47:16 Guest1: Auch nicht schlecht
16:48:07 Guest1: Jetzt weiß ich wieder, warum ich die Nickchangezeit auf 10 s gestellt hab
16:48:14 Guest1: Damit ich in solchen Fällen Guest1 bekomme
16:48:20 Guest82: lol
16:48:52 Guest42: Och, der ist ja frei
16:49:01 Guest1: :)
16:49:10 Guest1337: hihi
16:49:25 Guest1F05: so
16:49:27 Guest42: Bäh, Tabkonflikt
16:49:32 Guest1: hehe
16:49:36 Guest1F05: meine übliche USB PID
16:49:44 Guest1F05: die linux tage haben angefangen
16:49:50 Guest1F05: und es gibt eine onlinewelt
16:49:57 Guest1F05: https://play.live.linux-tage.de
16:50:29 LittleFox: hm was war mein Nick? LittleFox oder LittleFox94?
16:51:44 Guest42: XanClic: Wenn wir was nicht pro BDS, sondern feiner machen wollte, frage ich mich gerade auch noch, ob die List von Allokationen (s->cluster_allocs und m->dependent_requests) da auch reinpasst, oder ob das fundamental was anderes ist
16:52:00 XanClic: Uääh
16:52:05 XanClic: Also das ist definitiv keine Frage für Freitagabend
16:52:28 Guest42: Da möchte ich jetzt nicht widersprechen *g*
22:18:09 Paddy joined the channel
22:47:39 Paddy: Tufel schreibt ja richtig komplizierten Code. Jetzt müsste man C können...