From: Dave Chinner <david@fromorbit.com> To: Yafang Shao <laoar.shao@gmail.com> Cc: kernel test robot <oliver.sang@intel.com>, 0day robot <lkp@intel.com>, "Darrick J. Wong" <darrick.wong@oracle.com>, Matthew Wilcox <willy@infradead.org>, Christoph Hellwig <hch@lst.de>, Michal Hocko <mhocko@kernel.org>, David Howells <dhowells@redhat.com>, Jeff Layton <jlayton@redhat.com>, LKML <linux-kernel@vger.kernel.org>, lkp@lists.01.org, Christoph Hellwig <hch@infradead.org>, Andrew Morton <akpm@linux-foundation.org>, linux-fsdevel@vger.kernel.org, linux-cachefs@redhat.com, linux-xfs@vger.kernel.org, Linux MM <linux-mm@kvack.org> Subject: Re: [xfs] db962cd266: Assertion_failed Date: Sat, 2 Jan 2021 08:53:53 +1100 [thread overview] Message-ID: <20210101215353.GB331610@dread.disaster.area> (raw) In-Reply-To: <CALOAHbD+mLMJSizToKPsx0iUd5Z71sJBOyMaV2enVvUHfHwLzg@mail.gmail.com> On Fri, Jan 01, 2021 at 05:10:49PM +0800, Yafang Shao wrote: > On Thu, Dec 31, 2020 at 10:46 AM kernel test robot > <oliver.sang@intel.com> wrote: ..... > > [ 552.905799] XFS: Assertion failed: !current->journal_info, file: fs/xfs/xfs_trans.h, line: 280 > > [ 553.104459] xfs_trans_reserve+0x225/0x320 [xfs] > > [ 553.110556] xfs_trans_roll+0x6e/0xe0 [xfs] > > [ 553.116134] xfs_defer_trans_roll+0x104/0x2a0 [xfs] > > [ 553.122489] ? xfs_extent_free_create_intent+0x62/0xc0 [xfs] > > [ 553.129780] xfs_defer_finish_noroll+0xb8/0x620 [xfs] > > [ 553.136299] xfs_defer_finish+0x11/0xa0 [xfs] > > [ 553.142017] xfs_itruncate_extents_flags+0x141/0x440 [xfs] > > [ 553.149053] xfs_setattr_size+0x3da/0x480 [xfs] > > [ 553.154939] ? setattr_prepare+0x6a/0x1e0 > > [ 553.160250] xfs_vn_setattr+0x70/0x120 [xfs] > > [ 553.165833] notify_change+0x364/0x500 > > [ 553.170820] ? do_truncate+0x76/0xe0 > > [ 553.175673] do_truncate+0x76/0xe0 > > [ 553.180184] path_openat+0xe6c/0x10a0 > > [ 553.184981] do_filp_open+0x91/0x100 > > [ 553.189707] ? __check_object_size+0x136/0x160 > > [ 553.195493] do_sys_openat2+0x20d/0x2e0 > > [ 553.200481] do_sys_open+0x44/0x80 > > [ 553.204926] do_syscall_64+0x33/0x40 > > [ 553.209588] entry_SYSCALL_64_after_hwframe+0x44/0xa9 > > Thanks for the report. > > At a first glance, it seems we should make a similar change as we did > in xfs_trans_context_clear(). > > static inline void > xfs_trans_context_set(struct xfs_trans *tp) > { > /* > * We have already handed over the context via xfs_trans_context_swap(). > */ > if (current->journal_info) > return; > current->journal_info = tp; > tp->t_pflags = memalloc_nofs_save(); > } Ah, no. Remember how I said "split out the wrapping of transaction context setup in xfs_trans_reserve() from the lifting of the context setting into xfs_trans_alloc()"? Well, you did the former and dropped the latter out of the patch set. Now when a transaction rolls after xfs_trans_context_swap(), it calls xfs_trans_reserve() and tries to do transaction context setup work inside a transaction context that already exists. IOWs, you need to put the patch that lifts of the context setting up into xfs_trans_alloc() back into the patchset before adding the current->journal functionality patch. Also, you need to test XFS code with CONFIG_XFS_DEBUG=y so that asserts are actually built into the code and exercised, because this ASSERT should have fired on the first rolling transaction that the kernel executes... Cheers, Dave. -- Dave Chinner david@fromorbit.com
WARNING: multiple messages have this Message-ID (diff)
From: Dave Chinner <david@fromorbit.com> To: lkp@lists.01.org Subject: Re: [xfs] db962cd266: Assertion_failed Date: Sat, 02 Jan 2021 08:53:53 +1100 [thread overview] Message-ID: <20210101215353.GB331610@dread.disaster.area> (raw) In-Reply-To: <CALOAHbD+mLMJSizToKPsx0iUd5Z71sJBOyMaV2enVvUHfHwLzg@mail.gmail.com> [-- Attachment #1: Type: text/plain, Size: 2678 bytes --] On Fri, Jan 01, 2021 at 05:10:49PM +0800, Yafang Shao wrote: > On Thu, Dec 31, 2020 at 10:46 AM kernel test robot > <oliver.sang@intel.com> wrote: ..... > > [ 552.905799] XFS: Assertion failed: !current->journal_info, file: fs/xfs/xfs_trans.h, line: 280 > > [ 553.104459] xfs_trans_reserve+0x225/0x320 [xfs] > > [ 553.110556] xfs_trans_roll+0x6e/0xe0 [xfs] > > [ 553.116134] xfs_defer_trans_roll+0x104/0x2a0 [xfs] > > [ 553.122489] ? xfs_extent_free_create_intent+0x62/0xc0 [xfs] > > [ 553.129780] xfs_defer_finish_noroll+0xb8/0x620 [xfs] > > [ 553.136299] xfs_defer_finish+0x11/0xa0 [xfs] > > [ 553.142017] xfs_itruncate_extents_flags+0x141/0x440 [xfs] > > [ 553.149053] xfs_setattr_size+0x3da/0x480 [xfs] > > [ 553.154939] ? setattr_prepare+0x6a/0x1e0 > > [ 553.160250] xfs_vn_setattr+0x70/0x120 [xfs] > > [ 553.165833] notify_change+0x364/0x500 > > [ 553.170820] ? do_truncate+0x76/0xe0 > > [ 553.175673] do_truncate+0x76/0xe0 > > [ 553.180184] path_openat+0xe6c/0x10a0 > > [ 553.184981] do_filp_open+0x91/0x100 > > [ 553.189707] ? __check_object_size+0x136/0x160 > > [ 553.195493] do_sys_openat2+0x20d/0x2e0 > > [ 553.200481] do_sys_open+0x44/0x80 > > [ 553.204926] do_syscall_64+0x33/0x40 > > [ 553.209588] entry_SYSCALL_64_after_hwframe+0x44/0xa9 > > Thanks for the report. > > At a first glance, it seems we should make a similar change as we did > in xfs_trans_context_clear(). > > static inline void > xfs_trans_context_set(struct xfs_trans *tp) > { > /* > * We have already handed over the context via xfs_trans_context_swap(). > */ > if (current->journal_info) > return; > current->journal_info = tp; > tp->t_pflags = memalloc_nofs_save(); > } Ah, no. Remember how I said "split out the wrapping of transaction context setup in xfs_trans_reserve() from the lifting of the context setting into xfs_trans_alloc()"? Well, you did the former and dropped the latter out of the patch set. Now when a transaction rolls after xfs_trans_context_swap(), it calls xfs_trans_reserve() and tries to do transaction context setup work inside a transaction context that already exists. IOWs, you need to put the patch that lifts of the context setting up into xfs_trans_alloc() back into the patchset before adding the current->journal functionality patch. Also, you need to test XFS code with CONFIG_XFS_DEBUG=y so that asserts are actually built into the code and exercised, because this ASSERT should have fired on the first rolling transaction that the kernel executes... Cheers, Dave. -- Dave Chinner david(a)fromorbit.com
next prev parent reply other threads:[~2021-01-01 21:55 UTC|newest] Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-12-22 1:21 [PATCH v14 0/4] xfs: avoid transaction reservation recursion Yafang Shao 2020-12-22 1:21 ` [PATCH v14 1/4] mm: Add become_kswapd and restore_kswapd Yafang Shao 2020-12-22 1:21 ` [PATCH v14 2/4] xfs: use memalloc_nofs_{save,restore} in xfs transaction Yafang Shao 2020-12-22 1:21 ` [PATCH v14 3/4] xfs: introduce xfs_trans_context_swap() for rolling transaction Yafang Shao 2020-12-22 1:21 ` [PATCH v14 4/4] xfs: use current->journal_info to avoid transaction reservation recursion Yafang Shao 2020-12-31 3:01 ` [xfs] db962cd266: Assertion_failed kernel test robot 2020-12-31 3:01 ` kernel test robot 2021-01-01 9:10 ` Yafang Shao 2021-01-01 9:10 ` Yafang Shao 2021-01-01 9:10 ` Yafang Shao 2021-01-01 21:53 ` Dave Chinner [this message] 2021-01-01 21:53 ` Dave Chinner 2021-01-04 10:42 ` Yafang Shao 2021-01-04 10:42 ` Yafang Shao 2021-01-04 10:42 ` Yafang Shao
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=20210101215353.GB331610@dread.disaster.area \ --to=david@fromorbit.com \ --cc=akpm@linux-foundation.org \ --cc=darrick.wong@oracle.com \ --cc=dhowells@redhat.com \ --cc=hch@infradead.org \ --cc=hch@lst.de \ --cc=jlayton@redhat.com \ --cc=laoar.shao@gmail.com \ --cc=linux-cachefs@redhat.com \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=linux-xfs@vger.kernel.org \ --cc=lkp@intel.com \ --cc=lkp@lists.01.org \ --cc=mhocko@kernel.org \ --cc=oliver.sang@intel.com \ --cc=willy@infradead.org \ /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: linkBe 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.