On Mon, 30 Mar 2009, Ric Wheeler wrote:
> >
> > But turn that around, and say: if you don't have redundant disks, then
> > pretty much by definition those drive flushes won't be guaranteeing your
> > data _anyway_, so why pay the price?
>
> They do in fact provide that promise for the extremely common case of power
> outage and as such, can be used to build reliable storage if you need to.

No they really effectively don't. Not if the end result is "oops, the
whole track is now unreadable" (regardless of whether it happened due to a
write durign power-out or during some entirely unrelated disk error). Your
"flush" didn't result in a stable filesystem at all, it just resulted in a
dead one.

That's my point. Disks simply aren't that reliable. Anything you do with
flushing and ordering won't make them magically not have errors any more.

> Heat is a major killer of spinning drives (as is severe cold). A lot of times,
> drives that have read errors only (not failed writes) might be fully
> recoverable if you can re-write that injured sector.

It's not worked for me, and yes, I've tried. Maybe I've been unlucky, but
every single case I can remember of having read failures, that drive has
been dead. Trying to re-write just the sectors with the error (and around
it) didn't do squat, and rewriting the whole disk didn't work either.

I'm sure it works for some "ok, the write just failed to take, and the CRC
was bad" case, but that's apparently not what I've had. I suspect either
the track markers got overwritten (and maybe a disk-specific low-level
reformat would have helped, but at that point I was not going to trust the
drive anyway, so I didn't care), or there was actual major physical damage
due to heat and/or head crash and remapping was just not able to cope.

> > Sure. And those "write flushes" really only cover a rather small percentage.
> > For many setups, the other corruption issues (drive failure) are not just
> > more common, but generally more disastrous anyway. So why would a person
> > like that worry about the (rare) power failure?
>
> This is simply not a true statement from what I have seen personally.

You yourself said that software errors were your biggest issue. The write
flush wouldn't matter for those (but the elevator barrier would)

> The elevator does not issue write barriers on its own - those write barriers
> are sent down by the file systems for transaction commits.

Right. But "elevator write barrier" vs "sending a drive flush command" are
two totally independent issues. You can do one without the other (although
doing a drive flush command without the write barrier is admittedly kind
of pointless ;^)

And my point is, IT MAKES SENSE to just do the elevator barrier, _without_
the drive command. If you worry much more about software (or non-disk
component) failure than about power failures, you're better off just doing
the software-level synchronization, and leaving the hardware alone.

Linus

2009-03-30 18:01:56

On Fri, Apr 03, 2009 at 07:30:37PM -0400, Jeff Garzik wrote:
> David Rees wrote:
> > The *only* reason MythTV fsyncs (or fdatasyncs) the data to disk all
> > the time is to keep a large amount of dirty pages from building up and
> > then causing horrible latencies when that data starts getting flushed
> > to disk.
>
> sync_file_range() will definitely help that situation.

Jeff, could you please try following patch for 0.21 or update to the
latest trunk revision. I don't have a way to reproduce the high
latencies with fdatasync on ext3, data=ordered. Doing a parallel
"dd if=/dev/zero of=file" on the same partition introduces even with
sync_file_range latencies over 1 second.

Janne

---

Index: configure
===================================================================
--- configure (revision 20302)
+++ configure (working copy)
@@ -873,6 +873,7 @@
sdl_video_size
soundcard_h
stdint_h
+ sync_file_range
sys_poll_h
sys_soundcard_h
termios_h
@@ -2413,6 +2414,17 @@
int main( void ) { return (round(3.999f) > 0)?0:1; }
EOF

+# test for sync_file_range (linux only system call since 2.6.17)
+check_ld <<EOF && enable sync_file_range
+#define _GNU_SOURCE
+#include <fcntl.h>
+
+int main(int argc, char **argv){
+ sync_file_range(0,0,0,0);
+ return 0;
+}
+EOF
+
# test for sizeof(int)
for sizeof in 1 2 4 8 16; do
check_cc <<EOF && _sizeof_int=$sizeof && break
Index: libs/libmythtv/ThreadedFileWriter.cpp
===================================================================
--- libs/libmythtv/ThreadedFileWriter.cpp (revision 20302)
+++ libs/libmythtv/ThreadedFileWriter.cpp (working copy)
@@ -18,6 +18,7 @@
#include "ThreadedFileWriter.h"
#include "mythcontext.h"
#include "compat.h"
+#include "mythconfig.h"

#if defined(_POSIX_SYNCHRONIZED_IO) && _POSIX_SYNCHRONIZED_IO > 0
#define HAVE_FDATASYNC
@@ -122,6 +123,7 @@
// file stuff
filename(QDeepCopy<QString>(fname)), flags(pflags),
mode(pmode), fd(-1),
+ m_file_sync(0), m_file_wpos(0),
// state
no_writes(false), flush(false),
in_dtor(false), ignore_writes(false),
@@ -154,6 +156,8 @@
buf = new char[TFW_DEF_BUF_SIZE + 1024];
bzero(buf, TFW_DEF_BUF_SIZE + 64);

+ m_file_sync = m_file_wpos = 0;
+
tfw_buf_size = TFW_DEF_BUF_SIZE;
tfw_min_write_size = TFW_MIN_WRITE_SIZE;
pthread_create(&writer, NULL, boot_writer, this);
@@ -292,7 +296,22 @@
{
if (fd >= 0)
{
-#ifdef HAVE_FDATASYNC
+#ifdef HAVE_SYNC_FILE_RANGE
+ uint64_t write_position;
+
+ buflock.lock();
+ write_position = m_file_wpos;
+ buflock.unlock();
+
+ if ((write_position - m_file_sync) > TFW_MAX_WRITE_SIZE ||
+ (write_position && m_file_sync < (uint64_t)tfw_min_write_size))
+ {
+ sync_file_range(fd, m_file_sync, write_position - m_file_sync,
+ SYNC_FILE_RANGE_WRITE);
+ m_file_sync = write_position;
+ }
+
+#elif defined(HAVE_FDATASYNC)
fdatasync(fd);
#else
fsync(fd);
@@ -414,6 +433,7 @@

buflock.lock();
rpos = (rpos + size) % tfw_buf_size;
+ m_file_wpos += size;
buflock.unlock();

bufferWroteData.wakeAll();
Index: libs/libmythtv/ThreadedFileWriter.h
===================================================================
--- libs/libmythtv/ThreadedFileWriter.h (revision 20302)
+++ libs/libmythtv/ThreadedFileWriter.h (working copy)
@@ -40,6 +40,8 @@
int flags;
mode_t mode;
int fd;
+ uint64_t m_file_sync; ///< offset synced to disk
+ uint64_t m_file_wpos; ///< offset written to disk

// state
bool no_writes;

2009-04-04 21:10:43

unsubscribe linux-kernel