On Thu, Oct 14, 2010 at 12:22:45PM -0500, Alex Elder wrote:
> On Mon, 2010-10-04 at 21:13 +1100, Dave Chinner wrote:
> > @@ -427,11 +431,25 @@ xfs_iomap_eof_want_preallocate(
> > if ((imap[n].br_startblock != HOLESTARTBLOCK) &&
> > (imap[n].br_startblock != DELAYSTARTBLOCK))
> > return 0;
> > +
> > start_fsb += imap[n].br_blockcount;
> > count_fsb -= imap[n].br_blockcount;
> > +
> > + /* count delalloc blocks beyond EOF */
> > + if (imap[n].br_startblock == DELAYSTARTBLOCK)
> > + found_delalloc += imap[n].br_blockcount;
> > }
> > }
> > - *prealloc = 1;
>
> At this point, count_fsb will be 0 (a necessary condition
> for loop termination, since count_fsb is unsigned). Since
> found_delalloc is initially 0 (and is also unsigned), we
> can safely assume that found_delalloc >= count_fsb. The
> only case in which found_delalloc <= count_fsb is if
> found_delalloc is also 0 (a case you cover separately,
> first, below).
>
> Furthermore, *prealloc was assigned the value 0 at the top.
> So I think this bottom section can be simplified to:
> if (!found_delalloc)
> *prealloc = 1;
I'll have a look at this - there was some reason for the second
case, but the code has changed since I needed it and, as you
suggest, it might not be needed anymore.
> > error = xfs_iomap_eof_want_preallocate(mp, ip, offset, count,
> > ioflag, imap, XFS_WRITE_IMAPS, &prealloc);
> > if (error)
> > @@ -476,9 +495,25 @@ xfs_iomap_write_delay(
> >
> > retry:
> > if (prealloc) {
> > + xfs_fileoff_t alloc_blocks = 0;
> > + /*
> > + * If we don't have a user specified preallocation size, dynamically
> > + * increase the preallocation size as we do more preallocation.
> > + * Cap the maximum size at a single extent.
> > + */
> > + if (!(mp->m_flags & XFS_MOUNT_DFLT_IOSIZE)) {
>
> I note that this is circumventing the special code in
> xfs_set_rw_sizes() that tries to set up a different (smaller)
> size in the event the "sync" (generic) mount option was supplied
> (indicated by XFS_MOUNT_SYNC). If that is a good thing, then I
> suggest the code in xfs_set_rw_sizes() go away. But it would be
> good to have the case for making that change stated.
The new code based on file size is a bit different. It still
triggers off the absence of his flag, but it now uses the default
sizes as the minimum speculative allocation size.
> I guess the reason one might want the "allocsize" mount
> option now becomes the opposite of why one might have
> wanted it before. I.e., it would be used to *reduce*
> the size of the preallocated range beyond EOF, which I
> could envision might be reasonable in some circumstances.
It now becomes the minimum preallocation size, rather than both the
minimum and the maximum....
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
