Server-side copy with sendfile system call

[Teng-Feng Yang]

Hi David,

I already reimplement the vfswrap_copy_chunk_send with sendfile system call.
However, I have noticed that the throughput shown in my Windows Server
2012 machine keeps bouncing up and down through the whole copy
process.
The peak performance is around 140MB/s on a single 4TB harddisk, and
then it will drop down to 0MB/s for a couple of seconds before it
rises back to 140MB/s again.
This repeat pattern results in an overall worse performance than the
origin read-then-write version of copy_chunk function.

I don't know if I do anything wrong. The following is my version of
copy_chunk_send using sendfile().
Any comment would be highly appreciated.
Thanks.

Dennis

static struct tevent_req *vfswrap_copy_chunk_send(struct
vfs_handle_struct *handle,
                          TALLOC_CTX *mem_ctx,
                          struct tevent_context *ev,
                          struct files_struct *src_fsp,
                          off_t src_off,
                          struct files_struct *dest_fsp,
                          off_t dest_off,
                          off_t num)
{
    struct tevent_req *req;
    struct vfs_cc_state *vfs_cc_state;
    off_t dcur_off;
    ssize_t ret;
    struct lock_struct src_lck, dest_lck;
    int saved_errno;
    NTSTATUS status;

    DEBUG(10, ("performing server side copy chunk of length %lu\n",
           (unsigned long)num));

    req = tevent_req_create(mem_ctx, &vfs_cc_state, struct vfs_cc_state);
    if (req == NULL) {
        return NULL;
    }

    status = vfs_stat_fsp(src_fsp);
    if (tevent_req_nterror(req, status)) {
        return tevent_req_post(req, ev);
    }

    if (src_fsp->fsp_name->st.st_ex_size < src_off + num) {
        /*
         * [MS-SMB2] 3.3.5.15.6 Handling a Server-Side Data Copy Request
         *   If the SourceOffset or SourceOffset + Length extends beyond
         *   the end of file, the server SHOULD<240> treat this as a
         *   STATUS_END_OF_FILE error.
         * ...
         *   <240> Section 3.3.5.15.6: Windows servers will return
         *   STATUS_INVALID_VIEW_SIZE instead of STATUS_END_OF_FILE.
         */
        tevent_req_nterror(req, NT_STATUS_INVALID_VIEW_SIZE);
        return tevent_req_post(req, ev);
    }

    /* could use 2.6.33+ sendfile here to do this in kernel */
    init_strict_lock_struct(src_fsp,
                src_fsp->op->global->open_persistent_id,
                src_off,
                num,
                READ_LOCK,
                &src_lck);

    init_strict_lock_struct(dest_fsp,
                dest_fsp->op->global->open_persistent_id,
                dest_off,
                num,
                WRITE_LOCK,
                &dest_lck);

    if (!SMB_VFS_STRICT_LOCK(src_fsp->conn, src_fsp, &src_lck)) {
        tevent_req_nterror(req, NT_STATUS_FILE_LOCK_CONFLICT);
        return tevent_req_post(req, ev);
    }

    if (!SMB_VFS_STRICT_LOCK(dest_fsp->conn, dest_fsp, &dest_lck)) {
        SMB_VFS_STRICT_UNLOCK(src_fsp->conn, src_fsp, &src_lck);
        tevent_req_nterror(req, NT_STATUS_FILE_LOCK_CONFLICT);
        return tevent_req_post(req, ev);
    }

    dcur_off = SMB_VFS_LSEEK(dest_fsp, 0, SEEK_CUR);

    if (dcur_off == -1) {
        SMB_VFS_STRICT_UNLOCK(dest_fsp->conn, dest_fsp, &dest_lck);
        SMB_VFS_STRICT_UNLOCK(src_fsp->conn, src_fsp, &src_lck);
        tevent_req_nterror(req, map_nt_error_from_unix(errno));
        return tevent_req_post(req, ev);
    }

    if (SMB_VFS_LSEEK(dest_fsp, dest_off, SEEK_SET) == -1) {
        SMB_VFS_STRICT_UNLOCK(dest_fsp->conn, dest_fsp, &dest_lck);
        SMB_VFS_STRICT_UNLOCK(src_fsp->conn, src_fsp, &src_lck);
        tevent_req_nterror(req, map_nt_error_from_unix(errno));
        return tevent_req_post(req, ev);
    }

    ret = SMB_VFS_SENDFILE(dest_fsp->fh->fd, src_fsp, NULL, src_off, num);
    if (ret == -1) {
        saved_errno = errno;
    }

    DEBUG(10, ("sendfile return %lu\n",
           (unsigned long)ret));
    SMB_VFS_LSEEK(dest_fsp, dcur_off, SEEK_SET);

    SMB_VFS_STRICT_UNLOCK(dest_fsp->conn, dest_fsp, &dest_lck);
    SMB_VFS_STRICT_UNLOCK(src_fsp->conn, src_fsp, &src_lck);

    if (ret == -1) {
        errno = saved_errno;
        tevent_req_nterror(req, map_nt_error_from_unix(errno));
        return tevent_req_post(req, ev);
    }

    if (ret != num) {
        /* zero tolerance for short reads */
        tevent_req_nterror(req, NT_STATUS_IO_DEVICE_ERROR);
        return tevent_req_post(req, ev);
    }

    vfs_cc_state->copied = num;

    tevent_req_done(req);
    return tevent_req_post(req, ev);
}


2014-05-15 15:37 GMT+08:00 David Disseldorp <ddiss at suse.de>:
> Hi Dennis,
>
> On Thu, 15 May 2014 11:39:26 +0800, Teng-Feng Yang wrote:
>
> ...
>> Thanks for the tips.
>> I write a small program to test sendfile, and it turns out that
>> sendfile will do nothing when out_fd == in_fd.
>> Does this mean that we should fallback to read-then-write procedure to
>> deal with this condition?
>
> Yes, a fall-back would be needed for this case, as Windows handles such
> requests successfully. The test_ioctl_copy_chunk_src_is_dest smbtorture
> test covers this case.
>
> Cheers, David
>