From patchwork Fri Feb 1 20:04:11 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pavel Shilovsky X-Patchwork-Id: 1035144 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=linux-cifs-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="LXfkyAxV"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 43rp3s0pgjz9s6w for ; Sat, 2 Feb 2019 07:04:57 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730907AbfBAUE4 (ORCPT ); Fri, 1 Feb 2019 15:04:56 -0500 Received: from mail-pl1-f193.google.com ([209.85.214.193]:34149 "EHLO mail-pl1-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730787AbfBAUE4 (ORCPT ); Fri, 1 Feb 2019 15:04:56 -0500 Received: by mail-pl1-f193.google.com with SMTP id w4so3755715plz.1 for ; Fri, 01 Feb 2019 12:04:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:subject:date:message-id:in-reply-to:references; bh=eeODu+Anqq7007oRLFqaUWtcfY2hLJg5RcOxDwuXI7g=; b=LXfkyAxVwTCMS4JZwBxHtPNFV0RNZ8fuXrXMmO7zxoCdzkWM1mOhxKbmADbtjMoJwD y27haodbUV9WTlX09four8vVdOzAYJ+QPz2SaYabNCgEryQMYSPjRo8Jg/wZ/7TnqN7T 0cfM01IqKpo02AOf/dGUqpGIuXk5wDRg9/mwrfScyym4stH9nUfUPkqqgTtqBBClor+F LDcSMdARUwvixx1Q7eNvymcRMxMKteNjyNx1oR00n8XW4x2WYWsP9X6q6TtDqMQZ20Ws 0RPlcQEtp9vzDH2UH2wBikxUiHv5n5kLjpW85I3z1dD63DkMpl2jXHgYQNgq7erm3y6G byYA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references; bh=eeODu+Anqq7007oRLFqaUWtcfY2hLJg5RcOxDwuXI7g=; b=nDTGlV9DqYxqfYCyRfXU2025MziJLn5AbD7wr2IJyuT/CyUW+yJGzeE8lnlhKgYKOA L+bSBe0Rwi0GNhxhB1jx4bPKPKq41N6ACrwT6taiAkqWQ+7x8zYbJSr/yobC78tZz+/j bo2YCK1aBbUpnboOry48qoGP+zD4YwOdbw5TydPanZdeUvqvY0sWkAVfXMOFeJpsxsKY pBMfBuvsCUzKYxBr1bWJb0//bTYBv4GUQPnN++xHgeRF2rHFVtMdTq6g0fmDpf8V1XhX lZngtjo1lcPNPBv4gPUVqUZqvjQvd33y069X/wUy9judZ3yhQupM0HE6zxUAlIedNVCz St1w== X-Gm-Message-State: AJcUukeQIs9pVAhjeGvtPsykrasGW+kVSXUcecd09p23XJm+GQuNbNl7 3fN49X4aWJ7GdurYdquJX0LVMvE= X-Google-Smtp-Source: ALg8bN48Cmc2ZeK3aQ60CitzixNxKe6oWXvmv4728tSBgPie2CrWy0nhCkjW4fyhJJAGBLbD34bwew== X-Received: by 2002:a17:902:d891:: with SMTP id b17mr41621319plz.80.1549051495118; Fri, 01 Feb 2019 12:04:55 -0800 (PST) Received: from ubuntu-vm.corp.microsoft.com ([2001:4898:80e8:0:a18e:4e9f:6b7c:507d]) by smtp.gmail.com with ESMTPSA id x11sm24247451pfe.72.2019.02.01.12.04.52 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Fri, 01 Feb 2019 12:04:53 -0800 (PST) From: Pavel Shilovsky X-Google-Original-From: Pavel Shilovsky To: linux-cifs@vger.kernel.org Subject: [PATCH 22/23] CIFS: Return error code when getting file handle for writeback Date: Fri, 1 Feb 2019 12:04:11 -0800 Message-Id: <1549051452-5968-24-git-send-email-pshilov@microsoft.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1549051452-5968-1-git-send-email-pshilov@microsoft.com> References: <1549051452-5968-1-git-send-email-pshilov@microsoft.com> Sender: linux-cifs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-cifs@vger.kernel.org Now we just return NULL cifsFileInfo pointer in cases we didn't find or couldn't reopen a file. This hides errors from cifs_reopen_file() especially retryable errors which should be handled appropriately. Create new cifs_get_writable_file() routine that returns error codes from cifs_reopen_file() and use it in the writeback codepath. Signed-off-by: Pavel Shilovsky --- fs/cifs/cifsproto.h | 3 ++ fs/cifs/cifssmb.c | 9 ++++-- fs/cifs/file.c | 90 ++++++++++++++++++++++++++++++++++++----------------- 3 files changed, 70 insertions(+), 32 deletions(-) diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h index 9e8394f..4f96b3b 100644 --- a/fs/cifs/cifsproto.h +++ b/fs/cifs/cifsproto.h @@ -134,6 +134,9 @@ extern bool is_size_safe_to_change(struct cifsInodeInfo *, __u64 eof); extern void cifs_update_eof(struct cifsInodeInfo *cifsi, loff_t offset, unsigned int bytes_written); extern struct cifsFileInfo *find_writable_file(struct cifsInodeInfo *, bool); +extern int cifs_get_writable_file(struct cifsInodeInfo *cifs_inode, + bool fsuid_only, + struct cifsFileInfo **ret_file); extern struct cifsFileInfo *find_readable_file(struct cifsInodeInfo *, bool); extern unsigned int smbCalcSize(void *buf, struct TCP_Server_Info *server); extern int decode_negTokenInit(unsigned char *security_blob, int length, diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c index 6c89279..c0e3a04 100644 --- a/fs/cifs/cifssmb.c +++ b/fs/cifs/cifssmb.c @@ -2126,10 +2126,13 @@ cifs_writev_requeue(struct cifs_writedata *wdata) wdata2->tailsz = tailsz; wdata2->bytes = cur_len; - wdata2->cfile = find_writable_file(CIFS_I(inode), false); + rc = cifs_get_writable_file(CIFS_I(inode), false, + &wdata2->cfile); if (!wdata2->cfile) { - cifs_dbg(VFS, "No writable handle to retry writepages\n"); - rc = -EBADF; + cifs_dbg(VFS, "No writable handle to retry writepages rc=%d\n", + rc); + if (!is_retryable_error(rc)) + rc = -EBADF; } else { wdata2->pid = wdata2->cfile->pid; rc = server->ops->async_writev(wdata2, diff --git a/fs/cifs/file.c b/fs/cifs/file.c index 9de7ad5..85a034d 100644 --- a/fs/cifs/file.c +++ b/fs/cifs/file.c @@ -1842,24 +1842,30 @@ struct cifsFileInfo *find_readable_file(struct cifsInodeInfo *cifs_inode, return NULL; } -struct cifsFileInfo *find_writable_file(struct cifsInodeInfo *cifs_inode, - bool fsuid_only) +/* Return -EBADF if no handle is found and general rc otherwise */ +int +cifs_get_writable_file(struct cifsInodeInfo *cifs_inode, bool fsuid_only, + struct cifsFileInfo **ret_file) { struct cifsFileInfo *open_file, *inv_file = NULL; struct cifs_sb_info *cifs_sb; struct cifs_tcon *tcon; bool any_available = false; - int rc; + int rc = -EBADF; unsigned int refind = 0; - /* Having a null inode here (because mapping->host was set to zero by - the VFS or MM) should not happen but we had reports of on oops (due to - it being zero) during stress testcases so we need to check for it */ + *ret_file = NULL; + + /* + * Having a null inode here (because mapping->host was set to zero by + * the VFS or MM) should not happen but we had reports of on oops (due + * to it being zero) during stress testcases so we need to check for it + */ if (cifs_inode == NULL) { cifs_dbg(VFS, "Null inode passed to cifs_writeable_file\n"); dump_stack(); - return NULL; + return rc; } cifs_sb = CIFS_SB(cifs_inode->vfs_inode.i_sb); @@ -1873,7 +1879,7 @@ struct cifsFileInfo *find_writable_file(struct cifsInodeInfo *cifs_inode, refind_writable: if (refind > MAX_REOPEN_ATT) { spin_unlock(&tcon->open_file_lock); - return NULL; + return rc; } list_for_each_entry(open_file, &cifs_inode->openFileList, flist) { if (!any_available && open_file->pid != current->tgid) @@ -1885,7 +1891,8 @@ struct cifsFileInfo *find_writable_file(struct cifsInodeInfo *cifs_inode, /* found a good writable file */ cifsFileInfo_get(open_file); spin_unlock(&tcon->open_file_lock); - return open_file; + *ret_file = open_file; + return 0; } else { if (!inv_file) inv_file = open_file; @@ -1907,22 +1914,35 @@ struct cifsFileInfo *find_writable_file(struct cifsInodeInfo *cifs_inode, if (inv_file) { rc = cifs_reopen_file(inv_file, false); - if (!rc) - return inv_file; - else { - spin_lock(&tcon->open_file_lock); - list_move_tail(&inv_file->flist, - &cifs_inode->openFileList); - spin_unlock(&tcon->open_file_lock); - cifsFileInfo_put(inv_file); - ++refind; - inv_file = NULL; - spin_lock(&tcon->open_file_lock); - goto refind_writable; + if (!rc) { + *ret_file = inv_file; + return 0; } + + spin_lock(&tcon->open_file_lock); + list_move_tail(&inv_file->flist, &cifs_inode->openFileList); + spin_unlock(&tcon->open_file_lock); + cifsFileInfo_put(inv_file); + ++refind; + inv_file = NULL; + spin_lock(&tcon->open_file_lock); + goto refind_writable; } - return NULL; + return rc; +} + +struct cifsFileInfo * +find_writable_file(struct cifsInodeInfo *cifs_inode, bool fsuid_only) +{ + struct cifsFileInfo *cfile; + int rc; + + rc = cifs_get_writable_file(cifs_inode, fsuid_only, &cfile); + if (rc) + cifs_dbg(FYI, "couldn't find writable handle rc=%d", rc); + + return cfile; } static int cifs_partialpagewrite(struct page *page, unsigned from, unsigned to) @@ -1959,8 +1979,8 @@ static int cifs_partialpagewrite(struct page *page, unsigned from, unsigned to) if (mapping->host->i_size - offset < (loff_t)to) to = (unsigned)(mapping->host->i_size - offset); - open_file = find_writable_file(CIFS_I(mapping->host), false); - if (open_file) { + rc = cifs_get_writable_file(CIFS_I(mapping->host), false, &open_file); + if (!rc) { bytes_written = cifs_write(open_file, open_file->pid, write_data, to - from, &offset); cifsFileInfo_put(open_file); @@ -1970,9 +1990,12 @@ static int cifs_partialpagewrite(struct page *page, unsigned from, unsigned to) rc = 0; else if (bytes_written < 0) rc = bytes_written; + else + rc = -EFAULT; } else { - cifs_dbg(FYI, "No writeable filehandles for inode\n"); - rc = -EIO; + cifs_dbg(FYI, "No writable handle for write page rc=%d\n", rc); + if (!is_retryable_error(rc)) + rc = -EIO; } kunmap(page); @@ -2144,11 +2167,16 @@ static int cifs_writepages(struct address_space *mapping, pgoff_t next = 0, tofind, saved_index = index; struct cifs_credits credits_on_stack; struct cifs_credits *credits = &credits_on_stack; + int get_file_rc = 0; if (cfile) cifsFileInfo_put(cfile); - cfile = find_writable_file(CIFS_I(inode), false); + rc = cifs_get_writable_file(CIFS_I(inode), false, &cfile); + + /* in case of an error store it to return later */ + if (rc) + get_file_rc = rc; rc = server->ops->wait_mtu_credits(server, cifs_sb->wsize, &wsize, credits); @@ -2189,8 +2217,12 @@ static int cifs_writepages(struct address_space *mapping, cfile = NULL; if (!wdata->cfile) { - cifs_dbg(VFS, "No writable handle in writepages\n"); - rc = -EBADF; + cifs_dbg(VFS, "No writable handle in writepages rc=%d\n", + get_file_rc); + if (is_retryable_error(get_file_rc)) + rc = get_file_rc; + else + rc = -EBADF; } else rc = wdata_send_pages(wdata, nr_pages, mapping, wbc);