On 7/17/24 10:41, Shinichiro Kawasaki wrote: > Hi Ofir, thank you for this v3 series. The two patches look good to me, except > one unclear point below. > > On Jul 16, 2024 / 14:50, Ofir Gal wrote: > [...] >> diff --git a/tests/md/001 b/tests/md/001 >> new file mode 100755 >> index 0000000..e9578e8 >> --- /dev/null >> +++ b/tests/md/001 >> @@ -0,0 +1,85 @@ >> +#!/bin/bash >> +# SPDX-License-Identifier: GPL-3.0+ >> +# Copyright (C) 2024 Ofir Gal >> +# >> +# The bug is "visible" only when the underlying device of the raid is a network >> +# block device that utilize MSG_SPLICE_PAGES. nvme-tcp is used as the network device. >> +# >> +# Regression test for patch "md/md-bitmap: fix writing non bitmap pages" and >> +# for patch "nvme-tcp: use sendpages_ok() instead of sendpage_ok()" > > The cover letter of the series says that the new test case is the regression > test for the patch "md/md-bitmap: fix writing non bitmap pages". On the other > hand, the comment above says that this test case is for the two patches. Which > is correct? (Or which is more accurate?) > > When I ran this test case, it failed on the kernel v6.10, which is expected. > Then I applied the 1st patch "md/md-bitmap: fix writing non bitmap pages" only > to the v6.10 kernel, and the test case passed. It passed without the 2nd patch > "nvme-tcp: use sendpages_ok() instead of sendpage_ok()". So, I'm not sure if > this test case is the regression test for the 2nd patch. Sorry for the confusion, either one of the patches solves the issue. The "md/md-bitmap: fix writing non bitmap pages" patch fix the root cause issue. md-bitmap sent contiguous pages that it didn't allocate, that happened to be a page list that consists of non-slub and slub pages. The nvme-tcp assumed there won't be a mixed IO of slub and non-slub in order to turn on MSG_SPLICE_PAGES, "nvme-tcp: use sendpages_ok() instead of sendpage_ok()" patch address this assumption by checking each page instead of the first page. I think it's more accurate to say this regression test for the 1st patch, we can probably make a separate regression test for the 2nd patch. I can change the comment to prevent confusion.
