linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Lukas Czerner <lczerner@redhat.com>
To: Dmitry Monakhov <dmonakhov@openvz.org>
Cc: Lukas Czerner <lczerner@redhat.com>,
	linux-ext4@vger.kernel.org, jmoyer@redhat.com,
	rwheeler@redhat.com, eshishki@redhat.com, sandeen@redhat.com,
	jack@suse.cz, tytso@mit.edu
Subject: Re: [PATCH 1/3] Add ioctl FITRIM.
Date: Thu, 5 Aug 2010 10:36:47 +0200 (CEST)	[thread overview]
Message-ID: <alpine.LFD.2.00.1008051009100.3063@localhost> (raw)
In-Reply-To: <8739utfssc.fsf@dmon-lap.sw.ru>

On Thu, 5 Aug 2010, Dmitry Monakhov wrote:

> Lukas Czerner <lczerner@redhat.com> writes:
> 
> > On Wed, 4 Aug 2010, Dmitry Monakhov wrote:
> >
> >> Lukas Czerner <lczerner@redhat.com> writes:
> >> 
> >> > Adds an filesystem independent ioctl to allow implementation of file
> >> > system batched discard support.
> >> >
> >> > Signed-off-by: Lukas Czerner <lczerner@redhat.com>
> >> > ---
> >> >  fs/ioctl.c         |   31 +++++++++++++++++++++++++++++++
> >> >  include/linux/fs.h |    2 ++
> >> >  2 files changed, 33 insertions(+), 0 deletions(-)
> >> >
> >> > diff --git a/fs/ioctl.c b/fs/ioctl.c
> >> > index 2d140a7..6c01c3c 100644
> >> > --- a/fs/ioctl.c
> >> > +++ b/fs/ioctl.c
> >> > @@ -540,6 +540,33 @@ static int ioctl_fsthaw(struct file *filp)
> >> >  	return thaw_super(sb);
> >> >  }
> >> >  
> >> > +static int ioctl_fstrim(struct file *filp, unsigned long arg)
> >> BTW why do we have to trim fs in one shot ?
> >> IMHO it is much suitable to provide start,len parameters as we 
> >> do in most functions(truncate, bdevdiscard, getdents).
> >> It allow userspace caller to implement a fancy looking progress bars.
> >
> > Hi,
> >
> > do you think it is really needed when even with todays SSD's it takes
> > just a couple of seconds ? And I suppose it will improve in future. But
> > generally I think we can do that..I would like to hear some more
> > opinions before I start looking at this.
> Hi, Lukas
> we may face a really long delays due to bad layouts and slow devices
> Please read my response to Ted
> I'm agree with you what this interface is important, BTW i already
> enabled FITRIM support on my notebook, my speed difference is about 2-3%.
> But let's provide right user interface from very beginning.

Hi, Dimitry

I read the thread and really it makes sense to me. Sometimes it can be useful
to have more fine-grained control beside just specifying minlen argument,
which works quite well, however it is a little bit fuzzy because when you do
not know how much space was actually trimmed.

I think that there is no need to have two separate ioctls, even though
it would be more effective to specify block group instead of block range.
I am thinking about something like int optimize which will tell us to round
the range to block group boundaries. But I can not tell if it would
really help someone (probably not).

So I will try to do something to be able to break the FITRIM to smaller
pieces, uint64_t start, uint64_t len, uint64_t minlen seems good to me.

Thanks

-Lukas

  reply	other threads:[~2010-08-05  8:36 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-08-04 13:44 [PATCH 1/3] Add ioctl FITRIM Lukas Czerner
2010-08-04 13:44 ` [PATCH 2/3] Add batched discard support for ext3 Lukas Czerner
2010-08-04 14:03   ` Jan Kara
2010-08-04 14:32     ` Lukas Czerner
2010-08-04 19:39   ` Andreas Dilger
2010-08-05 14:00     ` Lukas Czerner
2010-08-04 13:44 ` [PATCH 3/3] Add batched discard support for ext4 Lukas Czerner
2010-08-04 14:17   ` Jan Kara
2010-08-04 14:57 ` [PATCH 1/3] Add ioctl FITRIM Dmitry Monakhov
2010-08-04 15:13   ` Lukas Czerner
2010-08-04 15:26     ` Greg Freemyer
2010-08-05  0:28       ` Ted Ts'o
2010-08-05  6:51         ` Dmitry Monakhov
2010-08-05 15:47         ` Andreas Dilger
2010-08-05  7:00     ` Dmitry Monakhov
2010-08-05  8:36       ` Lukas Czerner [this message]
  -- strict thread matches above, loose matches on Subject: below --
2010-09-24 15:35 [PATCH 0/3 v. 8] Ext3/Ext4 Batched discard support Lukas Czerner
2010-09-24 15:35 ` [PATCH 1/3] Add ioctl FITRIM Lukas Czerner
2010-09-24 17:03   ` Andreas Dilger
2010-09-27  9:20     ` Lukas Czerner
2010-08-10 14:19 [PATCH 0/3 ver. 7] Ext3/Ext4 Batched discard support Lukas Czerner
2010-08-10 14:19 ` [PATCH 1/3] Add ioctl FITRIM Lukas Czerner
2010-08-06 11:31 [PATCH 0/3] Batched discard support Lukas Czerner
2010-08-06 11:31 ` [PATCH 1/3] Add ioctl FITRIM Lukas Czerner
2010-07-27 12:41 [PATCH 0/3 v3] Batched discard support for Ext3/Ext4 Lukas Czerner
2010-07-27 12:41 ` [PATCH 1/3] Add ioctl FITRIM Lukas Czerner

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=alpine.LFD.2.00.1008051009100.3063@localhost \
    --to=lczerner@redhat.com \
    --cc=dmonakhov@openvz.org \
    --cc=eshishki@redhat.com \
    --cc=jack@suse.cz \
    --cc=jmoyer@redhat.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=rwheeler@redhat.com \
    --cc=sandeen@redhat.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).