[PATCH] Fix bug 9412 - SMB2 server doesn't support recvfile.

[Jeremy Allison]

On Mon, Mar 31, 2014 at 05:34:14PM +0800, Jones wrote:
> Hello Jeremy and Metze,
> 
> Excuse me to recall this old thread,
> i am trying to find bottleneck with samba-4.0.5 in the linux box,
> the iometer access spec is set to 512bytes sequential write.
> 
> And my samba-4.0.5 is patched with Bug#9412,
> https://bugzilla.samba.org/show_bug.cgi?id=9412
> 
> 
> With 'perf top',
> __libc_fcntl is at the top 2.
> 
>      8.18%  [kernel]               [k] ia32_syscall
>      3.05%  libpthread-2.6.1.so  [.] __libc_fcntl
>      2.40%  libc-2.6.1.so        [.] __gettimeofday
>    2.10%  [kernel]               [k] tcp_sendmsg
>      2.08%  [kernel]               [k] __ticket_spin_lock
> ... ...
> 
> With 'strace',
> 4 fcntl() are surrounding each splice().
> ... ...
> fcntl64(31, F_GETFL)                    = 0x802 (flags O_RDWR|O_NONBLOCK)
> fcntl64(31, F_GETFL)                    = 0x802 (flags O_RDWR|O_NONBLOCK)
> fcntl64(31, F_SETFL, O_RDWR)            = 0
> splice(0x1f, 0, 0x20, 0xff8118c0, 0x200, 0) = 512
> fcntl64(31, F_SETFL, O_RDWR|O_NONBLOCK) = 0
> ... ...
> 
> For testing, editing vfs.c and following code are removed.
> And compare the before and after,
> the iometer IOps is raised from 75,485 to 83,915.
> Hmm this change is a preliminary testing, maybe not recommended.
> 
> ssize_t vfs_pwrite_data(struct smb_request *req,
> ...
> -               old_flags = fcntl(sockfd, F_GETFL, 0);
> -               if (set_blocking(sockfd, true) == -1) {
> -                       return (ssize_t)-1;
> -               }
>                 ret = SMB_VFS_RECVFILE(sockfd,
>                                         fsp,
>                                         offset,
>                                         N);
> -               if (fcntl(sockfd, F_SETFL, old_flags) == -1) {
> -                      return (ssize_t)-1;
> -               }
> 
> Question:
> It looks like cpu cycles are spent a bit with 4 fcntl() call,
> is there a better way to reduce this overhead to improve performance?
> i have read following hint but still confused,
> any suggestion is appreciated, thanks.

Trouble is most of the server code depends on the
socket being non-blocking. But in order for RECVFILE
to actually work, the socket needs to be blocking (you
can't have a partial recvfile here, it's not safe).

I guess the code could be rewritten to do RECVFILE
in a loop and advance the offset (and reduce N)
by the amount read in previous RECVFILES, but it
seemed simpler to just make the socket blocking.

As there is no system call recvfile on Linux
then there's no real guidance on how it should
behave on a non-blocking socket, so making the
socket blocking seemed like the safer choice.

Try scoping out a loop around the RECVFILE
call to do what I mentioned earlier and
send it in as a proposed patch. If you don't
get time I might have a go at it :-).

Cheers,

	Jeremy.