On Fri, Apr 29 2022 at 4:37P -0400, Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> wrote: > On Thu, Apr 28, 2022 at 12:22:26PM -0400, Mikulas Patocka wrote: > > > > > > On Tue, 26 Apr 2022, Greg Kroah-Hartman wrote: > > > > > On Thu, Apr 21, 2022 at 02:08:30PM -0400, Mikulas Patocka wrote: > > > > Hi > > > > > > Not really needed in a changelog text :) > > > > > > > This is backport of patches d208b89401e0 ("dm: fix mempool NULL pointer > > > > race when completing IO") and 9f6dc6337610 ("dm: interlock pending dm_io > > > > and dm_wait_for_bios_completion") for the kernel 5.10. > > > > > > Can you just make these 2 different patches? > > > > > > > > > > > The bugs fixed by these patches can cause random crashing when reloading > > > > dm table, so it is eligible for stable backport. > > > > > > > > This patch is different from the upstream patches because the code > > > > diverged significantly. > > > > > > > > > > This change is _VERY_ different. I would need acks from the maintainers > > > of this code before I could accept this, along with a much more detailed > > > description of why the original commits will not work here as well. > > > > > > Same for the other backports. > > > > Regarding backporting of 9f6dc633: > > > > My reasoning was that introducing "md->pending_io" in the backported > > stable kernels is useless - it will just degrade performance by consuming > > one more cache line per I/O without providing any gain. > > > > In the upstream kernels, Mike needs that "md->pending_io" variable for > > other reasons (the I/O accounting was reworked there in order to avoid > > some spikes with dm-crypt), but there is no need for it in the stable > > kernels. > > > > In order to fix that race condition, all we need to do is to make sure > > that dm_stats_account_io is called before bio_end_io_acct - and the patch > > does that - it swaps them. > > > > Do you still insist that this useless percpu variable must be added to the > > stable kernels? If you do, I can make it, but I think it's better to just > > swap those two functions. > > I am no insisting on anything, I want the dm maintainers to agree that > this change is acceptable to take as it is not what is in Linus's tree. > Every time we take a "not upstream" commit, the odds are 90% that it > ends up being wrong, so I need extra review and assurances that it is > acceptable before I can apply it. FYI, I've reviewed Mikulas's latest stable backport patches (not yet posted) and provided by Reviewed-by. So once you see them you can trust I've looked at the changes and am fine with you picking them up. Thanks, Mike