hptiop: backout ioctl mess
Subsystems (2)
PATCH 1/1 hptiop: backout ioctl mess
The hptiop just got merged with a horrible amount of really bad ioctl code that is against the standards for new scsi drivers. This patch backs it out (and fixes a small bug where scsi_add_host is called to early). We can re-add proper APIs once we agree on them.
Please send this to Linus for 2.6.19 so that the ioctls don't make it into a release.
drivers/scsi/hptiop.c +8 -558
@@ -45,10 +45,6 @@static const char driver_name_long[] ="RocketRAID 3xxx SATA Controller driver" ;static const char driver_ver[] ="v1.0 (060426)" ; -static DEFINE_SPINLOCK (hptiop_hba_list_lock); -static LIST_HEAD (hptiop_hba_list); -static int hptiop_cdev_major =-1 ; -static void hptiop_host_request_callback (struct hptiop_hba *hba,u32 tag);static void hptiop_iop_request_callback (struct hptiop_hba *hba,u32 tag);static void hptiop_message_callback (struct hptiop_hba *hba,u32 msg);
@@ -620,532 +616,11 @@return queue_depth; } -struct hptiop_getinfo { -char __user *buffer; -loff_t buflength; -loff_t bufoffset; -loff_t buffillen; -loff_t filpos; -}; - -static void hptiop_copy_mem_info (struct hptiop_getinfo *pinfo, -char *data,int datalen) -{ -if (pinfo->filpos < pinfo->bufoffset) { -if (pinfo->filpos + datalen <= pinfo->bufoffset) { - pinfo->filpos += datalen; -return ; - }else { - data += (pinfo->bufoffset - pinfo->filpos); - datalen -= (pinfo->bufoffset - pinfo->filpos); - pinfo->filpos = pinfo->bufoffset; - } - } - - pinfo->filpos += datalen; -if (pinfo->buffillen == pinfo->buflength) -return ; - -if (pinfo->buflength - pinfo->buffillen < datalen) - datalen = pinfo->buflength - pinfo->buffillen; - -if (copy_to_user (pinfo->buffer + pinfo->buffillen, data, datalen)) -return ; - - pinfo->buffillen += datalen; -} - -static int hptiop_copy_info (struct hptiop_getinfo *pinfo,char *fmt, ...) -{ -va_list args; -char buf[128 ]; -int len; - -va_start (args, fmt); - len =vsnprintf (buf,sizeof (buf), fmt, args); -va_end (args); -hptiop_copy_mem_info (pinfo, buf, len); -return len; -} - -static void hptiop_ioctl_done (struct hpt_ioctl_k *arg) -{ - arg->done =NULL ; -wake_up (&arg->hba->ioctl_wq); -} - -static void hptiop_do_ioctl (struct hpt_ioctl_k *arg) -{ -struct hptiop_hba *hba = arg->hba; -u32 val; -struct hpt_iop_request_ioctl_command __iomem *req; -int ioctl_retry =0 ; - -dprintk ("scsi%d: hptiop_do_ioctl\n" , hba->host->host_no); - -/* -* check (in + out) buff size from application. -* outbuf must be dword aligned. -*/ -if (((arg->inbuf_size +3 ) & ~3 ) + arg->outbuf_size > - hba->max_request_size - -sizeof (struct hpt_iop_request_header ) - -4 *sizeof (u32)) { -dprintk ("scsi%d: ioctl buf size (%d/%d) is too large\n" , - hba->host->host_no, - arg->inbuf_size, arg->outbuf_size); - arg->result =HPT_IOCTL_RESULT_FAILED ; -return ; - } - -retry: -spin_lock_irq (hba->host->host_lock); - - val =readl (&hba->iop->inbound_queue); -if (val ==IOPMU_QUEUE_EMPTY ) { -spin_unlock_irq (hba->host->host_lock); -dprintk ("scsi%d: no free req for ioctl\n" , hba->host->host_no); - arg->result =-1 ; -return ; - } - - req = (struct hpt_iop_request_ioctl_command __iomem *) - ((unsigned long )hba->iop + val); - -writel (HPT_CTL_CODE_LINUX_TO_IOP (arg->ioctl_code), - &req->ioctl_code); -writel (arg->inbuf_size, &req->inbuf_size); -writel (arg->outbuf_size, &req->outbuf_size); - -/* -* use the buffer on the IOP local memory first, then copy it -* back to host. -* the caller's request buffer shoudl be little-endian. -*/ -if (arg->inbuf_size) -memcpy_toio (req->buf, arg->inbuf, arg->inbuf_size); - -/* correct the controller ID for IOP */ -if ((arg->ioctl_code ==HPT_IOCTL_GET_CHANNEL_INFO || - arg->ioctl_code ==HPT_IOCTL_GET_CONTROLLER_INFO_V2 || - arg->ioctl_code ==HPT_IOCTL_GET_CONTROLLER_INFO ) - && arg->inbuf_size >=sizeof (u32)) -writel (0 , req->buf); - -writel (IOP_REQUEST_TYPE_IOCTL_COMMAND , &req->header.type); -writel (0 , &req->header.flags); -writel (offsetof(struct hpt_iop_request_ioctl_command , buf) - + arg->inbuf_size, &req->header.size); -writel ((u32 )(unsigned long )arg, &req->header.context); -writel (BITS_PER_LONG >32 ? (u32)((unsigned long )arg>>32 ) :0 , - &req->header.context_hi32); -writel (IOP_RESULT_PENDING , &req->header.result); - - arg->result =HPT_IOCTL_RESULT_FAILED ; - arg->done = hptiop_ioctl_done; - -writel (val, &hba->iop->inbound_queue); -hptiop_pci_posting_flush (hba->iop); - -spin_unlock_irq (hba->host->host_lock); - -wait_event_timeout (hba->ioctl_wq, arg->done ==NULL ,60 *HZ ); - -if (arg->done !=NULL ) { -hptiop_reset_hba (hba); -if (ioctl_retry++ <3 ) - goto retry; - } - -dprintk ("hpt_iop_ioctl %x result %d\n" , - arg->ioctl_code, arg->result); -} - -static int __hpt_do_ioctl (struct hptiop_hba *hba,u32 code,void *inbuf, -u32 insize,void *outbuf,u32 outsize) -{ -struct hpt_ioctl_k arg; - arg.hba = hba; - arg.ioctl_code = code; - arg.inbuf = inbuf; - arg.outbuf = outbuf; - arg.inbuf_size = insize; - arg.outbuf_size = outsize; - arg.bytes_returned =NULL ; -hptiop_do_ioctl (&arg); -return arg.result; -} - -static inline int hpt_id_valid (__le32 id) -{ -return id !=0 && id !=cpu_to_le32 (0xffffffff ); -} - -static int hptiop_get_controller_info (struct hptiop_hba *hba, -struct hpt_controller_info *pinfo) -{ -int id =0 ; - -return __hpt_do_ioctl (hba,HPT_IOCTL_GET_CONTROLLER_INFO , - &id,sizeof (int ), pinfo,sizeof (*pinfo)); -} - - -static int hptiop_get_channel_info (struct hptiop_hba *hba,int bus, -struct hpt_channel_info *pinfo) -{ -u32 ids[2 ]; - - ids[0 ] =0 ; - ids[1 ] = bus; -return __hpt_do_ioctl (hba,HPT_IOCTL_GET_CHANNEL_INFO , - ids,sizeof (ids), pinfo,sizeof (*pinfo)); - -} - -static int hptiop_get_logical_devices (struct hptiop_hba *hba, -__le32 *pids,int maxcount) -{ -int i; -u32 count = maxcount -1 ; - -if (__hpt_do_ioctl (hba,HPT_IOCTL_GET_LOGICAL_DEVICES , - &count,sizeof (u32), - pids,sizeof (u32) * maxcount)) -return -1 ; - - maxcount =le32_to_cpu (pids[0 ]); -for (i =0 ; i < maxcount; i++) - pids[i] = pids[i+1 ]; - -return maxcount; -} - -static int hptiop_get_device_info_v3 (struct hptiop_hba *hba,__le32 id, -struct hpt_logical_device_info_v3 *pinfo) -{ -return __hpt_do_ioctl (hba,HPT_IOCTL_GET_DEVICE_INFO_V3 , - &id,sizeof (u32), - pinfo,sizeof (*pinfo)); -} - -static const char *get_array_status (struct hpt_logical_device_info_v3 *devinfo) -{ -static char s[64 ]; -u32 flags =le32_to_cpu (devinfo->u.array.flags); -u32 trans_prog =le32_to_cpu (devinfo->u.array.transforming_progress); -u32 reb_prog =le32_to_cpu (devinfo->u.array.rebuilding_progress); - -if (flags &ARRAY_FLAG_DISABLED ) -return "Disabled" ; -else if (flags &ARRAY_FLAG_TRANSFORMING ) -sprintf (s,"Expanding/Migrating %d.%d%%%s%s" , - trans_prog /100 , - trans_prog %100 , - (flags & (ARRAY_FLAG_NEEDBUILDING |ARRAY_FLAG_BROKEN ))? -", Critical" :"" , - ((flags &ARRAY_FLAG_NEEDINITIALIZING ) && - !(flags &ARRAY_FLAG_REBUILDING ) && - !(flags &ARRAY_FLAG_INITIALIZING ))? -", Unintialized" :"" ); -else if ((flags &ARRAY_FLAG_BROKEN ) && - devinfo->u.array.array_type !=AT_RAID6 ) -return "Critical" ; -else if (flags &ARRAY_FLAG_REBUILDING ) -sprintf (s, - (flags &ARRAY_FLAG_NEEDINITIALIZING )? -"%sBackground initializing %d.%d%%" : -"%sRebuilding %d.%d%%" , - (flags &ARRAY_FLAG_BROKEN )?"Critical, " :"" , - reb_prog /100 , - reb_prog %100 ); -else if (flags &ARRAY_FLAG_VERIFYING ) -sprintf (s,"%sVerifying %d.%d%%" , - (flags &ARRAY_FLAG_BROKEN )?"Critical, " :"" , - reb_prog /100 , - reb_prog %100 ); -else if (flags &ARRAY_FLAG_INITIALIZING ) -sprintf (s,"%sForground initializing %d.%d%%" , - (flags &ARRAY_FLAG_BROKEN )?"Critical, " :"" , - reb_prog /100 , - reb_prog %100 ); -else if (flags &ARRAY_FLAG_NEEDTRANSFORM ) -sprintf (s,"%s%s%s" ,"Need Expanding/Migrating" , - (flags &ARRAY_FLAG_BROKEN )?"Critical, " :"" , - ((flags &ARRAY_FLAG_NEEDINITIALIZING ) && - !(flags &ARRAY_FLAG_REBUILDING ) && - !(flags &ARRAY_FLAG_INITIALIZING ))? -", Unintialized" :"" ); -else if (flags &ARRAY_FLAG_NEEDINITIALIZING && - !(flags &ARRAY_FLAG_REBUILDING ) && - !(flags &ARRAY_FLAG_INITIALIZING )) -sprintf (s,"%sUninitialized" , - (flags &ARRAY_FLAG_BROKEN )?"Critical, " :"" ); -else if ((flags &ARRAY_FLAG_NEEDBUILDING ) || - (flags &ARRAY_FLAG_BROKEN )) -return "Critical" ; -else -return "Normal" ; -return s; -} - -static void hptiop_dump_devinfo (struct hptiop_hba *hba, -struct hptiop_getinfo *pinfo,__le32 id,int indent) -{ -struct hpt_logical_device_info_v3 devinfo; -int i; -u64 capacity; - -for (i =0 ; i < indent; i++) -hptiop_copy_info (pinfo,"\t" ); - -if (hptiop_get_device_info_v3 (hba, id, &devinfo)) { -hptiop_copy_info (pinfo,"unknown\n" ); -return ; - } - -switch (devinfo.type) { - -case LDT_DEVICE : { -struct hd_driveid *driveid; -u32 flags =le32_to_cpu (devinfo.u.device.flags); - - driveid = (struct hd_driveid *)devinfo.u.device.ident; -/* model[] is 40 chars long, but we just want 20 chars here */ - driveid->model[20 ] =0 ; - -if (indent) -if (flags &DEVICE_FLAG_DISABLED ) -hptiop_copy_info (pinfo,"Missing\n" ); -else -hptiop_copy_info (pinfo,"CH%d %s\n" , - devinfo.u.device.path_id +1 , - driveid->model); -else { - capacity =le64_to_cpu (devinfo.capacity) *512 ; -do_div (capacity,1000000 ); -hptiop_copy_info (pinfo, -"CH%d %s, %lluMB, %s %s%s%s%s\n" , - devinfo.u.device.path_id +1 , - driveid->model, - capacity, - (flags &DEVICE_FLAG_DISABLED )? -"Disabled" :"Normal" , - devinfo.u.device.read_ahead_enabled? -"[RA]" :"" , - devinfo.u.device.write_cache_enabled? -"[WC]" :"" , - devinfo.u.device.TCQ_enabled? -"[TCQ]" :"" , - devinfo.u.device.NCQ_enabled? -"[NCQ]" :"" - ); - } -break ; - } - -case LDT_ARRAY : -if (devinfo.target_id !=INVALID_TARGET_ID ) -hptiop_copy_info (pinfo,"[DISK %d_%d] " , - devinfo.vbus_id, devinfo.target_id); - - capacity =le64_to_cpu (devinfo.capacity) *512 ; -do_div (capacity,1000000 ); -hptiop_copy_info (pinfo,"%s (%s), %lluMB, %s\n" , - devinfo.u.array.name, - devinfo.u.array.array_type==AT_RAID0 ?"RAID0" : - devinfo.u.array.array_type==AT_RAID1 ?"RAID1" : - devinfo.u.array.array_type==AT_RAID5 ?"RAID5" : - devinfo.u.array.array_type==AT_RAID6 ?"RAID6" : - devinfo.u.array.array_type==AT_JBOD ?"JBOD" : -"unknown" , - capacity, -get_array_status (&devinfo)); -for (i =0 ; i < devinfo.u.array.ndisk; i++) { -if (hpt_id_valid (devinfo.u.array.members[i])) { -if (cpu_to_le16 (1 <<i) & - devinfo.u.array.critical_members) -hptiop_copy_info (pinfo,"\t*" ); -hptiop_dump_devinfo (hba, pinfo, - devinfo.u.array.members[i], indent+1 ); - } -else -hptiop_copy_info (pinfo,"\tMissing\n" ); - } -if (id == devinfo.u.array.transform_source) { -hptiop_copy_info (pinfo,"\tExpanding/Migrating to:\n" ); -hptiop_dump_devinfo (hba, pinfo, - devinfo.u.array.transform_target, indent+1 ); - } -break ; - } -} -static ssize_t hptiop_show_version (struct class_device *class_dev,char *buf) {return snprintf (buf,PAGE_SIZE ,"%s\n" , driver_ver); } -static ssize_t hptiop_cdev_read (struct file *filp,char __user *buf, -size_t count,loff_t *ppos) -{ -struct hptiop_hba *hba = filp->private_data; -struct hptiop_getinfo info; -int i, j, ndev; -struct hpt_controller_info con_info; -struct hpt_channel_info chan_info; -__le32 ids[32 ]; - - info.buffer = buf; - info.buflength = count; - info.bufoffset = ppos ? *ppos :0 ; - info.filpos =0 ; - info.buffillen =0 ; - -if (hptiop_get_controller_info (hba, &con_info)) -return -EIO ; - -for (i =0 ; i < con_info.num_buses; i++) { -if (hptiop_get_channel_info (hba, i, &chan_info) ==0 ) { -if (hpt_id_valid (chan_info.devices[0 ])) -hptiop_dump_devinfo (hba, &info, - chan_info.devices[0 ],0 ); -if (hpt_id_valid (chan_info.devices[1 ])) -hptiop_dump_devinfo (hba, &info, - chan_info.devices[1 ],0 ); - } - } - - ndev =hptiop_get_logical_devices (hba, ids, -sizeof (ids) /sizeof (ids[0 ])); - -/* -* if hptiop_get_logical_devices fails, ndev==-1 and it just -* output nothing here -*/ -for (j =0 ; j < ndev; j++) -hptiop_dump_devinfo (hba, &info, ids[j],0 ); - -if (ppos) - *ppos += info.buffillen; - -return info.buffillen; -} - -static int hptiop_cdev_ioctl (struct inode *inode,struct file *file, -unsigned int cmd,unsigned long arg) -{ -struct hptiop_hba *hba = file->private_data; -struct hpt_ioctl_u ioctl_u; -struct hpt_ioctl_k ioctl_k; -u32 bytes_returned; -int err = -EINVAL ; - -if (copy_from_user (&ioctl_u, - (void __user *)arg,sizeof (struct hpt_ioctl_u ))) -return -EINVAL ; - -if (ioctl_u.magic !=HPT_IOCTL_MAGIC ) -return -EINVAL ; - - ioctl_k.ioctl_code = ioctl_u.ioctl_code; - ioctl_k.inbuf =NULL ; - ioctl_k.inbuf_size = ioctl_u.inbuf_size; - ioctl_k.outbuf =NULL ; - ioctl_k.outbuf_size = ioctl_u.outbuf_size; - ioctl_k.hba = hba; - ioctl_k.bytes_returned = &bytes_returned; - -/* verify user buffer */ -if ((ioctl_k.inbuf_size && !access_ok (VERIFY_READ , - ioctl_u.inbuf, ioctl_k.inbuf_size)) || - (ioctl_k.outbuf_size && !access_ok (VERIFY_WRITE , - ioctl_u.outbuf, ioctl_k.outbuf_size)) || - (ioctl_u.bytes_returned && !access_ok (VERIFY_WRITE , - ioctl_u.bytes_returned,sizeof (u32))) || - ioctl_k.inbuf_size + ioctl_k.outbuf_size >0x10000 ) { - -dprintk ("scsi%d: got bad user address\n" , hba->host->host_no); -return -EINVAL ; - } - -/* map buffer to kernel. */ -if (ioctl_k.inbuf_size) { - ioctl_k.inbuf =kmalloc (ioctl_k.inbuf_size,GFP_KERNEL ); -if (!ioctl_k.inbuf) { -dprintk ("scsi%d: fail to alloc inbuf\n" , - hba->host->host_no); - err = -ENOMEM ; - goto err_exit; - } - -if (copy_from_user (ioctl_k.inbuf, - ioctl_u.inbuf, ioctl_k.inbuf_size)) { - goto err_exit; - } - } - -if (ioctl_k.outbuf_size) { - ioctl_k.outbuf =kmalloc (ioctl_k.outbuf_size,GFP_KERNEL ); -if (!ioctl_k.outbuf) { -dprintk ("scsi%d: fail to alloc outbuf\n" , - hba->host->host_no); - err = -ENOMEM ; - goto err_exit; - } - } - -hptiop_do_ioctl (&ioctl_k); - -if (ioctl_k.result ==HPT_IOCTL_RESULT_OK ) { -if (ioctl_k.outbuf_size && -copy_to_user (ioctl_u.outbuf, - ioctl_k.outbuf, ioctl_k.outbuf_size)) - goto err_exit; - -if (ioctl_u.bytes_returned && -copy_to_user (ioctl_u.bytes_returned, - &bytes_returned,sizeof (u32))) - goto err_exit; - - err =0 ; - } - -err_exit: -kfree (ioctl_k.inbuf); -kfree (ioctl_k.outbuf); - -return err; -} - -static int hptiop_cdev_open (struct inode *inode,struct file *file) -{ -struct hptiop_hba *hba; -unsigned i =0 , minor =iminor (inode); -int ret = -ENODEV ; - -spin_lock (&hptiop_hba_list_lock); -list_for_each_entry (hba, &hptiop_hba_list, link) { -if (i == minor) { - file->private_data = hba; - ret =0 ; - goto out; - } - i++; - } - -out: -spin_unlock (&hptiop_hba_list_lock); -return ret; -} - -static struct file_operations hptiop_cdev_fops = { - .owner =THIS_MODULE , - .read = hptiop_cdev_read, - .ioctl = hptiop_cdev_ioctl, - .open = hptiop_cdev_open, -}; -static ssize_t hptiop_show_fw_version (struct class_device *class_dev,char *buf) {struct Scsi_Host *host =class_to_shost (class_dev);
@@ -1296,19 +771,13 @@ goto unmap_pci_bar; } -if (scsi_add_host (host, &pcidev->dev)) { -printk (KERN_ERR "scsi%d: scsi_add_host failed\n" , - hba->host->host_no); - goto unmap_pci_bar; - } -pci_set_drvdata (pcidev, host);if (request_irq (pcidev->irq, hptiop_intr,IRQF_SHARED , driver_name, hba)) {printk (KERN_ERR "scsi%d: request irq %d failed\n" , hba->host->host_no, pcidev->irq); - goto remove_scsi_host; + goto unmap_pci_bar; }/* Allocate request mem */
@@ -1355,9 +824,12 @@if (hptiop_initialize_iop (hba)) goto free_request_mem; -spin_lock (&hptiop_hba_list_lock); -list_add_tail (&hba->link, &hptiop_hba_list); -spin_unlock (&hptiop_hba_list_lock); +if (scsi_add_host (host, &pcidev->dev)) { +printk (KERN_ERR "scsi%d: scsi_add_host failed\n" , + hba->host->host_no); + goto free_request_mem; + } +scsi_scan_host (host);
@@ -1372,9 +844,6 @@ free_request_irq:free_irq (hba->pcidev->irq, hba); -remove_scsi_host: -scsi_remove_host (host); - unmap_pci_bar:iounmap (hba->iop);
@@ -1422,10 +891,6 @@scsi_remove_host (host); -spin_lock (&hptiop_hba_list_lock); -list_del_init (&hba->link); -spin_unlock (&hptiop_hba_list_lock); -hptiop_shutdown (pcidev);free_irq (hba->pcidev->irq, hba);
@@ -1462,27 +927,12 @@static int __init hptiop_module_init (void ) { -int error; -printk (KERN_INFO "%s %s\n" , driver_name_long, driver_ver); - - error =pci_register_driver (&hptiop_pci_driver); -if (error <0 ) -return error; - - hptiop_cdev_major =register_chrdev (0 ,"hptiop" , &hptiop_cdev_fops); -if (hptiop_cdev_major <0 ) { -printk (KERN_WARNING "unable to register hptiop device.\n" ); -return hptiop_cdev_major; - } - -return 0 ; +return pci_register_driver (&hptiop_pci_driver); } static void __exit hptiop_module_exit(void) { -dprintk ("hptiop_module_exit\n" ); -unregister_chrdev (hptiop_cdev_major,"hptiop" );pci_unregister_driver (&hptiop_pci_driver); }
HighPoint Linux Team
maintainerQuoted 4 lines · Christoph Hellwig
The hptiop just got merged with a horrible amount of really bad ioctl code that is against the standards for new scsi drivers. This patch backs it out (and fixes a small bug where
scsi_add_hostis called to early). We can re-add proper APIs once we agree on them.
What is the standard implemetation of private ioctls for scsi drivers? Thanks.
HighPoint Linux Team
Quoted 7 lines · HighPoint Linux Team
Quoted 4 lines · Christoph Hellwig
The hptiop just got merged with a horrible amount of really bad ioctl code that is against the standards for new scsi drivers. This patch backs it out (and fixes a small bug where
scsi_add_hostis called to early). We can re-add proper APIs once we agree on them.What is the standard implemetation of private ioctls for scsi drivers?
Don't get caught.
You might try something creative like:
#define foo ioctl
More seriously see:
http://lwn.net/Articles/191653/
for a recent discussion.
Doug Gilbert
Quoted 8 lines · HighPoint Linux Team
Quoted 4 lines · Christoph Hellwig
The hptiop just got merged with a horrible amount of really bad ioctl code that is against the standards for new scsi drivers. This patch backs it out (and fixes a small bug where
scsi_add_hostis called to early). We can re-add proper APIs once we agree on them.What is the standard implemetation of private ioctls for scsi drivers? Thanks.
Hi,
the real answer is: try really hard to use generic ioctls for your functionality. What exactly do you need private ioctls for? It's quite possible that there is a generic way to do the same thing already, or something that can easily be extended to do what you need...
Greetings,
Arjan van de Ven
Signature · 1 line
if you want to mail me at work (you don't), use arjan (at) linux.intel.com
HighPoint Linux Team
maintainerQuoted 9 lines · Arjan van de Ven
Quoted 8 lines
Quoted 4 lines · Christoph Hellwig
The hptiop just got merged with a horrible amount of really bad ioctl code that is against the standards for new scsi drivers. This patch backs it out (and fixes a small bug where
scsi_add_hostis called to early). We can re-add proper APIs once we agree on them.What is the standard implemetation of private ioctls for scsi drivers? Thanks.
Quoted 6 lines
Hi,
the real answer is: try really hard to use generic ioctls for your functionality. What exactly do you need private ioctls for? It's quite possible that there is a generic way to do the same thing already, or something that can easily be extended to do what you need...
We use private ioctls for RAID management. The driver registers a char device for application to access the IOP. Several scsi drivers (aacraid, 3w-9xxx, megaraid) use register_chrdev() for ioctls - is this deprecated?
Another way is using scsi_host_template.ioctl interface but application has no device node to open when there is no device on the controller. Same problem on using sg_ioctl or scsi passthrough interface.
HighPoint Linux Team
Quoted 6 lines · Arjan van de Ven
Quoted 5 lines · HighPoint Linux Team
Quoted 4 lines · Christoph Hellwig
The hptiop just got merged with a horrible amount of really bad ioctl code that is against the standards for new scsi drivers. This patch backs it out (and fixes a small bug where
scsi_add_hostis called to early). We can re-add proper APIs once we agree on them.
Quoted 7 lines
Quoted 2 lines
What is the standard implemetation of private ioctls for scsi drivers? Thanks.
Hi,
the real answer is: try really hard to use generic ioctls for your functionality.
Now there is an idea :-) Pick a vendor specific opcode like 0xff (and maybe have a module parameter to override it to something else) and then use the SG_IO ioctl to send "highpoint" specific SCSI commands that provide the same functionality within the LLD that Christoph just stripped out.
One problem with this approach is interacting with the LLD in the absence of any (real) SCSI devices. It shouldn't be too hard to trick the mid level into believing there are SCSI devices connected (see the scsi_debug driver). May I suggest a LUN slightly above the WLUN range (e.g. 49409+10). The fake device could then be created with:
# cd /sys/class/scsi_host/host0 # echo "- - 49419" > scan
The fake device would need to respond to an INQUIRY and a few other simple SCSI commands to keep the mid-level happy. May I also suggest the "processor" peripheral device type which stops other ULDs getting involved. Then you can talk to the LLD via a sg device node.
What exactly do you need private ioctls for?
Quoted 2 lines
It's quite possible that there is a generic way to do the same thing already, or something that can easily be extended to do what you need...
Is the above generic enough?
Doug Gilbert
Quoted 4 lines · Douglas Gilbert
Quoted 2 lines
It's quite possible that there is a generic way to do the same thing already, or something that can easily be extended to do what you need...
Is the above generic enough?
obviously not since the semantics still don't make sense on a higher up level.
Signature · 1 line
if you want to mail me at work (you don't), use arjan (at) linux.intel.com
Quoted 4 lines · Christoph Hellwig
The hptiop just got merged with a horrible amount of really bad ioctl code that is against the standards for new scsi drivers. This patch backs it out (and fixes a small bug where
scsi_add_hostis called to early). We can re-add proper APIs once we agree on them.
I'm assuming the model you want to use is megaraid_sas, which only has a couple of defined ioctls?
Quoted 2 lines
Please send this to Linus for 2.6.19 so that the ioctls don't make it into a release.
OK, provided you take the lead in sorting out the ioctl infrastructure for this driver ...
James
HighPoint Linux Team
maintainerQuoted 9 lines · James Bottomley
Quoted 4 lines · Christoph Hellwig
The hptiop just got merged with a horrible amount of really bad ioctl code that is against the standards for new scsi drivers. This patch backs it out (and fixes a small bug where
scsi_add_hostis called to early). We can re-add proper APIs once we agree on them.I'm assuming the model you want to use is
megaraid_sas, which only has a couple of defined ioctls?
Quoted 8 lines
Quoted 2 lines
Please send this to Linus for 2.6.19 so that the ioctls don't make it into a release.
OK, provided you take the lead in sorting out the ioctl infrastructure for this driver ...
James
Is there a suggested solution now? We just want RAID management functions to work on this driver. Is it ok to use register_chrdev() for the management interface like megaraid_sas?
HighPoint Linux Team