> >>
> >> >> BTW, I just integrated all your patches in kernel for updates (will
> >> >> be in next cooker kernel). I just didn't delete put_inode() as
> >> >> your patch does.
> >>
> andrey> Strange. For all I can tell it was the exact reason for
> >> "disappearing
> andrey> files".
> >>
> >> Nope, you need it to have a working: extract cd, change cd, make it
> >> working.
> >>
>
> borzenkov> Nope, it is just an accidental side effect. Put_inode (as
> borzenkov> it was) is buggy and not needed.
>
> Are you sure? We are using put_inode() to _never_ cache inodes of
> supermounted media after use. Supermount relies on that behaviour.
>
No. It relies on inode generation number. When you hit stale (obsolete)
inode you get ESTALE. When you hit file on NFS that has been removed on
other side you get ESTALE. Same for both.
Inodes not busy go away with invalidate_inodes. Inodes that _are_ busy won't
ever execute your code anyway. Besides it contained spinlock bug (not that I
care on UP :)
The only problem is that shrink_dcache_sb does not remove reference to busy
directories that explains the last case of ESTALE I described. As much as I
hate to do it I am afraid subfs_umount must do it manually (check for childs
of root dentry, unlink them and let them die when last reference is gone).
May be shrink_dcache_parent does it, have to check.
As long as current state does not show obvious bugs do not waste time with
it, you have something better to do :)
Cheers
-andrey
> borzenkov> ESTALE was my fault, I did not expect that the same
> borzenkov> superblock is reused on remount with busy files. The
> borzenkov> attached patch plugs the hole (reinitialize
> borzenkov> s_media_changed) until I find a proper fix.
>
> Just saw that :( We have to reuse the superblock because we need a
> place to store the generation value.
>
> borzenkov> Also patch does
>
> borzenkov> - "mode" parameter was forgotten for ext2
>
> Applied.
>
> borzenkov> - move setting of s_media_changed to where it belongs -
> check_disk_change
>
> Done.
>
> borzenkov> - add MS_SUPERMOUNTED flag for future use
>
> Done but see below.
>
> borzenkov> - use MS_SUPERMOUNTED to properly respect reference count on CD
> eject
> borzenkov> instead of blindly disabling it
>
> Nice, adopted.
>
> borzenkov> - "self destroying" message is back, it is fixed for supermount
> and we
> borzenkov> should not hide errors for other cases.
>
> It was just a hack :p
>
>
> borzenkov> cd /mnt/cdrom/foo
> borzenkov> eject
> borzenkov> ls /mnt/cdrom
>
> borzenkov> will show /mnt/cdrom/foo as ESTALE. It is because d_invalidate
> never
> borzenkov> invalidates busy directory. I think I know how to fix it.
>
> will look at that.
>
> borzenkov> TODO
>
> borzenkov> - really fix ejecting with busy files
>
> It shouldn't be that problem, we are supposed to have the door closed
> if we have busy files. If you are using a floppy that don't have a
> locking mechanism and you retire the floppy in the middle of a write,
> well, you get what you asked for (an error).
>
> borzenkov> - fix the last case of improper ESTALE
>
> I thought this one should been fixed, will look at it.
>
> borzenkov> - some more vague ideas
>
> Humm, that is really vague.
>
> later, Juan.
>
> Now some comments about the patch:
>
> +#if defined(CONFIG_SUPERMOUNT) || defined(CONFIG_SUPERMOUNT_MODULE)
> +#define MS_SUPERMOUNTED (1<<29)
> +#endif
>
> That is a bad idea, changing it to (1 <<17) next unused value,
> MS_ACTIVE & MS_NOUSER are supposed to be very, very big numbers with
> respect to the rest of the flags.
>
>
> diff -ru -x '*~' -x '*.[oas]' -x '*.[oa].flags' -x .depend -x .config -x
> modversions.h -x '*.ver' -x autoconf.h -x version.h /usr/src/linux-2.4.21-
> 0.pre3.1mdk/fs/inode.c linux-2.4.21-0.pre3.1mdk/fs/inode.c
> --- /usr/src/linux-2.4.21-0.pre3.1mdk/fs/inode.c 2003-01-16
> 19:18:14.000000000 +0300
> +++ linux-2.4.21-0.pre3.1mdk/fs/inode.c 2003-01-27
23:04:24.000000000
> +0300
> @@ -704,9 +704,6 @@
> int res = 0;
>
> if (sb) {
> -#if defined(CONFIG_SUPERMOUNT) || defined(CONFIG_SUPERMOUNT_MODULE)
> - sb->s_media_changed = 1;
> -#endif
> res = invalidate_inodes(sb);
> drop_super(sb);
> }
>
> We lacked here also a shrink_dcache_sb(sb); :p
>
>
> --
> In theory, practice and theory are the same, but in practice they
> are different -- Larry McVoy
