lkml.org 
[lkml]   [2006]   [Mar]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 3/7] dm/md dependency tree in sysfs: bd_claim_by_kobject
"Jun'ichi Nomura" <j-nomura@ce.jp.nec.com> wrote:
>
> This patch is part of dm/md dependency tree in sysfs.
>
> This adds bd_claim_by_kobject() function which takes kobject as
> additional signature of holder device and creates sysfs symlinks
> between holder device and claimed device.
> bd_release_from_kobject() is a counter part of bd_claim_by_kobject.
>
> ...
>
> @@ -402,6 +402,7 @@ struct block_device {
> struct list_head bd_inodes;
> void * bd_holder;
> int bd_holders;
> + struct list_head bd_holder_list;
> struct block_device * bd_contains;
> unsigned bd_block_size;
> struct hd_struct * bd_part;

Bear in mind that sysfs is Kconfigurable (CONFIG_SYSFS). If possible,
newly-added code which doesn't make sense without sysfs should not appear
in a CONFIG_SYSFS=n vmlinux.

> +
> +static inline struct kobject * bdev_get_kobj(struct block_device *bdev)

pet-peeve: The space after asterisk has no use.

> +{
> + if (bdev->bd_contains != bdev)
> + return kobject_get(&bdev->bd_part->kobj);
> + else
> + return kobject_get(&bdev->bd_disk->kobj);
> +}
> +
> +static inline struct kobject * bdev_get_holder(struct block_device *bdev)
> +static inline void add_symlink(struct kobject *from, struct kobject *to)
> +static inline void del_symlink(struct kobject *from, struct kobject *to)
> +static inline int bd_holder_grab_dirs(struct block_device *bdev,
> +static inline void bd_holder_release_dirs(struct bd_holder *bo)

I suposse the inlines are OK, given that we "know" that there's only a
single callsite. But recent gcc's take care of that automatically.

> +static int add_bd_holder(struct block_device *bdev, struct bd_holder *bo)
> +{
> + struct bd_holder *tmp;
> +
> + if (!bo)
> + return 0;

whitespace went wild there.

> +static void free_bd_holder(struct bd_holder *bo)
> ...
> +
> + bo->sdir = kobj;
> + list_for_each_entry(tmp, &bdev->bd_holder_list, list) {
> + if (tmp->sdir == bo->sdir) {
> + tmp->count++;
> + return 0;
> + }
> + }
> +
> + if (!bd_holder_grab_dirs(bdev, bo))
> + return 0;
> +
> + add_symlink(bo->sdir, bo->sdev);
> + add_symlink(bo->hdir, bo->hdev);
> + list_add_tail(&bo->list, &bdev->bd_holder_list);
> + return 1;
> +}
>
> +static struct bd_holder *del_bd_holder(struct block_device *bdev,
> + struct kobject *kobj)
> +{
> + struct bd_holder *bo;
> +
> + list_for_each_entry(bo, &bdev->bd_holder_list, list) {
> + if (bo->sdir == kobj) {
> + bo->count--;
> + BUG_ON(bo->count < 0);
> + if (!bo->count) {
> + list_del(&bo->list);
> + del_symlink(bo->sdir, bo->sdev);
> + del_symlink(bo->hdir, bo->hdev);
> + bd_holder_release_dirs(bo);
> + return bo;
> + }
> + break;
> + }
> + }
> +
> + return NULL;
> +}

Some comments which describe what these do, what they return and why they
return it would be nice.

> +
> +int bd_claim_by_kobject(struct block_device *bdev, void *holder,
> + struct kobject *kobj)
> +{

Ditto.

> + int res = -EBUSY;
> + struct bd_holder *bo;
> +
> + if (!kobj)
> + return -EINVAL;
> +
> + bo = alloc_bd_holder(kobj);
> + if (!bo)
> + return -ENOMEM;
> +
> + down(&bdev->bd_sem);
> + res = bd_claim(bdev, holder);
> + if (res || !add_bd_holder(bdev, bo))
> + free_bd_holder(bo);
> + up(&bdev->bd_sem);
> +
> + return res;
> +}
> +
> +EXPORT_SYMBOL(bd_claim_by_kobject);

I don't think the blank line before the EXPORT_SYMBOL() does anything useful.

EXPORT_SYMBOL_GPL() might be preferred.

> +void bd_release_from_kobject(struct block_device *bdev, struct kobject *kobj)
> +{
> + struct bd_holder *bo;
> +
> + if (!kobj)
> + return;
> +
> + down(&bdev->bd_sem);
> + bd_release(bdev);
> + if ((bo = del_bd_holder(bdev, kobj)))
> + free_bd_holder(bo);
> + up(&bdev->bd_sem);
> +}
> +
> +EXPORT_SYMBOL(bd_release_from_kobject);

Ditto.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2006-03-14 03:32    [from the cache]
©2003-2011 Jasper Spaans