WARNING: at fs/inode.c:280 drop_nlink+0x31/0x33()

DISCUSSION Nick Pasich 4 participants 11 messages Aug 29, 2012 – Sep 25, 2012

Subsystems (4)

NFS client NFS server SMB client (CIFS) Kernel SMB3 server (ksmbd)

lore

Share & tools

lore

Nick Pasich

author

2012-08-29 16:25 UTC #0 lore raw

I'm using kernel 3.5.3 ...

It happens on 3.5.1 and 3.5.2 also.

I know that Nick Bowler has already reported this...

I'm experiencing the same thing.

It happens when moving files from one directory to another on the same partition (NFS).

--( Nick Pasich )--

#############################################################################
##
## Happens when PSTs are moved from one directory to another on the ISCSI ... ##
#############################################################################

Aug 29 08:06:16 localhost kernel: ------------[ cut here ]------------
Aug 29 08:06:16 localhost kernel: WARNING: at fs/inode.c:280 drop_nlink+0x31/0x33()
Aug 29 08:06:16 localhost kernel: Hardware name: To Be Filled By O.E.M.
Aug 29 08:06:16 localhost kernel: Modules linked in: ecb md4 cifs w83627hf eeprom asb100 hwmon_vid hwmon nfsd exportfs ipv6 psmouse usb_storage io_edgeport usbserial sg r8169 mii evdev intel_agp uhci_hcd i2c_i801 i2c_core shpchp intel_gtt agpgart ehci_hcd microcode serio_raw
Aug 29 08:06:16 localhost kernel: Pid: 31477, comm: rm Tainted: G        W    3.5.3 #1
Aug 29 08:06:16 localhost kernel: Call Trace:
Aug 29 08:06:16 localhost kernel:  [<c10a53e2>] ? drop_nlink+0x31/0x33
Aug 29 08:06:16 localhost kernel:  [<c1025dd2>] ? warn_slowpath_common+0x7b/0x90
Aug 29 08:06:16 localhost kernel:  [<c10a53e2>] ? drop_nlink+0x31/0x33
Aug 29 08:06:16 localhost kernel:  [<c1025e02>] ? warn_slowpath_null+0x1b/0x1f
Aug 29 08:06:16 localhost kernel:  [<c10a53e2>] ? drop_nlink+0x31/0x33
Aug 29 08:06:16 localhost kernel:  [<f83dfa81>] ? cifs_unlink+0x134/0x63d [cifs]
Aug 29 08:06:16 localhost kernel:  [<c10a37d4>] ? dput+0x11/0x117
Aug 29 08:06:16 localhost kernel:  [<c10a87d4>] ? mntput_no_expire+0xf/0xf7
Aug 29 08:06:16 localhost kernel:  [<c109ca9b>] ? vfs_unlink+0x4e/0xb6
Aug 29 08:06:16 localhost kernel:  [<c109b333>] ? __lookup_hash+0x54/0xac
Aug 29 08:06:16 localhost kernel:  [<c109e8c2>] ? do_unlinkat+0x10a/0x12d
Aug 29 08:06:16 localhost kernel:  [<c10a0124>] ? sys_ioctl+0x34/0x57
Aug 29 08:06:16 localhost kernel:  [<c140a8ed>] ? syscall_call+0x7/0xb
Aug 29 08:06:16 localhost kernel: ---[ end trace 756b427e3bd671f9 ]---

Jeff Layton

2012-08-29 22:16 UTC #1 lore raw

Quoted 33 lines · On Wed, 29 Aug 2012 09:25:27 -0700 Nick Pasich…

I'm using kernel 3.5.3 ...
It happens on 3.5.1 and 3.5.2 also.
I know that Nick Bowler has already reported this...
I'm experiencing the same thing.
It happens when moving files from one directory to another on the same partition (NFS).
--( Nick Pasich )--
#############################################################################
##
## Happens when PSTs are moved from one directory to another on the ISCSI ... ##
#############################################################################
Aug 29 08:06:16 localhost kernel: ------------[ cut here ]------------
Aug 29 08:06:16 localhost kernel: WARNING: at fs/inode.c:280 drop_nlink+0x31/0x33()
Aug 29 08:06:16 localhost kernel: Hardware name: To Be Filled By O.E.M.
Aug 29 08:06:16 localhost kernel: Modules linked in: ecb md4 cifs w83627hf eeprom asb100 hwmon_vid hwmon nfsd exportfs ipv6 psmouse usb_storage io_edgeport usbserial sg r8169 mii evdev intel_agp uhci_hcd i2c_i801 i2c_core shpchp intel_gtt agpgart ehci_hcd microcode serio_raw
Aug 29 08:06:16 localhost kernel: Pid: 31477, comm: rm Tainted: G        W    3.5.3 #1
Aug 29 08:06:16 localhost kernel: Call Trace:
Aug 29 08:06:16 localhost kernel:  [<c10a53e2>] ? drop_nlink+0x31/0x33
Aug 29 08:06:16 localhost kernel:  [<c1025dd2>] ? warn_slowpath_common+0x7b/0x90
Aug 29 08:06:16 localhost kernel:  [<c10a53e2>] ? drop_nlink+0x31/0x33
Aug 29 08:06:16 localhost kernel:  [<c1025e02>] ? warn_slowpath_null+0x1b/0x1f
Aug 29 08:06:16 localhost kernel:  [<c10a53e2>] ? drop_nlink+0x31/0x33
Aug 29 08:06:16 localhost kernel:  [<f83dfa81>] ? cifs_unlink+0x134/0x63d [cifs]

Quoted 9 lines

Aug 29 08:06:16 localhost kernel:  [<c10a37d4>] ? dput+0x11/0x117
Aug 29 08:06:16 localhost kernel:  [<c10a87d4>] ? mntput_no_expire+0xf/0xf7
Aug 29 08:06:16 localhost kernel:  [<c109ca9b>] ? vfs_unlink+0x4e/0xb6
Aug 29 08:06:16 localhost kernel:  [<c109b333>] ? __lookup_hash+0x54/0xac
Aug 29 08:06:16 localhost kernel:  [<c109e8c2>] ? do_unlinkat+0x10a/0x12d
Aug 29 08:06:16 localhost kernel:  [<c10a0124>] ? sys_ioctl+0x34/0x57
Aug 29 08:06:16 localhost kernel:  [<c140a8ed>] ? syscall_call+0x7/0xb
Aug 29 08:06:16 localhost kernel: ---[ end trace 756b427e3bd671f9 ]---

(cc'ing linux-cifs ml)

This stack trace comes from cifs, not nfs.

Steve French has a patch queued in his tree to silence this warning that I believe he intends to send to Linus for 3.6. Perhaps we should consider backporting it for 3.5.z too?

Signature · 1 line

Jeff Layton <[email protected]>

Nick Pasich

author

2012-08-30 00:20 UTC #2 lore raw

Thanks for replying Jeff...

Yes, it sure would be great to have 3.5.z patch...

--( Nick Pasich )--

Quoted 56 lines · Jeff Layton

Quoted 33 lines · On Wed, 29 Aug 2012 09:25:27 -0700 Nick Pasich <[email protected]> wrote:
I'm using kernel 3.5.3 ...
It happens on 3.5.1 and 3.5.2 also.
I know that Nick Bowler has already reported this...
I'm experiencing the same thing.
It happens when moving files from one directory to another on the same partition (NFS).
--( Nick Pasich )--
#############################################################################
##
## Happens when PSTs are moved from one directory to another on the ISCSI ... ##
#############################################################################
Aug 29 08:06:16 localhost kernel: ------------[ cut here ]------------
Aug 29 08:06:16 localhost kernel: WARNING: at fs/inode.c:280 drop_nlink+0x31/0x33()
Aug 29 08:06:16 localhost kernel: Hardware name: To Be Filled By O.E.M.
Aug 29 08:06:16 localhost kernel: Modules linked in: ecb md4 cifs w83627hf eeprom asb100 hwmon_vid hwmon nfsd exportfs ipv6 psmouse usb_storage io_edgeport usbserial sg r8169 mii evdev intel_agp uhci_hcd i2c_i801 i2c_core shpchp intel_gtt agpgart ehci_hcd microcode serio_raw
Aug 29 08:06:16 localhost kernel: Pid: 31477, comm: rm Tainted: G        W    3.5.3 #1
Aug 29 08:06:16 localhost kernel: Call Trace:
Aug 29 08:06:16 localhost kernel:  [<c10a53e2>] ? drop_nlink+0x31/0x33
Aug 29 08:06:16 localhost kernel:  [<c1025dd2>] ? warn_slowpath_common+0x7b/0x90
Aug 29 08:06:16 localhost kernel:  [<c10a53e2>] ? drop_nlink+0x31/0x33
Aug 29 08:06:16 localhost kernel:  [<c1025e02>] ? warn_slowpath_null+0x1b/0x1f
Aug 29 08:06:16 localhost kernel:  [<c10a53e2>] ? drop_nlink+0x31/0x33
Aug 29 08:06:16 localhost kernel:  [<f83dfa81>] ? cifs_unlink+0x134/0x63d [cifs]
Quoted 9 lines
Aug 29 08:06:16 localhost kernel:  [<c10a37d4>] ? dput+0x11/0x117
Aug 29 08:06:16 localhost kernel:  [<c10a87d4>] ? mntput_no_expire+0xf/0xf7
Aug 29 08:06:16 localhost kernel:  [<c109ca9b>] ? vfs_unlink+0x4e/0xb6
Aug 29 08:06:16 localhost kernel:  [<c109b333>] ? __lookup_hash+0x54/0xac
Aug 29 08:06:16 localhost kernel:  [<c109e8c2>] ? do_unlinkat+0x10a/0x12d
Aug 29 08:06:16 localhost kernel:  [<c10a0124>] ? sys_ioctl+0x34/0x57
Aug 29 08:06:16 localhost kernel:  [<c140a8ed>] ? syscall_call+0x7/0xb
Aug 29 08:06:16 localhost kernel: ---[ end trace 756b427e3bd671f9 ]---
(cc'ing linux-cifs ml)
This stack trace comes from cifs, not nfs.
Steve French has a patch queued in his tree to silence this warning that I believe he intends to send to Linus for 3.6. Perhaps we should consider backporting it for 3.5.z too?
Signature · 1 line
Jeff Layton <[email protected]>

Nick Pasich

author

2012-08-31 04:33 UTC #3 lore raw

In reply to Jeff Layton

Quoted 46 lines · Jeff Layton

Quoted 33 lines · On Wed, 29 Aug 2012 09:25:27 -0700 Nick Pasich <[email protected]> wrote:

I'm using kernel 3.5.3 ...
It happens on 3.5.1 and 3.5.2 also.
I know that Nick Bowler has already reported this...
I'm experiencing the same thing.
It happens when moving files from one directory to another on the same partition (NFS).
--( Nick Pasich )--
#############################################################################
##
## Happens when PSTs are moved from one directory to another on the ISCSI ... ##
#############################################################################
Aug 29 08:06:16 localhost kernel: ------------[ cut here ]------------
Aug 29 08:06:16 localhost kernel: WARNING: at fs/inode.c:280 drop_nlink+0x31/0x33()
Aug 29 08:06:16 localhost kernel: Hardware name: To Be Filled By O.E.M.
Aug 29 08:06:16 localhost kernel: Modules linked in: ecb md4 cifs w83627hf eeprom asb100 hwmon_vid hwmon nfsd exportfs ipv6 psmouse usb_storage io_edgeport usbserial sg r8169 mii evdev intel_agp uhci_hcd i2c_i801 i2c_core shpchp intel_gtt agpgart ehci_hcd microcode serio_raw
Aug 29 08:06:16 localhost kernel: Pid: 31477, comm: rm Tainted: G        W    3.5.3 #1
Aug 29 08:06:16 localhost kernel: Call Trace:
Aug 29 08:06:16 localhost kernel:  [<c10a53e2>] ? drop_nlink+0x31/0x33
Aug 29 08:06:16 localhost kernel:  [<c1025dd2>] ? warn_slowpath_common+0x7b/0x90
Aug 29 08:06:16 localhost kernel:  [<c10a53e2>] ? drop_nlink+0x31/0x33
Aug 29 08:06:16 localhost kernel:  [<c1025e02>] ? warn_slowpath_null+0x1b/0x1f
Aug 29 08:06:16 localhost kernel:  [<c10a53e2>] ? drop_nlink+0x31/0x33
Aug 29 08:06:16 localhost kernel:  [<f83dfa81>] ? cifs_unlink+0x134/0x63d [cifs]

Quoted 9 lines

Aug 29 08:06:16 localhost kernel:  [<c10a37d4>] ? dput+0x11/0x117
Aug 29 08:06:16 localhost kernel:  [<c10a87d4>] ? mntput_no_expire+0xf/0xf7
Aug 29 08:06:16 localhost kernel:  [<c109ca9b>] ? vfs_unlink+0x4e/0xb6
Aug 29 08:06:16 localhost kernel:  [<c109b333>] ? __lookup_hash+0x54/0xac
Aug 29 08:06:16 localhost kernel:  [<c109e8c2>] ? do_unlinkat+0x10a/0x12d
Aug 29 08:06:16 localhost kernel:  [<c10a0124>] ? sys_ioctl+0x34/0x57
Aug 29 08:06:16 localhost kernel:  [<c140a8ed>] ? syscall_call+0x7/0xb
Aug 29 08:06:16 localhost kernel: ---[ end trace 756b427e3bd671f9 ]---

Quoted 10 lines

(cc'ing linux-cifs ml)
This stack trace comes from cifs, not nfs.
Steve French has a patch queued in his tree to silence this warning that I believe he intends to send to Linus for 3.6. Perhaps we should consider backporting it for 3.5.z too?
Signature · 1 line
Jeff Layton <[email protected]>

Jeff,

I applied this patch to Kernel 3.5.3 from Pavel and the the warning is gone with no problems.

Thanks,

--( Nick Pasich

##########################################################

Quoted 1 line

From df2d6b1fbf2401c5ee04f2ac143ea0954e3a87a6 Mon Sep 17 00:00:00 2001

From: Pavel Shilovsky <[email protected]>
Date: Fri, 13 Jul 2012 11:59:45 +0400
Subject: [PATCH] CIFS: Protect i_nlink from being negative

that can cause warning messages.

Signed-off-by Pavel Shilovsky <[email protected]>

---
 fs/cifs/inode.c |   13 +++++++++++--
 1 files changed, 11 insertions(+), 2 deletions(-)

fs/cifs/inode.c +0 -0

index 7354877..88afb1a 100644
@@ -1110,6 +1110,15 @@ undo_setattr:

		goto out_close;

}

+/* copied from fs/nfs/dir.c with small changes */ +static void
+cifs_drop_nlink(struct inode *inode)
+{
+ spin_lock(&inode->i_lock);
+ if (inode->i_nlink > 0)
+ drop_nlink(inode);
+ spin_unlock(&inode->i_lock);
+}

/*
* If dentry->d_inode is null (usually meaning the cached dentry
@@ -1166,13 +1175,13 @@ retry_std_delete:
psx_del_no_retry:

		if (!rc) {
				if (inode)
-			drop_nlink(inode);
+			cifs_drop_nlink(inode);
		} else if (rc == -ENOENT) {
				d_drop(dentry);
		} else if (rc == -ETXTBSY) {
				rc = cifs_rename_pending_delete(full_path, dentry, xid);
				if (rc == 0)
-			drop_nlink(inode);
+			cifs_drop_nlink(inode);
		} else if ((rc == -EACCES) && (dosattr == 0) && inode) {
				attrs = kzalloc(sizeof(*attrs), GFP_KERNEL);
				if (attrs == NULL) {

Signature · 3 lines

1.7.3.3

##########################################################

Pavel Shilovsky

2012-08-31 08:00 UTC #4 lore raw

2012/8/31 Nick Pasich <[email protected]>:

Quoted 62 lines

Jeff,

I applied this patch to Kernel 3.5.3 from Pavel and the the warning is gone with no problems.

Thanks,

--( Nick Pasich

##########################################################

From df2d6b1fbf2401c5ee04f2ac143ea0954e3a87a6 Mon Sep 17 00:00:00 2001
From: Pavel Shilovsky <[email protected]>
Date: Fri, 13 Jul 2012 11:59:45 +0400
Subject: [PATCH] CIFS: Protect i_nlink from being negative

that can cause warning messages.

Signed-off-by Pavel Shilovsky <[email protected]>

---
 fs/cifs/inode.c |   13 +++++++++++--
 1 files changed, 11 insertions(+), 2 deletions(-)

fs/cifs/inode.c +11 -3

index 7354877..88afb1a 100644
@@ -1110,6 +1110,15 @@ undo_setattr:
                goto out_close;
 }
 
+/* copied from fs/nfs/dir.c with small changes */
+static void
+cifs_drop_nlink(struct inode *inode)
+{
+       spin_lock(&inode->i_lock);
+       if (inode->i_nlink > 0)
+               drop_nlink(inode);
+       spin_unlock(&inode->i_lock);
+}
 
 /*
  * If dentry->d_inode is null (usually meaning the cached dentry

@@ -1166,13 +1175,13 @@ retry_std_delete:
 psx_del_no_retry:
                if (!rc) {
                                if (inode)
-                       drop_nlink(inode);
+                       cifs_drop_nlink(inode);
                } else if (rc == -ENOENT) {
                                d_drop(dentry);
                } else if (rc == -ETXTBSY) {
                                rc = cifs_rename_pending_delete(full_path, dentry, xid);
                                if (rc == 0)
-                       drop_nlink(inode);
+                       cifs_drop_nlink(inode);
                } else if ((rc == -EACCES) && (dosattr == 0) && inode) {
                                attrs = kzalloc(sizeof(*attrs), GFP_KERNEL);
                                if (attrs == NULL) {
--

1.7.3.3

Quoted 6 lines

##########################################################
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

This one fixes only a part of the problem. Now we have another patch for this problem:

https://git.samba.org/sfrench/?p=sfrench/cifs-2.6.git;a=commitdiff;h=b7ca69289680cf631fb20b7d436467c4ec1153cd;hp=6dab7ede9390d4d937cb89feca932e4fd575d2da

Signature · 2 lines

Best regards,
Pavel Shilovsky.

Nick Pasich

author

2012-08-31 15:32 UTC #5 lore raw

Quoted 73 lines · Pavel Shilovsky

2012/8/31 Nick Pasich <[email protected]>:

Quoted 62 lines

Jeff,

I applied this patch to Kernel 3.5.3 from Pavel and the the warning is gone with no problems.

Thanks,

--( Nick Pasich

##########################################################

that can cause warning messages.

Signed-off-by Pavel Shilovsky <[email protected]>

---
 fs/cifs/inode.c |   13 +++++++++++--
 1 files changed, 11 insertions(+), 2 deletions(-)

fs/cifs/inode.c +11 -3

index 7354877..88afb1a 100644
@@ -1110,6 +1110,15 @@ undo_setattr:
                goto out_close;
 }
 
+/* copied from fs/nfs/dir.c with small changes */
+static void
+cifs_drop_nlink(struct inode *inode)
+{
+       spin_lock(&inode->i_lock);
+       if (inode->i_nlink > 0)
+               drop_nlink(inode);
+       spin_unlock(&inode->i_lock);
+}
 
 /*
  * If dentry->d_inode is null (usually meaning the cached dentry

@@ -1166,13 +1175,13 @@ retry_std_delete:
 psx_del_no_retry:
                if (!rc) {
                                if (inode)
-                       drop_nlink(inode);
+                       cifs_drop_nlink(inode);
                } else if (rc == -ENOENT) {
                                d_drop(dentry);
                } else if (rc == -ETXTBSY) {
                                rc = cifs_rename_pending_delete(full_path, dentry, xid);
                                if (rc == 0)
-                       drop_nlink(inode);
+                       cifs_drop_nlink(inode);
                } else if ((rc == -EACCES) && (dosattr == 0) && inode) {
                                attrs = kzalloc(sizeof(*attrs), GFP_KERNEL);
                                if (attrs == NULL) {
--

1.7.3.3

Quoted 6 lines

##########################################################
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

This one fixes only a part of the problem. Now we have another patch for this problem:

Quoted 5 lines

https://git.samba.org/sfrench/?p=sfrench/cifs-2.6.git;a=commitdiff;h=b7ca69289680cf631fb20b7d436467c4ec1153cd;hp=6dab7ede9390d4d937cb89feca932e4fd575d2da
Signature · 2 lines
Best regards,
Pavel Shilovsky.

Since I'm using kernel 3.5.3 , I get an error on hunk 7 of the patch.

I can do it by hand... But I want to check with you first.

Thanks,

--( Nick Pasich )--

####################################################################

Hmm... Looks like a unified diff to me...
The text leading up to this was:
--------------------------

|From b7ca69289680cf631fb20b7d436467c4ec1153cd Mon Sep 17 00:00:00 2001
|From: Steve French <[email protected]>
|Date: Fri, 3 Aug 2012 08:43:01 -0500
|Subject: [PATCH 1/1] CIFS: Protect i_nlink from being negative
|
|that can cause warning messages.  Pavel had initially
|suggested a smaller patch around drop_nlink, after
|a similar problem was discovered NFS.  Protecting
|additional places where nlink is touched was
|suggested by Jeff Layton and is included in this.
|
|Reviewed-by: Pavel Shilovsky <[email protected]>
|Reviewed-by: Jeff Layton <[email protected]>
|Signed-off-by: Steve French <sfrench-r/[email protected]>
|Signed-off-by: Steve French <[email protected]>
|---
| fs/cifs/inode.c |   24 ++++++++++++++++--------
| fs/cifs/link.c  |    2 ++
| 2 files changed, 18 insertions(+), 8 deletions(-)
|
|diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
|index 7354877..cb79c7e 100644
|--- a/fs/cifs/inode.c
|+++ b/fs/cifs/inode.c

--------------------------
Patching file fs/cifs/inode.c using Plan A...
Hunk #1 succeeded at 124.
Hunk #2 succeeded at 148.
Hunk #3 succeeded at 155.
Hunk #4 succeeded at 905 (offset 50 lines).
Hunk #5 succeeded at 1107 (offset -1 lines).
Hunk #6 succeeded at 1224 (offset 51 lines).
Hunk #7 FAILED at 1299.
1 out of 7 hunks FAILED -- saving rejects to file fs/cifs/inode.c.rej Hmm... The next patch looks like a unified diff to me... The text leading up to this was:
--------------------------

|diff --git a/fs/cifs/link.c b/fs/cifs/link.c
|index 09e4b3a..e6ce3b1 100644
|--- a/fs/cifs/link.c
|+++ b/fs/cifs/link.c

--------------------------
Patching file fs/cifs/link.c using Plan A...
Hunk #1 succeeded at 433.
Hmm... Ignoring the trailing garbage.
done

Jeff Layton

2012-08-31 16:21 UTC #6 lore raw

Quoted 82 lines · On Fri, 31 Aug 2012 08:32:06 -0700 Nick Pasich…

Quoted 73 lines · Pavel Shilovsky
2012/8/31 Nick Pasich <[email protected]>:
Quoted 62 lines
Jeff,
I applied this patch to Kernel 3.5.3 from Pavel and the the warning is gone with no problems.
Thanks,
--( Nick Pasich
##########################################################
From df2d6b1fbf2401c5ee04f2ac143ea0954e3a87a6 Mon Sep 17 00:00:00 2001
From: Pavel Shilovsky <[email protected]>
Date: Fri, 13 Jul 2012 11:59:45 +0400
Subject: [PATCH] CIFS: Protect i_nlink from being negative
that can cause warning messages.
Signed-off-by Pavel Shilovsky <[email protected]>
---
 fs/cifs/inode.c |   13 +++++++++++--
 1 files changed, 11 insertions(+), 2 deletions(-)
fs/cifs/inode.c +11 -3
index 7354877..88afb1a 100644
@@ -1110,6 +1110,15 @@ undo_setattr:
                goto out_close;
 }
 
+/* copied from fs/nfs/dir.c with small changes */
+static void
+cifs_drop_nlink(struct inode *inode)
+{
+       spin_lock(&inode->i_lock);
+       if (inode->i_nlink > 0)
+               drop_nlink(inode);
+       spin_unlock(&inode->i_lock);
+}
 
 /*
  * If dentry->d_inode is null (usually meaning the cached dentry
@@ -1166,13 +1175,13 @@ retry_std_delete:
 psx_del_no_retry:
                if (!rc) {
                                if (inode)
-                       drop_nlink(inode);
+                       cifs_drop_nlink(inode);
                } else if (rc == -ENOENT) {
                                d_drop(dentry);
                } else if (rc == -ETXTBSY) {
                                rc = cifs_rename_pending_delete(full_path, dentry, xid);
                                if (rc == 0)
-                       drop_nlink(inode);
+                       cifs_drop_nlink(inode);
                } else if ((rc == -EACCES) && (dosattr == 0) && inode) {
                                attrs = kzalloc(sizeof(*attrs), GFP_KERNEL);
                                if (attrs == NULL) {
--
1.7.3.3
Quoted 6 lines
##########################################################
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
This one fixes only a part of the problem. Now we have another patch for this problem:
Quoted 5 lines
https://git.samba.org/sfrench/?p=sfrench/cifs-2.6.git;a=commitdiff;h=b7ca69289680cf631fb20b7d436467c4ec1153cd;hp=6dab7ede9390d4d937cb89feca932e4fd575d2da
Signature · 2 lines
Best regards,
Pavel Shilovsky.

Quoted 8 lines

Since I'm using kernel 3.5.3 , I get an error on hunk 7 of the patch.
I can do it by hand... But I want to check with you first.
Thanks,
--( Nick Pasich )--

If you fix it up by hand, consider submitting it as a backport for the stable series as well.

Signature · 5 lines

Jeff Layton <[email protected]>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Nick Pasich

author

2012-08-31 18:03 UTC #7 lore raw

Quoted 89 lines · Jeff Layton

Quoted 86 lines · On Fri, 31 Aug 2012 08:32:06 -0700 Nick Pasich…
Quoted 73 lines · Pavel Shilovsky
2012/8/31 Nick Pasich <[email protected]>:
Quoted 62 lines
Jeff,
I applied this patch to Kernel 3.5.3 from Pavel and the the warning is gone with no problems.
Thanks,
--( Nick Pasich
##########################################################
From df2d6b1fbf2401c5ee04f2ac143ea0954e3a87a6 Mon Sep 17 00:00:00 2001
From: Pavel Shilovsky <[email protected]>
Date: Fri, 13 Jul 2012 11:59:45 +0400
Subject: [PATCH] CIFS: Protect i_nlink from being negative
that can cause warning messages.
Signed-off-by Pavel Shilovsky <[email protected]>
---
 fs/cifs/inode.c |   13 +++++++++++--
 1 files changed, 11 insertions(+), 2 deletions(-)
fs/cifs/inode.c +11 -3
index 7354877..88afb1a 100644
@@ -1110,6 +1110,15 @@ undo_setattr:
                goto out_close;
 }
 
+/* copied from fs/nfs/dir.c with small changes */
+static void
+cifs_drop_nlink(struct inode *inode)
+{
+       spin_lock(&inode->i_lock);
+       if (inode->i_nlink > 0)
+               drop_nlink(inode);
+       spin_unlock(&inode->i_lock);
+}
 
 /*
  * If dentry->d_inode is null (usually meaning the cached dentry
@@ -1166,13 +1175,13 @@ retry_std_delete:
 psx_del_no_retry:
                if (!rc) {
                                if (inode)
-                       drop_nlink(inode);
+                       cifs_drop_nlink(inode);
                } else if (rc == -ENOENT) {
                                d_drop(dentry);
                } else if (rc == -ETXTBSY) {
                                rc = cifs_rename_pending_delete(full_path, dentry, xid);
                                if (rc == 0)
-                       drop_nlink(inode);
+                       cifs_drop_nlink(inode);
                } else if ((rc == -EACCES) && (dosattr == 0) && inode) {
                                attrs = kzalloc(sizeof(*attrs), GFP_KERNEL);
                                if (attrs == NULL) {
--
1.7.3.3
Quoted 6 lines
##########################################################
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
This one fixes only a part of the problem. Now we have another patch for this problem:
Quoted 5 lines
https://git.samba.org/sfrench/?p=sfrench/cifs-2.6.git;a=commitdiff;h=b7ca69289680cf631fb20b7d436467c4ec1153cd;hp=6dab7ede9390d4d937cb89feca932e4fd575d2da
Signature · 2 lines
Best regards,
Pavel Shilovsky.
Since I'm using kernel 3.5.3 , I get an error on hunk 7 of the patch.
I can do it by hand... But I want to check with you first.

Quoted 10 lines

Quoted 4 lines
Thanks,
--( Nick Pasich )--
If you fix it up by hand, consider submitting it as a backport for the stable series as well.
Signature · 1 line
Jeff Layton <[email protected]>

Jeff / Pavel,

Here are my changes on hunk #7 that allow patching 3.5.3

Check it out....

--( Nick Pasich )--

##############################################################################

fs/cifs/inode.c +16 -8

diff -u a/fs/cifs/inode.c b/fs/cifs/inode.c
@@ -124,10 +124,10 @@
 {
 	struct cifsInodeInfo *cifs_i = CIFS_I(inode);
 	struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
-	unsigned long oldtime = cifs_i->time;
 
 	cifs_revalidate_cache(inode, fattr);
 
+	spin_lock(&inode->i_lock);
 	inode->i_atime = fattr->cf_atime;
 	inode->i_mtime = fattr->cf_mtime;
 	inode->i_ctime = fattr->cf_ctime;

@@ -148,9 +148,6 @@
 	else
 		cifs_i->time = jiffies;
 
-	cFYI(1, "inode 0x%p old_time=%ld new_time=%ld", inode,
-		 oldtime, cifs_i->time);
-
 	cifs_i->delete_pending = fattr->cf_flags & CIFS_FATTR_DELETE_PENDING;
 
 	cifs_i->server_eof = fattr->cf_eof;

@@ -158,7 +155,6 @@
 	 * Can't safely change the file size here if the client is writing to
 	 * it due to potential races.
 	 */
-	spin_lock(&inode->i_lock);
 	if (is_size_safe_to_change(cifs_i, fattr->cf_eof)) {
 		i_size_write(inode, fattr->cf_eof);

@@ -909,12 +905,14 @@
 
 	if (rc && tcon->ipc) {
 		cFYI(1, "ipc connection - fake read inode");
+		spin_lock(&inode->i_lock);
 		inode->i_mode |= S_IFDIR;
 		set_nlink(inode, 2);
 		inode->i_op = &cifs_ipc_inode_ops;
 		inode->i_fop = &simple_dir_operations;
 		inode->i_uid = cifs_sb->mnt_uid;
 		inode->i_gid = cifs_sb->mnt_gid;
+		spin_unlock(&inode->i_lock);
 	} else if (rc) {
 		iget_failed(inode);
 		inode = ERR_PTR(rc);

@@ -1159,6 +1157,15 @@
 	goto out_close;
 }
 
+/* copied from fs/nfs/dir.c with small changes */
+static void
+cifs_drop_nlink(struct inode *inode)
+{
+	spin_lock(&inode->i_lock);
+	if (inode->i_nlink > 0)
+		drop_nlink(inode);
+	spin_unlock(&inode->i_lock);
+}
 
 /*
  * If dentry->d_inode is null (usually meaning the cached dentry

@@ -1216,13 +1223,13 @@
 psx_del_no_retry:
 	if (!rc) {
 		if (inode)
-			drop_nlink(inode);
+			cifs_drop_nlink(inode);
 	} else if (rc == -ENOENT) {
 		d_drop(dentry);
 	} else if (rc == -ETXTBSY) {
 		rc = cifs_rename_pending_delete(full_path, dentry, xid);
 		if (rc == 0)
-			drop_nlink(inode);
+			cifs_drop_nlink(inode);
 	} else if ((rc == -EACCES) && (dosattr == 0) && inode) {
 		attrs = kzalloc(sizeof(*attrs), GFP_KERNEL);
 		if (attrs == NULL) {

@@ -1369,9 +1376,10 @@
 		d_instantiate(direntry, newinode);
 		 /* setting nlink not necessary except in cases where we
 		  * failed to get it from the server or was set bogus */
+	 	spin_lock(&direntry->d_inode->i_lock);
 		if ((direntry->d_inode) && (direntry->d_inode->i_nlink < 2))
 			set_nlink(direntry->d_inode, 2);
-
+	 	spin_unlock(&direntry->d_inode->i_lock);
 		mode &= ~current_umask();
 		/* must turn on setgid bit if parent dir has it */
 		if (inode->i_mode & S_ISGID)

fs/cifs/link.c +2 -0

diff -u a/fs/cifs/link.c b/fs/cifs/link.c
@@ -433,7 +433,9 @@
 	if (old_file->d_inode) {
 		cifsInode = CIFS_I(old_file->d_inode);
 		if (rc == 0) {
+			spin_lock(&old_file->d_inode->i_lock);
 			inc_nlink(old_file->d_inode);
+			spin_unlock(&old_file->d_inode->i_lock);
 /* BB should we make this contingent on superblock flag NOATIME? */
 /*			old_file->d_inode->i_ctime = CURRENT_TIME;*/
 			/* parent dir timestamps will update from srv

##############################################################################

Jeff Layton

Reviewed-by

2012-09-06 11:58 UTC #8 lore raw

Quoted 218 lines · On Fri, 31 Aug 2012 11:03:45 -0700 Nick Pasich…

Quoted 89 lines · Jeff Layton

Quoted 86 lines · On Fri, 31 Aug 2012 08:32:06 -0700 Nick Pasich…
Quoted 73 lines · Pavel Shilovsky
2012/8/31 Nick Pasich <[email protected]>:
Quoted 62 lines
Jeff,
I applied this patch to Kernel 3.5.3 from Pavel and the the warning is gone with no problems.
Thanks,
--( Nick Pasich
##########################################################
From df2d6b1fbf2401c5ee04f2ac143ea0954e3a87a6 Mon Sep 17 00:00:00 2001
From: Pavel Shilovsky <[email protected]>
Date: Fri, 13 Jul 2012 11:59:45 +0400
Subject: [PATCH] CIFS: Protect i_nlink from being negative
that can cause warning messages.
Signed-off-by Pavel Shilovsky <[email protected]>
---
 fs/cifs/inode.c |   13 +++++++++++--
 1 files changed, 11 insertions(+), 2 deletions(-)
fs/cifs/inode.c +11 -3
index 7354877..88afb1a 100644
@@ -1110,6 +1110,15 @@ undo_setattr:
                goto out_close;
 }
 
+/* copied from fs/nfs/dir.c with small changes */
+static void
+cifs_drop_nlink(struct inode *inode)
+{
+       spin_lock(&inode->i_lock);
+       if (inode->i_nlink > 0)
+               drop_nlink(inode);
+       spin_unlock(&inode->i_lock);
+}
 
 /*
  * If dentry->d_inode is null (usually meaning the cached dentry
@@ -1166,13 +1175,13 @@ retry_std_delete:
 psx_del_no_retry:
                if (!rc) {
                                if (inode)
-                       drop_nlink(inode);
+                       cifs_drop_nlink(inode);
                } else if (rc == -ENOENT) {
                                d_drop(dentry);
                } else if (rc == -ETXTBSY) {
                                rc = cifs_rename_pending_delete(full_path, dentry, xid);
                                if (rc == 0)
-                       drop_nlink(inode);
+                       cifs_drop_nlink(inode);
                } else if ((rc == -EACCES) && (dosattr == 0) && inode) {
                                attrs = kzalloc(sizeof(*attrs), GFP_KERNEL);
                                if (attrs == NULL) {
--
1.7.3.3
Quoted 6 lines
##########################################################
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
This one fixes only a part of the problem. Now we have another patch for this problem:
Quoted 5 lines
https://git.samba.org/sfrench/?p=sfrench/cifs-2.6.git;a=commitdiff;h=b7ca69289680cf631fb20b7d436467c4ec1153cd;hp=6dab7ede9390d4d937cb89feca932e4fd575d2da
Signature · 2 lines
Best regards,
Pavel Shilovsky.
Since I'm using kernel 3.5.3 , I get an error on hunk 7 of the patch.
I can do it by hand... But I want to check with you first.

Quoted 10 lines

Quoted 4 lines
Thanks,
--( Nick Pasich )--
If you fix it up by hand, consider submitting it as a backport for the stable series as well.
Signature · 1 line
Jeff Layton <[email protected]>

Jeff / Pavel,

Here are my changes on hunk #7 that allow patching 3.5.3

Check it out....

--( Nick Pasich )--

##############################################################################

diff -u a/fs/cifs/inode.c b/fs/cifs/inode.c

fs/cifs/inode.c +16 -8

@@ -124,10 +124,10 @@
 {
 	struct cifsInodeInfo *cifs_i = CIFS_I(inode);
 	struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
-	unsigned long oldtime = cifs_i->time;
 
 	cifs_revalidate_cache(inode, fattr);
 
+	spin_lock(&inode->i_lock);
 	inode->i_atime = fattr->cf_atime;
 	inode->i_mtime = fattr->cf_mtime;
 	inode->i_ctime = fattr->cf_ctime;

@@ -148,9 +148,6 @@
 	else
 		cifs_i->time = jiffies;
 
-	cFYI(1, "inode 0x%p old_time=%ld new_time=%ld", inode,
-		 oldtime, cifs_i->time);
-
 	cifs_i->delete_pending = fattr->cf_flags & CIFS_FATTR_DELETE_PENDING;
 
 	cifs_i->server_eof = fattr->cf_eof;

@@ -158,7 +155,6 @@
 	 * Can't safely change the file size here if the client is writing to
 	 * it due to potential races.
 	 */
-	spin_lock(&inode->i_lock);
 	if (is_size_safe_to_change(cifs_i, fattr->cf_eof)) {
 		i_size_write(inode, fattr->cf_eof);

@@ -909,12 +905,14 @@
 
 	if (rc && tcon->ipc) {
 		cFYI(1, "ipc connection - fake read inode");
+		spin_lock(&inode->i_lock);
 		inode->i_mode |= S_IFDIR;
 		set_nlink(inode, 2);
 		inode->i_op = &cifs_ipc_inode_ops;
 		inode->i_fop = &simple_dir_operations;
 		inode->i_uid = cifs_sb->mnt_uid;
 		inode->i_gid = cifs_sb->mnt_gid;
+		spin_unlock(&inode->i_lock);
 	} else if (rc) {
 		iget_failed(inode);
 		inode = ERR_PTR(rc);

@@ -1159,6 +1157,15 @@
 	goto out_close;
 }
 
+/* copied from fs/nfs/dir.c with small changes */
+static void
+cifs_drop_nlink(struct inode *inode)
+{
+	spin_lock(&inode->i_lock);
+	if (inode->i_nlink > 0)
+		drop_nlink(inode);
+	spin_unlock(&inode->i_lock);
+}
 
 /*
  * If dentry->d_inode is null (usually meaning the cached dentry

@@ -1216,13 +1223,13 @@
 psx_del_no_retry:
 	if (!rc) {
 		if (inode)
-			drop_nlink(inode);
+			cifs_drop_nlink(inode);
 	} else if (rc == -ENOENT) {
 		d_drop(dentry);
 	} else if (rc == -ETXTBSY) {
 		rc = cifs_rename_pending_delete(full_path, dentry, xid);
 		if (rc == 0)
-			drop_nlink(inode);
+			cifs_drop_nlink(inode);
 	} else if ((rc == -EACCES) && (dosattr == 0) && inode) {
 		attrs = kzalloc(sizeof(*attrs), GFP_KERNEL);
 		if (attrs == NULL) {

@@ -1369,9 +1376,10 @@
 		d_instantiate(direntry, newinode);
 		 /* setting nlink not necessary except in cases where we
 		  * failed to get it from the server or was set bogus */
+	 	spin_lock(&direntry->d_inode->i_lock);
 		if ((direntry->d_inode) && (direntry->d_inode->i_nlink < 2))
 			set_nlink(direntry->d_inode, 2);
-
+	 	spin_unlock(&direntry->d_inode->i_lock);
 		mode &= ~current_umask();
 		/* must turn on setgid bit if parent dir has it */
 		if (inode->i_mode & S_ISGID)

diff -u a/fs/cifs/link.c b/fs/cifs/link.c

fs/cifs/link.c +2 -0

@@ -433,7 +433,9 @@
 	if (old_file->d_inode) {
 		cifsInode = CIFS_I(old_file->d_inode);
 		if (rc == 0) {
+			spin_lock(&old_file->d_inode->i_lock);
 			inc_nlink(old_file->d_inode);
+			spin_unlock(&old_file->d_inode->i_lock);
 /* BB should we make this contingent on superblock flag NOATIME? */
 /*			old_file->d_inode->i_ctime = CURRENT_TIME;*/
 			/* parent dir timestamps will update from srv

Quoted 5 lines

##############################################################################
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Looks good to me. You can add:

Reviewed-by Jeff Layton <[email protected]>

NeilBrown

2012-09-25 05:27 UTC #9 lore raw

In reply to Jeff Layton

Quoted 1 line · On Wed, 29 Aug 2012 15:16:41 -0700 Jeff Layton <[email protected]> wrote:

This stack trace comes from cifs, not nfs.

It's quite easy to trigger on NFS too.

mount server:/path /mnt; exec 3>& /mnt/foo ; rm /mnt/foo; rm /mnt/.nfs* ; exec 3>&-

[634155.004438] WARNING:
at /home/abuild/rpmbuild/BUILD/kernel-desktop-3.5.0/lin [634155.004442]
Hardware name: Latitude E6510 [634155.004577]  crc_itu_t crc32c_intel
snd_hwdep snd_pcm snd_timer snd soundcor [634155.004609] Pid: 13402, comm:
bash Tainted: G        W    3.5.0-36-desktop # [634155.004611] Call Trace:
[634155.004630]  [<ffffffff8100444a>] dump_trace+0xaa/0x2b0
[634155.004641]  [<ffffffff815a23dc>] dump_stack+0x69/0x6f
[634155.004653]  [<ffffffff81041a0b>] warn_slowpath_common+0x7b/0xc0
[634155.004662]  [<ffffffff811832e4>] drop_nlink+0x34/0x40
[634155.004687]  [<ffffffffa05bb6c3>] nfs_dentry_iput+0x33/0x70 [nfs]
[634155.004714]  [<ffffffff8118049e>] dput+0x12e/0x230
[634155.004726]  [<ffffffff8116b230>] __fput+0x170/0x230
[634155.004735]  [<ffffffff81167c0f>] filp_close+0x5f/0x90
[634155.004743]  [<ffffffff81167cd7>] sys_close+0x97/0x100
[634155.004754]  [<ffffffff815c3b39>] system_call_fastpath+0x16/0x1b
[634155.004767]  [<00007f2a73a0d110>] 0x7f2a73a0d10f

Is this suitable for -stable? It seems like it is just a harmless warning.

NeilBrown

Subject: NFS: avoid warning from nfs_drop_nlink

If you remove a file which is open, NFS will 'silly-rename' it to a hidden file.
If you then remove that hidden file, and then close the open file, then nfs_dentry_iput will perform an extra drop_nlink(). Since 3.3-rc1, this has produced a warning.
The simplest way to suppress it is to use "nfs_drop_nlink" which checks for i_nlink being zero.

Signed-off-by NeilBrown <[email protected]>

fs/nfs/dir.c +0 -0

index 627f108..268af03 100644
@@ -1174,7 +1174,7 @@ static void nfs_dentry_iput(struct dentry *dentry,

struct inode *inode) NFS_I(inode)->cache_validity |= NFS_INO_INVALID_DATA;

if (dentry->d_flags & DCACHE_NFSFS_RENAMED) {
- drop_nlink(inode);
+ nfs_drop_nlink(inode);
nfs_complete_unlink(dentry, inode);
}
iput(inode);

Jeff Layton

Reviewed-by

2012-09-25 10:22 UTC #10 lore raw

Quoted 49 lines · On Tue, 25 Sep 2012 15:27:31 +1000 NeilBrown <[email protected]> wrote:

Quoted 1 line · On Wed, 29 Aug 2012 15:16:41 -0700 Jeff Layton <[email protected]> wrote:
This stack trace comes from cifs, not nfs.
It's quite easy to trigger on NFS too.
mount server:/path /mnt; exec 3>& /mnt/foo ; rm /mnt/foo; rm /mnt/.nfs* ; exec 3>&-
[634155.004438] WARNING:
at /home/abuild/rpmbuild/BUILD/kernel-desktop-3.5.0/lin [634155.004442]
Hardware name: Latitude E6510 [634155.004577]  crc_itu_t crc32c_intel
snd_hwdep snd_pcm snd_timer snd soundcor [634155.004609] Pid: 13402, comm:
bash Tainted: G        W    3.5.0-36-desktop # [634155.004611] Call Trace:
[634155.004630]  [<ffffffff8100444a>] dump_trace+0xaa/0x2b0
[634155.004641]  [<ffffffff815a23dc>] dump_stack+0x69/0x6f
[634155.004653]  [<ffffffff81041a0b>] warn_slowpath_common+0x7b/0xc0
[634155.004662]  [<ffffffff811832e4>] drop_nlink+0x34/0x40
[634155.004687]  [<ffffffffa05bb6c3>] nfs_dentry_iput+0x33/0x70 [nfs]
[634155.004714]  [<ffffffff8118049e>] dput+0x12e/0x230
[634155.004726]  [<ffffffff8116b230>] __fput+0x170/0x230
[634155.004735]  [<ffffffff81167c0f>] filp_close+0x5f/0x90
[634155.004743]  [<ffffffff81167cd7>] sys_close+0x97/0x100
[634155.004754]  [<ffffffff815c3b39>] system_call_fastpath+0x16/0x1b
[634155.004767]  [<00007f2a73a0d110>] 0x7f2a73a0d10f
Is this suitable for -stable? It seems like it is just a harmless warning.
NeilBrown
Subject: NFS: avoid warning from nfs_drop_nlink
If you remove a file which is open, NFS will 'silly-rename' it to a hidden file.
If you then remove that hidden file, and then close the open file, then nfs_dentry_iput will perform an extra drop_nlink(). Since 3.3-rc1, this has produced a warning.
The simplest way to suppress it is to use "nfs_drop_nlink" which checks for i_nlink being zero.
Signed-off-by NeilBrown <[email protected]>
fs/nfs/dir.c +0 -0
index 627f108..268af03 100644

fs/nfs/dir.c static void nfs_dentry_iput(struct dentry *dentry, · line 1180

Quoted 8 lines

fs/nfs/dir.c +1 -1

@@ -1174,7 +1174,7 @@ static void nfs_dentry_iput(struct dentry *dentry,
 struct inode *inode) NFS_I(inode)->cache_validity |= NFS_INO_INVALID_DATA;
 
 	if (dentry->d_flags & DCACHE_NFSFS_RENAMED) {
-		drop_nlink(inode);
+		nfs_drop_nlink(inode);
 		nfs_complete_unlink(dentry, inode);
 	}
 	iput(inode);

That looks reasonable to me. I agree that it's a harmless warning but it looks scary to users...

Just for the sake of argument though, I wonder whether NFS or CIFS has any business manipulating the nlink count like this. It seems like it's possible to end up with these manipulations racing with attribute updates.

Would it make more sense to replace these drop_nlink calls with a call to mark the attributes as invalid? We would need to come up with a new way to deal with drop_inode however...

In any case, you can add this to the patch above if you like:

Reviewed-by Jeff Layton <[email protected]>