On Wed 04-01-23 00:02:31, Al Viro wrote: > On Tue, Jan 03, 2023 at 11:44:30AM +0100, Jan Kara wrote: > > When __mpage_writepage() is called for a page beyond EOF, it will go and > > allocate all blocks underlying the page. This is not only unnecessary > > but this way blocks can get leaked (e.g. if a page beyond EOF is marked > > dirty but in the end write fails and i_size is not extended). > > > > Signed-off-by: Jan Kara <jack@xxxxxxx> > > --- > > fs/mpage.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/fs/mpage.c b/fs/mpage.c > > index 0f8ae954a579..9f040c1d5912 100644 > > --- a/fs/mpage.c > > +++ b/fs/mpage.c > > @@ -524,6 +524,12 @@ static int __mpage_writepage(struct page *page, struct writeback_control *wbc, > > */ > > BUG_ON(!PageUptodate(page)); > > block_in_file = (sector_t)page->index << (PAGE_SHIFT - blkbits); > > + /* > > + * Whole page beyond EOF? Skip allocating blocks to avoid leaking > > + * space. > > + */ > > + if (block_in_file >= (i_size + (1 << blkbits) - 1) >> blkbits) > > + goto page_is_mapped; > > last_block = (i_size - 1) >> blkbits; > > Why not simply > > if (block_in_file > last_block) > goto page_is_mapped; > > after last_block has been calculated? Because if i_size == 0, last_block is (~0 >> blkbits) (which was actually the case the test hit). Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
