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 from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
