This patch removes #ifdef for CONFIG_SYSCTL by defining empty
cdrom_sysctl_register and cdrom_sysctl_unregister when CONFIG_SYSCTL
is not defined.
Signed-off-by: Akinobu Mita <[email protected]>
Cc: Jens Axboe <[email protected]>
---
drivers/cdrom/cdrom.c | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)
Index: 2.6-git/drivers/cdrom/cdrom.c
===================================================================
--- 2.6-git.orig/drivers/cdrom/cdrom.c
+++ 2.6-git/drivers/cdrom/cdrom.c
@@ -360,9 +360,8 @@ static int cdrom_mrw_exit(struct cdrom_d
static int cdrom_get_disc_info(struct cdrom_device_info *cdi, disc_information *di);
-#ifdef CONFIG_SYSCTL
static void cdrom_sysctl_register(void);
-#endif /* CONFIG_SYSCTL */
+
static struct cdrom_device_info *topCdromPtr;
static int cdrom_dummy_generic_packet(struct cdrom_device_info *cdi,
@@ -398,9 +397,7 @@ int register_cdrom(struct cdrom_device_i
if (!banner_printed) {
printk(KERN_INFO "Uniform CD-ROM driver " REVISION "\n");
banner_printed = 1;
-#ifdef CONFIG_SYSCTL
cdrom_sysctl_register();
-#endif /* CONFIG_SYSCTL */
}
ENSURE(drive_status, CDC_DRIVE_STATUS );
@@ -3571,22 +3568,29 @@ static void cdrom_sysctl_unregister(void
unregister_sysctl_table(cdrom_sysctl_header);
}
+#else /* CONFIG_SYSCTL */
+
+static void cdrom_sysctl_register(void)
+{
+}
+
+static void cdrom_sysctl_unregister(void)
+{
+}
+
#endif /* CONFIG_SYSCTL */
static int __init cdrom_init(void)
{
-#ifdef CONFIG_SYSCTL
cdrom_sysctl_register();
-#endif
+
return 0;
}
static void __exit cdrom_exit(void)
{
printk(KERN_INFO "Uniform CD-ROM driver unloaded\n");
-#ifdef CONFIG_SYSCTL
cdrom_sysctl_unregister();
-#endif
}
module_init(cdrom_init);
This patch eliminates hardcoded return value of register_cdrom().
It also changes the return value to -EINVAL.
It is more appropriate than -2 (-ENOENT) because it is only
happen invalid usage of register_cdrom() by broken cdrom driver.
Signed-off-by: Akinobu Mita <[email protected]>
Cc: Jens Axboe <[email protected]>
---
drivers/cdrom/cdrom.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: 2.6-git/drivers/cdrom/cdrom.c
===================================================================
--- 2.6-git.orig/drivers/cdrom/cdrom.c
+++ 2.6-git/drivers/cdrom/cdrom.c
@@ -393,7 +393,7 @@ int register_cdrom(struct cdrom_device_i
cdinfo(CD_OPEN, "entering register_cdrom\n");
if (cdo->open == NULL || cdo->release == NULL)
- return -2;
+ return -EINVAL;
if (!banner_printed) {
printk(KERN_INFO "Uniform CD-ROM driver " REVISION "\n");
banner_printed = 1;
This patch protects the list of cdrom_device_info by cdrom_mutex
when the file in /proc/sys/dev/cdrom/ is written.
Signed-off-by: Akinobu Mita <[email protected]>
Cc: Jens Axboe <[email protected]>
---
drivers/cdrom/cdrom.c | 2 ++
1 file changed, 2 insertions(+)
Index: 2.6-git/drivers/cdrom/cdrom.c
===================================================================
--- 2.6-git.orig/drivers/cdrom/cdrom.c
+++ 2.6-git/drivers/cdrom/cdrom.c
@@ -3427,6 +3427,7 @@ static void cdrom_update_settings(void)
{
struct cdrom_device_info *cdi;
+ mutex_lock(&cdrom_mutex);
for (cdi = topCdromPtr; cdi != NULL; cdi = cdi->next) {
if (autoclose && CDROM_CAN(CDC_CLOSE_TRAY))
cdi->options |= CDO_AUTO_CLOSE;
@@ -3445,6 +3446,7 @@ static void cdrom_update_settings(void)
else
cdi->options &= ~CDO_CHECK_TYPE;
}
+ mutex_unlock(&cdrom_mutex);
}
static int cdrom_sysctl_handler(ctl_table *ctl, int write, struct file * filp,
Use list_head for cdrom_device_info list instead of opencoded
singly list handling.
Signed-off-by: Akinobu Mita <[email protected]>
Cc: Jens Axboe <[email protected]>
---
drivers/cdrom/cdrom.c | 29 ++++++-----------------------
include/linux/cdrom.h | 3 ++-
2 files changed, 8 insertions(+), 24 deletions(-)
Index: 2.6-git/drivers/cdrom/cdrom.c
===================================================================
--- 2.6-git.orig/drivers/cdrom/cdrom.c
+++ 2.6-git/drivers/cdrom/cdrom.c
@@ -362,7 +362,7 @@ static int cdrom_get_disc_info(struct cd
static void cdrom_sysctl_register(void);
-static struct cdrom_device_info *topCdromPtr;
+static LIST_HEAD(cdrom_list);
static int cdrom_dummy_generic_packet(struct cdrom_device_info *cdi,
struct packet_command *cgc)
@@ -436,35 +436,18 @@ int register_cdrom(struct cdrom_device_i
cdinfo(CD_REG_UNREG, "drive \"/dev/%s\" registered\n", cdi->name);
mutex_lock(&cdrom_mutex);
- cdi->next = topCdromPtr;
- topCdromPtr = cdi;
+ list_add(&cdi->list, &cdrom_list);
mutex_unlock(&cdrom_mutex);
return 0;
}
#undef ENSURE
-int unregister_cdrom(struct cdrom_device_info *unreg)
+int unregister_cdrom(struct cdrom_device_info *cdi)
{
- struct cdrom_device_info *cdi, *prev;
cdinfo(CD_OPEN, "entering unregister_cdrom\n");
- prev = NULL;
mutex_lock(&cdrom_mutex);
- cdi = topCdromPtr;
- while (cdi && cdi != unreg) {
- prev = cdi;
- cdi = cdi->next;
- }
-
- if (cdi == NULL) {
- mutex_unlock(&cdrom_mutex);
- return -2;
- }
- if (prev)
- prev->next = cdi->next;
- else
- topCdromPtr = cdi->next;
-
+ list_del(&cdi->list);
mutex_unlock(&cdrom_mutex);
if (cdi->exit)
@@ -3306,7 +3289,7 @@ static int cdrom_print_info(const char *
*pos += ret;
- for (cdi = topCdromPtr; cdi; cdi = cdi->next) {
+ list_for_each_entry(cdi, &cdrom_list, list) {
switch (option) {
case CTL_NAME:
ret = scnprintf(info + *pos, max_size - *pos,
@@ -3428,7 +3411,7 @@ static void cdrom_update_settings(void)
struct cdrom_device_info *cdi;
mutex_lock(&cdrom_mutex);
- for (cdi = topCdromPtr; cdi != NULL; cdi = cdi->next) {
+ list_for_each_entry(cdi, &cdrom_list, list) {
if (autoclose && CDROM_CAN(CDC_CLOSE_TRAY))
cdi->options |= CDO_AUTO_CLOSE;
else if (!autoclose)
Index: 2.6-git/include/linux/cdrom.h
===================================================================
--- 2.6-git.orig/include/linux/cdrom.h
+++ 2.6-git/include/linux/cdrom.h
@@ -910,6 +910,7 @@ struct mode_page_header {
#ifdef __KERNEL__
#include <linux/fs.h> /* not really needed, later.. */
#include <linux/device.h>
+#include <linux/list.h>
struct packet_command
{
@@ -934,7 +935,7 @@ struct packet_command
/* Uniform cdrom data structures for cdrom.c */
struct cdrom_device_info {
struct cdrom_device_ops *ops; /* link to device_ops */
- struct cdrom_device_info *next; /* next device_info for this major */
+ struct list_head list; /* linked list of all device_info */
struct gendisk *disk; /* matching block layer disk */
void *handle; /* driver-dependent data */
/* specifications */
Now unregister_cdrom() always returns 0.
Make it return void and update all callers that check the return value.
Signed-off-by: Akinobu Mita <[email protected]>
Cc: Adrian McMenamin <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Jens Axboe <[email protected]>
---
Documentation/cdrom/cdrom-standard.tex | 2 +-
drivers/cdrom/cdrom.c | 3 +--
drivers/cdrom/gdrom.c | 4 +++-
drivers/cdrom/viocd.c | 5 +----
drivers/ide/ide-cd.c | 5 ++---
include/linux/cdrom.h | 2 +-
6 files changed, 9 insertions(+), 12 deletions(-)
Index: 2.6-git/drivers/cdrom/cdrom.c
===================================================================
--- 2.6-git.orig/drivers/cdrom/cdrom.c
+++ 2.6-git/drivers/cdrom/cdrom.c
@@ -442,7 +442,7 @@ int register_cdrom(struct cdrom_device_i
}
#undef ENSURE
-int unregister_cdrom(struct cdrom_device_info *cdi)
+void unregister_cdrom(struct cdrom_device_info *cdi)
{
cdinfo(CD_OPEN, "entering unregister_cdrom\n");
@@ -455,7 +455,6 @@ int unregister_cdrom(struct cdrom_device
cdi->ops->n_minors--;
cdinfo(CD_REG_UNREG, "drive \"/dev/%s\" unregistered\n", cdi->name);
- return 0;
}
int cdrom_get_media_event(struct cdrom_device_info *cdi,
Index: 2.6-git/drivers/cdrom/gdrom.c
===================================================================
--- 2.6-git.orig/drivers/cdrom/gdrom.c
+++ 2.6-git/drivers/cdrom/gdrom.c
@@ -827,7 +827,9 @@ static int __devexit remove_gdrom(struct
del_gendisk(gd.disk);
if (gdrom_major)
unregister_blkdev(gdrom_major, GDROM_DEV_NAME);
- return unregister_cdrom(gd.cd_info);
+ unregister_cdrom(gd.cd_info);
+
+ return 0;
}
static struct platform_driver gdrom_driver = {
Index: 2.6-git/drivers/cdrom/viocd.c
===================================================================
--- 2.6-git.orig/drivers/cdrom/viocd.c
+++ 2.6-git/drivers/cdrom/viocd.c
@@ -650,10 +650,7 @@ static int viocd_remove(struct vio_dev *
{
struct disk_info *d = &viocd_diskinfo[vdev->unit_address];
- if (unregister_cdrom(&d->viocd_info) != 0)
- printk(VIOCD_KERN_WARNING
- "Cannot unregister viocd CD-ROM %s!\n",
- d->viocd_info.name);
+ unregister_cdrom(&d->viocd_info);
del_gendisk(d->viocd_disk);
blk_cleanup_queue(d->viocd_disk->queue);
put_disk(d->viocd_disk);
Index: 2.6-git/include/linux/cdrom.h
===================================================================
--- 2.6-git.orig/include/linux/cdrom.h
+++ 2.6-git/include/linux/cdrom.h
@@ -995,7 +995,7 @@ extern int cdrom_ioctl(struct file *file
extern int cdrom_media_changed(struct cdrom_device_info *);
extern int register_cdrom(struct cdrom_device_info *cdi);
-extern int unregister_cdrom(struct cdrom_device_info *cdi);
+extern void unregister_cdrom(struct cdrom_device_info *cdi);
typedef struct {
int data;
Index: 2.6-git/Documentation/cdrom/cdrom-standard.tex
===================================================================
--- 2.6-git.orig/Documentation/cdrom/cdrom-standard.tex
+++ 2.6-git/Documentation/cdrom/cdrom-standard.tex
@@ -777,7 +777,7 @@ Note that a driver must have one static
it may have as many structures $<device>_info$ as there are minor devices
active. $Register_cdrom()$ builds a linked list from these.
-\subsection{$Int\ unregister_cdrom(struct\ cdrom_device_info * cdi)$}
+\subsection{$Void\ unregister_cdrom(struct\ cdrom_device_info * cdi)$}
Unregistering device $cdi$ with minor number $MINOR(cdi\to dev)$ removes
the minor device from the list. If it was the last registered minor for
Index: 2.6-git/drivers/ide/ide-cd.c
===================================================================
--- 2.6-git.orig/drivers/ide/ide-cd.c
+++ 2.6-git/drivers/ide/ide-cd.c
@@ -2030,9 +2030,8 @@ static void ide_cd_release(struct kref *
kfree(info->buffer);
kfree(info->toc);
- if (devinfo->handle == drive && unregister_cdrom(devinfo))
- printk(KERN_ERR "%s: %s failed to unregister device from the cdrom "
- "driver.\n", __FUNCTION__, drive->name);
+ if (devinfo->handle == drive)
+ unregister_cdrom(devinfo);
drive->dsc_overlap = 0;
drive->driver_data = NULL;
blk_queue_prep_rq(drive->queue, NULL);
On Sat, Mar 22 2008, Akinobu Mita wrote:
> Use list_head for cdrom_device_info list instead of opencoded
> singly list handling.
Looks good, but you don't seem to be initializing ->list anywhere. Did
you test this?
I'd suggest just adding an INIT_LIST_HEAD() before the list_add() in
register_cdrom()
You also seem to remove the !cdi check, which would be an unrelated
change.
--
Jens Axboe
On Sat, Mar 22 2008, Jens Axboe wrote:
> You also seem to remove the !cdi check, which would be an unrelated
> change.
THat's of course fine, it was a relic of the old list search. So no
problem with that part.
--
Jens Axboe
On Sat, Mar 22 2008, Akinobu Mita wrote:
> Now unregister_cdrom() always returns 0.
> Make it return void and update all callers that check the return value.
Patches 1-3 and 5 are fine, thanks. If you can rediff #4 with the list
init part added, it's good to go.
--
Jens Axboe
2008/3/22, Jens Axboe <[email protected]>:
> On Sat, Mar 22 2008, Akinobu Mita wrote:
> > Use list_head for cdrom_device_info list instead of opencoded
> > singly list handling.
>
>
> Looks good, but you don't seem to be initializing ->list anywhere. Did
> you test this?
>
> I'd suggest just adding an INIT_LIST_HEAD() before the list_add() in
> register_cdrom()
It seems that current list_add() implementation doesn't need
initalized new entry with/without CONFIG_DEBUG_LIST.
And the following change will show too many warnings with CONFIG_DEBUG_LIST.
Is it recommended to add an INIT_LIST_HEAD() before the list_add()?
Index: 2.6-git/lib/list_debug.c
===================================================================
--- 2.6-git.orig/lib/list_debug.c
+++ 2.6-git/lib/list_debug.c
@@ -8,6 +8,7 @@
#include <linux/module.h>
#include <linux/list.h>
+#include <linux/bug.h>
/*
* Insert a new entry between two known consecutive entries.
@@ -49,6 +50,7 @@ EXPORT_SYMBOL(__list_add);
*/
void list_add(struct list_head *new, struct list_head *head)
{
+ WARN_ON(!list_empty(new));
__list_add(new, head, head->next);
}
EXPORT_SYMBOL(list_add);
On Sat, Mar 22, 2008 at 12:14:14PM +0900, Akinobu Mita wrote:
> Now unregister_cdrom() always returns 0.
> Make it return void and update all callers that check the return value.
>
> Signed-off-by: Akinobu Mita <[email protected]>
> Cc: Adrian McMenamin <[email protected]>
> Cc: Borislav Petkov <[email protected]>
> Cc: Jens Axboe <[email protected]>
> ---
> Documentation/cdrom/cdrom-standard.tex | 2 +-
> drivers/cdrom/cdrom.c | 3 +--
> drivers/cdrom/gdrom.c | 4 +++-
> drivers/cdrom/viocd.c | 5 +----
> drivers/ide/ide-cd.c | 5 ++---
> include/linux/cdrom.h | 2 +-
> 6 files changed, 9 insertions(+), 12 deletions(-)
Acked-by: Borislav Petkov <[email protected]>
On Sat, 2008-03-22 at 12:14 +0900, Akinobu Mita wrote:
> Now unregister_cdrom() always returns 0.
> Make it return void and update all callers that check the return value.
>
> Signed-off-by: Akinobu Mita <[email protected]>
> Cc: Adrian McMenamin <[email protected]>
> Cc: Borislav Petkov <[email protected]>
> Cc: Jens Axboe <[email protected]>
> ---
> Documentation/cdrom/cdrom-standard.tex | 2 +-
> drivers/cdrom/cdrom.c | 3 +--
> drivers/cdrom/gdrom.c | 4 +++-
> drivers/cdrom/viocd.c | 5 +----
> drivers/ide/ide-cd.c | 5 ++---
> include/linux/cdrom.h | 2 +-
> 6 files changed, 9 insertions(+), 12 deletions(-)
I'm not sure if I need to ack this - but in case anybody is waiting :)
Acked-by: Adrian McMenamin <[email protected]>
On Sun, Mar 23, 2008 at 12:10:45AM +0900, Akinobu Mita wrote:
> 2008/3/22, Jens Axboe <[email protected]>:
> > On Sat, Mar 22 2008, Akinobu Mita wrote:
> > > Use list_head for cdrom_device_info list instead of opencoded
> > > singly list handling.
> >
> >
> > Looks good, but you don't seem to be initializing ->list anywhere. Did
> > you test this?
> >
> > I'd suggest just adding an INIT_LIST_HEAD() before the list_add() in
> > register_cdrom()
>
> It seems that current list_add() implementation doesn't need
> initalized new entry with/without CONFIG_DEBUG_LIST.
it never did and never should. only the list head needs to be
initialized.
your patch is fine in that respect.
On Sat, Mar 22 2008, Christoph Hellwig wrote:
> On Sun, Mar 23, 2008 at 12:10:45AM +0900, Akinobu Mita wrote:
> > 2008/3/22, Jens Axboe <[email protected]>:
> > > On Sat, Mar 22 2008, Akinobu Mita wrote:
> > > > Use list_head for cdrom_device_info list instead of opencoded
> > > > singly list handling.
> > >
> > >
> > > Looks good, but you don't seem to be initializing ->list anywhere. Did
> > > you test this?
> > >
> > > I'd suggest just adding an INIT_LIST_HEAD() before the list_add() in
> > > register_cdrom()
> >
> > It seems that current list_add() implementation doesn't need
> > initalized new entry with/without CONFIG_DEBUG_LIST.
>
> it never did and never should. only the list head needs to be
> initialized.
>
> your patch is fine in that respect.
It is, my mistake.
--
Jens Axboe