2008-07-17 17:51:57

by Mark Fasheh

[permalink] [raw]
Subject: [git patches] Ocfs2 and Configfs updates for 2.6.27

These patches were sent out earlier this week for review. The only change I
made since then was a small update to Coly's bugfix to remove some code
which it obsoleted.

Original message and revised diffstat follow:

I'm running a bit late with the e-mail this time around, but I think that's
ok since there really isn't any major new features here - the bulk of the
Ocfs2 update is bug fixes, or cleanups. The same goes for configfs.

The only two things that could be described as features would be:

- Sunil has updated Ocfs2 to provide even more live cluster locking
information via debugfs.

- Joel updated configfs's ->make_item() and ->make_group() callbacks to
return more descriptive error types. This update is responsible for the
changes to drivers/net/netconsole.c and fs/dlm/config.c as shown in the diffstat.

Please pull from 'upstream-linus' branch of
git://git.kernel.org/pub/scm/linux/kernel/git/mfasheh/ocfs2.git upstream-linus

to receive the following updates:

Documentation/filesystems/configfs/configfs.txt | 10 +-
.../filesystems/configfs/configfs_example.c | 14 +-
drivers/net/netconsole.c | 10 +-
fs/Kconfig | 8 +
fs/configfs/configfs_internal.h | 4 +
fs/configfs/dir.c | 147 ++++++++++++++------
fs/configfs/inode.c | 2 +
fs/configfs/symlink.c | 16 ++-
fs/dlm/config.c | 45 ++++---
fs/ocfs2/aops.c | 13 ++-
fs/ocfs2/cluster/heartbeat.c | 17 ++-
fs/ocfs2/cluster/netdebug.c | 8 +-
fs/ocfs2/cluster/nodemanager.c | 45 ++++---
fs/ocfs2/dlmglue.c | 122 ++++++++++++++++-
fs/ocfs2/file.c | 2 +-
fs/ocfs2/journal.c | 2 +-
fs/ocfs2/localalloc.c | 2 +-
fs/ocfs2/ocfs2.h | 12 ++
fs/ocfs2/ocfs2_fs.h | 2 +-
fs/ocfs2/stack_user.c | 19 +--
fs/ocfs2/super.c | 6 +-
include/linux/configfs.h | 4 +-
22 files changed, 383 insertions(+), 127 deletions(-)

Akinobu Mita (1):
ocfs2: use simple_read_from_buffer()

Coly Li (1):
ocfs2: fix oops in mmap_truncate testing

Joel Becker (3):
ocfs2: Fix CONFIG_OCFS2_DEBUG_FS #ifdefs
ocfs2: Don't snprintf() without a format.
configfs: Allow ->make_item() and ->make_group() to return detailed errors.

Louis Rilling (6):
configfs: Introduce configfs_dirent_lock
configfs: Protect configfs_dirent s_links list mutations
configfs: Make configfs_new_dirent() return error code instead of NULL
configfs: Fix deadlock with racing rmdir() and rename()
configfs: Fix failing mkdir() making racing rmdir() fail
configfs: call drop_link() to cleanup after create_link() failure

Randy Dunlap (1):
ocfs2: fix printk format warnings with OCFS2_FS_STATS=n

Sunil Mushran (4):
ocfs2: Add CONFIG_OCFS2_FS_STATS config option
ocfs2: Instrument fs cluster locks
ocfs2: Silence an error message in ocfs2_file_aio_read()
ocfs2/net: Silence build warnings on sparc64

Wengang Wang (1):
ocfs2: Handle error during journal load

--
Mark Fasheh


2008-07-17 18:02:48

by Linus Torvalds

[permalink] [raw]
Subject: Re: [git patches] Ocfs2 and Configfs updates for 2.6.27



On Thu, 17 Jul 2008, Mark Fasheh wrote:
>
> The only two things that could be described as features would be:
>
> - Joel updated configfs's ->make_item() and ->make_group() callbacks to
> return more descriptive error types. This update is responsible for the
> changes to drivers/net/netconsole.c and fs/dlm/config.c as shown in the diffstat.


.. I really dislike how this was done.

This commit:

11c3b79218390a139f2d474ee1e983a672d5839a ("configfs: Allow ->make_item()
and ->make_group() to return detailed errors.")

is just ugly. Returning one value by passing a pointer to a pointer, and
the error value by returning an 'int' errno is _not_ how we do things in
the kernel.

No, we use error pointers.

IOW, you should have left the calling convention for "(*make_item)()" and
"(*make_group)()" alone, but allowed people to just do

return ERR_PTR(-ENOMEM);

etc for errors, and then in the caller you just check for IS_ERR().

That would have made the result much cleaner, and avoided that whole
"return two different things" issue.

I pulled it, but I want to register my protest.

Linus

2008-07-17 21:24:19

by Joel Becker

[permalink] [raw]
Subject: Re: [git patches] Ocfs2 and Configfs updates for 2.6.27

On Thu, Jul 17, 2008 at 11:01:40AM -0700, Linus Torvalds wrote:
> .. I really dislike how this was done.

Argh!

> This commit:
>
> 11c3b79218390a139f2d474ee1e983a672d5839a ("configfs: Allow ->make_item()
> and ->make_group() to return detailed errors.")
>
> is just ugly. Returning one value by passing a pointer to a pointer, and
> the error value by returning an 'int' errno is _not_ how we do things in
> the kernel.

See, I coded it up with PTR_ERR() first, and everyone I showed
it to said "I hate PTR_ERR()!".

> I pulled it, but I want to register my protest.

I wish you hadn't, because it won't be to difficult to go back
to PTR_ERR(). However, don't revert it - that affects some of the
follow-on patches that were affected by the change.
I'll spin up a new PTR_ERR() change. I have some more cleanups
coming too, they need to finish their time in linux-next.

Joel

--

"The cynics are right nine times out of ten."
- H. L. Mencken

Joel Becker
Principal Software Developer
Oracle
E-mail: [email protected]
Phone: (650) 506-8127

2008-07-17 21:46:11

by Joel Becker

[permalink] [raw]
Subject: Re: [Ocfs2-devel] [git patches] Ocfs2 and Configfs updates for 2.6.27

On Thu, Jul 17, 2008 at 02:20:38PM -0700, Joel Becker wrote:
> On Thu, Jul 17, 2008 at 11:01:40AM -0700, Linus Torvalds wrote:
> > I pulled it, but I want to register my protest.
>
> I wish you hadn't, because it won't be to difficult to go back
> to PTR_ERR(). However, don't revert it - that affects some of the
> follow-on patches that were affected by the change.

I lied. I'll cleanup the fallout. You can revert it or wait
for my revert + PTR_ERR. Let me know if you revert it.

Joel

--

"Against stupidity the Gods themselves contend in vain."
- Friedrich von Schiller

Joel Becker
Principal Software Developer
Oracle
E-mail: [email protected]
Phone: (650) 506-8127

2008-07-17 22:31:19

by Joel Becker

[permalink] [raw]
Subject: Re: [Ocfs2-devel] [git patches] Ocfs2 and Configfs updates for 2.6.27

On Thu, Jul 17, 2008 at 02:45:12PM -0700, Joel Becker wrote:
> On Thu, Jul 17, 2008 at 02:20:38PM -0700, Joel Becker wrote:
> > On Thu, Jul 17, 2008 at 11:01:40AM -0700, Linus Torvalds wrote:
> > > I pulled it, but I want to register my protest.
> >
> > I wish you hadn't, because it won't be to difficult to go back
> > to PTR_ERR(). However, don't revert it - that affects some of the
> > follow-on patches that were affected by the change.
>
> I lied. I'll cleanup the fallout. You can revert it or wait
> for my revert + PTR_ERR. Let me know if you revert it.

Linus,
Pull this to get the revert and the PTR_ERR() version:

git://oss.oracle.com/git/jlbec/linux-2.6.git configfs-fixup-ptr-error

It's tested with the same testing I did before - ocfs2 and netconsole.

Joel

--

"But all my words come back to me
In shades of mediocrity.
Like emptiness in harmony
I need someone to comfort me."

Joel Becker
Principal Software Developer
Oracle
E-mail: [email protected]
Phone: (650) 506-8127

2008-07-18 08:48:18

by Louis Rilling

[permalink] [raw]
Subject: Re: [Ocfs2-devel] [git patches] Ocfs2 and Configfs updates for 2.6.27

On Thu, Jul 17, 2008 at 03:30:18PM -0700, Joel Becker wrote:
> On Thu, Jul 17, 2008 at 02:45:12PM -0700, Joel Becker wrote:
> > On Thu, Jul 17, 2008 at 02:20:38PM -0700, Joel Becker wrote:
> > > On Thu, Jul 17, 2008 at 11:01:40AM -0700, Linus Torvalds wrote:
> > > > I pulled it, but I want to register my protest.
> > >
> > > I wish you hadn't, because it won't be to difficult to go back
> > > to PTR_ERR(). However, don't revert it - that affects some of the
> > > follow-on patches that were affected by the change.
> >
> > I lied. I'll cleanup the fallout. You can revert it or wait
> > for my revert + PTR_ERR. Let me know if you revert it.
>
> Linus,
> Pull this to get the revert and the PTR_ERR() version:
>
> git://oss.oracle.com/git/jlbec/linux-2.6.git configfs-fixup-ptr-error
>
> It's tested with the same testing I did before - ocfs2 and netconsole.

While this new version looks ok, it seems that it still allows client
subsystems to return NULL in ->make_group()/->make_item(), in which case
configfs_mkdir() turns the result into -ENOMEM. Maybe this is intended to
smoothly update out-of-tree subsystems? I'd suggest to schedule a removal
of this backward compatibility, and display an error log to encourage
fixing the subsystems that still return NULL.

Louis

--
Dr Louis Rilling Kerlabs - IRISA
Skype: louis.rilling Campus Universitaire de Beaulieu
Phone: (+33|0) 2 99 84 71 52 Avenue du General Leclerc
Fax: (+33|0) 2 99 84 71 71 35042 Rennes CEDEX - France
http://www.kerlabs.com/


Attachments:
(No filename) (1.49 kB)
signature.asc (189.00 B)
Digital signature
Download all attachments

2008-07-18 22:23:40

by Joel Becker

[permalink] [raw]
Subject: Re: [Ocfs2-devel] [git patches] Ocfs2 and Configfs updates for 2.6.27

On Fri, Jul 18, 2008 at 10:48:03AM +0200, Louis Rilling wrote:
> > Linus,
> > Pull this to get the revert and the PTR_ERR() version:
> >
> > git://oss.oracle.com/git/jlbec/linux-2.6.git configfs-fixup-ptr-error
> >
> > It's tested with the same testing I did before - ocfs2 and netconsole.
>
> While this new version looks ok, it seems that it still allows client
> subsystems to return NULL in ->make_group()/->make_item(), in which case
> configfs_mkdir() turns the result into -ENOMEM. Maybe this is intended to
> smoothly update out-of-tree subsystems? I'd suggest to schedule a removal
> of this backward compatibility, and display an error log to encourage
> fixing the subsystems that still return NULL.

It is precisely to keep out-of-tree modules from crashing.
There's no easy way to make them know it changed otherwise :-) It could
certainly BUG or WARN before making the fixup.

Joel

--

"I'm drifting and drifting
Just like a ship out on the sea.
Cause I ain't got nobody, baby,
In this world to care for me."

Joel Becker
Principal Software Developer
Oracle
E-mail: [email protected]
Phone: (650) 506-8127