mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [GIT PULL] 9p Patches for 2.6.23 merge window
@ 2007-07-14 20:20 Eric Van Hensbergen
  2007-07-15 23:49 ` Linus Torvalds
  0 siblings, 1 reply; 6+ messages in thread
From: Eric Van Hensbergen @ 2007-07-14 20:20 UTC (permalink / raw)
  To: torvalds; +Cc: V9FS Developers, Linux Kernel

Linus, please pull from the 'for-linus' branch of:
  git://git.kernel.org/pub/scm/linux/kernel/git/ericvh/v9fs.git/ for-linus

This tree contains the following:

Latchesar Ionkov(3):
  Reorganization of 9p file system code
  Change net/9p module name to 9pnet
  Set error to EREMOTEIO if transport write returns zero

Eric Van Hensbergen(3)
  Cache meta-data when loose cache option is set
  Renable mount-time debug option
  Fix a race condition bug in umount which caused segfault

The bulk of the changes were in the reorganization patch which mostly
moved files and interfaces around in preparation for work on an
in-kernel 9p server.

 b/fs/9p/Makefile             |    6
 b/fs/9p/fid.c                |  168 ++----
 b/fs/9p/fid.h                |   43 -
 b/fs/9p/v9fs.c               |  288 ++---------
 b/fs/9p/v9fs.h               |   32 -
 b/fs/9p/v9fs_vfs.h           |    6
 b/fs/9p/vfs_addr.c           |   57 --
 b/fs/9p/vfs_dentry.c         |   37 -
 b/fs/9p/vfs_dir.c            |  155 +-----
 b/fs/9p/vfs_file.c           |  160 +-----
 b/fs/9p/vfs_inode.c          |  753 +++++++++++-------------------
 b/fs/9p/vfs_super.c          |   91 +--
 b/fs/Kconfig                 |    2
 b/include/net/9p/9p.h        |  417 +++++++++++++++++
 b/include/net/9p/client.h    |   80 +++
 b/include/net/9p/conn.h      |   57 ++
 b/include/net/9p/transport.h |   49 ++
 b/net/9p/Kconfig             |   21
 b/net/9p/Makefile            |   13
 b/net/9p/client.c            |  965 +++++++++++++++++++++++++++++++++++++++
 b/net/9p/conv.c              |  903 ++++++++++++++++++++++++++++++++++++
 b/net/9p/error.c             |  240 +++++++++
 b/net/9p/fcprint.c           |  358 ++++++++++++++
 b/net/9p/mod.c               |   85 +++
 b/net/9p/mux.c               | 1050 +++++++++++++++++++++++++++++++++++++++++++
 b/net/9p/sysctl.c            |   86 +++
 b/net/9p/trans_fd.c          |  363 ++++++++++++++
 b/net/9p/util.c              |  125 +++++
 b/net/Kconfig                |    1
 b/net/Makefile               |    2
 fs/9p/9p.h                   |  375 ---------------
 fs/9p/conv.c                 |  845 ----------------------------------
 fs/9p/conv.h                 |   50 --
 fs/9p/debug.h                |   77 ---
 fs/9p/error.c                |   93 ---
 fs/9p/error.h                |  177 -------
 fs/9p/fcall.c                |  427 -----------------
 fs/9p/fcprint.c              |  345 --------------
 fs/9p/mux.c                  | 1033 ------------------------------------------
 fs/9p/mux.h                  |   55 --
 fs/9p/trans_fd.c             |  308 ------------
 fs/9p/transport.h            |   45 -
 fs/9p/v9fs.c                 |    7
 fs/9p/vfs_file.c             |   14
 fs/9p/vfs_inode.c            |    4
 fs/9p/vfs_super.c            |    3
 net/9p/Makefile              |    7
 net/9p/client.c              |    7
 net/9p/mux.c                 |    7
 49 files changed, 5400 insertions(+), 5092 deletions(-)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [GIT PULL] 9p Patches for 2.6.23 merge window
  2007-07-14 20:20 [GIT PULL] 9p Patches for 2.6.23 merge window Eric Van Hensbergen
@ 2007-07-15 23:49 ` Linus Torvalds
  2007-07-16  0:30   ` [V9fs-developer] " Latchesar Ionkov
  0 siblings, 1 reply; 6+ messages in thread
From: Linus Torvalds @ 2007-07-15 23:49 UTC (permalink / raw)
  To: Eric Van Hensbergen; +Cc: V9FS Developers, Linux Kernel



On Sat, 14 Jul 2007, Eric Van Hensbergen wrote:
> 
> The bulk of the changes were in the reorganization patch which mostly
> moved files and interfaces around in preparation for work on an
> in-kernel 9p server.

Please use "git diff -C --stat --summary" to generate the diffstat 
summary.

In particular, because you didn't use -C (or -M), git have you an 
old-style diffstat without renames, so the diffstat doesn't show that a 
lot of it was moving code around.

(That said, you seem to have changed names a lot too, so it's not pure 
code movement).

> 49 files changed, 5400 insertions(+), 5092 deletions(-)

With rename detection enabled, I get

 36 files changed, 3444 insertions(+), 3130 deletions(-)

due to finding some (partially dubious) renames:

 rename fs/9p/mux.h => include/net/9p/conn.h (53%)
 rename {fs => include/net}/9p/transport.h (59%)
 rename {fs => net}/9p/conv.c (55%)
 rename {fs => net}/9p/fcprint.c (64%)
 rename {fs => net}/9p/mux.c (55%)

even though is misses a lot of others (due to all the "v9fs" -> "p9" 
changes in the source code too - was that really worth it?).

			Linus

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [V9fs-developer] [GIT PULL] 9p Patches for 2.6.23 merge window
  2007-07-16  0:30   ` [V9fs-developer] " Latchesar Ionkov
@ 2007-07-16  0:28     ` david
  2007-07-16 16:29     ` Linus Torvalds
  1 sibling, 0 replies; 6+ messages in thread
From: david @ 2007-07-16  0:28 UTC (permalink / raw)
  To: Latchesar Ionkov
  Cc: Linus Torvalds, Eric Van Hensbergen, V9FS Developers, Linux Kernel

On Sun, 15 Jul 2007, Latchesar Ionkov wrote:

> I thought that it is not a good idea to keep the v9fs_ prefix for code
> that is in different places (fs/9p and net/9p). If keeping the old
> prefix is more acceptable, I can create a new patch without the
> "v9fs_"->"p9_" renames.

do the code reorg and the name changes as two different patches. it makes 
it much easier to see that the rename doesn't change any functionality, 
and makes the code reorg much easier to see.

David Lang

> Thanks,
>    Lucho
>
> On 7/15/07, Linus Torvalds <torvalds@linux-foundation.org> wrote:
>> 
>>
>>  On Sat, 14 Jul 2007, Eric Van Hensbergen wrote:
>> > 
>> >  The bulk of the changes were in the reorganization patch which mostly
>> >  moved files and interfaces around in preparation for work on an
>> >  in-kernel 9p server.
>>
>>  Please use "git diff -C --stat --summary" to generate the diffstat
>>  summary.
>>
>>  In particular, because you didn't use -C (or -M), git have you an
>>  old-style diffstat without renames, so the diffstat doesn't show that a
>>  lot of it was moving code around.
>>
>>  (That said, you seem to have changed names a lot too, so it's not pure
>>  code movement).
>> 
>> >  49 files changed, 5400 insertions(+), 5092 deletions(-)
>>
>>  With rename detection enabled, I get
>>
>>   36 files changed, 3444 insertions(+), 3130 deletions(-)
>>
>>  due to finding some (partially dubious) renames:
>>
>>   rename fs/9p/mux.h => include/net/9p/conn.h (53%)
>>   rename {fs => include/net}/9p/transport.h (59%)
>>   rename {fs => net}/9p/conv.c (55%)
>>   rename {fs => net}/9p/fcprint.c (64%)
>>   rename {fs => net}/9p/mux.c (55%)
>>
>>  even though is misses a lot of others (due to all the "v9fs" -> "p9"
>>  changes in the source code too - was that really worth it?).
>>
>>                          Linus
>>
>>  -------------------------------------------------------------------------
>>  This SF.net email is sponsored by DB2 Express
>>  Download DB2 Express C - the FREE version of DB2 express and take
>>  control of your XML. No limits. Just data. Click to get it now.
>>  http://sourceforge.net/powerbar/db2/
>>  _______________________________________________
>>  V9fs-developer mailing list
>>  V9fs-developer@lists.sourceforge.net
>>  https://lists.sourceforge.net/lists/listinfo/v9fs-developer
>> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [V9fs-developer] [GIT PULL] 9p Patches for 2.6.23 merge window
  2007-07-15 23:49 ` Linus Torvalds
@ 2007-07-16  0:30   ` Latchesar Ionkov
  2007-07-16  0:28     ` david
  2007-07-16 16:29     ` Linus Torvalds
  0 siblings, 2 replies; 6+ messages in thread
From: Latchesar Ionkov @ 2007-07-16  0:30 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Eric Van Hensbergen, V9FS Developers, Linux Kernel

I thought that it is not a good idea to keep the v9fs_ prefix for code
that is in different places (fs/9p and net/9p). If keeping the old
prefix is more acceptable, I can create a new patch without the
"v9fs_"->"p9_" renames.

Thanks,
    Lucho

On 7/15/07, Linus Torvalds <torvalds@linux-foundation.org> wrote:
>
>
> On Sat, 14 Jul 2007, Eric Van Hensbergen wrote:
> >
> > The bulk of the changes were in the reorganization patch which mostly
> > moved files and interfaces around in preparation for work on an
> > in-kernel 9p server.
>
> Please use "git diff -C --stat --summary" to generate the diffstat
> summary.
>
> In particular, because you didn't use -C (or -M), git have you an
> old-style diffstat without renames, so the diffstat doesn't show that a
> lot of it was moving code around.
>
> (That said, you seem to have changed names a lot too, so it's not pure
> code movement).
>
> > 49 files changed, 5400 insertions(+), 5092 deletions(-)
>
> With rename detection enabled, I get
>
>  36 files changed, 3444 insertions(+), 3130 deletions(-)
>
> due to finding some (partially dubious) renames:
>
>  rename fs/9p/mux.h => include/net/9p/conn.h (53%)
>  rename {fs => include/net}/9p/transport.h (59%)
>  rename {fs => net}/9p/conv.c (55%)
>  rename {fs => net}/9p/fcprint.c (64%)
>  rename {fs => net}/9p/mux.c (55%)
>
> even though is misses a lot of others (due to all the "v9fs" -> "p9"
> changes in the source code too - was that really worth it?).
>
>                         Linus
>
> -------------------------------------------------------------------------
> This SF.net email is sponsored by DB2 Express
> Download DB2 Express C - the FREE version of DB2 express and take
> control of your XML. No limits. Just data. Click to get it now.
> http://sourceforge.net/powerbar/db2/
> _______________________________________________
> V9fs-developer mailing list
> V9fs-developer@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/v9fs-developer
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [V9fs-developer] [GIT PULL] 9p Patches for 2.6.23 merge window
  2007-07-16  0:30   ` [V9fs-developer] " Latchesar Ionkov
  2007-07-16  0:28     ` david
@ 2007-07-16 16:29     ` Linus Torvalds
  2007-07-16 16:45       ` Latchesar Ionkov
  1 sibling, 1 reply; 6+ messages in thread
From: Linus Torvalds @ 2007-07-16 16:29 UTC (permalink / raw)
  To: Latchesar Ionkov; +Cc: Eric Van Hensbergen, V9FS Developers, Linux Kernel



On Sun, 15 Jul 2007, Latchesar Ionkov wrote:
>
> I thought that it is not a good idea to keep the v9fs_ prefix for code
> that is in different places (fs/9p and net/9p). If keeping the old
> prefix is more acceptable, I can create a new patch without the
> "v9fs_"->"p9_" renames.

It's fine, I don't care *that* much, and I already pulled. If it had been 
something more central, I'd have rejected it, but soemthing as specialized 
as the Plan9 fs, I just wanted to point out that this is now how we should 
do things.

In other words: when doing renames it is generally *much* nicer to do a 
100% rename (perhaps with just _trivial_ changes to make it compile - the 
include statements etc change, and maybe you want to change the name in 
the comment header too).

Doing "move the code and change it at the same time" is considered bad 
form. Movement diffs are much harder to read anyway (a traditional diff 
will show it as a new-file + delete, of course), so the general rule is:

 - move code around _without_ modifying it, so that code movement (whether 
   it's a whole file, or just a set of functions between files) doesn't 
   really introduce any real changes, and is easier to look through the 
   changes.

 - do the actual changes to the code as a separate thing.

This should be true in just about *any* development model, and it's 
especially true in Linux, where patches are the main way people 
communicate.

And when using git, the whole "keep code movement separate from changes" 
has an even more fundamental reason: git can track code movement (again, 
whether moving a whole file or just a function between files), and doing a 
"git blame -C" will actually follow code movement between files. It does 
that by similarity analysis, but it does mean that if you both move the 
code *and* change it at the same time, git cannot see that "oh, that 
function came originally from that other file", and now you get worse 
annotations about where code actually originated.

So next time, please don't move code and change it at the same time.

		Linus

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [V9fs-developer] [GIT PULL] 9p Patches for 2.6.23 merge window
  2007-07-16 16:29     ` Linus Torvalds
@ 2007-07-16 16:45       ` Latchesar Ionkov
  0 siblings, 0 replies; 6+ messages in thread
From: Latchesar Ionkov @ 2007-07-16 16:45 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Eric Van Hensbergen, V9FS Developers, Linux Kernel

The original patchset had three patches:

1. Renames all functions and macros
2. Moves the header files from fs/9p to include/net/9p and updates the
C files with the new header locations
3. Moves the C files from fs/9p to net/9p

Unfortunately the three patches were applied as a single one in Eric's
repository and made tracking of the changes harder. We will be more
careful next time.

Thanks for you comments,
    Lucho

On 7/16/07, Linus Torvalds <torvalds@linux-foundation.org> wrote:
>
>
> On Sun, 15 Jul 2007, Latchesar Ionkov wrote:
> >
> > I thought that it is not a good idea to keep the v9fs_ prefix for code
> > that is in different places (fs/9p and net/9p). If keeping the old
> > prefix is more acceptable, I can create a new patch without the
> > "v9fs_"->"p9_" renames.
>
> It's fine, I don't care *that* much, and I already pulled. If it had been
> something more central, I'd have rejected it, but soemthing as specialized
> as the Plan9 fs, I just wanted to point out that this is now how we should
> do things.
>
> In other words: when doing renames it is generally *much* nicer to do a
> 100% rename (perhaps with just _trivial_ changes to make it compile - the
> include statements etc change, and maybe you want to change the name in
> the comment header too).
>
> Doing "move the code and change it at the same time" is considered bad
> form. Movement diffs are much harder to read anyway (a traditional diff
> will show it as a new-file + delete, of course), so the general rule is:
>
>  - move code around _without_ modifying it, so that code movement (whether
>    it's a whole file, or just a set of functions between files) doesn't
>    really introduce any real changes, and is easier to look through the
>    changes.
>
>  - do the actual changes to the code as a separate thing.
>
> This should be true in just about *any* development model, and it's
> especially true in Linux, where patches are the main way people
> communicate.
>
> And when using git, the whole "keep code movement separate from changes"
> has an even more fundamental reason: git can track code movement (again,
> whether moving a whole file or just a function between files), and doing a
> "git blame -C" will actually follow code movement between files. It does
> that by similarity analysis, but it does mean that if you both move the
> code *and* change it at the same time, git cannot see that "oh, that
> function came originally from that other file", and now you get worse
> annotations about where code actually originated.
>
> So next time, please don't move code and change it at the same time.
>
>                 Linus
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2007-07-16 16:45 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-07-14 20:20 [GIT PULL] 9p Patches for 2.6.23 merge window Eric Van Hensbergen
2007-07-15 23:49 ` Linus Torvalds
2007-07-16  0:30   ` [V9fs-developer] " Latchesar Ionkov
2007-07-16  0:28     ` david
2007-07-16 16:29     ` Linus Torvalds
2007-07-16 16:45       ` Latchesar Ionkov

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®