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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180607205116.6rilmatbhv33b6vn@localhost.localdomain>
Date:   Thu, 7 Jun 2018 16:51:16 -0400
From:   Eric Whitney <enwlinux@...il.com>
To:     Jan Kara <jack@...e.cz>
Cc:     Eric Whitney <enwlinux@...il.com>, linux-ext4@...r.kernel.org,
        tytso@....edu
Subject: Re: [RFC PATCH 2/5] ext4: reduce reserved cluster count by number of
 allocated clusters

* Jan Kara <jack@...e.cz>:
> 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@...il.com>
> 
> Nice simplification. Just one small comment below. With that fixed you can
> add:
> 
> Reviewed-by: Jan Kara <jack@...e.cz>
> 
> ...
> > +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@...e.com>
> SUSE Labs, CR