2014-09-30 18:49 GMT+04:00 Arthur Marsh <[email protected]>: > Pardon the top post but I tried the same thing on my dual core Pentium 4 > machine mounting via cifs a filesytem on my AMD64 (quad core) machine, then > running aplay /mnt/remotefilesystem/somefile.wav > then restarting samba on the serving machine. > > The results were the same as what I've posted before with the client being > on the quad-core AMD64 machine and the server being the Pentium 4 machine, a > nasty lock-up pointing to similar code. > > Has anyone else been able to reproduce this problem? > > Regards, > > Arthur Marsh. > > Arthur Marsh wrote, on 27/09/14 01:45: > >> Arthur Marsh wrote on 16/09/14 03:17: >>> >>> Arthur Marsh wrote, on 14/09/14 21:15: >>>> >>>> On 3.16.0 kernels, whether compiled by myself or the stock Debian >>>> kernel, if I have program accessing a file on a CIFS-mounted file system >>>> (specifically playing an audio file), and restart the samba server on >>>> the remote machine, I might get a brief pause, but no major problems. >>>> >>>> On 3.17.0-rc1 and later I get a complete machine lock-up. >>>> >>>> Illustration: >>>> >>>> https://pbs.twimg.com/media/BxfPkdgCcAAlpUo.jpg:large >>>> >>>> Has anyone else experienced similar? >>>> >>>> I'm happy to supply any further details and try test fixes. >>>> >>>> Arthur. >>> >>> >>> I'm still getting the problem with kernel 3.17.0-rc5 >>> >>> Has anyone been able to reproduce the problem? >>> >>> Arthur. >>> >> >> I was off sick this past week but completed the bisect: >> >> git bisect good >> 66386c08be5d1a2eefc1f7ab8c008561b6c811e5 is the first bad commit >> commit 66386c08be5d1a2eefc1f7ab8c008561b6c811e5 >> Author: Pavel Shilovsky <[email protected]> >> Date: Fri Jun 20 15:48:40 2014 +0400 >> >> CIFS: Separate filling pages from iovec write >> >> Reviewed-by: Shirish Pargaonkar <[email protected]> >> Signed-off-by: Pavel Shilovsky <[email protected]> >> Signed-off-by: Steve French <[email protected]> >> >> :040000 040000 1be960992f3d75b4998082fda3f3c2fca213b9e5 >> 318c46c63c983fe898480e1b88e97b818af1e502 M fs >> >> >> git bisect log: >> >> git bisect start >> # good: [19583ca584d6f574384e17fe7613dfaeadcdc4a6] Linux 3.16 >> git bisect good 19583ca584d6f574384e17fe7613dfaeadcdc4a6 >> # bad: [7d1311b93e58ed55f3a31cc8f94c4b8fe988a2b9] Linux 3.17-rc1 >> git bisect bad 7d1311b93e58ed55f3a31cc8f94c4b8fe988a2b9 >> # good: [ae045e2455429c418a418a3376301a9e5753a0a8] Merge >> git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next >> git bisect good ae045e2455429c418a418a3376301a9e5753a0a8 >> # good: [44c916d58b9ef1f2c4aec2def57fa8289c716a60] Merge tag >> 'cleanup-for-3.17' of >> git://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc >> git bisect good 44c916d58b9ef1f2c4aec2def57fa8289c716a60 >> # skip: [023f78b02c729070116fa3a7ebd4107a032d3f5c] Merge branch >> 'for-next' of git://git.samba.org/sfrench/cifs-2.6 >> git bisect skip 023f78b02c729070116fa3a7ebd4107a032d3f5c >> # skip: [908790fa3b779d37365e6b28e3aa0f6e833020c3] dcache: >> d_splice_alias mustn't create directory aliases >> git bisect skip 908790fa3b779d37365e6b28e3aa0f6e833020c3 >> # bad: [58d08e3b2c2033354b91467da33deffa06360c28] Merge tag 'for-linus' >> of git://git.kernel.org/pub/scm/linux/kernel/git/olof/chrome-platform >> git bisect bad 58d08e3b2c2033354b91467da33deffa06360c28 >> # good: [27d438c56009e5ae632de36fe70985d1aab5e344] Revert "drm/i915: >> Enable PSR by default." >> git bisect good 27d438c56009e5ae632de36fe70985d1aab5e344 >> # good: [ee34fb97a96ceac3334705ebab8b541ca291699f] ARM: dts: exynos5420: >> remove disp_pd >> git bisect good ee34fb97a96ceac3334705ebab8b541ca291699f >> # good: [d1e458fe671baf1e60afafc88bda090202a412f1] svcrdma: remove >> rdma_create_qp() failure recovery logic >> git bisect good d1e458fe671baf1e60afafc88bda090202a412f1 >> # good: [95484b57265caa671a57efed06e322d56461774b] drm/nouveau/ltc: >> s/ltcg/ltc/ + cleanup >> git bisect good 95484b57265caa671a57efed06e322d56461774b >> # good: [63b12bdb0d21aca527996fb2c547387bfd3e14b8] Merge branch >> 'signal-cleanup' of git://git.kernel.org/pub/scm/linux/kernel/git/rw/misc >> git bisect good 63b12bdb0d21aca527996fb2c547387bfd3e14b8 >> # bad: [96784de59fb35077c2bb33c39328992b836d87d3] Merge branch >> 'stable-3.17' of git://git.infradead.org/users/pcmoore/selinux >> git bisect bad 96784de59fb35077c2bb33c39328992b836d87d3 >> # bad: [25f402598d2c8f0808d93715ad33e43b265c1604] CIFS: Fix rsize usage >> in user read >> git bisect bad 25f402598d2c8f0808d93715ad33e43b265c1604 >> # good: [90ac1387c2dfcd9b4bd302fce03b9ddff73d0093] CIFS: Separate pages >> initialization from writepages >> git bisect good 90ac1387c2dfcd9b4bd302fce03b9ddff73d0093 >> # skip: [6ec0b01b2691d1465bb7219e031e8bf38ccd9397] CIFS: Fix wsize usage >> in iovec write >> git bisect skip 6ec0b01b2691d1465bb7219e031e8bf38ccd9397 >> # bad: [66386c08be5d1a2eefc1f7ab8c008561b6c811e5] CIFS: Separate filling >> pages from iovec write >> git bisect bad 66386c08be5d1a2eefc1f7ab8c008561b6c811e5 >> # good: [66231a47965c551d3056d5104f8b06688065748c] CIFS: Fix wsize usage >> in writepages >> git bisect good 66231a47965c551d3056d5104f8b06688065748c >> # good: [7f6c50086a6f5bc0fee46548afc836070a439313] CIFS: Fix >> cifs_writev_requeue when wsize changes >> git bisect good 7f6c50086a6f5bc0fee46548afc836070a439313 >> # first bad commit: [66386c08be5d1a2eefc1f7ab8c008561b6c811e5] CIFS: >> Separate filling pages from iovec write >> >> >>
It seems strange how a direct write cleanup patch related to page reading code (according to the screenshot). I created a reverted version of the problem commit on top of the current codebase. Could you test it on your side to see if the problem is here, please? -- Best regards, Pavel Shilovsky.
From 1399a1c0fa2d0975dfda95d00ac6739bb3506362 Mon Sep 17 00:00:00 2001 From: Pavel Shilovsky <[email protected]> Date: Sat, 11 Oct 2014 15:13:49 +0400 Subject: [PATCH] Revert "CIFS: Separate filling pages from iovec write" This reverts commit 66386c08be5d1a2eefc1f7ab8c008561b6c811e5. --- fs/cifs/file.c | 82 +++++++++++++++++++++++----------------------------------- 1 file changed, 32 insertions(+), 50 deletions(-) diff --git a/fs/cifs/file.c b/fs/cifs/file.c index 7c018a1..f4c398e 100644 --- a/fs/cifs/file.c +++ b/fs/cifs/file.c @@ -2422,55 +2422,13 @@ cifs_uncached_writev_complete(struct work_struct *work) } static int -wdata_fill_from_iovec(struct cifs_writedata *wdata, struct iov_iter *from, - size_t *len, unsigned long *num_pages) -{ - size_t save_len, copied, bytes, cur_len = *len; - unsigned long i, nr_pages = *num_pages; - - save_len = cur_len; - for (i = 0; i < nr_pages; i++) { - bytes = min_t(const size_t, cur_len, PAGE_SIZE); - copied = copy_page_from_iter(wdata->pages[i], 0, bytes, from); - cur_len -= copied; - /* - * If we didn't copy as much as we expected, then that - * may mean we trod into an unmapped area. Stop copying - * at that point. On the next pass through the big - * loop, we'll likely end up getting a zero-length - * write and bailing out of it. - */ - if (copied < bytes) - break; - } - cur_len = save_len - cur_len; - *len = cur_len; - - /* - * If we have no data to send, then that probably means that - * the copy above failed altogether. That's most likely because - * the address in the iovec was bogus. Return -EFAULT and let - * the caller free anything we allocated and bail out. - */ - if (!cur_len) - return -EFAULT; - - /* - * i + 1 now represents the number of pages we actually used in - * the copy phase above. - */ - *num_pages = i + 1; - return 0; -} - -static int cifs_write_from_iter(loff_t offset, size_t len, struct iov_iter *from, struct cifsFileInfo *open_file, struct cifs_sb_info *cifs_sb, struct list_head *wdata_list) { int rc = 0; - size_t cur_len; - unsigned long nr_pages, num_pages, i; + size_t cur_len, save_len, copied, bytes; + unsigned long nr_pages, i; struct cifs_writedata *wdata; struct iov_iter saved_from; loff_t saved_offset = offset; @@ -2509,21 +2467,45 @@ cifs_write_from_iter(loff_t offset, size_t len, struct iov_iter *from, break; } - num_pages = nr_pages; - rc = wdata_fill_from_iovec(wdata, from, &cur_len, &num_pages); - if (rc) { + save_len = cur_len; + for (i = 0; i < nr_pages; i++) { + bytes = min_t(size_t, cur_len, PAGE_SIZE); + copied = copy_page_from_iter(wdata->pages[i], 0, bytes, + from); + cur_len -= copied; + /* + * If we didn't copy as much as we expected, then that + * may mean we trod into an unmapped area. Stop copying + * at that point. On the next pass through the big + * loop, we'll likely end up getting a zero-length + * write and bailing out of it. + */ + if (copied < bytes) + break; + } + cur_len = save_len - cur_len; + + /* + * If we have no data to send, then that probably means that + * the copy above failed altogether. That's most likely because + * the address in the iovec was bogus. Set the rc to -EFAULT, + * free anything we allocated and bail out. + */ + if (!cur_len) { for (i = 0; i < nr_pages; i++) put_page(wdata->pages[i]); kfree(wdata); add_credits_and_wake_if(server, credits, 0); + rc = -EFAULT; break; } /* - * Bring nr_pages down to the number of pages we actually used, - * and free any pages that we didn't use. + * i + 1 now represents the number of pages we actually used in + * the copy phase above. Bring nr_pages down to that, and free + * any pages that we didn't use. */ - for ( ; nr_pages > num_pages; nr_pages--) + for ( ; nr_pages > i + 1; nr_pages--) put_page(wdata->pages[nr_pages - 1]); wdata->sync_mode = WB_SYNC_ALL; -- 1.9.1
