* Latest version of zero-copy fix
@ 2014-02-10 23:21 David Miller
2014-02-11 1:31 ` Richard Yao
0 siblings, 1 reply; 3+ messages in thread
From: David Miller @ 2014-02-10 23:21 UTC (permalink / raw)
To: ryao; +Cc: netdev
So now the patch only tests is_vmalloc_addr(), did you test this
version with the situation that triggers the given backtrace?
[<ffffffff814878ce>] p9_virtio_zc_request+0x45e/0x510
[<ffffffff814814ed>] p9_client_zc_rpc.constprop.16+0xfd/0x4f0
[<ffffffff814839dd>] p9_client_read+0x15d/0x240
[<ffffffff811c8440>] v9fs_fid_readn+0x50/0xa0
[<ffffffff811c84a0>] v9fs_file_readn+0x10/0x20
[<ffffffff811c84e7>] v9fs_file_read+0x37/0x70
[<ffffffff8114e3fb>] vfs_read+0x9b/0x160
[<ffffffff81153571>] kernel_read+0x41/0x60
[<ffffffff810c83ab>] copy_module_from_fd.isra.34+0xfb/0x180
This is reading from v9fs into a module address.
It's going generate the same backtrace with your fix.
I don't think the situation was sufficiently explained to Linus. In
fact, I didn't see the above backtrace mentioned at all.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: Latest version of zero-copy fix
2014-02-10 23:21 Latest version of zero-copy fix David Miller
@ 2014-02-11 1:31 ` Richard Yao
2014-02-11 1:48 ` David Miller
0 siblings, 1 reply; 3+ messages in thread
From: Richard Yao @ 2014-02-11 1:31 UTC (permalink / raw)
To: David Miller; +Cc: netdev
[-- Attachment #1: Type: text/plain, Size: 2137 bytes --]
On 02/10/2014 06:21 PM, David Miller wrote:
>
> So now the patch only tests is_vmalloc_addr(), did you test this
> version with the situation that triggers the given backtrace?
>
> [<ffffffff814878ce>] p9_virtio_zc_request+0x45e/0x510
> [<ffffffff814814ed>] p9_client_zc_rpc.constprop.16+0xfd/0x4f0
> [<ffffffff814839dd>] p9_client_read+0x15d/0x240
> [<ffffffff811c8440>] v9fs_fid_readn+0x50/0xa0
> [<ffffffff811c84a0>] v9fs_file_readn+0x10/0x20
> [<ffffffff811c84e7>] v9fs_file_read+0x37/0x70
> [<ffffffff8114e3fb>] vfs_read+0x9b/0x160
> [<ffffffff81153571>] kernel_read+0x41/0x60
> [<ffffffff810c83ab>] copy_module_from_fd.isra.34+0xfb/0x180
>
> This is reading from v9fs into a module address.
>
> It's going generate the same backtrace with your fix.
>
> I don't think the situation was sufficiently explained to Linus. In
> fact, I didn't see the above backtrace mentioned at all.
>
I have tested it against Linux 3.13.0 and I can confirm that it solves
the problem. In fact, this was the original patch in December before I
made the last minute change to is_module_or_vmalloc_addr() only after
deciding that is_vmalloc_addr() was not general enough.
Linus' response had two key points. Linus' first point was that my
generalization of is_vmalloc_addr() to is_module_or_vmalloc_addr() was
unnecessary to fix this problem because the order of events is as follows:
1. We allocate a temporary buffer with vmalloc().
2. We read the module into that buffer.
3. We copy the module from that buffer into its final location.
Linus' second point was that my generalization permitted IO to be done
to a region of memory where IO operations should never be done. When I
wrote this back in December, I wanted to write as general a fix as
possible because Alexander Graf had pointed out to me that there had
Will Deacon had written a patch that should have included this, but
neglected it and I was afraid that that would become a cycle.
Linus' clear explanation of why this is a bad idea changed my mind, so I
submitted the original form of this patch with an updated commit message.
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-02-11 1:48 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-02-10 23:21 Latest version of zero-copy fix David Miller
2014-02-11 1:31 ` Richard Yao
2014-02-11 1:48 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox