I like the factoring, but it could go a littler further. Comments below: > /* > + * Loop over all clusters in a chunk for a given incore inode > + * allocation btree record. Do a readahead if there are any > + * allocated inodes in that cluster. > + */ Try to use the full 80 lines available to you for comment. > + xfs_agblock_t agbno = XFS_AGINO_TO_AGBNO(mp, > + irec->ir_startino); > + int nicluster, nbcluster; > + int chunkidx; > + > + nicluster = mp->m_sb.sb_blocksize >= XFS_INODE_CLUSTER_SIZE(mp) ? > + mp->m_sb.sb_inopblock : > + (XFS_INODE_CLUSTER_SIZE(mp) >> mp->m_sb.sb_inodelog); > + nbcluster = nicluster >> mp->m_sb.sb_inopblog; I'd prefer to factor this out even further. xfs_ialloc_inode_init and xfs_ifree_cluster already have two pieces of code that calculate these two (with more readable names) and an additional nuber buffers counter we won't need here, it might make most sense to factor that into a single common helper. > +/* > + * Lookup clusters in inode chunk for a given incore inobt record, > + * do readahead if there are any allocated inodes in that cluster. > + */ > +void xfs_inobt_reada_chunk(struct xfs_mount *mp, xfs_agnumber_t agno, > + struct xfs_inobt_rec_incore *irec); No need to duplicate the comment in the header. _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
