On Mon 03-01-22 19:59:40, Lukas Czerner wrote:
> On Mon, Jan 03, 2022 at 04:34:40PM +0100, Martin Wilck wrote:
> > On Mon, 2021-11-22 at 14:53 +0100, Lukas Czerner wrote:
> > > On Fri, Nov 19, 2021 at 04:43:53PM +0100, Martin Wilck wrote:
> > > > Hi Martin, Lukas,
> > > >
> > > > On Tue, 2021-11-16 at 23:35 -0500, Martin K. Petersen wrote:
> > > >
> > >
> > > Thanks you Martin P. for clarifying.
> > >
> > > >
> > > > I've checked btrfs and xfs, and they treat the minlen like Jan's
> > > > patch
> > > > would - the minlen is set to the device's granularity, and
> > > > discarding
> > > > smaller ranges is silently not even attempted.
> > > >
> > > > So this seems to be common practice among file system
> > > > implementations,
> > > > and thus Jan's patch would make ext4 behave like the others. But
> > > > I'm
> > > > still uncertain if this behavior is ideal, and your remarks seem to
> > > > confirm that.
> > >
> > > Yeah I modeled my change for ext4 on the work in xfs done by
> > > Christoph
> > > and so it kind of spread organically to other file systems.
> >
> > Ok. I still believe that it's not ideal this way, but this logic is
> > only applied in corner cases AFAICS, so it doesn't really hurt.
> >
> > I'm fine with Jan's patch for the time being.
> >
> > >
> > > >
> > > > The fstrim man page text is highly misleading. "The default value
> > > > is
> > > > zero, discarding every free block" is obviously wrong, given the
> > > > current actual behavior of the major filesystems.
> > >
> > > Originally there was no discard granularity optimization so it is
> > > what
> > > it meant.
> >
> > Not quite. That man page text is from 2019, commit ce3d198d7 ("fstrim:
> > document kernel return minlen explicitly"). At that time,
> > discard_granularity had existed for 10 years already.
>
> Not really, the sentence you're pointing out above was there from the
> beginning. Commit ce3d198d7 didn't change that.
>
> >
> >
> > > And it also says "fstrim will adjust the minimum if it's
> > > smaller than the device's minimum". So I am not sure if it's
> > > necessarily
> > > misleading.
> >
> > It is misleading, because it's not fstrim that adjusts anything, but
> > the file system code in the kernel.
>
> This makes absolutely no difference from the user POV. Enough nitpicking,
> feel free to cc me if you decide to send a patch.
So I think the conclusion is that we go with my original patch? Just I
should update it to return computed minlen back to the user, correct?
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
