Re: [PATCH] Fix for bad lock balance in Containers

["Paul Menage" <menage@google.com>]

On 6/26/07, Dhaval Giani <dhaval@linux.vnet.ibm.com> wrote:
>
> There are a few questions I had with respect to the current code,
>
> Why is the increment of s_active dependent on the return value of
> simple_set_mnt?

I think it's because, as you observed, grab_super() is static and
hence not reachable from container.c. But I thought that the only side
effect of it that we needed (since we had a safe pointer to the
superblock not obtained via the superblock list) was the increment of
sb_active. (As it turns out I was wrong and we needed the s_umount
lock too).

Incrementing the sb_active count (if simple_set_mnt() failed) seemed
as though it would be wrong.

>
> What should be the correct approach to get the locking balance? As far
> as I can see, the correct method would be to call sget which would then
> correctly handle everything. But that would require a test function. I
> saw functionality similar to a test function in the beginning of
> container_get_sb(). Should that be seperated and put in a seperate test
> function so that sget can be called?

I don't quite remember why I did it this way originally, now. I've got
no objection to the code being cleaned up to fit the more common
approach if it's practical to do so, but it seems to me that your
current patch fix seems enough at least for now.

Thanks,

Paul