Re: [RFC PATCH 2/5] ext4: reduce reserved cluster count by number of allocated clusters

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



* Jan Kara <jack@xxxxxxx>:
> On Sun 13-05-18 13:56:21, Eric Whitney wrote:
> > Ext4 does not always reduce the reserved cluster count by the number
> > of clusters allocated when mapping a delayed extent.  It sometimes
> > adds back one or more clusters after allocation if delalloc blocks
> > adjacent to the range allocated by ext4_ext_map_blocks() share the
> > clusters newly allocated for that range.  However, this overcounts
> > the number of clusters needed to satisfy future mapping requests
> > (holding one or more reservations for clusters that have already been
> > allocated) and premature ENOSPC and quota failures, etc., result.
> > 
> > The current ext4 code does not reduce the reserved cluster count
> > when allocating clusters for non-delalloc writes that have also been
> > previously reserved for delalloc writes.  This also results in a
> > reserved cluster overcount.
> > 
> > To make it possible to handle reserved cluster accounting for
> > fallocated regions in the same manner as used for other non-delayed
> > writes, do the reserved cluster accounting for them at the time of
> > allocation.  In the current code, this is only done later when a
> > delayed extent sharing the fallocated region is finally mapped.  This
> > behavior can also result in a temporary reserved cluster overcount.
> > 
> > Signed-off-by: Eric Whitney <enwlinux@xxxxxxxxx>
> 
> Nice simplification. Just one small comment below. With that fixed you can
> add:
> 
> Reviewed-by: Jan Kara <jack@xxxxxxx>
> 
> ...
> > +static unsigned int __es_delayed_clu(struct inode *inode, ext4_lblk_t start,
> > +				     ext4_lblk_t end)
> > +{
> > +	struct ext4_es_tree *tree = &EXT4_I(inode)->i_es_tree;
> > +	struct extent_status *es;
> > +	struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
> > +	struct rb_node *node;
> > +	ext4_lblk_t first_lclu, last_lclu;
> > +	unsigned long long last_counted_lclu;
> > +	unsigned int n = 0;
> > +
> > +	/* guaranteed to be unequal to any ext4_lblk_t value */
> > +	last_counted_lclu = ~0;
> 
> Missing ULL here? ~0 actually also is not valid lblk offset (look at
> ext4_max_size()) but then last_counted_lclu could be just ext4_lblk_t...

Oops, yes - good catch - will fix.  I did want a clearly invalid block
offset here.

Thanks for your review!

Eric


> 
> 								Honza
> -- 
> Jan Kara <jack@xxxxxxxx>
> SUSE Labs, CR



[Index of Archives]     [Reiser Filesystem Development]     [Ceph FS]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite National Park]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]     [Linux Media]

  Powered by Linux