linux-ext4 - Re: [RFC][PATCH] ext4: Convert uninitialized extent to initialized extent in case of file system full

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-id: <20080229180527.GD2997@webber.adilger.int>
Date:	Fri, 29 Feb 2008 10:05:27 -0800
From:	Andreas Dilger <adilger@....com>
To:	Mingming Cao <cmm@...ibm.com>
Cc:	"Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>,
	linux-ext4@...r.kernel.org
Subject: Re: [RFC][PATCH] ext4: Convert uninitialized extent to initialized
	extent in case of file system full

On Feb 28, 2008  15:14 -0800, Mingming Cao wrote:
> On Thu, 2008-02-28 at 23:35 +0530, Aneesh Kumar K.V wrote:
> A write to prealloc area cause the split of unititalized extent into
> a initialized and uninitialized extent. If we don't have space to
> add new extent information instead of returning error convert the
> existing uninitialized extent to initialized one. We need to zero out
> the blocks corresponding to the extent to prevent wrong data reaching
> userspace.

> > +/* FIXME!! we need to try to merge to left or right after zerout  */
> > +static int ext4_ext_zeroout(handle_t *handle, struct inode *inode,
> > +				ext4_lblk_t iblock, struct ext4_extent *ex)
> > +{
> > +}
> > +
> 
> The complexity added to the code to handle the corner case seems not
> worth the effort. 
> 
> One simple solution is submit bio directly to zero out the blocks on
> disk, and wait for that to finish before clear the uninitialized bit. On
> a 4K block size case, the max size of an uninitialized extents is 128MB,
> and since the blocks are all contigous on disk, a single IO could done
> the job, the latency should not be a too big issue. After all when a
> filesystem is full, it's already performs slowly.

Further to Mingming's comments:
- you can map the ZERO_PAGE to every entry in the bio, which will avoid
  the very significant problem of needing 128MB of pages to zero out the
  extent
- make sure you limit the extent size to BIO_MAX_PAGES
- submitting large bios to the block layer is MUCH more efficient than
  adding pages to the page cache because the block device can do a very
  good job of writing this out
- make sure you wait for bio completion before you allow the block IO
  to begin.  In Lustre we did this by passing a waitq and our own
  completion function to the bio and have the caller go to sleep until
  the bio completion function is called.  Note that the completion
  function may be called multiple times if there are block errors.
- zeroing out pages in the page cache is very dangerous because they
  may already have dirty data in them.
- please make a helper function like "ext4_zero_blocks()" because at
  some point in the future I'd like to add the ability to have the kernel
  zero out inode table blocks for filesystems formatted with
  "-O uninit_groups,lazy_bg"

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html