Re: [RFC PATCH 1/2] erofs-utils: support tail-packing inline compressed data

[Yue Hu <zbestahu@gmail.com>]

Hi Xiang,

On Wed, 27 Oct 2021 15:34:58 +0800
Gao Xiang <hsiangkao@linux.alibaba.com> wrote:

> Hi Yue,
> 
> On Wed, Oct 27, 2021 at 03:21:37PM +0800, Yue Hu wrote:
> 
> ...
> 
> > > > > -		if (len <= pclustersize) {
> > > > > +		if (!tail_pcluster && len <= pclustersize) {
> > > > >  			if (final) {
> > > > > -				if (len <= EROFS_BLKSIZ)
> > > > > +				if (erofs_sb_has_tailpacking()) {
> > > > > +					tail_pcluster = true;
> > > > > +					pclustersize = EROFS_BLKSIZ;    
> > > > 
> > > > Not quite sure if such condition can be trigged for many times...
> > > > 
> > > > Think about it. If the original pclustersize == 16 * EROFS_BLKSIZ, so we
> > > > could have at least 16 new pclustersize == EROFS_BLKSIZ then?
> > > > 
> > > > But only the last pclustersize == EROFS_BLKSIZ can be inlined...  
> > > 
> > > Let me think about it more.  
> > 
> > I understand we need to compress the tail pcluster(len <= pclustersize) by destsize
> > of fixed 4KB to get better inline result. rt?  
> 
> I think this is the tricky part of tail-packing inline support for
> compressed data.
> 
> As you may know, EROFS supports variable-sized blocks for each pcluster
> so you could change pclustersize accordingly for the last pclusters.
> 
> For example, originally if the size of the last one pcluster is
> 16 * EROFS_BLKSIZ (therefore it cannot be tail-packing directly), there
> are 2 policies in practice can be achieved:
>  1) compress with 2 pclusters ---
>       X pcluster size + Y (Y <= 4KiB) pcluster size (so the last one can
>    be tail-packing);

Sounds good. Let me check.

>  2) compress with 4KiB pclusters ---
>       4KiB pcluster + 4KiB pcluster + ... + Z (Z <= 4KiB) pcluster
> 
> I'm not sure which one is easier to implement, maybe 2) is easier, so we
> could implement it first.

Yeah, the patch already includes it.

Thanks.

> 
> Thanks,
> Gao Xiang
>