linux-kernel - Re: [PATCHv4 00/10] add on-demand device creation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150506082036.GB8163@blaptop>
Date:	Wed, 6 May 2015 17:20:37 +0900
From:	Minchan Kim <minchan@...nel.org>
To:	Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
Cc:	Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Nitin Gupta <ngupta@...are.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCHv4 00/10] add on-demand device creation

On Wed, May 06, 2015 at 05:10:29PM +0900, Sergey Senozhatsky wrote:
> On (05/06/15 16:28), Minchan Kim wrote:
> > > 
> > > from your logs:
> > > ...
> > > [ 98.756017] zram: Removed device: zram2
> > > [ 98.757087] ------------[ cut here ]------------
> > > ...
> > > 
> > > locked zram_index_mutex, removed zram2, unlocked zram_index_mutex,
> > > locked zram_index_mutex, attempted to create zram2: zram2 sysfs already exist.
> > > 
> > > 
> > > hm... need to think. zram hot/remove is serialized.
> > 
> > I never look at the code but I doubt others(ex, some admin process on my machine)
> > holds a ref so kobj doesn't disapper yet but zram_add try to create it?
> > 
> 
> well, something like this is certainly happening. hm, if this is the
> case, then a very quick thing I can think of is to stop re-using previously
> used zram ids. add_disk() doesn't give us a chance to handle any errors and
> testing for sysfs leftovers in zram_add() looks weird.
> 
> 
> btw, do you also want me to rename zram-control sysfs handlers?
> zram_add -> zram_hot_add() ? i think zram_add()/zram_remove() is just
> ok.

I'm fine, too.

> 
> 
> 
> can you please test this patch?

I tested it and couldn't reproduce it during 5 minutes so it seems
we are correct about culpit.
However, it's trick and we should find a right way. :)

> 
> ---
> 
>  drivers/block/zram/zram_drv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index e6c4316..b31f0c20 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -1184,7 +1184,7 @@ static int zram_add(void)
>  	if (!zram)
>  		return -ENOMEM;
>  
> -	ret = idr_alloc(&zram_index_idr, zram, 0, 0, GFP_KERNEL);
> +	ret = idr_alloc_cyclic(&zram_index_idr, zram, 0, 0, GFP_KERNEL);
>  	if (ret < 0)
>  		goto out_free_dev;
>  	device_id = ret;

-- 
Kind regards,
Minchan Kim
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/