[00:50:00] *** Joins: tomzawadzki (tzawadzk@nat/intel/x-lzvbdfywiemebwqt) [00:53:23] *** Joins: gila (~gila@5ED4FE92.cm-7-5d.dynamic.ziggo.nl) [00:58:11] *** Quits: ziyeyang_ (ziyeyang@nat/intel/x-cxqvmsvnfihuncey) (Quit: Leaving) [00:59:59] *** Quits: changpeng (changpeng@nat/intel/x-ilownhqcrsnrsyxa) (Quit: Leaving) [03:10:55] *** Quits: gila (~gila@5ED4FE92.cm-7-5d.dynamic.ziggo.nl) (Quit: My Mac Pro has gone to sleep. ZZZzzz…) [03:30:02] *** Joins: gila (~gila@5ED4FE92.cm-7-5d.dynamic.ziggo.nl) [05:11:50] *** Quits: gila (~gila@5ED4FE92.cm-7-5d.dynamic.ziggo.nl) (Ping timeout: 255 seconds) [05:12:28] *** Joins: gila (~gila@ec2-54-88-161-49.compute-1.amazonaws.com) [06:07:51] *** Joins: johnmeneghini (~johnmeneg@pool-96-252-112-122.bstnma.fios.verizon.net) [08:53:38] *** Quits: gila (~gila@ec2-54-88-161-49.compute-1.amazonaws.com) (Quit: My Mac Pro has gone to sleep. ZZZzzz…) [09:12:08] *** Joins: gila (~gila@ec2-54-88-161-49.compute-1.amazonaws.com) [09:16:55] *** Quits: sethhowe_ (~sethhowe@192.55.55.41) (Remote host closed the connection) [09:17:07] *** Joins: sethhowe (~sethhowe@134.134.139.72) [09:51:31] *** Quits: tomzawadzki (tzawadzk@nat/intel/x-lzvbdfywiemebwqt) (Ping timeout: 246 seconds) [09:58:48] bwalker, wanna chat about the last set of comments on https://review.gerrithub.io/#/c/362847/? [09:59:35] one sec need coffee [09:59:55] always :) [10:00:04] do you think if I expensed the equipment to get an intravenous caffeine drip that Nate would approve? [10:00:44] yes! Nate is good with anything IV related [10:04:42] ok I'm here [10:05:35] cool, take a look at my last comments on your comments and lets chat here to speed up the back and forth if that works, let me know when you're ready [10:09:52] ok, so bulk of the open discussion looks like it was around the first paragraph [10:10:33] cool, so anything on the others first or are you OK with those as is per my last set of comments. Lets do the easy ones first [10:11:12] well the GerritHub vs GitHub thing [10:11:16] I don't have a strong opinion [10:11:22] but GitHub certainly has a better CDN network [10:11:27] so people will get better download speeds on that [10:12:10] yeah, I understand that for sure. You see the thing I don't think is great about that though right - get someone to setup a repo with gthub and then make them setup another one if they devcide they want to participate. [10:12:29] Would rather make it easier to contribute than easier to look at [10:12:39] sure - but it's one command to add a new remote [10:12:47] true [10:12:54] in fact, both Daniel and I still have our main repositories pointed at GitHub for performance [10:13:02] and we push to GerritHub to create reviews only [10:13:09] hmmm.... [10:13:14] you guys are so advanced :) [10:13:21] *** Quits: gila (~gila@ec2-54-88-161-49.compute-1.amazonaws.com) (Quit: My Mac Pro has gone to sleep. ZZZzzz…) [10:13:25] I agree that we don't wan tot make it too complicated [10:13:48] can you clone from GerritHub without an account? [10:14:18] Hmmm, not sure [10:14:27] I'll check [10:15:19] looks like you can, I'll try now [10:15:40] I assume you can [10:15:47] yeah you can [10:15:59] the real annoyance is that you have to install the commit hook, generate an HTTPS password, etc. [10:16:01] to push a review [10:16:09] but you don't have to do that just to clone [10:17:06] yeah there are pros and cons to both approaches. one is easier but doesn't promote contribution 'as much' and the other is super easier but then adds a step to become a real participant [10:17:20] hmmm. [10:17:31] I don't feel strongly one way or the other, so maybe drv and jimharris can weigh in [10:17:44] I'm thinking we need to list both at this point but do so in a way that makes it super clear what the differences are [10:18:07] let me try that in the README patch and you guys can take another look. I'l also make the other small updates and push [10:18:09] I think a guide where you start off using GitHub quickly, but then have a guide that further sets you up to submit a patch [10:18:12] is probably the right path forward [10:18:13] I think we can keep the "how to contribute" stuff separate - make the front page readme as concise as possbile for a drive-by user [10:19:26] yeah, that's what I'm thinking for the quickstart too but let me try and list both methds and if its gets too ugly just go with github and maybe a small note that indicates there are a few more steps to contribute either there inline or in the advanced walk through [10:19:51] you'll see if today, have a Dr appt I have to leave for shortly... thanks guys [10:21:21] can you separate your patch into 1) changes to the quick start instructions 2) changes to the top level overview paragraph? [10:21:39] because there is a whole separate discussion about that intro paragraph that we haven't had yet [10:21:42] hey for "office hours" even now when we may not have anyone "extra" here, do you think it makes sense for one of you guys to kick it off with a few short notes on what the latest big things are that landed in master and what, if any, the upcoming major priorities are for the maintainers and see if there's any feedback? [10:22:19] bwalker, yeah I can do that. I'll just update the current one to leave the opening paragraph mostly alone and then tweak that in a subsequent patch if we deem needed [10:22:37] I can type out a summary [10:22:43] if I can remember what we're doing [10:22:48] LOL [10:23:02] it's monday morning, I don't even remember how to C yet [10:23:31] maybe shoot a note out too to let everyone know we'll have a small opening agenda item for those that want to tune in and see what's hot [10:23:42] yeah I'll link the IRC log on the mailing list or something [10:23:44] (and whats not) [10:23:53] rock n roll [10:24:13] we merged the VTune integration, some prep work for vhost-blk, and some gcc 7 warning fixes [10:24:18] I think that's the major stuff since last week [10:24:43] cool, did the vtune integration come from someone here or on the vtune team? [10:24:45] we're working on the intermittent failures in the continuous integration system - lots of work going into the vhost test suite righ tnow [10:24:53] the VTune team [10:25:02] Roman, who presented at the summit [10:25:08] excellent [10:25:10] https://github.com/spdk/spdk/commit/5712088a55e9b93361b5f09790555635650ed063 [10:25:44] this probably needs a changelog entry (we should do better at writing up stuff in the changelog as we add it) [10:25:47] is anyone out there, that we know of, making use of vtune in their use of SPDK right now? [10:26:02] not yet, because it needs VTune changes [10:26:06] and they haven't been released yet [10:26:06] *** Joins: gila (~gila@5ED4FE92.cm-7-5d.dynamic.ziggo.nl) [10:26:11] they'll be in the next VTune release [10:27:08] ahhh, good to know. thx [10:28:26] I've been pushing a ton of changes into the bdev layer [10:28:47] how much does one change weigh? [10:28:55] :) [10:29:10] my goals are: 1) Remove all direct DPDK calls (done as of this week), 2) Add really robust reset handling [10:29:21] 2000 lbs / # of changes I made [10:29:35] and then after (2) we should work towards really, really robust reset handling I think [10:29:54] it can't ever be too robust [10:30:10] *** Quits: gila (~gila@5ED4FE92.cm-7-5d.dynamic.ziggo.nl) (Ping timeout: 240 seconds) [10:30:15] 3) Add zero-copy up and down the stack [10:30:15] just keep adding a "really" with each release and call it CQI [10:30:34] on that one, how close are we now? [10:30:37] 4) Separate the bdev library from the SPDK event framework [10:30:55] 3 has a long way to go - it's kind of close for reads, but not there for writes [10:31:32] 5) incorporate the bdev library into a real application that doesn't utilize the SPDK event framework. For this, I'm going to just write an fio_plugin [10:31:45] #5 is sort of proving that I did 1-4 correctly [10:32:12] good list, thanks [10:32:35] then after I'm happy with bdev, I move back to the NVMe-oF target [10:32:47] which I sent out that mailing list email a month ago or so [10:32:51] about the new threading model [10:33:03] I just had to solve all of this bdev stuff before I work on that [10:33:30] oh, the Fujitsu guys also added support for NVMe passthru commands to the bdev library last week [10:34:28] nice [10:34:58] the guys from NetApp have some patches out to make some improvements to the logging and tracing facilities [10:35:12] they submitted last week - need code reviews [10:37:11] side question: how do I put something in italics in github readme markdown? [10:37:25] *italics* [10:37:28] **bold** [10:38:00] how confusing :) [10:40:12] *** Joins: gila (~gila@ec2-54-88-161-49.compute-1.amazonaws.com) [10:49:09] FYI readme patch updated... [11:23:02] *** Quits: gila (~gila@ec2-54-88-161-49.compute-1.amazonaws.com) (Quit: My Mac Pro has gone to sleep. ZZZzzz…) [11:47:46] *** Joins: gila (~gila@5ED4FE92.cm-7-5d.dynamic.ziggo.nl) [11:50:25] *** Joins: gila_ (~gila@ec2-54-88-161-49.compute-1.amazonaws.com) [11:51:50] *** Quits: gila (~gila@5ED4FE92.cm-7-5d.dynamic.ziggo.nl) (Ping timeout: 240 seconds) [11:55:12] *** Quits: gila_ (~gila@ec2-54-88-161-49.compute-1.amazonaws.com) (Ping timeout: 260 seconds) [11:57:06] *** Joins: gila (~gila@ec2-54-88-161-49.compute-1.amazonaws.com) [13:54:47] *** Quits: sethhowe (~sethhowe@134.134.139.72) (Remote host closed the connection) [14:23:03] drv: i added a comment to your test/stub review [14:23:10] https://review.gerrithub.io/#/c/365053/ [14:23:27] ok, I'll respin it [14:28:46] for the nvme socket allocation patch - do you have any data showing the performance improvement? [14:30:14] no, I didn't attempt to measure it [14:30:33] vishal has run a bunch of tests around this and we've seen negligible differences [14:30:38] I was going through our old gerrit patches and trying to port anything over that looked still important [14:30:53] and that was a quick hack based on Vishal's patch to make qpair memory allocation fully configurable [14:31:05] for SQ and PRP data, it's basically moot where the memory is located since the device will end up pulling data from CPU cache [14:31:42] I'm OK with just dropping it if it doesn't make any difference, although if it doesn't cause any negative effects, it seems simple enough [14:32:06] for CQ it is a bit better to allocate local to the device (rather than the CPU using the queue) [14:32:25] well I just think the commit message probably should be updated to reflect this - i can take a crack at that if you want [14:32:52] sure [14:33:58] what if there is no memory available on the specified socket id? [14:34:22] the new patch will make this more strict [14:35:32] rte_malloc_socket() falls back to other sockets if it can't find free memory on the requested socket [14:43:20] ah - I didn't know that [14:43:46] I wasn't sure either, but I just checked [14:45:54] for SQ and PRP lists, it is actually probably better to use memory local to the CPU core using the queue - that way when the cacheline gets flushed it gets flushed to the local socket instead of the remote socket [14:46:27] it's negligible [14:50:13] I think you pay the cost of the cross socket either way [14:50:25] either your CPU has to write or poll cross-socket [14:50:28] or the device does [14:51:10] I would actually have expected the CQ on the CPU socket to be better [14:51:12] with polling [14:51:34] and for submit I would expect essentially no difference [14:51:52] I guess you can't DDIO across sockets [14:52:17] so I don't know if a cross-socket read to CPU cache is better or worse than a read from main memory [14:55:00] cross-socket to CPU cache is better [14:56:27] drv: i updated the comment [14:56:34] or rather the commit message [14:57:50] ok, do we want to modify it to just do the CQ? or leave it as-is? [14:59:24] well i think it's ok to change it - it at least avoids the worst possible case which is device and polling core are on socket 1 but we allocate memory from socket 0 [15:00:27] performance wise the difference is negligible but that just seems wrong to allocate the memory from a different socket than both the app and the device [15:00:31] yeah [15:00:52] it looks like if you use rte_malloc() or leave socket id set to _ANY, DPDK will default to allocating from the calling thread's socket [15:01:30] so if you create the I/O queue from the correct socket, it would already be doing the right thing [15:02:00] well then maybe we just leave it as-is [15:02:19] the only case where it will make any difference is if you are creating it from a different socket than the device is on [15:02:28] yeah, leaving it alone is probably reasonable [15:02:51] bdev_nvme will already do the right thing since it creates the io_channel on the calling thread [15:03:25] well, "right thing" meaning allocate the queue memory on the CPU thread's socket, not necessarily the device's socket [16:15:43] *** Joins: ziyeyang_ (~ziyeyang@192.55.54.38) [16:16:10] *** Quits: ziyeyang_ (~ziyeyang@192.55.54.38) (Client Quit) [17:48:59] if I'm following correctly yeah I'd say alloc both SQ & CQ from thread socket memory even if one doesn't show as much benefit as the other, just better mojo :) [18:53:57] *** Joins: kon (cf8c2b51@gateway/web/freenode/ip.207.140.43.81) [19:04:16] *** kon is now known as konv81 [19:48:46] *** Quits: johnmeneghini (~johnmeneg@pool-96-252-112-122.bstnma.fios.verizon.net) (Quit: Leaving.) [23:35:55] *** Joins: tsuyoshi (b42b2067@gateway/web/freenode/ip.180.43.32.103)