Re: [PATCH v2 10/10] s390/maccess: Remove spinlock from __s390_kernel_write()

[Heiko Carstens]

On Wed, Oct 28, 2026 at 09:59:46PM +0100, Ilya Leoshkevich wrote:
> On 10/27/26 15:30, Heiko Carstens wrote:
> > The spinlock in __s390_kernel_write() was introduced to serialize the
> > read-modify-write sequence against concurrent callers. Now that all
> > runtime callers of s390_kernel_write() hold the text_mutex, the
> > spinlock is not required anymore.
> > 
> > Replace it with preempt_disable() / preempt_enable() and sync_core().
> > Previously, CPU serialization was implicitly done by other
> > instructions. Make this explicit with sync_core() instead of
> > relying on that.
> > 
> > Preemption is disabled so that sync_core() issues the CPU serialization
> > instruction on the same CPU that executed the stores. This completes all
> > previous stores and makes them visible to other CPUs.
> 
> Doesn't preemption happen only via interrupts, which always serialize?

Yes, but again, that's an implicit serialization. The idea of this was to make
the code not relying on rather subtle architecture details (as written above,
the "implicit" part).

Even if interrupts wouldn't serialize: when scheduling the task away there is
almost certainly a compare and swap instruction for taking some lock executed
on the CPU before the next task runs.

So this part is more about making things explicit, instead of relying on
something that is implicitly happening.

But the main point is of course to get rid of our private locking.

> >   static inline void *s390_kernel_write(void *dst, const void *src, size_t size)
> >   {
> > -	if (__is_defined(__DECOMPRESSOR))
> > -		return memcpy(dst, src, size);
> > +#ifdef __DECOMPRESSOR
> > +	return memcpy(dst, src, size);
> > +#else
> > +	lockdep_assert_held(&text_mutex);
> >   	return __s390_kernel_write(dst, src, size);
> > +#endif
> >   }
> 
> I assume #ifdef is required because lockdep_assert_held() is not
> available in decompressor and there are no stubs?

To be honest, I didn't even try; I didn't want to rely on even more things for
the decompressor.