[01:21:22] *** Joins: tomzawadzki (~tomzawadz@192.55.54.42) [05:14:02] *** Joins: ziyeyang_ (~ziyeyang@134.134.139.72) [05:16:30] *** Quits: ziyeyang_ (~ziyeyang@134.134.139.72) (Client Quit) [05:39:06] *** Joins: Param (~Param@106.51.65.217) [06:05:11] *** Quits: Param (~Param@106.51.65.217) (Quit: Param) [07:09:18] *** Joins: lhodev (~Adium@inet-hqmc03-o.oracle.com) [07:09:26] *** Parts: lhodev (~Adium@inet-hqmc03-o.oracle.com) () [08:56:44] *** Joins: Param (~Param@157.49.14.112) [09:08:46] *** Quits: tomzawadzki (~tomzawadz@192.55.54.42) (Ping timeout: 264 seconds) [10:25:21] *** Quits: Param (~Param@157.49.14.112) (Read error: Connection reset by peer) [10:52:58] *** Joins: Param (~Param@157.49.14.112) [10:55:43] *** Joins: felipef (~felipef@62.254.189.133) [11:29:42] drv: can you review https://review.gerrithub.io/#/c/388040/? then we can get that one in and more importantly the spdk_for_each_thread one after it [11:32:46] jimharris: I responded to that reset patch [11:33:10] no idea how people implement low level storage services in user space on top of aio [11:33:16] it just doesn't give you the control you need [11:33:39] Hi All, Today I tried creating 8 Target nodes in SPDK application and Tried running fio in parallel .There were two scenrios tried. With one core given to SPDK no problem it worked fine. When two cores had been given, found that after sometime there is new connection from initiator and the target tried to drop the old connection. When trying to drop the old connection conn->shutdown_timer is registered and there is an assert saying attempted reuse of poller.. [11:35:40] hi Param - can you describe the backing storage for each of these 8 target nodes? [11:36:22] there were 8 bdevs created.. [11:36:38] each target having one lun [11:36:55] what kind of bdev? malloc? [11:37:36] its bdev. volume basically [11:38:27] there are many different types of bdevs - malloc, nvme, rbd, null - knowing which one you are using is important to help debug the problem [11:39:07] (that is not the exhaustive list of bdev types - there are several more) [11:39:30] actually we are not using any of it. We have a separate type of bdev. We have our own bdev operations defined.. [11:40:33] ok - what happens if you use your same scenario with 8 target nodes and 2 (or more) cores with one of the existing SPDK bdev modules like malloc? [11:41:06] this will help us understand if the problem is with the iSCSI target or the new bdev module [11:41:42] I did not try that actually. Will it depend on the lower layer? [11:42:32] it could - for example, if your new bdev module does not correctly create new channels for each thread, you could easily see problems when running on two cores but it's fine on just one core [11:43:07] how do you manage I/O in your bdev module coming from different threads? do you have an io_channel abstraction for your bdev module? [11:43:17] the bdev/nvme and bdev/aio have good examples for how to plumb this [11:45:37] yes ..we do similar to that. [11:47:21] can you post your bdev module (or portion of it) so we can help review it? [11:51:36] bwalker: posted another comment on the reset patch [11:52:05] I'm concerned that continuously doing spdk_for_each_channel will keep the pollers from running [12:10:33] *** Quits: Param (~Param@157.49.14.112) (Quit: Param) [12:22:47] bwalker: https://review.gerrithub.io/#/c/388040/ needs to be rebased [12:32:58] drv: regarding https://review.gerrithub.io/c/388765/7/include/spdk/barrier.h - are doxygen comments for these defines even required? [12:34:12] the code would be probably more readable if I e.g. included all PPC barrier variants within a single #ifdef __PPC64__ [12:35:01] but then I would have to copy-paste the doxygen for each barrier on each architecture [12:35:09] yeah, we should probably reorganize those to have one section per arch, but it's still nice to have a Doxygen doc for each barrier type [12:35:40] Doxygen doesn't define any of the per-arch macros, so it's currently using the empty #define in the #else case for each one [12:42:25] drv, bwalker: i took care of the 388040 rebase [12:51:21] jimharris: the first patch needs your review again after the rebase: https://review.gerrithub.io/#/c/388040/ [13:12:20] jimharris: I don't understand how constant events block pollers [13:12:31] the reactor loop always processes one poller per iteration [13:13:36] and I don't see another easy way to implement it - the threads all have to coordinate until no more I/O is outstanding somehow [13:14:07] so you can send messages from the poller if it is in a reset state once the I/O count reaches 0 [13:14:24] but that message has to go back to one central place, and that one central place has to like count or something [13:14:33] so it can decrement back to 0 when every channel has reached 0 I/O [13:14:57] but the problem is you don't know what the count is - counting the number of threads that have a channel is never actually done [13:15:04] *** Quits: felipef (~felipef@62.254.189.133) (Quit: Leaving...) [13:15:05] it just loops through the linked list one by one [13:16:02] hmmm - you're right - I think we used to continue the loop if there were events [13:16:55] running that function over and over again just seems wrong to me - a lot of wasted cycles [13:17:05] I don't see another easy way to do it [13:17:25] I see hard ways to do it that are probably more efficient in the long term [13:18:01] it doesn't matter what the count is exactly - just whether it is zero or not (across all channels) [13:18:37] but how do you keep asking if it is 0 [13:19:10] you don't have to necessarily chain call spdk_for_each_channel - you could register a poller to call it every 500ms or something [13:19:47] but you have to pass a message to each channel to add up the outstanding I/O at some point periodically [13:19:51] i'm not understanding why we can't use the mechanism that cunyinch already has there - each channel, when it gets to zero, issues the for_each_channel call [13:20:06] then all of those for_each_channel chains are racing one another [13:20:41] and it's totally possible that two will get to the 'done' callback and complete the I/O [13:20:42] but we can protect against the race with an atomic comp/exch [13:20:56] and further, spdk_bdev_io_complete is getting called from the wrong thread [13:20:58] the exact way it is implemented now is not right - there is the race [13:21:16] spdk_for_each_channel calls the completion callback on the thread you initiated the iteration on [13:21:21] but it gets iterated from every poller [13:21:24] initiated* [13:21:34] none of this matters - doing a poller every 1ms would be perfect [13:22:07] yeah on reset he can just start a poller, when it expires do spdk_for_each_channel [13:22:16] when that finishes, if there are I/O outstanding, start the poller again [13:22:18] repeat [13:22:19] that was my biggest concern was all of the extra cycles to send those events over and over [13:23:02] 1ms on a reset is probably aggressive [13:23:28] 1ms would be in the noise I'd think - but we can make it more [13:23:32] but as long as start the poller only from the for_each done callback, it won't matter [13:24:00] if the system is very busy and it takes awhile for the events to propagate that's fine with that algorithm - the delay on starts from when the previous iteration stops [13:24:53] the generic bdev code already guarantees only one reset at a time, right? [13:24:59] correct [13:25:06] so we can put the poller in that fdisk structure [13:26:30] ok - are you summarizing this in the gerrit review? if not I'll do it [13:26:41] I'm happy to let you do it :) [13:29:18] done [14:15:43] drv: scan-build doesn't like the ioat/verify patch: https://ci.spdk.io/builds/review/2b9a8f03db25812aa8d49002a1955a13f317a416.1511902141/ [14:25:29] oh yeah, missing a free [14:33:00] "lwsync [...] provides the same ordering function as the sync instruction, except that a load caused by an instruction following the lwsync may be performed before a store caused by an instruction that precedes the lwsync," [14:33:02] from the ppc manual [14:33:10] after reading that, now I don't know what lwsync does at all [14:33:45] I guess it just guarantees the other way around - stores after an lwsync will execute after reads before the lwsync [14:33:58] s/reads/loads [14:35:44] now I think spdk_smp_rmb needs to be a sync on ppc [14:36:43] and I have no idea why spdk_smp_mb needs to be an mfence on x86_64 [14:38:16] i think we should just keep consistency with DPDK for now (to unblock darsto) and then figure out if we should deviate as a later patch (if we think DPDK is wrong) [14:38:55] I think DPDK is too heavy-handed on x86 here, but erring in that direction never introduces bugs [14:39:12] I'm happy to let him proceed [14:46:19] oh I get it now - ok the patch is right [14:46:40] the key is in clearly defining what mb, rmb, and wmb actually mean [14:50:30] I was thinking rmb meant loads couldn't be reordered past the barrier with any other instruction [14:50:33] but it's weaker than that [14:50:43] rmb just means loads can't be reordered past the barrier with other loads [14:50:54] but stores can get reordered all you want [14:52:58] I'm cool with the patch [14:53:03] I didn't check the ARM instructions [15:50:28] *** Joins: Shuhei (caf6fc61@gateway/web/freenode/ip.202.246.252.97) [15:53:21] Hi Daniel, Jim, I have updated https://review.gerrithub.io/#/c/385168/ but still failed. To be honest, I have tried to but not completed to run the whole auto test yet. Can I check the log of the autotest for specific patch? [15:53:45] Now I can know just failure or success. [15:55:29] hi Shuhei [15:55:42] the CI system links to logs in the comments of the review [15:55:52] https://ci.spdk.io/builds/review/2a742d7bb46f48073e00d4dbce7fb4bdd80e289e.1511885531/ [15:56:06] see at the bottom - under SPDK Automated Test System - it shows this link that bwalker just posted [15:56:40] Thanks so much, I'll check please wait. [15:56:42] sometimes there is a delay until you can see the results [15:57:50] there shouldn't be a delay anymore - the rsync happens before the reply is posted [15:58:18] Thanks, I clicked https://ci.spdk.io/builds/review/2a742d7bb46f48073e00d4dbce7fb4bdd80e289e.1511885531/ but I did not understand how to look. but now I think I understood it. [15:58:27] it looks like ./test/iscsi_tgt/filesystem/filesystem.sh is failing on both of the failed machines [15:59:16] you first look in "Agent Summary" section [15:59:21] it's a list of machines [15:59:33] the right side says pass or fail for each one [15:59:45] for instance, yours failed on the test agent "fedora-02" [15:59:54] and you click on the "fedora-02" link [16:00:04] and probably look at build.log [16:00:13] which is the output from autotest.sh on that system [16:00:23] looks like when iscsiadm does the discovery, it is not finding any portals [16:01:29] I think the bug is in iscsi.c - there is a case that assigns val = "ALL" for SessionType Discovery [16:01:43] which probably needs to be "ANY" after the change [16:02:28] actually, there is a second "ALL" in that file which probably needs to be changed as well [16:03:14] Thank you so much. [16:17:24] Hi All, Based on your feedback I checked the log of some of tests again and I noticed that I missed the failure of discovery. Thanks to your feedback I hope I will be able to do better test than before. Thanks again. [16:18:38] no problem, that's why we have tests :) [19:06:33] *** Joins: ziyeyang_ (~ziyeyang@192.55.54.39) [19:17:27] *** Quits: ziyeyang_ (~ziyeyang@192.55.54.39) (Ping timeout: 240 seconds) [19:22:21] *** Joins: ziyeyang_ (~ziyeyang@192.55.54.39) [19:23:32] *** Joins: ziyeyang__ (~ziyeyang@134.134.139.83) [19:23:32] *** Quits: ziyeyang_ (~ziyeyang@192.55.54.39) (Remote host closed the connection) [21:22:25] *** Quits: ziyeyang__ (~ziyeyang@134.134.139.83) (Ping timeout: 248 seconds) [21:33:42] *** Joins: ziyeyang_ (~ziyeyang@192.55.54.42) [21:34:33] *** Quits: ziyeyang_ (~ziyeyang@192.55.54.42) (Remote host closed the connection) [21:34:38] *** Joins: ziyeyang__ (~ziyeyang@134.134.139.82) [21:35:51] *** Joins: ziyeyang_ (~ziyeyang@192.55.54.42) [21:35:51] *** Quits: ziyeyang__ (~ziyeyang@134.134.139.82) (Remote host closed the connection) [21:44:51] *** Joins: Param (~Param@106.51.65.217) [22:15:19] Hi Jim, shuhei [22:15:39] Any conclusion for SCSI lun reset related to this patch: https://review.gerrithub.io/#/c/386817/ [22:27:40] Yes, I agree with you and think so too. [22:59:23] Besides, we need to also consider Cunyin's patch for AIO reset support [23:08:27] *** Quits: ziyeyang_ (~ziyeyang@192.55.54.42) (Ping timeout: 240 seconds) [23:14:55] *** Joins: ziyeyang_ (~ziyeyang@134.134.139.83) [23:16:11] *** Joins: waddy (~Daniel@32.97.110.56) [23:16:11] *** Quits: ziyeyang_ (~ziyeyang@134.134.139.83) (Remote host closed the connection) [23:16:20] *** Joins: ziyeyang_ (~ziyeyang@192.55.54.39) [23:19:42] *** Quits: dwaddington (Daniel@nat/ibm/x-uzleglwjtwzykmld) (Ping timeout: 258 seconds)