On Mon, Nov 04, 2013 at 01:12:45AM -0800, Christoph Hellwig wrote:
> >
> > +/*
> > + * 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?
Yes, probably should be.
> > - 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.
Will do.
>
> > + 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.
Sure - should I prepend your series to kill the icache to this one?
> > +#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?
I'd like to - eventually - turn it off. In fact, I'd like to end up
with a real debug xfsprogs build like we do for the kernel code so
that the version that distros ship don't have this noise enabled but
developers can easily enable all the different debug/tracing stuff
to be able to find problems in the code....
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
