2005-04-21 07:15:57

by Bodo Eggert

[permalink] [raw]
Subject: Re: [PATCH 2.6.12-rc2] aoe [1/6]: improve allowed interfaces configuration

Ed L Cashin <[email protected]> wrote:

> +++ b/Documentation/aoe/aoe.txt 2005-04-20 11:42:20.000000000 -0400

> + When the aoe driver is a module, use

Is there any reason for this inconsistent behaviour?

> + /sys/module/aoe/parameters/aoe_iflist instead of
^^^

Why does the module name need to be part of the attribute?
That's redundant. That's redundant.

> + There is a boot option for the built-in aoe driver and a
> + corresponding module parameter, aoe_iflist. Without this option,
> + all network interfaces may be used for ATA over Ethernet. Here is a
> + usage example for the module parameter.
> +
> + modprobe aoe_iflist="eth1 eth3"
^
"aoe"
--
Top 100 things you don't want the sysadmin to say:
63. Oracle will be down until 8pm, but you can come back in and finish your
work when it comes up tonight.


2005-04-21 13:41:00

by Ed L. Cashin

[permalink] [raw]
Subject: Re: [PATCH 2.6.12-rc2] aoe [1/6]: improve allowed interfaces configuration

"Bodo Eggert <[email protected]>" <[email protected]> writes:

> Ed L Cashin <[email protected]> wrote:
>
>> +++ b/Documentation/aoe/aoe.txt 2005-04-20 11:42:20.000000000 -0400
>
>> + When the aoe driver is a module, use
>
> Is there any reason for this inconsistent behaviour?

Yes, the /sys/module/aoe area is only present when the aoe driver is a
module. It would be nicer if there were a sysfs area where I could
put this file regardless of whether the driver is a module or built
into the kernel.

I could probably create one, but I got the file in
/sys/module/aoe/parameters for free when I used module_param_string.

>> + /sys/module/aoe/parameters/aoe_iflist instead of
> ^^^
>
> Why does the module name need to be part of the attribute?
> That's redundant. That's redundant.

Yes. That's true. Redundancy isn't always bad, though, and using the
"aoe_" prefix lets the kernel parameter for the built-in aoe driver be
the same as the parameter for the modular driver.

--
Ed L Cashin <[email protected]>

2005-04-21 14:57:38

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH 2.6.12-rc2] aoe [1/6]: improve allowed interfaces configuration

On Thu, Apr 21, 2005 at 09:36:17AM -0400, Ed L Cashin wrote:
> "Bodo Eggert <[email protected]>" <[email protected]> writes:
>
> > Ed L Cashin <[email protected]> wrote:
> >
> >> +++ b/Documentation/aoe/aoe.txt 2005-04-20 11:42:20.000000000 -0400
> >
> >> + When the aoe driver is a module, use
> >
> > Is there any reason for this inconsistent behaviour?
>
> Yes, the /sys/module/aoe area is only present when the aoe driver is a
> module.

Not true, have you looked in /sys/module lately? :)

> It would be nicer if there were a sysfs area where I could
> put this file regardless of whether the driver is a module or built
> into the kernel.

That's the place for it. It will be there if the driver is built as a
module or into the kernel.

thanks,

greg k-h

2005-04-21 16:01:58

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH 2.6.12-rc2] aoe [1/6]: improve allowed interfaces configuration

On Thu, Apr 21, 2005 at 11:30:06AM -0400, Ed L Cashin wrote:
> Greg KH <[email protected]> writes:
>
> > On Thu, Apr 21, 2005 at 09:36:17AM -0400, Ed L Cashin wrote:
> >> "Bodo Eggert <[email protected]>" <[email protected]> writes:
> >>
> >> > Ed L Cashin <[email protected]> wrote:
> >> >
> >> >> +++ b/Documentation/aoe/aoe.txt 2005-04-20 11:42:20.000000000 -0400
> >> >
> >> >> + When the aoe driver is a module, use
> >> >
> >> > Is there any reason for this inconsistent behaviour?
> >>
> >> Yes, the /sys/module/aoe area is only present when the aoe driver is a
> >> module.
> >
> > Not true, have you looked in /sys/module lately? :)
> >
> >> It would be nicer if there were a sysfs area where I could
> >> put this file regardless of whether the driver is a module or built
> >> into the kernel.
> >
> > That's the place for it. It will be there if the driver is built as a
> > module or into the kernel.
>
> Wow! Well, that's very convenient for driver writers, so I'm pleased,
> and I can update the docs. It surprises me, though, to find out that
> /sys/module is for things other than modules.

It's not for things other than modules, it's filling a real need that
you yourself just pointed out. Namely, we need to be able to have
access to module paramaters in a consistant place, no matter if the
driver is built into the kernel or not.

Man, you try to be nice to people... :)

thanks,

greg k-h

2005-04-21 16:22:07

by Ed L. Cashin

[permalink] [raw]
Subject: /sys/module (was Re: [PATCH 2.6.12-rc2] aoe [1/6]: improve allowed interfaces configuration)

Greg KH <[email protected]> writes:

...
> It's not for things other than modules, it's filling a real need that
> you yourself just pointed out. Namely, we need to be able to have
> access to module paramaters in a consistant place, no matter if the
> driver is built into the kernel or not.
>
> Man, you try to be nice to people... :)

It wasn't a complaint --- like I said, I'm pleased! I just wanted to
serve as a datum: one guy was surprised.

I wanted to put the interfaces list configuration into sysfs, but I
didn't really know how or where to put it, so I procrastinated. Then
I created the module parameter and was pleased to see it show up in
sysfs. I had read about module parameters before, but I had forgotten
about that feature, or maybe it's newer than the docs I read.

Thanks for working so hard to make sysfs useful.

--
Ed L Cashin <[email protected]>

2005-04-21 16:33:25

by Randy.Dunlap

[permalink] [raw]
Subject: Re: [PATCH 2.6.12-rc2] aoe [1/6]: improve allowed interfaces configuration

On Thu, 21 Apr 2005 11:30:06 -0400 Ed L Cashin wrote:

| Greg KH <[email protected]> writes:
|
| > On Thu, Apr 21, 2005 at 09:36:17AM -0400, Ed L Cashin wrote:
| >> "Bodo Eggert <[email protected]>" <[email protected]> writes:
| >>
| >> > Ed L Cashin <[email protected]> wrote:
| >> >
| >> >> +++ b/Documentation/aoe/aoe.txt 2005-04-20 11:42:20.000000000 -0400
| >> >
| >> >> + When the aoe driver is a module, use
| >> >
| >> > Is there any reason for this inconsistent behaviour?
| >>
| >> Yes, the /sys/module/aoe area is only present when the aoe driver is a
| >> module.
| >
| > Not true, have you looked in /sys/module lately? :)
| >
| >> It would be nicer if there were a sysfs area where I could
| >> put this file regardless of whether the driver is a module or built
| >> into the kernel.
| >
| > That's the place for it. It will be there if the driver is built as a
| > module or into the kernel.
|
| Wow! Well, that's very convenient for driver writers, so I'm pleased,
| and I can update the docs. It surprises me, though, to find out that
| /sys/module is for things other than modules.

Just depends on your definition of a module.

AOE (or just about any device driver) can be considered logically
as a module. You seem to be equating module with "loadable module"
vs. a builtin module. The good news is that /sys/module works
for loadable or builtin modules.

---
~Randy

2005-04-21 20:54:47

by Domen Puncer

[permalink] [raw]
Subject: Re: [PATCH 2.6.12-rc2] aoe [1/6]: improve allowed interfaces configuration

On 21/04/05 09:36 -0400, Ed L Cashin wrote:
> "Bodo Eggert <[email protected]>" <[email protected]> writes:
>
> > Ed L Cashin <[email protected]> wrote:
> >
...
> >> + /sys/module/aoe/parameters/aoe_iflist instead of
> > ^^^
> >
> > Why does the module name need to be part of the attribute?
> > That's redundant. That's redundant.
>
> Yes. That's true. Redundancy isn't always bad, though, and using the
> "aoe_" prefix lets the kernel parameter for the built-in aoe driver be
> the same as the parameter for the modular driver.

The __setup() stuff is redundancy too, as module parameters already
work as boot parameters (ie. aoe.iflist).


Domen