On 18 August 2010 17:46, Kostik Belousov <kostik...@gmail.com> wrote:
> On Wed, Aug 18, 2010 at 02:43:19PM +0400, pluknet wrote:
>> On 18 August 2010 12:07, pluknet <pluk...@gmail.com> wrote:
>> > On 17 August 2010 20:04, Kostik Belousov <kostik...@gmail.com> wrote:
>> >
>> >>
>> >> Also please take a note of the John' suggestion to use the taskqueue.
>> >
>> > I decided to go this road. Thank you both.
>> > Now I do nfs buildkernel survive and prepare some benchmark results.
>> >
>>
>> So, I modified the patch to defer proc_create() with taskqueue(9).
>> Below is `time make -j5 buildkernel WITHOUT_MODULES=yes` 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%       6538+2133k 87+43388io 226pf+0w
>>
>> clean new
>> 1134.755u 240.032s 7:41.25 298.0%       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.

-- 
wbr,
pluknet

Attachment: dmesg.out
Description: Binary data

Index: sys/nfsclient/nfs_subs.c
===================================================================
--- sys/nfsclient/nfs_subs.c	(revision 211279)
+++ sys/nfsclient/nfs_subs.c	(working copy)
@@ -59,6 +59,7 @@
 #include <sys/sysent.h>
 #include <sys/syscall.h>
 #include <sys/sysproto.h>
+#include <sys/taskqueue.h>
 
 #include <vm/vm.h>
 #include <vm/vm_object.h>
@@ -117,6 +118,7 @@
 
 struct nfs_bufq	nfs_bufq;
 static struct mtx nfs_xid_mtx;
+struct task	nfs_nfsiodnew_task;
 
 /*
  * and the reverse mapping from generic to Version 2 procedure numbers
@@ -354,6 +356,7 @@
 	 */
 	mtx_init(&nfs_iod_mtx, "NFS iod lock", NULL, MTX_DEF);
 	mtx_init(&nfs_xid_mtx, "NFS xid lock", NULL, MTX_DEF);
+	TASK_INIT(&nfs_nfsiodnew_task, 0, nfs_nfsiodnew_tq, NULL);
 
 	nfs_pbuf_freecnt = nswbuf / 2 + 1;
 
Index: sys/nfsclient/nfs_nfsiod.c
===================================================================
--- sys/nfsclient/nfs_nfsiod.c	(revision 211279)
+++ sys/nfsclient/nfs_nfsiod.c	(working copy)
@@ -59,6 +59,7 @@
 #include <sys/fcntl.h>
 #include <sys/lockf.h>
 #include <sys/mutex.h>
+#include <sys/taskqueue.h>
 
 #include <netinet/in.h>
 #include <netinet/tcp.h>
@@ -75,6 +76,17 @@
 
 static void	nfssvc_iod(void *);
 
+struct nfsiod_str {
+	SLIST_ENTRY(nfsiod_str) ni_list;
+	int *ni_inst;
+	int ni_iod;
+	int ni_error;
+	int ni_busy;
+	int ni_done;
+};
+static SLIST_HEAD(, nfsiod_str) nfsiodhead =
+    SLIST_HEAD_INITIALIZER(nfsiodhead);
+
 static int nfs_asyncdaemon[NFS_MAXASYNCDAEMON];
 
 SYSCTL_DECL(_vfs_nfs);
@@ -159,11 +171,34 @@
     sizeof (nfs_iodmax), sysctl_iodmax, "IU",
     "Max number of nfsiod kthreads");
 
+void
+nfs_nfsiodnew_tq(__unused void *arg, int pending)
+{
+	struct nfsiod_str *nip;
+
+	mtx_lock(&nfs_iod_mtx);
+	SLIST_FOREACH(nip, &nfsiodhead, ni_list) {
+		printf("tq: SLIST nip: %p\n", nip);
+		if (nip->ni_busy == 0) {
+			nip->ni_busy = 1;
+			break;
+		}
+	}
+	mtx_unlock(&nfs_iod_mtx);
+	KASSERT(nip != NULL, ("nfs_nfsiodnew_tq: nip is NULL"));
+	printf("tq: nip: %p\n", nip);
+	printf("tq: ni_inst: %p\n", nip->ni_inst);
+	printf("tq: ni_iod: %d\n", nip->ni_iod);
+	nip->ni_error = kproc_create(nfssvc_iod, nip->ni_inst, NULL,
+	    RFHIGHPID, 0, "nfsiod %d", nip->ni_iod);
+	nip->ni_done = 1;
+}
+
 int
 nfs_nfsiodnew(int set_iodwant)
 {
-	int error, i;
-	int newiod;
+	int i, newiod, error;
+	struct nfsiod_str *nip, *nip_temp;
 
 	if (nfs_numasync >= nfs_iodmax)
 		return (-1);
@@ -178,17 +213,34 @@
 		return (-1);
 	if (set_iodwant > 0)
 		nfs_iodwant[i] = NFSIOD_CREATED_FOR_NFS_ASYNCIO;
+	nip = malloc(sizeof(*nip), M_TEMP, M_NOWAIT | M_ZERO);
+	if (nip == NULL)
+		goto fail;
+	nip->ni_inst = nfs_asyncdaemon + i;
+	nip->ni_iod = newiod;
+	SLIST_INSERT_HEAD(&nfsiodhead, nip, ni_list);
 	mtx_unlock(&nfs_iod_mtx);
-	error = kproc_create(nfssvc_iod, nfs_asyncdaemon + i, NULL, RFHIGHPID,
-	    0, "nfsiod %d", newiod);
+	printf("new: nip: %p\n", nip);
+	printf("new: ni_inst: %p\n", nip->ni_inst);
+	printf("new: ni_iod: %d\n", nip->ni_iod);
+	taskqueue_enqueue(taskqueue_thread, &nfs_nfsiodnew_task);
 	mtx_lock(&nfs_iod_mtx);
-	if (error) {
-		if (set_iodwant > 0)
-			nfs_iodwant[i] = NFSIOD_NOT_AVAILABLE;
-		return (-1);
+	error = nip->ni_error;
+	SLIST_FOREACH_SAFE(nip, &nfsiodhead, ni_list, nip_temp) {
+		if (nip->ni_busy != 0 && nip->ni_done != 0) {
+			SLIST_REMOVE(&nfsiodhead, nip, nfsiod_str, ni_list);
+			free(nip, M_TEMP);
+			break;
+		}
 	}
+	if (error)
+		goto fail;
 	nfs_numasync++;
 	return (newiod);
+fail:
+	if (set_iodwant > 0)
+		nfs_iodwant[i] = NFSIOD_NOT_AVAILABLE;
+	return (-1);
 }
 
 static void
@@ -231,6 +283,8 @@
 
 	mtx_lock(&nfs_iod_mtx);
 	myiod = (int *)instance - nfs_asyncdaemon;
+	printf("instance: %p\n", instance);
+	printf("myiod: %d\n", myiod);
 	/*
 	 * Main loop
 	 */
Index: sys/nfsclient/nfs.h
===================================================================
--- sys/nfsclient/nfs.h	(revision 211279)
+++ sys/nfsclient/nfs.h	(working copy)
@@ -125,6 +125,7 @@
 
 extern struct nfsstats nfsstats;
 extern struct mtx nfs_iod_mtx;
+extern struct task nfs_nfsiodnew_task;
 
 extern int nfs_numasync;
 extern unsigned int nfs_iodmax;
@@ -253,6 +254,7 @@
 	    struct ucred *cred, struct thread *td);
 int	nfs_readdirrpc(struct vnode *, struct uio *, struct ucred *);
 int	nfs_nfsiodnew(int);
+void	nfs_nfsiodnew_tq(__unused void *, 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 *);
_______________________________________________
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Reply via email to