From 15968f9a55f27c2d993dcbfaf68c0ca2cfde04ea Mon Sep 17 00:00:00 2001 From: deen Date: Wed, 20 Jun 2007 18:39:38 +0000 Subject: [PATCH] Direct I/O operations should return actual amount of bytes transferred rather than requested size. b=11737 i=adilger i=shadow --- lustre/ChangeLog | 7 +++++++ lustre/include/lustre_net.h | 2 ++ lustre/include/obd_class.h | 5 +++++ lustre/llite/rw24.c | 14 ++++++-------- lustre/llite/rw26.c | 12 +++++------- lustre/osc/osc_request.c | 3 +++ lustre/tests/directio.c | 2 +- lustre/tests/sanity.sh | 35 ++++++++++++++++++++++++++++++++++- 8 files changed, 63 insertions(+), 17 deletions(-) diff --git a/lustre/ChangeLog b/lustre/ChangeLog index 5b3f40e..13feaee 100644 --- a/lustre/ChangeLog +++ b/lustre/ChangeLog @@ -311,6 +311,13 @@ Description: replay-single.sh test 52 fails Details : A lock's skiplist need to be cleanup when it being unlinked from its resource list. +Severity : normal +Bugzilla : 11737 +Description: Short directio read returns full requested size rather than + actual amount read. +Details : Direct I/O operations should return actual amount of bytes + transferred rather than requested size. + -------------------------------------------------------------------------------- 2007-05-03 Cluster File Systems, Inc. diff --git a/lustre/include/lustre_net.h b/lustre/include/lustre_net.h index dccf83e..45a9d5e 100644 --- a/lustre/include/lustre_net.h +++ b/lustre/include/lustre_net.h @@ -181,6 +181,8 @@ struct ptlrpc_request_set { struct list_head set_requests; set_interpreter_func set_interpret; /* completion callback */ void *set_arg; /* completion context */ + void *set_countp; /* pointer to NOB counter in case + * of directIO (bug11737) */ /* locked so that any old caller can communicate requests to * the set holder who can then fold them into the lock-free set */ spinlock_t set_new_req_lock; diff --git a/lustre/include/obd_class.h b/lustre/include/obd_class.h index d796405..f77c399 100644 --- a/lustre/include/obd_class.h +++ b/lustre/include/obd_class.h @@ -911,12 +911,15 @@ static inline int obd_brw_rqset(int cmd, struct obd_export *exp, { struct ptlrpc_request_set *set = NULL; struct obd_info oinfo = { { { 0 } } }; + atomic_t nob; int rc = 0; ENTRY; set = ptlrpc_prep_set(); if (set == NULL) RETURN(-ENOMEM); + atomic_set(&nob, 0); + set->set_countp = &nob; oinfo.oi_oa = oa; oinfo.oi_md = lsm; @@ -925,6 +928,8 @@ static inline int obd_brw_rqset(int cmd, struct obd_export *exp, rc = ptlrpc_set_wait(set); if (rc) CERROR("error from callback: rc = %d\n", rc); + else + rc = atomic_read(&nob); } else { CDEBUG(rc == -ENOSPC ? D_INODE : D_ERROR, "error from obd_brw_async: rc = %d\n", rc); diff --git a/lustre/llite/rw24.c b/lustre/llite/rw24.c index 151e00b..cf666da 100644 --- a/lustre/llite/rw24.c +++ b/lustre/llite/rw24.c @@ -66,13 +66,14 @@ static int ll_direct_IO_24(int rw, struct brw_page *pga; struct obdo oa; int length, i, flags, rc = 0; - loff_t offset; + loff_t offset, offset_orig; ENTRY; if (!lsm || !lsm->lsm_object_id) RETURN(-EBADF); offset = ((obd_off)blocknr << inode->i_blkbits); + offset_orig = offset; CDEBUG(D_VFSTRACE, "VFS Op:inode=%lu/%u(%p), size="LPSZ ", offset=%lld=%llx, pages %u\n", inode->i_ino, inode->i_generation, inode, iobuf->length, @@ -111,13 +112,10 @@ static int ll_direct_IO_24(int rw, ll_stats_ops_tally(ll_i2sbi(inode), LPROC_LL_DIRECT_READ, iobuf->length); rc = obd_brw_rqset(rw, ll_i2obdexp(inode), &oa, lsm, iobuf->nr_pages, pga, NULL); - if (rc == 0) { - rc = iobuf->length; - if (rw == OBD_BRW_WRITE) { - lov_stripe_lock(lsm); - obd_adjust_kms(ll_i2obdexp(inode), lsm, offset, 0); - lov_stripe_unlock(lsm); - } + if ((rc > 0) && (rw == OBD_BRW_WRITE)) { + lov_stripe_lock(lsm); + obd_adjust_kms(ll_i2obdexp(inode), lsm, offset_orig + rc, 0); + lov_stripe_unlock(lsm); } OBD_FREE(pga, sizeof(*pga) * iobuf->nr_pages); diff --git a/lustre/llite/rw26.c b/lustre/llite/rw26.c index 0c50dcf..3795de7 100644 --- a/lustre/llite/rw26.c +++ b/lustre/llite/rw26.c @@ -141,6 +141,7 @@ static ssize_t ll_direct_IO_26_seg(int rw, struct inode *inode, struct obdo oa; int i, rc = 0; size_t length; + loff_t file_offset_orig = file_offset; ENTRY; OBD_ALLOC(pga, sizeof(*pga) * page_count); @@ -166,13 +167,10 @@ static ssize_t ll_direct_IO_26_seg(int rw, struct inode *inode, rc = obd_brw_rqset(rw == WRITE ? OBD_BRW_WRITE : OBD_BRW_READ, ll_i2obdexp(inode), &oa, lsm, page_count, pga, NULL); - if (rc == 0) { - rc = size; - if (rw == WRITE) { - lov_stripe_lock(lsm); - obd_adjust_kms(ll_i2obdexp(inode), lsm, file_offset, 0); - lov_stripe_unlock(lsm); - } + if ((rc > 0) && (rw == WRITE)) { + lov_stripe_lock(lsm); + obd_adjust_kms(ll_i2obdexp(inode), lsm, file_offset_orig + rc, 0); + lov_stripe_unlock(lsm); } OBD_FREE(pga, sizeof(*pga) * page_count); diff --git a/lustre/osc/osc_request.c b/lustre/osc/osc_request.c index a2f9c48..0a5b9ff 100644 --- a/lustre/osc/osc_request.c +++ b/lustre/osc/osc_request.c @@ -1287,6 +1287,7 @@ static int brw_interpret(struct ptlrpc_request *request, void *data, int rc) { struct osc_brw_async_args *aa = data; int i; + int nob = rc; ENTRY; rc = osc_brw_fini_request(request, rc); @@ -1295,6 +1296,8 @@ static int brw_interpret(struct ptlrpc_request *request, void *data, int rc) if (rc == 0) RETURN(0); } + if ((rc >= 0) && request->rq_set && request->rq_set->set_countp) + atomic_add(nob, (atomic_t *)request->rq_set->set_countp); spin_lock(&aa->aa_cli->cl_loi_list_lock); for (i = 0; i < aa->aa_page_count; i++) diff --git a/lustre/tests/directio.c b/lustre/tests/directio.c index b68bd74..ebcedb2 100644 --- a/lustre/tests/directio.c +++ b/lustre/tests/directio.c @@ -106,7 +106,7 @@ int main(int argc, char **argv) rc = read(fd, rbuf, len); if (rc != len) { - printf("Read error: %s (rc = %d)\n",strerror(errno),rc); + printf("Read error: %s rc = %d\n",strerror(errno),rc); return 1; } diff --git a/lustre/tests/sanity.sh b/lustre/tests/sanity.sh index 81958f1..b1f6125 100644 --- a/lustre/tests/sanity.sh +++ b/lustre/tests/sanity.sh @@ -3121,6 +3121,7 @@ rm -f $F77_TMP unset F77_TMP test_78() { # bug 10901 + NSEQ=5 F78SIZE=$(($(awk '/MemFree:/ { print $2 }' /proc/meminfo) / 1024)) [ $F78SIZE -gt 512 ] && F78SIZE=512 [ $F78SIZE -gt $((MAXFREE / 1024)) ] && F78SIZE=$((MAXFREE / 1024)) @@ -3128,7 +3129,11 @@ test_78() { # bug 10901 [ $F78SIZE -gt $((SMALLESTOST * $OSTCOUNT / 1024)) ] && \ F78SIZE=$((SMALLESTOST * $OSTCOUNT / 1024)) $SETSTRIPE $DIR/$tfile 0 -1 -1 || error "setstripe failed" - $DIRECTIO rdwr $DIR/$tfile 0 $F78SIZE 1048576 || error "rdwr failed" + for i in `seq 1 $NSEQ` + do + echo directIO rdwr round $i of $NSEQ + $DIRECTIO rdwr $DIR/$tfile 0 $F78SIZE 1048576 || error "rdwr failed" + done rm -f $DIR/$tfile } @@ -3823,6 +3828,34 @@ test_118() #bug 11710 } run_test 118 "verify O_SYNC work" +test_119a() # bug 11737 +{ + BSIZE=$((512 * 1024)) + directio write $DIR/$tfile 0 1 $BSIZE + # We ask to read two blocks, which is more than a file size. + # directio will indicate an error when requested and actual + # sizes aren't equeal (a normal situation in this case) and + # print actual read amount. + NOB=`directio read $DIR/$tfile 0 2 $BSIZE | awk '/error/ {print $6}'` + if [ "$NOB" != "$BSIZE" ]; then + error "read $NOB bytes instead of $BSIZE" + fi + rm -f $DIR/$tfile +} +run_test 119a "Short directIO read must return actual read amount" + +test_119b() # bug 11737 +{ + [ "$OSTCOUNT" -lt "2" ] && echo "skipping 2-stripe test" && return + + lfs setstripe $DIR/$tfile 0 -1 2 + dd if=/dev/zero of=$DIR/$tfile bs=1M count=1 seek=1 || error "dd failed" + sync + multiop $DIR/$tfile oO_RDONLY:O_DIRECT:r$((2048 * 1024)) || \ + error "direct read failed" +} +run_test 119b "Sparse directIO read must return actual read amount" + TMPDIR=$OLDTMPDIR TMP=$OLDTMP HOME=$OLDHOME -- 1.8.3.1