diff mbox series

loop: Call loop_config_discard() only after new config is applied.

Message ID 20200331114116.21642-1-maco@android.com (mailing list archive)
State New, archived
Headers show
Series loop: Call loop_config_discard() only after new config is applied. | expand

Commit Message

Martijn Coenen March 31, 2020, 11:41 a.m. UTC
loop_set_status() calls loop_config_discard() to configure discard for
the loop device; however, the discard configuration depends on whether
the loop device uses encryption, and when we call it the encryption
configuration has not been updated yet. Move the call down so we apply
the correct discard configuration based on the new configuration.

Signed-off-by: Martijn Coenen <maco@android.com>
---
 drivers/block/loop.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Martijn Coenen April 14, 2020, 8:13 a.m. UTC | #1
Folks, any thoughts about this one?

Thanks,
Martijn

On Tue, Mar 31, 2020 at 1:41 PM Martijn Coenen <maco@android.com> wrote:
>
> loop_set_status() calls loop_config_discard() to configure discard for
> the loop device; however, the discard configuration depends on whether
> the loop device uses encryption, and when we call it the encryption
> configuration has not been updated yet. Move the call down so we apply
> the correct discard configuration based on the new configuration.
>
> Signed-off-by: Martijn Coenen <maco@android.com>
> ---
>  drivers/block/loop.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> index 739b372a5112..7c9dcb6007a6 100644
> --- a/drivers/block/loop.c
> +++ b/drivers/block/loop.c
> @@ -1312,8 +1312,6 @@ loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
>                 }
>         }
>
> -       loop_config_discard(lo);
> -
>         memcpy(lo->lo_file_name, info->lo_file_name, LO_NAME_SIZE);
>         memcpy(lo->lo_crypt_name, info->lo_crypt_name, LO_NAME_SIZE);
>         lo->lo_file_name[LO_NAME_SIZE-1] = 0;
> @@ -1337,6 +1335,8 @@ loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
>                 lo->lo_key_owner = uid;
>         }
>
> +       loop_config_discard(lo);
> +
>         /* update dio if lo_offset or transfer is changed */
>         __loop_update_dio(lo, lo->use_dio);
>
> --
> 2.26.0.rc2.310.g2932bb562d-goog
>
Christoph Hellwig April 14, 2020, 8:16 a.m. UTC | #2
On Tue, Mar 31, 2020 at 01:41:16PM +0200, Martijn Coenen wrote:
> loop_set_status() calls loop_config_discard() to configure discard for
> the loop device; however, the discard configuration depends on whether
> the loop device uses encryption, and when we call it the encryption
> configuration has not been updated yet. Move the call down so we apply
> the correct discard configuration based on the new configuration.
> 
> Signed-off-by: Martijn Coenen <maco@android.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>
Bob Liu April 17, 2020, 12:19 a.m. UTC | #3
On 3/31/20 7:41 PM, Martijn Coenen wrote:
> loop_set_status() calls loop_config_discard() to configure discard for
> the loop device; however, the discard configuration depends on whether
> the loop device uses encryption, and when we call it the encryption
> configuration has not been updated yet. Move the call down so we apply
> the correct discard configuration based on the new configuration.
> 
> Signed-off-by: Martijn Coenen <maco@android.com>

Looks fine to me.
Reviewed-by: Bob Liu <bob.liu@oracle.com>

> ---
>  drivers/block/loop.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> index 739b372a5112..7c9dcb6007a6 100644
> --- a/drivers/block/loop.c
> +++ b/drivers/block/loop.c
> @@ -1312,8 +1312,6 @@ loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
>  		}
>  	}
>  
> -	loop_config_discard(lo);
> -
>  	memcpy(lo->lo_file_name, info->lo_file_name, LO_NAME_SIZE);
>  	memcpy(lo->lo_crypt_name, info->lo_crypt_name, LO_NAME_SIZE);
>  	lo->lo_file_name[LO_NAME_SIZE-1] = 0;
> @@ -1337,6 +1335,8 @@ loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
>  		lo->lo_key_owner = uid;
>  	}
>  
> +	loop_config_discard(lo);
> +
>  	/* update dio if lo_offset or transfer is changed */
>  	__loop_update_dio(lo, lo->use_dio);
>  
>
Bart Van Assche April 18, 2020, 3:46 p.m. UTC | #4
On 2020-03-31 04:41, Martijn Coenen wrote:
> loop_set_status() calls loop_config_discard() to configure discard for
> the loop device; however, the discard configuration depends on whether
> the loop device uses encryption, and when we call it the encryption
> configuration has not been updated yet. Move the call down so we apply
> the correct discard configuration based on the new configuration.

Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Martijn Coenen May 4, 2020, 7:32 p.m. UTC | #5
Hi Jens,

Are you ok with this one? One of my later series depends on it, but so
far I've kept it separate because it's a bug fix.

Thanks,
Martijn

On Sat, Apr 18, 2020 at 5:46 PM Bart Van Assche <bvanassche@acm.org> wrote:
>
> On 2020-03-31 04:41, Martijn Coenen wrote:
> > loop_set_status() calls loop_config_discard() to configure discard for
> > the loop device; however, the discard configuration depends on whether
> > the loop device uses encryption, and when we call it the encryption
> > configuration has not been updated yet. Move the call down so we apply
> > the correct discard configuration based on the new configuration.
>
> Reviewed-by: Bart Van Assche <bvanassche@acm.org>
diff mbox series

Patch

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 739b372a5112..7c9dcb6007a6 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1312,8 +1312,6 @@  loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
 		}
 	}
 
-	loop_config_discard(lo);
-
 	memcpy(lo->lo_file_name, info->lo_file_name, LO_NAME_SIZE);
 	memcpy(lo->lo_crypt_name, info->lo_crypt_name, LO_NAME_SIZE);
 	lo->lo_file_name[LO_NAME_SIZE-1] = 0;
@@ -1337,6 +1335,8 @@  loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
 		lo->lo_key_owner = uid;
 	}
 
+	loop_config_discard(lo);
+
 	/* update dio if lo_offset or transfer is changed */
 	__loop_update_dio(lo, lo->use_dio);