Re: [RFC] [PATCHv3 7/9] reiser4: batch discard support: actually implement the FITRIM ioctl handler.

[Edward Shishkin <edward.shishkin@gmail.com>]

On 10/21/2014 09:00 PM, Ivan Shapovalov wrote:
> On Tuesday 21 October 2014 at 20:48:59, Edward Shishkin wrote:	
>> On 10/21/2014 08:11 PM, Ivan Shapovalov wrote:
>>> On Tuesday 21 October 2014 at 20:01:55, Edward Shishkin wrote:	
>>>> On 10/21/2014 06:42 PM, Ivan Shapovalov wrote:
>>>>> On Tuesday 21 October 2014 at 18:33:11, Edward Shishkin wrote:	
>>>>>> [...]
>>>>>>
>>>>>> Ok, it is clear, why we can not fail with -ENOSPC when trying to delete
>>>>>> a file from a full partition, yes?
>>>>> Sure. Please note that making reiser4_trim_fs's allocations BA_RESERVED will
>>>>> not hinder this: delete_mutex is there for a reason.
>>>> Sure, and this is the reason, because when I hear "let's take a mutex",
>>>> I start to feel bad :)
> Does this (ab)use of delete_mutex violates any ordering?
> If not, it should be OK.
>
>>>>>> I want to see explanation, why FITRIM ioclt can not finish the work when
>>>>>> there is no free space on disk.
>>>>> Suppose we do not use reserved space for reiser4_trim_fs's allocations.
>>>>> Let's analyze those two cases:
>>>>>
>>>>> 1. There is <= 5% free space on disk.
>>>>> Initial grabbing fails, nothing can be trimmed.
>>>>> This is wrong.
>>>>>
>>>>> 2. There is 5% + X (where X is some small number) free space on disk.
>>>>> We can grab only X blocks at a time, so a total of ((SIZE * 5% / X) + 1)
>>>>> transactions will be created. BTW, if X < erase unit, nothing can be trimmed.
>>>>> This is ineffective.
>>>>>
>>>>> Hope this makes sense.
>>>> This is already much better, thanks!
>>>> Then I should say that the whole idea to reserve % of free space is
>>>> incorrect.
>>>> Try to reserve 1% of total partition size with BA_CAN_COMMIT for an
>>>> iteration.
>>>> If it fails, then try to reserve 1 block with BA_CAN_COMMIT.
>>> Hm? Both described cases are still broken.
>>>
>>> In #1 (free space <= 5%) nothing changes; reiser4_trim_fs will never be able
>>> to do its task without using reserved space.
>>>
>>> In updated #2 (5% < free space < 6%), a transaction per every block (!)
>>> will be created, and none of them will trim anything (1 block < erase unit).
>>
>> If (5% < free_space < 5% + erase_unit), then fail with ENOSPC.
> ...and s/1 block/1 erase unit/ in your proposal?
> Still this will create too many transactions.
>
>> Why not? This is not the unlink case...
> It looks like fighting an artificially created problem. I fail to see why
> can't we use BA_RESERVED.


Let's assign you a pair of tickets with deadlock issues
in reiser4? I think you will change your opinion quickly :)


>   This will solve both cases at no harm.
>
> Note -- I'm not saying "let's take this resource, it does not harm", but
> "let's take this resource, it solves two cases AND does not harm" :)


OK, you won,
do it with grab_reserved :)

Edward.