softraid: factor out block I/O code

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

softraid: factor out block I/O code

Joel Sing-3
The following diff factors out the block I/O code that is used within
softraid(4) and also allows it to handle I/Os that exceeds MAXPHYS in
size. This is necessary for some upcoming work.

This diff needs extensive testing since the main purpose is to read and
write the softraid metadata. Bugs in this area will eat softraid metadata
and therefore destroy softraid volumes. If you are testing please ensure
that you have backed up your data or that the volume does not have
critical information. Please report test successes/failures directly to me.

Index: softraidvar.h
===================================================================
RCS file: /cvs/src/sys/dev/softraidvar.h,v
retrieving revision 1.96
diff -u -p -r1.96 softraidvar.h
--- softraidvar.h 6 Nov 2010 23:01:56 -0000 1.96
+++ softraidvar.h 6 Jan 2011 12:41:26 -0000
@@ -624,6 +624,8 @@ int sr_check_io_collision(struct sr_wo
 void sr_scsi_done(struct sr_discipline *,
     struct scsi_xfer *);
 int sr_chunk_in_use(struct sr_softc *, dev_t);
+int sr_rw(struct sr_softc *, dev_t, char *, size_t,
+    daddr64_t, long);
 
 /* discipline functions */
 int sr_raid_inquiry(struct sr_workunit *);
Index: softraid.c
===================================================================
RCS file: /cvs/src/sys/dev/softraid.c,v
retrieving revision 1.217
diff -u -p -r1.217 softraid.c
--- softraid.c 20 Dec 2010 17:47:48 -0000 1.217
+++ softraid.c 6 Jan 2011 12:41:26 -0000
@@ -387,57 +387,93 @@ sr_meta_getdevname(struct sr_softc *sc,
 }
 
 int
-sr_meta_rw(struct sr_discipline *sd, dev_t dev, void *md, size_t sz,
-    daddr64_t ofs, long flags)
+sr_rw(struct sr_softc *sc, dev_t dev, char *buf, size_t size, daddr64_t
offset,
+    long flags)
 {
- struct sr_softc *sc = sd->sd_sc;
+ struct vnode *vp;
  struct buf b;
+ size_t bufsize;
  int rv = 1;
 
- DNPRINTF(SR_D_META, "%s: sr_meta_rw(0x%x, %p, %d, %llu 0x%x)\n",
-    DEVNAME(sc), dev, md, sz, ofs, flags);
-
- bzero(&b, sizeof(b));
+ DNPRINTF(SR_D_MISC, "%s: sr_rw(0x%x, %p, %d, %llu 0x%x)\n",
+    DEVNAME(sc), dev, buf, size, offset, flags);
 
- if (md == NULL) {
- printf("%s: read invalid metadata pointer\n", DEVNAME(sc));
+ if (bdevvp(dev, &vp)) {
+ printf("%s: sr_rw: failed to allocate vnode\n", DEVNAME(sc));
  goto done;
  }
- b.b_flags = flags | B_PHYS;
- b.b_blkno = ofs;
- b.b_bcount = sz;
- b.b_bufsize = sz;
- b.b_resid = sz;
- b.b_data = md;
- b.b_error = 0;
- b.b_proc = curproc;
- b.b_dev = dev;
- b.b_iodone = NULL;
- if (bdevvp(dev, &b.b_vp)) {
- printf("%s: sr_meta_rw: can't allocate vnode\n", DEVNAME(sc));
- goto done;
- }
- if ((b.b_flags & B_READ) == 0)
- b.b_vp->v_numoutput++;
-
- LIST_INIT(&b.b_dep);
- VOP_STRATEGY(&b);
- biowait(&b);
-
- if (b.b_flags & B_ERROR) {
- printf("%s: 0x%x i/o error on block %llu while reading "
-    "metadata %d\n", DEVNAME(sc), dev, b.b_blkno, b.b_error);
- goto done;
+
+ while (size > 0) {
+
+ DNPRINTF(SR_D_MISC, "%s: buf %p, size %d, offset %llu)\n",
+    DEVNAME(sc), buf, size, offset);
+
+ bufsize = (size > MAXPHYS) ? MAXPHYS : size;
+
+ bzero(&b, sizeof(b));
+
+ b.b_flags = flags | B_PHYS;
+ b.b_proc = curproc;
+ b.b_dev = dev;
+ b.b_iodone = NULL;
+ b.b_error = 0;
+ b.b_blkno = offset;
+ b.b_data = buf;
+ b.b_bcount = bufsize;
+ b.b_bufsize = bufsize;
+ b.b_resid = bufsize;
+ b.b_vp = vp;
+
+ if ((b.b_flags & B_READ) == 0)
+ vp->v_numoutput++;
+
+ LIST_INIT(&b.b_dep);
+ VOP_STRATEGY(&b);
+ biowait(&b);
+
+ if (b.b_flags & B_ERROR) {
+ printf("%s: I/O error %d on dev 0x%x at block %llu\n",
+    DEVNAME(sc), b.b_error, dev, b.b_blkno);
+ goto done;
+ }
+
+ size -= bufsize;
+ buf += bufsize;
+ offset += howmany(bufsize, DEV_BSIZE);
+
  }
+
  rv = 0;
+
 done:
- if (b.b_vp)
- vput(b.b_vp);
+ if (vp)
+ vput(vp);
 
  return (rv);
 }
 
 int
+sr_meta_rw(struct sr_discipline *sd, dev_t dev, void *md, size_t size,
+    daddr64_t offset, long flags)
+{
+ int rv = 1;
+
+ DNPRINTF(SR_D_META, "%s: sr_meta_rw(0x%x, %p, %d, %llu 0x%x)\n",
+    DEVNAME(sd->sd_sc), dev, md, size, offset, flags);
+
+ if (md == NULL) {
+ printf("%s: sr_meta_rw: invalid metadata pointer\n",
+    DEVNAME(sd->sd_sc));
+ goto done;
+ }
+
+ rv = sr_rw(sd->sd_sc, dev, md, size, offset, flags);
+
+done:
+ return (rv);
+}
+
+int
 sr_meta_clear(struct sr_discipline *sd)
 {
  struct sr_softc *sc = sd->sd_sc;
@@ -3167,7 +3203,6 @@ sr_ioctl_installboot(struct sr_softc *sc
  void *bootblk = NULL, *bootldr = NULL;
  struct sr_discipline *sd = NULL;
  struct sr_chunk *chunk;
- struct buf b;
  u_int32_t bbs, bls;
  int rv = EINVAL;
  int i;
@@ -3216,34 +3251,9 @@ sr_ioctl_installboot(struct sr_softc *sc
     "sr_ioctl_installboot: saving boot block to %s "
     "(%u bytes)\n", chunk->src_devname, bbs);
 
- bzero(&b, sizeof(b));
- b.b_flags = B_WRITE | B_PHYS;
- b.b_blkno = SR_BOOT_BLOCKS_OFFSET;
- b.b_bcount = bbs;
- b.b_bufsize = bbs;
- b.b_resid = bbs;
- b.b_data = bootblk;
- b.b_error = 0;
- b.b_proc = curproc;
- b.b_dev = chunk->src_dev_mm;
- b.b_vp = NULL;
- b.b_iodone = NULL;
- if (bdevvp(chunk->src_dev_mm, &b.b_vp)) {
- printf("%s: sr_ioctl_installboot: vnode allocation "
-    "failed\n", DEVNAME(sc));
- goto done;
- }
- if ((b.b_flags & B_READ) == 0)
- b.b_vp->v_numoutput++;
- LIST_INIT(&b.b_dep);
- VOP_STRATEGY(&b);
- biowait(&b);
- vput(b.b_vp);
-
- if (b.b_flags & B_ERROR) {
- printf("%s: 0x%x i/o error on block %llu while "
-    "writing boot block %d\n", DEVNAME(sc),
-    chunk->src_dev_mm, b.b_blkno, b.b_error);
+ if (sr_rw(sc, chunk->src_dev_mm, bootblk, bbs,
+    SR_BOOT_BLOCKS_OFFSET, B_WRITE)) {
+ printf("%s: failed to write boot block\n", DEVNAME(sc));
  goto done;
  }
 
@@ -3252,34 +3262,10 @@ sr_ioctl_installboot(struct sr_softc *sc
     "sr_ioctl_installboot: saving boot loader to %s "
     "(%u bytes)\n", chunk->src_devname, bls);
 
- bzero(&b, sizeof(b));
- b.b_flags = B_WRITE | B_PHYS;
- b.b_blkno = SR_BOOT_LOADER_OFFSET;
- b.b_bcount = bls;
- b.b_bufsize = bls;
- b.b_resid = bls;
- b.b_data = bootldr;
- b.b_error = 0;
- b.b_proc = curproc;
- b.b_dev = chunk->src_dev_mm;
- b.b_vp = NULL;
- b.b_iodone = NULL;
- if (bdevvp(chunk->src_dev_mm, &b.b_vp)) {
- printf("%s: sr_ioctl_installboot: vnode alocation "
-    "failed\n", DEVNAME(sc));
- goto done;
- }
- if ((b.b_flags & B_READ) == 0)
- b.b_vp->v_numoutput++;
- LIST_INIT(&b.b_dep);
- VOP_STRATEGY(&b);
- biowait(&b);
- vput(b.b_vp);
-
- if (b.b_flags & B_ERROR) {
- printf("%s: 0x%x i/o error on block %llu while "
-    "writing boot blocks %d\n", DEVNAME(sc),
-    chunk->src_dev_mm, b.b_blkno, b.b_error);
+ if (sr_rw(sc, chunk->src_dev_mm, bootldr, bls,
+    SR_BOOT_LOADER_OFFSET, B_WRITE)) {
+ printf("%s: failed to write boot loader\n",
+   DEVNAME(sc));
  goto done;
  }
--

   "Stop assuming that systems are secure unless demonstrated insecure;
    start assuming that systems are insecure unless designed securely."
          - Bruce Schneier

Reply | Threaded
Open this post in threaded view
|

Re: softraid: factor out block I/O code

Marco Peereboom
This one needs lots of testing folks.  Please oblige.

On Sat, Jan 15, 2011 at 01:22:24AM +1100, Joel Sing wrote:

> The following diff factors out the block I/O code that is used within
> softraid(4) and also allows it to handle I/Os that exceeds MAXPHYS in
> size. This is necessary for some upcoming work.
>
> This diff needs extensive testing since the main purpose is to read and
> write the softraid metadata. Bugs in this area will eat softraid metadata
> and therefore destroy softraid volumes. If you are testing please ensure
> that you have backed up your data or that the volume does not have
> critical information. Please report test successes/failures directly to me.
>
> Index: softraidvar.h
> ===================================================================
> RCS file: /cvs/src/sys/dev/softraidvar.h,v
> retrieving revision 1.96
> diff -u -p -r1.96 softraidvar.h
> --- softraidvar.h 6 Nov 2010 23:01:56 -0000 1.96
> +++ softraidvar.h 6 Jan 2011 12:41:26 -0000
> @@ -624,6 +624,8 @@ int sr_check_io_collision(struct sr_wo
>  void sr_scsi_done(struct sr_discipline *,
>      struct scsi_xfer *);
>  int sr_chunk_in_use(struct sr_softc *, dev_t);
> +int sr_rw(struct sr_softc *, dev_t, char *, size_t,
> +    daddr64_t, long);
>  
>  /* discipline functions */
>  int sr_raid_inquiry(struct sr_workunit *);
> Index: softraid.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/softraid.c,v
> retrieving revision 1.217
> diff -u -p -r1.217 softraid.c
> --- softraid.c 20 Dec 2010 17:47:48 -0000 1.217
> +++ softraid.c 6 Jan 2011 12:41:26 -0000
> @@ -387,57 +387,93 @@ sr_meta_getdevname(struct sr_softc *sc,
>  }
>  
>  int
> -sr_meta_rw(struct sr_discipline *sd, dev_t dev, void *md, size_t sz,
> -    daddr64_t ofs, long flags)
> +sr_rw(struct sr_softc *sc, dev_t dev, char *buf, size_t size, daddr64_t
> offset,
> +    long flags)
>  {
> - struct sr_softc *sc = sd->sd_sc;
> + struct vnode *vp;
>   struct buf b;
> + size_t bufsize;
>   int rv = 1;
>  
> - DNPRINTF(SR_D_META, "%s: sr_meta_rw(0x%x, %p, %d, %llu 0x%x)\n",
> -    DEVNAME(sc), dev, md, sz, ofs, flags);
> -
> - bzero(&b, sizeof(b));
> + DNPRINTF(SR_D_MISC, "%s: sr_rw(0x%x, %p, %d, %llu 0x%x)\n",
> +    DEVNAME(sc), dev, buf, size, offset, flags);
>  
> - if (md == NULL) {
> - printf("%s: read invalid metadata pointer\n", DEVNAME(sc));
> + if (bdevvp(dev, &vp)) {
> + printf("%s: sr_rw: failed to allocate vnode\n", DEVNAME(sc));
>   goto done;
>   }
> - b.b_flags = flags | B_PHYS;
> - b.b_blkno = ofs;
> - b.b_bcount = sz;
> - b.b_bufsize = sz;
> - b.b_resid = sz;
> - b.b_data = md;
> - b.b_error = 0;
> - b.b_proc = curproc;
> - b.b_dev = dev;
> - b.b_iodone = NULL;
> - if (bdevvp(dev, &b.b_vp)) {
> - printf("%s: sr_meta_rw: can't allocate vnode\n", DEVNAME(sc));
> - goto done;
> - }
> - if ((b.b_flags & B_READ) == 0)
> - b.b_vp->v_numoutput++;
> -
> - LIST_INIT(&b.b_dep);
> - VOP_STRATEGY(&b);
> - biowait(&b);
> -
> - if (b.b_flags & B_ERROR) {
> - printf("%s: 0x%x i/o error on block %llu while reading "
> -    "metadata %d\n", DEVNAME(sc), dev, b.b_blkno, b.b_error);
> - goto done;
> +
> + while (size > 0) {
> +
> + DNPRINTF(SR_D_MISC, "%s: buf %p, size %d, offset %llu)\n",
> +    DEVNAME(sc), buf, size, offset);
> +
> + bufsize = (size > MAXPHYS) ? MAXPHYS : size;
> +
> + bzero(&b, sizeof(b));
> +
> + b.b_flags = flags | B_PHYS;
> + b.b_proc = curproc;
> + b.b_dev = dev;
> + b.b_iodone = NULL;
> + b.b_error = 0;
> + b.b_blkno = offset;
> + b.b_data = buf;
> + b.b_bcount = bufsize;
> + b.b_bufsize = bufsize;
> + b.b_resid = bufsize;
> + b.b_vp = vp;
> +
> + if ((b.b_flags & B_READ) == 0)
> + vp->v_numoutput++;
> +
> + LIST_INIT(&b.b_dep);
> + VOP_STRATEGY(&b);
> + biowait(&b);
> +
> + if (b.b_flags & B_ERROR) {
> + printf("%s: I/O error %d on dev 0x%x at block %llu\n",
> +    DEVNAME(sc), b.b_error, dev, b.b_blkno);
> + goto done;
> + }
> +
> + size -= bufsize;
> + buf += bufsize;
> + offset += howmany(bufsize, DEV_BSIZE);
> +
>   }
> +
>   rv = 0;
> +
>  done:
> - if (b.b_vp)
> - vput(b.b_vp);
> + if (vp)
> + vput(vp);
>  
>   return (rv);
>  }
>  
>  int
> +sr_meta_rw(struct sr_discipline *sd, dev_t dev, void *md, size_t size,
> +    daddr64_t offset, long flags)
> +{
> + int rv = 1;
> +
> + DNPRINTF(SR_D_META, "%s: sr_meta_rw(0x%x, %p, %d, %llu 0x%x)\n",
> +    DEVNAME(sd->sd_sc), dev, md, size, offset, flags);
> +
> + if (md == NULL) {
> + printf("%s: sr_meta_rw: invalid metadata pointer\n",
> +    DEVNAME(sd->sd_sc));
> + goto done;
> + }
> +
> + rv = sr_rw(sd->sd_sc, dev, md, size, offset, flags);
> +
> +done:
> + return (rv);
> +}
> +
> +int
>  sr_meta_clear(struct sr_discipline *sd)
>  {
>   struct sr_softc *sc = sd->sd_sc;
> @@ -3167,7 +3203,6 @@ sr_ioctl_installboot(struct sr_softc *sc
>   void *bootblk = NULL, *bootldr = NULL;
>   struct sr_discipline *sd = NULL;
>   struct sr_chunk *chunk;
> - struct buf b;
>   u_int32_t bbs, bls;
>   int rv = EINVAL;
>   int i;
> @@ -3216,34 +3251,9 @@ sr_ioctl_installboot(struct sr_softc *sc
>      "sr_ioctl_installboot: saving boot block to %s "
>      "(%u bytes)\n", chunk->src_devname, bbs);
>  
> - bzero(&b, sizeof(b));
> - b.b_flags = B_WRITE | B_PHYS;
> - b.b_blkno = SR_BOOT_BLOCKS_OFFSET;
> - b.b_bcount = bbs;
> - b.b_bufsize = bbs;
> - b.b_resid = bbs;
> - b.b_data = bootblk;
> - b.b_error = 0;
> - b.b_proc = curproc;
> - b.b_dev = chunk->src_dev_mm;
> - b.b_vp = NULL;
> - b.b_iodone = NULL;
> - if (bdevvp(chunk->src_dev_mm, &b.b_vp)) {
> - printf("%s: sr_ioctl_installboot: vnode allocation "
> -    "failed\n", DEVNAME(sc));
> - goto done;
> - }
> - if ((b.b_flags & B_READ) == 0)
> - b.b_vp->v_numoutput++;
> - LIST_INIT(&b.b_dep);
> - VOP_STRATEGY(&b);
> - biowait(&b);
> - vput(b.b_vp);
> -
> - if (b.b_flags & B_ERROR) {
> - printf("%s: 0x%x i/o error on block %llu while "
> -    "writing boot block %d\n", DEVNAME(sc),
> -    chunk->src_dev_mm, b.b_blkno, b.b_error);
> + if (sr_rw(sc, chunk->src_dev_mm, bootblk, bbs,
> +    SR_BOOT_BLOCKS_OFFSET, B_WRITE)) {
> + printf("%s: failed to write boot block\n", DEVNAME(sc));
>   goto done;
>   }
>  
> @@ -3252,34 +3262,10 @@ sr_ioctl_installboot(struct sr_softc *sc
>      "sr_ioctl_installboot: saving boot loader to %s "
>      "(%u bytes)\n", chunk->src_devname, bls);
>  
> - bzero(&b, sizeof(b));
> - b.b_flags = B_WRITE | B_PHYS;
> - b.b_blkno = SR_BOOT_LOADER_OFFSET;
> - b.b_bcount = bls;
> - b.b_bufsize = bls;
> - b.b_resid = bls;
> - b.b_data = bootldr;
> - b.b_error = 0;
> - b.b_proc = curproc;
> - b.b_dev = chunk->src_dev_mm;
> - b.b_vp = NULL;
> - b.b_iodone = NULL;
> - if (bdevvp(chunk->src_dev_mm, &b.b_vp)) {
> - printf("%s: sr_ioctl_installboot: vnode alocation "
> -    "failed\n", DEVNAME(sc));
> - goto done;
> - }
> - if ((b.b_flags & B_READ) == 0)
> - b.b_vp->v_numoutput++;
> - LIST_INIT(&b.b_dep);
> - VOP_STRATEGY(&b);
> - biowait(&b);
> - vput(b.b_vp);
> -
> - if (b.b_flags & B_ERROR) {
> - printf("%s: 0x%x i/o error on block %llu while "
> -    "writing boot blocks %d\n", DEVNAME(sc),
> -    chunk->src_dev_mm, b.b_blkno, b.b_error);
> + if (sr_rw(sc, chunk->src_dev_mm, bootldr, bls,
> +    SR_BOOT_LOADER_OFFSET, B_WRITE)) {
> + printf("%s: failed to write boot loader\n",
> +   DEVNAME(sc));
>   goto done;
>   }
> --
>
>    "Stop assuming that systems are secure unless demonstrated insecure;
>     start assuming that systems are insecure unless designed securely."
>           - Bruce Schneier

Reply | Threaded
Open this post in threaded view
|

Re: softraid: factor out block I/O code

Bob Beck-4
Or to reiterate - if you want this working in 4.9 now is the time to
make time to test it :)


On 14 January 2011 09:42, Marco Peereboom <[hidden email]> wrote:

> This one needs lots of testing folks.  Please oblige.
>
> On Sat, Jan 15, 2011 at 01:22:24AM +1100, Joel Sing wrote:
>> The following diff factors out the block I/O code that is used within
>> softraid(4) and also allows it to handle I/Os that exceeds MAXPHYS in
>> size. This is necessary for some upcoming work.
>>
>> This diff needs extensive testing since the main purpose is to read and
>> write the softraid metadata. Bugs in this area will eat softraid metadata
>> and therefore destroy softraid volumes. If you are testing please ensure
>> that you have backed up your data or that the volume does not have
>> critical information. Please report test successes/failures directly to
me.
>>
>> Index: softraidvar.h
>> ===================================================================
>> RCS file: /cvs/src/sys/dev/softraidvar.h,v
>> retrieving revision 1.96
>> diff -u -p -r1.96 softraidvar.h
>> --- softraidvar.h     6 Nov 2010 23:01:56 -0000       1.96
>> +++ softraidvar.h     6 Jan 2011 12:41:26 -0000
>> @@ -624,6 +624,8 @@ int                       sr_check_io_collision(struct
sr_wo

>>  void                 sr_scsi_done(struct sr_discipline *,
>>                           struct scsi_xfer *);
>>  int                  sr_chunk_in_use(struct sr_softc *, dev_t);
>> +int                  sr_rw(struct sr_softc *, dev_t, char *, size_t,
>> +                         daddr64_t, long);
>>
>>  /* discipline functions */
>>  int                  sr_raid_inquiry(struct sr_workunit *);
>> Index: softraid.c
>> ===================================================================
>> RCS file: /cvs/src/sys/dev/softraid.c,v
>> retrieving revision 1.217
>> diff -u -p -r1.217 softraid.c
>> --- softraid.c        20 Dec 2010 17:47:48 -0000      1.217
>> +++ softraid.c        6 Jan 2011 12:41:26 -0000
>> @@ -387,57 +387,93 @@ sr_meta_getdevname(struct sr_softc *sc,
>>  }
>>
>>  int
>> -sr_meta_rw(struct sr_discipline *sd, dev_t dev, void *md, size_t sz,
>> -    daddr64_t ofs, long flags)
>> +sr_rw(struct sr_softc *sc, dev_t dev, char *buf, size_t size, daddr64_t
>> offset,
>> +    long flags)
>>  {
>> -     struct sr_softc         *sc = sd->sd_sc;
>> +     struct vnode            *vp;
>>       struct buf              b;
>> +     size_t                  bufsize;
>>       int                     rv = 1;
>>
>> -     DNPRINTF(SR_D_META, "%s: sr_meta_rw(0x%x, %p, %d, %llu 0x%x)\n",
>> -         DEVNAME(sc), dev, md, sz, ofs, flags);
>> -
>> -     bzero(&b, sizeof(b));
>> +     DNPRINTF(SR_D_MISC, "%s: sr_rw(0x%x, %p, %d, %llu 0x%x)\n",
>> +         DEVNAME(sc), dev, buf, size, offset, flags);
>>
>> -     if (md == NULL) {
>> -             printf("%s: read invalid metadata pointer\n", DEVNAME(sc));
>> +     if (bdevvp(dev, &vp)) {
>> +             printf("%s: sr_rw: failed to allocate vnode\n",
DEVNAME(sc));

>>               goto done;
>>       }
>> -     b.b_flags = flags | B_PHYS;
>> -     b.b_blkno = ofs;
>> -     b.b_bcount = sz;
>> -     b.b_bufsize = sz;
>> -     b.b_resid = sz;
>> -     b.b_data = md;
>> -     b.b_error = 0;
>> -     b.b_proc = curproc;
>> -     b.b_dev = dev;
>> -     b.b_iodone = NULL;
>> -     if (bdevvp(dev, &b.b_vp)) {
>> -             printf("%s: sr_meta_rw: can't allocate vnode\n",
DEVNAME(sc));

>> -             goto done;
>> -     }
>> -     if ((b.b_flags & B_READ) == 0)
>> -             b.b_vp->v_numoutput++;
>> -
>> -     LIST_INIT(&b.b_dep);
>> -     VOP_STRATEGY(&b);
>> -     biowait(&b);
>> -
>> -     if (b.b_flags & B_ERROR) {
>> -             printf("%s: 0x%x i/o error on block %llu while reading "
>> -                 "metadata %d\n", DEVNAME(sc), dev, b.b_blkno,
b.b_error);

>> -             goto done;
>> +
>> +     while (size > 0) {
>> +
>> +             DNPRINTF(SR_D_MISC, "%s: buf %p, size %d, offset %llu)\n",
>> +                 DEVNAME(sc), buf, size, offset);
>> +
>> +             bufsize = (size > MAXPHYS) ? MAXPHYS : size;
>> +
>> +             bzero(&b, sizeof(b));
>> +
>> +             b.b_flags = flags | B_PHYS;
>> +             b.b_proc = curproc;
>> +             b.b_dev = dev;
>> +             b.b_iodone = NULL;
>> +             b.b_error = 0;
>> +             b.b_blkno = offset;
>> +             b.b_data = buf;
>> +             b.b_bcount = bufsize;
>> +             b.b_bufsize = bufsize;
>> +             b.b_resid = bufsize;
>> +             b.b_vp = vp;
>> +
>> +             if ((b.b_flags & B_READ) == 0)
>> +                     vp->v_numoutput++;
>> +
>> +             LIST_INIT(&b.b_dep);
>> +             VOP_STRATEGY(&b);
>> +             biowait(&b);
>> +
>> +             if (b.b_flags & B_ERROR) {
>> +                     printf("%s: I/O error %d on dev 0x%x at block
%llu\n",

>> +                         DEVNAME(sc), b.b_error, dev, b.b_blkno);
>> +                     goto done;
>> +             }
>> +
>> +             size -= bufsize;
>> +             buf += bufsize;
>> +             offset += howmany(bufsize, DEV_BSIZE);
>> +
>>       }
>> +
>>       rv = 0;
>> +
>>  done:
>> -     if (b.b_vp)
>> -             vput(b.b_vp);
>> +     if (vp)
>> +             vput(vp);
>>
>>       return (rv);
>>  }
>>
>>  int
>> +sr_meta_rw(struct sr_discipline *sd, dev_t dev, void *md, size_t size,
>> +    daddr64_t offset, long flags)
>> +{
>> +     int                     rv = 1;
>> +
>> +     DNPRINTF(SR_D_META, "%s: sr_meta_rw(0x%x, %p, %d, %llu 0x%x)\n",
>> +         DEVNAME(sd->sd_sc), dev, md, size, offset, flags);
>> +
>> +     if (md == NULL) {
>> +             printf("%s: sr_meta_rw: invalid metadata pointer\n",
>> +                 DEVNAME(sd->sd_sc));
>> +             goto done;
>> +     }
>> +
>> +     rv = sr_rw(sd->sd_sc, dev, md, size, offset, flags);
>> +
>> +done:
>> +     return (rv);
>> +}
>> +
>> +int
>>  sr_meta_clear(struct sr_discipline *sd)
>>  {
>>       struct sr_softc         *sc = sd->sd_sc;
>> @@ -3167,7 +3203,6 @@ sr_ioctl_installboot(struct sr_softc *sc
>>       void                    *bootblk = NULL, *bootldr = NULL;
>>       struct sr_discipline    *sd = NULL;
>>       struct sr_chunk         *chunk;
>> -     struct buf              b;
>>       u_int32_t               bbs, bls;
>>       int                     rv = EINVAL;
>>       int                     i;
>> @@ -3216,34 +3251,9 @@ sr_ioctl_installboot(struct sr_softc *sc
>>                   "sr_ioctl_installboot: saving boot block to %s "
>>                   "(%u bytes)\n", chunk->src_devname, bbs);
>>
>> -             bzero(&b, sizeof(b));
>> -             b.b_flags = B_WRITE | B_PHYS;
>> -             b.b_blkno = SR_BOOT_BLOCKS_OFFSET;
>> -             b.b_bcount = bbs;
>> -             b.b_bufsize = bbs;
>> -             b.b_resid = bbs;
>> -             b.b_data = bootblk;
>> -             b.b_error = 0;
>> -             b.b_proc = curproc;
>> -             b.b_dev = chunk->src_dev_mm;
>> -             b.b_vp = NULL;
>> -             b.b_iodone = NULL;
>> -             if (bdevvp(chunk->src_dev_mm, &b.b_vp)) {
>> -                     printf("%s: sr_ioctl_installboot: vnode allocation "
>> -                         "failed\n", DEVNAME(sc));
>> -                     goto done;
>> -             }
>> -             if ((b.b_flags & B_READ) == 0)
>> -                     b.b_vp->v_numoutput++;
>> -             LIST_INIT(&b.b_dep);
>> -             VOP_STRATEGY(&b);
>> -             biowait(&b);
>> -             vput(b.b_vp);
>> -
>> -             if (b.b_flags & B_ERROR) {
>> -                     printf("%s: 0x%x i/o error on block %llu while "
>> -                         "writing boot block %d\n", DEVNAME(sc),
>> -                         chunk->src_dev_mm, b.b_blkno, b.b_error);
>> +             if (sr_rw(sc, chunk->src_dev_mm, bootblk, bbs,
>> +                 SR_BOOT_BLOCKS_OFFSET, B_WRITE)) {
>> +                     printf("%s: failed to write boot block\n",
DEVNAME(sc));

>>                       goto done;
>>               }
>>
>> @@ -3252,34 +3262,10 @@ sr_ioctl_installboot(struct sr_softc *sc
>>                   "sr_ioctl_installboot: saving boot loader to %s "
>>                   "(%u bytes)\n", chunk->src_devname, bls);
>>
>> -             bzero(&b, sizeof(b));
>> -             b.b_flags = B_WRITE | B_PHYS;
>> -             b.b_blkno = SR_BOOT_LOADER_OFFSET;
>> -             b.b_bcount = bls;
>> -             b.b_bufsize = bls;
>> -             b.b_resid = bls;
>> -             b.b_data = bootldr;
>> -             b.b_error = 0;
>> -             b.b_proc = curproc;
>> -             b.b_dev = chunk->src_dev_mm;
>> -             b.b_vp = NULL;
>> -             b.b_iodone = NULL;
>> -             if (bdevvp(chunk->src_dev_mm, &b.b_vp)) {
>> -                     printf("%s: sr_ioctl_installboot: vnode alocation "
>> -                         "failed\n", DEVNAME(sc));
>> -                     goto done;
>> -             }
>> -             if ((b.b_flags & B_READ) == 0)
>> -                     b.b_vp->v_numoutput++;
>> -             LIST_INIT(&b.b_dep);
>> -             VOP_STRATEGY(&b);
>> -             biowait(&b);
>> -             vput(b.b_vp);
>> -
>> -             if (b.b_flags & B_ERROR) {
>> -                     printf("%s: 0x%x i/o error on block %llu while "
>> -                         "writing boot blocks %d\n", DEVNAME(sc),
>> -                         chunk->src_dev_mm, b.b_blkno, b.b_error);
>> +             if (sr_rw(sc, chunk->src_dev_mm, bootldr, bls,
>> +                 SR_BOOT_LOADER_OFFSET, B_WRITE)) {
>> +                     printf("%s: failed to write boot loader\n",
>> +                        DEVNAME(sc));
>>                       goto done;
>>               }
>> --
>>
>>    "Stop assuming that systems are secure unless demonstrated insecure;
>>     start assuming that systems are insecure unless designed securely."
>>           - Bruce Schneier

Reply | Threaded
Open this post in threaded view
|

Re: softraid: factor out block I/O code

Marco Peereboom
In reply to this post by Joel Sing-3
I know you guys aren't testing this patch because it doesn't apply.

I attached the correct one so get to work!

Joel's patch:
You'll probably need this as well if you're testing on USB devices.

Factor out the block level I/O code. Also teach it how to handle writes that
are larger than MAXPHYS. This obviously needs some thorough testing since
it has the potential to eat metadata.

Index: softraidvar.h
===================================================================
RCS file: /cvs/src/sys/dev/softraidvar.h,v
retrieving revision 1.96
diff -u -p -r1.96 softraidvar.h
--- softraidvar.h 6 Nov 2010 23:01:56 -0000 1.96
+++ softraidvar.h 6 Jan 2011 12:41:26 -0000
@@ -624,6 +624,8 @@ int sr_check_io_collision(struct sr_wo
 void sr_scsi_done(struct sr_discipline *,
     struct scsi_xfer *);
 int sr_chunk_in_use(struct sr_softc *, dev_t);
+int sr_rw(struct sr_softc *, dev_t, char *, size_t,
+    daddr64_t, long);
 
 /* discipline functions */
 int sr_raid_inquiry(struct sr_workunit *);
Index: softraid.c
===================================================================
RCS file: /cvs/src/sys/dev/softraid.c,v
retrieving revision 1.217
diff -u -p -r1.217 softraid.c
--- softraid.c 20 Dec 2010 17:47:48 -0000 1.217
+++ softraid.c 6 Jan 2011 12:41:26 -0000
@@ -387,57 +387,93 @@ sr_meta_getdevname(struct sr_softc *sc,
 }
 
 int
-sr_meta_rw(struct sr_discipline *sd, dev_t dev, void *md, size_t sz,
-    daddr64_t ofs, long flags)
+sr_rw(struct sr_softc *sc, dev_t dev, char *buf, size_t size, daddr64_t offset,
+    long flags)
 {
- struct sr_softc *sc = sd->sd_sc;
+ struct vnode *vp;
  struct buf b;
+ size_t bufsize;
  int rv = 1;
 
- DNPRINTF(SR_D_META, "%s: sr_meta_rw(0x%x, %p, %d, %llu 0x%x)\n",
-    DEVNAME(sc), dev, md, sz, ofs, flags);
-
- bzero(&b, sizeof(b));
+ DNPRINTF(SR_D_MISC, "%s: sr_rw(0x%x, %p, %d, %llu 0x%x)\n",
+    DEVNAME(sc), dev, buf, size, offset, flags);
 
- if (md == NULL) {
- printf("%s: read invalid metadata pointer\n", DEVNAME(sc));
+ if (bdevvp(dev, &vp)) {
+ printf("%s: sr_rw: failed to allocate vnode\n", DEVNAME(sc));
  goto done;
  }
- b.b_flags = flags | B_PHYS;
- b.b_blkno = ofs;
- b.b_bcount = sz;
- b.b_bufsize = sz;
- b.b_resid = sz;
- b.b_data = md;
- b.b_error = 0;
- b.b_proc = curproc;
- b.b_dev = dev;
- b.b_iodone = NULL;
- if (bdevvp(dev, &b.b_vp)) {
- printf("%s: sr_meta_rw: can't allocate vnode\n", DEVNAME(sc));
- goto done;
- }
- if ((b.b_flags & B_READ) == 0)
- b.b_vp->v_numoutput++;
-
- LIST_INIT(&b.b_dep);
- VOP_STRATEGY(&b);
- biowait(&b);
-
- if (b.b_flags & B_ERROR) {
- printf("%s: 0x%x i/o error on block %llu while reading "
-    "metadata %d\n", DEVNAME(sc), dev, b.b_blkno, b.b_error);
- goto done;
+
+ while (size > 0) {
+
+ DNPRINTF(SR_D_MISC, "%s: buf %p, size %d, offset %llu)\n",
+    DEVNAME(sc), buf, size, offset);
+
+ bufsize = (size > MAXPHYS) ? MAXPHYS : size;
+
+ bzero(&b, sizeof(b));
+
+ b.b_flags = flags | B_PHYS;
+ b.b_proc = curproc;
+ b.b_dev = dev;
+ b.b_iodone = NULL;
+ b.b_error = 0;
+ b.b_blkno = offset;
+ b.b_data = buf;
+ b.b_bcount = bufsize;
+ b.b_bufsize = bufsize;
+ b.b_resid = bufsize;
+ b.b_vp = vp;
+
+ if ((b.b_flags & B_READ) == 0)
+ vp->v_numoutput++;
+
+ LIST_INIT(&b.b_dep);
+ VOP_STRATEGY(&b);
+ biowait(&b);
+
+ if (b.b_flags & B_ERROR) {
+ printf("%s: I/O error %d on dev 0x%x at block %llu\n",
+    DEVNAME(sc), b.b_error, dev, b.b_blkno);
+ goto done;
+ }
+
+ size -= bufsize;
+ buf += bufsize;
+ offset += howmany(bufsize, DEV_BSIZE);
+
  }
+
  rv = 0;
+
 done:
- if (b.b_vp)
- vput(b.b_vp);
+ if (vp)
+ vput(vp);
 
  return (rv);
 }
 
 int
+sr_meta_rw(struct sr_discipline *sd, dev_t dev, void *md, size_t size,
+    daddr64_t offset, long flags)
+{
+ int rv = 1;
+
+ DNPRINTF(SR_D_META, "%s: sr_meta_rw(0x%x, %p, %d, %llu 0x%x)\n",
+    DEVNAME(sd->sd_sc), dev, md, size, offset, flags);
+
+ if (md == NULL) {
+ printf("%s: sr_meta_rw: invalid metadata pointer\n",
+    DEVNAME(sd->sd_sc));
+ goto done;
+ }
+
+ rv = sr_rw(sd->sd_sc, dev, md, size, offset, flags);
+
+done:
+ return (rv);
+}
+
+int
 sr_meta_clear(struct sr_discipline *sd)
 {
  struct sr_softc *sc = sd->sd_sc;
@@ -3167,7 +3203,6 @@ sr_ioctl_installboot(struct sr_softc *sc
  void *bootblk = NULL, *bootldr = NULL;
  struct sr_discipline *sd = NULL;
  struct sr_chunk *chunk;
- struct buf b;
  u_int32_t bbs, bls;
  int rv = EINVAL;
  int i;
@@ -3216,34 +3251,9 @@ sr_ioctl_installboot(struct sr_softc *sc
     "sr_ioctl_installboot: saving boot block to %s "
     "(%u bytes)\n", chunk->src_devname, bbs);
 
- bzero(&b, sizeof(b));
- b.b_flags = B_WRITE | B_PHYS;
- b.b_blkno = SR_BOOT_BLOCKS_OFFSET;
- b.b_bcount = bbs;
- b.b_bufsize = bbs;
- b.b_resid = bbs;
- b.b_data = bootblk;
- b.b_error = 0;
- b.b_proc = curproc;
- b.b_dev = chunk->src_dev_mm;
- b.b_vp = NULL;
- b.b_iodone = NULL;
- if (bdevvp(chunk->src_dev_mm, &b.b_vp)) {
- printf("%s: sr_ioctl_installboot: vnode allocation "
-    "failed\n", DEVNAME(sc));
- goto done;
- }
- if ((b.b_flags & B_READ) == 0)
- b.b_vp->v_numoutput++;
- LIST_INIT(&b.b_dep);
- VOP_STRATEGY(&b);
- biowait(&b);
- vput(b.b_vp);
-
- if (b.b_flags & B_ERROR) {
- printf("%s: 0x%x i/o error on block %llu while "
-    "writing boot block %d\n", DEVNAME(sc),
-    chunk->src_dev_mm, b.b_blkno, b.b_error);
+ if (sr_rw(sc, chunk->src_dev_mm, bootblk, bbs,
+    SR_BOOT_BLOCKS_OFFSET, B_WRITE)) {
+ printf("%s: failed to write boot block\n", DEVNAME(sc));
  goto done;
  }
 
@@ -3252,34 +3262,10 @@ sr_ioctl_installboot(struct sr_softc *sc
     "sr_ioctl_installboot: saving boot loader to %s "
     "(%u bytes)\n", chunk->src_devname, bls);
 
- bzero(&b, sizeof(b));
- b.b_flags = B_WRITE | B_PHYS;
- b.b_blkno = SR_BOOT_LOADER_OFFSET;
- b.b_bcount = bls;
- b.b_bufsize = bls;
- b.b_resid = bls;
- b.b_data = bootldr;
- b.b_error = 0;
- b.b_proc = curproc;
- b.b_dev = chunk->src_dev_mm;
- b.b_vp = NULL;
- b.b_iodone = NULL;
- if (bdevvp(chunk->src_dev_mm, &b.b_vp)) {
- printf("%s: sr_ioctl_installboot: vnode alocation "
-    "failed\n", DEVNAME(sc));
- goto done;
- }
- if ((b.b_flags & B_READ) == 0)
- b.b_vp->v_numoutput++;
- LIST_INIT(&b.b_dep);
- VOP_STRATEGY(&b);
- biowait(&b);
- vput(b.b_vp);
-
- if (b.b_flags & B_ERROR) {
- printf("%s: 0x%x i/o error on block %llu while "
-    "writing boot blocks %d\n", DEVNAME(sc),
-    chunk->src_dev_mm, b.b_blkno, b.b_error);
+ if (sr_rw(sc, chunk->src_dev_mm, bootldr, bls,
+    SR_BOOT_LOADER_OFFSET, B_WRITE)) {
+ printf("%s: failed to write boot loader\n",
+   DEVNAME(sc));
  goto done;
  }
 

--

   "Stop assuming that systems are secure unless demonstrated insecure;
    start assuming that systems are insecure unless designed securely."
          - Bruce Schneier