xfs
[Top] [All Lists]

Re: [PATCH] O_DIRECT hole filling bug

To: Andrew Morton <akpm@xxxxxxxx>
Subject: Re: [PATCH] O_DIRECT hole filling bug
From: Janet Morgan <janetmor@xxxxxxxxxx>
Date: Mon, 07 Jun 2004 17:00:26 -0700
Cc: Chris Mason <mason@xxxxxxxx>, daniel@xxxxxxxx, janetinc@xxxxxxxxxx, linux-aio@xxxxxxxxx, linux-xfs@xxxxxxxxxxx
In-reply-to: <20040602223236.3a7dcee8.akpm@xxxxxxxx>
References: <1085753950.22636.3208.camel@xxxxxxxxxxxxx> <1085782365.22798.35.camel@xxxxxxxxxxxxxxxxxx> <20040528152839.44bdb7dc.akpm@xxxxxxxx> <1086011001.22636.3379.camel@xxxxxxxxxxxxx> <20040601025641.1721966e.akpm@xxxxxxxx> <1086090295.22636.3398.camel@xxxxxxxxxxxxx> <20040602223236.3a7dcee8.akpm@xxxxxxxx>
Sender: linux-xfs-bounce@xxxxxxxxxxx
User-agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.5) Gecko/20031007
Andrew Morton wrote:

Chris Mason <mason@xxxxxxxx> wrote:
It should pass, since it should be redoing things as buffered io the
same way my patch did.  We'll call dio_complete differently, but I'm
unclear on how much that really matters.

That only affects XFS and yes, it was wrong.  This will hopefully fix it
up.

--- 25/fs/direct-io.c~direct-io-hole-fix        2004-06-02 18:27:16.021168440 
-0700
+++ 25-akpm/fs/direct-io.c      2004-06-02 21:40:52.660170880 -0700
@@ -987,13 +987,6 @@ direct_io_worker(int rw, struct kiocb *i
                }
        } /* end iovec loop */

-       if (ret == -ENOTBLK && rw == WRITE) {
-               /*
-                * The remaining part of the request will be
-                * be handled by buffered I/O when we return
-                */
-               ret = 0;
-       }
        /*
         * There may be some unwritten disk at the end of a part-written
         * fs-block-sized block.  Go zero that now.
@@ -1060,24 +1053,29 @@ direct_io_worker(int rw, struct kiocb *i
                        kfree(dio);
                }
        } else {
+               ssize_t transferred = 0;
+
                finished_one_bio(dio);
                ret2 = dio_await_completion(dio);
                if (ret == 0)
                        ret = ret2;
                if (ret == 0)
                        ret = dio->page_errors;
-               if (ret == 0 && dio->result) {
+               if (dio->result) {
                        loff_t i_size = i_size_read(inode);

-                       ret = dio->result;
+                       transferred = dio->result;
                        /*
                         * Adjust the return value if the read crossed a
                         * non-block-aligned EOF.
                         */
-                       if (rw == READ && (offset + ret > i_size))
-                               ret = i_size - offset;
+                       if (rw == READ && (offset + transferred > i_size))
+                               transferred = i_size - offset;
                }
-               dio_complete(dio, offset, ret);
+               dio_complete(dio, offset, transferred);
+               if (ret == 0)
+                       ret = transferred;
+
                /* We could have also come here on an AIO file extend */
                if (!is_sync_kiocb(iocb) && rw == WRITE &&
                    ret >= 0 && dio->result == dio->size)
@@ -1088,6 +1086,13 @@ direct_io_worker(int rw, struct kiocb *i
                        aio_complete(iocb, ret, 0);
                kfree(dio);
        }
+       if (ret == -ENOTBLK && rw == WRITE) {
+               /*
+                * The entire request will be be handled by buffered I/O
+                * when we return
+                */
+               ret = 0;
+       }
        return ret;
}

_


But it's hard to tell because XFS fails the below tests.  Tested with and
without the recent direct-io fixups. Tested 2.6.7-rc2. Tested 2.6.2. 2.4.27-pre4 as well. In all cases, XFS fails.



Tested your patch on 2.6.6; XFS does not seem to fail if "-o 8192" or "-o 16384" are specified.

Thanks,
-Janet

fsx-linux -l 500000 -r 4096 -t 4096 -w 4096 -Z -N 10000 junk  -R -W
fsx-linux -l 500000 -r 4096 -t 2048 -w 2048 -Z -N 10000 junk  -R -W
fsx-linux -N 10000 -o 8192 -l 500000 -r 4096 -t 2048 -w 2048 -Z -R -W junk fsx-linux -N 10000 -o 16384 -l 500000 -r 4096 -t 4096 -w 4096 -Z -R -W junk fsx-linux -N 10000 -o 32768 -l 500000 -r 4096 -t 2048 -w 2048 -Z -R -W junk fsx-linux -N 10000 -o 128000 -l 500000 -r 4096 -t 4096 -w 4096 -Z -R -W junk fsx-linux -N 10000 -o 8192 -l 500000 -r 4096 -t 2048 -w 2048 -Z -R -W junk fsx-linux -N 10000 -o 16384 -l 500000 -r 4096 -t 4096 -w 4096 -Z -R -W junk fsx-linux -N 10000 -o 32768 -l 500000 -r 4096 -t 2048 -w 2048 -Z -R -W junk fsx-linux -N 10000 -o 128000 -l 500000 -r 4096 -t 4096 -w 4096 -Z -W junk fsx-linux -N 10000 -o 128000 -l 500000 -r 4096 -t 4096 -w 4096 -Z -W junk
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@xxxxxxxxxx  For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@xxxxxxxxx";>aart@xxxxxxxxx</a>




<Prev in Thread] Current Thread [Next in Thread>