Re: [PATCH v4 2/2] virtio_balloon: Use a workqueue instead of "vballoon" kthread

2016-01-03 Thread Tejun Heo
Hello, Michael. On Sat, Jan 02, 2016 at 11:36:03PM +0200, Michael S. Tsirkin wrote: > > Why so? As long as the maximum concurrently used workers are not > > high, 1/5 second or even a lot longer sleeps are completely fine. > > I always thought the right way to defer executing a work queue item

Re: [PATCH v4 2/2] virtio_balloon: Use a workqueue instead of "vballoon" kthread

2016-01-02 Thread Tejun Heo
Hello, On Fri, Jan 01, 2016 at 12:18:17PM +0200, Michael S. Tsirkin wrote: > > My initial idea was to use a dedicated workqueue. Michael S. Tsirkin > > suggested using a system one. Tejun Heo confirmed that the system > > workqueue has a pretty high concurrency leve

Re: DEFINE_IDA causing memory leaks? (was Re: [PATCH 1/2] virtio: fix memory leak of virtio ida cache layers)

2015-09-17 Thread Tejun Heo
Hello, On Thu, Sep 17, 2015 at 07:15:44AM -0700, James Bottomley wrote: > I don't understand why you'd want to forbid DEFINE_IDA ... all it does I guess to require the use of explicit init / creation so that it's clear the data structure needs to be destroyed? > is pre-initialise a usually

Re: DEFINE_IDA causing memory leaks? (was Re: [PATCH 1/2] virtio: fix memory leak of virtio ida cache layers)

2015-09-17 Thread Tejun Heo
Hello, On Thu, Sep 17, 2015 at 09:48:37AM -0700, James Bottomley wrote: > Well, there's an easy fix for that. We could have ida_remove() actually > free the bitmap and not cache it if it's the last layer. That way ida > would naturally empty and we wouldn't need a destructor. Tejun, would >

Re: DEFINE_IDA causing memory leaks? (was Re: [PATCH 1/2] virtio: fix memory leak of virtio ida cache layers)

2015-09-17 Thread Tejun Heo
Hello, James. On Thu, Sep 17, 2015 at 10:58:29AM -0700, James Bottomley wrote: > The argument is that we shouldn't have to explicitly destroy a > statically initialized object, so > > DEFINE_IDA(someida); > > Should just work without having to explicitly do > > ida_destory(someida); > >

Re: [PATCH v3] virtio_balloon: Convert vballoon kthread into a workqueue

2014-11-20 Thread Tejun Heo
On Thu, Nov 20, 2014 at 05:03:17PM +0100, Petr Mladek wrote: ... @@ -476,7 +460,6 @@ static void virtballoon_remove(struct virtio_device *vdev) { struct virtio_balloon *vb = vdev-priv; - kthread_stop(vb-thread); remove_common(vb); kfree(vb); } Shouldn't the work

Re: [PATCH v3] virtio_balloon: Convert vballoon kthread into a workqueue

2014-11-20 Thread Tejun Heo
On Thu, Nov 20, 2014 at 06:26:24PM +0200, Michael S. Tsirkin wrote: On Thu, Nov 20, 2014 at 06:25:43PM +0200, Michael S. Tsirkin wrote: On Thu, Nov 20, 2014 at 11:07:46AM -0500, Tejun Heo wrote: On Thu, Nov 20, 2014 at 05:03:17PM +0100, Petr Mladek wrote: ... @@ -476,7 +460,6

Re: [PATCH v3] virtio_balloon: Convert vballoon kthread into a workqueue

2014-11-20 Thread Tejun Heo
On Thu, Nov 20, 2014 at 06:47:11PM +0200, Michael S. Tsirkin wrote: There's cancel_work_sync() to stop the self-requeueing ones. What happens if queue_work runs while cancel_work_sync is in progress? Does it fail to queue? cancel_work_sync() is guaranteed to take self-requeueing work items

Re: [PATCH v3] virtio_balloon: Convert vballoon kthread into a workqueue

2014-11-20 Thread Tejun Heo
On Thu, Nov 20, 2014 at 11:49:33AM -0500, Tejun Heo wrote: On Thu, Nov 20, 2014 at 06:47:11PM +0200, Michael S. Tsirkin wrote: There's cancel_work_sync() to stop the self-requeueing ones. What happens if queue_work runs while cancel_work_sync is in progress? Does it fail to queue

Re: [PATCH v2] virtio_balloon: Convert vballon kthread into a workqueue

2014-11-14 Thread Tejun Heo
Hello, Michael, Petr. On Wed, Nov 12, 2014 at 03:32:04PM +0200, Michael S. Tsirkin wrote: + /* The workqueue servicing the balloon. */ + struct workqueue_struct *wq; + struct work_struct wq_work; We could use system_freezable_wq instead. I do agree a dedicated wq is better since

Re: [PATCH] virtio_balloon: Convert vballon kthread into a workqueue

2014-09-28 Thread Tejun Heo
On Thu, Sep 25, 2014 at 06:20:41PM +0200, Petr Mladek wrote: ... 1st create_freezable_workqueue(vballoon_wq); All create_*workqueue() interfaces are deprecated. Please don't create new usages. They map to alloc_*workqueue() anyway. 2nd alloc_workqueue(vballoon_wq, WQ_FREEZABLE |

Re: [PATCH driver-core-linus] kernfs: kernfs_notify() must be useable from non-sleepable contexts

2014-07-02 Thread Tejun Heo
Hello, On Tue, Jul 01, 2014 at 01:51:48PM -0700, Greg Kroah-Hartman wrote: Looks good to me, do you want to take this with your other kernfs patches for 3.16-final? Or if you don't have that, I can take it through my tree, it's your choice, either is fine for me. If you want it in your

[PATCH driver-core-linus] kernfs: kernfs_notify() must be useable from non-sleepable contexts

2014-07-01 Thread Tejun Heo
to kernfs_elem_attr enlarging kernfs_node by a pointer, which is not ideal but not a very big deal either. If this turns out to be an actual issue, we can move kernfs_elem_attr-size to kernfs_node-iattr later. Signed-off-by: Tejun Heo t...@kernel.org Reported-by: Josh Boyer jwbo...@fedoraproject.org Cc

Re: virt_blk BUG: sleeping function called from invalid context

2014-06-30 Thread Tejun Heo
On Sun, Jun 29, 2014 at 02:55:36PM -0600, Jens Axboe wrote: commit d911d98748018f7c8facc035ba39c30f5cce6f9c Author: Tejun Heo t...@kernel.org Date: Wed Apr 9 11:07:31 2014 -0400 kernfs: make kernfs_notify() trigger inotify events too Tejun, what do you think? Josh, Brian

Re: [PATCH 1/3] block: Introduce __blk_segment_map_sg() helper

2012-06-19 Thread Tejun Heo
Hello, On Mon, Jun 18, 2012 at 7:02 PM, Asias He as...@redhat.com wrote: I *hope* this is a bit prettier.  e.g. Do we really need to pass in @sglist and keep using goto new_segment? I think this deserves another patch on top of this splitting one. I'd like to clean this up later. Yeap,

Re: [PATCH 3/3] virtio-blk: Add bio-based IO path for virtio-blk

2012-06-18 Thread Tejun Heo
Hello, guys. On Mon, Jun 18, 2012 at 07:35:22PM +0930, Rusty Russell wrote: On Mon, 18 Jun 2012 16:03:23 +0800, Asias He as...@redhat.com wrote: On 06/18/2012 03:46 PM, Rusty Russell wrote: On Mon, 18 Jun 2012 14:53:10 +0800, Asias He as...@redhat.com wrote: This patch introduces

Re: [PATCH 1/3] block: Introduce __blk_segment_map_sg() helper

2012-06-18 Thread Tejun Heo
Hello, Asias. On Mon, Jun 18, 2012 at 02:53:08PM +0800, Asias He wrote: Split the mapping code in blk_rq_map_sg() to a helper __blk_segment_map_sg(), so that other mapping function, e.g. blk_bio_map_sg(), can share the code. Cc: Jens Axboe ax...@kernel.dk Cc: Tejun Heo t...@kernel.org Cc

Re: [PATCH RFC 1/2] block: Add blk_bio_map_sg() helper

2012-06-13 Thread Tejun Heo
On Wed, Jun 13, 2012 at 03:41:46PM +0800, Asias He wrote: Add a helper to map a bio to a scatterlist, modelled after blk_rq_map_sg. This helper is useful for any driver that wants to create a scatterlist from its -make_request_fn method. This may not be possible but I really wanna avoid

Re: [RFC PATCH 1/5] block: Introduce q-abort_queue_fn()

2012-05-21 Thread Tejun Heo
On Mon, May 21, 2012 at 05:08:29PM +0800, Asias He wrote: When user hot-unplug a disk which is busy serving I/O, __blk_run_queue might be unable to drain all the requests. As a result, the blk_drain_queue() would loop forever and blk_cleanup_queue would not return. So hot-unplug will fail.

Re: [PATCH] virtio_blk: Add help function to format mass of disks

2012-04-10 Thread Tejun Heo
Hello, guys. On Tue, Apr 10, 2012 at 04:34:06PM +0300, Michael S. Tsirkin wrote: Why not use 'base' below? neither unit nor base change. Yes it's a bit strange, it was the same in Tejun's patch. Tejun, any idea? It was years ago, so I don't recall much. I think I wanted to use a variable

Re: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming

2012-04-02 Thread Tejun Heo
Hello, On Mon, Apr 02, 2012 at 10:20:09AM +0300, Michael S. Tsirkin wrote: Pleae don't rename virtio disks, it is way too late for that: virtio block driver was merged around 2007, it is not new by any measure, and there are many systems out there using the current naming scheme. There's no

Re: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming

2012-04-02 Thread Tejun Heo
Hello, James. On Mon, Apr 02, 2012 at 11:56:18AM -0700, James Bottomley wrote: So if we're agreed no other devices going forwards should ever use this interface, is there any point unifying the interface? No matter how many caveats you hedge it round with, putting the API in a central place

Re: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming

2012-03-30 Thread Tejun Heo
On Fri, Mar 30, 2012 at 05:53:52PM +0800, Ren Mingxin wrote: The current virtblk's naming algorithm only supports 263 disks. If there are mass of virtblks(exceeding 263), there will be disks with the same name. By renaming sd_format_disk_name() to disk_name_format() and moving it into

Re: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming

2012-03-30 Thread Tejun Heo
On Fri, Mar 30, 2012 at 08:26:06AM -0700, Tejun Heo wrote: On Fri, Mar 30, 2012 at 05:53:52PM +0800, Ren Mingxin wrote: The current virtblk's naming algorithm only supports 263 disks. If there are mass of virtblks(exceeding 263), there will be disks with the same name. By renaming

Re: [PATCH 4/4] virtio_blk: use disk_name_format() to support mass of disks naming

2012-03-30 Thread Tejun Heo
On Fri, Mar 30, 2012 at 08:26:06AM -0700, Tejun Heo wrote: On Fri, Mar 30, 2012 at 05:53:52PM +0800, Ren Mingxin wrote: The current virtblk's naming algorithm only supports 263 disks. If there are mass of virtblks(exceeding 263), there will be disks with the same name. By renaming

Re: [PATCH RFC] virtio_net: fix refill related races

2011-12-20 Thread Tejun Heo
Hello, Michael. On Tue, Dec 20, 2011 at 09:09:08PM +0200, Michael S. Tsirkin wrote: Another question, wanted to make sure: virtnet_poll does schedule_delayed_work(vi-refill, 0); separately refill work itself also does schedule_delayed_work(vi-refill, HZ/2); If two such events happen twice,

Re: [PATCH RFC] virtio_net: fix refill related races

2011-12-20 Thread Tejun Heo
On Tue, Dec 20, 2011 at 09:30:55PM +0200, Michael S. Tsirkin wrote: Hmm, in that case it looks like a nasty race could get triggered, with try_fill_recv run on multiple CPUs in parallel, corrupting the linked list within the vq. Using the mutex as my patch did will fix that naturally, as

Re: [PATCH RFC] virtio_net: fix refill related races

2011-12-14 Thread Tejun Heo
Hello, Rusty. On Tue, Dec 13, 2011 at 01:05:11PM +1030, Rusty Russell wrote: Both places where we call: cancel_delayed_work_sync(vi-refill); Do not actually guarantee that vi-refill isn't running, because it can requeue itself. A 'bool no_more_refill' field seems like the simplest

Re: [PATCH 1/1] [virt] virtio-blk: Use ida to allocate disk index

2011-06-16 Thread Tejun Heo
Hello, On Thu, Jun 16, 2011 at 09:35:34AM +0930, Rusty Russell wrote: On Wed, 15 Jun 2011 09:06:38 +0200, Tejun Heo t...@kernel.org wrote: It's inherited from idr which was designed to have separate prepare/allocation stages so that allocation can happen inside an outer spinlock

Re: [PATCH 1/1] [virt] virtio-blk: Use ida to allocate disk index

2011-06-15 Thread Tejun Heo
Hello, On Wed, Jun 15, 2011 at 02:21:51PM +0930, Rusty Russell wrote: + if (index_to_minor(index) = 1 MINORBITS) { + err = -ENOSPC; + goto out_free_index; + } Is this *really* how this is supposed to be used? Tejun, this is your code. What do you think of

Re: [PATCH 1/1] [virt] virtio-blk: Use ida to allocate disk index

2011-06-09 Thread Tejun Heo
for safety. What's your opinion? commit 4034cc68157bfa0b6622efe368488d3d3e20f4e6 Author: Tejun Heo t...@kernel.org Date: Sat Feb 21 11:04:45 2009 +0900 [SCSI] sd: revive sd_index_lock Commit f27bac2761cab5a2e212dea602d22457a9aa6943 which converted sd to use ida

Re: [PATCH] vhost: locking/rcu cleanup

2010-08-02 Thread Tejun Heo
Hello, On 07/29/2010 02:23 PM, Michael S. Tsirkin wrote: I saw WARN_ON(!list_empty(dev-work_list)) trigger so our custom flush is not as airtight as need be. Could be but it's also possible that something has queued something after the last flush? Is the problem reproducible? This patch

Re: [PATCH 3/3] Makes lguest's irq handler typesafe

2008-01-18 Thread Tejun Heo
Rusty Russell wrote: Just a trivial example. --- drivers/lguest/lguest_device.c |3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff -r 00ab7672f658 drivers/lguest/lguest_device.c --- a/drivers/lguest/lguest_device.c Thu Jan 17 16:54:00 2008 +1100 +++

Re: [PATCH 3/3] Makes lguest's irq handler typesafe

2008-01-18 Thread Tejun Heo
Tejun Heo wrote: so I think the question is do we want to change all callbacks to take native pointer type instead of void pointer?. Lemme clarity myself a bit. I'm not saying that we should convert all at once or literally every callback should be converted. What I'm saying is whether we're

Re: [PATCH 3/3] Makes lguest's irq handler typesafe

2008-01-18 Thread Tejun Heo
Hello, Rusty Russell wrote: There are three possibilities: (1) force everyone to use void *, (2) force everyone to be type-correct, (3) allow both with some tricks. Currently we're on (1). For kthread, with only dozens of users, I chose (2) (very simple, easy to understand). I