xfs
[Top] [All Lists]

Re: [PATCH 4/6] xfs: swalloc doesn't align allocations properly

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>, Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 4/6] xfs: swalloc doesn't align allocations properly
From: Ben Myers <bpm@xxxxxxx>
Date: Mon, 16 Dec 2013 17:14:14 -0600
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20131213120123.GA32749@xxxxxxxxxxxxx>
References: <1386826478-13846-1-git-send-email-david@xxxxxxxxxxxxx> <1386826478-13846-5-git-send-email-david@xxxxxxxxxxxxx> <20131213120123.GA32749@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.20 (2009-06-14)
Hi Gents,

On Fri, Dec 13, 2013 at 04:01:23AM -0800, Christoph Hellwig wrote:
> Looks good.
> 
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
> 
> Two very minor nitpicks below:
> 
> > +   int             stripe_align;
> >  
> >     ASSERT(ap->length);
> >  
> >     mp = ap->ip->i_mount;
> > +
> > +   /* stripe alignment for allocation is determined by mount parameters */
> > +   stripe_align = 0;
> > +   if (mp->m_swidth && (mp->m_flags & XFS_MOUNT_SWALLOC))
> > +           stripe_align = mp->m_swidth;
> > +   else if (mp->m_dalign)
> > +           stripe_align = mp->m_dalign;
> 
> nipick: I'd either initialize the variable to zero at the point of the
> declaration or do if .. else if .. else here.
> 
> >     }
> > +
> > +
> >     nullfb = *ap->firstblock == NULLFSBLOCK;
> 
> Two newlines seem odd here.  I'd support one even if that's an unrelated
> change :)

This is probably not the right thing to do for small files.  They will all end
up in the first stripe unit.

Quoting jpk from
http://lwn.net/Articles/87526/                                                  
                                                                                
                                                 
                                                                                
                                                                                
                                                 
"  o [XFS] Add support for allocating additional file space in stripe width
sized chunks. A new fstab/mount option, "swalloc" has been defined. If
specified when mounting a striped file system, allocation requests will be
rounded up to a stripe width if the file size is >= stripe width, and the data
is being appended to eof. The 'swalloc' option is "off" by default. "


This feature was likely designed with volume stripes in mind as opposed to to
raid stripes.

-Ben

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