From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Oleg Drokin <green@linuxhacker.ru>
Cc: "devel@driverdev.osuosl.org" <devel@driverdev.osuosl.org>,
Andreas Dilger <andreas.dilger@intel.com>,
Peng Tao <bergwolf@gmail.com>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Lai Siyao <lai.siyao@intel.com>
Subject: Re: [PATCH] staging/lustre: use rcu_dereference to access rcu protected current->real_parent field
Date: Tue, 12 Aug 2014 10:39:11 +0800 [thread overview]
Message-ID: <20140812023911.GA17509@kroah.com> (raw)
In-Reply-To: <0E60DB89-5757-4841-A416-D12C56413C58@linuxhacker.ru>
On Mon, Aug 11, 2014 at 09:44:47PM -0400, Oleg Drokin wrote:
>
> On Aug 9, 2014, at 11:47 AM, Greg Kroah-Hartman wrote:
>
> > On Sat, Aug 09, 2014 at 10:34:36AM -0400, Oleg Drokin wrote:
> >>
> >>> Because maybe these stats preceed the introduction of perf and other
> >>> tracing/debug tools? I don't know, it's really low down on the list of
> >>> reasons why lustre can't be merged out of staging at the moment, you all
> >>> have much bigger issues to address first.
> >>
> >> I wonder what is the prioritized list you have in mind?
> >
> > Do I really have to spell out what is wrong with that codebase that
> > needs to be fixed up? It took me 50+ patches for 3.17-rc1 to just fix
> > up a _portion_ of the include file mess that is in there. Now is the
> > first time the code actually _builds_ properly in the kernel tree (i.e.
> > no magic header file include path modifications in Makefiles preventing
> > individual files from being built correctly.)
>
> Well, last time we discussed this topic, you said that moving most of lustre proc files
> into sysfs and other suitable venues is what prevents moving lustre out of the
> staging tree under fs/
Yes, that's one of the big things, but the structure of the code itself
still needs lots of work. You have wrapper functions and defines that
are not needed. You have version-specific checks and still a quite
complex and unnecessary include directory structure.
> There well might be include mess, but this is the first time I hear about it, and I have not seen
> any build breakage or other obviously broken behavior.
The build breakage came about in a thread on this mailing list when we
had a tool that could build an individual .o file, which I found to
break on all of the lustre files due to the odd way you were redefining
the include search path of the .c files. I unwound all of that to use
direct paths and now things build properly. But even then, there are
way too many .h files and nested mess that is not needed and is the
result of trying to get this code to build on other platforms a long
time ago.
> I am sure there are more things too, that's why I am asking.
> We are trying to deal with stuff we know about, but it's a bit harder
> to deal with the stuff we don't know about that does not manifest
> itself in some clear way.
Try getting rid of the typedefs and wrapper functions and #defines for
function names and coding style issues. That would be a great first
step and would then expose what really is left to do. Right now, it's
just to hard to see the real issues.
thanks,
greg k-h
prev parent reply other threads:[~2014-08-12 2:39 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-08-06 18:22 [PATCH] staging/lustre: use rcu_dereference to access rcu protected current->real_parent field Evgeny Budilovsky
2014-08-06 21:42 ` Greg Kroah-Hartman
2014-08-07 11:13 ` Evgeny Budilovsky
2014-08-08 3:49 ` Greg Kroah-Hartman
2014-08-08 4:03 ` Oleg Drokin
2014-08-08 4:42 ` Greg Kroah-Hartman
2014-08-08 5:06 ` Oleg Drokin
2014-08-08 5:32 ` Greg Kroah-Hartman
2014-08-09 11:05 ` Peng Tao
2014-08-09 14:04 ` Greg Kroah-Hartman
2014-08-09 14:34 ` Oleg Drokin
2014-08-09 15:47 ` Greg Kroah-Hartman
2014-08-12 1:44 ` Oleg Drokin
2014-08-12 2:39 ` Greg Kroah-Hartman [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=20140812023911.GA17509@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=andreas.dilger@intel.com \
--cc=bergwolf@gmail.com \
--cc=devel@driverdev.osuosl.org \
--cc=green@linuxhacker.ru \
--cc=lai.siyao@intel.com \
--cc=linux-kernel@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.