On Thu, 11 Jul 2019 14:58:38 +0200 Jan Kara jack@suse.cz wrote:
buffer_migrate_page_norefs() can race with bh users in a following way:
CPU1 CPU2 buffer_migrate_page_norefs() buffer_migrate_lock_buffers() checks bh refs spin_unlock(&mapping->private_lock) __find_get_block() spin_lock(&mapping->private_lock) grab bh ref spin_unlock(&mapping->private_lock) move page do bh work
This can result in various issues like lost updates to buffers (i.e. metadata corruption) or use after free issues for the old page.
Closing this race window is relatively difficult. We could hold mapping->private_lock in buffer_migrate_page_norefs() until we are finished with migrating the page but the lock hold times would be rather big. So let's revert to a more careful variant of page migration requiring eviction of buffers on migrated page. This is effectively fallback_migrate_page() that additionally invalidates bh LRUs in case try_to_free_buffers() failed.
Is this premature optimization? Holding ->private_lock while messing with the buffers would be the standard way of addressing this. The longer hold times *might* be an issue, but we don't know this, do we? If there are indeed such problems then they could be improved by, say, doing more of the newpage preparation prior to taking ->private_lock.