Bug 26042

Comment 1 John Baldwin 2001-03-24 05:16:38 UTC

On 24-Mar-01 leclercn@videotron.ca wrote:
> 
>>Number:         26042
>>Category:       alpha
>>Synopsis:       dev_t size mismatch for DEC Alpha CPUs - kernel / userland
>>Confidential:   no
>>Severity:       serious
>>Priority:       medium
>>Responsible:    freebsd-alpha
>>State:          open
>>Quarter:        
>>Keywords:       
>>Date-Required:
>>Class:          sw-bug
>>Submitter-Id:   current-users
>>Arrival-Date:   Fri Mar 23 20:50:00 PST 2001
>>Closed-Date:
>>Last-Modified:
>>Originator:     Normand Leclerc
>>Release:        4.2 stable cvsed 23rd March
>>Organization:
>>Environment:
> FreeBSD atom.quanta.ca 4.3-RC FreeBSD 4.3-RC #3: Fri Mar 23 22:41:15 EST 2001
> root@atom.quanta.ca:/usr/src/sys/compile/ATOM  alpha
>>Description:
>   I Had problems with vinum.  While debugging, I found that the disk
> structure of vinum had a size mismatch between kernel module and userland.  I
> investigated and found that dev_t is a 64bits in kernel but 32 bits in
> userland.  (/usr/src/sys/sys/types.h).
> 
>   Once dev_t is converted to 64 bits for userland, vinum works perfectly. 
> This will affect more than just vinum.
>>How-To-Repeat:
> On an Alpha machine, vinum can't print configuration when one exists.  When
> vinum attempts to create a configuration, it will fail.
>>Fix:
> In /usr/src/sys/sys/types.h, I changed the line
> 
>       typedef u_int32_t dev_t;        /* device number */
> 
> to
> 
>#ifdef __alpha__
>       typedef u_int64_t       dev_t;  /* device number */
>#else
>       typedef u_int32_t       dev_t;  /* device number */
>#endif

Like Drew mentioned, it should be uintptr_t, but I'm curious how dev_t's could
have ever worked on the alpha?  Unless this is the user dev_t and thus shouldn't
need changing?  Poul?  Is vinum not using udev2dev(), etc. properly or
something?  Hmm, that seems somewhat likely actually.  Vinum should be using
what is a udev_t in the kernel whenever it exports data to userland.

dev_t in the kernel is a pointer:

#ifdef _KERNEL
typedef u_int32_t       udev_t;         /* device number */
typedef struct specinfo *dev_t;

#else /* !_KERNEL */

typedef u_int32_t       dev_t;          /* device number */
#define udev_t dev_t

#endif

However, dev_t in userland == udev_t in the kernel, and they are both the same
type, so if vinum is properly handling kernel -> userland transitions this
shouldn't be a problem.  This sounds more like a bug in vinum.  I'm sure Poul
will correct me if I'm wrong. :)  (And it's probably a bug in the kernel side
of vinum rather than the user side.)

-- 

John Baldwin <jhb@FreeBSD.org> -- http://www.FreeBSD.org/~jhb/
PGP Key: http://www.baldwin.cx/~john/pgpkey.asc
"Power Users Use the Power to Serve!"  -  http://www.FreeBSD.org/

Comment 2 David E. O'Brien 2001-03-24 09:16:00 UTC

On Fri, Mar 23, 2001 at 08:49:26PM -0800, leclercn@videotron.ca wrote:
> #ifdef __alpha__
> 	typedef	u_int64_t	dev_t;	/* device number */
> #else
> 	typedef	u_int32_t 	dev_t;	/* device number */
> #endif

This change is wrong.  Note in sys/sys/types.h that the kernel and
userland definitions are in sync -- compair udev_t in kernel land
and dev_t in userland.

However, we have dev_t defined in places it shouldn't be, such as
svr4_types.h and sys/coda/coda.h.  Both of these define it as u_long
which will give a size mismatch on the Alpha.

Comment 3 Poul-Henning Kamp 2001-03-28 18:36:05 UTC

State Changed
From-To: open->closed

The proposed change is wrong in every significant aspect. 

The bug is vinum bogusly exporting dev_t to the kernel. 

For a potential way to fix vinum, see <sys/stat.h>.