Re: git: c98a764c681f - main - cp(1): fix performance issue for large non-sparse file copies

On Mon, Apr 10, 2023 at 11:31 AM Alexey Dokuchaev <danfe@freebsd.org> wrote:
>
> On Mon, Apr 10, 2023 at 08:31:28AM -0700, Rick Macklem wrote:
> > On Mon, Apr 10, 2023 at 12:54???AM Alexey Dokuchaev wrote:
> > > On Sun, Jan 03, 2021 at 01:02:18AM +0000, Rick Macklem wrote:
> > > > commit c98a764c681f8b70812a9f13a6e61c96aa1a69d2
> > > >
> > > >   cp(1): fix performance issue for large non-sparse file copies
> > > >
> > > > @@ -236,7 +235,7 @@ copy_file(const FTSENT *entp, int dne)
> > > >               do {
> > > >               if (use_copy_file_range) {
> > > >                       rcount = copy_file_range(from_fd, NULL,
> > > > -                         to_fd, NULL, bufsize, 0);
> > > > +                         to_fd, NULL, SSIZE_MAX, 0);
> > >
> > > Hi Rick,
> > >
> > > This change unfortunately breaks copying files in resource-limited
> > > environments (e.g. many port builders do that to prevent runaway
> > > processes):
> > >
> > > ulimit -f 16384000
> > > cp -p packages/13.0-i386-wip/All/perl5-5.32.1_3.tbz /tmp ; echo $?
> > > Filesize limit exceeded
> > > 153
> >
> > I think zfs_copy_file_range() needs to use vn_rlimit_fsizex() the
> > same way that vn_generic_copy_file_range() does.
> >
> > I have posted the attached patch to D39419.
> >
> > danfe@. Assuming you were using zfs, could you test this patch?
> > (You will need an up to date main kernel and, hopefully, the block
> >  cloning stuff has not trashed your zpool.)
>
> We had already discussed with with Rick privately, but for the archives,
> looks like my problem had been solved somewhere between 13.1 and 13.2,
> and it's not related to ZFS.
>
> sh -c 'cd /tmp ; truncate -s 20m foo ; limits -f 8386560000 cp foo bar ;
> stat -f %z bar'
>
> ==> 13.1:
> Filesize limit exceeded
> 0
Yes. If you think this should be done as an Errata for 13.1, please
email asomers@, who committed the fix to main and stable/13 after
the 13.1 release.

>
> ==> 13.2:
> 20971520
>
> Allegedly this is due to commit 52360ca, but I'd have to look closer into
> it to be sure.  Sorry for distracting you guys from that recent ZoL merge
> fallout, I should've not be lazy and reported it earlier. :(
>
I do believe the recent ZoL merge introduced the same bug to ZFS.
I have posted a proposed patch to D39419.
(Basically call vn_rlimit_fsizex() and not vn_rlimit_fsize().)

rick

> ./danfe