netdev - Re: [PATCH 00/14] AF_RXRPC socket family and AFS rewrite [net-2.6]

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 26 Apr 2007 20:41:12 -0700 (PDT)
From:	David Miller <davem@...emloft.net>
To:	dhowells@...hat.com
Cc:	netdev@...r.kernel.org
Subject: Re: [PATCH 00/14] AF_RXRPC socket family and AFS rewrite [net-2.6]

From: David Miller <davem@...emloft.net>
Date: Thu, 26 Apr 2007 20:08:09 -0700 (PDT)

> I just found a problem in your work, you cannot use cmpxchg() in
> generic code, it is not available on all processors:

Here is how I'm fixing this for now to get the tree into
a buildable state.

If you have a better fix, please send it to me relative
to what's in net-2.6, thanks.

commit 39bf09493042200b967cdf2ee6e3f670b7963903
Author: David S. Miller <davem@...set.davemloft.net>
Date:   Thu Apr 26 20:39:14 2007 -0700

    [AFS]: Eliminate cmpxchg() usage in vlocation code.
    
    cmpxchg() is not available on every processor so can't
    be used in generic code.
    
    Replace with spinlock protection on the ->state changes,
    wakeups, and wait loops.
    
    Add what appears to be a missing wakeup on transition
    to AFS_VL_VALID state in afs_vlocation_updater().
    
    Signed-off-by: David S. Miller <davem@...emloft.net>

diff --git a/fs/afs/internal.h b/fs/afs/internal.h
index 73bfa0b..6dd3197 100644
--- a/fs/afs/internal.h
+++ b/fs/afs/internal.h
@@ -219,7 +219,7 @@ struct afs_vlocation {
 	struct afs_volume	*vols[3];	/* volume access record pointer (index by type) */
 	wait_queue_head_t	waitq;		/* status change waitqueue */
 	time_t			update_at;	/* time at which record should be updated */
-	rwlock_t		lock;		/* access lock */
+	spinlock_t		lock;		/* access lock */
 	afs_vlocation_state_t	state;		/* volume location state */
 	unsigned short		upd_rej_cnt;	/* ENOMEDIUM count during update */
 	unsigned short		upd_busy_cnt;	/* EBUSY count during update */
diff --git a/fs/afs/vlocation.c b/fs/afs/vlocation.c
index 7d9815e..74cce17 100644
--- a/fs/afs/vlocation.c
+++ b/fs/afs/vlocation.c
@@ -178,7 +178,7 @@ static struct afs_vlocation *afs_vlocation_alloc(struct afs_cell *cell,
 		INIT_LIST_HEAD(&vl->grave);
 		INIT_LIST_HEAD(&vl->update);
 		init_waitqueue_head(&vl->waitq);
-		rwlock_init(&vl->lock);
+		spin_lock_init(&vl->lock);
 		memcpy(vl->vldb.name, name, namesz);
 	}
 
@@ -414,8 +414,10 @@ fill_in_record:
 	ret = afs_vlocation_fill_in_record(vl, key);
 	if (ret < 0)
 		goto error_abandon;
+	spin_lock(&vl->lock);
 	vl->state = AFS_VL_VALID;
 	wake_up(&vl->waitq);
+	spin_unlock(&vl->lock);
 
 	/* schedule for regular updates */
 	afs_vlocation_queue_for_updates(vl);
@@ -434,22 +436,23 @@ found_in_memory:
 	up_write(&cell->vl_sem);
 
 	/* see if it was an abandoned record that we might try filling in */
+	spin_lock(&vl->lock);
 	while (vl->state != AFS_VL_VALID) {
 		afs_vlocation_state_t state = vl->state;
 
 		_debug("invalid [state %d]", state);
 
 		if ((state == AFS_VL_NEW || state == AFS_VL_NO_VOLUME)) {
-			if (cmpxchg(&vl->state, state, AFS_VL_CREATING) ==
-			    state)
-				goto fill_in_record;
-			continue;
+			vl->state = AFS_VL_CREATING;
+			spin_unlock(&vl->lock);
+			goto fill_in_record;
 		}
 
 		/* must now wait for creation or update by someone else to
 		 * complete */
 		_debug("wait");
 
+		spin_unlock(&vl->lock);
 		ret = wait_event_interruptible(
 			vl->waitq,
 			vl->state == AFS_VL_NEW ||
@@ -457,15 +460,19 @@ found_in_memory:
 			vl->state == AFS_VL_NO_VOLUME);
 		if (ret < 0)
 			goto error;
+		spin_lock(&vl->lock);
 	}
+	spin_unlock(&vl->lock);
 
 success:
 	_leave(" = %p",vl);
 	return vl;
 
 error_abandon:
+	spin_lock(&vl->lock);
 	vl->state = AFS_VL_NEW;
 	wake_up(&vl->waitq);
+	spin_unlock(&vl->lock);
 error:
 	ASSERT(vl != NULL);
 	afs_put_vlocation(vl);
@@ -663,10 +670,12 @@ static void afs_vlocation_updater(struct work_struct *work)
 	vl->upd_busy_cnt = 0;
 
 	ret = afs_vlocation_update_record(vl, NULL, &vldb);
+	spin_lock(&vl->lock);
 	switch (ret) {
 	case 0:
 		afs_vlocation_apply_update(vl, &vldb);
 		vl->state = AFS_VL_VALID;
+		wake_up(&vl->waitq);
 		break;
 	case -ENOMEDIUM:
 		vl->state = AFS_VL_VOLUME_DELETED;
@@ -675,6 +684,7 @@ static void afs_vlocation_updater(struct work_struct *work)
 		vl->state = AFS_VL_UNCERTAIN;
 		break;
 	}
+	spin_unlock(&vl->lock);
 
 	/* and then reschedule */
 	_debug("reschedule");
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html