Re: smp race fix between invalidate_inode_pages* and do_no_page

[Nick Piggin <nickpiggin@yahoo.com.au>]

Hugh Dickins wrote:
> On Wed, 11 Jan 2006, Andrea Arcangeli wrote:
> 
>>On Wed, Jan 11, 2006 at 01:06:38AM -0800, Andrew Morton wrote:
>>
>>>Confused.  do_no_page() doesn't have a page to lock until it has called
>>>->nopage.
>>
>>yes, I mean doing lock_page after ->nopage returned it here:
>>
>>	lock_page(page);
>>	if (mapping && !page->mapping)
>>		goto bail_out;
>>	page_table = pte_offset_map_lock(mm, pmd, address, &ptl);
>>[..]
>>			page_add_file_rmap()
>>			unlock_page()
> 
> 
> I've rather chosen this mail at random from the thread, and can't
> come up with better than a few random remarks on it all, sorry.
> 
> Though using lock_page may solve more than one problem, without seeing
> a full implementation I'm less sure than the rest of you that it will
> really do the job.
> 
> And less confident than you and Nick that adding a sleeping lock here
> in nopage won't present a scalability issue.  Though it gives me a
> special thrill to see Nick arguing in favour of extra locking ;)
> 

I argue in favour of extra locking simply because the concept doesn't
fundamentally harm scalability ie. because it is very short held and
there are other unavoidable cacheline conflicts already there.

Having it a sleeping lock rather than spinning is another thing though.
However unless you imagine it running into other long held page lockers
(I guess reclaim might be one, but quite rare -- any other significant
lock_page users that we might hit?), then I don't think it would impact
scalability much on workloads that don't already hurt.

-- 
SUSE Labs, Novell Inc.

Send instant messages to your online friends http://au.messenger.yahoo.com