From c75184b8a14ee686dacbf2dcf01eeade0327b648 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 5 Apr 2010 14:16:21 -0700 Subject: Fix issue with aio where r/w lock wasn't kept across aio read operations. Change schedule_aio_read_and_X/schedule_aio_write_and_X to return NTSTATUS. Move the grant and release of the lock into the aio code. Jeremy --- source3/smbd/aio.c | 79 +++++++++++++++++++++++++++++++++++++--------------- source3/smbd/reply.c | 78 ++++++++++++++++++++++++++++++++++----------------- 2 files changed, 108 insertions(+), 49 deletions(-) (limited to 'source3/smbd') diff --git a/source3/smbd/aio.c b/source3/smbd/aio.c index a04f3d55d7..e172b27d5a 100644 --- a/source3/smbd/aio.c +++ b/source3/smbd/aio.c @@ -81,6 +81,7 @@ struct aio_extra { files_struct *fsp; struct smb_request *req; char *outbuf; + struct lock_struct lock; int (*handle_completion)(struct aio_extra *ex, int errcode); }; @@ -145,7 +146,7 @@ static struct aio_extra *find_aio_ex(uint16 mid) Set up an aio request from a SMBreadX call. *****************************************************************************/ -bool schedule_aio_read_and_X(connection_struct *conn, +NTSTATUS schedule_aio_read_and_X(connection_struct *conn, struct smb_request *req, files_struct *fsp, SMB_OFF_T startpos, size_t smb_maxcnt) @@ -159,7 +160,7 @@ bool schedule_aio_read_and_X(connection_struct *conn, if (fsp->base_fsp != NULL) { /* No AIO on streams yet */ DEBUG(10, ("AIO on streams not yet supported\n")); - return false; + return NT_STATUS_RETRY; } if ((!min_aio_read_size || (smb_maxcnt < min_aio_read_size)) @@ -169,20 +170,20 @@ bool schedule_aio_read_and_X(connection_struct *conn, "for minimum aio_read of %u\n", (unsigned int)smb_maxcnt, (unsigned int)min_aio_read_size )); - return False; + return NT_STATUS_RETRY; } /* Only do this on non-chained and non-chaining reads not using the * write cache. */ if (req_is_in_chain(req) || (lp_write_cache_size(SNUM(conn)) != 0)) { - return False; + return NT_STATUS_RETRY; } if (outstanding_aio_calls >= aio_pending_size) { DEBUG(10,("schedule_aio_read_and_X: Already have %d aio " "activities outstanding.\n", outstanding_aio_calls )); - return False; + return NT_STATUS_RETRY; } /* The following is safe from integer wrap as we've already checked @@ -195,7 +196,7 @@ bool schedule_aio_read_and_X(connection_struct *conn, if ((aio_ex = create_aio_extra(fsp, bufsize)) == NULL) { DEBUG(10,("schedule_aio_read_and_X: malloc fail.\n")); - return False; + return NT_STATUS_NO_MEMORY; } aio_ex->handle_completion = handle_aio_read_complete; @@ -203,6 +204,16 @@ bool schedule_aio_read_and_X(connection_struct *conn, srv_set_message(aio_ex->outbuf, 12, 0, True); SCVAL(aio_ex->outbuf,smb_vwv0,0xFF); /* Never a chained reply. */ + init_strict_lock_struct(fsp, (uint32)req->smbpid, + (uint64_t)startpos, (uint64_t)smb_maxcnt, READ_LOCK, + &aio_ex->lock); + + /* Take the lock until the AIO completes. */ + if (!SMB_VFS_STRICT_LOCK(conn, fsp, &aio_ex->lock)) { + TALLOC_FREE(aio_ex); + return NT_STATUS_FILE_LOCK_CONFLICT; + } + a = &aio_ex->acb; /* Now set up the aio record for the read call. */ @@ -219,8 +230,9 @@ bool schedule_aio_read_and_X(connection_struct *conn, if (ret == -1) { DEBUG(0,("schedule_aio_read_and_X: aio_read failed. " "Error %s\n", strerror(errno) )); + SMB_VFS_STRICT_UNLOCK(conn, fsp, &aio_ex->lock); TALLOC_FREE(aio_ex); - return False; + return NT_STATUS_RETRY; } outstanding_aio_calls++; @@ -231,14 +243,14 @@ bool schedule_aio_read_and_X(connection_struct *conn, fsp_str_dbg(fsp), (double)startpos, (unsigned int)smb_maxcnt, (unsigned int)aio_ex->req->mid )); - return True; + return NT_STATUS_OK; } /**************************************************************************** Set up an aio request from a SMBwriteX call. *****************************************************************************/ -bool schedule_aio_write_and_X(connection_struct *conn, +NTSTATUS schedule_aio_write_and_X(connection_struct *conn, struct smb_request *req, files_struct *fsp, char *data, SMB_OFF_T startpos, @@ -254,7 +266,7 @@ bool schedule_aio_write_and_X(connection_struct *conn, if (fsp->base_fsp != NULL) { /* No AIO on streams yet */ DEBUG(10, ("AIO on streams not yet supported\n")); - return false; + return NT_STATUS_RETRY; } if ((!min_aio_write_size || (numtowrite < min_aio_write_size)) @@ -264,13 +276,13 @@ bool schedule_aio_write_and_X(connection_struct *conn, "small for minimum aio_write of %u\n", (unsigned int)numtowrite, (unsigned int)min_aio_write_size )); - return False; + return NT_STATUS_RETRY; } /* Only do this on non-chained and non-chaining reads not using the * write cache. */ if (req_is_in_chain(req) || (lp_write_cache_size(SNUM(conn)) != 0)) { - return False; + return NT_STATUS_RETRY; } if (outstanding_aio_calls >= aio_pending_size) { @@ -283,7 +295,7 @@ bool schedule_aio_write_and_X(connection_struct *conn, fsp_str_dbg(fsp), (double)startpos, (unsigned int)numtowrite, (unsigned int)req->mid )); - return False; + return NT_STATUS_RETRY; } /* Ensure aio is initialized. */ @@ -293,7 +305,7 @@ bool schedule_aio_write_and_X(connection_struct *conn, if (!(aio_ex = create_aio_extra(fsp, bufsize))) { DEBUG(0,("schedule_aio_write_and_X: malloc fail.\n")); - return False; + return NT_STATUS_NO_MEMORY; } aio_ex->handle_completion = handle_aio_write_complete; @@ -301,6 +313,16 @@ bool schedule_aio_write_and_X(connection_struct *conn, srv_set_message(aio_ex->outbuf, 6, 0, True); SCVAL(aio_ex->outbuf,smb_vwv0,0xFF); /* Never a chained reply. */ + init_strict_lock_struct(fsp, (uint32)req->smbpid, + (uint64_t)startpos, (uint64_t)numtowrite, WRITE_LOCK, + &aio_ex->lock); + + /* Take the lock until the AIO completes. */ + if (!SMB_VFS_STRICT_LOCK(conn, fsp, &aio_ex->lock)) { + TALLOC_FREE(aio_ex); + return NT_STATUS_FILE_LOCK_CONFLICT; + } + a = &aio_ex->acb; /* Now set up the aio record for the write call. */ @@ -317,8 +339,9 @@ bool schedule_aio_write_and_X(connection_struct *conn, if (ret == -1) { DEBUG(3,("schedule_aio_wrote_and_X: aio_write failed. " "Error %s\n", strerror(errno) )); + SMB_VFS_STRICT_UNLOCK(conn, fsp, &aio_ex->lock); TALLOC_FREE(aio_ex); - return False; + return NT_STATUS_RETRY; } outstanding_aio_calls++; @@ -352,10 +375,9 @@ bool schedule_aio_write_and_X(connection_struct *conn, fsp_str_dbg(fsp), (double)startpos, (unsigned int)numtowrite, (unsigned int)aio_ex->req->mid, outstanding_aio_calls )); - return True; + return NT_STATUS_OK; } - /**************************************************************************** Complete the read and return the data or error back to the client. Returns errno or zero if all ok. @@ -511,6 +533,7 @@ static int handle_aio_write_complete(struct aio_extra *aio_ex, int errcode) static bool handle_aio_completed(struct aio_extra *aio_ex, int *perr) { + files_struct *fsp = NULL; int err; if(!aio_ex) { @@ -518,14 +541,21 @@ static bool handle_aio_completed(struct aio_extra *aio_ex, int *perr) return false; } + fsp = aio_ex->fsp; + /* Ensure the operation has really completed. */ - err = SMB_VFS_AIO_ERROR(aio_ex->fsp, &aio_ex->acb); + err = SMB_VFS_AIO_ERROR(fsp, &aio_ex->acb); if (err == EINPROGRESS) { DEBUG(10,( "handle_aio_completed: operation mid %u still in " "process for file %s\n", aio_ex->req->mid, fsp_str_dbg(aio_ex->fsp))); return False; - } else if (err == ECANCELED) { + } + + /* Unlock now we're done. */ + SMB_VFS_STRICT_UNLOCK(fsp->conn, fsp, &aio_ex->lock); + + if (err == ECANCELED) { /* If error is ECANCELED then don't return anything to the * client. */ DEBUG(10,( "handle_aio_completed: operation mid %u" @@ -696,6 +726,9 @@ void cancel_aio_by_fsp(files_struct *fsp) for( aio_ex = aio_list_head; aio_ex; aio_ex = aio_ex->next) { if (aio_ex->fsp == fsp) { + /* Unlock now we're done. */ + SMB_VFS_STRICT_UNLOCK(fsp->conn, fsp, &aio_ex->lock); + /* Don't delete the aio_extra record as we may have completed and don't yet know it. Just do the aio_cancel call and return. */ @@ -707,21 +740,21 @@ void cancel_aio_by_fsp(files_struct *fsp) } #else -bool schedule_aio_read_and_X(connection_struct *conn, +NTSTATUS schedule_aio_read_and_X(connection_struct *conn, struct smb_request *req, files_struct *fsp, SMB_OFF_T startpos, size_t smb_maxcnt) { - return False; + return NT_STATUS_RETRY; } -bool schedule_aio_write_and_X(connection_struct *conn, +NTSTATUS schedule_aio_write_and_X(connection_struct *conn, struct smb_request *req, files_struct *fsp, char *data, SMB_OFF_T startpos, size_t numtowrite) { - return False; + return NT_STATUS_RETRY; } void cancel_aio_by_fsp(files_struct *fsp) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index e2aca3793a..0355cb7083 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -3698,9 +3698,23 @@ void reply_read_and_X(struct smb_request *req) } - if (!big_readX && - schedule_aio_read_and_X(conn, req, fsp, startpos, smb_maxcnt)) { - goto out; + if (!big_readX) { + NTSTATUS status = schedule_aio_read_and_X(conn, + req, + fsp, + startpos, + smb_maxcnt); + if (NT_STATUS_IS_OK(status)) { + /* Read scheduled - we're done. */ + goto out; + } + if (!NT_STATUS_EQUAL(status, NT_STATUS_RETRY)) { + /* Real error - report to client. */ + END_PROFILE(SMBreadX); + reply_nterror(req, status); + return; + } + /* NT_STATUS_RETRY - fall back to sync read. */ } smbd_lock_socket(smbd_server_conn); @@ -4320,6 +4334,7 @@ void reply_write_and_X(struct smb_request *req) unsigned int smblen; char *data; NTSTATUS status; + int saved_errno = 0; START_PROFILE(SMBwriteX); @@ -4414,16 +4429,6 @@ void reply_write_and_X(struct smb_request *req) #endif /* LARGE_SMB_OFF_T */ } - init_strict_lock_struct(fsp, (uint32)req->smbpid, - (uint64_t)startpos, (uint64_t)numtowrite, WRITE_LOCK, - &lock); - - if (!SMB_VFS_STRICT_LOCK(conn, fsp, &lock)) { - reply_nterror(req, NT_STATUS_FILE_LOCK_CONFLICT); - END_PROFILE(SMBwriteX); - return; - } - /* X/Open SMB protocol says that, unlike SMBwrite if the length is zero then NO truncation is done, just a write of zero. To truncate a file, @@ -4432,24 +4437,49 @@ void reply_write_and_X(struct smb_request *req) if(numtowrite == 0) { nwritten = 0; } else { + if (req->unread_bytes == 0) { + status = schedule_aio_write_and_X(conn, + req, + fsp, + data, + startpos, + numtowrite); - if ((req->unread_bytes == 0) && - schedule_aio_write_and_X(conn, req, fsp, data, startpos, - numtowrite)) { - goto strict_unlock; + if (NT_STATUS_IS_OK(status)) { + /* write scheduled - we're done. */ + goto out; + } + if (!NT_STATUS_EQUAL(status, NT_STATUS_RETRY)) { + /* Real error - report to client. */ + reply_nterror(req, status); + goto out; + } + /* NT_STATUS_RETRY - fall through to sync write. */ + } + + init_strict_lock_struct(fsp, (uint32)req->smbpid, + (uint64_t)startpos, (uint64_t)numtowrite, WRITE_LOCK, + &lock); + + if (!SMB_VFS_STRICT_LOCK(conn, fsp, &lock)) { + reply_nterror(req, NT_STATUS_FILE_LOCK_CONFLICT); + goto out; } nwritten = write_file(req,fsp,data,startpos,numtowrite); + saved_errno = errno; + + SMB_VFS_STRICT_UNLOCK(conn, fsp, &lock); } if(nwritten < 0) { - reply_nterror(req, map_nt_error_from_unix(errno)); - goto strict_unlock; + reply_nterror(req, map_nt_error_from_unix(saved_errno)); + goto out; } if((nwritten == 0) && (numtowrite != 0)) { reply_nterror(req, NT_STATUS_DISK_FULL); - goto strict_unlock; + goto out; } reply_outbuf(req, 6, 0); @@ -4469,18 +4499,14 @@ void reply_write_and_X(struct smb_request *req) DEBUG(5,("reply_write_and_X: sync_file for %s returned %s\n", fsp_str_dbg(fsp), nt_errstr(status))); reply_nterror(req, status); - goto strict_unlock; + goto out; } - SMB_VFS_STRICT_UNLOCK(conn, fsp, &lock); - END_PROFILE(SMBwriteX); chain_reply(req); return; -strict_unlock: - SMB_VFS_STRICT_UNLOCK(conn, fsp, &lock); - +out: END_PROFILE(SMBwriteX); return; } -- cgit