From: Ashok Raj <ashok.raj@intel.com>
To: Nathan Lynch <ntl@pobox.com>
Cc: Ashok Raj <ashok.raj@intel.com>,
akpm@osdl.org, linux@brodo.de, davej@redhat.com,
zwane@arm.linux.org.uk, linux-kernel@vger.kernel.org,
Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
Subject: Re: [patch 4/4] create and destroy cpufreq sysfs entries based on cpu notifiers.
Date: Mon, 7 Nov 2005 06:27:39 -0800 [thread overview]
Message-ID: <20051107062738.A1197@unix-os.sc.intel.com> (raw)
In-Reply-To: <20051107114144.GH7806@otto>; from ntl@pobox.com on Mon, Nov 07, 2005 at 05:41:44AM -0600
On Mon, Nov 07, 2005 at 05:41:44AM -0600, Nathan Lynch wrote:
> >
> > Primary goal was to just make these directories appear/disapper dynamically.
>
> I see this patch series has already been merged, but in light of the
> issues that it has caused[1], and the hack that Andrew is carrying to
> deal with them[2], could we revisit the original justification for
> these changes?
Agreed, the patch wasnt clean, just a sde effect that i didnt
want to make very many changes in a subsystem to have to validate
if the changes were extensive. So i took some shortcuts that made it
ugly and now those reworks. (saying that is like beating the dead horse)
the problem isnt creating and destroyoing sysfs files, we need to put the
cpu going away to the lowst possible freq state to save power etc which
was the code path that caused trouble.
>
> Why is it important that cpufreq-related files in sysfs be added and
> removed as cpus go online and offline? I see that the information
> that these entries provide can be derived only when the cpu is online,
> is that the primary justification?
There are just not for informational purposes, there are some files
that get created for control as well, say to set a cpu frequency.
Sure you can do cpu_online() to each step, but iam sure that kind of looks
ugly workaround. There is no reason for them to exist when the cpu is offline.
The data also gets populated upfront at creation time. say if i start
with maxcpus=2, in a 4 cpu system, only 2 online cpus get their
cpufreq entries get created. since these are sysfs subdriver, when
the sysfs entrries are created, the subdriver _add functions get called
for all present cpus, but the _add only succeeds for online cpus.
now we could re-organize all of that to just create the entries
and collect the data each time rather than just create them
when its possible, and remove them when not necessary anymore.
the orign of the patch fiasco was since the lock was smack in the
lowest function, that had about 14 different places it got called from.
So if i need to move the lock to higher caller, we need to properly
analyze which ones are truly higher level functions in cpufreq.
I wasnt motivated to that, not having understood the entire cpufreq mechanics
tried to solve the symtom instead.
>
> Would it be undesirable for the cpufreq drivers to create their
> entries under all cpu sysdevs at init time, regardless of whether the
> cpus are online? The "show" methods for entries attached to offline
> cpus could be made to return "Unavailable" or some equivalent.
>
> I'm not terribly familiar with x86 or cpufreq, so forgive me if I'm
> missing something obvious.
Hope this helps.
>
--
Cheers,
Ashok Raj
- Open Source Technology Center
prev parent reply other threads:[~2005-11-07 14:28 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2005-10-21 20:38 [patch 0/4] dynamically create "cache" and "cpufreq" entries with CPU hotplug Ashok Raj
2005-10-21 20:38 ` [patch 1/4] introduce get_cpu_sysdev() to retrieve a sysfs entry for a cpu Ashok Raj
2005-10-21 20:38 ` [patch 2/4] create and destroy cache sysfs entries based on cpu notifiers Ashok Raj
2005-10-21 20:38 ` [patch 3/4] Remove cpu_sys_devices in cpufreq subsystem Ashok Raj
2005-10-21 20:38 ` [patch 4/4] create and destroy cpufreq sysfs entries based on cpu notifiers Ashok Raj
2005-11-07 11:41 ` Nathan Lynch
2005-11-07 14:27 ` Ashok Raj [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20051107062738.A1197@unix-os.sc.intel.com \
--to=ashok.raj@intel.com \
--cc=akpm@osdl.org \
--cc=davej@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@brodo.de \
--cc=ntl@pobox.com \
--cc=venkatesh.pallipadi@intel.com \
--cc=zwane@arm.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®