On Thu, Mar 14, 2013 at 11:03:21AM -0500, Ben Myers wrote:
> Dave,
>
> On Tue, Mar 12, 2013 at 11:30:42PM +1100, Dave Chinner wrote:
> > From: Christoph Hellwig <hch@xxxxxx>
> >
> > Add a new inode version with a larger core. The primary objective is
> > to allow for a crc of the inode, and location information (uuid and ino)
> > to verify it was written in the right place. We also extend it by:
> >
> > a creation time (for Samba);
> > a changecount (for NFSv4);
> > a flush sequence (in LSN format for recovery);
> > an additional inode flags field; and
> > some additional padding.
> >
> > These additional fields are not implemented yet, but already laid
> > out in the structure.
> >
> > [dchinner@xxxxxxxxxx] Added LSN and flags field, some factoring and rework to
> > capture all the necessary information in the crc calculation.
>
> Comments and questions below.
...
> > @@ -190,8 +191,18 @@ xfs_ialloc_inode_init(
> > * the new inode format, then use the new inode version. Otherwise
> > * use the old version so that old kernels will continue to be
> > * able to use the file system.
> > + *
> > + * For v3 inodes, we also need to write the inode number into the inode,
> > + * so calculate the first inode number of the chunk here as
> > + * XFS_OFFBNO_TO_AGINO() only works on filesystem block boundaries, not
> > + * cluster boundaries and so cannot be used in the cluster buffer loop
> > + * below.
>
> I'm having some trouble understanding your comment. Maybe you can help me:
>
> > */
> > - if (xfs_sb_version_hasnlink(&mp->m_sb))
> > + if (xfs_sb_version_hascrc(&mp->m_sb)) {
> > + version = 3;
> > + ino = XFS_AGINO_TO_INO(mp, agno,
> > + XFS_OFFBNO_TO_AGINO(mp, agbno, 0));
> > + } else if (xfs_sb_version_hasnlink(&mp->m_sb))
> > version = 2;
> > else
> > version = 1;
> > @@ -217,13 +228,21 @@ xfs_ialloc_inode_init(
>
> My reading of the loop here is ...
>
> 210 for (j = 0; j < nbufs; j++) {
>
> for each inode cluster, j
>
> 211 /*
> 212 * Get the block.
> 213 */
> 214 d = XFS_AGB_TO_DADDR(mp, agno, agbno + (j * blks_per_cluster));
>
> convert to disk address ( this AG, the AGBLOCK of the initial inode cluster plus
> (current cluster j * blocks per cluster))
>
> 215 fbuf = xfs_trans_get_buf(tp, mp->m_ddev_targp, d,
> 216 mp->m_bsize * blks_per_cluster,
> 217 XBF_UNMAPPED);
>
> get a buffer at that disk address of length (filesystem block size times the number of blocks per cluster)
>
> which is the full length of the inode cluster
>
> 218 if (!fbuf)
> 219 return ENOMEM;
> 220 /*
> 221 * Initialize all inodes in this buffer and then log them.
> 222 *
> 223 * XXX: It would be much better if we had just one transaction
> 224 * to log a whole cluster of inodes instead of all the
> 225 * individual transactions causing a lot of log traffic.
> 226 */
> 227 fbuf->b_ops = &xfs_inode_buf_ops;
> 228 xfs_buf_zero(fbuf, 0, ninodes << mp->m_sb.sb_inodelog);
>
> Zero the whole cluster, including literal areas
>
> 229 for (i = 0; i < ninodes; i++) {
>
> for each inode, i
>
> 230 int ioffset = i << mp->m_sb.sb_inodelog;
> 231 uint isize = xfs_dinode_size(version);
> 232
> 233 free = xfs_make_iptr(mp, fbuf, i);
>
> get a pointer into the buf to the beginning of i's inode core
>
> 234 free->di_magic = cpu_to_be16(XFS_DINODE_MAGIC);
> 235 free->di_version = version;
> 236 free->di_gen = cpu_to_be32(gen);
> 237 free->di_next_unlinked = cpu_to_be32(NULLAGINO);
>
> initialize some important stuff
>
> 238
> 239 if (version == 3) {
> 240 free->di_ino = cpu_to_be64(ino);
> 241 ino++;
>
> initialize ino on verion 3 inodes. and add one to ino for the next run of this loop.
>
> It appears that for subsequent clusters where j > 1 this would stamp the wrong
j > 0
D'oh.
-Ben
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
