diff options
author | Artem Bityutskiy <Artem.Bityutskiy@nokia.com> | 2010-07-25 07:29:25 -0400 |
---|---|---|
committer | Jens Axboe <jaxboe@fusionio.com> | 2010-08-07 12:53:57 -0400 |
commit | c284de61db31669cce547ffc99efda971146719d (patch) | |
tree | d4b05dd00d1e33dd34917f3d2a9476f69ca9ac99 /mm | |
parent | 603320239fb436f175c8b6bfa43d5023c47a6dc2 (diff) |
writeback: cleanup bdi_register
This patch makes sure we first initialize everything and set the BDI_registered
flag, and only after this we add the bdi to 'bdi_list'. Current code adds the
bdi to the list too early, and as a result I the
WARN(!test_bit(BDI_registered, &bdi->state)
in bdi forker is triggered. Also, it is in general good practice to make things
visible only when they are fully initialized.
Also, this patch does few micro clean-ups:
1. Removes the 'exit' label which does not do anything, just returns. This
allows to get rid of few braces and 'ret' variable and make the code smaller.
2. If 'kthread_run()' fails, remove the error code it returns, not hard-coded
'-ENOMEM'. Theoretically, some day 'kthread_run()' can return something
else. Also, in case of failure it is not necessary to set 'bdi->wb.task' to
NULL.
Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
Signed-off-by: Jens Axboe <jaxboe@fusionio.com>
Diffstat (limited to 'mm')
-rw-r--r-- | mm/backing-dev.c | 30 |
1 files changed, 11 insertions, 19 deletions
diff --git a/mm/backing-dev.c b/mm/backing-dev.c index 9008c4e207f6..0b8ee66993dd 100644 --- a/mm/backing-dev.c +++ b/mm/backing-dev.c | |||
@@ -511,23 +511,16 @@ int bdi_register(struct backing_dev_info *bdi, struct device *parent, | |||
511 | const char *fmt, ...) | 511 | const char *fmt, ...) |
512 | { | 512 | { |
513 | va_list args; | 513 | va_list args; |
514 | int ret = 0; | ||
515 | struct device *dev; | 514 | struct device *dev; |
516 | 515 | ||
517 | if (bdi->dev) /* The driver needs to use separate queues per device */ | 516 | if (bdi->dev) /* The driver needs to use separate queues per device */ |
518 | goto exit; | 517 | return 0; |
519 | 518 | ||
520 | va_start(args, fmt); | 519 | va_start(args, fmt); |
521 | dev = device_create_vargs(bdi_class, parent, MKDEV(0, 0), bdi, fmt, args); | 520 | dev = device_create_vargs(bdi_class, parent, MKDEV(0, 0), bdi, fmt, args); |
522 | va_end(args); | 521 | va_end(args); |
523 | if (IS_ERR(dev)) { | 522 | if (IS_ERR(dev)) |
524 | ret = PTR_ERR(dev); | 523 | return PTR_ERR(dev); |
525 | goto exit; | ||
526 | } | ||
527 | |||
528 | spin_lock_bh(&bdi_lock); | ||
529 | list_add_tail_rcu(&bdi->bdi_list, &bdi_list); | ||
530 | spin_unlock_bh(&bdi_lock); | ||
531 | 524 | ||
532 | bdi->dev = dev; | 525 | bdi->dev = dev; |
533 | 526 | ||
@@ -541,20 +534,19 @@ int bdi_register(struct backing_dev_info *bdi, struct device *parent, | |||
541 | 534 | ||
542 | wb->task = kthread_run(bdi_forker_thread, wb, "bdi-%s", | 535 | wb->task = kthread_run(bdi_forker_thread, wb, "bdi-%s", |
543 | dev_name(dev)); | 536 | dev_name(dev)); |
544 | if (IS_ERR(wb->task)) { | 537 | if (IS_ERR(wb->task)) |
545 | wb->task = NULL; | 538 | return PTR_ERR(wb->task); |
546 | ret = -ENOMEM; | ||
547 | |||
548 | bdi_remove_from_list(bdi); | ||
549 | goto exit; | ||
550 | } | ||
551 | } | 539 | } |
552 | 540 | ||
553 | bdi_debug_register(bdi, dev_name(dev)); | 541 | bdi_debug_register(bdi, dev_name(dev)); |
554 | set_bit(BDI_registered, &bdi->state); | 542 | set_bit(BDI_registered, &bdi->state); |
543 | |||
544 | spin_lock_bh(&bdi_lock); | ||
545 | list_add_tail_rcu(&bdi->bdi_list, &bdi_list); | ||
546 | spin_unlock_bh(&bdi_lock); | ||
547 | |||
555 | trace_writeback_bdi_register(bdi); | 548 | trace_writeback_bdi_register(bdi); |
556 | exit: | 549 | return 0; |
557 | return ret; | ||
558 | } | 550 | } |
559 | EXPORT_SYMBOL(bdi_register); | 551 | EXPORT_SYMBOL(bdi_register); |
560 | 552 | ||