Re: [PATCH] ext4: restart ext4_ext_remove_space() after transaction restart

[tytso@mit.edu]

On Tue, May 25, 2010 at 06:28:29PM +0400, Dmitry Monakhov wrote:
> tytso@mit.edu writes:
> 
> > On Thu, Apr 22, 2010 at 08:31:11AM +0400, Dmitry Monakhov wrote:
> >> @@ -2480,6 +2480,11 @@ static int ext4_ext_remove_space(struct inode *inode, ext4_lblk_t start)
> >>  out:
> >>  	ext4_ext_drop_refs(path);
> >>  	kfree(path);
> >> +	if (err == EAGAIN) {
> >
> > Surely this should be "err == -EAGAIN", no?  I'm curious how this
> > patch worked for with this typo....
> As usually it fix one thing, and broke another :(.
> So in case of alloc/truncate restart truncate will be aborted,
> so i_size != i_disk_size which must be caught by fsck (my test run
> it every time) but this never happens which is very strange.
> The only reason i can explain this that truncate was called second
> time which is probable due to should_retry_alloc logic.

Does adding the optimization I suggested help?  I was nervous because
we don't immediately abort the loop after the rm_leaf function returns
-EAGAIN.  And disentangling the code to free the buffer references
from the other processing that was happening was difficult, and I was
worried about other potential side effects when the code tried to
modify blocks that were already added to the transaction.

       	      	   		      	     - Ted