RE: [PATCH 3/4] zsmalloc: add details to zs_map_object boiler plate

[Dan Magenheimer]

> From: Minchan Kim [mailto:minchan@xxxxxxxxxx]
> Subject: Re: [PATCH 3/4] zsmalloc: add details to zs_map_object boiler plate
> 
> On Wed, Jul 11, 2012 at 09:15:43AM -0500, Seth Jennings wrote:
> > On 07/11/2012 02:42 AM, Minchan Kim wrote:
> > > On 07/11/2012 12:17 AM, Seth Jennings wrote:
> > >> On 07/09/2012 09:35 PM, Minchan Kim wrote:
> > >>> Maybe we need local_irq_save/restore in zs_[un]map_object path.
> > >>
> > >> I'd rather not disable interrupts since that will create
> > >> unnecessary interrupt latency for all users, even if they
> > >
> > > Agreed.
> > > Although we guide k[un]map atomic is so fast, it isn't necessary
> > > to force irq_[enable|disable]. Okay.
> > >
> > >> don't need interrupt protection.  If a particular user uses
> > >> zs_map_object() in an interrupt path, it will be up to that
> > >> user to disable interrupts to ensure safety.
> > >
> > > Nope. It shouldn't do that.
> > > Any user in interrupt context can't assume that there isn't any other user using per-cpu buffer
> > > right before interrupt happens.
> > >
> > > The concern is that if such bug happens, it's very hard to find a bug.
> > > So, how about adding this?
> > >
> > > void zs_map_object(...)
> > > {
> > > 	BUG_ON(in_interrupt());
> > > }
> >
> > I not completely following you, but I think I'm following
> > enough.  Your point is that the per-cpu buffers are shared
> > by all zsmalloc users and one user doesn't know if another
> > user is doing a zs_map_object() in an interrupt path.
> 
> And vise versa is yes.
> 
> > However, I think what you are suggesting is to disallow
> > mapping in interrupt context.  This is a problem for zcache
> > as it already does mapping in interrupt context, namely for
> > page decompression in the page fault handler.
> 
> I don't get it.
> Page fault handler isn't interrupt context.
> 
> > What do you think about making the per-cpu buffers local to
> > each zsmalloc pool? That way each user has their own per-cpu
> > buffers and don't step on each other's toes.
> 
> Maybe, It could be a solution if you really need it in interrupt context.
> But the concern is it could hurt zsmalloc's goal which is memory
> space efficiency if your system has lots of CPUs.

Sorry to be so far behind on this thread.

For frontswap and zram, the "put" calls are not in interrupt
context.  For cleancache, the put call IS in interrupt context.
So if you want to use zsmalloc for zcache+cleancache, interrupt
context is a concern.  As discussed previously in a separate
thread though, zsmalloc will take a lot of work to support the full
needs of zcache.  So, pick your poison.

The x86 architecture is far ahead of ARM on many CPU optimizations
including fast copying.  Most x86 systems are also now 64-bit,
which means 64-bit+ data buses.  These differences probably account
for the difference in copy-vs-TLBmapping performance between (64-bit)
x86 and (32-bit ARM).  However, ARM may eventually catch up, especially
when 64-bit ARM chips are more common, so
it would be nice if zsmalloc could determine at runtime which
is faster and use it.

Dan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/