Re: 2.6.4-rc1 oops on HPFS filesystem file rename
From: Maurice van der Stee (stee_at_planet.nl)
Date: 02/29/04
- Previous message: Guennadi Liakhovetski: "[OT] Copyright infringement by Walmart?"
- In reply to: viro_at_parcelfarce.linux.theplanet.co.uk: "Re: 2.6.4-rc1 oops on HPFS filesystem file rename"
- Next in thread: viro_at_parcelfarce.linux.theplanet.co.uk: "Re: 2.6.4-rc1 oops on HPFS filesystem file rename"
- Reply: viro_at_parcelfarce.linux.theplanet.co.uk: "Re: 2.6.4-rc1 oops on HPFS filesystem file rename"
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
Date: Sun, 29 Feb 2004 12:31:30 +0100 To: viro@parcelfarce.linux.theplanet.co.uk
I think I tested most of the basic file system operations, rename,
move, copy, edit, truncate etc. and found no problems with the second
patch applied..
--
Maurice van der Stee
stee@planet.nl
On 28-02-04 20:06:49, viro@parcelfarce.linux.theplanet.co.uk wrote:
> On Sat, Feb 28, 2004 at 06:12:59PM +0100, Maurice van der Stee wrote:
> > Thanks, looks like it fixes my problem.
>
> BTW, while we are at it, patch below kills the hpfs_lock_{2,3}inodes
> ()
> monstrosities completely. We can always use parents-first ordering
> on the inodes, since these guys would only see contention with plain
> hpfs_lock_inode() and never with each other (all inodes involved
> already
> had semaphores held by callers in VFS).
>
> See if that patch (on top of the previous one) works for you...
>
> diff -urN RC4-rc1-hpfs1/fs/hpfs/buffer.c
> RC4-rc1-current/fs/hpfs/buffer.c
> --- RC4-rc1-hpfs1/fs/hpfs/buffer.c Sat Feb 28 13:55:26 2004
> +++ RC4-rc1-current/fs/hpfs/buffer.c Sat Feb 28 12:40:52 2004
> @@ -60,74 +60,6 @@
> }
> }
>
> -void hpfs_lock_2inodes(struct inode *i1, struct inode *i2)
> -{
> - if (!i2 || i1 == i2) {
> - hpfs_lock_inode(i1);
> - } else if (!i1) {
> - hpfs_lock_inode(i2);
> - } else {
> - struct hpfs_inode_info *hpfs_i1 = hpfs_i(i1);
> - struct hpfs_inode_info *hpfs_i2 = hpfs_i(i2);
> - if (i1->i_ino < i2->i_ino) {
> - down(&hpfs_i1->i_sem);
> - down(&hpfs_i2->i_sem);
> - } else {
> - down(&hpfs_i2->i_sem);
> - down(&hpfs_i1->i_sem);
> - }
> - }
> -}
> -
> -void hpfs_unlock_2inodes(struct inode *i1, struct inode *i2)
> -{
> - /* order of up() doesn't matter here */
> - hpfs_unlock_inode(i1);
> - hpfs_unlock_inode(i2);
> -}
> -
> -void hpfs_lock_3inodes(struct inode *i1, struct inode *i2, struct
> inode *i3)
> -{
> - if (!i1) { hpfs_lock_2inodes(i2, i3); return; }
> - if (!i2) { hpfs_lock_2inodes(i1, i3); return; }
> - if (!i3) { hpfs_lock_2inodes(i1, i2); return; }
> - if (i1->i_ino < i2->i_ino && i1->i_ino < i3->i_ino) {
> - struct hpfs_inode_info *hpfs_i1 = hpfs_i(i1);
> - down(&hpfs_i1->i_sem);
> - hpfs_lock_2inodes(i2, i3);
> - } else if (i2->i_ino < i1->i_ino && i2->i_ino < i3->i_ino) {
> - struct hpfs_inode_info *hpfs_i2 = hpfs_i(i2);
> - down(&hpfs_i2->i_sem);
> - hpfs_lock_2inodes(i1, i3);
> - } else if (i3->i_ino < i1->i_ino && i3->i_ino < i2->i_ino) {
> - struct hpfs_inode_info *hpfs_i3 = hpfs_i(i3);
> - down(&hpfs_i3->i_sem);
> - hpfs_lock_2inodes(i1, i2);
> - } else if (i1->i_ino != i2->i_ino) hpfs_lock_2inodes(i1, i2);
> - else hpfs_lock_2inodes(i1, i3);
> -}
> -
> -void hpfs_unlock_3inodes(struct inode *i1, struct inode *i2, struct
> inode *i3)
> -{
> - if (!i1) { hpfs_unlock_2inodes(i2, i3); return; }
> - if (!i2) { hpfs_unlock_2inodes(i1, i3); return; }
> - if (!i3) { hpfs_unlock_2inodes(i1, i2); return; }
> - if (i1->i_ino < i2->i_ino && i1->i_ino < i3->i_ino) {
> - struct hpfs_inode_info *hpfs_i1 = hpfs_i(i1);
> - hpfs_unlock_2inodes(i2, i3);
> - up(&hpfs_i1->i_sem);
> - } else if (i2->i_ino < i1->i_ino && i2->i_ino < i3->i_ino) {
> - struct hpfs_inode_info *hpfs_i2 = hpfs_i(i2);
> - hpfs_unlock_2inodes(i1, i3);
> - up(&hpfs_i2->i_sem);
> - } else if (i3->i_ino < i1->i_ino && i3->i_ino < i2->i_ino) {
> - struct hpfs_inode_info *hpfs_i3 = hpfs_i(i3);
> - hpfs_unlock_2inodes(i1, i2);
> - up(&hpfs_i3->i_sem);
> - } else if (i1->i_ino != i2->i_ino) hpfs_unlock_2inodes(i1,
> i2);
> - else hpfs_unlock_2inodes(i1, i3);
> -}
> -
> /* Map a sector into a buffer and return pointers to it and to the
> buffer. */
>
> void *hpfs_map_sector(struct super_block *s, unsigned secno, struct
> buffer_head **bhp,
> diff -urN RC4-rc1-hpfs1/fs/hpfs/hpfs_fn.h
> RC4-rc1-current/fs/hpfs/hpfs_fn.h
> --- RC4-rc1-hpfs1/fs/hpfs/hpfs_fn.h Thu Oct 9 17:34:47 2003
> +++ RC4-rc1-current/fs/hpfs/hpfs_fn.h Sat Feb 28 12:40:06 2004
> @@ -196,10 +196,6 @@
> void hpfs_unlock_iget(struct super_block *);
> void hpfs_lock_inode(struct inode *);
> void hpfs_unlock_inode(struct inode *);
> -void hpfs_lock_2inodes(struct inode *, struct inode *);
> -void hpfs_unlock_2inodes(struct inode *, struct inode *);
> -void hpfs_lock_3inodes(struct inode *, struct inode *, struct inode
> *);
> -void hpfs_unlock_3inodes(struct inode *, struct inode *, struct
> inode
> *);
> void *hpfs_map_sector(struct super_block *, unsigned, struct
> buffer_head **, int);
> void *hpfs_get_sector(struct super_block *, unsigned, struct
> buffer_head **);
> void *hpfs_map_4sectors(struct super_block *, unsigned, struct
> quad_buffer_head *, int);
> diff -urN RC4-rc1-hpfs1/fs/hpfs/namei.c
> RC4-rc1-current/fs/hpfs/namei.c
> --- RC4-rc1-hpfs1/fs/hpfs/namei.c Sat Sep 27 22:04:56 2003
> +++ RC4-rc1-current/fs/hpfs/namei.c Sat Feb 28 13:57:52 2004
> @@ -340,39 +340,41 @@
> fnode_secno fno;
> int r;
> int rep = 0;
> + int err;
>
> lock_kernel();
> hpfs_adjust_length((char *)name, &len);
> again:
> - hpfs_lock_2inodes(dir, inode);
> - if (!(de = map_dirent(dir, hpfs_i(dir)->i_dno, (char *)name,
> len, &dno, &qbh))) {
> - hpfs_unlock_2inodes(dir, inode);
> - unlock_kernel();
> - return -ENOENT;
> - }
> - if (de->first) {
> - hpfs_brelse4(&qbh);
> - hpfs_unlock_2inodes(dir, inode);
> - unlock_kernel();
> - return -EPERM;
> - }
> - if (de->directory) {
> - hpfs_brelse4(&qbh);
> - hpfs_unlock_2inodes(dir, inode);
> - unlock_kernel();
> - return -EISDIR;
> - }
> + hpfs_lock_inode(dir);
> + hpfs_lock_inode(inode);
> + de = map_dirent(dir, hpfs_i(dir)->i_dno, (char *)name, len,
> &dno, &qbh);
> + err = -ENOENT;
> + if (!de)
> + goto out;
> +
> + err = -EPERM;
> + if (de->first)
> + goto out1;
> +
> + err = -EISDIR;
> + if (de->directory)
> + goto out1;
> +
> fno = de->fnode;
> - if ((r = hpfs_remove_dirent(dir, dno, de, &qbh, 1)) == 1)
> hpfs_error(dir->i_sb, "there was error when removing dirent");
> - if (r != 2) {
> - inode->i_nlink--;
> - hpfs_unlock_2inodes(dir, inode);
> - } else { /* no space for deleting, try to truncate
> file */
> - struct iattr newattrs;
> - int err;
> - hpfs_unlock_2inodes(dir, inode);
> - if (rep)
> - goto ret;
> + r = hpfs_remove_dirent(dir, dno, de, &qbh, 1);
> + switch (r) {
> + case 1:
> + hpfs_error(dir->i_sb, "there was error when removing
> dirent");
> + err = -EFSERROR;
> + break;
> + case 2: /* no space for deleting, try to
> truncate file */
> +
> + err = -ENOSPC;
> + if (rep++)
> + break;
> +
> + hpfs_unlock_inode(inode);
> + hpfs_unlock_inode(dir);
> d_drop(dentry);
> spin_lock(&dentry->d_lock);
> if (atomic_read(&dentry->d_count) > 1 ||
> @@ -381,22 +383,32 @@
> get_write_access(inode)) {
> spin_unlock(&dentry->d_lock);
> d_rehash(dentry);
> - goto ret;
> + } else {
> + struct iattr newattrs;
> + spin_unlock(&dentry->d_lock);
> + /*printk("HPFS: truncating file before
> delete.\n");*/
> + newattrs.ia_size = 0;
> + newattrs.ia_valid = ATTR_SIZE | ATTR_CTIME;
> + err = notify_change(dentry, &newattrs);
> + put_write_access(inode);
> + if (!err)
> + goto again;
> }
> - spin_unlock(&dentry->d_lock);
> - /*printk("HPFS: truncating file before delete.\n");*/
> - newattrs.ia_size = 0;
> - newattrs.ia_valid = ATTR_SIZE | ATTR_CTIME;
> - err = notify_change(dentry, &newattrs);
> - put_write_access(inode);
> - if (err)
> - goto ret;
> - rep = 1;
> - goto again;
> + unlock_kernel();
> + return -ENOSPC;
> + default:
> + inode->i_nlink--;
> + err = 0;
> }
> -ret:
> + goto out;
> +
> +out1:
> + hpfs_brelse4(&qbh);
> +out:
> + hpfs_unlock_inode(inode);
> + hpfs_unlock_inode(dir);
> unlock_kernel();
> - return r == 2 ? -ENOSPC : r == 1 ? -EFSERROR : 0;
> + return err;
> }
>
> int hpfs_rmdir(struct inode *dir, struct dentry *dentry)
> @@ -409,44 +421,54 @@
> dnode_secno dno;
> fnode_secno fno;
> int n_items = 0;
> + int err;
> int r;
> +
> hpfs_adjust_length((char *)name, &len);
> lock_kernel();
> - hpfs_lock_2inodes(dir, inode);
> - if (!(de = map_dirent(dir, hpfs_i(dir)->i_dno, (char *)name,
> len, &dno, &qbh))) {
> - hpfs_unlock_2inodes(dir, inode);
> - unlock_kernel();
> - return -ENOENT;
> - }
> - if (de->first) {
> - hpfs_brelse4(&qbh);
> - hpfs_unlock_2inodes(dir, inode);
> - unlock_kernel();
> - return -EPERM;
> - }
> - if (!de->directory) {
> - hpfs_brelse4(&qbh);
> - hpfs_unlock_2inodes(dir, inode);
> - unlock_kernel();
> - return -ENOTDIR;
> - }
> + hpfs_lock_inode(dir);
> + hpfs_lock_inode(inode);
> + err = -ENOENT;
> + de = map_dirent(dir, hpfs_i(dir)->i_dno, (char *)name, len,
> &dno, &qbh);
> + if (!de)
> + goto out;
> +
> + err = -EPERM;
> + if (de->first)
> + goto out1;
> +
> + err = -ENOTDIR;
> + if (!de->directory)
> + goto out1;
> +
> hpfs_count_dnodes(dir->i_sb, hpfs_i(inode)->i_dno, NULL,
> NULL, &n_items);
> - if (n_items) {
> - hpfs_brelse4(&qbh);
> - hpfs_unlock_2inodes(dir, inode);
> - unlock_kernel();
> - return -ENOTEMPTY;
> - }
> + err = -ENOTEMPTY;
> + if (n_items)
> + goto out1;
> +
> fno = de->fnode;
> - if ((r = hpfs_remove_dirent(dir, dno, de, &qbh, 1)) == 1)
> + r = hpfs_remove_dirent(dir, dno, de, &qbh, 1);
> + switch (r) {
> + case 1:
> hpfs_error(dir->i_sb, "there was error when removing
> dirent");
> - if (r != 2) {
> + err = -EFSERROR;
> + break;
> + case 2:
> + err = -ENOSPC;
> + break;
> + default:
> dir->i_nlink--;
> inode->i_nlink = 0;
> - hpfs_unlock_2inodes(dir, inode);
> - } else hpfs_unlock_2inodes(dir, inode);
> + err = 0;
> + }
> + goto out;
> +out1:
> + hpfs_brelse4(&qbh);
> +out:
> + hpfs_unlock_inode(inode);
> + hpfs_unlock_inode(dir);
> unlock_kernel();
> - return r == 2 ? -ENOSPC : r == 1 ? -EFSERROR : 0;
> + return err;
> }
>
> int hpfs_symlink_readpage(struct file *file, struct page *page)
> @@ -501,7 +523,10 @@
> hpfs_adjust_length((char *)old_name, &old_len);
>
> lock_kernel();
> - hpfs_lock_3inodes(old_dir, new_dir, i);
> + hpfs_lock_inode(old_dir);
> + if (new_dir != old_dir)
> + hpfs_lock_inode(new_dir);
> + hpfs_lock_inode(i);
>
> /* Erm? Moving over the empty non-busy directory is perfectly
> legal */
> if (new_inode && S_ISDIR(new_inode->i_mode)) {
> @@ -580,7 +605,10 @@
> hpfs_i(i)->i_conv = hpfs_sb(i->i_sb)->sb_conv;
> hpfs_decide_conv(i, (char *)new_name, new_len);
> end1:
> - hpfs_unlock_3inodes(old_dir, new_dir, i);
> + hpfs_unlock_inode(i);
> + if (old_dir != new_dir)
> + hpfs_unlock_inode(new_dir);
> + hpfs_unlock_inode(old_dir);
> unlock_kernel();
> return err;
> }
>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
- Previous message: Guennadi Liakhovetski: "[OT] Copyright infringement by Walmart?"
- In reply to: viro_at_parcelfarce.linux.theplanet.co.uk: "Re: 2.6.4-rc1 oops on HPFS filesystem file rename"
- Next in thread: viro_at_parcelfarce.linux.theplanet.co.uk: "Re: 2.6.4-rc1 oops on HPFS filesystem file rename"
- Reply: viro_at_parcelfarce.linux.theplanet.co.uk: "Re: 2.6.4-rc1 oops on HPFS filesystem file rename"
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
Relevant Pages
|
|
