Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <201528df-4474-b544-e25a-9746e30df04d@oracle.com>
Date:   Tue, 18 Jan 2022 13:00:39 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.5.0
Subject: Re: [PATCH V6 01/10] Use copy_process in vhost layer
Content-Language: en-US
From:   Mike Christie <michael.christie@oracle.com>
To:     "Eric W. Biederman" <ebiederm@xmission.com>
Cc:     geert@linux-m68k.org, vverma@digitalocean.com, hdanton@sina.com,
        hch@infradead.org, stefanha@redhat.com, jasowang@redhat.com,
        mst@redhat.com, sgarzare@redhat.com,
        virtualization@lists.linux-foundation.org,
        christian.brauner@ubuntu.com, axboe@kernel.dk,
        linux-kernel@vger.kernel.org
References: <20211129194707.5863-1-michael.christie@oracle.com>
 <87tuf79gni.fsf@email.froward.int.ebiederm.org>
 <a171238e-d731-1c22-af72-0f7faf7f4bea@oracle.com>
 <87tuf11oe6.fsf@email.froward.int.ebiederm.org>
 <a3bd6fe7-1775-6fdd-3a02-e779c4d4e498@oracle.com>
 <87pmpoxzuf.fsf@email.froward.int.ebiederm.org>
 <783145b7-243b-b85e-e274-44ef6c0696b9@oracle.com>
 <874k62b76d.fsf@email.froward.int.ebiederm.org>
 <68ba89ae-108e-c14a-02a0-db72b169c9b1@oracle.com>
In-Reply-To: <68ba89ae-108e-c14a-02a0-db72b169c9b1@oracle.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?RGJaYmtIbXk4dmt6M0pkQnhWSElZL24wR1FRQXJxSldvbmZOTTQ3Sm5CbzlN?=
 =?utf-8?B?STBzaTZwUG9tVVJMbUJhNmI0MjZIdEc2NkdaWWFwSW0vQnpwd0pycTQxcGVE?=
 =?utf-8?B?WHJlUmN4V0hlcDN3bjc4NEtBOEJ6UjRyVUxGeUl0MFJ3VEkrS2tvVzRpeDEz?=
 =?utf-8?B?SFl0WHZIR1NXbWhGVUk0ZDBHNXRFU0UxWDIya01pTW5mbDZQMitLOGZ6aUox?=
 =?utf-8?B?NGVZZFc3SXpwd2lsSW8zLzlaSTFiQlJaeFEvQ05CeXBDY2RKd0tGcEllOExu?=
 =?utf-8?B?MFhwcmhSZlZIUUk5Z2RDQjlTUUdmUjZ4Q29Rd014cnl4TCtZWktEcVVqdUJX?=
 =?utf-8?B?NEJ6R1ZUZW5SVXJLWkJpcmJHY2pCU3ZabzNiK2xiUW1ua2YvQWk0L2xwYWFF?=
 =?utf-8?B?TFhWbkdRaTV1MGozcEY5eWVqQjNicjdMZXNFaXZBcnhJOXkxOWI0RVRsOXky?=
 =?utf-8?B?SGxMU2VQd1NVMllJQkpDaFRpQlJpc3hYYmNrWmtVclBBMWw4V2M5RHpYaXNR?=
 =?utf-8?B?djF4emR1MzJ1OFlreGxaVTZuaHAwZEdVSmQxOUlYaW9QT0FzVmQyYUdlMm0r?=
 =?utf-8?B?b2RzamVINzM0UlM2Y0ZOaXhQWU4yaW9TZHhZSFRWOFlqbnNPWmxPWFltUEJ1?=
 =?utf-8?B?TThiWTdqRlMrM1g5N0hTMUJ5REF5T1FJbzc5SzlvWlRvZ3QySlM2U25BZ3Zx?=
 =?utf-8?B?NWhPUWZZUWhXc2pBTTZweEVwTVVTdFFSUzNiUDJBUVFkcDRaU3grRk8zZnhM?=
 =?utf-8?B?d3U2dFlCc3kxdWhEcU9RQ0t3Zlc3TjNtT0lHcWRmdXRpMHZ2dWk0bnY5NU40?=
 =?utf-8?B?cmVnbTVYMU8wRm5vNEY2MVU2cmxYNlN0ZWZ2SU5ja2U0cXlxRjQ5Qm11VU1w?=
 =?utf-8?B?cnNsd0RNak9vMFNuOGVwdURDLzZZelBMYml6Y21mYzhlV3JQKzBHbEVMR2s2?=
 =?utf-8?B?Rm05OEx2VWI1VEJKYjFzUi9LUm9wSEpPY0lkakpVc09kUEpvMSt5S1FGNml1?=
 =?utf-8?B?Y055aU9LVzFyWGZ1T2tVUWp3UEJORTNPcXhZaDJYNzdNK2VjVnJFTTNCL3ht?=
 =?utf-8?B?VDJTbnQ5VzJiUUFyM1Y3UWRLcjVXc21lamJIc3JCT0NsRlNCRWJMQ1dKMXQv?=
 =?utf-8?B?TzF0bFVCWGVUMURMNWpzM3NZWUNkSGk5bUE5VEVuTDZiaVA2RytVRWtzM3Zj?=
 =?utf-8?B?eFlBZ2pQcm9TVW53QXJ5OW9hUnVOV0llZFo3ZFp2cDhMditDdXg2Sm5ZelRW?=
 =?utf-8?B?L0g0M2xzc2VWaTBqL0JGdGxvREdWYWpEcE5lMTgzcDgrTmh4cFNnSGN2aHF0?=
 =?utf-8?B?TUpRNFBOVWRZUkVlRE5uN0UzZ2Jydm5VdldYNFBscndMaW9ManBVRy9LaEh3?=
 =?utf-8?B?ZFlxSCtkcm9lQkhma2dKcS9zckEzMkVScUJDN2h5dDllYVFzTE5oQnZkTmtD?=
 =?utf-8?B?Q3VSUnhqck1waDVoUklLM0dJNUkxR2V6b3NHdHFmdW14WHJRL1kyRFpLSnEy?=
 =?utf-8?B?d05zK1BsZEsxY1kvSWVoa1lhM1ZoOUpDREpXY2hpc2E5RzVCc05lWnVNM0sv?=
 =?utf-8?B?L3dvUkJ6cGpvOEVQQzdFQWlWYnYxK3d6QU41dmhOYktKUkZ1b2orYmZWeUhY?=
 =?utf-8?B?L25yeWJtR210Skg4d3RMdmRGR0FXRGRYWjhINFhldWU2V3J5UFNVU1Jtakly?=
 =?utf-8?B?MU13L3ZxWThHdDFEZngyMG4xSTJDUTdabTY0QWRaZ2dveFljWExHT2xvVUJa?=
 =?utf-8?B?TklCOUxJNzdBV1B2ZzJESVpPNThmRlVzelRUU01PMmFLRjBYN3J2c1QvV3Ry?=
 =?utf-8?B?NzhEUDIwVmJDYU10L2plODFnSTZlNE85dS96aTFHREgxZ2RUaUZUTCtzS0cy?=
 =?utf-8?B?V2pMUWh0YnZiMTV4TE9hRm9pczBwSWlFRjhUY3hEamhIRytWOTdWd0ZzYWV5?=
 =?utf-8?B?WWdKZTRuSFlodmNheFI3Y1RVL0EwM0hRNXVSQWZVckMyaVdBeS8yTjZyNDdu?=
 =?utf-8?B?WEZnT2QwLzNnd1FkWDd4MWx5UHl0bFJNZzZCVXZMZW5hL0c0LzJRUG5oYkVS?=
 =?utf-8?B?bTdmdE8vajVKNGhJWHdJN2UyclY4SnpUOU9zQ29PczNGREg3NDFaTjk3TVVL?=
 =?utf-8?B?Qy9BTW05Mi9hb0ViL0IzYmFGd3pJNEFqN0lRbFlpb01aT1BjbE1lNjUzSVhZ?=
 =?utf-8?B?MGtZZlZQQ05EazZPRlhPSnRvZTdRQWNJbUdpaDdmaTl2MTAwcWU3bUdoK3VN?=
 =?utf-8?B?T1ovR1VmclZ4dUQwc3R4MXc4Z0FnPT0=?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 8478894c-785d-4e2d-ae22-08d9dab4d2c9
X-MS-Exchange-CrossTenant-AuthSource: DM5PR10MB1466.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 18 Jan 2022 19:00:41.7224
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: okCPvJgD1Iqd1Aenu/hGT3jZ+4GgPKsdBo9s9hT63FqWsdKuM9dyIDoq5g42z9PFuold88LuTo/rPwF4sUI6Pac0ZiNghr8sI77MWfEZc6A=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR10MB5795
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10231 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 adultscore=0 phishscore=0
 mlxlogscore=999 malwarescore=0 bulkscore=0 suspectscore=0 spamscore=0
 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2110150000 definitions=main-2201180113
X-Proofpoint-GUID: PhsSQegpFunzt2h9r632j-a8CAOOGFmf
X-Proofpoint-ORIG-GUID: PhsSQegpFunzt2h9r632j-a8CAOOGFmf
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 1/18/22 12:51 PM, Mike Christie wrote:
> On 1/17/22 11:31 AM, Eric W. Biederman wrote:
>> Mike Christie <michael.christie@oracle.com> writes:
>>
>>> On 12/22/21 12:24 PM, Eric W. Biederman wrote:
>>>> All I am certain of is that you need to set
>>>> "args->exit_signal = -1;".  This prevents having to play games with
>>>> do_notify_parent.
>>>
>>> Hi Eric,
>>>
>>> I have all your review comments handled except this one. It's looking like it's
>>> more difficult than just setting the exit_signal=-1, so I wanted to check that
>>> I understood you.
>>
>> [snip problems with exit_signal = -1]
>>
>>>
>>> What do you think?
>>
>> I was wrong.  I appear to have confused the thread and the non-thread
>> cases.
>>
>> Perhaps I meant "args->exit_signal = 0".  That looks like
>> do_notify_parent won't send it, and thread_group_leader continues to do
>> the right thing.
> 
> That doesn't work too. exit_notify will call do_notify_parent but 
> our parent, qemu, does not ignore SIGCHILD so we will not drop
> down in into this chunk:
> 
>         psig = tsk->parent->sighand;
>         spin_lock_irqsave(&psig->siglock, flags);
>         if (!tsk->ptrace && sig == SIGCHLD &&
>             (psig->action[SIGCHLD-1].sa.sa_handler == SIG_IGN ||
>              (psig->action[SIGCHLD-1].sa.sa_flags & SA_NOCLDWAIT))) {
> 
> do_notify_parent will return false and so autoreap in exit_notify will
> be false.
> 
> 
> 
>>
>> Baring any additional confusion on my part that cleanly solves the
>> problem of how not to send a signal from a child process cleanly.
>>


Oh yeah, maybe we are thinking about different things.

The issue I am hitting is that the parent, qemu, does not know about
these task_structs. And, userspace can add/delete these vhost devices
dynamically. So qemu could continue to run, but the vhost device could
be deleted. In this case I want to free up the task_struct resources
since we are no longer using it.

With kthreads do_notify_parent returns true and exit_notify calls
release_task, so it's handled for me. I had stuck in the USER/VHOST
check in the patch you didn't like to get that behavior.

If you prefer not adding the VHOST/USER task check in exit_notify then
instead of auto reaping, would another possible alternative be to add a
modified kernel_wait like function in the vhost layer to reap the task?