On Wed, 2006-08-16 at 14:28 +1000, Nathan Scott wrote:
> On Fri, Aug 11, 2006 at 01:26:26PM +1000, David Chinner wrote:
> > ...
> > Looks OK.
>
> Thanks mate.
>
> > ...
> > If you are hinting this branch to be unlikely, then we should also
> > check the start block first before checking the io_flags. We only
> > need to check the io_flags if we are actually accessing block zero.
>
> I've rolled this into a new version. I also got prodded to still
> provide an option to panic on detecting this, for debugging - so I
> rewrote this to report through the panic_mask mechanism, which now
> gives us the option to optionally panic if need be... could you do
> a second pass over this for me?
>
> cheers.
>
> --
> Nathan
>
>
> Index: xfs-linux/xfs_bmap.c
> ===================================================================
> --- xfs-linux.orig/xfs_bmap.c 2006-08-15 15:22:53.198187750 +1000
> +++ xfs-linux/xfs_bmap.c 2006-08-16 12:40:49.669518000 +1000
> @@ -3705,7 +3705,7 @@ STATIC xfs_bmbt_rec_t *
> xfs_bmap_search_extents(
> xfs_inode_t *ip, /* incore inode pointer */
> xfs_fileoff_t bno, /* block number searched for */
> - int whichfork, /* data or attr fork */
> + int fork, /* data or attr fork */
> int *eofp, /* out: end of file found */
> xfs_extnum_t *lastxp, /* out: last extent index */
> xfs_bmbt_irec_t *gotp, /* out: extent entry found */
> @@ -3713,25 +3713,28 @@ xfs_bmap_search_extents(
> {
> xfs_ifork_t *ifp; /* inode fork pointer */
> xfs_bmbt_rec_t *ep; /* extent record pointer */
> - int rt; /* realtime flag */
>
> XFS_STATS_INC(xs_look_exlist);
> - ifp = XFS_IFORK_PTR(ip, whichfork);
> + ifp = XFS_IFORK_PTR(ip, fork);
>
> ep = xfs_bmap_search_multi_extents(ifp, bno, eofp, lastxp, gotp, prevp);
>
> - rt = (whichfork == XFS_DATA_FORK) && XFS_IS_REALTIME_INODE(ip);
> - if (unlikely(!rt && !gotp->br_startblock && (*lastxp != NULLEXTNUM))) {
> - cmn_err(CE_PANIC,"Access to block zero: fs: <%s> inode: %lld
> "
> - "start_block : %llx start_off : %llx blkcnt : %llx "
> - "extent-state : %x \n",
> - (ip->i_mount)->m_fsname, (long long)ip->i_ino,
> + if (unlikely(!(gotp->br_startblock) && (*lastxp != NULLEXTNUM) &&
> + !(XFS_IS_REALTIME_INODE(ip) && fork == XFS_DATA_FORK))) {
> + xfs_cmn_err(XFS_PTAG_FSBLOCK_ZERO, CE_ALERT, ip->i_mount,
> + "Access to block zero in inode %llu "
> + "start_block: %llx start_off: %llx "
> + "blkcnt: %llx extent-state: %x lastx: %x\n",
> + (unsigned long long)ip->i_ino,
> (unsigned long long)gotp->br_startblock,
> (unsigned long long)gotp->br_startoff,
> (unsigned long long)gotp->br_blockcount,
> - gotp->br_state);
> - }
> - return ep;
> + gotp->br_state, *lastxp);
> + *lastxp = NULLEXTNUM;
> + *eofp = 1;
> + return NULL;
> + }
> + return ep;
> }
>
>
> Index: xfs-linux/xfs_iomap.c
> ===================================================================
> --- xfs-linux.orig/xfs_iomap.c 2006-08-15 15:22:53.238190250 +1000
> +++ xfs-linux/xfs_iomap.c 2006-08-16 12:40:41.385000250 +1000
> @@ -536,23 +536,27 @@ xfs_iomap_write_direct(
> * Copy any maps to caller's array and return any error.
> */
> if (nimaps == 0) {
> - error = (ENOSPC);
> + error = ENOSPC;
> goto error_out;
> }
>
> - *ret_imap = imap;
> - *nmaps = 1;
> - if ( !(io->io_flags & XFS_IOCORE_RT) && !ret_imap->br_startblock) {
> - cmn_err(CE_PANIC,"Access to block zero: fs <%s> inode: %lld
> "
> - "start_block : %llx start_off : %llx blkcnt : %llx "
> - "extent-state : %x \n",
> - (ip->i_mount)->m_fsname,
> - (long long)ip->i_ino,
> - (unsigned long long)ret_imap->br_startblock,
> + if (unlikely(!ret_imap->br_startblock &&
> + !(io->io_flags & XFS_IOCORE_RT))) {
> + xfs_cmn_err(XFS_PTAG_FSBLOCK_ZERO, CE_ALERT, ip->i_mount,
> + "Access to block zero in inode %llu "
> + "start_block: %llx start_off: %llx "
> + "blkcnt: %llx extent-state: %x\n",
> + (unsigned long long)ip->i_ino,
> + (unsigned long long)ret_imap->br_startblock,
> (unsigned long long)ret_imap->br_startoff,
> - (unsigned long long)ret_imap->br_blockcount,
> + (unsigned long long)ret_imap->br_blockcount,
> ret_imap->br_state);
> - }
> + error = EFSCORRUPTED;
should this be XFS_ERROR(EFSCORRUPTED) ?
perhaps change to use it for consistency if the conversion is done in
error_out?
> + goto error_out;
--
Stewart Smith (stewart@xxxxxxxxxxxxxxxx)
http://www.flamingspork.com/
signature.asc
Description: This is a digitally signed message part
|