Kyungmin Park <kmpark@xxxxxxxxxxxxx> writes:
>>> Do you still object to merge this feature for .40? As I said the
>>> batched discard design is out-of-scope of this patch.
>>> It's implemented at other batched discard, ext4, xfs and so on.
>>>
>>> I hope fat is also support the batched discard.
>>>
>>> Any opinions?
>>
>> I'm also thinking implementing this is good though. Sorry, I'm not going
>> to apply this for now, and would like to wait to be used by real
>> userland (I hope guys notice the problem, or userland tackles it somehow
>> sadly).
>>
>> I think, to expose the wrong behavior like this would be worse.
>>
>> E.g. one of problems, userland might do like this (trim chunk from 0 to
>> number of block)
>>
>> for chunk in number_of_blocks
>> do_trim chunk
>> done
>
> It's handled at trim implementation. It just trim the fat aware block.
> Not trim the blocks which fat doesn't know.
> As fat don't use the block 0, 1, it adjust the start block at kernel.
>
> + if (start < FAT_START_ENT)
> + start = FAT_START_ENT;
>
> and don't exceed the max cluster size.
>
> + len = (len > sbi->max_cluster) ? sbi->max_cluster : len;
>
> + for (count = start; count <= len; count++) {
Yes. We _adjust_ from 0 to 2 here, so, the end of block also have to be
_adjusted_.
>From other point of view, if userland specified 0 - max-length
(i.e. number of blocks), what happens? It would trim block of 2 -
(max-length - 2), right?
So, actually, userland specify 2 - (max-length + 2). And userland has to
know 2 is real start, not 0.
But how to know it? Yes, it's FS internal layout, AFAICS, other FSes also
expose internal like this.
>> But this is actually wrong, this interface doesn't map blocks to 0-max,
>> so userland have to know real maximum-block-number somehow for each FS
>> (and maybe have to know real minimum-block-number).
>>
>> So, how to fix this? The solutions would be userland workaround, or fix
>> kernel. If it was userland, userland have to know FS internal sadly. If
>> it was kernel, we would have backward compatible nightmare, or ignore
>> compatible :(.
>
> I think basic concept of batched discard is trim at once instead of
> deleted entries at filesystem (original one).
> So it can set the specific start address or any start (usually 0) with
> maximum length.
Yes, I think so too (i.e. 0 - max length). But the implement is not
working like it. It exposes FS internal layout.
>> I really think we have to rethink this and have agreement about common
>> interface. Or there was real userland app, I think FAT can implement to
>> work that app.
>
> IMO, we should use the same user space application. user program
> doesn't know the which filesystem are underlying.
Indeed.
> So sample user space program used at ext4, xfs should be used. and I
> tested it.
I bet it doesn't trim whole FS (due to expose FS layout) actually even
if user asked like above example.
Thanks.
--
OGAWA Hirofumi <hirofumi@xxxxxxxxxxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
