Re: fstrim discarding too many or wrong blocks on Linux 5.1, leading to data loss

[Mike Snitzer <snitzer@redhat.com>]

On Tue, May 21 2019 at  3:00pm -0400,
Andrea Gelmini <andrea.gelmini@linux.it> wrote:

> On Tue, May 21, 2019 at 06:46:20PM +0200, Michael Laß wrote:
> > > I finished bisecting. Here’s the responsible commit:
> > > 
> > > commit 61697a6abd24acba941359c6268a94f4afe4a53d
> > > Author: Mike Snitzer <snitzer@redhat.com>
> > > Date:   Fri Jan 18 14:19:26 2019 -0500
> > > 
> > >    dm: eliminate 'split_discard_bios' flag from DM target interface
> > > 
> > >    There is no need to have DM core split discards on behalf of a DM target
> > >    now that blk_queue_split() handles splitting discards based on the
> > >    queue_limits.  A DM target just needs to set max_discard_sectors,
> > >    discard_granularity, etc, in queue_limits.
> > > 
> > >    Signed-off-by: Mike Snitzer <snitzer@redhat.com>
> > 
> > Reverting that commit solves the issue for me on Linux 5.1.3. Would
> that be an option until the root cause has been identified? I’d rather
> not let more people run into this issue.
> 
> Thanks a lot Michael, for your time/work.
> 
> This kind of bisecting are very boring and time consuming.
> 
> I CC: also the patch author.

Thanks for cc'ing me, this thread didn't catch my eye.

Sorry for your troubles.  Can you please try this patch?

Thanks,
Mike

diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 1fb1333fefec..997385c1ca54 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -1469,7 +1469,7 @@ static unsigned get_num_write_zeroes_bios(struct dm_target *ti)
 static int __send_changing_extent_only(struct clone_info *ci, struct dm_target *ti,
 				       unsigned num_bios)
 {
-	unsigned len = ci->sector_count;
+	unsigned len;
 
 	/*
 	 * Even though the device advertised support for this type of
@@ -1480,6 +1480,8 @@ static int __send_changing_extent_only(struct clone_info *ci, struct dm_target *
 	if (!num_bios)
 		return -EOPNOTSUPP;
 
+	len = min((sector_t)ci->sector_count, max_io_len_target_boundary(ci->sector, ti));
+
 	__send_duplicate_bios(ci, ti, num_bios, &len);
 
 	ci->sector += len;