Re: [PATCHv3 9/9] zram: add dynamic device add/remove functionality

From: Minchan Kim
Date: Sun May 03 2015 - 22:20:29 EST


Hello Sergey,

On Thu, Apr 30, 2015 at 03:51:12PM +0900, Sergey Senozhatsky wrote:
> On (04/30/15 15:44), Minchan Kim wrote:
> > > > I think the problem of deadlock is that you are trying to remove sysfs file
> > > > in sysfs handler.
> > > >
> > > > #> echo 1 > /sys/xxx/zram_remove
> > > >
> > > > kernfs_fop_write - hold s_active
> > > > -> zram_remove_store
> > > > -> zram_remove
> > > > -> sysfs_remove_group - hold s_active *again*
> > > >
> > > > Right?
> > > >
> > >
> > > are those same s_active locks?
> > >
> > >
> > > we hold (s_active#163) and (&bdev->bd_mutex) and want to acquire (s_active#162)
> >
> > Thanks for sharing the message.
> > You're right. It's another lock so it shouldn't be a reason.
> > Okay, I will review it. Please give me time.
> >
>
> sure, no problem and no rush. thanks!

I had a time to think over it.

I think your patch is rather tricky so someone cannot see sysfs
although he already opened /dev/zram but after a while he can see sysfs.
It's weired.

I want to fix it more generic way. Othewise, we might have trouble with
locking problem sometime. We already have experieced it with init_lock
although we finally fixed it.

I think we can fix it with below patch I hope it's more general and right
approach. It's based on your [zram: return zram device_id from zram_add()]

What do you think about?