Re: [PATCHv5 3/5] ext4: mballoc: Introduce pcpu seqcnt for freeing PA to improve ENOSPC handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Marek,

On 6/3/20 12:18 PM, Marek Szyprowski wrote:
Hi Ritesh,

On 20.05.2020 08:40, Ritesh Harjani wrote:
There could be a race in function ext4_mb_discard_group_preallocations()
where the 1st thread may iterate through group's bb_prealloc_list and
remove all the PAs and add to function's local list head.
Now if the 2nd thread comes in to discard the group preallocations,
it will see that the group->bb_prealloc_list is empty and will return 0.

Consider for a case where we have less number of groups
(for e.g. just group 0),
this may even return an -ENOSPC error from ext4_mb_new_blocks()
(where we call for ext4_mb_discard_group_preallocations()).
But that is wrong, since 2nd thread should have waited for 1st thread
to release all the PAs and should have retried for allocation.
Since 1st thread was anyway going to discard the PAs.

The algorithm using this percpu seq counter goes below:
1. We sample the percpu discard_pa_seq counter before trying for block
     allocation in ext4_mb_new_blocks().
2. We increment this percpu discard_pa_seq counter when we either allocate
     or free these blocks i.e. while marking those blocks as used/free in
     mb_mark_used()/mb_free_blocks().
3. We also increment this percpu seq counter when we successfully identify
     that the bb_prealloc_list is not empty and hence proceed for discarding
     of those PAs inside ext4_mb_discard_group_preallocations().

Now to make sure that the regular fast path of block allocation is not
affected, as a small optimization we only sample the percpu seq counter
on that cpu. Only when the block allocation fails and when freed blocks
found were 0, that is when we sample percpu seq counter for all cpus using
below function ext4_get_discard_pa_seq_sum(). This happens after making
sure that all the PAs on grp->bb_prealloc_list got freed or if it's empty.

It can be well argued that why don't just check for grp->bb_free to
see if there are any free blocks to be allocated. So here are the two
concerns which were discussed:-

1. If for some reason the blocks available in the group are not
     appropriate for allocation logic (say for e.g.
     EXT4_MB_HINT_GOAL_ONLY, although this is not yet implemented), then
     the retry logic may result into infinte looping since grp->bb_free is
     non-zero.

2. Also before preallocation was clubbed with block allocation with the
     same ext4_lock_group() held, there were lot of races where grp->bb_free
     could not be reliably relied upon.
Due to above, this patch considers discard_pa_seq logic to determine if
we should retry for block allocation. Say if there are are n threads
trying for block allocation and none of those could allocate or discard
any of the blocks, then all of those n threads will fail the block
allocation and return -ENOSPC error. (Since the seq counter for all of
those will match as no block allocation/discard was done during that
duration).

Signed-off-by: Ritesh Harjani <riteshh@xxxxxxxxxxxxx>

This patch landed in yesterday's linux-next and causes following
WARNING/BUG on various Samsung Exynos-based boards:

   BUG: using smp_processor_id() in preemptible [00000000] code: logsave/552
   caller is ext4_mb_new_blocks+0x404/0x1300

Yes, this is being discussed in the community.
I have submitted a patch which should help fix this warning msg.
Feel free to give this a try on your setup.

https://marc.info/?l=linux-ext4&m=159110574414645&w=2


-ritesh




[Index of Archives]     [Reiser Filesystem Development]     [Ceph FS]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite National Park]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]     [Linux Media]

  Powered by Linux