* [PATCH] xfs: fix directory readahead offset off-by-one
@ 2014-05-06 3:42 Dave Chinner
2014-05-06 7:56 ` Christoph Hellwig
0 siblings, 1 reply; 4+ messages in thread
From: Dave Chinner @ 2014-05-06 3:42 UTC (permalink / raw)
To: xfs
From: Dave Chinner <dchinner@redhat.com>
Directory readahead can throw loud scary but harmless warnings
when multiblock directories are in use a specific pattern of
discontiguous blocks are found in the directory. That is, if a hole
follows a discontiguous block, it will throw a warning like:
XFS (dm-1): xfs_da_do_buf: bno 637 dir: inode 34363923462
XFS (dm-1): [00] br_startoff 637 br_startblock 1917954575 br_blockcount 1 br_state 0
XFS (dm-1): [01] br_startoff 638 br_startblock -2 br_blockcount 1 br_state 0
And dump a stack trace.
This is because the readahead offset increment loop does a double
increment of the block index - it does an increment for the loop
iteration as well as increase the loop counter by the number of
blocks in the extent. As a result, the readahead offset does not get
incremented correctly for discontiguous blocks and hence can ask for
readahead of a directory block from an offset part way through a
directory block. If that directory block is followed by a hole, it
will trigger a mapping warning like the above.
The bad readahead will be ignored, though, because the main
directory block read loop uses the correct mapping offsets rather
than the readahead offset and so will ignore the bad readahead
altogether.
Fix the warning by ensuring that the readahead offset is correctly
incremented.
Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
fs/xfs/xfs_dir2_readdir.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/xfs/xfs_dir2_readdir.c b/fs/xfs/xfs_dir2_readdir.c
index 50b72f7..fe2db98 100644
--- a/fs/xfs/xfs_dir2_readdir.c
+++ b/fs/xfs/xfs_dir2_readdir.c
@@ -456,7 +456,7 @@ xfs_dir2_leaf_readbuf(
/*
* Advance offset through the mapping table.
*/
- for (j = 0; j < mp->m_dirblkfsbs; j++) {
+ for (j = 0; j < mp->m_dirblkfsbs; ) {
/*
* The rest of this extent but not more than a dir
* block.
--
1.9.0
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] xfs: fix directory readahead offset off-by-one
2014-05-06 3:42 [PATCH] xfs: fix directory readahead offset off-by-one Dave Chinner
@ 2014-05-06 7:56 ` Christoph Hellwig
2014-05-06 8:02 ` Dave Chinner
0 siblings, 1 reply; 4+ messages in thread
From: Christoph Hellwig @ 2014-05-06 7:56 UTC (permalink / raw)
To: Dave Chinner; +Cc: xfs
On Tue, May 06, 2014 at 01:42:08PM +1000, Dave Chinner wrote:
> Fix the warning by ensuring that the readahead offset is correctly
> incremented.
>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
> fs/xfs/xfs_dir2_readdir.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/xfs/xfs_dir2_readdir.c b/fs/xfs/xfs_dir2_readdir.c
> index 50b72f7..fe2db98 100644
> --- a/fs/xfs/xfs_dir2_readdir.c
> +++ b/fs/xfs/xfs_dir2_readdir.c
> @@ -456,7 +456,7 @@ xfs_dir2_leaf_readbuf(
> /*
> * Advance offset through the mapping table.
> */
> - for (j = 0; j < mp->m_dirblkfsbs; j++) {
> + for (j = 0; j < mp->m_dirblkfsbs; ) {
> /*
> * The rest of this extent but not more than a dir
> * block.
This looks correct, but it would seem a little more idiomatic to write
this as:
for (j = 0; j < mp->m_dirblkfsbs; j += length) {
and remove the j increment from the body.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] xfs: fix directory readahead offset off-by-one
2014-05-06 7:56 ` Christoph Hellwig
@ 2014-05-06 8:02 ` Dave Chinner
2014-05-06 8:05 ` Christoph Hellwig
0 siblings, 1 reply; 4+ messages in thread
From: Dave Chinner @ 2014-05-06 8:02 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
On Tue, May 06, 2014 at 12:56:37AM -0700, Christoph Hellwig wrote:
> On Tue, May 06, 2014 at 01:42:08PM +1000, Dave Chinner wrote:
> > Fix the warning by ensuring that the readahead offset is correctly
> > incremented.
> >
> > Signed-off-by: Dave Chinner <dchinner@redhat.com>
> > ---
> > fs/xfs/xfs_dir2_readdir.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/fs/xfs/xfs_dir2_readdir.c b/fs/xfs/xfs_dir2_readdir.c
> > index 50b72f7..fe2db98 100644
> > --- a/fs/xfs/xfs_dir2_readdir.c
> > +++ b/fs/xfs/xfs_dir2_readdir.c
> > @@ -456,7 +456,7 @@ xfs_dir2_leaf_readbuf(
> > /*
> > * Advance offset through the mapping table.
> > */
> > - for (j = 0; j < mp->m_dirblkfsbs; j++) {
> > + for (j = 0; j < mp->m_dirblkfsbs; ) {
> > /*
> > * The rest of this extent but not more than a dir
> > * block.
>
> This looks correct, but it would seem a little more idiomatic to write
> this as:
>
> for (j = 0; j < mp->m_dirblkfsbs; j += length) {
>
> and remove the j increment from the body.
yeah, I can do that.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] xfs: fix directory readahead offset off-by-one
2014-05-06 8:02 ` Dave Chinner
@ 2014-05-06 8:05 ` Christoph Hellwig
0 siblings, 0 replies; 4+ messages in thread
From: Christoph Hellwig @ 2014-05-06 8:05 UTC (permalink / raw)
To: Dave Chinner; +Cc: Christoph Hellwig, xfs
On Tue, May 06, 2014 at 06:02:36PM +1000, Dave Chinner wrote:
> yeah, I can do that.
Reviewed-by: Christoph Hellwig <hch@lst.de>
with that.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-05-06 8:05 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-06 3:42 [PATCH] xfs: fix directory readahead offset off-by-one Dave Chinner
2014-05-06 7:56 ` Christoph Hellwig
2014-05-06 8:02 ` Dave Chinner
2014-05-06 8:05 ` Christoph Hellwig
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox