On Tue, Oct 05, 1999 at 05:20:09PM -0700, Bill Studenmund wrote:
> On Tue, 5 Oct 1999, Chuck Silvers wrote:
> 
> > cool.  I had a start on this in the UBC branch, but I'm glad someone else
> > is doing the rest of it.  I have a few comments:
> > 
> > 1.  do we really want to pretend to support non-power-of-two devices at all
> >     in the device interface?  we kinda went thru this when this whole subject
> >     came up before, and I thought the consensus was that it wasn't worthwhile.
> 
> Ick. That wasn't the consensus I took away from the previous thread. I
> thought the idea was that if supporting non-power-of-two devices meant we
> had to divide all the time in dbtob() and btodb(), THAT would be bad.
> whenever possible we want to avoid those divides.
> 
> But if we could have the non-PO2 support without loosing the bit shifting
> ability, we'd be fine.
> 
> By using a shift value, and rolling over to multiply and divide when its
> 0, I thought we could have the best of both. :-)

well, after all the hoo-ha last time, I was convinced that it wasn't worth it,
but you seem keen on it so I'm not going to argue.  if you could combine
the two values somehow (eg. having a negative shift represent a size or
vice-versa), that would mean only one extra argument everywhere instead of
two, which is the main thing I don't like about the way you've described.


> > 2.  do we really need a (*d_bsize)() ?  I recall that there was already a
> >     device ioctl that returns this info... or maybe I was just thinking
> >     that that's how I'd do it eventually.
> 
> I don't remember seeing such an ioctl. 

yea, I was probably just wishing there were such an ioctl.


> The advantage of this routine is
> that the setup for the caller is simple - get[bc]devsize take a dev_t &
> look this routine up in the [bc]devsw tables. So checkalias gains:
> 
>                 } else if (nvp->v_type == VBLK) {
>                         getbdevsize(nvp_rdev, &nvp->v_specbshift,
>                                                 &nvp->v_specbsize);
>                 } else {
>                         getcdevsize(nvp_rdev, &nvp->v_specbshift,
>                                                 &nvp->v_specbsize);
>                 }
> 
> and the meaty part of these routines is just:
> 
> 		(*bdevsw[major].d_bsize)(dev, bshift, bsize);
> 
> If we did an ioctl, the meaty setup would be more. We could do it, but
> this is the way the info is going to be used anyway. :-)

slightly more setup goo in exactly two places, at the cost of adding
another devops interface that doesn't apply to all devices when there's
an existing interface that will work perfectly well.

actually you could just pass in the function pointer to getdevsize() and
then there would be only one copy of the setup goo.

the naming daemon in me insists I suggest [bc]dev_getsize()
(or dev_getsize() if you combine the functions).  :-)

-Chuck


> > 3.  I'm not sure what "swap blocks" are, but I would guess they should
> >     be in pagesize units, since that's how swap space is managed.
> > 
> > 4.  I'd think the goal should be to get rid of DEF_BSIZE eventually too.
> >     purely software devices like md can define their own constants.
> > 
> > 5.  perhaps the sector size info in the on-disk disklabel should be
> >     ignored and replaced with the info from the device itself?
> >     there are probably further disklabel implications to all this.
> 
> Not sure. See Chris's reply about this too.
> 
> Take care,
> 
> Bill