Currently, per-CPU decompression streams use a mutex, so there is a chance for Reader vs Reader priority inversion - a low priority Reader can get preempted by a higher priority Reader, which simply will get blocked on the stream mutex. Switch zcomp_dstrm to permit parallel usage. However, things are not completely straightforward. Many backends have stateless decompression and don't modify decompression context, so those can be used concurrently. But then there are backends that do modify decompression context. We also can have compressed object span across physical pages, which zcomp linearises using stream's ->local_copy buffer before decompression. That buffer, clearly, cannot be used concurrently. Give zcomp_dstrm two locks: - ->lock (rw_semaphore) pins the stream against CPU hotplug teardown. Every decompression request takes it in read mode, and only zcomp_cpu_dead() takes it in write mode. - ->excl_lock (mutex) serializes access to mutable stream state (->local_copy and ->ctx). When the backend has no per-stream decompression context (stateless) and zsmalloc object fits in a single page (zcomp_shared_decomp()), the request touches neither ->local_copy nor ->ctx, so it only takes ->lock in read mode (ZCOMP_DSTRM_SHARED). Otherwise, when ->local_copy is needed or the backend has a decompression context (zstd, deflate), additionally take ->excl_lock (ZCOMP_DSTRM_EXCLUSIVE) so exclusive requests serialize only against each other without blocking shared readers. A quick synthetic fio test for Reader vs Reader inversion on 24 CPUs (preempt=full, lz4, 48 low priority (nice 19) readers, 24 normal priority (nice 0) CPU hogs and 24 high priority (nice -19) readers): Metric Base Patched ---------------------------------------------------------- Read IOPS 857k 1,382k (+61.3%) Read clat avg 14.40 us 4.07 us (-3.5x) Read clat stdev 3,270.31 us 671.46 us (-4.9x) Read clat p99.99 1,500 us 24.64 us (-60.9x) Read clat max 2,430,200 us 1,022,758 us (-2.4x) Suggested-by: Brian Geffon Signed-off-by: Sergey Senozhatsky --- drivers/block/zram/zcomp.c | 56 ++++++++++++++++++++++++++++------- drivers/block/zram/zcomp.h | 21 +++++++++++-- drivers/block/zram/zram_drv.c | 12 +++++--- 3 files changed, 71 insertions(+), 18 deletions(-) diff --git a/drivers/block/zram/zcomp.c b/drivers/block/zram/zcomp.c index 1c9f6d5c8cd1..da122bf4dd15 100644 --- a/drivers/block/zram/zcomp.c +++ b/drivers/block/zram/zcomp.c @@ -165,7 +165,7 @@ struct zcomp_cstrm *zcomp_cstrm_get(struct zcomp *comp) * so then unlock and re-try on the current CPU. */ mutex_lock(&zstrm->lock); - if (likely(zstrm->buffer)) + if (zstrm->buffer) return zstrm; mutex_unlock(&zstrm->lock); } @@ -176,21 +176,49 @@ void zcomp_cstrm_put(struct zcomp_cstrm *zstrm) mutex_unlock(&zstrm->lock); } -struct zcomp_dstrm *zcomp_dstrm_get(struct zcomp *comp) +bool zcomp_shared_decomp(struct zcomp *comp, struct scatterlist *sg) +{ + /* + * Shared decompression is only safe when the backend has no + * mutable per-stream decompression context and the object is + * contained within a single page (no ->local_copy bounce needed). + */ + return !comp->ops->create_dctx && sg_is_last(sg); +} + +struct zcomp_dstrm *zcomp_dstrm_get(struct zcomp *comp, + enum zcomp_dstrm_mode mode) { for (;;) { struct zcomp_dstrm *zstrm = raw_cpu_ptr(comp->dstream); - mutex_lock(&zstrm->lock); - if (likely(zstrm->local_copy)) - return zstrm; - mutex_unlock(&zstrm->lock); + /* + * Pin the stream. The write side of ->lock is only taken + * by zcomp_cpu_dead(), so this never contends with other + * decompression requests, regardless of their mode. + */ + down_read(&zstrm->lock); + if (!zstrm->local_copy) { + /* The stream has been destroyed, re-try */ + up_read(&zstrm->lock); + continue; + } + + /* + * Exclusive requests additionally own ->local_copy and + * ->ctx and hence can only wait for each other. + */ + if (mode == ZCOMP_DSTRM_EXCLUSIVE) + mutex_lock(&zstrm->excl_lock); + return zstrm; } } -void zcomp_dstrm_put(struct zcomp_dstrm *zstrm) +void zcomp_dstrm_put(struct zcomp_dstrm *zstrm, enum zcomp_dstrm_mode mode) { - mutex_unlock(&zstrm->lock); + if (mode == ZCOMP_DSTRM_EXCLUSIVE) + mutex_unlock(&zstrm->excl_lock); + up_read(&zstrm->lock); } int zcomp_compress(struct zcomp *comp, struct zcomp_cstrm *zstrm, @@ -223,6 +251,9 @@ int zcomp_decompress(struct zcomp *comp, struct zcomp_dstrm *zstrm, int ret; might_sleep(); + lockdep_assert_held(&zstrm->lock); + if (!zcomp_shared_decomp(comp, sg)) + lockdep_assert_held(&zstrm->excl_lock); if (sg_is_last(sg)) { /* the object is contained within one page, read it in-place */ @@ -277,9 +308,9 @@ int zcomp_cpu_dead(unsigned int cpu, struct hlist_node *node) mutex_unlock(&cstrm->lock); dstrm = per_cpu_ptr(comp->dstream, cpu); - mutex_lock(&dstrm->lock); + down_write(&dstrm->lock); zcomp_dstrm_free(comp, dstrm); - mutex_unlock(&dstrm->lock); + up_write(&dstrm->lock); return 0; } @@ -306,8 +337,11 @@ static int zcomp_init(struct zcomp *comp, struct zcomp_params *params) } for_each_possible_cpu(cpu) { + struct zcomp_dstrm *zstrm = per_cpu_ptr(comp->dstream, cpu); + mutex_init(&per_cpu_ptr(comp->cstream, cpu)->lock); - mutex_init(&per_cpu_ptr(comp->dstream, cpu)->lock); + init_rwsem(&zstrm->lock); + mutex_init(&zstrm->excl_lock); } ret = cpuhp_state_add_instance(CPUHP_ZCOMP_PREPARE, &comp->node); diff --git a/drivers/block/zram/zcomp.h b/drivers/block/zram/zcomp.h index 22be50dc51ba..bf41b27958c4 100644 --- a/drivers/block/zram/zcomp.h +++ b/drivers/block/zram/zcomp.h @@ -4,11 +4,17 @@ #define _ZCOMP_H_ #include +#include struct scatterlist; #define ZCOMP_PARAM_NOT_SET INT_MIN +enum zcomp_dstrm_mode { + ZCOMP_DSTRM_SHARED, + ZCOMP_DSTRM_EXCLUSIVE, +}; + struct deflate_params { s32 winbits; }; @@ -47,7 +53,14 @@ struct zcomp_cstrm { }; struct zcomp_dstrm { - struct mutex lock; + /* + * Pins the stream: held in read mode by every user, regardless + * of the decompression mode, and in write mode only by + * zcomp_cpu_dead(). + */ + struct rw_semaphore lock; + /* Serializes users of the mutable stream state: ->local_copy, ->ctx */ + struct mutex excl_lock; /* local copy of handle memory */ void *local_copy; struct zcomp_ctx ctx; @@ -99,8 +112,10 @@ void zcomp_destroy(struct zcomp *comp); struct zcomp_cstrm *zcomp_cstrm_get(struct zcomp *comp); void zcomp_cstrm_put(struct zcomp_cstrm *zstrm); -struct zcomp_dstrm *zcomp_dstrm_get(struct zcomp *comp); -void zcomp_dstrm_put(struct zcomp_dstrm *zstrm); +bool zcomp_shared_decomp(struct zcomp *comp, struct scatterlist *sg); +struct zcomp_dstrm *zcomp_dstrm_get(struct zcomp *comp, + enum zcomp_dstrm_mode mode); +void zcomp_dstrm_put(struct zcomp_dstrm *zstrm, enum zcomp_dstrm_mode mode); int zcomp_compress(struct zcomp *comp, struct zcomp_cstrm *zstrm, const void *src, unsigned int *dst_len); diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c index def5266679b0..a34859ff0461 100644 --- a/drivers/block/zram/zram_drv.c +++ b/drivers/block/zram/zram_drv.c @@ -1373,7 +1373,7 @@ static int decompress_bdev_page(struct zram *zram, struct page *page, sg_init_table(sg, 1); sg_set_page(sg, page, size, 0); - zstrm = zcomp_dstrm_get(zram->comps[prio]); + zstrm = zcomp_dstrm_get(zram->comps[prio], ZCOMP_DSTRM_EXCLUSIVE); ret = zcomp_decompress(zram->comps[prio], zstrm, sg, size, zstrm->local_copy); if (!ret) { @@ -1382,7 +1382,7 @@ static int decompress_bdev_page(struct zram *zram, struct page *page, copy_page(dst, zstrm->local_copy); kunmap_local(dst); } - zcomp_dstrm_put(zstrm); + zcomp_dstrm_put(zstrm, ZCOMP_DSTRM_EXCLUSIVE); slot_unlock(zram, index); return ret; @@ -2115,6 +2115,7 @@ static int read_incompressible_page(struct zram *zram, struct page *page, static int read_compressed_page(struct zram *zram, struct page *page, unsigned long index) { + enum zcomp_dstrm_mode mode = ZCOMP_DSTRM_EXCLUSIVE; struct zcomp_dstrm *zstrm; struct scatterlist sg[2]; unsigned long handle; @@ -2126,13 +2127,16 @@ static int read_compressed_page(struct zram *zram, struct page *page, size = get_slot_size(zram, index); prio = get_slot_comp_priority(zram, index); - zstrm = zcomp_dstrm_get(zram->comps[prio]); zs_obj_read_sg_begin(zram->mem_pool, handle, sg, size); + if (zcomp_shared_decomp(zram->comps[prio], sg)) + mode = ZCOMP_DSTRM_SHARED; + + zstrm = zcomp_dstrm_get(zram->comps[prio], mode); dst = kmap_local_page(page); ret = zcomp_decompress(zram->comps[prio], zstrm, sg, size, dst); kunmap_local(dst); + zcomp_dstrm_put(zstrm, mode); zs_obj_read_sg_end(zram->mem_pool, handle); - zcomp_dstrm_put(zstrm); return ret; } -- 2.56.0.385.gd3acb90ef8-goog