[Linux-cluster] Freeze with cluster-2.03.11

[Wendy Cheng]

Kadlecsik Jozsef wrote:
> On Fri, 3 Apr 2009, Wendy Cheng wrote:
>
>   
>> Kadlecsik Jozsef wrote:
>>     
>>> On Thu, 2 Apr 2009, Wendy Cheng wrote:
>>>       
>>>>>> Kadlecsik Jozsef wrote:
>>>>>>             
>>>>>>             
>>>>>>> - commit 82d176ba485f2ef049fd303b9e41868667cebbdb
>>>>>>>   gfs_drop_inode as .drop_inode replacing .put_inode.
>>>>>>>   .put_inode was called without holding a lock, but .drop_inode
>>>>>>>   is called under inode_lock held. Might it be a problem
>>>>>>>                 
>>>>>>>               
>>>> Based on code reading ...
>>>> 1. iput() gets inode_lock (a spin lock)
>>>> 2. iput() calls iput_final()
>>>> 3. iput_final() calls filesystem drop_inode(), followed by
>>>> generic_drop_inode()
>>>> 4. generic_drop_inode() unlock inode_lock after doing all sorts of fun
>>>> things
>>>> with the inode
>>>>
>>>> So look to me that generic_drop_inode() statement within gfs_drop_inode()
>>>> should be removed. Otherwise you would get double unlock and double list
>>>> free.
>>>>         
>>> I think those function calls are right: iput_final calls either the
>>> filesystem drop_inode function (in this case gfs_drop_inode) or
>>> generic_drop_inode. There's no double call of generic_drop_inode. However
>>> gfs_sync_page_i (and in turn filemap_fdatawrite and filemap_fdatawait) is
>>> now called under inode_lock held and that was not so in previous versions.
>>> But I'm just speculating.
>>>       
>> It *is* called twice unless my eyes deceive me
>>
>> static inline void iput_final(struct inode *inode)
>> {
>> const struct super_operations *op = inode->i_sb->s_op;
>> void (*drop)(struct inode *) = generic_drop_inode;
>>
>> if (op && op->drop_inode)
>> drop = op->drop_inode; /* gfs call generic_drop_inode() */
>> drop(inode); /* second call into generic_drop_inode() again. */
>> }
>>     
>
> No, the line 'drop = op->drop_inode;' is just an assignment (there's no 
>   

ok, I see ... my eyes do deceive me :) - actually it is my brain that 
was not working ...

Then don't remove it yet. The ramification needs more thoughts ...

-- Wendy