From patchwork Fri Feb 7 16:02:41 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Layton X-Patchwork-Id: 3606351 Return-Path: X-Original-To: patchwork-cifs-client@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id C1949BF418 for ; Fri, 7 Feb 2014 16:02:51 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id BE6352012B for ; Fri, 7 Feb 2014 16:02:50 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 9291320125 for ; Fri, 7 Feb 2014 16:02:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752126AbaBGQCt (ORCPT ); Fri, 7 Feb 2014 11:02:49 -0500 Received: from mail-qa0-f50.google.com ([209.85.216.50]:63230 "EHLO mail-qa0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751785AbaBGQCs (ORCPT ); Fri, 7 Feb 2014 11:02:48 -0500 Received: by mail-qa0-f50.google.com with SMTP id cm18so5617505qab.9 for ; Fri, 07 Feb 2014 08:02:47 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:sender:from:to:cc:subject:date:message-id :in-reply-to:references; bh=Ix5N8H37QKUfCBRUJXfUic2j+D0QFHRbY7RWY53zb3A=; b=dpAsSufsNIXYG6zjuENejlPP+T5NKKHwPY/QciwdIWvq5AvewkoVG37Bm00wWTTeB1 DUjCb2LWZi0DibaPwbIySr+KouGXz7FXDqQnPFJ1fU8+dqm7r72V0IYOVoYSR2lMyZFK 3BRJlbC6fjZv15d/Y/JpSX31DpiPGo6ltoma0pz7YBuenuwQvhpYr+30r6Gy0eTDibgc rw/pEY7/2TabfI4OzTgBV6ZqF0TO72DWpknsHXaGGDgytb4I6C/G3hmVKMRQ1kXYH3od YGyMPUZFRHJY+oWgfVqvX8ufP0jGbuoyjwcZ8pGnxiQaj6ME8Z4Dq4tEDKXfVFECq0kb kmlg== X-Gm-Message-State: ALoCoQkH1nc0Sre5nQusOGTgpXlU0RLAy0YXMGOjTRBn74f7SVUFrLdGy/uM8+fSFhwoTaiSywaL X-Received: by 10.224.14.70 with SMTP id f6mr23588923qaa.31.1391788967370; Fri, 07 Feb 2014 08:02:47 -0800 (PST) Received: from tlielax.poochiereds.net ([2001:470:8:d63:3a60:77ff:fe93:a95d]) by mx.google.com with ESMTPSA id k1sm14523210qat.16.2014.02.07.08.02.46 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 07 Feb 2014 08:02:46 -0800 (PST) From: Jeff Layton To: smfrench@gmail.com Cc: linux-cifs@vger.kernel.org Subject: [PATCH v2] cifs: clean up page array when uncached write send fails Date: Fri, 7 Feb 2014 11:02:41 -0500 Message-Id: <1391788961-26028-1-git-send-email-jlayton@redhat.com> X-Mailer: git-send-email 1.8.5.3 In-Reply-To: <1391715764-21225-1-git-send-email-jlayton@redhat.com> References: <1391715764-21225-1-git-send-email-jlayton@redhat.com> Sender: linux-cifs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-cifs@vger.kernel.org X-Spam-Status: No, score=-7.4 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP In the event that a send fails in an uncached write, or we end up needing to reissue it (-EAGAIN case), we'll kfree the wdata but the pages currently leak. Fix this by adding a new kref release routine for uncached writedata that releases the pages, and have the uncached codepaths use that. Signed-off-by: Jeff Layton Reviewed-by: Pavel Shilovsky --- fs/cifs/cifsglob.h | 2 +- fs/cifs/cifsproto.h | 2 +- fs/cifs/cifssmb.c | 6 +++--- fs/cifs/file.c | 28 +++++++++++++++++----------- fs/cifs/smb2pdu.c | 4 ++-- fs/cifs/smb2proto.h | 2 +- 6 files changed, 25 insertions(+), 19 deletions(-) diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h index a245d1809ed8..57d07e704d97 100644 --- a/fs/cifs/cifsglob.h +++ b/fs/cifs/cifsglob.h @@ -323,7 +323,7 @@ struct smb_version_operations { /* async read from the server */ int (*async_readv)(struct cifs_readdata *); /* async write to the server */ - int (*async_writev)(struct cifs_writedata *); + int (*async_writev)(struct cifs_writedata *, void (*release)(struct kref *)); /* sync read from the server */ int (*sync_read)(const unsigned int, struct cifsFileInfo *, struct cifs_io_parms *, unsigned int *, char **, diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h index 79e6e9a93a8c..a4f90c0d9950 100644 --- a/fs/cifs/cifsproto.h +++ b/fs/cifs/cifsproto.h @@ -488,7 +488,7 @@ void cifs_readdata_release(struct kref *refcount); int cifs_async_readv(struct cifs_readdata *rdata); int cifs_readv_receive(struct TCP_Server_Info *server, struct mid_q_entry *mid); -int cifs_async_writev(struct cifs_writedata *wdata); +int cifs_async_writev(struct cifs_writedata *wdata, void (*release)(struct kref *kref)); void cifs_writev_complete(struct work_struct *work); struct cifs_writedata *cifs_writedata_alloc(unsigned int nr_pages, work_func_t complete); diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c index 4d881c35eeca..0726ab413b8c 100644 --- a/fs/cifs/cifssmb.c +++ b/fs/cifs/cifssmb.c @@ -1910,7 +1910,7 @@ cifs_writev_requeue(struct cifs_writedata *wdata) do { server = tlink_tcon(wdata->cfile->tlink)->ses->server; - rc = server->ops->async_writev(wdata); + rc = server->ops->async_writev(wdata, cifs_writedata_release); } while (rc == -EAGAIN); for (i = 0; i < wdata->nr_pages; i++) { @@ -2031,7 +2031,7 @@ cifs_writev_callback(struct mid_q_entry *mid) /* cifs_async_writev - send an async write, and set up mid to handle result */ int -cifs_async_writev(struct cifs_writedata *wdata) +cifs_async_writev(struct cifs_writedata *wdata, void (*release)(struct kref *kref)) { int rc = -EACCES; WRITE_REQ *smb = NULL; @@ -2105,7 +2105,7 @@ cifs_async_writev(struct cifs_writedata *wdata) if (rc == 0) cifs_stats_inc(&tcon->stats.cifs_stats.num_writes); else - kref_put(&wdata->refcount, cifs_writedata_release); + kref_put(&wdata->refcount, release); async_writev_out: cifs_small_buf_release(smb); diff --git a/fs/cifs/file.c b/fs/cifs/file.c index 853d6d1cc822..03d1f454c713 100644 --- a/fs/cifs/file.c +++ b/fs/cifs/file.c @@ -2043,7 +2043,7 @@ retry: } wdata->pid = wdata->cfile->pid; server = tlink_tcon(wdata->cfile->tlink)->ses->server; - rc = server->ops->async_writev(wdata); + rc = server->ops->async_writev(wdata, cifs_writedata_release); } while (wbc->sync_mode == WB_SYNC_ALL && rc == -EAGAIN); for (i = 0; i < nr_pages; ++i) @@ -2331,9 +2331,20 @@ size_t get_numpages(const size_t wsize, const size_t len, size_t *cur_len) } static void -cifs_uncached_writev_complete(struct work_struct *work) +cifs_uncached_writedata_release(struct kref *refcount) { int i; + struct cifs_writedata *wdata = container_of(refcount, + struct cifs_writedata, refcount); + + for (i = 0; i < wdata->nr_pages; i++) + put_page(wdata->pages[i]); + cifs_writedata_release(refcount); +} + +static void +cifs_uncached_writev_complete(struct work_struct *work) +{ struct cifs_writedata *wdata = container_of(work, struct cifs_writedata, work); struct inode *inode = wdata->cfile->dentry->d_inode; @@ -2347,12 +2358,7 @@ cifs_uncached_writev_complete(struct work_struct *work) complete(&wdata->done); - if (wdata->result != -EAGAIN) { - for (i = 0; i < wdata->nr_pages; i++) - put_page(wdata->pages[i]); - } - - kref_put(&wdata->refcount, cifs_writedata_release); + kref_put(&wdata->refcount, cifs_uncached_writedata_release); } /* attempt to send write to server, retry on any -EAGAIN errors */ @@ -2370,7 +2376,7 @@ cifs_uncached_retry_writev(struct cifs_writedata *wdata) if (rc != 0) continue; } - rc = server->ops->async_writev(wdata); + rc = server->ops->async_writev(wdata, cifs_uncached_writedata_release); } while (rc == -EAGAIN); return rc; @@ -2454,7 +2460,7 @@ cifs_iovec_write(struct file *file, const struct iovec *iov, wdata->tailsz = cur_len - ((nr_pages - 1) * PAGE_SIZE); rc = cifs_uncached_retry_writev(wdata); if (rc) { - kref_put(&wdata->refcount, cifs_writedata_release); + kref_put(&wdata->refcount, cifs_uncached_writedata_release); break; } @@ -2496,7 +2502,7 @@ restart_loop: } } list_del_init(&wdata->list); - kref_put(&wdata->refcount, cifs_writedata_release); + kref_put(&wdata->refcount, cifs_uncached_writedata_release); } if (total_written > 0) diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c index 2013234b73ad..8c04bdeda136 100644 --- a/fs/cifs/smb2pdu.c +++ b/fs/cifs/smb2pdu.c @@ -1890,7 +1890,7 @@ smb2_writev_callback(struct mid_q_entry *mid) /* smb2_async_writev - send an async write, and set up mid to handle result */ int -smb2_async_writev(struct cifs_writedata *wdata) +smb2_async_writev(struct cifs_writedata *wdata, void (*release)(struct kref *kref)) { int rc = -EACCES; struct smb2_write_req *req = NULL; @@ -1938,7 +1938,7 @@ smb2_async_writev(struct cifs_writedata *wdata) smb2_writev_callback, wdata, 0); if (rc) { - kref_put(&wdata->refcount, cifs_writedata_release); + kref_put(&wdata->refcount, release); cifs_stats_fail_inc(tcon, SMB2_WRITE_HE); } diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h index 93adc64666f3..1833724542cd 100644 --- a/fs/cifs/smb2proto.h +++ b/fs/cifs/smb2proto.h @@ -123,7 +123,7 @@ extern int SMB2_get_srv_num(const unsigned int xid, struct cifs_tcon *tcon, extern int smb2_async_readv(struct cifs_readdata *rdata); extern int SMB2_read(const unsigned int xid, struct cifs_io_parms *io_parms, unsigned int *nbytes, char **buf, int *buf_type); -extern int smb2_async_writev(struct cifs_writedata *wdata); +extern int smb2_async_writev(struct cifs_writedata *wdata, void (*release)(struct kref *kref)); extern int SMB2_write(const unsigned int xid, struct cifs_io_parms *io_parms, unsigned int *nbytes, struct kvec *iov, int n_vec); extern int SMB2_echo(struct TCP_Server_Info *server);