[Samba] stat between reads

Frank Kautz frank.kautz at itwm.fraunhofer.de
Wed Mar 27 02:30:27 MDT 2013


Hello,

thanks for the very quick replay and the patch. I tested the patch with
the v3-6-stable branch (3.6.14) and it works as expected. The stat is
gone and I could recognize a increased performance.

kind regards,
Frank


Am 03/22/2013 10:28 PM, schrieb Jeremy Allison:
> On Fri, Mar 22, 2013 at 02:07:29PM -0700, Jeremy Allison wrote:
>> On Fri, Mar 22, 2013 at 05:24:20PM +0100, Volker Lendecke wrote:
>>> 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.
> 
> Yep, I was right. This would be a problem (ftruncate
> and other file-size changing calls don't automatically
> update the st_ex_size on an fsp).
> 
>> Moving the fstat and ISREG check to the use_sendfile()
>> path is an obviously correct no brainer though.
> 
> And here is that patch. Should apply cleanly to
> 4.0.x and 3.6.x (with a few offsets). I'm testing
> here and will propose as an official optimization
> if 'make test' passes locally.
> 
> Jeremy.
> 
> 
> 



More information about the samba mailing list