linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Eric Sandeen <esandeen@redhat.com>
Cc: Andreas Dilger <adilger@dilger.ca>,
	Namjae Jeon <linkinjeon@gmail.com>,
	"tytso@mit.edu" <tytso@mit.edu>,
	"adilger.kernel@dilger.ca" <adilger.kernel@dilger.ca>,
	"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-ext4@vger.kernel.org" <linux-ext4@vger.kernel.org>,
	"a.sangwan@samsung.com" <a.sangwan@samsung.com>,
	Namjae Jeon <namjae.jeon@samsung.com>
Subject: Re: [PATCH 0/3] ext4: introduce two new ioctls
Date: Mon, 24 Jun 2013 12:44:59 +1000	[thread overview]
Message-ID: <20130624024459.GJ29376@dastard> (raw)
In-Reply-To: <7D1878F6-0387-48F3-8724-4A8946AECF9E@redhat.com>

On Sun, Jun 23, 2013 at 08:32:32PM -0400, Eric Sandeen wrote:
> On Jun 23, 2013, at 12:01 PM, Andreas Dilger <adilger@dilger.ca> wrote:
> 
> > On 2013-06-23, at 0:07, Namjae Jeon <linkinjeon@gmail.com> wrote:
> > 
> >> From: Namjae Jeon <namjae.jeon@samsung.com>
> >> 
> >> This patch series introduces 2 new ioctls for ext4.
> >> 
> >> Truncate_block_range ioctl truncates blocks from source file.
> > 
> > How is this different from fallocate(FALLOC_FL_PUNCH_HOLE)?  That is already in existing kernels, and portable across multiple filesystems. 
> > 
> Same question.  Punch hole should do this already...

Hole punch doesn't change the offsets of subsequent extents in the
file - it leaves a hole. This doesn't leave a hole at all - all the 
extents above the range are shifted down to offset where the extents
being punched out started.

> >> Transfer_block_range ioctl transfers data blocks from source file
> >> and append them at the end of destination file.
> > 
> > There is already a similar ioctl for defragmenting files. Is it possible to use that, or does it have different semantics?
> > 
> >> Ioctl1:        EXT4_IOC_TRUNCATE_BLOCK_RANGE:
> >> This ioctl truncates a range of data blocks from file.
> >> It is useful to remove easily and quickly the garbage data
> >> at the middle of file.
> >> 
> >> e.g. we have a movie file and there is long advertisement in movie file.
> >> user want to remove only advertisement range.
> > 
> > While this works in theory, there is very little chance that the movie data will align exactly to filesystem block boundaries. 
> > 
> Or more importantly on compression codec boundaries.   Wouldn't this look like corruption at playback time?

Not necessarily. Video codecs are encapsulated in a container that
can be used to link key frames together so you can do this sort of
manipulation of the file contents and just change an
offset-to-next-keyframe value in the container and it all just
works. Non linear editting (NLE) software has been doing this
manually for 15 years by copying data around - this just optimises
the operation by manipulating the extent mapping rather than needing
to physically copy the data.

FWIW, I've heard persistent rumors going back several years of
various DVR companies shipping equivalent ioctl-based functionality
for XFS filesystems to both insert and remove chunks in video
streams, but I've never been able to find the code for it anywhere.

Hence, at minimum, this should be a fallocate() operation, not a ext4
specific ioctl as it is relatively trivial to implement on most
extent based filesystems.

However, My conditions for merging such functionality into fallocate
are:
	1. it needs xfs_io support [to provide]
	2. comprehensive xfstests coverage, similar to the current
	   hole punch coverage we have.

> >> #define EXT4_IOC_TRUNCATE_BLOCK_RANGE  _IOW('f', 18, struct truncate_range)
> >> struct truncate_range {
> >>      __u32 start_block;
> >>      __u32 length;
> >> };

And have 64 bit file size support, please!

Also FALLOC_FL_COLLAPSE_RANGE is probably a better name for the
operation being done ;)

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

  reply	other threads:[~2013-06-24  2:44 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-23  6:07 [PATCH 0/3] ext4: introduce two new ioctls Namjae Jeon
2013-06-23  6:22 ` Namjae Jeon
2013-06-23 17:00 ` Andreas Dilger
2013-06-24  0:32   ` Eric Sandeen
2013-06-24  2:44     ` Dave Chinner [this message]
2013-06-24  3:12       ` Theodore Ts'o
2013-06-24  8:08         ` Dave Chinner
2013-06-24  7:06       ` Christoph Hellwig
2013-06-24  9:35         ` Namjae Jeon
2013-06-24 10:37           ` Sidorov, Andrei
2013-06-24 14:14             ` Zheng Liu
2013-06-24 21:29             ` Dave Chinner
2013-06-24  6:48   ` Namjae Jeon
  -- strict thread matches above, loose matches on Subject: below --
2013-06-23  6:05 Namjae Jeon

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=20130624024459.GJ29376@dastard \
    --to=david@fromorbit.com \
    --cc=a.sangwan@samsung.com \
    --cc=adilger.kernel@dilger.ca \
    --cc=adilger@dilger.ca \
    --cc=esandeen@redhat.com \
    --cc=linkinjeon@gmail.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=namjae.jeon@samsung.com \
    --cc=tytso@mit.edu \
    /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).