On Mon, Jul 17, 2017 at 19:29:57 -1000, Richard Henderson wrote: > On 07/17/2017 06:54 PM, Emilio G. Cota wrote: > >What threw me off was that in lookup_tb_ptr we're not checking tb->invalid, > >and that biased me into thinking that it's not needed. But I should have > >tried harder. Also, that's a bug, and yet another reason to have tb_lookup, > >so that we fix these things at once in one place. > > Yes, me as well. Quite right we need only one copy of this code. > > >> (tb->flags & (CF_HASH_MASK | CF_INVALID)) == cf_mask > >> > >>So that we continue to check CF_INVALID each time we lookup a TB, but now we > >>get it for free as a part of the other flags check. > > > >With the annoying atomic_read thrown in there :-) but yes, will do. > > Yes of course. ;-)
Gaah, we'll need to update all readers of tb->cflags, of which we have plenty (~145, mostly in target code) to avoid C11 undefined behaviour and make Paolo happy. Should I do those updates in the same patch where tb->invalid is brought over to cflags? Alternatives: have a later patch where all readers are converted to atomic_read, or keep tb->invalid as a separate field (we could use that 4-byte hole in struct tb_tc..) E.