Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:1 as permitted sender) client-ip=2620:137:e000::3:1;
Message-ID: <b9e9eca9-413c-0301-3839-bb7534ac76d2@linux.ibm.com>
Date:   Fri, 22 Sep 2023 21:15:48 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.14.0
Subject: Re: [PATCH] s390/cio: Fix a memleak in css_alloc_subchannel
Content-Language: en-US
To:     Halil Pasic <pasic@linux.ibm.com>,
        Cornelia Huck <cohuck@redhat.com>
Cc:     Dinghao Liu <dinghao.liu@zju.edu.cn>,
        Peter Oberparleiter <oberpar@linux.ibm.com>,
        Heiko Carstens <hca@linux.ibm.com>,
        Vasily Gorbik <gor@linux.ibm.com>,
        Alexander Gordeev <agordeev@linux.ibm.com>,
        Christian Borntraeger <borntraeger@linux.ibm.com>,
        Sven Schnelle <svens@linux.ibm.com>,
        Julian Wiedmann <jwi@linux.ibm.com>,
        linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20230921071412.13806-1-dinghao.liu@zju.edu.cn>
 <20230922141700.10895474.pasic@linux.ibm.com> <87sf76z961.fsf@redhat.com>
 <20230922152035.3ef1cb7e.pasic@linux.ibm.com>
From:   Vineeth Vijayan <vneethv@linux.ibm.com>
In-Reply-To: <20230922152035.3ef1cb7e.pasic@linux.ibm.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk


On 9/22/23 15:20, Halil Pasic wrote:
>> Author of 2ec2298412e1 here. If I don't completely misremember things,
>> this was for the orphanage stuff (i.e. ccw devices that were still kept
>> as disconnected, like dasd still in use, that had to be moved from their
>> old subchannel object because a different device appeared on that
>> subchannel.) That orphanage used a single dummy subchannel for all ccw
>> devices moved there.
>>
>> I have no idea how the current common I/O layer works, but that might
>> give you a hint about what to look for ????
> Yes, that is what the commit states and what the series is about. I hope
> Vineeth can give us some answers ???? maybe even out of the top of his
> head... If not, I would trust his judgment on whether figuring things
> out is worthwhile or not.
> 
As Corny mentioned, orphanage is the only case i remember where
this scenario of dynamically allocated sch->lock  being used. I hope
you remember the cdev->ccwlock, which is nothing but the copy of
sch->lock pointer. This is rather a tricky design, where we are using 
the sch->lock and cdev->ccwlock, which are same pointers.
Because this sch is exclusively for the cdev ops. But at the same time,
a CC3 code in the stsch can make the attached device an orphanage and
remove the sch.

We have already seen an issue with this approach and had couple of
discussions about avoiding this pointer usage without using an extra
lock but do not have a right solution for this now.