From 500a1d77a49ed18fcf36a87258805827a8350085 Mon Sep 17 00:00:00 2001 From: Johann Lombardi Date: Thu, 24 Jun 2010 10:43:00 +0200 Subject: [PATCH] b=11742 OBD_FL_MMAP should only be used in conjunction with OBD_MD_FLFLAGS i=adilger i=dmitry The patch from bug 11742 uses oa.o_flags without setting OBD_MD_FLFLAGS in oa.o_valid. This is wrong since anybody can legitimately reset o_flags if OBD_MD_FLFLAGS is not set (e.g. the checksum code does this). --- lustre/llite/rw.c | 10 ++++++++-- lustre/osc/osc_request.c | 5 +++-- lustre/ost/ost_handler.c | 14 +++++++------- 3 files changed, 18 insertions(+), 11 deletions(-) diff --git a/lustre/llite/rw.c b/lustre/llite/rw.c index 688fa37..acd9264 100644 --- a/lustre/llite/rw.c +++ b/lustre/llite/rw.c @@ -493,8 +493,14 @@ void ll_inode_fill_obdo(struct inode *inode, int cmd, struct obdo *oa) obdo_from_inode(oa, inode, valid_flags); /* Bug11742 - set the OBD_FL_MMAP flag for memory mapped files */ - if (atomic_read(&(ll_i2info(inode)->lli_mmap_cnt)) != 0) - oa->o_flags |= OBD_FL_MMAP; + if (atomic_read(&(ll_i2info(inode)->lli_mmap_cnt)) != 0) { + if (!(oa->o_valid & OBD_MD_FLFLAGS)) { + oa->o_valid |= OBD_MD_FLFLAGS; + oa->o_flags = OBD_FL_MMAP; + } else { + oa->o_flags |= OBD_FL_MMAP; + } + } } static void ll_ap_fill_obdo(void *data, int cmd, struct obdo *oa) diff --git a/lustre/osc/osc_request.c b/lustre/osc/osc_request.c index 9b31f01..b839d2f 100644 --- a/lustre/osc/osc_request.c +++ b/lustre/osc/osc_request.c @@ -1370,7 +1370,7 @@ static int check_write_checksum(struct obdo *oa, const lnet_process_id_t *peer, } /* If this is mmaped file - it can be changed at any time */ - if (oa->o_flags & OBD_FL_MMAP) + if (oa->o_valid & OBD_MD_FLFLAGS && oa->o_flags & OBD_FL_MMAP) return 1; if (oa->o_valid & OBD_MD_FLFLAGS) @@ -2255,7 +2255,8 @@ static int brw_interpret(struct ptlrpc_request *request, void *data, int rc) * on the network, that was not caused by mmap() modifying * the page. bug 11742 */ if ((rc == -EAGAIN) && (aa->aa_resends > 0) && - (aa->aa_oa->o_flags & OBD_FL_MMAP)) { + aa->aa_oa->o_valid & OBD_MD_FLFLAGS && + aa->aa_oa->o_flags & OBD_FL_MMAP) { rc = 0; } else { rc = osc_brw_redo_request(request, aa); diff --git a/lustre/ost/ost_handler.c b/lustre/ost/ost_handler.c index c057c79..7cbb0e0 100644 --- a/lustre/ost/ost_handler.c +++ b/lustre/ost/ost_handler.c @@ -905,7 +905,7 @@ static int ost_brw_write(struct ptlrpc_request *req, struct obd_trans_info *oti) int rc, i, j; obd_count client_cksum = 0, server_cksum = 0; cksum_type_t cksum_type = OBD_CKSUM_CRC32; - int no_reply = 0; + int no_reply = 0, mmap = 0; struct ost_thread_local_cache *tls; ENTRY; @@ -993,6 +993,8 @@ static int ost_brw_write(struct ptlrpc_request *req, struct obd_trans_info *oti) if (body->oa.o_valid & OBD_MD_FLFLAGS) cksum_type = cksum_type_unpack(body->oa.o_flags); } + if (body->oa.o_valid & OBD_MD_FLFLAGS && body->oa.o_flags & OBD_FL_MMAP) + mmap = 1; /* Because we already sync grant info with client when reconnect, * grant info will be cleared for resent req, then fed_grant and @@ -1090,10 +1092,9 @@ static int ost_brw_write(struct ptlrpc_request *req, struct obd_trans_info *oti) repbody->oa.o_cksum = server_cksum; cksum_counter++; if (unlikely(client_cksum != server_cksum)) { - CDEBUG_LIMIT((body->oa.o_flags&OBD_FL_MMAP) ? D_INFO - : D_ERROR, - "client csum %x, server csum %x\n", - client_cksum, server_cksum); + CDEBUG_LIMIT(mmap ? D_INFO : D_ERROR, + "client csum %x, server csum %x\n", + client_cksum, server_cksum); cksum_counter = 0; } else if ((cksum_counter & (-cksum_counter)) == cksum_counter){ CDEBUG(D_INFO, "Checksum %u from %s OK: %x\n", @@ -1130,8 +1131,7 @@ static int ost_brw_write(struct ptlrpc_request *req, struct obd_trans_info *oti) */ repbody->oa.o_valid &= ~(OBD_MD_FLMTIME | OBD_MD_FLATIME); - if (unlikely(client_cksum != server_cksum && rc == 0 && - !(body->oa.o_flags & OBD_FL_MMAP))) { + if (unlikely(client_cksum != server_cksum && rc == 0 && !mmap)) { int new_cksum = ost_checksum_bulk(desc, OST_WRITE, cksum_type); char *msg; char *via; -- 1.8.3.1