diff mbox

cifs: drop spinlock before calling cifs_put_tlink

Message ID 1310393794-12407-1-git-send-email-jlayton@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jeff Layton July 11, 2011, 2:16 p.m. UTC
...as that function can sleep.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/cifs/connect.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Comments

Jeff Layton July 11, 2011, 2:21 p.m. UTC | #1
On Mon, 11 Jul 2011 10:16:34 -0400
Jeff Layton <jlayton@redhat.com> wrote:

> ...as that function can sleep.
> 
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
>  fs/cifs/connect.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index ceab134..dd695c5 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -2241,8 +2241,8 @@ cifs_match_super(struct super_block *sb, void *data)
>  
>  	rc = compare_mount_options(sb, mnt_data);
>  out:
> -	cifs_put_tlink(tlink);
>  	spin_unlock(&cifs_tcp_ses_lock);
> +	cifs_put_tlink(tlink);
>  	return rc;
>  }
>  

Steve, this should probably be pushed to 3.0 if possible. This can
deadlock if you end up putting the last tcon reference in this call.
Steve French July 11, 2011, 4:40 p.m. UTC | #2
ok - merged.

Am testing now.  If anyone else has test data on current cifs-2.6.git
I could send pull request faster.

On Mon, Jul 11, 2011 at 9:16 AM, Jeff Layton <jlayton@redhat.com> wrote:
> ...as that function can sleep.
>
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
>  fs/cifs/connect.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index ceab134..dd695c5 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -2241,8 +2241,8 @@ cifs_match_super(struct super_block *sb, void *data)
>
>        rc = compare_mount_options(sb, mnt_data);
>  out:
> -       cifs_put_tlink(tlink);
>        spin_unlock(&cifs_tcp_ses_lock);
> +       cifs_put_tlink(tlink);
>        return rc;
>  }
>
> --
> 1.7.6
>
>
Jeff Layton July 11, 2011, 4:49 p.m. UTC | #3
On Mon, 11 Jul 2011 11:40:12 -0500
Steve French <smfrench@gmail.com> wrote:

> ok - merged.
> 
> Am testing now.  If anyone else has test data on current cifs-2.6.git
> I could send pull request faster.
> 
> On Mon, Jul 11, 2011 at 9:16 AM, Jeff Layton <jlayton@redhat.com> wrote:
> > ...as that function can sleep.
> >
> > Signed-off-by: Jeff Layton <jlayton@redhat.com>
> > ---
> >  fs/cifs/connect.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> > index ceab134..dd695c5 100644
> > --- a/fs/cifs/connect.c
> > +++ b/fs/cifs/connect.c
> > @@ -2241,8 +2241,8 @@ cifs_match_super(struct super_block *sb, void *data)
> >
> >        rc = compare_mount_options(sb, mnt_data);
> >  out:
> > -       cifs_put_tlink(tlink);
> >        spin_unlock(&cifs_tcp_ses_lock);
> > +       cifs_put_tlink(tlink);
> >        return rc;
> >  }
> >
> > --
> > 1.7.6
> >
> >
> 
> 
> 

Testing with all of the patches in my cifs-3.0 and cifs-3.1 branches worked fine.
Pavel Shilovsky July 12, 2011, 5:50 a.m. UTC | #4
2011/7/11 Jeff Layton <jlayton@redhat.com>:
> On Mon, 11 Jul 2011 10:16:34 -0400
> Jeff Layton <jlayton@redhat.com> wrote:
>
>> ...as that function can sleep.
>>
>> Signed-off-by: Jeff Layton <jlayton@redhat.com>
>> ---
>>  fs/cifs/connect.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
>> index ceab134..dd695c5 100644
>> --- a/fs/cifs/connect.c
>> +++ b/fs/cifs/connect.c
>> @@ -2241,8 +2241,8 @@ cifs_match_super(struct super_block *sb, void *data)
>>
>>       rc = compare_mount_options(sb, mnt_data);
>>  out:
>> -     cifs_put_tlink(tlink);
>>       spin_unlock(&cifs_tcp_ses_lock);
>> +     cifs_put_tlink(tlink);
>>       return rc;
>>  }
>>
>
> Steve, this should probably be pushed to 3.0 if possible. This can
> deadlock if you end up putting the last tcon reference in this call.
>
> --
> Jeff Layton <jlayton@redhat.com>
>

This looks good to me. Thanks!
diff mbox

Patch

diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index ceab134..dd695c5 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -2241,8 +2241,8 @@  cifs_match_super(struct super_block *sb, void *data)
 
 	rc = compare_mount_options(sb, mnt_data);
 out:
-	cifs_put_tlink(tlink);
 	spin_unlock(&cifs_tcp_ses_lock);
+	cifs_put_tlink(tlink);
 	return rc;
 }