Re: block/bsg.c

FUJITA Tomonori
2007-07-17T22:28:33Z

From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Subject: Re: block/bsg.c
Date: Tue, 17 Jul 2007 22:52:25 +0200

> /*
> * TODO
> * - Should this get merged, block/scsi_ioctl.c will be migrated into
> * this file. To keep maintenance down, it's easier to have them
> * seperated right now.
> *
> */
>
> This TODO should be fixed/removed.

Yeah, this should be removed now. I'll do.

> Firstly bsg got merged. ;) Moreover bsg depends on SCSI and scsi_ioctl doesn't.
> Even if SCSI dependency is fixed bsg requires block driver to have struct
> class devices which is not a case for scsi_ioctl.
>
> ...
>
> static struct bsg_device *__bsg_get_device(int minor)
> {
> struct hlist_head *list = &bsg_device_list[bsg_list_idx(minor)];
>
> bsg_device_list[] access should be done under bsg_mutex lock.
>
> May not be a problem currently because of lock_kernel but worth fixing anyway.
>
> struct bsg_device *bd = NULL;
> struct hlist_node *entry;
>
> mutex_lock(&bsg_mutex);
>

They were fixed. Please check the latest code:

git://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-2.6-block.git bsg

We wait for Linus to pull from it.

> static int
> bsg_ioctl(struct inode *inode, struct file *file, unsigned int cmd,
> unsigned long arg)
> {
> ...
> case SCSI_IOCTL_SEND_COMMAND: {
>
> Do we really want to add support for this *deprecated* ioctl to
> the *new* and shiny bsg driver?
>
> void __user *uarg = (void __user *) arg;
> return scsi_cmd_ioctl(file, bd->queue, NULL, cmd, uarg);
> }

We might remove it.

> int bsg_register_queue(struct request_queue *q, const char *name)
> {
> ...
> memset(bcd, 0, sizeof(*bcd));
> ...
> dev = MKDEV(BSG_MAJOR, bcd->minor);
> class_dev = class_device_create(bsg_class, NULL, dev, bcd->dev, "%s", name);
>
> bcd->dev is always 0 (NULL).
>
> Is it OK to pass NULL struct device *dev argument to class_device_create()?

It's ok, I guess.

> It should be fixed by either removing bcd->dev or by setting it to something
> other than zero.
>
> ...
>
> MODULE_AUTHOR("Jens Axboe");
> MODULE_DESCRIPTION("Block layer SGSI generic (sg) driver");
>
> SGSI? :)

It was fixed in the latest code.

Thanks,