On 4.07.2018 19:55, David Sterba wrote:
> On Mon, Jul 02, 2018 at 10:49:46AM +0300, Nikolay Borisov wrote:
>> On 2.07.2018 10:44, Ethan Lien wrote:
>>> We use customized, nodesize batch value to update dirty_metadata_bytes.
>>> We should also use batch version of compare function or we will easily
>>> goto fast path and get false result from percpu_counter_compare().
>>>
>>> Signed-off-by: Ethan Lien <ethanlien@xxxxxxxxxxxx>
>>
>> Reviewed-by: Nikolay Borisov <nborisov@xxxxxxxx>
>>
>> Also this needs the following tag:
>>
>> Fixes: e2d845211eda ("Btrfs: use percpu counter for dirty metadata count")
>
> I'll add the tags, thanks.
>
> 2939 fs_info->dirty_metadata_batch = nodesize * (1 + ilog2(nr_cpu_ids));
>
> What a commit, sigh. Arbitrary constants without any explanation. I
> don't quite understand why the ilog2(nr_cpui_ids) was chosen. If cpus
> are onlined/offlined there's no adjustment. I'd think that there should
> be some amount of data that are supposed to be dirtied per-cpu and this
> value is independent of the number of cpus. That the online/offline
> status is abstracted by the percpu API. I don't have time to study that
> too deeply, but this is something that needs to be revisited.
TBH, what is perhaps the safest approach would be to modify the code to
eliminate the custom batch related variables/code. The code has been
working like that for years.
> --
> 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
