> fsync() is completely orthogonal to unbuffered writes and shouldn't be
> included in the patch.

I hesitated to add it, but I discovered that I needed it for writing to
actually show up in the file system, at least when writing to SMB.

> On many journaled filesystems, fsync() essentially forces a complete
> sync(), affecting throughput of the entire system. It's a big stick
> that should be used sparingly.

I know, I have previously developed an flash translation layer (think SW
making flash behave as if it was sector-based storage) where sync() or
fsync() could take many hundreds of ms in the worst-case. This is
why I hesitated. On the other hand fsync() is not the default, and a
CURLOPT needs to be set to activate this behaviour. It is likely that
buffered file I/O is desired in most cases, and in the cases where a
user enables unbuffered I/O, care must already have been taken to ensure
that the I/O requests are few and large (if not using buffered I/O would
make more sense).

> If it's truly needed, then sync() could always be called in the
> application's write callback that occurs immediately after the file
> write.

Now I'm not following your reasoning. What write callback?
WRITEFUNCTION? Where and why would this callback be called? I can only
see that file_upload() would do statistics updates and then again call
Curl_fillreadbuffer(), which in turn would call READFUNCTION which in my
case might block for many seconds or minutes before realizing that the
data to be written to the file is now completed. This is the basic
premise causing me to want unbuffered writes in the first place.

> Admittedly, that's not as efficient on systems where fsync() doesn't
> effectively equal sync(), but it should be needed in so few
> applications that I'm not too worried.

I might be developing one of these applications which causes me to be
worried. ;)

>> I'm not confident enough about how curl's test harness works to add a
>> test that sets this option and tests it. I'm not even sure what such
>> a test can do apart from just making sure that the CURLOPT is
>> accepted. But as you can see from the patch I'm hoping for the patch
>> to eventually be included in 7.27.1. :)
>
> You could do it pretty easily with an external libtest program, using
> test 513 as a base. It might be tricky to make completely portable
> due to differing stdio buffering implementations, but it should be
> possible to make it work most places.

Ok, I'll take a look at this on monday and see if I can figure out how
this works and see if I can do something based on test 513. Thanks for
the pointer, without it I would have been forever lost looking at the
vast test code!

/ Sebastian
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Received on 2012-10-07