From 25058cb821ae2f880156ed9e9b8cd23f7bdc945c Mon Sep 17 00:00:00 2001 From: deen Date: Wed, 23 May 2007 23:46:28 +0000 Subject: [PATCH] Direct I/O operations should return actual amount of bytes transferred rather than requested size. b=11737 i=green 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 | 14 ++++++++++++++ 8 files changed, 43 insertions(+), 16 deletions(-) diff --git a/lustre/ChangeLog b/lustre/ChangeLog index 9f4148d..88c3df1 100644 --- a/lustre/ChangeLog +++ b/lustre/ChangeLog @@ -152,6 +152,13 @@ Description: random memory corruption Details : size of struct ll_inode_info is to big for union inode.u and this can be cause of random memory corruption. +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 e6e7d21..fc731cb 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 508636b..646dedc 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, @@ -113,13 +114,10 @@ static int ll_direct_IO_24(int rw, 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 4fb767f..eac8c84 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 e7aa732..31a54ad 100644 --- a/lustre/osc/osc_request.c +++ b/lustre/osc/osc_request.c @@ -1086,6 +1086,9 @@ static int osc_brw_fini_request(struct ptlrpc_request *req, int rc) if (rc < 0) RETURN(rc); + if (req->rq_set && req->rq_set->set_countp) + atomic_add(rc, (atomic_t *)req->rq_set->set_countp); + if (unlikely(aa->aa_oa->o_valid & OBD_MD_FLCKSUM)) client_cksum = aa->aa_oa->o_cksum; /* save for later */ 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 0d71a90..759e7cb 100644 --- a/lustre/tests/sanity.sh +++ b/lustre/tests/sanity.sh @@ -3796,6 +3796,20 @@ test_118() #bug 11710 } run_test 118 "verify O_SYNC work" +test_119() # bug 11737 +{ + dd if=/dev/zero of=$DIR/$tfile bs=4k count=2 + sync + # we ask to read 3 blocks -- more than a file size + NOB=`directio read $DIR/$tfile 0 3 | awk '/error/ {print $6}'` + # check if we have read num of bytes not equal to file size (8k) + if [ "$NOB" != "8192" ]; then + error "read $NOB bytes instead of 8192" + fi + rm -f $DIR/$tfile +} +run_test 119 "Short directIO read must return actual read amount" + TMPDIR=$OLDTMPDIR TMP=$OLDTMP HOME=$OLDHOME -- 1.8.3.1