Re: [PATCH 3.2 060/110] fs/buffer.c: support buffer cache allocations with gfp modifiers

[Luis Henriques <luis.henriques@canonical.com>]

On Mon, Aug 10, 2015 at 12:12:31PM +0200, Ben Hutchings wrote:
> 3.2.71-rc1 review patch.  If anyone has any objections, please let me know.
> 
> ------------------
> 
> From: Gioh Kim <gioh.kim@lge.com>
> 
> commit 3b5e6454aaf6b4439b19400d8365e2ec2d24e411 upstream.
> 
> A buffer cache is allocated from movable area because it is referred
> for a while and released soon.  But some filesystems are taking buffer
> cache for a long time and it can disturb page migration.
> 
> New APIs are introduced to allocate buffer cache with user specific
> flag.  *_gfp APIs are for user want to set page allocation flag for
> page cache allocation.  And *_unmovable APIs are for the user wants to
> allocate page cache from non-movable area.
> 
> Signed-off-by: Gioh Kim <gioh.kim@lge.com>
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> Reviewed-by: Jan Kara <jack@suse.cz>
> [bwh: Prerequisite for "bufferhead: Add _gfp version for sb_getblk()".
>  Backported to 3.2: adjust context]

Great, I'll queue this and bd7ade3cd9b0 ("bufferhead: Add _gfp version
for sb_getblk()") for the 3.16 as well.

Cheers,
--
Lu�s

> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> ---
> --- a/fs/buffer.c
> +++ b/fs/buffer.c
> @@ -1002,7 +1002,7 @@ init_page_buffers(struct page *page, str
>   */
>  static int
>  grow_dev_page(struct block_device *bdev, sector_t block,
> -		pgoff_t index, int size, int sizebits)
> +	      pgoff_t index, int size, int sizebits, gfp_t gfp)
>  {
>  	struct inode *inode = bdev->bd_inode;
>  	struct page *page;
> @@ -1011,7 +1011,7 @@ grow_dev_page(struct block_device *bdev,
>  	int ret = 0;		/* Will call free_more_memory() */
>  
>  	page = find_or_create_page(inode->i_mapping, index,
> -		(mapping_gfp_mask(inode->i_mapping) & ~__GFP_FS)|__GFP_MOVABLE);
> +		(mapping_gfp_mask(inode->i_mapping) & ~__GFP_FS) | gfp);
>  	if (!page)
>  		return ret;
>  
> @@ -1059,7 +1059,7 @@ failed:
>   * that page was dirty, the buffers are set dirty also.
>   */
>  static int
> -grow_buffers(struct block_device *bdev, sector_t block, int size)
> +grow_buffers(struct block_device *bdev, sector_t block, int size, gfp_t gfp)
>  {
>  	pgoff_t index;
>  	int sizebits;
> @@ -1086,11 +1086,12 @@ grow_buffers(struct block_device *bdev,
>  	}
>  
>  	/* Create a page with the proper size buffers.. */
> -	return grow_dev_page(bdev, block, index, size, sizebits);
> +	return grow_dev_page(bdev, block, index, size, sizebits, gfp);
>  }
>  
> -static struct buffer_head *
> -__getblk_slow(struct block_device *bdev, sector_t block, int size)
> +struct buffer_head *
> +__getblk_slow(struct block_device *bdev, sector_t block,
> +	     unsigned size, gfp_t gfp)
>  {
>  	/* Size must be multiple of hard sectorsize */
>  	if (unlikely(size & (bdev_logical_block_size(bdev)-1) ||
> @@ -1112,13 +1113,14 @@ __getblk_slow(struct block_device *bdev,
>  		if (bh)
>  			return bh;
>  
> -		ret = grow_buffers(bdev, block, size);
> +		ret = grow_buffers(bdev, block, size, gfp);
>  		if (ret < 0)
>  			return NULL;
>  		if (ret == 0)
>  			free_more_memory();
>  	}
>  }
> +EXPORT_SYMBOL(__getblk_slow);
>  
>  /*
>   * The relationship between dirty buffers and dirty pages:
> @@ -1369,24 +1371,25 @@ __find_get_block(struct block_device *bd
>  EXPORT_SYMBOL(__find_get_block);
>  
>  /*
> - * __getblk will locate (and, if necessary, create) the buffer_head
> + * __getblk_gfp() will locate (and, if necessary, create) the buffer_head
>   * which corresponds to the passed block_device, block and size. The
>   * returned buffer has its reference count incremented.
>   *
> - * __getblk() will lock up the machine if grow_dev_page's try_to_free_buffers()
> - * attempt is failing.  FIXME, perhaps?
> + * __getblk_gfp() will lock up the machine if grow_dev_page's
> + * try_to_free_buffers() attempt is failing.  FIXME, perhaps?
>   */
>  struct buffer_head *
> -__getblk(struct block_device *bdev, sector_t block, unsigned size)
> +__getblk_gfp(struct block_device *bdev, sector_t block,
> +	     unsigned size, gfp_t gfp)
>  {
>  	struct buffer_head *bh = __find_get_block(bdev, block, size);
>  
>  	might_sleep();
>  	if (bh == NULL)
> -		bh = __getblk_slow(bdev, block, size);
> +		bh = __getblk_slow(bdev, block, size, gfp);
>  	return bh;
>  }
> -EXPORT_SYMBOL(__getblk);
> +EXPORT_SYMBOL(__getblk_gfp);
>  
>  /*
>   * Do async read-ahead on a buffer..
> @@ -1402,24 +1405,28 @@ void __breadahead(struct block_device *b
>  EXPORT_SYMBOL(__breadahead);
>  
>  /**
> - *  __bread() - reads a specified block and returns the bh
> + *  __bread_gfp() - reads a specified block and returns the bh
>   *  @bdev: the block_device to read from
>   *  @block: number of block
>   *  @size: size (in bytes) to read
> - * 
> + *  @gfp: page allocation flag
> + *
>   *  Reads a specified block, and returns buffer head that contains it.
> + *  The page cache can be allocated from non-movable area
> + *  not to prevent page migration if you set gfp to zero.
>   *  It returns NULL if the block was unreadable.
>   */
>  struct buffer_head *
> -__bread(struct block_device *bdev, sector_t block, unsigned size)
> +__bread_gfp(struct block_device *bdev, sector_t block,
> +		   unsigned size, gfp_t gfp)
>  {
> -	struct buffer_head *bh = __getblk(bdev, block, size);
> +	struct buffer_head *bh = __getblk_gfp(bdev, block, size, gfp);
>  
>  	if (likely(bh) && !buffer_uptodate(bh))
>  		bh = __bread_slow(bh);
>  	return bh;
>  }
> -EXPORT_SYMBOL(__bread);
> +EXPORT_SYMBOL(__bread_gfp);
>  
>  /*
>   * invalidate_bh_lrus() is called rarely - but not only at unmount.
> --- a/include/linux/buffer_head.h
> +++ b/include/linux/buffer_head.h
> @@ -166,12 +166,13 @@ void __wait_on_buffer(struct buffer_head
>  wait_queue_head_t *bh_waitq_head(struct buffer_head *bh);
>  struct buffer_head *__find_get_block(struct block_device *bdev, sector_t block,
>  			unsigned size);
> -struct buffer_head *__getblk(struct block_device *bdev, sector_t block,
> -			unsigned size);
> +struct buffer_head *__getblk_gfp(struct block_device *bdev, sector_t block,
> +				  unsigned size, gfp_t gfp);
>  void __brelse(struct buffer_head *);
>  void __bforget(struct buffer_head *);
>  void __breadahead(struct block_device *, sector_t block, unsigned int size);
> -struct buffer_head *__bread(struct block_device *, sector_t block, unsigned size);
> +struct buffer_head *__bread_gfp(struct block_device *,
> +				sector_t block, unsigned size, gfp_t gfp);
>  void invalidate_bh_lrus(void);
>  struct buffer_head *alloc_buffer_head(gfp_t gfp_flags);
>  void free_buffer_head(struct buffer_head * bh);
> @@ -286,7 +287,13 @@ static inline void bforget(struct buffer
>  static inline struct buffer_head *
>  sb_bread(struct super_block *sb, sector_t block)
>  {
> -	return __bread(sb->s_bdev, block, sb->s_blocksize);
> +	return __bread_gfp(sb->s_bdev, block, sb->s_blocksize, __GFP_MOVABLE);
> +}
> +
> +static inline struct buffer_head *
> +sb_bread_unmovable(struct super_block *sb, sector_t block)
> +{
> +	return __bread_gfp(sb->s_bdev, block, sb->s_blocksize, 0);
>  }
>  
>  static inline void
> @@ -298,7 +305,7 @@ sb_breadahead(struct super_block *sb, se
>  static inline struct buffer_head *
>  sb_getblk(struct super_block *sb, sector_t block)
>  {
> -	return __getblk(sb->s_bdev, block, sb->s_blocksize);
> +	return __getblk_gfp(sb->s_bdev, block, sb->s_blocksize, __GFP_MOVABLE);
>  }
>  
>  static inline struct buffer_head *
> @@ -335,6 +342,36 @@ static inline void lock_buffer(struct bu
>  		__lock_buffer(bh);
>  }
>  
> +static inline struct buffer_head *getblk_unmovable(struct block_device *bdev,
> +						   sector_t block,
> +						   unsigned size)
> +{
> +	return __getblk_gfp(bdev, block, size, 0);
> +}
> +
> +static inline struct buffer_head *__getblk(struct block_device *bdev,
> +					   sector_t block,
> +					   unsigned size)
> +{
> +	return __getblk_gfp(bdev, block, size, __GFP_MOVABLE);
> +}
> +
> +/**
> + *  __bread() - reads a specified block and returns the bh
> + *  @bdev: the block_device to read from
> + *  @block: number of block
> + *  @size: size (in bytes) to read
> + *
> + *  Reads a specified block, and returns buffer head that contains it.
> + *  The page cache is allocated from movable area so that it can be migrated.
> + *  It returns NULL if the block was unreadable.
> + */
> +static inline struct buffer_head *
> +__bread(struct block_device *bdev, sector_t block, unsigned size)
> +{
> +	return __bread_gfp(bdev, block, size, __GFP_MOVABLE);
> +}
> +
>  extern int __set_page_dirty_buffers(struct page *page);
>  
>  #else /* CONFIG_BLOCK */
> 
> --
> To unsubscribe from this list: send the line "unsubscribe stable" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html