[Samba] stat between reads

[Jeremy Allison]

On Fri, Mar 22, 2013 at 05:24:20PM +0100, Volker Lendecke wrote:
> On Fri, Mar 22, 2013 at 03:18:53PM +0100, Frank Kautz wrote:
> > Hello,
> > 
> > when re-exporting the parallel (network) file-system FhGFS, I noticed a
> > significantly reduced read performance. In the output of strace, I see a
> > stat call between the reads of the blocks of a file. In a network based
> > file system a stat could be expensive. I looked into the source code and
> > I think the stat is done in the method send_file_readX() (file:
> > source3/smbd/reply.c) which calls fsp_stat(). What is the reason for
> > this stat? Is it possible to disable this stat by a configuration option
> > or just avoid it in the code?
> 
> If I see it right, we could avoid most of those calls.
> First, they are only relevant to see whether we can do
> sendfile. That choice is racy, we first look and have to
> deal with the mess if we have a short read afterwards
> anyway.
> 
> Jeremy, what do you think? Only do that stat call in the
> sendfile if-branch, and there only if the read call in
> question would go beyond what we currently have in
> st.st_ex_size?

Yes.... we could certainly make that change. It's not
relevent in the non-sendfile read path but we'd have to be
careful about not doing it by checking the existing
fsp->fsp_name->st.st_ex_size, as I don't think we
update that on an ftruncate call.

My worry would be (to one single smbd):

open 1mb file
ftruncate to 10k
readX of 50k at offset 0.

Unless the ftruncate call updates fsp->fsp_name->st.st_ex_size
then we'll return 10k of real data + 40k of zeros if
sendfile is turned on, rather than a correct short read of
10k.

Let me look at the code some and revisit this.

Moving the fstat and ISREG check to the use_sendfile()
path is an obviously correct no brainer though.

Cheers,

	Jeremy.