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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 08 Jun 2011 09:08:29 -0400
From:	Mark Wu <dwu@...hat.com>
To:	Rusty Russell <rusty@...tcorp.com.au>
CC:	"Michael S. Tsirkin" <mst@...hat.com>,
	virtualization@...ts.linux-foundation.org, kvm@...r.kernel.org,
	linux-kernel@...r.kernel.org, Jens Axboe <axboe@...nel.dk>,
	tj@...nel.org
Subject: Re: [PATCH 1/1] [virt] virtio-blk: Use ida to allocate disk index

On 06/01/2011 07:57 PM, Rusty Russell wrote:
> On Wed,  1 Jun 2011 03:24:29 -0400, Mark Wu <dwu@...hat.com> 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 maintaining a simple
>> index. So it's changed to use ida to allocate index via referring
>> to the index allocation in scsi disk.
>> 
>> Signed-off-by: Mark Wu <dwu@...hat.com>
> 
> Hi Mark,
> 
> I don't believe that we do disk probes in parallel, so the spinlock 
> is unnecessary.  Otherwise, this looks good.
> 
> Thanks, Rusty.
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
Author: Tejun Heo <tj@...nel.org>
Date:   Sat Feb 21 11:04:45 2009 +0900

    [SCSI] sd: revive sd_index_lock

    Commit f27bac2761cab5a2e212dea602d22457a9aa6943 which converted sd to
    use ida instead of idr incorrectly removed sd_index_lock around id
    allocation and free.  idr/ida do have internal locks but they protect
    their free object lists not the allocation itself.  The caller is
    responsible for that.  This missing synchronization led to the same id
    being assigned to multiple devices leading to oops.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/