Hello,
On (06/17/16 11:31), Geliang Tang wrote:
[..]
> Since the device attributes store provides write access. This patch uses
> down_write()/up_write() instead of down_read()/up_read() in
> mem_used_max_store() and compact_store().
we use ->init_lock not to make attrs exclusive, but to prevent
concurrent reset/etc.
> Signed-off-by: Geliang Tang <geliangt...@gmail.com>
> ---
> drivers/block/zram/zram_drv.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index 7454cf1..cfed743 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -293,13 +293,13 @@ static ssize_t mem_used_max_store(struct device *dev,
> if (err || val != 0)
> return -EINVAL;
>
> - down_read(&zram->init_lock);
> + down_write(&zram->init_lock);
> if (init_done(zram)) {
> struct zram_meta *meta = zram->meta;
> atomic_long_set(&zram->stats.max_used_pages,
> zs_get_total_pages(meta->mem_pool));
> }
> - up_read(&zram->init_lock);
> + up_write(&zram->init_lock);
not critical. can work.
> return len;
> }
> @@ -372,15 +372,15 @@ static ssize_t compact_store(struct device *dev,
> struct zram *zram = dev_to_zram(dev);
> struct zram_meta *meta;
>
> - down_read(&zram->init_lock);
> + down_write(&zram->init_lock);
> if (!init_done(zram)) {
> - up_read(&zram->init_lock);
> + up_write(&zram->init_lock);
> return -EINVAL;
> }
>
> meta = zram->meta;
> zs_compact(meta->mem_pool);
> - up_read(&zram->init_lock);
> + up_write(&zram->init_lock);
pool compaction can take some time. *probably* seconds in the worst
case, when the device is under IO pressure and class' locks are heavily
contended and every class has a considerable number of objects to move.
so I think we don't want to block other attrs while we compact the pool,
compaction takes care of the concurrency internally.
-ss
