On Mon, Jul 28, 2014 at 12:19:25PM -0400, Brian Foster wrote: > On Fri, Jul 25, 2014 at 08:41:12AM +1000, Dave Chinner wrote: > > On Thu, Jul 24, 2014 at 10:22:58AM -0400, Brian Foster wrote: > > > + struct xfs_btree_cur *cur; > > > + struct xfs_agi *agi = XFS_BUF_TO_AGI(agbp); > > > + xfs_agnumber_t agno = be32_to_cpu(agi->agi_seqno); > > > + xfs_agino_t previno; > > > + int error; > > > + int i; > > > + struct xfs_inobt_rec_incore rec; > > > + > > > + orec->ir_startino = NULLAGINO; > > > + > > > + cur = xfs_inobt_init_cursor(mp, tp, agbp, agno, btnum); > > > + > > > + previno = newino + count - XFS_INODES_PER_CHUNK; > > > + error = xfs_inobt_lookup(cur, previno, XFS_LOOKUP_GE, &i); > > > > You want XFS_LOOKUP_EQ, yes? i.e. XFS_LOOKUP_GE won't fail if the > > exact record for the inode chunk does not exist - it will return the > > next one in the btree. > > > > Assuming variable sparse chunk granularity, Isn't the granularity fixed for the specific filesystem configuration as part of the on-disk format? > I don't really know the > start ino of the record that potentially covers the new inode chunk. > Given that, we use the smallest possible start ino that could include > this chunk and search forward from there. As you've noted below, I > wasn't relying on failure here to detect the scenario where there is no > existing record. Ok, that's not how I thought the code was attempting to implement the "has record" check. My mistake - a comment explaining how the match is supposed to work would be helpful, I think. However, with that in mind, why do you even bother calculating at "previno"? If you want the chunk that the "newino" lies in, then by definition it's going to be the first record at an equal or lower start inode number than newino. i.e.: xfs_inobt_lookup(cur, newino, XFS_LOOKUP_LE, &i); Will return either: - a match with startino <= newino < startino + XFS_INODES_PER_CHUNK - a match with startino + XFS_INODES_PER_CHUNK <= newino - a failure due to no record. i.e. the first case is the chunk record we want, the others are "does not exist" failures. We don't need to calculate the "previno" at all. Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
