svn commit: r217828 - projects/graid/head/sys/geom/raid

[Alexander Motin]

On 25.01.2011 19:07, John Baldwin wrote:
> On Tuesday, January 25, 2011 11:33:03 am Alexander Motin wrote:
>> On 25.01.2011 18:17, John Baldwin wrote:
>>> On Tuesday, January 25, 2011 10:34:07 am Alexander Motin wrote:
>>>> Author: mav
>>>> Date: Tue Jan 25 15:34:07 2011
>>>> New Revision: 217828
>>>> URL: http://svn.freebsd.org/changeset/base/217828
>>>>
>>>> Log:
>>>>     Implement kernel dumping to geom_raid volumes. Dumping mechanism supports
>>>>     any RAID levels without any additional magic. Dumping to RAID0 and RAID1
>>>>     verified to work right now.
>>>>
>>>> Modified:
>>>>     projects/graid/head/sys/geom/raid/g_raid.c
>>>>     projects/graid/head/sys/geom/raid/g_raid.h
>>>>     projects/graid/head/sys/geom/raid/md_intel.c
>>>>
>>>> Modified: projects/graid/head/sys/geom/raid/g_raid.c
>>>>
>>> ==============================================================================
>>>> --- projects/graid/head/sys/geom/raid/g_raid.c	Tue Jan 25 15:18:10 2011	
>>> (r217827)
>>>> +++ projects/graid/head/sys/geom/raid/g_raid.c	Tue Jan 25 15:34:07 2011	
>>> (r217828)
>>>> +static int
>>>> +g_raid_dump(void *arg,
>>>> +    void *virtual, vm_offset_t physical, off_t offset, size_t length)
>>>> +{
>>>> +	struct g_raid_softc *sc;
>>>> +	struct g_raid_volume *vol;
>>>> +	struct bio *bp;
>>>> +
>>>> +	vol = (struct g_raid_volume *)arg;
>>>> +	sc = vol->v_softc;
>>>> +	G_RAID_DEBUG(3, "Dumping at off %llu len %llu.",
>>>> +	    (long long unsigned)offset, (long long unsigned)length);
>>>> +
>>>> +	bp = g_alloc_bio();
>>>> +	bp->bio_cmd = BIO_WRITE;
>>>> +	bp->bio_done = g_raid_dumpdone;
>>>> +	bp->bio_attribute = NULL;
>>>> +	bp->bio_offset = offset;
>>>> +	bp->bio_length = length;
>>>> +	bp->bio_data = virtual;
>>>> +	bp->bio_to = vol->v_provider;
>>>> +
>>>> +	g_raid_start(bp);
>>>> +
>>>> +	while (!(bp->bio_flags&   BIO_DONE)) {
>>>> +		G_RAID_DEBUG(4, "Poll...");
>>>> +		g_raid_poll(sc);
>>>> +		DELAY(10);
>>>> +	}
>>>> +
>>>> +	G_RAID_DEBUG(3, "Dumping at off %llu len %llu done.",
>>>> +	    (long long unsigned)offset, (long long unsigned)length);
>>>> +
>>>> +	g_destroy_bio(bp);
>>>> +	return (0);
>>>> +}
>>>
>>> Hmm, so this allocates bio's to make the dump work.  I believer other dump
>>> routines in other drivers do not do this, but instead use pre-allocated
>>> commands to schedule dump I/O requests.  Would it be possible to pre-allocate
>>> the bio that is used here when dumping is enabled and reuse it for each
>>> g_raid_dump() call without free'ing it when the I/O is finished?
>>
>> Actually I've also thought about it. It is trivial to use static
>> variable in this particular place. But transformation modules
>> (RAID0/RAID1/...) are also allocating some BIOs via g_clone_bio() and
>> there it can be more difficult to fix, as several BIOs are allocated
>> same time to fulfill original request.
>
> Hummm.  That's a bit unfortunate.

It is hardly an excuse, but just for note, ataraid(4) does the same - 
on-stack allocation for the main request and uma_zalloc() for the 
children. I will think about possible alternatives.

-- 
Alexander Motin