* [PATCH 1/2] xfsprogs: xfs_fsr: Interpret arguments of qsort's compare function correctly
@ 2021-01-25 9:58 Chandan Babu R
2021-01-25 9:58 ` [PATCH 2/2] xfsprogs: xfs_fsr: Limit the scope of cmp() Chandan Babu R
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Chandan Babu R @ 2021-01-25 9:58 UTC (permalink / raw)
To: linux-xfs; +Cc: Chandan Babu R, sandeen
The first argument passed to qsort() in fsrfs() is an array of "struct
xfs_bulkstat". Hence the two arguments to the cmp() function must be
interpreted as being of type "struct xfs_bulkstat *" as against "struct
xfs_bstat *" that is being used to currently typecast them.
Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
---
fsr/xfs_fsr.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c
index 77a10a1d..635e4c70 100644
--- a/fsr/xfs_fsr.c
+++ b/fsr/xfs_fsr.c
@@ -702,9 +702,8 @@ out0:
int
cmp(const void *s1, const void *s2)
{
- return( ((struct xfs_bstat *)s2)->bs_extents -
- ((struct xfs_bstat *)s1)->bs_extents);
-
+ return( ((struct xfs_bulkstat *)s2)->bs_extents -
+ ((struct xfs_bulkstat *)s1)->bs_extents);
}
/*
--
2.29.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] xfsprogs: xfs_fsr: Limit the scope of cmp()
2021-01-25 9:58 [PATCH 1/2] xfsprogs: xfs_fsr: Interpret arguments of qsort's compare function correctly Chandan Babu R
@ 2021-01-25 9:58 ` Chandan Babu R
2021-01-25 14:19 ` Eric Sandeen
2021-01-25 14:17 ` [PATCH 1/2] xfsprogs: xfs_fsr: Interpret arguments of qsort's compare function correctly Eric Sandeen
2021-01-26 17:58 ` Darrick J. Wong
2 siblings, 1 reply; 9+ messages in thread
From: Chandan Babu R @ 2021-01-25 9:58 UTC (permalink / raw)
To: linux-xfs; +Cc: Chandan Babu R, sandeen
cmp() function is being referred to by only from within fsr/xfs_fsr.c. Hence
this commit limits its scope to the current file.
Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
---
fsr/xfs_fsr.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c
index 635e4c70..2d070320 100644
--- a/fsr/xfs_fsr.c
+++ b/fsr/xfs_fsr.c
@@ -81,7 +81,7 @@ char * gettmpname(char *fname);
char * getparent(char *fname);
int fsrprintf(const char *fmt, ...);
int read_fd_bmap(int, struct xfs_bstat *, int *);
-int cmp(const void *, const void *);
+static int cmp(const void *, const void *);
static void tmp_init(char *mnt);
static char * tmp_next(char *mnt);
static void tmp_close(char *mnt);
@@ -699,7 +699,7 @@ out0:
/*
* To compare bstat structs for qsort.
*/
-int
+static int
cmp(const void *s1, const void *s2)
{
return( ((struct xfs_bulkstat *)s2)->bs_extents -
--
2.29.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] xfsprogs: xfs_fsr: Interpret arguments of qsort's compare function correctly
2021-01-25 9:58 [PATCH 1/2] xfsprogs: xfs_fsr: Interpret arguments of qsort's compare function correctly Chandan Babu R
2021-01-25 9:58 ` [PATCH 2/2] xfsprogs: xfs_fsr: Limit the scope of cmp() Chandan Babu R
@ 2021-01-25 14:17 ` Eric Sandeen
2021-01-26 5:14 ` Chandan Babu R
2021-01-26 17:58 ` Darrick J. Wong
2 siblings, 1 reply; 9+ messages in thread
From: Eric Sandeen @ 2021-01-25 14:17 UTC (permalink / raw)
To: Chandan Babu R, linux-xfs
On 1/25/21 3:58 AM, Chandan Babu R wrote:
> The first argument passed to qsort() in fsrfs() is an array of "struct
> xfs_bulkstat". Hence the two arguments to the cmp() function must be
> interpreted as being of type "struct xfs_bulkstat *" as against "struct
> xfs_bstat *" that is being used to currently typecast them.
>
> Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
Yikes. Broken since 5.3.0, and the structures have different sizes and
different bs_extents offsets. :(
Fixes: 4cca629d6 ("misc: convert xfrog_bulkstat functions to have v5 semantics")
Reviewed-by: Eric Sandeen <sandeen@redhat.com>
At least it's only affecting the whole-fs defragment which is generally not
recommended, but is still available and does get used.
I wonder if it explains this bug report:
Jan 07 20:52:44 <Tharn> hey, quick question... the first time I ran xfs_fsr last night, it ran for 2 hours and looking at the console log, it ended with a lot of "start inode=0" repeating
> ---
> fsr/xfs_fsr.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c
> index 77a10a1d..635e4c70 100644
> --- a/fsr/xfs_fsr.c
> +++ b/fsr/xfs_fsr.c
> @@ -702,9 +702,8 @@ out0:
> int
> cmp(const void *s1, const void *s2)
> {
> - return( ((struct xfs_bstat *)s2)->bs_extents -
> - ((struct xfs_bstat *)s1)->bs_extents);
> -
> + return( ((struct xfs_bulkstat *)s2)->bs_extents -
> + ((struct xfs_bulkstat *)s1)->bs_extents);
> }
>
> /*
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] xfsprogs: xfs_fsr: Limit the scope of cmp()
2021-01-25 9:58 ` [PATCH 2/2] xfsprogs: xfs_fsr: Limit the scope of cmp() Chandan Babu R
@ 2021-01-25 14:19 ` Eric Sandeen
2021-01-26 4:42 ` [PATCH V1.1] " Chandan Babu R
0 siblings, 1 reply; 9+ messages in thread
From: Eric Sandeen @ 2021-01-25 14:19 UTC (permalink / raw)
To: Chandan Babu R, linux-xfs
On 1/25/21 3:58 AM, Chandan Babu R wrote:
> cmp() function is being referred to by only from within fsr/xfs_fsr.c. Hence
> this commit limits its scope to the current file.
>
> Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
Might be nice to move cmp just ahead of fsrfs() so we don't need the
forward declaration but *shrug*
Reviewed-by: Eric Sandeen <sandeen@redhat.com>
> ---
> fsr/xfs_fsr.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c
> index 635e4c70..2d070320 100644
> --- a/fsr/xfs_fsr.c
> +++ b/fsr/xfs_fsr.c
> @@ -81,7 +81,7 @@ char * gettmpname(char *fname);
> char * getparent(char *fname);
> int fsrprintf(const char *fmt, ...);
> int read_fd_bmap(int, struct xfs_bstat *, int *);
> -int cmp(const void *, const void *);
> +static int cmp(const void *, const void *);
> static void tmp_init(char *mnt);
> static char * tmp_next(char *mnt);
> static void tmp_close(char *mnt);
> @@ -699,7 +699,7 @@ out0:
> /*
> * To compare bstat structs for qsort.
> */
> -int
> +static int
> cmp(const void *s1, const void *s2)
> {
> return( ((struct xfs_bulkstat *)s2)->bs_extents -
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH V1.1] xfsprogs: xfs_fsr: Limit the scope of cmp()
2021-01-25 14:19 ` Eric Sandeen
@ 2021-01-26 4:42 ` Chandan Babu R
2021-02-02 20:21 ` Eric Sandeen
0 siblings, 1 reply; 9+ messages in thread
From: Chandan Babu R @ 2021-01-26 4:42 UTC (permalink / raw)
To: linux-xfs; +Cc: Chandan Babu R, sandeen
cmp() function is being referred to from within fsr/xfs_fsr.c. Hence
this commit limits its scope to the current file.
Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
---
fsr/xfs_fsr.c | 21 ++++++++++-----------
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c
index 635e4c70..b885395e 100644
--- a/fsr/xfs_fsr.c
+++ b/fsr/xfs_fsr.c
@@ -81,7 +81,6 @@ char * gettmpname(char *fname);
char * getparent(char *fname);
int fsrprintf(const char *fmt, ...);
int read_fd_bmap(int, struct xfs_bstat *, int *);
-int cmp(const void *, const void *);
static void tmp_init(char *mnt);
static char * tmp_next(char *mnt);
static void tmp_close(char *mnt);
@@ -577,6 +576,16 @@ fsrall_cleanup(int timeout)
}
}
+/*
+ * To compare bstat structs for qsort.
+ */
+static int
+cmp(const void *s1, const void *s2)
+{
+ return( ((struct xfs_bulkstat *)s2)->bs_extents -
+ ((struct xfs_bulkstat *)s1)->bs_extents);
+}
+
/*
* fsrfs -- reorganize a file system
*/
@@ -696,16 +705,6 @@ out0:
return 0;
}
-/*
- * To compare bstat structs for qsort.
- */
-int
-cmp(const void *s1, const void *s2)
-{
- return( ((struct xfs_bulkstat *)s2)->bs_extents -
- ((struct xfs_bulkstat *)s1)->bs_extents);
-}
-
/*
* reorganize by directory hierarchy.
* Stay in dev (a restriction based on structure of this program -- either
--
2.29.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] xfsprogs: xfs_fsr: Interpret arguments of qsort's compare function correctly
2021-01-25 14:17 ` [PATCH 1/2] xfsprogs: xfs_fsr: Interpret arguments of qsort's compare function correctly Eric Sandeen
@ 2021-01-26 5:14 ` Chandan Babu R
0 siblings, 0 replies; 9+ messages in thread
From: Chandan Babu R @ 2021-01-26 5:14 UTC (permalink / raw)
To: Eric Sandeen; +Cc: Chandan Babu R, linux-xfs
On 25 Jan 2021 at 19:47, Eric Sandeen wrote:
> On 1/25/21 3:58 AM, Chandan Babu R wrote:
>> The first argument passed to qsort() in fsrfs() is an array of "struct
>> xfs_bulkstat". Hence the two arguments to the cmp() function must be
>> interpreted as being of type "struct xfs_bulkstat *" as against "struct
>> xfs_bstat *" that is being used to currently typecast them.
>>
>> Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
>
> Yikes. Broken since 5.3.0, and the structures have different sizes and
> different bs_extents offsets. :(
>
> Fixes: 4cca629d6 ("misc: convert xfrog_bulkstat functions to have v5 semantics")
> Reviewed-by: Eric Sandeen <sandeen@redhat.com>
>
> At least it's only affecting the whole-fs defragment which is generally not
> recommended, but is still available and does get used.
>
> I wonder if it explains this bug report:
>
> Jan 07 20:52:44 <Tharn> hey, quick question... the first time I ran xfs_fsr last night, it ran for 2 hours and looking at the console log, it ended with a lot of "start inode=0" repeating
With my limited understanding of xfs_fsr code, looks like the bug reporter saw
the logs corresponding to the default (i.e. 10) number of passes that xfs_fsr
executes on the inodes of the filesystem. Sorry, I couldn't be of much help in
this case.
>
>> ---
>> fsr/xfs_fsr.c | 5 ++---
>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c
>> index 77a10a1d..635e4c70 100644
>> --- a/fsr/xfs_fsr.c
>> +++ b/fsr/xfs_fsr.c
>> @@ -702,9 +702,8 @@ out0:
>> int
>> cmp(const void *s1, const void *s2)
>> {
>> - return( ((struct xfs_bstat *)s2)->bs_extents -
>> - ((struct xfs_bstat *)s1)->bs_extents);
>> -
>> + return( ((struct xfs_bulkstat *)s2)->bs_extents -
>> + ((struct xfs_bulkstat *)s1)->bs_extents);
>> }
>>
>> /*
>>
--
chandan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] xfsprogs: xfs_fsr: Interpret arguments of qsort's compare function correctly
2021-01-25 9:58 [PATCH 1/2] xfsprogs: xfs_fsr: Interpret arguments of qsort's compare function correctly Chandan Babu R
2021-01-25 9:58 ` [PATCH 2/2] xfsprogs: xfs_fsr: Limit the scope of cmp() Chandan Babu R
2021-01-25 14:17 ` [PATCH 1/2] xfsprogs: xfs_fsr: Interpret arguments of qsort's compare function correctly Eric Sandeen
@ 2021-01-26 17:58 ` Darrick J. Wong
2021-01-26 18:12 ` Chandan Babu R
2 siblings, 1 reply; 9+ messages in thread
From: Darrick J. Wong @ 2021-01-26 17:58 UTC (permalink / raw)
To: Chandan Babu R; +Cc: linux-xfs, sandeen
On Mon, Jan 25, 2021 at 03:28:08PM +0530, Chandan Babu R wrote:
> The first argument passed to qsort() in fsrfs() is an array of "struct
> xfs_bulkstat". Hence the two arguments to the cmp() function must be
> interpreted as being of type "struct xfs_bulkstat *" as against "struct
> xfs_bstat *" that is being used to currently typecast them.
>
> Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
> ---
> fsr/xfs_fsr.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c
> index 77a10a1d..635e4c70 100644
> --- a/fsr/xfs_fsr.c
> +++ b/fsr/xfs_fsr.c
> @@ -702,9 +702,8 @@ out0:
> int
> cmp(const void *s1, const void *s2)
> {
> - return( ((struct xfs_bstat *)s2)->bs_extents -
> - ((struct xfs_bstat *)s1)->bs_extents);
> -
> + return( ((struct xfs_bulkstat *)s2)->bs_extents -
> + ((struct xfs_bulkstat *)s1)->bs_extents);
It might be a good idea to check bs_version here to avoid future
maintainer screwups <coughs this maintainer>
Thanks for catching this,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
> }
>
> /*
> --
> 2.29.2
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] xfsprogs: xfs_fsr: Interpret arguments of qsort's compare function correctly
2021-01-26 17:58 ` Darrick J. Wong
@ 2021-01-26 18:12 ` Chandan Babu R
0 siblings, 0 replies; 9+ messages in thread
From: Chandan Babu R @ 2021-01-26 18:12 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Chandan Babu R, linux-xfs, sandeen
On 26 Jan 2021 at 23:28, Darrick J. Wong wrote:
> On Mon, Jan 25, 2021 at 03:28:08PM +0530, Chandan Babu R wrote:
>> The first argument passed to qsort() in fsrfs() is an array of "struct
>> xfs_bulkstat". Hence the two arguments to the cmp() function must be
>> interpreted as being of type "struct xfs_bulkstat *" as against "struct
>> xfs_bstat *" that is being used to currently typecast them.
>>
>> Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
>> ---
>> fsr/xfs_fsr.c | 5 ++---
>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/fsr/xfs_fsr.c b/fsr/xfs_fsr.c
>> index 77a10a1d..635e4c70 100644
>> --- a/fsr/xfs_fsr.c
>> +++ b/fsr/xfs_fsr.c
>> @@ -702,9 +702,8 @@ out0:
>> int
>> cmp(const void *s1, const void *s2)
>> {
>> - return( ((struct xfs_bstat *)s2)->bs_extents -
>> - ((struct xfs_bstat *)s1)->bs_extents);
>> -
>> + return( ((struct xfs_bulkstat *)s2)->bs_extents -
>> + ((struct xfs_bulkstat *)s1)->bs_extents);
>
> It might be a good idea to check bs_version here to avoid future
> maintainer screwups <coughs this maintainer>
>
Sure, I will write a new patch to add that.
> Thanks for catching this,
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
>
--
chandan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH V1.1] xfsprogs: xfs_fsr: Limit the scope of cmp()
2021-01-26 4:42 ` [PATCH V1.1] " Chandan Babu R
@ 2021-02-02 20:21 ` Eric Sandeen
0 siblings, 0 replies; 9+ messages in thread
From: Eric Sandeen @ 2021-02-02 20:21 UTC (permalink / raw)
To: Chandan Babu R, linux-xfs
On 1/25/21 10:42 PM, Chandan Babu R wrote:
> cmp() function is being referred to from within fsr/xfs_fsr.c. Hence
> this commit limits its scope to the current file.
>
> Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
Thanks,
Reviewed-by: Eric Sandeen <sandeen@redhat.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-02-02 20:24 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-25 9:58 [PATCH 1/2] xfsprogs: xfs_fsr: Interpret arguments of qsort's compare function correctly Chandan Babu R
2021-01-25 9:58 ` [PATCH 2/2] xfsprogs: xfs_fsr: Limit the scope of cmp() Chandan Babu R
2021-01-25 14:19 ` Eric Sandeen
2021-01-26 4:42 ` [PATCH V1.1] " Chandan Babu R
2021-02-02 20:21 ` Eric Sandeen
2021-01-25 14:17 ` [PATCH 1/2] xfsprogs: xfs_fsr: Interpret arguments of qsort's compare function correctly Eric Sandeen
2021-01-26 5:14 ` Chandan Babu R
2021-01-26 17:58 ` Darrick J. Wong
2021-01-26 18:12 ` Chandan Babu R
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.