Date: Thu, 23 Mar 1995 17:35:35 +1000 From: Bruce Evans <bde@zeta.org.au> To: PVinci@ix.netcom.com, hackers@FreeBSD.org Subject: Re: suggestions/questions for WD.C Message-ID: <199503230735.RAA05683@godzilla.zeta.org.au>
next in thread | raw e-mail | index | archive | help
>in wd.c,
> static int wdcommand(struct disk *du, u_int clyinder, u_int
> head, u_int sector, u_int count, u_int command);
>Can't this be reduced to:
> static int wdcommand(struct disk *du, u_int command);
>because blknum is derived from du and cyl,head,sector are translated
>before each call. Wouldn't this be cleaner?
Yes. John fixed the multiple translation a couple of days ago, so the
only wastage no is passing all those parameters around, but it's still
wastage. The translation used to be cleaner if done in wdstart() (in
the source code if not in space/time) because the C/H/S values were
required for comparision with the raw BAD144 data. Several things would
become less clean: the wdcommand() in wdsetctlr() would have to combine
the parameters; so would new (misplaced) wdcommand()s in wdgetctlr() to
set multi-mode and features; and the `#if 0' code in wddump() to print
the C/H/S values would become mouldier.
>in wdcommand:
>if LBAmode {
> LBA Translation }
>else
> {
> CHS Translation }
>This seems to be much cleaner to me...
It's would be messier if you handled all the special commands. E.g.,
wdsetctlr() would have to check `LBAmode' to decide how to combine
the paramters. wdcommand() should probably be split up. It's like
it is because I got annoyed at sloppy and inconsistent timeout and
error handling for different commands.
>(I'd be glad to submit the changes, if someone would tell me how to
>submit them via e-mail or ftp. I assume I DL WD.C in -current --make
>changes and UL modified WD.C to WHO??)
Small changes can be mailed the current or hackers mailing list. Lots
of people are hacking on wd.c now so don't expect any particular
changes to be used.
>also, repeatedly, int wdc is locally defined over and over again as
>du->dk_port. Isn't it nore efficient to replace this with a
>#define WDC du->dk_port?? In my scanning the code WDC is really a
>constant, so isn't it better to resolve it at compile time?
It isn't constant. `wdc = du->dk_port' is better than du->dk_port
iff the compiler can keep wdc in a register. `#define WDC(du)
((du)->dk_port)' is worse for efficiency because it hides the
indirection -- unless the compiler can tell that du->dk_port can be
kept in a register...hmm, gcc seems to do the right thing for `const'
struct members. The FreeBSD sources currently aren't const-poisoned
enough for gcc to optimize well :-).
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199503230735.RAA05683>