On Tue, May 06, 2014 at 04:42:18PM +0200, Vlastimil Babka wrote: > >>>+unsigned long get_pageblock_flags_mask(struct page *page, > >>>+ unsigned long end_bitidx, > >>>+ unsigned long nr_flag_bits, > >>>+ unsigned long mask) > >>> { > >>> struct zone *zone; > >>> unsigned long *bitmap; > >>>- unsigned long pfn, bitidx; > >>>- unsigned long flags = 0; > >>>- unsigned long value = 1; > >>>+ unsigned long pfn, bitidx, word_bitidx; > >>>+ unsigned long word; > >>> > >>> zone = page_zone(page); > >>> pfn = page_to_pfn(page); > >>> bitmap = get_pageblock_bitmap(zone, pfn); > >>> bitidx = pfn_to_bitidx(zone, pfn); > >>>+ word_bitidx = bitidx / BITS_PER_LONG; > >>>+ bitidx &= (BITS_PER_LONG-1); > >>> > >>>- for (; start_bitidx <= end_bitidx; start_bitidx++, value <<= 1) > >>>- if (test_bit(bitidx + start_bitidx, bitmap)) > >>>- flags |= value; > >>>- > >>>- return flags; > >>>+ word = bitmap[word_bitidx]; > >> > >>I wonder if on some architecture this may result in inconsistent > >>word when racing with set(), i.e. cmpxchg? We need consistency at > >>least on the granularity of byte to prevent the problem with bogus > >>migratetype values being read. > >>fix: > > > >The number of bits align on the byte boundary so I do not think there is > >a problem there. There is a BUILD_BUG_ON check in set_pageblock_flags_mask > >in case this changes so it can be revisited if necessary. > > I was wondering about hardware guarantees in that case (e.g. > consistency at least on the granularity of byte when a simple memory > read races with write) but after some discussion in the office I > understand that hardware without such guarantees wouldn't be able to > run Linux anyway :) > > Still I wonder if ACCESS_ONCE would be safer in the 'word' variable > assignment to protect against compiler trying to be too smart? > I couldn't see a case in the get path where it would matter. I put an ACCESS_ONCE in the set path in case the compiler accidentally determined that old_word was invariant in that loop. > Anyway with the nr_flag_bits removed: > > Acked-by: Vlastimil Babka <vbabka@xxxxxxx> > Thanks. -- Mel Gorman SUSE Labs -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx"> email@xxxxxxxxx </a>