Re: [PATCH v2] rust: page: add byte-wise atomic memory copy methods

[Peter Zijlstra <peterz@infradead.org>]

On Tue, Feb 17, 2026 at 01:09:39PM +0000, Alice Ryhl wrote:
> On Tue, Feb 17, 2026 at 01:09:20PM +0100, Peter Zijlstra wrote:
> > On Tue, Feb 17, 2026 at 11:51:20AM +0000, Alice Ryhl wrote:
> > 
> > > In my experience with dealing with `struct page` that is mapped into a
> > > vma, you need memcpy because the struct might be split across two
> > > different pages in the vma. The pages are adjacent in userspace's
> > > address space, but not necessarily adjacent from the kernel's POV.
> > > 
> > > So you might end up with something that looks like this:
> > > 
> > > struct foo val;
> > > void *ptr1 = kmap_local_page(p1);
> > > void *ptr2 = kmap_local_page(p2);
> > > memcpy(ptr1 + offset, val, PAGE_SIZE - offset);
> > > memcpy(ptr2, val + offset, sizeof(struct foo) - (PAGE_SIZE - offset));
> > > kunmap_local(ptr2);
> > > kunmap_local(ptr1);
> > 
> >   barrier();
> > 
> > > if (is_valid(&val)) {
> > >     // use val
> > > }
> > > 
> > > This exact thing happens in Binder. It has to be a memcpy.
> > 
> > Sure, but then stick that one barrier() in and you're good.
> 
> Are we really good? Consider this code:
> 
> 	bool is_valid(struct foo *val)
> 	{
> 		// for the sake of example
> 		return val->my_field != 0;
> 	}
> 
> 	struct foo val;
> 
> 	void *ptr = kmap_local_page(p1);
> 	memcpy(ptr, val, sizeof(struct foo));
> 	kunmap_local(p);
> 	barrier();
> 	if (is_valid(&val)) {
> 	    // use val
> 	}
> 
> optimize it into this first:
> 
> 	struct foo val;
> 	int my_field_copy;
> 
> 	void *ptr = kmap_local_page(p1);
> 	memcpy(ptr, val, sizeof(struct foo));
> 	my_field_copy = val->my_field;
> 	kunmap_local(p);
> 	barrier();
> 	if (my_field_copy != 0) {
> 	    // use val
> 	}
> 
> then optimize it into:
> 
> 	struct foo val;
> 	int my_field_copy;
> 
> 	void *ptr = kmap_local_page(p1);
> 	memcpy(ptr, val, sizeof(struct foo));
> 	my_field_copy = ((struct foo *) ptr)->my_field;
> 	kunmap_local(p);
> 	barrier();
> 	if (my_field_copy != 0) {
> 	    // use val
> 	}

I don;t think this is allowed. You're lifting the load over the
barrier(), that is invalid.

So the initial version is:

	ptr = kmap_local_page()
	memcpy(ptr, val, sizeof(val))
	kunmap_local(ptr);

	barrier()

	if (val.field)
	  // do stuff

So the 'val.field' load is after the barrier(); and it must stay there,
because the barrier() just told the compiler that all of memory changed
-- this is what barrier() does.