Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1320275 > unrolled thread

Re: [PATCH v2 04/10] crypto/compress: add asynchronous compression support

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v2 04/10] crypto/compress: add asynchronous compression  support Joonsoo Kim <iamjoonsoo.kim@lge.com> - 2016-01-28 05:00 +0100
    Re: [PATCH v2 04/10] crypto/compress: add asynchronous compression  support Herbert Xu <herbert@gondor.apana.org.au> - 2016-01-29 11:10 +0100
      Re: [PATCH v2 04/10] crypto/compress: add asynchronous compression  support Joonsoo Kim <iamjoonsoo.kim@lge.com> - 2016-02-01 03:20 +0100
        Re: [PATCH v2 04/10] crypto/compress: add asynchronous compression  support Herbert Xu <herbert@gondor.apana.org.au> - 2016-02-04 04:30 +0100
          Re: [PATCH v2 04/10] crypto/compress: add asynchronous compression  support Herbert Xu <herbert@gondor.apana.org.au> - 2016-02-04 04:40 +0100
            Re: [PATCH v2 04/10] crypto/compress: add asynchronous compression  support "Li, Weigang" <weigang.li@intel.com> - 2016-02-04 05:00 +0100
              Re: [PATCH v2 04/10] crypto/compress: add asynchronous compression  support Herbert Xu <herbert@gondor.apana.org.au> - 2016-02-04 16:00 +0100
          Re: [PATCH v2 04/10] crypto/compress: add asynchronous compression support Joonsoo Kim <js1304@gmail.com> - 2016-02-04 08:20 +0100
            Re: [PATCH v2 04/10] crypto/compress: add asynchronous compression  support Herbert Xu <herbert@gondor.apana.org.au> - 2016-02-04 16:00 +0100
              Re: [PATCH v2 04/10] crypto/compress: add asynchronous compression support Joonsoo Kim <js1304@gmail.com> - 2016-02-04 17:20 +0100
        Re: [PATCH v2 04/10] crypto/compress: add asynchronous compression  support "Li, Weigang" <weigang.li@intel.com> - 2016-02-04 04:30 +0100

#1320275 — Re: [PATCH v2 04/10] crypto/compress: add asynchronous compression support

Hello, Herbert.

On Wed, Jan 27, 2016 at 04:09:26PM +0800, Herbert Xu wrote:
> On Wed, Jan 27, 2016 at 04:03:55PM +0800, Herbert Xu wrote:
> > On Wed, Jan 27, 2016 at 03:59:05PM +0800, Li, Weigang wrote:
> > >
> > > The acomp is also SG-based, while scomp only accepts flat buffer.
> > 
> > Right, but do we need a pointer-based scomp at all? IPComp would
> > certainly be better off with an SG-based interface.  Any other
> > users of compression are presumably dealing with large amounts
> > of data where an SG interface would make more sense.
> > 
> > A pointer interface makes sense for shash because you may be hashing
> > 16 bytes at a time.  Nobody sane is going to be compressing 16 bytes,
> > or are they?

Hmm... I'm not an expert on this area so below of my analysis would be
wrong.

Some of compression example in kernel do compression with PAGE_SIZE and
compressed size is naturally less than PAGE_SIZE. There are many cases
that compressed size is below than 100 bytes. To keep and handle data,
they somtimes use kmalloced buffer and I guess it isn't suitable for
SG-based interface. Is it okay to use SG-based interface
if kmalloced object covers two pages?

And, even, someone uses vmalloced buffer that's also not suitable for
SG-based interface. For large amount data case, vmalloced buffer is
more suitable and it needs pointer interface.

> Note that I'm fine with keeping an scomp interface underneath
> for those algorithms where the best way to handle SG input is
> to linearise things.  But I would prefer that this interface is
> not exposed to kernel users unless it is absolutely required.

I have tested asynchronous compression APIs in zram and I saw
regression. Atomic allocation and setting up SG lists are culprit
for this regression. Moreover, zram optimizes linearisation
to get the best performance so it has two Kconfig options. One of
them cannot be support in the general layer. Not supporting pointer
based APIs unavoidably causes regression to zram in this case.

And, S/W compression algorithms that exists in kernel
are pointer based so it's natural to support it first
in crypto compression. That will help existing users to change
their direct library call to crypto compression without any regression.
They may be happy to change it because they just could get more
algorithm support without any loss.

I think that supporting pointer-based interface has some merits
mentioned above. However, I'm not sure what's the benefit if we only
support SG-based interface and it's bigger than above.

Thanks.

[toc] | [next] | [standalone]

#1321625

On Thu, Jan 28, 2016 at 12:19:42PM +0900, Joonsoo Kim wrote:
>
> I have tested asynchronous compression APIs in zram and I saw
> regression. Atomic allocation and setting up SG lists are culprit
> for this regression. Moreover, zram optimizes linearisation

So which is it, atomic allocations or setting up SG lists? There
is nothing in acomp that requires you to do an atomic allocation.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[toc] | [prev] | [next] | [standalone]

#1322785

On Fri, Jan 29, 2016 at 06:09:01PM +0800, Herbert Xu wrote:
> On Thu, Jan 28, 2016 at 12:19:42PM +0900, Joonsoo Kim wrote:
> >
> > I have tested asynchronous compression APIs in zram and I saw
> > regression. Atomic allocation and setting up SG lists are culprit
> > for this regression. Moreover, zram optimizes linearisation
> 
> So which is it, atomic allocations or setting up SG lists? There
> is nothing in acomp that requires you to do an atomic allocation.

Atomic allocation are called for linearisation when needed. Zram's
compressed content is usually stored in two physically separate pages
so linearisation is needed. See scomp_map().

Setting up SG lists means that to use acomp, sg_init_table(),
sg_set_page() are need to be called by zram unlike the case just
passing the pointer based buffer.

Thanks.

[toc] | [prev] | [next] | [standalone]

#1326363

On Thu, Feb 04, 2016 at 11:25:27AM +0800, Li, Weigang wrote:
>
> Please can you advise how to get the acomp patch accepted?

Can you do a posting of these patches without scomp so we can
evaluate the effects?

Thanks!
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[toc] | [prev] | [next] | [standalone]

#1326367

On Thu, Feb 04, 2016 at 11:28:50AM +0800, Herbert Xu wrote:
> On Thu, Feb 04, 2016 at 11:25:27AM +0800, Li, Weigang wrote:
> >
> > Please can you advise how to get the acomp patch accepted?
> 
> Can you do a posting of these patches without scomp so we can
> evaluate the effects?

Of course you can keep the driver-side scomp interface as otherwise
the implementation would be unnecessarily complicated.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[toc] | [prev] | [next] | [standalone]

#1326371

On 2/4/2016 11:29 AM, Herbert Xu wrote:
> On Thu, Feb 04, 2016 at 11:28:50AM +0800, Herbert Xu wrote:
>> On Thu, Feb 04, 2016 at 11:25:27AM +0800, Li, Weigang wrote:
>>>
>>> Please can you advise how to get the acomp patch accepted?
>>
>> Can you do a posting of these patches without scomp so we can
>> evaluate the effects?
>
> Of course you can keep the driver-side scomp interface as otherwise
> the implementation would be unnecessarily complicated.
>
> Cheers,
>
Seems I need go back to my first acomp patch.. Assuming we shall still 
keep the comp i/f, and the linearisation of sg-list in acomp to fit the 
"comp" API? What do you mean by the driver-side scomp? Thanks!

[toc] | [prev] | [next] | [standalone]

#1326888

On Thu, Feb 04, 2016 at 11:50:46AM +0800, Li, Weigang wrote:
>
> Seems I need go back to my first acomp patch.. Assuming we shall
> still keep the comp i/f, and the linearisation of sg-list in acomp
> to fit the "comp" API? What do you mean by the driver-side scomp?

What I mean is that the bottom half of the scomp patches can still
be used.  We just want to hide it away from the users so won't
provide the direct entry points such as crypto_alloc_scomp, etc..

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[toc] | [prev] | [next] | [standalone]

#1326452 — Re: [PATCH v2 04/10] crypto/compress: add asynchronous compression support

2016-02-04 12:28 GMT+09:00 Herbert Xu <herbert@gondor.apana.org.au>:
> On Thu, Feb 04, 2016 at 11:25:27AM +0800, Li, Weigang wrote:
>>
>> Please can you advise how to get the acomp patch accepted?
>
> Can you do a posting of these patches without scomp so we can
> evaluate the effects?
>

Do you think not to merge scomp? Please let me know your overall
plan about this.?

Thanks.

[toc] | [prev] | [next] | [standalone]

#1326895

On Thu, Feb 04, 2016 at 04:17:41PM +0900, Joonsoo Kim wrote:
>
> Do you think not to merge scomp? Please let me know your overall
> plan about this.?

I'm fine with a driver-side scomp interface.  But I'd rather
avoid having yet another user-side compression interface in the
form of scomp if we can avoid it.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[toc] | [prev] | [next] | [standalone]

#1326954 — Re: [PATCH v2 04/10] crypto/compress: add asynchronous compression support

2016-02-04 23:53 GMT+09:00 Herbert Xu <herbert@gondor.apana.org.au>:
> On Thu, Feb 04, 2016 at 04:17:41PM +0900, Joonsoo Kim wrote:
>>
>> Do you think not to merge scomp? Please let me know your overall
>> plan about this.?
>
> I'm fine with a driver-side scomp interface.  But I'd rather
> avoid having yet another user-side compression interface in the
> form of scomp if we can avoid it.

I mentioned that there are usecases that scomp is needed for performance,
it means that we can't avoid it. Or do you think this usecase differently?
I understand it's rather pain that we have two interfaces but scomp interface
is just wrapping layer to handle various S/W compression algorithms and
implementation is not that complex. I guess it doesn't cause much
maintenance cost.

Thanks.

[toc] | [prev] | [next] | [standalone]

#1326366

On 2/1/2016 10:11 AM, Joonsoo Kim wrote:
> On Fri, Jan 29, 2016 at 06:09:01PM +0800, Herbert Xu wrote:
>> On Thu, Jan 28, 2016 at 12:19:42PM +0900, Joonsoo Kim wrote:
>>>
>>> I have tested asynchronous compression APIs in zram and I saw
>>> regression. Atomic allocation and setting up SG lists are culprit
>>> for this regression. Moreover, zram optimizes linearisation
>>
>> So which is it, atomic allocations or setting up SG lists? There
>> is nothing in acomp that requires you to do an atomic allocation.
>
> Atomic allocation are called for linearisation when needed. Zram's
> compressed content is usually stored in two physically separate pages
> so linearisation is needed. See scomp_map().
>
> Setting up SG lists means that to use acomp, sg_init_table(),
> sg_set_page() are need to be called by zram unlike the case just
> passing the pointer based buffer.
>
> Thanks.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
Hello Herbert & Joonsoo,
Please can you advise how to get the acomp patch accepted?

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web