nbd: remove unneeded nbd_open/nbd_release and refcnt

PATCH New Paul Clements 4 participants 6 messages Jul 6, 2003

Subsystems (2)

Block layer Network block device (NBD)

lore

Share & tools

lore

Paul Clements

author

2003-07-06 21:01 UTC #0 lore raw

PATCH 1/1 nbd: remove unneeded nbd_open/nbd_release and refcnt

Andrew,

per Jeff's comments, here's the revised patch for removing nbd_open/release

--
Paul

drivers/block/nbd.c renamed from drivers/block/nbd.c.MINUS_LL +2 ~~-34~~

@@ -77,8 +77,6 @@
 #define dprintk(flags, fmt...) do { \
 	if (debugflags & (flags)) printk(KERN_DEBUG fmt); \
 } while (0)
-#define DBG_OPEN        0x0001
-#define DBG_RELEASE     0x0002
 #define DBG_IOCTL       0x0004
 #define DBG_INIT        0x0010
 #define DBG_EXIT        0x0020

@@ -521,33 +519,6 @@ static void do_nbd_request(request_queue
 	return;
 }
 
-static int nbd_open(struct inode *inode, struct file *file)
-{
-	struct nbd_device *lo = inode->i_bdev->bd_disk->private_data;
-
-	dprintk(DBG_OPEN, "%s: nbd_open refcnt=%d\n", lo->disk->disk_name,
-			lo->refcnt);
-	lo->refcnt++;
-	return 0;
-}
-
-static int nbd_release(struct inode *inode, struct file *file)
-{
-	struct nbd_device *lo = inode->i_bdev->bd_disk->private_data;
-
-#ifdef PARANOIA
-	if (lo->refcnt <= 0) {
-		printk(KERN_ALERT "%s: nbd_release: refcount(%d) <= 0\n",
-				lo->disk->disk_name, lo->refcnt);
-		BUG();
-	}
-#endif
-	lo->refcnt--;
-	dprintk(DBG_RELEASE, "%s: nbd_release: refcnt=%d\n",
-			lo->disk->disk_name, lo->refcnt);
-	return 0;
-}
-
 static int nbd_ioctl(struct inode *inode, struct file *file,
 		     unsigned int cmd, unsigned long arg)
 {

@@ -555,6 +526,8 @@ static int nbd_ioctl(struct inode *inode
 	int error;
 	struct request sreq ;
 
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
 #ifdef PARANOIA
 	BUG_ON(lo->magic != LO_MAGIC);
 #endif

@@ -562,8 +535,6 @@ static int nbd_ioctl(struct inode *inode
 	dprintk(DBG_IOCTL, "%s: nbd_ioctl cmd=%s(0x%x) arg=%lu\n",
 			lo->disk->disk_name, ioctl_cmd_to_ascii(cmd), cmd, arg);
 
-	if (!capable(CAP_SYS_ADMIN))
-		return -EPERM;
 	switch (cmd) {
 	case NBD_DISCONNECT:
 	        printk(KERN_INFO "%s: NBD_DISCONNECT\n", lo->disk->disk_name);

@@ -678,8 +649,6 @@ static int nbd_ioctl(struct inode *inode
 static struct block_device_operations nbd_fops =
 {
 	.owner =	THIS_MODULE,
-	.open =		nbd_open,
-	.release =	nbd_release,
 	.ioctl =	nbd_ioctl,
 };

@@ -730,7 +699,6 @@ static int __init nbd_init(void)
 	devfs_mk_dir("nbd");
 	for (i = 0; i < MAX_NBD; i++) {
 		struct gendisk *disk = nbd_dev[i].disk;
-		nbd_dev[i].refcnt = 0;
 		nbd_dev[i].file = NULL;
 #ifdef PARANOIA
 		nbd_dev[i].magic = LO_MAGIC;

include/linux/nbd.h renamed from include/linux/nbd.h.PRISTINE +0 -1

@@ -36,7 +36,6 @@
 #define PARANOIA
 
 struct nbd_device {
-	int refcnt;	
 	int flags;
 	int harderror;		/* Code of hard error			*/
 #define NBD_READ_ONLY 0x0001

Paul Clements

author

2003-07-06 21:06 UTC #1 lore raw

PATCH 1/1 nbd: make nbd and block layer agree about device and block sizes

Andrew,

here's the revised patch for block and device size changes, using set_blocksize() as Jeff suggested

Thanks,
Paul

drivers/block/nbd.c renamed from drivers/block/nbd.c.MINUS_OPEN_RELEASE +8 -4

@@ -588,18 +588,22 @@ static int nbd_ioctl(struct inode *inode
 		}
 		return error;
 	case NBD_SET_BLKSIZE:
-		if ((arg & (arg-1)) || (arg < 512) || (arg > PAGE_SIZE))
-			return -EINVAL;
 		lo->blksize = arg;
-		lo->bytesize &= ~(lo->blksize-1); 
+		lo->bytesize &= ~(lo->blksize-1);
+		inode->i_bdev->bd_inode->i_size = lo->bytesize;
+		set_blocksize(inode->i_bdev, lo->blksize);
 		set_capacity(lo->disk, lo->bytesize >> 9);
 		return 0;
 	case NBD_SET_SIZE:
-		lo->bytesize = arg & ~(lo->blksize-1); 
+		lo->bytesize = arg & ~(lo->blksize-1);
+		inode->i_bdev->bd_inode->i_size = lo->bytesize;
+		set_blocksize(inode->i_bdev, lo->blksize);
 		set_capacity(lo->disk, lo->bytesize >> 9);
 		return 0;
 	case NBD_SET_SIZE_BLOCKS:
 		lo->bytesize = ((u64) arg) * lo->blksize;
+		inode->i_bdev->bd_inode->i_size = lo->bytesize;
+		set_blocksize(inode->i_bdev, lo->blksize);
 		set_capacity(lo->disk, lo->bytesize >> 9);
 		return 0;
 	case NBD_DO_IT:

Jeff Garzik

2003-07-06 21:24 UTC #2 lore raw

Re: [PATCH 2.5.74-mm2] nbd: make nbd and block layer agree about device and block sizes

Quoted 14 lines · Paul Clements

drivers/block/nbd.c renamed from drivers/block/nbd.c.MINUS_OPEN_RELEASE +5 -4

 	case NBD_SET_BLKSIZE:
-		if ((arg & (arg-1)) || (arg < 512) || (arg > PAGE_SIZE))
-			return -EINVAL;
 		lo->blksize = arg;
-		lo->bytesize &= ~(lo->blksize-1); 
+		lo->bytesize &= ~(lo->blksize-1);
+		inode->i_bdev->bd_inode->i_size = lo->bytesize;
+		set_blocksize(inode->i_bdev, lo->blksize);
 		set_capacity(lo->disk, lo->bytesize >> 9);
 		return 0;
 	case NBD_SET_SIZE:
-		lo->bytesize = arg & ~(lo->blksize-1); 
+		lo->bytesize = arg & ~(lo->blksize-1);
+		inode->i_bdev->bd_inode->i_size = lo->bytesize;

drivers/block/nbd.c static int nbd_ioctl(struct inode *inode · line 609

Quoted 10 lines

drivers/block/nbd.c renamed from drivers/block/nbd.c.MINUS_OPEN_RELEASE +3 -0

+		set_blocksize(inode->i_bdev, lo->blksize);
 		set_capacity(lo->disk, lo->bytesize >> 9);
 		return 0;
 	case NBD_SET_SIZE_BLOCKS:
 		lo->bytesize = ((u64) arg) * lo->blksize;
+		inode->i_bdev->bd_inode->i_size = lo->bytesize;
+		set_blocksize(inode->i_bdev, lo->blksize);
 		set_capacity(lo->disk, lo->bytesize >> 9);
 		return 0;
 	case NBD_DO_IT:

Thanks. You forgot to check set_blocksize's return value for errors, though.

Also, I wonder if you found a bug/oversight in set_blocksize -- it sets bd_inode->i_blkbits but not bd_inode->i_size. I think it should set i_size also. Maybe Andrew or Al can confirm/refute this assertion.

Jeff

Paul Clements

2003-07-06 21:41 UTC #3 lore raw

Re: [PATCH 2.5.74-mm2] nbd: make nbd and block layer agree about device and block sizes

Quoted 30 lines · Jeff Garzik

Quoted 14 lines · Paul Clements

drivers/block/nbd.c renamed from drivers/block/nbd.c.MINUS_OPEN_RELEASE +5 -4

 	case NBD_SET_BLKSIZE:
-		if ((arg & (arg-1)) || (arg < 512) || (arg > PAGE_SIZE))
-			return -EINVAL;
 		lo->blksize = arg;
-		lo->bytesize &= ~(lo->blksize-1); 
+		lo->bytesize &= ~(lo->blksize-1);
+		inode->i_bdev->bd_inode->i_size = lo->bytesize;
+		set_blocksize(inode->i_bdev, lo->blksize);
 		set_capacity(lo->disk, lo->bytesize >> 9);
 		return 0;
 	case NBD_SET_SIZE:
-		lo->bytesize = arg & ~(lo->blksize-1); 
+		lo->bytesize = arg & ~(lo->blksize-1);
+		inode->i_bdev->bd_inode->i_size = lo->bytesize;

drivers/block/nbd.c static int nbd_ioctl(struct inode *inode · line 609

Quoted 10 lines

drivers/block/nbd.c renamed from drivers/block/nbd.c.MINUS_OPEN_RELEASE +3 -0

+		set_blocksize(inode->i_bdev, lo->blksize);
 		set_capacity(lo->disk, lo->bytesize >> 9);
 		return 0;
 	case NBD_SET_SIZE_BLOCKS:
 		lo->bytesize = ((u64) arg) * lo->blksize;
+		inode->i_bdev->bd_inode->i_size = lo->bytesize;
+		set_blocksize(inode->i_bdev, lo->blksize);
 		set_capacity(lo->disk, lo->bytesize >> 9);
 		return 0;
 	case NBD_DO_IT:

Thanks. You forgot to check set_blocksize's return value for errors, though.

Quoted 3 lines

Also, I wonder if you found a bug/oversight in set_blocksize -- it sets bd_inode->i_blkbits but not bd_inode->i_size. I think it should set i_size also. Maybe Andrew or Al can confirm/refute this assertion.

OK, I'll wait for a response on this, and then redo the patch as appropriate...

--
Paul

Jeff Garzik

2003-07-06 21:48 UTC #4 lore raw

Re: [PATCH 2.5.74-mm2] nbd: make nbd and block layer agree about device and block sizes

Quoted 5 lines · Paul Clements

Quoted 1 line · Jeff Garzik
Also, I wonder if you found a bug/oversight in set_blocksize -- it sets bd_inode->i_blkbits but not bd_inode->i_size. I think it should set i_size also. Maybe Andrew or Al can confirm/refute this assertion.
OK, I'll wait for a response on this, and then redo the patch as appropriate...

No, nevermind. What I was said dumb ;-)

The inode's size should not be the block size.

Jeff

Andrew Morton

2003-07-06 21:49 UTC #5 lore raw

Re: [PATCH 2.5.74-mm2] nbd: make nbd and block layer agree about device and block sizes

In reply to Jeff Garzik

Quoted 4 lines · Jeff Garzik

Also, I wonder if you found a bug/oversight in set_blocksize -- it sets
bd_inode->i_blkbits but not bd_inode->i_size. I think it should set i_size also. Maybe Andrew or Al can confirm/refute this assertion.

set_blocksize() sets, err, the blocksize.

Lou was wanting to export bd_set_size() for setting i_size. But bd_set_size() goes and tries to set the blocksize too.

So the patch as-is will do I think, unless someone feels there's a need to go and rework these things.