RE: [RFC PATCH v1] devres: align devres.data strictly only for devm_kmalloc()

[Alexey Brodkin]

Hi Robin, Peter, all,

[snip]
 
> On 2019-12-20 2:06 pm, Peter Zijlstra wrote:
> > On Fri, Dec 20, 2019 at 11:19:27AM +0100, Marc Gonzalez wrote:
> >> Would anyone else have any suggestions, comments, insights, recommendations,
> >> improvements, guidance, or wisdom? :-)
> >
> > Flip devres upside down!
> 
> Which doesn't really help :(
> 
> > **WARNING, wear protective glasses when reading the below**
> >
> >
> > struct devres {
> > 	struct devres_node	node;
> > 	void			*data;
> > };
> >
> > /*
> >   * We place struct devres at the tail of the memory allocation
> >   * such that data retains the ARCH_KMALLOC_MINALIGN alignment.
> >   * struct devres itself is just 4 pointers and should therefore
> >   * only require trivial alignment.
> >   */
> > static inline struct devres *data2devres(void *data)
> > {
> > 	return (struct devres *)(data + ksize(data) - sizeof(struct devres));
> > }
> >
> > void *alloc_dr(...)
> > {
> > 	struct devres *dr;
> > 	void *data;
> >
> > 	data = kmalloc(size + sizeof(struct devres), GFP_KERNEL);
> 
> At this point, you'd still need to special-case devm_kmalloc() to ensure
> size is rounded up to the next ARCH_KMALLOC_MINALIGN granule, or you'd
> go back to the original problem of the struct devres fields potentially
> sharing a cache line with the data buffer. That needs to be avoided,
> because if the devres list is modified while the buffer is mapped for
> noncoherent DMA (which could legitimately happen as they are nominally
> distinct allocations with different owners) there's liable to be data
> corruption one way or the other.

Well it somehow used to work for quite some time now with the data-buffer
being allocated with 4 words offset (which is 16 bytes for 32-bit platform
and 32 for 64-bit which is still much less than mentioned 128 bytes).
Or we just never managed to identify those rare cases when data corruption
really happened?

> No matter which way round you allocate devres and data, by necessity
> they're always going to consume the same total amount of memory.

So then the next option I guess is to separate meta-data from data buffers
completely. Are there any reasons to not do that other than the hack we're
discussing here (meta-data in the beginning of the buffer) used to work OK-ish?

-Alexey