The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* Re: Kernel 6.0.5 breaks virtual machines that boot from nfs mounted qcow2 images
       [not found] <65115206-ec17-443e-8495-91661a2fd9be@control.lth.se>
@ 2022-11-23 10:08 ` Anders Blomdell
  2022-11-23 16:11   ` Chuck Lever III
                     ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Anders Blomdell @ 2022-11-23 10:08 UTC (permalink / raw)
  To: linux-nfs, Chuck Lever, Jeff Layton, linux-kernel
  Cc: Anna Schumaker, Alexander Viro, Trond Myklebust,
	Benjamin Coddington

Our problems turned out to be a fallout of Al Viros's splice rework, where nfsd reads with non-zero offsets and not ending
on a page boundary failed to remap the last page. I belive that this is a decent fix for that problem (tested on v6.1-rc6,
6.0.7 and 6.0.9)

---- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -873,7 +873,7 @@ nfsd_splice_actor(struct pipe_inode_info *pipe, struct pipe_buffer *buf,
         unsigned offset = buf->offset;
  
         page += offset / PAGE_SIZE;
-       for (int i = sd->len; i > 0; i -= PAGE_SIZE)
+       for (int i = sd->len + offset % PAGE_SIZE; i > 0; i -= PAGE_SIZE)
                 svc_rqst_replace_page(rqstp, page++);
         if (rqstp->rq_res.page_len == 0)        // first call
                 rqstp->rq_res.page_base = offset % PAGE_SIZE;

On 2022-11-11 20:35, Anders Blomdell wrote:
> The text below is an excerpt of  https://bugzilla.redhat.com/show_bug.cgi?id=2142132
> 
> Description of problem:
> 
> Virtual machines that boots from nfs mounted qcow2 images fails to boot when
> the nfs server runs kernel 6.0.5
> 
> The erros are often
> 
>    Booting from Hard Disk...
>    GRUB loading.
>    Welcome to GRUB!
> 
>    error ../../grub-core/kern/dl.c:ELF header smaller than expected.
>    Entering rescue mode...
>    grub rescue>
> 
> Version-Release number of selected component (if applicable):
> 
>    qemu-6.2.0-16.fc36.x86_64
>    kernel-6.0.5-200.fc36.x86_64
> 
> How reproducible:
> 
>    always
> 
> Steps to Reproduce:
> 1. Run a virtual machine with boot disk qcow2 formatted mounted from an nfs server
>     running kernel-6.0.5-200.fc36.x86_64
> 
> Actual results:
> 
> Machine fails to boot
> 
> Expected results:
> 
> Clean boot
> 
> Additional info:
> 
> Git repo is linux-stable
> 
> A git bisect lands me at 310d9d5a5009a93377200b98daa2d84aa2bd8160
> 
> So commit 746de1f86fcd works
> 
> But cherry picking f0f6b614f83d 310d9d5a5009 bfbfb6182ad1 on top of 746de1f86fcd is broken.
> 
> The bfbfb6182ad1 is picked due to the commit comment
> 
> commit bfbfb6182ad1d7d184b16f25165faad879147f79
> Author: Al Viro <viro@zeniv.linux.org.uk>
> Date:   Sat Sep 10 22:14:02 2022 +0100
> 
>      nfsd_splice_actor(): handle compound pages
>      pipe_buffer might refer to a compound page (and contain more than a PAGE_SIZE
>      worth of data).  Theoretically it had been possible since way back, but
>      nfsd_splice_actor() hadn't run into that until copy_page_to_iter() change.
>      Fortunately, the only thing that changes for compound pages is that we
>      need to stuff each relevant subpage in and convert the offset into offset
>      in the first subpage.
>      Acked-by: Chuck Lever <chuck.lever@oracle.com>
>      Tested-by: Benjamin Coddington <bcodding@redhat.com>
>      Fixes: f0f6b614f83d "copy_page_to_iter(): don't split high-order page in case of ITER_PIPE"
>      Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> 
> The bisect I did:
> 
> git bisect start
> # status: waiting for both good and bad commits
> # bad: [3829606fc5dffeccdf80aebeed3aa75255257f35] Linux 6.0.5
> git bisect bad 3829606fc5dffeccdf80aebeed3aa75255257f35
> # status: waiting for good commit(s), bad commit known
> # good: [0b0649b1d27a768d37f23acf4d88e6e90cca7856] Linux 5.19.4
> git bisect good 0b0649b1d27a768d37f23acf4d88e6e90cca7856
> # good: [0b0649b1d27a768d37f23acf4d88e6e90cca7856] Linux 5.19.4
> git bisect good 0b0649b1d27a768d37f23acf4d88e6e90cca7856
> # good: [3d7cb6b04c3f3115719235cc6866b10326de34cd] Linux 5.19
> git bisect good 3d7cb6b04c3f3115719235cc6866b10326de34cd
> # good: [7c5c3a6177fa9646884114fc7f2e970b0bc50dc9] Merge tag 'for-linus' of git://git.kernel.org/pub/scm/virt/kvm/kvm
> git bisect good 7c5c3a6177fa9646884114fc7f2e970b0bc50dc9
> # bad: [eeac7730418563152b0e3172bce9bac4ff6d6bc4] perf pmu-events: Avoid passing pmu_events_map
> git bisect bad eeac7730418563152b0e3172bce9bac4ff6d6bc4
> # good: [f20c95b46b8fa3ad34b3ea2e134337f88591468b] Merge tag 'tpmdd-next-v5.20' of git://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd
> git bisect good f20c95b46b8fa3ad34b3ea2e134337f88591468b
> # good: [c993e07be023acdeec8e84e2e0743c52adb5fc94] Merge tag 'dma-mapping-5.20-2022-08-06' of git://git.infradead.org/users/hch/dma-mapping
> git bisect good c993e07be023acdeec8e84e2e0743c52adb5fc94
> # good: [e2ebff9c57fe4eb104ce4768f6ebcccf76bef849] vfs: Check the truncate maximum size in inode_newsize_ok()
> git bisect good e2ebff9c57fe4eb104ce4768f6ebcccf76bef849
> # good: [c8a684e2e110376c58f0bfa30fb3855d1e319670] Merge tag 'leds-5.20-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/pavel/linux-leds
> git bisect good c8a684e2e110376c58f0bfa30fb3855d1e319670
> # good: [c72687614b3627b3ea55d8d169e31cac70f56f3e] Merge tag 'rpmsg-v5.20' of git://git.kernel.org/pub/scm/linux/kernel/git/remoteproc/linux
> git bisect good c72687614b3627b3ea55d8d169e31cac70f56f3e
> # bad: [f30adc0d332fdfe5315cb98bd6a7ff0d5cf2aa38] Merge tag 'pull-work.iov_iter-rebased' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs
> git bisect bad f30adc0d332fdfe5315cb98bd6a7ff0d5cf2aa38
> # good: [60349fd52ecbb8b14545ff25aba2f2e230c4d618] remoteproc: using pm_runtime_resume_and_get instead of pm_runtime_get_sync
> git bisect good 60349fd52ecbb8b14545ff25aba2f2e230c4d618
> # good: [8520008417c581c4c22e39597f92b9814ae34c31] fold __pipe_get_pages() into pipe_get_pages()
> git bisect good 8520008417c581c4c22e39597f92b9814ae34c31
> # good: [746de1f86fcd33464acac047f111eea877f2f7a0] pipe_get_pages(): switch to append_pipe()
> git bisect good 746de1f86fcd33464acac047f111eea877f2f7a0
> # good: [5d5d353bed32dc3ea52e2619e0d1c60b17133b91] Merge tag 'rproc-v5.20' of git://git.kernel.org/pub/scm/linux/kernel/git/remoteproc/linux
> git bisect good 5d5d353bed32dc3ea52e2619e0d1c60b17133b91
> # bad: [f0f6b614f83dbae99d283b7b12ab5dd2e04df979] copy_page_to_iter(): don't split high-order page in case of ITER_PIPE
> git bisect bad f0f6b614f83dbae99d283b7b12ab5dd2e04df979
> # bad: [310d9d5a5009a93377200b98daa2d84aa2bd8160] expand those iov_iter_advance()...
> git bisect bad 310d9d5a5009a93377200b98daa2d84aa2bd8160
> # first bad commit: [310d9d5a5009a93377200b98daa2d84aa2bd8160] expand those iov_iter_advance()...
> 
> 

-- 
Anders Blomdell                  Email: anders.blomdell@control.lth.se
Department of Automatic Control
Lund University                  Phone:    +46 46 222 4625
P.O. Box 118
SE-221 00 Lund, Sweden

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Kernel 6.0.5 breaks virtual machines that boot from nfs mounted qcow2 images
  2022-11-23 10:08 ` Kernel 6.0.5 breaks virtual machines that boot from nfs mounted qcow2 images Anders Blomdell
@ 2022-11-23 16:11   ` Chuck Lever III
  2022-11-23 17:29   ` Al Viro
  2022-11-23 17:49   ` Benjamin Coddington
  2 siblings, 0 replies; 7+ messages in thread
From: Chuck Lever III @ 2022-11-23 16:11 UTC (permalink / raw)
  To: Anders Blomdell
  Cc: Linux NFS Mailing List, Jeff Layton, linux-kernel@vger.kernel.org,
	Anna Schumaker, Al Viro, Trond Myklebust, Benjamin Coddington



> On Nov 23, 2022, at 5:08 AM, Anders Blomdell <anders.blomdell@control.lth.se> wrote:
> 
> Our problems turned out to be a fallout of Al Viros's splice rework, where nfsd reads with non-zero offsets and not ending
> on a page boundary failed to remap the last page. I belive that this is a decent fix for that problem (tested on v6.1-rc6,
> 6.0.7 and 6.0.9)
> 
> ---- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -873,7 +873,7 @@ nfsd_splice_actor(struct pipe_inode_info *pipe, struct pipe_buffer *buf,
>        unsigned offset = buf->offset;
>         page += offset / PAGE_SIZE;
> -       for (int i = sd->len; i > 0; i -= PAGE_SIZE)
> +       for (int i = sd->len + offset % PAGE_SIZE; i > 0; i -= PAGE_SIZE)
>                svc_rqst_replace_page(rqstp, page++);
>        if (rqstp->rq_res.page_len == 0)        // first call
>                rqstp->rq_res.page_base = offset % PAGE_SIZE;

Hello Anders, I will wait for Al's review.

The official version of your patch will need to include Signed-off-by and
Fixes tags.


> On 2022-11-11 20:35, Anders Blomdell wrote:
>> The text below is an excerpt of  https://bugzilla.redhat.com/show_bug.cgi?id=2142132
>> Description of problem:
>> Virtual machines that boots from nfs mounted qcow2 images fails to boot when
>> the nfs server runs kernel 6.0.5
>> The erros are often
>>   Booting from Hard Disk...
>>   GRUB loading.
>>   Welcome to GRUB!
>>   error ../../grub-core/kern/dl.c:ELF header smaller than expected.
>>   Entering rescue mode...
>>   grub rescue>
>> Version-Release number of selected component (if applicable):
>>   qemu-6.2.0-16.fc36.x86_64
>>   kernel-6.0.5-200.fc36.x86_64
>> How reproducible:
>>   always
>> Steps to Reproduce:
>> 1. Run a virtual machine with boot disk qcow2 formatted mounted from an nfs server
>>    running kernel-6.0.5-200.fc36.x86_64
>> Actual results:
>> Machine fails to boot
>> Expected results:
>> Clean boot
>> Additional info:
>> Git repo is linux-stable
>> A git bisect lands me at 310d9d5a5009a93377200b98daa2d84aa2bd8160
>> So commit 746de1f86fcd works
>> But cherry picking f0f6b614f83d 310d9d5a5009 bfbfb6182ad1 on top of 746de1f86fcd is broken.
>> The bfbfb6182ad1 is picked due to the commit comment
>> commit bfbfb6182ad1d7d184b16f25165faad879147f79
>> Author: Al Viro <viro@zeniv.linux.org.uk>
>> Date:   Sat Sep 10 22:14:02 2022 +0100
>>     nfsd_splice_actor(): handle compound pages
>>     pipe_buffer might refer to a compound page (and contain more than a PAGE_SIZE
>>     worth of data).  Theoretically it had been possible since way back, but
>>     nfsd_splice_actor() hadn't run into that until copy_page_to_iter() change.
>>     Fortunately, the only thing that changes for compound pages is that we
>>     need to stuff each relevant subpage in and convert the offset into offset
>>     in the first subpage.
>>     Acked-by: Chuck Lever <chuck.lever@oracle.com>
>>     Tested-by: Benjamin Coddington <bcodding@redhat.com>
>>     Fixes: f0f6b614f83d "copy_page_to_iter(): don't split high-order page in case of ITER_PIPE"
>>     Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
>> The bisect I did:
>> git bisect start
>> # status: waiting for both good and bad commits
>> # bad: [3829606fc5dffeccdf80aebeed3aa75255257f35] Linux 6.0.5
>> git bisect bad 3829606fc5dffeccdf80aebeed3aa75255257f35
>> # status: waiting for good commit(s), bad commit known
>> # good: [0b0649b1d27a768d37f23acf4d88e6e90cca7856] Linux 5.19.4
>> git bisect good 0b0649b1d27a768d37f23acf4d88e6e90cca7856
>> # good: [0b0649b1d27a768d37f23acf4d88e6e90cca7856] Linux 5.19.4
>> git bisect good 0b0649b1d27a768d37f23acf4d88e6e90cca7856
>> # good: [3d7cb6b04c3f3115719235cc6866b10326de34cd] Linux 5.19
>> git bisect good 3d7cb6b04c3f3115719235cc6866b10326de34cd
>> # good: [7c5c3a6177fa9646884114fc7f2e970b0bc50dc9] Merge tag 'for-linus' of git://git.kernel.org/pub/scm/virt/kvm/kvm
>> git bisect good 7c5c3a6177fa9646884114fc7f2e970b0bc50dc9
>> # bad: [eeac7730418563152b0e3172bce9bac4ff6d6bc4] perf pmu-events: Avoid passing pmu_events_map
>> git bisect bad eeac7730418563152b0e3172bce9bac4ff6d6bc4
>> # good: [f20c95b46b8fa3ad34b3ea2e134337f88591468b] Merge tag 'tpmdd-next-v5.20' of git://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd
>> git bisect good f20c95b46b8fa3ad34b3ea2e134337f88591468b
>> # good: [c993e07be023acdeec8e84e2e0743c52adb5fc94] Merge tag 'dma-mapping-5.20-2022-08-06' of git://git.infradead.org/users/hch/dma-mapping
>> git bisect good c993e07be023acdeec8e84e2e0743c52adb5fc94
>> # good: [e2ebff9c57fe4eb104ce4768f6ebcccf76bef849] vfs: Check the truncate maximum size in inode_newsize_ok()
>> git bisect good e2ebff9c57fe4eb104ce4768f6ebcccf76bef849
>> # good: [c8a684e2e110376c58f0bfa30fb3855d1e319670] Merge tag 'leds-5.20-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/pavel/linux-leds
>> git bisect good c8a684e2e110376c58f0bfa30fb3855d1e319670
>> # good: [c72687614b3627b3ea55d8d169e31cac70f56f3e] Merge tag 'rpmsg-v5.20' of git://git.kernel.org/pub/scm/linux/kernel/git/remoteproc/linux
>> git bisect good c72687614b3627b3ea55d8d169e31cac70f56f3e
>> # bad: [f30adc0d332fdfe5315cb98bd6a7ff0d5cf2aa38] Merge tag 'pull-work.iov_iter-rebased' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs
>> git bisect bad f30adc0d332fdfe5315cb98bd6a7ff0d5cf2aa38
>> # good: [60349fd52ecbb8b14545ff25aba2f2e230c4d618] remoteproc: using pm_runtime_resume_and_get instead of pm_runtime_get_sync
>> git bisect good 60349fd52ecbb8b14545ff25aba2f2e230c4d618
>> # good: [8520008417c581c4c22e39597f92b9814ae34c31] fold __pipe_get_pages() into pipe_get_pages()
>> git bisect good 8520008417c581c4c22e39597f92b9814ae34c31
>> # good: [746de1f86fcd33464acac047f111eea877f2f7a0] pipe_get_pages(): switch to append_pipe()
>> git bisect good 746de1f86fcd33464acac047f111eea877f2f7a0
>> # good: [5d5d353bed32dc3ea52e2619e0d1c60b17133b91] Merge tag 'rproc-v5.20' of git://git.kernel.org/pub/scm/linux/kernel/git/remoteproc/linux
>> git bisect good 5d5d353bed32dc3ea52e2619e0d1c60b17133b91
>> # bad: [f0f6b614f83dbae99d283b7b12ab5dd2e04df979] copy_page_to_iter(): don't split high-order page in case of ITER_PIPE
>> git bisect bad f0f6b614f83dbae99d283b7b12ab5dd2e04df979
>> # bad: [310d9d5a5009a93377200b98daa2d84aa2bd8160] expand those iov_iter_advance()...
>> git bisect bad 310d9d5a5009a93377200b98daa2d84aa2bd8160
>> # first bad commit: [310d9d5a5009a93377200b98daa2d84aa2bd8160] expand those iov_iter_advance()...
> 
> -- 
> Anders Blomdell                  Email: anders.blomdell@control.lth.se
> Department of Automatic Control
> Lund University                  Phone:    +46 46 222 4625
> P.O. Box 118
> SE-221 00 Lund, Sweden

--
Chuck Lever




^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Kernel 6.0.5 breaks virtual machines that boot from nfs mounted qcow2 images
  2022-11-23 10:08 ` Kernel 6.0.5 breaks virtual machines that boot from nfs mounted qcow2 images Anders Blomdell
  2022-11-23 16:11   ` Chuck Lever III
@ 2022-11-23 17:29   ` Al Viro
  2022-11-23 17:49   ` Benjamin Coddington
  2 siblings, 0 replies; 7+ messages in thread
From: Al Viro @ 2022-11-23 17:29 UTC (permalink / raw)
  To: Anders Blomdell
  Cc: linux-nfs, Chuck Lever, Jeff Layton, linux-kernel, Anna Schumaker,
	Trond Myklebust, Benjamin Coddington

On Wed, Nov 23, 2022 at 11:08:42AM +0100, Anders Blomdell wrote:
> Our problems turned out to be a fallout of Al Viros's splice rework, where nfsd reads with non-zero offsets and not ending
> on a page boundary failed to remap the last page. I belive that this is a decent fix for that problem (tested on v6.1-rc6,
> 6.0.7 and 6.0.9)
> 
> ---- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -873,7 +873,7 @@ nfsd_splice_actor(struct pipe_inode_info *pipe, struct pipe_buffer *buf,
>         unsigned offset = buf->offset;
>         page += offset / PAGE_SIZE;
> -       for (int i = sd->len; i > 0; i -= PAGE_SIZE)
> +       for (int i = sd->len + offset % PAGE_SIZE; i > 0; i -= PAGE_SIZE)
>                 svc_rqst_replace_page(rqstp, page++);

Hmm...  A minor nit:
	last_page = page + (offset + sd->len - 1) / PAGE_SIZE;
	for (page += offset / PAGE_SIZE; page <= last_page; page++)
		svc_rqst_replace_page(rqstp, page);
might be easier to follow.  Matter of taste, though...

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Kernel 6.0.5 breaks virtual machines that boot from nfs mounted qcow2 images
  2022-11-23 10:08 ` Kernel 6.0.5 breaks virtual machines that boot from nfs mounted qcow2 images Anders Blomdell
  2022-11-23 16:11   ` Chuck Lever III
  2022-11-23 17:29   ` Al Viro
@ 2022-11-23 17:49   ` Benjamin Coddington
  2022-11-23 17:51     ` Chuck Lever III
  2 siblings, 1 reply; 7+ messages in thread
From: Benjamin Coddington @ 2022-11-23 17:49 UTC (permalink / raw)
  To: Anders Blomdell, Chuck Lever, Jeff Layton, Alexander Viro
  Cc: linux-nfs, linux-kernel, Anna Schumaker, Trond Myklebust

On 23 Nov 2022, at 5:08, Anders Blomdell wrote:

> Our problems turned out to be a fallout of Al Viros's splice rework, where nfsd reads with non-zero offsets and not ending
> on a page boundary failed to remap the last page. I belive that this is a decent fix for that problem (tested on v6.1-rc6,
> 6.0.7 and 6.0.9)
>
> ---- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -873,7 +873,7 @@ nfsd_splice_actor(struct pipe_inode_info *pipe, struct pipe_buffer *buf,
>         unsigned offset = buf->offset;
>          page += offset / PAGE_SIZE;
> -       for (int i = sd->len; i > 0; i -= PAGE_SIZE)
> +       for (int i = sd->len + offset % PAGE_SIZE; i > 0; i -= PAGE_SIZE)
>                 svc_rqst_replace_page(rqstp, page++);
>         if (rqstp->rq_res.page_len == 0)        // first call
>                 rqstp->rq_res.page_base = offset % PAGE_SIZE;


Does anyone have insight into how we could possibly have caught this in testing?

I have been unable to make my knfsd hit this problem: it returns unaligned READs all day long, no problems.

Ben


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Kernel 6.0.5 breaks virtual machines that boot from nfs mounted qcow2 images
  2022-11-23 17:49   ` Benjamin Coddington
@ 2022-11-23 17:51     ` Chuck Lever III
  2022-11-23 17:57       ` Anders Blomdell
  0 siblings, 1 reply; 7+ messages in thread
From: Chuck Lever III @ 2022-11-23 17:51 UTC (permalink / raw)
  To: Benjamin Coddington
  Cc: Anders Blomdell, Jeff Layton, Al Viro, Linux NFS Mailing List,
	linux-kernel@vger.kernel.org, Anna Schumaker, Trond Myklebust


> On Nov 23, 2022, at 12:49 PM, Benjamin Coddington <bcodding@redhat.com> wrote:
> 
> On 23 Nov 2022, at 5:08, Anders Blomdell wrote:
> 
>> Our problems turned out to be a fallout of Al Viros's splice rework, where nfsd reads with non-zero offsets and not ending
>> on a page boundary failed to remap the last page. I belive that this is a decent fix for that problem (tested on v6.1-rc6,
>> 6.0.7 and 6.0.9)
>> 
>> ---- a/fs/nfsd/vfs.c
>> +++ b/fs/nfsd/vfs.c
>> @@ -873,7 +873,7 @@ nfsd_splice_actor(struct pipe_inode_info *pipe, struct pipe_buffer *buf,
>>        unsigned offset = buf->offset;
>>         page += offset / PAGE_SIZE;
>> -       for (int i = sd->len; i > 0; i -= PAGE_SIZE)
>> +       for (int i = sd->len + offset % PAGE_SIZE; i > 0; i -= PAGE_SIZE)
>>                svc_rqst_replace_page(rqstp, page++);
>>        if (rqstp->rq_res.page_len == 0)        // first call
>>                rqstp->rq_res.page_base = offset % PAGE_SIZE;
> 
> 
> Does anyone have insight into how we could possibly have caught this in testing?

Was also wondering this. I had though fstests (via fsx) would have exercised
this usage scenario.


--
Chuck Lever




^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Kernel 6.0.5 breaks virtual machines that boot from nfs mounted qcow2 images
  2022-11-23 17:51     ` Chuck Lever III
@ 2022-11-23 17:57       ` Anders Blomdell
  2022-11-24  9:12         ` Anders Blomdell
  0 siblings, 1 reply; 7+ messages in thread
From: Anders Blomdell @ 2022-11-23 17:57 UTC (permalink / raw)
  To: Chuck Lever III, Benjamin Coddington
  Cc: Jeff Layton, Al Viro, Linux NFS Mailing List,
	linux-kernel@vger.kernel.org, Anna Schumaker, Trond Myklebust



On 2022-11-23 18:51, Chuck Lever III wrote:
> 
>> On Nov 23, 2022, at 12:49 PM, Benjamin Coddington <bcodding@redhat.com> wrote:
>>
>> On 23 Nov 2022, at 5:08, Anders Blomdell wrote:
>>
>>> Our problems turned out to be a fallout of Al Viros's splice rework, where nfsd reads with non-zero offsets and not ending
>>> on a page boundary failed to remap the last page. I belive that this is a decent fix for that problem (tested on v6.1-rc6,
>>> 6.0.7 and 6.0.9)
>>>
>>> ---- a/fs/nfsd/vfs.c
>>> +++ b/fs/nfsd/vfs.c
>>> @@ -873,7 +873,7 @@ nfsd_splice_actor(struct pipe_inode_info *pipe, struct pipe_buffer *buf,
>>>         unsigned offset = buf->offset;
>>>          page += offset / PAGE_SIZE;
>>> -       for (int i = sd->len; i > 0; i -= PAGE_SIZE)
>>> +       for (int i = sd->len + offset % PAGE_SIZE; i > 0; i -= PAGE_SIZE)
>>>                 svc_rqst_replace_page(rqstp, page++);
>>>         if (rqstp->rq_res.page_len == 0)        // first call
>>>                 rqstp->rq_res.page_base = offset % PAGE_SIZE;
>>
>>
>> Does anyone have insight into how we could possibly have caught this in testing?
> 
> Was also wondering this. I had though fstests (via fsx) would have exercised
> this usage scenario.
My guess is that one has to look very hard at qcow2 handling in qemu...

/Anders


-- 
Anders Blomdell                  Email: anders.blomdell@control.lth.se
Department of Automatic Control
Lund University                  Phone:    +46 46 222 4625
P.O. Box 118
SE-221 00 Lund, Sweden

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Kernel 6.0.5 breaks virtual machines that boot from nfs mounted qcow2 images
  2022-11-23 17:57       ` Anders Blomdell
@ 2022-11-24  9:12         ` Anders Blomdell
  0 siblings, 0 replies; 7+ messages in thread
From: Anders Blomdell @ 2022-11-24  9:12 UTC (permalink / raw)
  To: Chuck Lever III, Benjamin Coddington
  Cc: Jeff Layton, Al Viro, Linux NFS Mailing List,
	linux-kernel@vger.kernel.org, Anna Schumaker, Trond Myklebust



On 2022-11-23 18:57, Anders Blomdell wrote:
> 
> 
> On 2022-11-23 18:51, Chuck Lever III wrote:
>>
>>> On Nov 23, 2022, at 12:49 PM, Benjamin Coddington <bcodding@redhat.com> wrote:
>>>
>>> On 23 Nov 2022, at 5:08, Anders Blomdell wrote:
>>>
>>>> Our problems turned out to be a fallout of Al Viros's splice rework, where nfsd reads with non-zero offsets and not ending
>>>> on a page boundary failed to remap the last page. I belive that this is a decent fix for that problem (tested on v6.1-rc6,
>>>> 6.0.7 and 6.0.9)
>>>>
>>>> ---- a/fs/nfsd/vfs.c
>>>> +++ b/fs/nfsd/vfs.c
>>>> @@ -873,7 +873,7 @@ nfsd_splice_actor(struct pipe_inode_info *pipe, struct pipe_buffer *buf,
>>>>         unsigned offset = buf->offset;
>>>>          page += offset / PAGE_SIZE;
>>>> -       for (int i = sd->len; i > 0; i -= PAGE_SIZE)
>>>> +       for (int i = sd->len + offset % PAGE_SIZE; i > 0; i -= PAGE_SIZE)
>>>>                 svc_rqst_replace_page(rqstp, page++);
>>>>         if (rqstp->rq_res.page_len == 0)        // first call
>>>>                 rqstp->rq_res.page_base = offset % PAGE_SIZE;
>>>
>>>
>>> Does anyone have insight into how we could possibly have caught this in testing?
>>
>> Was also wondering this. I had though fstests (via fsx) would have exercised
>> this usage scenario.
> My guess is that one has to look very hard at qcow2 handling in qemu...
aio_read seems to trigger the problem, but there is a lot of buffering going on that I don't understand
(e.g even short aio_reads leads to 16384 bytes read)

/Anders
-- 
Anders Blomdell                  Email: anders.blomdell@control.lth.se
Department of Automatic Control
Lund University                  Phone:    +46 46 222 4625
P.O. Box 118
SE-221 00 Lund, Sweden

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-11-24  9:14 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <65115206-ec17-443e-8495-91661a2fd9be@control.lth.se>
2022-11-23 10:08 ` Kernel 6.0.5 breaks virtual machines that boot from nfs mounted qcow2 images Anders Blomdell
2022-11-23 16:11   ` Chuck Lever III
2022-11-23 17:29   ` Al Viro
2022-11-23 17:49   ` Benjamin Coddington
2022-11-23 17:51     ` Chuck Lever III
2022-11-23 17:57       ` Anders Blomdell
2022-11-24  9:12         ` Anders Blomdell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox