All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Kara <jack@suse.cz>
To: Michal Hocko <mhocko@kernel.org>
Cc: linux-mm@kvack.org, linux-fsdevel@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>,
	Dave Chinner <david@fromorbit.com>,
	"Theodore Ts'o" <tytso@mit.edu>, Chris Mason <clm@fb.com>,
	David Sterba <dsterba@suse.cz>, Jan Kara <jack@suse.cz>,
	ceph-devel@vger.kernel.org, cluster-devel@redhat.com,
	linux-nfs@vger.kernel.org, logfs@logfs.org,
	linux-xfs@vger.kernel.org, linux-ext4@vger.kernel.org,
	linux-btrfs@vger.kernel.org, linux-mtd@lists.infradead.org,
	reiserfs-devel@vger.kernel.org,
	linux-ntfs-dev@lists.sourceforge.net,
	linux-f2fs-devel@lists.sourceforge.net,
	linux-afs@lists.infradead.org,
	LKML <linux-kernel@vger.kernel.org>,
	Michal Hocko <mhocko@suse.com>
Subject: Re: [PATCH 8/9] Revert "ext4: avoid deadlocks in the writeback path by using sb_getblk_gfp"
Date: Fri, 16 Dec 2016 09:41:26 +0100	[thread overview]
Message-ID: <20161216084126.GF26608@quack2.suse.cz> (raw)
In-Reply-To: <20161215140715.12732-9-mhocko@kernel.org>

On Thu 15-12-16 15:07:14, Michal Hocko wrote:
> From: Michal Hocko <mhocko@suse.com>
> 
> This reverts commit c45653c341f5c8a0ce19c8f0ad4678640849cb86 because
> sb_getblk_gfp is not really needed as
> sb_getblk
>   __getblk_gfp
>     __getblk_slow
>       grow_buffers
>         grow_dev_page
> 	  gfp_mask = mapping_gfp_constraint(inode->i_mapping, ~__GFP_FS) | gfp
> 
> so __GFP_FS is cleared unconditionally and therefore the above commit
> didn't have any real effect in fact.
> 
> This patch should not introduce any functional change. The main point
> of this change is to reduce explicit GFP_NOFS usage inside ext4 code to
> make the review of the remaining usage easier.
> 
> Signed-off-by: Michal Hocko <mhocko@suse.com>

Looks good to me. You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/ext4/extents.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index b1f8416923ab..ef815eb72389 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -518,7 +518,7 @@ __read_extent_tree_block(const char *function, unsigned int line,
>  	struct buffer_head		*bh;
>  	int				err;
>  
> -	bh = sb_getblk_gfp(inode->i_sb, pblk, __GFP_MOVABLE | GFP_NOFS);
> +	bh = sb_getblk(inode->i_sb, pblk);
>  	if (unlikely(!bh))
>  		return ERR_PTR(-ENOMEM);
>  
> @@ -1096,7 +1096,7 @@ static int ext4_ext_split(handle_t *handle, struct inode *inode,
>  		err = -EFSCORRUPTED;
>  		goto cleanup;
>  	}
> -	bh = sb_getblk_gfp(inode->i_sb, newblock, __GFP_MOVABLE | GFP_NOFS);
> +	bh = sb_getblk(inode->i_sb, newblock);
>  	if (unlikely(!bh)) {
>  		err = -ENOMEM;
>  		goto cleanup;
> @@ -1290,7 +1290,7 @@ static int ext4_ext_grow_indepth(handle_t *handle, struct inode *inode,
>  	if (newblock == 0)
>  		return err;
>  
> -	bh = sb_getblk_gfp(inode->i_sb, newblock, __GFP_MOVABLE | GFP_NOFS);
> +	bh = sb_getblk(inode->i_sb, newblock);
>  	if (unlikely(!bh))
>  		return -ENOMEM;
>  	lock_buffer(bh);
> -- 
> 2.10.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

WARNING: multiple messages have this Message-ID (diff)
From: Jan Kara <jack@suse.cz>
To: Michal Hocko <mhocko@kernel.org>
Cc: linux-mm@kvack.org, linux-fsdevel@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>,
	Dave Chinner <david@fromorbit.com>, Theodore Ts'o <tytso@mit.edu>,
	Chris Mason <clm@fb.com>, David Sterba <dsterba@suse.cz>,
	Jan Kara <jack@suse.cz>,
	ceph-devel@vger.kernel.org, cluster-devel@redhat.com,
	linux-nfs@vger.kernel.org, logfs@logfs.org,
	linux-xfs@vger.kernel.org, linux-ext4@vger.kernel.org,
	linux-btrfs@vger.kernel.org, linux-mtd@lists.infradead.org,
	reiserfs-devel@vger.kernel.org,
	linux-ntfs-dev@lists.sourceforge.net,
	linux-f2fs-devel@lists.sourceforge.net,
	linux-afs@lists.infradead.org,
	LKML <linux-kernel@vger.kernel.org>,
	Michal Hocko <mhocko@suse.com>
Subject: Re: [PATCH 8/9] Revert "ext4: avoid deadlocks in the writeback path by using sb_getblk_gfp"
Date: Fri, 16 Dec 2016 09:41:26 +0100	[thread overview]
Message-ID: <20161216084126.GF26608@quack2.suse.cz> (raw)
In-Reply-To: <20161215140715.12732-9-mhocko@kernel.org>

On Thu 15-12-16 15:07:14, Michal Hocko wrote:
> From: Michal Hocko <mhocko@suse.com>
> 
> This reverts commit c45653c341f5c8a0ce19c8f0ad4678640849cb86 because
> sb_getblk_gfp is not really needed as
> sb_getblk
>   __getblk_gfp
>     __getblk_slow
>       grow_buffers
>         grow_dev_page
> 	  gfp_mask = mapping_gfp_constraint(inode->i_mapping, ~__GFP_FS) | gfp
> 
> so __GFP_FS is cleared unconditionally and therefore the above commit
> didn't have any real effect in fact.
> 
> This patch should not introduce any functional change. The main point
> of this change is to reduce explicit GFP_NOFS usage inside ext4 code to
> make the review of the remaining usage easier.
> 
> Signed-off-by: Michal Hocko <mhocko@suse.com>

Looks good to me. You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/ext4/extents.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index b1f8416923ab..ef815eb72389 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -518,7 +518,7 @@ __read_extent_tree_block(const char *function, unsigned int line,
>  	struct buffer_head		*bh;
>  	int				err;
>  
> -	bh = sb_getblk_gfp(inode->i_sb, pblk, __GFP_MOVABLE | GFP_NOFS);
> +	bh = sb_getblk(inode->i_sb, pblk);
>  	if (unlikely(!bh))
>  		return ERR_PTR(-ENOMEM);
>  
> @@ -1096,7 +1096,7 @@ static int ext4_ext_split(handle_t *handle, struct inode *inode,
>  		err = -EFSCORRUPTED;
>  		goto cleanup;
>  	}
> -	bh = sb_getblk_gfp(inode->i_sb, newblock, __GFP_MOVABLE | GFP_NOFS);
> +	bh = sb_getblk(inode->i_sb, newblock);
>  	if (unlikely(!bh)) {
>  		err = -ENOMEM;
>  		goto cleanup;
> @@ -1290,7 +1290,7 @@ static int ext4_ext_grow_indepth(handle_t *handle, struct inode *inode,
>  	if (newblock == 0)
>  		return err;
>  
> -	bh = sb_getblk_gfp(inode->i_sb, newblock, __GFP_MOVABLE | GFP_NOFS);
> +	bh = sb_getblk(inode->i_sb, newblock);
>  	if (unlikely(!bh))
>  		return -ENOMEM;
>  	lock_buffer(bh);
> -- 
> 2.10.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

WARNING: multiple messages have this Message-ID (diff)
From: Jan Kara <jack@suse.cz>
To: cluster-devel.redhat.com
Subject: [Cluster-devel] [PATCH 8/9] Revert "ext4: avoid deadlocks in the writeback path by using sb_getblk_gfp"
Date: Fri, 16 Dec 2016 09:41:26 +0100	[thread overview]
Message-ID: <20161216084126.GF26608@quack2.suse.cz> (raw)
In-Reply-To: <20161215140715.12732-9-mhocko@kernel.org>

On Thu 15-12-16 15:07:14, Michal Hocko wrote:
> From: Michal Hocko <mhocko@suse.com>
> 
> This reverts commit c45653c341f5c8a0ce19c8f0ad4678640849cb86 because
> sb_getblk_gfp is not really needed as
> sb_getblk
>   __getblk_gfp
>     __getblk_slow
>       grow_buffers
>         grow_dev_page
> 	  gfp_mask = mapping_gfp_constraint(inode->i_mapping, ~__GFP_FS) | gfp
> 
> so __GFP_FS is cleared unconditionally and therefore the above commit
> didn't have any real effect in fact.
> 
> This patch should not introduce any functional change. The main point
> of this change is to reduce explicit GFP_NOFS usage inside ext4 code to
> make the review of the remaining usage easier.
> 
> Signed-off-by: Michal Hocko <mhocko@suse.com>

Looks good to me. You can add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/ext4/extents.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index b1f8416923ab..ef815eb72389 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -518,7 +518,7 @@ __read_extent_tree_block(const char *function, unsigned int line,
>  	struct buffer_head		*bh;
>  	int				err;
>  
> -	bh = sb_getblk_gfp(inode->i_sb, pblk, __GFP_MOVABLE | GFP_NOFS);
> +	bh = sb_getblk(inode->i_sb, pblk);
>  	if (unlikely(!bh))
>  		return ERR_PTR(-ENOMEM);
>  
> @@ -1096,7 +1096,7 @@ static int ext4_ext_split(handle_t *handle, struct inode *inode,
>  		err = -EFSCORRUPTED;
>  		goto cleanup;
>  	}
> -	bh = sb_getblk_gfp(inode->i_sb, newblock, __GFP_MOVABLE | GFP_NOFS);
> +	bh = sb_getblk(inode->i_sb, newblock);
>  	if (unlikely(!bh)) {
>  		err = -ENOMEM;
>  		goto cleanup;
> @@ -1290,7 +1290,7 @@ static int ext4_ext_grow_indepth(handle_t *handle, struct inode *inode,
>  	if (newblock == 0)
>  		return err;
>  
> -	bh = sb_getblk_gfp(inode->i_sb, newblock, __GFP_MOVABLE | GFP_NOFS);
> +	bh = sb_getblk(inode->i_sb, newblock);
>  	if (unlikely(!bh))
>  		return -ENOMEM;
>  	lock_buffer(bh);
> -- 
> 2.10.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR



  reply	other threads:[~2016-12-19  9:36 UTC|newest]

Thread overview: 138+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-15 14:07 [PATCH 0/9 v2] scope GFP_NOFS api Michal Hocko
2016-12-15 14:07 ` [Cluster-devel] " Michal Hocko
2016-12-15 14:07 ` Michal Hocko
2016-12-15 14:07 ` Michal Hocko
2016-12-15 14:07 ` Michal Hocko
2016-12-15 14:07 ` [PATCH 1/9] lockdep: allow to disable reclaim lockup detection Michal Hocko
2016-12-15 14:07   ` [Cluster-devel] " Michal Hocko
2016-12-15 14:07   ` Michal Hocko
2016-12-15 14:07   ` Michal Hocko
2016-12-15 14:07   ` Michal Hocko
2016-12-15 14:07 ` [PATCH 2/9] xfs: introduce and use KM_NOLOCKDEP to silence reclaim lockdep false positives Michal Hocko
2016-12-15 14:07   ` [Cluster-devel] " Michal Hocko
2016-12-15 14:07   ` Michal Hocko
2016-12-15 14:07   ` Michal Hocko
2016-12-15 14:07   ` Michal Hocko
2016-12-16 15:40   ` [PATCH 2/9 v2] " Michal Hocko
2016-12-16 15:40     ` [Cluster-devel] " Michal Hocko
2016-12-16 15:40     ` Michal Hocko
2016-12-16 15:40     ` Michal Hocko
2016-12-16 15:40     ` Michal Hocko
2016-12-16 16:37     ` Brian Foster
2016-12-16 16:37       ` [Cluster-devel] " Brian Foster
2016-12-16 16:37       ` Brian Foster
2016-12-16 22:01       ` Michal Hocko
2016-12-16 22:01         ` [Cluster-devel] " Michal Hocko
2016-12-16 22:01         ` Michal Hocko
2016-12-19 21:24   ` [PATCH 2/9] " Dave Chinner
2016-12-19 21:24     ` [Cluster-devel] " Dave Chinner
2016-12-19 21:24     ` Dave Chinner
2016-12-19 22:06     ` Darrick J. Wong
2016-12-19 22:07       ` [Cluster-devel] " Darrick J. Wong
2016-12-19 22:06       ` Darrick J. Wong
2016-12-20 21:39       ` Dave Chinner
2016-12-20 21:39         ` [Cluster-devel] " Dave Chinner
2016-12-20 21:39         ` Dave Chinner
2016-12-20  8:38     ` Michal Hocko
2016-12-20  8:38       ` [Cluster-devel] " Michal Hocko
2016-12-20  8:38       ` Michal Hocko
2016-12-15 14:07 ` [PATCH 3/9] xfs: abstract PF_FSTRANS to PF_MEMALLOC_NOFS Michal Hocko
2016-12-15 14:07   ` [Cluster-devel] " Michal Hocko
2016-12-15 14:07   ` Michal Hocko
2016-12-15 14:07   ` Michal Hocko
2016-12-15 14:07   ` Michal Hocko
2016-12-16 16:38   ` Brian Foster
2016-12-16 16:38     ` [Cluster-devel] " Brian Foster
2016-12-16 16:38     ` Brian Foster
2016-12-15 14:07 ` [PATCH 4/9] mm: introduce memalloc_nofs_{save,restore} API Michal Hocko
2016-12-15 14:07   ` [Cluster-devel] [PATCH 4/9] mm: introduce memalloc_nofs_{save, restore} API Michal Hocko
2016-12-15 14:07   ` [PATCH 4/9] mm: introduce memalloc_nofs_{save,restore} API Michal Hocko
2016-12-15 14:07   ` Michal Hocko
2016-12-15 14:07   ` Michal Hocko
2016-12-17 10:44   ` Tetsuo Handa
2016-12-18 16:21     ` Michal Hocko
2016-12-15 14:07 ` [PATCH 5/9] xfs: use memalloc_nofs_{save,restore} instead of memalloc_noio* Michal Hocko
2016-12-15 14:07   ` [Cluster-devel] [PATCH 5/9] xfs: use memalloc_nofs_{save, restore} " Michal Hocko
2016-12-15 14:07   ` Michal Hocko
2016-12-15 14:07   ` [PATCH 5/9] xfs: use memalloc_nofs_{save,restore} " Michal Hocko
2016-12-15 14:07   ` Michal Hocko
2016-12-15 14:07   ` Michal Hocko
2016-12-16 16:38   ` Brian Foster
2016-12-16 16:38     ` [Cluster-devel] [PATCH 5/9] xfs: use memalloc_nofs_{save, restore} " Brian Foster
2016-12-16 16:38     ` [PATCH 5/9] xfs: use memalloc_nofs_{save,restore} " Brian Foster
2016-12-16 22:00     ` [PATCH 5/9 v2] " Michal Hocko
2016-12-16 22:00       ` [Cluster-devel] [PATCH 5/9 v2] xfs: use memalloc_nofs_{save, restore} " Michal Hocko
2016-12-16 22:00       ` [PATCH 5/9 v2] xfs: use memalloc_nofs_{save,restore} " Michal Hocko
2016-12-16 22:00       ` Michal Hocko
2016-12-16 22:00       ` Michal Hocko
2016-12-15 14:07 ` [PATCH 6/9] jbd2: mark the transaction context with the scope GFP_NOFS context Michal Hocko
2016-12-15 14:07   ` [Cluster-devel] " Michal Hocko
2016-12-15 14:07   ` Michal Hocko
2016-12-15 14:07   ` Michal Hocko
2016-12-15 14:07   ` Michal Hocko
2016-12-16  8:39   ` Jan Kara
2016-12-16  8:39     ` [Cluster-devel] " Jan Kara
2016-12-16  8:39     ` Jan Kara
2016-12-15 14:07 ` [PATCH 7/9] jbd2: make the whole kjournald2 kthread NOFS safe Michal Hocko
2016-12-15 14:07   ` [Cluster-devel] " Michal Hocko
2016-12-15 14:07   ` Michal Hocko
2016-12-15 14:07   ` Michal Hocko
2016-12-15 14:07   ` Michal Hocko
2016-12-16  8:40   ` Jan Kara
2016-12-16  8:40     ` [Cluster-devel] " Jan Kara
2016-12-16  8:40     ` Jan Kara
2016-12-15 14:07 ` [PATCH 8/9] Revert "ext4: avoid deadlocks in the writeback path by using sb_getblk_gfp" Michal Hocko
2016-12-15 14:07   ` [Cluster-devel] " Michal Hocko
2016-12-15 14:07   ` Michal Hocko
2016-12-15 14:07   ` Michal Hocko
2016-12-15 14:07   ` Michal Hocko
2016-12-16  8:41   ` Jan Kara [this message]
2016-12-16  8:41     ` [Cluster-devel] " Jan Kara
2016-12-16  8:41     ` Jan Kara
2016-12-15 14:07 ` [PATCH 9/9] Revert "ext4: fix wrong gfp type under transaction" Michal Hocko
2016-12-15 14:07   ` [Cluster-devel] " Michal Hocko
2016-12-15 14:07   ` Michal Hocko
2016-12-15 14:07   ` Michal Hocko
2016-12-15 14:07   ` Michal Hocko
2016-12-16  8:43   ` Jan Kara
2016-12-16  8:43     ` [Cluster-devel] " Jan Kara
2016-12-16  8:43     ` Jan Kara
2016-12-16 12:46 ` [DEBUG PATCH 0/2] debug explicit GFP_NO{FS,IO} usage from the scope context Michal Hocko
2016-12-16 12:46   ` [Cluster-devel] [DEBUG PATCH 0/2] debug explicit GFP_NO{FS, IO} " Michal Hocko
2016-12-16 12:46   ` Michal Hocko
2016-12-16 12:46   ` [DEBUG PATCH 0/2] debug explicit GFP_NO{FS,IO} " Michal Hocko
2016-12-16 12:46   ` Michal Hocko
2016-12-16 12:46   ` Michal Hocko
2016-12-16 12:46   ` [DEBUG PATCH 1/2] mm, debug: report when GFP_NO{FS,IO} is used explicitly from memalloc_no{fs,io}_{save,restore} context Michal Hocko
2016-12-16 12:46     ` [Cluster-devel] [DEBUG PATCH 1/2] mm, debug: report when GFP_NO{FS, IO} is used explicitly from memalloc_no{fs, io}_{save, restore} context Michal Hocko
2016-12-16 12:46     ` Michal Hocko
2016-12-16 12:46     ` [DEBUG PATCH 1/2] mm, debug: report when GFP_NO{FS,IO} is used explicitly from memalloc_no{fs,io}_{save,restore} context Michal Hocko
2016-12-16 12:46     ` Michal Hocko
2016-12-16 12:46     ` Michal Hocko
2016-12-16 12:46   ` [DEBUG PATCH 2/2] silent warnings which we cannot do anything about Michal Hocko
2016-12-16 12:46     ` [Cluster-devel] " Michal Hocko
2016-12-16 12:46     ` Michal Hocko
2016-12-16 12:46     ` Michal Hocko
2016-12-16 12:46     ` Michal Hocko
2016-12-16 15:05 ` [PATCH 0/9 v2] scope GFP_NOFS api Mike Galbraith
2016-12-16 15:06   ` [Cluster-devel] " Mike Galbraith
2016-12-16 15:05   ` Mike Galbraith
2016-12-16 15:05   ` Mike Galbraith
2016-12-16 15:05   ` Mike Galbraith
2016-12-16 15:05   ` Mike Galbraith
2016-12-16 15:05   ` Mike Galbraith
2016-12-16 15:35   ` Michal Hocko
2016-12-16 15:35     ` [Cluster-devel] " Michal Hocko
2016-12-16 15:35     ` Michal Hocko
2016-12-16 16:27     ` Mike Galbraith
2016-12-16 16:27       ` [Cluster-devel] " Mike Galbraith
2016-12-16 16:27       ` Mike Galbraith
2016-12-16 16:27       ` Mike Galbraith
2016-12-16 16:27       ` Mike Galbraith
2016-12-19  9:25       ` Jan Kara
2016-12-19  9:25         ` [Cluster-devel] " Jan Kara
2016-12-19  9:25         ` Jan Kara
2016-12-22  9:38 ` Michal Hocko
2016-12-22  9:38   ` [Cluster-devel] " Michal Hocko
2016-12-22  9:38   ` Michal Hocko
2016-12-22  9:38   ` Michal Hocko

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=20161216084126.GF26608@quack2.suse.cz \
    --to=jack@suse.cz \
    --cc=akpm@linux-foundation.org \
    --cc=ceph-devel@vger.kernel.org \
    --cc=clm@fb.com \
    --cc=cluster-devel@redhat.com \
    --cc=david@fromorbit.com \
    --cc=dsterba@suse.cz \
    --cc=linux-afs@lists.infradead.org \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-f2fs-devel@lists.sourceforge.net \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=linux-ntfs-dev@lists.sourceforge.net \
    --cc=linux-xfs@vger.kernel.org \
    --cc=logfs@logfs.org \
    --cc=mhocko@kernel.org \
    --cc=mhocko@suse.com \
    --cc=reiserfs-devel@vger.kernel.org \
    --cc=tytso@mit.edu \
    /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.