* [PATCH v2] ltp/fsx: avoid infinite loop while finding offset2 in clone/dedupe/copy range ops
@ 2019-08-24 14:41 Eryu Guan
2019-09-26 9:24 ` Eryu Guan
2019-09-26 14:37 ` Brian Foster
0 siblings, 2 replies; 3+ messages in thread
From: Eryu Guan @ 2019-08-24 14:41 UTC (permalink / raw)
To: fstests; +Cc: Eryu Guan
From: Eryu Guan <eguan@linux.alibaba.com>
In CLONE/DEDUPE/COPY RANGE operations, we pick a "offset" and "size"
first, then find a suitable "offset2" by looping if there's overlap
(|offset2-offset| < size) or final file size is greater than max
file size (offset2 + size > maxfilelen).
But it's possible that there's no such suitable offset2 and we loop
forever. e.g. block_size = 4096, offset = 0, size = 4096 and
maxfilelen is a value smaller than 8212 (which could be set via '-l'
option).
Fix it by making sure maxfilelen/file_size is big enough to hold
'size' bytes from 'offset2', and just skip this operation if not.
Signed-off-by: Eryu Guan <eguan@linux.alibaba.com>
---
v2:
- don't use macro with ugly hacks, use an inline function instead
ltp/fsx.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/ltp/fsx.c b/ltp/fsx.c
index 06d08e4e93f3..890666ab6140 100644
--- a/ltp/fsx.c
+++ b/ltp/fsx.c
@@ -1911,6 +1911,15 @@ fail:
return 0;
}
+/* Check if range operations are possible to find a suitable offset */
+static inline bool check_range(unsigned long op, unsigned long off,
+ unsigned long len, unsigned long size)
+{
+ bool ret = ((off + len * 2) <= size);
+ if (!ret)
+ log5(op, off, len, -1, FL_SKIPPED);
+ return ret;
+}
int
test(void)
@@ -1989,6 +1998,8 @@ test(void)
TRIM_OFF_LEN(offset, size, file_size);
offset = offset & ~(block_size - 1);
size = size & ~(block_size - 1);
+ if (!check_range(op, offset, size, maxfilelen))
+ goto out;
do {
offset2 = random();
TRIM_OFF(offset2, maxfilelen);
@@ -2003,6 +2014,8 @@ test(void)
TRIM_OFF_LEN(offset, size, file_size);
offset = offset & ~(block_size - 1);
size = size & ~(block_size - 1);
+ if (!check_range(op, offset, size, file_size))
+ goto out;
do {
if (tries++ >= 30) {
size = 0;
@@ -2020,6 +2033,8 @@ test(void)
offset -= offset % readbdy;
if (o_direct)
size -= size % readbdy;
+ if (!check_range(op, offset, size, maxfilelen))
+ goto out;
do {
offset2 = random();
TRIM_OFF(offset2, maxfilelen);
--
2.21.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] ltp/fsx: avoid infinite loop while finding offset2 in clone/dedupe/copy range ops
2019-08-24 14:41 [PATCH v2] ltp/fsx: avoid infinite loop while finding offset2 in clone/dedupe/copy range ops Eryu Guan
@ 2019-09-26 9:24 ` Eryu Guan
2019-09-26 14:37 ` Brian Foster
1 sibling, 0 replies; 3+ messages in thread
From: Eryu Guan @ 2019-09-26 9:24 UTC (permalink / raw)
To: Eryu Guan; +Cc: fstests
On Sat, Aug 24, 2019 at 10:41:07PM +0800, Eryu Guan wrote:
> From: Eryu Guan <eguan@linux.alibaba.com>
>
> In CLONE/DEDUPE/COPY RANGE operations, we pick a "offset" and "size"
> first, then find a suitable "offset2" by looping if there's overlap
> (|offset2-offset| < size) or final file size is greater than max
> file size (offset2 + size > maxfilelen).
>
> But it's possible that there's no such suitable offset2 and we loop
> forever. e.g. block_size = 4096, offset = 0, size = 4096 and
> maxfilelen is a value smaller than 8212 (which could be set via '-l'
> option).
>
> Fix it by making sure maxfilelen/file_size is big enough to hold
> 'size' bytes from 'offset2', and just skip this operation if not.
>
> Signed-off-by: Eryu Guan <eguan@linux.alibaba.com>
Ping on this patch.
Eryu
> ---
> v2:
> - don't use macro with ugly hacks, use an inline function instead
>
> ltp/fsx.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/ltp/fsx.c b/ltp/fsx.c
> index 06d08e4e93f3..890666ab6140 100644
> --- a/ltp/fsx.c
> +++ b/ltp/fsx.c
> @@ -1911,6 +1911,15 @@ fail:
> return 0;
> }
>
> +/* Check if range operations are possible to find a suitable offset */
> +static inline bool check_range(unsigned long op, unsigned long off,
> + unsigned long len, unsigned long size)
> +{
> + bool ret = ((off + len * 2) <= size);
> + if (!ret)
> + log5(op, off, len, -1, FL_SKIPPED);
> + return ret;
> +}
>
> int
> test(void)
> @@ -1989,6 +1998,8 @@ test(void)
> TRIM_OFF_LEN(offset, size, file_size);
> offset = offset & ~(block_size - 1);
> size = size & ~(block_size - 1);
> + if (!check_range(op, offset, size, maxfilelen))
> + goto out;
> do {
> offset2 = random();
> TRIM_OFF(offset2, maxfilelen);
> @@ -2003,6 +2014,8 @@ test(void)
> TRIM_OFF_LEN(offset, size, file_size);
> offset = offset & ~(block_size - 1);
> size = size & ~(block_size - 1);
> + if (!check_range(op, offset, size, file_size))
> + goto out;
> do {
> if (tries++ >= 30) {
> size = 0;
> @@ -2020,6 +2033,8 @@ test(void)
> offset -= offset % readbdy;
> if (o_direct)
> size -= size % readbdy;
> + if (!check_range(op, offset, size, maxfilelen))
> + goto out;
> do {
> offset2 = random();
> TRIM_OFF(offset2, maxfilelen);
> --
> 2.21.0
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] ltp/fsx: avoid infinite loop while finding offset2 in clone/dedupe/copy range ops
2019-08-24 14:41 [PATCH v2] ltp/fsx: avoid infinite loop while finding offset2 in clone/dedupe/copy range ops Eryu Guan
2019-09-26 9:24 ` Eryu Guan
@ 2019-09-26 14:37 ` Brian Foster
1 sibling, 0 replies; 3+ messages in thread
From: Brian Foster @ 2019-09-26 14:37 UTC (permalink / raw)
To: Eryu Guan; +Cc: fstests, Eryu Guan
On Sat, Aug 24, 2019 at 10:41:07PM +0800, Eryu Guan wrote:
> From: Eryu Guan <eguan@linux.alibaba.com>
>
> In CLONE/DEDUPE/COPY RANGE operations, we pick a "offset" and "size"
> first, then find a suitable "offset2" by looping if there's overlap
> (|offset2-offset| < size) or final file size is greater than max
> file size (offset2 + size > maxfilelen).
>
> But it's possible that there's no such suitable offset2 and we loop
> forever. e.g. block_size = 4096, offset = 0, size = 4096 and
> maxfilelen is a value smaller than 8212 (which could be set via '-l'
> option).
>
> Fix it by making sure maxfilelen/file_size is big enough to hold
> 'size' bytes from 'offset2', and just skip this operation if not.
>
> Signed-off-by: Eryu Guan <eguan@linux.alibaba.com>
> ---
> v2:
> - don't use macro with ugly hacks, use an inline function instead
>
> ltp/fsx.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/ltp/fsx.c b/ltp/fsx.c
> index 06d08e4e93f3..890666ab6140 100644
> --- a/ltp/fsx.c
> +++ b/ltp/fsx.c
> @@ -1911,6 +1911,15 @@ fail:
> return 0;
> }
>
> +/* Check if range operations are possible to find a suitable offset */
> +static inline bool check_range(unsigned long op, unsigned long off,
> + unsigned long len, unsigned long size)
> +{
> + bool ret = ((off + len * 2) <= size);
A more self-documenting name than ret would be helpful, though I'd
probably just do something like:
if ((off + len * 2) > size) {
log5(op, off, len, -1, FL_SKIPPED);
return false;
}
...
return true;
> + if (!ret)
> + log5(op, off, len, -1, FL_SKIPPED);
> + return ret;
> +}
Instead of offloading just this check, could we factor out the
associated offset2 loop into this function as well (and probably rename
it)? I.e., this function can return true if it assigned offset2 or false
and let the caller jump out. It could also take an optional max loop
count for the dedupe case.
Brian
>
> int
> test(void)
> @@ -1989,6 +1998,8 @@ test(void)
> TRIM_OFF_LEN(offset, size, file_size);
> offset = offset & ~(block_size - 1);
> size = size & ~(block_size - 1);
> + if (!check_range(op, offset, size, maxfilelen))
> + goto out;
> do {
> offset2 = random();
> TRIM_OFF(offset2, maxfilelen);
> @@ -2003,6 +2014,8 @@ test(void)
> TRIM_OFF_LEN(offset, size, file_size);
> offset = offset & ~(block_size - 1);
> size = size & ~(block_size - 1);
> + if (!check_range(op, offset, size, file_size))
> + goto out;
> do {
> if (tries++ >= 30) {
> size = 0;
> @@ -2020,6 +2033,8 @@ test(void)
> offset -= offset % readbdy;
> if (o_direct)
> size -= size % readbdy;
> + if (!check_range(op, offset, size, maxfilelen))
> + goto out;
> do {
> offset2 = random();
> TRIM_OFF(offset2, maxfilelen);
> --
> 2.21.0
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-09-26 14:37 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-24 14:41 [PATCH v2] ltp/fsx: avoid infinite loop while finding offset2 in clone/dedupe/copy range ops Eryu Guan
2019-09-26 9:24 ` Eryu Guan
2019-09-26 14:37 ` Brian Foster
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).