All of lore.kernel.org
 help / color / mirror / Atom feed
From: ChenXiaoSong <chenxiaosongemail@foxmail.com>
To: Trond Myklebust <trondmy@hammerspace.com>,
	"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>
Cc: "linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>,
	"chenxiaosong@kylinos.cn" <chenxiaosong@kylinos.cn>,
	"stable@vger.kernel.org" <stable@vger.kernel.org>,
	"huangjinhui@kylinos.cn" <huangjinhui@kylinos.cn>,
	"liuzhengyuan@kylinos.cn" <liuzhengyuan@kylinos.cn>,
	"liuyun01@kylinos.cn" <liuyun01@kylinos.cn>,
	"huhai@kylinos.cn" <huhai@kylinos.cn>,
	"sashal@kernel.org" <sashal@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Anna.Schumaker@netapp.com" <Anna.Schumaker@netapp.com>
Subject: Re: Question about LTS 4.19 patch "89047634f5ce NFS: Don't interrupt file writeout due to fatal errors"
Date: Fri, 17 Nov 2023 12:09:37 +0800	[thread overview]
Message-ID: <tencent_F89651CE8E1BFCEC42C4BFEDD0CA77F82609@qq.com> (raw)
In-Reply-To: <3b8caab5918d06f436a889bc1dba09686fc0fad5.camel@hammerspace.com>

On 2023/10/30 22:56, Trond Myklebust wrote:
> A refactoring is by definition a change that does not affect code
> behaviour. It is obvious that this was never intended to be such a
> patch.
>
> The reason that the bug is occurring in 4.19.x, and not in the latest
> kernels, is because the former is missing another bugfix (one which
> actually is missing a "Fixes:" tag).
>
> Can you therefore please check if applying commit 22876f540bdf ("NFS:
> Don't call generic_error_remove_page() while holding locks") fixes the
> issue.
>
> Note that the latter patch is needed in any case in order to fix a read
> deadlock (as indicated on the label).
>
> Thanks,
>    Trond

Sorry, the previous email had formatting issues. I'll resend it.


After applying commit 22876f540bdf ("NFS: Don't call 
generic_error_remove_page() while holding locks"), I encountered an 
issue of infinite loop:

write
   ...
   nfs_updatepage
     nfs_writepage_setup
       nfs_setup_write_request
         nfs_try_to_update_request
           nfs_wb_page
             if (clear_page_dirty_for_io(page)) // true
             nfs_writepage_locked // return 0
               nfs_do_writepage // return 0
                 nfs_page_async_flush // return 0
                   nfs_error_is_fatal_on_server
                   nfs_write_error_remove_page
                     SetPageError // instead of generic_error_remove_page
             // loop begin
             if (clear_page_dirty_for_io(page)) // false
             if (!PagePrivate(page)) // false
             ret = nfs_commit_inode = 0
             // loop again, never quit


before applying commit 22876f540bdf ("NFS: Don't call 
generic_error_remove_page() while holding locks"), 
generic_error_remove_page() will clear PG_private, and infinite loop 
will never happen:

generic_error_remove_page
   truncate_inode_page
     truncate_cleanup_page
       do_invalidatepage
         nfs_invalidate_page
           nfs_wb_page_cancel
             nfs_inode_remove_request
               ClearPagePrivate(head->wb_page)


If applying this patch, are other patches required? And I cannot 
reproducethe read deadlock bug that the patch want to fix, are there 
specific conditions required to reproduce this read deadlock bug?



      parent reply	other threads:[~2023-11-17  4:09 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-30  8:39 Question about LTS 4.19 patch "89047634f5ce NFS: Don't interrupt file writeout due to fatal errors" ChenXiaoSong
2023-10-30  8:43 ` Greg KH
2023-10-30  8:54   ` ChenXiaoSong
2023-10-30  8:58     ` Greg KH
2023-10-30  9:04       ` ChenXiaoSong
2023-10-30  9:19         ` Greg KH
2023-10-30 14:56         ` Trond Myklebust
2023-11-17  3:28           ` ChenXiaoSong
2023-11-17  4:09           ` ChenXiaoSong [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=tencent_F89651CE8E1BFCEC42C4BFEDD0CA77F82609@qq.com \
    --to=chenxiaosongemail@foxmail.com \
    --cc=Anna.Schumaker@netapp.com \
    --cc=chenxiaosong@kylinos.cn \
    --cc=gregkh@linuxfoundation.org \
    --cc=huangjinhui@kylinos.cn \
    --cc=huhai@kylinos.cn \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=liuyun01@kylinos.cn \
    --cc=liuzhengyuan@kylinos.cn \
    --cc=sashal@kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=trondmy@hammerspace.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.