On Fri, Mar 22, 2013 at 02:28:34PM -0700, Jeremy Allison wrote: > 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.
>From a quick glance this looks exactly like what I had in mind. Thanks! Volker -- SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen phone: +49-551-370000-0, fax: +49-551-370000-9 AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen http://www.sernet.de, mailto:kont...@sernet.de -- To unsubscribe from this list go to the following URL and read the instructions: https://lists.samba.org/mailman/options/samba