From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> To: "Michael S. Tsirkin" <mst@redhat.com>, Wei Wang <wei.w.wang@intel.com> Cc: virtio-dev@lists.oasis-open.org, linux-kernel@vger.kernel.org, virtualization@lists.linux-foundation.org, kvm@vger.kernel.org, linux-mm@kvack.org, mhocko@kernel.org, akpm@linux-foundation.org, pbonzini@redhat.com, liliang.opensource@gmail.com, yang.zhang.wz@gmail.com, quan.xu0@gmail.com, nilal@redhat.com, riel@redhat.com Subject: Re: [PATCH v22 2/3] virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_VQ Date: Thu, 18 Jan 2018 22:30:18 +0900 [thread overview] Message-ID: <2bb0e3d9-1679-9ad3-b402-f0781f6cf094@I-love.SAKURA.ne.jp> (raw) In-Reply-To: <20180117180337-mutt-send-email-mst@kernel.org> On 2018/01/18 1:44, Michael S. Tsirkin wrote: >> +static void add_one_sg(struct virtqueue *vq, unsigned long pfn, uint32_t len) >> +{ >> + struct scatterlist sg; >> + unsigned int unused; >> + int err; >> + >> + sg_init_table(&sg, 1); >> + sg_set_page(&sg, pfn_to_page(pfn), len, 0); >> + >> + /* Detach all the used buffers from the vq */ >> + while (virtqueue_get_buf(vq, &unused)) >> + ; >> + >> + /* >> + * Since this is an optimization feature, losing a couple of free >> + * pages to report isn't important. >> We simply resturn > > return > >> without adding >> + * the page if the vq is full. We are adding one entry each time, >> + * which essentially results in no memory allocation, so the >> + * GFP_KERNEL flag below can be ignored. >> + */ >> + if (vq->num_free) { >> + err = virtqueue_add_inbuf(vq, &sg, 1, vq, GFP_KERNEL); > > Should we kick here? At least when ring is close to > being full. Kick at half way full? > Otherwise it's unlikely ring will > ever be cleaned until we finish the scan. Since this add_one_sg() is called between spin_lock_irqsave(&zone->lock, flags) and spin_unlock_irqrestore(&zone->lock, flags), it is not permitted to sleep. And walk_free_mem_block() is not ready to handle resume. By the way, specifying GFP_KERNEL here is confusing even though it is never used. walk_free_mem_block() says: * The callback itself must not sleep or perform any operations which would * require any memory allocations directly (not even GFP_NOWAIT/GFP_ATOMIC) * or via any lock dependency. > >> + /* >> + * This is expected to never fail, because there is always an >> + * entry available on the vq. >> + */ >> + BUG_ON(err); >> + } >> +}
WARNING: multiple messages have this Message-ID (diff)
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> To: "Michael S. Tsirkin" <mst@redhat.com>, Wei Wang <wei.w.wang@intel.com> Cc: virtio-dev@lists.oasis-open.org, linux-kernel@vger.kernel.org, virtualization@lists.linux-foundation.org, kvm@vger.kernel.org, linux-mm@kvack.org, mhocko@kernel.org, akpm@linux-foundation.org, pbonzini@redhat.com, liliang.opensource@gmail.com, yang.zhang.wz@gmail.com, quan.xu0@gmail.com, nilal@redhat.com, riel@redhat.com Subject: Re: [PATCH v22 2/3] virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_VQ Date: Thu, 18 Jan 2018 22:30:18 +0900 [thread overview] Message-ID: <2bb0e3d9-1679-9ad3-b402-f0781f6cf094@I-love.SAKURA.ne.jp> (raw) In-Reply-To: <20180117180337-mutt-send-email-mst@kernel.org> On 2018/01/18 1:44, Michael S. Tsirkin wrote: >> +static void add_one_sg(struct virtqueue *vq, unsigned long pfn, uint32_t len) >> +{ >> + struct scatterlist sg; >> + unsigned int unused; >> + int err; >> + >> + sg_init_table(&sg, 1); >> + sg_set_page(&sg, pfn_to_page(pfn), len, 0); >> + >> + /* Detach all the used buffers from the vq */ >> + while (virtqueue_get_buf(vq, &unused)) >> + ; >> + >> + /* >> + * Since this is an optimization feature, losing a couple of free >> + * pages to report isn't important. >> We simply resturn > > return > >> without adding >> + * the page if the vq is full. We are adding one entry each time, >> + * which essentially results in no memory allocation, so the >> + * GFP_KERNEL flag below can be ignored. >> + */ >> + if (vq->num_free) { >> + err = virtqueue_add_inbuf(vq, &sg, 1, vq, GFP_KERNEL); > > Should we kick here? At least when ring is close to > being full. Kick at half way full? > Otherwise it's unlikely ring will > ever be cleaned until we finish the scan. Since this add_one_sg() is called between spin_lock_irqsave(&zone->lock, flags) and spin_unlock_irqrestore(&zone->lock, flags), it is not permitted to sleep. And walk_free_mem_block() is not ready to handle resume. By the way, specifying GFP_KERNEL here is confusing even though it is never used. walk_free_mem_block() says: * The callback itself must not sleep or perform any operations which would * require any memory allocations directly (not even GFP_NOWAIT/GFP_ATOMIC) * or via any lock dependency. > >> + /* >> + * This is expected to never fail, because there is always an >> + * entry available on the vq. >> + */ >> + BUG_ON(err); >> + } >> +} -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2018-01-18 13:32 UTC|newest] Thread overview: 83+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-01-17 5:10 [PATCH v22 0/3] Virtio-balloon: support free page reporting Wei Wang 2018-01-17 5:10 ` [virtio-dev] " Wei Wang 2018-01-17 5:10 ` Wei Wang 2018-01-17 5:10 ` [PATCH v22 1/3] mm: support reporting free page blocks Wei Wang 2018-01-17 5:10 ` Wei Wang 2018-01-17 5:10 ` [virtio-dev] " Wei Wang 2018-01-17 5:10 ` Wei Wang 2018-01-17 5:10 ` [PATCH v22 2/3] virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_VQ Wei Wang 2018-01-17 5:10 ` Wei Wang 2018-01-17 5:10 ` [virtio-dev] " Wei Wang 2018-01-17 5:10 ` Wei Wang 2018-01-17 8:21 ` Pankaj Gupta 2018-01-17 8:21 ` Pankaj Gupta 2018-01-17 9:00 ` Wei Wang 2018-01-17 9:00 ` [virtio-dev] " Wei Wang 2018-01-17 9:00 ` Wei Wang 2018-01-17 9:27 ` Pankaj Gupta 2018-01-17 9:27 ` Pankaj Gupta 2018-01-17 9:27 ` Pankaj Gupta 2018-01-17 10:47 ` Wei Wang 2018-01-17 10:47 ` Wei Wang 2018-01-17 10:47 ` [virtio-dev] " Wei Wang 2018-01-17 10:47 ` Wei Wang 2018-01-17 9:00 ` Wei Wang 2018-01-17 8:21 ` Pankaj Gupta 2018-01-17 16:44 ` Michael S. Tsirkin 2018-01-17 16:44 ` Michael S. Tsirkin 2018-01-17 16:44 ` [virtio-dev] " Michael S. Tsirkin 2018-01-17 16:44 ` Michael S. Tsirkin 2018-01-18 13:30 ` Tetsuo Handa [this message] 2018-01-18 13:30 ` Tetsuo Handa 2018-01-18 19:09 ` Michael S. Tsirkin 2018-01-18 19:09 ` [virtio-dev] " Michael S. Tsirkin 2018-01-18 19:09 ` Michael S. Tsirkin 2018-01-18 19:09 ` Michael S. Tsirkin 2018-01-18 21:11 ` Tetsuo Handa 2018-01-18 21:11 ` Tetsuo Handa 2018-01-18 22:32 ` Michael S. Tsirkin 2018-01-18 22:32 ` [virtio-dev] " Michael S. Tsirkin 2018-01-18 22:32 ` Michael S. Tsirkin 2018-01-18 22:32 ` Michael S. Tsirkin 2018-01-20 14:23 ` Tetsuo Handa 2018-01-20 14:23 ` Tetsuo Handa 2018-01-19 3:44 ` Wei Wang 2018-01-19 3:44 ` [virtio-dev] " Wei Wang 2018-01-19 3:44 ` Wei Wang 2018-01-19 12:39 ` Michael S. Tsirkin 2018-01-19 12:39 ` [virtio-dev] " Michael S. Tsirkin 2018-01-19 12:39 ` Michael S. Tsirkin 2018-01-22 11:25 ` [virtio-dev] " Wei Wang 2018-01-22 11:25 ` Wei Wang 2018-01-22 11:25 ` Wei Wang 2018-01-24 3:18 ` Wei Wang 2018-01-24 3:18 ` Wei Wang 2018-01-24 3:18 ` Wei Wang 2018-01-24 3:18 ` Wei Wang 2018-01-24 4:31 ` Michael S. Tsirkin 2018-01-24 4:31 ` Michael S. Tsirkin 2018-01-24 4:31 ` Michael S. Tsirkin 2018-01-24 4:31 ` Michael S. Tsirkin 2018-01-24 4:29 ` Michael S. Tsirkin 2018-01-24 4:29 ` Michael S. Tsirkin 2018-01-24 4:29 ` Michael S. Tsirkin 2018-01-24 11:28 ` Wei Wang 2018-01-24 11:28 ` Wei Wang 2018-01-24 11:28 ` Wei Wang 2018-01-24 11:28 ` Wei Wang 2018-01-24 4:29 ` Michael S. Tsirkin 2018-01-22 11:25 ` Wei Wang 2018-01-19 12:39 ` Michael S. Tsirkin 2018-01-19 3:44 ` Wei Wang 2018-01-19 6:24 ` Wei Wang 2018-01-19 6:24 ` [virtio-dev] " Wei Wang 2018-01-19 6:24 ` Wei Wang 2018-01-19 6:24 ` Wei Wang 2018-01-17 5:10 ` [PATCH v22 3/3] virtio-balloon: don't report free pages when page poisoning is enabled Wei Wang 2018-01-17 5:10 ` Wei Wang 2018-01-17 5:10 ` [virtio-dev] " Wei Wang 2018-01-17 5:10 ` Wei Wang 2018-01-18 22:37 ` Michael S. Tsirkin 2018-01-18 22:37 ` [virtio-dev] " Michael S. Tsirkin 2018-01-18 22:37 ` Michael S. Tsirkin 2018-01-18 22:37 ` Michael S. Tsirkin
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=2bb0e3d9-1679-9ad3-b402-f0781f6cf094@I-love.SAKURA.ne.jp \ --to=penguin-kernel@i-love.sakura.ne.jp \ --cc=akpm@linux-foundation.org \ --cc=kvm@vger.kernel.org \ --cc=liliang.opensource@gmail.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=mhocko@kernel.org \ --cc=mst@redhat.com \ --cc=nilal@redhat.com \ --cc=pbonzini@redhat.com \ --cc=quan.xu0@gmail.com \ --cc=riel@redhat.com \ --cc=virtio-dev@lists.oasis-open.org \ --cc=virtualization@lists.linux-foundation.org \ --cc=wei.w.wang@intel.com \ --cc=yang.zhang.wz@gmail.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.