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 9/18/23 15:15, Matthew Wilcox wrote:
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[].

Will have a look. Just tried the obvious conversion as I was still trying to get the thing to work at that point. So bear with me.

Cheers,

Hannes
--
Dr. Hannes Reinecke                Kernel Storage Architect
hare@xxxxxxx                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman




[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux