Re: bug in scsi.c
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 @www.physik.uni-wuerzburg.de _ | | | Long live our gracious AMIGA! | |___| - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: bug in scsi.c
> 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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: bug in scsi.c
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.origThu 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 @www.physik.uni-wuerzburg.de _ | | | Long live our gracious AMIGA! | |___| - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: bug in scsi.c
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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: bug in scsi.c
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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: bug in scsi.c
> > > 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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: bug in scsi.c
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); } - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: bug in scsi.c
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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
bug in scsi.c
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 @www.physik.uni-wuerzburg.de _ | | | Long live our gracious AMIGA! | |___| - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/