public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Jörn Engel" <joern@logfs.org>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Stephen Rothwell <sfr@canb.auug.org.au>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linus Torvalds <torvalds@osdl.org>,
	David Woodhouse <dwmw2@infradead.org>,
	linux-kernel@vger.kernel.org
Subject: Re: LogFS merge
Date: Fri, 2 May 2008 23:47:04 +0200	[thread overview]
Message-ID: <20080502214704.GF24080@logfs.org> (raw)
In-Reply-To: <200805021852.22768.arnd@arndb.de>

On Fri, 2 May 2008 18:52:21 +0200, Arnd Bergmann wrote:
> 
> There are still a few i's and t's left to dot and cross:
> 
> * the changeset comments needs a Signed-off-by: line

Doh!  When sending patches that happens automatically.  I should teach
git the same trick.

> * The MAINTAINERS file should list your name and logfs mailing list

Definitely.

> * you have a few instances of '#if LINUX_VERSION_CODE > 
>   KERNEL_VERSION(2, 6, 23)', that should go away for the merge

Yes.  I would like to keep the merge version roughly in sync with the
external patch, at least for a while.  Not sure how to deal with one
needing the multi-version hackery and the other not wanting it.

> * The copyright notice says 2005-2007, it should probably be 2005-2008

On most files, yes.

> * You may want to add a Documentation/filesystems/logfs.txt file explaining
>   the supported mount options.

Sure.  I don't have any logfs-specific ones yet, but even that fact
should be made explicit.

> * CONFIG_LOGFS should be tristate, not bool. Unfortunately, you are still
>   using three symbols that are not exported: swapper_space (through
>   BUG_ON(!page_mapping(page)->a_ops->set_page_dirty)), add_to_page_cache_lru
>  and inode_lock. Not sure what to do about this.

inode_lock will get fixed.  The BUG_ON could get removed.  Not sure
about add_to_page_cache_lru yet.

> * You should really make sure the version you check in compiles, 
>   fs/logfs/logfs.h is missing an #endif. ;-)

Making a merge dash whilst moving is not always the wisest choice. ;)

> Otherwise, I don't see any reasons why logfs shouldn't go in. The code is
> clean, feature-complete, and there is demand for it. The main question
> I can still see is the timing with the merge window. It's almost closed,
> so if logfs doesn't go in really soon, it should probably wait for the
> 2.6.27 window.

2.6.27 appears to make more sense, yes.

> This patch fixes some of the problems mentined above.

Excellent!  Thanks a bunch!

> +L:	logfs@logfs.org

I believe it is currently subscribers-only with the usual bounces
everyone holds so dear.  I should change that and add a spam filter to
make it bearable.

Jörn

-- 
The only real mistake is the one from which we learn nothing.
-- John Powell

  reply	other threads:[~2008-05-02 21:47 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-05-02 13:32 LogFS merge Jörn Engel
2008-05-02 14:49 ` Pekka Enberg
2008-05-02 15:31   ` Christoph Hellwig
2008-05-02 20:33     ` Jörn Engel
2008-05-02 16:52 ` Arnd Bergmann
2008-05-02 21:47   ` Jörn Engel [this message]
2008-05-02 17:17 ` Linus Torvalds
2008-05-02 20:21   ` Jörn Engel
2008-05-02 20:33     ` Linus Torvalds
2008-05-02 21:31       ` Jörn Engel
2008-05-02 21:39         ` Linus Torvalds
2008-05-02 21:58           ` Jörn Engel
2008-05-03  7:03             ` Willy Tarreau
2008-05-03  9:11               ` Adrian Bunk
2008-05-03  9:18                 ` Willy Tarreau
2008-05-03  9:37                   ` Adrian Bunk
2008-05-03  9:44                 ` Pekka Enberg
2008-05-03 11:06                   ` Adrian Bunk
2008-05-03 17:16               ` Linus Torvalds
2008-05-03 17:59                 ` Offtopic to: " David Collier-Brown
2008-05-02 17:29 ` Andi Kleen
2008-05-02 21:34   ` Jörn Engel
2008-05-05 20:31 ` Andrew Morton
  -- strict thread matches above, loose matches on Subject: below --
2008-05-03  9:29 matthieu castet
2008-05-03 12:00 ` Jörn Engel
2008-05-04 16:20   ` Thomas Gleixner
2008-05-04 18:04     ` Josh Boyer

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=20080502214704.GF24080@logfs.org \
    --to=joern@logfs.org \
    --cc=akpm@linux-foundation.org \
    --cc=arnd@arndb.de \
    --cc=dwmw2@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sfr@canb.auug.org.au \
    --cc=torvalds@osdl.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