Messages in this thread | | | Date | Tue, 10 May 2011 11:55:26 -0700 | Subject | Re: [PATCH v2] tmpfs: fix race between umount and writepage | From | Hugh Dickins <> |
| |
On Tue, May 10, 2011 at 2:52 AM, Konstantin Khlebnikov <khlebnikov@parallels.com> wrote: > Hugh Dickins wrote: >> >> On Sun, 8 May 2011, Konstantin Khlebnikov wrote: >>> >>> Ok, I can test final patch-set on the next week. >>> Also I can try to add some swapoff test-cases. >> >> That would be helpful if you have the time: thank you. > > I Confirm, patch 1/3 really fixes race between writepage and umount, as > expected.
Good, thank you (but that path was identical to what you'd already tested).
> > In patch 2/3: race-window between unlock_page and iput extremely small.
(I should clarify that the main race window is actually much wider than that. That page lock is only effective at holding off shmem_evict_inode() while the page is in the file's pagecache - between the (old positioning of) mutex_unlock(&shmem_swaplist_mutex) and the add_to_page_cache_locked(), the page is just in swapcache and so not recognizably attached to the file: shmem_evict_inode() will call shmem_truncate_range(), and that would find the swp_entry_t, but it frees it with a free_swap_and_cache() - which does not wait if it cannot trylock the page.)
> My test works fine in parallel with thirty random swapon-swapoff, > but it works without this patch too, thus I cannot catch this race.
Thanks for trying. Given my difficulty in reproducing your umount case, I'm not at all surprised that you didn't manage to reproduce this swapoff case. Indeed, I didn't even try to reproduce it myself: I just saw the theoretical possibility once you'd warned me of igrab(), and tested that this igrab-less approach works as well as the old approach, without risking that race.
> > I apply patch 3/3 too, but have not tested this case.
Fine, that part I could reproduce fairly easily for myself, and the fix tested out fine.
Thanks, Hugh
| |