Date: Wed, 21 Dec 2016 22:37:42 +0100 From: Vincenzo Maffione <v.maffione@gmail.com> To: Luigi Rizzo <rizzo@iet.unipi.it> Cc: Adrian Chadd <adrian.chadd@gmail.com>, Navdeep Parhar <np@freebsd.org>, "freebsd-net@freebsd.org" <freebsd-net@freebsd.org>, Luigi Rizzo <luigi@freebsd.org>, Giuseppe Lettieri <g.lettieri@iet.unipi.it> Subject: Re: cxgbe's native netmap support broken since r307394 Message-ID: <CA%2B_eA9jGaUJPX9b=8r8xZ4bmryqbFnNogbb2KEOCRcXNpqBmrg@mail.gmail.com> In-Reply-To: <CA%2BhQ2%2Bjnrk%2Bz0iCcDsk1uk6u-QagrmkvbZMqE5BQZJCnRggpLw@mail.gmail.com> References: <20161217222812.GA4979@ox> <CA%2B_eA9jxs3FatSLs8fseb%2BJSrqWrYkegDHyKA1q2QH=Q4_LbJQ@mail.gmail.com> <CAPFoGT9VoRWF306gBYgSKd2wi1rx3_2-yLpnxO98GyM0V-0Xog@mail.gmail.com> <CA%2B_eA9iUOZ9JLyx3bbDUZVc=cR6SqxReoOLRHUYQy-BU6Eb-UQ@mail.gmail.com> <CAJ-VmomPW7%2BknAKbaLYjKXfgd=xAH3bYZi_LoxtNf5cuejJeJw@mail.gmail.com> <CA%2B_eA9hAjK-i762t=ZCKqzrk5EDeEJM9gf%2BOAmDJqyUGrAYcFg@mail.gmail.com> <CA%2BhQ2%2Bjnrk%2Bz0iCcDsk1uk6u-QagrmkvbZMqE5BQZJCnRggpLw@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--001a11417c3882e509054431f6ed Content-Type: text/plain; charset=UTF-8 Hi Luigi, I attached a minimal change containing two fixes: - change IFNET_WLOCK into IFNET_RLOCK, to fix the cxgbe issue related to this thread - use the proper locking functions for the "worker_lock", unrelated but needed to avoid the O.S. to trap because of a mismatch between MTX_SPIN and MTX_DEF. Cheers, Vincenzo 2016-12-21 20:30 GMT+01:00 Luigi Rizzo <rizzo@iet.unipi.it>: > On Wed, Dec 21, 2016 at 11:15 AM, Vincenzo Maffione > <v.maffione@gmail.com> wrote: >> Hi, >> There is no commit related to that in the FreeBSD svn or git. >> >> The fix has been published to the github netmap repository here >> (branch master): https://github.com/luigirizzo/netmap >> >> What we should do is to import all the recent updates from the github >> into HEAD. I can prepare a patch for HEAD, if you wish. Just let me >> know. > > I just checked and the diff between FreeBSD head and netmap head > in github is almost 3k lines due to a lot of recent refactoring. > So, if there is an easy way to extract just the locking change that would > be preferable as an interim solution. > > cheers > luigi > >> >> Cheers, >> Vincenzo >> >> >> 2016-12-20 21:45 GMT+01:00 Adrian Chadd <adrian.chadd@gmail.com>: >>> hi, >>> >>> What's the commit? We should get it into -HEAD asap. >>> >>> >>> -adrian >>> >>> >>> On 20 December 2016 at 01:25, Vincenzo Maffione <v.maffione@gmail.com> wrote: >>>> Ok, applied to the netmap github repo. >>>> This fix will be published when Luigi does the next commit on FreeBSD. >>>> >>>> Cheers, >>>> Vincenzo >>>> >>>> 2016-12-19 20:05 GMT+01:00 Navdeep Parhar <np@freebsd.org>: >>>>> IFNET_RLOCK will work, thanks. >>>>> >>>>> Navdeep >>>>> >>>>> On Mon, Dec 19, 2016 at 3:21 AM, Vincenzo Maffione <v.maffione@gmail.com> wrote: >>>>>> Hi Navdeep, >>>>>> >>>>>> Indeed, we have reviewed the code, and we think it is ok to >>>>>> implement nm_os_ifnet_lock() with IFNET_RLOCK(), instead of using >>>>>> IFNET_WLOCK(). >>>>>> Since IFNET_RLOCK() results into sx_slock(), this should fix the issue. >>>>>> >>>>>> On FreeBSD, this locking is needed to protect a flag read by nm_iszombie(). >>>>>> However, on Linux the same lock is also needed to protect the call to >>>>>> the nm_hw_register() callback, so we prefer to have an "unified" >>>>>> locking scheme, i.e. always calling nm_hw_register under the lock. >>>>>> >>>>>> Does this make sense to you? Would it be easy for you to make a quick >>>>>> test by replacing IFNET_WLOCK with IFNET_RLOCK? >>>>>> >>>>>> Thanks, >>>>>> Vincenzo >>>>>> >>>>>> 2016-12-17 23:28 GMT+01:00 Navdeep Parhar <np@freebsd.org>: >>>>>>> Luigi, Vincenzo, >>>>>>> >>>>>>> The last major update to netmap (r307394 and followups) broke cxgbe's >>>>>>> native netmap support. The problem is that netmap_hw_reg now holds an >>>>>>> rw_lock around the driver's netmap_on/off routines. It has always been >>>>>>> safe for the driver to sleep during these operations but now it panics >>>>>>> instead. >>>>>>> >>>>>>> Why is IFNET_WLOCK needed here? It seems like a regression to disallow >>>>>>> sleep on the control path. >>>>>>> >>>>>>> Regards, >>>>>>> Navdeep >>>>>>> >>>>>>> begin_synchronized_op with the following non-sleepable locks held: >>>>>>> exclusive rw ifnet_rw (ifnet_rw) r = 0 (0xffffffff8271d680) locked @ >>>>>>> /root/ws/head/sys/dev/netmap/netmap_freebsd.c:95 >>>>>>> stack backtrace: >>>>>>> #0 0xffffffff810837a5 at witness_debugger+0xe5 >>>>>>> #1 0xffffffff81084d88 at witness_warn+0x3b8 >>>>>>> #2 0xffffffff83ef2bcc at begin_synchronized_op+0x6c >>>>>>> #3 0xffffffff83f14beb at cxgbe_netmap_reg+0x5b >>>>>>> #4 0xffffffff809846f1 at netmap_hw_reg+0x81 >>>>>>> #5 0xffffffff809806de at netmap_do_regif+0x19e >>>>>>> #6 0xffffffff8098121d at netmap_ioctl+0x7ad >>>>>>> #7 0xffffffff8098682f at freebsd_netmap_ioctl+0x5f >>>>>> >>>>>> >>>>>> >>>>>> -- >>>>>> Vincenzo Maffione >>>>>> _______________________________________________ >>>>>> freebsd-net@freebsd.org mailing list >>>>>> https://lists.freebsd.org/mailman/listinfo/freebsd-net >>>>>> To unsubscribe, send any mail to "freebsd-net-unsubscribe@freebsd.org" >>>> >>>> >>>> >>>> -- >>>> Vincenzo Maffione >>>> _______________________________________________ >>>> freebsd-net@freebsd.org mailing list >>>> https://lists.freebsd.org/mailman/listinfo/freebsd-net >>>> To unsubscribe, send any mail to "freebsd-net-unsubscribe@freebsd.org" >> >> >> >> -- >> Vincenzo Maffione > > > > -- > -----------------------------------------+------------------------------- > Prof. Luigi RIZZO, rizzo@iet.unipi.it . Dip. di Ing. dell'Informazione > http://www.iet.unipi.it/~luigi/ . Universita` di Pisa > TEL +39-050-2217533 . via Diotisalvi 2 > Mobile +39-338-6809875 . 56122 PISA (Italy) > -----------------------------------------+------------------------------- -- Vincenzo Maffione --001a11417c3882e509054431f6ed Content-Type: text/x-patch; charset=US-ASCII; name="netmap-fix.patch" Content-Disposition: attachment; filename="netmap-fix.patch" Content-Transfer-Encoding: base64 X-Attachment-Id: f_iwzglhe60 ZGlmZiAtLWdpdCBhL3N5cy9kZXYvbmV0bWFwL25ldG1hcF9mcmVlYnNkLmMgYi9zeXMvZGV2L25l dG1hcC9uZXRtYXBfZnJlZWJzZC5jCmluZGV4IGZkNDU5MmI3YjVhLi41MTViZTY2NDhlYSAxMDA2 NDQKLS0tIGEvc3lzL2Rldi9uZXRtYXAvbmV0bWFwX2ZyZWVic2QuYworKysgYi9zeXMvZGV2L25l dG1hcC9uZXRtYXBfZnJlZWJzZC5jCkBAIC05Miw3ICs5Miw3IEBAIG5tX29zX3NlbGluZm9fdW5p bml0KE5NX1NFTElORk9fVCAqc2kpCiB2b2lkCiBubV9vc19pZm5ldF9sb2NrKHZvaWQpCiB7Ci0J SUZORVRfV0xPQ0soKTsKKwlJRk5FVF9STE9DSygpOwogfQogCiB2b2lkCkBAIC0xMDIxLDEyICsx MDIxLDEyIEBAIG5tX29zX2t0aHJlYWRfd2FrZXVwX3dvcmtlcihzdHJ1Y3Qgbm1fa3RocmVhZCAq bm1rKQogCSAqIGJ1dCBzaW1wbHkgdGhhdCBpdCBoYXMgY2hhbmdlZCBzaW5jZSB0aGUgbGFzdAog CSAqIHRpbWUgdGhlIGt0aHJlYWQgc2F3IGl0LgogCSAqLwotCW10eF9sb2NrKCZubWstPndvcmtl cl9sb2NrKTsKKwltdHhfbG9ja19zcGluKCZubWstPndvcmtlcl9sb2NrKTsKIAlubWstPnNjaGVk dWxlZCsrOwogCWlmIChubWstPndvcmtlcl9jdHguY2ZnLndjaGFuKSB7CiAJCXdha2V1cCgodm9p ZCAqKSh1aW50cHRyX3Qpbm1rLT53b3JrZXJfY3R4LmNmZy53Y2hhbik7CiAJfQotCW10eF91bmxv Y2soJm5tay0+d29ya2VyX2xvY2spOworCW10eF91bmxvY2tfc3Bpbigmbm1rLT53b3JrZXJfbG9j ayk7CiB9CiAKIHZvaWQgaW5saW5lCkBAIC0xMDgwLDEwICsxMDgwLDEwIEBAIG5tX2t0aHJlYWRf d29ya2VyKHZvaWQgKmRhdGEpCiAJCQljdHgtPndvcmtlcl9mbihjdHgtPndvcmtlcl9wcml2YXRl KTsgLyogd29ya2VyIGJvZHkgKi8KIAkJfSBlbHNlIHsKIAkJCS8qIGNoZWNrcyBpZiB0aGVyZSBp cyBhIHBlbmRpbmcgbm90aWZpY2F0aW9uICovCi0JCQltdHhfbG9jaygmbm1rLT53b3JrZXJfbG9j ayk7CisJCQltdHhfbG9ja19zcGluKCZubWstPndvcmtlcl9sb2NrKTsKIAkJCWlmIChsaWtlbHko bm1rLT5zY2hlZHVsZWQgIT0gb2xkX3NjaGVkdWxlZCkpIHsKIAkJCQlvbGRfc2NoZWR1bGVkID0g bm1rLT5zY2hlZHVsZWQ7Ci0JCQkJbXR4X3VubG9jaygmbm1rLT53b3JrZXJfbG9jayk7CisJCQkJ bXR4X3VubG9ja19zcGluKCZubWstPndvcmtlcl9sb2NrKTsKIAogCQkJCWN0eC0+d29ya2VyX2Zu KGN0eC0+d29ya2VyX3ByaXZhdGUpOyAvKiB3b3JrZXIgYm9keSAqLwogCkBAIC0xMDk0LDcgKzEw OTQsNyBAQCBubV9rdGhyZWFkX3dvcmtlcih2b2lkICpkYXRhKQogCQkJCSAgICAmbm1rLT53b3Jr ZXJfbG9jaywgIm5ta19ldiIsIGh6KTsKIAkJCQlubWstPnNjaGVkdWxlZCsrOwogCQkJfQotCQkJ bXR4X3VubG9jaygmbm1rLT53b3JrZXJfbG9jayk7CisJCQltdHhfdW5sb2NrX3NwaW4oJm5tay0+ d29ya2VyX2xvY2spOwogCQl9CiAJfQogCg== --001a11417c3882e509054431f6ed--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CA%2B_eA9jGaUJPX9b=8r8xZ4bmryqbFnNogbb2KEOCRcXNpqBmrg>