Date: Thu, 19 Aug 2010 13:29:25 +0400 From: pluknet <pluknet@gmail.com> To: John Baldwin <jhb@freebsd.org> Cc: Kostik Belousov <kostikbel@gmail.com>, FreeBSD Current <freebsd-current@freebsd.org> Subject: Re: LOR on nfs: vfs_vnops.c:301 kern_descrip.c:1580 Message-ID: <AANLkTikpC9REEQKNUUHtLDxSLB1MmiqKdgbJ0Ee7O%2BGJ@mail.gmail.com> In-Reply-To: <201008181607.52798.jhb@freebsd.org> References: <AANLkTimJ=d06D2z24QyRQ98zEa1Pemk4=vkNGLNiX90N@mail.gmail.com> <AANLkTi=V8bumpGKgi1aLXxhJkdKdpG1jfyrcXbMyc3Yw@mail.gmail.com> <AANLkTikwiTsiZtp5Zej8yDsR2b4%2B=TSL1oD-M_uRFqBd@mail.gmail.com> <201008181607.52798.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--001485ebae8876b28f048e29d13d Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable On 19 August 2010 00:07, John Baldwin <jhb@freebsd.org> wrote: > On Wednesday, August 18, 2010 3:17:56 pm pluknet wrote: >> On 18 August 2010 23:11, pluknet <pluknet@gmail.com> wrote: >> > On 18 August 2010 17:46, Kostik Belousov <kostikbel@gmail.com> wrote: >> >> On Wed, Aug 18, 2010 at 02:43:19PM +0400, pluknet wrote: >> >>> On 18 August 2010 12:07, pluknet <pluknet@gmail.com> wrote: >> >>> > On 17 August 2010 20:04, Kostik Belousov <kostikbel@gmail.com> wro= te: >> >>> > >> >>> >> >> >>> >> Also please take a note of the John' suggestion to use the taskqu= eue. >> >>> > >> >>> > I decided to go this road. Thank you both. >> >>> > Now I do nfs buildkernel survive and prepare some benchmark result= s. >> >>> > >> >>> >> >>> So, I modified the patch to defer proc_create() with taskqueue(9). >> >>> Below is `time make -j5 buildkernel WITHOUT_MODULES=3Dyes` perf. > evaluation. >> >>> Done on 4-way CPU on clean /usr/obj with /usr/src & /usr/obj both >> >>> nfs-mounted over 1Gbit LAN. >> >>> >> >>> clean old >> >>> 1137.985u 239.411s 7:42.15 298.0% =A0 =A0 =A0 6538+2133k 87+43388io = 226pf+0w >> >>> >> >>> clean new >> >>> 1134.755u 240.032s 7:41.25 298.0% =A0 =A0 =A0 6553+2133k 87+43367io = 224pf+0w >> >>> >> >>> Patch needs polishing, though it generally works. >> >>> Not sure if shep_chan (or whatever name it will get) needs locking. >> >> As I said yesterday, if several requests to create nfsiod coming one >> >> after another, you would loose all but the last. >> >> >> >> You should put the requests into the list, probably protected by >> >> nfs_iod_mtx. >> >> >> > >> > How about this patch? Still several things to ask. >> > 1) I used malloc instance w/ M_NOWAIT, since it's called with nfs_iod_= mtx > held. >> > 2) Probably busy/done gymnastics is a wrong mess. Your help is > appreciated. >> > 3) if (1) is fine, is it right to use fail: logic (i.e. set >> > NFSIOD_NOT_AVAILABLE) >> > on memory shortage? Not tested. >> > >> > There are debug printf() left intentionally to see how 3 contexts run > under load >> > to each other. I attached these messages as well if that makes sense. >> > >> >> Ah, yes. Sorry, forgot about that. > > Your task handler needs to run in a loop until the list of requests is em= pty. > If multiple threads call taskqueue_enqueue() before your task is schedule= d, > they will be consolidated down to a single call of your task handler. Thanks for your suggestions. So I converted nfs_nfsiodnew_tq() into loop, and as such I changed a cleanup SLIST_FOREACH() as well to free a bulk of (potentially consolidated) completed requests in one pass. kproc_create() is still out of the SLIST_FOREACH() to avoid calling it with nfs_iod_mtx held. > > - =A0 =A0 =A0 int error, i; > - =A0 =A0 =A0 int newiod; > + =A0 =A0 =A0 int i, newiod, error; > > You should sort these alphabetically if you are going to change this. =A0= I would > probably just leave it as-is. Err.. that's resulted after a set of changes. Thanks for noting that. > > Also, I'm not sure how this works as you don't actually wait for the task= to > complete. =A0If the taskqueue_enqueue() doesn't preempt, then you may rea= d > ni_error as zero before the kproc has actually been created and return su= ccess > even though an nfsiod hasn't been created. > I put taskqueue_drain() right after taskqueue_enqueue() to be in sync with task handler. That was part to think about, as I didn't find such a use pat= tern. It seems though that tasks are launched now strictly one-by-one, without visible parallelism (as far as debug printf()s don't overlap anymore). --=20 wbr, pluknet --001485ebae8876b28f048e29d13d Content-Type: application/octet-stream; name="nfsiod_tq_lock.2.diff" Content-Disposition: attachment; filename="nfsiod_tq_lock.2.diff" Content-Transfer-Encoding: base64 X-Attachment-Id: f_gd1esq641 SW5kZXg6IHN5cy9uZnNjbGllbnQvbmZzX3N1YnMuYwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBzeXMvbmZzY2xp ZW50L25mc19zdWJzLmMJKHJldmlzaW9uIDIxMTI3OSkKKysrIHN5cy9uZnNjbGllbnQvbmZzX3N1 YnMuYwkod29ya2luZyBjb3B5KQpAQCAtNTksNiArNTksNyBAQAogI2luY2x1ZGUgPHN5cy9zeXNl bnQuaD4KICNpbmNsdWRlIDxzeXMvc3lzY2FsbC5oPgogI2luY2x1ZGUgPHN5cy9zeXNwcm90by5o PgorI2luY2x1ZGUgPHN5cy90YXNrcXVldWUuaD4KIAogI2luY2x1ZGUgPHZtL3ZtLmg+CiAjaW5j bHVkZSA8dm0vdm1fb2JqZWN0Lmg+CkBAIC0xMTcsNiArMTE4LDcgQEAKIAogc3RydWN0IG5mc19i dWZxCW5mc19idWZxOwogc3RhdGljIHN0cnVjdCBtdHggbmZzX3hpZF9tdHg7CitzdHJ1Y3QgdGFz awluZnNfbmZzaW9kbmV3X3Rhc2s7CiAKIC8qCiAgKiBhbmQgdGhlIHJldmVyc2UgbWFwcGluZyBm cm9tIGdlbmVyaWMgdG8gVmVyc2lvbiAyIHByb2NlZHVyZSBudW1iZXJzCkBAIC0zNTQsNiArMzU2 LDcgQEAKIAkgKi8KIAltdHhfaW5pdCgmbmZzX2lvZF9tdHgsICJORlMgaW9kIGxvY2siLCBOVUxM LCBNVFhfREVGKTsKIAltdHhfaW5pdCgmbmZzX3hpZF9tdHgsICJORlMgeGlkIGxvY2siLCBOVUxM LCBNVFhfREVGKTsKKwlUQVNLX0lOSVQoJm5mc19uZnNpb2RuZXdfdGFzaywgMCwgbmZzX25mc2lv ZG5ld190cSwgTlVMTCk7CiAKIAluZnNfcGJ1Zl9mcmVlY250ID0gbnN3YnVmIC8gMiArIDE7CiAK SW5kZXg6IHN5cy9uZnNjbGllbnQvbmZzX25mc2lvZC5jCj09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIHN5cy9uZnNj bGllbnQvbmZzX25mc2lvZC5jCShyZXZpc2lvbiAyMTEyNzkpCisrKyBzeXMvbmZzY2xpZW50L25m c19uZnNpb2QuYwkod29ya2luZyBjb3B5KQpAQCAtNTksNiArNTksNyBAQAogI2luY2x1ZGUgPHN5 cy9mY250bC5oPgogI2luY2x1ZGUgPHN5cy9sb2NrZi5oPgogI2luY2x1ZGUgPHN5cy9tdXRleC5o PgorI2luY2x1ZGUgPHN5cy90YXNrcXVldWUuaD4KIAogI2luY2x1ZGUgPG5ldGluZXQvaW4uaD4K ICNpbmNsdWRlIDxuZXRpbmV0L3RjcC5oPgpAQCAtNzUsNiArNzYsMTcgQEAKIAogc3RhdGljIHZv aWQJbmZzc3ZjX2lvZCh2b2lkICopOwogCitzdHJ1Y3QgbmZzaW9kX3N0ciB7CisJU0xJU1RfRU5U UlkobmZzaW9kX3N0cikgbmlfbGlzdDsKKwlpbnQgKm5pX2luc3Q7CisJaW50IG5pX2lvZDsKKwlp bnQgbmlfZXJyb3I7CisJaW50IG5pX2J1c3k7CisJaW50IG5pX2RvbmU7Cit9Oworc3RhdGljIFNM SVNUX0hFQUQoLCBuZnNpb2Rfc3RyKSBuZnNpb2RoZWFkID0KKyAgICBTTElTVF9IRUFEX0lOSVRJ QUxJWkVSKG5mc2lvZGhlYWQpOworCiBzdGF0aWMgaW50IG5mc19hc3luY2RhZW1vbltORlNfTUFY QVNZTkNEQUVNT05dOwogCiBTWVNDVExfREVDTChfdmZzX25mcyk7CkBAIC0xNTksMTEgKzE3MSw0 MyBAQAogICAgIHNpemVvZiAobmZzX2lvZG1heCksIHN5c2N0bF9pb2RtYXgsICJJVSIsCiAgICAg Ik1heCBudW1iZXIgb2YgbmZzaW9kIGt0aHJlYWRzIik7CiAKK3ZvaWQKK25mc19uZnNpb2RuZXdf dHEoX191bnVzZWQgdm9pZCAqYXJnLCBpbnQgcGVuZGluZykKK3sKKwlzdHJ1Y3QgbmZzaW9kX3N0 ciAqbmlwOworCisJZm9yICg7OykgeworCQltdHhfbG9jaygmbmZzX2lvZF9tdHgpOworCQlTTElT VF9GT1JFQUNIKG5pcCwgJm5mc2lvZGhlYWQsIG5pX2xpc3QpIHsKKwkJCWlmIChuaXAtPm5pX2J1 c3kgPT0gMCkgeworCQkJCW5pcC0+bmlfYnVzeSA9IDE7CisJCQkJLyoKKwkJCQkgKiBTbyB3ZSBj YWxsIGtwcm9jX2NyZWF0ZSgpCisJCQkJICogYWZ0ZXIgZHJvcHBpbmcgbmZzX2lvZF9tdHggbG9j ay4KKwkJCQkgKi8KKwkJCQlicmVhazsKKwkJCX0KKwkJfQorCQltdHhfdW5sb2NrKCZuZnNfaW9k X210eCk7CisJCS8qIENvbXBsZXRlIHRoZSB0YXNrIGlmIHRoZSBsaXN0IG9mIHJlcXVlc3RzIGlz IGVtcHR5LiAqLworCQlpZiAobmlwID09IE5VTEwpCisJCQlicmVhazsKKy8vCQlLQVNTRVJUKG5p cCAhPSBOVUxMLCAoIm5mc19uZnNpb2RuZXdfdHE6IG5pcCBpcyBOVUxMIikpOworCQlwcmludGYo InRxOiBuaXA6ICVwXG4iLCBuaXApOworCQlwcmludGYoInRxOiBuaV9pbnN0OiAlcFxuIiwgbmlw LT5uaV9pbnN0KTsKKwkJcHJpbnRmKCJ0cTogbmlfaW9kOiAlZFxuIiwgbmlwLT5uaV9pb2QpOwor CQluaXAtPm5pX2Vycm9yID0ga3Byb2NfY3JlYXRlKG5mc3N2Y19pb2QsIG5pcC0+bmlfaW5zdCwg TlVMTCwKKwkJICAgIFJGSElHSFBJRCwgMCwgIm5mc2lvZCAlZCIsIG5pcC0+bmlfaW9kKTsKKwkJ bmlwLT5uaV9kb25lID0gMTsKKwl9Cit9CisKIGludAogbmZzX25mc2lvZG5ldyhpbnQgc2V0X2lv ZHdhbnQpCiB7CiAJaW50IGVycm9yLCBpOwogCWludCBuZXdpb2Q7CisJc3RydWN0IG5mc2lvZF9z dHIgKm5pcCwgKm5pcF90ZW1wOwogCiAJaWYgKG5mc19udW1hc3luYyA+PSBuZnNfaW9kbWF4KQog CQlyZXR1cm4gKC0xKTsKQEAgLTE3OCwxNyArMjIyLDM1IEBACiAJCXJldHVybiAoLTEpOwogCWlm IChzZXRfaW9kd2FudCA+IDApCiAJCW5mc19pb2R3YW50W2ldID0gTkZTSU9EX0NSRUFURURfRk9S X05GU19BU1lOQ0lPOworCW5pcCA9IG1hbGxvYyhzaXplb2YoKm5pcCksIE1fVEVNUCwgTV9OT1dB SVQgfCBNX1pFUk8pOworCWlmIChuaXAgPT0gTlVMTCkKKwkJZ290byBmYWlsOworCW5pcC0+bmlf aW5zdCA9IG5mc19hc3luY2RhZW1vbiArIGk7CisJbmlwLT5uaV9pb2QgPSBuZXdpb2Q7CisJU0xJ U1RfSU5TRVJUX0hFQUQoJm5mc2lvZGhlYWQsIG5pcCwgbmlfbGlzdCk7CiAJbXR4X3VubG9jaygm bmZzX2lvZF9tdHgpOwotCWVycm9yID0ga3Byb2NfY3JlYXRlKG5mc3N2Y19pb2QsIG5mc19hc3lu Y2RhZW1vbiArIGksIE5VTEwsIFJGSElHSFBJRCwKLQkgICAgMCwgIm5mc2lvZCAlZCIsIG5ld2lv ZCk7CisJcHJpbnRmKCJuZXc6IG5pcDogJXBcbiIsIG5pcCk7CisJcHJpbnRmKCJuZXc6IG5pX2lu c3Q6ICVwXG4iLCBuaXAtPm5pX2luc3QpOworCXByaW50ZigibmV3OiBuaV9pb2Q6ICVkXG4iLCBu aXAtPm5pX2lvZCk7CisJdGFza3F1ZXVlX2VucXVldWUodGFza3F1ZXVlX3RocmVhZCwgJm5mc19u ZnNpb2RuZXdfdGFzayk7CisJdGFza3F1ZXVlX2RyYWluKHRhc2txdWV1ZV90aHJlYWQsICZuZnNf bmZzaW9kbmV3X3Rhc2spOwogCW10eF9sb2NrKCZuZnNfaW9kX210eCk7Ci0JaWYgKGVycm9yKSB7 Ci0JCWlmIChzZXRfaW9kd2FudCA+IDApCi0JCQluZnNfaW9kd2FudFtpXSA9IE5GU0lPRF9OT1Rf QVZBSUxBQkxFOwotCQlyZXR1cm4gKC0xKTsKKwllcnJvciA9IG5pcC0+bmlfZXJyb3I7CisJU0xJ U1RfRk9SRUFDSF9TQUZFKG5pcCwgJm5mc2lvZGhlYWQsIG5pX2xpc3QsIG5pcF90ZW1wKSB7CisJ CWlmIChuaXAtPm5pX2J1c3kgIT0gMCAmJiBuaXAtPm5pX2RvbmUgIT0gMCkgeworCQkJU0xJU1Rf UkVNT1ZFKCZuZnNpb2RoZWFkLCBuaXAsIG5mc2lvZF9zdHIsIG5pX2xpc3QpOworCQkJZnJlZShu aXAsIE1fVEVNUCk7CisvLwkJCWJyZWFrOworCQl9CiAJfQorCWlmIChlcnJvcikKKwkJZ290byBm YWlsOwogCW5mc19udW1hc3luYysrOwogCXJldHVybiAobmV3aW9kKTsKK2ZhaWw6CisJaWYgKHNl dF9pb2R3YW50ID4gMCkKKwkJbmZzX2lvZHdhbnRbaV0gPSBORlNJT0RfTk9UX0FWQUlMQUJMRTsK KwlyZXR1cm4gKC0xKTsKIH0KIAogc3RhdGljIHZvaWQKQEAgLTIzMSw2ICsyOTMsOCBAQAogCiAJ bXR4X2xvY2soJm5mc19pb2RfbXR4KTsKIAlteWlvZCA9IChpbnQgKilpbnN0YW5jZSAtIG5mc19h c3luY2RhZW1vbjsKKwlwcmludGYoImluc3RhbmNlOiAlcFxuIiwgaW5zdGFuY2UpOworCXByaW50 ZigibXlpb2Q6ICVkXG4iLCBteWlvZCk7CiAJLyoKIAkgKiBNYWluIGxvb3AKIAkgKi8KSW5kZXg6 IHN5cy9uZnNjbGllbnQvbmZzLmgKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gc3lzL25mc2NsaWVudC9uZnMuaAko cmV2aXNpb24gMjExMjc5KQorKysgc3lzL25mc2NsaWVudC9uZnMuaAkod29ya2luZyBjb3B5KQpA QCAtMTI1LDYgKzEyNSw3IEBACiAKIGV4dGVybiBzdHJ1Y3QgbmZzc3RhdHMgbmZzc3RhdHM7CiBl eHRlcm4gc3RydWN0IG10eCBuZnNfaW9kX210eDsKK2V4dGVybiBzdHJ1Y3QgdGFzayBuZnNfbmZz aW9kbmV3X3Rhc2s7CiAKIGV4dGVybiBpbnQgbmZzX251bWFzeW5jOwogZXh0ZXJuIHVuc2lnbmVk IGludCBuZnNfaW9kbWF4OwpAQCAtMjUzLDYgKzI1NCw3IEBACiAJICAgIHN0cnVjdCB1Y3JlZCAq Y3JlZCwgc3RydWN0IHRocmVhZCAqdGQpOwogaW50CW5mc19yZWFkZGlycnBjKHN0cnVjdCB2bm9k ZSAqLCBzdHJ1Y3QgdWlvICosIHN0cnVjdCB1Y3JlZCAqKTsKIGludAluZnNfbmZzaW9kbmV3KGlu dCk7Cit2b2lkCW5mc19uZnNpb2RuZXdfdHEoX191bnVzZWQgdm9pZCAqLCBpbnQpOwogaW50CW5m c19hc3luY2lvKHN0cnVjdCBuZnNtb3VudCAqLCBzdHJ1Y3QgYnVmICosIHN0cnVjdCB1Y3JlZCAq LCBzdHJ1Y3QgdGhyZWFkICopOwogaW50CW5mc19kb2lvKHN0cnVjdCB2bm9kZSAqLCBzdHJ1Y3Qg YnVmICosIHN0cnVjdCB1Y3JlZCAqLCBzdHJ1Y3QgdGhyZWFkICopOwogdm9pZAluZnNfZG9pb19k aXJlY3R3cml0ZSAoc3RydWN0IGJ1ZiAqKTsK --001485ebae8876b28f048e29d13d--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTikpC9REEQKNUUHtLDxSLB1MmiqKdgbJ0Ee7O%2BGJ>