On Mon, Jul 04, 2022 at 06:26:08PM +0100, Matthew Wilcox wrote:
Hi Matthew,
cc02e6e21aa5 ("s390/crash: add missing iterator advance in copy_oldmem_page()") af2debd58bd7 ("s390/crash: make copy_oldmem_page() return number of bytes copied")
Both of them are also in the 5.18-rc queue here, right?
Yes, these are:
[PATCH 5.18 113/181] s390/crash: add missing iterator advance in copy_oldmem_page() Greg Kroah-Hartman
It's generally considered polite to cc the original author when you fix one of their patches. I wasn't aware of this patch.
Apologies for not doing that - I did not realize this patch could be of interest for non-s390.
While the code change looks right, the commit message is wrong; copy_oldmem_user() and copy_oldmem_kernel() need to GO AWAY. You need to be more like the other architectures and end up calling copy_to_iter(). I have no idea what this memcpy_hsa_kernel() and memcpy_hsa_user() are all about, but I was hoping that somebody from the s390 team would react to:
s390 needs more work to pass the iov_iter down further, or refactor, but I'd be more comfortable if someone who can test on s390 did that work.Maybe you'll do it.
I considered going with copy_to_iter(), but unfortunately getting rid of copy_oldmem_user() and copy_oldmem_kernel() is not an easy thing to do, if possible. At least for the time being we have to stay with these two and handle copy_oldmem_page() on our own.
Yet, a hope that a single-segment iterator on s390 would be enough turned out to be wrong and a follow-up fix is coming. Hopefully, it will make s390 code one step closer to others.
Thanks!