Re: [RFC 1/3] vfs: get_next_ino(), never inum=0

[Eric Dumazet <edumazet@google.com>]

On Tue, May 27, 2014 at 9:28 PM, Hugh Dickins <hughd@google.com> wrote:

>> Signed-off-by: J. R. Okajima <hooanon05g@gmail.com>
>
> Reviewed-by: Jan Kara <jack@suse.cz>
> Acked-by: Hugh Dickins <hughd@google.com>
>
>> ---
>>  fs/inode.c |    6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/inode.c b/fs/inode.c
>> index f96d2a6..a3e274a 100644
>> --- a/fs/inode.c
>> +++ b/fs/inode.c
>> @@ -848,7 +848,11 @@ unsigned int get_next_ino(void)
>>       }
>>  #endif
>>
>> -     *p = ++res;
>> +     res++;
>> +     /* never zero */
>> +     if (unlikely(!res))
>> +             res++;
>> +     *p = res;
>>       put_cpu_var(last_ino);
>>       return res;
>>  }

This is not right.

Each cpu 'owns' a window of 1024 consecutive inums.

If res == 0, then 1 does not necessarily belong to this cpu, and you
risk that about 1024 duplicate inums are given.

You need something like (sorry for bad formatting)

diff --git a/fs/inode.c b/fs/inode.c
index f96d2a6f88cc..66fd1189ddaa 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -839,6 +839,8 @@ unsigned int get_next_ino(void)
        unsigned int *p = &get_cpu_var(last_ino);
        unsigned int res = *p;

+start:
+
 #ifdef CONFIG_SMP
        if (unlikely((res & (LAST_INO_BATCH-1)) == 0)) {
                static atomic_t shared_last_ino;
@@ -848,7 +850,10 @@ unsigned int get_next_ino(void)
        }
 #endif

-       *p = ++res;
+       if (unlikely(++res == 0))
+               goto start;
+       *p = res;
+
        put_cpu_var(last_ino);
        return res;
 }