Date: Fri, 31 Aug 2001 13:45:48 -0700 From: Joe Kelsey <joe@zircon.seattle.wa.us> To: Maxim Sobolev <sobomax@FreeBSD.org>, current@freebsd.org Subject: Re: Junior Kernel Hacker task: Get rid of NCCD constant. Message-ID: <15247.63356.238011.410567@zircon.zircon.seattle.wa.us> In-Reply-To: <15247.50296.431624.991103@zircon.zircon.seattle.wa.us> References: <68364.998941145@critter> <3B8F8194.1CC1DAE3@FreeBSD.org> <15247.50296.431624.991103@zircon.zircon.seattle.wa.us>
next in thread | previous in thread | raw e-mail | index | archive | help
Joe Kelsey writes:
> Maxim Sobolev writes:
> > Poul-Henning Kamp wrote:
> >
> > > Assignment:
> > >
> > > There is no reason for the NCCD constant to exist anymore.
> > >
> > > The CCD driver already has cloning support but CCDs "softc"
> > > structure is statically allocated for NCCD devices.
> > >
> > > Change the CCD driver to dynamically allocate memory as needed,
> > > the MD driver can be used as example as the overall morphology
> > > of the two drivers are the same.
> >
> > See attached patch. Due to the fact that statically allocated array
> > was replaced with queue(9) list, it became big PITA to use kvm in the
> > ccdconfig(8) utility, so in addition I've replaced kvm with ioctl
> > interface, which simplifies things and allows to lift sugid bit from
> > it (I bet rwatson and other trusted folks would like that ;) Also
> > I've converted function prototypes into single style (deK&Rified
> > them), because most of them were affected by the my changes anyway.
> >
> > I would like to hear comments and suggestions.
>
> I only have one real suggestion. You added sc_vpp to ccd_s in order to
> store the vnodes to pass between the ioctl and the init procedures.
> This is a duplication of resources, since the init procedure merely
> copies the vnode into the appropriate info structure, and the vpp array
> is never used again. A better solution might be to simply pass the
> local vpp array as an extra argument to ccdinit (after cpaths, for
> instance). Then, after ccdinit is done, simply free the ioctl copy of
> vpp and you are done. It just seems like a lot of overhead carrying
> around that malloc'd sc_vpp for no purpose. Unless, of course, you can
> tell me the purpose for it!
After further review, one other thing I would do is store the cdd_s
pointer in the device structure using device_{get,put}_softc. Then,
there is no messing with ccdfind() in the low-level routines just to get
the softc pointer, just use the buffer pointer in strategy to find the
device, or whatever. In ccdiodone, you have to either use obp to get
the original buffer pointer and get the device there, or else change the
ccdbuf structure so that it saves the softc pointer instead of the unit
number, since we can get the unit number directly out of the softc
pointer.
Admittedly, the list of ccd_s is likely to be small, so traversing the
list in ccdfind is also likely to be fast, but storing the softc pointer
in the device structure is generally what most drivers do anyway, so
this would make it look more like most drivers.
/Joe
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?15247.63356.238011.410567>