Github version: https://github.com/mmatuska/freebsd-src/commits/openzfs_gc1c31a835
@mjg please review the modification of zfs_freebsd_readlink() in sys/contrib/openzfs/module/os/freebsd/zfs/zfs_vnops_os.c

Huh. Looks like upstream added the following everywhere:

static int
zfs_freebsd_read(struct vop_read_args *ap)
{
        zfs_uio_t uio;
        zfs_uio_init(&uio, ap->a_uio);
        return (zfs_read(VTOZ(ap->a_vp), &uio, ioflags(ap->a_ioflag),
            ap->a_cred));
}

Not only this adds a likely avoidable data copy, this can no longer be tail called as return has to clean up the stack. That's a major bummer and should get reimplemented. I don't have time to get into it now.

That said, the readlink bit looks fine.

I don't think it's feasible to really review the entire patchset tough.

@mjg I don't think much has changed under the hood because:

typedef struct zfs_uio {
        struct uio      *uio;
} zfs_uio_t;

#define GET_UIO_STRUCT(u)       (u)->uio
#define zfs_uio_segflg(u)       GET_UIO_STRUCT(u)->uio_segflg
#define zfs_uio_offset(u)       GET_UIO_STRUCT(u)->uio_offset
#define zfs_uio_resid(u)        GET_UIO_STRUCT(u)->uio_resid
#define zfs_uio_iovcnt(u)       GET_UIO_STRUCT(u)->uio_iovcnt
#define zfs_uio_iovlen(u, idx)  GET_UIO_STRUCT(u)->uio_iov[(idx)].iov_len
#define zfs_uio_iovbase(u, idx) GET_UIO_STRUCT(u)->uio_iov[(idx)].iov_base
#define zfs_uio_td(u)           GET_UIO_STRUCT(u)->uio_td
#define zfs_uio_rw(u)           GET_UIO_STRUCT(u)->uio_rw
#define zfs_uio_fault_disable(u, set)
#define zfs_uio_prefaultpages(size, u)  (0)


static __inline void
zfs_uio_init(zfs_uio_t *uio, struct uio *uio_s)
{
        GET_UIO_STRUCT(uio) = uio_s;
}

But if it is like you say, then we do symlink_len -= zfs_uio_resid(&uio); after line error = zfs_readlink(ap->a_vp, &uio, ap->a_cred, NULL); and that would be a no-go in zfs_freebsd_readlink().

The zfs test suite was not very happy with this in my bhyve, I guess we postpone this or someone else could continue with the merge. I have cherry-picked two important commits for 13.0-RELEASE.

Is the test suite happy with zfs as is? I know it used to pass few months back, but to my understanding some of the assertions were not being compiled in.

I ran the few zfs tests I have on openzfs_gc1c31a835-n244815-1f4c076e9e38 without observing any problems.

To be true guys, what is much more interesting than this diff is the diff between vendor/openzfs and sys/contrib/openzfs - I guess we want to keep it as small as possible (long-term). And currently there is already some difference.
So if you have no objections I might throw this in but I am not really into having this in 13.0-RELEASE, 13-stable after a reasonable amount of time (at least 2 weeks) would be o.k. for me.

The most notable improvement for me is the speedup in zpool import. I have a 102-drive pool with 60TB of L2 cache and the import time is reduced by a factor of 10.

The intent is to have next to no local patches, but upstreaming got stalled at some point with CI failures and I did not pick it up later. It is a little iffy right now but should be sorted out soon(tm).

Sorry I didn't get a chance to review this in time. Thank you for updating to 436ab35a5. I share the concern about our diff against upstream.