Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

blktrace: split do_blk_trace_setup into two functions

Split do_blk_trace_setup into two functions, this is done to prepare for
an incoming new BLKTRACESETUP2 ioctl(2) which can receive extended
parameters from user-space.

Also move the size verification logic to the callers in preparation for
using a new internal structure later.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>

authored by

Johannes Thumshirn and committed by
Jens Axboe
42da88a7 370cd70a

+56 -38
+56 -38
kernel/trace/blktrace.c
··· 518 518 /* 519 519 * Setup everything required to start tracing 520 520 */ 521 - static int do_blk_trace_setup(struct request_queue *q, char *name, dev_t dev, 522 - struct block_device *bdev, 523 - struct blk_user_trace_setup *buts) 521 + static struct blk_trace *blk_trace_setup_prepare(struct request_queue *q, 522 + char *name, dev_t dev, 523 + u32 buf_size, u32 buf_nr, 524 + struct block_device *bdev) 524 525 { 525 526 struct blk_trace *bt = NULL; 526 527 struct dentry *dir = NULL; ··· 529 528 530 529 lockdep_assert_held(&q->debugfs_mutex); 531 530 532 - if (!buts->buf_size || !buts->buf_nr) 533 - return -EINVAL; 534 - 535 - strscpy_pad(buts->name, name, BLKTRACE_BDEV_SIZE); 536 - 537 - /* 538 - * some device names have larger paths - convert the slashes 539 - * to underscores for this to work as expected 540 - */ 541 - strreplace(buts->name, '/', '_'); 542 - 543 531 /* 544 532 * bdev can be NULL, as with scsi-generic, this is a helpful as 545 533 * we can be. 546 534 */ 547 535 if (rcu_dereference_protected(q->blk_trace, 548 536 lockdep_is_held(&q->debugfs_mutex))) { 549 - pr_warn("Concurrent blktraces are not allowed on %s\n", 550 - buts->name); 551 - return -EBUSY; 537 + pr_warn("Concurrent blktraces are not allowed on %s\n", name); 538 + return ERR_PTR(-EBUSY); 552 539 } 553 540 554 541 bt = kzalloc(sizeof(*bt), GFP_KERNEL); 555 542 if (!bt) 556 - return -ENOMEM; 543 + return ERR_PTR(-ENOMEM); 557 544 558 545 ret = -ENOMEM; 559 546 bt->sequence = alloc_percpu(unsigned long); ··· 561 572 if (bdev && !bdev_is_partition(bdev)) 562 573 dir = q->debugfs_dir; 563 574 else 564 - bt->dir = dir = debugfs_create_dir(buts->name, blk_debugfs_root); 575 + bt->dir = dir = debugfs_create_dir(name, blk_debugfs_root); 565 576 566 577 /* 567 578 * As blktrace relies on debugfs for its interface the debugfs directory ··· 569 580 * files or directories. 570 581 */ 571 582 if (IS_ERR_OR_NULL(dir)) { 572 - pr_warn("debugfs_dir not present for %s so skipping\n", 573 - buts->name); 583 + pr_warn("debugfs_dir not present for %s so skipping\n", name); 574 584 ret = -ENOENT; 575 585 goto err; 576 586 } ··· 581 593 debugfs_create_file("dropped", 0444, dir, bt, &blk_dropped_fops); 582 594 debugfs_create_file("msg", 0222, dir, bt, &blk_msg_fops); 583 595 584 - bt->rchan = relay_open("trace", dir, buts->buf_size, 585 - buts->buf_nr, &blk_relay_callbacks, bt); 596 + bt->rchan = relay_open("trace", dir, buf_size, buf_nr, 597 + &blk_relay_callbacks, bt); 586 598 if (!bt->rchan) 587 599 goto err; 600 + 601 + blk_trace_setup_lba(bt, bdev); 602 + 603 + return bt; 604 + 605 + err: 606 + blk_trace_free(q, bt); 607 + 608 + return ERR_PTR(ret); 609 + } 610 + 611 + static void blk_trace_setup_finalize(struct request_queue *q, 612 + char *name, struct blk_trace *bt, 613 + struct blk_user_trace_setup *buts) 614 + 615 + { 616 + strscpy_pad(buts->name, name, BLKTRACE_BDEV_SIZE); 617 + 618 + /* 619 + * some device names have larger paths - convert the slashes 620 + * to underscores for this to work as expected 621 + */ 622 + strreplace(buts->name, '/', '_'); 588 623 589 624 bt->act_mask = buts->act_mask; 590 625 if (!bt->act_mask) 591 626 bt->act_mask = (u16) -1; 592 - 593 - blk_trace_setup_lba(bt, bdev); 594 627 595 628 /* overwrite with user settings */ 596 629 if (buts->start_lba) ··· 624 615 625 616 rcu_assign_pointer(q->blk_trace, bt); 626 617 get_probe_ref(); 627 - 628 - ret = 0; 629 - err: 630 - if (ret) 631 - blk_trace_free(q, bt); 632 - return ret; 633 618 } 634 619 635 620 int blk_trace_setup(struct request_queue *q, char *name, dev_t dev, ··· 631 628 char __user *arg) 632 629 { 633 630 struct blk_user_trace_setup buts; 631 + struct blk_trace *bt; 634 632 int ret; 635 633 636 634 ret = copy_from_user(&buts, arg, sizeof(buts)); 637 635 if (ret) 638 636 return -EFAULT; 639 637 638 + if (!buts.buf_size || !buts.buf_nr) 639 + return -EINVAL; 640 + 640 641 mutex_lock(&q->debugfs_mutex); 641 - ret = do_blk_trace_setup(q, name, dev, bdev, &buts); 642 + bt = blk_trace_setup_prepare(q, name, dev, buts.buf_size, buts.buf_nr, 643 + bdev); 644 + if (IS_ERR(bt)) { 645 + mutex_unlock(&q->debugfs_mutex); 646 + return PTR_ERR(bt); 647 + } 648 + blk_trace_setup_finalize(q, name, bt, &buts); 642 649 mutex_unlock(&q->debugfs_mutex); 643 - if (ret) 644 - return ret; 645 650 646 651 if (copy_to_user(arg, &buts, sizeof(buts))) { 647 652 blk_trace_remove(q); ··· 666 655 { 667 656 struct blk_user_trace_setup buts; 668 657 struct compat_blk_user_trace_setup cbuts; 669 - int ret; 658 + struct blk_trace *bt; 670 659 671 660 if (copy_from_user(&cbuts, arg, sizeof(cbuts))) 672 661 return -EFAULT; 662 + 663 + if (!cbuts.buf_size || !cbuts.buf_nr) 664 + return -EINVAL; 673 665 674 666 buts = (struct blk_user_trace_setup) { 675 667 .act_mask = cbuts.act_mask, ··· 684 670 }; 685 671 686 672 mutex_lock(&q->debugfs_mutex); 687 - ret = do_blk_trace_setup(q, name, dev, bdev, &buts); 673 + bt = blk_trace_setup_prepare(q, name, dev, buts.buf_size, buts.buf_nr, 674 + bdev); 675 + if (IS_ERR(bt)) { 676 + mutex_unlock(&q->debugfs_mutex); 677 + return PTR_ERR(bt); 678 + } 679 + blk_trace_setup_finalize(q, name, bt, &buts); 688 680 mutex_unlock(&q->debugfs_mutex); 689 - if (ret) 690 - return ret; 691 681 692 682 if (copy_to_user(arg, &buts.name, ARRAY_SIZE(buts.name))) { 693 683 blk_trace_remove(q);