diff mbox

tcmu: Oops in unmap_thread_fn()

Message ID 20170801200917.brigs5x47eujfx7a@mwanda (mailing list archive)
State New, archived
Headers show

Commit Message

Dan Carpenter Aug. 1, 2017, 8:09 p.m. UTC
Calling list_del() on the iterator pointer in list_for_each_entry() will
cause an oops.  We need to user the _safe() version for that.

Fixes: c73d02f63c16 ("tcmu: Add fifo type waiter list support to avoid starvation")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Xiubo Li Aug. 2, 2017, 3:25 a.m. UTC | #1
On 2017年08月02日 04:09, Dan Carpenter wrote:
> Calling list_del() on the iterator pointer in list_for_each_entry() will
> cause an oops.  We need to user the _safe() version for that.
>
> Fixes: c73d02f63c16 ("tcmu: Add fifo type waiter list support to avoid starvation")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c
> index 9258b7dd2c30..fd9fcea68d23 100644
> --- a/drivers/target/target_core_user.c
> +++ b/drivers/target/target_core_user.c
> @@ -1985,7 +1985,7 @@ static struct target_backend_ops tcmu_ops = {
>   
>   static int unmap_thread_fn(void *data)
>   {
> -	struct tcmu_dev *udev;
> +	struct tcmu_dev *udev, *tmp;
>   	loff_t off;
>   	uint32_t start, end, block;
>   	static uint32_t free_blocks;
> @@ -2056,7 +2056,7 @@ static int unmap_thread_fn(void *data)
>   		 * for the global data pool blocks.
>   		 */
>   		mutex_lock(&root_udev_waiter_mutex);
> -		list_for_each_entry(udev, &root_udev_waiter, waiter) {
> +		list_for_each_entry_safe(udev, tmp, &root_udev_waiter, waiter) {
>   			mutex_lock(&udev->cmdr_lock);
>   			if (udev->waiting_blocks < free_blocks) {
>   				mutex_unlock(&udev->cmdr_lock);
Good catch and it looks nice for me.

Thanks,

BRs
Xiubo


--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mike Christie Aug. 3, 2017, 5:35 p.m. UTC | #2
On 08/01/2017 03:09 PM, Dan Carpenter wrote:
> Calling list_del() on the iterator pointer in list_for_each_entry() will
> cause an oops.  We need to user the _safe() version for that.
> 
> Fixes: c73d02f63c16 ("tcmu: Add fifo type waiter list support to avoid starvation")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c
> index 9258b7dd2c30..fd9fcea68d23 100644
> --- a/drivers/target/target_core_user.c
> +++ b/drivers/target/target_core_user.c
> @@ -1985,7 +1985,7 @@ static struct target_backend_ops tcmu_ops = {
>  
>  static int unmap_thread_fn(void *data)
>  {
> -	struct tcmu_dev *udev;
> +	struct tcmu_dev *udev, *tmp;
>  	loff_t off;
>  	uint32_t start, end, block;
>  	static uint32_t free_blocks;
> @@ -2056,7 +2056,7 @@ static int unmap_thread_fn(void *data)
>  		 * for the global data pool blocks.
>  		 */
>  		mutex_lock(&root_udev_waiter_mutex);
> -		list_for_each_entry(udev, &root_udev_waiter, waiter) {
> +		list_for_each_entry_safe(udev, tmp, &root_udev_waiter, waiter) {
>  			mutex_lock(&udev->cmdr_lock);
>  			if (udev->waiting_blocks < free_blocks) {
>  				mutex_unlock(&udev->cmdr_lock);
> 

Thanks.

Reviewed-by: Mike Christie <mchristi@redhat.com>
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Nicholas A. Bellinger Aug. 6, 2017, 11:27 p.m. UTC | #3
On Tue, 2017-08-01 at 23:09 +0300, Dan Carpenter wrote:
> Calling list_del() on the iterator pointer in list_for_each_entry() will
> cause an oops.  We need to user the _safe() version for that.
> 
> Fixes: c73d02f63c16 ("tcmu: Add fifo type waiter list support to avoid starvation")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 

Applied to target-pending/for-next.

Thanks DanC.

--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bryant G. Ly Nov. 8, 2017, 10:41 p.m. UTC | #4
On 8/6/17 6:27 PM, Nicholas A. Bellinger wrote:
> On Tue, 2017-08-01 at 23:09 +0300, Dan Carpenter wrote:
>> Calling list_del() on the iterator pointer in list_for_each_entry() will
>> cause an oops.  We need to user the _safe() version for that.
>>
>> Fixes: c73d02f63c16 ("tcmu: Add fifo type waiter list support to avoid starvation")
>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>>
> Applied to target-pending/for-next.
>
> Thanks DanC.
>
> --
> To unsubscribe from this list: send the line "unsubscribe target-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
Also missing.

-Bryant

--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c
index 9258b7dd2c30..fd9fcea68d23 100644
--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c
@@ -1985,7 +1985,7 @@  static struct target_backend_ops tcmu_ops = {
 
 static int unmap_thread_fn(void *data)
 {
-	struct tcmu_dev *udev;
+	struct tcmu_dev *udev, *tmp;
 	loff_t off;
 	uint32_t start, end, block;
 	static uint32_t free_blocks;
@@ -2056,7 +2056,7 @@  static int unmap_thread_fn(void *data)
 		 * for the global data pool blocks.
 		 */
 		mutex_lock(&root_udev_waiter_mutex);
-		list_for_each_entry(udev, &root_udev_waiter, waiter) {
+		list_for_each_entry_safe(udev, tmp, &root_udev_waiter, waiter) {
 			mutex_lock(&udev->cmdr_lock);
 			if (udev->waiting_blocks < free_blocks) {
 				mutex_unlock(&udev->cmdr_lock);