linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* WARNING: at fs/inode.c:280 drop_nlink+0x31/0x33()
@ 2012-08-29 16:25 Nick Pasich
  2012-08-29 22:16 ` Jeff Layton
  0 siblings, 1 reply; 12+ messages in thread
From: Nick Pasich @ 2012-08-29 16:25 UTC (permalink / raw)
  To: linux-nfs, linux-kernel


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 ]---


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: WARNING: at fs/inode.c:280 drop_nlink+0x31/0x33()
  2012-08-29 16:25 WARNING: at fs/inode.c:280 drop_nlink+0x31/0x33() Nick Pasich
@ 2012-08-29 22:16 ` Jeff Layton
  2012-08-30  0:20   ` Nick Pasich
                     ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Jeff Layton @ 2012-08-29 22:16 UTC (permalink / raw)
  To: Nick Pasich, linux-cifs; +Cc: linux-nfs, linux-kernel

On Wed, 29 Aug 2012 09:25:27 -0700
Nick Pasich <Nick@NickAndBarb.net> 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]
> 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?

-- 
Jeff Layton <jlayton@redhat.com>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: WARNING: at fs/inode.c:280 drop_nlink+0x31/0x33()
  2012-08-29 22:16 ` Jeff Layton
@ 2012-08-30  0:20   ` Nick Pasich
  2012-08-31  4:33   ` Nick Pasich
  2012-09-25  5:27   ` NeilBrown
  2 siblings, 0 replies; 12+ messages in thread
From: Nick Pasich @ 2012-08-30  0:20 UTC (permalink / raw)
  To: Jeff Layton; +Cc: linux-nfs, linux-kernel

Thanks for replying Jeff...

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


--( Nick Pasich )--



On Wed, Aug 29, 2012 at 03:16:41PM -0700, Jeff Layton wrote:
> On Wed, 29 Aug 2012 09:25:27 -0700
> Nick Pasich <Nick@NickAndBarb.net> 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]
> > 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?
> 
> -- 
> Jeff Layton <jlayton@redhat.com>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: WARNING: at fs/inode.c:280 drop_nlink+0x31/0x33()
  2012-08-29 22:16 ` Jeff Layton
  2012-08-30  0:20   ` Nick Pasich
@ 2012-08-31  4:33   ` Nick Pasich
  2012-08-31  8:00     ` Pavel Shilovsky
  2012-09-25  5:27   ` NeilBrown
  2 siblings, 1 reply; 12+ messages in thread
From: Nick Pasich @ 2012-08-31  4:33 UTC (permalink / raw)
  To: Jeff Layton; +Cc: linux-cifs, linux-nfs, linux-kernel

On Wed, Aug 29, 2012 at 03:16:41PM -0700, Jeff Layton wrote:
> On Wed, 29 Aug 2012 09:25:27 -0700
> Nick Pasich <Nick@NickAndBarb.net> 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]
> > 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?
> 
> -- 
> Jeff Layton <jlayton@redhat.com>

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 <pshilovsky@samba.org>
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 <pshilovsky@samba.org>
---
 fs/cifs/inode.c |   13 +++++++++++--
 1 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index 7354877..88afb1a 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -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

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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: WARNING: at fs/inode.c:280 drop_nlink+0x31/0x33()
  2012-08-31  4:33   ` Nick Pasich
@ 2012-08-31  8:00     ` Pavel Shilovsky
  2012-08-31 15:32       ` Nick Pasich
  0 siblings, 1 reply; 12+ messages in thread
From: Pavel Shilovsky @ 2012-08-31  8:00 UTC (permalink / raw)
  To: Nick Pasich; +Cc: Jeff Layton, linux-cifs, linux-nfs, linux-kernel

2012/8/31 Nick Pasich <Nick@nickandbarb.net>:
> 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 <pshilovsky@samba.org>
> 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 <pshilovsky@samba.org>
> ---
>  fs/cifs/inode.c |   13 +++++++++++--
>  1 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
> index 7354877..88afb1a 100644
> --- a/fs/cifs/inode.c
> +++ b/fs/cifs/inode.c
> @@ -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
>
> ##########################################################
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo@vger.kernel.org
> 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

-- 
Best regards,
Pavel Shilovsky.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: WARNING: at fs/inode.c:280 drop_nlink+0x31/0x33()
  2012-08-31  8:00     ` Pavel Shilovsky
@ 2012-08-31 15:32       ` Nick Pasich
  2012-08-31 16:21         ` Jeff Layton
  0 siblings, 1 reply; 12+ messages in thread
From: Nick Pasich @ 2012-08-31 15:32 UTC (permalink / raw)
  To: Pavel Shilovsky; +Cc: Jeff Layton, linux-cifs, linux-nfs, linux-kernel

On Fri, Aug 31, 2012 at 12:00:26PM +0400, Pavel Shilovsky wrote:
> 2012/8/31 Nick Pasich <Nick@nickandbarb.net>:
> > 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 <pshilovsky@samba.org>
> > 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 <pshilovsky@samba.org>
> > ---
> >  fs/cifs/inode.c |   13 +++++++++++--
> >  1 files changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
> > index 7354877..88afb1a 100644
> > --- a/fs/cifs/inode.c
> > +++ b/fs/cifs/inode.c
> > @@ -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
> >
> > ##########################################################
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> > the body of a message to majordomo@vger.kernel.org
> > 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
> 
> -- 
> 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 <smfrench@gmail.com>
|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 <pshilovsky@samba.org>
|Reviewed-by: Jeff Layton <jlayton@redhat.com>
|Signed-off-by: Steve French <sfrench@us.ibm.com>
|Signed-off-by: Steve French <smfrench@gmail.com>
|---
| 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



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: WARNING: at fs/inode.c:280 drop_nlink+0x31/0x33()
  2012-08-31 15:32       ` Nick Pasich
@ 2012-08-31 16:21         ` Jeff Layton
  2012-08-31 18:03           ` Nick Pasich
  0 siblings, 1 reply; 12+ messages in thread
From: Jeff Layton @ 2012-08-31 16:21 UTC (permalink / raw)
  To: Nick Pasich; +Cc: Pavel Shilovsky, linux-cifs, linux-nfs, linux-kernel

On Fri, 31 Aug 2012 08:32:06 -0700
Nick Pasich <Nick@NickAndBarb.net> wrote:

> On Fri, Aug 31, 2012 at 12:00:26PM +0400, Pavel Shilovsky wrote:
> > 2012/8/31 Nick Pasich <Nick@nickandbarb.net>:
> > > 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 <pshilovsky@samba.org>
> > > 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 <pshilovsky@samba.org>
> > > ---
> > >  fs/cifs/inode.c |   13 +++++++++++--
> > >  1 files changed, 11 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
> > > index 7354877..88afb1a 100644
> > > --- a/fs/cifs/inode.c
> > > +++ b/fs/cifs/inode.c
> > > @@ -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
> > >
> > > ##########################################################
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> > > the body of a message to majordomo@vger.kernel.org
> > > 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
> > 
> > -- 
> > 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 )--
> 

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

-- 
Jeff Layton <jlayton@redhat.com>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: WARNING: at fs/inode.c:280 drop_nlink+0x31/0x33()
  2012-08-31 16:21         ` Jeff Layton
@ 2012-08-31 18:03           ` Nick Pasich
  2012-09-06 11:58             ` Jeff Layton
  0 siblings, 1 reply; 12+ messages in thread
From: Nick Pasich @ 2012-08-31 18:03 UTC (permalink / raw)
  To: Jeff Layton; +Cc: Pavel Shilovsky, linux-cifs, linux-nfs, linux-kernel

On Fri, Aug 31, 2012 at 09:21:38AM -0700, Jeff Layton wrote:
> On Fri, 31 Aug 2012 08:32:06 -0700
> Nick Pasich <Nick@NickAndBarb.net> wrote:
> 
> > On Fri, Aug 31, 2012 at 12:00:26PM +0400, Pavel Shilovsky wrote:
> > > 2012/8/31 Nick Pasich <Nick@nickandbarb.net>:
> > > > 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 <pshilovsky@samba.org>
> > > > 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 <pshilovsky@samba.org>
> > > > ---
> > > >  fs/cifs/inode.c |   13 +++++++++++--
> > > >  1 files changed, 11 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
> > > > index 7354877..88afb1a 100644
> > > > --- a/fs/cifs/inode.c
> > > > +++ b/fs/cifs/inode.c
> > > > @@ -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
> > > >
> > > > ##########################################################
> > > >
> > > > --
> > > > To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> > > > the body of a message to majordomo@vger.kernel.org
> > > > 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
> > > 
> > > -- 
> > > 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 )--
> > 
> 
> If you fix it up by hand, consider submitting it as a backport for the
> stable series as well.
> 
> -- 
> Jeff Layton <jlayton@redhat.com>

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
--- 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)
diff -u a/fs/cifs/link.c b/fs/cifs/link.c
--- 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
##############################################################################

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: WARNING: at fs/inode.c:280 drop_nlink+0x31/0x33()
  2012-08-31 18:03           ` Nick Pasich
@ 2012-09-06 11:58             ` Jeff Layton
  0 siblings, 0 replies; 12+ messages in thread
From: Jeff Layton @ 2012-09-06 11:58 UTC (permalink / raw)
  To: Nick Pasich; +Cc: Pavel Shilovsky, linux-cifs, linux-nfs, linux-kernel

On Fri, 31 Aug 2012 11:03:45 -0700
Nick Pasich <Nick@NickAndBarb.net> wrote:

> On Fri, Aug 31, 2012 at 09:21:38AM -0700, Jeff Layton wrote:
> > On Fri, 31 Aug 2012 08:32:06 -0700
> > Nick Pasich <Nick@NickAndBarb.net> wrote:
> > 
> > > On Fri, Aug 31, 2012 at 12:00:26PM +0400, Pavel Shilovsky wrote:
> > > > 2012/8/31 Nick Pasich <Nick@nickandbarb.net>:
> > > > > 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 <pshilovsky@samba.org>
> > > > > 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 <pshilovsky@samba.org>
> > > > > ---
> > > > >  fs/cifs/inode.c |   13 +++++++++++--
> > > > >  1 files changed, 11 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
> > > > > index 7354877..88afb1a 100644
> > > > > --- a/fs/cifs/inode.c
> > > > > +++ b/fs/cifs/inode.c
> > > > > @@ -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
> > > > >
> > > > > ##########################################################
> > > > >
> > > > > --
> > > > > To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> > > > > the body of a message to majordomo@vger.kernel.org
> > > > > 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
> > > > 
> > > > -- 
> > > > 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 )--
> > > 
> > 
> > If you fix it up by hand, consider submitting it as a backport for the
> > stable series as well.
> > 
> > -- 
> > Jeff Layton <jlayton@redhat.com>
> 
> 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
> --- 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)
> diff -u a/fs/cifs/link.c b/fs/cifs/link.c
> --- 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
> ##############################################################################
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Looks good to me. You can add:

Reviewed-by: Jeff Layton <jlayton@redhat.com>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: WARNING: at fs/inode.c:280 drop_nlink+0x31/0x33()
  2012-08-29 22:16 ` Jeff Layton
  2012-08-30  0:20   ` Nick Pasich
  2012-08-31  4:33   ` Nick Pasich
@ 2012-09-25  5:27   ` NeilBrown
  2012-09-25 10:21     ` Jeff Layton
  2 siblings, 1 reply; 12+ messages in thread
From: NeilBrown @ 2012-09-25  5:27 UTC (permalink / raw)
  To: Jeff Layton; +Cc: Myklebust, Trond, linux-nfs, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 2233 bytes --]

On Wed, 29 Aug 2012 15:16:41 -0700 Jeff Layton <jlayton@redhat.com> 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 <neilb@suse.de>

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 627f108..268af03 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -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);

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: WARNING: at fs/inode.c:280 drop_nlink+0x31/0x33()
  2012-09-25  5:27   ` NeilBrown
@ 2012-09-25 10:21     ` Jeff Layton
  0 siblings, 0 replies; 12+ messages in thread
From: Jeff Layton @ 2012-09-25 10:21 UTC (permalink / raw)
  To: NeilBrown; +Cc: Myklebust, Trond, linux-nfs, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 3044 bytes --]

On Tue, 25 Sep 2012 15:27:31 +1000
NeilBrown <neilb@suse.de> wrote:

> On Wed, 29 Aug 2012 15:16:41 -0700 Jeff Layton <jlayton@redhat.com> 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 <neilb@suse.de>
> 
> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> index 627f108..268af03 100644
> --- a/fs/nfs/dir.c
> +++ b/fs/nfs/dir.c
> @@ -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 <jlayton@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* WARNING: at fs/inode.c:280 drop_nlink+0x31/0x33()
@ 2012-08-29 16:16 Nick Pasich
  0 siblings, 0 replies; 12+ messages in thread
From: Nick Pasich @ 2012-08-29 16:16 UTC (permalink / raw)
  To: linux-kernel, linux-nfs, Andrea Arcangeli


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 ]---



^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2012-09-25 10:22 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-29 16:25 WARNING: at fs/inode.c:280 drop_nlink+0x31/0x33() Nick Pasich
2012-08-29 22:16 ` Jeff Layton
2012-08-30  0:20   ` Nick Pasich
2012-08-31  4:33   ` Nick Pasich
2012-08-31  8:00     ` Pavel Shilovsky
2012-08-31 15:32       ` Nick Pasich
2012-08-31 16:21         ` Jeff Layton
2012-08-31 18:03           ` Nick Pasich
2012-09-06 11:58             ` Jeff Layton
2012-09-25  5:27   ` NeilBrown
2012-09-25 10:21     ` Jeff Layton
  -- strict thread matches above, loose matches on Subject: below --
2012-08-29 16:16 Nick Pasich

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).