Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 2 Aug 2017 12:35:47 +0200
From:      Hans Petter Selasky <hps@selasky.org>
To:        Konstantin Belousov <kib@FreeBSD.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r321920 - head/sys/sys
Message-ID:  <7b1d274f-a531-f788-742a-a8375fe76763@selasky.org>
In-Reply-To: <201708021014.v72AEHEk061037@repo.freebsd.org>
References:  <201708021014.v72AEHEk061037@repo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On 08/02/17 12:14, Konstantin Belousov wrote:
> Author: kib
> Date: Wed Aug  2 10:14:17 2017
> New Revision: 321920
> URL: https://svnweb.freebsd.org/changeset/base/321920
> 
> Log:
>    Change major()/minor() to work with 64bit dev_t.
>    
>    Since traditional types for the macros values are int, remove the
>    cookie trick and just split the dev_t at the word boundary.
>    
>    Reported by:	Victor Stinner <victor.stinner@gmail.com>
>    PR:	221048
>    Sponsored by:	The FreeBSD Foundation
> 
> Modified:
>    head/sys/sys/types.h
> 
> Modified: head/sys/sys/types.h
> ==============================================================================
> --- head/sys/sys/types.h	Wed Aug  2 10:12:10 2017	(r321919)
> +++ head/sys/sys/types.h	Wed Aug  2 10:14:17 2017	(r321920)
> @@ -364,14 +364,9 @@ __bitcount64(__uint64_t _x)
>   
>   #include <sys/select.h>
>   
> -/*
> - * minor() gives a cookie instead of an index since we don't want to
> - * change the meanings of bits 0-15 or waste time and space shifting
> - * bits 16-31 for devices that don't use them.
> - */
> -#define	major(x)	((int)(((u_int)(x) >> 8)&0xff))	/* major number */
> -#define	minor(x)	((int)((x)&0xffff00ff))		/* minor number */
> -#define	makedev(x,y)	((dev_t)(((x) << 8) | (y)))	/* create dev_t */
> +#define	major(x)	((int)((dev_t)(x) >> 32))	/* major number */
> +#define	minor(x)	((int)((x) & 0xffffffff))	/* minor number */
> +#define	makedev(x, y)	(((dev_t)(x) << 32) | (y))	/* create dev_t */
>   
>   /*
>    * These declarations belong elsewhere, but are repeated here and in
> 

Hi,

This change breaks the LinuxKPI:

> struct linux_cdev *
> linux_find_cdev(const char *name, unsigned major, unsigned minor)
> {
>         int unit = MKDEV(major, minor);
>         struct cdev *cdev;
>   
>         dev_lock();
>         LIST_FOREACH(cdev, &linuxcdevsw.d_devs, si_list) {
>                 struct linux_cdev *ldev = cdev->si_drv1;
>                 if (dev2unit(cdev) == unit &&
>                     strcmp(kobject_name(&ldev->kobj), name) == 0) {
>                         break;
>                 }
>         }
>         dev_unlock();
>  
>         return (cdev != NULL ? cdev->si_drv1 : NULL);
> }

You will also need to change cdev->si_drv0 to dev_t  and fix the 
make_dev() functions too.

> struct cdev {
>         void            *si_spare0;
>         u_int           si_flags;
> #define SI_ETERNAL      0x0001  /* never destroyed */
> #define SI_ALIAS        0x0002  /* carrier of alias name */
> #define SI_NAMED        0x0004  /* make_dev{_alias} has been called */
> #define SI_CHEAPCLONE   0x0008  /* can be removed_dev'ed when vnode reclaims */
> #define SI_CHILD        0x0010  /* child of another struct cdev **/
> #define SI_DUMPDEV      0x0080  /* is kernel dumpdev */
> #define SI_CLONELIST    0x0200  /* on a clone list */
> #define SI_UNMAPPED     0x0400  /* can handle unmapped I/O */
> #define SI_NOSPLIT      0x0800  /* I/O should not be split up */
>         struct timespec si_atime;
>         struct timespec si_ctime;
>         struct timespec si_mtime;
>         uid_t           si_uid;
>         gid_t           si_gid;
>         mode_t          si_mode;
>         struct ucred    *si_cred;       /* cached clone-time credential */
>         int             si_drv0;


Further this update assigns a 64-bit value to tfsid.val[0], which is 
32-bit ??

 > sys/sys/mount.h:typedef struct fsid { int32_t val[2]; } fsid_t;	/* 
filesystem id type */

sys/kern/vfs_subr.c:            tfsid.val[0] = makedev(255,

And there are more places like this!

Did you properly check all uses of makedev() in the kernel before making 
this change?

--HPS



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?7b1d274f-a531-f788-742a-a8375fe76763>