Re: [PATCH 06/33] readahead: refactor __do_page_cache_readahead()

[Andrew Morton]

Paul Mackerras <paulus@xxxxxxxxx> wrote:
>
> Andrew Morton writes:
> 
> > Wu Fengguang <wfg@xxxxxxxxxxxxxxxx> wrote:
> > > @@ -302,6 +303,8 @@ __do_page_cache_readahead(struct address
> > >  			break;
> > >  		page->index = page_offset;
> > >  		list_add(&page->lru, &page_pool);
> > > +		if (page_idx == nr_to_read - lookahead_size)
> > > +			__SetPageReadahead(page);
> > >  		ret++;
> > >  	}
> > 
> > OK.  But the __SetPageFoo() things still give me the creeps.
> 
> I just hope that Wu Fengguang, or whoever is making these patches,
> realizes that on some architectures, doing __set_bit on one CPU
> concurrently with another CPU doing set_bit on a different bit in the
> same word can result in the second CPU's update getting lost...
> 

That's true even on x86.

Yes, this is understood - in this case he's following Nick's dubious lead
in leveraging our knowledge that no other code path will be attempting to
modify this page's flags at this time.  It's just been taken off the
freelist, it's not yet on the LRU and we own the only ref to it.

The only hole I was able to shoot in this is swsusp, which walks mem_map[]
fiddling with page flags.  But when it does this, only one CPU is running.

But I'm itching for an excuse to extirpate it all ;)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/