[Patch] camcontrol - determine the defect list length

Mark Johnston mjohnston at sandvine.com
Tue Oct 26 14:40:59 UTC 2010


>> Hi,
>> 
>> Below is a patch for camcontrol from the tree at my work. It was introduced several years ago to resolve some issues that we were seeing in Adaptec's RAID controller firmware. The change is to the readdefects() function - when sending the request to read the defect list, instead of using a 65000-entry buffer, we wrote a new function to query the drive and determine the size of the defect list before allocating any memory. Apparently, if we send a maximum length longer than the number of bytes in the defect list, the cam call returns an overflow error due to what is probably a firmware bug.
>> 
>> It's probable that this issue has been fixed in Adaptec's firmware for a while now, and that this change is no longer necessary to us. However, it's conceivable that other devices could have the same problem, and as far as I can see, this patch shouldn't introduce any regressions - it essentially consists of sending an additional CAM request before getting the defect list.
>> 
>> I'd like to know whether this patch is worth trying to get committed into FreeBSD. If anyone has some comments or suggestions on the patch, I'd highly appreciate it.
>> 

> Should the whole operation of reading the defect list abort if readdefectlen() fails, or should it just fall back to a modest length and try the operation anyways?  > Also, if readdefectlen() succeeds, how do the defect_list and ccb get freed?

> Scott

Ah, thanks for pointing that out. The patch below fixes the missing frees.

I would argue that there's not much point in retrying the operation if readdefectlen() fails since
readdefectlen() performs essentially the same operations as readdefects() - readdefectlen() will fail if:

1. malloc() returns NULL somewhere,
2. the defect list format isn't specified
3. cam_send_ccb() fails
4. the request defect list format isn't supported
5. there was an error when reading the defect list data

Of these, I don't see any reason that readdefects() would succeed if readdefectlen() fails. cam_send_ccb()
performs a cam ioctl which in turn fails if functions like xpt_find_bus() and xpt_find_target() do. It doesn't
seem likely to me that a second call would succeed if the first failed.

Thanks,
-Mark

--- camcontrol.c@@/main/RELENG_8/RELENG_8_1/svos_8_1/1  2010-10-21 15:39:04.000000000 -0400
+++ camcontrol.c        2010-10-26 10:12:12.000000000 -0400
@@ -2168,7 +2168,7 @@ static int readdefectlen(struct cam_devi
        }
 
        *ret_len = returned_length;
-       return 0;
+
 defect_len_bailout:
 
        if (defect_list != NULL)


More information about the freebsd-scsi mailing list