[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#762306: lockup when accessing cifs-mounted file on 3.17.0-rc1 and later when remote samba server is restarted



2014-10-01 15:34 GMT+04:00 Arthur Marsh <arthur.marsh@internode.on.net>:
> I managed to complete a git bisect -- fs/cifs:
>
>  git bisect good
> 69cebd75606f8b9162ad5d0104367370ceabeeba is the first bad commit
> commit 69cebd75606f8b9162ad5d0104367370ceabeeba
> Author: Pavel Shilovsky <pshilovsky@samba.org>
> Date:   Tue Jun 24 13:42:03 2014 +0400
>
>     CIFS: Fix rsize usage in readpages
>
>     If a server changes maximum buffer size for read (rsize) requests
>     on reconnect we can fail on repeating with a big size buffer on
>     -EAGAIN error in readpages. Fix this by checking rsize all the
>     time before repeating requests.
>
>     Reviewed-by: Shirish Pargaonkar <spargaonkar@suse.com>
>     Signed-off-by: Pavel Shilovsky <pshilovsky@samba.org>
>     Signed-off-by: Steve French <smfrench@gmail.com>
>
> :040000 040000 8ed2e10a1581bab7b47a7dab4c5763bfdebe0e69
> 9eca94b68e9a21e1e5e6712fd37012cfe6e7d6d6 M      fs
> am64:/usr/src/linux# git bisect log
> git bisect start '--' 'fs/cifs'
> # good: [19583ca584d6f574384e17fe7613dfaeadcdc4a6] Linux 3.16
> git bisect good 19583ca584d6f574384e17fe7613dfaeadcdc4a6
> # bad: [7d1311b93e58ed55f3a31cc8f94c4b8fe988a2b9] Linux 3.17-rc1
> git bisect bad 7d1311b93e58ed55f3a31cc8f94c4b8fe988a2b9
> # bad: [0ada36b244e8316bb47b46b84b33c5a507bed7a4] CIFS: Separate page
> reading from user read
> git bisect bad 0ada36b244e8316bb47b46b84b33c5a507bed7a4
> # good: [619aa48edbab47367fa8a65e568f63fd64d6b4af] CIFS: Separate page
> sending from writepages
> git bisect good 619aa48edbab47367fa8a65e568f63fd64d6b4af
> # good: [43de94eadf0ceda54509335343bdc1349a2c5ab3] CIFS: Separate writing
> from iovec write
> git bisect good 43de94eadf0ceda54509335343bdc1349a2c5ab3
> # good: [cb7e9eabb2b584884db0d11ae0376d31ac1cfdc1] CIFS: Use multicredits
> for SMB 2.1/3 writes
> git bisect good cb7e9eabb2b584884db0d11ae0376d31ac1cfdc1
> # bad: [69cebd75606f8b9162ad5d0104367370ceabeeba] CIFS: Fix rsize usage in
> readpages
> git bisect bad 69cebd75606f8b9162ad5d0104367370ceabeeba
> # good: [387eb92ac6892518fb67e423f65fcaca76e256a8] CIFS: Separate page
> search from readpages
> git bisect good 387eb92ac6892518fb67e423f65fcaca76e256a8
> # first bad commit: [69cebd75606f8b9162ad5d0104367370ceabeeba] CIFS: Fix
> rsize usage in readpages
>
> It points to a different "first bad commit" and I apologise if this has made
> it more difficult to locate the problem.
>
> I am happy to try more tests to help identify the problem.

Thank you for pointing it out!

I've reproduced and created a patch that fixes the problem to me (see
the attachment). Can you test it, please?

-- 
Best regards,
Pavel Shilovsky.
From ca4df0717fbf3f99e728bc534e91ea64bc138528 Mon Sep 17 00:00:00 2001
From: Pavel Shilovsky <pshilovsky@samba.org>
Date: Thu, 2 Oct 2014 13:16:40 +0400
Subject: [PATCH] CIFS: Fix readpages retrying on reconnects

If we got a reconnect error from async readv we re-add pages back
to page_list and continue loop. That is wrong because these pages
have been already added to the pagecache but page_list has pages that
have not been added to the pagecache yet. This ends up with a general
protection fault in put_pages after readpages. Fix it by not retrying
the read of these pages and falling back to readpage instead.

Signed-off-by: Pavel Shilovsky <pshilovsky@samba.org>
---
 fs/cifs/file.c |    8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 7c018a1..5f29354 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -3568,15 +3568,9 @@ static int cifs_readpages(struct file *file, struct address_space *mapping,
 				lru_cache_add_file(page);
 				unlock_page(page);
 				page_cache_release(page);
-				if (rc == -EAGAIN)
-					list_add_tail(&page->lru, &tmplist);
 			}
+			/* Fallback to the readpage in error/reconnect cases */
 			kref_put(&rdata->refcount, cifs_readdata_release);
-			if (rc == -EAGAIN) {
-				/* Re-add pages to the page_list and retry */
-				list_splice(&tmplist, page_list);
-				continue;
-			}
 			break;
 		}
 
-- 
1.7.10.4


Reply to: