Re: [PATCH 02/18] fs/mpage: use blocks_per_folio instead of blocks_per_page

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

 



On Mon, Sep 18, 2023 at 01:04:54PM +0200, Hannes Reinecke wrote:
> @@ -161,7 +161,7 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args)
>  	struct folio *folio = args->folio;
>  	struct inode *inode = folio->mapping->host;
>  	const unsigned blkbits = inode->i_blkbits;
> -	const unsigned blocks_per_page = PAGE_SIZE >> blkbits;
> +	const unsigned blocks_per_folio = folio_size(folio) >> blkbits;
>  	const unsigned blocksize = 1 << blkbits;
>  	struct buffer_head *map_bh = &args->map_bh;
>  	sector_t block_in_file;
> @@ -169,7 +169,7 @@ static struct bio *do_mpage_readpage(struct mpage_readpage_args *args)
>  	sector_t last_block_in_file;
>  	sector_t blocks[MAX_BUF_PER_PAGE];
>  	unsigned page_block;
> -	unsigned first_hole = blocks_per_page;
> +	unsigned first_hole = blocks_per_folio;
>  	struct block_device *bdev = NULL;
>  	int length;
>  	int fully_mapped = 1;

I feel like we need an assertion that blocks_per_folio <=
MAX_BUF_PER_PAGE.  Otherwise this function runs off the end of the
'blocks' array.

Or (and I tried to do this once before getting bogged down), change this
function to not need the blocks array.  We need (first_block, length),
because this function will punt to 'confused' if theree is an on-disk
discontiguity.

ie this line needs to change:

-		if (page_block && blocks[page_block-1] != map_bh->b_blocknr-1)
+		if (page_block == 0)
+			first_block = map_bh->b_blocknr;
+		else if (first_block + page_block != map_bh->b_blocknr)
			goto confused;

> @@ -474,12 +474,12 @@ static int __mpage_writepage(struct folio *folio, struct writeback_control *wbc,
>  	struct address_space *mapping = folio->mapping;
>  	struct inode *inode = mapping->host;
>  	const unsigned blkbits = inode->i_blkbits;
> -	const unsigned blocks_per_page = PAGE_SIZE >> blkbits;
> +	const unsigned blocks_per_folio = folio_size(folio) >> blkbits;
>  	sector_t last_block;
>  	sector_t block_in_file;
>  	sector_t blocks[MAX_BUF_PER_PAGE];
>  	unsigned page_block;
> -	unsigned first_unmapped = blocks_per_page;
> +	unsigned first_unmapped = blocks_per_folio;
>  	struct block_device *bdev = NULL;
>  	int boundary = 0;
>  	sector_t boundary_block = 0;

Similarly, this function needss an assert.  Or remove blocks[].




[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