Hi Yosry,
On Tue, Dec 19, 2023 at 3:05 PM Yosry Ahmed <yosryahmed@xxxxxxxxxx> wrote:
> > The compression and decompression requires the buffer and mutex. The
> > mutex is not used other than compress and decompress, right?
> > In my mind, they are fine staying in the struct. I am not sure adding
> > an level acomp_ctx provides anything. It makes access structure
> > members deeper.
> >
> > If you care about separating out the crypto acomp, how about just
> > remove acomp_ctx and make it an anonymous structure.
> > struct zswap_comp_ctx {
> > struct /* cryto acomp context */ {
> > struct crypto_acomp *acomp;
> > struct acomp_req *req;
> > struct crypto_wait wait;
> > };
> > u8 *dstmem;
> > struct mutex *mutex;
> > };
>
> I prefer naming the internal struct, but I am fine with an anonymous
> struct as well. I just think it's a semantically sound separation.
Ack.
>
> >
> > Then we remove other per_cpu_load as well.
> >
> > I also think the original struct name is fine, we don't need to change
> > the struct name.
>
> The original struct name makes it seems like the data in the struct is
> only used for the crypto acomp API, which is not the case.
The mutex and buffer are used associated with the crypto acomp API
that is why I think it is fine to stay within the struct as well.
Using the anonymous struct to separate it out is marginally better. I
think we are in agreement here.
Chris
