Re: [PATCH] e2fsck: Discard free data and inode blocks.

[Ric Wheeler <ricwheeler@gmail.com>]

  On 10/22/2010 07:43 AM, Lukas Czerner wrote:
> On Fri, 22 Oct 2010, Ric Wheeler wrote:
>
>>   On 10/22/2010 05:12 AM, Lukas Czerner wrote:
>>> On Thu, 21 Oct 2010, Andreas Dilger wrote:
>>>
>>>> On 2010-10-21, at 08:15, Lukas Czerner wrote:
>>>>> In Pass 5 when we are checking block and inode bitmaps we have great
>>>>> opportunity to discard free space and unused inodes on the device,
>>>>> because bitmaps has just been verified as valid. This commit takes
>>>>> advantage of this opportunity and discards both, all free space and
>>>>> unused inodes.
>>>>>
>>>>> I have added new option '-K' which when set, disables discard. Also when
>>>>> the underlying device does not support discard, or BLKDISCARD ioctl
>>>>> returns any kind of error, or when some errors occurred in bitmaps, the
>>>>> discard is disabled.
>>>> I'm always a bit nervous with patches like this, that will prevent data
>>>> recovery after an e2fsck run (which seems like the opposite of what we
>>>> want from e2fsck).
>>>>
>>>> Two suggestions:
>>>> - it probably makes sense to disable this by default, and allow it to be
>>>>     specified on the command-line and e2fsck.conf
>>>> - should we really have a short option, or a "-E discard" and "-E
>>>> nodiscard"
>>>>     options, which allow us to change the default easily at some later time
>>>>     (which we can't do with a single -K flag)
>>> Right, I agree it would be probably better to disable this by default.
>>>
>>>
>> If we do disable it by default, I think that we might also want to be
>> consistent and disable the discard support in mkfs by default as well?
>>
>> thanks!
>>
>> Ric
>>
> I think that this will not be necessary. There is a concern that it might
> prevent data recovery after fsck because it might be already discarded
> (some weird fs corruption?) in pass 5. However in my opinion this is a
> very small window (if there even is any), because we have already passed
> check 1-4 and we have just confirmed that group descriptors should be ok.
> But when there is an even slight chance this might happen I would suggest
> that we really disable it by default (at least for a while - we will see
> then).
>
> On the other hand there is nothing to be afraid of in the case of mkfs,
> because we can not possibly lose any relevant data, because discard is
> done before the filesystem gets created.
>
> -Lukas

My concern with mkfs is that we have seen several devices which don't handle 
this well.

We will be using this TRIM (or UNMAP, etc) on lots of old, creaky hardware with 
old firmware, so having it try on all devices is almost certainly going to cause 
breakages, hangs, etc in the field....

Ric