Properly stop kernel threads on aic7xxx

Properly stop kernel threads on aic7xxx

[Pavel Machek]

Hi!

This is totally untested patch that should make aic7xxx one step
closer to working with software suspend... Plus it kills ugly #if in
the process.
								Pavel 

--- tmp/linux/drivers/scsi/aic7xxx/aic79xx_osm.c	2004-03-11 18:11:12.000000000 +0100
+++ linux/drivers/scsi/aic7xxx/aic79xx_osm.c	2004-04-01 19:01:29.000000000 +0200
@@ -2581,17 +2581,8 @@
 	 * Complete thread creation.
 	 */
 	lock_kernel();
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,5,60)
-	/*
-	 * Don't care about any signals.
-	 */
-	siginitsetinv(&current->blocked, 0);
-
-	daemonize();
-	sprintf(current->comm, "ahd_dv_%d", ahd->unit);
-#else
 	daemonize("ahd_dv_%d", ahd->unit);
-#endif
+	current->flags |= PF_IOTHREAD;
 	unlock_kernel();
 
 	while (1) {
--- tmp/linux/drivers/scsi/aic7xxx/aic7xxx_osm.c	2004-03-11 18:11:12.000000000 +0100
+++ linux/drivers/scsi/aic7xxx/aic7xxx_osm.c	2004-04-01 19:01:08.000000000 +0200
@@ -2286,17 +2286,8 @@
 	 * Complete thread creation.
 	 */
 	lock_kernel();
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,5,0)
-	/*
-	 * Don't care about any signals.
-	 */
-	siginitsetinv(&current->blocked, 0);
-
-	daemonize();
-	sprintf(current->comm, "ahc_dv_%d", ahc->unit);
-#else
 	daemonize("ahc_dv_%d", ahc->unit);
-#endif
+	current->flags |= PF_IOTHREAD;
 	unlock_kernel();
 
 	while (1) {


-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: Properly stop kernel threads on aic7xxx

[Andrea Arcangeli]

On Thu, Apr 01, 2004 at 07:08:08PM +0200, Pavel Machek wrote:
> Hi!
> 
> This is totally untested patch that should make aic7xxx one step
> closer to working with software suspend... Plus it kills ugly #if in
> the process.
> 								Pavel 
> 
> --- tmp/linux/drivers/scsi/aic7xxx/aic79xx_osm.c	2004-03-11 18:11:12.000000000 +0100
> +++ linux/drivers/scsi/aic7xxx/aic79xx_osm.c	2004-04-01 19:01:29.000000000 +0200
> @@ -2581,17 +2581,8 @@
>  	 * Complete thread creation.
>  	 */
>  	lock_kernel();
> -#if LINUX_VERSION_CODE < KERNEL_VERSION(2,5,60)
> -	/*
> -	 * Don't care about any signals.
> -	 */
> -	siginitsetinv(&current->blocked, 0);
> -
> -	daemonize();
> -	sprintf(current->comm, "ahd_dv_%d", ahd->unit);
> -#else
>  	daemonize("ahd_dv_%d", ahd->unit);
> -#endif
> +	current->flags |= PF_IOTHREAD;
>  	unlock_kernel();
>  
>  	while (1) {
> --- tmp/linux/drivers/scsi/aic7xxx/aic7xxx_osm.c	2004-03-11 18:11:12.000000000 +0100
> +++ linux/drivers/scsi/aic7xxx/aic7xxx_osm.c	2004-04-01 19:01:08.000000000 +0200
> @@ -2286,17 +2286,8 @@
>  	 * Complete thread creation.
>  	 */
>  	lock_kernel();
> -#if LINUX_VERSION_CODE < KERNEL_VERSION(2,5,0)
> -	/*
> -	 * Don't care about any signals.
> -	 */
> -	siginitsetinv(&current->blocked, 0);
> -
> -	daemonize();
> -	sprintf(current->comm, "ahc_dv_%d", ahc->unit);
> -#else
>  	daemonize("ahc_dv_%d", ahc->unit);
> -#endif
> +	current->flags |= PF_IOTHREAD;
>  	unlock_kernel();
>  
>  	while (1) {

This fixed the hang, I can reproduce the oops now.

Pavel the current CVS would work fine if only you wouldn't try to do I/O
on compound pages via rw_page_swap_sync. That thing now collides with
anon-vma (though it should work fine if you would backout anon-vma since
the rw_page_swap_sync wouldn't touch page->private anymore).

I'm not exactly sure how to fix this collision right now. there are
various ways, probably something that will work perfectly is to just clear
PG_compound in rw_swap_page_sync and to set it back before returning (if
it was set at the entry point), though I'd take it as last resort...
thinking about it. Comments welcome.

I'm also unsure why _all_ multipage allocations really need this
compound thing setup and why can't the owner of the page take care of
the refcounting itself by always using the head page. I may actually
add a GFP bitflag asking for a multipage but w/o a compound setup. There
are million ways to fix this, none of which is obvious.

Re: Properly stop kernel threads on aic7xxx

[Andrew Morton]

Andrea Arcangeli <andrea@suse.de> wrote:
>
> I'm also unsure why _all_ multipage allocations really need this
> compound thing setup and why can't the owner of the page take care of
> the refcounting itself by always using the head page. I may actually
> add a GFP bitflag asking for a multipage but w/o a compound setup. There
> are million ways to fix this, none of which is obvious.

For direct-io into higher-order pages.  When doing direct-io into a hugetlb
page we need to make sure that get_user_pages() pins the correct pageframe.

Possibly we need to turn it on regardless of CONFIG_HUGETLBPAGE for people
who want to do direct-io or PEEKTEXT/POKETEXT into mmapped soundcard
buffers, for example.  Perhaps there's a race which could permit direct-io
to write to a freed page via this route..

Davem had some reason why he might want to turn on the compound logic
permanently - related to TCP access to/from higher-order pages, I think.

Re: Properly stop kernel threads on aic7xxx

[Andrea Arcangeli]

On Thu, Apr 01, 2004 at 05:04:03PM -0800, Andrew Morton wrote:
> Andrea Arcangeli <andrea@suse.de> wrote:
> >
> > I'm also unsure why _all_ multipage allocations really need this
> > compound thing setup and why can't the owner of the page take care of
> > the refcounting itself by always using the head page. I may actually
> > add a GFP bitflag asking for a multipage but w/o a compound setup. There
> > are million ways to fix this, none of which is obvious.
> 
> For direct-io into higher-order pages.  When doing direct-io into a hugetlb
> page we need to make sure that get_user_pages() pins the correct pageframe.

we do the same in bigpages 2.4 just fine with a basic math on the page_t
knowing it's 2M naturally aligned, very easy to find the head page on
vmas with VM_HUGETLBFS.

> Possibly we need to turn it on regardless of CONFIG_HUGETLBPAGE for people
> who want to do direct-io or PEEKTEXT/POKETEXT into mmapped soundcard
> buffers, for example.  Perhaps there's a race which could permit direct-io
> to write to a freed page via this route..
> 
> Davem had some reason why he might want to turn on the compound logic
> permanently - related to TCP access to/from higher-order pages, I think.

currently it's turned on only with HUGTLBFS=y though.