A quick follow up...
To summarise what seems to be going on, the problem is occuring because there
is an inode in the cache for an obect that no longer exists in YAFFS. When
asked to create a new object, YAFFS choses an objectId (same as an inode
number) that is the same as the one in cache. This means that when iget() is
called, no callback happens to yaffs_read_inode and the new object's info is
not associated with the inode in the cache and we get an inconsistency.
This happens very infrequently because the bucket size is relatively large
(256), though Michael has made a useful test case below. It occurs to me
that by changing the bucket size to a smaller power of 2 (say 4), it will
become easier to force the issue.
Michael has also provided a patch that would seem to get around the problem
by aborting an object creation when the inode refernce count is too high
(which means that this problem would have occurred). While this hack would
seem to work, I think it is, as Michael says, a "dirty hack" and cause extra
writes to flash etc.
I would prefer to do one of the following:
1) At the time of generating the objectId (inode number) for a new object,
first check that the object does not relate to an existing inode in the cache
and don't allocate that number if there is a conflict.
2)Don't just rely on the callback to yaffs_read_inode to fill out the inode
details. Also fill them out for other cases. The problem with this is that
we then end up with the thing in the cache being revalidated and cross-linked
to a differnt object which seems rather unhealthy to me! So I don't think
this will work.
3) Stop trying to reuse object ids. Instead of just always trying to reuse
the lowest value objectId in any bucket, we can rather keep allocating
upwards and wrap around when the objectId space is depleated (18 bits ==
0x40000). While this would not absolutely guarantee we don't get reuse, it
would reduce the odds by a significant amount.
4) When we delete an object keep the object id for that object "in use" until
the last iput releases it from the cache.
While (3) will likely work very well it does leave a bitter taste in the
mouth. I'd prefer (4) and (1) in that order. I don't trust (2), so dismiss
that immediately.
Comments/thoughts more than welcome.
-- Charles
On Monday 11 October 2004 07:26, Charles Manning wrote:
> Michael
>
> Thank you very much for this. I too have struggled to sometimes get my head
> around the vfs and how it works and the input from various people has beem
> very useful.
>
> I will be investigating this ASAP.
>
> Can you tell me what version of Linux you're using here? Some of these
> things might be version specific.
>
> Thanx
>
> -- CHarles
>
> On Saturday 09 October 2004 00:54, Michael Fischer wrote:
> > Hello everyone,
> > i am new to the list as well as to the linux vfs/fs stuff and i am
> > looking for any help/comments on the following:
> > In some cases when copying a big number of files i got complains from the
> > cp command that a newly created file was a directory thus not beeing able
> > to write data into it. The following steps would reproduce the behavior:
> > - delete a directory with many files and subdirs recursively.
> > - have some process which still has a reference to the dir or some file
> > open (e.g. cd /test; rm -rf /test)
> > - move or copy back the deleted dir-tree from some backup location.
> >
> > Debuging it further i found that the yaffs layer deletes objects which
> > due to references still exist in the inode cache of the vfs layer.
> > Looking into the code i found that yaffs_fs.c/yaffs_mknod =>
> > __yaffs_mknod => yaffs_FindNiceObjectBucket returns the object id .
> > Depending on
> > YAFFS_NOBJECT_BUCKETS you probably get an id of a recently deleted
> > object. When afterwards calling yaffs_get_inode(iget4) you get an
> > possibly allready existing inode (inode reference count > 1). This inode
> > doesnt get initialized so attributes like directory are still valid and
> > cause the described behavior.
> >
> > so a small patch (read: fast dirty hack) of yaffs_mknod (see below) does
> > solve the problem in my case:
> >
> > doitagain:
> > obj = __yaffs_mknod ( parent, dentry, mode, rdev);
> > if(obj)
> > {
> > inode = yaffs_get_inode(dir->i_sb, mode, rdev, obj);
> > T(YAFFS_TRACE_OS,(KERN_DEBUG"yaffs_mknod created object %d count =
> > %d\n",obj->objectId,atomic_read(&inode->i_count)));
> > if (atomic_read(&inode->i_count) > 1) {
> > iput(inode);
> > yaffs_DeleteFile(obj);
> > goto doitagain;
> > }
> > d_instantiate(dentry, inode);
> > error = 0;
> > }
> > else
> > {
> >
> > ok, as mentioned before i am new to all this therefore i am of cause
> > unsure if i havnt overseen something or this is fixed in later versions
> > or how a clean patch could look like.
> >
> > Thanks a lot for any comments and best regards,
> > Michael
> >
> >
> >
> >
> >
> >
> >
> > _______________________________________________
> > yaffs mailing list
> > yaffs@stoneboat.aleph1.co.uk
> > http://stoneboat.aleph1.co.uk/cgi-bin/mailman/listinfo/yaffs
>
> _______________________________________________
> yaffs mailing list
> yaffs@stoneboat.aleph1.co.uk
> http://stoneboat.aleph1.co.uk/cgi-bin/mailman/listinfo/yaffs
(text/plain)