Re: [Scst-devel] [PATCH] scst_fileio thread/module shutdown

[Vladislav B. <vs...@vl...>]

Probably, this thread http://lkml.org/lkml/2005/11/9/162 could give a 
bit more info about kernel threads at module unload time.

Vladislav Bolkhovitin wrote:
> Thanks for the patch, but I think more correct (not only from this point 
> of view) way will be to convert SCST threads to kthread API. It's not 
> much harder than implementing the completitions.
> 
> In any case your patch also randomly crashed for me on rmmod as all my 
> previous attempts to handle this issue more greacefully, than waiting 1 
> sec. The problem is that the module could get unloaded and its memory 
> freed before the thread(s) finish(es) its final tasks after leaving its 
> working func. I was told, than kthreads can handle this.
> 
> Vlad
> 
> Arne Redlich wrote:
> 
>>Use a completion instead of waiting an arbitrary amount of time (1s) to
>>make sure the threads are really gone.
>>
>>Only mildly tested, but I didn't run into any troubles. Mostly because
>>the threads usually exited quite early, i.e. the while(atomic_read())
>>loop wasn't even entered.
>>
>>Any feedback is welcome.
>>
>>Signed-off-by: Arne Redlich <arn...@xi...>
>>
>>Index: src/dev_handlers/scst_fileio.c
>>===================================================================
>>RCS file: /cvsroot/scst/src/dev_handlers/scst_fileio.c,v
>>retrieving revision 1.29
>>diff -a -u -p -r1.29 scst_fileio.c
>>--- src/dev_handlers/scst_fileio.c	10 Jun 2006 16:05:28 -0000	1.29
>>+++ src/dev_handlers/scst_fileio.c	2 Jul 2006 18:58:54 -0000
>>@@ -110,6 +110,7 @@ struct scst_fileio_tgt_dev {
>> 	atomic_t threads_count;
>> 	struct semaphore shutdown_mutex;
>> 	struct list_head ftgt_list_entry;
>>+	struct completion done;
>> };
>> 
>> static int fileio_attach(struct scst_device *dev);
>>@@ -582,6 +583,7 @@ static int fileio_cmd_thread(void *arg)
>> 		up(&ftgt_dev->shutdown_mutex);
>> 	}
>> 
>>+	complete(&ftgt_dev->done);
>> 	TRACE_EXIT();
>> 	return 0;
>> }
>>@@ -610,6 +612,7 @@ static int fileio_attach_tgt(struct scst
>> 	init_waitqueue_head(&ftgt_dev->fdev_waitQ);
>> 	atomic_set(&ftgt_dev->threads_count, 0);
>> 	init_MUTEX_LOCKED(&ftgt_dev->shutdown_mutex);
>>+	init_completion(&ftgt_dev->done);
>> 
>> 	ftgt_dev->fd = fileio_open(virt_dev);
>> 	if (IS_ERR(ftgt_dev->fd)) {
>>@@ -668,6 +671,9 @@ static void fileio_detach_tgt(struct scs
>> 	wake_up_all(&ftgt_dev->fdev_waitQ);
>> 	down(&ftgt_dev->shutdown_mutex);
>> 
>>+	while (atomic_read(&ftgt_dev->threads_count))
>>+		wait_for_completion(&ftgt_dev->done);
>>+
>> 	filp_close(ftgt_dev->fd, NULL);
>> 
>> 	if (ftgt_dev->iv != NULL) {
>>@@ -3007,17 +3013,6 @@ static void __exit exit_scst_fileio_driv
>> {
>> 	exit_scst_fileio(&disk_devtype_fileio, &disk_fileio_dev_list);
>> 	exit_scst_fileio(&cdrom_devtype_fileio, &cdrom_fileio_dev_list);
>>-
>>-	/* 
>>-	 * Wait for one sec. to allow the thread(s) actually exit,
>>-	 * otherwise we can get Oops. Any better way?
>>-	 */
>>-	{
>>-		unsigned long t = jiffies;
>>-		TRACE_DBG("%s", "Waiting 1 sec...");
>>-		while ((jiffies - t) < HZ)
>>-			schedule();
>>-	}
>> }
>> 
>> module_init(init_scst_fileio_driver);
>>
>>
>>
>>Using Tomcat but need to do more? Need to support web services, security?
>>Get stuff done quickly with pre-integrated technology to make your job easier
>>Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
>>http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
>>_______________________________________________
>>Scst-devel mailing list
>>Scs...@li...
>>https://lists.sourceforge.net/lists/listinfo/scst-devel
>>
> 
> 
> 
> Using Tomcat but need to do more? Need to support web services, security?
> Get stuff done quickly with pre-integrated technology to make your job easier
> Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
> http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
> _______________________________________________
> Scst-devel mailing list
> Scs...@li...
> https://lists.sourceforge.net/lists/listinfo/scst-devel
>