xfs
[Top] [All Lists]

Re: [PATCH 2/7] xfs: fix min bufsize bugs in two places

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH 2/7] xfs: fix min bufsize bugs in two places
From: Alex Elder <aelder@xxxxxxx>
Date: Thu, 25 Mar 2010 09:23:34 -0500
Cc: xfs@xxxxxxxxxxx
In-reply-to: <20100320164249.GB31444@xxxxxxxxxxxxx>
References: <201003182253.o2IMrp54001850@xxxxxxxxxxxxxxxxxxxxxx> <20100320164249.GB31444@xxxxxxxxxxxxx>
Reply-to: aelder@xxxxxxx
On Sat, 2010-03-20 at 12:42 -0400, Christoph Hellwig wrote:
> > +/* Number of basic blocks in a log sector */
> > +#define xlog_sectbb(log) (1 << (log)->l_sectbb_log)
> 
> Looking at all uses of (log)->l_sectbb_log I wonder if we should
> bother storing this in the log structure in this form, or rather
> as the multipler of the basic block size, ala l_sectsize.  All the

I agree, and I was sort of headed in that direction.  I have
more work in this file that eventually will lead to some larger
scale (algorithmic) simplification.  But for now I'm starting
small.

>       if (log->l_sectbb_log) {
> 
> checks would just become
> 
>       if (log->l_sectsize > 1) {
> 
> and the xlog_find_verify_cycle/xlog_write_log_records checks
> would also be a natural
> 
>       if (bufblks < log->l_sectsize)

I'll attack this in a later patch.

> The comments added are defintively useful, btw.

Thanks, I felt they were necessary.  More to come.

I got "looks good" from you on all but this patch,
and numbers 5 and 7 in the series.  Is this one OK,
and do you plan to review those other two?

                                        -Alex

<Prev in Thread] Current Thread [Next in Thread>