On 2023/12/18 17:37, Yosry Ahmed wrote: > On Mon, Dec 18, 2023 at 12:22 AM Chengming Zhou > <zhouchengming@xxxxxxxxxxxxx> wrote: >> >> Since the introduce of reusing the dstmem in the load path, it seems >> confusing that we are now using acomp_ctx->dstmem and acomp_ctx->mutex >> now for purposes other than what the naming suggests. >> >> Yosry suggested removing these two fields from acomp_ctx, and directly >> using zswap_dstmem and zswap_mutex in both the load and store paths, >> rename them, and add proper comments above their definitions that they >> are for generic percpu buffering on the load and store paths. >> >> So this patch remove dstmem and mutex from acomp_ctx, and rename the >> zswap_dstmem to zswap_buffer, using the percpu mutex and buffer on >> the load and store paths. And refactor out __zswap_store() to only >> include the compress & store, since I found zswap_store() is too long. > > I am not sure refactoring out __zswap_store() is useful to be honest, > but I am not objecting to it, it mirrors __zswap_load() in a sense. Yes, it mirrors __zswap_load() and only includes compress and store. And it makes easy for me to only concentrate on __zswap_store/load() when renaming the percpu buffers and mutex. But if anyone has objection, I can drop it. > However, if you want to do so, please do it in a separate patch from > renaming the percpu buffers and mutex. This will make reviewing easier > (and make my Suggested-by correctly scoped). Right, will do. > > Also, any reason why raw_smp_processor_id() is used here instead of > smp_processor_id()? > Here we don't need the CPU id stable, since we only need to pick one CPU and use the mutex to serialize. And from the comments below in <include/linux/smp.h>, WARN would happen if we use smp_processor_id() here without other helpers. * The CPU id is stable when: * * - IRQs are disabled; * - preemption is disabled; * - the task is CPU affine. * When CONFIG_DEBUG_PREEMPT; we verify these assumption and WARN * when smp_processor_id() is used when the CPU id is not stable. Thanks!
