Hi,

I'm not completely sure whether this is the bug or not, nor what would be
the performance consequences of my attached fix (wrt the block layer). So
you're probably cc'ed because I've found similar threads with your names
on them.


lock_page (and other waiters on page flags bits) use sync_page when sleeping
on a bit. sync_page, however, requires that the page's mapping be pinned
(which is what we're sometimes trying to lock the page for).

Blatant offender is set_page_dirty_lock, which falls to the race it purports
to fix. Perhaps all callers could be fixed, however it seems that the pinned
mapping lock_page precondition is counter-intuitive and I'd bet other callers
to lock_page or wait_on_page_bit have got it wrong too.

Also: splice can change a page's mapping, so it would have been possible to
use the wrong mapping to "sync" a page.

Can we get rid of the whole thing, confusing memory barriers and all? Nobody
uses anything but the default sync_page, and if block rq plugging is terribly
bad for performance, perhaps it should be reworked anyway? It shouldn't be a
correctness thing, right?

Alternatives include RCU freeing of mappings and other things that increase
in complexity from there (eg. audit and fix all callers). It is so much cooler
to get rid of code though ;)

Comments?

-- 
SUSE Labs, Novell Inc.