Re: ext2_discard_prealloc() called on each iput?

[Theodore Tso <tytso@mit.edu>]

On Tue, May 22, 2007 at 06:11:27PM +0200, Jan Kara wrote:
> 
>   while fixing some problems with preallocation in UDF, I had a look how
> ext2 solves similar problems. I found out that ext2_discard_prealloc() is
> called on every iput() from ext2_put_inode(). Is it really appropriate? I
> don't see a reason for doing so...

I agree, it's probably not appropriate.  It's been that way for a long
time, though (since 2.4.20).  It's not as horrible as it seems since
unlike traditional Unix systems, we don't call iput() as often, since
for example operations like close() end up calling dput(), which
decrements the ref. count on dentry, not the inode.  But it would
probably be better to check to see if i_count is 1 before deciding to
discard the preallocation.

>   Also I found slightly misleading the comment at ext2_release_file().
> As far as I understand the code it isn't when /all/ files are closed but
> rather when all fd's for given filp are closed. I.e. if you open the same
> file two times, ->release will get called once for each open. Am I right?

Yep!

> If so, then also calling ext2_discard_prealloc() from ext2_release_file()
> is suboptimal, isn't it?

Yes, although it's a bit better because only discaord the
preallocation if the file descriptor was opened for writing.  The file
could be opened for writing by multiple file descriptors, true, but in
that case it's likely that the write pattern will be a random access one
anyway, so the preallocated region is less useful.

Regards,

						- Ted

P.S.  Note that ext3 and ext4 use a different preallocation scheme;
still patches to fix the comments might not be a bad idea, since it
might save confusion by others later on.