On 16.04.2018 21:53, David Sterba wrote:
> On Wed, Apr 11, 2018 at 11:21:20AM +0300, Nikolay Borisov wrote:
>> do_chunk_alloc implements logic to detect whether there is currently
>> pending chunk allocation (by means of space_info->chunk_alloc being
>> set) and if so it loops around to the 'again' label. Additionally,
>> based on the state of the space_info (e.g. whether it's full or not)
>> and the return value of should_alloc_chunk() it decides whether this
>> is a "hard" error (ENOSPC) or we can just return 0.
>>
>> This patch refactors all of this:
>>
>> 1. Put order to the scattered ifs handling the various cases in an
>> easy-to-read if {} else if{} branches. This makes clear the various
>> cases we are interested in handling.
>>
>> 2. Call should_alloc_chunk only once and use the result in the
>> if/else if constructs. All of this is done under space_info->lock, so
>> even before multiple calls of should_alloc_chunk were unnecessary.
>>
>> 3. Rewrite the "do {} while()" loop currently implemented via label
>> into an explicit loop construct.
>>
>> 4. Move the mutex locking outside of the while loop and remove the
>> comment. The comment in fact was misleading since in the old code if the
>> mutex is acquired and we don't need to loop again (wait_for_alloc) is
>> 0 then we don't recheck if we need to allocate (the comment was stating
>> the opposite).
>
> The comment seems correct to me. It is referring to the actual
> allocation that happens in paralell (a few lines below calling
> btrfs_alloc_chunk). So the first part attempts to determine if we should
> wait, and if yes, then take the mutex and block until the other
> allocation finishes.
>
> This is completely mising in the new code so the loop is only blocking
> on the space info spinlock, but not going to sleep. Busy looping.
With your explanation the code now makes a bit more sense and indeed
I've missed that. Will send an updated patch.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
