Date: Wed, 13 Mar 2002 09:09:52 +0100 (CET) From: Søren Schmidt <sos@freebsd.dk> To: John Baldwin <jhb@FreeBSD.org> Cc: SXren Schmidt <sos@FreeBSD.org>, cvs-all@FreeBSD.org, cvs-committers@FreeBSD.org Subject: Re: cvs commit: src/sys/dev/ata ata-all.c ata-all.h ata-disk.c Message-ID: <200203130809.g2D89qu25345@freebsd.dk> In-Reply-To: <XFMail.20020311163641.jhb@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200203130809.g2D89qu25345>