Re: [WIP 0/3] Memory model and atomic API in Rust

From: Matthew Wilcox
Date: Mon Apr 08 2024 - 13:03:34 EST


On Mon, Apr 08, 2024 at 09:55:23AM -0700, Paul E. McKenney wrote:
> On Mon, Apr 08, 2024 at 05:02:37PM +0100, Matthew Wilcox wrote:
> > In my ideal world, the compiler would turn this into:
> >
> > newfolio->flags |= folio->flags & MIGRATE_MASK;
>
> Why not accumulate the changes in a mask, and then apply the mask the
> one time? (In situations where __folio_set_foo() need not apply.)

Yes, absolutely, we can, should and probably eventually will do this
when it gets to the top of somebody's todo list. But it irks me that
we can't tell the compiler this is a safe transformation for it to make.
There are a number of places where similar things happen.

$ git grep folio_test.*folio_test

will find you 82 of them (where they happen to be on the same line)

if (folio_test_dirty(folio) || folio_test_locked(folio) ||
folio_test_writeback(folio))
break;

turns into:

1f41: 48 8b 29 mov (%rcx),%rbp
1f44: 48 c1 ed 04 shr $0x4,%rbp
1f48: 83 e5 01 and $0x1,%ebp
1f4b: 0f 85 d5 00 00 00 jne 2026 <filemap_range_has_writeback+0x1a6>
1f51: 48 8b 29 mov (%rcx),%rbp
1f54: 83 e5 01 and $0x1,%ebp
1f57: 0f 85 c9 00 00 00 jne 2026 <filemap_range_has_writeback+0x1a6>
1f5d: 48 8b 29 mov (%rcx),%rbp
1f60: 48 d1 ed shr $1,%rbp
1f63: 83 e5 01 and $0x1,%ebp
1f66: 0f 85 ba 00 00 00 jne 2026 <filemap_range_has_writeback+0x1a6>

rather than _one_ load from rcx and a test against a mask.

> If it turns out that we really do need a not-quite-volatile, what exactly
> does it do? You clearly want it to be able to be optimized so as to merge
> similar accesses. Is there a limit to the number of accesses that can
> be merged or to the region of code over which such merging is permitted?
> Either way, how is the compiler informed of these limits?

Right, like I said, it's not going to be easy to define exactly what we
want.