On Thu, 24 Jan 2013, thornber@xxxxxxxxxx wrote:
> On Thu, Jan 24, 2013 at 02:35:03AM +0000, Alasdair G Kergon wrote:
> > On Thu, Dec 13, 2012 at 08:19:13PM +0000, Joe Thornber wrote:
> > > diff --git a/drivers/md/dm-thin.c b/drivers/md/dm-thin.c
> > > index 504f3d6..8e47f44 100644
> > > --- a/drivers/md/dm-thin.c
> > > +++ b/drivers/md/dm-thin.c
> > > @@ -222,10 +222,28 @@ struct thin_c {
> > >
> > > struct pool *pool;
> > > struct dm_thin_device *td;
> > > +
> > > + /*
> > > + * The cell structures are too big to put on the stack, so we have
> > > + * a couple here for use by the main mapping function.
> > > + */
> > > + spinlock_t lock;
> > > + struct dm_bio_prison_cell cell1, cell2;
> >
> > We're also trying to cut down on locking on these code paths.
> > (High i/o load, many many cores?)
> >
> > Have you hit any problems while testing due to the stack size?
> > The cells don't seem ridiculously big - could we perhaps just put them on
> > the stack for now? If we do hit stack size problems in real world
> > configurations, then we can try to compare the locking approach with an
> > approach that uses a separate (local) mempool for each cell (or a
> > mempool with double-sized elements).
>
> I haven't hit any stack size issues. But the cell structures are 60
> bytes each and putting two of them on the stack seems wasteful. I
> don't have enough knowledge to say this will be ok for all
> architectures and so took the safe option.
I think it's better to put the structures on the stack (it is not that
much, the stack has 8k, so 120 bytes is 1/68 of the stack) and remove the
spinlock.
Mikulas
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
