linux-kernel - Re: let md auto-detect 128+ raid members, fix potential race condition

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20060730142137.b1789aff.akpm@osdl.org>
Date:	Sun, 30 Jul 2006 14:21:37 -0700
From:	Andrew Morton <akpm@...l.org>
To:	Alexandre Oliva <aoliva@...hat.com>
Cc:	linux-kernel@...r.kernel.org, neilb@...e.de
Subject: Re: let md auto-detect 128+ raid members, fix potential race
 condition

On Sun, 30 Jul 2006 17:56:31 -0300
Alexandre Oliva <aoliva@...hat.com> wrote:

> On Jul 30, 2006, Andrew Morton <akpm@...l.org> wrote:
> 
> > On Sun, 30 Jul 2006 03:56:21 -0300
> > Alexandre Oliva <aoliva@...hat.com> wrote:
> 
> >> -void md_autodetect_dev(dev_t dev);
> >> +int md_register_autodetect_dev(dev_t dev);
> 
> > Put it in a header file, please.
>  
> AFAICT it really isn't supposed to be used elsewhere.  I suppose I
> could add it to either blkdev.h, fs.h or raid/md.h, since it's more of
> glue code between two modules than something that belongs to one
> specific module.  E.g., if it goes in raid/md.h, where it feels the
> most appropriate, then fs/parititions/check.c has to include it, which
> doesn't sound right.  OTOH, if it goes in blkdev.h or fs.h, then a lot
> of code ends up seeing the declaration that shouldn't be available.
> Thoughts?

If the function is exported by md then md.h would be an appropriate place
for the declaration.

> Maybe we could replace this with some register/unregister notifier
> interface, such that add_partitions() could then notify multiple
> watchers when a new partition is configured.  This would remove the
> backwards dependency here, but I feel it should be done in a separate
> patch.  I don't mind if they're integrated at once, but I don't feel
> that changing two unrelated issues at once is a good approach.

If we went that way then patch #1 would be "add a notifier" and patch #2
would be "use it in md".

Do we anticipate that there would ever be other users of this notifier
callback API?

> >> #ifdef CONFIG_BLK_DEV_MD
> >> -		if (state->parts[p].flags)
> >> -			md_autodetect_dev(bdev->bd_dev+p);
> >> +		if (state->parts[p].flags
> >> +		    && md_register_autodetect_dev(bdev->bd_dev+p))
> >> +			printk(KERN_ERR "md: out of memory registering %s%d\n",
> >> +			       disk->disk_name, p);
> >> #endif
> 
> > What happens if CONFIG_BLK_DEV_MD=m?
> 
> AFAIK then you'd get a link failure.  One more reason to go with the
> notifier approach, I guess.  It wouldn't quite enable md to
> auto-detect from partitions set up before the module was loaded, but
> it would at least remove this presumed link error.
> 
> Another approach would be to split the autodetect stuff out of md.c
> into a separate file that goes in the main kernel image (if
> CONFIG_MD=y, it's never m) even if CONFIG_BLK_DEV_MD=m.  Would this be
> a desirable arrangement?

I guess it sounds logical, but I'm not sure what the end result would look
like.  Which amounts to noncommittal Sunday afternoon waffling ;)

The notifier certainly makes sense if we anticipate other users of it.  If
we think it'll always be an md-special thing then yeah, I guess we can code
it that way.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/