From owner-cvs-all  Wed Mar 13  0: 9:58 2002
Delivered-To: cvs-all@freebsd.org
Received: from freebsd.dk (fw-rl0.freebsd.dk [212.242.86.114])
	by hub.freebsd.org (Postfix) with ESMTP
	id 32A1937B405; Wed, 13 Mar 2002 00:09:53 -0800 (PST)
Received: (from sos@localhost)
	by freebsd.dk (8.11.6/8.11.6) id g2D89qu25345;
	Wed, 13 Mar 2002 09:09:52 +0100 (CET)
	(envelope-from sos)
From: Søren Schmidt <sos@freebsd.dk>
Message-Id: <200203130809.g2D89qu25345@freebsd.dk>
Subject: Re: cvs commit: src/sys/dev/ata ata-all.c ata-all.h ata-disk.c
In-Reply-To: <XFMail.20020311163641.jhb@FreeBSD.org>
To: John Baldwin <jhb@FreeBSD.org>
Date: Wed, 13 Mar 2002 09:09:52 +0100 (CET)
Cc: SXren Schmidt <sos@FreeBSD.org>, cvs-all@FreeBSD.org,
	cvs-committers@FreeBSD.org
Reply-To: sos@freebsd.dk
X-Mailer: ELM [version 2.4ME+ PL94b (25)]
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Content-Type: text/plain; charset=ISO-8859-1
Sender: owner-cvs-all@FreeBSD.ORG
Precedence: bulk
List-ID: <cvs-all.FreeBSD.ORG>
List-Archive: <http://docs.freebsd.org/mail/> (Web Archive)
List-Help: <mailto:majordomo@FreeBSD.ORG?subject=help> (List Instructions)
List-Subscribe: <mailto:majordomo@FreeBSD.ORG?subject=subscribe%20cvs-all>
List-Unsubscribe: <mailto:majordomo@FreeBSD.ORG?subject=unsubscribe%20cvs-all>
X-Loop: FreeBSD.ORG

It seems John Baldwin wrote:
> On 11-Mar-2002 SXren Schmidt wrote:
> > sos         2002/03/11 13:04:32 PST
> > 
> >   Modified files:
> >     sys/dev/ata          ata-all.c ata-all.h ata-disk.c ata-raid.c 
> >                          atapi-all.c atapi-cd.c atapi-fd.c 
> >                          atapi-tape.c 
> >   Log:
> >   Add new support for locking an ATA channel and use that throughout
> >   the ATA/ATAPI driver. This solves the concurrency problem with
> >   the new GEOM code, and also cuts a good deal of the patch size
> >   in the upcoming MFC.
> 
> Hmm, could you try using a mutex or sx lock instead of using atomic operations?
> You aren't using any memory barriers, thus this won't work on SMP sparc64,
> ia64, Pentium III+, or Alpha EV6+ machines.  Also, the use of
> ATA_FORCELOCK_CH()in ata_reinit() looks very evil as you are just assuming that
> no threads on any other CPU's are messing with the channel at that point in
> time.

Hmm, this is (for now) just the interdevice locking broken out into
a macro, the code does the same as it always has done. Anyhow the
reasoning behind it is that it should be turned into "real" locking
on current, but stay as it is on -stable. Having this as macros
minimises my diff between stable & current. So I agree with your
reasoning, I'm just not gotten that far yet :)

And yes *FORCELOCK* looks evil, and it is and should be :)

-Søren

To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe cvs-all" in the body of the message