* [PATCH] xfs_db: Scan entire file system when using 'frag'
@ 2019-04-26 22:59 Jorge Guerra
2019-04-27 0:13 ` Eric Sandeen
0 siblings, 1 reply; 6+ messages in thread
From: Jorge Guerra @ 2019-04-26 22:59 UTC (permalink / raw)
To: linux-xfs; +Cc: osandov, Jorge Guerra
From: Jorge Guerra <jorgeguerra@fb.com>
While running the 'frag' command of 'xfs_db' we noticed that the
tool is not scanning all the files in the file system. We noticed
this when we modified the tool to print the inodes of all the files
scanned. For example:
$ find /mnt/xfsdisk -type f | wc -l
1782674
$ xfs_db -r -c frag /dev/sdXX | grep MB | awk '{print $5}' | paste -s -d+ | bc
656818
Upon inspecting the code we noticed that the scanfunc_ino function
stops processing a given inode block once it encounters a free leaf.
However, in practice we see that inodes are necessarily always layed
out contiguously on the leaf node. This resulted in the 'frag'
command skipping some valid inodes.
In this change we modify the scanfunc_ino function to skip freed
inodes. With the change in place we ran the same experiment again
and noticed a more accurate file count:
$ find /mnt/d0 -type f | wc -l
1810442
$ xfs_db -r -c frag /dev/sdXX | grep MB | awk '{print $5}' | paste -s -d+ | bc
1810442
Signed-off-by: Jorge Guerra <jorgeguerra@fb.com>
---
db/frag.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/db/frag.c b/db/frag.c
index 5f33cb73..91395234 100644
--- a/db/frag.c
+++ b/db/frag.c
@@ -507,7 +507,7 @@ scanfunc_ino(
for (j = 0; j < inodes_per_buf; j++) {
if (XFS_INOBT_IS_FREE_DISK(&rp[i], ioff + j))
- goto next_buf;
+ continue;
dip = (xfs_dinode_t *)((char *)iocur_top->data +
((off + j) << mp->m_sb.sb_inodelog));
process_inode(agf, agino + ioff + j, dip);
--
2.13.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] xfs_db: Scan entire file system when using 'frag'
2019-04-26 22:59 [PATCH] xfs_db: Scan entire file system when using 'frag' Jorge Guerra
@ 2019-04-27 0:13 ` Eric Sandeen
2019-04-27 1:24 ` Darrick J. Wong
2019-04-27 3:32 ` Jorge Guerra
0 siblings, 2 replies; 6+ messages in thread
From: Eric Sandeen @ 2019-04-27 0:13 UTC (permalink / raw)
To: Jorge Guerra, linux-xfs; +Cc: osandov, Jorge Guerra
On 4/26/19 5:59 PM, Jorge Guerra wrote:
> From: Jorge Guerra <jorgeguerra@fb.com>
>
> While running the 'frag' command of 'xfs_db' we noticed that the
> tool is not scanning all the files in the file system. We noticed
> this when we modified the tool to print the inodes of all the files
> scanned. For example:
>
> $ find /mnt/xfsdisk -type f | wc -l
> 1782674
> $ xfs_db -r -c frag /dev/sdXX | grep MB | awk '{print $5}' | paste -s -d+ | bc
> 656818
>
> Upon inspecting the code we noticed that the scanfunc_ino function
> stops processing a given inode block once it encounters a free leaf.
> However, in practice we see that inodes are necessarily always layed
> out contiguously on the leaf node. This resulted in the 'frag'
> command skipping some valid inodes.
>
> In this change we modify the scanfunc_ino function to skip freed
> inodes. With the change in place we ran the same experiment again
> and noticed a more accurate file count:
>
> $ find /mnt/d0 -type f | wc -l
> 1810442
> $ xfs_db -r -c frag /dev/sdXX | grep MB | awk '{print $5}' | paste -s -d+ | bc
> 1810442
>
> Signed-off-by: Jorge Guerra <jorgeguerra@fb.com>
This looks right, but I'll warn you that xfs_db's frag command is largely
useless in the first place. ;)
Also, I think:
Fixes: 2a5eb70c ("xfs_db: teach the frag command about sparse inode chunks")
Reviewed-by: Eric Sandeen <sandeen@redhat.com>
Thanks!
> ---
> db/frag.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/db/frag.c b/db/frag.c
> index 5f33cb73..91395234 100644
> --- a/db/frag.c
> +++ b/db/frag.c
> @@ -507,7 +507,7 @@ scanfunc_ino(
>
> for (j = 0; j < inodes_per_buf; j++) {
> if (XFS_INOBT_IS_FREE_DISK(&rp[i], ioff + j))
> - goto next_buf;
> + continue;
> dip = (xfs_dinode_t *)((char *)iocur_top->data +
> ((off + j) << mp->m_sb.sb_inodelog));
> process_inode(agf, agino + ioff + j, dip);
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xfs_db: Scan entire file system when using 'frag'
2019-04-27 0:13 ` Eric Sandeen
@ 2019-04-27 1:24 ` Darrick J. Wong
2019-04-27 3:32 ` Jorge Guerra
2019-04-27 3:32 ` Jorge Guerra
1 sibling, 1 reply; 6+ messages in thread
From: Darrick J. Wong @ 2019-04-27 1:24 UTC (permalink / raw)
To: Eric Sandeen; +Cc: Jorge Guerra, linux-xfs, osandov, Jorge Guerra
On Fri, Apr 26, 2019 at 07:13:49PM -0500, Eric Sandeen wrote:
> On 4/26/19 5:59 PM, Jorge Guerra wrote:
> > From: Jorge Guerra <jorgeguerra@fb.com>
> >
> > While running the 'frag' command of 'xfs_db' we noticed that the
> > tool is not scanning all the files in the file system. We noticed
> > this when we modified the tool to print the inodes of all the files
> > scanned. For example:
> >
> > $ find /mnt/xfsdisk -type f | wc -l
> > 1782674
> > $ xfs_db -r -c frag /dev/sdXX | grep MB | awk '{print $5}' | paste -s -d+ | bc
> > 656818
> >
> > Upon inspecting the code we noticed that the scanfunc_ino function
> > stops processing a given inode block once it encounters a free leaf.
> > However, in practice we see that inodes are necessarily always layed
> > out contiguously on the leaf node. This resulted in the 'frag'
> > command skipping some valid inodes.
> >
> > In this change we modify the scanfunc_ino function to skip freed
> > inodes. With the change in place we ran the same experiment again
> > and noticed a more accurate file count:
> >
> > $ find /mnt/d0 -type f | wc -l
> > 1810442
> > $ xfs_db -r -c frag /dev/sdXX | grep MB | awk '{print $5}' | paste -s -d+ | bc
> > 1810442
> >
> > Signed-off-by: Jorge Guerra <jorgeguerra@fb.com>
>
> This looks right, but I'll warn you that xfs_db's frag command is largely
> useless in the first place. ;)
I was looking through the manpages for the part where it says frag is
useless but couldn't find it. Oh well. :(
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
>
> Also, I think:
>
> Fixes: 2a5eb70c ("xfs_db: teach the frag command about sparse inode chunks")
>
> Reviewed-by: Eric Sandeen <sandeen@redhat.com>
>
> Thanks!
>
> > ---
> > db/frag.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/db/frag.c b/db/frag.c
> > index 5f33cb73..91395234 100644
> > --- a/db/frag.c
> > +++ b/db/frag.c
> > @@ -507,7 +507,7 @@ scanfunc_ino(
> >
> > for (j = 0; j < inodes_per_buf; j++) {
> > if (XFS_INOBT_IS_FREE_DISK(&rp[i], ioff + j))
> > - goto next_buf;
> > + continue;
> > dip = (xfs_dinode_t *)((char *)iocur_top->data +
> > ((off + j) << mp->m_sb.sb_inodelog));
> > process_inode(agf, agino + ioff + j, dip);
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xfs_db: Scan entire file system when using 'frag'
2019-04-27 0:13 ` Eric Sandeen
2019-04-27 1:24 ` Darrick J. Wong
@ 2019-04-27 3:32 ` Jorge Guerra
2019-04-27 10:51 ` Eric Sandeen
1 sibling, 1 reply; 6+ messages in thread
From: Jorge Guerra @ 2019-04-27 3:32 UTC (permalink / raw)
To: Eric Sandeen; +Cc: linux-xfs, Omar Sandoval, Jorge Guerra
Thanks Eric,
I'll update the title.
We are using the frag command to quickly scan the file system and
obtain info such as file size distribution and overheads. I'll send
that change out for review soon :)
Hopefully that will make the frag command great again! :D
On Fri, Apr 26, 2019 at 5:13 PM Eric Sandeen <sandeen@sandeen.net> wrote:
>
> On 4/26/19 5:59 PM, Jorge Guerra wrote:
> > From: Jorge Guerra <jorgeguerra@fb.com>
> >
> > While running the 'frag' command of 'xfs_db' we noticed that the
> > tool is not scanning all the files in the file system. We noticed
> > this when we modified the tool to print the inodes of all the files
> > scanned. For example:
> >
> > $ find /mnt/xfsdisk -type f | wc -l
> > 1782674
> > $ xfs_db -r -c frag /dev/sdXX | grep MB | awk '{print $5}' | paste -s -d+ | bc
> > 656818
> >
> > Upon inspecting the code we noticed that the scanfunc_ino function
> > stops processing a given inode block once it encounters a free leaf.
> > However, in practice we see that inodes are necessarily always layed
> > out contiguously on the leaf node. This resulted in the 'frag'
> > command skipping some valid inodes.
> >
> > In this change we modify the scanfunc_ino function to skip freed
> > inodes. With the change in place we ran the same experiment again
> > and noticed a more accurate file count:
> >
> > $ find /mnt/d0 -type f | wc -l
> > 1810442
> > $ xfs_db -r -c frag /dev/sdXX | grep MB | awk '{print $5}' | paste -s -d+ | bc
> > 1810442
> >
> > Signed-off-by: Jorge Guerra <jorgeguerra@fb.com>
>
> This looks right, but I'll warn you that xfs_db's frag command is largely
> useless in the first place. ;)
>
> Also, I think:
>
> Fixes: 2a5eb70c ("xfs_db: teach the frag command about sparse inode chunks")
>
> Reviewed-by: Eric Sandeen <sandeen@redhat.com>
>
> Thanks!
>
> > ---
> > db/frag.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/db/frag.c b/db/frag.c
> > index 5f33cb73..91395234 100644
> > --- a/db/frag.c
> > +++ b/db/frag.c
> > @@ -507,7 +507,7 @@ scanfunc_ino(
> >
> > for (j = 0; j < inodes_per_buf; j++) {
> > if (XFS_INOBT_IS_FREE_DISK(&rp[i], ioff + j))
> > - goto next_buf;
> > + continue;
> > dip = (xfs_dinode_t *)((char *)iocur_top->data +
> > ((off + j) << mp->m_sb.sb_inodelog));
> > process_inode(agf, agino + ioff + j, dip);
> >
--
Jorge E Guerra D
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xfs_db: Scan entire file system when using 'frag'
2019-04-27 1:24 ` Darrick J. Wong
@ 2019-04-27 3:32 ` Jorge Guerra
0 siblings, 0 replies; 6+ messages in thread
From: Jorge Guerra @ 2019-04-27 3:32 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Eric Sandeen, linux-xfs, Omar Sandoval, Jorge Guerra
Lol!
Thanks Darrick!
On Fri, Apr 26, 2019 at 6:24 PM Darrick J. Wong <darrick.wong@oracle.com> wrote:
>
> On Fri, Apr 26, 2019 at 07:13:49PM -0500, Eric Sandeen wrote:
> > On 4/26/19 5:59 PM, Jorge Guerra wrote:
> > > From: Jorge Guerra <jorgeguerra@fb.com>
> > >
> > > While running the 'frag' command of 'xfs_db' we noticed that the
> > > tool is not scanning all the files in the file system. We noticed
> > > this when we modified the tool to print the inodes of all the files
> > > scanned. For example:
> > >
> > > $ find /mnt/xfsdisk -type f | wc -l
> > > 1782674
> > > $ xfs_db -r -c frag /dev/sdXX | grep MB | awk '{print $5}' | paste -s -d+ | bc
> > > 656818
> > >
> > > Upon inspecting the code we noticed that the scanfunc_ino function
> > > stops processing a given inode block once it encounters a free leaf.
> > > However, in practice we see that inodes are necessarily always layed
> > > out contiguously on the leaf node. This resulted in the 'frag'
> > > command skipping some valid inodes.
> > >
> > > In this change we modify the scanfunc_ino function to skip freed
> > > inodes. With the change in place we ran the same experiment again
> > > and noticed a more accurate file count:
> > >
> > > $ find /mnt/d0 -type f | wc -l
> > > 1810442
> > > $ xfs_db -r -c frag /dev/sdXX | grep MB | awk '{print $5}' | paste -s -d+ | bc
> > > 1810442
> > >
> > > Signed-off-by: Jorge Guerra <jorgeguerra@fb.com>
> >
> > This looks right, but I'll warn you that xfs_db's frag command is largely
> > useless in the first place. ;)
>
> I was looking through the manpages for the part where it says frag is
> useless but couldn't find it. Oh well. :(
>
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
>
> --D
>
> >
> > Also, I think:
> >
> > Fixes: 2a5eb70c ("xfs_db: teach the frag command about sparse inode chunks")
> >
> > Reviewed-by: Eric Sandeen <sandeen@redhat.com>
> >
> > Thanks!
> >
> > > ---
> > > db/frag.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/db/frag.c b/db/frag.c
> > > index 5f33cb73..91395234 100644
> > > --- a/db/frag.c
> > > +++ b/db/frag.c
> > > @@ -507,7 +507,7 @@ scanfunc_ino(
> > >
> > > for (j = 0; j < inodes_per_buf; j++) {
> > > if (XFS_INOBT_IS_FREE_DISK(&rp[i], ioff + j))
> > > - goto next_buf;
> > > + continue;
> > > dip = (xfs_dinode_t *)((char *)iocur_top->data +
> > > ((off + j) << mp->m_sb.sb_inodelog));
> > > process_inode(agf, agino + ioff + j, dip);
> > >
--
Jorge E Guerra D
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xfs_db: Scan entire file system when using 'frag'
2019-04-27 3:32 ` Jorge Guerra
@ 2019-04-27 10:51 ` Eric Sandeen
0 siblings, 0 replies; 6+ messages in thread
From: Eric Sandeen @ 2019-04-27 10:51 UTC (permalink / raw)
To: Jorge Guerra; +Cc: linux-xfs, Omar Sandoval, Jorge Guerra
On 4/26/19 10:32 PM, Jorge Guerra wrote:
> Thanks Eric,
>
> I'll update the title.
No need, I can add the fixes tag when I queue it up.
> We are using the frag command to quickly scan the file system and
> obtain info such as file size distribution and overheads. I'll send
> that change out for review soon :)
>
> Hopefully that will make the frag command great again! :D
;) ok, if it becomes less meaningless, you may want to remove the printf
which claims that it is meaningless. :)
-Eric
> On Fri, Apr 26, 2019 at 5:13 PM Eric Sandeen <sandeen@sandeen.net> wrote:
>>
>> On 4/26/19 5:59 PM, Jorge Guerra wrote:
>>> From: Jorge Guerra <jorgeguerra@fb.com>
>>>
>>> While running the 'frag' command of 'xfs_db' we noticed that the
>>> tool is not scanning all the files in the file system. We noticed
>>> this when we modified the tool to print the inodes of all the files
>>> scanned. For example:
>>>
>>> $ find /mnt/xfsdisk -type f | wc -l
>>> 1782674
>>> $ xfs_db -r -c frag /dev/sdXX | grep MB | awk '{print $5}' | paste -s -d+ | bc
>>> 656818
>>>
>>> Upon inspecting the code we noticed that the scanfunc_ino function
>>> stops processing a given inode block once it encounters a free leaf.
>>> However, in practice we see that inodes are necessarily always layed
>>> out contiguously on the leaf node. This resulted in the 'frag'
>>> command skipping some valid inodes.
>>>
>>> In this change we modify the scanfunc_ino function to skip freed
>>> inodes. With the change in place we ran the same experiment again
>>> and noticed a more accurate file count:
>>>
>>> $ find /mnt/d0 -type f | wc -l
>>> 1810442
>>> $ xfs_db -r -c frag /dev/sdXX | grep MB | awk '{print $5}' | paste -s -d+ | bc
>>> 1810442
>>>
>>> Signed-off-by: Jorge Guerra <jorgeguerra@fb.com>
>>
>> This looks right, but I'll warn you that xfs_db's frag command is largely
>> useless in the first place. ;)
>>
>> Also, I think:
>>
>> Fixes: 2a5eb70c ("xfs_db: teach the frag command about sparse inode chunks")
>>
>> Reviewed-by: Eric Sandeen <sandeen@redhat.com>
>>
>> Thanks!
>>
>>> ---
>>> db/frag.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/db/frag.c b/db/frag.c
>>> index 5f33cb73..91395234 100644
>>> --- a/db/frag.c
>>> +++ b/db/frag.c
>>> @@ -507,7 +507,7 @@ scanfunc_ino(
>>>
>>> for (j = 0; j < inodes_per_buf; j++) {
>>> if (XFS_INOBT_IS_FREE_DISK(&rp[i], ioff + j))
>>> - goto next_buf;
>>> + continue;
>>> dip = (xfs_dinode_t *)((char *)iocur_top->data +
>>> ((off + j) << mp->m_sb.sb_inodelog));
>>> process_inode(agf, agino + ioff + j, dip);
>>>
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-04-27 10:51 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-04-26 22:59 [PATCH] xfs_db: Scan entire file system when using 'frag' Jorge Guerra
2019-04-27 0:13 ` Eric Sandeen
2019-04-27 1:24 ` Darrick J. Wong
2019-04-27 3:32 ` Jorge Guerra
2019-04-27 3:32 ` Jorge Guerra
2019-04-27 10:51 ` Eric Sandeen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox