From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,HTML_MESSAGE,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0C0E7C433DF for ; Mon, 19 Oct 2020 15:56:51 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 43D5C22283 for ; Mon, 19 Oct 2020 15:56:49 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=bytedance-com.20150623.gappssmtp.com header.i=@bytedance-com.20150623.gappssmtp.com header.b="DoezrG7/" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 43D5C22283 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=bytedance.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 6AB576B005C; Mon, 19 Oct 2020 11:56:49 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 65BB96B0062; Mon, 19 Oct 2020 11:56:49 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 525E76B0068; Mon, 19 Oct 2020 11:56:49 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0058.hostedemail.com [216.40.44.58]) by kanga.kvack.org (Postfix) with ESMTP id 245366B005C for ; Mon, 19 Oct 2020 11:56:49 -0400 (EDT) Received: from smtpin29.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 95862805CECD for ; Mon, 19 Oct 2020 15:56:48 +0000 (UTC) X-FDA: 77389128096.29.chess48_2f01f2427238 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin29.hostedemail.com (Postfix) with ESMTP id 629EE18086CCF for ; Mon, 19 Oct 2020 15:56:48 +0000 (UTC) X-HE-Tag: chess48_2f01f2427238 X-Filterd-Recvd-Size: 9835 Received: from mail-ej1-f68.google.com (mail-ej1-f68.google.com [209.85.218.68]) by imf18.hostedemail.com (Postfix) with ESMTP for ; Mon, 19 Oct 2020 15:56:47 +0000 (UTC) Received: by mail-ej1-f68.google.com with SMTP id dt13so14583124ejb.12 for ; Mon, 19 Oct 2020 08:56:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bytedance-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=cfO4yF2rYNUt/OPuJN8ZbL2Z4Io5CzI0Aq7ckn9yvW0=; b=DoezrG7/n3HGzTyG4zWvjFYdmdoXy0q2gTx1IBen/FVnvVU8+94yS6gasAzu6XtNNn P30ALAqUQylrgYe7q5svEjbJ8EgKTmMDP1k5Z4hXOlqG/p451Z9rJmQvfC9kAoKbhNDH ZqgeF86OpEZgumbbr2I7DmWoCynjUCuq9ev7Nr2YzVfr0hI2s3siCsk7J9K6v5htoiL9 fEW3ESn3ytfJsLGOtwH8ODkEvAnBIEHTVW9YF/06zOVJ7p4Gfd88yJR5rQ1ysXiTSye7 qEQjuh7KlYsXXVd5R3dOnD3w3MxlEWcTsE9Mr3k2Zc/hgL5C5gmHCReZDk5T2CHh5/6e A+WQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=cfO4yF2rYNUt/OPuJN8ZbL2Z4Io5CzI0Aq7ckn9yvW0=; b=jYkrw2jGBFBP6iTqd/KF6OJhQ32sLUa8psMRBAuCHRwjXnUILIwGF+VYpkmOKPC7W9 nCD3vPQdcyamzkXxbuuPKJH04Wj1mjaqBSAgjs2bu4oJci0sDjcByNYdvmYTauq1f2GB Mz7/zpzWMZUu5D1yi8lnYhfrw1cSCljajpbTCyEzMDPsTWlGdgTHrnGe9cuEZfQryII8 JHj8kukdDT8rLebjuPyg0B2MRpw8vdGJY0CtarxhHU1eOIHnFDlXuvMv5EerukWQgbNx HWIXsz/cFgD/WNr2oKrinmqd9MiaX0HICf7yCMH9FO1TKBs3fmZqo80leCNQHMstmuV4 dL3A== X-Gm-Message-State: AOAM532srdE0bSWhtQkEnGF/TM3bF1pIf1Q01aFPksnrMnXll1J3rIb1 DjFqhiqCj/uvwZYCZCdD48iPga15eF4SmMHxrke3 X-Google-Smtp-Source: ABdhPJwoC/L8uyX8/SmZ30lA1ZQWAlNmvr2uWdNcdfabMTIUNW20JKnJ8Y2sNTGzOC0y+rfbMIChPe5Xc2GLT7iSMxw= X-Received: by 2002:a17:906:f118:: with SMTP id gv24mr565645ejb.174.1603123006508; Mon, 19 Oct 2020 08:56:46 -0700 (PDT) MIME-Version: 1.0 References: <20201019145623.671-1-xieyongji@bytedance.com> <20201019145623.671-4-xieyongji@bytedance.com> <20201019110359-mutt-send-email-mst@kernel.org> <20201019114701-mutt-send-email-mst@kernel.org> In-Reply-To: <20201019114701-mutt-send-email-mst@kernel.org> From: =?UTF-8?B?6LCi5rC45ZCJ?= Date: Mon, 19 Oct 2020 23:56:35 +0800 Message-ID: Subject: Re: [External] Re: [RFC 3/4] vduse: grab the module's references until there is no vduse device To: "Michael S. Tsirkin" Cc: jasowang@redhat.com, akpm@linux-foundation.org, linux-mm@kvack.org, virtualization@lists.linux-foundation.org Content-Type: multipart/alternative; boundary="00000000000055b6a005b20829a6" X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: --00000000000055b6a005b20829a6 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Mon, Oct 19, 2020 at 11:47 PM Michael S. Tsirkin wrote: > On Mon, Oct 19, 2020 at 11:44:36PM +0800, =E8=B0=A2=E6=B0=B8=E5=90=89 wro= te: > > > > > > On Mon, Oct 19, 2020 at 11:05 PM Michael S. Tsirkin > wrote: > > > > On Mon, Oct 19, 2020 at 10:56:22PM +0800, Xie Yongji wrote: > > > The module should not be unloaded if any vduse device exists. > > > So increase the module's reference count when creating vduse > > > device. And the reference count is kept until the device is > > > destroyed. > > > > > > Signed-off-by: Xie Yongji > > > --- > > > drivers/vdpa/vdpa_user/vduse_dev.c | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c > b/drivers/vdpa/vdpa_user/ > > vduse_dev.c > > > index 6787ba66725c..f04aa02de8c1 100644 > > > --- a/drivers/vdpa/vdpa_user/vduse_dev.c > > > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c > > > @@ -887,6 +887,7 @@ static int vduse_destroy_dev(u32 id) > > > kfree(dev->vqs); > > > vduse_iova_domain_destroy(dev->domain); > > > vduse_dev_destroy(dev); > > > + module_put(THIS_MODULE); > > > > > > return 0; > > > } > > > @@ -931,6 +932,7 @@ static int vduse_create_dev(struct > vduse_dev_config > > *config) > > > > > > dev->connected =3D true; > > > list_add(&dev->list, &vduse_devs); > > > + __module_get(THIS_MODULE); > > > > > > return fd; > > > err_fd: > > > > This kind of thing is usually an indicator of a bug. E.g. > > if the refcount drops to 0 on module_put(THIS_MODULE) it > > will be unloaded and the following return will not run. > > > > > > > > Should this happen? The refcount should be only decreased to 0 after t= he > > misc_device is closed? > > > > Thanks, > > Yongji > > > > OTOH if it never drops to 0 anyway then why do you need to increase it? > > To prevent unloading the module in the case that the device is created, but no user process using it (e.g. the user process crashed). Thanks, Yongji > > > > > > -- > > > 2.25.1 > > > > > > --00000000000055b6a005b20829a6 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable



On Mon, Oct 19, 2020 at 11:47 PM Michael S. Tsirkin <mst@redhat.com> wrote:
On Mon, Oct 19, 2020 = at 11:44:36PM +0800, =E8=B0=A2=E6=B0=B8=E5=90=89 wrote:
>
>
> On Mon, Oct 19, 2020 at 11:05 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
>=C2=A0 =C2=A0 =C2=A0On Mon, Oct 19, 2020 at 10:56:22PM +0800, Xie Yongj= i wrote:
>=C2=A0 =C2=A0 =C2=A0> The module should not be unloaded if any vduse= device exists.
>=C2=A0 =C2=A0 =C2=A0> So increase the module's reference count w= hen creating vduse
>=C2=A0 =C2=A0 =C2=A0> device. And the reference count is kept until = the device is
>=C2=A0 =C2=A0 =C2=A0> destroyed.
>=C2=A0 =C2=A0 =C2=A0>
>=C2=A0 =C2=A0 =C2=A0> Signed-off-by: Xie Yongji <xieyongji@bytedance.com>= ;
>=C2=A0 =C2=A0 =C2=A0> ---
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 drivers/vdpa/vdpa_user/vduse_dev.c | 2 += +
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 1 file changed, 2 insertions(+)
>=C2=A0 =C2=A0 =C2=A0>
>=C2=A0 =C2=A0 =C2=A0> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.= c b/drivers/vdpa/vdpa_user/
>=C2=A0 =C2=A0 =C2=A0vduse_dev.c
>=C2=A0 =C2=A0 =C2=A0> index 6787ba66725c..f04aa02de8c1 100644
>=C2=A0 =C2=A0 =C2=A0> --- a/drivers/vdpa/vdpa_user/vduse_dev.c
>=C2=A0 =C2=A0 =C2=A0> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
>=C2=A0 =C2=A0 =C2=A0> @@ -887,6 +887,7 @@ static int vduse_destroy_d= ev(u32 id)
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0 =C2=A0kfree(dev->vqs);<= br> >=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0 =C2=A0vduse_iova_domain_de= stroy(dev->domain);
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0 =C2=A0vduse_dev_destroy(de= v);
>=C2=A0 =C2=A0 =C2=A0> +=C2=A0 =C2=A0 =C2=A0module_put(THIS_MODULE);<= br> >=C2=A0 =C2=A0 =C2=A0>=C2=A0
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0 =C2=A0return 0;
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 }
>=C2=A0 =C2=A0 =C2=A0> @@ -931,6 +932,7 @@ static int vduse_create_de= v(struct vduse_dev_config
>=C2=A0 =C2=A0 =C2=A0*config)
>=C2=A0 =C2=A0 =C2=A0>=C2=A0
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0 =C2=A0dev->connected = =3D true;
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0 =C2=A0list_add(&dev-&g= t;list, &vduse_devs);
>=C2=A0 =C2=A0 =C2=A0> +=C2=A0 =C2=A0 =C2=A0__module_get(THIS_MODULE)= ;
>=C2=A0 =C2=A0 =C2=A0>=C2=A0
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0 =C2=A0return fd;
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 err_fd:
>
>=C2=A0 =C2=A0 =C2=A0This kind of thing is usually an indicator of a bug= . E.g.
>=C2=A0 =C2=A0 =C2=A0if the refcount drops to 0 on module_put(THIS_MODUL= E) it
>=C2=A0 =C2=A0 =C2=A0will be unloaded and the following return will not = run.
>
>
>
> Should this happen?=C2=A0 The refcount should be only decreased to 0 a= fter the
> misc_device is closed?
>
> Thanks,
> Yongji
>

OTOH if it never drops to 0 anyway then why do you need to increase it?


To prevent unloading the module in the= case that the device is created, but no user process using it (e.g. the us= er process crashed).=C2=A0

Thanks,
Yongj= i

>
>
>=C2=A0 =C2=A0 =C2=A0> --
>=C2=A0 =C2=A0 =C2=A0> 2.25.1
>
>

--00000000000055b6a005b20829a6--