svn commit: r273549 - head/sys/kern
Author: mjg
Date: Thu Oct 23 15:35:47 2014
New Revision: 273549
URL: https://svnweb.freebsd.org/changeset/base/273549
Log:
Avoid taking the lock in selfdfree when not needed.
Modified:
head/sys/kern/sys_generic.c
Modified: head/sys/kern/sys_generic.c
==
--- head/sys/kern/sys_generic.c Thu Oct 23 15:16:40 2014(r273548)
+++ head/sys/kern/sys_generic.c Thu Oct 23 15:35:47 2014(r273549)
@@ -1600,10 +1600,11 @@ static void
selfdfree(struct seltd *stp, struct selfd *sfp)
{
STAILQ_REMOVE(stp-st_selq, sfp, selfd, sf_link);
- mtx_lock(sfp-sf_mtx);
- if (sfp-sf_si)
+ if (sfp-sf_si != NULL) {
+ mtx_lock(sfp-sf_mtx);
TAILQ_REMOVE(sfp-sf_si-si_tdlist, sfp, sf_threads);
- mtx_unlock(sfp-sf_mtx);
+ mtx_unlock(sfp-sf_mtx);
+ }
uma_zfree(selfd_zone, sfp);
}
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
Re: svn commit: r273549 - head/sys/kern
On Thursday, October 23, 2014 11:35:47 am Mateusz Guzik wrote:
Author: mjg
Date: Thu Oct 23 15:35:47 2014
New Revision: 273549
URL: https://svnweb.freebsd.org/changeset/base/273549
Log:
Avoid taking the lock in selfdfree when not needed.
Modified:
head/sys/kern/sys_generic.c
Modified: head/sys/kern/sys_generic.c
==
--- head/sys/kern/sys_generic.c Thu Oct 23 15:16:40 2014
(r273548)
+++ head/sys/kern/sys_generic.c Thu Oct 23 15:35:47 2014
(r273549)
@@ -1600,10 +1600,11 @@ static void
selfdfree(struct seltd *stp, struct selfd *sfp)
{
STAILQ_REMOVE(stp-st_selq, sfp, selfd, sf_link);
- mtx_lock(sfp-sf_mtx);
- if (sfp-sf_si)
+ if (sfp-sf_si != NULL) {
+ mtx_lock(sfp-sf_mtx);
TAILQ_REMOVE(sfp-sf_si-si_tdlist, sfp, sf_threads);
- mtx_unlock(sfp-sf_mtx);
+ mtx_unlock(sfp-sf_mtx);
+ }
uma_zfree(selfd_zone, sfp);
How do you ensure that the value you read for sf_si here is up to date? In
particular, if a thread is selecting on multiple fds and one awakens it,
another fd can invoke selwakeup() while the thread is in seltdclear().
In that case, you might see a stale value of sf_si and not realize it is
cleared by the selwakeup() after you get the lock and you will invoke
TAILQ_REMOVE an extra time.
--
John Baldwin
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
Re: svn commit: r273549 - head/sys/kern
Dnia 23 paź 2014 o godz. 20:38 John Baldwin j...@freebsd.org napisał(a):
On Thursday, October 23, 2014 11:35:47 am Mateusz Guzik wrote:
Author: mjg
Date: Thu Oct 23 15:35:47 2014
New Revision: 273549
URL: https://svnweb.freebsd.org/changeset/base/273549
Log:
Avoid taking the lock in selfdfree when not needed.
Modified:
head/sys/kern/sys_generic.c
Modified: head/sys/kern/sys_generic.c
==
--- head/sys/kern/sys_generic.cThu Oct 23 15:16:40 2014(r273548)
+++ head/sys/kern/sys_generic.cThu Oct 23 15:35:47 2014(r273549)
@@ -1600,10 +1600,11 @@ static void
selfdfree(struct seltd *stp, struct selfd *sfp)
{
STAILQ_REMOVE(stp-st_selq, sfp, selfd, sf_link);
-mtx_lock(sfp-sf_mtx);
-if (sfp-sf_si)
+if (sfp-sf_si != NULL) {
+mtx_lock(sfp-sf_mtx);
TAILQ_REMOVE(sfp-sf_si-si_tdlist, sfp, sf_threads);
-mtx_unlock(sfp-sf_mtx);
+mtx_unlock(sfp-sf_mtx);
+}
uma_zfree(selfd_zone, sfp);
How do you ensure that the value you read for sf_si here is up to date? In
particular, if a thread is selecting on multiple fds and one awakens it,
another fd can invoke selwakeup() while the thread is in seltdclear().
In that case, you might see a stale value of sf_si and not realize it is
cleared by the selwakeup() after you get the lock and you will invoke
TAILQ_REMOVE an extra time.
FWIW, I've just hit a panic in selfdfree().
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
Re: svn commit: r273549 - head/sys/kern
Please back this out; it looks like the lock is protecting sf_si.
-adrian
On 23 October 2014 11:45, Edward Tomasz Napierała tr...@freebsd.org wrote:
Dnia 23 paź 2014 o godz. 20:38 John Baldwin j...@freebsd.org napisał(a):
On Thursday, October 23, 2014 11:35:47 am Mateusz Guzik wrote:
Author: mjg
Date: Thu Oct 23 15:35:47 2014
New Revision: 273549
URL: https://svnweb.freebsd.org/changeset/base/273549
Log:
Avoid taking the lock in selfdfree when not needed.
Modified:
head/sys/kern/sys_generic.c
Modified: head/sys/kern/sys_generic.c
==
--- head/sys/kern/sys_generic.cThu Oct 23 15:16:40 2014(r273548)
+++ head/sys/kern/sys_generic.cThu Oct 23 15:35:47 2014(r273549)
@@ -1600,10 +1600,11 @@ static void
selfdfree(struct seltd *stp, struct selfd *sfp)
{
STAILQ_REMOVE(stp-st_selq, sfp, selfd, sf_link);
-mtx_lock(sfp-sf_mtx);
-if (sfp-sf_si)
+if (sfp-sf_si != NULL) {
+mtx_lock(sfp-sf_mtx);
TAILQ_REMOVE(sfp-sf_si-si_tdlist, sfp, sf_threads);
-mtx_unlock(sfp-sf_mtx);
+mtx_unlock(sfp-sf_mtx);
+}
uma_zfree(selfd_zone, sfp);
How do you ensure that the value you read for sf_si here is up to date? In
particular, if a thread is selecting on multiple fds and one awakens it,
another fd can invoke selwakeup() while the thread is in seltdclear().
In that case, you might see a stale value of sf_si and not realize it is
cleared by the selwakeup() after you get the lock and you will invoke
TAILQ_REMOVE an extra time.
FWIW, I've just hit a panic in selfdfree().
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
Re: svn commit: r273549 - head/sys/kern
On Thursday, October 23, 2014 3:39:39 pm Adrian Chadd wrote:
Please back this out; it looks like the lock is protecting sf_si.
The followup fix should be fine. The lock does indeed protect sf_si, but the
value
can only transition from non-NULL to NULL at this point, so if it is == NULL
without
the lock, it is safe to assume it has already been cleared.
-adrian
On 23 October 2014 11:45, Edward Tomasz Napierała tr...@freebsd.org wrote:
Dnia 23 paź 2014 o godz. 20:38 John Baldwin j...@freebsd.org napisał(a):
On Thursday, October 23, 2014 11:35:47 am Mateusz Guzik wrote:
Author: mjg
Date: Thu Oct 23 15:35:47 2014
New Revision: 273549
URL: https://svnweb.freebsd.org/changeset/base/273549
Log:
Avoid taking the lock in selfdfree when not needed.
Modified:
head/sys/kern/sys_generic.c
Modified: head/sys/kern/sys_generic.c
==
--- head/sys/kern/sys_generic.cThu Oct 23 15:16:40 2014(r273548)
+++ head/sys/kern/sys_generic.cThu Oct 23 15:35:47 2014(r273549)
@@ -1600,10 +1600,11 @@ static void
selfdfree(struct seltd *stp, struct selfd *sfp)
{
STAILQ_REMOVE(stp-st_selq, sfp, selfd, sf_link);
-mtx_lock(sfp-sf_mtx);
-if (sfp-sf_si)
+if (sfp-sf_si != NULL) {
+mtx_lock(sfp-sf_mtx);
TAILQ_REMOVE(sfp-sf_si-si_tdlist, sfp, sf_threads);
-mtx_unlock(sfp-sf_mtx);
+mtx_unlock(sfp-sf_mtx);
+}
uma_zfree(selfd_zone, sfp);
How do you ensure that the value you read for sf_si here is up to date? In
particular, if a thread is selecting on multiple fds and one awakens it,
another fd can invoke selwakeup() while the thread is in seltdclear().
In that case, you might see a stale value of sf_si and not realize it is
cleared by the selwakeup() after you get the lock and you will invoke
TAILQ_REMOVE an extra time.
FWIW, I've just hit a panic in selfdfree().
--
John Baldwin
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org
