From patchwork Sat Feb 21 01:21:05 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Steve French X-Patchwork-Id: 8235 Received: from lists.samba.org (mail.samba.org [66.70.73.150]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n1L1LYo2006432 for ; Sat, 21 Feb 2009 01:21:34 GMT Received: from dp.samba.org (localhost [127.0.0.1]) by lists.samba.org (Postfix) with ESMTP id 0633E163C56 for ; Sat, 21 Feb 2009 01:21:21 +0000 (GMT) X-Spam-Checker-Version: SpamAssassin 3.1.7 (2006-10-05) on dp.samba.org X-Spam-Level: X-Spam-Status: No, score=-3.2 required=3.8 tests=AWL,BAYES_00, DNS_FROM_RFC_POST,SPF_PASS autolearn=no version=3.1.7 X-Original-To: linux-cifs-client@lists.samba.org Delivered-To: linux-cifs-client@lists.samba.org Received: from mail-gx0-f178.google.com (mail-gx0-f178.google.com [209.85.217.178]) by lists.samba.org (Postfix) with ESMTP id 2A620163BC4 for ; Sat, 21 Feb 2009 01:20:52 +0000 (GMT) Received: by gxk26 with SMTP id 26so3217679gxk.20 for ; Fri, 20 Feb 2009 17:21:06 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:in-reply-to:references :date:message-id:subject:from:to:cc:content-type; bh=qTg8sJ3tH894fE13mqSF0FCCkvEUGKpIhpbv45/F/Vg=; b=J1vT5VcSCK5W7aYgn42vxUJcOIfFh1DVPCjjNkZdPU6xoy5nz1wmLCZazJW4tuPmt6 wHWDgKdHCHw4UUKuy4dtpR4fdiNGw1No29snwnZGCLmrTOsZZypzC+t+E54Q/cg2UCpb vyso4OTP0lq7othC2qUpmaLPsR+Vz/Y/EEid0= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; b=XIQdHIKgyQOLYFptWPwNnYAbUd47RcqAeUVIki8R4DZC/Ulm+8W0PPZNNx73j7DtOs t/payZNAsiwdqZstNBBvl7EhLnwxY5nkU36CpOfB6B8Gp449Y8AYFVN8+ZUTwkPxdzgd EZWcUQGX+C+7V09NUEywVBz0hx0bsa1+75uAY= MIME-Version: 1.0 Received: by 10.231.12.12 with SMTP id v12mr1802551ibv.43.1235179265672; Fri, 20 Feb 2009 17:21:05 -0800 (PST) In-Reply-To: References: Date: Fri, 20 Feb 2009 19:21:05 -0600 Message-ID: <524f69650902201721l1c7f0d34p4708980a90d7fc8f@mail.gmail.com> From: Steve French To: Horst Reiterer Cc: linux-cifs-client@lists.samba.org, linux-kernel@vger.kernel.org Subject: [linux-cifs-client] Re: [PATCH] fs/cifs: send SMB_COM_FLUSH in cifs_fsync X-BeenThere: linux-cifs-client@lists.samba.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: The Linux CIFS VFS client List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-cifs-client-bounces+patchwork-cifs-client=patchwork.kernel.org@lists.samba.org Errors-To: linux-cifs-client-bounces+patchwork-cifs-client=patchwork.kernel.org@lists.samba.org On Fri, Feb 20, 2009 at 2:51 PM, Horst Reiterer wrote: > Hi, > > In contrast to the now-obsolete smbfs, cifs does not send SMB_COM_FLUSH > in response to an explicit fsync(2) to guarantee that all volatile data > is written to stable storage on the server side, provided the server > honors the request (which, to my knowledge, is true for Windows and > Samba with 'strict sync' enabled). > This patch modifies the cifs_fsync implementation I modified your patch slightly to not lose the writeback rc in one case, and to change camel case pTcon to tcon and remove one unnecessary local variable. See attached. Thanks for the submission - looks fine otherwise. If you have any performance numbers before and after (with e.g. dbench, iozone, bonnie etc. or perhaps something which calls fsync more often - that would be helpful in determining whether we need a mount option to optionally disable it - as the samba server does) diff --git a/fs/cifs/CHANGES b/fs/cifs/CHANGES index 851388f..d43e0fe 100644 --- a/fs/cifs/CHANGES +++ b/fs/cifs/CHANGES @@ -6,7 +6,9 @@ the server to treat subsequent connections, especially those that are authenticated as guest, as reconnections, invalidating the earlier user's smb session. This fix allows cifs to mount multiple times to the same server with different userids without risking invalidating earlier -established security contexts. +established security contexts. fsync now sends SMB Flush operation +to better ensure that we wait for server to write all of the data to +server disk (not just write it over the network). Version 1.56 ------------ diff --git a/fs/cifs/cifs_debug.c b/fs/cifs/cifs_debug.c index 490e34b..877e4d9 100644 --- a/fs/cifs/cifs_debug.c +++ b/fs/cifs/cifs_debug.c @@ -340,6 +340,8 @@ static int cifs_stats_proc_show(struct seq_file *m, void *v) seq_printf(m, "\nWrites: %d Bytes: %lld", atomic_read(&tcon->num_writes), (long long)(tcon->bytes_written)); + seq_printf(m, "\nFlushes: %d", + atomic_read(&tcon->num_flushes)); seq_printf(m, "\nLocks: %d HardLinks: %d " "Symlinks: %d", atomic_read(&tcon->num_locks), diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h index e004f6d..44ff94d 100644 --- a/fs/cifs/cifsglob.h +++ b/fs/cifs/cifsglob.h @@ -254,6 +254,7 @@ struct cifsTconInfo { atomic_t num_smbs_sent; atomic_t num_writes; atomic_t num_reads; + atomic_t num_flushes; atomic_t num_oplock_brks; atomic_t num_opens; atomic_t num_closes; diff --git a/fs/cifs/cifspdu.h b/fs/cifs/cifspdu.h index b4e2e9f..eda6e51 100644 --- a/fs/cifs/cifspdu.h +++ b/fs/cifs/cifspdu.h @@ -43,6 +43,7 @@ #define SMB_COM_CREATE_DIRECTORY 0x00 /* trivial response */ #define SMB_COM_DELETE_DIRECTORY 0x01 /* trivial response */ #define SMB_COM_CLOSE 0x04 /* triv req/rsp, timestamp ignored */ +#define SMB_COM_FLUSH 0x05 /* triv req/rsp */ #define SMB_COM_DELETE 0x06 /* trivial response */ #define SMB_COM_RENAME 0x07 /* trivial response */ #define SMB_COM_QUERY_INFORMATION 0x08 /* aka getattr */ @@ -790,6 +791,12 @@ typedef struct smb_com_close_rsp { __u16 ByteCount; /* bct = 0 */ } __attribute__((packed)) CLOSE_RSP; +typedef struct smb_com_flush_req { + struct smb_hdr hdr; /* wct = 1 */ + __u16 FileID; + __u16 ByteCount; /* 0 */ +} __attribute__((packed)) FLUSH_REQ; + typedef struct smb_com_findclose_req { struct smb_hdr hdr; /* wct = 1 */ __u16 FileID; diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h index 083dfc5..596fc86 100644 --- a/fs/cifs/cifsproto.h +++ b/fs/cifs/cifsproto.h @@ -281,6 +281,9 @@ extern int CIFSPOSIXCreate(const int xid, struct cifsTconInfo *tcon, extern int CIFSSMBClose(const int xid, struct cifsTconInfo *tcon, const int smb_file_id); +extern int CIFSSMBFlush(const int xid, struct cifsTconInfo *tcon, + const int smb_file_id); + extern int CIFSSMBRead(const int xid, struct cifsTconInfo *tcon, const int netfid, unsigned int count, const __u64 lseek, unsigned int *nbytes, char **buf, diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c index 939e2f7..4c344fe 100644 --- a/fs/cifs/cifssmb.c +++ b/fs/cifs/cifssmb.c @@ -1934,6 +1934,27 @@ CIFSSMBClose(const int xid, struct cifsTconInfo *tcon, int smb_file_id) } int +CIFSSMBFlush(const int xid, struct cifsTconInfo *tcon, int smb_file_id) +{ + int rc = 0; + FLUSH_REQ *pSMB = NULL; + cFYI(1, ("In CIFSSMBFlush")); + + rc = small_smb_init(SMB_COM_FLUSH, 1, tcon, (void **) &pSMB); + if (rc) + return rc; + + pSMB->FileID = (__u16) smb_file_id; + pSMB->ByteCount = 0; + rc = SendReceiveNoRsp(xid, tcon->ses, (struct smb_hdr *) pSMB, 0); + cifs_stats_inc(&tcon->num_flushes); + if (rc) + cERROR(1, ("Send error in Flush = %d", rc)); + + return rc; +} + +int CIFSSMBRename(const int xid, struct cifsTconInfo *tcon, const char *fromName, const char *toName, const struct nls_table *nls_codepage, int remap) diff --git a/fs/cifs/file.c b/fs/cifs/file.c index 12bb656..83b4741 100644 --- a/fs/cifs/file.c +++ b/fs/cifs/file.c @@ -1523,6 +1523,9 @@ int cifs_fsync(struct file *file, struct dentry *dentry, int datasync) { int xid; int rc = 0; + struct cifsTconInfo *tcon; + struct cifsFileInfo *smbfile = + (struct cifsFileInfo *)file->private_data; struct inode *inode = file->f_path.dentry->d_inode; xid = GetXid(); @@ -1534,7 +1537,11 @@ int cifs_fsync(struct file *file, struct dentry *dentry, int datasync) if (rc == 0) { rc = CIFS_I(inode)->write_behind_rc; CIFS_I(inode)->write_behind_rc = 0; + tcon = CIFS_SB(inode->i_sb)->tcon; + if (!rc && tcon && smbfile) + rc = CIFSSMBFlush(xid, tcon, smbfile->netfid); } + FreeXid(xid); return rc; }