Re: [PATCH 3/5] fs: remove the nobh helpers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed 08-06-22 17:04:49, Christoph Hellwig wrote:
> All callers are gone, so remove the now dead code.
> 
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>

Looks good to me. Feel free to add:

Reviewed-by: Jan Kara <jack@xxxxxxx>

								Honza

> ---
>  fs/buffer.c                 | 324 ------------------------------------
>  fs/mpage.c                  |  25 +--
>  include/linux/buffer_head.h |   8 -
>  include/linux/mpage.h       |   2 -
>  4 files changed, 1 insertion(+), 358 deletions(-)
> 
> diff --git a/fs/buffer.c b/fs/buffer.c
> index ce9844d7c10fa..5717d1881d2fa 100644
> --- a/fs/buffer.c
> +++ b/fs/buffer.c
> @@ -2537,330 +2537,6 @@ int block_page_mkwrite(struct vm_area_struct *vma, struct vm_fault *vmf,
>  }
>  EXPORT_SYMBOL(block_page_mkwrite);
>  
> -/*
> - * nobh_write_begin()'s prereads are special: the buffer_heads are freed
> - * immediately, while under the page lock.  So it needs a special end_io
> - * handler which does not touch the bh after unlocking it.
> - */
> -static void end_buffer_read_nobh(struct buffer_head *bh, int uptodate)
> -{
> -	__end_buffer_read_notouch(bh, uptodate);
> -}
> -
> -/*
> - * Attach the singly-linked list of buffers created by nobh_write_begin, to
> - * the page (converting it to circular linked list and taking care of page
> - * dirty races).
> - */
> -static void attach_nobh_buffers(struct page *page, struct buffer_head *head)
> -{
> -	struct buffer_head *bh;
> -
> -	BUG_ON(!PageLocked(page));
> -
> -	spin_lock(&page->mapping->private_lock);
> -	bh = head;
> -	do {
> -		if (PageDirty(page))
> -			set_buffer_dirty(bh);
> -		if (!bh->b_this_page)
> -			bh->b_this_page = head;
> -		bh = bh->b_this_page;
> -	} while (bh != head);
> -	attach_page_private(page, head);
> -	spin_unlock(&page->mapping->private_lock);
> -}
> -
> -/*
> - * On entry, the page is fully not uptodate.
> - * On exit the page is fully uptodate in the areas outside (from,to)
> - * The filesystem needs to handle block truncation upon failure.
> - */
> -int nobh_write_begin(struct address_space *mapping, loff_t pos, unsigned len,
> -			struct page **pagep, void **fsdata,
> -			get_block_t *get_block)
> -{
> -	struct inode *inode = mapping->host;
> -	const unsigned blkbits = inode->i_blkbits;
> -	const unsigned blocksize = 1 << blkbits;
> -	struct buffer_head *head, *bh;
> -	struct page *page;
> -	pgoff_t index;
> -	unsigned from, to;
> -	unsigned block_in_page;
> -	unsigned block_start, block_end;
> -	sector_t block_in_file;
> -	int nr_reads = 0;
> -	int ret = 0;
> -	int is_mapped_to_disk = 1;
> -
> -	index = pos >> PAGE_SHIFT;
> -	from = pos & (PAGE_SIZE - 1);
> -	to = from + len;
> -
> -	page = grab_cache_page_write_begin(mapping, index);
> -	if (!page)
> -		return -ENOMEM;
> -	*pagep = page;
> -	*fsdata = NULL;
> -
> -	if (page_has_buffers(page)) {
> -		ret = __block_write_begin(page, pos, len, get_block);
> -		if (unlikely(ret))
> -			goto out_release;
> -		return ret;
> -	}
> -
> -	if (PageMappedToDisk(page))
> -		return 0;
> -
> -	/*
> -	 * Allocate buffers so that we can keep track of state, and potentially
> -	 * attach them to the page if an error occurs. In the common case of
> -	 * no error, they will just be freed again without ever being attached
> -	 * to the page (which is all OK, because we're under the page lock).
> -	 *
> -	 * Be careful: the buffer linked list is a NULL terminated one, rather
> -	 * than the circular one we're used to.
> -	 */
> -	head = alloc_page_buffers(page, blocksize, false);
> -	if (!head) {
> -		ret = -ENOMEM;
> -		goto out_release;
> -	}
> -
> -	block_in_file = (sector_t)page->index << (PAGE_SHIFT - blkbits);
> -
> -	/*
> -	 * We loop across all blocks in the page, whether or not they are
> -	 * part of the affected region.  This is so we can discover if the
> -	 * page is fully mapped-to-disk.
> -	 */
> -	for (block_start = 0, block_in_page = 0, bh = head;
> -		  block_start < PAGE_SIZE;
> -		  block_in_page++, block_start += blocksize, bh = bh->b_this_page) {
> -		int create;
> -
> -		block_end = block_start + blocksize;
> -		bh->b_state = 0;
> -		create = 1;
> -		if (block_start >= to)
> -			create = 0;
> -		ret = get_block(inode, block_in_file + block_in_page,
> -					bh, create);
> -		if (ret)
> -			goto failed;
> -		if (!buffer_mapped(bh))
> -			is_mapped_to_disk = 0;
> -		if (buffer_new(bh))
> -			clean_bdev_bh_alias(bh);
> -		if (PageUptodate(page)) {
> -			set_buffer_uptodate(bh);
> -			continue;
> -		}
> -		if (buffer_new(bh) || !buffer_mapped(bh)) {
> -			zero_user_segments(page, block_start, from,
> -							to, block_end);
> -			continue;
> -		}
> -		if (buffer_uptodate(bh))
> -			continue;	/* reiserfs does this */
> -		if (block_start < from || block_end > to) {
> -			lock_buffer(bh);
> -			bh->b_end_io = end_buffer_read_nobh;
> -			submit_bh(REQ_OP_READ, 0, bh);
> -			nr_reads++;
> -		}
> -	}
> -
> -	if (nr_reads) {
> -		/*
> -		 * The page is locked, so these buffers are protected from
> -		 * any VM or truncate activity.  Hence we don't need to care
> -		 * for the buffer_head refcounts.
> -		 */
> -		for (bh = head; bh; bh = bh->b_this_page) {
> -			wait_on_buffer(bh);
> -			if (!buffer_uptodate(bh))
> -				ret = -EIO;
> -		}
> -		if (ret)
> -			goto failed;
> -	}
> -
> -	if (is_mapped_to_disk)
> -		SetPageMappedToDisk(page);
> -
> -	*fsdata = head; /* to be released by nobh_write_end */
> -
> -	return 0;
> -
> -failed:
> -	BUG_ON(!ret);
> -	/*
> -	 * Error recovery is a bit difficult. We need to zero out blocks that
> -	 * were newly allocated, and dirty them to ensure they get written out.
> -	 * Buffers need to be attached to the page at this point, otherwise
> -	 * the handling of potential IO errors during writeout would be hard
> -	 * (could try doing synchronous writeout, but what if that fails too?)
> -	 */
> -	attach_nobh_buffers(page, head);
> -	page_zero_new_buffers(page, from, to);
> -
> -out_release:
> -	unlock_page(page);
> -	put_page(page);
> -	*pagep = NULL;
> -
> -	return ret;
> -}
> -EXPORT_SYMBOL(nobh_write_begin);
> -
> -int nobh_write_end(struct file *file, struct address_space *mapping,
> -			loff_t pos, unsigned len, unsigned copied,
> -			struct page *page, void *fsdata)
> -{
> -	struct inode *inode = page->mapping->host;
> -	struct buffer_head *head = fsdata;
> -	struct buffer_head *bh;
> -	BUG_ON(fsdata != NULL && page_has_buffers(page));
> -
> -	if (unlikely(copied < len) && head)
> -		attach_nobh_buffers(page, head);
> -	if (page_has_buffers(page))
> -		return generic_write_end(file, mapping, pos, len,
> -					copied, page, fsdata);
> -
> -	SetPageUptodate(page);
> -	set_page_dirty(page);
> -	if (pos+copied > inode->i_size) {
> -		i_size_write(inode, pos+copied);
> -		mark_inode_dirty(inode);
> -	}
> -
> -	unlock_page(page);
> -	put_page(page);
> -
> -	while (head) {
> -		bh = head;
> -		head = head->b_this_page;
> -		free_buffer_head(bh);
> -	}
> -
> -	return copied;
> -}
> -EXPORT_SYMBOL(nobh_write_end);
> -
> -/*
> - * nobh_writepage() - based on block_full_write_page() except
> - * that it tries to operate without attaching bufferheads to
> - * the page.
> - */
> -int nobh_writepage(struct page *page, get_block_t *get_block,
> -			struct writeback_control *wbc)
> -{
> -	struct inode * const inode = page->mapping->host;
> -	loff_t i_size = i_size_read(inode);
> -	const pgoff_t end_index = i_size >> PAGE_SHIFT;
> -	unsigned offset;
> -	int ret;
> -
> -	/* Is the page fully inside i_size? */
> -	if (page->index < end_index)
> -		goto out;
> -
> -	/* Is the page fully outside i_size? (truncate in progress) */
> -	offset = i_size & (PAGE_SIZE-1);
> -	if (page->index >= end_index+1 || !offset) {
> -		unlock_page(page);
> -		return 0; /* don't care */
> -	}
> -
> -	/*
> -	 * The page straddles i_size.  It must be zeroed out on each and every
> -	 * writepage invocation because it may be mmapped.  "A file is mapped
> -	 * in multiples of the page size.  For a file that is not a multiple of
> -	 * the  page size, the remaining memory is zeroed when mapped, and
> -	 * writes to that region are not written out to the file."
> -	 */
> -	zero_user_segment(page, offset, PAGE_SIZE);
> -out:
> -	ret = mpage_writepage(page, get_block, wbc);
> -	if (ret == -EAGAIN)
> -		ret = __block_write_full_page(inode, page, get_block, wbc,
> -					      end_buffer_async_write);
> -	return ret;
> -}
> -EXPORT_SYMBOL(nobh_writepage);
> -
> -int nobh_truncate_page(struct address_space *mapping,
> -			loff_t from, get_block_t *get_block)
> -{
> -	pgoff_t index = from >> PAGE_SHIFT;
> -	struct inode *inode = mapping->host;
> -	unsigned blocksize = i_blocksize(inode);
> -	struct folio *folio;
> -	struct buffer_head map_bh;
> -	size_t offset;
> -	sector_t iblock;
> -	int err;
> -
> -	/* Block boundary? Nothing to do */
> -	if (!(from & (blocksize - 1)))
> -		return 0;
> -
> -	folio = __filemap_get_folio(mapping, index, FGP_LOCK | FGP_CREAT,
> -			mapping_gfp_mask(mapping));
> -	err = -ENOMEM;
> -	if (!folio)
> -		goto out;
> -
> -	if (folio_buffers(folio))
> -		goto has_buffers;
> -
> -	iblock = from >> inode->i_blkbits;
> -	map_bh.b_size = blocksize;
> -	map_bh.b_state = 0;
> -	err = get_block(inode, iblock, &map_bh, 0);
> -	if (err)
> -		goto unlock;
> -	/* unmapped? It's a hole - nothing to do */
> -	if (!buffer_mapped(&map_bh))
> -		goto unlock;
> -
> -	/* Ok, it's mapped. Make sure it's up-to-date */
> -	if (!folio_test_uptodate(folio)) {
> -		err = mapping->a_ops->read_folio(NULL, folio);
> -		if (err) {
> -			folio_put(folio);
> -			goto out;
> -		}
> -		folio_lock(folio);
> -		if (!folio_test_uptodate(folio)) {
> -			err = -EIO;
> -			goto unlock;
> -		}
> -		if (folio_buffers(folio))
> -			goto has_buffers;
> -	}
> -	offset = offset_in_folio(folio, from);
> -	folio_zero_segment(folio, offset, round_up(offset, blocksize));
> -	folio_mark_dirty(folio);
> -	err = 0;
> -
> -unlock:
> -	folio_unlock(folio);
> -	folio_put(folio);
> -out:
> -	return err;
> -
> -has_buffers:
> -	folio_unlock(folio);
> -	folio_put(folio);
> -	return block_truncate_page(mapping, from, get_block);
> -}
> -EXPORT_SYMBOL(nobh_truncate_page);
> -
>  int block_truncate_page(struct address_space *mapping,
>  			loff_t from, get_block_t *get_block)
>  {
> diff --git a/fs/mpage.c b/fs/mpage.c
> index 0d25f44f5707c..31a97a0acf5f5 100644
> --- a/fs/mpage.c
> +++ b/fs/mpage.c
> @@ -402,7 +402,6 @@ struct mpage_data {
>  	struct bio *bio;
>  	sector_t last_block_in_bio;
>  	get_block_t *get_block;
> -	unsigned use_writepage;
>  };
>  
>  /*
> @@ -622,15 +621,10 @@ static int __mpage_writepage(struct page *page, struct writeback_control *wbc,
>  	if (bio)
>  		bio = mpage_bio_submit(bio);
>  
> -	if (mpd->use_writepage) {
> -		ret = mapping->a_ops->writepage(page, wbc);
> -	} else {
> -		ret = -EAGAIN;
> -		goto out;
> -	}
>  	/*
>  	 * The caller has a ref on the inode, so *mapping is stable
>  	 */
> +	ret = mapping->a_ops->writepage(page, wbc);
>  	mapping_set_error(mapping, ret);
>  out:
>  	mpd->bio = bio;
> @@ -672,7 +666,6 @@ mpage_writepages(struct address_space *mapping,
>  			.bio = NULL,
>  			.last_block_in_bio = 0,
>  			.get_block = get_block,
> -			.use_writepage = 1,
>  		};
>  
>  		ret = write_cache_pages(mapping, wbc, __mpage_writepage, &mpd);
> @@ -683,19 +676,3 @@ mpage_writepages(struct address_space *mapping,
>  	return ret;
>  }
>  EXPORT_SYMBOL(mpage_writepages);
> -
> -int mpage_writepage(struct page *page, get_block_t get_block,
> -	struct writeback_control *wbc)
> -{
> -	struct mpage_data mpd = {
> -		.bio = NULL,
> -		.last_block_in_bio = 0,
> -		.get_block = get_block,
> -		.use_writepage = 0,
> -	};
> -	int ret = __mpage_writepage(page, wbc, &mpd);
> -	if (mpd.bio)
> -		mpage_bio_submit(mpd.bio);
> -	return ret;
> -}
> -EXPORT_SYMBOL(mpage_writepage);
> diff --git a/include/linux/buffer_head.h b/include/linux/buffer_head.h
> index b0366c89d6a4d..61afb81cfdaea 100644
> --- a/include/linux/buffer_head.h
> +++ b/include/linux/buffer_head.h
> @@ -258,14 +258,6 @@ static inline vm_fault_t block_page_mkwrite_return(int err)
>  }
>  sector_t generic_block_bmap(struct address_space *, sector_t, get_block_t *);
>  int block_truncate_page(struct address_space *, loff_t, get_block_t *);
> -int nobh_write_begin(struct address_space *, loff_t, unsigned len,
> -				struct page **, void **, get_block_t*);
> -int nobh_write_end(struct file *, struct address_space *,
> -				loff_t, unsigned, unsigned,
> -				struct page *, void *);
> -int nobh_truncate_page(struct address_space *, loff_t, get_block_t *);
> -int nobh_writepage(struct page *page, get_block_t *get_block,
> -                        struct writeback_control *wbc);
>  
>  #ifdef CONFIG_MIGRATION
>  extern int buffer_migrate_folio(struct address_space *,
> diff --git a/include/linux/mpage.h b/include/linux/mpage.h
> index 43986f7ec4dd3..1bdc39daac0a3 100644
> --- a/include/linux/mpage.h
> +++ b/include/linux/mpage.h
> @@ -19,7 +19,5 @@ void mpage_readahead(struct readahead_control *, get_block_t get_block);
>  int mpage_read_folio(struct folio *folio, get_block_t get_block);
>  int mpage_writepages(struct address_space *mapping,
>  		struct writeback_control *wbc, get_block_t get_block);
> -int mpage_writepage(struct page *page, get_block_t *get_block,
> -		struct writeback_control *wbc);
>  
>  #endif
> -- 
> 2.30.2
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux