Hello,
On (08/07/15 10:24), Dan Streetman wrote:
> > On (08/05/15 09:46), Dan Streetman wrote:
> > [..]
> >> -enum comp_op {
> >> - ZSWAP_COMPOP_COMPRESS,
> >> - ZSWAP_COMPOP_DECOMPRESS
> >> +struct zswap_pool {
> >> + struct zpool *zpool;
> >> + struct kref kref;
> >> + struct list_head list;
> >> + struct rcu_head rcu_head;
> >> + struct notifier_block notifier;
> >> + char tfm_name[CRYPTO_MAX_ALG_NAME];
> >
> > do you need to keep a second CRYPTO_MAX_ALG_NAME copy? shouldn't it
> > be `tfm->__crt_alg->cra_name`, which is what
> > crypto_tfm_alg_name(struct crypto_tfm *tfm)
> > does?
>
> well, we don't absolutely have to keep a copy of tfm_name. However,
> ->tfm is a __percpu variable, so each time we want to check the pool's
> tfm name, we would need to do:
> crypto_comp_name(this_cpu_ptr(pool->tfm))
>
> nothing wrong with that really, just adds a bit more code each time we
> want to check the tfm name. I'll send a patch to change it.
>
> >
> >> + struct crypto_comp * __percpu *tfm;
> >> };
> >
> > ->tfm will be access pretty often, right? did you intentionally put it
> > at the bottom offset of `struct zswap_pool'?
>
> no it wasn't intentional; does moving it up provide a benefit?
well, I just prefer to keep 'read mostly' pointers together. all
those cache lines, etc.
gcc 5.1, x86_64
struct zswap_pool {
struct zpool *zpool;
+ struct crypto_comp * __percpu *tfm;
struct kref kref;
struct list_head list;
struct rcu_head rcu_head;
struct notifier_block notifier;
char tfm_name[CRYPTO_MAX_ALG_NAME];
- struct crypto_comp * __percpu *tfm;
};
../scripts/bloat-o-meter zswap.o.old zswap.o
add/remove: 0/0 grow/shrink: 0/6 up/down: 0/-27 (-27)
function old new delta
zswap_writeback_entry 659 656 -3
zswap_frontswap_store 1445 1442 -3
zswap_frontswap_load 417 414 -3
zswap_pool_create 438 432 -6
__zswap_cpu_comp_notifier.part 152 146 -6
__zswap_cpu_comp_notifier 122 116 -6
you know it better ;-)
[..]
> > this one seems to be used only once. do you want to replace
> > that single usage (well, if it's really needed)
>
> it's actually used twice, in __zswap_pool_empty() and
> __zswap_param_set(). The next patch adds __zswap_param_set().
Aha, sorry, didn't read the next patch in advance.
-ss
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>