[OpenAFS-devel] OpenAFS, Linux and truncate_inode_pages()

chas williams - CONTRACTOR chas@cmf.nrl.navy.mil
Thu, 02 Mar 2006 07:56:45 -0500


In message <4406B16F.7030806@pclella.cern.ch>,Rainer Toebbicke writes:
>I'd prefer a
>	osi_Assert(down_trylock(&ip->i_sem) == 0);

i wouldnt bother with the assert.  i already know this is the 
case.  but you want to use an assert be my guest.  i just want
to know if this patch fixes your problem.

>> +    if (down_trylock(&ip->i_sem))
>> +	need_unlock = 0;
>> +
>
>This doesn't look sane to me! If you manage to down the i_sem then 
>you're ok if the i_sem is really needed for filemap_fdatasync, however 
>if you don't then you don't really know whether it's because you 
>already did it yourself higher up or if "somebody else" (not 
>necessarily AFS code) did and is then dangerously close to messing 
>around with the structures.

if you read my previous mail, there are three paths into this code
section.  one already holds i_sem (the kernel takes it), and two
others dont.  i could take i_sem outside this routine for the others
(afs_linux_close/afs_linux_flush and BStore), but that's just changing
more files than i would like.  for a test its simple enough to see if we
should take the lock.

if you dont like the patch, dont try it.