Re: [PATCH] fat: serialize primary and mirror updates

[OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>]

Matthias Goergens <matthias.goergens@gmail.com> writes:

> OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> writes:
>
>> What is the real issue?
>
> Only the data race syzbot reports, "KCSAN: data-race in fat16_ent_put /
> fat_mirror_bhs": fat_mirror_bhs() copies a FAT block to the backup FAT
> while another task updates an entry in that block.  I have not found any
> effect on disk.  fat_mirror_bhs() holds the backup buffer's lock across
> the copy, and every writer mirrors the block again after its own update,
> so a copy that caught an entry mid-update is replaced by that writer's
> copy.  The commit message should have said so, and the patch also lacked
> my Signed-off-by: it went out before it was ready, sorry.
>
> That leaves two ways to deal with the report.  Would you prefer marking
> the copy as an intended race, data_race() around the memcpy() with a
> comment on why it is harmless?  Or the locking in this patch, which
> holds fat_lock across fat_sync_bhs() for synchronous inodes and so
> makes other allocations wait for that I/O?

I think former. And Actually I explained a few times for similar issues
in past, I didn't add the comment though.

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>