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 C8162C433DF for ; Tue, 20 Oct 2020 07:42:44 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id EC09E2224F for ; Tue, 20 Oct 2020 07:42:43 +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="nB7eN8wZ" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org EC09E2224F 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 585456B005C; Tue, 20 Oct 2020 03:42:43 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 537846B0062; Tue, 20 Oct 2020 03:42:43 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3FD9B6B0068; Tue, 20 Oct 2020 03:42:43 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0191.hostedemail.com [216.40.44.191]) by kanga.kvack.org (Postfix) with ESMTP id 10D5B6B005C for ; Tue, 20 Oct 2020 03:42:43 -0400 (EDT) Received: from smtpin05.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 94267180AD807 for ; Tue, 20 Oct 2020 07:42:42 +0000 (UTC) X-FDA: 77391511764.05.spot68_4f053422723d Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin05.hostedemail.com (Postfix) with ESMTP id 770CC1801E8EE for ; Tue, 20 Oct 2020 07:42:42 +0000 (UTC) X-HE-Tag: spot68_4f053422723d X-Filterd-Recvd-Size: 12300 Received: from mail-ej1-f67.google.com (mail-ej1-f67.google.com [209.85.218.67]) by imf12.hostedemail.com (Postfix) with ESMTP for ; Tue, 20 Oct 2020 07:42:41 +0000 (UTC) Received: by mail-ej1-f67.google.com with SMTP id u8so1260705ejg.1 for ; Tue, 20 Oct 2020 00:42:41 -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=bw3rA0lCoZzKIG9o5rB644z6DTO7wiylR78cJVmm5Xg=; b=nB7eN8wZXncDhA3MOqANEqVkrZlY2648hCz6LAg/kXZOily8JvxNUTZaNv7xM6oHAo TOIhkQtN28c0ekwhA1zlGJpyNi1cP1MCybfqEglXNK2w9c2N6TwXB+7fWW/L49Bjb9Vg T6kgX1YR9ruvJAUFk9bJc1Mh1Sm/c+I+KuawppEdhK00pv/nOBLMvR9mOmbbgrUvM/k/ ct/xXwl7/waE7FiehBYw0k+tCkxz0tcuBC+gLC0EOr11V44oV2Qo6KYiAQuzCXoOy0tp hSHJF9begK8q5MM4NJGT0D02N0ebz0k3huF523v6ua+ngRA9a0kceYN8r/JKDEfvo2Hs rgLg== 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=bw3rA0lCoZzKIG9o5rB644z6DTO7wiylR78cJVmm5Xg=; b=r9CRG49UJFBiblFix/i7hFJziFzCSw9ZTMGrhEtnEQIymng/e2YaJTKucmyK9QO673 o9q0T6ajhmS7wmxAbbqDZb38LG2d/dq3CIicmfzPFoDMBvPnXvOC+/T4ujRPlMv3pp4u ogQxmrHYShPIR8xDhJ4M88Fpe7k+UijXZunUxTgRvelJhHW8zZoE3+hllY9sA0kfuXm4 FAQRNUUiJvGPK3FOHIeU8xGxTzgkfBVJQ+1pvfOaXUrIHACxpUlyudQ5RWsgL9iZYLOv EDd+8RV2ag2jJzrDcrtE+nTNDAZudjvYCmGNpgwLx7HxoYXkzvmI96b5pndSiIFLzQh8 Lt7Q== X-Gm-Message-State: AOAM531cl8SnKfC7UvWeFVvMqXWyfWF3LOAHaHwg2c06HE+EyZw9ELu6 Nlk1g+I0cd/AS4I05ArfbT62U2UkCzA/pyZElSJA X-Google-Smtp-Source: ABdhPJx1XySyF14JNE6JHLeQHi/f/8aUvrcoa9GhimKlahg5sMKnSFbIXRTm9m9sozZVgPlS6F6z593JK8+vJN1GGIU= X-Received: by 2002:a17:907:2089:: with SMTP id pv9mr1884029ejb.427.1603179760457; Tue, 20 Oct 2020 00:42:40 -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> <20201019123835-mutt-send-email-mst@kernel.org> In-Reply-To: <20201019123835-mutt-send-email-mst@kernel.org> From: Yongji Xie Date: Tue, 20 Oct 2020 15:42:29 +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: Jason Wang , akpm@linux-foundation.org, linux-mm@kvack.org, virtualization@lists.linux-foundation.org Content-Type: multipart/alternative; boundary="0000000000002255fa05b215605d" 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: --0000000000002255fa05b215605d Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Tue, Oct 20, 2020 at 12:41 AM Michael S. Tsirkin wrote: > On Mon, Oct 19, 2020 at 11:56:35PM +0800, =E8=B0=A2=E6=B0=B8=E5=90=89 wro= te: > > > > > > > > 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 wrote: > > > > > > > > > On Mon, Oct 19, 2020 at 11:05 PM Michael S. Tsirkin < > mst@redhat.com> > > wrote: > > > > > > On Mon, Oct 19, 2020 at 10:56:22PM +0800, Xie Yongji wrote: > > > > The module should not be unloaded if any vduse device exist= s. > > > > So increase the module's reference count when creating vdus= e > > > > 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 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 user process crashed). > > > > Thanks, > > Yongji > > Looks like it can drop to 0 if that is the case then? > > Could you give me an example? In my understanding, vduse_create_dev() should be called only after we open the chardev which will grab the module's reference. Thanks, Yongji --0000000000002255fa05b215605d Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Tue, Oct 20, 2020 at 12:41 AM Mich= ael S. Tsirkin <mst@redhat.com>= wrote:
On Mon, = Oct 19, 2020 at 11:56:35PM +0800, =E8=B0=A2=E6=B0=B8=E5=90=89 wrote:
>
>
>
> On Mon, Oct 19, 2020 at 11:47 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
>=C2=A0 =C2=A0 =C2=A0On Mon, Oct 19, 2020 at 11:44:36PM +0800, =E8=B0=A2= =E6=B0=B8=E5=90=89 wrote:
>=C2=A0 =C2=A0 =C2=A0>
>=C2=A0 =C2=A0 =C2=A0>
>=C2=A0 =C2=A0 =C2=A0> On Mon, Oct 19, 2020 at 11:05 PM Michael S. Ts= irkin <mst@redhat.co= m>
>=C2=A0 =C2=A0 =C2=A0wrote:
>=C2=A0 =C2=A0 =C2=A0>
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0On Mon, Oct 19, 2020 at 10:= 56:22PM +0800, Xie Yongji wrote:
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0> The module should not = be unloaded if any vduse device exists.
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0> So increase the module= 's reference count when creating vduse
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0> device. And the refere= nce count is kept until the device is
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0> destroyed.
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0>
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0> Signed-off-by: Xie Yon= gji <xieyon= gji@bytedance.com>
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0> ---
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0>=C2=A0 drivers/vdpa/vdp= a_user/vduse_dev.c | 2 ++
>=C2=A0 =C2=A0 =C2=A0>=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>
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0> diff --git a/drivers/v= dpa/vdpa_user/vduse_dev.c b/drivers/vdpa/
>=C2=A0 =C2=A0 =C2=A0vdpa_user/
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0vduse_dev.c
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0> index 6787ba66725c..f0= 4aa02de8c1 100644
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0> --- a/drivers/vdpa/vdp= a_user/vduse_dev.c
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0> +++ b/drivers/vdpa/vdp= a_user/vduse_dev.c
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0> @@ -887,6 +887,7 @@ st= atic int vduse_destroy_dev(u32 id)
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0 = =C2=A0kfree(dev->vqs);
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0 = =C2=A0vduse_iova_domain_destroy(dev->domain);
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0 = =C2=A0vduse_dev_destroy(dev);
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0> +=C2=A0 =C2=A0 =C2=A0m= odule_put(THIS_MODULE);
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0>=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 }
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0> @@ -931,6 +932,7 @@ st= atic int vduse_create_dev(struct
>=C2=A0 =C2=A0 =C2=A0vduse_dev_config
>=C2=A0 =C2=A0 =C2=A0>=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=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=A0 =C2=A0 =C2=A0 = =C2=A0list_add(&dev->list, &vduse_devs);
>=C2=A0 =C2=A0 =C2=A0>=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=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0 = =C2=A0return fd;
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0>=C2=A0 err_fd:
>=C2=A0 =C2=A0 =C2=A0>
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0This kind of thing is usual= ly an indicator of a bug. E.g.
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0if the refcount drops to 0 = on module_put(THIS_MODULE) it
>=C2=A0 =C2=A0 =C2=A0>=C2=A0 =C2=A0 =C2=A0will be unloaded and the fo= llowing return will not run.
>=C2=A0 =C2=A0 =C2=A0>
>=C2=A0 =C2=A0 =C2=A0>
>=C2=A0 =C2=A0 =C2=A0>
>=C2=A0 =C2=A0 =C2=A0> Should this happen?=C2=A0 The refcount should = be only decreased to 0 after the
>=C2=A0 =C2=A0 =C2=A0> misc_device is closed?
>=C2=A0 =C2=A0 =C2=A0>
>=C2=A0 =C2=A0 =C2=A0> Thanks,
>=C2=A0 =C2=A0 =C2=A0> Yongji
>=C2=A0 =C2=A0 =C2=A0>
>
>=C2=A0 =C2=A0 =C2=A0OTOH 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).=C2=A0
>
> Thanks,
> Yongji

Looks like it can drop to 0 if that is the case then?

Could you give me an example?=C2=A0 In my understandi= ng, vduse_create_dev() should be called only after we open the chardev whic= h will grab the module's reference.

Thanks,
Yongji
--0000000000002255fa05b215605d--