All of lore.kernel.org
 help / color / mirror / Atom feed
From: Minchan Kim <minchan@kernel.org>
To: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Cc: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Nitin Gupta <ngupta@vflare.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCHv4 00/10] add on-demand device creation
Date: Wed, 6 May 2015 17:20:37 +0900	[thread overview]
Message-ID: <20150506082036.GB8163@blaptop> (raw)
In-Reply-To: <20150506081029.GA8385@swordfish>

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

  reply	other threads:[~2015-05-06  8:20 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-04 12:38 [PATCHv4 00/10] add on-demand device creation Sergey Senozhatsky
2015-05-04 12:38 ` [PATCHv4 01/10] zram: add `compact` sysfs entry to documentation Sergey Senozhatsky
2015-05-04 12:38 ` [PATCHv4 02/10] zram: cosmetic ZRAM_ATTR_RO code formatting tweak Sergey Senozhatsky
2015-05-04 12:38 ` [PATCHv4 03/10] zram: use idr instead of `zram_devices' array Sergey Senozhatsky
2015-05-04 12:38 ` [PATCHv4 04/10] zram: reorganize code layout Sergey Senozhatsky
2015-05-04 12:38 ` [PATCHv4 05/10] zram: remove max_num_devices limitation Sergey Senozhatsky
2015-05-04 12:38 ` [PATCHv4 06/10] zram: report every added and removed device Sergey Senozhatsky
2015-05-04 12:38 ` [PATCHv4 07/10] zram: trivial: correct flag operations comment Sergey Senozhatsky
2015-05-04 12:39 ` [PATCHv4 08/10] zram: return zram device_id from zram_add() Sergey Senozhatsky
2015-05-04 12:39 ` [PATCHv4 09/10] zram: close race by open overriding Sergey Senozhatsky
2015-05-04 12:39 ` [PATCHv4 10/10] zram: add dynamic device add/remove functionality Sergey Senozhatsky
2015-05-06  5:01 ` [PATCHv4 00/10] add on-demand device creation Minchan Kim
2015-05-06  5:25   ` Sergey Senozhatsky
2015-05-06  6:52     ` Minchan Kim
2015-05-06  7:07     ` Sergey Senozhatsky
2015-05-06  7:28       ` Minchan Kim
2015-05-06  8:10         ` Sergey Senozhatsky
2015-05-06  8:20           ` Minchan Kim [this message]
2015-05-07  0:33             ` Sergey Senozhatsky
2015-05-07  7:41               ` Minchan Kim
2015-05-07 12:09                 ` Sergey Senozhatsky
2015-05-07 13:04                   ` Sergey Senozhatsky
2015-05-07 15:11                     ` Minchan Kim
2015-05-08  0:15                       ` Sergey Senozhatsky
2015-05-10  8:47                       ` Sergey Senozhatsky
2015-05-06  5:31 ` Sergey Senozhatsky

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150506082036.GB8163@blaptop \
    --to=minchan@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ngupta@vflare.org \
    --cc=sergey.senozhatsky.work@gmail.com \
    --cc=sergey.senozhatsky@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.