Re: [PATCH 1/3]fs/inode: iunique() Optimize Performance

["Jérémy Cochoy" <jeremy.cochoy@gmail.com>]

Hello,

There is something strange in iunique : what will happend if all inode
between max_reserved+1 and (unsinged in)(0-1) ? Will it make an
infinite loop or an interruption can happen and make an inode become
free?

In that case, it will be better to stop search when counter overflow, no?

Will it not be better to use a field max_ino_used (in superblock, for
exemple) where we store the last inode allocated with iunique and make
a search only if max_ino_used become to (unsigned)(-1) ?

But, if iunique is here to provide a solution in order to generate
unused inode in filesystem which have various inode number, it's
better to use a list of used ino, in a short hash table which use the
first 8 bits of the inode, always use the same function to create a
new inode and look at the head if we can add a new inode with bigger
ino and still in the range. (But i think filesystems developper prefer
to write ther own functions in order to do that, no?)

Well, if we want to stop in case of full inode filesystem, we can put
the first condition in the head and add change return as :
return inode->i_ino > max_reserved ? res : 0; // 0 might "i can't find
an inode after max_reserved"

2009/11/25 Matthew Wilcox <matthew@wil.cx>:
> On Wed, Nov 25, 2009 at 10:09:45PM +0800, Liuweni wrote:
>> ---
>> move the if condition out the while{}.
>> While the function executing, the if
>> condition won't check again and again.
>> And this code won't change the function
>> of iunique().
>
> That's not true.
>
>> @@ -838,9 +838,10 @@ ino_t iunique(struct super_block *sb, ino_t max_reserved)
>>       ino_t res;
>>
>>       spin_lock(&inode_lock);
>> +
>> +     if (counter <= max_reserved)
>> +             counter = max_reserved + 1;
>>       do {
>> -             if (counter <= max_reserved)
>> -                     counter = max_reserved + 1;
>>               res = counter++;
>
> 'counter' is incremented here, so if it wraps around, we'll blunder into
> the reserved space again.
>
>>               head = inode_hashtable + hash(sb, res);
>>               inode = find_inode_fast(sb, head, res);
>>
>>
>> --------------
>> Best Regards,
>> Liuweni
>> 2009-11-25
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
> --
> Matthew Wilcox                          Intel Open Source Technology Centre
> "Bill, look, we understand that you're interested in selling us this
> operating system, but compare it to ours.  We can't possibly take such
> a retrograde step."
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



-- 
Jérémy Cochoy - L1 MI (Lyon1)
Mail : jeremy.cochoy@gmail.com
Tel : 06-43-01-74-02
Alias Zenol - http://zenol.fr