Re: please revert kthread from loop.c

[Andrew Morton]

"Serge E. Hallyn" <serue@xxxxxxxxxx> wrote:
>
> Convert loop.c from the deprecated kernel_thread to kthread.
>

I think you have a racelet here:

> +		}
>  		spin_unlock_irq(&lo->lo_lock);
>  
> -		BUG_ON(!bio);
> -		loop_handle_bio(lo, bio);
> -
> -		/*
> -		 * upped both for pending work and tear-down, lo_pending
> -		 * will hit zero then
> -		 */
> -		if (unlikely(!pending))
> -			break;
> +		__set_current_state(TASK_INTERRUPTIBLE);
> +		schedule();
>  	}
>  
> -	complete(&lo->lo_done);
>  	return 0;
>  }


:		if (kthread_should_stop()) {
:			spin_unlock_irq(&lo->lo_lock);
:			break;
:		}
:		spin_unlock_irq(&lo->lo_lock);
:
:		__set_current_state(TASK_INTERRUPTIBLE);
:		schedule();
:

If the wake_up_process() is delivered before the __set_current_state(),
we'll miss the wakeup.

If so, this should plug it.  The same race is not possible against the
loop_set_fd() wakeup because the thread isn't running at that stage, yes?


diff -puN drivers/block/loop.c~kthread-convert-loopc-to-kthread-race-fix drivers/block/loop.c
--- a/drivers/block/loop.c~kthread-convert-loopc-to-kthread-race-fix
+++ a/drivers/block/loop.c
@@ -525,8 +525,8 @@ static int loop_make_request(request_que
 		goto out;
 	lo->lo_pending++;
 	loop_add_bio(lo, old_bio);
-	spin_unlock_irq(&lo->lo_lock);
 	wake_up_process(lo->lo_thread);
+	spin_unlock_irq(&lo->lo_lock);
 	return 0;
 
 out:
@@ -600,9 +600,8 @@ static int loop_thread(void *data)
 			spin_unlock_irq(&lo->lo_lock);
 			break;
 		}
-		spin_unlock_irq(&lo->lo_lock);
-
 		__set_current_state(TASK_INTERRUPTIBLE);
+		spin_unlock_irq(&lo->lo_lock);
 		schedule();
 	}
 
_

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/