Re: [PATCHv2 5/5] xfs: kill off l_sectbb_mask

[Alex Elder <aelder@xxxxxxx>]

On Sun, 2010-04-18 at 13:29 -0400, Christoph Hellwig wrote:
> On Fri, Apr 16, 2010 at 03:54:05PM -0500, Alex Elder wrote:
> > + * Return the address of the start of the given block number's data
> > + * in a log buffer.  The buffer covers a log sector-aligned region.
> > + */
> >  STATIC xfs_caddr_t
> >  xlog_align(
> >     xlog_t          *log,
> > @@ -128,14 +132,14 @@ xlog_align(
> >     int             nbblks,
> >     xfs_buf_t       *bp)
> >  {
> > +   xfs_daddr_t     offset;
> >     xfs_caddr_t     ptr;
> >  
> > -   if (log->l_sectBBsize == 1)
> > -           return XFS_BUF_PTR(bp);
> > +   offset = blk_no & ((xfs_daddr_t) log->l_sectBBsize - 1);
> > +   ptr = XFS_BUF_PTR(bp) + BBTOB(offset);
> > +
> > +   ASSERT(ptr + BBTOB(nbblks) <= XFS_BUF_PTR(bp) + XFS_BUF_SIZE(bp));
> >  
> > -   ptr = XFS_BUF_PTR(bp) + BBTOB((int)blk_no & log->l_sectbb_mask);
> > -   ASSERT(XFS_BUF_SIZE(bp) >=
> > -           BBTOB(nbblks + (blk_no & log->l_sectbb_mask)));
> >     return ptr;
> 
> And btw, now that I think about it we can just remove the
> log->l_sectBBsize == 1 case entirely, so the whole thing could become:

Yes, I contemplated that and am happy to do it that way.
I chose this way because there seemed to be "so much" cruft
that was not needed in that special case...  Anyway, I'll
change it to the way you suggest before committing it.

Thanks for the review.

                                        -Alex

> STATIC xfs_caddr_t
> xlog_align(
>       struct log      *log,
>       xfs_daddr_t     blk_no,
>       int             nbblks,
>       struct xfs_buf  *bp)
> {
>       xfs_daddr_t     offset;
> 
>       offset = blk_no & ((xfs_daddr_t) log->l_sectBBsize - 1);
>       ASSERT(BBTOB(offset + nbblks) <= XFS_BUF_SIZE(bp));
> 
>       return XFS_BUF_PTR(bp) + BBTOB(offset);
> }