linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Andreas Dilger <adilger@dilger.ca>
Cc: Kirill Tkhai <ktkhai@virtuozzo.com>,
	Christoph Hellwig <hch@infradead.org>,
	Theodore Ts'o <tytso@mit.edu>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Mike Snitzer <snitzer@redhat.com>, Jan Kara <jack@suse.cz>,
	Eric Biggers <ebiggers@google.com>,
	riteshh@linux.ibm.com, krisman@collabora.com, surajjs@amazon.com,
	dmonakhov@gmail.com, mbobrowski@mbobrowski.org,
	Eric Whitney <enwlinux@gmail.com>,
	sblbir@amazon.com, Khazhismel Kumykov <khazhy@google.com>,
	linux-ext4 <linux-ext4@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Linux FS Devel <linux-fsdevel@vger.kernel.org>
Subject: Re: [PATCH RFC 5/5] ext4: Add fallocate2() support
Date: Sat, 29 Feb 2020 08:16:10 +1100	[thread overview]
Message-ID: <20200228211610.GQ10737@dread.disaster.area> (raw)
In-Reply-To: <4933D88C-2A2D-4ACA-823E-BDFEE0CE143F@dilger.ca>

On Fri, Feb 28, 2020 at 08:35:19AM -0700, Andreas Dilger wrote:
> On Feb 27, 2020, at 5:24 AM, Kirill Tkhai <ktkhai@virtuozzo.com> wrote:
> > On 27.02.2020 00:51, Andreas Dilger wrote:
> >> On Feb 26, 2020, at 1:05 PM, Kirill Tkhai <ktkhai@virtuozzo.com> wrote:
> >> In that case, an interesting userspace interface would be an array of
> >> inode numbers (64-bit please) that should be packed together densely in
> >> the order they are provided (maybe a flag for that).  That allows the
> >> filesystem the freedom to find the physical blocks for the allocation,
> >> while userspace can tell which files are related to each other.
> > 
> > So, this interface is 3-in-1:
> > 
> > 1)finds a placement for inodes extents;
> 
> The target allocation size would be sum(size of inodes), which should
> be relatively small in your case).
> 
> > 2)assigns this space to some temporary donor inode;
> 
> Maybe yes, or just reserves that space from being allocated by anyone.
> 
> > 3)calls ext4_move_extents() for each of them.
> 
> ... using the target space that was reserved earlier
> 
> > Do I understand you right?
> 
> Correct.  That is my "5 minutes thinking about an interface for grouping
> small files together without exposing kernel internals" proposal for this.

You don't need any special kernel interface with XFS for this. It is
simply:

	mkdir tmpdir
	create O_TMPFILEs in tmpdir

Now all the tmpfiles you create and their data will be co-located
around the location of the tmpdir inode. This is the natural
placement policy of the filesystem. i..e the filesystem assumes that
files in the same directory are all related, so will be accessed
together and so should be located in relatively close proximity to
each other.

This is a locality optimisation technique that is older than XFS. It
works remarkably well when the filesystem can spread directories
effectively across it's address space.  It also allows userspace to
use simple techniques to group (or separate) data files as desired.
Indeed, this is how xfs_fsr directs locality for it's tmpfiles when
relocating/defragmenting data....

> > If so, then IMO it's good to start from two inodes, because here may code
> > a very difficult algorithm of placement of many inodes, which may require
> > much memory. Is this OK?
> 
> Well, if the files are small then it won't be a lot of memory.  Even so,
> the kernel would only need to copy a few MB at a time in order to get
> any decent performance, so I don't think that is a huge problem to have
> several MB of dirty data in flight.
> 
> > Can we introduce a flag, that some of inode is unmovable?
> 
> There are very few flags left in the ext4_inode->i_flags for use.
> You could use "IMMUTABLE" or "APPEND_ONLY" to mean that, but they
> also have other semantics.  The EXT4_NOTAIL_FL is for not merging the
> tail of a file, but ext4 doesn't have tails (that was in Reiserfs),
> so we might consider it a generic "do not merge" flag if set?

We've had that in XFS for as long as I can remember. Many
applications were sensitive to the exact layout of the files they
created themselves, so having xfs_fsr defrag/move them about would
cause performance SLAs to be broken.

Indeed, thanks to XFS, ext4 already has an interface that can be
used to set/clear a "no defrag" flag such as you are asking for.
It's the FS_XFLAG_NODEFRAG bit in the FS_IOC_FS[GS]ETXATTR ioctl.
In XFS, that manages the XFS_DIFLAG_NODEFRAG on-disk inode flag,
and it has special meaning for directories. From the 'man 3 xfsctl'
man page where this interface came from:

      Bit 13 (0x2000) - XFS_XFLAG_NODEFRAG
	No defragment file bit - the file should be skipped during a
	defragmentation operation. When applied to  a directory,
	new files and directories created will inherit the no-defrag
	bit.

> > Can this interface use a knowledge about underlining device discard granuality?
> 
> As I wrote above, ext4+mballoc has a very good appreciation for alignment.
> That was written for RAID storage devices, but it doesn't matter what
> the reason is.  It isn't clear if flash discard alignment is easily
> used (it may not be a power-of-two value or similar), but wouldn't be
> harmful to try.

Yup, XFS has the similar (but more complex) alignment controls for
directing allocation to match the underlying storage
characteristics. e.g. stripe unit is also the "small file size
threshold" where the allocation policy changes from packing to
aligning and separating.

> > In the answer to Dave, I wrote a proposition to make fallocate() care about
> > i_write_hint. Could you please comment what you think about that too?
> 
> I'm not against that.  How the two interact would need to be documented
> first and discussed to see if that makes sene, and then implemented.

Individual filesystems can make their own choices as to what they do
with write hints, including ignoring them and leaving it for the
storage device to decide where to physically place the data. Which,
in many cases, ignoring the hint is the right thing for the
filesystem to do...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

  reply	other threads:[~2020-02-28 21:16 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-26 13:40 [PATCH RFC 0/5] fs, ext4: Physical blocks placement hint for fallocate(0): fallocate2(). TP defrag Kirill Tkhai
2020-02-26 13:40 ` [PATCH RFC 1/5] fs: Add new argument to file_operations::fallocate() Kirill Tkhai
2020-02-26 13:41 ` [PATCH RFC 2/5] fs: Add new argument to vfs_fallocate() Kirill Tkhai
2020-02-26 13:41 ` [PATCH RFC 3/5] fs: Add fallocate2() syscall Kirill Tkhai
2020-02-26 13:41 ` [PATCH RFC 4/5] ext4: Prepare ext4_mb_discard_preallocations() for handling EXT4_MB_HINT_GOAL_ONLY Kirill Tkhai
2020-02-26 13:41 ` [PATCH RFC 5/5] ext4: Add fallocate2() support Kirill Tkhai
2020-02-26 15:55   ` Christoph Hellwig
2020-02-26 20:05     ` Kirill Tkhai
2020-02-26 21:51       ` Andreas Dilger
2020-02-27 12:24         ` Kirill Tkhai
2020-02-28 15:35           ` Andreas Dilger
2020-02-28 21:16             ` Dave Chinner [this message]
2020-02-29 20:12               ` Andreas Dilger
2020-03-01  0:06                 ` Dave Chinner
2020-03-02 10:33               ` Kirill Tkhai
2020-03-02 11:07             ` Kirill Tkhai
2020-02-27  6:59       ` Konstantin Khlebnikov
2020-02-27 10:42         ` Kirill Tkhai
2020-02-27  7:33       ` Dave Chinner
2020-02-27 11:12         ` Kirill Tkhai
2020-02-27 21:56           ` Dave Chinner
2020-02-28 12:41             ` Kirill Tkhai
2020-02-29 22:41               ` Dave Chinner
2020-03-02 10:17                 ` Kirill Tkhai
2020-02-27 10:39 ` [PATCH RFC 0/5] fs, ext4: Physical blocks placement hint for fallocate(0): fallocate2(). TP defrag Ritesh Harjani
2020-02-28  7:07 ` xiaohui li
2020-02-28 12:46   ` Kirill Tkhai
2020-03-02 16:56 ` Theodore Y. Ts'o
2020-03-03  9:57   ` Kirill Tkhai
2020-03-03 16:55     ` Theodore Y. Ts'o
2020-03-03 17:36       ` Kirill Tkhai
2020-03-11 19:26     ` Andreas Dilger
2020-03-11 20:29       ` Kirill Tkhai
2020-03-12  0:31         ` Andreas Dilger
2020-03-12  9:23           ` Kirill Tkhai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200228211610.GQ10737@dread.disaster.area \
    --to=david@fromorbit.com \
    --cc=adilger@dilger.ca \
    --cc=dmonakhov@gmail.com \
    --cc=ebiggers@google.com \
    --cc=enwlinux@gmail.com \
    --cc=hch@infradead.org \
    --cc=jack@suse.cz \
    --cc=khazhy@google.com \
    --cc=krisman@collabora.com \
    --cc=ktkhai@virtuozzo.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mbobrowski@mbobrowski.org \
    --cc=riteshh@linux.ibm.com \
    --cc=sblbir@amazon.com \
    --cc=snitzer@redhat.com \
    --cc=surajjs@amazon.com \
    --cc=tytso@mit.edu \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).