* [PATCH] xfs: Use WARN_ON rather than BUG() for bailout mount-operation @ 2019-08-28 6:47 Austin Kim 2019-08-28 15:14 ` Darrick J. Wong 2019-08-29 7:56 ` Christoph Hellwig 0 siblings, 2 replies; 8+ messages in thread From: Austin Kim @ 2019-08-28 6:47 UTC (permalink / raw) To: darrick.wong; +Cc: linux-xfs, linux-kernel, austindh.kim If the CONFIG_BUG is enabled, BUG() is executed and then system is crashed. However, the bailout for mount is no longer proceeding. For this reason, using WARN_ON rather than BUG() could prevent this situation. --- fs/xfs/xfs_mount.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c index 322da69..10fe000 100644 --- a/fs/xfs/xfs_mount.c +++ b/fs/xfs/xfs_mount.c @@ -213,8 +213,7 @@ xfs_initialize_perag( goto out_hash_destroy; spin_lock(&mp->m_perag_lock); - if (radix_tree_insert(&mp->m_perag_tree, index, pag)) { - BUG(); + if (WARN_ON(radix_tree_insert(&mp->m_perag_tree, index, pag))){ spin_unlock(&mp->m_perag_lock); radix_tree_preload_end(); error = -EEXIST; -- 2.6.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] xfs: Use WARN_ON rather than BUG() for bailout mount-operation 2019-08-28 6:47 [PATCH] xfs: Use WARN_ON rather than BUG() for bailout mount-operation Austin Kim @ 2019-08-28 15:14 ` Darrick J. Wong 2019-08-28 21:30 ` Austin Kim 2019-08-29 7:56 ` Christoph Hellwig 1 sibling, 1 reply; 8+ messages in thread From: Darrick J. Wong @ 2019-08-28 15:14 UTC (permalink / raw) To: Austin Kim; +Cc: linux-xfs, linux-kernel On Wed, Aug 28, 2019 at 03:47:49PM +0900, Austin Kim wrote: > If the CONFIG_BUG is enabled, BUG() is executed and then system is crashed. > However, the bailout for mount is no longer proceeding. > > For this reason, using WARN_ON rather than BUG() could prevent this situation. > --- > fs/xfs/xfs_mount.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c > index 322da69..10fe000 100644 > --- a/fs/xfs/xfs_mount.c > +++ b/fs/xfs/xfs_mount.c > @@ -213,8 +213,7 @@ xfs_initialize_perag( > goto out_hash_destroy; > > spin_lock(&mp->m_perag_lock); > - if (radix_tree_insert(&mp->m_perag_tree, index, pag)) { > - BUG(); > + if (WARN_ON(radix_tree_insert(&mp->m_perag_tree, index, pag))){ Need a space before the brace. Will fix on import, Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> --D > spin_unlock(&mp->m_perag_lock); > radix_tree_preload_end(); > error = -EEXIST; > -- > 2.6.2 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] xfs: Use WARN_ON rather than BUG() for bailout mount-operation 2019-08-28 15:14 ` Darrick J. Wong @ 2019-08-28 21:30 ` Austin Kim 2019-08-28 21:51 ` Darrick J. Wong 0 siblings, 1 reply; 8+ messages in thread From: Austin Kim @ 2019-08-28 21:30 UTC (permalink / raw) To: Darrick J. Wong; +Cc: linux-xfs, linux-kernel Dear Mr. Darrick J. Wong Thanks for reviewing patch. BTW, I have a question for you. Do I have to update the patch again with 'a space before the brace'? Or could I just wait for the patch to be imported? It would be thankful if you give me a feedback. BR, Guillermo Austin Kim 2019년 8월 29일 (목) 오전 12:14, Darrick J. Wong <darrick.wong@oracle.com>님이 작성: > > On Wed, Aug 28, 2019 at 03:47:49PM +0900, Austin Kim wrote: > > If the CONFIG_BUG is enabled, BUG() is executed and then system is crashed. > > However, the bailout for mount is no longer proceeding. > > > > For this reason, using WARN_ON rather than BUG() could prevent this situation. > > --- > > fs/xfs/xfs_mount.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c > > index 322da69..10fe000 100644 > > --- a/fs/xfs/xfs_mount.c > > +++ b/fs/xfs/xfs_mount.c > > @@ -213,8 +213,7 @@ xfs_initialize_perag( > > goto out_hash_destroy; > > > > spin_lock(&mp->m_perag_lock); > > - if (radix_tree_insert(&mp->m_perag_tree, index, pag)) { > > - BUG(); > > + if (WARN_ON(radix_tree_insert(&mp->m_perag_tree, index, pag))){ > > Need a space before the brace. > > Will fix on import, > Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> > > --D > > > spin_unlock(&mp->m_perag_lock); > > radix_tree_preload_end(); > > error = -EEXIST; > > -- > > 2.6.2 > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] xfs: Use WARN_ON rather than BUG() for bailout mount-operation 2019-08-28 21:30 ` Austin Kim @ 2019-08-28 21:51 ` Darrick J. Wong 0 siblings, 0 replies; 8+ messages in thread From: Darrick J. Wong @ 2019-08-28 21:51 UTC (permalink / raw) To: Austin Kim; +Cc: linux-xfs, linux-kernel On Thu, Aug 29, 2019 at 06:30:43AM +0900, Austin Kim wrote: > Dear Mr. Darrick J. Wong > > Thanks for reviewing patch. BTW, I have a question for you. > > Do I have to update the patch again with 'a space before the brace'? > Or could I just wait for the patch to be imported? > > It would be thankful if you give me a feedback. I made the correction when I imported your patch into my testing tree, so you don't need to resubmit the patch just to fix that one thing. However, if you decide to send a new revision *for some other reason*, then please make the correction in your own tree before resubmitting. --D > BR, > Guillermo Austin Kim > > 2019년 8월 29일 (목) 오전 12:14, Darrick J. Wong <darrick.wong@oracle.com>님이 작성: > > > > On Wed, Aug 28, 2019 at 03:47:49PM +0900, Austin Kim wrote: > > > If the CONFIG_BUG is enabled, BUG() is executed and then system is crashed. > > > However, the bailout for mount is no longer proceeding. > > > > > > For this reason, using WARN_ON rather than BUG() could prevent this situation. > > > --- > > > fs/xfs/xfs_mount.c | 3 +-- > > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > > > diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c > > > index 322da69..10fe000 100644 > > > --- a/fs/xfs/xfs_mount.c > > > +++ b/fs/xfs/xfs_mount.c > > > @@ -213,8 +213,7 @@ xfs_initialize_perag( > > > goto out_hash_destroy; > > > > > > spin_lock(&mp->m_perag_lock); > > > - if (radix_tree_insert(&mp->m_perag_tree, index, pag)) { > > > - BUG(); > > > + if (WARN_ON(radix_tree_insert(&mp->m_perag_tree, index, pag))){ > > > > Need a space before the brace. > > > > Will fix on import, > > Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> > > > > --D > > > > > spin_unlock(&mp->m_perag_lock); > > > radix_tree_preload_end(); > > > error = -EEXIST; > > > -- > > > 2.6.2 > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] xfs: Use WARN_ON rather than BUG() for bailout mount-operation 2019-08-28 6:47 [PATCH] xfs: Use WARN_ON rather than BUG() for bailout mount-operation Austin Kim 2019-08-28 15:14 ` Darrick J. Wong @ 2019-08-29 7:56 ` Christoph Hellwig 2019-08-29 12:59 ` Austin Kim 2019-08-29 15:01 ` Darrick J. Wong 1 sibling, 2 replies; 8+ messages in thread From: Christoph Hellwig @ 2019-08-29 7:56 UTC (permalink / raw) To: Austin Kim; +Cc: darrick.wong, linux-xfs, linux-kernel On Wed, Aug 28, 2019 at 03:47:49PM +0900, Austin Kim wrote: > If the CONFIG_BUG is enabled, BUG() is executed and then system is crashed. > However, the bailout for mount is no longer proceeding. > > For this reason, using WARN_ON rather than BUG() could prevent this situation. > --- > fs/xfs/xfs_mount.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c > index 322da69..10fe000 100644 > --- a/fs/xfs/xfs_mount.c > +++ b/fs/xfs/xfs_mount.c > @@ -213,8 +213,7 @@ xfs_initialize_perag( > goto out_hash_destroy; > > spin_lock(&mp->m_perag_lock); > - if (radix_tree_insert(&mp->m_perag_tree, index, pag)) { > - BUG(); > + if (WARN_ON(radix_tree_insert(&mp->m_perag_tree, index, pag))){ Please make this a WARN_ON_ONCE so that we don't see a flodding of messages in case of this error. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] xfs: Use WARN_ON rather than BUG() for bailout mount-operation 2019-08-29 7:56 ` Christoph Hellwig @ 2019-08-29 12:59 ` Austin Kim 2019-08-29 15:58 ` Darrick J. Wong 2019-08-29 15:01 ` Darrick J. Wong 1 sibling, 1 reply; 8+ messages in thread From: Austin Kim @ 2019-08-29 12:59 UTC (permalink / raw) To: Christoph Hellwig, Darrick J. Wong; +Cc: linux-xfs, linux-kernel 2019년 8월 29일 (목) 오후 4:56, Christoph Hellwig <hch@infradead.org>님이 작성: > > On Wed, Aug 28, 2019 at 03:47:49PM +0900, Austin Kim wrote: > > If the CONFIG_BUG is enabled, BUG() is executed and then system is crashed. > > However, the bailout for mount is no longer proceeding. > > > > For this reason, using WARN_ON rather than BUG() could prevent this situation. > > --- > > fs/xfs/xfs_mount.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c > > index 322da69..10fe000 100644 > > --- a/fs/xfs/xfs_mount.c > > +++ b/fs/xfs/xfs_mount.c > > @@ -213,8 +213,7 @@ xfs_initialize_perag( > > goto out_hash_destroy; > > > > spin_lock(&mp->m_perag_lock); > > - if (radix_tree_insert(&mp->m_perag_tree, index, pag)) { > > - BUG(); > > + if (WARN_ON(radix_tree_insert(&mp->m_perag_tree, index, pag))){ > > Please make this a WARN_ON_ONCE so that we don't see a flodding of > messages in case of this error. > Hello, Mr. Christoph Thanks for good feedback. If the kernel log is flooded with error message, as you pointed out, it may cause other side-effect.(e.g: system non-responsive or lockup) To. Mr. Darrick J. Wong If you or other kernel developers do not disagree with the idea(WARN_ON_ONCE instead of WARN_ON), do I have to resend the patch with new revision? The title, the commit message and patch might be changed as followings; ====== xfs: Use WARN_ON_ONCE rather than BUG() for bailout mount-operation If the CONFIG_BUG is enabled, BUG() is executed and then system is crashed. However, the bailout for mount is no longer proceeding. For this reason, using WARN_ON_ONCE rather than BUG() could prevent this situation. diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c index 322da69..d831c13 100644 --- a/fs/xfs/xfs_mount.c +++ b/fs/xfs/xfs_mount.c @@ -213,8 +213,7 @@ xfs_initialize_perag( goto out_hash_destroy; spin_lock(&mp->m_perag_lock); - if (radix_tree_insert(&mp->m_perag_tree, index, pag)) { - BUG(); + if (WARN_ON_ONCE(radix_tree_insert(&mp->m_perag_tree, index, pag))) { spin_unlock(&mp->m_perag_lock); radix_tree_preload_end(); error = -EEXIST; ====== BR, Guillermo Austin Kim ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] xfs: Use WARN_ON rather than BUG() for bailout mount-operation 2019-08-29 12:59 ` Austin Kim @ 2019-08-29 15:58 ` Darrick J. Wong 0 siblings, 0 replies; 8+ messages in thread From: Darrick J. Wong @ 2019-08-29 15:58 UTC (permalink / raw) To: Austin Kim; +Cc: Christoph Hellwig, linux-xfs, linux-kernel On Thu, Aug 29, 2019 at 09:59:17PM +0900, Austin Kim wrote: > 2019년 8월 29일 (목) 오후 4:56, Christoph Hellwig <hch@infradead.org>님이 작성: > > > > On Wed, Aug 28, 2019 at 03:47:49PM +0900, Austin Kim wrote: > > > If the CONFIG_BUG is enabled, BUG() is executed and then system is crashed. > > > However, the bailout for mount is no longer proceeding. > > > > > > For this reason, using WARN_ON rather than BUG() could prevent this situation. > > > --- > > > fs/xfs/xfs_mount.c | 3 +-- > > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > > > diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c > > > index 322da69..10fe000 100644 > > > --- a/fs/xfs/xfs_mount.c > > > +++ b/fs/xfs/xfs_mount.c > > > @@ -213,8 +213,7 @@ xfs_initialize_perag( > > > goto out_hash_destroy; > > > > > > spin_lock(&mp->m_perag_lock); > > > - if (radix_tree_insert(&mp->m_perag_tree, index, pag)) { > > > - BUG(); > > > + if (WARN_ON(radix_tree_insert(&mp->m_perag_tree, index, pag))){ > > > > Please make this a WARN_ON_ONCE so that we don't see a flodding of > > messages in case of this error. > > > Hello, Mr. Christoph > Thanks for good feedback. > If the kernel log is flooded with error message, as you pointed out, > it may cause other side-effect.(e.g: system non-responsive or lockup) > > To. Mr. Darrick J. Wong > If you or other kernel developers do not disagree with the > idea(WARN_ON_ONCE instead of WARN_ON), > do I have to resend the patch with new revision? Yes, and please add your Signed-off-by in the new revision. --D > The title, the commit message and patch might be changed as followings; > ====== > xfs: Use WARN_ON_ONCE rather than BUG() for bailout mount-operation > > If the CONFIG_BUG is enabled, BUG() is executed and then system is crashed. > However, the bailout for mount is no longer proceeding. > > For this reason, using WARN_ON_ONCE rather than BUG() could prevent > this situation. > > diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c > index 322da69..d831c13 100644 > --- a/fs/xfs/xfs_mount.c > +++ b/fs/xfs/xfs_mount.c > @@ -213,8 +213,7 @@ xfs_initialize_perag( > goto out_hash_destroy; > > spin_lock(&mp->m_perag_lock); > - if (radix_tree_insert(&mp->m_perag_tree, index, pag)) { > - BUG(); > + if (WARN_ON_ONCE(radix_tree_insert(&mp->m_perag_tree, > index, pag))) { > spin_unlock(&mp->m_perag_lock); > radix_tree_preload_end(); > error = -EEXIST; > ====== > > BR, > Guillermo Austin Kim ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] xfs: Use WARN_ON rather than BUG() for bailout mount-operation 2019-08-29 7:56 ` Christoph Hellwig 2019-08-29 12:59 ` Austin Kim @ 2019-08-29 15:01 ` Darrick J. Wong 1 sibling, 0 replies; 8+ messages in thread From: Darrick J. Wong @ 2019-08-29 15:01 UTC (permalink / raw) To: Christoph Hellwig; +Cc: Austin Kim, linux-xfs, linux-kernel On Thu, Aug 29, 2019 at 12:56:55AM -0700, Christoph Hellwig wrote: > On Wed, Aug 28, 2019 at 03:47:49PM +0900, Austin Kim wrote: > > If the CONFIG_BUG is enabled, BUG() is executed and then system is crashed. > > However, the bailout for mount is no longer proceeding. > > > > For this reason, using WARN_ON rather than BUG() could prevent this situation. > > --- > > fs/xfs/xfs_mount.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c > > index 322da69..10fe000 100644 > > --- a/fs/xfs/xfs_mount.c > > +++ b/fs/xfs/xfs_mount.c > > @@ -213,8 +213,7 @@ xfs_initialize_perag( > > goto out_hash_destroy; > > > > spin_lock(&mp->m_perag_lock); > > - if (radix_tree_insert(&mp->m_perag_tree, index, pag)) { > > - BUG(); > > + if (WARN_ON(radix_tree_insert(&mp->m_perag_tree, index, pag))){ > > Please make this a WARN_ON_ONCE so that we don't see a flodding of > messages in case of this error. How would it flood? If the radix tree insertion fails we dump all the pag structures and fail log recovery / mount / growfs. I suppose if one were out of memory and hammering the system hard with repeated mount calls... --D ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-08-29 15:58 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-08-28 6:47 [PATCH] xfs: Use WARN_ON rather than BUG() for bailout mount-operation Austin Kim 2019-08-28 15:14 ` Darrick J. Wong 2019-08-28 21:30 ` Austin Kim 2019-08-28 21:51 ` Darrick J. Wong 2019-08-29 7:56 ` Christoph Hellwig 2019-08-29 12:59 ` Austin Kim 2019-08-29 15:58 ` Darrick J. Wong 2019-08-29 15:01 ` Darrick J. Wong
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.