From: Dan Williams <dan.j.williams@intel.com>
To: Christoph Hellwig <hch@lst.de>
Cc: linux-scsi <linux-scsi@vger.kernel.org>,
"Jiang, Dave" <dave.jiang@intel.com>,
David Milburn <dmilburn@redhat.com>,
"Ciechanowski, Ed" <ed.ciechanowski@intel.com>,
"Nadolski, Edmund" <edmund.nadolski@intel.com>,
"Danecki, Jacek" <jacek.danecki@intel.com>
Subject: Re: [GIT] isci: remote_device state_handlers and base_object removal
Date: Wed, 04 May 2011 07:59:57 -0700 [thread overview]
Message-ID: <4DC169ED.5050101@intel.com> (raw)
In-Reply-To: <20110504095507.GA30387@lst.de>
On 5/4/2011 2:55 AM, Christoph Hellwig wrote:
> In general this looks like the way to go. The only think I'm not overly
> happy wih is how the sci_base_object removal is handled. In general it
> shouldn't be replaced by untyped void pointers, by proper container_of
> usage. I guess for most instances it doesn't matter too much as the
> different layers of structures for the same object are in the process of
> beeing merged anyway, but it's fairly significant for the state machine.
As it turns out I gave this same feedback internally when reviewing the
series. I grabbed this early version for a couple reasons:
1/ wanted to get wider testing of the removal of structure member
position assumptions.
2/ this arrived before I had the patches to start killing off the
substate machines so it was not quite ready for this conversion.
Will circle back and re-add the type-safety after the substate machines
are gone and the unification is completed.
> All the enter/exit handler should be passed a struct sci_base_state_machine *,
> and then use container_of to get at the containing structure, thus removing
> the need for the state_machine_owner field in struct sci_base_state_machine.
Ah yes, forgot about that field.
> And while you're at it _please_ remove all the utterly pointless kerneldoc
> comments for the state enter/exit handlers.
Looks like I managed to kill them all for remote_device, will double
check that this gets done for the rest.
prev parent reply other threads:[~2011-05-04 14:59 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-05-03 16:45 [GIT] isci: remote_device state_handlers and base_object removal Dan Williams
2011-05-04 9:55 ` Christoph Hellwig
2011-05-04 14:59 ` Dan Williams [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=4DC169ED.5050101@intel.com \
--to=dan.j.williams@intel.com \
--cc=dave.jiang@intel.com \
--cc=dmilburn@redhat.com \
--cc=ed.ciechanowski@intel.com \
--cc=edmund.nadolski@intel.com \
--cc=hch@lst.de \
--cc=jacek.danecki@intel.com \
--cc=linux-scsi@vger.kernel.org \
/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;
as well as URLs for NNTP newsgroup(s).