linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Kai Bankett <chaosman@ontika.net>
Cc: linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH]QNX6 filesystem (RO) driver
Date: Wed, 15 Feb 2012 14:40:37 +0000	[thread overview]
Message-ID: <20120215144037.GH23916@ZenIV.linux.org.uk> (raw)
In-Reply-To: <20120215075750.GG23916@ZenIV.linux.org.uk>

On Wed, Feb 15, 2012 at 07:57:50AM +0000, Al Viro wrote:
> On Wed, Feb 15, 2012 at 07:11:19AM +0000, Al Viro wrote:
> 
> > One more thing - unless I'm misreading the code it looks like your ->sb becomes
> > a dangling pointer once fill_super finishes and ->statfs() is dereferencing
> > it...
> 
> BTW, what happens if root directory grows to more than one level of
> pointers?  AFAICS, qnx6_checkroot() assumes that it won't happen...
> And why not do that after getting the root inode, anyway?

Ho-hum...  I've added conversion of qnx6_iget() to pagecache and
checkroot move past getting root; result:
	* qnx6_bread/qnx6_getblk are gone
	* qnx6_block_map() has only one caller left
	* s_inodebits/s_dirent_blk/s_inodes_blk gone
s_ptrbits remains, of course, but that one has to survive - it determines
the way indirect blocks' tree is branched.

Remaining issues I see:
	* sbi->sb dangling pointer (those brelse() in success case of
fill_super()).  Probably only one of them needs to be dropped at that
point, leaving the other in sbi->sb_bh to be dropped by put_super
	* sb_set_blocksize() can and will invalidate buffer_heads of the
wrong size.  qnx6_fill_super() probably needs to redo sb_bread() on the
primary superblock after it has done the second sb_set_blocksize().
	* readdir handling of errors (especially in longname case) needs
to be done better.

Other than that... this stuff could use some tidying up, but that's not
a big deal.  I really wonder if these config options are worthwhile -
might be better to stick endianness and mmi to y unconditionally.  And
of course all those incrementals are completely untested and might very
well br broken.

  reply	other threads:[~2012-02-15 14:40 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-01 20:41 [PATCH]QNX6 filesystem (RO) driver Kai Bankett
2012-02-12  4:56 ` Al Viro
2012-02-12 22:14   ` Kai Bankett
2012-02-12 22:43     ` Al Viro
2012-02-15  2:52       ` Kai Bankett
2012-02-15  6:10         ` Al Viro
2012-02-15  6:14           ` Al Viro
2012-02-15  6:47           ` Al Viro
2012-02-15  7:11             ` Al Viro
2012-02-15  7:57               ` Al Viro
2012-02-15 14:40                 ` Al Viro [this message]
2012-02-15 16:27                   ` Kai Bankett
2012-02-15 21:35                     ` Al Viro
2012-02-16 10:00                   ` Al Viro
2012-02-17 15:06                     ` Kai Bankett
2012-02-17 16:20                       ` Al Viro
2012-02-17 17:53                         ` Kai Bankett
2012-02-17 18:35                           ` Al Viro
2012-02-17 18:53                             ` Al Viro
2012-02-17 21:38                               ` Kai Bankett
2012-02-21 22:04                                 ` Al Viro
2012-02-21 22:09                                   ` Al Viro
2012-02-22 11:58                                     ` Kai Bankett

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=20120215144037.GH23916@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=chaosman@ontika.net \
    --cc=linux-fsdevel@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).