hello,
I have found a problem in scsi.c which in present in the 2.2 and 2.4
series. the scsi error handler thread is created with:
kernel_thread((int (*)(void *)) scsi_error_handler,
(void *) shpnt, 0);
This will lead to problems, when you have to umount the filesystem on
which the scsi-hostapter module is located.
To solve to problem I would propose to change this to:
kernel_thread((int (*)(void *)) scsi_error_handler,
(void *) shpnt, CLONE_FILES);
Bye,
-- Andreas Klein
[email protected]
root / webmaster @cip.physik.uni-wuerzburg.de
root / webmaster @http://www.physik.uni-wuerzburg.de
_____________________________________
| |
| Long live our gracious AMIGA! |
|___________________________________|
On Thu, 7 Dec 2000, Andreas Klein wrote:
> hello,
>
> I have found a problem in scsi.c which in present in the 2.2 and 2.4
> series. the scsi error handler thread is created with:
>
> kernel_thread((int (*)(void *)) scsi_error_handler,
> (void *) shpnt, 0);
>
> This will lead to problems, when you have to umount the filesystem on
> which the scsi-hostapter module is located.
> To solve to problem I would propose to change this to:
>
> kernel_thread((int (*)(void *)) scsi_error_handler,
> (void *) shpnt, CLONE_FILES);
Hi Andreas,
Unfortunately, CLONE_FILES is not enough because the module may be loaded
from the directory containing it, i.e. the thread's cwd may point to that
filesystem and that would keep it busy. Or-ing CLONE_FS into flags
wouldn't help either...
A proper way to release the references to resources is to call daemonize()
function from within the kernel thread function, which calls
exit_fs()/exit_files() internally.
Regards,
Tigran
On Thu, 7 Dec 2000, Andreas Klein wrote:
> > A proper way to release the references to resources is to call daemonize()
> > function from within the kernel thread function, which calls
> > exit_fs()/exit_files() internally.
>
> Nearly correct, the daemonize function does NOT call exit_files.
I do not post messages to linux-kernel without checking the facts
first. Read the daemonize() function and see for yourself that you are
wrong.
Regards,
Tigran
PS, Here it is, to save you time opening kernel/sched.c. The kernel is, of
course, test12-pre7.
/*
* Put all the gunge required to become a kernel thread without
* attached user resources in one place where it belongs.
*/
void daemonize(void)
{
struct fs_struct *fs;
/*
* If we were started as result of loading a module, close all of
the
* user space pages. We don't need them, and if we didn't close
them
* they would be locked into memory.
*/
exit_mm(current);
current->session = 1;
current->pgrp = 1;
/* Become as one with the init task */
exit_fs(current); /* current->fs->count--; */
fs = init_task.fs;
current->fs = fs;
atomic_inc(&fs->count);
exit_files(current);
current->files = init_task.files;
atomic_inc(¤t->files->count);
}
> > > A proper way to release the references to resources is to call daemonize()
> > > function from within the kernel thread function, which calls
> > > exit_fs()/exit_files() internally.
> >
> > Nearly correct, the daemonize function does NOT call exit_files.
>
> I do not post messages to linux-kernel without checking the facts
> first. Read the daemonize() function and see for yourself that you are
> wrong.
Andreas is looking at a slightly older kernel, and was right for that. Every
caller to daemonize either then did the file stuff or needed to and forgot
so I fixed daemonize
On Thu, 7 Dec 2000, Tigran Aivazian wrote:
> PS, Here it is, to save you time opening kernel/sched.c. The kernel is, of
> course, test12-pre7.
~~~~~~~~~~~
Before you tell me "it was not so in the earlier versions!" I am tempted
to quote a famous russian proverb
"whosoever remembereth the past shall have his eye plucked out"
which, paraphrased into Linux kernel development would sound like:
"whosoever keepeth Linux kernel trees under CVS, his disk shall rot"
;)
Tigran
On Thu, 7 Dec 2000, Alan Cox wrote:
> > > > A proper way to release the references to resources is to call daemonize()
> > > > function from within the kernel thread function, which calls
> > > > exit_fs()/exit_files() internally.
> > >
> > > Nearly correct, the daemonize function does NOT call exit_files.
> >
> > I do not post messages to linux-kernel without checking the facts
> > first. Read the daemonize() function and see for yourself that you are
> > wrong.
>
> Andreas is looking at a slightly older kernel, and was right for that. Every
> caller to daemonize either then did the file stuff or needed to and forgot
> so I fixed daemonize
Yes, yes, Alan, I do know that. I just took it for granted that the
correctness of the statement when applied to the latest kernel should not
upset someone who is looking at the older version so me calling someone
"wrong" is not a strong offensive term, just a mild thing saying "guess
what -- things changed". Just trying to exercise the human mind to get
used to quick changes in the Linux world -- that is all :)
Regards,
Tigran
On Thu, 7 Dec 2000, Tigran Aivazian wrote:
> On Thu, 7 Dec 2000, Andreas Klein wrote:
>
> > hello,
> >
> > I have found a problem in scsi.c which in present in the 2.2 and 2.4
> > series. the scsi error handler thread is created with:
> >
> > kernel_thread((int (*)(void *)) scsi_error_handler,
> > (void *) shpnt, 0);
> >
> > This will lead to problems, when you have to umount the filesystem on
> > which the scsi-hostapter module is located.
> > To solve to problem I would propose to change this to:
> >
> > kernel_thread((int (*)(void *)) scsi_error_handler,
> > (void *) shpnt, CLONE_FILES);
>
> Hi Andreas,
>
> Unfortunately, CLONE_FILES is not enough because the module may be loaded
> from the directory containing it, i.e. the thread's cwd may point to that
> filesystem and that would keep it busy. Or-ing CLONE_FS into flags
> wouldn't help either...
Yes, you are right with that.
> A proper way to release the references to resources is to call daemonize()
> function from within the kernel thread function, which calls
> exit_fs()/exit_files() internally.
Nearly correct, the daemonize function does NOT call exit_files. This has
to be done manually. Looking at the 2.4.0-test10 source I saw, that
someone has already fixed the problem by calling exit_files and daemonize.
In the 2.2 series someone tried cut-copy-paste programing from the
daemonize function, but exit_files was forgotten. The following patch
should fix the problem for 2.2.16, while leaving scsi.c untouched.
--- linux/drivers/scsi/scsi_error.c.orig Thu Dec 7 23:56:47 2000
+++ linux/drivers/scsi/scsi_error.c Fri Dec 8 00:13:20 2000
@@ -1935,6 +1935,7 @@
* user space pages. We don't need them, and if we didn't close
them
* they would be locked into memory.
*/
+ exit_files(current);
exit_mm(current);
current->session = 1;
Bye,
-- Andreas Klein
[email protected]
root / webmaster @cip.physik.uni-wuerzburg.de
root / webmaster @http://www.physik.uni-wuerzburg.de
_____________________________________
| |
| Long live our gracious AMIGA! |
|___________________________________|
> In sched.c, function daemonize, line 1216 you call exit_mm.
Yep
> time, it has to segvault. If I am not wrong at this point CLONE_VM simply
> has to be removed from kernel_thread. The kernel-thread will free his mem
> in daemonize (calling exit_mm) and the user-space-application will free
> the mem when exiting.
Providing the use counter is being bumped properly it wont go away any more
than if a thread of a user space app exits before the others as far as I can
see
On Thu, 7 Dec 2000, Alan Cox wrote:
> Andreas is looking at a slightly older kernel, and was right for that. Every
> caller to daemonize either then did the file stuff or needed to and forgot
> so I fixed daemonize
I think, there ist still a small bug.
(This time I even checked 2.4.0-test12-pre8)
In linux/arch/i386/kernel/process.c, function kernel_thread, line 453 the
flag CLONE_VM is always used.
In sched.c, function daemonize, line 1216 you call exit_mm.
Since the memory is cloned, you will take away the mem from your
user-space-application as well. So if insmod is already running at that
time, it has to segvault. If I am not wrong at this point CLONE_VM simply
has to be removed from kernel_thread. The kernel-thread will free his mem
in daemonize (calling exit_mm) and the user-space-application will free
the mem when exiting.
Bye,
-- Andreas Klein
[email protected]
root / webmaster @cip.physik.uni-wuerzburg.de
root / webmaster @http://www.physik.uni-wuerzburg.de
_____________________________________
| |
| Long live our gracious AMIGA! |
|___________________________________|