* [PATCH 1/2] clean up blktap and remove private structure
2006-09-27 16:16 [PATCH 0/2] clean up blktap and change to use a dynamic major Steven Rostedt
@ 2006-09-27 16:19 ` Steven Rostedt
2006-09-28 19:05 ` Andrew Warfield
2006-09-27 16:21 ` [PATCH 2/2] have blktap use a dynamic major Steven Rostedt
1 sibling, 1 reply; 5+ messages in thread
From: Steven Rostedt @ 2006-09-27 16:19 UTC (permalink / raw)
To: xen-devel; +Cc: andrew.warfield
[-- Attachment #1: Type: text/plain, Size: 341 bytes --]
This patch cleans up the blktap.c code to make it form to the Linux
coding style a little better.
It also removes the private data structure that is only used to store
the index of the tabfds descriptor. Instead the filp->private_data now
points to the descriptor itself.
-- Steve
Signed-off-by: Steven Rostedt <srostedt@redhat.com>
[-- Attachment #2: xen-linux-blktap-cleanup.patch --]
[-- Type: text/x-patch, Size: 8542 bytes --]
diff -r c4f3f719d997 linux-2.6-xen-sparse/drivers/xen/blktap/blktap.c
--- a/linux-2.6-xen-sparse/drivers/xen/blktap/blktap.c Sat Sep 23 14:54:58 2006 +0100
+++ b/linux-2.6-xen-sparse/drivers/xen/blktap/blktap.c Wed Sep 27 10:37:03 2006 -0400
@@ -102,17 +102,11 @@ typedef struct tap_blkif {
blkif_t *blkif; /*Associate blkif with tapdev */
} tap_blkif_t;
-/*Private data struct associated with the inode*/
-typedef struct private_info {
- int idx;
-} private_info_t;
-
/*Data struct handed back to userspace for tapdisk device to VBD mapping*/
typedef struct domid_translate {
unsigned short domid;
unsigned short busid;
} domid_translate_t ;
-
static domid_translate_t translate_domid[MAX_TAP_DEV];
static tap_blkif_t *tapfds[MAX_TAP_DEV];
@@ -200,7 +194,7 @@ static struct grant_handle_pair
+ (_i)])
-static int blktap_read_ufe_ring(int idx); /*local prototypes*/
+static int blktap_read_ufe_ring(tap_blkif_t *info); /*local prototypes*/
#define BLKTAP_MINOR 0 /*/dev/xen/blktap resides at device number
major=254, minor numbers begin at 0 */
@@ -264,7 +258,8 @@ static inline int GET_NEXT_REQ(unsigned
{
int i;
for (i = 0; i < MAX_PENDING_REQS; i++)
- if (idx_map[i] == INVALID_REQ) return i;
+ if (idx_map[i] == INVALID_REQ)
+ return i;
return INVALID_REQ;
}
@@ -369,9 +364,8 @@ void signal_tapdisk(int idx)
info = tapfds[idx];
if ( (idx > 0) && (idx < MAX_TAP_DEV) && (info->pid > 0) ) {
ptask = find_task_by_pid(info->pid);
- if (ptask) {
+ if (ptask)
info->status = CLEANSHUTDOWN;
- }
}
info->blkif = NULL;
return;
@@ -382,7 +376,6 @@ static int blktap_open(struct inode *ino
blkif_sring_t *sring;
int idx = iminor(inode) - BLKTAP_MINOR;
tap_blkif_t *info;
- private_info_t *prv;
int i;
if (tapfds[idx] == NULL) {
@@ -410,9 +403,7 @@ static int blktap_open(struct inode *ino
SHARED_RING_INIT(sring);
FRONT_RING_INIT(&info->ufe_ring, sring, PAGE_SIZE);
- prv = kzalloc(sizeof(private_info_t),GFP_KERNEL);
- prv->idx = idx;
- filp->private_data = prv;
+ filp->private_data = info;
info->vma = NULL;
info->idx_map = kmalloc(sizeof(unsigned long) * MAX_PENDING_REQS,
@@ -433,17 +424,16 @@ static int blktap_open(struct inode *ino
static int blktap_release(struct inode *inode, struct file *filp)
{
- int idx = iminor(inode) - BLKTAP_MINOR;
- tap_blkif_t *info;
-
- if (tapfds[idx] == NULL) {
+ tap_blkif_t *info = filp->private_data;
+
+ /* can this ever happen? - sdr */
+ if (!info) {
WPRINTK("Trying to free device that doesn't exist "
- "[/dev/xen/blktap%d]\n",idx);
+ "[/dev/xen/blktap%d]\n",iminor(inode) - BLKTAP_MINOR);
return -1;
}
- info = tapfds[idx];
info->dev_inuse = 0;
- DPRINTK("Freeing device [/dev/xen/blktap%d]\n",idx);
+ DPRINTK("Freeing device [/dev/xen/blktap%d]\n",info->minor);
/* Free the ring page. */
ClearPageReserved(virt_to_page(info->ufe_ring.sring));
@@ -457,8 +447,6 @@ static int blktap_release(struct inode *
info->vma = NULL;
}
- if (filp->private_data) kfree(filp->private_data);
-
if ( (info->status != CLEANSHUTDOWN) && (info->blkif != NULL) ) {
kthread_stop(info->blkif->xenblkd);
info->blkif->xenblkd = NULL;
@@ -491,16 +479,12 @@ static int blktap_mmap(struct file *filp
int size;
struct page **map;
int i;
- private_info_t *prv;
- tap_blkif_t *info;
-
- /*Retrieve the dev info*/
- prv = (private_info_t *)filp->private_data;
- if (prv == NULL) {
+ tap_blkif_t *info = filp->private_data;
+
+ if (info == NULL) {
WPRINTK("blktap: mmap, retrieving idx failed\n");
return -ENOMEM;
}
- info = tapfds[prv->idx];
vma->vm_flags |= VM_RESERVED;
vma->vm_ops = &blktap_vm_ops;
@@ -556,20 +540,17 @@ static int blktap_ioctl(struct inode *in
static int blktap_ioctl(struct inode *inode, struct file *filp,
unsigned int cmd, unsigned long arg)
{
- int idx = iminor(inode) - BLKTAP_MINOR;
+ tap_blkif_t *info = filp->private_data;
+
switch(cmd) {
case BLKTAP_IOCTL_KICK_FE:
{
/* There are fe messages to process. */
- return blktap_read_ufe_ring(idx);
+ return blktap_read_ufe_ring(info);
}
case BLKTAP_IOCTL_SETMODE:
{
- tap_blkif_t *info = tapfds[idx];
-
- if ( (idx > 0) && (idx < MAX_TAP_DEV)
- && (tapfds[idx] != NULL) )
- {
+ if (info) {
if (BLKTAP_MODE_VALID(arg)) {
info->mode = arg;
/* XXX: may need to flush rings here. */
@@ -582,11 +563,7 @@ static int blktap_ioctl(struct inode *in
}
case BLKTAP_IOCTL_PRINT_IDXS:
{
- tap_blkif_t *info = tapfds[idx];
-
- if ( (idx > 0) && (idx < MAX_TAP_DEV)
- && (tapfds[idx] != NULL) )
- {
+ if (info) {
printk("User Rings: \n-----------\n");
printk("UF: rsp_cons: %2d, req_prod_prv: %2d "
"| req_prod: %2d, rsp_prod: %2d\n",
@@ -599,11 +576,7 @@ static int blktap_ioctl(struct inode *in
}
case BLKTAP_IOCTL_SENDPID:
{
- tap_blkif_t *info = tapfds[idx];
-
- if ( (idx > 0) && (idx < MAX_TAP_DEV)
- && (tapfds[idx] != NULL) )
- {
+ if (info) {
info->pid = (pid_t)arg;
DPRINTK("blktap: pid received %d\n",
info->pid);
@@ -631,9 +604,12 @@ static int blktap_ioctl(struct inode *in
case BLKTAP_IOCTL_FREEINTF:
{
unsigned long dev = arg;
- tap_blkif_t *info = NULL;
-
- if ( (dev > 0) && (dev < MAX_TAP_DEV) ) info = tapfds[dev];
+
+ /* Looking at another device */
+ info = NULL;
+
+ if ( (dev > 0) && (dev < MAX_TAP_DEV) )
+ info = tapfds[dev];
if ( (info != NULL) && (info->dev_pending) )
info->dev_pending = 0;
@@ -642,12 +618,17 @@ static int blktap_ioctl(struct inode *in
case BLKTAP_IOCTL_MINOR:
{
unsigned long dev = arg;
- tap_blkif_t *info = NULL;
+
+ /* Looking at another device */
+ info = NULL;
- if ( (dev > 0) && (dev < MAX_TAP_DEV) ) info = tapfds[dev];
+ if ( (dev > 0) && (dev < MAX_TAP_DEV) )
+ info = tapfds[dev];
- if (info != NULL) return info->minor;
- else return -1;
+ if (info != NULL)
+ return info->minor;
+ else
+ return -1;
}
case BLKTAP_IOCTL_MAJOR:
return BLKTAP_DEV_MAJOR;
@@ -662,23 +643,20 @@ static int blktap_ioctl(struct inode *in
return -ENOIOCTLCMD;
}
-static unsigned int blktap_poll(struct file *file, poll_table *wait)
-{
- private_info_t *prv;
- tap_blkif_t *info;
-
- /*Retrieve the dev info*/
- prv = (private_info_t *)file->private_data;
- if (prv == NULL) {
+static unsigned int blktap_poll(struct file *filp, poll_table *wait)
+{
+ tap_blkif_t *info = filp->private_data;
+
+ if (!info) {
WPRINTK(" poll, retrieving idx failed\n");
return 0;
}
-
- if (prv->idx == 0) return 0;
-
- info = tapfds[prv->idx];
-
- poll_wait(file, &info->wait, wait);
+
+ /* do not work on the control device */
+ if (!info->minor)
+ return 0;
+
+ poll_wait(filp, &info->wait, wait);
if (info->ufe_ring.req_prod_pvt != info->ufe_ring.sring->req_prod) {
flush_tlb_all();
RING_PUSH_REQUESTS(&info->ufe_ring);
@@ -691,11 +669,14 @@ void blktap_kick_user(int idx)
{
tap_blkif_t *info;
- if (idx == 0) return;
+ if (idx == 0)
+ return;
info = tapfds[idx];
- if (info != NULL) wake_up_interruptible(&info->wait);
+ if (info != NULL)
+ wake_up_interruptible(&info->wait);
+
return;
}
@@ -837,7 +818,8 @@ static void req_decrease(void)
mmap_inuse--;
}
}
- if (mmap_inuse == 0) mmap_req_del(mmap_alloc-1);
+ if (mmap_inuse == 0)
+ mmap_req_del(mmap_alloc-1);
done:
spin_unlock_irqrestore(&pending_free_lock, flags);
return;
@@ -1002,7 +984,7 @@ int tap_blkif_schedule(void *arg)
* COMPLETION CALLBACK -- Called by user level ioctl()
*/
-static int blktap_read_ufe_ring(int idx)
+static int blktap_read_ufe_ring(tap_blkif_t *info)
{
/* This is called to read responses from the UFE ring. */
RING_IDX i, j, rp;
@@ -1010,12 +992,9 @@ static int blktap_read_ufe_ring(int idx)
blkif_t *blkif=NULL;
int pending_idx, usr_idx, mmap_idx;
pending_req_t *pending_req;
- tap_blkif_t *info;
-
- info = tapfds[idx];
- if (info == NULL) {
+
+ if (!info)
return 0;
- }
/* We currently only forward packets in INTERCEPT_FE mode. */
if (!(info->mode & BLKTAP_MODE_INTERCEPT_FE))
@@ -1063,7 +1042,7 @@ static int blktap_read_ufe_ring(int idx)
>> PAGE_SHIFT;
map[offset] = NULL;
}
- fast_flush_area(pending_req, pending_idx, usr_idx, idx);
+ fast_flush_area(pending_req, pending_idx, usr_idx, info->minor);
make_response(blkif, pending_req->id, resp->operation,
resp->status);
info->idx_map[usr_idx] = INVALID_REQ;
[-- Attachment #3: Type: text/plain, Size: 138 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH 2/2] have blktap use a dynamic major
2006-09-27 16:16 [PATCH 0/2] clean up blktap and change to use a dynamic major Steven Rostedt
2006-09-27 16:19 ` [PATCH 1/2] clean up blktap and remove private structure Steven Rostedt
@ 2006-09-27 16:21 ` Steven Rostedt
2006-09-28 19:48 ` Andrew Warfield
1 sibling, 1 reply; 5+ messages in thread
From: Steven Rostedt @ 2006-09-27 16:21 UTC (permalink / raw)
To: xen-devel; +Cc: andrew.warfield
[-- Attachment #1: Type: text/plain, Size: 416 bytes --]
blktap currently uses a hardcoded major of 254 for the device. This is
not robust in anyway and needs to be dynamic.
Note: it is better not to have the daemon create the node, and have udev
create it instead. But since the daemon currently creates the node
anyway, it is still the way this is done. That change needs to be made
at another time.
-- Steve
Signed-off-by: Steven Rostedt <srostedt@redhat.com>
[-- Attachment #2: xen-linux-blktap-dynamic-major.patch --]
[-- Type: text/x-patch, Size: 4753 bytes --]
diff -r 0b662eb8ad52 linux-2.6-xen-sparse/drivers/xen/blktap/blktap.c
--- a/linux-2.6-xen-sparse/drivers/xen/blktap/blktap.c Wed Sep 27 10:40:59 2006 -0400
+++ b/linux-2.6-xen-sparse/drivers/xen/blktap/blktap.c Wed Sep 27 10:43:58 2006 -0400
@@ -196,12 +196,10 @@ static struct grant_handle_pair
static int blktap_read_ufe_ring(tap_blkif_t *info); /*local prototypes*/
-#define BLKTAP_MINOR 0 /*/dev/xen/blktap resides at device number
- major=254, minor numbers begin at 0 */
-#define BLKTAP_DEV_MAJOR 254 /* TODO: Make major number dynamic *
- * and create devices in the kernel *
- */
+#define BLKTAP_MINOR 0 /*/dev/xen/blktap has a dynamic major */
#define BLKTAP_DEV_DIR "/dev/xen"
+
+static int blktap_major;
/* blktap IOCTLs: */
#define BLKTAP_IOCTL_KICK_FE 1
@@ -631,7 +629,7 @@ static int blktap_ioctl(struct inode *in
return -1;
}
case BLKTAP_IOCTL_MAJOR:
- return BLKTAP_DEV_MAJOR;
+ return blktap_major;
case BLKTAP_QUERY_ALLOC_REQS:
{
@@ -1395,7 +1393,8 @@ static int __init blkif_init(void)
/*Create the blktap devices, but do not map memory or waitqueue*/
for(i = 0; i < MAX_TAP_DEV; i++) translate_domid[i].domid = 0xFFFF;
- ret = register_chrdev(BLKTAP_DEV_MAJOR,"blktap",&blktap_fops);
+ /* Dynamically allocate a major for this device */
+ ret = register_chrdev(0, "blktap", &blktap_fops);
blktap_dir = devfs_mk_dir(NULL, "xen", 0, NULL);
if ( (ret < 0)||(blktap_dir < 0) ) {
@@ -1403,6 +1402,8 @@ static int __init blkif_init(void)
return -ENOMEM;
}
+ blktap_major = ret;
+
for(i = 0; i < MAX_TAP_DEV; i++ ) {
info = tapfds[i] = kzalloc(sizeof(tap_blkif_t),GFP_KERNEL);
if(tapfds[i] == NULL) return -ENOMEM;
@@ -1410,7 +1411,7 @@ static int __init blkif_init(void)
info->pid = 0;
info->blkif = NULL;
- ret = devfs_mk_cdev(MKDEV(BLKTAP_DEV_MAJOR, i),
+ ret = devfs_mk_cdev(MKDEV(blktap_major, i),
S_IFCHR|S_IRUGO|S_IWUSR, "xen/blktap%d", i);
if(ret != 0) return -ENOMEM;
diff -r 0b662eb8ad52 tools/blktap/drivers/blktapctrl.c
--- a/tools/blktap/drivers/blktapctrl.c Wed Sep 27 10:40:59 2006 -0400
+++ b/tools/blktap/drivers/blktapctrl.c Wed Sep 27 10:43:58 2006 -0400
@@ -67,6 +67,8 @@ int max_timeout = MAX_TIMEOUT;
int max_timeout = MAX_TIMEOUT;
int ctlfd = 0;
+int blktap_major;
+
static int open_ctrl_socket(char *devname);
static int write_msg(int fd, int msgtype, void *ptr, void *ptr2);
static int read_msg(int fd, int msgtype, void *ptr);
@@ -108,7 +110,18 @@ static void make_blktap_dev(char *devnam
if (mknod(devname, S_IFCHR|0600,
makedev(major, minor)) == 0)
DPRINTF("Created %s device\n",devname);
- } else DPRINTF("%s device already exists\n",devname);
+ } else {
+ DPRINTF("%s device already exists\n",devname);
+ /* it already exists, but is it the same major number */
+ if (((st.st_rdev>>8) & 0xff) != major) {
+ DPRINTF("%s has old major %d\n",
+ devname,
+ (unsigned int)((st.st_rdev >> 8) & 0xff));
+ /* only try again if we succed in deleting it */
+ if (!unlink(devname))
+ make_blktap_dev(devname, major, minor);
+ }
+ }
}
static int get_new_dev(int *major, int *minor, blkif_t *blkif)
@@ -623,6 +636,30 @@ static void print_drivers(void)
DPRINTF("Found driver: [%s]\n",dtypes[i]->name);
}
+static int find_blktap_major(void)
+{
+ FILE *fp;
+ int major;
+ char device[256];
+
+ if ((fp = fopen("/proc/devices", "r")) == NULL)
+ return -1;
+
+ /* Skip title */
+ fscanf(fp,"%*s %*s\n");
+ while (fscanf(fp, "%d %255s\n", &major, device) == 2) {
+ if (strncmp("blktap", device, 6) == 0)
+ break;
+ }
+
+ fclose(fp);
+
+ if (strncmp("blktap", device, 6) == 0)
+ return major;
+
+ return -1;
+}
+
int main(int argc, char *argv[])
{
char *devname;
@@ -646,7 +683,10 @@ int main(int argc, char *argv[])
/*Attach to blktap0 */
asprintf(&devname,"%s/%s0", BLKTAP_DEV_DIR, BLKTAP_DEV_NAME);
- make_blktap_dev(devname,254,0);
+ blktap_major = find_blktap_major();
+ if (blktap_major < 0)
+ goto open_failed;
+ make_blktap_dev(devname,blktap_major,0);
ctlfd = open(devname, O_RDWR);
if (ctlfd == -1) {
DPRINTF("blktap0 open failed\n");
diff -r 0b662eb8ad52 tools/blktap/lib/blktaplib.h
--- a/tools/blktap/lib/blktaplib.h Wed Sep 27 10:40:59 2006 -0400
+++ b/tools/blktap/lib/blktaplib.h Wed Sep 27 10:43:58 2006 -0400
@@ -80,8 +80,9 @@ static inline int BLKTAP_MODE_VALID(unsi
#define MAX_PENDING_REQS 64
#define BLKTAP_DEV_DIR "/dev/xen"
#define BLKTAP_DEV_NAME "blktap"
-#define BLKTAP_DEV_MAJOR 254
#define BLKTAP_DEV_MINOR 0
+
+extern int blktap_major;
#define BLKTAP_RING_PAGES 1 /* Front */
#define BLKTAP_MMAP_REGION_SIZE (BLKTAP_RING_PAGES + MMAP_PAGES)
[-- Attachment #3: Type: text/plain, Size: 138 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
^ permalink raw reply [flat|nested] 5+ messages in thread