warning for EOPNOTSUPP vfs_copy_file_range

warning for EOPNOTSUPP vfs_copy_file_range

[He Zhe]

Hi,

We are experiencing the following warning from
"WARN_ON_ONCE(ret == -EOPNOTSUPP);" in vfs_copy_file_range, from
64bf5ff58dff ("vfs: no fallback for ->copy_file_range")

# cat /sys/class/net/can0/phys_switch_id

WARNING: CPU: 7 PID: 673 at fs/read_write.c:1516 vfs_copy_file_range+0x380/0x440
Modules linked in: llce_can llce_logger llce_mailbox llce_core sch_fq_codel
openvswitch nsh nf_conncount nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4
CPU: 7 PID: 673 Comm: cat Not tainted 5.15.38-yocto-standard #1
Hardware name: Freescale S32G399A (DT)
pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : vfs_copy_file_range+0x380/0x440
lr : vfs_copy_file_range+0x16c/0x440
sp : ffffffc00e0f3ce0
x29: ffffffc00e0f3ce0 x28: ffffff88157b5a40 x27: 0000000000000000
x26: ffffff8816ac3230 x25: ffffff881c060008 x24: 0000000000001000
x23: 0000000000000000 x22: 0000000000000000 x21: ffffff881cc99540
x20: ffffff881cc9a340 x19: ffffffffffffffa1 x18: ffffffffffffffff
x17: 0000000000000001 x16: 0000adfbb5178cde x15: ffffffc08e0f3647
x14: 0000000000000000 x13: 34613178302f3061 x12: 3178302b636e7973
x11: 0000000000058395 x10: 00000000fd1c5755 x9 : ffffffc008361950
x8 : ffffffc00a7d4d58 x7 : 0000000000000000 x6 : 0000000000000001
x5 : ffffffc009e81000 x4 : ffffffc009e817f8 x3 : 0000000000000000
x2 : 0000000000000000 x1 : ffffff88157b5a40 x0 : ffffffffffffffa1
Call trace:
 vfs_copy_file_range+0x380/0x440
 __do_sys_copy_file_range+0x178/0x3a4
 __arm64_sys_copy_file_range+0x34/0x4c
 invoke_syscall+0x5c/0x130
 el0_svc_common.constprop.0+0x68/0x124
 do_el0_svc+0x50/0xbc
 el0_svc+0x54/0x130
 el0t_64_sync_handler+0xa4/0x130
 el0t_64_sync+0x1a0/0x1a4
cat: /sys/class/net/can0/phys_switch_id: Operation not supported

And we found this is triggered by the following stack. Specifically, all
netdev_ops in CAN drivers we can find now do not have ndo_get_port_parent_id and
ndo_get_devlink_port, which makes phys_switch_id_show return -EOPNOTSUPP all the
way back to vfs_copy_file_range.

phys_switch_id_show+0xf4/0x11c
dev_attr_show+0x2c/0x6c
sysfs_kf_seq_show+0xb8/0x150
kernfs_seq_show+0x38/0x44
seq_read_iter+0x1c4/0x4c0
kernfs_fop_read_iter+0x44/0x50
generic_file_splice_read+0xdc/0x190
do_splice_to+0xa0/0xfc
splice_direct_to_actor+0xc4/0x250
do_splice_direct+0x94/0xe0
vfs_copy_file_range+0x16c/0x440
__do_sys_copy_file_range+0x178/0x3a4
__arm64_sys_copy_file_range+0x34/0x4c
invoke_syscall+0x5c/0x130
el0_svc_common.constprop.0+0x68/0x124
do_el0_svc+0x50/0xbc
el0_svc+0x54/0x130
el0t_64_sync_handler+0xa4/0x130
el0t_64_sync+0x1a0/0x1a4

According to the original commit log, this warning is for operational validity
checks to generic_copy_file_range(). The reading will eventually return as
not supported as printed above. But is this warning still necessary? If so we
might want to remove it to have a cleaner dmesg.


Thanks,
Zhe

Re: warning for EOPNOTSUPP vfs_copy_file_range

[Amir Goldstein]

On Thu, May 19, 2022 at 11:22 AM He Zhe <zhe.he@windriver.com> wrote:
>
> Hi,
>
> We are experiencing the following warning from
> "WARN_ON_ONCE(ret == -EOPNOTSUPP);" in vfs_copy_file_range, from
> 64bf5ff58dff ("vfs: no fallback for ->copy_file_range")
>
> # cat /sys/class/net/can0/phys_switch_id
>
> WARNING: CPU: 7 PID: 673 at fs/read_write.c:1516 vfs_copy_file_range+0x380/0x440
> Modules linked in: llce_can llce_logger llce_mailbox llce_core sch_fq_codel
> openvswitch nsh nf_conncount nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4
> CPU: 7 PID: 673 Comm: cat Not tainted 5.15.38-yocto-standard #1
> Hardware name: Freescale S32G399A (DT)
> pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : vfs_copy_file_range+0x380/0x440
> lr : vfs_copy_file_range+0x16c/0x440
> sp : ffffffc00e0f3ce0
> x29: ffffffc00e0f3ce0 x28: ffffff88157b5a40 x27: 0000000000000000
> x26: ffffff8816ac3230 x25: ffffff881c060008 x24: 0000000000001000
> x23: 0000000000000000 x22: 0000000000000000 x21: ffffff881cc99540
> x20: ffffff881cc9a340 x19: ffffffffffffffa1 x18: ffffffffffffffff
> x17: 0000000000000001 x16: 0000adfbb5178cde x15: ffffffc08e0f3647
> x14: 0000000000000000 x13: 34613178302f3061 x12: 3178302b636e7973
> x11: 0000000000058395 x10: 00000000fd1c5755 x9 : ffffffc008361950
> x8 : ffffffc00a7d4d58 x7 : 0000000000000000 x6 : 0000000000000001
> x5 : ffffffc009e81000 x4 : ffffffc009e817f8 x3 : 0000000000000000
> x2 : 0000000000000000 x1 : ffffff88157b5a40 x0 : ffffffffffffffa1
> Call trace:
>  vfs_copy_file_range+0x380/0x440
>  __do_sys_copy_file_range+0x178/0x3a4
>  __arm64_sys_copy_file_range+0x34/0x4c
>  invoke_syscall+0x5c/0x130
>  el0_svc_common.constprop.0+0x68/0x124
>  do_el0_svc+0x50/0xbc
>  el0_svc+0x54/0x130
>  el0t_64_sync_handler+0xa4/0x130
>  el0t_64_sync+0x1a0/0x1a4
> cat: /sys/class/net/can0/phys_switch_id: Operation not supported
>
> And we found this is triggered by the following stack. Specifically, all
> netdev_ops in CAN drivers we can find now do not have ndo_get_port_parent_id and
> ndo_get_devlink_port, which makes phys_switch_id_show return -EOPNOTSUPP all the
> way back to vfs_copy_file_range.
>
> phys_switch_id_show+0xf4/0x11c
> dev_attr_show+0x2c/0x6c
> sysfs_kf_seq_show+0xb8/0x150
> kernfs_seq_show+0x38/0x44
> seq_read_iter+0x1c4/0x4c0
> kernfs_fop_read_iter+0x44/0x50
> generic_file_splice_read+0xdc/0x190
> do_splice_to+0xa0/0xfc
> splice_direct_to_actor+0xc4/0x250
> do_splice_direct+0x94/0xe0
> vfs_copy_file_range+0x16c/0x440
> __do_sys_copy_file_range+0x178/0x3a4
> __arm64_sys_copy_file_range+0x34/0x4c
> invoke_syscall+0x5c/0x130
> el0_svc_common.constprop.0+0x68/0x124
> do_el0_svc+0x50/0xbc
> el0_svc+0x54/0x130
> el0t_64_sync_handler+0xa4/0x130
> el0t_64_sync+0x1a0/0x1a4
>
> According to the original commit log, this warning is for operational validity
> checks to generic_copy_file_range(). The reading will eventually return as
> not supported as printed above. But is this warning still necessary? If so we
> might want to remove it to have a cleaner dmesg.
>

Sigh! Those filesystems have no business doing copy_file_range()

Here is a patch that Luis has been trying to push last year
to fix a problem with copy_file_range() from tracefs:

https://lore.kernel.org/linux-fsdevel/20210702090012.28458-1-lhenriques@suse.de/

Luis gave up on it, because no maintainer stepped up to take
the patch, but I think that is the right way to go.

Maybe this bug report can raise awareness to that old patch.

Al, could you have a look?

Thanks,
Amir.

Re: warning for EOPNOTSUPP vfs_copy_file_range

[Luís Henriques]

Amir Goldstein <amir73il@gmail.com> writes:

> On Thu, May 19, 2022 at 11:22 AM He Zhe <zhe.he@windriver.com> wrote:
>>
>> Hi,
>>
>> We are experiencing the following warning from
>> "WARN_ON_ONCE(ret == -EOPNOTSUPP);" in vfs_copy_file_range, from
>> 64bf5ff58dff ("vfs: no fallback for ->copy_file_range")
>>
>> # cat /sys/class/net/can0/phys_switch_id
>>
>> WARNING: CPU: 7 PID: 673 at fs/read_write.c:1516 vfs_copy_file_range+0x380/0x440
>> Modules linked in: llce_can llce_logger llce_mailbox llce_core sch_fq_codel
>> openvswitch nsh nf_conncount nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4
>> CPU: 7 PID: 673 Comm: cat Not tainted 5.15.38-yocto-standard #1
>> Hardware name: Freescale S32G399A (DT)
>> pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>> pc : vfs_copy_file_range+0x380/0x440
>> lr : vfs_copy_file_range+0x16c/0x440
>> sp : ffffffc00e0f3ce0
>> x29: ffffffc00e0f3ce0 x28: ffffff88157b5a40 x27: 0000000000000000
>> x26: ffffff8816ac3230 x25: ffffff881c060008 x24: 0000000000001000
>> x23: 0000000000000000 x22: 0000000000000000 x21: ffffff881cc99540
>> x20: ffffff881cc9a340 x19: ffffffffffffffa1 x18: ffffffffffffffff
>> x17: 0000000000000001 x16: 0000adfbb5178cde x15: ffffffc08e0f3647
>> x14: 0000000000000000 x13: 34613178302f3061 x12: 3178302b636e7973
>> x11: 0000000000058395 x10: 00000000fd1c5755 x9 : ffffffc008361950
>> x8 : ffffffc00a7d4d58 x7 : 0000000000000000 x6 : 0000000000000001
>> x5 : ffffffc009e81000 x4 : ffffffc009e817f8 x3 : 0000000000000000
>> x2 : 0000000000000000 x1 : ffffff88157b5a40 x0 : ffffffffffffffa1
>> Call trace:
>>  vfs_copy_file_range+0x380/0x440
>>  __do_sys_copy_file_range+0x178/0x3a4
>>  __arm64_sys_copy_file_range+0x34/0x4c
>>  invoke_syscall+0x5c/0x130
>>  el0_svc_common.constprop.0+0x68/0x124
>>  do_el0_svc+0x50/0xbc
>>  el0_svc+0x54/0x130
>>  el0t_64_sync_handler+0xa4/0x130
>>  el0t_64_sync+0x1a0/0x1a4
>> cat: /sys/class/net/can0/phys_switch_id: Operation not supported
>>
>> And we found this is triggered by the following stack. Specifically, all
>> netdev_ops in CAN drivers we can find now do not have ndo_get_port_parent_id and
>> ndo_get_devlink_port, which makes phys_switch_id_show return -EOPNOTSUPP all the
>> way back to vfs_copy_file_range.
>>
>> phys_switch_id_show+0xf4/0x11c
>> dev_attr_show+0x2c/0x6c
>> sysfs_kf_seq_show+0xb8/0x150
>> kernfs_seq_show+0x38/0x44
>> seq_read_iter+0x1c4/0x4c0
>> kernfs_fop_read_iter+0x44/0x50
>> generic_file_splice_read+0xdc/0x190
>> do_splice_to+0xa0/0xfc
>> splice_direct_to_actor+0xc4/0x250
>> do_splice_direct+0x94/0xe0
>> vfs_copy_file_range+0x16c/0x440
>> __do_sys_copy_file_range+0x178/0x3a4
>> __arm64_sys_copy_file_range+0x34/0x4c
>> invoke_syscall+0x5c/0x130
>> el0_svc_common.constprop.0+0x68/0x124
>> do_el0_svc+0x50/0xbc
>> el0_svc+0x54/0x130
>> el0t_64_sync_handler+0xa4/0x130
>> el0t_64_sync+0x1a0/0x1a4
>>
>> According to the original commit log, this warning is for operational validity
>> checks to generic_copy_file_range(). The reading will eventually return as
>> not supported as printed above. But is this warning still necessary? If so we
>> might want to remove it to have a cleaner dmesg.
>>
>
> Sigh! Those filesystems have no business doing copy_file_range()
>
> Here is a patch that Luis has been trying to push last year
> to fix a problem with copy_file_range() from tracefs:
>
> https://lore.kernel.org/linux-fsdevel/20210702090012.28458-1-lhenriques@suse.de/

Yikes!  It's been a while and I completely forgot about it.  I can
definitely try to respin this patch if someone's interested in picking
it.  I'll have to go re-read everything again and see what's missing and
what has changed in between.

Cheers,
-- 
Luís

> Luis gave up on it, because no maintainer stepped up to take
> the patch, but I think that is the right way to go.
>
> Maybe this bug report can raise awareness to that old patch.
>
> Al, could you have a look?
>
> Thanks,
> Amir.
>

Re: warning for EOPNOTSUPP vfs_copy_file_range

[He Zhe]

On 5/19/22 22:31, Luís Henriques wrote:
> Amir Goldstein <amir73il@gmail.com> writes:
>
>> On Thu, May 19, 2022 at 11:22 AM He Zhe <zhe.he@windriver.com> wrote:
>>> Hi,
>>>
>>> We are experiencing the following warning from
>>> "WARN_ON_ONCE(ret == -EOPNOTSUPP);" in vfs_copy_file_range, from
>>> 64bf5ff58dff ("vfs: no fallback for ->copy_file_range")
>>>
>>> # cat /sys/class/net/can0/phys_switch_id
>>>
>>> WARNING: CPU: 7 PID: 673 at fs/read_write.c:1516 vfs_copy_file_range+0x380/0x440
>>> Modules linked in: llce_can llce_logger llce_mailbox llce_core sch_fq_codel
>>> openvswitch nsh nf_conncount nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4
>>> CPU: 7 PID: 673 Comm: cat Not tainted 5.15.38-yocto-standard #1
>>> Hardware name: Freescale S32G399A (DT)
>>> pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>>> pc : vfs_copy_file_range+0x380/0x440
>>> lr : vfs_copy_file_range+0x16c/0x440
>>> sp : ffffffc00e0f3ce0
>>> x29: ffffffc00e0f3ce0 x28: ffffff88157b5a40 x27: 0000000000000000
>>> x26: ffffff8816ac3230 x25: ffffff881c060008 x24: 0000000000001000
>>> x23: 0000000000000000 x22: 0000000000000000 x21: ffffff881cc99540
>>> x20: ffffff881cc9a340 x19: ffffffffffffffa1 x18: ffffffffffffffff
>>> x17: 0000000000000001 x16: 0000adfbb5178cde x15: ffffffc08e0f3647
>>> x14: 0000000000000000 x13: 34613178302f3061 x12: 3178302b636e7973
>>> x11: 0000000000058395 x10: 00000000fd1c5755 x9 : ffffffc008361950
>>> x8 : ffffffc00a7d4d58 x7 : 0000000000000000 x6 : 0000000000000001
>>> x5 : ffffffc009e81000 x4 : ffffffc009e817f8 x3 : 0000000000000000
>>> x2 : 0000000000000000 x1 : ffffff88157b5a40 x0 : ffffffffffffffa1
>>> Call trace:
>>>  vfs_copy_file_range+0x380/0x440
>>>  __do_sys_copy_file_range+0x178/0x3a4
>>>  __arm64_sys_copy_file_range+0x34/0x4c
>>>  invoke_syscall+0x5c/0x130
>>>  el0_svc_common.constprop.0+0x68/0x124
>>>  do_el0_svc+0x50/0xbc
>>>  el0_svc+0x54/0x130
>>>  el0t_64_sync_handler+0xa4/0x130
>>>  el0t_64_sync+0x1a0/0x1a4
>>> cat: /sys/class/net/can0/phys_switch_id: Operation not supported
>>>
>>> And we found this is triggered by the following stack. Specifically, all
>>> netdev_ops in CAN drivers we can find now do not have ndo_get_port_parent_id and
>>> ndo_get_devlink_port, which makes phys_switch_id_show return -EOPNOTSUPP all the
>>> way back to vfs_copy_file_range.
>>>
>>> phys_switch_id_show+0xf4/0x11c
>>> dev_attr_show+0x2c/0x6c
>>> sysfs_kf_seq_show+0xb8/0x150
>>> kernfs_seq_show+0x38/0x44
>>> seq_read_iter+0x1c4/0x4c0
>>> kernfs_fop_read_iter+0x44/0x50
>>> generic_file_splice_read+0xdc/0x190
>>> do_splice_to+0xa0/0xfc
>>> splice_direct_to_actor+0xc4/0x250
>>> do_splice_direct+0x94/0xe0
>>> vfs_copy_file_range+0x16c/0x440
>>> __do_sys_copy_file_range+0x178/0x3a4
>>> __arm64_sys_copy_file_range+0x34/0x4c
>>> invoke_syscall+0x5c/0x130
>>> el0_svc_common.constprop.0+0x68/0x124
>>> do_el0_svc+0x50/0xbc
>>> el0_svc+0x54/0x130
>>> el0t_64_sync_handler+0xa4/0x130
>>> el0t_64_sync+0x1a0/0x1a4
>>>
>>> According to the original commit log, this warning is for operational validity
>>> checks to generic_copy_file_range(). The reading will eventually return as
>>> not supported as printed above. But is this warning still necessary? If so we
>>> might want to remove it to have a cleaner dmesg.
>>>
>> Sigh! Those filesystems have no business doing copy_file_range()
>>
>> Here is a patch that Luis has been trying to push last year
>> to fix a problem with copy_file_range() from tracefs:
>>
>> https://lore.kernel.org/linux-fsdevel/20210702090012.28458-1-lhenriques@suse.de/
> Yikes!  It's been a while and I completely forgot about it.  I can
> definitely try to respin this patch if someone's interested in picking
> it.  I'll have to go re-read everything again and see what's missing and
> what has changed in between.

Thank you both for quick replies.

It would be good if this could be sorted out, as folks who are not familiar with
it might be confused by the call trace. But if this is supposed to cost a long
time, maybe we can first solve the false positive warning for the drivers in this
case, as it seems the "operational validity checks" was not for these drivers.

Regards,
Zhe

>
> Cheers,

Re: warning for EOPNOTSUPP vfs_copy_file_range

[Paul Gortmaker]

[Re: warning for EOPNOTSUPP vfs_copy_file_range] On 19/05/2022 (Thu 15:31) Luís Henriques wrote:

> Amir Goldstein <amir73il@gmail.com> writes:
> 
> > On Thu, May 19, 2022 at 11:22 AM He Zhe <zhe.he@windriver.com> wrote:
> >>
> >> Hi,
> >>
> >> We are experiencing the following warning from
> >> "WARN_ON_ONCE(ret == -EOPNOTSUPP);" in vfs_copy_file_range, from
> >> 64bf5ff58dff ("vfs: no fallback for ->copy_file_range")
> >>
> >> # cat /sys/class/net/can0/phys_switch_id
> >>
> >> WARNING: CPU: 7 PID: 673 at fs/read_write.c:1516 vfs_copy_file_range+0x380/0x440

[...]

> > Sigh! Those filesystems have no business doing copy_file_range()
> >
> > Here is a patch that Luis has been trying to push last year
> > to fix a problem with copy_file_range() from tracefs:
> >
> > https://lore.kernel.org/linux-fsdevel/20210702090012.28458-1-lhenriques@suse.de/
> 
> Yikes!  It's been a while and I completely forgot about it.  I can
> definitely try to respin this patch if someone's interested in picking
> it.  I'll have to go re-read everything again and see what's missing and
> what has changed in between.

If it helps any, you don't need CAN bus or anything complicated.  I was
able to reproduce it with the loopback device on linux-next of today
with a new userspace (from Yocto) that actively uses copy_file_range()

Note that a cp or redirect is needed to trigger the copy - with the
new userspace requiremet.  Or write your own reproducer that goes at the
syscall directly on an old userspace (untested by me).

  root@qemux86-64:/sys/class/net/lo# cat phys_switch_id
  cat: phys_switch_id: Operation not supported
  root@qemux86-64:/sys/class/net/lo# cat phys_switch_id > /tmp/foo
  [   87.527506] ------------[ cut here ]------------
  [   87.527675] WARNING: CPU: 2 PID: 238 at /home/paul/git/linux-head/fs/read_write.c:1511 vfs_copy_file_range+0x47c/0x4e0
  
Paul.
--

> 
> Cheers,
> -- 
> Luís
> 
> > Luis gave up on it, because no maintainer stepped up to take
> > the patch, but I think that is the right way to go.
> >
> > Maybe this bug report can raise awareness to that old patch.
> >
> > Al, could you have a look?
> >
> > Thanks,
> > Amir.
> >

Re: warning for EOPNOTSUPP vfs_copy_file_range

[Al Viro]

On Thu, May 19, 2022 at 04:53:15PM +0300, Amir Goldstein wrote:

> Luis gave up on it, because no maintainer stepped up to take
> the patch, but I think that is the right way to go.
> 
> Maybe this bug report can raise awareness to that old patch.
> 
> Al, could you have a look?

IIRC, you had objections to that variant back then...

Re: warning for EOPNOTSUPP vfs_copy_file_range

[Amir Goldstein]

On Fri, May 20, 2022 at 6:56 AM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> On Thu, May 19, 2022 at 04:53:15PM +0300, Amir Goldstein wrote:
>
> > Luis gave up on it, because no maintainer stepped up to take
> > the patch, but I think that is the right way to go.
> >
> > Maybe this bug report can raise awareness to that old patch.
> >
> > Al, could you have a look?
>
> IIRC, you had objections to that variant back then...

Right. But not about the "main" patch.
The patch had an "also" part:

The short-circuit code for the case where the copy length is zero has also
been dropped from the VFS code.  This is because a zero size copy between
two files shall provide a clear indication on whether or not the
filesystem supports non-zero copies.

-     if (len == 0)
-         return 0;
-

Which would have been a regression for nfs client, because
nfs protocol treats length 0 from ->copy_file_range() as "copy everything":

https://lore.kernel.org/linux-fsdevel/CAOQ4uxgwcNwWEqYKBg3fMHD3aXOsYUmPeexBe9EVP9Nb53b-Hw@mail.gmail.com/

This api impedance should be fixed in the nfs client, but I'm
not sure if that was already done.

I will test and re-post Luis' patch without removing the short-circuit
unless Luis gets to it first.

BTW, IIRC, there were already LTP tests and man page fixes posted for
the copt_file_range() behavior change.

Thanks,
Amir.

Re: warning for EOPNOTSUPP vfs_copy_file_range

[Amir Goldstein]

On Fri, May 20, 2022 at 6:03 AM He Zhe <zhe.he@windriver.com> wrote:
>
>
>
> On 5/19/22 22:31, Luís Henriques wrote:
> > Amir Goldstein <amir73il@gmail.com> writes:
> >
> >> On Thu, May 19, 2022 at 11:22 AM He Zhe <zhe.he@windriver.com> wrote:
> >>> Hi,
> >>>
> >>> We are experiencing the following warning from
> >>> "WARN_ON_ONCE(ret == -EOPNOTSUPP);" in vfs_copy_file_range, from
> >>> 64bf5ff58dff ("vfs: no fallback for ->copy_file_range")
> >>>
> >>> # cat /sys/class/net/can0/phys_switch_id
> >>>
> >>> WARNING: CPU: 7 PID: 673 at fs/read_write.c:1516 vfs_copy_file_range+0x380/0x440
> >>> Modules linked in: llce_can llce_logger llce_mailbox llce_core sch_fq_codel
> >>> openvswitch nsh nf_conncount nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4
> >>> CPU: 7 PID: 673 Comm: cat Not tainted 5.15.38-yocto-standard #1
> >>> Hardware name: Freescale S32G399A (DT)
> >>> pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> >>> pc : vfs_copy_file_range+0x380/0x440
> >>> lr : vfs_copy_file_range+0x16c/0x440
> >>> sp : ffffffc00e0f3ce0
> >>> x29: ffffffc00e0f3ce0 x28: ffffff88157b5a40 x27: 0000000000000000
> >>> x26: ffffff8816ac3230 x25: ffffff881c060008 x24: 0000000000001000
> >>> x23: 0000000000000000 x22: 0000000000000000 x21: ffffff881cc99540
> >>> x20: ffffff881cc9a340 x19: ffffffffffffffa1 x18: ffffffffffffffff
> >>> x17: 0000000000000001 x16: 0000adfbb5178cde x15: ffffffc08e0f3647
> >>> x14: 0000000000000000 x13: 34613178302f3061 x12: 3178302b636e7973
> >>> x11: 0000000000058395 x10: 00000000fd1c5755 x9 : ffffffc008361950
> >>> x8 : ffffffc00a7d4d58 x7 : 0000000000000000 x6 : 0000000000000001
> >>> x5 : ffffffc009e81000 x4 : ffffffc009e817f8 x3 : 0000000000000000
> >>> x2 : 0000000000000000 x1 : ffffff88157b5a40 x0 : ffffffffffffffa1
> >>> Call trace:
> >>>  vfs_copy_file_range+0x380/0x440
> >>>  __do_sys_copy_file_range+0x178/0x3a4
> >>>  __arm64_sys_copy_file_range+0x34/0x4c
> >>>  invoke_syscall+0x5c/0x130
> >>>  el0_svc_common.constprop.0+0x68/0x124
> >>>  do_el0_svc+0x50/0xbc
> >>>  el0_svc+0x54/0x130
> >>>  el0t_64_sync_handler+0xa4/0x130
> >>>  el0t_64_sync+0x1a0/0x1a4
> >>> cat: /sys/class/net/can0/phys_switch_id: Operation not supported
> >>>
> >>> And we found this is triggered by the following stack. Specifically, all
> >>> netdev_ops in CAN drivers we can find now do not have ndo_get_port_parent_id and
> >>> ndo_get_devlink_port, which makes phys_switch_id_show return -EOPNOTSUPP all the
> >>> way back to vfs_copy_file_range.
> >>>
> >>> phys_switch_id_show+0xf4/0x11c
> >>> dev_attr_show+0x2c/0x6c
> >>> sysfs_kf_seq_show+0xb8/0x150
> >>> kernfs_seq_show+0x38/0x44
> >>> seq_read_iter+0x1c4/0x4c0
> >>> kernfs_fop_read_iter+0x44/0x50
> >>> generic_file_splice_read+0xdc/0x190
> >>> do_splice_to+0xa0/0xfc
> >>> splice_direct_to_actor+0xc4/0x250
> >>> do_splice_direct+0x94/0xe0
> >>> vfs_copy_file_range+0x16c/0x440
> >>> __do_sys_copy_file_range+0x178/0x3a4
> >>> __arm64_sys_copy_file_range+0x34/0x4c
> >>> invoke_syscall+0x5c/0x130
> >>> el0_svc_common.constprop.0+0x68/0x124
> >>> do_el0_svc+0x50/0xbc
> >>> el0_svc+0x54/0x130
> >>> el0t_64_sync_handler+0xa4/0x130
> >>> el0t_64_sync+0x1a0/0x1a4
> >>>
> >>> According to the original commit log, this warning is for operational validity
> >>> checks to generic_copy_file_range(). The reading will eventually return as
> >>> not supported as printed above. But is this warning still necessary? If so we
> >>> might want to remove it to have a cleaner dmesg.
> >>>
> >> Sigh! Those filesystems have no business doing copy_file_range()
> >>
> >> Here is a patch that Luis has been trying to push last year
> >> to fix a problem with copy_file_range() from tracefs:
> >>
> >> https://lore.kernel.org/linux-fsdevel/20210702090012.28458-1-lhenriques@suse.de/
> > Yikes!  It's been a while and I completely forgot about it.  I can
> > definitely try to respin this patch if someone's interested in picking
> > it.  I'll have to go re-read everything again and see what's missing and
> > what has changed in between.
>
> Thank you both for quick replies.
>
> It would be good if this could be sorted out, as folks who are not familiar with
> it might be confused by the call trace. But if this is supposed to cost a long
> time, maybe we can first solve the false positive warning for the drivers in this
> case, as it seems the "operational validity checks" was not for these drivers.
>

Yes, technically, you are right.
Userspace should not be able to trigger a code validity assertion.
But the reason that assertion is there is to warn us developers
if we had overlooked a logic case and IMO we did.
The entire concept of calling ->copy_file_range() on random
filesystems has more than one problem and I would like for the kernel to
stop doing that.

Thanks,
Amir.

Re: warning for EOPNOTSUPP vfs_copy_file_range

[Amir Goldstein]

On Fri, May 20, 2022 at 7:39 AM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Fri, May 20, 2022 at 6:56 AM Al Viro <viro@zeniv.linux.org.uk> wrote:
> >
> > On Thu, May 19, 2022 at 04:53:15PM +0300, Amir Goldstein wrote:
> >
> > > Luis gave up on it, because no maintainer stepped up to take
> > > the patch, but I think that is the right way to go.
> > >
> > > Maybe this bug report can raise awareness to that old patch.
> > >
> > > Al, could you have a look?
> >
> > IIRC, you had objections to that variant back then...
>
> Right. But not about the "main" patch.
> The patch had an "also" part:
>
> The short-circuit code for the case where the copy length is zero has also
> been dropped from the VFS code.  This is because a zero size copy between
> two files shall provide a clear indication on whether or not the
> filesystem supports non-zero copies.
>
> -     if (len == 0)
> -         return 0;
> -
>
> Which would have been a regression for nfs client, because
> nfs protocol treats length 0 from ->copy_file_range() as "copy everything":
>
> https://lore.kernel.org/linux-fsdevel/CAOQ4uxgwcNwWEqYKBg3fMHD3aXOsYUmPeexBe9EVP9Nb53b-Hw@mail.gmail.com/
>
> This api impedance should be fixed in the nfs client, but I'm
> not sure if that was already done.
>
> I will test and re-post Luis' patch without removing the short-circuit
> unless Luis gets to it first.
>

Urgh! That old patch passes the fstests -g copy_range group
on nfs, but fails almost all of them on xfs/btrfs.

The reason is that when we allow to perform copy_range
with remap_file_range() it fails for sizes smaller than block size
and returning short read of 0 from copy_range is not an option.

So what I am going to do is to keep the basic restriction in this patch of:
"copy_range allowed for fs that implement either ->copy_file_range()
or ->remap_file_range() (for same sb copy)"

But will change the logic of:
"try clone and then copy then fail"
to:
- try ->copy_file_range()
- try ->remap_file_range()
- fall back to kernel copy

Patch coming shortly.

Thanks,
Amir.