On Wed, 3 Aug 2016, Konstantin Belousov wrote:
Log:
Remove unneeded (recursing) Giant acquisition around vprintf(9).
Reviewed by: rmacklem
Sponsored by: The FreeBSD Foundation
MFC after: 1 week
Modified:
head/sys/fs/nfsclient/nfs_clsubs.c
Modified: head/sys/fs/nfsclient/nfs_clsubs.c
==============================================================================
--- head/sys/fs/nfsclient/nfs_clsubs.c Wed Aug 3 10:23:42 2016
(r303709)
+++ head/sys/fs/nfsclient/nfs_clsubs.c Wed Aug 3 11:49:17 2016
(r303710)
@@ -166,11 +166,9 @@ ncl_printf(const char *fmt, ...)
{
va_list ap;
- mtx_lock(&Giant);
va_start(ap, fmt);
vprintf(fmt, ap);
va_end(ap);
- mtx_unlock(&Giant);
}
#ifdef NFS_ACDEBUG
This leaves the less than needed function nfs_printf(). Old versions
of nfs used the correct function printf(). All nfs_printf() ever did
was:
- when it was first implemented, almost never work. It had
printf(fmt, ap) instead of vprintf(fmt, ap). This only worked when
fmt had no % directives in it. Otherwise, it gave undefined behaviour.
FreeBSD-7 still has this version.
- to hold the Giant locking. This was not completely unneeded. It had
the side effect of serializing printfs within nfs.
Serialization in -current is normally done using PRINTF_BUFR_SIZE, but
this is not the default and it gives deadlocks or drops output so I
don't use it.
I refined my old fix for serializing printf() and it now works very well.
@@ -197,9 +195,6 @@ ncl_getattrcache(struct vnode *vp, struc
vap = &np->n_vattr.na_vattr;
nmp = VFSTONFS(vp->v_mount);
mustflush = nfscl_mustflush(vp); /* must be before mtx_lock() */
-#ifdef NFS_ACDEBUG
- mtx_lock(&Giant); /* ncl_printf() */
-#endif
mtx_lock(&np->n_mtx);
/* XXX n_mtime doesn't seem to be updated on a miss-and-reload */
timeo = (time_second - np->n_mtime.tv_sec) / 10;
@@ -236,9 +231,6 @@ ncl_getattrcache(struct vnode *vp, struc
(mustflush != 0 || np->n_attrstamp == 0)) {
newnfsstats.attrcache_misses++;
mtx_unlock(&np->n_mtx);
-#ifdef NFS_ACDEBUG
- mtx_unlock(&Giant); /* ncl_printf() */
-#endif
KDTRACE_NFS_ATTRCACHE_GET_MISS(vp);
return( ENOENT);
}
@@ -266,9 +258,6 @@ ncl_getattrcache(struct vnode *vp, struc
vaper->va_mtime = np->n_mtim;
}
mtx_unlock(&np->n_mtx);
-#ifdef NFS_ACDEBUG
- mtx_unlock(&Giant); /* ncl_printf() */
-#endif
KDTRACE_NFS_ATTRCACHE_GET_HIT(vp, vap);
return (0);
}
Unfortunately, these are probably still "needed". printf() serialization
using PRINTF_BUFR_SIZE or my method only works for individual printf()s
or possibly single lines. Just about any lock around a group of printf()s
serializes them as a group (only across subsystems using the same lock of
course).
Bruce
_______________________________________________
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
