target: don't call an unmap callback if a range length is zero

Submitted by Andrei Vagin on Dec. 13, 2017, 9:23 p.m.

Details

Message ID 1513200205-5283-1-git-send-email-avagin@openvz.org
State New
Series "target: don't call an unmap callback if a range length is zero"
Headers show

Commit Message

Andrei Vagin Dec. 13, 2017, 9:23 p.m.
If a length of a range is zero, it means there is nothing to unmap
and we can skip this range.

Here is one more reason, why we have to skip such ranges.  An unmap
callback calls file_operations->fallocate(), but the man page for the
fallocate syscall says that fallocate(fd, mode, offset, let) returns
EINVAL, if len is zero. It means that file_operations->fallocate() isn't
obligated to handle zero ranges too.

Cc: Alexey.Kuznetsov@acronis.com
---
 drivers/target/target_core_sbc.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c
index 59a1235..99fb25f 100644
--- a/drivers/target/target_core_sbc.c
+++ b/drivers/target/target_core_sbc.c
@@ -1132,9 +1132,11 @@  sbc_execute_unmap(struct se_cmd *cmd,
 			goto err;
 		}
 
-		ret = do_unmap_fn(cmd, priv, lba, range);
-		if (ret)
-			goto err;
+		if (range) {
+			ret = do_unmap_fn(cmd, priv, lba, range);
+			if (ret)
+				goto err;
+		}
 
 		ptr += 16;
 		size -= 16;

Comments

Konstantin Khorenko Dec. 19, 2017, 12:41 p.m.
i've filed a bug for this issue: https://jira.sw.ru/browse/PSBM-79534

Andrey, please, post the usecase when we faced fallocating 0 range - to the bug at least,
otherwise in a year we'll forget about the reason completely.

Thank you.

--
Best regards,

Konstantin Khorenko,
Virtuozzo Linux Kernel Team

On 12/14/2017 12:23 AM, Andrei Vagin wrote:
> If a length of a range is zero, it means there is nothing to unmap
> and we can skip this range.
>
> Here is one more reason, why we have to skip such ranges.  An unmap
> callback calls file_operations->fallocate(), but the man page for the
> fallocate syscall says that fallocate(fd, mode, offset, let) returns
> EINVAL, if len is zero. It means that file_operations->fallocate() isn't
> obligated to handle zero ranges too.
>
> Cc: Alexey.Kuznetsov@acronis.com
> ---
>  drivers/target/target_core_sbc.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c
> index 59a1235..99fb25f 100644
> --- a/drivers/target/target_core_sbc.c
> +++ b/drivers/target/target_core_sbc.c
> @@ -1132,9 +1132,11 @@ sbc_execute_unmap(struct se_cmd *cmd,
>  			goto err;
>  		}
>
> -		ret = do_unmap_fn(cmd, priv, lba, range);
> -		if (ret)
> -			goto err;
> +		if (range) {
> +			ret = do_unmap_fn(cmd, priv, lba, range);
> +			if (ret)
> +				goto err;
> +		}
>
>  		ptr += 16;
>  		size -= 16;
>