Date: Thu, 14 Jun 2018 18:09:51 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Matthew Macy <mmacy@freebsd.org> Cc: Ryan Libby <rlibby@freebsd.org>, src-committers <src-committers@freebsd.org>, svn-src-all@freebsd.org, svn-src-head@freebsd.org, bde@freebsd.org Subject: Re: svn commit: r335094 - head/sys/ofed/drivers/infiniband/core Message-ID: <20180614165804.L1156@besplex.bde.org> In-Reply-To: <CAPrugNq-dGjOhJ7PW5zniggowENW74P5ttCWH01U=AmXcj4gzA@mail.gmail.com> References: <201806132330.w5DNUsrE043573@repo.freebsd.org> <CAHgpiFwQza_CEeu687s-HwnHPT%2BfcPk80nHkbU3Kn-aUM2AjHQ@mail.gmail.com> <CAPrugNq-dGjOhJ7PW5zniggowENW74P5ttCWH01U=AmXcj4gzA@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, 13 Jun 2018, Matthew Macy wrote: > On Wed, Jun 13, 2018 at 4:47 PM, Ryan Libby <rlibby@freebsd.org> wrote: >> On Wed, Jun 13, 2018 at 4:30 PM, Matt Macy <mmacy@freebsd.org> wrote: >>> Author: mmacy >>> Date: Wed Jun 13 23:30:54 2018 >>> New Revision: 335094 >>> URL: https://svnweb.freebsd.org/changeset/base/335094 >>> >>> Log: >>> fix OFED build after r335053 Ooops. makedev(), etc., will have to go back to being plain macros. But not so plain since they will need to use unportable statement-expressions to be safe macros. >>> Modified: >>> head/sys/ofed/drivers/infiniband/core/ib_user_mad.c >>> >>> Modified: head/sys/ofed/drivers/infiniband/core/ib_user_mad.c >>> ============================================================================== >>> --- head/sys/ofed/drivers/infiniband/core/ib_user_mad.c Wed Jun 13 23:19:54 2018 (r335093) >>> +++ head/sys/ofed/drivers/infiniband/core/ib_user_mad.c Wed Jun 13 23:30:54 2018 (r335094) >>> @@ -130,7 +130,8 @@ struct ib_umad_packet { >>> >>> static struct class *umad_class; >>> >>> -static const dev_t base_dev = MKDEV(IB_UMAD_MAJOR, IB_UMAD_MINOR_BASE); >>> +#define IBMKDEV(x, y) (((dev_t)(x) << 32) | (unsigned)(y)) >>> +static const dev_t base_dev = IBMKDEV(IB_UMAD_MAJOR, IB_UMAD_MINOR_BASE); >>> >>> static DEFINE_SPINLOCK(port_lock); >>> static DECLARE_BITMAP(dev_map, IB_UMAD_MAX_PORTS); >>> >> >> The scheme for major/minor encoding is different as of r335053. Won't >> that matter? Using the host makedev() seems to be correct here. So is wrapping it in MKDEV() (except the definition of MKDEV() is broken -- it is missing parentheses around args). Since the major and minor number here are small (231, 0), there is no problem with the encoding and you can just use the old makedev() macro for a quick fix (shift by 8 instead of 32) until the new one is fixed. I seem to have broken the 64-bit linux l_dev_t in a different way than before. Linux device numbers are only 16 bits for linux32 (even for stat64) and also for everything in linux[64] except newstat. For 64-bit (amd64) newstat, they are 64 bits. They can actually support large major and minor numbers in this case. I don't know there encoding for this case, but it is unlikely to be the same as the FreeBSD one. This gives the following bugs: - st_rdev is still translated by old code that does (major << 8 | minor). This is supposed to give the Linux encoding. For major < 256 and minor < 256, it works to give the old common 16-bit encoding. Otherwises, it first combines the bits in a wrong way and then assignment to 16-bit ldev_t may lose bits while assignment to 64-bit ldev_t gives wrong bits instead of lost bits for the high 48 bits. - st_dev was translated in the same way. Now it truncates to 16 bits in the 64-bit case too, so it loses bits consistently. > Yes. > >> In sys/ofed/drivers/infiniband/core/{ib_ucm.c,ib_uverbs_main.c} the >> pattern is to #define the MKDEV(). Following that would in >> ib_user_mad.c would also resolve this. Or makedev could be >> re-macroized with the new scheme. > > That's probably the best course of action. > Will follow up. It is not best to churn makedev() just for ofed, but some applications probably need it to be a plain macro too. Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20180614165804.L1156>