On Apr 22, 2021, at 3:44 AM, Jan Kara <jack@suse.cz> wrote:

Hello!

On Thu 22-04-21 11:22:07, Joseph Qi wrote:

Checked the code flow, it seems the race you worried truly exists.
We have to take ip_alloc_sem before calling into ocfs2_get_block().

OK, that's what I assumed but this won't be easy. Because ocfs2_writepage()
is called with a page locked but ocfs2_remove_inode_range() gets called
under ip_alloc_sem and locks pages in ocfs2_truncate_cluster_pages() ->
truncate_inode_pages_range(). Which creates ABBA deadlock. So you'll
probably need to come up with a similar dance like in ocfs2_readpage().

But after fixing this, ip_alloc_sem should provide you with enough
protection so that ocfs2 isn't prone to races between hole punch and
readahead / fault my work will be mostly irrelevant for OCFS2.

Thanks for confirmation!

Honza

On 4/22/21 12:29 AM, Jan Kara wrote:

Hello,

I'm unifying protection various filesystems use to protect hole punch
operations from racing with other operations (like readahead, page fault,
writepage etc.). I was looking into OCFS2 and I think it is prone to a
following race which can possibly lead to filesystem corruption. But maybe
I miss something so that's why I'm writing here. The scenario I'm concerned
about is:

CPU1 CPU2
ocfs2_remove_inode_range() ocfs2_writepage()
 ...   block_write_full_page()
 ocfs2_remove_btree_range()     ocfs2_extent_map_get_blocks()

Now ocfs2_extent_map_get_blocks() runs without protection of ip_alloc_sem
AFAICT and so both these operations can be modifying extent map at the same
time? What am I missing?

Honza

-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel