linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jan Kara <jack@suse.cz>
To: Lukas Czerner <lczerner@redhat.com>
Cc: Tao Ma <tm@tao.ma>, linux-ext4@vger.kernel.org, Jan Kara <jack@suse.cz>
Subject: Re: [PATCH] ext3: fix trim length underflow with small trim length.
Date: Wed, 19 Jan 2011 12:39:21 +0100	[thread overview]
Message-ID: <20110119113921.GA4246@quack.suse.cz> (raw)
In-Reply-To: <alpine.LFD.2.00.1101191125120.2686@dhcp-lab-213.englab.brq.redhat.com>

On Wed 19-01-11 11:42:50, Lukas Czerner wrote:
> On Wed, 19 Jan 2011, Tao Ma wrote:
> 
> > From: Tao Ma <boyu.mt@taobao.com>
> > 
> > We adjust 'len' with s_first_data_block - start in case of start is less
> > than s_first_data_block, but it could underflow in case blocksize=1K, while
> > fstrim_range.len=512 and fstrim_range.start = 0. In this case len happens
> > to be underflow and in the end, although we are safe that last_group check
> > will limit the trim to the whole volume, I am afraid that isn't what the user
> > really want.
> > 
> > So this patch fix it. It also adds a new variable s_first_data_block so that
> > the 4 le32_to_cpu can be replaced with 1.
> 
> Well, I just realized that what are we doing is not exactly what will
> user expect. User does not really care where the first data block is.
> What the user will expect is, to trim let's say first one gigabyte
> of his filesystem, not gigabyte - first data block.
> 
> So what I suggest is to always add first_data_block to
> fstrim_range.start and do all the necessary checks for overflow. If no
> one has any objections I'll put it to the patch.
  Well, since we speak about at most 1KB (s_first_data_block is non-zero
only when blocksize == 1024 and in that case it is 1), I don't think it
really matters and I don't mind whatever solution. What user expects is
a bit hard to guess (whether he views 'start' as a start of the filesystem
or a start of the device). Maybe the former makes a tad bit more sense
but as I said I don't really care so since you're the author of the code I
leave it up to you.

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

  reply	other threads:[~2011-01-19 11:39 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-01-19  9:49 [PATCH] ext3: fix trim length underflow with small trim length Tao Ma
2011-01-19 10:42 ` Lukas Czerner
2011-01-19 11:39   ` Jan Kara [this message]
2011-01-21  2:52     ` [PATCH v2] ext3: Adjust trim start with first_data_block Tao Ma
2011-01-21 10:36       ` Lukas Czerner
2011-01-21 13:59         ` Tao Ma
2011-01-21 15:00       ` Jan Kara
2011-01-19 13:50   ` [PATCH] ext3: fix trim length underflow with small trim length Tao Ma

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=20110119113921.GA4246@quack.suse.cz \
    --to=jack@suse.cz \
    --cc=lczerner@redhat.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=tm@tao.ma \
    /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).