From: "Aneesh Kumar K.V" Subject: Re: ENOSPC returned during writepages Date: Thu, 21 Aug 2008 23:01:21 +0530 Message-ID: <20080821173121.GF6509@skywalker> References: <20080820054339.GB6381@skywalker> <20080821164531.GE6509@skywalker> <1219338457.6342.15.camel@mingming-laptop> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: "Theodore Ts'o" , Andreas Dilger , ext4 development To: Mingming Cao Return-path: Received: from E23SMTP06.au.ibm.com ([202.81.18.175]:58139 "EHLO e23smtp06.au.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753093AbYHURbn (ORCPT ); Thu, 21 Aug 2008 13:31:43 -0400 Received: from d23relay03.au.ibm.com (d23relay03.au.ibm.com [202.81.18.234]) by e23smtp06.au.ibm.com (8.13.1/8.13.1) with ESMTP id m7LHUwFW011257 for ; Fri, 22 Aug 2008 03:30:58 +1000 Received: from d23av01.au.ibm.com (d23av01.au.ibm.com [9.190.234.96]) by d23relay03.au.ibm.com (8.13.8/8.13.8/NCO v9.0) with ESMTP id m7LHVfdL4579562 for ; Fri, 22 Aug 2008 03:31:41 +1000 Received: from d23av01.au.ibm.com (loopback [127.0.0.1]) by d23av01.au.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id m7LHVfH7026458 for ; Fri, 22 Aug 2008 03:31:41 +1000 Content-Disposition: inline In-Reply-To: <1219338457.6342.15.camel@mingming-laptop> Sender: linux-ext4-owner@vger.kernel.org List-ID: On Thu, Aug 21, 2008 at 10:07:37AM -0700, Mingming Cao wrote: >=20 > =E5=9C=A8 2008-08-21=E5=9B=9B=E7=9A=84 22:15 +0530=EF=BC=8CAneesh Kum= ar K.V=E5=86=99=E9=81=93=EF=BC=9A > > On Wed, Aug 20, 2008 at 11:13:39AM +0530, Aneesh Kumar K.V wrote: > > > Hi, > > >=20 > > > I am getting this even with the latest patch queue. The test prog= ram is > > > a modified fsstress with fallocate support. > > >=20 > > > mpage_da_map_blocks block allocation failed for inode 377954 at l= ogical > > > offset 313 with max blocks 4 with error -28 > > > mpage_da_map_blocks block allocation failed for inode 336367 at l= ogical > > > offset 74 with max blocks 9 with error -28 > > > mpage_da_map_blocks block allocation failed for inode 345560 at l= ogical > > > offset 542 with max blocks 7 with error -28 > > > This should not happen.!! Data will be lost > > > mpage_da_map_blocks block allocation failed for inode 355317 at l= ogical > > > offset 152 with max blocks 10 with error -28 > > > This should not happen.!! Data will be lost > > > mpage_da_map_blocks block allocation failed for inode 395261 at l= ogical > > > offset 462 with max blocks 1 with error -28 > > > This should not happen.!! Data will be lost > > > mpage_da_map_blocks block allocation failed for inode 323784 at l= ogical > > > offset 313 with max blocks 11 with error -28 > > > This should not happen.!! Data will be lost > > >=20 > >=20 > > With this patch i am not seeing error. It does the below > >=20 > > a) use ext4_claim_free_blocks that also update the free blocks coun= t > > b) Later after block allocation update the free blocks count if we > > allocated less with non-delayed mode > > c) Switch to non delay mode if we are low on free blocks.=20 > >=20 > I had sent a patch to do c) yesterday, I noticed that we can't switch= to > non delayed mode if the inode already have some delalloc dirty pages. >=20 >=20 > > @@ -2462,11 +2464,21 @@ static int ext4_da_write_begin(struct file = *file, struct address_space *mapping, > > unsigned from, to; > > struct inode *inode =3D mapping->host; > > handle_t *handle; > > + s64 free_blocks; > > + struct ext4_sb_info *sbi =3D EXT4_SB(inode->i_sb); > >=20 > > index =3D pos >> PAGE_CACHE_SHIFT; > > from =3D pos & (PAGE_CACHE_SIZE - 1); > > to =3D from + len; > >=20 > > + free_blocks =3D percpu_counter_read_positive(&sbi->s_freeblocks_c= ounter); > > + if (free_blocks < (4 * (FBC_BATCH * nr_cpu_ids))) { > > + /* switch to non delalloc mode */ > > + *fsdata =3D (void *)1; > > + return ext4_write_begin(file, mapping, pos, > > + len, flags, pagep, fsdata); > > + } > > + *fsdata =3D (void *)0; >=20 > No, calling ext4_write_begin() directly won't work, as it start a > handle, do the block allocation , and leave the handle there. It > expect later the write_end aops to file the inode to the transaction > list, and close that handle.=20 >=20 > With your change, the aops write_end still points to the > ext4_da_write_end(), which doesn't match the ext4_write_begin. We nee= d > to switch the aop write_begin/write_end function pointers all togeth= er. >=20 My patch does it in a simple way. I am attaching only switch to non delalloc patch below. It is still being tested. commit 490b69bd47b9ea27b1bb86bbdfb85a2911047149 Author: Aneesh Kumar K.V Date: Thu Aug 21 22:10:50 2008 +0530 switch to non-delalloc diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c index d965a05..087abca 100644 --- a/fs/ext4/inode.c +++ b/fs/ext4/inode.c @@ -1030,11 +1030,17 @@ static void ext4_da_update_reserve_space(struct= inode *inode, int used) BUG_ON(mdb > EXT4_I(inode)->i_reserved_meta_blocks); mdb_free =3D EXT4_I(inode)->i_reserved_meta_blocks - mdb; =20 - /* Account for allocated meta_blocks */ - mdb_free -=3D EXT4_I(inode)->i_allocated_meta_blocks; + if (mdb_free) { + /* Account for allocated meta_blocks */ + mdb_free -=3D EXT4_I(inode)->i_allocated_meta_blocks; =20 - /* update fs free blocks counter for truncate case */ - percpu_counter_add(&sbi->s_freeblocks_counter, mdb_free); + /* + * We have reserved more blocks. + * Now free the extra blocks reserved + */ + percpu_counter_add(&sbi->s_freeblocks_counter, mdb_free); + EXT4_I(inode)->i_allocated_meta_blocks =3D 0; + } =20 /* update per-inode reservations */ BUG_ON(used > EXT4_I(inode)->i_reserved_data_blocks); @@ -1042,7 +1048,6 @@ static void ext4_da_update_reserve_space(struct i= node *inode, int used) =20 BUG_ON(mdb > EXT4_I(inode)->i_reserved_meta_blocks); EXT4_I(inode)->i_reserved_meta_blocks =3D mdb; - EXT4_I(inode)->i_allocated_meta_blocks =3D 0; spin_unlock(&EXT4_I(inode)->i_block_reservation_lock); } =20 @@ -2459,11 +2464,21 @@ static int ext4_da_write_begin(struct file *fil= e, struct address_space *mapping, unsigned from, to; struct inode *inode =3D mapping->host; handle_t *handle; + s64 free_blocks; + struct ext4_sb_info *sbi =3D EXT4_SB(inode->i_sb); =20 index =3D pos >> PAGE_CACHE_SHIFT; from =3D pos & (PAGE_CACHE_SIZE - 1); to =3D from + len; =20 + free_blocks =3D percpu_counter_read_positive(&sbi->s_freeblocks_count= er); + if (free_blocks < (4 * (FBC_BATCH * nr_cpu_ids))) { + /* switch to non delalloc mode */ + *fsdata =3D (void *)1; + return ext4_write_begin(file, mapping, pos, + len, flags, pagep, fsdata); + } + *fsdata =3D (void *)0; retry: /* * With delayed allocation, we don't log the i_disksize update @@ -2532,6 +2547,19 @@ static int ext4_da_write_end(struct file *file, handle_t *handle =3D ext4_journal_current_handle(); loff_t new_i_size; unsigned long start, end; + int low_free_blocks =3D (int)fsdata; + + if (low_free_blocks) { + if (ext4_should_order_data(inode)) { + return ext4_ordered_write_end(file, mapping, pos, + len, copied, page, fsdata); + } else if (ext4_should_writeback_data(inode)) { + return ext4_writeback_write_end(file, mapping, pos, + len, copied, page, fsdata); + } else { + BUG(); + } + } =20 start =3D pos & (PAGE_CACHE_SIZE - 1); end =3D start + copied -1; -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" i= n the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html