xfs
[Top] [All Lists]

Re: [PATCH 26/30] xfs_db: avoid libxfs buffer lookup warnings

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 26/30] xfs_db: avoid libxfs buffer lookup warnings
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Mon, 4 Nov 2013 01:12:45 -0800
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1383107481-28937-27-git-send-email-david@xxxxxxxxxxxxx>
References: <1383107481-28937-1-git-send-email-david@xxxxxxxxxxxxx> <1383107481-28937-27-git-send-email-david@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
>  
> +/*
> + * We are now using libxfs for our IO backend, so we should always try to use
> + * inode cluster buffers rather than filesystem block sized buffers for 
> reading
> + * inodes. This means that we always use the same buffers as libxfs 
> operations
> + * does, and that avoids buffer cache issues caused by overlapping buffers. 
> This
> + * can be seen clearly when trying to read the root inode. Much of this 
> logic is
> + * similar to libxfs_imap().
> + */
>  void
>  set_cur_inode(
>       xfs_ino_t       ino)
> @@ -632,6 +640,9 @@ set_cur_inode(
>       xfs_agnumber_t  agno;
>       xfs_dinode_t    *dip;
>       int             offset;
> +     int             numblks = blkbb;
> +     xfs_agblock_t   cluster_agbno;
> +
>  
>       agno = XFS_INO_TO_AGNO(mp, ino);
>       agino = XFS_INO_TO_AGINO(mp, ino);
> @@ -644,6 +655,24 @@ set_cur_inode(
>               return;
>       }
>       cur_agno = agno;
> +
> +     if (mp->m_inode_cluster_size > mp->m_sb.sb_blocksize &&
> +         mp->m_inoalign_mask) {
> +             xfs_agblock_t   chunk_agbno;
> +             xfs_agblock_t   offset_agbno;
> +             int             blks_per_cluster;
> +
> +             blks_per_cluster = mp->m_inode_cluster_size >>
> +                                                     mp->m_sb.sb_blocklog;
> +             offset_agbno = agbno & mp->m_inoalign_mask;
> +             chunk_agbno = agbno - offset_agbno;
> +             cluster_agbno = chunk_agbno +
> +                     ((offset_agbno / blks_per_cluster) * blks_per_cluster);
> +             offset += ((agbno - cluster_agbno) * mp->m_sb.sb_inopblock);
> +             numblks = XFS_FSB_TO_BB(mp, blks_per_cluster);
> +     } else
> +             cluster_agbno = agbno;
> +

Seems like this should be a separate patch?

> -     if (iocur_top->bp)
> +     if (iocur_top->bp) {
>               libxfs_putbuf(iocur_top->bp);
> +             iocur_top->bp = NULL;
> +     }

This should probably be folded into the patch that started using buffers
in xfs_db.

> +     int             icache_flags;   /* cache init flags */
> +     int             bcache_flags;   /* cache init flags */

The icache_flags aren't used anywhere.  Also I'd really prefer to have
my patch to kill the inode cache istead of adding more changes to it.

> +#ifdef IO_BCOMPARE_CHECK
> +             if (!(libxfs_bcache->c_flags & CACHE_MISCOMPARE_PURGE)) {
> +                     fprintf(stderr,
> +     "%lx: Badness in key lookup (length)\n"
> +     "bp=(bno 0x%llx, len %u bytes) key=(bno 0x%llx, len %u bytes)\n",
> +                             pthread_self(),
> +                             (unsigned long long)bp->b_bn, (int)bp->b_bcount,
> +                             (unsigned long long)bkey->blkno,
> +                             BBTOB(bkey->bblen));
> +             }
>  #endif

What is the point of the IO_BCOMPARE_CHECK ifdef if we unconditionally
define it?

<Prev in Thread] Current Thread [Next in Thread>