Received-SPF: pass (google.com: domain of linux-kernel+bounces-65716-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <e1b80fee-30df-4733-9072-ce67e3edc72f@oracle.com>
Date: Wed, 14 Feb 2024 12:50:31 -0500
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH V2 3/3] vdpa_sim: flush workers on suspend
Content-Language: en-US
To: Eugenio Perez Martin <eperezma@redhat.com>
Cc: virtualization@lists.linux-foundation.org, linux-kernel@vger.kernel.org,
        "Michael S. Tsirkin" <mst@redhat.com>,
        Jason Wang <jasowang@redhat.com>, Si-Wei Liu <si-wei.liu@oracle.com>,
        Xie Yongji <xieyongji@bytedance.com>,
        Stefano Garzarella <sgarzare@redhat.com>
References: <1707758174-142161-1-git-send-email-steven.sistare@oracle.com>
 <1707758174-142161-4-git-send-email-steven.sistare@oracle.com>
 <CAJaqyWfYHqf2=8BMo5ReKEB137fxGZR4XEJ2d4imXOOXAX2wHQ@mail.gmail.com>
From: Steven Sistare <steven.sistare@oracle.com>
Organization: Oracle Corporation
In-Reply-To: <CAJaqyWfYHqf2=8BMo5ReKEB137fxGZR4XEJ2d4imXOOXAX2wHQ@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?utf-8?B?V1JVaGI3eGxXY1dMT2ppMlhDTEtRVlN6REkwM3VVdUExaDArZFQ0QVlwMDlt?=
 =?utf-8?B?SGtxbTNsSStMN2ZMV2N3ZjdheVRwNXozWmkzb1E0Yzd5ZWF0VC9pUHlvNi91?=
 =?utf-8?B?RVRRL25IZThPZm52elVzN2FOM1BiOXQxQnh3ZXhxRWhWT1FNS2RQZUZ0emlk?=
 =?utf-8?B?dEtSMi85eDhYUGtkUWpkTFNBSDlFQ2IwSHljUlJaa2VRMUJydk9XT0x1K3E0?=
 =?utf-8?B?aWhoTTFNckF0WHI3bGQvY0tWRkxIOFVrOUR4K3pzbUZsVlRrWVNNVHRUL3RK?=
 =?utf-8?B?UkRWM2NNRUM3UC8zSmN0ZnZwZW0rMWt6c2V2TlFQcWt4WE11dFlFNmY4TXRH?=
 =?utf-8?B?T1Zta29ydDNQMzRxVmhtUjNWY3NhYitsOU9vZ25NWDQ5YThBVFo1NXpwSDdC?=
 =?utf-8?B?Q05sRmNOUDAwV0JKZ0ZrbHpLL1lNZzhETnl4MjFIc2VvRFJJZXBDZUNRUmlH?=
 =?utf-8?B?YjFEOEJYVW02b3BsM282aHpDdHhaUytEaW8zYURNYk90QUk1bnJqVDk4aDVh?=
 =?utf-8?B?WE4wOVVoaWpFMStEeUxZQng1Vll2M2c5KzluU2dqRnhWWEhkRHJ2U0pldlpO?=
 =?utf-8?B?NGVHdWpxQ0kveTVERmUycHVoV0Nnbzk5ZE41UVlBbXovaTJWL1J3cVY4Zi80?=
 =?utf-8?B?ZU15S0NVK1ErQ3QvMWNYY1Ayd2ZzUXZ5bUZzWXpzSHpPRlhwSXMzYkJmbmdl?=
 =?utf-8?B?cUxnSXRVZHVOM3FVTExaSFJERVNTT0V1azd3ZVBGdWpwelpTd2VoTHREU1J4?=
 =?utf-8?B?R05RMVZCN2F4LzBobFc3K3lONHFMS3RDS3RiZ21jZlg2NkRHYnAzUUM3WnJB?=
 =?utf-8?B?RFFSRHE3UWRGT1RFK1JXK2VFUkZ1SE84azFMdFl6Q0s0OHlPSUE3c1laWVd4?=
 =?utf-8?B?Vm0rVEV3Y2trWVVDcjNUMGRUSXZTMFFQTzY0bG81M251Tm84YnQzKzZaTzh1?=
 =?utf-8?B?N0hWdnNsTkxqbklYd0pmdlJaa1ZlTWNWUDNPTGgrbmRIQXFvMGFzQStYREJp?=
 =?utf-8?B?V0J1WUtXYjVQRzQ2S2JJSU1GN0duQnBaN2xKMXIxeUFyQU92TmYrc3Y2cTI5?=
 =?utf-8?B?UW1aRFBuemllcWttYWNncmFPdXdma3ZFNWN1c25HeW1vdGlxQ05PQTlPeGdw?=
 =?utf-8?B?MFFORXBnaitXRXpIU1VXZW1mM3FGVU1JUEJxa3M5b2ZKc0FaUGlXbUIwYmRU?=
 =?utf-8?B?UncxNE1yWmdMNkFBWDF1R3REMGVJRmtKay90c2ZSYXpkejN0LzYrU1owSnRa?=
 =?utf-8?B?M1VqSXJDVThGc001bWZpQldRU3FwQU52OS8zYW4ramU3RUZLckYzN21qSlVG?=
 =?utf-8?B?ckVqUDhtWkhad3RoNk9CUFZ3NE4wNWNNUVVacHNMVDU2RHVQaGZxR2lSMWl2?=
 =?utf-8?B?YVdUUlBVUHMxQ1ZlRXBodEdMVFlrZmJWMUFIbEFoUXFvenB5c1kyS3VOOW42?=
 =?utf-8?B?clMrK0VJQlUyeW5lYnlYOElqaDhVSU5YMmpRQXhNVG5IQVo3VENTU0xlanI5?=
 =?utf-8?B?SGxvRXAvV2diTDZKZmc3eWtkZGJaK2RydUdHd3RZMmV5eWlFVVdNVEs1NWlr?=
 =?utf-8?B?VEk1aTZhMUpPVHAvdzU0dFBSV2plV09kVmdudjdQbVJhTldKblp1T2NPV1dw?=
 =?utf-8?B?dkEwOENuUlFaKzZQY2pRS2ZUbDdFUis2eDROWXAraFhTSHZKUE5aZlpTeFF3?=
 =?utf-8?B?Wnp4eVlqMW1ucmtRT0RMSHNPdFREOXAvVytzUFpQd1F6bUZxY3NJMTV3WE82?=
 =?utf-8?B?QWNxd3NuZVhjTWhmL0VEZVp1OWp5UVFyK0g4M1N6MHMyaXBVbGdWZVhubEtk?=
 =?utf-8?B?TjFVSEJldkcrM0ZVd3JsSzJCcjQrVXg3R2piYkZ2eTU4M2F3ZXBaRFJKVlJy?=
 =?utf-8?B?bmZXQkVCcW15dHVoc0h3NHJuN2NwQ0UxMmNnSldCWlpnQnc4N0xHSzhBNllX?=
 =?utf-8?B?YkUvODhtVENtbWY1bFR5dGl0VGVWalhLYS9YbVlEcXA4ekJnWGs2eFN5aE9u?=
 =?utf-8?B?SGhHNVBDKzQwaER4a05tRGdIQ1ljbW9ySnZaTmZpdnFRNktGalVtR0J5emQ4?=
 =?utf-8?B?TVNRLzd1SVJPWHRYc3pad054azVsYUZhZFRtT2hvSWhUdU9EcFBpYzlhdm12?=
 =?utf-8?B?UHZBazhiZDlNT3Jsakc2NE1Gbyt6aTYvYWwvblNKZXVybldxdHdlSWRxZTlN?=
 =?utf-8?B?d1E9PQ==?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	ueXHhJmPkX4+S6NqeRFXSPc7tl2tL/6RobbHBMVzdwIN30dLQWDlYMMgzTmF2lvltMzO5tsXemCvV+xgx84xVfNBmtnjUVnfAV0X00B8SlFWCcWgS6iLbiiCLqd+TwC5aKDrTb41/Jvy8MykSN3mIfiOd3guIIITnlro1Tir6RIeNEx5H0cjyGFgGePRT+zBEjsve/XMuhgYA1Ui8Ptg7Ai5c/kTVLK7XMRUy2lAorXLW1RLbLkk/IAnTHlqc+axZuNV3vrE9UqGiJ/ye0PIpqGJIjevdFynaZK/TSrfZLwcQCJEa0H7Gkdjw6McF63UfSLb3XKiAn0MPTwsequnZ9T4t78VBPb3lzTkFaqE9J3bnfBBhamD4jB9cH/hVKb/IzNs/E9uLJU9pexn1TrFs9sqb6Cw5Iu3jooOwRNB0TW+yRvdTThzItz3Qv25c1ia/wq+Pz3KjulSz9zfVtngZIBWBQZKcWCHjjfwYXOqm446XgBjcS+L7TCcZs9J6C7XsQb40xbCNlRBEDYJ44kQLJGrYaljL2aY6tyY51zuaGjMLbLBqEsrq3cR6mN1lNUkRKwEjQ3UtP1vFU7VVxgI0EhYH2wZAnhSlZDFb4idRXw=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: e54c1c7e-5a5a-40d5-4658-08dc2d857270
X-MS-Exchange-CrossTenant-AuthSource: SA2PR10MB4684.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Feb 2024 17:50:35.4704
 (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: KJQi43yykvXyc0uv66h0qHBKclSGLv9AZGsobLrdz8M5gnvtMQUc2Uknwsqn8/in0VzRpFKlIuo2pjMqY7QTZ0bOCaa7hI92zy0Yd/vQELI=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR10MB5551
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.1011,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2024-02-14_10,2024-02-14_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=999 phishscore=0 mlxscore=0
 bulkscore=0 spamscore=0 malwarescore=0 adultscore=0 suspectscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2311290000
 definitions=main-2402140139
X-Proofpoint-ORIG-GUID: OX3ltFf26ux8nknJfT4aLvQei1PYMdAe
X-Proofpoint-GUID: OX3ltFf26ux8nknJfT4aLvQei1PYMdAe

On 2/13/2024 11:10 AM, Eugenio Perez Martin wrote:
> On Mon, Feb 12, 2024 at 6:16 PM Steve Sistare <steven.sistare@oracle.com> wrote:
>>
>> Flush to guarantee no workers are running when suspend returns.
>>
>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>> ---
>>  drivers/vdpa/vdpa_sim/vdpa_sim.c | 13 +++++++++++++
>>  1 file changed, 13 insertions(+)
>>
>> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
>> index be2925d0d283..a662b90357c3 100644
>> --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
>> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
>> @@ -74,6 +74,17 @@ static void vdpasim_worker_change_mm_sync(struct vdpasim *vdpasim,
>>         kthread_flush_work(work);
>>  }
>>
>> +static void flush_work_fn(struct kthread_work *work) {}
>> +
>> +static void vdpasim_flush_work(struct vdpasim *vdpasim)
>> +{
>> +       struct kthread_work work;
>> +
>> +       kthread_init_work(&work, flush_work_fn);
> 
> If the work is already queued, doesn't it break the linked list
> because of the memset in kthread_init_work?

work is a local variable.  It completes before vdpasim_flush_work returns,
thus is never already queued on entry to vdpasim_flush_work.
Am I missing your point?

>> +       kthread_queue_work(vdpasim->worker, &work);
>> +       kthread_flush_work(&work);
>> +}
>> +
>>  static struct vdpasim *vdpa_to_sim(struct vdpa_device *vdpa)
>>  {
>>         return container_of(vdpa, struct vdpasim, vdpa);
>> @@ -511,6 +522,8 @@ static int vdpasim_suspend(struct vdpa_device *vdpa)
>>         vdpasim->running = false;
>>         mutex_unlock(&vdpasim->mutex);
>>
>> +       vdpasim_flush_work(vdpasim);
> 
> Do we need to protect the case where vdpasim_kick_vq and
> vdpasim_suspend are called "at the same time"? Correct userland should
> not be doing it but buggy or mailious could be. Just calling
> vdpasim_flush_work with the mutex acquired would solve the issue,
> doesn't it?

Good catch.  I need to serialize access to vdpasim->running plus the worker queue
in these two functions.  vdpasim_kick_vq currently takes no locks. In case it is called 
from non-task contexts, I should define a new spinlock to be acquired in both functions.

- Steve