[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20080429155917.GD27202@devserv.devel.redhat.com>
Date: Tue, 29 Apr 2008 11:59:17 -0400
From: Alan Cox <alan@...hat.com>
To: Olivier Galibert <galibert@...ox.com>,
linux-kernel@...r.kernel.org, alan@...hat.com
Subject: Re: [PATCH] isicom: Fix buffer allocation
On Tue, Apr 29, 2008 at 05:26:07PM +0200, Olivier Galibert wrote:
> > @@ -813,15 +813,13 @@
> > return 0;
> > if (!port->xmit_buf) {
> > /* Relies on BKL */
> > + unsigned long page = get_zeroed_page(GFP_KERNEL);
> > + if (page == 0)
> > return -ENOMEM;
> > + if (port->xmit_buf)
> > + free_page(page);
> > + else
> > + port->xmit_buf = (unsigned char *) page;
> > }
>
>
> Still looks rather strange. An if(x) inside an if(!x) ?
The joys of parallelism. The serial drivers mostly do this because they
are using the BKL for open/close paths being a bit prehistoric (its on
the hit list ;))
CPU #1
open
port->xmit_buf == NULL
get_zeroed_page [Can sleep dropping BKL]
CPU #2
get_zeroed_page
port->xmit_buf == NULL
port->xmit_buf = page
CPU #1
port->xmit_buf != NULL
free page
A fine example of why proper locking is good ;)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists