A number of drivers like to print out the values of variables
which have type dma_addr_t. But there's no sane safe way
of doing this, because the size of the dma_addr_t type depends
upon platform and config.
This code:
dma_addr_t a;
char *s;
printk("stuff: %lx %s", a, s);
will crash the kernel if dma_addr_t is 64-bit, because printk
will get the string's address wrong.
The patch introduces a DMA_ADDR_T_FMT macro which is the
appropriate printf conversion string for the selected
dma_addr_t type. So the above usage will become
printk("stuff: " DMA_ADDR_T_FMT " %s", a, s);
A patch which fixes all the drivers which I could find
follows.
Ralf, could you please double-check the mips implementation?
--- linux-2.4.18-pre9/include/asm-i386/types.h Fri Oct 12 15:35:54 2001
+++ linux-akpm/include/asm-i386/types.h Tue Feb 12 21:48:04 2002
@@ -47,8 +47,10 @@ typedef unsigned long long u64;
#ifdef CONFIG_HIGHMEM
typedef u64 dma_addr_t;
+#define DMA_ADDR_T_FMT "%Lx"
#else
typedef u32 dma_addr_t;
+#define DMA_ADDR_T_FMT "%lx"
#endif
typedef u64 dma64_addr_t;
--- linux-2.4.18-pre9/include/asm-alpha/types.h Fri Oct 12 15:35:54 2001
+++ linux-akpm/include/asm-alpha/types.h Tue Feb 12 21:48:07 2002
@@ -50,5 +50,7 @@ typedef unsigned long u64;
typedef u64 dma_addr_t;
typedef u64 dma64_addr_t;
+#define DMA_ADDR_T_FMT "%Lx"
+
#endif /* __KERNEL__ */
#endif /* _ALPHA_TYPES_H */
--- linux-2.4.18-pre9/include/asm-arm/types.h Sun Feb 6 17:45:26 2000
+++ linux-akpm/include/asm-arm/types.h Tue Feb 12 21:48:12 2002
@@ -44,6 +44,7 @@ typedef unsigned long long u64;
/* Dma addresses are 32-bits wide. */
typedef u32 dma_addr_t;
+#define DMA_ADDR_T_FMT "%lx"
#endif /* __KERNEL__ */
--- linux-2.4.18-pre9/include/asm-cris/types.h Thu Feb 8 16:32:44 2001
+++ linux-akpm/include/asm-cris/types.h Tue Feb 12 21:48:15 2002
@@ -44,6 +44,7 @@ typedef unsigned long long u64;
/* Dma addresses are 32-bits wide, just like our other addresses. */
typedef u32 dma_addr_t;
+#define DMA_ADDR_T_FMT "%lx"
#endif /* __KERNEL__ */
--- linux-2.4.18-pre9/include/asm-ia64/types.h Fri Apr 21 15:21:24 2000
+++ linux-akpm/include/asm-ia64/types.h Tue Feb 12 21:48:18 2002
@@ -63,6 +63,7 @@ typedef __u64 u64;
/* DMA addresses are 64-bits wide, in general. */
typedef u64 dma_addr_t;
+#define DMA_ADDR_T_FMT "%Lx"
# endif /* __KERNEL__ */
#endif /* !__ASSEMBLY__ */
--- linux-2.4.18-pre9/include/asm-m68k/types.h Mon Nov 27 18:00:49 2000
+++ linux-akpm/include/asm-m68k/types.h Tue Feb 12 21:48:21 2002
@@ -52,6 +52,7 @@ typedef unsigned long long u64;
/* DMA addresses are 32-bits wide */
typedef u32 dma_addr_t;
+#define DMA_ADDR_T_FMT "%lx"
#endif /* __KERNEL__ */
--- linux-2.4.18-pre9/include/asm-mips64/types.h Sun Sep 9 10:43:02 2001
+++ linux-akpm/include/asm-mips64/types.h Tue Feb 12 21:48:26 2002
@@ -70,6 +70,7 @@ typedef unsigned long long u64;
#define BITS_PER_LONG _MIPS_SZLONG
typedef unsigned long dma_addr_t;
+#define DMA_ADDR_T_FMT "%lx"
#endif /* __KERNEL__ */
--- linux-2.4.18-pre9/include/asm-mips/types.h Sun Jul 9 22:18:15 2000
+++ linux-akpm/include/asm-mips/types.h Tue Feb 12 21:48:30 2002
@@ -71,6 +71,7 @@ typedef unsigned long long u64;
#define BITS_PER_LONG _MIPS_SZLONG
typedef unsigned long dma_addr_t;
+#define DMA_ADDR_T_FMT "%lx"
#endif /* __KERNEL__ */
--- linux-2.4.18-pre9/include/asm-parisc/types.h Tue Dec 5 12:29:39 2000
+++ linux-akpm/include/asm-parisc/types.h Tue Feb 12 21:48:35 2002
@@ -48,6 +48,7 @@ typedef unsigned long long u64;
/* Dma addresses are 32-bits wide. */
typedef u32 dma_addr_t;
+#define DMA_ADDR_T_FMT "%lx"
#endif /* __KERNEL__ */
--- linux-2.4.18-pre9/include/asm-ppc/types.h Sun Oct 21 10:13:07 2001
+++ linux-akpm/include/asm-ppc/types.h Tue Feb 12 21:48:38 2002
@@ -46,6 +46,8 @@ typedef __vector128 vector128;
/* DMA addresses are 32-bits wide */
typedef u32 dma_addr_t;
+#define DMA_ADDR_T_FMT "%lx"
+
typedef u64 dma64_addr_t;
#endif /* __KERNEL__ */
--- linux-2.4.18-pre9/include/asm-s390/types.h Wed Apr 11 19:02:28 2001
+++ linux-akpm/include/asm-s390/types.h Tue Feb 12 21:48:42 2002
@@ -54,6 +54,7 @@ typedef unsigned long long u64;
#define BITS_PER_LONG 32
typedef u32 dma_addr_t;
+#define DMA_ADDR_T_FMT "%lx"
typedef union {
unsigned long long pair;
--- linux-2.4.18-pre9/include/asm-s390x/types.h Wed Apr 11 19:02:29 2001
+++ linux-akpm/include/asm-s390x/types.h Tue Feb 12 21:48:45 2002
@@ -56,6 +56,7 @@ typedef unsigned long u64;
#define BITS_PER_LONG 64
typedef u32 dma_addr_t;
+#define DMA_ADDR_T_FMT "%lx"
#endif /* __KERNEL__ */
#endif
--- linux-2.4.18-pre9/include/asm-sh/types.h Sun Mar 5 09:33:55 2000
+++ linux-akpm/include/asm-sh/types.h Tue Feb 12 21:48:49 2002
@@ -44,6 +44,7 @@ typedef unsigned long long u64;
/* Dma addresses are 32-bits wide. */
typedef u32 dma_addr_t;
+#define DMA_ADDR_T_FMT "%lx"
#endif /* __KERNEL__ */
--- linux-2.4.18-pre9/include/asm-sparc64/types.h Fri Oct 12 15:35:54 2001
+++ linux-akpm/include/asm-sparc64/types.h Tue Feb 12 21:48:53 2002
@@ -48,6 +48,8 @@ typedef unsigned long u64;
/* Dma addresses come in generic and 64-bit flavours. */
typedef u32 dma_addr_t;
+#define DMA_ADDR_T_FMT "%lx"
+
typedef u64 dma64_addr_t;
#endif /* __KERNEL__ */
-
> dma_addr_t type. So the above usage will become
>
> printk("stuff: " DMA_ADDR_T_FMT " %s", a, s);
Vomit. How about adding a dma_addr_t %code to the printk function ?
Alan Cox wrote:
>
> > dma_addr_t type. So the above usage will become
> >
> > printk("stuff: " DMA_ADDR_T_FMT " %s", a, s);
>
> Vomit.
See, I said I should have used [email protected] for this one.
> How about adding a dma_addr_t %code to the printk function ?
The compiler's printf arg checking will choke on that.
-
Alan Cox wrote:
>
> > dma_addr_t type. So the above usage will become
> >
> > printk("stuff: " DMA_ADDR_T_FMT " %s", a, s);
>
> Vomit. How about adding a dma_addr_t %code to the printk function ?
heh, my comment on the patch was, "about as good as you can do without
teaching printk and gcc about the type"
Can we hack gcc warnings, too?
--
Jeff Garzik | "I went through my candy like hot oatmeal
Building 1024 | through an internally-buttered weasel."
MandrakeSoft | - goats.com
From: Alan Cox <[email protected]>
Date: Wed, 13 Feb 2002 09:40:44 +0000 (GMT)
Vomit. How about adding a dma_addr_t %code to the printk function ?
And gcc will discover this via what? Osmosis perhaps? :-)
Jeff Garzik wrote:
>
> Alan Cox wrote:
> >
> > > dma_addr_t type. So the above usage will become
> > >
> > > printk("stuff: " DMA_ADDR_T_FMT " %s", a, s);
> >
> > Vomit. How about adding a dma_addr_t %code to the printk function ?
>
> heh, my comment on the patch was, "about as good as you can do without
> teaching printk and gcc about the type"
we could do:
char *format_dma_addr_t(char *buf, dma_addr_t a);
-
> From: Alan Cox <[email protected]>
> Date: Wed, 13 Feb 2002 09:40:44 +0000 (GMT)
>
> Vomit. How about adding a dma_addr_t %code to the printk function ?
>
> And gcc will discover this via what? Osmosis perhaps? :-)
Yeah, seems gcc is a bit lacking in its overclever printf handlers. It
lacks the ability to declare additional non standard % code - despite the
fact glibc itself expands on the standard
glibc 2.0 adds conversion characters C and S.
glibc 2.1 adds length modifiers hh,j,t,z and conversion
characters a,A.
glibc 2.2 adds the conversion character F with C99 seman-
tics, and the flag character I.
So how do they modify the printf format rules in gcc ?
Alan Cox wrote:
>
> So how do they modify the printf format rules in gcc ?
Good question. It'd be nice for NIPQUAD and such.
Here's an alternative fix. Less vomitous?
Sorry about sticking a prototype in types.h. It needs to be
somewhere where all dma_addr_t users will see it, and where
dma_addr_t is already in scope. Maybe there's a better place?
--- linux-2.4.18-pre9/include/linux/types.h Tue Oct 23 21:59:05 2001
+++ linux-akpm/include/linux/types.h Wed Feb 13 02:03:55 2002
@@ -127,4 +127,8 @@ struct ustat {
char f_fpack[6];
};
+#ifdef __KERNEL__
+char *form_dma_addr_t(char *buf, dma_addr_t a);
+#endif
+
#endif /* _LINUX_TYPES_H */
--- linux-2.4.18-pre9/lib/vsprintf.c Thu Oct 11 11:17:22 2001
+++ linux-akpm/lib/vsprintf.c Wed Feb 13 02:24:31 2002
@@ -709,3 +709,20 @@ int sscanf(const char * buf, const char
va_end(args);
return i;
}
+
+/*
+ * dma_addr_t's have different sizes on different platforms,
+ * so we use this helper when we want to pass one to printk.
+ */
+char *form_dma_addr_t(char *buf, dma_addr_t a)
+{
+ char *fmt; /* Funny code to prevent a printf warning */
+
+ if (sizeof(dma_addr_t) == sizeof(long))
+ fmt = "%lx";
+ else
+ fmt = "%Lx";
+
+ sprintf(buf, fmt, a);
+ return buf;
+}
--- linux-2.4.18-pre9/kernel/ksyms.c Thu Feb 7 13:04:22 2002
+++ linux-akpm/kernel/ksyms.c Wed Feb 13 02:20:50 2002
@@ -458,6 +458,7 @@ EXPORT_SYMBOL(sscanf);
EXPORT_SYMBOL(vsprintf);
EXPORT_SYMBOL(vsnprintf);
EXPORT_SYMBOL(vsscanf);
+EXPORT_SYMBOL(form_dma_addr_t);
EXPORT_SYMBOL(kdevname);
EXPORT_SYMBOL(bdevname);
EXPORT_SYMBOL(cdevname);
--- linux-2.4.18-pre9/drivers/atm/idt77252.c Fri Dec 21 11:19:13 2001
+++ linux-akpm/drivers/atm/idt77252.c Wed Feb 13 02:20:50 2002
@@ -641,6 +641,7 @@ static struct scq_info *
alloc_scq(struct idt77252_dev *card, int class)
{
struct scq_info *scq;
+ char da_buf[33];
scq = (struct scq_info *) kmalloc(sizeof(struct scq_info), GFP_KERNEL);
if (!scq)
@@ -665,8 +666,9 @@ alloc_scq(struct idt77252_dev *card, int
skb_queue_head_init(&scq->transmit);
skb_queue_head_init(&scq->pending);
- TXPRINTK("idt77252: SCQ: base 0x%p, next 0x%p, last 0x%p, paddr %08x\n",
- scq->base, scq->next, scq->last, scq->paddr);
+ TXPRINTK("idt77252: SCQ: base 0x%p, next 0x%p, last 0x%p, paddr %s\n",
+ scq->base, scq->next, scq->last,
+ form_dma_addr_t(da_buf, scq->paddr));
return scq;
}
--- linux-2.4.18-pre9/drivers/message/fusion/mptbase.c Sun Sep 30 12:26:06 2001
+++ linux-akpm/drivers/message/fusion/mptbase.c Wed Feb 13 02:20:50 2002
@@ -3186,6 +3186,7 @@ mpt_print_ioc_facts(MPT_ADAPTER *ioc, ch
{
char expVer[32];
char iocName[16];
+ char da_buf[33];
int sz;
int y;
int p;
@@ -3217,8 +3218,9 @@ mpt_print_ioc_facts(MPT_ADAPTER *ioc, ch
y += sprintf(buffer+len+y, " MaxChainDepth = 0x%02x frames\n", ioc->facts.MaxChainDepth);
y += sprintf(buffer+len+y, " MinBlockSize = 0x%02x bytes\n", 4*ioc->facts.BlockSize);
- y += sprintf(buffer+len+y, " RequestFrames @ 0x%p (Dma @ 0x%08x)\n",
- ioc->req_alloc, ioc->req_alloc_dma);
+ y += sprintf(buffer+len+y, " RequestFrames @ 0x%p (Dma @ 0x%s)\n",
+ ioc->req_alloc,
+ form_dma_addr_t(da_buf, ioc->req_alloc_dma));
/*
* Rounding UP to nearest 4-kB boundary here...
*/
@@ -3230,8 +3232,9 @@ mpt_print_ioc_facts(MPT_ADAPTER *ioc, ch
4*ioc->facts.RequestFrameSize,
ioc->facts.GlobalCredits);
- y += sprintf(buffer+len+y, " ReplyFrames @ 0x%p (Dma @ 0x%08x)\n",
- ioc->reply_alloc, ioc->reply_alloc_dma);
+ y += sprintf(buffer+len+y, " ReplyFrames @ 0x%p (Dma @ 0x%s)\n",
+ ioc->reply_alloc,
+ form_dma_addr_t(da_buf, ioc->reply_alloc_dma));
sz = (ioc->reply_sz * ioc->reply_depth) + 128;
y += sprintf(buffer+len+y, " {CurRepSz=%d} x {CurRepDepth=%d} = %d bytes ^= 0x%x\n",
ioc->reply_sz, ioc->reply_depth, ioc->reply_sz*ioc->reply_depth, sz);
--- linux-2.4.18-pre9/drivers/net/tokenring/tms380tr.c Thu Sep 13 16:04:43 2001
+++ linux-akpm/drivers/net/tokenring/tms380tr.c Wed Feb 13 02:20:50 2002
@@ -1428,6 +1428,7 @@ static int tms380tr_bringup_diags(struct
static int tms380tr_init_adapter(struct net_device *dev)
{
struct net_local *tp = (struct net_local *)dev->priv;
+ char da_buf[33];
const unsigned char SCB_Test[6] = {0x00, 0x00, 0xC1, 0xE2, 0xD4, 0x8B};
const unsigned char SSB_Test[8] = {0xFF, 0xFF, 0xD1, 0xD7,
@@ -1446,7 +1447,8 @@ static int tms380tr_init_adapter(struct
if(tms380tr_debug > 3)
{
printk(KERN_INFO "%s: buffer (real): %lx\n", dev->name, (long) &tp->scb);
- printk(KERN_INFO "%s: buffer (virt): %lx\n", dev->name, (long) ((char *)&tp->scb - (char *)tp) + tp->dmabuffer);
+ printk(KERN_INFO "%s: buffer (virt): %s\n", dev->name,
+ form_dma_addr_t(da_buf, (long) ((char *)&tp->scb - (char *)tp) + tp->dmabuffer));
printk(KERN_INFO "%s: buffer (DMA) : %lx\n", dev->name, (long) tp->dmabuffer);
printk(KERN_INFO "%s: buffer (tp) : %lx\n", dev->name, (long) tp);
}
--- linux-2.4.18-pre9/drivers/net/pcnet32.c Thu Feb 7 13:04:20 2002
+++ linux-akpm/drivers/net/pcnet32.c Wed Feb 13 02:22:55 2002
@@ -529,6 +529,7 @@ pcnet32_probe1(unsigned long ioaddr, uns
char *chipname;
struct net_device *dev;
struct pcnet32_access *a = NULL;
+ char da_buf[33];
/* reset the chip */
pcnet32_dwio_reset(ioaddr);
@@ -711,7 +712,8 @@ pcnet32_probe1(unsigned long ioaddr, uns
memset(lp, 0, sizeof(*lp));
lp->dma_addr = lp_dma_addr;
lp->pci_dev = pdev;
- printk("\n" KERN_INFO "pcnet32: pcnet32_private lp=%p lp_dma_addr=%#08x", lp, lp_dma_addr);
+ printk("\n" KERN_INFO "pcnet32: pcnet32_private lp=%p lp_dma_addr=%s",
+ lp, form_dma_addr_t(da_buf, lp_dma_addr));
spin_lock_init(&lp->lock);
--- linux-2.4.18-pre9/drivers/net/yellowfin.c Fri Dec 21 11:19:13 2001
+++ linux-akpm/drivers/net/yellowfin.c Wed Feb 13 02:20:50 2002
@@ -1273,7 +1273,10 @@ static int yellowfin_close(struct net_de
#if defined(__i386__)
if (yellowfin_debug > 2) {
- printk("\n"KERN_DEBUG" Tx ring at %8.8x:\n", yp->tx_ring_dma);
+ char da_buf[33];
+
+ printk("\n"KERN_DEBUG" Tx ring at %s:\n",
+ form_dma_addr_t(da_buf, yp->tx_ring_dma));
for (i = 0; i < TX_RING_SIZE*2; i++)
printk(" %c #%d desc. %8.8x %8.8x %8.8x %8.8x.\n",
inl(ioaddr + TxPtr) == (long)&yp->tx_ring[i] ? '>' : ' ',
@@ -1285,7 +1288,8 @@ static int yellowfin_close(struct net_de
i, yp->tx_status[i].tx_cnt, yp->tx_status[i].tx_errs,
yp->tx_status[i].total_tx_cnt, yp->tx_status[i].paused);
- printk("\n"KERN_DEBUG " Rx ring %8.8x:\n", yp->rx_ring_dma);
+ printk("\n"KERN_DEBUG " Rx ring %s:\n",
+ form_dma_addr_t(da_buf, yp->rx_ring_dma));
for (i = 0; i < RX_RING_SIZE; i++) {
printk(KERN_DEBUG " %c #%d desc. %8.8x %8.8x %8.8x\n",
inl(ioaddr + RxPtr) == (long)&yp->rx_ring[i] ? '>' : ' ',
--- linux-2.4.18-pre9/drivers/net/starfire.c Thu Feb 7 13:04:21 2002
+++ linux-akpm/drivers/net/starfire.c Wed Feb 13 02:20:50 2002
@@ -1911,15 +1911,17 @@ static int netdev_close(struct net_devic
#ifdef __i386__
if (debug > 2) {
- printk("\n"KERN_DEBUG" Tx ring at %8.8x:\n",
- np->tx_ring_dma);
+ char da_buf[33];
+
+ printk("\n"KERN_DEBUG" Tx ring at %s:\n",
+ form_dma_addr_t(da_buf, np->tx_ring_dma));
for (i = 0; i < 8 /* TX_RING_SIZE is huge! */; i++)
printk(KERN_DEBUG " #%d desc. %8.8x %8.8x -> %8.8x.\n",
i, le32_to_cpu(np->tx_ring[i].status),
le32_to_cpu(np->tx_ring[i].first_addr),
le32_to_cpu(np->tx_done_q[i].status));
- printk(KERN_DEBUG " Rx ring at %8.8x -> %p:\n",
- np->rx_ring_dma, np->rx_done_q);
+ printk(KERN_DEBUG " Rx ring at %s -> %p:\n",
+ form_dma_addr_t(da_buf, np->rx_ring_dma), np->rx_done_q);
if (np->rx_done_q)
for (i = 0; i < 8 /* RX_RING_SIZE */; i++) {
printk(KERN_DEBUG " #%d desc. %8.8x -> %8.8x\n",
-
From: Alan Cox <[email protected]>
Date: Wed, 13 Feb 2002 10:23:50 +0000 (GMT)
So how do they modify the printf format rules in gcc ?
Because they can claim that they are part of the C environment, and
for the most part they are right so their extensions go into gcc's
magic list.
In fact I'd claim their case to be plugging holes in the standards
specified set of printf format strings. :-)
Hey... we could "borrow" one of these printf format strings we
don't have any need for in the kernel and pretend that is for
"dma_addr_t". :-)
David S. Miller writes:
> From: Alan Cox <[email protected]>
> Date: Wed, 13 Feb 2002 10:23:50 +0000 (GMT)
>
> So how do they modify the printf format rules in gcc ?
>
> Because they can claim that they are part of the C environment, and
> for the most part they are right so their extensions go into gcc's
> magic list.
>
> In fact I'd claim their case to be plugging holes in the standards
> specified set of printf format strings. :-)
>
> Hey... we could "borrow" one of these printf format strings we
> don't have any need for in the kernel and pretend that is for
> "dma_addr_t". :-)
Eeeep! I know you put a smiley there, but don't even think that!
Regards,
Richard....
Permanent: [email protected]
Current: [email protected]
In article <[email protected]>,
you write:
>+#ifdef __KERNEL__
>+char *form_dma_addr_t(char *buf, dma_addr_t a);
>+#endif
>+
How about an typedef for da_buf_t (or similar) so that drivers don't
have to worry about the size of the buffer? And that would also let you
reduce the stack footprint on systems where dma_addr_t is only 32 bits,
although that's probably not going to produce a noticeable benefit.
Paul
On Wed, Feb 13, 2002 at 02:26:46AM -0800, Andrew Morton wrote:
> Alan Cox wrote:
> >
> > So how do they modify the printf format rules in gcc ?
>
> Good question. It'd be nice for NIPQUAD and such.
>
> Here's an alternative fix. Less vomitous?
>
> Sorry about sticking a prototype in types.h. It needs to be
> somewhere where all dma_addr_t users will see it, and where
> dma_addr_t is already in scope. Maybe there's a better place?
Personally, I like this less.. I don't think the previous #define
DMA_ADDR_T_FMT (or whatever it was) is that bad, considering the options.
Once we set the precedent of having little 'char
* form_some_random_type()' functions, they will show up all over the
place.
I need something like this for the MTD patch I've got that supports 64
bit buswidths on 32 bit machines and needs to printk the data on error.
I found that #define CFI_FMT '%lx' or whatever to be more
straightforward, and easier to understand.
> + */
> +char *form_dma_addr_t(char *buf, dma_addr_t a)
> +{
> + char *fmt; /* Funny code to prevent a printf warning */
> +
> + if (sizeof(dma_addr_t) == sizeof(long))
> + fmt = "%lx";
> + else
> + fmt = "%Lx";
> +
> + sprintf(buf, fmt, a);
> + return buf;
> +}
--
Troy Benjegerdes | master of mispeeling | 'da hozer' | [email protected]
-----"If this message isn't misspelled, I didn't write it" -- Me -----
"Why do musicians compose symphonies and poets write poems? They do it
because life wouldn't have any meaning for them if they didn't. That's
why I draw cartoons. It's my life." -- Charles Schulz
[email protected] said:
> I need something like this for the MTD patch I've got that supports
> 64 bit buswidths on 32 bit machines and needs to printk the data on
> error.
> I found that #define CFI_FMT '%lx' or whatever to be more
> straightforward, and easier to understand.
I got annoyed with the original implementation that turned up in CVS, with
a completely separate printk line #ifdef CFI_WORD_64, and I changed it to
always cast to (long long) before printing. Although I half suspect I'll
end up redoing it again before actually sending it to Linus.
--
dwmw2