On Fri, Nov 14, 2008 at 5:43 AM, Andrew McKay <amckay@iders.ca> wrote:
>> While I was debugging my issue posted in "a bug regarding when space
>> running out - shown in example",
>> I managed to narrow it down to the same issue described here. I'm
>> using 2.4.30 kernel with the latest Yaffs2
>> cvs check out, hopefully my discussion will still apply here.
>
> I have a newer kernel, and your patch won't compile with it, so I have a
> couple of quick questions.

Yes, my patch is for 2.4 kernels.

>
>>
>> My tracing showed the endless loop is caused by,
>
> Good job tracing and understanding that.  I was heading down that road, but
> got lost along the way.  I don't have enough understanding of Linux file
> systems to feel confident making changes like this.  But I thought that
> there could be a similar solution to this.
>
>> 1. busybox calls to readdir()
>
> ...
>
>> 4. default_llseek() sets file->f_version to ++event, which causes it
>> no longer same as inode->i_version, which
>> then hits Todd's patch in yaffs_readdir() and causes yaffs_readdir()
>> to list the directory files from the beginning,
>> over and over again.
>
> My default_llseek() sets file->f_version to 0.  I guess this changed with
> newer kernels?
>

In the later kernels, they seem to have got rid of the global event variable, or
maybe use for other purpose. But the idea of tracking directory changes with
f_version and i_version should remain the same.


>> This issue only happens if the listed directory has too many files to
>> be returned in one yaffs_readdir() call.
>
>> So here comes my proposal to solve the endless listing issue, It's
>> actually pretty simple - define a llssek() on directory
>> operation. What I did for my 2.4 kernel was just copying
>> generic_file_llseek(), deleting the line changing f_version, and
>> pasting
>> the rest to yaffs_fs.c, shown below.
>
> Seems to make sense.
>
>
>> Comments?
>
> Since your patch doesn't compile for newer kernels, I'll attach my patch for
> a 2.6.20.4 Kernel.  Exactly same method of fixing it as yours.  I've tested
> it and not found any issues with it.  I'm going to the kernel on some of the
> other developers systems and see how it works out there.

Nice to here it also works for you. Thanks.

> retrieving revision 1.1.8.1.24.1
> diff -u -r1.1.8.1.24.1 yaffs_fs.c
> --- yaffs_fs.c  5 Sep 2008 19:36:29 -0000       1.1.8.1.24.1
> +++ yaffs_fs.c  13 Nov 2008 18:38:16 -0000
> @@ -137,6 +137,7 @@
>  static int yaffs_sync_object(struct file *file, struct dentry *dentry,
>                             int datasync);
>
> +static loff_t yaffs_dir_llseek(struct file *file, loff_t offset, int
> origin);
>  static int yaffs_readdir(struct file *f, void *dirent, filldir_t filldir);
>
>  #if (LINUX_VERSION_CODE > KERNEL_VERSION(2,5,0))
> @@ -280,6 +281,7 @@
>        .read = generic_read_dir,
>        .readdir = yaffs_readdir,
>        .fsync = yaffs_sync_object,
> +       .llseek = yaffs_dir_llseek,
>  };
>
>  static struct super_operations yaffs_super_ops = {
> @@ -910,6 +912,29 @@
>        return nWritten == 0 ? -ENOSPC : nWritten;
>  }
>
> +static loff_t yaffs_dir_llseek(struct file *file, loff_t offset, int
> origin)
> +{
> +       long long retval;
> +
> +       lock_kernel();
> +       switch (origin) {
> +               case 2:
> +                       offset += i_size_read(file->f_path.dentry->d_inode);
> +                       break;
> +               case 1:
> +                       offset += file->f_pos;
> +       }
> +       retval = -EINVAL;
> +       if (offset >= 0) {
> +               if (offset != file->f_pos) {
> +                       file->f_pos = offset;
> +               }
> +               retval = offset;
> +       }
> +       unlock_kernel();
> +       return retval;
> +}
> +
>  static int yaffs_readdir(struct file *f, void *dirent, filldir_t filldir)
>  {
>        yaffs_Object *obj;
>
>
>
> Andrew
>

Rong