public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Mitchell Blank Jr <mitch@sfgoth.com>
To: Amit Choudhary <amit2030@gmail.com>
Cc: Linux Kernel <linux-kernel@vger.kernel.org>, chas@cmf.nrl.navy.mil
Subject: Re: [PATCH 2.6.20-rc2] [BUGFIX] drivers/atm/firestream.c: Fix infinite recursion when alignment passed is 0.
Date: Sat, 30 Dec 2006 21:59:07 -0800	[thread overview]
Message-ID: <20061231055906.GE35756@gaz.sfgoth.com> (raw)
In-Reply-To: <20061230182603.d3815dcb.amit2030@gmail.com>

First, if you want to get patches merged you should send them to the subsystem
maintained (in this case Chas, who I've cc:'ed)  Also if you feel it needs to
be sent to mailing list you should usually use a more specific list first
(like the ATM list or maybe netdev)  Please see Documentation/SubmittingPatches

Amit Choudhary wrote:
> Description: Fix infinite recursion when alignment passed is 0 in function aligned_kmalloc(),

I'm curious how you hit this -- the only caller to aligned_kmalloc() passes
a constant "0x10"

Looking at aligned_kmalloc() it seems to be pretty badly broken -- its fallback
if it gets a non-aligned buffer is to just try a larger size which doesn't
necessarily fix the problem.  It looks like explicitly aligning the buffer
is a better solution.

Could you test this patch?  If it works feel free to forward it on to Chas.

-Mitch

[PATCH] [ATM] remove firestream.c's aligned_kmalloc()

Signed-off-by: Mitchell Blank Jr <mitch@sfgoth.com>

diff --git a/drivers/atm/firestream.c b/drivers/atm/firestream.c
index 9c67df5..df8b0c0 100644
--- a/drivers/atm/firestream.c
+++ b/drivers/atm/firestream.c
@@ -1379,38 +1379,22 @@ static void reset_chip (struct fs_dev *d
 	}
 }
 
-static void __devinit *aligned_kmalloc (int size, gfp_t flags, int alignment)
-{
-	void  *t;
-
-	if (alignment <= 0x10) {
-		t = kmalloc (size, flags);
-		if ((unsigned long)t & (alignment-1)) {
-			printk ("Kmalloc doesn't align things correctly! %p\n", t);
-			kfree (t);
-			return aligned_kmalloc (size, flags, alignment * 4);
-		}
-		return t;
-	}
-	printk (KERN_ERR "Request for > 0x10 alignment not yet implemented (hard!)\n");
-	return NULL;
-}
-
 static int __devinit init_q (struct fs_dev *dev, 
 			  struct queue *txq, int queue, int nentries, int is_rq)
 {
-	int sz = nentries * sizeof (struct FS_QENTRY);
+	unsigned sz = nentries * sizeof (struct FS_QENTRY);
 	struct FS_QENTRY *p;
+	void *vp;
 
 	func_enter ();
 
 	fs_dprintk (FS_DEBUG_INIT, "Inititing queue at %x: %d entries:\n", 
 		    queue, nentries);
-
-	p = aligned_kmalloc (sz, GFP_KERNEL, 0x10);
+	vp = kmalloc(sz + 0xF, GFP_KERNEL);
+	p = (struct FS_QENTRY *) ALIGN((unsigned long) vp, 0x10);
 	fs_dprintk (FS_DEBUG_ALLOC, "Alloc queue: %p(%d)\n", p, sz);
 
-	if (!p) return 0;
+	if (!vp) return 0;
 
 	write_fs (dev, Q_SA(queue), virt_to_bus(p));
 	write_fs (dev, Q_EA(queue), virt_to_bus(p+nentries-1));
@@ -1423,8 +1407,7 @@ static int __devinit init_q (struct fs_d
 		write_fs (dev, Q_CNF(queue), 0 ); 
 	}
 
-	txq->sa = p;
-	txq->ea = p;
+	txq->buf = vp;
 	txq->offset = queue; 
 
 	func_exit ();
@@ -1529,8 +1512,8 @@ static void __devexit free_queue (struct
 	write_fs (dev, Q_WP(txq->offset), 0);
 	/* Configuration ? */
 
-	fs_dprintk (FS_DEBUG_ALLOC, "Free queue: %p\n", txq->sa);
-	kfree (txq->sa);
+	fs_dprintk (FS_DEBUG_ALLOC, "Free queue: %p\n", txq->buf);
+	kfree (txq->buf);
 
 	func_exit ();
 }
diff --git a/drivers/atm/firestream.h b/drivers/atm/firestream.h
index 49e783e..5408766 100644
--- a/drivers/atm/firestream.h
+++ b/drivers/atm/firestream.h
@@ -455,7 +455,7 @@ struct fs_vcc {
 
 
 struct queue {
-	struct FS_QENTRY *sa, *ea;  
+	void *buf;  
 	int offset;
 };
 


  reply	other threads:[~2006-12-31  5:53 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-12-31  2:26 [PATCH 2.6.20-rc2] [BUGFIX] drivers/atm/firestream.c: Fix infinite recursion when alignment passed is 0 Amit Choudhary
2006-12-31  5:59 ` Mitchell Blank Jr [this message]
2007-01-04  7:29   ` Pekka Enberg
2007-01-04  2:47 ` David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20061231055906.GE35756@gaz.sfgoth.com \
    --to=mitch@sfgoth.com \
    --cc=amit2030@gmail.com \
    --cc=chas@cmf.nrl.navy.mil \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox