Re: [rfc][patch] remove racy sync_page?

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hugh Dickins wrote:
On Tue, 30 May 2006, Nick Piggin wrote:

But for 2.6.17, how's this?


It was a great emperor's-clothes-like discovery.  But we've survived
for so many years without noticing, does it have to be fixed right
now for 2.6.17?  (I bet I'd be insisting yes if I'd found it.)

It's up to Linus and Andrew I guess. I don't see why not, but I
don't much care one way or the other. But thanks having a quick
look at it, we may want it for the Suse kernel.


The thing I don't like about your lock_page_nosync (reasonable as
it is) is that the one case you're using it, set_page_dirty_nolock,
would be so much happier not to have to lock the page in the first
place - it's only doing _that_ to stabilize page->mapping, and the
lock_page forbids it from being called from anywhere that can't
sleep, which is often just where we want to call it from.  Neil's
suggestion, using a spin_lock against the mapping changing, would
help there; but seems like more work than I'd want to get into.

But making PG_lock a spinning lock is completely unrelated to the
bug at hand.


So, although I think lock_page_nosync fixes the bug (at least in
that one place we've identified there's likely to be such a bug),
it seems to be aiming at the wrong target.  I'm pacing and thinking,
doubt I'll come up with anything better, please don't hold breath.

It is the correct target. I know all about your set_page_dirty_lock
problems, but they aren't what I'm trying to fix.

AFAIKS, you could also make set_page_dirty_lock non sleeping quite
easily by making inode slabs RCU freed.

What places want to use set_page_dirty_lock without sleeping?
The only place in drivers/ apart from sg/st that SetPageDirty are
rd.c and via_dmablit.c, both of which look OK, if a bit crufty.

--
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com -
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux