linux-kernel - Re: [V3 PATCH 1/2] tmpfs: add fallocate support

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: <alpine.LSU.2.00.1111231407170.2573@sister.anvils>
Date:	Wed, 23 Nov 2011 14:20:12 -0800 (PST)
From:	Hugh Dickins <hughd@...gle.com>
To:	Pekka Enberg <penberg@...nel.org>
cc:	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	Cong Wang <amwang@...hat.com>, linux-kernel@...r.kernel.org,
	akpm@...ux-foundation.org, Christoph Hellwig <hch@....de>,
	Dave Hansen <dave@...ux.vnet.ibm.com>,
	Lennart Poettering <lennart@...ttering.net>,
	Kay Sievers <kay.sievers@...y.org>, linux-mm@...ck.org
Subject: Re: [V3 PATCH 1/2] tmpfs: add fallocate support

On Wed, 23 Nov 2011, Pekka Enberg wrote:
> On Wed, Nov 23, 2011 at 9:59 PM, KOSAKI Motohiro
> <kosaki.motohiro@...fujitsu.com> wrote:
> >> +
> >> +       goto unlock;
> >> +
> >> +undo:
> >> +       while (index > start) {
> >> +               shmem_truncate_page(inode, index);
> >> +               index--;
> >
> > Hmmm...
> > seems too aggressive truncate if the file has pages before starting fallocate.
> > but I have no idea to make better undo. ;)
> 
> Why do we need to undo anyway?

One answer comes earlier in this thread:

On Mon, Nov 21, 2011, Christoph Hellwig write:
> On Sun, Nov 20, 2011 at 01:22:10PM -0800, Hugh Dickins wrote:
> > First question that springs to mind (to which I shall easily find
> > an answer): is it actually acceptable for fallocate() to return
> > -ENOSPC when it has already completed a part of the work?
> 
> No, it must undo all allocations if it returns ENOSPC.

Another answer would be: if fallocate() had been defined to return
the length that has been successfully allocated (as write() returns
the length written), then it would be reasonable to return partial
length instead of failing with ENOSPC, and not undo.  But it was
defined to return -1 on failure or 0 on success, so cannot report
partial success.

Another answer would be: if the disk is near full, it's not good
for a fallocate() to fail with -ENOSPC while nonetheless grabbing
all the remaining blocks; even worse if another fallocate() were
racing with it.

Hugh