Fix a race that can occur when nfs nfsiod threads are being created.

Without this patch it was possible for a different thread that calls
nfs_asyncio() to snitch a newly created nfsiod thread that was
intended for another caller of nfs_asyncio(), because the nfs_iod_mtx
mutex was unlocked while the new nfsiod thread was created. This patch
labels the newly created nfsiod, so that it is not taken by another
caller of nfs_asyncio(). This is believed to fix the problem reported
on the freebsd-stable email list under the subject:
FreeBSD NFS client/Linux NFS server issue.

Tested by:	to DOT my DOT trociny AT gmail DOT com
Reviewed by:	jhb
MFC after:	2 weeks
This commit is contained in:
Rick Macklem 2010-01-27 15:22:20 +00:00
parent 47a933e3df
commit 651ff543f8
Notes: svn2git 2020-12-20 02:59:44 +00:00
svn path=/head/; revision=203072
6 changed files with 34 additions and 15 deletions

View File

@ -252,7 +252,7 @@ int nfs_writerpc(struct vnode *, struct uio *, struct ucred *, int *,
int nfs_commit(struct vnode *vp, u_quad_t offset, int cnt,
struct ucred *cred, struct thread *td);
int nfs_readdirrpc(struct vnode *, struct uio *, struct ucred *);
int nfs_nfsiodnew(void);
int nfs_nfsiodnew(int);
int nfs_asyncio(struct nfsmount *, struct buf *, struct ucred *, struct thread *);
int nfs_doio(struct vnode *, struct buf *, struct ucred *, struct thread *);
void nfs_doio_directwrite (struct buf *);

View File

@ -1377,7 +1377,7 @@ nfs_asyncio(struct nfsmount *nmp, struct buf *bp, struct ucred *cred, struct thr
* Find a free iod to process this request.
*/
for (iod = 0; iod < nfs_numasync; iod++)
if (nfs_iodwant[iod]) {
if (nfs_iodwant[iod] == NFSIOD_AVAILABLE) {
gotiod = TRUE;
break;
}
@ -1386,7 +1386,7 @@ nfs_asyncio(struct nfsmount *nmp, struct buf *bp, struct ucred *cred, struct thr
* Try to create one if none are free.
*/
if (!gotiod) {
iod = nfs_nfsiodnew();
iod = nfs_nfsiodnew(1);
if (iod != -1)
gotiod = TRUE;
}
@ -1398,7 +1398,7 @@ nfs_asyncio(struct nfsmount *nmp, struct buf *bp, struct ucred *cred, struct thr
*/
NFS_DPF(ASYNCIO, ("nfs_asyncio: waking iod %d for mount %p\n",
iod, nmp));
nfs_iodwant[iod] = NULL;
nfs_iodwant[iod] = NFSIOD_NOT_AVAILABLE;
nfs_iodmount[iod] = nmp;
nmp->nm_bufqiods++;
wakeup(&nfs_iodwant[iod]);

View File

@ -113,7 +113,7 @@ sysctl_iodmin(SYSCTL_HANDLER_ARGS)
* than the new minimum, create some more.
*/
for (i = nfs_iodmin - nfs_numasync; i > 0; i--)
nfs_nfsiodnew();
nfs_nfsiodnew(0);
out:
mtx_unlock(&nfs_iod_mtx);
return (0);
@ -147,7 +147,7 @@ sysctl_iodmax(SYSCTL_HANDLER_ARGS)
*/
iod = nfs_numasync - 1;
for (i = 0; i < nfs_numasync - nfs_iodmax; i++) {
if (nfs_iodwant[iod])
if (nfs_iodwant[iod] == NFSIOD_AVAILABLE)
wakeup(&nfs_iodwant[iod]);
iod--;
}
@ -160,7 +160,7 @@ SYSCTL_PROC(_vfs_nfs, OID_AUTO, iodmax, CTLTYPE_UINT | CTLFLAG_RW, 0,
"Max number of nfsiod kthreads");
int
nfs_nfsiodnew(void)
nfs_nfsiodnew(int set_iodwant)
{
int error, i;
int newiod;
@ -176,12 +176,17 @@ nfs_nfsiodnew(void)
}
if (newiod == -1)
return (-1);
if (set_iodwant > 0)
nfs_iodwant[i] = NFSIOD_CREATED_FOR_NFS_ASYNCIO;
mtx_unlock(&nfs_iod_mtx);
error = kproc_create(nfssvc_iod, nfs_asyncdaemon + i, NULL, RFHIGHPID,
0, "nfsiod %d", newiod);
mtx_lock(&nfs_iod_mtx);
if (error)
if (error) {
if (set_iodwant > 0)
nfs_iodwant[i] = NFSIOD_NOT_AVAILABLE;
return (-1);
}
nfs_numasync++;
return (newiod);
}
@ -199,7 +204,7 @@ nfsiod_setup(void *dummy)
nfs_iodmin = NFS_MAXASYNCDAEMON;
for (i = 0; i < nfs_iodmin; i++) {
error = nfs_nfsiodnew();
error = nfs_nfsiodnew(0);
if (error == -1)
panic("nfsiod_setup: nfs_nfsiodnew failed");
}
@ -236,7 +241,8 @@ nfssvc_iod(void *instance)
goto finish;
if (nmp)
nmp->nm_bufqiods--;
nfs_iodwant[myiod] = curthread->td_proc;
if (nfs_iodwant[myiod] == NFSIOD_NOT_AVAILABLE)
nfs_iodwant[myiod] = NFSIOD_AVAILABLE;
nfs_iodmount[myiod] = NULL;
/*
* Always keep at least nfs_iodmin kthreads.
@ -303,7 +309,7 @@ nfssvc_iod(void *instance)
nfs_asyncdaemon[myiod] = 0;
if (nmp)
nmp->nm_bufqiods--;
nfs_iodwant[myiod] = NULL;
nfs_iodwant[myiod] = NFSIOD_NOT_AVAILABLE;
nfs_iodmount[myiod] = NULL;
/* Someone may be waiting for the last nfsiod to terminate. */
if (--nfs_numasync == 0)

View File

@ -347,7 +347,7 @@ nfs_init(struct vfsconf *vfsp)
nfs_ticks = 1;
/* Ensure async daemons disabled */
for (i = 0; i < NFS_MAXASYNCDAEMON; i++) {
nfs_iodwant[i] = NULL;
nfs_iodwant[i] = NFSIOD_NOT_AVAILABLE;
nfs_iodmount[i] = NULL;
}
nfs_nhinit(); /* Init the nfsnode table */
@ -375,7 +375,7 @@ nfs_uninit(struct vfsconf *vfsp)
mtx_lock(&nfs_iod_mtx);
nfs_iodmax = 0;
for (i = 0; i < nfs_numasync; i++)
if (nfs_iodwant[i])
if (nfs_iodwant[i] == NFSIOD_AVAILABLE)
wakeup(&nfs_iodwant[i]);
/* The last nfsiod to exit will wake us up when nfs_numasync hits 0 */
while (nfs_numasync)

View File

@ -212,7 +212,7 @@ static int nfs_renameit(struct vnode *sdvp, struct componentname *scnp,
* Global variables
*/
struct mtx nfs_iod_mtx;
struct proc *nfs_iodwant[NFS_MAXASYNCDAEMON];
enum nfsiod_state nfs_iodwant[NFS_MAXASYNCDAEMON];
struct nfsmount *nfs_iodmount[NFS_MAXASYNCDAEMON];
int nfs_numasync = 0;
vop_advlock_t *nfs_advlock_p = nfs_dolock;

View File

@ -176,11 +176,24 @@ struct nfsnode {
#define NFS_TIMESPEC_COMPARE(T1, T2) (((T1)->tv_sec != (T2)->tv_sec) || ((T1)->tv_nsec != (T2)->tv_nsec))
/*
* NFS iod threads can be in one of these three states once spawned.
* NFSIOD_NOT_AVAILABLE - Cannot be assigned an I/O operation at this time.
* NFSIOD_AVAILABLE - Available to be assigned an I/O operation.
* NFSIOD_CREATED_FOR_NFS_ASYNCIO - Newly created for nfs_asyncio() and
* will be used by the thread that called nfs_asyncio().
*/
enum nfsiod_state {
NFSIOD_NOT_AVAILABLE = 0,
NFSIOD_AVAILABLE = 1,
NFSIOD_CREATED_FOR_NFS_ASYNCIO = 2,
};
/*
* Queue head for nfsiod's
*/
extern TAILQ_HEAD(nfs_bufq, buf) nfs_bufq;
extern struct proc *nfs_iodwant[NFS_MAXASYNCDAEMON];
extern enum nfsiod_state nfs_iodwant[NFS_MAXASYNCDAEMON];
extern struct nfsmount *nfs_iodmount[NFS_MAXASYNCDAEMON];
#if defined(_KERNEL)