* [bug report] xfs: scrub directory freespace
@ 2017-11-04 7:54 Dan Carpenter
2017-11-06 19:34 ` Darrick J. Wong
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2017-11-04 7:54 UTC (permalink / raw)
To: darrick.wong; +Cc: linux-xfs
Hello Darrick J. Wong,
The patch df481968f33b: "xfs: scrub directory freespace" from Oct 17,
2017, leads to the following static checker warning:
fs/xfs/scrub/dir.c:446 xfs_scrub_directory_check_freesp()
info: ignoring unreachable code.
fs/xfs/scrub/dir.c
431 STATIC void
432 xfs_scrub_directory_check_freesp(
433 struct xfs_scrub_context *sc,
434 xfs_dablk_t lblk,
435 struct xfs_buf *dbp,
436 unsigned int len)
437 {
438 struct xfs_dir2_data_free *bf;
439 struct xfs_dir2_data_free *dfp;
440 int offset;
441
442 if (len == 0)
443 return;
444
445 bf = sc->ip->d_ops->data_bestfree_p(dbp->b_addr);
446 for (dfp = &bf[0]; dfp < &bf[XFS_DIR2_DATA_FD_COUNT]; dfp++) {
^^^^^
This looks like a loop
447 offset = be16_to_cpu(dfp->offset);
448 if (offset == 0)
449 break;
450 if (len == be16_to_cpu(dfp->length))
451 return;
452 /* Didn't find the best length in the bestfree data */
453 break;
^^^^^^
but we always either break or return on the first iteration. What's
going on?
454 }
455
456 xfs_scrub_fblock_set_corrupt(sc, XFS_DATA_FORK, lblk);
457 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [bug report] xfs: scrub directory freespace
2017-11-04 7:54 [bug report] xfs: scrub directory freespace Dan Carpenter
@ 2017-11-06 19:34 ` Darrick J. Wong
0 siblings, 0 replies; 2+ messages in thread
From: Darrick J. Wong @ 2017-11-06 19:34 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-xfs
On Sat, Nov 04, 2017 at 10:54:57AM +0300, Dan Carpenter wrote:
> Hello Darrick J. Wong,
>
> The patch df481968f33b: "xfs: scrub directory freespace" from Oct 17,
> 2017, leads to the following static checker warning:
>
> fs/xfs/scrub/dir.c:446 xfs_scrub_directory_check_freesp()
> info: ignoring unreachable code.
>
> fs/xfs/scrub/dir.c
> 431 STATIC void
> 432 xfs_scrub_directory_check_freesp(
> 433 struct xfs_scrub_context *sc,
> 434 xfs_dablk_t lblk,
> 435 struct xfs_buf *dbp,
> 436 unsigned int len)
> 437 {
> 438 struct xfs_dir2_data_free *bf;
> 439 struct xfs_dir2_data_free *dfp;
> 440 int offset;
> 441
> 442 if (len == 0)
> 443 return;
> 444
> 445 bf = sc->ip->d_ops->data_bestfree_p(dbp->b_addr);
> 446 for (dfp = &bf[0]; dfp < &bf[XFS_DIR2_DATA_FD_COUNT]; dfp++) {
> ^^^^^
> This looks like a loop
>
> 447 offset = be16_to_cpu(dfp->offset);
> 448 if (offset == 0)
> 449 break;
> 450 if (len == be16_to_cpu(dfp->length))
> 451 return;
> 452 /* Didn't find the best length in the bestfree data */
> 453 break;
> ^^^^^^
> but we always either break or return on the first iteration. What's
> going on?
I think this is a broken refactoring of the bestfree checker --
previously we'd examine every slot in a directory's bestfree data block,
read the corresponding directory data block, and check that the slot
matches any of the data block's bestfree entries. Then Dave pointed out
that the slot must match the /first/ entry in the dir data block, so we
only need to examine bf[0], and this loop can be unrolled.
Thanks for catching this!
--D
> 454 }
> 455
> 456 xfs_scrub_fblock_set_corrupt(sc, XFS_DATA_FORK, lblk);
> 457 }
>
> regards,
> dan carpenter
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-11-06 19:34 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-04 7:54 [bug report] xfs: scrub directory freespace Dan Carpenter
2017-11-06 19:34 ` Darrick J. Wong
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.