On 3/6/23 02:25, Huang, Ying wrote: > Hi, Linus, > > Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> writes: > >> On Sat, Mar 4, 2023 at 3:21 PM Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote: >>> >>> Ah. Ying did it this way: >> >> Yeah, I saw that patch flying past, but I actually think that it only >> silences the warning almost by mistake. There's nothing fundamental in >> there that a compiler wouldn't just follow across two assignments, and >> it just happens to now not trigger any more. >> >> Assigning to a union entry is a more fundamental operation in that >> respect. Not that the compiler still doesn't see that it's assigning a >> value that in the end is not really type compatible, so a different >> version of gcc could still warn, but at that point I feel like it's >> more of an actual compiler bug than just "oh, the compiler didn't >> happen to follow the cast through a temporary". > > Yes. Your fix is much better. This can be used for > __page_set_anon_rmap() family too to make the code look better? Those are trickier due to the PAGE_MAPPING_ANON tagging bit which your code doesn't need to handle because you can simply store an untagged anon_vma pointer. Otherwise we could have the union at the struct page level already (but probably not at this point as IIRC Matthew is planning to have completely separate types for anon and file folios). So right now we have e.g. folio_get_anon_vma() using unsigned long as the intermediate variable, page_move_anon_rmap() using a void * variable, __page_set_anon_rmap() casting through a (void *) ... Is there a single recommended way for "tagged pointers" that we could unify that to? > Best Regards, > Huang, Ying >
