Re: [PATCH] zsmalloc: introduce SG-list based object read API

[Sergey Senozhatsky <senozhatsky@chromium.org>]

On (26/01/16 19:53), Yosry Ahmed wrote:
[..]
> >  struct zs_pool *zs_create_pool(const char *name);
> >  void zs_destroy_pool(struct zs_pool *pool);
> > @@ -43,6 +44,9 @@ void *zs_obj_read_begin(struct zs_pool *pool, unsigned long handle,
> >  			size_t mem_len, void *local_copy);
> >  void zs_obj_read_end(struct zs_pool *pool, unsigned long handle,
> >  		     size_t mem_len, void *handle_mem);
> > +int zs_obj_read_sg_begin(struct zs_pool *pool, unsigned long handle,
> 
> void? The return value is always 0.

I thought about returning sg_nents().  Probably can be just void after all.

> There is a lot of duplication between this and zs_obj_read_begin(). I
> wanted to create a common helper for them both that returns the zpdesc
> and offset, but we cannot do the same on the read end side as the unlock
> needs to happen after kunmap() in zs_obj_read_end().
> 
> Putting parts of this code in helpers makes it a bit obscure due to the
> locking rules :/
> 
> I wonder if we can drop zs_obj_read_*() and move the spanning logic into
> zram. Looking at zram code, seems like read_from_zspool_raw() and
> read_incompressible_page() just copy the return address, so I think they
> can trivially move to using the SG list helpers and
> memcpy_from_sglist().
> 
> The only non-trivial caller is read_compressed_page(), because it passes
> the compressed object to zcomp. So I think we only need to handle the
> linearization there, something like this (completely untested):

So I was thinking about leaving things as they currently are for this
dev cycle, because both zram and zsmalloc have enough of new code queued
up.  If you don't mind let's remove memcpy() API and convert zram during
next cycle (after the upcoming merge window).