linux-kernel - Re: [Patch] 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]

Date:	Wed, 16 Nov 2011 17:31:36 -0800 (PST)
From:	Hugh Dickins <hughd@...gle.com>
To:	Cong Wang <amwang@...hat.com>
cc:	Pekka Enberg <penberg@...nel.org>,
	Lennart Poettering <lennart@...ttering.net>,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>,
	Dave Hansen <dave@...ux.vnet.ibm.com>,
	linux-kernel@...r.kernel.org, akpm@...ux-foundation.org,
	linux-mm@...ck.org, kay.sievers@...y.org
Subject: Re: [Patch] tmpfs: add fallocate support

On Wed, 16 Nov 2011, Cong Wang wrote:
> 于 2011年11月16日 15:12, Pekka Enberg 写道:
> > On Wed, 16 Nov 2011, Cong Wang wrote:
> > > > What's the use case for this?
> > > 
> > > Systemd needs it, see http://lkml.org/lkml/2011/10/20/275.
> > > I am adding Kay into Cc.
> > 
> > The post doesn't mention why it needs it, though.
> > 
> 
> Right, I should mention this in the changelog. :-/

Yes, but I think Pekka's point is that the page which you link to does not
explain why Plumbers would want tmpfs to support fallocate() properly.

What good is it going to do for them?  Why not just do it in userspace,
either by dd if=/dev/zero of=tmpfsfile, or by mmap() and touch if very
anxious to avoid the triple memset/memcpy (once reading from /dev/zero,
once allocating tmpfs pages, once copying to tmpfs pages)?  Or splice().

I don't want to stand in the way of progress, but there's a lot of
things tmpfs does not support (a persistent filesystem would be top
of the list; but readahead, direct I/O, AIO, ....), and it may be
better to continue not to support them unless there's good reason.
tmpfs does not have a disk layout that we need to optimize.

I did not study your implementation in detail, but agree with Dave
and Kame that (if it needs to be in kernel at all) you should reuse
the existing code rather than repeating extracts: shmem_getpage_gfp()
is the one place which looks after all of shmem page allocation, so
I'd prefer you just make a loop of calls to that (with a new sgp_type
if there's particular reason to do something differently in there).

I've not yet looked up the specification of fallocate(), but it
looked surprising to be allocating pages up to the point where a
page already exists (when shmem_add_to_page_cache will fail) and
then giving up with -EEXIST.

Seeing your Subject, I imagined at first that you would be implementing
FALLOC_FL_PUNCH_HOLE support. That is on my list to do: tmpfs has its
own peculiar madvise(MADV_REMOVE) support (and yes, you may question
whether we were right to add that in) - we should be converting
MADV_REMOVE to use FALLOC_FL_PUNCH_HOLE, and tmpfs to support that.

Thanks,
Hugh