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 12:39 AM, Ivan Shapovalov wrote:
> On Monday 20 October 2014 at 12:54:13, Edward Shishkin wrote:	
>> On 08/17/2014 11:52 PM, Ivan Shapovalov wrote:
>>> [...]
>>> +		/*
>>> +		 * Grab some sane amount of space.
>>> +		 * We will allocate blocks until end of the partition or until
>>> +		 * the grabbed space is exhausted.
>>> +		 */
>>> +		ret = reiser4_grab_reserved(super, 0, BA_CAN_COMMIT | BA_SOME_SPACE);
>>
>> reiser4_grab_reserved() grabs space from the reserved area (5%).
>> This is needed to make sure that unlink(), truncate(), etc. won't
>> fail, if there is no free space on disk. I don't think that FITRIM
>> ioctl needs this reserved area.
> Well, IIUC, it doesn't hurt:


"doesn't hurt" is not enough. If you want to take a resource,
you should be going to explain, why do you need this.


>   any "legitimate" user of the reserved space
> will block on the mutex and eventually proceed. At the same time, given
> a filesystem with (5% + eps) free space left, not using the reserved space
> will result in trimming of (eps) blocks at a time.
>
> Anyway, I should also fix calculation of space to grab in 1/9: if BA_RESERVED
> is not set, then the amount to grab should be calculated as 25% of non-reserved
> free space, not as 25% of all free space...
>
>> If BA_SOME_SPACE is set, then you lock a superblock, and grab 25% of
>> the current free space. It means that your grab will always succeed,
>> so that BA_CAN_COMMIT flag is also not needed.
> If we decide on dropping BA_RESERVED, then you're right here;
> reiser4_grab(...BA_SOME_SPACE...) won't return ENOSPC and so this is pointless.
>
> If we decide on keeping BA_RESERVED, then this should also stay in order not to
> trigger the assertion.
>
> Thanks,