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

Andrew Morton
2006-05-25T22:41:26Z

Paul Mackerras <paulus@samba.org> wrote:
>
> Andrew Morton writes:
>
> > Wu Fengguang <wfg@mail.ustc.edu.cn> 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 ;)