block_device_operations change since 2.6.27.X

block_device_operations change since 2.6.27.X

[Ralf Hornik Mailings]

Dear list,

I hope I catched the right list.
I'm trying to port adics cvfs module to any newer kernel and had to  
figure out that the prototype of "block_device_operations" has become  
incompatible with code formerly used against kernel <= 2.6.27.46

Unfortunately the documentation in filesystems/Locking does still  
point to the old prototype.
Can somebody give me some hints how to use the new  
block_device_operations defined in blkdev.h or send me some new  
dokumentation?
Thank you and best regards

Ralf

Re: block_device_operations change since 2.6.27.X

[Ralf Hornik Mailings]

Ralf Hornik Mailings <ralf@best.homeunix.org> wrote:

> I'm trying to port adics cvfs module to any newer kernel and had to  
> figure out that the prototype of "block_device_operations" has  
> become incompatible with code formerly used against kernel <=  
> 2.6.27.46
>
> Unfortunately the documentation in filesystems/Locking does still  
> point to the old prototype.
> Can somebody give me some hints how to use the new  
> block_device_operations defined in blkdev.h or send me some new  
> dokumentation?

Can noone tell me how to use the new object?

The new struct is:

struct block_device_operations {
         int (*open) (struct block_device *, fmode_t);
         int (*release) (struct gendisk *, fmode_t);
         int (*locked_ioctl) (struct block_device *, fmode_t,  
unsigned, unsigned long);
         int (*ioctl) (struct block_device *, fmode_t, unsigned,  
unsigned long);
         int (*compat_ioctl) (struct block_device *, fmode_t,  
unsigned, unsigned long);
         int (*direct_access) (struct block_device *, sector_t,
                                                 void **, unsigned long *);
         int (*media_changed) (struct gendisk *);
         unsigned long long (*set_capacity) (struct gendisk *,
                                                 unsigned long long);
         int (*revalidate_disk) (struct gendisk *);
         int (*getgeo)(struct block_device *, struct hd_geometry *);
         struct module *owner;
};

and here is the stuct used in <= 2.6.27

struct block_device_operations {
         int (*open) (struct inode *, struct file *);
         int (*release) (struct inode *, struct file *);
         int (*ioctl) (struct inode *, struct file *, unsigned, unsigned long);
         long (*unlocked_ioctl) (struct file *, unsigned, unsigned long);
         long (*compat_ioctl) (struct file *, unsigned, unsigned long);
         int (*direct_access) (struct block_device *, sector_t,  
unsigned long *);
         int (*media_changed) (struct gendisk *);
         int (*revalidate_disk) (struct gendisk *);
         int (*getgeo)(struct block_device *, struct hd_geometry *);
         struct module *owner;
};

And the dokumentation of any kernel still points to the 2.6.27.
Any help would greatly appreciated

Ralf

Re: block_device_operations change since 2.6.27.X

[Jan Kara]

On Thu 06-05-10 15:00:00, Ralf Hornik Mailings wrote:
> Ralf Hornik Mailings <ralf@best.homeunix.org> wrote:
> 
> >I'm trying to port adics cvfs module to any newer kernel and had
> >to figure out that the prototype of "block_device_operations" has
> >become incompatible with code formerly used against kernel <=
> >2.6.27.46
> >
> >Unfortunately the documentation in filesystems/Locking does still
> >point to the old prototype.
> >Can somebody give me some hints how to use the new
> >block_device_operations defined in blkdev.h or send me some new
> >dokumentation?
> 
> Can noone tell me how to use the new object?
  It's a bit hard to tell without knowing more details about the problem
you have. I guess your problem is that originally the functions were passed
struct inode * and struct file * while now they are passed struct
block_device and fmode_t. That should be fairly easy to solve as most
drivers use just inode->i_bdev (now passed directly) and file->f_flags
(also passed directly in the new convention). You can have a look e.g. at
commit 0338e29178795f0df0e1f3705b5d3effe9c95ff7 where drivers/scsi/sd.c is
converted.

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: FWD: block_device_operations change since 2.6.27.X

[Al Viro]

On Fri, May 07, 2010 at 09:26:23AM +0200, Ralf Hornik Mailings wrote:

> Unfortunately the documentation in filesystems/Locking does still
> point to the old prototype.
> Can you give me some hints how to use the new
> block_device_operations defined in blkdev.h or send me some new
> dokumentation?

What is a filesystem doing with block_device_operations in the first
place?  In any case, what happened is this:
	* ->open(inode, file) became ->open(inode->i_bdev, file->f_mode)
	* ->release(inode, file) became ->release(inode->i_bdev->bd_disk,
file->f_mode)
	* similar for ->ioctl() and its ilk
	* ->set_capacity() is IDE-only crap

Note that you *really* don't want to issue any of those manually from e.g.
fs code.  Use ioctl_by_bdev() and friends instead.  Block device driver
probably needs to define those (provided that it needs unusual ioctls,
etc.), but conversion from the old API to new one ought to be simple enough
with the above.

Note that O_NDELAY and O_EXCL are mirrored in ->f_mode (as FMODE_NDELAY
and FMODE_EXCL resp.), so block device drivers that used to look at
file->f_flags for those bits are just looking at fmode_t value they
are getting passed directly.

i_minor(inode) is equal to MINOR(inode->i_bdev->bd_dev), so if ->open()
used to use the former, you can replace it with MINOR(bdev->bd_dev)
in new variant; same for i_major (use MAJOR(...)).  Most of the places
using those actually used them to locate inode->i_bdev->bd_disk->private_data,
and could be completely eliminated in either version.  Faster and cleaner
that way...

If your block device uses other information that used to be reachable from
the old arguments, it had been broken.  A long time ago, since the presence
of anything other that inode->i_bdev, inode->i_rdev, file->f_mode and
file->f_flags hadn't been guaranteed for a long time.

Re: FWD: block_device_operations change since 2.6.27.X

[Ralf Hornik Mailings]

Hello Al, hello Jan

Al Viro <viro@ZenIV.linux.org.uk> wrote:

> i_minor(inode) is equal to MINOR(inode->i_bdev->bd_dev), so if ->open()
> used to use the former, you can replace it with MINOR(bdev->bd_dev)
> in new variant; same for i_major (use MAJOR(...)).  Most of the places
> using those actually used them to locate  
> inode->i_bdev->bd_disk->private_data,
> and could be completely eliminated in either version.  Faster and cleaner
> that way...

Sorry for the late answer, but I'm littel busy at present so I  
couldn't spend some time for testing. Thank you for your hints, I now  
see anything more clear.

As soon as I can look at the code again, I will try to transform the  
objects and functions to the new kernel code.

The corresponding code that happens in cvfs is (linuxif_fsctl.c):

---
static int cvfsctl_major;

extern int cvfsctl_ioctl(struct inode *f_inode, struct file *f_file,
                             unsigned int f_cmd, unsigned long f_argbuf);

static
int cvfsctl_open(
         struct inode *f_inode,
         struct file *f_file)
{
     return 0;
}


extern int cvfsctl_ioctl(
         struct inode    *f_inode,
         struct file     *f_file,
         unsigned int    f_cmd,
         unsigned long   f_argbuf);

static struct block_device_operations cvfsctl_fops = {
     owner:      THIS_MODULE,

/* ### here are the objects loaded ### */
     open:       cvfsctl_open,
     ioctl:      cvfsctl_ioctl,
};

#define MAXDISKS        1024

static struct gendisk *disks[MAXDISKS];

static struct kobject *cvfsctl_probe(dev_t dev, int *part, void *data)
{
     static DECLARE_MUTEX(disks_sem);
     struct gendisk *disk;

     int unit = MINOR(dev);

     down (&disks_sem);
     if (disks[unit]) {
         up(&disks_sem);
         return NULL;
     }

     disk = alloc_disk(1);
     if (!disk) {
         up(&disks_sem);
         return NULL;

     disk->major = MAJOR(dev);
     disk->first_minor = unit;
     sprintf(disk->disk_name, "cvfsctl%d", unit);

     /* ### and this here will fail ### */
     disk->fops = &cvfsctl_fops;
     add_disk(disk);
     disks[unit] = disk;
     up(&disks_sem);
     return NULL;
}
---
Regards

Ralf