2004-01-28 15:42:46

by Ian Kent

[permalink] [raw]
Subject: [PATCH 4/8] autofs4-2.6 - to support autofs 4.1.x


Patch:

4-autofs4-2.6.0-test9-waitq2.patch

Adds a spin lock to serialize access to wait queue in the super block info
struct.

diff -Nur linux-2.6.0-0.test9.waitq1/fs/autofs4/waitq.c linux-2.6.0-0.test9.waitq2/fs/autofs4/waitq.c
--- linux-2.6.0-0.test9.waitq1/fs/autofs4/waitq.c 2003-11-30 08:58:47.000000000 +0800
+++ linux-2.6.0-0.test9.waitq2/fs/autofs4/waitq.c 2003-11-30 09:07:29.000000000 +0800
@@ -16,6 +16,8 @@
#include <linux/file.h>
#include "autofs_i.h"

+static spinlock_t waitq_lock __cacheline_aligned_in_smp = SPIN_LOCK_UNLOCKED;
+
/* We make this a static variable rather than a part of the superblock; it
is better if we don't reassign numbers easily even across filesystems */
static autofs_wqt_t autofs4_next_wait_queue = 1;
@@ -138,12 +140,14 @@
if ( name->len > NAME_MAX )
return -ENOENT;

+ spin_lock(&waitq_lock);
for ( wq = sbi->queues ; wq ; wq = wq->next ) {
if ( wq->hash == name->hash &&
wq->len == name->len &&
wq->name && !memcmp(wq->name,name->name,name->len) )
break;
}
+ spin_unlock(&waitq_lock);

if ( !wq ) {
/* Create a new wait queue */
@@ -164,11 +168,13 @@
wq->len = name->len;
wq->status = -EINTR; /* Status return if interrupted */
memcpy(wq->name, name->name, name->len);
+ spin_lock(&waitq_lock);
wq->next = sbi->queues;
sbi->queues = wq;
+ spin_unlock(&waitq_lock);

DPRINTK(("autofs_wait: new wait id = 0x%08lx, name = %.*s, nfy=%d\n",
- wq->wait_queue_token, wq->len, wq->name, notify));
+ (unsigned long) wq->wait_queue_token, wq->len, wq->name, notify));
/* autofs4_notify_daemon() may block */
wq->wait_ctr = 2;
if (notify != NFY_NONE) {
@@ -179,7 +185,7 @@
} else {
wq->wait_ctr++;
DPRINTK(("autofs_wait: existing wait id = 0x%08lx, name = %.*s, nfy=%d\n",
- wq->wait_queue_token, wq->len, wq->name, notify));
+ (unsigned long) wq->wait_queue_token, wq->len, wq->name, notify));
}

/* wq->name is NULL if and only if the lock is already released */
@@ -238,14 +244,19 @@
{
struct autofs_wait_queue *wq, **wql;

+ spin_lock(&waitq_lock);
for ( wql = &sbi->queues ; (wq = *wql) ; wql = &wq->next ) {
if ( wq->wait_queue_token == wait_queue_token )
break;
}
- if ( !wq )
+
+ if ( !wq ) {
+ spin_unlock(&waitq_lock);
return -EINVAL;
+ }

*wql = wq->next; /* Unlink from chain */
+ spin_unlock(&waitq_lock);
kfree(wq->name);
wq->name = NULL; /* Do not wait on this queue */



2004-01-28 17:07:05

by Mike Waychison

[permalink] [raw]
Subject: Re: [PATCH 4/8] autofs4-2.6 - to support autofs 4.1.x

[email protected] wrote:

>
>Patch:
>
>4-autofs4-2.6.0-test9-waitq2.patch
>
>Adds a spin lock to serialize access to wait queue in the super block info
>struct.
>
>
>
A while back I wrote up a patch for autofs3 that serialized waitq access
and changed the waitq counters to atomic_t. I never sent it out because
I had realized that the changes I made weren't needed as all waitq
code-paths were running under the BKL (the big ones were ->lookup and
the ioctls).

Looking briefly at autofs4, this appears to be the case as well, so this
serializing probably isn't needed as long as care is made not block
while walking the list.

--
Mike Waychison
Sun Microsystems, Inc.
1 (650) 352-5299 voice
1 (416) 202-8336 voice
mailto: [email protected]
http://www.sun.com

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
NOTICE: The opinions expressed in this email are held by me,
and may not represent the views of Sun Microsystems, Inc.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

2004-01-29 00:36:39

by Ian Kent

[permalink] [raw]
Subject: Re: [PATCH 4/8] autofs4-2.6 - to support autofs 4.1.x

On Wed, 28 Jan 2004, Mike Waychison wrote:

> [email protected] wrote:
>
> >
> >Patch:
> >
> >4-autofs4-2.6.0-test9-waitq2.patch
> >
> >Adds a spin lock to serialize access to wait queue in the super block info
> >struct.
> >
> >
> >
> A while back I wrote up a patch for autofs3 that serialized waitq access
> and changed the waitq counters to atomic_t. I never sent it out because
> I had realized that the changes I made weren't needed as all waitq
> code-paths were running under the BKL (the big ones were ->lookup and
> the ioctls).

My understanding is that this code can be reached at least via lookup,
readdir and revalidate. I thought that in 2.6 none of these held the BKL
on entry (I'll have to check). Certainly this is the case for revalidate.

Ian


2004-01-29 01:28:05

by Mike Waychison

[permalink] [raw]
Subject: Re: [PATCH 4/8] autofs4-2.6 - to support autofs 4.1.x

Ian Kent wrote:

>On Wed, 28 Jan 2004, Mike Waychison wrote:
>
>
>
>>[email protected] wrote:
>>
>>
>>
>>>Patch:
>>>
>>>4-autofs4-2.6.0-test9-waitq2.patch
>>>
>>>Adds a spin lock to serialize access to wait queue in the super block info
>>>struct.
>>>
>>>
>>>
>>>
>>>
>>A while back I wrote up a patch for autofs3 that serialized waitq access
>>and changed the waitq counters to atomic_t. I never sent it out because
>>I had realized that the changes I made weren't needed as all waitq
>>code-paths were running under the BKL (the big ones were ->lookup and
>>the ioctls).
>>
>>
>
>My understanding is that this code can be reached at least via lookup,
>readdir and revalidate. I thought that in 2.6 none of these held the BKL
>on entry (I'll have to check). Certainly this is the case for revalidate.
>
>
>
>
I just took a look at the autofs4 code and the BKL is explicitly held in
the lookup code. It is also implicitly held in the ioctl code.

My understanding is that when the dcache (and subsequently
->d_revalidate) was introduced into the Linux kernel, autofs3 was
modified to do revalidates, and would have had it's lock_kernel calls
added then. If autofs4 had forked off before the dcache addition, it
wouldn't have picked the lock_kernel in d_revalidate.

Peter/Jeremy, could you confirm this?


--
Mike Waychison
Sun Microsystems, Inc.
1 (650) 352-5299 voice
1 (416) 202-8336 voice
mailto: [email protected]
http://www.sun.com

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
NOTICE: The opinions expressed in this email are held by me,
and may not represent the views of Sun Microsystems, Inc.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~