NetBSD Problem Report #39552

From www@NetBSD.org  Mon Sep 15 12:15:33 2008
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by narn.NetBSD.org (Postfix) with ESMTP id B939563B8E3
	for <gnats-bugs@gnats.netbsd.org>; Mon, 15 Sep 2008 12:15:33 +0000 (UTC)
Message-Id: <20080915121533.913B763B842@narn.NetBSD.org>
Date: Mon, 15 Sep 2008 12:15:33 +0000 (UTC)
From: xtraeme@gmail.com
Reply-To: xtraeme@gmail.com
To: gnats-bugs@NetBSD.org
Subject: ataraid(4): Intel MatrixRAID multiple volumes support
X-Send-Pr-Version: www-1.0

>Number:         39552
>Category:       kern
>Synopsis:       ataraid(4): Intel MatrixRAID multiple volumes support
>Confidential:   no
>Severity:       non-critical
>Priority:       high
>Responsible:    tron
>State:          closed
>Class:          support
>Submitter-Id:   net
>Arrival-Date:   Mon Sep 15 12:20:00 +0000 2008
>Closed-Date:    Tue Sep 16 11:55:06 +0000 2008
>Last-Modified:  Tue Sep 16 11:55:06 +0000 2008
>Originator:     Juan RP
>Release:        Latest
>Organization:
>Environment:
NetBSD sasha 4.99.72 NetBSD 4.99.72 (MASTER) #70: Mon Sep 15 14:06:47 CEST 2008  juan@sasha:/home/juan/build/amd64/obj/sys/arch/amd64/compile/MASTER amd64
>Description:
The current code in ataraid(4) for Intel MatrixRAID controllers, doesn't
support multiple volumes attached to the same disks (known as matrix
RAID).

Will send a patch in a few minutes.
>How-To-Repeat:

>Fix:

>Release-Note:

>Audit-Trail:
From: Juan RP <xtraeme@gmail.com>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: kern/39552: ataraid(4): Intel MatrixRAID multiple volumes
 support
Date: Mon, 15 Sep 2008 14:32:35 +0200

 Here's the patch:

 Index: share/man/man4/ataraid.4
 ===================================================================
 RCS file: /cvsroot/src/share/man/man4/ataraid.4,v
 retrieving revision 1.13
 diff -b -u -p -r1.13 ataraid.4
 --- share/man/man4/ataraid.4	15 Sep 2008 11:44:50 -0000	1.13
 +++ share/man/man4/ataraid.4	15 Sep 2008 12:11:44 -0000
 @@ -92,7 +92,3 @@ state, and it does not do the right thin
  .Pp
  At least part of the reason for this is that the publically-available
  information on these formats is quite limited.
 -.Pp
 -The support with Intel MatrixRAID controller is incomplete, and only
 -the first volume found will be used.
 -This will be fixed in future revisions of this driver.
 Index: sys/dev/ata/ata_raid_intel.c
 ===================================================================
 RCS file: /cvsroot/src/sys/dev/ata/ata_raid_intel.c,v
 retrieving revision 1.2
 diff -b -u -p -r1.2 ata_raid_intel.c
 --- sys/dev/ata/ata_raid_intel.c	15 Sep 2008 11:44:50 -0000	1.2
 +++ sys/dev/ata/ata_raid_intel.c	15 Sep 2008 12:11:44 -0000
 @@ -140,7 +140,7 @@ ata_raid_read_config_intel(struct wd_sof
  	struct vnode *vp;
  	uint32_t checksum, *ptr;
  	static int curdrive;
 -	int bmajor, count, error = 0;
 +	int bmajor, count, curvol = 0, error = 0;
  	char *tmp;
  	dev_t dev;

 @@ -203,14 +203,11 @@ ata_raid_read_config_intel(struct wd_sof
  	/* This one points to the first volume */
  	map = (struct intel_raid_mapping *)&info->disk[info->total_disks];

 +findvol:
  	/*
  	 * Lookup or allocate a new array info structure for this array.
 -	 *
 -	 * TODO:
 -	 * We only look at the first volume. Need to solve a few issues before
 -	 * multiple volumes are working correctly.
  	 */
 -	aai = ata_raid_get_array_info(ATA_RAID_TYPE_INTEL, 0); 
 +	aai = ata_raid_get_array_info(ATA_RAID_TYPE_INTEL, curvol); 

  	/* Fill in array info */
  	aai->aai_generation = info->generation;
 @@ -244,7 +241,7 @@ ata_raid_read_config_intel(struct wd_sof

  	aai->aai_type = ATA_RAID_TYPE_INTEL;
  	aai->aai_capacity = map->total_sectors;
 -	aai->aai_interleave = map->stripe_sectors;
 +	aai->aai_interleave = map->stripe_sectors / 2;
  	aai->aai_ndisks = map->total_disks;
  	aai->aai_heads = 255;
  	aai->aai_sectors = 63;
 @@ -274,6 +271,16 @@ ata_raid_read_config_intel(struct wd_sof
  		adi->adi_dev = sc->sc_dev;
  		adi->adi_sectors = info->disk[curdrive].sectors;
  		adi->adi_compsize = adi->adi_sectors - aai->aai_reserved;
 +		/*
 +		 * Check if that is the only volume, otherwise repeat
 +		 * the process to find more.
 +		 */
 +		if ((curvol + 1) < info->total_volumes) {
 +			curvol++;
 +			map = (struct intel_raid_mapping *)
 +			    &map->disk_idx[map->total_disks];
 +			goto findvol;
 +		}
  		curdrive++;
  	}

 Index: sys/dev/ata/ata_raid_subr.c
 ===================================================================
 RCS file: sys/dev/ata/ata_raid_subr.c
 diff -N sys/dev/ata/ata_raid_subr.c
 --- /dev/null	1 Jan 1970 00:00:00 -0000
 +++ sys/dev/ata/ata_raid_subr.c	15 Sep 2008 12:11:44 -0000
 @@ -0,0 +1,101 @@
 +/* $NetBSD$ */
 +
 +/*-
 + * Copyright (c) 2008 Juan Romero Pardines.
 + * All rights reserved.
 + *
 + * Redistribution and use in source and binary forms, with or without
 + * modification, are permitted provided that the following conditions
 + * are met:
 + * 1. Redistributions of source code must retain the above copyright
 + *    notice, this list of conditions and the following disclaimer.
 + * 2. Redistributions in binary form must reproduce the above copyright
 + *    notice, this list of conditions and the following disclaimer in the
 + *    documentation and/or other materials provided with the distribution.
 + *
 + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
 + * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
 + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
 + * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
 + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
 + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
 + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
 + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
 + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
 + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 + */
 +
 +#include <sys/cdefs.h>
 +__KERNEL_RCSID(0, "$NetBSD$");
 +
 +#include <sys/param.h>
 +#include <sys/systm.h>
 +#include <sys/conf.h>
 +#include <sys/kernel.h>
 +#include <sys/dkio.h>
 +#include <sys/disk.h>
 +#include <sys/disklabel.h>
 +#include <sys/fcntl.h>
 +#include <sys/vnode.h>
 +#include <sys/kauth.h>
 +#include <sys/kmem.h>
 +
 +#include <dev/ata/ata_raidvar.h>
 +
 +struct ataraid_disk_vnode {
 +	struct ataraid_disk_info *adv_adi;
 +	struct vnode *adv_vnode;
 +	SLIST_ENTRY(ataraid_disk_vnode) adv_next;
 +};
 +
 +static SLIST_HEAD(, ataraid_disk_vnode) ataraid_disk_vnode_list =
 +    SLIST_HEAD_INITIALIZER(ataraid_disk_vnode_list);
 +
 +/* 
 + * Finds the RAW_PART vnode of the block device associated with a component
 + * by looking at the singly linked list; otherwise creates, opens and
 + * returns the vnode to the caller.
 + */
 +struct vnode *
 +ata_raid_disk_vnode_find(struct ataraid_disk_info *adi)
 +{
 +	struct ataraid_disk_vnode *adv = NULL;
 +	struct vnode *vp = NULL;
 +	device_t devlist;
 +	int bmajor, error = 0;
 +	dev_t dev;
 +
 +	SLIST_FOREACH(adv, &ataraid_disk_vnode_list, adv_next) {
 +		devlist = adv->adv_adi->adi_dev;
 +		if (strcmp(device_xname(devlist),
 +		    device_xname(adi->adi_dev)) == 0)
 +			return adv->adv_vnode;
 +	}
 +
 +	adv = NULL;
 +	adv = kmem_zalloc(sizeof(struct ataraid_disk_vnode), KM_SLEEP);
 +
 +	bmajor = devsw_name2blk(device_xname(adi->adi_dev), NULL, 0);
 +	dev = MAKEDISKDEV(bmajor, device_unit(adi->adi_dev), RAW_PART);
 +
 +	error = bdevvp(dev, &vp);
 +	if (error) {
 +		kmem_free(adv, sizeof(struct ataraid_disk_vnode));
 +		return NULL;
 +	}
 +	error = VOP_OPEN(vp, FREAD|FWRITE, NOCRED);
 +	if (error) {
 +		vput(vp);
 +		kmem_free(adv, sizeof(struct ataraid_disk_vnode));
 +		return NULL;
 +	}
 +	vn_lock(vp, LK_EXCLUSIVE | LK_RETRY);
 +	VOP_UNLOCK(vp, 0);
 +
 +	adv->adv_adi = adi;
 +	adv->adv_vnode = vp;
 +
 +	SLIST_INSERT_HEAD(&ataraid_disk_vnode_list, adv, adv_next);
 +
 +	return adv->adv_vnode;
 +}
 Index: sys/dev/ata/ata_raidvar.h
 ===================================================================
 RCS file: /cvsroot/src/sys/dev/ata/ata_raidvar.h,v
 retrieving revision 1.9
 diff -b -u -p -r1.9 ata_raidvar.h
 --- sys/dev/ata/ata_raidvar.h	15 Sep 2008 11:44:50 -0000	1.9
 +++ sys/dev/ata/ata_raidvar.h	15 Sep 2008 12:11:45 -0000
 @@ -126,6 +126,8 @@ struct ataraid_array_info *ata_raid_get_
  int	ata_raid_config_block_rw(struct vnode *, daddr_t, void *,
  	    size_t, int);

 +struct vnode *ata_raid_disk_vnode_find(struct ataraid_disk_info *);
 +
  /* Promise RAID support */
  int	ata_raid_read_config_promise(struct wd_softc *);

 Index: sys/dev/ata/files.ata
 ===================================================================
 RCS file: /cvsroot/src/sys/dev/ata/files.ata,v
 retrieving revision 1.19
 diff -b -u -p -r1.19 files.ata
 --- sys/dev/ata/files.ata	11 Sep 2008 11:08:50 -0000	1.19
 +++ sys/dev/ata/files.ata	15 Sep 2008 12:11:45 -0000
 @@ -18,6 +18,7 @@ file	dev/ata/ata.c			(ata_hl | atapi) & 
  # ATA RAID configuration support
  defpseudodev ataraid {[vendtype = -1], [unit = -1]}
  file	dev/ata/ata_raid.c		ataraid			needs-flag
 +file	dev/ata/ata_raid_subr.c		ataraid
  file	dev/ata/ata_raid_promise.c	ataraid
  file	dev/ata/ata_raid_adaptec.c	ataraid
  file	dev/ata/ata_raid_nvidia.c	ataraid
 Index: sys/dev/ata/ld_ataraid.c
 ===================================================================
 RCS file: /cvsroot/src/sys/dev/ata/ld_ataraid.c,v
 retrieving revision 1.31
 diff -b -u -p -r1.31 ld_ataraid.c
 --- sys/dev/ata/ld_ataraid.c	15 Sep 2008 11:53:52 -0000	1.31
 +++ sys/dev/ata/ld_ataraid.c	15 Sep 2008 12:11:45 -0000
 @@ -143,6 +143,7 @@ ld_ataraid_attach(device_t parent, devic
  	struct ld_ataraid_softc *sc = device_private(self);
  	struct ld_softc *ld = &sc->sc_ld;
  	struct ataraid_array_info *aai = aux;
 +	struct ataraid_disk_info *adi = NULL;
  	const char *level;
  	struct vnode *vp;
  	char unklev[32];
 @@ -215,18 +216,9 @@ ld_ataraid_attach(device_t parent, devic
  	 * Configure all the component disks.
  	 */
  	for (i = 0; i < aai->aai_ndisks; i++) {
 -		struct ataraid_disk_info *adi = &aai->aai_disks[i];
 -		int bmajor, error;
 -		dev_t dev;
 -
 -		bmajor = devsw_name2blk(device_xname(adi->adi_dev), NULL, 0);
 -		dev = MAKEDISKDEV(bmajor, device_unit(adi->adi_dev), RAW_PART);
 -		error = bdevvp(dev, &vp);
 -		if (error)
 -			break;
 -		error = VOP_OPEN(vp, FREAD|FWRITE, NOCRED);
 -		if (error) {
 -			vput(vp);
 +		adi = &aai->aai_disks[i];
 +		vp = ata_raid_disk_vnode_find(adi);
 +		if (vp == NULL) {
  			/*
  			 * XXX This is bogus.  We should just mark the
  			 * XXX component as FAILED, and write-back new
 @@ -234,8 +226,6 @@ ld_ataraid_attach(device_t parent, devic
  			 */
  			break;
  		}
 -
 -		VOP_UNLOCK(vp, 0);
  		sc->sc_vnodes[i] = vp;
  	}
  	if (i == aai->aai_ndisks) {

Responsible-Changed-From-To: kern-bug-people->tron
Responsible-Changed-By: tron@NetBSD.org
Responsible-Changed-When: Tue, 16 Sep 2008 11:43:06 +0000
Responsible-Changed-Why:
I'll handle this PR.


From: Matthias Scheler <tron@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/39552 CVS commit: src
Date: Tue, 16 Sep 2008 11:45:30 +0000 (UTC)

 Module Name:	src
 Committed By:	tron
 Date:		Tue Sep 16 11:45:30 UTC 2008

 Modified Files:
 	src/share/man/man4: ataraid.4
 	src/sys/dev/ata: ata_raid_intel.c ata_raidvar.h files.ata ld_ataraid.c
 Added Files:
 	src/sys/dev/ata: ata_raid_subr.c

 Log Message:
 Support multiple volumes connected to Intel MatrixRAID controllers.
 Code contributed by Juan RP in PR kern/39552.


 To generate a diff of this commit:
 cvs rdiff -r1.13 -r1.14 src/share/man/man4/ataraid.4
 cvs rdiff -r1.2 -r1.3 src/sys/dev/ata/ata_raid_intel.c
 cvs rdiff -r0 -r1.1 src/sys/dev/ata/ata_raid_subr.c
 cvs rdiff -r1.9 -r1.10 src/sys/dev/ata/ata_raidvar.h
 cvs rdiff -r1.19 -r1.20 src/sys/dev/ata/files.ata
 cvs rdiff -r1.31 -r1.32 src/sys/dev/ata/ld_ataraid.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->closed
State-Changed-By: tron@NetBSD.org
State-Changed-When: Tue, 16 Sep 2008 11:55:06 +0000
State-Changed-Why:
I've committed your changes. Thanks a lot for the contribution.


>Unformatted:

NetBSD Home
NetBSD PR Database Search

(Contact us) $NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2007 The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.