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

2011-10-24 Thread Rusty Russell
On Mon, 24 Oct 2011 12:02:18 +0200, Jens Axboe wrote: > On 2011-10-24 12:02, Michael S. Tsirkin wrote: > > On Wed, Oct 19, 2011 at 12:12:20PM +0200, Michael S. Tsirkin wrote: > > Rusty, any opinion on merging this for 3.2? > > I expect merge window will open right after the summit, > > I can toss

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

2011-10-24 Thread Jens Axboe
On 2011-10-24 12:02, Michael S. Tsirkin wrote: > On Wed, Oct 19, 2011 at 12:12:20PM +0200, Michael S. Tsirkin wrote: >> On Thu, Jun 09, 2011 at 06:41:56AM -0400, Mark Wu wrote: >>> On 06/09/2011 05:14 AM, Tejun Heo wrote: Hello, On Thu, Jun 09, 2011 at 08:51:05AM +0930, Rusty Russell

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

2011-10-24 Thread Michael S. Tsirkin
On Wed, Oct 19, 2011 at 12:12:20PM +0200, Michael S. Tsirkin wrote: > On Thu, Jun 09, 2011 at 06:41:56AM -0400, Mark Wu wrote: > > On 06/09/2011 05:14 AM, Tejun Heo wrote: > > > Hello, > > > > > > On Thu, Jun 09, 2011 at 08:51:05AM +0930, Rusty Russell wrote: > > >> On Wed, 08 Jun 2011 09:08:29 -0

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

2011-10-19 Thread Michael S. Tsirkin
On Thu, Jun 09, 2011 at 06:41:56AM -0400, Mark Wu wrote: > On 06/09/2011 05:14 AM, Tejun Heo wrote: > > Hello, > > > > On Thu, Jun 09, 2011 at 08:51:05AM +0930, Rusty Russell wrote: > >> On Wed, 08 Jun 2011 09:08:29 -0400, Mark Wu wrote: > >>> Hi Rusty, > >>> Yes, I can't figure out an instance o

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 wrote: > > It's inherited from idr which was designed to have separate > > prepare/allocation stages so that allocation can happen inside an > > outer spinlock. It doesn't have t

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

2011-06-15 Thread Rusty Russell
On Wed, 15 Jun 2011 09:06:38 +0200, Tejun Heo wrote: > It's inherited from idr which was designed to have separate > prepare/allocation stages so that allocation can happen inside an > outer spinlock. It doesn't have too much to do with optimization. > It's mostly to be able to use sleepable cont

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

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

2011-06-14 Thread Rusty Russell
> Since virtio blk driver doesn't use async probe, it needn't use spinlock to > protect ida. > So remove the lock from patch. OK, that's fine, but: > - if (index_to_minor(index) >= 1 << MINORBITS) > - return -ENOSPC; > + do { > + if (!ida_pre_get(&vd_index_ida, GFP

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

2011-06-09 Thread Mark Wu
On 06/09/2011 05:14 AM, Tejun Heo wrote: > Hello, > > On Thu, Jun 09, 2011 at 08:51:05AM +0930, Rusty Russell wrote: >> On Wed, 08 Jun 2011 09:08:29 -0400, Mark Wu wrote: >>> Hi Rusty, >>> Yes, I can't figure out an instance of disk probing in parallel either, but >>> as >>> per the following co

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

2011-06-09 Thread Tejun Heo
Hello, On Thu, Jun 09, 2011 at 08:51:05AM +0930, Rusty Russell wrote: > On Wed, 08 Jun 2011 09:08:29 -0400, Mark Wu wrote: > > Hi Rusty, > > Yes, I can't figure out an instance of disk probing in parallel either, but > > as > > per the following commit, I think we still need use lock for safety.

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

2011-06-08 Thread Greg KH
On Thu, Jun 09, 2011 at 08:51:05AM +0930, Rusty Russell wrote: > On Wed, 08 Jun 2011 09:08:29 -0400, Mark Wu wrote: > > Hi Rusty, > > Yes, I can't figure out an instance of disk probing in parallel either, but > > as > > per the following commit, I think we still need use lock for safety. What's

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

2011-06-08 Thread Rusty Russell
On Wed, 08 Jun 2011 09:08:29 -0400, Mark Wu wrote: > Hi Rusty, > Yes, I can't figure out an instance of disk probing in parallel either, but as > per the following commit, I think we still need use lock for safety. What's > your opinion? > > commit 4034cc68157bfa0b6622efe368488d3d3e20f4e6 > Auth

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

2011-06-08 Thread Mark Wu
On 06/02/2011 06:34 AM, Michael S. Tsirkin wrote: > On Wed, Jun 01, 2011 at 04:25:48AM -0400, Mark Wu wrote: >> On 06/01/2011 03:24 AM, Mark Wu wrote: >>> - if (index_to_minor(index)>= 1<< MINORBITS) >>> - return -ENOSPC; >>> + do { >>> + if (!ida_pre_get(&vd_index_ida, GFP

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

2011-06-08 Thread Mark Wu
On 06/01/2011 07:57 PM, Rusty Russell wrote: > On Wed, 1 Jun 2011 03:24:29 -0400, Mark Wu wrote: >> Current index allocation in virtio-blk is based on a monotonically >> increasing variable "index". It could cause some confusion about >> disk name in the case of hot-plugging disks. And it's imp

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

2011-06-02 Thread Michael S. Tsirkin
On Wed, Jun 01, 2011 at 03:24:29AM -0400, Mark Wu wrote: > Current index allocation in virtio-blk is based on a monotonically > increasing variable "index". It could cause some confusion about disk > name in the case of hot-plugging disks. And it's impossible to find the > lowest available index by

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

2011-06-02 Thread Michael S. Tsirkin
On Wed, Jun 01, 2011 at 04:25:48AM -0400, Mark Wu wrote: > On 06/01/2011 03:24 AM, Mark Wu wrote: > >-if (index_to_minor(index)>= 1<< MINORBITS) > >-return -ENOSPC; > >+do { > >+if (!ida_pre_get(&vd_index_ida, GFP_KERNEL)) > >+return err; > >+ >

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

2011-06-01 Thread Rusty Russell
On Wed, 1 Jun 2011 03:24:29 -0400, Mark Wu wrote: > Current index allocation in virtio-blk is based on a monotonically > increasing variable "index". It could cause some confusion about disk > name in the case of hot-plugging disks. And it's impossible to find the > lowest available index by just

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

2011-06-01 Thread Mark Wu
On 06/01/2011 03:24 AM, Mark Wu wrote: - if (index_to_minor(index)>= 1<< MINORBITS) - return -ENOSPC; + do { + if (!ida_pre_get(&vd_index_ida, GFP_KERNEL)) + return err; + There's a problem in above code: err is not initialized before